Skip to content

Instantly share code, notes, and snippets.

Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save josevalim/924ce7cc4c0e5039fd79 to your computer and use it in GitHub Desktop.
Save josevalim/924ce7cc4c0e5039fd79 to your computer and use it in GitHub Desktop.
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