From c721ce31305e6fcbc2603c1263682697ba1b1f32 Mon Sep 17 00:00:00 2001 From: Benoit BERAUD Date: Tue, 2 Nov 2021 11:55:53 +0100 Subject: [PATCH] Support multiple SAML providers for external authentification of users Add support for multiple SAML providers (https://gitlab.com/gitlab-org/gitlab/-/issues/14361) Fixes as well an issue in app/views/profiles/accounts/_providers.html.haml for providers wihtout icons where the display was incorrect. Changelog: changed --- .../omniauth_callbacks_controller.rb | 2 +- app/helpers/auth_helper.rb | 11 ++ .../profiles/accounts/_providers.html.haml | 7 +- doc/integration/saml.md | 68 +++++++ spec/helpers/auth_helper_spec.rb | 166 ++++++++++++++++++ 5 files changed, 251 insertions(+), 3 deletions(-) diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index 1696eef09a8c33..dc5b22e16067f6 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -9,7 +9,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController after_action :verify_known_sign_in - protect_from_forgery except: [:kerberos, :saml, :cas3, :failure], with: :exception, prepend: true + protect_from_forgery except: [:kerberos, :saml, :cas3, :failure] + AuthHelper.saml_providers, with: :exception, prepend: true feature_category :authentication_and_authorization diff --git a/app/helpers/auth_helper.rb b/app/helpers/auth_helper.rb index 2032b7e8bb7e8e..c1a74382d46f1f 100644 --- a/app/helpers/auth_helper.rb +++ b/app/helpers/auth_helper.rb @@ -86,6 +86,17 @@ def form_based_providers auth_providers.select { |provider| form_based_provider?(provider) } end + def saml_providers + auth_providers.select { |provider| auth_strategy_class(provider) == 'OmniAuth::Strategies::SAML' } + end + + def auth_strategy_class(provider) + config = Gitlab::Auth::OAuth::Provider.config_for(provider) + return if config.nil? || config['args'].blank? + + config.args['strategy_class'] + end + def any_form_based_providers_enabled? form_based_providers.any? { |provider| form_enabled_for_sign_in?(provider) } end diff --git a/app/views/profiles/accounts/_providers.html.haml b/app/views/profiles/accounts/_providers.html.haml index 5c0044ed8255e5..73a437a0702e5b 100644 --- a/app/views/profiles/accounts/_providers.html.haml +++ b/app/views/profiles/accounts/_providers.html.haml @@ -6,11 +6,13 @@ - providers.each do |provider| - unlink_allowed = unlink_provider_allowed?(provider) - link_allowed = link_provider_allowed?(provider) + - has_icon = provider_has_icon?(provider) - if unlink_allowed || link_allowed - if auth_active?(provider) - if unlink_allowed = link_to unlink_profile_account_path(provider: provider), method: :delete, class: button_class do - .social-provider-btn-image.gl-button-icon= provider_image_tag(provider) + - if has_icon + .social-provider-btn-image.gl-button-icon= provider_image_tag(provider) .gl-button-text = s_('Profiles|Disconnect %{provider}') % { provider: label_for_provider(provider) } - else @@ -19,7 +21,8 @@ = s_('Profiles|%{provider} Active') % { provider: label_for_provider(provider) } - elsif link_allowed = link_to omniauth_authorize_path(:user, provider), method: :post, class: button_class do - .social-provider-btn-image.gl-button-icon= provider_image_tag(provider) + - if has_icon + .social-provider-btn-image.gl-button-icon= provider_image_tag(provider) .gl-button-text = s_('Profiles|Connect %{provider}') % { provider: label_for_provider(provider) } = render_if_exists 'profiles/accounts/group_saml_unlink_buttons', group_saml_identities: group_saml_identities diff --git a/doc/integration/saml.md b/doc/integration/saml.md index d5f01aeb3b41e5..70d6932b9eb954 100644 --- a/doc/integration/saml.md +++ b/doc/integration/saml.md @@ -163,6 +163,74 @@ On the sign in page there should now be a SAML button below the regular sign in Click the icon to begin the authentication process. If everything goes well the user is returned to GitLab and signed in. +### Use multiple SAML identity providers + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/14361) in GitLab 14.6. + +You can configure GitLab to use multiple SAML identity providers if: + +- Each provider has a unique name set that matches a name set in `args`. +- The providers' names are: + - Used in OmniAuth configuration for properties based on the provider name. For example, `allowBypassTwoFactor`, `allowSingleSignOn`, and + `syncProfileFromProvider`. + - Used for association to each existing user as an additional identity. +- The `assertion_consumer_service_url` matches the provider name. +- The `strategy_class` is explicitly set because it cannot be inferred from provider name. + +Example multiple providers configuration for Omnibus GitLab: + +```ruby +gitlab_rails['omniauth_providers'] = [ + { + name: 'saml_1', + args: { + name: 'saml_1', # This is mandatory and must match the provider name + strategy_class: 'OmniAuth::Strategies::SAML' + assertion_consumer_service_url: 'https://gitlab.example.com/users/auth/saml_1/callback', # URL must match the name of the provider + ... # Put here all the required arguments similar to a single provider + }, + label: 'Provider 1' # Differentiate the two buttons and providers in the UI + }, + { + name: 'saml_2', + args: { + name: 'saml_2', # This is mandatory and must match the provider name + strategy_class: 'OmniAuth::Strategies::SAML' + assertion_consumer_service_url: 'https://gitlab.example.com/users/auth/saml_2/callback', # URL must match the name of the provider + ... # Put here all the required arguments similar to a single provider + }, + label: 'Provider 2' # Differentiate the two buttons and providers in the UI + } +] +``` + +Example providers configuration for installations from source: + +```yaml +omniauth: + providers: + - { + name: 'saml_1', + args: { + name: 'saml_1', # This is mandatory and must match the provider name + strategy_class: 'OmniAuth::Strategies::SAML', + assertion_consumer_service_url: 'https://gitlab.example.com/users/auth/saml_1/callback', # URL must match the name of the provider + ... # Put here all the required arguments similar to a single provider + }, + label: 'Provider 1' # Differentiate the two buttons and providers in the UI + } + - { + name: 'saml_2', + args: { + name: 'saml_2', # This is mandatory and must match the provider name + strategy_class: 'OmniAuth::Strategies::SAML', + assertion_consumer_service_url: 'https://gitlab.example.com/users/auth/saml_2/callback', # URL must match the name of the provider + ... # Put here all the required arguments similar to a single provider + }, + label: 'Provider 2' # Differentiate the two buttons and providers in the UI + } +``` + ### Notes on configuring your identity provider When configuring a SAML app on the IdP, you need at least: diff --git a/spec/helpers/auth_helper_spec.rb b/spec/helpers/auth_helper_spec.rb index 5a596526a7e3ae..b481c214ca1294 100644 --- a/spec/helpers/auth_helper_spec.rb +++ b/spec/helpers/auth_helper_spec.rb @@ -395,4 +395,170 @@ def auth_active? end end end + + describe '#auth_strategy_class' do + subject(:auth_strategy_class) { helper.auth_strategy_class(name) } + + context 'when configuration specifies no provider' do + let(:name) { 'does_not_exist' } + + before do + allow(Gitlab.config.omniauth).to receive(:providers).and_return([]) + end + + it 'returns false' do + expect(auth_strategy_class).to be_falsey + end + end + + context 'when configuration specifies a provider with args but without strategy_class' do + let(:name) { 'google_oauth2' } + let(:provider) do + Struct.new(:name, :args).new( + name, + 'app_id' => 'YOUR_APP_ID' + ) + end + + before do + allow(Gitlab.config.omniauth).to receive(:providers).and_return([provider]) + end + + it 'returns false' do + expect(auth_strategy_class).to be_falsey + end + end + + context 'when configuration specifies a provider with args and strategy_class' do + let(:name) { 'provider1' } + let(:strategy) { 'OmniAuth::Strategies::LDAP' } + let(:provider) do + Struct.new(:name, :args).new( + name, + 'strategy_class' => strategy + ) + end + + before do + allow(Gitlab.config.omniauth).to receive(:providers).and_return([provider]) + end + + it 'returns the class' do + expect(auth_strategy_class).to eq(strategy) + end + end + + context 'when configuration specifies another provider with args and another strategy_class' do + let(:name) { 'provider1' } + let(:strategy) { 'OmniAuth::Strategies::LDAP' } + let(:provider) do + Struct.new(:name, :args).new( + 'another_name', + 'strategy_class' => strategy + ) + end + + before do + allow(Gitlab.config.omniauth).to receive(:providers).and_return([provider]) + end + + it 'returns false' do + expect(auth_strategy_class).to be_falsey + end + end + end + + describe '#saml_providers' do + subject(:saml_providers) { helper.saml_providers } + + let(:saml_strategy) { 'OmniAuth::Strategies::SAML' } + + let(:saml_provider_1_name) { 'saml_provider_1' } + let(:saml_provider_1) do + Struct.new(:name, :args).new( + saml_provider_1_name, + 'strategy_class' => saml_strategy + ) + end + + let(:saml_provider_2_name) { 'saml_provider_2' } + let(:saml_provider_2) do + Struct.new(:name, :args).new( + saml_provider_2_name, + 'strategy_class' => saml_strategy + ) + end + + let(:ldap_provider_name) { 'ldap_provider' } + let(:ldap_strategy) { 'OmniAuth::Strategies::LDAP' } + let(:ldap_provider) do + Struct.new(:name, :args).new( + ldap_provider_name, + 'strategy_class' => ldap_strategy + ) + end + + let(:google_oauth2_provider_name) { 'google_oauth2' } + let(:google_oauth2_provider) do + Struct.new(:name, :args).new( + google_oauth2_provider_name, + 'app_id' => 'YOUR_APP_ID' + ) + end + + context 'when configuration specifies no provider' do + before do + allow(Devise).to receive(:omniauth_providers).and_return([]) + allow(Gitlab.config.omniauth).to receive(:providers).and_return([]) + end + + it 'returns an empty list' do + expect(saml_providers).to be_empty + end + end + + context 'when configuration specifies a provider with a SAML strategy_class' do + before do + allow(Devise).to receive(:omniauth_providers).and_return([saml_provider_1_name]) + allow(Gitlab.config.omniauth).to receive(:providers).and_return([saml_provider_1]) + end + + it 'returns the provider' do + expect(saml_providers).to match_array([saml_provider_1_name]) + end + end + + context 'when configuration specifies two providers with a SAML strategy_class' do + before do + allow(Devise).to receive(:omniauth_providers).and_return([saml_provider_1_name, saml_provider_2_name]) + allow(Gitlab.config.omniauth).to receive(:providers).and_return([saml_provider_1, saml_provider_2]) + end + + it 'returns the provider' do + expect(saml_providers).to match_array([saml_provider_1_name, saml_provider_2_name]) + end + end + + context 'when configuration specifies a provider with a non-SAML strategy_class' do + before do + allow(Devise).to receive(:omniauth_providers).and_return([ldap_provider_name]) + allow(Gitlab.config.omniauth).to receive(:providers).and_return([ldap_provider]) + end + + it 'returns an empty list' do + expect(saml_providers).to be_empty + end + end + + context 'when configuration specifies four providers but only two with SAML strategy_class' do + before do + allow(Devise).to receive(:omniauth_providers).and_return([saml_provider_1_name, ldap_provider_name, saml_provider_2_name, google_oauth2_provider_name]) + allow(Gitlab.config.omniauth).to receive(:providers).and_return([saml_provider_1, ldap_provider, saml_provider_2, google_oauth2_provider]) + end + + it 'returns the provider' do + expect(saml_providers).to match_array([saml_provider_1_name, saml_provider_2_name]) + end + end + end end -- GitLab