From 0b535b0035087e91648f561486a1e85b5b4d5b0a Mon Sep 17 00:00:00 2001
From: Gerardo Navarro <gerardo@b310.de>
Date: Thu, 13 Nov 2025 16:30:58 +0000
Subject: [PATCH] Step-up auth: Fix provider-specific session validation

Fixes step-up authentication to correctly validate sessions against
the specific OAuth provider required by a namespace, rather than
accepting any provider's successful authentication. This ensures
proper access control when multiple OAuth providers are configured.

Previously, the system would accept step-up authentication from any
configured provider, regardless of which provider the namespace
actually required. This could allow unintended access if a user had
authenticated with a different provider than the one specified in
the namespace settings.

Test coverage includes scenarios for:
- Multiple OAuth providers with different step-up configurations
- Validation that only the correct provider grants access
- Rejection of authentication from non-matching providers
- Proper handling of providers without step-up auth capability

This strengthens the security boundary for protected namespaces by
ensuring authentication requirements are strictly enforced per the
configured provider.

Changelog: other
---
 ...es_step_up_authentication_for_namespace.rb | 18 +++--
 .../enforce_step_up_auth_shared_examples.rb   | 71 +++++++++++++++++++
 2 files changed, 83 insertions(+), 6 deletions(-)

diff --git a/app/controllers/concerns/enforces_step_up_authentication_for_namespace.rb b/app/controllers/concerns/enforces_step_up_authentication_for_namespace.rb
index 101a2b685ba4f4..ebcf99b18f01a5 100644
--- a/app/controllers/concerns/enforces_step_up_authentication_for_namespace.rb
+++ b/app/controllers/concerns/enforces_step_up_authentication_for_namespace.rb
@@ -44,7 +44,7 @@ def enforce_step_up_auth_for(namespace)
 
     return unless step_up_auth_supported_for_namespace?(namespace)
     return unless step_up_auth_required_for_namespace?(namespace)
-    return if step_up_auth_succeeded_for_namespace?
+    return if step_up_auth_succeeded_for_namespace?(namespace)
 
     handle_failed_step_up_auth_for_namespace(namespace)
   end
@@ -66,11 +66,17 @@ def step_up_auth_required_for_namespace?(namespace)
     )
   end
 
-  def step_up_auth_succeeded_for_namespace?
-    ::Gitlab::Auth::Oidc::StepUpAuthentication.succeeded?(
-      session,
-      scope: ::Gitlab::Auth::Oidc::StepUpAuthentication::SCOPE_NAMESPACE
-    )
+  def step_up_auth_succeeded_for_namespace?(namespace)
+    provider_name = namespace.step_up_auth_required_oauth_provider_from_self_or_inherited
+    return false unless provider_name
+
+    ::Gitlab::Auth::Oidc::StepUpAuthentication
+      .build_flow(
+        provider: provider_name,
+        session: session,
+        scope: ::Gitlab::Auth::Oidc::StepUpAuthentication::SCOPE_NAMESPACE
+      )
+      .succeeded?
   end
 
   def handle_failed_step_up_auth_for_namespace(namespace)
diff --git a/spec/support/shared_examples/enforce_step_up_auth_shared_examples.rb b/spec/support/shared_examples/enforce_step_up_auth_shared_examples.rb
index 7176ea50982fc2..b5cbbe3e235d5e 100644
--- a/spec/support/shared_examples/enforce_step_up_auth_shared_examples.rb
+++ b/spec/support/shared_examples/enforce_step_up_auth_shared_examples.rb
@@ -162,6 +162,77 @@
       end
     end
   end
+
+  context 'with multiple OAuth providers configured' do
+    let(:provider_without_step_up) do
+      GitlabSettings::Options.new(name: 'github')
+    end
+
+    let(:another_step_up_provider) do
+      GitlabSettings::Options.new(
+        name: 'saml',
+        step_up_auth: {
+          namespace: {
+            id_token: {
+              required: { acr: 'silver' }
+            }
+          }
+        }
+      )
+    end
+
+    before do
+      # Configure multiple providers
+      stub_omniauth_setting(
+        enabled: true,
+        providers: [oidc_provider_config, provider_without_step_up, another_step_up_provider]
+      )
+      allow(Devise).to receive(:omniauth_providers).and_return(
+        [oidc_provider_name, provider_without_step_up.name, another_step_up_provider.name]
+      )
+    end
+
+    context 'when group requires specific step-up auth provider' do
+      before do
+        group.namespace_settings.update!(step_up_auth_required_oauth_provider: oidc_provider_name)
+      end
+
+      context 'when user has succeeded with a different step-up provider' do
+        let(:session_data) do
+          {
+            'omniauth_step_up_auth' => {
+              another_step_up_provider.name => { 'namespace' => { 'state' => 'succeeded' } }
+            }
+          }
+        end
+
+        it 'still requires step-up auth with the configured provider' do
+          subject
+
+          # User has step-up auth succeeded with 'saml' but group requires 'openid_connect'
+          expect(response).to redirect_to(new_group_step_up_auth_path(group))
+          expect(response).to have_gitlab_http_status(:found)
+        end
+      end
+
+      context 'when user has succeeded with the correct provider' do
+        let(:session_data) do
+          {
+            'omniauth_step_up_auth' => {
+              oidc_provider_name => { 'namespace' => { 'state' => 'succeeded' } }
+            }
+          }
+        end
+
+        it 'allows access with matching provider session' do
+          subject
+
+          expect(response).to have_gitlab_http_status(expected_success_status)
+          expect(response).not_to redirect_to(new_group_step_up_auth_path(group))
+        end
+      end
+    end
+  end
 end
 
 RSpec.shared_examples 'enforces step-up authentication (request spec)' do
-- 
GitLab