Add config option to disable signin after password reset #952
Conversation
tillprochaska
force-pushed
the
configure-signin-after-password-reset
branch
from
e766a6c
to
645fcdc
Compare
|
|
||
| expect(response).to redirect_to(Clearance.configuration.redirect_url) | ||
| expect(cookies["remember_token"]).to be_present | ||
| it "redirects, but does not sign in the user" do |
There was a problem hiding this comment.
I wasn’t sure wether this should go into the controller spec for PasswordsController or into this request spec, but decided to add it to the request spec as this one tests the current behavior.
| @@ -66,6 +66,7 @@ Clearance.configure do |config| | |||
| config.sign_in_guards = [] | |||
| config.user_model = "User" | |||
| config.parent_controller = "ApplicationController" | |||
| config.sign_in_on_password_reset = false | |||
There was a problem hiding this comment.
I think this should be sufficient as the configuration option is quiet self-explanatory and there is additional information in the respective lib/clerance/configuration.rb.
| @@ -134,6 +140,7 @@ def initialize | |||
| @secure_cookie = false | |||
| @signed_cookie = false | |||
| @sign_in_guards = [] | |||
| @sign_in_on_password_reset = true | |||
There was a problem hiding this comment.
Is there any preference regarding the order of instance variables and method definitions in this class? I simply added the new configuration option at the end. They seem to be grouped somewhat semantically.
|
I've made some changes and credited you in #969 I think it's better for now to stick to controller specs. Migrating to requests specs would be nice though but the tests are already organized around controller specs. |
|
@dorianmariefr That's great, thanks a lot! |
Right now, users are automatically signed in after resetting their password. However, for some use cases, it might make sense that they have to sign in explicitly.
For example, in an app we’re building, we have implemented 2FA on top of Clearance. After a password reset, we want users to sign in themselves using the second factor. Otherwise, password resets could be used to circumvent 2FA. (While we could ask for the second factor before resetting the password, we decided to allow password resets without the second factor, but then force users to sign in.)
This PR adds a new configuration option to Clearance,
sign_in_on_password_reset. It defaults totrue, i.e. the default behavior does not change.Closes #949.