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

Disable auto-login after password reset #949

Open
tillprochaska opened this issue Oct 30, 2021 · 4 comments
Open

Disable auto-login after password reset #949

tillprochaska opened this issue Oct 30, 2021 · 4 comments

Comments

@tillprochaska
Copy link
Contributor

Currently, users are automatically logged in after resetting a password. This is definitely a smooth UX in most cases. In our application, we’re extending Clearance with MFA support. Therefore, we’d like users to explicitly sign-in after resetting their password.

Currently, this requires overwriting the PasswordsController#update action. I’d prefer not having to do that for such a small change and was wondering if you think either a configuration option or extracting that behavior into a separate method that can easily be overwritten might make sense. I’d be willing to put in a PR! :)

https://github.com/thoughtbot/clearance/blob/main/app/controllers/clearance/passwords_controller.rb#L36

@tillprochaska
Copy link
Contributor Author

This ticket from 2019 might be related: #878

@MottiniMauro
Copy link
Contributor

Hi @tillprochaska ! First of all, thank you for raising this issue. I think your idea of adding a configuration option for this would be great, and having it default to its current behavior should help avoid any issues. If you are willing to create a PR for this change that you be appreciated, just let me know if I can provide any help.

@tillprochaska
Copy link
Contributor Author

Hi @MottiniMauro, thanks a lot for your reply! As far as I can see, this should be a small change. Thanks for offering help -- might get back to this as I’ve never contributed to this gem before!

@tillprochaska
Copy link
Contributor Author

Hi @MottiniMauro, just wanted to ask if you had a chance to look into the respective PR (#952). No pressure at all, just wanted to make sure this didn’t get lost in day-to-day business.

If there’s anything else I can do to get this PR merged, just let me know! I’ve also added a few comments to the PR where I wasn’t sure about code/documentation style.

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 a pull request may close this issue.

2 participants