From 617e142e3408efe17a50de041a1605f2eceeb2bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sat, 25 Sep 2010 17:24:42 +0200 Subject: [PATCH] Store the salt in session and expire the session if the user changes his password --- CHANGELOG.rdoc | 1 + .../devise/registrations_controller.rb | 1 + lib/devise/controllers/helpers.rb | 17 +++++++++-- lib/devise/models/authenticatable.rb | 3 ++ lib/devise/rails/warden_compat.rb | 28 +++++++++++++------ test/controllers/helpers_test.rb | 7 +++++ test/integration/registerable_test.rb | 14 ++++++++++ 7 files changed, 59 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.rdoc b/CHANGELOG.rdoc index 12e34c58..4d2bf445 100644 --- a/CHANGELOG.rdoc +++ b/CHANGELOG.rdoc @@ -11,6 +11,7 @@ * Ensure the friendly token does not include "_" or "-" since some e-mails may not autolink it properly (by github.com/rymai) * Extracted encryptors into :encryptable for better bcrypt support * :rememberable is now able to use salt as token if no remember_token is provided + * Store the salt in session and expire the session if the user changes his password * bugfix * after_sign_in_path_for always receives a resource diff --git a/app/controllers/devise/registrations_controller.rb b/app/controllers/devise/registrations_controller.rb index 57fc1adf..57c82aee 100644 --- a/app/controllers/devise/registrations_controller.rb +++ b/app/controllers/devise/registrations_controller.rb @@ -31,6 +31,7 @@ class Devise::RegistrationsController < ApplicationController def update if resource.update_with_password(params[resource_name]) set_flash_message :notice, :updated + sign_in resource_name, resource, :bypass => true redirect_to after_update_path_for(resource) else clean_up_passwords(resource) diff --git a/lib/devise/controllers/helpers.rb b/lib/devise/controllers/helpers.rb index 1e8755b4..525dcdb2 100644 --- a/lib/devise/controllers/helpers.rb +++ b/lib/devise/controllers/helpers.rb @@ -87,18 +87,29 @@ module Devise # Sign in an user that already was authenticated. This helper is useful for logging # users in after sign up. # + # All options given to sign_in is passed forward to the set_user method in warden. + # The only exception is the :bypass option, which bypass warden callbacks and stores + # the user straight in session. This option is useful in cases the user is already + # signed in, but we want to refresh the credentials in session. + # # Examples: # # sign_in :user, @user # sign_in(scope, resource) # sign_in @user # sign_in(resource) # sign_in @user, :event => :authentication # sign_in(resource, options) - # + # sign_in @user, :bypass => true # sign_in(resource, options) + # def sign_in(resource_or_scope, *args) options = args.extract_options! scope = Devise::Mapping.find_scope!(resource_or_scope) resource = args.last || resource_or_scope - expire_session_data_after_sign_in! - warden.set_user(resource, options.merge!(:scope => scope)) + + if options[:bypass] + warden.session_serializer.store(resource, scope) + else + expire_session_data_after_sign_in! + warden.set_user(resource, options.merge!(:scope => scope)) + end end # Sign out a given user or scope. This helper is useful for signing out an user diff --git a/lib/devise/models/authenticatable.rb b/lib/devise/models/authenticatable.rb index a0ede5bc..7f322263 100644 --- a/lib/devise/models/authenticatable.rb +++ b/lib/devise/models/authenticatable.rb @@ -73,6 +73,9 @@ module Devise :inactive end + def authenticatable_salt + end + module ClassMethods Devise::Models.config(self, :authentication_keys, :request_keys, :http_authenticatable, :params_authenticatable) diff --git a/lib/devise/rails/warden_compat.rb b/lib/devise/rails/warden_compat.rb index c352023f..cfd9d8e1 100644 --- a/lib/devise/rails/warden_compat.rb +++ b/lib/devise/rails/warden_compat.rb @@ -15,18 +15,28 @@ end class Warden::SessionSerializer def serialize(record) - [record.class.name, record.id] + [record.class.name, record.id, record.authenticatable_salt] end def deserialize(keys) - klass, id = keys - klass.constantize.find(:first, :conditions => { :id => id }) - rescue NameError => e - if e.message =~ /uninitialized constant/ - Rails.logger.debug "Trying to deserialize invalid class #{klass}" - nil - else - raise + if keys.size == 2 + raise "Devise changed how it stores objects in session. If you are seeing this message, " << + "you can fix it by changing one character in your cookie secret, forcing all previous " << + "cookies to expire, or cleaning up your database sessions if you are using a db store." + end + + klass, id, salt = keys + + begin + record = klass.constantize.find(:first, :conditions => { :id => id }) + record if record && record.authenticatable_salt == salt + rescue NameError => e + if e.message =~ /uninitialized constant/ + Rails.logger.debug "Trying to deserialize invalid class #{klass}" + nil + else + raise + end end end end \ No newline at end of file diff --git a/test/controllers/helpers_test.rb b/test/controllers/helpers_test.rb index db9c4030..ca50a831 100644 --- a/test/controllers/helpers_test.rb +++ b/test/controllers/helpers_test.rb @@ -100,6 +100,13 @@ class ControllerAuthenticableTest < ActionController::TestCase @controller.sign_in(user) end + test 'sign in accepts bypass as option' do + user = User.new + @mock_warden.expects(:session_serializer).returns(serializer = mock()) + serializer.expects(:store).with(user, :user) + @controller.sign_in(user, :bypass => true) + end + test 'sign out proxy to logout on warden' do @mock_warden.expects(:user).with(:user).returns(true) @mock_warden.expects(:logout).with(:user).returns(true) diff --git a/test/integration/registerable_test.rb b/test/integration/registerable_test.rb index 533b2279..7d084f60 100644 --- a/test/integration/registerable_test.rb +++ b/test/integration/registerable_test.rb @@ -98,6 +98,20 @@ class RegistrationTest < ActionController::IntegrationTest assert_equal "user.new@email.com", User.first.email end + test 'a signed in user should still be able to use the website after changing his password' do + sign_in_as_user + get edit_user_registration_path + + fill_in 'password', :with => '12345678' + fill_in 'password confirmation', :with => '12345678' + fill_in 'current password', :with => '123456' + click_button 'Update' + + assert_contain 'You updated your account successfully.' + get users_path + assert warden.authenticated?(:user) + end + test 'a signed in user should not change his current user with invalid password' do sign_in_as_user get edit_user_registration_path