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 config option to disable signin after password reset #952

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

end
```

Expand Down
2 changes: 1 addition & 1 deletion app/controllers/clearance/passwords_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def update
@user = find_user_for_update

if @user.update_password(password_from_password_reset_params)
sign_in @user
sign_in @user if Clearance.configuration.sign_in_on_password_reset?
redirect_to url_after_update
session[:password_reset_token] = nil
else
Expand Down
11 changes: 11 additions & 0 deletions lib/clearance/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,12 @@ class Configuration
# @return [Array<String>]
attr_accessor :allowed_backdoor_environments

# Controls wether users are automatically signed in after successfully
# resetting their password.
# Defaults to `true`.
# @return [Boolean]
attr_writer :sign_in_on_password_reset

def initialize
@allow_sign_up = true
@allowed_backdoor_environments = ["test", "ci", "development"]
Expand All @@ -134,6 +140,7 @@ def initialize
@secure_cookie = false
@signed_cookie = false
@sign_in_guards = []
@sign_in_on_password_reset = true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

end

def signed_cookie=(value)
Expand Down Expand Up @@ -214,6 +221,10 @@ def reload_user_model
def rotate_csrf_on_sign_in?
!!rotate_csrf_on_sign_in
end

def sign_in_on_password_reset?
@sign_in_on_password_reset
end
end

# @return [Clearance::Configuration] Clearance's current configuration
Expand Down
27 changes: 23 additions & 4 deletions spec/requests/password_maintenance_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,36 @@

describe "Password maintenance" do
context "when updating the password" do
it "signs the user in and redirects" do
user = create(:user, :with_forgotten_password)
let(:user) { create(:user, :with_forgotten_password) }

before(:each) do
Clearance.configure do |config|
config.sign_in_on_password_reset = sign_in_on_password_reset
end

put user_password_url(user), params: {
user_id: user,
token: user.confirmation_token,
password_reset: { password: "my_new_password" },
}
end

context "if `sign_in_on_password_reset` option is true" do
let(:sign_in_on_password_reset) { true }

it "signs the user in and redirects" do
expect(response).to redirect_to(Clearance.configuration.redirect_url)
expect(cookies["remember_token"]).to be_present
end
end

context "if `sign_in_on_password_reset` option is false" do
let(:sign_in_on_password_reset) { false }

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
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

expect(response).to redirect_to(Clearance.configuration.redirect_url)
expect(cookies["remember_token"]).not_to be_present
end
end
end
end