diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index fc06c348ad8e4efd9013ac99c2297c3d2ba0610d..95024a07576893a9bfbe7002e30b165aad6f99d4 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -426,10 +426,21 @@ def store_redirect_fragment(redirect_fragment) end def handle_step_up_auth - return if Feature.disabled?(:omniauth_step_up_auth_for_admin_mode, current_user) + handle_step_up_auth_for_admin_mode if Feature.enabled?(:omniauth_step_up_auth_for_admin_mode, current_user) + handle_step_up_auth_for_namespace if Feature.enabled?(:omniauth_step_up_auth_for_namespace, current_user) + end + + def handle_step_up_auth_for_admin_mode + handle_step_up_auth_for(::Gitlab::Auth::Oidc::StepUpAuthentication::SCOPE_ADMIN_MODE) + end + + def handle_step_up_auth_for_namespace + handle_step_up_auth_for(::Gitlab::Auth::Oidc::StepUpAuthentication::SCOPE_NAMESPACE) + end + def handle_step_up_auth_for(scope) step_up_auth_flow = - ::Gitlab::Auth::Oidc::StepUpAuthentication.build_flow(session: session, provider: oauth.provider) + ::Gitlab::Auth::Oidc::StepUpAuthentication.build_flow(session: session, provider: oauth.provider, scope: scope) return unless step_up_auth_flow.enabled_by_config? return unless step_up_auth_flow.requested? diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb index 68785e97e38970fb7833d827809b86a55c1846b7..6f085735dc83fe02070a622c3e2b18e512c99062 100644 --- a/app/helpers/groups_helper.rb +++ b/app/helpers/groups_helper.rb @@ -354,7 +354,7 @@ def localized_jobs_to_be_done_choices def available_step_up_auth_providers_for_namespace Gitlab::Auth::Oidc::StepUpAuthentication.enabled_providers( - scope: Gitlab::Auth::Oidc::StepUpAuthentication::STEP_UP_AUTH_SCOPE_NAMESPACE + scope: Gitlab::Auth::Oidc::StepUpAuthentication::SCOPE_NAMESPACE ) end end diff --git a/app/models/namespace_setting.rb b/app/models/namespace_setting.rb index eee4a1ca0a6f4f1f1a3cb28dd93336694955bba2..91b4879553520a5bbcd51afba0f3aa5a98dd49b9 100644 --- a/app/models/namespace_setting.rb +++ b/app/models/namespace_setting.rb @@ -61,7 +61,7 @@ class NamespaceSetting < ApplicationRecord validates :step_up_auth_required_oauth_provider, presence: true, allow_nil: true validates :step_up_auth_required_oauth_provider, inclusion: { in: ->(_) { Gitlab::Auth::Oidc::StepUpAuthentication - .enabled_providers(scope: Gitlab::Auth::Oidc::StepUpAuthentication::STEP_UP_AUTH_SCOPE_NAMESPACE) + .enabled_providers(scope: Gitlab::Auth::Oidc::StepUpAuthentication::SCOPE_NAMESPACE) .map(&:to_s) } }, allow_nil: true validates :duo_agent_platform_request_count, numericality: { greater_than_or_equal_to: 0 } diff --git a/lib/gitlab/auth/oidc/step_up_auth_before_request_phase.rb b/lib/gitlab/auth/oidc/step_up_auth_before_request_phase.rb index 2b75a371fc241f701b6761df26673309f83e678b..d28f4fea2cd8ae9f294b67f64abb90c2b3957148 100644 --- a/lib/gitlab/auth/oidc/step_up_auth_before_request_phase.rb +++ b/lib/gitlab/auth/oidc/step_up_auth_before_request_phase.rb @@ -10,18 +10,31 @@ module Oidc module StepUpAuthBeforeRequestPhase class << self def call(env) - return if current_user_from(env).blank? - return if Feature.disabled?(:omniauth_step_up_auth_for_admin_mode, current_user_from(env)) + requested_scope = request_param_step_up_auth_scope_from(env).to_sym + current_user = current_user_from(env) - # If the step-up authentication scope is not included in the request params, - # then step-up authentication is likely not requested and we do not need to proceed. - return unless step_up_auth_requested_for_admin_mode?(env) + return if current_user.blank? + return if requested_scope.blank? + + return if requested_scope_admin_mode?(requested_scope) && + Feature.disabled?(:omniauth_step_up_auth_for_admin_mode, current_user) + + return if requested_scope_namespace?(requested_scope) && + Feature.disabled?(:omniauth_step_up_auth_for_namespace, current_user) session = session_from(env) provider = current_provider_from(env) + step_up_auth_flow = - ::Gitlab::Auth::Oidc::StepUpAuthentication.build_flow(session: session, provider: provider) + ::Gitlab::Auth::Oidc::StepUpAuthentication.build_flow( + session: session, + provider: provider, + scope: requested_scope + ) + # TODO: Integrate the state uuid in the step-up auth session. + # Why? At the moment, there is a small security vulnerability + # where simultaneous authentication requests could lead to privilege escalation, https://gitlab.com/gitlab-org/gitlab/-/issues/555349. return unless step_up_auth_flow.enabled_by_config? # This method will set the state to 'requested' in the session @@ -30,9 +43,12 @@ def call(env) private - def step_up_auth_requested_for_admin_mode?(env) - request_param_step_up_auth_scope_from(env) == - ::Gitlab::Auth::Oidc::StepUpAuthentication::STEP_UP_AUTH_SCOPE_ADMIN_MODE.to_s + def requested_scope_admin_mode?(scope) + scope == ::Gitlab::Auth::Oidc::StepUpAuthentication::SCOPE_ADMIN_MODE + end + + def requested_scope_namespace?(scope) + scope == ::Gitlab::Auth::Oidc::StepUpAuthentication::SCOPE_NAMESPACE end def current_user_from(env) diff --git a/lib/gitlab/auth/oidc/step_up_authentication.rb b/lib/gitlab/auth/oidc/step_up_authentication.rb index cc36795933c51a39ad0d1cce0e3735f7240f1b1e..434c8b26d911e6691bb781ee9afa42e7f4488ad8 100644 --- a/lib/gitlab/auth/oidc/step_up_authentication.rb +++ b/lib/gitlab/auth/oidc/step_up_authentication.rb @@ -10,19 +10,19 @@ module Oidc module StepUpAuthentication SESSION_STORE_KEY = 'omniauth_step_up_auth' - STEP_UP_AUTH_SCOPE_ADMIN_MODE = :admin_mode - STEP_UP_AUTH_SCOPE_NAMESPACE = :namespace + SCOPE_ADMIN_MODE = :admin_mode + SCOPE_NAMESPACE = :namespace ALLOWED_SCOPES = [ - STEP_UP_AUTH_SCOPE_ADMIN_MODE, - STEP_UP_AUTH_SCOPE_NAMESPACE + SCOPE_ADMIN_MODE, + SCOPE_NAMESPACE ].freeze class << self # Checks if step-up authentication is enabled for the step-up auth scope 'admin_mode' # # @return [Boolean] true if any OAuth provider requires step-up auth for admin mode - def enabled_by_config?(scope: STEP_UP_AUTH_SCOPE_ADMIN_MODE) + def enabled_by_config?(scope: SCOPE_ADMIN_MODE) oauth_providers.any? do |provider| enabled_for_provider?(provider_name: provider, scope: scope) end @@ -33,12 +33,12 @@ def enabled_by_config?(scope: STEP_UP_AUTH_SCOPE_ADMIN_MODE) # @param oauth_provider_name [String] the name of the OAuth provider # @param scope [Symbol] the scope to check configuration for (default: :admin_mode) # @return [Boolean] true if configuration exists - def enabled_for_provider?(provider_name:, scope: STEP_UP_AUTH_SCOPE_ADMIN_MODE) + def enabled_for_provider?(provider_name:, scope: SCOPE_ADMIN_MODE) has_required_claims?(provider_name, scope) || has_included_claims?(provider_name, scope) end - def enabled_providers(scope: STEP_UP_AUTH_SCOPE_ADMIN_MODE) + def enabled_providers(scope: SCOPE_ADMIN_MODE) oauth_providers.select do |provider| enabled_for_provider?(provider_name: provider, scope: scope) end @@ -49,7 +49,7 @@ def enabled_providers(scope: STEP_UP_AUTH_SCOPE_ADMIN_MODE) # # @param session [Hash] the session hash containing authentication state # @return [Boolean] true if step-up authentication is authenticated - def succeeded?(session, scope: STEP_UP_AUTH_SCOPE_ADMIN_MODE) + def succeeded?(session, scope: SCOPE_ADMIN_MODE) step_up_auth_flows(session) .select do |step_up_auth_flow| step_up_auth_flow.scope.to_s == scope.to_s @@ -63,7 +63,7 @@ def succeeded?(session, scope: STEP_UP_AUTH_SCOPE_ADMIN_MODE) # @param oauth [OAuth2::AccessToken] the OAuth object to validate # @param scope [Symbol] the scope to validate conditions for (default: :admin_mode) # @return [Boolean] true if all conditions are fulfilled - def conditions_fulfilled?(oauth_extra_metadata:, provider:, scope: STEP_UP_AUTH_SCOPE_ADMIN_MODE) + def conditions_fulfilled?(oauth_extra_metadata:, provider:, scope: SCOPE_ADMIN_MODE) conditions = [] if has_required_claims?(provider, scope) @@ -79,7 +79,7 @@ def conditions_fulfilled?(oauth_extra_metadata:, provider:, scope: STEP_UP_AUTH_ conditions.present? && conditions.all? end - def build_flow(provider:, session:, scope: STEP_UP_AUTH_SCOPE_ADMIN_MODE) + def build_flow(provider:, session:, scope: SCOPE_ADMIN_MODE) Gitlab::Auth::Oidc::StepUpAuthenticationFlow.new(provider: provider, scope: scope, session: session) end @@ -87,9 +87,9 @@ def build_flow(provider:, session:, scope: STEP_UP_AUTH_SCOPE_ADMIN_MODE) # # @param oauth_raw_info [Hash] The raw information received from the OAuth provider. # @param provider [String] The name of the OAuth provider. - # @param scope [String] The scope of the authentication request, default is STEP_UP_AUTH_SCOPE_ADMIN_MODE. + # @param scope [String] The scope of the authentication request, default is SCOPE_ADMIN_MODE. # @return [Hash] A hash containing only the relevant ID token claims. - def slice_relevant_id_token_claims(oauth_raw_info:, provider:, scope: STEP_UP_AUTH_SCOPE_ADMIN_MODE) + def slice_relevant_id_token_claims(oauth_raw_info:, provider:, scope: SCOPE_ADMIN_MODE) relevant_id_token_claims = [ *get_id_token_claims_required_conditions(provider, scope)&.keys, *get_id_token_claims_included_conditions(provider, scope)&.keys @@ -101,7 +101,7 @@ def omniauth_step_up_auth_session_data(session) Gitlab::NamespacedSessionStore.new(SESSION_STORE_KEY, session) end - def disable_step_up_authentication!(session:, scope: STEP_UP_AUTH_SCOPE_ADMIN_MODE) + def disable_step_up_authentication!(session:, scope: SCOPE_ADMIN_MODE) omniauth_step_up_auth_session_data(session) &.to_h &.each_value do |step_up_auth_object| @@ -114,7 +114,7 @@ def disable_step_up_authentication!(session:, scope: STEP_UP_AUTH_SCOPE_ADMIN_MO # @param session [Hash] the session hash containing authentication state # @param scope [Symbol] the scope to check (default: :admin_mode) # @return [Array] list of failed authentication flows - def failed_step_up_auth_flows(session, scope: STEP_UP_AUTH_SCOPE_ADMIN_MODE) + def failed_step_up_auth_flows(session, scope: SCOPE_ADMIN_MODE) (step_up_auth_flows(session) || []).select do |flow| flow.enabled_by_config? && flow.failed? && @@ -186,8 +186,12 @@ def step_up_auth_flows(session) &.flat_map do |provider, step_up_auth_object| step_up_auth_object.map do |step_up_auth_scope, _| build_flow(provider: provider, session: session, scope: step_up_auth_scope) + rescue ArgumentError => e + Gitlab::AppLogger.error("Error building step-up authentication flow: #{e.message}") + nil end end + &.compact end end end diff --git a/lib/gitlab/auth/oidc/step_up_authentication_flow.rb b/lib/gitlab/auth/oidc/step_up_authentication_flow.rb index 7699f30ee69a923bb36023ed00605f7329cd6f36..a3f64fe3eaf43039897b1616b8b9c0bb2de96e13 100644 --- a/lib/gitlab/auth/oidc/step_up_authentication_flow.rb +++ b/lib/gitlab/auth/oidc/step_up_authentication_flow.rb @@ -11,6 +11,7 @@ class StepUpAuthenticationFlow attr_reader :session, :provider, :scope def initialize(session:, provider:, scope:) + validate_scope!(scope) @session = session @provider = provider @scope = scope @@ -92,6 +93,14 @@ def conditions_fulfilled?(oidc_id_token_claims) ::Gitlab::Auth::Oidc::StepUpAuthentication .conditions_fulfilled?(oauth_extra_metadata: oidc_id_token_claims, provider: provider, scope: scope) end + + def validate_scope!(scope) + return if ::Gitlab::Auth::Oidc::StepUpAuthentication::ALLOWED_SCOPES.include?(scope&.to_sym) + + allowed = ::Gitlab::Auth::Oidc::StepUpAuthentication::ALLOWED_SCOPES + raise ArgumentError, + "Invalid scope '#{scope}'. Allowed scopes are: #{allowed.join(', ')}" + end end end end diff --git a/spec/controllers/omniauth_callbacks_controller_spec.rb b/spec/controllers/omniauth_callbacks_controller_spec.rb index b192e54b67237a5767fa0e332ef99d9ffa6f2d77..4ce2792951913cc19529461a92b49d002a508808 100644 --- a/spec/controllers/omniauth_callbacks_controller_spec.rb +++ b/spec/controllers/omniauth_callbacks_controller_spec.rb @@ -926,7 +926,159 @@ it 'does not add session key "step_up_auth"' do get provider - expect(session).not_to include 'step_up_auth' + expect(session).not_to include 'omniauth_step_up_auth' + end + end + + context 'for namespace scope' do + using RSpec::Parameterized::TableSyntax + + let(:ommiauth_provider_config_with_namespace_step_up_auth) do + GitlabSettings::Options.new( + name: "openid_connect", + step_up_auth: { + namespace: { + id_token: { + required: required_id_token_claims, + included: included_id_token_claims + } + } + } + ) + end + + before do + mock_auth_hash(provider, extern_uid, user.email, additional_info: { extra: { raw_info: mock_auth_hash_extra_raw_info } }) + + request.env['omniauth.auth'] = Rails.application.env_config['omniauth.auth'] + session['omniauth_step_up_auth'] = { 'openid_connect' => { 'namespace' => { 'state' => 'requested' } } } + + stub_omniauth_setting(enabled: true, auto_link_user: true, block_auto_created_users: false, providers: [ommiauth_provider_config_with_namespace_step_up_auth]) + end + + where(:required_id_token_claims, :included_id_token_claims, :mock_auth_hash_extra_raw_info, :step_up_auth_authenticated) do + { claim_1: 'silver' } | nil | { claim_1: 'silver' } | 'succeeded' + { claim_1: 'silver' } | nil | { claim_1: 'gold' } | 'failed' + nil | { claim_2: %w[mfa fpt] } | { claim_2: 'mfa' } | 'succeeded' + { claim_1: 'silver' } | { claim_2: %w[mfa fpt] } | { claim_1: 'silver', claim_2: 'mfa' } | 'succeeded' + { claim_1: 'silver' } | { claim_2: %w[mfa fpt] } | { claim_1: 'silver', claim_2: 'other_amr' } | 'failed' + end + + with_them do + context 'when user is signed in' do + before do + sign_in user + end + + it 'evaluates step-up authentication conditions for namespace scope' do + get provider + + expect(session.to_h.dig('omniauth_step_up_auth', 'openid_connect', 'namespace', 'state')) + .to eq step_up_auth_authenticated + end + + context 'when feature flag :omniauth_step_up_auth_for_namespace is disabled' do + before do + stub_feature_flags(omniauth_step_up_auth_for_namespace: false) + session.delete 'omniauth_step_up_auth' + end + + it 'does not store step-up authentication evaluation result in session' do + get provider + + expect(session).not_to include 'omniauth_step_up_auth' + end + end + end + end + end + + context 'with both admin_mode and namespace scopes configured' do + let(:ommiauth_provider_config_with_both_scopes) do + GitlabSettings::Options.new( + name: "openid_connect", + step_up_auth: { + admin_mode: { + id_token: { + required: { claim_1: 'gold' } + } + }, + namespace: { + id_token: { + required: { claim_1: 'silver' } + } + } + } + ) + end + + before do + mock_auth_hash(provider, extern_uid, user.email, additional_info: { extra: { raw_info: { claim_1: 'gold' } } }) + request.env['omniauth.auth'] = Rails.application.env_config['omniauth.auth'] + stub_omniauth_setting(enabled: true, auto_link_user: true, block_auto_created_users: false, providers: [ommiauth_provider_config_with_both_scopes]) + sign_in user + end + + context 'when both scopes are requested' do + before do + session['omniauth_step_up_auth'] = { + 'openid_connect' => { + 'admin_mode' => { 'state' => 'requested' }, + 'namespace' => { 'state' => 'requested' } + } + } + end + + it 'evaluates both admin_mode and namespace scopes' do + get provider + + expect(session.to_h.dig('omniauth_step_up_auth', 'openid_connect', 'admin_mode', 'state')) + .to eq 'succeeded' + expect(session.to_h.dig('omniauth_step_up_auth', 'openid_connect', 'namespace', 'state')) + .to eq 'failed' # gold claim doesn't meet silver requirement + end + end + + context 'when only admin_mode feature flag is enabled' do + before do + stub_feature_flags(omniauth_step_up_auth_for_namespace: false) + session['omniauth_step_up_auth'] = { + 'openid_connect' => { + 'admin_mode' => { 'state' => 'requested' }, + 'namespace' => { 'state' => 'requested' } + } + } + end + + it 'only evaluates admin_mode scope' do + get provider + + expect(session.to_h.dig('omniauth_step_up_auth', 'openid_connect', 'admin_mode', 'state')) + .to eq 'succeeded' + expect(session.to_h.dig('omniauth_step_up_auth', 'openid_connect', 'namespace', 'state')) + .to eq 'requested' # unchanged + end + end + + context 'when only namespace feature flag is enabled' do + before do + stub_feature_flags(omniauth_step_up_auth_for_admin_mode: false) + session['omniauth_step_up_auth'] = { + 'openid_connect' => { + 'admin_mode' => { 'state' => 'requested' }, + 'namespace' => { 'state' => 'requested' } + } + } + end + + it 'only evaluates namespace scope' do + get provider + + expect(session.to_h.dig('omniauth_step_up_auth', 'openid_connect', 'admin_mode', 'state')) + .to eq 'requested' # unchanged + expect(session.to_h.dig('omniauth_step_up_auth', 'openid_connect', 'namespace', 'state')) + .to eq 'failed' + end end end end diff --git a/spec/lib/gitlab/auth/oidc/step_up_auth_before_request_phase_spec.rb b/spec/lib/gitlab/auth/oidc/step_up_auth_before_request_phase_spec.rb index 56e41502a2ef951d4ff4a32a59ebc68546648ca4..bbfb1ae604d37910ce49550a5ddbf0044b106bf3 100644 --- a/spec/lib/gitlab/auth/oidc/step_up_auth_before_request_phase_spec.rb +++ b/spec/lib/gitlab/auth/oidc/step_up_auth_before_request_phase_spec.rb @@ -19,6 +19,10 @@ { 'omniauth_step_up_auth' => { 'openid_connect' => { 'admin_mode' => { 'state' => 'requested' } } } } end + let(:session_with_namespace_step_up_state_requested) do + { 'omniauth_step_up_auth' => { 'openid_connect' => { 'namespace' => { 'state' => 'requested' } } } } + end + let(:provider_step_up_auth_name) { provider_step_up_auth.name } let(:provider_step_up_auth) do GitlabSettings::Options.new( @@ -30,6 +34,13 @@ acr: 'gold' } } + }, + namespace: { + id_token: { + required: { + acr: 'silver' + } + } } } ) @@ -44,16 +55,25 @@ # rubocop:disable Layout/LineLength -- Avoid formatting to keep one-line table syntax where(:provider, :step_up_auth_scope, :session, :expected_session) do + # admin_mode tests ref(:provider_step_up_auth_name) | 'admin_mode' | {} | ref(:session_with_step_up_state_requested) ref(:provider_step_up_auth_name) | 'admin_mode' | ref(:session_with_step_up_state_requested) | ref(:session_with_step_up_state_requested) ref(:provider_step_up_auth_name) | 'admin_mode' | { 'omniauth_step_up_auth' => { 'other_provider' => { 'admin_mode' => { 'state' => 'requested' } } } } | lazy { session.deep_merge(session_with_step_up_state_requested) } ref(:provider_step_up_auth_name) | 'admin_mode' | { 'other_session_key' => 'other_session_value' } | lazy { session.deep_merge(session_with_step_up_state_requested) } - ref(:provider_step_up_auth_name) | 'other_scope' | {} | {} - ref(:provider_step_up_auth_name) | 'other_scope' | ref(:session_with_step_up_state_requested) | ref(:session_with_step_up_state_requested) + + # namespace tests + ref(:provider_step_up_auth_name) | 'namespace' | {} | ref(:session_with_namespace_step_up_state_requested) + ref(:provider_step_up_auth_name) | 'namespace' | ref(:session_with_namespace_step_up_state_requested) | ref(:session_with_namespace_step_up_state_requested) + ref(:provider_step_up_auth_name) | 'namespace' | { 'omniauth_step_up_auth' => { 'other_provider' => { 'namespace' => { 'state' => 'requested' } } } } | lazy { session.deep_merge(session_with_namespace_step_up_state_requested) } + ref(:provider_step_up_auth_name) | 'namespace' | { 'other_session_key' => 'other_session_value' } | lazy { session.deep_merge(session_with_namespace_step_up_state_requested) } + + # invalid scope tests ref(:provider_step_up_auth_name) | '' | {} | {} ref(:provider_step_up_auth_name) | nil | {} | {} 'other_provider' | 'admin_mode' | {} | {} + 'other_provider' | 'namespace' | {} | {} nil | 'admin_mode' | {} | {} + nil | 'namespace' | {} | {} nil | nil | {} | {} end # rubocop:enable Layout/LineLength @@ -64,35 +84,109 @@ expect(session).to eq(expected_session) end + end + + context 'when invalid scope is given' do + let(:provider) { provider_step_up_auth_name } + let(:session) { {} } + let(:step_up_auth_scope) { 'other_scope' } + + it 'does not modify the session' do + expect { call_middleware }.to raise_error(ArgumentError) + end + end + + context 'when feature flag :omniauth_step_up_auth_for_admin_mode is disabled' do + before do + stub_feature_flags(omniauth_step_up_auth_for_admin_mode: false) + end + + let(:provider) { provider_step_up_auth_name } + let(:session) { {} } - context 'when feature flag :omniauth_step_up_auth_for_admin_mode is disabled' do - before do - stub_feature_flags(omniauth_step_up_auth_for_admin_mode: false) + context 'with admin_mode scope' do + let(:step_up_auth_scope) { 'admin_mode' } + + it 'does not modify the session' do + expect { call_middleware }.not_to change { session } end + end + + context 'with namespace scope' do + let(:step_up_auth_scope) { 'namespace' } + + it 'sets the session state to requested for namespace scope' do + call_middleware + expect(session).to eq(session_with_namespace_step_up_state_requested) + end + end + end + + context 'when feature flag :omniauth_step_up_auth_for_namespace is disabled' do + before do + stub_feature_flags(omniauth_step_up_auth_for_namespace: false) + end + + let(:provider) { provider_step_up_auth_name } + let(:session) { {} } + + context 'with namespace scope' do + let(:step_up_auth_scope) { 'namespace' } it 'does not modify the session' do expect { call_middleware }.not_to change { session } end end + + context 'with admin_mode scope' do + let(:step_up_auth_scope) { 'admin_mode' } + + it 'sets the session state to requested for admin_mode scope' do + call_middleware + expect(session).to eq(session_with_step_up_state_requested) + end + end end context 'when user is not authenticated' do let(:env) { super().merge('warden' => instance_double(Warden::Proxy, user: nil)) } let(:provider) { provider_step_up_auth_name } - let(:step_up_auth_scope) { 'admin_mode' } - it 'does not modify the session' do - expect { call_middleware }.not_to change { session } + context 'with admin_mode scope' do + let(:step_up_auth_scope) { 'admin_mode' } + + it 'does not modify the session' do + expect { call_middleware }.not_to change { session } + end + end + + context 'with namespace scope' do + let(:step_up_auth_scope) { 'namespace' } + + it 'does not modify the session' do + expect { call_middleware }.not_to change { session } + end end end context 'when warden is blank' do let(:env) { super().merge('warden' => nil) } let(:provider) { provider_step_up_auth_name } - let(:step_up_auth_scope) { 'admin_mode' } - it 'does not modify the session' do - expect { call_middleware }.not_to change { session } + context 'with admin_mode scope' do + let(:step_up_auth_scope) { 'admin_mode' } + + it 'does not modify the session' do + expect { call_middleware }.not_to change { session } + end + end + + context 'with namespace scope' do + let(:step_up_auth_scope) { 'namespace' } + + it 'does not modify the session' do + expect { call_middleware }.not_to change { session } + end end end end diff --git a/spec/lib/gitlab/auth/oidc/step_up_authentication_flow_spec.rb b/spec/lib/gitlab/auth/oidc/step_up_authentication_flow_spec.rb index 2bcf234489710bedd67226326d555d23107a7aa4..8aea602473a8038b51bdb31a8903c37e038cf2a8 100644 --- a/spec/lib/gitlab/auth/oidc/step_up_authentication_flow_spec.rb +++ b/spec/lib/gitlab/auth/oidc/step_up_authentication_flow_spec.rb @@ -9,6 +9,35 @@ subject(:flow) { described_class.new(session: session, provider: provider, scope: scope) } + describe '#initialize' do + using RSpec::Parameterized::TableSyntax + + shared_examples 'a valid flow' do + it { is_expected.to have_attributes(scope: scope) } + end + + shared_examples 'an invalid flow' do + it 'raises an error' do + expect { flow } + .to raise_error(ArgumentError, "Invalid scope '#{scope}'. Allowed scopes are: admin_mode, namespace") + end + end + + where(:scope, :expected_example) do + 'admin_mode' | 'a valid flow' + :admin_mode | 'a valid flow' + 'namespace' | 'a valid flow' + :namespace | 'a valid flow' + 'custom_scope' | 'an invalid flow' + :invalid_scope | 'an invalid flow' + nil | 'an invalid flow' + end + + with_them do + it_behaves_like params[:expected_example] + end + end + describe '#requested?, #succeeded?, #failed?' do using RSpec::Parameterized::TableSyntax @@ -134,7 +163,7 @@ where(:provider_config, :provider_name, :scope, :expected_result) do { 'step_up_auth' => { 'admin_mode' => { 'documentation_link' => 'https://example.com/company/internal/auth-help' } } } | 'openid_connect' | 'admin_mode' | 'https://example.com/company/internal/auth-help' { 'step_up_auth' => { 'admin_mode' => { 'documentation_link' => 'https://example.com/company/internal/auth-help' } } } | :openid_connect | 'admin_mode' | 'https://example.com/company/internal/auth-help' - { 'step_up_auth' => { 'custom_scope' => { 'documentation_link' => 'https://example.com/company/internal/custom-auth-help' } } } | 'openid_connect' | 'custom_scope' | 'https://example.com/company/internal/custom-auth-help' + { 'step_up_auth' => { 'namespace' => { 'documentation_link' => 'https://example.com/company/internal/namespace-auth-help' } } } | 'openid_connect' | 'namespace' | 'https://example.com/company/internal/namespace-auth-help' { 'step_up_auth' => { 'admin_mode' => {} } } | 'openid_connect' | 'admin_mode' | nil {} | 'openid_connect' | 'admin_mode' | nil end diff --git a/spec/lib/gitlab/auth/oidc/step_up_authentication_spec.rb b/spec/lib/gitlab/auth/oidc/step_up_authentication_spec.rb index 9a734f610878d2aa8cdd7cbaa6b7c8a6cf411e48..fc006c3a8b7e484339280b6a25ff901a8cbbcdef 100644 --- a/spec/lib/gitlab/auth/oidc/step_up_authentication_spec.rb +++ b/spec/lib/gitlab/auth/oidc/step_up_authentication_spec.rb @@ -157,7 +157,7 @@ lazy { { 'openid_connect' => { 'admin_mode' => { 'state' => 'succeeded' } }, 'other_provider' => { 'admin_mode' => { 'state' => 'succeeded' } } } } | true lazy { { 'openid_connect' => { 'admin_mode' => { 'state' => 'failed' } }, 'other_provider' => { 'admin_mode' => { 'state' => 'failed' } } } } | false lazy { { 'openid_connect' => { 'admin_mode' => { 'state' => 'failed' } }, 'other_provider' => { 'admin_mode' => { 'state' => 'succeeded' } } } } | false - lazy { { 'openid_connect' => { 'other_mode' => { 'state' => 'succeeded' } }, 'other_provider' => { 'admin_mode' => { 'state' => 'failed' } } } } | false + lazy { { 'openid_connect' => { 'namespace' => { 'state' => 'succeeded' } }, 'other_provider' => { 'admin_mode' => { 'state' => 'failed' } } } } | false nil | false {} | false @@ -177,7 +177,7 @@ 'omniauth_step_up_auth' => { 'openid_connect' => { 'admin_mode' => { 'state' => 'succeeded' }, - 'other_scope' => { 'state' => 'succeeded' } + 'namespace' => { 'state' => 'succeeded' } }, 'other_provider' => { 'admin_mode' => { 'state' => 'failed' } @@ -199,7 +199,7 @@ disable_step_up_authentication! expect(session['omniauth_step_up_auth']['openid_connect']).not_to have_key('admin_mode') - expect(session['omniauth_step_up_auth']['openid_connect']).to have_key('other_scope') + expect(session['omniauth_step_up_auth']['openid_connect']).to have_key('namespace') expect(session['omniauth_step_up_auth']['other_provider']).not_to have_key('admin_mode') expect(described_class.succeeded?(session)).to be_falsey @@ -218,7 +218,7 @@ disable_step_up_authentication! expect(session['omniauth_step_up_auth']['openid_connect']).to have_key('admin_mode') - expect(session['omniauth_step_up_auth']['openid_connect']).to have_key('other_scope') + expect(session['omniauth_step_up_auth']['openid_connect']).to have_key('namespace') expect(session['omniauth_step_up_auth']['other_provider']).to have_key('admin_mode') end end