parse-community
feat: allow transparent forgot password flow
New Pull Request Checklist
- [x] I am not disclosing a vulnerability.
- [x] I am creating this PR in reference to an issue.
Issue Description
Allows the option for returning an error "A user with the email com does not exist." when an invalid password reset is called, instead of the current return true.
Related issue: #7434
Approach
Adds password policy option: resetPasswordSuccessOnInvalidEmail, which defaults to true. Does not change any existing functionality unless implicitly set.
TODOs before merging
- [x] Add test cases
- [x] Add entry to changelog
Thanks for opening this pull request!
- π We are excited about your hands-on contribution!
![parse-github-assistant[bot] avatar](https://avatars.githubusercontent.com/in/136195?v=4 "Published by a pub.dev verified publisher"){kind=small}
Codecov Report
Base: 94.15% // Head: 94.32% // Increases project coverage by +0.16% :tada:
Coverage data is based on head (
0aa7f3f) compared to base (a49e323). Patch coverage: 90.00% of modified lines in pull request are covered.
:exclamation: Current head 0aa7f3f differs from pull request most recent head 1d77a97. Consider uploading reports for the commit 1d77a97 to get more accurate results
Additional details and impacted files
@@ Coverage Diff @@
## alpha #7551 +/- ##
==========================================
+ Coverage 94.15% 94.32% +0.16%
==========================================
Files 182 182
Lines 14400 14405 +5
==========================================
+ Hits 13559 13587 +28
+ Misses 841 818 -23
| Impacted Files | Coverage Ξ | |
|---|---|---|
| src/Options/Definitions.js | 100.00% <ΓΈ> (ΓΈ) |
|
| src/Options/index.js | 100.00% <ΓΈ> (ΓΈ) |
|
| src/Routers/UsersRouter.js | 97.10% <87.50%> (+0.87%) |
:arrow_up: |
| src/Config.js | 90.47% <100.00%> (+0.06%) |
:arrow_up: |
| src/RestWrite.js | 94.31% <0.00%> (-0.30%) |
:arrow_down: |
| src/Controllers/DatabaseController.js | 93.93% <0.00%> (+0.14%) |
:arrow_up: |
| src/Adapters/Storage/Mongo/MongoTransform.js | 88.45% <0.00%> (+0.15%) |
:arrow_up: |
| src/Controllers/SchemaController.js | 97.41% <0.00%> (+0.18%) |
:arrow_up: |
| src/Adapters/Storage/Mongo/MongoStorageAdapter.js | 92.14% <0.00%> (+2.33%) |
:arrow_up: |
| src/batch.js | 92.98% <0.00%> (+3.50%) |
:arrow_up: |
| ... and 2 more |
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
I think so, I'm pretty sure the postgres tests are unrelated but not 100% sure
If you don't see the "Re-run" button in the Actions, you can alway close and re-open the PR, that will retrigger the test runs. Then the failing tests should pass (and possibly others fail)
I will reformat the title to use the proper commit message syntax.
Can this be merged in to PS6 as a breaking change @mtrezza? Some refactoring will be needed
Yes, well spotted, this is a breaking change it seems, so perfectly suitable for PS6. I'll add it to the list.
@dblythy Is it correct that this is not a breaking change anymore? The default behavior will not change, but the developer can use a new Parse Server option to return an error if the email doesn't exist, correct? If so, I'll remove the breaking label here and we can also remove this from the Parse Server 6 list, as we can merge it anytime after release.
It's a breaking change - previously I had it as a server parameter but now it's change in default behaviour
Not sure if that has been discussed previously, but should we keep the default behavior to be obscure and return success? And allow the developer to change it to return an error. The opposite of how the PR does it now it seems.
- No breaking change
- By default security through obscurity; the code comment also says: "Return success so that this endpoint can't be used to enumerate valid emails"
The whole point of removing the obscurity was that you can check whether a user exists by calling .signup
https://github.com/parse-community/parse-server/security/advisories/GHSA-cfgm-jqhh-xmhr#advisory-comment-64808
I've had another look at the options. Should this go under PasswordPolicyOptions since we also have things like resetTokenValidityDuration in there?
So this could be named passwordPolicy.resetResponseSuccessOnInvalidEmail. On the other hand it may not be strictly understood as a "password policy".
Anyway, let's just merge and hopefully someday restructure all options in a more meaningful way.
