diff --git a/app/controllers/admin/application_controller.rb b/app/controllers/admin/application_controller.rb index bab2d5639a7943c63e7e3cf0fae513bde5045287..7ecbe4d2ae81f141f67e167a1accd71060da0478 100644 --- a/app/controllers/admin/application_controller.rb +++ b/app/controllers/admin/application_controller.rb @@ -5,6 +5,7 @@ # Automatically sets the layout and ensures an administrator is logged in class Admin::ApplicationController < ApplicationController include EnforcesAdminAuthentication + include EnforcesStepUpAuthentication layout 'admin' end diff --git a/app/controllers/concerns/enforces_step_up_authentication.rb b/app/controllers/concerns/enforces_step_up_authentication.rb new file mode 100644 index 0000000000000000000000000000000000000000..29f508f61f1a59de82d9b679b4f4d82ddec35133 --- /dev/null +++ b/app/controllers/concerns/enforces_step_up_authentication.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +# Enforces step-up authentication requirements for admin access +# +# This controller concern ensures users complete step-up authentication +# before accessing admin functionality. Include this module in admin +# controllers to enforce the authentication check. +# +# @example +# class Admin::ApplicationController < ApplicationController +# include EnforcesStepUpAuthentication +# end +module EnforcesStepUpAuthentication + extend ActiveSupport::Concern + + included do + before_action :enforce_step_up_authentication + end + + private + + def enforce_step_up_authentication + return if Feature.disabled?(:omniauth_step_up_auth_for_admin_mode, current_user) + + return if step_up_auth_disabled_for_admin_mode? + return if step_up_auth_flow_state_success? + + handle_failed_authentication + end + + def step_up_auth_disabled_for_admin_mode? + !::Gitlab::Auth::Oidc::StepUpAuthentication.enabled_by_config? + end + + def step_up_auth_flow_state_success? + ::Gitlab::Auth::Oidc::StepUpAuthentication.succeeded?(session) + end + + def handle_failed_authentication + # We need to disable (reset) the admin mode in order to redirect the user to the admin login page. + # If we do not do this, the Admin::SessionsController will thinks that the admin mode has been successfully reached + # and will redirect the user to the path 'admin/dashboard'. But, the check in this EnforceStepUpAuthentication + # will fail again and redirect the user to the login page which will end up in a loop. + disable_admin_mode + + redirect_to(new_admin_session_path, notice: _('Step-up auth not successful')) + end + + def disable_admin_mode + current_user_mode.disable_admin_mode! if current_user_mode.admin_mode? + end +end diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index 8c5bc248efd0d87ffab1e04dcfc441ef46d3bda5..e61445b34b5ffe89d44e1e6f08011bea2e2a2e13 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -163,6 +163,8 @@ def omniauth_flow(auth_module, identity_linker: nil) set_session_active_since(oauth['provider']) if ::AuthHelper.saml_providers.include?(oauth['provider'].to_sym) track_event(current_user, oauth['provider'], 'succeeded') + handle_step_up_auth + if Gitlab::CurrentSettings.admin_mode return admin_mode_flow(auth_module::User) if current_user_mode.admin_mode_requested? end @@ -408,6 +410,18 @@ def store_redirect_fragment(redirect_fragment) end end + def handle_step_up_auth + return if Feature.disabled?(:omniauth_step_up_auth_for_admin_mode, current_user) + + step_up_auth_flow = + ::Gitlab::Auth::Oidc::StepUpAuthentication.build_flow(session: session, provider: oauth.provider) + + return unless step_up_auth_flow.enabled_by_config? + return unless step_up_auth_flow.requested? + + step_up_auth_flow.evaluate!(oauth.extra.raw_info) + end + def admin_mode_flow(auth_user_class) auth_user = build_auth_user(auth_user_class) diff --git a/app/helpers/auth_helper.rb b/app/helpers/auth_helper.rb index 7fb09d04d25226373634ed52a07f52c87b69d38b..101bcc9a28bff851e987ba34138fadffb4bdfade 100644 --- a/app/helpers/auth_helper.rb +++ b/app/helpers/auth_helper.rb @@ -165,6 +165,27 @@ def button_based_providers_enabled? enabled_button_based_providers.any? end + def step_up_auth_params(provider_name, step_up_auth_scope) + return {} if Feature.disabled?(:omniauth_step_up_auth_for_admin_mode, current_user) + + # Get provider configuration for step up auth scope + provider_config = Gitlab::Auth::OAuth::Provider + .config_for(provider_name) + &.dig('step_up_auth', step_up_auth_scope.to_s) + &.to_h + + return {} if provider_config.blank? + + base_params = { step_up_auth_scope: step_up_auth_scope } + config_params = provider_config['params'].to_h + + base_params + .merge!(config_params) + .transform_values do |v| + v.is_a?(Hash) ? v.to_json : v + end + end + def provider_image_tag(provider, size = 64) label = label_for_provider(provider) diff --git a/app/views/admin/sessions/new.html.haml b/app/views/admin/sessions/new.html.haml index 304266c674a3bd7c49b4559268b6910cf0bf5ee6..03580cbb71d738aa855e72c83fa3d91f9f36d271 100644 --- a/app/views/admin/sessions/new.html.haml +++ b/app/views/admin/sessions/new.html.haml @@ -15,4 +15,4 @@ = _('No authentication methods configured.') - if omniauth_enabled? && button_based_providers_enabled? - = render 'devise/shared/omniauth_box', render_remember_me: false + = render 'devise/shared/omniauth_box', render_remember_me: false, step_up_auth_scope: 'admin_mode' diff --git a/app/views/devise/shared/_omniauth_box.html.haml b/app/views/devise/shared/_omniauth_box.html.haml index 19296a7c006af3b2b5d7b539458674d161c87f87..8918c8d6adaba782a3808e61c8110f3a13ec6e92 100644 --- a/app/views/devise/shared/_omniauth_box.html.haml +++ b/app/views/devise/shared/_omniauth_box.html.haml @@ -1,14 +1,21 @@ - render_remember_me = remember_me_enabled? && local_assigns.fetch(:render_remember_me, true) +- step_up_auth_scope = local_assigns[:step_up_auth_scope] - if any_form_based_providers_enabled? || password_authentication_enabled_for_web? = render 'shared/divider', text: _("or sign in with") .gl-mt-5.gl-text-center.gl-flex.gl-flex-col.gl-gap-3.js-oauth-login - enabled_button_based_providers.each do |provider| - = render 'devise/shared/omniauth_provider_button', - href: omniauth_authorize_path(:user, provider), - provider: provider, - data: { testid: test_id_for_provider(provider) } + - if step_up_auth_scope.present? + = render 'devise/shared/omniauth_provider_button', + href: omniauth_authorize_path(:user, provider, **step_up_auth_params(provider, step_up_auth_scope)), + provider: provider, + data: { testid: test_id_for_provider(provider) } + - else + = render 'devise/shared/omniauth_provider_button', + href: omniauth_authorize_path(:user, provider), + provider: provider, + data: { testid: test_id_for_provider(provider) } - if render_remember_me = render Pajamas::CheckboxTagComponent.new(name: 'js-remember-me-omniauth', value: nil) do |c| diff --git a/config/feature_flags/gitlab_com_derisk/omniauth_step_up_auth_for_admin_mode.yml b/config/feature_flags/gitlab_com_derisk/omniauth_step_up_auth_for_admin_mode.yml new file mode 100644 index 0000000000000000000000000000000000000000..abfbbe2e5eef8d49750325e99e1aad8ad4e631a7 --- /dev/null +++ b/config/feature_flags/gitlab_com_derisk/omniauth_step_up_auth_for_admin_mode.yml @@ -0,0 +1,9 @@ +--- +name: omniauth_step_up_auth_for_admin_mode +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/474650 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/171643 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/502544 +milestone: '17.11' +group: group::authentication +type: gitlab_com_derisk +default_enabled: false diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index 706cfc5c90446f0f4d3eb71c3312c619da11b8ec..1af38f49647ae19d56e034a6601ff0c168631c55 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -1664,7 +1664,8 @@ test: app_id: 'YOUR_CLIENT_ID', app_secret: 'YOUR_CLIENT_SECRET', args: { scope: 'offline_access read:jira-user read:jira-work', prompt: 'consent' } - } + } + - { name: 'openid_connect' } ldap: enabled: false servers: diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb index 3ea1157adc8a569f70964897bf2a2ac964d78c71..e12fc1e0162171cb52aa02e783f4262238588346 100644 --- a/config/initializers/omniauth.rb +++ b/config/initializers/omniauth.rb @@ -22,4 +22,5 @@ module OmniAuth::Strategies OmniAuth.config.before_request_phase do |env| Gitlab::Auth::OAuth::BeforeRequestPhaseOauthLoginCounterIncrement.call(env) + Gitlab::Auth::Oidc::StepUpAuthBeforeRequestPhase.call(env) end diff --git a/doc/administration/auth/oidc.md b/doc/administration/auth/oidc.md index a2f0af0de986a57fdd224e779594a580b1097872..2573d10441114a6fb257c2e9e730f99dade70102 100644 --- a/doc/administration/auth/oidc.md +++ b/doc/administration/auth/oidc.md @@ -1421,6 +1421,146 @@ To configure a custom duration for your ID tokens: {{< /tabs >}} +## Step-up authentication for Admin Mode + +{{< details >}} + +- Tier: Free, Premium, Ultimate +- Offering: GitLab Self-Managed +- Status: Experiment + +{{< /details >}} + +{{< history >}} + +- [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/474650) in GitLab 17.11 [with a flag](../feature_flags.md) named `omniauth_step_up_auth_for_admin_mode`. Disabled by default. + +{{< /history >}} + +{{< alert type="flag" >}} + +The availability of this feature is controlled by a feature flag. +For more information, see the history. +This feature is available for testing, but not ready for production use. + +{{< /alert >}} + +In some cases, default authentication methods don't sufficiently protect critical resources or +high-risk actions. Step-up authentication adds an extra authentication layer for privileged actions +or sensitive operations, such as accessing the Admin area. + +With step-up authentication, users must provide additional credentials before they can access +certain features or perform specific actions. These additional methods can include methods such +as two-factor authentication (2FA), biometric authentication, or one-time passwords (OTP). + +The OIDC standard includes authentication context class references (`ACR`). The `ACR` concept +helps configure and implement step-up authentication for different scenarios, such as Admin Mode. + +This feature is an [experiment](../../policy/development_stages_support.md) and subject to change without notice. This feature is not ready for production use. If you want to use this feature, you should test outside of production first. + +### Enable step-up authentication for Admin Mode + +To enable step-up authentication for Admin Mode: + +1. Edit your GitLab configuration file (`gitlab.yml` or `/etc/gitlab/gitlab.rb`) to enable + step-up authentication for an specific OmniAuth provider. + + ```yaml + production: &base + omniauth: + providers: + - { name: 'openid_connect', + label: 'Provider name', + args: { + name: 'openid_connect', + # ... + allow_authorize_params: ["claims"], # Match this to the parameters defined in `step_up_auth => admin_mode => params` + }, + step_up_auth: { + admin_mode: { + id_token: { + # The `required` field declares which claims are required in the ID token with exact match. + # In this example, the 'acr' (Authentication Context Class Reference) claim + # must have the value 'gold' to pass the step-up authentication challenge. + # This ensures a specific level of authentication assurance. + required: { + acr: 'gold' + } + }, + # The `params` field defines any additional parameters that are sent during the authentication process. + # In this example, the `claims` parameter is added to the authorization request and instructs the + # identity provider to include an 'acr' claim with the value 'gold' in the ID token. + # The 'essential: true' indicates that this claim is required for successful authentication. + params: { + claims: { + id_token: { + acr: { + essential: true, + values: ['gold'] + } + } + } + } + }, + } + } + ``` + +1. Save the configuration file and restart GitLab for the changes to take effect. + +{{< alert type="note" >}} + +Although OIDC is standardized, different Identity Providers (IdPs) might have unique requirements. +The `params` setting allows a flexible hash to define necessary parameters for step-up authentication. +These values can vary based on the requirements for each IdP. + +{{< /alert >}} + +#### Enable step-up authentication for Admin Mode using Keycloak + +Keycloak supports step-up authentication by defining levels of authentication and custom browser login flows. + +To configure step-up authentication for Admin Mode in GitLab using Keycloak: + +1. [Configure Keycloak](#configure-keycloak) in GitLab. + +1. Follow the steps in the Keycloak documentation to [create a browser login flow with step-up authentication in Keycloak](https://www.keycloak.org/docs/latest/server_admin/#_step-up-flow). + +1. Edit your GitLab configuration file (`gitlab.yml` or `/etc/gitlab/gitlab.rb`) to enable + step-up authentication in the Keycloak OIDC provider configuration. + + Keycloak defines two different authentication levels: `silver` and `gold`. The following example + uses `gold` to represent the increased security level. + + ```yaml + production: &base + omniauth: + providers: + - { name: 'openid_connect', + label: 'Keycloak', + args: { + name: 'openid_connect', + # ... + allow_authorize_params: ["claims"] # Match this to the parameters defined in `step_up_auth => admin_mode => params` + }, + step_up_auth: { + admin_mode: { + id_token: { + # In this example, the 'acr' claim must have the value 'gold' that is also defined in the Keycloak documentation. + required: { + acr: 'gold' + } + }, + params: { + claims: { id_token: { acr: { essential: true, values: ['gold'] } } } + } + }, + } + } + ``` + +1. Save the configuration file and restart GitLab for the changes to take effect. + ## Troubleshooting 1. Ensure `discovery` is set to `true`. If you set it to `false`, you must @@ -1439,3 +1579,8 @@ To configure a custom duration for your ID tokens: your OpenID web server configuration. For example, for [`oauth2-server-php`](https://github.com/bshaffer/oauth2-server-php), you may have to [add a configuration parameter to Apache](https://github.com/bshaffer/oauth2-server-php/issues/926#issuecomment-387502778). + +1. **Step-up authentication only**: Ensure that any parameters defined in + `step_up_auth => admin_mode => params` are also defined in `args => allow_authorize_params`. + This includes the parameters in the request query parameters used to + redirect to the IdP authorization endpoint. 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 new file mode 100644 index 0000000000000000000000000000000000000000..2b75a371fc241f701b6761df26673309f83e678b --- /dev/null +++ b/lib/gitlab/auth/oidc/step_up_auth_before_request_phase.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +module Gitlab + module Auth + module Oidc + # Handles the step-up authentication request phase for OAuth flow + # + # This module manages the initial phase of step-up authentication, + # setting up the session state for admin mode authentication. + 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)) + + # 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) + + session = session_from(env) + provider = current_provider_from(env) + step_up_auth_flow = + ::Gitlab::Auth::Oidc::StepUpAuthentication.build_flow(session: session, provider: provider) + + return unless step_up_auth_flow.enabled_by_config? + + # This method will set the state to 'requested' in the session + step_up_auth_flow.request! + end + + 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 + end + + def current_user_from(env) + env['warden']&.user + end + + def current_provider_from(env) + env['omniauth.strategy']&.name + end + + def request_param_step_up_auth_scope_from(env) + env.dig('rack.request.query_hash', 'step_up_auth_scope').to_s + end + + def session_from(env) + env['rack.session'] + end + end + end + end + end +end diff --git a/lib/gitlab/auth/oidc/step_up_authentication.rb b/lib/gitlab/auth/oidc/step_up_authentication.rb new file mode 100644 index 0000000000000000000000000000000000000000..c4c8e1f665a188d9a98d759bb32eee2931290698 --- /dev/null +++ b/lib/gitlab/auth/oidc/step_up_authentication.rb @@ -0,0 +1,117 @@ +# frozen_string_literal: true + +module Gitlab + module Auth + module Oidc + # Handles step-up authentication configuration and validation for OAuth providers + # + # This module manages the configuration and validation of step-up authentication + # requirements for OAuth providers, particularly focusing on admin mode access. + module StepUpAuthentication + STEP_UP_AUTH_SCOPE_ADMIN_MODE = :admin_mode + + 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) + oauth_providers.any? do |provider| + enabled_for_provider?(provider_name: provider, scope: scope) + end + end + + # Checks if step-up authentication configuration exists for a provider name + # + # @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) + has_required_claims?(provider_name, scope) + end + + # Verifies if step-up authentication has succeeded for any provider + # with the 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) + step_up_auth_flows = + session&.dig('omniauth_step_up_auth') + .to_h + .flat_map do |provider, step_up_auth_object| + step_up_auth_object.map do |step_up_auth_scope, _| + ::Gitlab::Auth::Oidc::StepUpAuthenticationFlow.new( + session: session, + provider: provider, + scope: step_up_auth_scope + ) + end + end + step_up_auth_flows + .select do |step_up_auth_flow| + step_up_auth_flow.scope.to_s == scope.to_s + end + .select(&:enabled_by_config?) + .any?(&:succeeded?) + end + + # Validates if all step-up authentication conditions are met + # + # @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) + required_conditions_fulfilled?(oauth_extra_metadata: oauth_extra_metadata, provider: provider, scope: scope) + end + + def build_flow(provider:, session:, scope: STEP_UP_AUTH_SCOPE_ADMIN_MODE) + Gitlab::Auth::Oidc::StepUpAuthenticationFlow.new(provider: provider, scope: scope, session: session) + end + + # Slices the relevant ID token claims from the provided OAuth raw information. + # + # @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. + # @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) + relevant_id_token_claims = (get_id_token_claims_required_conditions(provider, scope) || {}).keys + oauth_raw_info.slice(*relevant_id_token_claims) + end + + private + + def oauth_providers + Gitlab::Auth::OAuth::Provider.providers || [] + end + + def has_required_claims?(provider_name, scope) + get_id_token_claims_required_conditions(provider_name, scope).present? + end + + def get_id_token_claims_required_conditions(provider_name, scope) + dig_provider_config(provider_name, scope, 'required') + end + + def dig_provider_config(provider_name, scope, claim_type) + Gitlab::Auth::OAuth::Provider + .config_for(provider_name.to_s) + &.dig('step_up_auth', scope.to_s, 'id_token', claim_type) + end + + def required_conditions_fulfilled?(oauth_extra_metadata:, provider:, scope:) + conditions = get_id_token_claims_required_conditions(provider, scope) + return false if conditions.blank? + + raw_info = oauth_extra_metadata.presence || {} + subset?(raw_info, conditions) + end + + def subset?(hash, subset_hash) + hash.with_indifferent_access >= subset_hash.with_indifferent_access + end + end + end + 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 new file mode 100644 index 0000000000000000000000000000000000000000..a45b1537b5ce1540f465f1f6340584c928f1aa16 --- /dev/null +++ b/lib/gitlab/auth/oidc/step_up_authentication_flow.rb @@ -0,0 +1,86 @@ +# frozen_string_literal: true + +module Gitlab + module Auth + module Oidc + class StepUpAuthenticationFlow + STATE_REQUESTED = :requested + STATE_SUCCEEDED = :succeeded + STATE_FAILED = :failed + + attr_reader :session, :provider, :scope + + def initialize(session:, provider:, scope:) + @session = session + @provider = provider + @scope = scope + end + + def requested? + state.to_s == STATE_REQUESTED.to_s + end + + def succeeded? + state.to_s == STATE_SUCCEEDED.to_s + end + + def rejected? + state.to_s == STATE_FAILED.to_s + end + + def enabled_by_config? + ::Gitlab::Auth::Oidc::StepUpAuthentication.enabled_for_provider?(provider_name: provider, scope: scope) + end + + def evaluate!(oidc_id_token_claims) + oidc_id_token_claims = + ::Gitlab::Auth::Oidc::StepUpAuthentication.slice_relevant_id_token_claims( + oauth_raw_info: oidc_id_token_claims, + provider: provider, + scope: scope + ) + + if conditions_fulfilled?(oidc_id_token_claims) + succeed! + else + fail! + end + end + + def request! + update_session_state(STATE_REQUESTED) + end + + def succeed! + update_session_state(STATE_SUCCEEDED) + end + + def fail! + update_session_state(STATE_FAILED) + end + + private + + def state + provider_scope_session_data&.[]('state').to_s.presence + end + + def provider_scope_session_data + session.dig('omniauth_step_up_auth', provider.to_s, scope.to_s) + end + + def update_session_state(new_state) + session['omniauth_step_up_auth'] ||= {} + session['omniauth_step_up_auth'][provider.to_s] ||= {} + session['omniauth_step_up_auth'][provider.to_s][scope.to_s] ||= {} + session['omniauth_step_up_auth'][provider.to_s][scope.to_s]['state'] = new_state.to_s + end + + 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 + end + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 049196ff533a6b27ea52941abdb0c473a9502be5..1afed3351159bef1ab44c00cf20a964425af9860 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -58063,6 +58063,9 @@ msgstr "" msgid "Step 4." msgstr "" +msgid "Step-up auth not successful" +msgstr "" + msgid "Still loading..." msgstr "" diff --git a/spec/controllers/concerns/enforces_admin_authentication_spec.rb b/spec/controllers/concerns/enforces_admin_authentication_spec.rb index 331e1ada73b2ab20b4b7bab26778c96ea231b857..9b2b2fbdb235a0bc38b0304f58494b3d2d978588 100644 --- a/spec/controllers/concerns/enforces_admin_authentication_spec.rb +++ b/spec/controllers/concerns/enforces_admin_authentication_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe EnforcesAdminAuthentication do +RSpec.describe EnforcesAdminAuthentication, feature_category: :system_access do let(:user) { create(:user) } before do diff --git a/spec/controllers/concerns/enforces_step_up_authentication_spec.rb b/spec/controllers/concerns/enforces_step_up_authentication_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..76b7f8e23c82c9e9d1174379232fcf5956f3edd1 --- /dev/null +++ b/spec/controllers/concerns/enforces_step_up_authentication_spec.rb @@ -0,0 +1,91 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe EnforcesStepUpAuthentication, feature_category: :system_access do + include AdminModeHelper + + controller(ApplicationController) do + include EnforcesStepUpAuthentication + + def index + head :ok + end + end + + let_it_be(:user) { create(:user) } + + subject do + get :index + response + end + + before do + sign_in(user) + end + + shared_examples 'passing check for step up authentication' do + it { is_expected.to have_gitlab_http_status(:ok) } + end + + shared_examples 'redirecting to new_admin_session_path' do + it { is_expected.to redirect_to(new_admin_session_path) } + + 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 + + it_behaves_like 'passing check for step up authentication' + end + end + + describe '#enforce_step_up_authentication' do + using RSpec::Parameterized::TableSyntax + + let(:provider_without_step_up_auth) { GitlabSettings::Options.new(name: 'google_oauth2') } + let(:provider_with_step_up_auth) do + GitlabSettings::Options.new( + name: 'openid_connect', + step_up_auth: { + admin_mode: { + id_token: { + required: { + acr: 'gold' + } + } + } + } + ) + end + + let(:step_up_auth_session_succeeded) do + { 'openid_connect' => { 'admin_mode' => { 'state' => 'succeeded' } } } + end + + let(:step_up_auth_session_failed) { { 'openid_connect' => { 'admin_mode' => { 'state' => 'failed' } } } } + + before do + stub_omniauth_setting(enabled: true, providers: oauth_providers) + + session.merge!(omniauth_step_up_auth: step_up_auth_session) + end + + # rubocop:disable Layout/LineLength -- Avoid formatting table to keep oneline table syntax + where(:oauth_providers, :step_up_auth_session, :expected_examples) do + [] | nil | 'passing check for step up authentication' + [] | ref(:step_up_auth_session_succeeded) | 'passing check for step up authentication' + [] | ref(:step_up_auth_session_failed) | 'passing check for step up authentication' + [ref(:provider_without_step_up_auth)] | nil | 'passing check for step up authentication' + + [ref(:provider_with_step_up_auth), ref(:provider_without_step_up_auth)] | nil | 'redirecting to new_admin_session_path' + [ref(:provider_with_step_up_auth)] | ref(:step_up_auth_session_succeeded) | 'passing check for step up authentication' + [ref(:provider_with_step_up_auth)] | ref(:step_up_auth_session_failed) | 'redirecting to new_admin_session_path' + end + # rubocop:enable Layout/LineLength + + with_them do + it_behaves_like params[:expected_examples] + end + end +end diff --git a/spec/controllers/omniauth_callbacks_controller_spec.rb b/spec/controllers/omniauth_callbacks_controller_spec.rb index 562377490e8f6afefd3a0265bfc6134657b0eb37..b5c8167d1213742ce027b423b0e34d45ff0ae98f 100644 --- a/spec/controllers/omniauth_callbacks_controller_spec.rb +++ b/spec/controllers/omniauth_callbacks_controller_spec.rb @@ -809,6 +809,95 @@ end it_behaves_like "sets provider_2FA session variable according to bypass_two_factor return value" + + context 'for step-up authentication' do + context 'with different step-up authentication configurations' do + using RSpec::Parameterized::TableSyntax + + let(:ommiauth_provider_config_with_step_up_auth) do + GitlabSettings::Options.new( + name: "openid_connect", + step_up_auth: { + admin_mode: { + id_token: { + required: required_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' => { 'admin_mode' => { 'state' => 'requested' } } } + + stub_omniauth_setting(enabled: true, auto_link_user: true, block_auto_created_users: false, providers: [ommiauth_provider_config_with_step_up_auth]) + end + + where(:required_id_token_claims, :mock_auth_hash_extra_raw_info, :step_up_auth_authenticated) do + { claim_1: 'gold' } | { claim_1: 'gold' } | 'succeeded' + { claim_1: 'gold' } | { claim_1: 'gold', claim_2: 'mfa' } | 'succeeded' + { claim_1: 'gold' } | { claim_1: 'gold', claim_2: 'other_amr' } | 'succeeded' + { claim_1: 'gold' } | { claim_1: 'silver' } | 'failed' + { claim_1: 'gold' } | { claim_1: 'silver', claim_2: 'other_amr' } | 'failed' + { claim_1: 'gold' } | { claim_1: nil } | 'failed' + { claim_1: 'gold' } | { claim_3: 'other_value' } | 'failed' + { claim_1: 'gold' } | {} | 'failed' + end + + with_them do + context 'when user is signed in' do + before do + sign_in user + end + + it 'evaluates step-up authentication conditions and stores result in session' do + get provider + + expect(session.to_h.dig('omniauth_step_up_auth', 'openid_connect', 'admin_mode', 'state')) + .to eq step_up_auth_authenticated + end + + context 'when feature flag :omniauth_step_up_auth_for_admin_mode disabled' do + before do + stub_feature_flags(omniauth_step_up_auth_for_admin_mode: 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 + + context 'when user is not signed in' do + before do + 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 + + context 'without step-up authentication configuration' do + let(:ommiauth_provider_config_with_step_up_auth) { GitlabSettings::Options.new(name: "openid_connect") } + + it 'does not add session key "step_up_auth"' do + get provider + + expect(session).not_to include 'step_up_auth' + end + end + end end describe '#saml' do diff --git a/spec/helpers/auth_helper_spec.rb b/spec/helpers/auth_helper_spec.rb index 032ffd280a717046dc917030231d0f63fde79d2d..52f07b3ddcad9bdabedf92ee86f5b4808ae5add0 100644 --- a/spec/helpers/auth_helper_spec.rb +++ b/spec/helpers/auth_helper_spec.rb @@ -708,4 +708,80 @@ def auth_active? .to eq('http://www.w3.org/2001/04/xmlenc#sha256') end end + + describe '#step_up_auth_params' do + using RSpec::Parameterized::TableSyntax + + let(:current_user) { instance_double('User', flipper_id: '1') } + + let(:oidc_setting_with_step_up_auth) do + GitlabSettings::Options.new( + name: "openid_connect", + step_up_auth: { + admin_mode: { + params: { + claims: { acr_values: 'gold' }, + prompt: 'login' + } + } + } + ) + end + + let(:oidc_setting_without_step_up_auth_params) do + GitlabSettings::Options.new(name: "openid_connect", + step_up_auth: { admin_mode: { id_token: { required: { acr: 'gold' } } } }) + end + + let(:oidc_setting_without_step_up_auth) do + GitlabSettings::Options.new(name: "openid_connect") + end + + subject { helper.step_up_auth_params(provider_name, scope) } + + before do + allow(helper).to receive(:current_user).and_return(current_user) + stub_omniauth_setting(enabled: true, providers: omniauth_setting_providers) + end + + # rubocop:disable Layout/LineLength -- Ensure one-line table syntax for better readability + where(:omniauth_setting_providers, :provider_name, :scope, :expected_result) do + [ref(:oidc_setting_with_step_up_auth)] | 'openid_connect' | :admin_mode | { step_up_auth_scope: :admin_mode, 'claims' => '{"acr_values":"gold"}', 'prompt' => 'login' } + [ref(:oidc_setting_with_step_up_auth)] | 'openid_connect' | 'admin_mode' | { step_up_auth_scope: 'admin_mode', 'claims' => '{"acr_values":"gold"}', 'prompt' => 'login' } + [ref(:oidc_setting_with_step_up_auth)] | 'openid_connect' | nil | {} + [ref(:oidc_setting_with_step_up_auth)] | 'missing_provider_name' | :admin_mode | {} + [ref(:oidc_setting_with_step_up_auth)] | nil | nil | {} + + [] | 'openid_connect' | :admin_mode | {} + [ref(:oidc_setting_without_step_up_auth)] | 'openid_connect' | :admin_mode | {} + [ref(:oidc_setting_without_step_up_auth_params)] | 'openid_connect' | :admin_mode | { step_up_auth_scope: :admin_mode } + end + # rubocop:enable Layout/LineLength + + with_them do + it { is_expected.to eq expected_result } + + 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 + + it { is_expected.to eq({}) } + end + end + end + + def omniauth_providers_with_step_up_auth_config(step_up_auth_scope) + auth_providers.map { |provider| Gitlab::Auth::OAuth::Provider.config_for(provider) } + .select { |provider_config| provider_config.dig("step_up_auth", step_up_auth_scope.to_s).present? } + end + + def step_up_auth_params(provider_name, scope) + return {} if Feature.disabled?(:omniauth_step_up_auth_for_admin_mode, current_user) + + provider_config_for_scope = Gitlab::Auth::OAuth::Provider.config_for(provider_name)&.dig('step_up_auth', scope) + return {} if provider_config_for_scope&.dig('enabled').blank? + + provider_config_for_scope&.dig('params')&.transform_values { |v| v.is_a?(Hash) ? v.to_json : v } + 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 new file mode 100644 index 0000000000000000000000000000000000000000..56e41502a2ef951d4ff4a32a59ebc68546648ca4 --- /dev/null +++ b/spec/lib/gitlab/auth/oidc/step_up_auth_before_request_phase_spec.rb @@ -0,0 +1,99 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Auth::Oidc::StepUpAuthBeforeRequestPhase, feature_category: :system_access do + using RSpec::Parameterized::TableSyntax + + let(:env) do + { + 'omniauth.strategy' => class_double(OmniAuth::Strategies, name: provider), + 'rack.request.query_hash' => { 'step_up_auth_scope' => step_up_auth_scope }, + 'rack.session' => session, + 'warden' => instance_double(Warden::Proxy, user: build_stubbed(:user)) + } + end + + let(:session) { {} } + let(:session_with_step_up_state_requested) do + { 'omniauth_step_up_auth' => { 'openid_connect' => { 'admin_mode' => { 'state' => 'requested' } } } } + end + + let(:provider_step_up_auth_name) { provider_step_up_auth.name } + let(:provider_step_up_auth) do + GitlabSettings::Options.new( + name: 'openid_connect', + step_up_auth: { + admin_mode: { + id_token: { + required: { + acr: 'gold' + } + } + } + } + ) + end + + before do + stub_omniauth_setting(enabled: true, providers: [provider_step_up_auth]) + end + + describe '.call' do + subject(:call_middleware) { described_class.call(env) } + + # rubocop:disable Layout/LineLength -- Avoid formatting to keep one-line table syntax + where(:provider, :step_up_auth_scope, :session, :expected_session) do + 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) + ref(:provider_step_up_auth_name) | '' | {} | {} + ref(:provider_step_up_auth_name) | nil | {} | {} + 'other_provider' | 'admin_mode' | {} | {} + nil | 'admin_mode' | {} | {} + nil | nil | {} | {} + end + # rubocop:enable Layout/LineLength + + with_them do + it 'sets the session state to requested' do + call_middleware + + expect(session).to eq(expected_session) + 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 + + it 'does not modify the session' do + expect { call_middleware }.not_to change { session } + 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 } + 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 } + 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 new file mode 100644 index 0000000000000000000000000000000000000000..9f645d21030707b1c819c92e54260d22774f072f --- /dev/null +++ b/spec/lib/gitlab/auth/oidc/step_up_authentication_flow_spec.rb @@ -0,0 +1,121 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Auth::Oidc::StepUpAuthenticationFlow, feature_category: :system_access do + let(:session) { { 'omniauth_step_up_auth' => { provider => { scope => { 'state' => 'requested' } } } } } + let(:provider) { 'openid_connect' } + let(:scope) { 'admin_mode' } + + subject(:flow) { described_class.new(session: session, provider: provider, scope: scope) } + + describe '#requested?' do + context 'when state is requested' do + let(:session) do + { 'omniauth_step_up_auth' => { provider => { scope => { 'state' => 'requested' } } } } + end + + it { is_expected.to be_requested } + end + + context 'when state is not requested' do + let(:session) do + { 'omniauth_step_up_auth' => { provider => { scope => { 'state' => 'succeeded' } } } } + end + + it { is_expected.not_to be_requested } + end + end + + describe '#succeeded?' do + context 'when state is authenticated' do + let(:session) do + { 'omniauth_step_up_auth' => { provider => { scope => { 'state' => 'succeeded' } } } } + end + + it { is_expected.to be_succeeded } + end + + context 'when state is not authenticated' do + let(:session) do + { 'omniauth_step_up_auth' => { provider => { scope => { 'state' => 'requested' } } } } + end + + it { is_expected.not_to be_succeeded } + end + end + + describe '#enabled_by_config?' do + before do + allow(::Gitlab::Auth::Oidc::StepUpAuthentication).to receive(:enabled_for_provider?).and_return(true) + end + + it { is_expected.to be_enabled_by_config } + + context 'when not enabled by config' do + before do + allow(::Gitlab::Auth::Oidc::StepUpAuthentication).to receive(:enabled_for_provider?).and_return(false) + end + + it { is_expected.not_to be_enabled_by_config } + end + end + + describe '#evaluate!' do + let(:oidc_id_token_claims) { { 'claim_1' => 'gold' } } + + subject(:evaluate_flow) { flow.evaluate!(oidc_id_token_claims) } + + context 'when conditions are fulfilled' do + before do + allow(::Gitlab::Auth::Oidc::StepUpAuthentication).to receive(:conditions_fulfilled?).and_return(true) + end + + it 'sets the state to authenticated' do + expect { evaluate_flow }.to change { flow.succeeded? }.from(false).to(true) + end + end + + context 'when conditions are not fulfilled' do + before do + allow(::Gitlab::Auth::Oidc::StepUpAuthentication).to receive(:conditions_fulfilled?).and_return(false) + end + + it 'sets the state to authenticated' do + expect { evaluate_flow }.not_to change { flow.succeeded? } + end + + it 'sets the state to rejected' do + expect { evaluate_flow }.to change { flow.rejected? }.from(false).to(true) + end + end + end + + describe '#request!' do + let(:session) { { 'omniauth_step_up_auth' => { provider => { scope => { 'state' => 'requested' } } } } } + + it 'does not change the state' do + expect { flow.request! }.not_to change { flow.requested? } + end + + context 'when the state is authenticated' do + let(:session) { { 'omniauth_step_up_auth' => { provider => { scope => { 'state' => 'succeeded' } } } } } + + it 'does not change the state' do + expect { flow.request! }.to change { flow.requested? }.from(false).to(true) + end + end + end + + describe '#succeed!' do + it 'sets the state to authenticated' do + expect { flow.succeed! }.to change { flow.succeeded? }.from(false).to(true) + end + end + + describe '#fail!' do + it 'sets the state to rejected' do + expect { flow.fail! }.to change { flow.rejected? }.from(false).to(true) + end + end +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 new file mode 100644 index 0000000000000000000000000000000000000000..c01df2518ef70975104d44028e69d7af405247c7 --- /dev/null +++ b/spec/lib/gitlab/auth/oidc/step_up_authentication_spec.rb @@ -0,0 +1,135 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Auth::Oidc::StepUpAuthentication, feature_category: :system_access do + using RSpec::Parameterized::TableSyntax + + let(:ommiauth_provider_config) do + GitlabSettings::Options.new( + name: "openid_connect", + step_up_auth: { + admin_mode: { + id_token: { + required: required_id_token_claims + } + } + } + ) + end + + let(:auth_hash_openid_connect) do + OmniAuth::AuthHash.new({ + provider: 'openid_connect', + info: { + name: 'mockuser', + email: 'mockuser@example.com', + image: 'mock_user_thumbnail_url' + }, + extra: { + raw_info: { + info: { + name: 'mockuser', + email: 'mockuser@example.com', + image: 'mock_user_thumbnail_url' + }, + **auth_hash_openid_connect_extra_raw_info + } + } + }) + end + + let(:auth_hash_openid_connect_extra_raw_info) { {} } + + describe '.config_exists?' do + let(:auth_hash_other_provider) { OmniAuth::AuthHash.new({ provider: 'other_provider' }) } + + subject { described_class.enabled_for_provider?(provider_name: oauth_auth_hash.provider, scope: scope) } + + before do + stub_omniauth_setting(enabled: true, providers: [ommiauth_provider_config]) + end + + where(:required_id_token_claims, :oauth_auth_hash, :scope, :expected_result) do + { claim_1: 'gold' } | ref(:auth_hash_openid_connect) | :admin_mode | true + { claim_1: 'gold' } | ref(:auth_hash_openid_connect) | :unsupported_scope | false + {} | ref(:auth_hash_openid_connect) | :admin_mode | false + {} | ref(:auth_hash_openid_connect) | 'admin_mode' | false + {} | ref(:auth_hash_openid_connect) | nil | false + {} | ref(:auth_hash_other_provider) | :admin_mode | false + nil | ref(:auth_hash_openid_connect) | :admin_mode | false + end + + with_them do + it { is_expected.to eq expected_result } + end + end + + describe '.conditions_fulfilled?' do + subject do + described_class.conditions_fulfilled?( + oauth_extra_metadata: auth_hash_openid_connect_extra_raw_info, + provider: 'openid_connect', + scope: :admin_mode + ) + end + + before do + stub_omniauth_setting(enabled: true, providers: [ommiauth_provider_config]) + end + + # -- Avoid formatting to ensure one-line table syntax + where(:required_id_token_claims, :auth_hash_openid_connect_extra_raw_info, :expected_result) do + { claim_1: 'gold' } | { claim_1: 'gold' } | true + { 'claim_1' => 'gold' } | { claim_1: 'gold' } | true + { claim_1: 'gold' } | { 'claim_1' => 'gold' } | true + { claim_1: 'gold' } | { claim_1: 'gold', claim_3: 'other_value' } | true + { claim_1: 'gold' } | { claim_1: 'silver' } | false + { claim_1: 'gold' } | { claim_1: ['gold'] } | false + { claim_1: 'gold' } | { claim_1: nil } | false + { claim_1: 'gold' } | { claim_3: 'other_value' } | false + { claim_1: 'gold' } | {} | false + { claim_1: 'gold', claim_3: 'other_value' } | { claim_1: 'gold' } | false + { claim_1: 'gold', claim_3: 'other_value' } | { claim_1: 'gold', claim_3: 'other_value' } | true + { claim_1: ['gold'] } | { claim_1: 'gold' } | false + { claim_1: ['gold'] } | { claim_1: ['gold'] } | true + {} | { claim_1: 'gold' } | false + nil | { claim_1: 'gold' } | false + end + with_them do + it { is_expected.to eq expected_result } + end + end + + describe '.succeeded?' do + let(:required_id_token_claims) { { claim_1: 'gold' } } + + let(:session) { { 'omniauth_step_up_auth' => step_up_auth_session } } + + subject { described_class.succeeded?(session) } + + before do + stub_omniauth_setting(enabled: true, providers: [ommiauth_provider_config]) + end + + # rubocop:disable Layout/LineLength -- Avoid formatting to ensure one-line table syntax + where(:step_up_auth_session, :expected_result) do + lazy { { 'openid_connect' => { 'admin_mode' => { 'state' => 'succeeded' } } } } | true + lazy { { 'openid_connect' => { 'admin_mode' => { 'state' => 'failed' } } } } | false + lazy { { 'other_provider' => { 'admin_mode' => { 'state' => 'succeeded' } } } } | false + lazy { { 'openid_connect' => { 'admin_mode' => { 'state' => 'succeeded' } }, 'other_provider' => { 'admin_mode' => { 'state' => 'failed' } } } } | true + 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 + + nil | false + {} | false + end + # rubocop:enable Layout/LineLength + + with_them do + it { is_expected.to eq expected_result } + end + end +end diff --git a/spec/requests/rack_middlewares/omniauth_spec.rb b/spec/requests/rack_middlewares/omniauth_spec.rb index ae5fddb0ec0e4e713cd9990d9920dc266e7ab559..be4ff6c76c03f672148ceaea985c711130989154 100644 --- a/spec/requests/rack_middlewares/omniauth_spec.rb +++ b/spec/requests/rack_middlewares/omniauth_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe 'OmniAuth Rack middlewares', feature_category: :system_access do + include SessionHelpers + describe 'OmniAuth before_request_phase callback' do it 'increments Prometheus counter' do expect { post('/users/auth/google_oauth2') } @@ -13,4 +15,85 @@ }.by(1) end end + + describe 'OmniAuth before_request_phase callback for step-up authentication' do + let_it_be(:user) { create(:user) } + + let(:oauth_provider_config_with_step_up_auth) do + GitlabSettings::Options.new( + name: "openid_connect", + step_up_auth: { + admin_mode: { + id_token: { + required: { + acr: 'gold' + } + } + } + } + ) + end + + let(:step_up_auth_scope) { 'admin_mode' } + + let(:expected_step_up_auth_session_hash) do + { 'omniauth_step_up_auth' => { 'openid_connect' => { 'admin_mode' => { 'state' => 'requested' } } } } + end + + subject(:response_session) do + post("/users/auth/openid_connect?step_up_auth_scope=#{step_up_auth_scope}") + session.to_h + end + + before do + stub_omniauth_setting(enabled: true, providers: [oauth_provider_config_with_step_up_auth]) + + sign_in(user) + end + + it { is_expected.to include(expected_step_up_auth_session_hash) } + + context 'with blank step_up_auth_scope param' do + let(:step_up_auth_scope) { '' } + + it { is_expected.not_to include('omniauth_step_up_auth') } + end + + context 'with invalid step_up_auth_scope param' do + let(:step_up_auth_scope) { 'invalid_scope' } + + it { is_expected.not_to include('omniauth_step_up_auth') } + end + + context 'without step_up_auth_scope param' do + subject(:response_session) do + post('/users/auth/openid_connect') + session.to_h + end + + it { is_expected.not_to include('omniauth_step_up_auth') } + end + + context 'when session for omniauth_step_up_auth is available', :clean_gitlab_redis_sessions do + before do + stub_session( + session_data: { + 'omniauth_step_up_auth' => { 'openid_connect' => { 'admin_mode' => { 'state' => 'requested' } } } + }, + user_id: user.id + ) + end + + it { is_expected.to include(expected_step_up_auth_session_hash) } + end + + context 'when requesting step-up auth for unconfigured provider' do + subject(:response_session) do + post('/users/auth/auth0?step_up_auth_scope=admin_mode') + session.to_h + end + + it { is_expected.not_to include('omniauth_step_up_auth') } + end + end end diff --git a/spec/views/admin/sessions/new.html.haml_spec.rb b/spec/views/admin/sessions/new.html.haml_spec.rb index f244c124f5f26f63edd4956c7c128ef4a22ece88..4707abc8e9116a01bf0942bd46edf24f5f15c3c0 100644 --- a/spec/views/admin/sessions/new.html.haml_spec.rb +++ b/spec/views/admin/sessions/new.html.haml_spec.rb @@ -2,9 +2,13 @@ require 'spec_helper' -RSpec.describe 'admin/sessions/new.html.haml' do +RSpec.describe 'admin/sessions/new.html.haml', feature_category: :system_access do + include RenderedHtml + let(:user) { create(:admin) } + let(:page) { rendered_html } + before do disable_all_signin_methods @@ -39,6 +43,12 @@ allow(view).to receive(:password_authentication_enabled_for_web?).and_return(true) end + let(:openid_connect_button_action_url) do + URI(rendered_html.find_button('Openid Connect').ancestor('form')[:action]) + end + + let(:openid_connect_button_action_url_query) { Rack::Utils.parse_query(openid_connect_button_action_url.query) } + it 'shows omniauth form' do render @@ -48,6 +58,54 @@ end expect(rendered).to have_css('.js-oauth-login') end + + context 'when step-up auth config is set' do + let(:oidc_step_up_auth_options) do + GitlabSettings::Options.new( + name: "openid_connect", + step_up_auth: { + admin_mode: { + params: { + claims: { acr_values: 'gold' } + } + } + } + ) + end + + let(:oidc_step_up_auth_options_without_params) do + GitlabSettings::Options.new(name: "openid_connect", step_up_auth: { admin_mode: {} }) + end + + before do + stub_omniauth_setting(enabled: true, providers: [oidc_step_up_auth_options]) + + allow(view).to receive(:omniauth_enabled?).and_return(true) + allow(view).to receive(:auth_providers).and_return(['openid_connect']) + end + + it 'includes additional params related to step-up auth in form action url' do + render + + expect(rendered).to have_button('Openid Connect') + + expect(openid_connect_button_action_url).to have_attributes(path: '/users/auth/openid_connect') + expect(openid_connect_button_action_url_query).to include('claims' => '{"acr_values":"gold"}') + 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 + + it 'does not include additional params related to step-up auth in form action url' do + render + + expect(rendered).to have_button('Openid Connect') + expect(openid_connect_button_action_url).to have_attributes(path: '/users/auth/openid_connect', query: nil) + end + end + end end context 'ldap authentication' do