diff --git a/db/migrate/20240813095256_add_disable_password_authentication_for_enterprise_users_to_saml_providers.rb b/db/migrate/20240813095256_add_disable_password_authentication_for_enterprise_users_to_saml_providers.rb new file mode 100644 index 0000000000000000000000000000000000000000..c33f50a2c3de1cf9368d63ea1c4a6a5402c6aad3 --- /dev/null +++ b/db/migrate/20240813095256_add_disable_password_authentication_for_enterprise_users_to_saml_providers.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class AddDisablePasswordAuthenticationForEnterpriseUsersToSamlProviders < Gitlab::Database::Migration[2.2] + milestone '17.4' + + enable_lock_retries! + + def change + add_column :saml_providers, :disable_password_authentication_for_enterprise_users, :boolean, default: false + end +end diff --git a/db/schema_migrations/20240813095256 b/db/schema_migrations/20240813095256 new file mode 100644 index 0000000000000000000000000000000000000000..aaad71dbb2249628bb8a7c2de4c744ec8f03e181 --- /dev/null +++ b/db/schema_migrations/20240813095256 @@ -0,0 +1 @@ +7d9a9b901e4f9088b323568e4a220f63a44ebf1d0e7332a3709a643956326316 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 9543952b1fca067fee49c6cf8810b841b48b9287..026d6bfe72f765ab8a1e497d6f3b7747182d31e6 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -17263,7 +17263,8 @@ CREATE TABLE saml_providers ( prohibited_outer_forks boolean DEFAULT true NOT NULL, default_membership_role smallint DEFAULT 10 NOT NULL, git_check_enforced boolean DEFAULT false NOT NULL, - member_role_id bigint + member_role_id bigint, + disable_password_authentication_for_enterprise_users boolean DEFAULT false ); CREATE SEQUENCE saml_providers_id_seq diff --git a/doc/user/group/saml_sso/index.md b/doc/user/group/saml_sso/index.md index 1587c08c0416ea4cbde79b35a813da1e40a45422..81d1f6a5ceec2ce4dbc79d0e40b5025a225051e0 100644 --- a/doc/user/group/saml_sso/index.md +++ b/doc/user/group/saml_sso/index.md @@ -261,6 +261,8 @@ After you set up your identity provider to work with GitLab, you must configure as the default membership role. 1. Select the **Enable SAML authentication for this group** checkbox. 1. Optional. Select: + - In GitLab 17.4 and later, **Disable password authentication for enterprise users**. + For more information, see the [Disable password authentication for enterprise users documentation](#disable-password-authentication-for-enterprise-users). - **Enforce SSO-only authentication for web activity for this group**. - **Enforce SSO-only authentication for Git activity for this group**. For more information, see the [SSO enforcement documentation](#sso-enforcement). @@ -416,6 +418,26 @@ automatically confirms user accounts. Users still receive an - The user is provisioned with SAML or SCIM. - The user has an email address that belongs to the verified domain. +### Disable password authentication for enterprise users + +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/373718) in GitLab 17.4. + +Prerequisites: + +- You must have the Owner role in the group that the enterprise user belongs to. +- The group SSO must be enabled. + +You can disable password authentication for the group's [enterprise users](../../enterprise_user/index.md). +This stops the enterprise users from using their password to authenticate. This behavior applies even +if an enterprise user is also an administrator of the group. + +To disable password authentication for enterprise users: + +1. On the left sidebar, select **Search or go to** and find your group. +1. Select **Settings > SAML SSO**. +1. Under **Configuration**, select **Disable password authentication for enterprise users**. +1. Select **Save changes**. + ### Block user access To rescind a user's access to the group when only SAML SSO is configured, either: diff --git a/ee/app/assets/javascripts/saml_providers/saml_settings_form.js b/ee/app/assets/javascripts/saml_providers/saml_settings_form.js index e9603212e19a9d40470fc6df37de6791fb000ef9..4582c879382d7d63af300b21d6c81db5e2767639 100644 --- a/ee/app/assets/javascripts/saml_providers/saml_settings_form.js +++ b/ee/app/assets/javascripts/saml_providers/saml_settings_form.js @@ -35,6 +35,13 @@ export default class SamlSettingsForm { name: 'group-saml', el: this.form.querySelector('.js-group-saml-enabled-input'), }, + { + name: 'disable-password-authentication-for-enterprise-users', + el: this.form.querySelector( + '.js-group-saml-disable-password-authentication-for-enterprise-users-input', + ), + dependsOn: 'group-saml', + }, { name: 'enforced-sso', el: this.form.querySelector('.js-group-saml-enforced-sso-input'), diff --git a/ee/app/controllers/groups/saml_providers_controller.rb b/ee/app/controllers/groups/saml_providers_controller.rb index d04ec76890788969e0d2937e47289ef66bec1d95..7d35aaf60b5085382560e502a047512057458030 100644 --- a/ee/app/controllers/groups/saml_providers_controller.rb +++ b/ee/app/controllers/groups/saml_providers_controller.rb @@ -51,7 +51,15 @@ def load_test_response end def saml_provider_params - allowed_params = %i[sso_url certificate_fingerprint enabled enforced_sso default_membership_role git_check_enforced] + allowed_params = [ + :sso_url, + :certificate_fingerprint, + :enabled, + :disable_password_authentication_for_enterprise_users, + :enforced_sso, + :default_membership_role, + :git_check_enforced + ] allowed_params << :member_role_id if group.custom_roles_enabled? if Feature.enabled?(:group_managed_accounts, group) diff --git a/ee/app/models/ee/user.rb b/ee/app/models/ee/user.rb index 9f9a5936d0cd94b661a07821abddcb35ee59769f..92ca577e8ad4ebdc15471b61206129f8da62fb36 100644 --- a/ee/app/models/ee/user.rb +++ b/ee/app/models/ee/user.rb @@ -536,6 +536,7 @@ def ldap_sync_time def allow_password_authentication_for_web?(*) return false if group_managed_account? return false if user_authorized_by_provisioning_group? + return false if password_authentication_disabled_by_enterprise_group? super end @@ -544,10 +545,18 @@ def allow_password_authentication_for_web?(*) def allow_password_authentication_for_git?(*) return false if group_managed_account? return false if user_authorized_by_provisioning_group? + return false if password_authentication_disabled_by_enterprise_group? super end + def password_authentication_disabled_by_enterprise_group? + return false unless enterprise_user? + return false unless enterprise_group.saml_provider + + enterprise_group.saml_provider.enabled? && enterprise_group.saml_provider.disable_password_authentication_for_enterprise_users? + end + override :password_based_login_forbidden? def password_based_login_forbidden? user_authorized_by_provisioning_group? || super diff --git a/ee/app/views/groups/saml_providers/_form.html.haml b/ee/app/views/groups/saml_providers/_form.html.haml index 6df7bf961601c96e2bbcadaa2e6306eaac1ebf0c..79a72ab22fb05925761d9423ef5f3716b39d89e0 100644 --- a/ee/app/views/groups/saml_providers/_form.html.haml +++ b/ee/app/views/groups/saml_providers/_form.html.haml @@ -3,6 +3,9 @@ .form-group = form_errors(saml_provider) = f.gitlab_ui_checkbox_component :enabled, s_('GroupSAML|Enable SAML authentication for this group'), checkbox_options: { class: 'js-group-saml-enabled-input' } + .form-group + - disable_password_authentication_for_enterprise_users_help_text = s_("GroupSAML|Before disabling password authentication, enable SAML authentication.") + = f.gitlab_ui_checkbox_component :disable_password_authentication_for_enterprise_users, s_('GroupSAML|Disable password authentication for enterprise users'), help_text: saml_sso_settings_generate_helper_text(display_none: saml_provider.enabled?, text: disable_password_authentication_for_enterprise_users_help_text), checkbox_options: { class: 'js-group-saml-disable-password-authentication-for-enterprise-users-input' } .form-group - enable_saml_help_text = s_('GroupSAML|Before enforcing SSO, enable SAML authentication.') - web_activity_warning_link_start = ''.html_safe % { url: help_page_path('user/group/saml_sso/index', anchor: 'sso-enforcement') } diff --git a/ee/lib/gitlab/auth/group_saml/user.rb b/ee/lib/gitlab/auth/group_saml/user.rb index 33712634bc3c30c9a0299799a3260075412e6ecd..787596b50d09f96e79631a32c62ae01cb12e71ce 100644 --- a/ee/lib/gitlab/auth/group_saml/user.rb +++ b/ee/lib/gitlab/auth/group_saml/user.rb @@ -17,7 +17,8 @@ def initialize(auth_hash) override :find_and_update! def find_and_update! - add_or_update_user_identities + add_or_update_identity_for_enterprise_user! + set_attributes_for_enterprise_user!(gl_user) save("GroupSaml Provider ##{@saml_provider.id}") @@ -42,7 +43,7 @@ def signup_identity_verification_enabled?(_) override :gl_user def gl_user strong_memoize(:gl_user) do - identity&.user || find_by_email || build_new_user + identity&.user || find_enterprise_user_by_email || build_new_user end end @@ -52,10 +53,10 @@ def identity end end - override :find_by_email - def find_by_email - user = super - return unless user&.authorized_by_provisioning_group?(saml_provider.group) + def find_enterprise_user_by_email + user = find_by_email + + return unless user&.enterprise_user_of_group?(saml_provider.group) user end @@ -77,9 +78,8 @@ def user_attributes end end - override :add_or_update_user_identities - def add_or_update_user_identities - return unless gl_user + def add_or_update_identity_for_enterprise_user! + return unless gl_user.enterprise_user_of_group?(saml_provider.group) return if self.identity # extern_uid hasn't changed # find_or_initialize_by doesn't update `gl_user.identities`, and isn't autosaved. diff --git a/ee/spec/controllers/ee/sessions_controller_spec.rb b/ee/spec/controllers/ee/sessions_controller_spec.rb index 06bb3ce8153cfb843fabca64b10da5c9bd380df1..a445eb899f0562f9def46f14e82c8d87094e17e2 100644 --- a/ee/spec/controllers/ee/sessions_controller_spec.rb +++ b/ee/spec/controllers/ee/sessions_controller_spec.rb @@ -148,5 +148,27 @@ def authenticate_2fa(otp_user_id: user.id, **user_params) expect(flash[:alert]).to include(I18n.t('devise.failure.invalid')) end end + + context 'when password authentication disabled by enterprise group' do + let_it_be(:enterprise_group) { create(:group) } + let_it_be(:saml_provider) do + create( + :saml_provider, + group: enterprise_group, + enabled: true, + disable_password_authentication_for_enterprise_users: true + ) + end + + let_it_be(:user) { create(:enterprise_user, enterprise_group: enterprise_group) } + + it 'does not authenticate the user' do + post(:create, params: { user: { login: user.username, password: user.password } }) + + expect(response).to have_gitlab_http_status(:ok) + expect(@request.env['warden']).not_to be_authenticated + expect(flash[:alert]).to include(I18n.t('devise.failure.invalid')) + end + end end end diff --git a/ee/spec/controllers/groups/omniauth_callbacks_controller_spec.rb b/ee/spec/controllers/groups/omniauth_callbacks_controller_spec.rb index a7570bbcf9ce904445e01067beeacc12623d7201..f34c623016632c7339961e0ab50e8d75a975bb4c 100644 --- a/ee/spec/controllers/groups/omniauth_callbacks_controller_spec.rb +++ b/ee/spec/controllers/groups/omniauth_callbacks_controller_spec.rb @@ -229,48 +229,6 @@ def stub_last_request_id(id) end end - context 'when user used to be a member of a group' do - before do - user.provisioned_by_group = group - user.save! - end - - it "displays a flash message verifying group sign in" do - post provider, params: { group_id: group } - - expect(flash[:notice]).to match(/Signed in with SAML/i) - end - - it 'adds linked identity' do - expect { post provider, params: { group_id: group } }.to change { linked_accounts.count } - end - - it 'adds group membership' do - expect { post provider, params: { group_id: group } }.to change { group.members.count } - end - end - - context 'when user was provisioned by other group' do - before do - user.provisioned_by_group = create(:group) - user.save! - end - - it "displays a flash message verifying group sign in" do - post provider, params: { group_id: group } - - expect(flash[:notice]).to eq(s_("SAML|There is already a GitLab account associated with this email address. Sign in with your existing credentials to connect your organization's account")) - end - - it 'does not add linked identity' do - expect { post provider, params: { group_id: group } }.not_to change { linked_accounts.count } - end - - it 'does not add group membership' do - expect { post provider, params: { group_id: group } }.not_to change { group.members.count } - end - end - context "when signed in" do before do sign_in(user) @@ -432,10 +390,92 @@ def stub_last_request_id(id) expect(response).to redirect_to(new_user_session_path) expect(flash[:notice]).to eq(s_("SAML|There is already a GitLab account associated with this email address. Sign in with your existing credentials to connect your organization's account")) end + + context 'when user is an enterprise user of the group' do + let(:user) { create(:enterprise_user, enterprise_group: group) } + + it_behaves_like 'SAML session initiated' + + it 'find the user by email and authenticates' do + post provider, params: { group_id: group } + + expect(request.env['warden']).to be_authenticated + expect(controller.current_user).to eq user + expect(flash[:notice]).to match(/Signed in with SAML/i) + end + + it 'links identity' do + expect { post provider, params: { group_id: group } } + .to change { Identity.exists?(user: user, extern_uid: uid, provider: provider, saml_provider_id: group.saml_provider.id) } + .from(false).to(true) + end + end + + context 'when user is an enterprise user of another group' do + let(:user) { create(:enterprise_user) } + + it "redirects to sign in page with flash notice" do + post provider, params: { group_id: group } + + expect(response).to redirect_to(new_user_session_path) + expect(flash[:notice]).to eq(s_("SAML|There is already a GitLab account associated with this email address. Sign in with your existing credentials to connect your organization's account")) + end + + it 'does not link identity' do + expect { post provider, params: { group_id: group } } + .not_to change { Identity.count } + end + end end it_behaves_like "and identity already linked" + context 'oauth linked with different NameID' do + let(:linked_identity) { create(:identity, user: user, extern_uid: 'some-other-name-id', provider: provider, saml_provider: saml_provider) } + + it "redirects to sign in page with flash notice" do + post provider, params: { group_id: group } + + expect(response).to redirect_to(new_user_session_path) + expect(flash[:notice]).to eq(s_("SAML|There is already a GitLab account associated with this email address. Sign in with your existing credentials to connect your organization's account")) + end + + context 'when user is an enterprise user of the group' do + let(:user) { create(:user, enterprise_group: group) } + + it_behaves_like 'SAML session initiated' + + it 'find the user by email and authenticates' do + post provider, params: { group_id: group } + + expect(request.env['warden']).to be_authenticated + expect(controller.current_user).to eq user + expect(flash[:notice]).to match(/Signed in with SAML/i) + end + + it 'updates linked identity' do + expect { post provider, params: { group_id: group } } + .to change { linked_identity.reload.extern_uid } + .from('some-other-name-id').to(uid) + end + end + + context 'when user is an enterprise user of another group' do + let(:user) { create(:enterprise_user) } + + it "redirects to sign in page with flash notice" do + post provider, params: { group_id: group } + + expect(response).to redirect_to(new_user_session_path) + expect(flash[:notice]).to eq(s_("SAML|There is already a GitLab account associated with this email address. Sign in with your existing credentials to connect your organization's account")) + end + + it 'does not update linked identity' do + expect { post provider, params: { group_id: group } }.not_to change { linked_identity.reload.extern_uid } + end + end + end + context 'for sign up', :aggregate_failures do let(:user) { build_stubbed(:user) } diff --git a/ee/spec/controllers/groups/saml_providers_controller_spec.rb b/ee/spec/controllers/groups/saml_providers_controller_spec.rb index 8dff78a3777ee7f570539885560ba155e106fcf2..62fb0ea9c3e6fc88fd0ad19278273898940af363 100644 --- a/ee/spec/controllers/groups/saml_providers_controller_spec.rb +++ b/ee/spec/controllers/groups/saml_providers_controller_spec.rb @@ -121,6 +121,7 @@ def stub_saml_config(enabled:) { group_id: group, saml_provider: { + disable_password_authentication_for_enterprise_users: 'true', enforced_sso: 'true', default_membership_role: Gitlab::Access::MAINTAINER } @@ -136,6 +137,7 @@ def stub_saml_config(enabled:) subject saml_provider.reload end.to change { saml_provider.enforced_sso? }.to(true) + .and change { saml_provider.disable_password_authentication_for_enterprise_users }.to(true) .and change { saml_provider.default_membership_role }.to(Gitlab::Access::MAINTAINER) end diff --git a/ee/spec/features/groups/saml_providers_spec.rb b/ee/spec/features/groups/saml_providers_spec.rb index 0afb38741ab7f59924f5dd2da07d792027a0d910..2ebbc6c9a0c41596308e1a6e9fde024d03780389 100644 --- a/ee/spec/features/groups/saml_providers_spec.rb +++ b/ee/spec/features/groups/saml_providers_spec.rb @@ -111,6 +111,14 @@ def stub_saml_config expect(login_url).to end_with "?token=#{group.reload.saml_discovery_token}" end + it 'updates disable_password_authentication_for_enterprise_users setting', :js do + visit group_saml_providers_path(group) + + check 'Disable password authentication for enterprise users' + + expect { submit }.to change { saml_provider.reload.disable_password_authentication_for_enterprise_users }.to(true) + end + it 'updates the enforced sso setting', :js do visit group_saml_providers_path(group) diff --git a/ee/spec/frontend/saml_providers/saml_settings_form_spec.js b/ee/spec/frontend/saml_providers/saml_settings_form_spec.js index 6566169db76b6116ecc8caec1ae9d4b7deef0fb3..86b8509d3fc89eccd19eb95ffbf6d073303e69b4 100644 --- a/ee/spec/frontend/saml_providers/saml_settings_form_spec.js +++ b/ee/spec/frontend/saml_providers/saml_settings_form_spec.js @@ -16,6 +16,10 @@ describe('SamlSettingsForm', () => { }); const findGroupSamlSetting = () => samlSettingsForm.settings.find((s) => s.name === 'group-saml'); + const findDisablePasswordAuthenticationForEnterpriseUsersSetting = () => + samlSettingsForm.settings.find( + (s) => s.name === 'disable-password-authentication-for-enterprise-users', + ); const findEnforcedSsoSetting = () => samlSettingsForm.settings.find((s) => s.name === 'enforced-sso'); const findEnforcedGitActivity = () => @@ -94,11 +98,17 @@ describe('SamlSettingsForm', () => { }); it('correctly disables multiple dependent toggles', () => { + expect( + findDisablePasswordAuthenticationForEnterpriseUsersSetting().el.hasAttribute('disabled'), + ).toBe(false); expect(findEnforcedSsoSetting().el.hasAttribute('disabled')).toBe(false); expect(findEnforcedGitActivity().el.hasAttribute('disabled')).toBe(false); uncheckSetting(findGroupSamlSetting()); + expect( + findDisablePasswordAuthenticationForEnterpriseUsersSetting().el.hasAttribute('disabled'), + ).toBe(true); expect(findEnforcedSsoSetting().el.hasAttribute('disabled')).toBe(true); expect(findEnforcedGitActivity().el.hasAttribute('disabled')).toBe(true); }); diff --git a/ee/spec/lib/gitlab/auth/group_saml/user_spec.rb b/ee/spec/lib/gitlab/auth/group_saml/user_spec.rb index 7ea458113dd93b6f1961f13294de5b7882479a0a..832905692cd47e4e0a4a70ba588ed333bf6653e9 100644 --- a/ee/spec/lib/gitlab/auth/group_saml/user_spec.rb +++ b/ee/spec/lib/gitlab/auth/group_saml/user_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' RSpec.describe Gitlab::Auth::GroupSaml::User, :aggregate_failures, feature_category: :system_access do - let(:uid) { 1234 } + let(:uid) { '1234' } let(:saml_provider) { create(:saml_provider) } let(:group) { saml_provider.group } let(:auth_hash) { OmniAuth::AuthHash.new(uid: uid, provider: 'group_saml', info: info_hash, extra: { raw_info: OneLogin::RubySaml::Attributes.new }) } @@ -113,6 +113,10 @@ def create_existing_identity expect { find_and_update }.to change { User.count }.by(1) end + it 'updates group membership' do + expect { find_and_update }.to change { group.members.count }.by(1) + end + it 'does not confirm the user' do is_expected.not_to be_confirmed end @@ -165,84 +169,39 @@ def create_existing_identity expect(response.errors['email']).to include(_('has already been taken')) end - context 'when user was provisioned by this group' do + context 'when user is an enterprise user of the group' do before do - user.update!(provisioned_by_group: group) + user.user_detail.update!(enterprise_group: saml_provider.group) end - it 'updates membership' do + it 'updates group membership' do expect { find_and_update }.to change { group.members.count }.by(1) end - it 'returns a user' do + it 'returns the user' do expect(find_and_update).to eq user end - it 'updates identity' do - expect { find_and_update }.to change { user.group_saml_identities.count }.by(1) - end - - context 'when user attributes are present' do - before do - user.update!(can_create_group: false, projects_limit: 10) - - auth_hash[:extra][:raw_info] = - OneLogin::RubySaml::Attributes.new( - 'can_create_group' => %w[true], 'projects_limit' => %w[20] - ) - end - - context 'when user is managed by group', :saas do - before do - stub_licensed_features(domain_verification: true) - user.user_detail.update!(enterprise_group: group) - end - - it 'updates the user can_create_group attribute' do - expect(find_and_update.can_create_group).to eq(true) - end - - it 'updates the user projects_limit attribute' do - expect(find_and_update.projects_limit).to eq(20) - end - end - - context 'when user is not managed by group' do - it 'does not update the user can_create_group attribute' do - expect(find_and_update.can_create_group).to eq(false) - end - - it 'does not update the user projects_limit attribute' do - expect(find_and_update.projects_limit).to eq(10) - end - end + it 'adds group_saml identity' do + expect { find_and_update } + .to change { Identity.exists?(user: user, extern_uid: uid, provider: :group_saml, saml_provider_id: group.saml_provider.id) } + .from(false).to(true) end - context 'with block_password_auth_for_saml_users feature flag disabled' do - before do - stub_feature_flags(block_password_auth_for_saml_users: false) - end - - it 'does not update membership' do - expect { find_and_update }.not_to change { group.members.count } - end - - it 'returns a user with errors' do - response = find_and_update + context 'when user has group_saml identity with different extern_uid' do + let!(:existing_group_saml_identity) { create(:group_saml_identity, user: user, extern_uid: 'some-other-name-id', saml_provider: saml_provider) } - expect(response).to be_a(User) - expect(response.errors['email']).to include(_('has already been taken')) - end - - it 'does not update identity' do - expect { find_and_update }.not_to change { user.group_saml_identities.count } + it "updates the identity's extern_uid" do + expect { find_and_update } + .to change { existing_group_saml_identity.reload.extern_uid } + .from('some-other-name-id').to(uid) end end end - context 'when user was provisioned by different group' do + context 'when user is an enterprise user of another group' do before do - user.update!(provisioned_by_group: create(:group)) + user.user_detail.update!(enterprise_group: create(:group)) end it 'does not update membership' do @@ -256,8 +215,16 @@ def create_existing_identity expect(response.errors['email']).to include(_('has already been taken')) end - it 'does not update identity' do - expect { find_and_update }.not_to change { user.group_saml_identities.count } + it 'does not add group_saml identity' do + expect { find_and_update }.not_to change { Identity.count } + end + + context 'when user has group_saml identity with different extern_uid' do + let!(:existing_group_saml_identity) { create(:group_saml_identity, user: user, extern_uid: 'some-other-name-id', saml_provider: saml_provider) } + + it "does not update the identity's extern_uid" do + expect { find_and_update }.not_to change { existing_group_saml_identity.reload.extern_uid } + end end end end diff --git a/ee/spec/models/ee/user_spec.rb b/ee/spec/models/ee/user_spec.rb index f40454844197dab378e722a9102339872374997a..f9cd01534389c02afcad3f32815ce58d5d7b1290 100644 --- a/ee/spec/models/ee/user_spec.rb +++ b/ee/spec/models/ee/user_spec.rb @@ -1535,6 +1535,34 @@ end end + describe '#valid_password?' do + subject(:validate_password) { user.valid_password?(password) } + + context 'when password authentication disabled by enterprise group' do + let_it_be(:enterprise_group) { create(:group) } + let_it_be(:saml_provider) { create(:saml_provider, group: enterprise_group, enabled: true, disable_password_authentication_for_enterprise_users: true) } + + let_it_be(:user) { create(:enterprise_user, enterprise_group: enterprise_group) } + + let(:password) { user.password } + + it { is_expected.to eq(false) } + end + end + + describe '#allow_password_authentication?' do + context 'when password authentication disabled by enterprise group' do + let_it_be(:enterprise_group) { create(:group) } + let_it_be(:saml_provider) { create(:saml_provider, group: enterprise_group, enabled: true, disable_password_authentication_for_enterprise_users: true) } + + let_it_be(:user) { create(:enterprise_user, enterprise_group: enterprise_group) } + + it 'is false' do + expect(user.allow_password_authentication?).to eq false + end + end + end + describe '#allow_password_authentication_for_web?' do context 'when user has managing group linked' do before do @@ -1565,6 +1593,17 @@ end end end + + context 'when password authentication disabled by enterprise group' do + let_it_be(:enterprise_group) { create(:group) } + let_it_be(:saml_provider) { create(:saml_provider, group: enterprise_group, enabled: true, disable_password_authentication_for_enterprise_users: true) } + + let_it_be(:user) { create(:enterprise_user, enterprise_group: enterprise_group) } + + it 'is false' do + expect(user.allow_password_authentication_for_web?).to eq false + end + end end describe '#allow_password_authentication_for_git?' do @@ -1597,6 +1636,17 @@ end end end + + context 'when password authentication disabled by enterprise group' do + let_it_be(:enterprise_group) { create(:group) } + let_it_be(:saml_provider) { create(:saml_provider, group: enterprise_group, enabled: true, disable_password_authentication_for_enterprise_users: true) } + + let_it_be(:user) { create(:enterprise_user, enterprise_group: enterprise_group) } + + it 'is false' do + expect(user.allow_password_authentication_for_git?).to eq false + end + end end describe '#password_expired_if_applicable?' do @@ -1758,6 +1808,43 @@ end end + describe '#password_authentication_disabled_by_enterprise_group?' do + subject(:password_authentication_disabled_by_enterprise_group?) { user.password_authentication_disabled_by_enterprise_group? } + + let_it_be(:user) { create(:user) } + + let_it_be(:root_group) { create(:group) } + + let_it_be(:root_group_with_saml_provider) { create(:group) } + let_it_be(:saml_provider) { create(:saml_provider, group: root_group_with_saml_provider) } + + using RSpec::Parameterized::TableSyntax + + where(:enterprise_group, :saml_enabled?, :disable_password_authentication_for_enterprise_users?, :result) do + nil | nil | nil | false + ref(:root_group) | nil | nil | false + ref(:root_group_with_saml_provider) | false | false | false + ref(:root_group_with_saml_provider) | false | true | false + ref(:root_group_with_saml_provider) | true | false | false + ref(:root_group_with_saml_provider) | true | true | true + end + + with_them do + before do + user.user_detail.update!(enterprise_group: enterprise_group) + + if enterprise_group&.saml_provider + enterprise_group.saml_provider.update!( + enabled: saml_enabled?, + disable_password_authentication_for_enterprise_users: disable_password_authentication_for_enterprise_users? + ) + end + end + + it { is_expected.to eq(result) } + end + end + describe '#enterprise_user_of_group?' do let_it_be(:group) { create(:group) } diff --git a/ee/spec/requests/git_http_spec.rb b/ee/spec/requests/git_http_spec.rb index ba48962cc5ce381aefc43266072b9397c08128ac..c17973bba6e6b9aa4e4a8c57ce5fc25dea883ace 100644 --- a/ee/spec/requests/git_http_spec.rb +++ b/ee/spec/requests/git_http_spec.rb @@ -253,6 +253,42 @@ end end + context 'when password authentication disabled by enterprise group' do + let_it_be(:enterprise_group) { create(:group) } + let_it_be(:saml_provider) { create(:saml_provider, group: enterprise_group, enabled: true, disable_password_authentication_for_enterprise_users: true) } + + let_it_be(:user) { create(:enterprise_user, enterprise_group: enterprise_group) } + + let_it_be(:project) { create(:project, :repository, :private, group: enterprise_group) } + + let(:env) { { user: user.username, password: user.password } } + let(:path) { "#{project.full_path}.git" } + + before do + project.add_developer(user) + end + + it 'responds with status 401 Unauthorized for pull action' do + download(path, **env) do |response| + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + it 'responds with status 401 Unauthorized for push action' do + upload(path, **env) do |response| + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + context 'when username and personal access token are provided' do + let(:access_token) { create(:personal_access_token, user: user) } + let(:env) { { user: user.username, password: access_token.token } } + + it_behaves_like 'pulls are allowed' + it_behaves_like 'pushes are allowed' + end + end + describe 'when namespace storage limits are enforced', :saas do let_it_be(:user) { create(:user) } let_it_be(:group, refind: true) { create(:group) } diff --git a/locale/gitlab.pot b/locale/gitlab.pot index b14c856e07e17f302fffbb94f65d47198795f0fd..4bfa1c8a1708339724e0c304d93b3a21e9a97c0b 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -25591,6 +25591,9 @@ msgstr "" msgid "GroupSAML|Are you sure you want to reset the SCIM token? SCIM provisioning will stop working until the new token is updated." msgstr "" +msgid "GroupSAML|Before disabling password authentication, enable SAML authentication." +msgstr "" + msgid "GroupSAML|Before enforcing SSO, enable SAML authentication." msgstr "" @@ -25612,6 +25615,9 @@ msgstr "" msgid "GroupSAML|Default membership role" msgstr "" +msgid "GroupSAML|Disable password authentication for enterprise users" +msgstr "" + msgid "GroupSAML|Enable SAML authentication for this group" msgstr ""