Merge branch 'dblessing_self_managed_sso_enforcement' into 'master'

Disable password authentication for SSO users

See merge request !168111



Merged-by: Drew Blessing <drew@gitlab.com>
Approved-by: Daniele Rossetti <drossetti@gitlab.com>
Approved-by: Harsha Muralidhar <hmuralidhar@gitlab.com>
Approved-by: Michał Zając <mzajac@gitlab.com>
Reviewed-by: Drew Blessing <drew@gitlab.com>
Reviewed-by: Andrew Evans <aevans@gitlab.com>
Reviewed-by: Bogdan Denkovych <bdenkovych@gitlab.com>
Reviewed-by: Daniele Rossetti <drossetti@gitlab.com>

app/helpers/application_settings_helper.rb

@@ -256,6 +256,7 @@ def visible_attributes
       :deny_all_requests_except_allowed,
       :disable_admin_oauth_scopes,
       :disable_feed_token,
+      :disable_password_authentication_for_users_with_sso_identities,
       :disabled_oauth_sign_in_sources,
       :domain_denylist,
       :domain_denylist_enabled,

app/models/application_setting.rb

@@ -652,6 +652,11 @@ def self.kroki_formats_attributes
     silent_admin_exports_enabled: [:boolean, { default: false }],
     allow_contribution_mapping_to_admins: [:boolean, { default: false }]
 
+  jsonb_accessor :sign_in_restrictions,
+    disable_password_authentication_for_users_with_sso_identities: [:boolean, { default: false }]
+
+  validates :sign_in_restrictions, json_schema: { filename: 'application_setting_sign_in_restrictions' }
+
   validates :rate_limits, json_schema: { filename: "application_setting_rate_limits" }
 
   validates :importers, json_schema: { filename: "application_setting_importers" }

app/models/application_setting_implementation.rb

@@ -73,6 +73,7 @@ def defaults # rubocop:disable Metrics/AbcSize
         disable_feed_token: false,
         disabled_direct_code_suggestions: false,
         disabled_oauth_sign_in_sources: [],
+        disable_password_authentication_for_users_with_sso_identities: false,
         dns_rebinding_protection_enabled: true,
         domain_allowlist: Settings.gitlab['domain_allowlist'],
         dsa_key_restriction: default_min_key_size(:dsa),

app/models/user.rb

@@ -1106,7 +1106,7 @@ def recently_sent_password_reset?
   def valid_password?(password)
     return false unless password_allowed?(password)
     return false if password_automatically_set?
-    return false unless allow_password_authentication?
+    return false unless allow_password_authentication_for_web?
 
     super
   end
@@ -1455,11 +1455,17 @@ def allow_password_authentication?
   end
 
   def allow_password_authentication_for_web?
-    Gitlab::CurrentSettings.password_authentication_enabled_for_web? && !ldap_user?
+    return false if ldap_user?
+    return false if disable_password_authentication_for_sso_users?
+
+    Gitlab::CurrentSettings.password_authentication_enabled_for_web?
   end
 
   def allow_password_authentication_for_git?
-    Gitlab::CurrentSettings.password_authentication_enabled_for_git? && !password_based_omniauth_user?
+    return false if password_based_omniauth_user?
+    return false if disable_password_authentication_for_sso_users?
+
+    Gitlab::CurrentSettings.password_authentication_enabled_for_git?
   end
 
   # method overriden in EE
@@ -2532,6 +2538,14 @@ def consume_otp!
 
   private
 
+  def disable_password_authentication_for_sso_users?
+    ::Gitlab::CurrentSettings.disable_password_authentication_for_users_with_sso_identities? && omniauth_user?
+  end
+
+  def omniauth_user?
+    identities.any?
+  end
+
   def optional_namespace?
     Feature.enabled?(:optional_personal_namespace, self)
   end

app/validators/json_schemas/application_setting_sign_in_restrictions.json

+{
+  "$schema": "http://json-schema.org/draft-07/schema#",
+  "description": "Application sign in restrictions",
+  "type": "object",
+  "additionalProperties": false,
+  "properties": {
+    "disable_password_authentication_for_users_with_sso_identities": {
+      "type": "boolean",
+      "description": "Restrict users to SSO sign-in when the user has one or more associated SSO identities"
+    }
+  }
+}

app/views/admin/application_settings/_signin.html.haml

@@ -11,6 +11,10 @@
         _('Allow password authentication for Git over HTTP(S)'),
         help_text: Gitlab::Auth::Ldap::Config.enabled? ? _('Clear this checkbox to use a personal access token or LDAP password instead.') : _('Clear this checkbox to use a personal access token instead.')
     - if omniauth_enabled? && button_based_providers.any?
+      .form-group
+        = f.gitlab_ui_checkbox_component :disable_password_authentication_for_users_with_sso_identities,
+            _('Disable password authentication for users with an SSO identity'),
+            help_text: _('Ensure users with SSO identities cannot sign in with password even when password authentication is enabled.')
       %fieldset.form-group
         %legend.gl-text-base.gl-mb-3.gl-border-none.gl-font-bold= _('Enabled OAuth authentication sources')
         = hidden_field_tag 'application_setting[enabled_oauth_sign_in_sources][]'

config/application_setting_columns/sign_in_restrictions.yml

+---
+api_type:
+attr: sign_in_restrictions
+clusterwide: false
+column: sign_in_restrictions
+db_type: jsonb
+default: "'{}'::jsonb"
+description: "Settings related to sign-in restrictions"
+encrypted: false
+gitlab_com_different_than_default: true
+jihu: false
+not_null: true

db/migrate/20241003173147_add_sign_in_restrictions_to_application_settings.rb

+# frozen_string_literal: true
+
+class AddSignInRestrictionsToApplicationSettings < Gitlab::Database::Migration[2.2]
+  milestone '17.5'
+  disable_ddl_transaction!
+
+  def up
+    add_column :application_settings, :sign_in_restrictions, :jsonb, default: {}, null: false, if_not_exists: true
+
+    add_check_constraint(
+      :application_settings,
+      "(jsonb_typeof(sign_in_restrictions) = 'object')",
+      'check_application_settings_sign_in_restrictions_is_hash'
+    )
+  end
+
+  def down
+    remove_column :application_settings, :sign_in_restrictions
+  end
+end

db/schema_migrations/20241003173147

+a8786052b716ff2c1172f15744cbec604df20fb9d9f3abf44436ae2060c02835
\ No newline at end of file

db/structure.sql

@@ -6507,6 +6507,7 @@ CREATE TABLE application_settings (
     required_instance_ci_template text,
     enforce_ci_inbound_job_token_scope_enabled boolean DEFAULT false NOT NULL,
     allow_top_level_group_owners_to_create_service_accounts boolean DEFAULT false NOT NULL,
+    sign_in_restrictions jsonb DEFAULT '{}'::jsonb NOT NULL,
     CONSTRAINT app_settings_container_reg_cleanup_tags_max_list_size_positive CHECK ((container_registry_cleanup_tags_service_max_list_size >= 0)),
     CONSTRAINT app_settings_dep_proxy_ttl_policies_worker_capacity_positive CHECK ((dependency_proxy_ttl_group_policy_worker_capacity >= 0)),
     CONSTRAINT app_settings_ext_pipeline_validation_service_url_text_limit CHECK ((char_length(external_pipeline_validation_service_url) <= 255)),
@@ -6565,6 +6566,7 @@ CREATE TABLE application_settings (
     CONSTRAINT check_application_settings_rate_limits_unauth_git_http_is_hash CHECK ((jsonb_typeof(rate_limits_unauthenticated_git_http) = 'object'::text)),
     CONSTRAINT check_application_settings_security_policies_is_hash CHECK ((jsonb_typeof(security_policies) = 'object'::text)),
     CONSTRAINT check_application_settings_service_ping_settings_is_hash CHECK ((jsonb_typeof(service_ping_settings) = 'object'::text)),
+    CONSTRAINT check_application_settings_sign_in_restrictions_is_hash CHECK ((jsonb_typeof(sign_in_restrictions) = 'object'::text)),
     CONSTRAINT check_b8c74ea5b3 CHECK ((char_length(deactivation_email_additional_text) <= 1000)),
     CONSTRAINT check_bf5157a366 CHECK ((char_length(required_instance_ci_template) <= 1024)),
     CONSTRAINT check_cdfbd99405 CHECK ((char_length(security_txt_content) <= 2048)),

doc/administration/settings/sign_in_restrictions.md

@@ -32,6 +32,14 @@ You can restrict the password authentication for web interface and Git over HTTP
 
 In the event of an external authentication provider outage, use the [GitLab Rails console](../operations/rails_console.md) to [re-enable the standard web sign-in form](#re-enable-standard-web-sign-in-form-in-rails-console). This configuration can also be changed over the [Application settings REST API](../../api/settings.md#change-application-settings) while authenticating with an administrator account's personal access token.
 
+### Disable password authentication for users with an SSO identity
+
+Even when password authentication is enabled, it may be desirable to restrict SSO users ability to sign in with a
+password. Select **Disable password authentication for users with an SSO identity** to ensure SSO users always sign in
+with their external provider.
+
+This restricts password authentication for both the web interface and Git over HTTP(S).
+
 ## Admin Mode
 
 If you're an administrator, you might want to work in GitLab without administrator access.

ee/spec/features/users/login_spec.rb

@@ -296,4 +296,18 @@
       end
     end
   end
+
+  describe 'when password authentication is disabled for SSO users' do
+    let_it_be(:user) { create(:omniauth_user, password_automatically_set: false) }
+
+    before do
+      stub_ee_application_setting(disable_password_authentication_for_users_with_sso_identities?: true)
+    end
+
+    it 'does not allow password authentication' do
+      gitlab_sign_in(user, password: user.password)
+
+      expect(page).to have_content("Invalid login or password")
+    end
+  end
 end

locale/gitlab.pot

@@ -19820,6 +19820,9 @@ msgstr ""
 msgid "Disable group runners"
 msgstr ""
+msgid "Disable password authentication for users with an SSO identity"
+msgstr ""
+
 msgid "Disable personal access tokens"
 msgstr ""
@@ -21174,6 +21177,9 @@ msgstr ""
 msgid "Enhance security by storing service account keys in secret managers - learn more about %{docLinkStart}secret management with GitLab%{docLinkEnd}"
 msgstr ""
+msgid "Ensure users with SSO identities cannot sign in with password even when password authentication is enabled."
+msgstr ""
+
 msgid "Ensure your %{linkStart}environment is part of the deploy stage%{linkEnd} of your CI pipeline to track deployments to your cluster."
 msgstr ""

spec/helpers/application_settings_helper_spec.rb

@@ -92,6 +92,12 @@
         ])
     end
 
+    it 'contains sign_in_restrictions values' do
+      expect(visible_attributes).to include(*%i[
+        disable_password_authentication_for_users_with_sso_identities
+      ])
+    end
+
     context 'when on SaaS', :saas do
       it 'does not contain :deactivate_dormant_users' do
         expect(helper.visible_attributes).not_to include(:deactivate_dormant_users)

spec/models/application_setting_spec.rb

@@ -48,6 +48,7 @@
     it { expect(setting.user_contributed_projects_api_limit).to eq(100) }
     it { expect(setting.user_projects_api_limit).to eq(300) }
     it { expect(setting.user_starred_projects_api_limit).to eq(100) }
+    it { expect(setting.disable_password_authentication_for_users_with_sso_identities).to eq(false) }
   end
 
   describe 'USERS_UNCONFIRMED_SECONDARY_EMAILS_DELETE_AFTER_DAYS' do
@@ -93,6 +94,7 @@
     it { expect(described_class).to validate_jsonb_schema(['application_setting_rate_limits']) }
     it { expect(described_class).to validate_jsonb_schema(['application_setting_package_registry']) }
     it { expect(described_class).to validate_jsonb_schema(['application_setting_service_ping_settings']) }
+    it { expect(described_class).to validate_jsonb_schema(['application_setting_sign_in_restrictions']) }
 
     it { is_expected.to allow_value(nil).for(:home_page_url) }
     it { is_expected.to allow_value(http).for(:home_page_url) }

spec/models/user_spec.rb

@@ -6159,6 +6159,8 @@ def add_user(access)
   end
 
   describe '#allow_password_authentication_for_web?' do
+    subject(:allow_password_authentication_for_web?) { user.allow_password_authentication_for_web? }
+
     context 'regular user' do
       let(:user) { build(:user) }
 
@@ -6178,9 +6180,13 @@ def add_user(access)
 
       expect(user.allow_password_authentication_for_web?).to be_falsey
     end
+
+    it_behaves_like 'OmniAuth user password authentication'
   end
 
   describe '#allow_password_authentication_for_git?' do
+    subject(:allow_password_authentication_for_git?) { user.allow_password_authentication_for_git? }
+
     context 'regular user' do
       let(:user) { build(:user) }
 
@@ -6200,6 +6206,8 @@ def add_user(access)
 
       expect(user.allow_password_authentication_for_git?).to be_falsey
     end
+
+    it_behaves_like 'OmniAuth user password authentication'
   end
 
   describe '#assigned_open_merge_requests_count' do
@@ -7445,35 +7453,40 @@ def access_levels(groups)
   describe '#valid_password?' do
     subject(:validate_password) { user.valid_password?(password) }
 
+    let(:password) { user.password }
+
     context 'user with disallowed password' do
       let(:user) { create(:user, :disallowed_password) }
-      let(:password) { user.password }
 
       it { is_expected.to eq(false) }
     end
 
     context 'using a correct password' do
-      let(:user) { create(:user) }
-      let(:password) { user.password }
+      context 'with a regular user' do
+        let(:user) { create(:user) }
+        let(:password) { user.password }
 
-      it { is_expected.to eq(true) }
+        it { is_expected.to eq(true) }
 
-      context 'when password authentication is disabled' do
-        before do
-          stub_application_setting(password_authentication_enabled_for_web: false)
-          stub_application_setting(password_authentication_enabled_for_git: false)
+        context 'when password authentication for web is disabled' do
+          before do
+            stub_application_setting(password_authentication_enabled_for_web: false)
+            stub_application_setting(password_authentication_enabled_for_git: true)
+          end
+
+          it { is_expected.to eq(false) }
         end
 
-        it { is_expected.to eq(false) }
-      end
+        context 'when user with LDAP identity' do
+          before do
+            create(:identity, provider: 'ldapmain', user: user)
+          end
 
-      context 'when user with LDAP identity' do
-        before do
-          create(:identity, provider: 'ldapmain', user: user)
+          it { is_expected.to eq(false) }
         end
-
-        it { is_expected.to eq(false) }
       end
+
+      it_behaves_like 'OmniAuth user password authentication'
     end
 
     context 'using a wrong password' do

spec/support/shared_examples/users/ominauth_user_password_authentication_shared_examples.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples 'OmniAuth user password authentication' do
+  let(:user) { create(:omniauth_user) }
+
+  context 'when omniauth user sets a local password' do
+    before do
+      user.update!(password_automatically_set: false)
+    end
+
+    it { is_expected.to eq(true) }
+
+    context 'when password authentication is disabled for users with an SSO identity' do
+      before do
+        stub_application_setting(disable_password_authentication_for_users_with_sso_identities: true)
+      end
+
+      context 'when the user has no SSO identity' do
+        let!(:user) { create(:user) }
+
+        it { is_expected.to eq(true) }
+      end
+
+      context 'when the user has a SAML identity' do
+        let!(:user) { create(:omniauth_user, provider: 'saml', password_automatically_set: false) }
+
+        it { is_expected.to eq(false) }
+      end
+
+      context 'when the user has a different identity' do
+        let!(:user) { create(:omniauth_user, password_automatically_set: false) }
+
+        it { is_expected.to eq(false) }
+      end
+    end
+  end
+end