-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Case insensitive login & request password reset #7150
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
Case insensitive login & request password reset #7150
Conversation
Codecov Report
@@ Coverage Diff @@
## master #7150 +/- ##
=======================================
Coverage 93.90% 93.91%
=======================================
Files 169 169
Lines 12535 12547 +12
=======================================
+ Hits 11771 11783 +12
Misses 764 764
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good work with the test cases, just some minor comments.
Any changes necessary for case insensitivity on email verification? |
I don't think so because the verification calls the |
There may be manipulation by the developer in between, for example when modifying the email verification link or parsing query parameters from the URL. A developer could also provide a custom process for resending the email verification for an expired token, for example require the user to re-enter their email address in a form, hence this is not a closed process. Do you think it makes sense to add a test case for case insensitive email verification and password reset where the casing is changed between the casing in the email link and the casing that is fed back to Parse Server? |
|
Yes, at link generation before the email is sent, for example to append a custom query parameter. For expired verification tokens, a developer could require the user to enter the email address on a website for additional validation. Anyways, the general point is that case insensitivity should ideally apply to all routes for consistency. |
You're right. I think my solution is the wrong approach. If you merge that, some queries are case insensitive, and another queries are not. Like, if you want to do a normal find query on the username. My suggestion is now that all queries on the username/email are adjusted by default. It's really easy for postgres, the only line that has to be adjusted is this one(?). For mongo it's not so easy. The
could be added. But that would slow down the database, a text search would be better. But that requires a new index. My suggestion is now to close this PR, I think your skills are higher than mine and you won't need so much time to write that as me. |
If you like let's go through to queries one by one and see if/how they need to be adapted. I think your PR is actually close to what we need. If it covers 100% of user facing endpoints it should be fine. And if it covers 80% of developer facing endpoints and the rest is for the developer to be mindful about, that would also be fine I think. |
New Pull Request Checklist
Issue Description
Adds the possibility to login case insensitive with username / email and request password reset.
This is my first PR that changes the source code of parse-server. I hope that everything is correct, especially the tests.
Closes #7143.
TODOs before merging