fix: identify username and password will fail if username have colon

huytd / pomoday-v2

A keyboard only task management web app

https://pomoday.vercel.app

BSD 3-Clause "New" or "Revised" License

585 stars 69 forks source link

fix: identify username and password will fail if username have colon #65

Closed rknguyen closed 4 years ago

rknguyen commented 4 years ago

🇻🇳

huytd commented 4 years ago

Thanks for the PR but there are two things I want to point out here:

1) I don’t think username has colon should be a valid case, and it should be handled not only here in the login step but also in user creation as well, so it should be at an upper level or somewhere we can reuse.

2) I believe we are supposed to have more than just a colon case for username sanitation, so I suggest that we should update this PR to cover more cases as well.

rknguyen commented 4 years ago

Eh, maybe you got something wrong, this is my case: I read your code and built my server for sync my tasks. And this is the problem:

You are making a token by base64_encode(${username}:${password}). So I got this points and on my server, I parse the header to get token in order to identity the user, I decode the token and get the username from that. But in case username have the colon, it's impossible to identity what is the username. Thanks for review my PR.

huytd commented 4 years ago

What I'm suggesting is making the check for username to be more generic and reusable.

And you may know it already, base64(username:password) is the standard for Basic Auth.

huytd commented 4 years ago

It's been a while since the last comment and I still didn't get any response. Hence, closing this.

Please feel free to open this again if you're still interested. Thanks.