Open mathieulb opened 2 months ago
parse-github-assistant[bot]
commented
2 months ago 
mtrezza
commented
2 months ago Could you given an example of a username that would cause such an invalid link?
mathieulb
commented
2 months ago 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.
mathieulb
commented
2 months ago ... the whole link looks like https://myapp.herokuapp.com/parse/apps/0123456789ab-cdef-0123-456789abcdef/request_password_reset?token=3zgBwAwZ7mk5bAxyECGnVwK2h&username=Hello%20%3A)
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)
mtrezza
commented
2 months ago 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?
mathieulb
commented
1 month ago 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 sendPasswordResetEmail uses encodeURIComponent, which doesn't do it because it's not required according to the spec of URLs. But GMail/GitHub/Facebook (etc) require escaping because people often put a closing parenthesis or square bracket or comma or period (etc) right after a URL in a sentence. In almost all cases, the URL didn't end with that punctuation, the punctuation was added by the user after pasting the URL. So the problem is that encodeURIComponent strictly follows the standard instead of dealing with certain non-conforming aspects of reality (that have good reasons to be non-conforming, imho).
But is there anything I have to do so that this change be applied ?
mtrezza
commented
1 month ago 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.requestPasswordResetor 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
buildEmailLinkdoes not escape the username at all.Environment
Server
Database
Client
Logs