Skip to content

Commit

Permalink
make "stay logged in" use a one-time token
Browse files Browse the repository at this point in the history
closes #6382

Previously, the "stay logged in" cookie just used the authlogic default
implementation, which is the pseudonym persistence_token. This is a
problem, because that persistence_token only ever changes when the
pseudonym password changes, so it's the same everywhere; so if that
cookie is stolen, it's valid for a very long time.

This switches us to one-time-use tokens that expire as soon as the token
logs the user in once. Each user agent also gets a different
one-time-use token.

Change-Id: I4f20cd7759fd74590e82ed55797552e342243d49
testplan:
  * Check that no token is set at all when "stay logged in" isn't
    selected.
  * Check "stay logged in", and verify:
    * That you don't have to login again after restarting your browser,
      but your _normandy_session got reset.
    * That if you save and try to replay using the same
      pseudonym_credentials, they don't work the second time.
    * That a second browser will get a different pseudonym_credentials
      value, and using one token doesn't affect the other.
    * That once the token is used, a new one is generated and set in
      your cookies. Verify this new token works as well.
    * That logging out removes the pseudonym_credentials cookie in your
      browser. And also that manually restoring this cookie still
      doesn't log you in, since it was removed server-side as well.
  * Change your password, and verify that the existing "stay logged in"
    tokens no longer work.
  * Delete your pseudonym, and verify the same.
Reviewed-on: https://gerrit.instructure.com/7093
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
Reviewed-by: Zach Wily <zach@instructure.com>
  • Loading branch information
codekitchen committed Nov 22, 2011
1 parent 4757b59 commit 4ef50c1
Show file tree
Hide file tree
Showing 9 changed files with 298 additions and 9 deletions.
1 change: 1 addition & 0 deletions Gemfile
Expand Up @@ -6,6 +6,7 @@ gem 'authlogic', '2.1.3'
# use custom gem until pull request at https://github.com/marcel/aws-s3/pull/41
# is merged into mainline. gem built from https://github.com/lukfugl/aws-s3
gem "aws-s3-instructure", "~> 0.6.2.1319643167", :require => 'aws/s3'
gem 'bcrypt-ruby', '3.0.1'
gem 'builder', '2.1.2'
gem 'closure-compiler','1.0.0'
gem 'compass', '0.11.5'
Expand Down
3 changes: 2 additions & 1 deletion app/models/pseudonym.rb
Expand Up @@ -20,7 +20,8 @@ class Pseudonym < ActiveRecord::Base
include Workflow

attr_accessible :user, :account, :password, :password_confirmation, :path, :path_type, :password_auto_generated, :unique_id


has_many :session_persistence_tokens
belongs_to :account
belongs_to :user
has_many :communication_channels, :order => 'position'
Expand Down
39 changes: 32 additions & 7 deletions app/models/pseudonym_session.rb
Expand Up @@ -44,24 +44,49 @@ def used_basic_auth?
end

# modifications to authlogic's cookie persistence (used for the "remember me" token)
# much of the theory here is based on this blog post:
# http://fishbowl.pastiche.org/2004/01/19/persistent_login_cookie_best_practice/
# see the SessionPersistenceToken class for details
#
# also, authlogic doesn't support httponly (or secure-only) for the "remember me"
# cookie yet, so we add that support here. there's an open pull request still
# pending:
# https://github.com/binarylogic/authlogic/issues/issue/210
# also, the version of authlogic canvas is on doesn't support httponly (or
# secure-only) for the "remember me" cookie yet, so we add that support here.
def save_cookie
return unless remember_me?
token = SessionPersistenceToken.generate(record)
controller.cookies[cookie_key] = {
:value => "#{record.persistence_token}::#{record.send(record.class.primary_key)}",
:value => token.pseudonym_credentials,
:expires => remember_me_until,
:domain => controller.cookie_domain,
:httponly => true,
:secure => ActionController::Base.session_options[:secure],
}
end

def persist_by_cookie
cookie = controller.cookies[cookie_key]
if cookie
token = SessionPersistenceToken.find_by_pseudonym_credentials(cookie)
self.unauthorized_record = token.use! if token
is_valid = self.valid?
if is_valid
# this token has been used -- destroy it, and generate a new one
# remember_me is implicitly true when they login via the remember_me token
self.remember_me = true
self.save!
end
is_valid
else
false
end
end

# added behavior: destroy the server-side SessionPersistenceToken as well as the browser cookie
def destroy_cookie
cookie = controller.cookies.delete cookie_key, :domain => controller.cookie_domain
return true unless cookie
token = SessionPersistenceToken.find_by_pseudonym_credentials(cookie)
token.try(:destroy)
true
end

# Validate the session using password auth (either local or LDAP, but not
# SSO). If too many failed attempts have occured, the validation will fail.
# In this case, `too_many_attempts?` will be true, rather than
Expand Down
89 changes: 89 additions & 0 deletions app/models/session_persistence_token.rb
@@ -0,0 +1,89 @@
#
# Copyright (C) 2011 Instructure, Inc.
#
# This file is part of Canvas.
#
# Canvas is free software: you can redistribute it and/or modify it under
# the terms of the GNU Affero General Public License as published by the Free
# Software Foundation, version 3 of the License.
#
# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
# details.
#
# You should have received a copy of the GNU Affero General Public License along
# with this program. If not, see <http://www.gnu.org/licenses/>.
#

require 'authlogic/crypto_providers/bcrypt'

# A SessionPersistenceToken is a one-time-use "remember me" token to maintain a
# user's login across browser sessions. It has an expiry, and it's destroyed
# when the user logs out, but most importantly, it is destroyed after the first
# time it is used to authenticate the user.
#
# The token is comprised of three fields:
# <token_id:pseudonym.persistence_token:random_uuid>
#
# The first field allows efficient lookup, the second field verifies that the
# pseudonym hasn't changed their password or anything else that changes the
# persistence_token, and the last field is an unguessable token identifier
# generated at random. This last field is password-equivalent, so it's stored
# as a salt+hash server-side.
#
# much of the theory here is based on this blog post:
# http://fishbowl.pastiche.org/2004/01/19/persistent_login_cookie_best_practice/
#
# See the PseudonymSession model for the *_cookie methods that use this class.
class SessionPersistenceToken < ActiveRecord::Base
belongs_to :pseudonym

attr_accessible :pseudonym, :crypted_token, :token_salt, :uncrypted_token
attr_accessor :uncrypted_token
validates_presence_of :pseudonym_id, :crypted_token, :token_salt

def self.generate(pseudonym)
salt = ActiveSupport::SecureRandom.hex(8)
token = ActiveSupport::SecureRandom.hex(32)
self.create!(:pseudonym => pseudonym,
:token_salt => salt,
:uncrypted_token => token,
:crypted_token => self.hashed_token(salt, token))
end

def self.hashed_token(salt, token)
self.crypto.encrypt(salt, token)
end

def self.crypto
Authlogic::CryptoProviders::BCrypt
end

def self.find_by_pseudonym_credentials(creds)
token_id, persistence_token, uuid = creds.split("::")
return unless token_id.present? && persistence_token.present? && uuid.present?
token = self.find_by_id(token_id)
return unless token
return unless token.valid_token?(persistence_token, uuid)
return token
end

def valid_token?(persistence_token, uncrypted_token)
# if the pseudonym is marked deleted, the token can still be marked as
# valid, but the actual login step will fail as expected.
self.pseudonym &&
self.pseudonym.persistence_token == persistence_token &&
self.class.crypto.matches?(self.crypted_token, self.token_salt, uncrypted_token)
end

def pseudonym_credentials
raise "can't build pseudonym_credentials except on just-generated token" unless uncrypted_token
"#{id}::#{pseudonym.persistence_token}::#{uncrypted_token}"
end

def use!
destroy
return pseudonym
end
end
7 changes: 6 additions & 1 deletion config/initializers/authlogic_mods.rb
Expand Up @@ -8,6 +8,11 @@ class RailsAdapter < AbstractAdapter
end
end

# we need http basic auth to take precedence over the session cookie, for the api
# we need http basic auth to take precedence over the session cookie, for the api.
cb = Authlogic::Session::Base.persist_callback_chain.delete(:persist_by_http_auth)
Authlogic::Session::Base.persist_callback_chain.unshift(cb) if cb
# we also need the session cookie to take precendence over the "remember me" cookie,
# otherwise we'll use the "remember me" cookie every request, which triggers
# generating a new "remember me" cookie since they're one-time use.
cb = Authlogic::Session::Base.persist_callback_chain.delete(:persist_by_cookie)
Authlogic::Session::Base.persist_callback_chain.push(cb) if cb
6 changes: 6 additions & 0 deletions config/periodic_jobs.rb
Expand Up @@ -18,6 +18,12 @@
end
end

persistence_token_expire_after = (Setting.from_config("session_store") || {})[:expire_remember_me_after]
persistence_token_expire_after ||= 1.month
Delayed::Periodic.cron 'SessionPersistenceToken.delete_all', '35 11 * * *' do
SessionPersistenceToken.delete_all(['updated_at < ?', persistence_token_expire_after.ago])
end

Delayed::Periodic.cron 'ExternalFeedAggregator.process', '*/30 * * * *' do
ExternalFeedAggregator.process
end
Expand Down
4 changes: 4 additions & 0 deletions config/session_store.yml.example
Expand Up @@ -6,6 +6,10 @@ production:
#
# secure: true
#
# change the time that "stay logged in" tokens are valid for, defaults to 1 month
#
# expire_remember_me_after: 2592000
#
# for memcache, the list of servers will be read from config/memcache.yml (or
# default to localhost:11211)
#
Expand Down
15 changes: 15 additions & 0 deletions db/migrate/20111118221746_add_persistence_token_table.rb
@@ -0,0 +1,15 @@
class AddPersistenceTokenTable < ActiveRecord::Migration
def self.up
create_table :session_persistence_tokens do |t|
t.string :token_salt
t.string :crypted_token
t.integer :pseudonym_id, :limit => 8
t.timestamps
end
add_index :session_persistence_tokens, :pseudonym_id
end

def self.down
drop_table :session_persistence_tokens
end
end
143 changes: 143 additions & 0 deletions spec/integration/security_spec.rb
Expand Up @@ -168,6 +168,149 @@
end
end

describe "remember me" do
before do
@u = user_with_pseudonym :active_all => true,
:username => "nobody@example.com",
:password => "asdfasdf"
@u.save!
@p = @u.pseudonym
https!
end

it "should not remember me when the wrong token is given" do
# plain persistence_token no longer works
get "/", {}, "HTTP_COOKIE" => "pseudonym_credentials=#{@p.persistence_token}"
response.should redirect_to("https://www.example.com/login")
token = SessionPersistenceToken.generate(@p)
# correct token id, but nonsense uuid and persistence_token
get "/", {}, "HTTP_COOKIE" => "pseudonym_credentials=#{token.id}::blah::blah"
response.should redirect_to("https://www.example.com/login")
# correct token id and persistence_token, but nonsense uuid
get "/", {}, "HTTP_COOKIE" => "pseudonym_credentials=#{token.id}::#{@p.persistence_token}::blah"
response.should redirect_to("https://www.example.com/login")
end

it "should login via persistence token when no session exists" do
token = SessionPersistenceToken.generate(@p)
get "/", {}, "HTTP_COOKIE" => "pseudonym_credentials=#{token.pseudonym_credentials}"
response.should be_success
cookies['_normandy_session'].should be_present
end

it "should not allow login via the same valid token twice" do
token = SessionPersistenceToken.generate(@p)
get "/", {}, "HTTP_COOKIE" => "pseudonym_credentials=#{token.pseudonym_credentials}"
response.should be_success
SessionPersistenceToken.find_by_id(token.id).should be_nil
reset!
https!
get "/", {}, "HTTP_COOKIE" => "pseudonym_credentials=#{token.pseudonym_credentials}"
response.should redirect_to("https://www.example.com/login")
end

it "should generate a new valid token when a token is used" do
token = SessionPersistenceToken.generate(@p)
get "/", {}, "HTTP_COOKIE" => "pseudonym_credentials=#{token.pseudonym_credentials}"
response.should be_success
s1 = cookies['_normandy_session']
s1.should be_present
cookie = cookies['pseudonym_credentials']
cookie.should be_present
token2 = SessionPersistenceToken.find_by_pseudonym_credentials(CGI.unescape(cookie))
token2.should be_present
token2.should_not == token
token2.pseudonym.should == @p
reset!
https!
# check that the new token is valid too
get "/", {}, "HTTP_COOKIE" => "pseudonym_credentials=#{cookie}"
response.should be_success
s2 = cookies['_normandy_session']
s2.should be_present
s2.should_not == s1
end

it "should generate and return a token when remember_me is checked" do
post "/login", "pseudonym_session[unique_id]" => "nobody@example.com",
"pseudonym_session[password]" => "asdfasdf",
"pseudonym_session[remember_me]" => "1"
assert_response 302
cookie = cookies['pseudonym_credentials']
cookie.should be_present
token = SessionPersistenceToken.find_by_pseudonym_credentials(CGI.unescape(cookie))
token.should be_present
token.pseudonym.should == @p

# verify that the session is now persisting via the session cookie, not
# using and re-generating a one-time-use pseudonym_credentials token on each request
get "/"
cookies['pseudonym_credentials'].should == cookie
end

it "should destroy the token both user agent and server side on logout" do
expect {
post "/login", "pseudonym_session[unique_id]" => "nobody@example.com",
"pseudonym_session[password]" => "asdfasdf",
"pseudonym_session[remember_me]" => "1"
}.to change(SessionPersistenceToken, :count).by(1)
c = cookies['pseudonym_credentials']
c.should be_present

expect {
get "/logout"
}.to change(SessionPersistenceToken, :count).by(-1)
cookies['pseudonym_credentials'].should_not be_present
SessionPersistenceToken.find_by_pseudonym_credentials(CGI.unescape(c)).should be_nil
end

it "should allow multiple remember_me tokens for the same user" do
s1 = open_session
s1.https!
s2 = open_session
s2.https!
s1.post "/login", "pseudonym_session[unique_id]" => "nobody@example.com",
"pseudonym_session[password]" => "asdfasdf",
"pseudonym_session[remember_me]" => "1"
c1 = s1.cookies['pseudonym_credentials']
s2.post "/login", "pseudonym_session[unique_id]" => "nobody@example.com",
"pseudonym_session[password]" => "asdfasdf",
"pseudonym_session[remember_me]" => "1"
c2 = s2.cookies['pseudonym_credentials']
c1.should_not == c2

s3 = open_session
s3.https!
s3.get "/", {}, "HTTP_COOKIE" => "pseudonym_credentials=#{c1}"
s3.response.should be_success
s3.get "/logout"
# make sure c2 can still work
s4 = open_session
s4.https!
s4.get "/", {}, "HTTP_COOKIE" => "pseudonym_credentials=#{c2}"
s4.response.should be_success
end

it "should not login if the pseudonym is deleted" do
token = SessionPersistenceToken.generate(@p)
@p.destroy
get "/", {}, "HTTP_COOKIE" => "pseudonym_credentials=#{token.pseudonym_credentials}"
response.should redirect_to("https://www.example.com/login")
end

it "should not login if the pseudonym.persistence_token gets changed (pw change)" do
token = SessionPersistenceToken.generate(@p)
creds = token.pseudonym_credentials
pers1 = @p.persistence_token
@p.password = @p.password_confirmation = 'newpass'
@p.save!
pers2 = @p.persistence_token
pers1.should_not == pers2
get "/", {}, "HTTP_COOKIE" => "pseudonym_credentials=#{creds}"
response.should redirect_to("https://www.example.com/login")
end
end

if Canvas.redis_enabled?
describe "max login attempts" do
before do
Expand Down

0 comments on commit 4ef50c1

Please sign in to comment.