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

Add revokeSessionOnPasswordReset option. Closes #1584 #1597

Merged

Conversation

drew-gross
Copy link
Contributor

No description provided.

@drew-gross drew-gross changed the title Add revokeSessionOnPasswordReset option Add revokeSessionOnPasswordReset option. Closes #1584 Apr 22, 2016
}) {
if (typeof revokeSessionOnPasswordReset !== 'boolean') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe move to the config validate ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do

@codecov-io
Copy link

Current coverage is 92.77%

Merging #1597 into master will decrease coverage by -0.02% as of 79423c9

@@            master   #1597   diff @@
======================================
  Files           87      87       
  Stmts         5521    5525     +4
  Branches      1052    1054     +2
  Methods          0       0       
======================================
+ Hit           5123    5126     +3
  Partial         14      14       
- Missed         384     385     +1

Review entire Coverage Diff as of 79423c9

Powered by Codecov. Updated on successful CI builds.

@facebook-github-bot
Copy link

@drew-gross updated the pull request.

@flovilmart
Copy link
Contributor

So after this PR' sessions won't be revoked on password reset? That seems like a security flow as an impersonator would be able to use and old session.

@drew-gross
Copy link
Contributor Author

Thats true, it's a marginal security risk. But, it's available in Parse.com, and there could be apps depending on this behaviour, so I don't think it's too bad, especially if your app has a session management page. Also sessions are revoked by default, you need to opt in to the less secure behaviour.

@flovilmart
Copy link
Contributor

Looks like it's The opposite as the default value is false. Is that intended? I'm fine with either, but that changes the default behavior and may impact existing deployments replacing the behavior by a slightly less secure onez

@drew-gross
Copy link
Contributor Author

@flovilmart
Copy link
Contributor

Was reading the wrong line. Sorry about that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants