Fixes #17487 - support sessions for api calls

app/controllers/api/base_controller.rb

@@ -371,7 +371,7 @@ def prioritize_friendly_name_records(base_scope, friendly_field_query)
     end
 
     def protect_api_from_forgery?
-      session[:user].present?
+      session[:user].present? && !session[:api_authenticated_session]
     end
 
     def parameter_filter_context

app/controllers/concerns/foreman/controller/authentication.rb

@@ -80,9 +80,15 @@ def sso_authentication
   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?
+    # API access resets the whole session and marks the session as initialized from API
+    # such sessions aren't checked for CSRF
+    # UI access resets only session ID
+    if api_request?
+      reset_session
+      session[:user] = user.id if SETTINGS[:login]
+      session[:api_authenticated_session] = true
+      set_activity_time
+    else
       backup_session_content { reset_session }
       session[:user] = user.id
       update_activity_time

app/controllers/concerns/foreman/controller/session.rb

@@ -4,7 +4,7 @@ module Foreman::Controller::Session
   def session_expiry
     return if ignore_api_request?
     if session[:expires_at].blank? || (Time.at(session[:expires_at]).utc - Time.now.utc).to_i < 0
-      session[:original_uri] = request.fullpath
+      session[:original_uri] = request.fullpath unless session[:api_authenticated_session]
       expire_session
     end
   rescue => e
@@ -22,6 +22,10 @@ def backup_session_content
 
   def update_activity_time
     return if ignore_api_request?
+    set_activity_time
+  end
+
+  def set_activity_time
     session[:expires_at] = Setting[:idle_timeout].minutes.from_now.to_i
   end
 

test/controllers/api/base_controller_subclass_test.rb

@@ -51,29 +51,43 @@ class Api::TestableControllerTest < ActionController::TestCase
   end
 
   context "API session expiration" do
-    test "request succeeds if no session[:expires_at] is included" do
-      # this would be typical of API initiated directly or from cli
-      get :index
-      assert_response :success
-    end
+    context "with credentials being sent" do
+      test "request succeeds if there's no existing session" do
+        # this would be typical API call initiated directly or from cli
+        get :index
+        assert_response :success
+      end
 
-    test "request fails if session expired" do
-      # this would be typical of API initiated from a web ui session
-      get :index, {}, { :expires_at => 5.days.ago.utc }
-      assert_response :unauthorized
+      test "request fails even if the session expired" do
+        # this would be typical API call  initiated directly or from cli
+        get :index, {}, { :expires_at => 5.days.ago.utc, :user => users(:apiadmin).id }
+        assert_response :unauthorized
+      end
     end
 
-    test "request succeeds if session has not expired" do
-      # this would be typical of API initiated from a web ui session
-      get :index, {}, { :expires_at => 5.days.from_now.utc }
-      assert_response :success
+    context "without credentials being sent" do
+      setup do
+        reset_api_credentials
+      end
+
+      test "request fails if the session expired" do
+        # this would be typical API call  initiated from a web ui session
+        get :index, {}, { :expires_at => 5.days.ago.utc, :user => users(:apiadmin).id }
+        assert_response :unauthorized
+      end
+
+      test "request succeeds if the session has not expired" do
+        # this would be typical API call  initiated from a web ui session
+        get :index, {}, { :expires_at => 5.days.from_now.utc, :user => users(:apiadmin).id }
+        assert_response :success
+      end
     end
   end
 
   context "API usage when authentication is disabled" do
     setup do
       User.current = nil
-      request.env['HTTP_AUTHORIZATION'] = nil
+      reset_api_credentials
       SETTINGS[:login] = false
     end
 
@@ -86,11 +100,16 @@ class Api::TestableControllerTest < ActionController::TestCase
       assert_response :success
     end
 
-    it "does not set session data for API requests" do
+    it "does not set user session data for API requests" do
       get :index
       assert_not session[:user]
     end
 
+    it "sets api_sesion flag for API requests" do
+      get :index
+      assert session[:api_authenticated_session]
+    end
+
     it "uses an accessible admin user" do
       User.unscoped.except_hidden.only_admin.where('login <> ?', users(:apiadmin).login).destroy_all
       @controller.expects(:set_current_user).with(responds_with(:login, users(:apiadmin).login)).returns(true)
@@ -101,7 +120,7 @@ class Api::TestableControllerTest < ActionController::TestCase
   context "API usage when authentication is enabled" do
     setup do
       User.current = nil
-      request.env['HTTP_AUTHORIZATION'] = nil
+      reset_api_credentials
       SETTINGS[:login] = true
     end
 
@@ -129,9 +148,9 @@ class Api::TestableControllerTest < ActionController::TestCase
         get :index
       end
 
-      it "doesn't escalate privileges in the session" do
+      it "saves user id into the session" do
         get :index
-        refute session[:user], "session contains user #{session[:user]}"
+        assert_equal users(:admin).id, session[:user]
       end
     end
   end
@@ -148,7 +167,7 @@ class Api::TestableControllerTest < ActionController::TestCase
       ActionController::Base.allow_forgery_protection = true
       SETTINGS[:login] = true
       User.current = nil
-      request.env['HTTP_AUTHORIZATION'] = nil
+      reset_api_credentials
     end
 
     teardown do
@@ -161,6 +180,12 @@ class Api::TestableControllerTest < ActionController::TestCase
       assert_response :unauthorized
     end
 
+    it "permits access without CSRF token when the session was authenticated via api" do
+      request.headers['X-CSRF-Token'] = nil
+      post :index, {}, set_session_user.merge(:api_authenticated_session => true)
+      assert_response :success
+    end
+
     it "works with a CSRF token when there is a session user" do
       token = @controller.send(:form_authenticity_token)
       request.headers['X-CSRF-Token'] = token

test/controllers/api/v2/base_controller_subclass_test.rb

@@ -28,4 +28,26 @@ def setup
       assert_response 200
     end
   end
+
+  context "when authentication is enabled" do
+    setup do
+      User.current = nil
+      SETTINGS[:login] = true
+
+      @sso = mock('dummy_sso')
+      @sso.stubs(:authenticated?).returns(true)
+      @sso.stubs(:current_user).returns(users(:admin))
+      @sso.stubs(:support_expiration?).returns(true)
+      @sso.stubs(:expiration_url).returns("/users/extlogin")
+      @sso.stubs(:controller).returns(@controller)
+      @controller.instance_variable_set(:@available_sso, @sso)
+      @controller.stubs(:get_sso_method).returns(@sso)
+    end
+
+    it "sets the session user" do
+      get :index
+      assert_response :success
+      assert_equal users(:admin).id, session[:user]
+    end
+  end
 end

test/test_helper.rb

@@ -59,6 +59,10 @@ def set_api_user
     set_basic_auth(users(:apiadmin), "secret")
   end
 
+  def reset_api_credentials
+    @request.env.delete('HTTP_AUTHORIZATION')
+  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)