Skip to content
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

Improve transparency of reset password flow #7434

Closed
6 tasks done
dblythy opened this issue Jun 18, 2021 · 12 comments · Fixed by #7551
Closed
6 tasks done

Improve transparency of reset password flow #7434

dblythy opened this issue Jun 18, 2021 · 12 comments · Fixed by #7551
Labels
state:released Released as stable version state:released-alpha Released as alpha version state:released-beta Released as beta version type:feature New feature or improvement of existing feature

Comments

@dblythy
Copy link
Member

dblythy commented Jun 18, 2021

New Issue Checklist

Issue Description

As discussed here, password reset should throw with an invalid email, rather than returning success.

Steps to reproduce

Request a password reset with an invalid email.

Actual Outcome

Response is success.

Expected Outcome

Response should be "invalid email" or something useful.

Failing Test Case / Pull Request

  • 🤩 I submitted a PR with a fix and a test case.
  • 🧐 I submitted a PR with a failing test case.

Environment

Server

  • Parse Server version: master (4.5.0)
@mtrezza
Copy link
Member

mtrezza commented Jun 19, 2021

Thanks for opening this issue.

As described in your references discussion, this change makes sense to give the developer more control over the password reset flow.

The security implication is marginal because it allows to determine whether a specific email address is assigned to a user, but that is also possible via the sign-up endpoint.

However, since obfuscation is also a security strategy, there is a security implication which has to be considered and mentioned in the changelog as a significant change.

In addition, it is a breaking change if instead of success an error response is returned in some cases. According to our deprecation policy this change should be phased in.

@dblythy
Copy link
Member Author

dblythy commented Sep 5, 2021

Ok. Sounds good. How should we go about depreciating it? Add a warning and then remove it at a later date?

Also, do you think we could have a Parse Server option called deprecations, where all the deprecated options are set? Otherwise the docs for server options will become full with deprecated options.

@mtrezza
Copy link
Member

mtrezza commented Sep 5, 2021

We would add a new parse server option where the developer can choose whether the endpoint should return an error or success if the email doesn't exist.

The default would be to return success (as it currently does), with a deprecation log if the option is not manually set return an error.

Instead of deprecating we could also make it a permanent option, without deprecation. So we keep the obfuscating default setting to return success. If a developer wants to build a custom flow, they can choose to set it to return an error. The benefit would be that it's no breaking change, no deprecation needed, while keeping the more secure obfuscation behavior as default.

@dblythy
Copy link
Member Author

dblythy commented Sep 6, 2021

Right, so rather than deprecating the current flow, we could add a passwordPolicy.validateEmails option or something, that defaults to false?

@mtrezza
Copy link
Member

mtrezza commented Sep 6, 2021

Yes, what do you think?

@dblythy
Copy link
Member Author

dblythy commented Sep 6, 2021

Sounds like an elegant solution. Thanks for the discussion! I'll work on it.

@parse-github-assistant
Copy link

parse-github-assistant bot commented Sep 9, 2021

Thanks for opening this issue!

  • 🚀 You can help us to fix this issue faster by opening a pull request with a failing test. See our Contribution Guide for how to make a pull request, or read our New Contributor's Guide if this is your first time contributing.

@mtrezza mtrezza added type:feature New feature or improvement of existing feature and removed type:improvement labels Dec 6, 2021
@mtrezza mtrezza added state:breaking Breaking change requires major version increment and `BREAKING CHANGE` commit message block:major Needs to be resolved before next major release; remove label afterwards labels Dec 17, 2022
@mtrezza mtrezza mentioned this issue Dec 17, 2022
31 tasks
@mtrezza mtrezza removed state:breaking Breaking change requires major version increment and `BREAKING CHANGE` commit message block:major Needs to be resolved before next major release; remove label afterwards labels Jan 14, 2023
@mtrezza
Copy link
Member

mtrezza commented Jan 14, 2023

I've removed this from Parse Server 6 release list as it's not a breaking change anymore, see #7551 (review).

@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 6.0.0-alpha.34

@parseplatformorg parseplatformorg added the state:released-alpha Released as alpha version label Feb 24, 2023
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 6.1.0-beta.1

@parseplatformorg parseplatformorg added the state:released-beta Released as beta version label Mar 2, 2023
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 6.1.0-alpha.1

@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 6.1.0

@parseplatformorg parseplatformorg added the state:released Released as stable version label May 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state:released Released as stable version state:released-alpha Released as alpha version state:released-beta Released as beta version type:feature New feature or improvement of existing feature
Projects
None yet
3 participants