-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
request_password_reset: insufficiently escaped URL is incorrectly converted turned into a clickable link by GMail receiving plain text email #9111
Comments
Thanks for opening this issue!
|
Could you given an example of a username that would cause such an invalid link? |
In the case that happened to us, a user added a smiley as part of her username, and that's not forbidden in our app, nor in Parse Server. I suppose that several other characters could cause the same problem, but I haven't tried them before. Now I have. Among all ASCII punctuations, they are ! " ' ) , . : ; < > ? ] ^ } But the contents of that set are not totally obvious, such that another mail client might decide to exclude a different set of characters, so it's best to not take chances. |
... the whole link looks like and you can see that Github also excludes the closing parenthesis when turning that into a clickable link (when not using the explicit link feature of markdown with square brackets) |
I assume there are restrictions as to what chars a username can contain, even if not documented. For example how about the username "🙂"? Anyway, for this issue, adding the missing escape should be sufficient, I guess. But why is it not escaping the whole username in the first place? |
Parse Server accepts the creation of a user named "🙂", as well as a user named "hello :)". Apps don't necessarily add restrictions of their own. Anyway, there is no reason to add any such restriction now, and it can't reasonably apply to users that have already been created anyway, and the only place where there is a problem with special chars is in request_password_reset. The reason it's not escaping the whole username is because But is there anything I have to do so that this change be applied ? |
Well, if you would like to open a PR for this change? |
New Issue Checklist
Issue Description
If the username ends with punctuation (e.g. closing parenthesis), the password change URL is made clickable by GMail, except for that character, which causes a redirection to
invalid_link.html
.Steps to reproduce
Create a username ending with a closing parenthesis. Call
Parse.User.requestPasswordReset
or the equivalent. Receive the plain text email. Email client detects a URL.Actual Outcome
302 invalid_link.html
Expected Outcome
One of :
a) HTML message, so as to prevent link detection ;
b) a plain text message in which the link is somewhat more escaped than what is supposed to be required. Unfortunately I don't have a list of characters. The username is the only part of the link that has characters that might have to be escaped, so I say that you could escape all non \w characters in the username. E.g.
user.username.replace(/\W/g, x=>"%"+x.charCodeAt(0).toString(16).padStart(2))
instead ofencodeURIComponent(user.username)
in sendPasswordResetEmail at lib/Controllers/UserController.js.BTW there's another mail sender that doesn't get called, and whose
buildEmailLink
does not escape the username at all.Environment
Server
Database
Client
Logs
The text was updated successfully, but these errors were encountered: