Project

General

Profile

Download (14.9 KB)

Bug #4457 » 0001-fixes-4457-Session-fixation-new-session-IDs-are-not-.patch

v4 patch - Dominic Cleal, 03/18/2014 03:52 PM

View differences:

app/controllers/application_controller.rb
before_filter :require_ssl, :require_login
before_filter :set_gettext_locale_db, :set_gettext_locale
before_filter :session_expiry, :update_activity_time, :unless => proc {|c| c.remote_user_provided? || c.api_request? } if SETTINGS[:login]
before_filter :session_expiry, :update_activity_time, :unless => proc {|c| !SETTINGS[:login] || c.remote_user_provided? || c.api_request? }
before_filter :set_taxonomy, :require_mail, :check_empty_taxonomy
before_filter :welcome, :only => :index, :unless => :api_request?
before_filter :authorize
......
def session_expiry
if session[:expires_at].blank? or (session[:expires_at].utc - Time.now.utc).to_i < 0
# Before we expire the session, save the current taxonomies and the originally
# requested URL so they can be restored later.
save_items = session.to_hash.slice("organization_id", "location_id").merge("original_uri" => request.fullpath)
expire_session
session.merge!(save_items)
session[:original_uri] = request.fullpath
backup_session_content { expire_session }
end
rescue => e
logger.warn "failed to determine if user sessions needs to be expired, expiring anyway: #{e}"
expire_session
end
# Backs up some state from a user's session around a supplied block, which
# will usually expire or reset the session in some way
def backup_session_content
save_items = session.to_hash.slice('organization_id', 'location_id', 'original_uri').symbolize_keys
yield if block_given?
session.merge!(save_items)
end
def update_activity_time
session[:expires_at] = Setting[:idle_timeout].minutes.from_now.utc
end
app/controllers/concerns/foreman/controller/authentication.rb
if user.is_a?(User)
logger.info("Authorized user #{user.login}(#{user.to_label})")
User.current = user
session[:user] = User.current.id unless api_request?
return User.current.present?
set_current_user user
else
if api_request?
false
......
(redirect_to @available_sso.login_url and return) unless @available_sso.has_rendered
end
end
else
# We assume we always have a user logged in, if authentication is disabled, the user is the built-in admin account.
User.current = User.admin
session[:user] = User.current.id unless api_request?
true
# We assume we always have a user logged in
# if authentication is disabled, the user is the built-in admin account
set_current_user User.admin
end
end
......
return false
end
private
def sso_authentication
if available_sso.present?
if available_sso.authenticated?
......
user
end
end
def set_current_user(user)
User.current = user
# API access shouldn't modify the session, its authentication should be
# stateless. Other successful logins should create new session IDs.
unless api_request?
backup_session_content { reset_session }
session[:user] = user.id
update_activity_time
end
user.present?
end
end
app/controllers/users_controller.rb
# Called from the login form.
# Stores the user id in the session and redirects required URL or default homepage
def login
session[:user] = User.current = nil
session[:locale] = nil
User.current = nil
if request.post?
backup_session_content { reset_session }
user = User.try_to_login(params[:login]['login'].downcase, params[:login]['password'])
if user.nil?
#failed to authenticate, and/or to generate the account on the fly
config/routes/test.rb
if Rails.env.test?
Foreman::Application.routes.draw do
resources :testable, :only => :index
namespace :api do
resources :testable, :only => :index do
get :raise_error, :on => :collection
test/functional/api/base_controller_subclass_test.rb
end
end
context "API authentication" do
context "API usage when authentication is disabled" do
setup do
User.current = nil
request.env['HTTP_AUTHORIZATION'] = nil
SETTINGS[:login] = false
end
......
SETTINGS[:login] = true
end
it "does not need an username and password when Settings[:login]=false" do
it "does not need a username and password" do
get :index
assert_response :success
end
......
end
end
context "API usage when authentication is enabled" do
setup do
User.current = nil
request.env['HTTP_AUTHORIZATION'] = nil
SETTINGS[:login] = true
end
it "requires a username and password" do
@controller.stubs(:available_sso).returns(nil)
get :index
assert_response :unauthorized
end
context "and SSO (plain) authenticates" do
setup do
@sso = mock('dummy_sso')
@sso.stubs(:authenticated?).returns(true)
@sso.stubs(:user).returns(users(:admin).login)
@controller.stubs(:available_sso).returns(@sso)
end
it "doesn't escalate privileges in the session" do
get :index
refute session[:user], "session contains user #{session[:user]}"
end
end
end
context 'errors' do
test "top level key is error, no metadata included" do
test "top level key is error, no metadata included" do
get :raise_error
assert_equal ['error'], ActiveSupport::JSON.decode(@response.body).keys
end
test/functional/application_controller_subclass_test.rb
require 'test_helper'
class ::TestableController < ::ApplicationController
def index
render :text => 'dummy', :status => 200
end
end
class TestableControllerTest < ActionController::TestCase
tests ::TestableController
context "when authentication is disabled" do
setup do
User.current = nil
SETTINGS[:login] = false
end
teardown do
SETTINGS[:login] = true
end
it "does not need a username and password" do
get :index
assert_response :success
end
end
context "when authentication is enabled" do
setup do
User.current = nil
SETTINGS[:login] = true
end
it "requires a username and password" do
get :index
assert_response :redirect
end
it "retains original request URI in session" do
get :index
assert_equal '/testable', session[:original_uri]
end
context "and SSO authenticates" do
setup do
@sso = mock('dummy_sso')
@sso.stubs(:authenticated?).returns(true)
@sso.stubs(:user).returns(users(:admin).login)
@controller.stubs(:available_sso).returns(@sso)
end
it "sets the session user" do
get :index
assert_response :success
assert_equal users(:admin).id, session[:user]
end
it "changes the session ID to prevent fixation" do
@controller.expects(:reset_session)
get :index
end
it "doesn't escalate privileges in the old session" do
old_session = session
get :index
refute old_session.keys.include?(:user), "old session contains user"
assert session[:user], "new session doesn't contain user"
end
it "retains taxonomy session attributes in new session" do
get :index, {}, {:location_id => taxonomies(:location1).id,
:organization_id => taxonomies(:organization1).id,
:foo => 'bar'}
assert_equal taxonomies(:location1).id, session[:location_id]
assert_equal taxonomies(:organization1).id, session[:organization_id]
refute session[:foo], "session contains 'foo', but should have been reset"
end
end
end
end
test/functional/locations_controller_test.rb
# session is reset, redirected to login, but org id remains
assert_redirected_to "/users/login"
assert_match /Your session has expired, please login again/, flash[:warning]
assert_equal session["location_id"], taxonomies(:location1).id
assert_equal session[:location_id], taxonomies(:location1).id
end
test "should display a warning if current location has been deleted" do
test/functional/organizations_controller_test.rb
# session is reset, redirected to login, but org id remains
assert_redirected_to "/users/login"
assert_match /Your session has expired, please login again/, flash[:warning]
assert_equal session["organization_id"], taxonomies(:organization1).id
assert_equal session[:organization_id], taxonomies(:organization1).id
end
test "should display a warning if current organization has been deleted" do
test/functional/users_controller_test.rb
assert_response :redirect
end
test "#login sets the session user" do
post :login, {:login => {'login' => users(:admin).login, 'password' => 'secret'}}
assert_redirected_to hosts_path
assert_equal users(:admin).id, session[:user]
end
test "#login resets the session ID to prevent fixation" do
@controller.expects(:reset_session)
post :login, {:login => {'login' => users(:admin).login, 'password' => 'secret'}}
end
test "#login doesn't escalate privileges in the old session" do
old_session = session
post :login, {:login => {'login' => users(:admin).login, 'password' => 'secret'}}
refute old_session.keys.include?(:user), "old session contains user"
assert session[:user], "new session doesn't contain user"
end
test "#login retains taxonomy session attributes in new session" do
post :login, {:login => {'login' => users(:admin).login, 'password' => 'secret'}},
{:location_id => taxonomies(:location1).id,
:organization_id => taxonomies(:organization1).id,
:foo => 'bar'}
assert_equal taxonomies(:location1).id, session[:location_id]
assert_equal taxonomies(:organization1).id, session[:organization_id]
refute session[:foo], "session contains 'foo', but should have been reset"
end
end
test/lib/foreman/access_permissions_test.rb
# Apipie
"apipie/apipies/index", "apipie/apipies/apipie_checksum",
# API app controller stub
"api/testable/index", "api/testable/raise_error"
# App controller stubs
"testable/index", "api/testable/index", "api/testable/raise_error"
]
MAY_SKIP_AUTHORIZED = [ "about/index" ]
test/test_helper.rb
def set_api_user
return unless self.class.to_s[/api/i]
@request.env['HTTP_AUTHORIZATION'] = ActionController::HttpAuthentication::Basic.encode_credentials(users(:apiadmin).login, "secret")
set_basic_auth(users(:apiadmin), "secret")
end
def set_basic_auth(user, password)
login = user.is_a?(User) ? user.login : user
@request.env['HTTP_AUTHORIZATION'] = ActionController::HttpAuthentication::Basic.encode_credentials(login, password)
end
end
(4-4/4)