Here’s a snippet of code that I found in our production codebase. I found this while fixing a bug. This is not part of the bug though but I see this a problem. Should I say this to my senior that his logic is off here?

First he doesnt have to do explicitly strict comparison since the return type is boolean and it could be true or false. Also the returnUrl will alway be undefined because redirectIfUserLoggedIn is called first before it was set.

He dosent like me. Hell get mad for sure. I think ill let this go.

I mean, all of the code present is bush league angular, so I'm not even sure where to begin.

The strict comparison can be left in as it is good practice to do so.

Use a debugger to determine if returnUrl can ever not be undefined. If it's always undefined, bring it up.

@SortOfTested you hate angular?

Also fuck Angular lmao

@kescherRant yea I tested it its always undefined. I think the strict comparison has no benefit here since typescript will shout at you if you return something else

@kescherRant HAHAHAHAHA. Same energy bro. Fuck angular.

@r-fu
No, I love angular. The code present is sloppy, mixed paradigm and amateurish.

@SortOfTested HAHAHAHA. I dont hes my senior though

@r-fu
His programming ability isn't senior.

Half the code should be in a route guard, the other half belongs in a service or interceptor. Mutable variables, not a pure function in sight. It's impressive that you don't spend 90% of your time debugging.

@SortOfTested ohh we do. This is another hot fix that I have to fix. Haha

If you tell him and he’s professional senior, he’ll respect you for it.

But don’t tell him the way you told us. Be polite and professional and tell him you had a look at the code and it seems off. Is it intended (we know it’s not but we don’t wanna harm his ego) and if so, can he clarify it for you.

@SortOfTested is right.
Also, manage those damn observable subscriptions 😥

This should be something that's picked up in a code review and a senior should always take constructive criticism from someome senior, same level or junior to them.

Javascript is weird with the triple equals but if they are a senior who uses that in their job as their main language, they should know the difference

Just as @just8littleBit alluded to, be political about it and pose it as a question. Is the data variable an object that is truth or is data expicitly a boolean value.

@just8littleBit ohh maam im really polite. I dont want any trouble. Mater of fact ill just let it go. Haha. Im afriad hell get mad so. Ill just let it be.

@Commodore its in some of the code but here I guess just subscribing to the state is just fine since we’re not doing any data manipulation.

@cmarshall10450 we dont have a code review. Hahaha. I mean my code gets reviewed but his I dont know. Its just me and him. Haha

Yeah! Beat the shoes out of him!

@r-fu you should review each others code. You may know about some features or better ways of somethings in a language/tech than him.

I felt like I couldn't say when I first joined my company (first tech job) but I regretted not suggesting using one tech another when one senior discovered it and made it team wide.

@cmarshall10450
As much as I agree, there are many countries represented here still of this, "you are my subordinate, you have no value until you ascend" classist mindset. Rocking the boat is not advised for them.

@cmarshall10450 review his code? Haha. I dont know man. But he doesn’t like talking or doing pair programming. He looks at me weird.

I had a super chill senior. But I had this one wannabe co-worker that nagged about this and that for the first few weeks. The typical „youre just a junior“ stuff

Not long and I had implicit lead of the core frontend architecture we had. After I tackled some of the most difficult things, he slowly shut up and eventually got along nicely. But I‘m sure he still feels bad that I knew better. Before I had left the company, I had to train my replacement and he was interested in joining that.

@SortOfTested this is said but its true. Good thing I have peace. When he gets mad i dont care. I will act like he didnt get mad. Haha.

@010001111 I wish I could just tell him that this part of the code is bad or we can change this code. I feel like hell get mad or something.

Leave the strict comparison because then datatype mismatch wont fail silently, which is a common problem in weakly typed languages.

@arcsector even though we’re using type script? It wont compile if the return type is mismatch.

@r-fu actually it wont in some cases. Take a look at the following snippet:

It prints "true".

@arcsector
Accurate. The shortcut here would be !!data. I don't recommend that though.

While you fix the code, add a check to make sure that returnUrl is from a trusted domain. Otherwise, you open the door to a class of juicy pushing attacks.

@arcsector i know what you mean but what I was thinking its overkill since the data itself returns boolean. But im gonna take ur advice. Thanks.:)

@stacked how? Do you have any link? Damn wanted to break our app and fix it so i can be a hero. Lols

@SHA-16384 i havent talked this to my senior yet. I chicken out. Haha. I might let it go this ill just wait for a bug and fix this.

@r-fu well, suppose you own goodsite.com, and I own evilsite.com. I want to steal the credentials of an user. Here's what I can do:

- I tell the user to visit goodsite.com/login?redirect=evilsite.com/login (the url can be made more concealed by adding nonsense params)

- The user insets their credentials and is redirected to evilsite.com/login

- The page at evilsite.com/login contains the same login form as before, with a message "invalid username/password, please try again"

- The average user will think they had a typo somewhere and they won't check the domain name, so they will try again with their correct credentials

- evilsite.com gets those credentials and redirects the user back to goodsite.com

End of the story: the user didn't notice anything wrong, I got their email/password

@r-fu oh, and if you want a link, just search for "phishing redirect". Here's one result:

https://hacksplaining.com/preventio...

This is also why, as a user, you should always be paranoid about where you enter your credentials and check every detail.

@stacked thank you sir. You are a goat!

@stacked holy fck!!! Thank you for this site!! I always wanted to learn computer security. I just watch it on youtube and follow but this is different. Love you!!

@r-fu quick note on the redirect part: if you really intended to always redirect on the same domain as the login page, the best solution is to only pass the path as a parameter. (you know, least privilege and all that...)