[37553] Make user run through authentication stages after header SSO (#9458)

https://community.openproject.org/wp/37553

app/controllers/account_controller.rb

@@ -32,8 +32,6 @@ class AccountController < ApplicationController
   include CustomFieldsHelper
   include OmniauthHelper
   include Accounts::OmniauthLogin
-  include Accounts::RedirectAfterLogin
-  include Accounts::AuthenticationStages
   include Accounts::UserConsent
   include Accounts::UserLimits
   include Accounts::UserPasswordChange
@@ -440,54 +438,6 @@ class AccountController < ApplicationController
     end
   end
 
-  def successful_authentication(user, reset_stages: true, just_registered: false)
-    stages = authentication_stages after_activation: just_registered, reset: reset_stages
-
-    if stages.empty?
-      # setting params back_url to be used by redirect_after_login
-      params[:back_url] = session.delete :back_url if session.include?(:back_url)
-
-      if just_registered || session[:just_registered]
-        finish_registration! user
-      else
-        login_user! user
-      end
-    else
-      stage = stages.first
-
-      session[:just_registered] = just_registered
-      session[:authenticated_user_id] = user.id
-
-      redirect_to stage.path
-    end
-  end
-
-  def login_user!(user)
-    # generate a key and set cookie if autologin
-    if Setting.autologin? && (params[:autologin] || session.delete(:autologin_requested))
-      set_autologin_cookie(user)
-    end
-
-    # Set the logged user, resetting their session
-    self.logged_user = user
-
-    call_hook(:controller_account_success_authentication_after, user: user)
-
-    redirect_after_login(user)
-  end
-
-  def set_autologin_cookie(user)
-    token = Token::AutoLogin.create(user: user)
-    cookie_options = {
-      value: token.plain_value,
-      expires: 1.year.from_now,
-      path: OpenProject::Configuration['autologin_cookie_path'],
-      secure: OpenProject::Configuration['autologin_cookie_secure'],
-      httponly: true
-    }
-    cookies[OpenProject::Configuration['autologin_cookie_name']] = cookie_options
-  end
-
   def login_user_if_active(user, just_registered:)
     if user.active?
       successful_authentication(user, just_registered: just_registered)
@@ -518,15 +468,6 @@ class AccountController < ApplicationController
     render action: 'register'
   end
 
-  def finish_registration!(user)
-    session[:just_registered] = nil
-    self.logged_user = user
-    user.update last_login_on: Time.now
-
-    flash[:notice] = I18n.t(:notice_account_registered_and_logged_in)
-    redirect_after_login user
-  end
-
   def self_registration_disabled
     flash[:error] = I18n.t('account.error_self_registration_disabled')
     redirect_to signin_url

app/controllers/application_controller.rb

@@ -47,6 +47,7 @@ class ApplicationController < ActionController::Base
   include HookHelper
   include ErrorsHelper
   include Accounts::CurrentUser
+  include Accounts::UserLogin
   include ::OpenProject::Authentication::SessionExpiry
   include AdditionalUrlHelpers
   include OpenProjectErrorHelper

app/controllers/concerns/accounts/authentication_stages.rb

@@ -1,4 +1,26 @@
 module Accounts::AuthenticationStages
+  def successful_authentication(user, reset_stages: true, just_registered: false)
+    stages = authentication_stages after_activation: just_registered, reset: reset_stages
+
+    if stages.empty?
+      # setting params back_url to be used by redirect_after_login
+      params[:back_url] = session.delete :back_url if session.include?(:back_url)
+
+      if just_registered || session[:just_registered]
+        finish_registration! user
+      else
+        login_user! user
+      end
+    else
+      stage = stages.first
+
+      session[:just_registered] = just_registered
+      session[:authenticated_user_id] = user.id
+
+      redirect_to stage.path
+    end
+  end
+
   def stage_success
     stage = session[:authentication_stages]&.first
 
@@ -31,6 +53,15 @@ module Accounts::AuthenticationStages
 
   private
 
+  def finish_registration!(user)
+    session[:just_registered] = nil
+    self.logged_user = user
+    user.update last_login_on: Time.now
+
+    flash[:notice] = I18n.t(:notice_account_registered_and_logged_in)
+    redirect_after_login user
+  end
+
   def authentication_stages(after_activation: false, reset: true)
     if OpenProject::Authentication::Stage.stages.select(&:active?).any?
       session.delete %i[authentication_stages stage_secrets back_url] if reset

app/controllers/concerns/accounts/user_login.rb

@@ -0,0 +1,30 @@
+module Accounts::UserLogin
+  include ::Accounts::AuthenticationStages
+  include ::Accounts::RedirectAfterLogin
+
+  def login_user!(user)
+    # generate a key and set cookie if autologin
+    if Setting.autologin? && (params[:autologin] || session.delete(:autologin_requested))
+      set_autologin_cookie(user)
+    end
+
+    # Set the logged user, resetting their session
+    self.logged_user = user
+
+    call_hook(:controller_account_success_authentication_after, user: user)
+
+    redirect_after_login(user)
+  end
+
+  def set_autologin_cookie(user)
+    token = Token::AutoLogin.create(user: user)
+    cookie_options = {
+      value: token.plain_value,
+      expires: 1.year.from_now,
+      path: OpenProject::Configuration['autologin_cookie_path'],
+      secure: OpenProject::Configuration['autologin_cookie_secure'],
+      httponly: true
+    }
+    cookies[OpenProject::Configuration['autologin_cookie_name']] = cookie_options
+  end
+end

app/controllers/concerns/auth_source_sso.rb

@@ -110,7 +110,7 @@ module AuthSourceSSO
   end
 
   def sso_in_progress!
-    sso_failure_in_progress! || session[:auth_source_registration]
+    sso_failure_in_progress! || session[:auth_source_registration] || session[:authenticated_user_id]
   end
 
   def sso_failure_in_progress!
@@ -136,23 +136,22 @@ module AuthSourceSSO
       handle_sso_failure!({ user: user, login: login })
     else # valid user
       # If a user is invited, ensure it gets activated
+      activated = user.invited?
       activate_user_if_invited! user
 
-      handle_sso_success user
+      handle_sso_success user, activated
     end
   end
 
-  def handle_sso_success(user)
-    login_user(user)
+  def handle_sso_success(user, just_activated)
     session[:user_from_auth_header] = true
-
-    user
+    successful_authentication(user, reset_stages: true, just_registered: just_activated)
   end
 
   def activate_user_if_invited!(user)
     return unless user.invited?
 
-    user.activate!
+    user.active!
   end
 
   def perform_post_logout(prev_session, previous_user)

spec/controllers/concerns/auth_source_sso_spec.rb

@@ -28,9 +28,6 @@
 
 require 'spec_helper'
 
-##
-# The concern is implemented in the ApplicationController and is therewore applicable
-# in every controller. We're just using this particular one for the test.
 describe MyController, type: :controller do
   render_views
 
@@ -98,15 +95,17 @@ describe MyController, type: :controller do
     end
 
     it "should log in given user" do
-      expect(response.body.squish).to have_content("Username   h.wurst")
+      expect(response).to redirect_to my_page_path
+      expect(session[:user_id]).to eq user.id
     end
 
     context 'when the secret being null' do
       let(:secret) { nil }
 
       it "should log in given user" do
-        expect(response.body.squish).to have_content("Username   h.wurst")
+        expect(response).to redirect_to my_page_path
         expect(user.reload.last_login_on).to be_within(10.seconds).of(Time.now)
+        expect(session[:user_id]).to eq user.id
       end
     end
 
@@ -116,8 +115,9 @@ describe MyController, type: :controller do
       end
 
       it "should log in given user and activate it" do
-        expect(response.body.squish).to have_content("Username   h.wurst")
+        expect(response).to redirect_to my_page_path
         expect(user.reload).to be_active
+        expect(session[:user_id]).to eq user.id
       end
     end