Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Remember me & timeout #308

Closed
wants to merge 1 commit into from

9 participants

@jeyb

Allow remember_me to be set alongside of timeout, with remember_me taking precedence. @slavik112211 implemented this a long-while back but I don't see a pull request for it. Not sure why, if you see an issue with the work let me know and I can adjust/fix.

I've tested this manually as well and it works as expected, with remember me setting taking precedence over timeout.

There are a few reported issues this fixes, #126, #130 and #134.

@jeyb jeyb Allow remember_me to be set alongside of timeout, with remember_me ta…
…king precedence. Pulled from slavik112211/authlogic.
bb5c8a8
@sailing

+1 Would love to see this.

@jefmathiot jefmathiot referenced this pull request from a commit in servebox/authlogic
@jefmathiot jefmathiot Manually integrated changes from pull request #308 (remember me vs. t…
…imeout)
a95d8c0
@binarylogic
Owner

Thanks, this has been pulled in

@phuibonhoa

I believe there is a vulnerability in storing the remember_me timeout in the cookie. A user can edit this timeout on the cookie and stay authenticated indefinitely. Doesn't the timeout need to be handled server-side (by adding a remember_expires_at or similar type of column)

@tiegz
Collaborator

@phuibonhoa just saw your comment here-- I added a PR a while ago to set this cookie as a signed cookie, which would require the app's secret token to set/read the persistence cookie: #342 . I think that would make this feature more secure?

@md5

I believe this issue can be closed. I see a remember_me_expired? method in master. cf. https://github.com/binarylogic/authlogic/blob/423f53a9c604c95c634b00d7a05bb6382fe28119/lib/authlogic/session/cookies.rb#L123

@tiegz
Collaborator

Thanks @md5

@tiegz tiegz closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 14, 2012
  1. @jeyb

    Allow remember_me to be set alongside of timeout, with remember_me ta…

    jeyb authored
    …king precedence. Pulled from slavik112211/authlogic.
This page is out of date. Refresh to see the latest.
View
1  Gemfile
@@ -8,4 +8,5 @@ group :test do
gem 'rake'
gem 'ruby-debug19'
gem 'sqlite3'
+ gem 'timecop'
end
View
2  Gemfile.lock
@@ -38,6 +38,7 @@ GEM
ruby_core_source (0.1.5)
archive-tar-minitar (>= 0.5.2)
sqlite3 (1.3.4)
+ timecop (0.3.5)
tzinfo (0.3.30)
PLATFORMS
@@ -50,3 +51,4 @@ DEPENDENCIES
rake
ruby-debug19
sqlite3
+ timecop
View
9 lib/authlogic/session/cookies.rb
@@ -110,6 +110,12 @@ def remember_me_until
remember_me_for.from_now
end
+ # Has the cookie expired due to current time being greater than remember_me_until.
+ def remember_me_expired?
+ return unless remember_me?
+ (Time.parse(cookie_credentials[2]) < Time.now)
+ end
+
# If the cookie should be marked as secure (SSL only)
def secure
return @secure if defined?(@secure)
@@ -164,8 +170,9 @@ def persist_by_cookie
end
def save_cookie
+ remember_me_until_value = "::#{remember_me_until}" if remember_me?
controller.cookies[cookie_key] = {
- :value => "#{record.persistence_token}::#{record.send(record.class.primary_key)}",
+ :value => "#{record.persistence_token}::#{record.send(record.class.primary_key)}#{remember_me_until_value}",
:expires => remember_me_until,
:secure => secure,
:httponly => httponly,
View
3  lib/authlogic/session/persistence.rb
@@ -51,6 +51,7 @@ module InstanceMethods
def persisting?
return true if !record.nil?
self.attempted_record = nil
+ self.remember_me = !cookie_credentials.nil? && !cookie_credentials[2].nil?
before_persisting
persist
ensure_authentication_attempted
@@ -67,4 +68,4 @@ def persisting?
end
end
end
-end
+end
View
8 lib/authlogic/session/timeout.rb
@@ -58,7 +58,11 @@ module InstanceMethods
# Tells you if the record is stale or not. Meaning the record has timed out. This will only return true if you set logout_on_timeout to true in your configuration.
# Basically how a bank website works. If you aren't active over a certain period of time your session becomes stale and requires you to log back in.
def stale?
- !stale_record.nil? || (logout_on_timeout? && record && record.logged_out?)
+ if remember_me?
+ remember_me_expired?
+ else
+ !stale_record.nil? || (logout_on_timeout? && record && record.logged_out?)
+ end
end
private
@@ -79,4 +83,4 @@ def logout_on_timeout?
end
end
end
-end
+end
View
21 test/session_test/cookies_test.rb
@@ -116,6 +116,19 @@ def test_persist_persist_by_cookie
assert_equal ben, session.record
end
+ def test_remember_me_expired
+ ben = users(:ben)
+ session = UserSession.new(ben)
+ session.remember_me = true
+ assert session.save
+ assert !session.remember_me_expired?
+
+ session = UserSession.new(ben)
+ session.remember_me = false
+ assert session.save
+ assert !session.remember_me_expired?
+ end
+
def test_after_save_save_cookie
ben = users(:ben)
session = UserSession.new(ben)
@@ -123,6 +136,14 @@ def test_after_save_save_cookie
assert_equal "#{ben.persistence_token}::#{ben.id}", controller.cookies["user_credentials"]
end
+ def test_after_save_save_cookie_with_remember_me
+ ben = users(:ben)
+ session = UserSession.new(ben)
+ session.remember_me = true
+ assert session.save
+ assert_equal "#{ben.persistence_token}::#{ben.id}::#{session.remember_me_until}", controller.cookies["user_credentials"]
+ end
+
def test_after_destroy_destroy_cookie
ben = users(:ben)
set_cookie_for(ben)
View
13 test/session_test/persistence_test.rb
@@ -17,5 +17,16 @@ def test_find
def test_persisting
# tested thoroughly in test_find
end
+
+ def test_should_set_remember_me_on_the_next_request
+ ben = users(:ben)
+ session = UserSession.new(ben)
+ session.remember_me = true
+ assert !UserSession.remember_me
+ assert session.save
+ assert session.remember_me?
+ session = UserSession.find(ben)
+ assert session.remember_me?
+ end
end
-end
+end
View
30 test/session_test/timeout_test.rb
@@ -38,6 +38,34 @@ def test_stale_state
UserSession.logout_on_timeout = false
end
+
+ def test_should_be_stale_with_expired_remember_date
+ UserSession.logout_on_timeout = true
+ UserSession.remember_me = true
+ UserSession.remember_me_for = 3.months
+ ben = users(:ben)
+ assert ben.save
+ session = UserSession.new(ben)
+ assert session.save
+ Timecop.freeze(Time.now + 4.month)
+ assert session.persisting?
+ assert session.stale?
+ UserSession.remember_me = false
+ end
+
+ def test_should_not_be_stale_with_valid_remember_date
+ UserSession.logout_on_timeout = true # Default is 10.minutes
+ UserSession.remember_me = true
+ UserSession.remember_me_for = 3.months
+ ben = users(:ben)
+ assert ben.save
+ session = UserSession.new(ben)
+ assert session.save
+ Timecop.freeze(Time.now + 2.months)
+ assert session.persisting?
+ assert !session.stale?
+ UserSession.remember_me = false
+ end
def test_successful_login
UserSession.logout_on_timeout = true
@@ -49,4 +77,4 @@ def test_successful_login
end
end
end
-end
+end
View
1  test/test_helper.rb
@@ -3,6 +3,7 @@
require "ruby-debug"
require "active_record"
require "active_record/fixtures"
+require "timecop"
#ActiveRecord::Schema.verbose = false
ActiveRecord::Base.establish_connection(:adapter => "sqlite3", :database => ":memory:")
Something went wrong with that request. Please try again.