Project

General

Profile

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
14 14

  
15 15
  before_filter :require_ssl, :require_login
16 16
  before_filter :set_gettext_locale_db, :set_gettext_locale
17
  before_filter :session_expiry, :update_activity_time, :unless => proc {|c| c.remote_user_provided? || c.api_request? } if SETTINGS[:login]
17
  before_filter :session_expiry, :update_activity_time, :unless => proc {|c| !SETTINGS[:login] || c.remote_user_provided? || c.api_request? }
18 18
  before_filter :set_taxonomy, :require_mail, :check_empty_taxonomy
19 19
  before_filter :welcome, :only => :index, :unless => :api_request?
20 20
  before_filter :authorize
......
193 193

  
194 194
  def session_expiry
195 195
    if session[:expires_at].blank? or (session[:expires_at].utc - Time.now.utc).to_i < 0
196
      # Before we expire the session, save the current taxonomies and the originally
197
      # requested URL so they can be restored later.
198
      save_items = session.to_hash.slice("organization_id", "location_id").merge("original_uri" => request.fullpath)
199
      expire_session
200
      session.merge!(save_items)
196
      session[:original_uri] = request.fullpath
197
      backup_session_content { expire_session }
201 198
    end
202 199
  rescue => e
203 200
    logger.warn "failed to determine if user sessions needs to be expired, expiring anyway: #{e}"
204 201
    expire_session
205 202
  end
206 203

  
204
  # Backs up some state from a user's session around a supplied block, which
205
  # will usually expire or reset the session in some way
206
  def backup_session_content
207
    save_items = session.to_hash.slice('organization_id', 'location_id', 'original_uri').symbolize_keys
208
    yield if block_given?
209
    session.merge!(save_items)
210
  end
211

  
207 212
  def update_activity_time
208 213
    session[:expires_at] = Setting[:idle_timeout].minutes.from_now.utc
209 214
  end
app/controllers/concerns/foreman/controller/authentication.rb
15 15

  
16 16
      if user.is_a?(User)
17 17
        logger.info("Authorized user #{user.login}(#{user.to_label})")
18
        User.current = user
19
        session[:user] = User.current.id unless api_request?
20
        return User.current.present?
18
        set_current_user user
21 19
      else
22 20
        if api_request?
23 21
          false
......
28 26

  
29 27
          (redirect_to @available_sso.login_url and return) unless @available_sso.has_rendered
30 28
        end
31

  
32 29
      end
33

  
34 30
    else
35
      # We assume we always have a user logged in, if authentication is disabled, the user is the built-in admin account.
36
      User.current = User.admin
37
      session[:user] = User.current.id unless api_request?
38
      true
31
      # We assume we always have a user logged in
32
      # if authentication is disabled, the user is the built-in admin account
33
      set_current_user User.admin
39 34
    end
40 35
  end
41 36

  
......
55 50
    return false
56 51
  end
57 52

  
53
  private
54

  
58 55
  def sso_authentication
59 56
    if available_sso.present?
60 57
      if available_sso.authenticated?
......
75 72
    user
76 73
  end
77 74

  
78
end
75
  def set_current_user(user)
76
    User.current = user
79 77

  
78
    # API access shouldn't modify the session, its authentication should be
79
    # stateless.  Other successful logins should create new session IDs.
80
    unless api_request?
81
      backup_session_content { reset_session }
82
      session[:user] = user.id
83
      update_activity_time
84
    end
85
    user.present?
86
  end
87

  
88
end
app/controllers/users_controller.rb
62 62
  # Called from the login form.
63 63
  # Stores the user id in the session and redirects required URL or default homepage
64 64
  def login
65
    session[:user] = User.current = nil
66
    session[:locale] = nil
65
    User.current = nil
67 66
    if request.post?
67
      backup_session_content { reset_session }
68 68
      user = User.try_to_login(params[:login]['login'].downcase, params[:login]['password'])
69 69
      if user.nil?
70 70
        #failed to authenticate, and/or to generate the account on the fly
config/routes/test.rb
1 1
if Rails.env.test?
2 2
  Foreman::Application.routes.draw do
3
    resources :testable, :only => :index
4

  
3 5
    namespace :api do
4 6
      resources :testable, :only => :index do
5 7
        get :raise_error, :on => :collection
test/functional/api/base_controller_subclass_test.rb
26 26
    end
27 27
  end
28 28

  
29
  context "API authentication" do
29
  context "API usage when authentication is disabled" do
30 30
    setup do
31 31
      User.current = nil
32
      request.env['HTTP_AUTHORIZATION'] = nil
32 33
      SETTINGS[:login] = false
33 34
    end
34 35

  
......
36 37
      SETTINGS[:login] = true
37 38
    end
38 39

  
39
    it "does not need an username and password when Settings[:login]=false" do
40
    it "does not need a username and password" do
40 41
      get :index
41 42
      assert_response :success
42 43
    end
......
47 48
    end
48 49
  end
49 50

  
51
  context "API usage when authentication is enabled" do
52
    setup do
53
      User.current = nil
54
      request.env['HTTP_AUTHORIZATION'] = nil
55
      SETTINGS[:login] = true
56
    end
57

  
58
    it "requires a username and password" do
59
      @controller.stubs(:available_sso).returns(nil)
60
      get :index
61
      assert_response :unauthorized
62
    end
63

  
64
    context "and SSO (plain) authenticates" do
65
      setup do
66
        @sso = mock('dummy_sso')
67
        @sso.stubs(:authenticated?).returns(true)
68
        @sso.stubs(:user).returns(users(:admin).login)
69
        @controller.stubs(:available_sso).returns(@sso)
70
      end
71

  
72
      it "doesn't escalate privileges in the session" do
73
        get :index
74
        refute session[:user], "session contains user #{session[:user]}"
75
      end
76
    end
77
  end
78

  
50 79
  context 'errors' do
51
   test "top level key is error, no metadata included" do
80
    test "top level key is error, no metadata included" do
52 81
      get :raise_error
53 82
      assert_equal ['error'], ActiveSupport::JSON.decode(@response.body).keys
54 83
    end
test/functional/application_controller_subclass_test.rb
1
require 'test_helper'
2

  
3
class ::TestableController < ::ApplicationController
4
  def index
5
    render :text => 'dummy', :status => 200
6
  end
7
end
8

  
9
class TestableControllerTest < ActionController::TestCase
10
  tests ::TestableController
11

  
12
  context "when authentication is disabled" do
13
    setup do
14
      User.current = nil
15
      SETTINGS[:login] = false
16
    end
17

  
18
    teardown do
19
      SETTINGS[:login] = true
20
    end
21

  
22
    it "does not need a username and password" do
23
      get :index
24
      assert_response :success
25
    end
26
  end
27

  
28
  context "when authentication is enabled" do
29
    setup do
30
      User.current = nil
31
      SETTINGS[:login] = true
32
    end
33

  
34
    it "requires a username and password" do
35
      get :index
36
      assert_response :redirect
37
    end
38

  
39
    it "retains original request URI in session" do
40
      get :index
41
      assert_equal '/testable', session[:original_uri]
42
    end
43

  
44
    context "and SSO authenticates" do
45
      setup do
46
        @sso = mock('dummy_sso')
47
        @sso.stubs(:authenticated?).returns(true)
48
        @sso.stubs(:user).returns(users(:admin).login)
49
        @controller.stubs(:available_sso).returns(@sso)
50
      end
51

  
52
      it "sets the session user" do
53
        get :index
54
        assert_response :success
55
        assert_equal users(:admin).id, session[:user]
56
      end
57

  
58
      it "changes the session ID to prevent fixation" do
59
        @controller.expects(:reset_session)
60
        get :index
61
      end
62

  
63
      it "doesn't escalate privileges in the old session" do
64
        old_session = session
65
        get :index
66
        refute old_session.keys.include?(:user), "old session contains user"
67
        assert session[:user], "new session doesn't contain user"
68
      end
69

  
70
      it "retains taxonomy session attributes in new session" do
71
        get :index, {}, {:location_id => taxonomies(:location1).id,
72
                         :organization_id => taxonomies(:organization1).id,
73
                         :foo => 'bar'}
74
        assert_equal taxonomies(:location1).id, session[:location_id]
75
        assert_equal taxonomies(:organization1).id, session[:organization_id]
76
        refute session[:foo], "session contains 'foo', but should have been reset"
77
      end
78
    end
79
  end
80
end
test/functional/locations_controller_test.rb
76 76
    # session is reset, redirected to login, but org id remains
77 77
    assert_redirected_to "/users/login"
78 78
    assert_match /Your session has expired, please login again/, flash[:warning]
79
    assert_equal session["location_id"], taxonomies(:location1).id
79
    assert_equal session[:location_id], taxonomies(:location1).id
80 80
  end
81 81

  
82 82
  test "should display a warning if current location has been deleted" do
test/functional/organizations_controller_test.rb
73 73
    # session is reset, redirected to login, but org id remains
74 74
    assert_redirected_to "/users/login"
75 75
    assert_match /Your session has expired, please login again/, flash[:warning]
76
    assert_equal session["organization_id"], taxonomies(:organization1).id
76
    assert_equal session[:organization_id], taxonomies(:organization1).id
77 77
  end
78 78

  
79 79
  test "should display a warning if current organization has been deleted" do
test/functional/users_controller_test.rb
258 258
    assert_response :redirect
259 259
  end
260 260

  
261
  test "#login sets the session user" do
262
    post :login, {:login => {'login' => users(:admin).login, 'password' => 'secret'}}
263
    assert_redirected_to hosts_path
264
    assert_equal users(:admin).id, session[:user]
265
  end
266

  
267
  test "#login resets the session ID to prevent fixation" do
268
    @controller.expects(:reset_session)
269
    post :login, {:login => {'login' => users(:admin).login, 'password' => 'secret'}}
270
  end
271

  
272
  test "#login doesn't escalate privileges in the old session" do
273
    old_session = session
274
    post :login, {:login => {'login' => users(:admin).login, 'password' => 'secret'}}
275
    refute old_session.keys.include?(:user), "old session contains user"
276
    assert session[:user], "new session doesn't contain user"
277
  end
278

  
279
  test "#login retains taxonomy session attributes in new session" do
280
    post :login, {:login => {'login' => users(:admin).login, 'password' => 'secret'}},
281
                 {:location_id => taxonomies(:location1).id,
282
                  :organization_id => taxonomies(:organization1).id,
283
                  :foo => 'bar'}
284
    assert_equal taxonomies(:location1).id, session[:location_id]
285
    assert_equal taxonomies(:organization1).id, session[:organization_id]
286
    refute session[:foo], "session contains 'foo', but should have been reset"
287
  end
288

  
261 289
end
test/lib/foreman/access_permissions_test.rb
26 26
    # Apipie
27 27
    "apipie/apipies/index", "apipie/apipies/apipie_checksum",
28 28

  
29
    # API app controller stub
30
    "api/testable/index", "api/testable/raise_error"
29
    # App controller stubs
30
    "testable/index", "api/testable/index", "api/testable/raise_error"
31 31
  ]
32 32

  
33 33
  MAY_SKIP_AUTHORIZED = [ "about/index" ]
test/test_helper.rb
235 235

  
236 236
    def set_api_user
237 237
      return unless self.class.to_s[/api/i]
238
      @request.env['HTTP_AUTHORIZATION'] = ActionController::HttpAuthentication::Basic.encode_credentials(users(:apiadmin).login, "secret")
238
      set_basic_auth(users(:apiadmin), "secret")
239
    end
240

  
241
    def set_basic_auth(user, password)
242
      login = user.is_a?(User) ? user.login : user
243
      @request.env['HTTP_AUTHORIZATION'] = ActionController::HttpAuthentication::Basic.encode_credentials(login, password)
239 244
    end
240 245
  end
241 246

  
242
-