Instantly share code, notes, and snippets.
josevalim/0001-Store-creation-timestamp-on-remember-cookies.patch Secret
Created Jan 18, 2016
From 2c104cdbcac4ff1dcb0fdc1bf2142aea3e835ea4 Mon Sep 17 00:00:00 2001 | |
From: =?UTF-8?q?Jos=C3=A9=20Valim?= <jose.valim@plataformatec.com.br> | |
Date: Wed, 16 Dec 2015 17:13:46 +0100 | |
Subject: [PATCH] Store creation timestamp on remember cookies | |
--- | |
lib/devise.rb | 1 + | |
lib/devise/controllers/rememberable.rb | 2 +- | |
lib/devise/models/rememberable.rb | 54 +++++++------- | |
lib/devise/models/timeoutable.rb | 6 -- | |
test/integration/rememberable_test.rb | 4 +- | |
test/integration/timeoutable_test.rb | 10 --- | |
test/models/rememberable_test.rb | 131 +++++++++------------------------ | |
7 files changed, 66 insertions(+), 142 deletions(-) | |
diff --git a/lib/devise.rb b/lib/devise.rb | |
index d186965..012fbe4 100644 | |
--- a/lib/devise.rb | |
+++ b/lib/devise.rb | |
@@ -116,6 +116,7 @@ module Devise | |
mattr_accessor :remember_for | |
@@remember_for = 2.weeks | |
+ # TODO: extend_remember_period is no longer used | |
# If true, extends the user's remember period when remembered via cookie. | |
mattr_accessor :extend_remember_period | |
@@extend_remember_period = false | |
diff --git a/lib/devise/controllers/rememberable.rb b/lib/devise/controllers/rememberable.rb | |
index 5700fde..fcd2c0f 100644 | |
--- a/lib/devise/controllers/rememberable.rb | |
+++ b/lib/devise/controllers/rememberable.rb | |
@@ -13,7 +13,7 @@ module Devise | |
def remember_me(resource) | |
return if env["devise.skip_storage"] | |
scope = Devise::Mapping.find_scope!(resource) | |
- resource.remember_me!(resource.extend_remember_period) | |
+ resource.remember_me! | |
cookies.signed[remember_key(resource, scope)] = remember_cookie_values(resource) | |
end | |
diff --git a/lib/devise/models/rememberable.rb b/lib/devise/models/rememberable.rb | |
index 62b052f..9849b3c 100644 | |
--- a/lib/devise/models/rememberable.rb | |
+++ b/lib/devise/models/rememberable.rb | |
@@ -45,11 +45,11 @@ module Devise | |
[:remember_created_at] | |
end | |
- # Generate a new remember token and save the record without validations | |
- # if remember expired (token is no longer valid) or extend_remember_period is true | |
- def remember_me!(extend_period=false) | |
- self.remember_token = self.class.remember_token if generate_remember_token? | |
- self.remember_created_at = Time.now.utc if generate_remember_timestamp?(extend_period) | |
+ # TODO: We were used to receive a extend period argument but we no longer do. | |
+ # Remove this for Devise 4.0. | |
+ def remember_me!(*) | |
+ self.remember_token = self.class.remember_token if respond_to?(:remember_token) | |
+ self.remember_created_at ||= Time.now.utc | |
save(validate: false) if self.changed? | |
end | |
@@ -57,19 +57,13 @@ module Devise | |
# it exists), and save the record without validations. | |
def forget_me! | |
return unless persisted? | |
- self.remember_token = nil if respond_to?(:remember_token=) | |
+ self.remember_token = nil if respond_to?(:remember_token) | |
self.remember_created_at = nil if self.class.expire_all_remember_me_on_sign_out | |
save(validate: false) | |
end | |
- # Remember token should be expired if expiration time not overpass now. | |
- def remember_expired? | |
- remember_created_at.nil? || (remember_expires_at <= Time.now.utc) | |
- end | |
- | |
- # Remember token expires at created time + remember_for configuration | |
def remember_expires_at | |
- remember_created_at + self.class.remember_for | |
+ self.class.remember_for.from_now | |
end | |
def rememberable_value | |
@@ -104,27 +98,30 @@ module Devise | |
protected | |
- def generate_remember_token? #:nodoc: | |
- respond_to?(:remember_token) && remember_expired? | |
- end | |
- | |
- # Generate a timestamp if extend_remember_period is true, if no remember_token | |
- # exists, or if an existing remember token has expired. | |
- def generate_remember_timestamp?(extend_period) #:nodoc: | |
- extend_period || remember_expired? | |
- end | |
- | |
module ClassMethods | |
# Create the cookie key using the record id and remember_token | |
def serialize_into_cookie(record) | |
- [record.to_key, record.rememberable_value] | |
+ [record.to_key, record.rememberable_value, Time.now.utc] | |
end | |
# Recreate the user based on the stored cookie | |
- def serialize_from_cookie(id, remember_token) | |
- record = to_adapter.get(id) | |
- record if record && !record.remember_expired? && | |
- Devise.secure_compare(record.rememberable_value, remember_token) | |
+ def serialize_from_cookie(*args) | |
+ id, token, generated_at = args | |
+ | |
+ # The token is only valid if: | |
+ # 1. we have a date | |
+ # 2. the current time does not pass the expiry period | |
+ # 3. there is a record with the given id | |
+ # 4. the record has a remember_created_at date | |
+ # 5. the token date is bigger than the remember_created_at | |
+ # 6. the token matches | |
+ if generated_at && | |
+ (self.remember_for.ago < generated_at) && | |
+ (record = to_adapter.get(id)) && | |
+ (generated_at > (record.remember_created_at || Time.now).utc) && | |
+ Devise.secure_compare(record.rememberable_value, token) | |
+ record | |
+ end | |
end | |
# Generate a token checking if one does not already exist in the database. | |
@@ -135,6 +132,7 @@ module Devise | |
end | |
end | |
+ # TODO: extend_remember_period is no longer used | |
Devise::Models.config(self, :remember_for, :extend_remember_period, :rememberable_options, :expire_all_remember_me_on_sign_out) | |
end | |
end | |
diff --git a/lib/devise/models/timeoutable.rb b/lib/devise/models/timeoutable.rb | |
index bdc2abc..d589e24 100644 | |
--- a/lib/devise/models/timeoutable.rb | |
+++ b/lib/devise/models/timeoutable.rb | |
@@ -26,7 +26,6 @@ module Devise | |
# Checks whether the user session has expired based on configured time. | |
def timedout?(last_access) | |
- return false if remember_exists_and_not_expired? | |
!timeout_in.nil? && last_access && last_access <= timeout_in.ago | |
end | |
@@ -36,11 +35,6 @@ module Devise | |
private | |
- def remember_exists_and_not_expired? | |
- return false unless respond_to?(:remember_created_at) && respond_to?(:remember_expired?) | |
- remember_created_at && !remember_expired? | |
- end | |
- | |
module ClassMethods | |
Devise::Models.config(self, :timeout_in) | |
end | |
diff --git a/test/integration/rememberable_test.rb b/test/integration/rememberable_test.rb | |
index a762eb1..2d82cca 100644 | |
--- a/test/integration/rememberable_test.rb | |
+++ b/test/integration/rememberable_test.rb | |
@@ -4,7 +4,7 @@ class RememberMeTest < ActionDispatch::IntegrationTest | |
def create_user_and_remember(add_to_token='') | |
user = create_user | |
user.remember_me! | |
- raw_cookie = User.serialize_into_cookie(user).tap { |a| a.last << add_to_token } | |
+ raw_cookie = User.serialize_into_cookie(user).tap { |a| a[1] << add_to_token } | |
cookies['remember_user_token'] = generate_signed_cookie(raw_cookie) | |
user | |
end | |
@@ -135,7 +135,7 @@ class RememberMeTest < ActionDispatch::IntegrationTest | |
test 'do not remember with expired token' do | |
create_user_and_remember | |
- swap Devise, remember_for: 0 do | |
+ swap Devise, remember_for: 0.days do | |
get users_path | |
assert_not warden.authenticated?(:user) | |
assert_redirected_to new_user_session_path | |
diff --git a/test/integration/timeoutable_test.rb b/test/integration/timeoutable_test.rb | |
index 1160571..04302eb 100644 | |
--- a/test/integration/timeoutable_test.rb | |
+++ b/test/integration/timeoutable_test.rb | |
@@ -165,16 +165,6 @@ class SessionTimeoutTest < ActionDispatch::IntegrationTest | |
end | |
end | |
- test 'time out not triggered if remembered' do | |
- user = sign_in_as_user remember_me: true | |
- get expire_user_path(user) | |
- assert_not_nil last_request_at | |
- | |
- get users_path | |
- assert_response :success | |
- assert warden.authenticated?(:user) | |
- end | |
- | |
test 'does not crashes when the last_request_at is a String' do | |
user = sign_in_as_user | |
diff --git a/test/models/rememberable_test.rb b/test/models/rememberable_test.rb | |
index c69643c..eb13463 100644 | |
--- a/test/models/rememberable_test.rb | |
+++ b/test/models/rememberable_test.rb | |
@@ -13,6 +13,7 @@ class RememberableTest < ActiveSupport::TestCase | |
user = create_user | |
user.expects(:valid?).never | |
user.remember_me! | |
+ assert user.remember_created_at | |
end | |
test 'forget_me should not clear remember token if using salt' do | |
@@ -33,13 +34,45 @@ class RememberableTest < ActiveSupport::TestCase | |
test 'serialize into cookie' do | |
user = create_user | |
user.remember_me! | |
- assert_equal [user.to_key, user.authenticatable_salt], User.serialize_into_cookie(user) | |
+ id, token, date = User.serialize_into_cookie(user) | |
+ assert_equal id, user.to_key | |
+ assert_equal token, user.authenticatable_salt | |
+ assert date.is_a?(Time) | |
end | |
test 'serialize from cookie' do | |
user = create_user | |
user.remember_me! | |
- assert_equal user, User.serialize_from_cookie(user.to_key, user.authenticatable_salt) | |
+ assert_equal user, User.serialize_from_cookie(user.to_key, user.authenticatable_salt, Time.now.utc) | |
+ end | |
+ | |
+ test 'serialize from cookie should return nil if no resource is found' do | |
+ assert_nil resource_class.serialize_from_cookie([0], "123", Time.now.utc) | |
+ end | |
+ | |
+ test 'serialize from cookie should return nil if no timestamp' do | |
+ user = create_user | |
+ user.remember_me! | |
+ assert_nil User.serialize_from_cookie(user.to_key, user.authenticatable_salt) | |
+ end | |
+ | |
+ test 'serialize from cookie should return nil if timestamp is earlier than token creation' do | |
+ user = create_user | |
+ user.remember_me! | |
+ assert_nil User.serialize_from_cookie(user.to_key, user.authenticatable_salt, 1.day.ago) | |
+ end | |
+ | |
+ test 'serialize from cookie should return nil if timestamp is older than remember_for' do | |
+ user = create_user | |
+ user.remember_created_at = 1.month.ago | |
+ user.remember_me! | |
+ assert_nil User.serialize_from_cookie(user.to_key, user.authenticatable_salt, 3.weeks.ago) | |
+ end | |
+ | |
+ test 'serialize from cookie me return nil if is a valid resource with invalid token' do | |
+ user = create_user | |
+ user.remember_me! | |
+ assert_nil User.serialize_from_cookie(user.to_key, "123", Time.now.utc) | |
end | |
test 'raises a RuntimeError if authenticatable_salt is nil or empty' do | |
@@ -93,28 +126,7 @@ class RememberableTest < ActiveSupport::TestCase | |
resource.forget_me! | |
end | |
- test 'remember is expired if not created at timestamp is set' do | |
- assert create_resource.remember_expired? | |
- end | |
- | |
- test 'serialize should return nil if no resource is found' do | |
- assert_nil resource_class.serialize_from_cookie([0], "123") | |
- end | |
- | |
- test 'remember me return nil if is a valid resource with invalid token' do | |
- resource = create_resource | |
- assert_nil resource_class.serialize_from_cookie([resource.id], "123") | |
- end | |
- | |
- test 'remember for should fallback to devise remember for default configuration' do | |
- swap Devise, remember_for: 1.day do | |
- resource = create_resource | |
- resource.remember_me! | |
- assert_not resource.remember_expired? | |
- end | |
- end | |
- | |
- test 'remember expires at should sum date of creation with remember for configuration' do | |
+ test 'remember expires at uses remember for configuration' do | |
swap Devise, remember_for: 3.days do | |
resource = create_resource | |
resource.remember_me! | |
@@ -125,77 +137,6 @@ class RememberableTest < ActiveSupport::TestCase | |
end | |
end | |
- test 'remember should be expired if remember_for is zero' do | |
- swap Devise, remember_for: 0.days do | |
- Devise.remember_for = 0.days | |
- resource = create_resource | |
- resource.remember_me! | |
- assert resource.remember_expired? | |
- end | |
- end | |
- | |
- test 'remember should be expired if it was created before limit time' do | |
- swap Devise, remember_for: 1.day do | |
- resource = create_resource | |
- resource.remember_me! | |
- resource.remember_created_at = 2.days.ago | |
- resource.save | |
- assert resource.remember_expired? | |
- end | |
- end | |
- | |
- test 'remember should not be expired if it was created within the limit time' do | |
- swap Devise, remember_for: 30.days do | |
- resource = create_resource | |
- resource.remember_me! | |
- resource.remember_created_at = (30.days.ago + 2.minutes) | |
- resource.save | |
- assert_not resource.remember_expired? | |
- end | |
- end | |
- | |
- test 'if extend_remember_period is false, remember_me! should generate a new timestamp if expired' do | |
- swap Devise, remember_for: 5.minutes do | |
- resource = create_resource | |
- resource.remember_me!(false) | |
- assert resource.remember_created_at | |
- | |
- resource.remember_created_at = old = 10.minutes.ago | |
- resource.save | |
- | |
- resource.remember_me!(false) | |
- assert_not_equal old.to_i, resource.remember_created_at.to_i | |
- end | |
- end | |
- | |
- test 'if extend_remember_period is false, remember_me! should not generate a new timestamp' do | |
- swap Devise, remember_for: 1.year do | |
- resource = create_resource | |
- resource.remember_me!(false) | |
- assert resource.remember_created_at | |
- | |
- resource.remember_created_at = old = 10.minutes.ago.utc | |
- resource.save | |
- | |
- resource.remember_me!(false) | |
- assert_equal old.to_i, resource.remember_created_at.to_i | |
- end | |
- end | |
- | |
- test 'if extend_remember_period is true, remember_me! should always generate a new timestamp' do | |
- swap Devise, remember_for: 1.year do | |
- resource = create_resource | |
- resource.remember_me!(true) | |
- assert resource.remember_created_at | |
- | |
- resource.remember_created_at = old = 10.minutes.ago | |
- resource.save | |
- | |
- resource.remember_me!(true) | |
- assert_not_equal old, resource.remember_created_at | |
- end | |
- end | |
- | |
test 'should have the required_fields array' do | |
assert_same_content Devise::Models::Rememberable.required_fields(User), [ | |
:remember_created_at | |
-- | |
2.5.4 (Apple Git-61) | |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment