From aa9748d937ca186a9cd9cdca5d3ba1600c25fc80 Mon Sep 17 00:00:00 2001 From: Gerardo Navarro Date: Thu, 18 Jul 2024 08:33:30 +0200 Subject: [PATCH 01/15] OIDC Step-up auth: Enhanced reauth for admin mode [POC] --- app/controllers/admin/application_controller.rb | 15 +++++++++++++++ app/controllers/admin/sessions_controller.rb | 2 ++ .../concerns/enforces_admin_authentication.rb | 8 ++++++++ ...page_with_step_up_authentication_controller.rb | 9 +++++++++ app/controllers/omniauth_callbacks_controller.rb | 9 +++++++++ app/views/admin/sessions/new.html.haml | 8 ++++++++ config/routes/dashboard.rb | 3 +++ locale/gitlab.pot | 3 +++ 8 files changed, 57 insertions(+) create mode 100644 app/controllers/dashboard/page_with_step_up_authentication_controller.rb diff --git a/app/controllers/admin/application_controller.rb b/app/controllers/admin/application_controller.rb index bab2d5639a7943..1641d8b759c463 100644 --- a/app/controllers/admin/application_controller.rb +++ b/app/controllers/admin/application_controller.rb @@ -6,7 +6,22 @@ class Admin::ApplicationController < ApplicationController include EnforcesAdminAuthentication + before_action :check_current_user_auth_mode + layout 'admin' + + def check_current_user_auth_mode + # This will be defined by the GitLab config + allowed_acr_values = ['gold'] + + return if allowed_acr_values.include?(session[:current_user_auth_mode][:authentication_context_class]) + + current_user_mode.disable_admin_mode! + redirect_to( + new_admin_session_path, + notice: _('Re-authentication with higher authentication context required') + ) + end end Admin::ApplicationController.prepend_mod_with('Admin::ApplicationController') diff --git a/app/controllers/admin/sessions_controller.rb b/app/controllers/admin/sessions_controller.rb index 2ac204bd9d0543..c4a9f30e697ded 100644 --- a/app/controllers/admin/sessions_controller.rb +++ b/app/controllers/admin/sessions_controller.rb @@ -7,6 +7,8 @@ class Admin::SessionsController < ApplicationController before_action :user_is_admin! + # skip_before_action :check_current_user_auth_mode, only: [:new, :create] + feature_category :system_access def new diff --git a/app/controllers/concerns/enforces_admin_authentication.rb b/app/controllers/concerns/enforces_admin_authentication.rb index 94c0e98c91a7a5..07a779ba7f72c9 100644 --- a/app/controllers/concerns/enforces_admin_authentication.rb +++ b/app/controllers/concerns/enforces_admin_authentication.rb @@ -17,6 +17,10 @@ def authenticate_admin! return render_404 unless current_user.admin? return unless Gitlab::CurrentSettings.admin_mode + # if step_up_authentication_required? + # return + # end + unless current_user_mode.admin_mode? current_user_mode.request_admin_mode! store_location_for(:redirect, request.fullpath) if storable_location? @@ -27,4 +31,8 @@ def authenticate_admin! def storable_location? request.path != new_admin_session_path end + + def step_up_authentication_required? + true + end end diff --git a/app/controllers/dashboard/page_with_step_up_authentication_controller.rb b/app/controllers/dashboard/page_with_step_up_authentication_controller.rb new file mode 100644 index 00000000000000..ed92d3edfe9c92 --- /dev/null +++ b/app/controllers/dashboard/page_with_step_up_authentication_controller.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module Dashboard + class PageWithStepUpAuthenticationController < ApplicationController + def index + render plain: "Hello, world!" + end + end +end diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index 3af432cce1df61..ea9ff2b7c4d19e 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -144,6 +144,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') + set_current_user_auth_mode + if Gitlab::CurrentSettings.admin_mode return admin_mode_flow(auth_module::User) if current_user_mode.admin_mode_requested? end @@ -345,6 +347,13 @@ def store_redirect_fragment(redirect_fragment) end end + def set_current_user_auth_mode + session[:current_user_auth_mode] = { + authentication_context_class: oauth.extra.dig('raw_info', 'acr'), + authentication_context_class_expiration: oauth.extra.dig('raw_info', 'exp') + } + end + def admin_mode_flow(auth_user_class) auth_user = build_auth_user(auth_user_class) diff --git a/app/views/admin/sessions/new.html.haml b/app/views/admin/sessions/new.html.haml index 304266c674a3bd..4d89178286d409 100644 --- a/app/views/admin/sessions/new.html.haml +++ b/app/views/admin/sessions/new.html.haml @@ -16,3 +16,11 @@ - if omniauth_enabled? && button_based_providers_enabled? = render 'devise/shared/omniauth_box', render_remember_me: false +%hr + +.row.gl-mt-5.justify-content-center + .col-md-5 + = render 'devise/shared/omniauth_provider_button', + href: omniauth_authorize_path(:user, :openid_connect, acr_values: 'gold'), + provider: :openid_connect_with_acr_value_gold, + data: { testid: 'step-up-auth-admin-button' } \ No newline at end of file diff --git a/config/routes/dashboard.rb b/config/routes/dashboard.rb index 6c979b959907f9..8103c744a4e2e8 100644 --- a/config/routes/dashboard.rb +++ b/config/routes/dashboard.rb @@ -34,6 +34,9 @@ get :contributed, :personal, :member, :inactive, to: 'projects#index' end end + + # get :page_with_step_up_authentications, to: 'dashboard/page_with_step_up_authentication#index' + resources :page_with_step_up_authentication, only: [:index] end root to: "dashboard/projects#index" diff --git a/locale/gitlab.pot b/locale/gitlab.pot index ec653620dd616c..42db79bac126bc 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -44906,6 +44906,9 @@ msgstr "" msgid "Re-authentication required" msgstr "" +msgid "Re-authentication with higher authentication context required" +msgstr "" + msgid "Re-import" msgstr "" -- GitLab From 6caf0bb18febc20f8b5257cfdf0155320e18d93f Mon Sep 17 00:00:00 2001 From: Gerardo Navarro Date: Thu, 15 Aug 2024 15:32:39 +0200 Subject: [PATCH 02/15] Pass acr value to IdP via individual claim parameter --- app/views/admin/sessions/new.html.haml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/app/views/admin/sessions/new.html.haml b/app/views/admin/sessions/new.html.haml index 4d89178286d409..3f999f7d58a434 100644 --- a/app/views/admin/sessions/new.html.haml +++ b/app/views/admin/sessions/new.html.haml @@ -23,4 +23,11 @@ = render 'devise/shared/omniauth_provider_button', href: omniauth_authorize_path(:user, :openid_connect, acr_values: 'gold'), provider: :openid_connect_with_acr_value_gold, - data: { testid: 'step-up-auth-admin-button' } \ No newline at end of file + data: { testid: 'step-up-auth-admin-button' } + +.row.gl-mt-5.justify-content-center + .col-md-5 + = render 'devise/shared/omniauth_provider_button', + href: omniauth_authorize_path(:user, :openid_connect, claims: { id_token: { acr: {essential: true, values: ["gold"] } } }.to_json), + provider: :openid_connect_with_param_claims_id_token_acr_values_gold, + data: { testid: 'step-up-auth-admin-button-2' } \ No newline at end of file -- GitLab From 4042f53189df931182eb2c4475ddc42aece54b9b Mon Sep 17 00:00:00 2001 From: Gerardo Navarro Date: Thu, 15 Aug 2024 17:01:57 +0200 Subject: [PATCH 03/15] Propose configuration for step-up auth in `gitlab.yml` - Integrate the configuration in view to trigger step-up auth in IdP - Integrate config step-up auth when checking if the correct auth context is set for the admin area --- .../concerns/enforces_admin_authentication.rb | 4 --- app/helpers/auth_helper.rb | 8 +++++ app/views/admin/sessions/new.html.haml | 15 --------- .../devise/shared/_omniauth_box.html.haml | 2 +- config/gitlab.yml.example | 33 +++++++++++++++++++ 5 files changed, 42 insertions(+), 20 deletions(-) diff --git a/app/controllers/concerns/enforces_admin_authentication.rb b/app/controllers/concerns/enforces_admin_authentication.rb index 07a779ba7f72c9..bcba841f5d355a 100644 --- a/app/controllers/concerns/enforces_admin_authentication.rb +++ b/app/controllers/concerns/enforces_admin_authentication.rb @@ -17,10 +17,6 @@ def authenticate_admin! return render_404 unless current_user.admin? return unless Gitlab::CurrentSettings.admin_mode - # if step_up_authentication_required? - # return - # end - unless current_user_mode.admin_mode? current_user_mode.request_admin_mode! store_location_for(:redirect, request.fullpath) if storable_location? diff --git a/app/helpers/auth_helper.rb b/app/helpers/auth_helper.rb index c842842f023ce3..68c86cebbde008 100644 --- a/app/helpers/auth_helper.rb +++ b/app/helpers/auth_helper.rb @@ -154,6 +154,14 @@ def button_based_providers_enabled? enabled_button_based_providers.any? end + def step_up_auth_params(provider_name, scope) + provider_config = Gitlab::Auth::OAuth::Provider.config_for(provider_name) + provider_config_for_scope = provider_config.fetch('step_up_auth', {})[scope] + return {} unless provider_config_for_scope&.fetch('enabled', false) + + provider_config_for_scope.fetch('params', false).transform_values { |v| v.to_h.to_json } + 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 3f999f7d58a434..304266c674a3bd 100644 --- a/app/views/admin/sessions/new.html.haml +++ b/app/views/admin/sessions/new.html.haml @@ -16,18 +16,3 @@ - if omniauth_enabled? && button_based_providers_enabled? = render 'devise/shared/omniauth_box', render_remember_me: false -%hr - -.row.gl-mt-5.justify-content-center - .col-md-5 - = render 'devise/shared/omniauth_provider_button', - href: omniauth_authorize_path(:user, :openid_connect, acr_values: 'gold'), - provider: :openid_connect_with_acr_value_gold, - data: { testid: 'step-up-auth-admin-button' } - -.row.gl-mt-5.justify-content-center - .col-md-5 - = render 'devise/shared/omniauth_provider_button', - href: omniauth_authorize_path(:user, :openid_connect, claims: { id_token: { acr: {essential: true, values: ["gold"] } } }.to_json), - provider: :openid_connect_with_param_claims_id_token_acr_values_gold, - data: { testid: 'step-up-auth-admin-button-2' } \ No newline at end of file diff --git a/app/views/devise/shared/_omniauth_box.html.haml b/app/views/devise/shared/_omniauth_box.html.haml index 19296a7c006af3..9c7bc89629f51b 100644 --- a/app/views/devise/shared/_omniauth_box.html.haml +++ b/app/views/devise/shared/_omniauth_box.html.haml @@ -6,7 +6,7 @@ .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), + href: omniauth_authorize_path(:user, provider, **step_up_auth_params(provider, :admin_mode)), provider: provider, data: { testid: test_id_for_provider(provider) } diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index cbef4a662f350c..1c03d06461aefe 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -1152,6 +1152,39 @@ production: &base # client_secret: 'YOUR_AUTH0_CLIENT_SECRET', # domain: 'YOUR_AUTH0_DOMAIN', # scope: 'openid profile email' } } + # + # - { name: 'openid_connect', + # label: 'OpenID Connect', + # args: { + # "name":"openid_connect", + # "scope":["openid", + # "profile", + # "email"],"response_type":"code", + # "issuer":"http://localhost:8080/realms/step-up-auth-gitlab-realm", + # "client_auth_method":"query", + # "discovery":false,"uid_field":"preferred_username", + # "pkce":true,"allow_authorize_params":["acr_values", "claims"], + # "client_options": { + # ... + # } + # }, + # step_up_auth: { + # admin_mode: { + # enabled: true, + # documentation_link: "https://openid.net/specs/openid-connect-core-1_0.html#IDToken", + # params: { + # claims: { id_token: { acr: { essential: true, values: ['gold'] } } } + # } + # }, + # group_scope: { + # enabled: true, + # documentation_link: "https://openid.net/specs/openid-connect-core-1_0.html#IDToken", + # params: { + # claims: { id_token: { acr: { essential: false, values: ['silver'] } } } + # } + # } + # } + # } # FortiAuthenticator settings forti_authenticator: -- GitLab From 62cec3b193cc9daa6eb096a63381971b16004d79 Mon Sep 17 00:00:00 2001 From: Gerardo Navarro Date: Fri, 16 Aug 2024 18:42:42 +0200 Subject: [PATCH 04/15] Integrate step-up auth related data to CurrentUserMode - Add new config for required / expected values per omniauth provider, e.g. the required `acr` value - Ensure that omniauth metadata results are integrated in the session - Retrieve the omniauth metadata from the current session and check if the exepcted values from the configuration are included --- .../admin/application_controller.rb | 23 +++++++++++++++---- .../omniauth_callbacks_controller.rb | 8 +++---- config/gitlab.yml.example | 3 +++ lib/gitlab/auth/current_user_mode.rb | 23 +++++++++++++++++++ 4 files changed, 49 insertions(+), 8 deletions(-) diff --git a/app/controllers/admin/application_controller.rb b/app/controllers/admin/application_controller.rb index 1641d8b759c463..58c60b5e8d96b5 100644 --- a/app/controllers/admin/application_controller.rb +++ b/app/controllers/admin/application_controller.rb @@ -11,10 +11,9 @@ class Admin::ApplicationController < ApplicationController layout 'admin' def check_current_user_auth_mode - # This will be defined by the GitLab config - allowed_acr_values = ['gold'] - - return if allowed_acr_values.include?(session[:current_user_auth_mode][:authentication_context_class]) + auth_mode_metadata = current_user_mode.get_auth_mode_metadata + step_up_auth_extra_conditions = step_up_auth_extra_conditions(auth_mode_metadata[:provider], :admin_mode) + return if auth_mode_metadata[:provider_extra_values] >= step_up_auth_extra_conditions current_user_mode.disable_admin_mode! redirect_to( @@ -22,6 +21,22 @@ def check_current_user_auth_mode notice: _('Re-authentication with higher authentication context required') ) end + + private + + def step_up_auth_extra_conditions(provider_name, scope) + provider_config = Gitlab::Auth::OAuth::Provider.config_for(provider_name) + provider_config_for_scope = provider_config.fetch('step_up_auth', {})[scope] + return {} unless provider_config_for_scope&.fetch('enabled', false) + + provider_config_for_scope.fetch('required_extra_values', nil) + end + + def compare_auth_mode(current_user_mode_session_data, step_up_auth_extra_conditions) + return false unless current_user_mode_session_data || step_up_auth_extra_conditions + + current_user_mode_session_data >= step_up_auth_extra_conditions + end end Admin::ApplicationController.prepend_mod_with('Admin::ApplicationController') diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index ea9ff2b7c4d19e..14d05122d10f7e 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -348,10 +348,10 @@ def store_redirect_fragment(redirect_fragment) end def set_current_user_auth_mode - session[:current_user_auth_mode] = { - authentication_context_class: oauth.extra.dig('raw_info', 'acr'), - authentication_context_class_expiration: oauth.extra.dig('raw_info', 'exp') - } + current_user_mode.set_auth_mode_metadata( + provider: oauth.provider, + provider_extra_values: oauth.extra.raw_info + ) end def admin_mode_flow(auth_user_class) diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index 1c03d06461aefe..2dad3ff3565194 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -1172,6 +1172,9 @@ production: &base # admin_mode: { # enabled: true, # documentation_link: "https://openid.net/specs/openid-connect-core-1_0.html#IDToken", + # required_extra_values: { + # acr: 'gold' + # }, # params: { # claims: { id_token: { acr: { essential: true, values: ['gold'] } } } # } diff --git a/lib/gitlab/auth/current_user_mode.rb b/lib/gitlab/auth/current_user_mode.rb index 7354574248ccd0..d5ff0db133b857 100644 --- a/lib/gitlab/auth/current_user_mode.rb +++ b/lib/gitlab/auth/current_user_mode.rb @@ -22,6 +22,8 @@ class CurrentUserMode ADMIN_MODE_REQUESTED_TIME_KEY = :admin_mode_requested MAX_ADMIN_MODE_TIME = 6.hours ADMIN_MODE_REQUESTED_GRACE_PERIOD = 5.minutes + AUTH_MODE_PROVIDER_KEY = :auth_mode_auth_provider + AUTH_MODE_PROVIDER_EXTRA_VALUES_KEY = :auth_mode_auth_provider_extra_values class << self # Admin mode activation requires storing a flag in the user session. Using this @@ -160,6 +162,27 @@ def current_session_data end strong_memoize_attr :current_session_data + def authentication_context + current_session_data[:authentication_context_class] + end + + def set_auth_mode_metadata(provider:, provider_extra_values:) + current_session_data[AUTH_MODE_PROVIDER_KEY] = provider + current_session_data[AUTH_MODE_PROVIDER_EXTRA_VALUES_KEY] = provider_extra_values + end + + def get_auth_mode_metadata + { + provider: current_session_data[AUTH_MODE_PROVIDER_KEY], + provider_extra_values: current_session_data[AUTH_MODE_PROVIDER_EXTRA_VALUES_KEY] + } + end + + def required_auth_mode + # For the correct implemetation, we should extract this in a separate class + { required_acr_values: Gitlab.config.admin_mode&.omniauth&.providers&.openid_connect&.required_acr_values } + end + private attr_reader :user -- GitLab From ec85882170f029182b7c3e722f371506941cf726 Mon Sep 17 00:00:00 2001 From: Gerardo Navarro Date: Sat, 17 Aug 2024 08:48:10 +0200 Subject: [PATCH 05/15] Clean up poc changes --- app/controllers/admin/sessions_controller.rb | 2 -- .../concerns/enforces_admin_authentication.rb | 4 ---- .../page_with_step_up_authentication_controller.rb | 9 --------- config/routes/dashboard.rb | 3 --- lib/gitlab/auth/current_user_mode.rb | 9 --------- 5 files changed, 27 deletions(-) delete mode 100644 app/controllers/dashboard/page_with_step_up_authentication_controller.rb diff --git a/app/controllers/admin/sessions_controller.rb b/app/controllers/admin/sessions_controller.rb index c4a9f30e697ded..2ac204bd9d0543 100644 --- a/app/controllers/admin/sessions_controller.rb +++ b/app/controllers/admin/sessions_controller.rb @@ -7,8 +7,6 @@ class Admin::SessionsController < ApplicationController before_action :user_is_admin! - # skip_before_action :check_current_user_auth_mode, only: [:new, :create] - feature_category :system_access def new diff --git a/app/controllers/concerns/enforces_admin_authentication.rb b/app/controllers/concerns/enforces_admin_authentication.rb index bcba841f5d355a..94c0e98c91a7a5 100644 --- a/app/controllers/concerns/enforces_admin_authentication.rb +++ b/app/controllers/concerns/enforces_admin_authentication.rb @@ -27,8 +27,4 @@ def authenticate_admin! def storable_location? request.path != new_admin_session_path end - - def step_up_authentication_required? - true - end end diff --git a/app/controllers/dashboard/page_with_step_up_authentication_controller.rb b/app/controllers/dashboard/page_with_step_up_authentication_controller.rb deleted file mode 100644 index ed92d3edfe9c92..00000000000000 --- a/app/controllers/dashboard/page_with_step_up_authentication_controller.rb +++ /dev/null @@ -1,9 +0,0 @@ -# frozen_string_literal: true - -module Dashboard - class PageWithStepUpAuthenticationController < ApplicationController - def index - render plain: "Hello, world!" - end - end -end diff --git a/config/routes/dashboard.rb b/config/routes/dashboard.rb index 8103c744a4e2e8..6c979b959907f9 100644 --- a/config/routes/dashboard.rb +++ b/config/routes/dashboard.rb @@ -34,9 +34,6 @@ get :contributed, :personal, :member, :inactive, to: 'projects#index' end end - - # get :page_with_step_up_authentications, to: 'dashboard/page_with_step_up_authentication#index' - resources :page_with_step_up_authentication, only: [:index] end root to: "dashboard/projects#index" diff --git a/lib/gitlab/auth/current_user_mode.rb b/lib/gitlab/auth/current_user_mode.rb index d5ff0db133b857..ab4ab49120768a 100644 --- a/lib/gitlab/auth/current_user_mode.rb +++ b/lib/gitlab/auth/current_user_mode.rb @@ -162,10 +162,6 @@ def current_session_data end strong_memoize_attr :current_session_data - def authentication_context - current_session_data[:authentication_context_class] - end - def set_auth_mode_metadata(provider:, provider_extra_values:) current_session_data[AUTH_MODE_PROVIDER_KEY] = provider current_session_data[AUTH_MODE_PROVIDER_EXTRA_VALUES_KEY] = provider_extra_values @@ -178,11 +174,6 @@ def get_auth_mode_metadata } end - def required_auth_mode - # For the correct implemetation, we should extract this in a separate class - { required_acr_values: Gitlab.config.admin_mode&.omniauth&.providers&.openid_connect&.required_acr_values } - end - private attr_reader :user -- GitLab From f338d50a50a56b188a6e0d3dee72ec86da1b9db6 Mon Sep 17 00:00:00 2001 From: Gerardo Navarro Date: Sun, 18 Aug 2024 21:25:09 +0200 Subject: [PATCH 06/15] Add feature flag for step-up auth --- app/controllers/admin/application_controller.rb | 2 ++ .../wip/omniauth_step_up_auth_admin_mode.yml | 9 +++++++++ 2 files changed, 11 insertions(+) create mode 100644 config/feature_flags/wip/omniauth_step_up_auth_admin_mode.yml diff --git a/app/controllers/admin/application_controller.rb b/app/controllers/admin/application_controller.rb index 58c60b5e8d96b5..d907eb13b4c6d3 100644 --- a/app/controllers/admin/application_controller.rb +++ b/app/controllers/admin/application_controller.rb @@ -11,6 +11,8 @@ class Admin::ApplicationController < ApplicationController layout 'admin' def check_current_user_auth_mode + return if Feature.disabled?(:omniauth_step_up_auth_admin_mode, current_user) + auth_mode_metadata = current_user_mode.get_auth_mode_metadata step_up_auth_extra_conditions = step_up_auth_extra_conditions(auth_mode_metadata[:provider], :admin_mode) return if auth_mode_metadata[:provider_extra_values] >= step_up_auth_extra_conditions diff --git a/config/feature_flags/wip/omniauth_step_up_auth_admin_mode.yml b/config/feature_flags/wip/omniauth_step_up_auth_admin_mode.yml new file mode 100644 index 00000000000000..52fd7d857102d0 --- /dev/null +++ b/config/feature_flags/wip/omniauth_step_up_auth_admin_mode.yml @@ -0,0 +1,9 @@ +--- +name: omniauth_step_up_auth_admin_mode +feature_issue_url: +introduced_by_url: +rollout_issue_url: +milestone: '17.4' +group: group::authentication +type: wip +default_enabled: false -- GitLab From 0f0d25141e582ce1eef4e060d929392f26db9d7a Mon Sep 17 00:00:00 2001 From: Gerardo Navarro Date: Sun, 18 Aug 2024 21:26:25 +0200 Subject: [PATCH 07/15] refactor: Improve check for required extra values in admin application controller --- .../admin/application_controller.rb | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/app/controllers/admin/application_controller.rb b/app/controllers/admin/application_controller.rb index d907eb13b4c6d3..4e987fa3879673 100644 --- a/app/controllers/admin/application_controller.rb +++ b/app/controllers/admin/application_controller.rb @@ -14,8 +14,14 @@ def check_current_user_auth_mode return if Feature.disabled?(:omniauth_step_up_auth_admin_mode, current_user) auth_mode_metadata = current_user_mode.get_auth_mode_metadata - step_up_auth_extra_conditions = step_up_auth_extra_conditions(auth_mode_metadata[:provider], :admin_mode) - return if auth_mode_metadata[:provider_extra_values] >= step_up_auth_extra_conditions + return if auth_mode_metadata.blank? || auth_mode_metadata[:provider].blank? + + step_up_auth_required_extra_values = step_up_auth_required_extra_values(auth_mode_metadata[:provider], :admin_mode) + return if step_up_auth_required_extra_values.blank? + + return if required_extra_values_included_in_session_data?( + session_data: auth_mode_metadata[:provider_extra_values], + required_extra_values: step_up_auth_required_extra_values) current_user_mode.disable_admin_mode! redirect_to( @@ -26,18 +32,18 @@ def check_current_user_auth_mode private - def step_up_auth_extra_conditions(provider_name, scope) + def step_up_auth_required_extra_values(provider_name, scope) provider_config = Gitlab::Auth::OAuth::Provider.config_for(provider_name) provider_config_for_scope = provider_config.fetch('step_up_auth', {})[scope] - return {} unless provider_config_for_scope&.fetch('enabled', false) - provider_config_for_scope.fetch('required_extra_values', nil) + provider_config_for_scope&.dig('enabled') && + provider_config_for_scope&.dig('required_extra_values') end - def compare_auth_mode(current_user_mode_session_data, step_up_auth_extra_conditions) - return false unless current_user_mode_session_data || step_up_auth_extra_conditions + def required_extra_values_included_in_session_data?(session_data:, required_extra_values:) + return false if session_data.blank? || required_extra_values.blank? - current_user_mode_session_data >= step_up_auth_extra_conditions + session_data >= required_extra_values end end -- GitLab From 262a5250fb08f41920de7250a65d322a95d468c7 Mon Sep 17 00:00:00 2001 From: Gerardo Navarro Date: Mon, 19 Aug 2024 13:50:25 +0200 Subject: [PATCH 08/15] refactor: Adjust omniauth_box to work for normal signin and admin area signin --- app/views/admin/sessions/new.html.haml | 2 +- app/views/devise/shared/_omniauth_box.html.haml | 15 +++++++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/app/views/admin/sessions/new.html.haml b/app/views/admin/sessions/new.html.haml index 304266c674a3bd..8867a1e2fcef90 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, admin_area: true diff --git a/app/views/devise/shared/_omniauth_box.html.haml b/app/views/devise/shared/_omniauth_box.html.haml index 9c7bc89629f51b..c2914e8c915270 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) +- admin_area = remember_me_enabled? && local_assigns.fetch(:admin_area, false) - 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, **step_up_auth_params(provider, :admin_mode)), - provider: provider, - data: { testid: test_id_for_provider(provider) } + - if admin_area + = render 'devise/shared/omniauth_provider_button', + href: omniauth_authorize_path(:user, provider, **step_up_auth_params(provider, :admin_mode)), + 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| -- GitLab From 08bd1e3037ebfa7751032c41e496165a15e38b65 Mon Sep 17 00:00:00 2001 From: Gerardo Navarro Date: Mon, 26 Aug 2024 14:40:59 +0200 Subject: [PATCH 09/15] Better matching to determine autentication context - Better output to debug auth context --- app/controllers/admin/application_controller.rb | 13 +++++++++++-- app/helpers/auth_helper.rb | 8 +++++++- locale/gitlab.pot | 2 +- 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/app/controllers/admin/application_controller.rb b/app/controllers/admin/application_controller.rb index 4e987fa3879673..2890d20b22a3c5 100644 --- a/app/controllers/admin/application_controller.rb +++ b/app/controllers/admin/application_controller.rb @@ -26,7 +26,15 @@ def check_current_user_auth_mode current_user_mode.disable_admin_mode! redirect_to( new_admin_session_path, - notice: _('Re-authentication with higher authentication context required') + notice: format( + _( + 'Re-authentication with authentication context required. ' \ + 'Current auth context: %{current_auth_mode} . ' \ + 'Expected auth context: %{expected_auth_mode}' + ), + current_auth_mode: auth_mode_metadata[:provider_extra_values].to_h, + expected_auth_mode: step_up_auth_required_extra_values.to_h + ) ) end @@ -41,7 +49,8 @@ def step_up_auth_required_extra_values(provider_name, scope) end def required_extra_values_included_in_session_data?(session_data:, required_extra_values:) - return false if session_data.blank? || required_extra_values.blank? + session_data = {} if session_data.blank? + required_extra_values = {} if required_extra_values.blank? session_data >= required_extra_values end diff --git a/app/helpers/auth_helper.rb b/app/helpers/auth_helper.rb index 68c86cebbde008..164398eb36d748 100644 --- a/app/helpers/auth_helper.rb +++ b/app/helpers/auth_helper.rb @@ -159,7 +159,13 @@ def step_up_auth_params(provider_name, scope) provider_config_for_scope = provider_config.fetch('step_up_auth', {})[scope] return {} unless provider_config_for_scope&.fetch('enabled', false) - provider_config_for_scope.fetch('params', false).transform_values { |v| v.to_h.to_json } + provider_config_for_scope.fetch('params', false).transform_values do |v| + if v.is_a?(Hash) + v.to_json + else + v + end + end end def provider_image_tag(provider, size = 64) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 42db79bac126bc..2be4add39101cb 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -44906,7 +44906,7 @@ msgstr "" msgid "Re-authentication required" msgstr "" -msgid "Re-authentication with higher authentication context required" +msgid "Re-authentication with authentication context required. Current auth context: %{current_auth_mode} . Expected auth context: %{expected_auth_mode}" msgstr "" msgid "Re-import" -- GitLab From f11ec2edda731978983411825db4a514443560f1 Mon Sep 17 00:00:00 2001 From: Gerardo Navarro Date: Tue, 27 Aug 2024 11:19:10 +0200 Subject: [PATCH 10/15] docs: Add documentation for step-up auth for admin mode --- doc/administration/auth/oidc.md | 47 +++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/doc/administration/auth/oidc.md b/doc/administration/auth/oidc.md index 3a822f65390c36..a92983ac317437 100644 --- a/doc/administration/auth/oidc.md +++ b/doc/administration/auth/oidc.md @@ -1307,6 +1307,53 @@ response to assign users as administrator based on group membership, configure G ::EndTabs +## Enable step-up authentication for admin mode + +Step-up authentication is a security mechanism that provides an additional layer of authentication for certain privileged actions or sensitive operations. It is commonly used in scenarios where the default level of authentication is not sufficient to protect critical resources or perform high-risk actions. With step-up authentication, users are required to provide additional credentials or undergo a more rigorous authentication process before they can access certain features or perform specific actions. This can include methods such as multi-factor authentication (MFA), biometric authentication, or one-time passwords (OTP). + +The OIDC (OpenID Connect) standard includes the concept of authentication context class references (ACR). We can leverage this ACR concept to implement step-up authentication for the admin mode in combination with the OIDC OmniAuth provider. + +To enable step-up authentication for admin mode in GitLab, follow the following steps: + +1. Go to the configuration of the OIDC OmniAuth provider in the `gitlab.yml` +1. Extend the configuration of this OIDC OmniAuth provider with the following entries + +```yaml +production: &base + omniauth: + providers: + - { name: 'openid_connect', + label: 'Provider name', + args: { + name: 'openid_connect', + ... + }, + step_up_auth: { + admin_mode: { + enabled: true, + documentation_link: "https://openid.net/specs/openid-connect-core-1_0.html#IDToken", + required_extra_values: { + acr: 'gold' + }, + params: { + claims: { id_token: { acr: { essential: true, values: ['gold'] } } } + } + } + } + } +``` + +In the above example: + +- `enabled` specifies whether the step-up authentication for admin mode is enabled or not. +- `required_extra_values` specifies the additional values that are required for step-up authentication. In this case, the `acr` value is required and has to be the value `gold` +- `params` specifies the additional parameters that are sent during the authentication process. In this case, the `acr_values` parameter is set to `'urn:example:step-up'`. + +After you have made these changes, save the `gitlab.yml` file and restart GitLab for the changes to take effect. + +NOTE: +OIDC-compatible Identity providers (IdPs) have slightly different behavior. Therefore, the setting `params` accepts a hash value that allows to define the necessary parameters to trigger step-up auth on the OIDC-compatible IdPs. The `params` values vary from what is passed to the IdP. + ## Troubleshooting 1. Ensure `discovery` is set to `true`. If you set it to `false`, you must -- GitLab From 2f61fcfc1a4bb42007a0ee8513bcb2ebfc7386b2 Mon Sep 17 00:00:00 2001 From: Gerardo Navarro Date: Mon, 26 Aug 2024 16:36:48 +0200 Subject: [PATCH 11/15] feature: Add step-up auth to group - New UI for enabling step-up auth in group settings - Add before_action filter to enforce step-up auth for group paths - Add intermediary screen to trigger step-up auth --- .../enforces_step_up_authentication.rb | 67 +++++++++++++++++++ app/controllers/concerns/groups/params.rb | 1 + .../groups/application_controller.rb | 5 ++ app/controllers/groups_controller.rb | 2 + .../profiles/step_up_auths_controller.rb | 7 ++ .../projects/application_controller.rb | 8 +++ app/controllers/projects_controller.rb | 13 ++++ app/helpers/auth_helper.rb | 5 ++ .../groups/settings/_permissions.html.haml | 1 + .../groups/settings/_step_up_auth.html.haml | 19 ++++++ .../profiles/step_up_auths/show.html.haml | 12 ++++ .../omniauth_step_up_auth_for_group_scope.yml | 9 +++ config/routes/profile.rb | 4 ++ ...re_step_up_authentication_to_namespaces.rb | 20 ++++++ db/schema_migrations/20240829163136 | 1 + db/structure.sql | 4 +- locale/gitlab.pot | 27 ++++++++ 17 files changed, 204 insertions(+), 1 deletion(-) create mode 100644 app/controllers/concerns/enforces_step_up_authentication.rb create mode 100644 app/controllers/profiles/step_up_auths_controller.rb create mode 100644 app/views/groups/settings/_step_up_auth.html.haml create mode 100644 app/views/profiles/step_up_auths/show.html.haml create mode 100644 config/feature_flags/gitlab_com_derisk/omniauth_step_up_auth_for_group_scope.yml create mode 100644 db/migrate/20240829163136_add_require_step_up_authentication_to_namespaces.rb create mode 100644 db/schema_migrations/20240829163136 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 00000000000000..06aa66223e0937 --- /dev/null +++ b/app/controllers/concerns/enforces_step_up_authentication.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +# == EnforcesStepUpAuthentication +# +# Controller concern to enforce step-up authentication requirements +module EnforcesStepUpAuthentication + extend ActiveSupport::Concern + + included do + before_action :check_current_user_auth_mode, except: [:route_not_found] # rubocop:disable Rails/LexicallyScopedActionFilter -- As part of the POC + end + + def check_current_user_auth_mode + namespace = step_up_auth_protected_namespace + + return if Feature.disabled?(:omniauth_step_up_auth_for_group_scope, namespace) + + return if namespace.blank? || namespace.require_step_up_authentication_through_omniauth_provider.blank? + + step_up_auth_required_extra_values = step_up_auth_required_extra_values( + namespace.require_step_up_authentication_through_omniauth_provider, :group_scope) + return if step_up_auth_required_extra_values.blank? + + auth_mode_metadata = Gitlab::Auth::CurrentUserMode.new(current_user, request.session).get_auth_mode_metadata + if auth_mode_metadata[:provider] == namespace.require_step_up_authentication_through_omniauth_provider && + required_extra_values_included_in_session_data?( + session_data: auth_mode_metadata[:provider_extra_values], + required_extra_values: step_up_auth_required_extra_values) + + return + end + + # Include the information where to redirect after the successful step-up authentication + store_location_for(:redirect, request.fullpath) + + redirect_to( + profile_step_up_auth_path, + notice: format( + _( + 'Step-up authentication required. ' \ + 'Current auth context: %{current_auth_mode} . ' \ + 'Expected auth context: %{expected_auth_mode}' + ), + current_auth_mode: auth_mode_metadata[:provider_extra_values].to_h, + expected_auth_mode: step_up_auth_required_extra_values.to_h + ) + ) + end + + def step_up_auth_protected_namespace + raise 'Not implemented' + end + + def step_up_auth_required_extra_values(provider_name, scope) + provider_config = Gitlab::Auth::OAuth::Provider.config_for(provider_name) + provider_config_for_scope = provider_config.fetch('step_up_auth', {})[scope] + + provider_config_for_scope&.dig('required_extra_values') + end + + def required_extra_values_included_in_session_data?(session_data:, required_extra_values:) + session_data = {} if session_data.blank? + required_extra_values = {} if required_extra_values.blank? + + session_data >= required_extra_values + end +end diff --git a/app/controllers/concerns/groups/params.rb b/app/controllers/concerns/groups/params.rb index 7649dc971171e4..54b6a6391167c1 100644 --- a/app/controllers/concerns/groups/params.rb +++ b/app/controllers/concerns/groups/params.rb @@ -30,6 +30,7 @@ def group_params_attributes :parent_id, :create_chat_team, :chat_team_name, + :require_step_up_authentication_through_omniauth_provider, :require_two_factor_authentication, :two_factor_grace_period, :enabled_git_access_protocol, diff --git a/app/controllers/groups/application_controller.rb b/app/controllers/groups/application_controller.rb index 2c38625fd1f767..e133aa1f1cb614 100644 --- a/app/controllers/groups/application_controller.rb +++ b/app/controllers/groups/application_controller.rb @@ -2,6 +2,7 @@ class Groups::ApplicationController < ApplicationController include RoutableActions + include EnforcesStepUpAuthentication include ControllerWithCrossProjectAccessCheck include SortingHelper include SortingPreference @@ -102,6 +103,10 @@ def method_missing(method_sym, *arguments, &block) super end end + + def step_up_auth_protected_namespace + group + end end Groups::ApplicationController.prepend_mod_with('Groups::ApplicationController') diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index b056b94eb19b0c..69ffa407cb0fad 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -49,6 +49,8 @@ class GroupsController < Groups::ApplicationController push_frontend_feature_flag(:mr_approved_filter, type: :ops) end + skip_before_action :check_current_user_auth_mode, only: [:new, :create] + helper_method :captcha_required? skip_cross_project_access_check :index, :new, :create, :edit, :update, :destroy, :projects diff --git a/app/controllers/profiles/step_up_auths_controller.rb b/app/controllers/profiles/step_up_auths_controller.rb new file mode 100644 index 00000000000000..49ecfe93bdae8d --- /dev/null +++ b/app/controllers/profiles/step_up_auths_controller.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +module Profiles + class StepUpAuthsController < Profiles::ApplicationController + def show; end + end +end diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb index c59de67901c838..00d1cc57f73505 100644 --- a/app/controllers/projects/application_controller.rb +++ b/app/controllers/projects/application_controller.rb @@ -3,6 +3,7 @@ class Projects::ApplicationController < ApplicationController include CookiesHelper include RoutableActions + include EnforcesStepUpAuthentication include ChecksCollaboration skip_before_action :authenticate_user! @@ -113,6 +114,13 @@ def handle_update_result(result) render 'edit' end end + + def step_up_auth_protected_namespace + return project&.namespace if project&.namespace.present? + + Namespace.find(params[:namespace_id]) if params[:namespace_id].present? + params[:namespace_id].present? + end end Projects::ApplicationController.prepend_mod_with('Projects::ApplicationController') diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 6b839cb8156f71..252626b8dca9a0 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -595,6 +595,19 @@ def check_export_rate_limit! def render_edit render 'edit' end + + def step_up_auth_protected_namespace + if action_name == 'create' + namespace_id = params.dig(:project, :namespace_id) + Namespace.find(namespace_id) if namespace_id.present? + + elsif action_name == 'new' + namespace_id = params[:namespace_id] + Namespace.find(namespace_id) if namespace_id.present? + else + project&.namespace + end + end end ProjectsController.prepend_mod_with('ProjectsController') diff --git a/app/helpers/auth_helper.rb b/app/helpers/auth_helper.rb index 164398eb36d748..347dc007759248 100644 --- a/app/helpers/auth_helper.rb +++ b/app/helpers/auth_helper.rb @@ -154,6 +154,11 @@ def button_based_providers_enabled? enabled_button_based_providers.any? 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) provider_config = Gitlab::Auth::OAuth::Provider.config_for(provider_name) provider_config_for_scope = provider_config.fetch('step_up_auth', {})[scope] diff --git a/app/views/groups/settings/_permissions.html.haml b/app/views/groups/settings/_permissions.html.haml index 25b0b34b32ef68..0c3d160e51a2af 100644 --- a/app/views/groups/settings/_permissions.html.haml +++ b/app/views/groups/settings/_permissions.html.haml @@ -40,6 +40,7 @@ = render_if_exists 'groups/settings/service_access_tokens_expiration_enforced', f: f, group: @group = render_if_exists 'groups/settings/enforce_ssh_certificates', f: f, group: @group = render 'groups/settings/two_factor_auth', f: f, group: @group + = render 'groups/settings/step_up_auth', f: f, group: @group = render_if_exists 'groups/personal_access_token_expiration_policy', f: f, group: @group = render 'groups/settings/membership', f: f, group: @group = render 'groups/settings/remove_dormant_members', f: f, group: @group diff --git a/app/views/groups/settings/_step_up_auth.html.haml b/app/views/groups/settings/_step_up_auth.html.haml new file mode 100644 index 00000000000000..6fa905e0dd5ea0 --- /dev/null +++ b/app/views/groups/settings/_step_up_auth.html.haml @@ -0,0 +1,19 @@ +%h5= _('Step-up authentication') + +%p + = _('What omniauth provider should be used for step-up auth?') + - link = link_to('', help_page_path('administration/auth/oidc.md', anchor: 'enable-step-up-authentication-for-admin-mode'), target: '_blank', rel: 'noopener noreferrer') + = safe_format(_('%{docs_link_start}Learn about step-up authentication.%{docs_link_end}'), tag_pair(link, :docs_link_start, :docs_link_end)) + +.form-group + = f.gitlab_ui_radio_component :require_step_up_authentication_through_omniauth_provider, '', _('No step-up authentication necessary'), help_text: _('Users accessing this group do not require step-up authentication.') + + - omniauth_providers_with_step_up_auth_config(:group_scope).each do |provider_config| + - help_text = _("Users accessing this group require step-up authentication through this omniauth provider.") + - step_up_auth_docs_link_url = provider_config.dig('step_up_auth', 'group_scope', 'documentation_link') + - if step_up_auth_docs_link_url.present? + - link = link_to('', step_up_auth_docs_link_url, target: '_blank', rel: 'noopener noreferrer') + - help_text_step_up_auth_docs = safe_format(_('%{docs_link_start}Learn more about the step-up auth requirements in your organization.%{docs_link_end}'), tag_pair(link, :docs_link_start, :docs_link_end)) + - help_text += ' ' + help_text_step_up_auth_docs + = f.gitlab_ui_radio_component :require_step_up_authentication_through_omniauth_provider, provider_config[:name], provider_config[:label], help_text: help_text.html_safe + diff --git a/app/views/profiles/step_up_auths/show.html.haml b/app/views/profiles/step_up_auths/show.html.haml new file mode 100644 index 00000000000000..768d9737e90ca5 --- /dev/null +++ b/app/views/profiles/step_up_auths/show.html.haml @@ -0,0 +1,12 @@ +- page_title _('Enter step-up auth mode') +- add_page_specific_style 'page_bundles/login' + +.row.gl-mt-5.justify-content-center + .col-md-5 + .login-page + .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, **step_up_auth_params(provider, :group_scope)), + provider: provider, + data: { testid: test_id_for_provider(provider) } diff --git a/config/feature_flags/gitlab_com_derisk/omniauth_step_up_auth_for_group_scope.yml b/config/feature_flags/gitlab_com_derisk/omniauth_step_up_auth_for_group_scope.yml new file mode 100644 index 00000000000000..dac64d37ea2aff --- /dev/null +++ b/config/feature_flags/gitlab_com_derisk/omniauth_step_up_auth_for_group_scope.yml @@ -0,0 +1,9 @@ +--- +name: omniauth_step_up_auth_for_group_scope +feature_issue_url: +introduced_by_url: +rollout_issue_url: +milestone: '17.5' +group: group::authentication +type: gitlab_com_derisk +default_enabled: false diff --git a/config/routes/profile.rb b/config/routes/profile.rb index 6542a50798df62..a7429c416f6d56 100644 --- a/config/routes/profile.rb +++ b/config/routes/profile.rb @@ -68,6 +68,10 @@ end end + resource :step_up_auth, only: [:show] + + resources :webauthn_registrations, only: [:destroy] + resources :usage_quotas, only: [:index] end end diff --git a/db/migrate/20240829163136_add_require_step_up_authentication_to_namespaces.rb b/db/migrate/20240829163136_add_require_step_up_authentication_to_namespaces.rb new file mode 100644 index 00000000000000..1a9dace1df8755 --- /dev/null +++ b/db/migrate/20240829163136_add_require_step_up_authentication_to_namespaces.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +# See https://docs.gitlab.com/ee/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddRequireStepUpAuthenticationToNamespaces < Gitlab::Database::Migration[2.2] + milestone '17.4' + + disable_ddl_transaction! + + def up + add_column :namespaces, :require_step_up_authentication_through_omniauth_provider, :text, default: '', null: false # rubocop:disable Migration/AddColumnsToWideTables -- Disabling this cop as part of this POC + add_text_limit :namespaces, :require_step_up_authentication_through_omniauth_provider, 255 + end + + def down + remove_column :namespaces, :require_step_up_authentication_through_omniauth_provider + remove_text_limit :namespaces, :require_step_up_authentication_through_omniauth_provider + end +end diff --git a/db/schema_migrations/20240829163136 b/db/schema_migrations/20240829163136 new file mode 100644 index 00000000000000..c67c395d3abc08 --- /dev/null +++ b/db/schema_migrations/20240829163136 @@ -0,0 +1 @@ +56de57d13af984619c447f7f5074228220fe2d2bc1e93636e7c5d9fef65b4f10 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 435803d0bd5275..f576ded8beac6d 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -175,7 +175,9 @@ CREATE TABLE namespaces ( shared_runners_enabled boolean DEFAULT true NOT NULL, allow_descendants_override_disabled_shared_runners boolean DEFAULT false NOT NULL, traversal_ids bigint[] DEFAULT '{}'::bigint[] NOT NULL, - organization_id bigint DEFAULT 1 + organization_id bigint DEFAULT 1, + require_step_up_authentication_through_omniauth_provider text DEFAULT ''::text NOT NULL, + CONSTRAINT check_e953598df2 CHECK ((char_length(require_step_up_authentication_through_omniauth_provider) <= 255)) ); CREATE FUNCTION find_namespaces_by_id(namespaces_id bigint) RETURNS namespaces diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 2be4add39101cb..2858d2140b05bf 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -795,9 +795,15 @@ msgstr[1] "" msgid "%{docs_link_start}How to add certificates to your project?%{docs_link_end}" msgstr "" +msgid "%{docs_link_start}Learn about step-up authentication.%{docs_link_end}" +msgstr "" + msgid "%{docs_link_start}Learn about visibility levels.%{docs_link_end}" msgstr "" +msgid "%{docs_link_start}Learn more about the step-up auth requirements in your organization.%{docs_link_end}" +msgstr "" + msgid "%{docs_link_start}Setting up a verified domain%{docs_link_end} requires being linked to a project." msgstr "" @@ -21277,6 +21283,9 @@ msgstr "" msgid "Enter one or more user ID separated by commas" msgstr "" +msgid "Enter step-up auth mode" +msgstr "" + msgid "Enter the %{name} description" msgstr "" @@ -36371,6 +36380,9 @@ msgstr "" msgid "No start date – %{dueDate}" msgstr "" +msgid "No step-up authentication necessary" +msgstr "" + msgid "No suggestions found" msgstr "" @@ -53188,6 +53200,12 @@ msgstr "" msgid "Step 4." msgstr "" +msgid "Step-up authentication" +msgstr "" + +msgid "Step-up authentication required. Current auth context: %{current_auth_mode} . Expected auth context: %{expected_auth_mode}" +msgstr "" + msgid "Still loading..." msgstr "" @@ -59969,6 +59987,12 @@ msgstr "" msgid "Users API rate limits" msgstr "" +msgid "Users accessing this group do not require step-up authentication." +msgstr "" + +msgid "Users accessing this group require step-up authentication through this omniauth provider." +msgstr "" + msgid "Users can launch a development environment from a GitLab browser tab when the %{linkStart}Gitpod%{linkEnd} integration is enabled." msgstr "" @@ -61642,6 +61666,9 @@ msgstr "" msgid "What is squashing?" msgstr "" +msgid "What omniauth provider should be used for step-up auth?" +msgstr "" + msgid "What templates can I create?" msgstr "" -- GitLab From ad4aeffb357a4a02c862a089e3414cdc4aa832fb Mon Sep 17 00:00:00 2001 From: Gerardo Navarro Date: Tue, 1 Oct 2024 17:25:27 +0200 Subject: [PATCH 12/15] refactor: Add capability to check for deep included id token claims - After looking deeper into the OIDC implementation of Microsoft Entra ID, it became clear that we needed the capability to check for deep included id token claims because MS Entra ID does not return the id token claim `acr` with a single value. Instead, it returns the id token claim `acrs` as an array of values. We were not able to workaoround this. Our only option was to check if the id token claim `acrs` includes certain values. Therefore, we added this capability. - We also redesigned the structure for the configuration of the step-up auth in the gitlab.yml. --- .../admin/application_controller.rb | 77 +++++++++++++++---- .../omniauth_callbacks_controller.rb | 2 +- lib/gitlab/auth/current_user_mode.rb | 8 +- locale/gitlab.pot | 5 +- 4 files changed, 70 insertions(+), 22 deletions(-) diff --git a/app/controllers/admin/application_controller.rb b/app/controllers/admin/application_controller.rb index 2890d20b22a3c5..886ad49da7dd10 100644 --- a/app/controllers/admin/application_controller.rb +++ b/app/controllers/admin/application_controller.rb @@ -16,36 +16,67 @@ def check_current_user_auth_mode auth_mode_metadata = current_user_mode.get_auth_mode_metadata return if auth_mode_metadata.blank? || auth_mode_metadata[:provider].blank? - step_up_auth_required_extra_values = step_up_auth_required_extra_values(auth_mode_metadata[:provider], :admin_mode) - return if step_up_auth_required_extra_values.blank? + step_up_auth_errors = [] - return if required_extra_values_included_in_session_data?( - session_data: auth_mode_metadata[:provider_extra_values], - required_extra_values: step_up_auth_required_extra_values) + expected_included_id_token_claims = expected_included_id_token_claims(auth_mode_metadata[:provider], :admin_mode) + if expected_included_id_token_claims.present? && !included_id_token_claims_deep_included_in_session_data?( + session_data: auth_mode_metadata[:provider_id_token], + included_extra_values: expected_included_id_token_claims) + step_up_auth_errors << format( + _( + 'Re-authentication requires step-up authentication. ' \ + 'Given id token claims: %{given_id_token_claims} . ' \ + 'Expected included id token claims: %{expected_included_id_token_claims}' + ), + given_id_token_claims: auth_mode_metadata[:provider_id_token].to_h, + expected_included_id_token_claims: expected_included_id_token_claims.to_h + ) + end - current_user_mode.disable_admin_mode! - redirect_to( - new_admin_session_path, - notice: format( + if step_up_auth_errors.present? + current_user_mode.disable_admin_mode! + redirect_to(new_admin_session_path, notice: step_up_auth_errors.join("\n")) + return + end + + expected_required_id_token_claims = expected_required_id_token_claims(auth_mode_metadata[:provider], :admin_mode) + if expected_required_id_token_claims.present? && !required_extra_values_included_in_session_data?( + session_data: auth_mode_metadata[:provider_id_token], + required_extra_values: expected_required_id_token_claims) + step_up_auth_errors << format( _( 'Re-authentication with authentication context required. ' \ - 'Current auth context: %{current_auth_mode} . ' \ - 'Expected auth context: %{expected_auth_mode}' + 'Given id token claims: %{given_id_token_claims} . ' \ + 'Expected required id token claims: %{expected_required_id_token_claims}' ), - current_auth_mode: auth_mode_metadata[:provider_extra_values].to_h, - expected_auth_mode: step_up_auth_required_extra_values.to_h + given_id_token_claims: auth_mode_metadata[:provider_id_token].to_h, + expected_required_id_token_claims: expected_required_id_token_claims.to_h ) - ) + end + + return if step_up_auth_errors.blank? + + current_user_mode.disable_admin_mode! + redirect_to(new_admin_session_path, notice: step_up_auth_errors.join("\n")) end private - def step_up_auth_required_extra_values(provider_name, scope) + def expected_required_id_token_claims(provider_name, scope) provider_config = Gitlab::Auth::OAuth::Provider.config_for(provider_name) provider_config_for_scope = provider_config.fetch('step_up_auth', {})[scope] provider_config_for_scope&.dig('enabled') && - provider_config_for_scope&.dig('required_extra_values') + provider_config_for_scope&.dig('id_token', 'required') + end + + def expected_included_id_token_claims(provider_name, scope) + provider_config = Gitlab::Auth::OAuth::Provider.config_for(provider_name) + provider_config_for_scope = provider_config.fetch('step_up_auth', {})[scope] + + {} unless provider_config_for_scope&.dig('enabled') + + provider_config_for_scope&.dig('id_token', 'included') end def required_extra_values_included_in_session_data?(session_data:, required_extra_values:) @@ -54,6 +85,20 @@ def required_extra_values_included_in_session_data?(session_data:, required_extr session_data >= required_extra_values end + + def included_id_token_claims_deep_included_in_session_data?(session_data:, included_extra_values:) + session_data = {} if session_data.blank? + included_extra_values = {} if included_extra_values.blank? + + included_extra_values.to_h.all? do |key, value| + session_data_value = session_data[key] + + next Array.wrap(value).any? { |i| session_data_value.include?(i) } if value.is_a?(Array) || value.is_a?(String) + next value.to_a.any? { |i| session_data_value.include?(i) } if value.is_a?(Hash) + + true + end + end end Admin::ApplicationController.prepend_mod_with('Admin::ApplicationController') diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index 14d05122d10f7e..1199890a243d00 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -350,7 +350,7 @@ def store_redirect_fragment(redirect_fragment) def set_current_user_auth_mode current_user_mode.set_auth_mode_metadata( provider: oauth.provider, - provider_extra_values: oauth.extra.raw_info + provider_id_token: oauth.extra.raw_info ) end diff --git a/lib/gitlab/auth/current_user_mode.rb b/lib/gitlab/auth/current_user_mode.rb index ab4ab49120768a..b2f5e46ae2d89f 100644 --- a/lib/gitlab/auth/current_user_mode.rb +++ b/lib/gitlab/auth/current_user_mode.rb @@ -23,7 +23,7 @@ class CurrentUserMode MAX_ADMIN_MODE_TIME = 6.hours ADMIN_MODE_REQUESTED_GRACE_PERIOD = 5.minutes AUTH_MODE_PROVIDER_KEY = :auth_mode_auth_provider - AUTH_MODE_PROVIDER_EXTRA_VALUES_KEY = :auth_mode_auth_provider_extra_values + AUTH_MODE_PROVIDER_ID_TOKEN_KEY = :auth_mode_auth_provider_id_token class << self # Admin mode activation requires storing a flag in the user session. Using this @@ -162,15 +162,15 @@ def current_session_data end strong_memoize_attr :current_session_data - def set_auth_mode_metadata(provider:, provider_extra_values:) + def set_auth_mode_metadata(provider:, provider_id_token:) current_session_data[AUTH_MODE_PROVIDER_KEY] = provider - current_session_data[AUTH_MODE_PROVIDER_EXTRA_VALUES_KEY] = provider_extra_values + current_session_data[AUTH_MODE_PROVIDER_ID_TOKEN_KEY] = provider_id_token end def get_auth_mode_metadata { provider: current_session_data[AUTH_MODE_PROVIDER_KEY], - provider_extra_values: current_session_data[AUTH_MODE_PROVIDER_EXTRA_VALUES_KEY] + provider_id_token: current_session_data[AUTH_MODE_PROVIDER_ID_TOKEN_KEY] } end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 2858d2140b05bf..f6266297fa1095 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -44918,7 +44918,10 @@ msgstr "" msgid "Re-authentication required" msgstr "" -msgid "Re-authentication with authentication context required. Current auth context: %{current_auth_mode} . Expected auth context: %{expected_auth_mode}" +msgid "Re-authentication requires step-up authentication. Given id token claims: %{given_id_token_claims} . Expected included id token claims: %{expected_included_id_token_claims}" +msgstr "" + +msgid "Re-authentication with authentication context required. Given id token claims: %{given_id_token_claims} . Expected required id token claims: %{expected_required_id_token_claims}" msgstr "" msgid "Re-import" -- GitLab From 9ceadaa44bd6bda909fc54c4eedd526bb3e64809 Mon Sep 17 00:00:00 2001 From: Gerardo Navarro Date: Wed, 2 Oct 2024 10:35:59 +0200 Subject: [PATCH 13/15] feature: Add capability for checking deep included id token claims for the group scope --- .../enforces_step_up_authentication.rb | 87 ++++++++++++++----- .../omniauth_callbacks_controller.rb | 2 + locale/gitlab.pot | 3 - 3 files changed, 67 insertions(+), 25 deletions(-) diff --git a/app/controllers/concerns/enforces_step_up_authentication.rb b/app/controllers/concerns/enforces_step_up_authentication.rb index 06aa66223e0937..eafbf28f86209b 100644 --- a/app/controllers/concerns/enforces_step_up_authentication.rb +++ b/app/controllers/concerns/enforces_step_up_authentication.rb @@ -17,45 +17,74 @@ def check_current_user_auth_mode return if namespace.blank? || namespace.require_step_up_authentication_through_omniauth_provider.blank? - step_up_auth_required_extra_values = step_up_auth_required_extra_values( - namespace.require_step_up_authentication_through_omniauth_provider, :group_scope) - return if step_up_auth_required_extra_values.blank? - auth_mode_metadata = Gitlab::Auth::CurrentUserMode.new(current_user, request.session).get_auth_mode_metadata - if auth_mode_metadata[:provider] == namespace.require_step_up_authentication_through_omniauth_provider && - required_extra_values_included_in_session_data?( - session_data: auth_mode_metadata[:provider_extra_values], - required_extra_values: step_up_auth_required_extra_values) - return + return unless auth_mode_metadata[:provider] == namespace.require_step_up_authentication_through_omniauth_provider + + required_omniauth_provider = namespace.require_step_up_authentication_through_omniauth_provider + + step_up_auth_errors = [] + + expected_included_id_token_claims = expected_included_id_token_claims(required_omniauth_provider, :group_scope) + if expected_included_id_token_claims.present? && !included_id_token_claims_deep_included_in_session_data?( + session_data: auth_mode_metadata[:provider_id_token], + included_extra_values: expected_included_id_token_claims) + step_up_auth_errors << format( + _( + 'Re-authentication requires step-up authentication. ' \ + 'Given id token claims: %{given_id_token_claims} . ' \ + 'Expected included id token claims: %{expected_included_id_token_claims}' + ), + given_id_token_claims: auth_mode_metadata[:provider_id_token].to_h, + expected_included_id_token_claims: expected_included_id_token_claims.to_h + ) end - # Include the information where to redirect after the successful step-up authentication - store_location_for(:redirect, request.fullpath) + if step_up_auth_errors.present? + current_user_mode.disable_admin_mode! + redirect_to(new_admin_session_path, notice: step_up_auth_errors.join("\n")) + return + end - redirect_to( - profile_step_up_auth_path, - notice: format( + expected_required_id_token_claims = expected_required_id_token_claims(required_omniauth_provider, :group_scope) + if expected_required_id_token_claims.present? && !required_extra_values_included_in_session_data?( + session_data: auth_mode_metadata[:provider_id_token], + required_extra_values: expected_required_id_token_claims) + step_up_auth_errors << format( _( - 'Step-up authentication required. ' \ - 'Current auth context: %{current_auth_mode} . ' \ - 'Expected auth context: %{expected_auth_mode}' + 'Re-authentication with authentication context required. ' \ + 'Given id token claims: %{given_id_token_claims} . ' \ + 'Expected required id token claims: %{expected_required_id_token_claims}' ), - current_auth_mode: auth_mode_metadata[:provider_extra_values].to_h, - expected_auth_mode: step_up_auth_required_extra_values.to_h + given_id_token_claims: auth_mode_metadata[:provider_id_token].to_h, + expected_required_id_token_claims: expected_required_id_token_claims.to_h ) - ) + end + + return if step_up_auth_errors.blank? + + # Include the information where to redirect after the successful step-up authentication + store_location_for(:redirect, request.fullpath) + + redirect_to(new_admin_session_path, notice: step_up_auth_errors.join("\n")) end def step_up_auth_protected_namespace raise 'Not implemented' end - def step_up_auth_required_extra_values(provider_name, scope) + def expected_required_id_token_claims(provider_name, scope) + provider_config = Gitlab::Auth::OAuth::Provider.config_for(provider_name) + provider_config_for_scope = provider_config.fetch('step_up_auth', {})[scope] + + provider_config_for_scope&.dig('id_token', 'required') + end + + def expected_included_id_token_claims(provider_name, scope) provider_config = Gitlab::Auth::OAuth::Provider.config_for(provider_name) provider_config_for_scope = provider_config.fetch('step_up_auth', {})[scope] - provider_config_for_scope&.dig('required_extra_values') + provider_config_for_scope&.dig('id_token', 'included') end def required_extra_values_included_in_session_data?(session_data:, required_extra_values:) @@ -64,4 +93,18 @@ def required_extra_values_included_in_session_data?(session_data:, required_extr session_data >= required_extra_values end + + def included_id_token_claims_deep_included_in_session_data?(session_data:, included_extra_values:) + session_data = {} if session_data.blank? + included_extra_values = {} if included_extra_values.blank? + + included_extra_values.to_h.all? do |key, value| + session_data_value = session_data[key] + + next Array.wrap(value).any? { |i| session_data_value.include?(i) } if value.is_a?(Array) || value.is_a?(String) + next value.to_a.any? { |i| session_data_value.include?(i) } if value.is_a?(Hash) + + true + end + end end diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index 1199890a243d00..45e8dae16b1443 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -170,6 +170,8 @@ def omniauth_flow(auth_module, identity_linker: nil) else sign_in_user_flow(auth_module::User) end + + set_current_user_auth_mode end def link_identity(identity_linker) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index f6266297fa1095..e57b44d40f7573 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -53206,9 +53206,6 @@ msgstr "" msgid "Step-up authentication" msgstr "" -msgid "Step-up authentication required. Current auth context: %{current_auth_mode} . Expected auth context: %{expected_auth_mode}" -msgstr "" - msgid "Still loading..." msgstr "" -- GitLab From e481ce89e5f412920973fd9edcd31cd9f36724b7 Mon Sep 17 00:00:00 2001 From: Gerardo Navarro Date: Wed, 2 Oct 2024 13:51:16 +0200 Subject: [PATCH 14/15] docs: Extend documentation for the step-up auth configuration with different IdPs --- doc/administration/auth/oidc.md | 117 +++++++++++++++++++++++++++++++- 1 file changed, 116 insertions(+), 1 deletion(-) diff --git a/doc/administration/auth/oidc.md b/doc/administration/auth/oidc.md index a92983ac317437..651be073b326e3 100644 --- a/doc/administration/auth/oidc.md +++ b/doc/administration/auth/oidc.md @@ -1309,7 +1309,7 @@ response to assign users as administrator based on group membership, configure G ## Enable step-up authentication for admin mode -Step-up authentication is a security mechanism that provides an additional layer of authentication for certain privileged actions or sensitive operations. It is commonly used in scenarios where the default level of authentication is not sufficient to protect critical resources or perform high-risk actions. With step-up authentication, users are required to provide additional credentials or undergo a more rigorous authentication process before they can access certain features or perform specific actions. This can include methods such as multi-factor authentication (MFA), biometric authentication, or one-time passwords (OTP). +Step-up authentication is a security mechanism that provides an additional layer of authentication for certain privileged actions or sensitive operations, e.g. admin area access. It is commonly used in scenarios where the default level of authentication is not sufficient to protect critical resources or perform high-risk actions. With step-up authentication, users are required to provide additional credentials or undergo a more rigorous authentication process before they can access certain features or perform specific actions. This can include methods such as multi-factor authentication (MFA), biometric authentication, or one-time passwords (OTP). The OIDC (OpenID Connect) standard includes the concept of authentication context class references (ACR). We can leverage this ACR concept to implement step-up authentication for the admin mode in combination with the OIDC OmniAuth provider. @@ -1354,6 +1354,121 @@ After you have made these changes, save the `gitlab.yml` file and restart GitLab NOTE: OIDC-compatible Identity providers (IdPs) have slightly different behavior. Therefore, the setting `params` accepts a hash value that allows to define the necessary parameters to trigger step-up auth on the OIDC-compatible IdPs. The `params` values vary from what is passed to the IdP. +### Require step-up authentication for admin area using Keycloak + +Keycloak supports step-up authentication through the definition of levels of authentication and custom browser login flows. + +To configure step-up authentication for admin mode in GitLab using Keycloak, follow these steps: + +1. **Configure Keycloak OIDC provider in GitLab** + +- Follow the steps covered in the [Configure Keycloak](#configure-keycloak) section. + +1. **Configure step-up authentication in Keycloak** + +- Follow [this detailed documenation guide](https://www.keycloak.org/docs/latest/server_admin/#_step-up-flow) that covers the configuration of the browser login flow. + +1. **Enable step-up authentication in Keycloak oidc provider configuration** + +- Edit your GitLab configuration file (`gitlab.yml` or `/etc/gitlab/gitlab.rb`) to include the step-up authentication settings: + + ```yaml + production: &base + omniauth: + providers: + - { name: 'openid_connect', + label: 'Keycloak', + args: { + name: 'openid_connect', + ... + allow_authorize_params: ["claims"] # <= These allowed authorize parameters should match the params used for the step-up mechanism + }, + step_up_auth: { + admin_mode: { + enabled: true, + documentation_link: "", + id_token: { + required: { + acr: 'gold' + } + }, + # <= Make sure that the individual params are also allowed in the configuration entry argument "args => allow_authorize_params", see above + params: { + claims: { + id_token: { acr: { essential: true, values: ['gold'] } } + } + } + }, + } + } + ``` + +- NOTE: This example assumes that the acr 'gold' is a higher security level. + +1. **Restart GitLab** + +- After making these changes, save the configuration file and restart GitLab for the changes to take effect. + +### Require step-up authentication for admin area using Microsoft Azure (MS Entra ID) + +Microsoft Entra ID (formerly Azure AD) supports step-up authentication through Conditional Access policies. + +To configure step-up authentication for admin mode in GitLab using MS Entra ID, follow these steps: + +1. **Configure MS Entra ID OIDC provider in GitLab** + +- Follow the steps covered in the [Configure Microsoft Azure](#configure-microsoft-azure) section. + +1. **Configure step-up authentication in MS Entra ID** + +- Follow [this detailed documenation guide](https://learn.microsoft.com/en-us/entra/identity/conditional-access/overview) to design the conditional access policies. + +1. **Enable step-up authentication in MS Entra ID oidc provider configuration** + +- Edit your GitLab configuration file (`gitlab.yml` or `/etc/gitlab/gitlab.rb`) to include the step-up authentication settings: + + ```yaml + production: &base + omniauth: + providers: + - { name: 'openid_connect', + label: 'Azure OIDC', + args: { + name: 'openid_connect', + ... + allow_authorize_params: ["claims"] # <= These allowed authorize parameters should match the params used for the step-up mechanism + }, + step_up_auth: { + admin_mode: { + enabled: true, + documentation_link: "", + access_token: { + included: { + acrs: ["gold"], + }, + }, + # <= Make sure that the individual params are also allowed in the configuration entry argument "args => allow_authorize_params", see above + params: { + claims: { + id_token: { acrs: { essential: true, value: 'gold' } } + } + } + }, + } + } + ``` + +- NOTE: This example assumes that the acr 'gold' is a higher security level. +- NOTE: The MS Entra ID is highly customizable and flexible. Therefore, some MS Entra instances can deviate from the official OIDC standard. For example, instead of exposing [the official claim `acr` as a single value in the ID token](https://openid.net/specs/openid-connect-core-1_0.html#IDToken), MS Entra exposes [the claim `acrs` as a JSON-array value in the access token](https://learn.microsoft.com/en-us/entra/identity-platform/access-token-claims-reference#payload-claims). Please collaborate with your IT department team to define the correct configuration. + +1. **Restart GitLab** + +- After making these changes, save the configuration file and restart GitLab for the changes to take effect. + +### Troubleshooting + +1. Ensure that the parameters (see `step_up_auth => admin_mode => params`) are allowed in the configuration entry argument "args => allow_authorize_params". This ensures that the parameters are included in the request query parameters used to redirect to authorization endpoint of the IdP. + ## Troubleshooting 1. Ensure `discovery` is set to `true`. If you set it to `false`, you must -- GitLab From 56df2bd72b53916bf6004e487ae7e3488a51e250 Mon Sep 17 00:00:00 2001 From: Gerardo Navarro Date: Fri, 4 Oct 2024 12:37:08 +0200 Subject: [PATCH 15/15] feat: Show session data related step-up auth --- app/models/active_session.rb | 5 +++-- .../user_settings/active_sessions/_active_session.html.haml | 5 +++++ locale/gitlab.pot | 3 +++ 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/app/models/active_session.rb b/app/models/active_session.rb index cce5580325fe06..8820f74ef7da8c 100644 --- a/app/models/active_session.rb +++ b/app/models/active_session.rb @@ -29,7 +29,7 @@ class ActiveSession :ip_address, :browser, :os, :device_name, :device_type, :is_impersonated, :session_id, :session_private_id, - :admin_mode + :admin_mode, :acr_values ].freeze ATTR_READER_LIST = [ :created_at, :updated_at @@ -88,7 +88,8 @@ def self.set(user, request) updated_at: timestamp, session_private_id: session_private_id, is_impersonated: request.session[:impersonator_id].present?, - admin_mode: Gitlab::Auth::CurrentUserMode.new(user, request.session).admin_mode? + admin_mode: Gitlab::Auth::CurrentUserMode.new(user, request.session).admin_mode?, + acr_values: request.session[:acr_values] ) Gitlab::Instrumentation::RedisClusterValidator.allow_cross_slot_commands do diff --git a/app/views/user_settings/active_sessions/_active_session.html.haml b/app/views/user_settings/active_sessions/_active_session.html.haml index c98af9776e37ce..02ee031195397d 100644 --- a/app/views/user_settings/active_sessions/_active_session.html.haml +++ b/app/views/user_settings/active_sessions/_active_session.html.haml @@ -27,6 +27,11 @@ - if active_session.try(:admin_mode) %strong= _('with Admin Mode') + %div + %strong + = _('ACR Values:') + = active_session.acr_values + - unless is_current_session .gl-float-right = link_button_to revoke_session_path(active_session), data: { confirm: _('Are you sure? The device will be signed out of GitLab and all remember me tokens revoked.'), confirm_btn_variant: :danger }, method: :delete, class: 'gl-ml-3', variant: :danger, 'aria-label': _('Revoke') do diff --git a/locale/gitlab.pot b/locale/gitlab.pot index e57b44d40f7573..597653b25f5b3f 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -2143,6 +2143,9 @@ msgstr "" msgid "A user with write access to the source branch selected this option" msgstr "" +msgid "ACR Values:" +msgstr "" + msgid "ACTION REQUIRED: Something went wrong while obtaining the Let's Encrypt certificate for GitLab Pages domain '%{domain}'" msgstr "" -- GitLab