Instantly share code, notes, and snippets.
Created
January 18, 2016 14:22
-
Star
(0)
0
You must be signed in to star a gist -
Fork
(0)
0
You must be signed in to fork a gist
-
Save josevalim/924ce7cc4c0e5039fd79 to your computer and use it in GitHub Desktop.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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