diff --git a/app/views/admin/application_settings/_visibility_and_access.html.haml b/app/views/admin/application_settings/_visibility_and_access.html.haml index 0305a9487ca7d7e1713e6bc04f33b08f6ef3b61d..b64617f3f11bc45b63ba60c7cb3b89173df290ce 100644 --- a/app/views/admin/application_settings/_visibility_and_access.html.haml +++ b/app/views/admin/application_settings/_visibility_and_access.html.haml @@ -38,6 +38,8 @@ = render_if_exists 'admin/application_settings/ldap_access_setting', form: f + = render_if_exists 'admin/application_settings/saml_group_locks_setting', form: f + .form-group{ data: { testid: 'project-export' } } = f.label :project_export, s_('AdminSettings|Project export'), class: 'label-bold' = f.gitlab_ui_checkbox_component :project_export_enabled, s_('AdminSettings|Enabled') diff --git a/db/migrate/20230224161346_add_saml_group_lock_to_application_settings.rb b/db/migrate/20230224161346_add_saml_group_lock_to_application_settings.rb new file mode 100644 index 0000000000000000000000000000000000000000..003dd5c5b6107ecf61b1b77c478284d7f5725f0f --- /dev/null +++ b/db/migrate/20230224161346_add_saml_group_lock_to_application_settings.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddSamlGroupLockToApplicationSettings < Gitlab::Database::Migration[2.1] + def change + add_column :application_settings, :lock_memberships_to_saml, :boolean, default: false, null: false + end +end diff --git a/db/schema_migrations/20230224161346 b/db/schema_migrations/20230224161346 new file mode 100644 index 0000000000000000000000000000000000000000..1c939bdafaf9c59af8a6a3dce3ce6512c80d250a --- /dev/null +++ b/db/schema_migrations/20230224161346 @@ -0,0 +1 @@ +191d7be803e9e3a2a5292bbcd562c34a67c07b73da2c429ac2f115b28d04f00c \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index e073c4f471138e7fbe9a3fd33dec1eed27c2ceb2..202ccec92e88d1cfb3374360b0177ee2263f8052 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -11748,6 +11748,7 @@ CREATE TABLE application_settings ( projects_api_rate_limit_unauthenticated integer DEFAULT 400 NOT NULL, deny_all_requests_except_allowed boolean DEFAULT false NOT NULL, product_analytics_data_collector_host text, + lock_memberships_to_saml boolean DEFAULT false 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_container_registry_pre_import_tags_rate_positive CHECK ((container_registry_pre_import_tags_rate >= (0)::numeric)), CONSTRAINT app_settings_dep_proxy_ttl_policies_worker_capacity_positive CHECK ((dependency_proxy_ttl_group_policy_worker_capacity >= 0)), diff --git a/doc/user/group/saml_sso/group_sync.md b/doc/user/group/saml_sso/group_sync.md index ff6e906b1443fce1cc7f9cd221c6542caeb15fcf..7c9b6effc436ef282ba02c609ee124cb85d47fb1 100644 --- a/doc/user/group/saml_sso/group_sync.md +++ b/doc/user/group/saml_sso/group_sync.md @@ -106,6 +106,30 @@ Users granted: SAML group membership is evaluated each time a user signs in. +### Global SAML group memberships lock **(PREMIUM SELF)** + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/386390) in GitLab 15.10. + +GitLab administrators can use the global SAML group memberships lock to prevent group members from inviting new members to subgroups that have their membership synchronized with SAML Group Links. + +Global group memberships lock only applies to subgroups of a top-level group where SAML Group Links synchronization is configured. No user can modify the +membership of a top-level group configured for SAML Group Links synchronization. + +When global group memberships lock is enabled: + +- Only an administrator can manage memberships of any group including access levels. +- Users cannot: + - Share a project with other groups. + - Invite members to a project created in a group. + +To enable global group memberships lock: + +1. [Configure SAML](../../../integration/saml.md) for your self-managed GitLab instance. +1. On the top bar, select **Main menu > Admin**. +1. On the left sidebar, select **Settings > General**. +1. Expand the **Visibility and access controls** section. +1. Ensure the **Lock memberships to SAML synchronization** checkbox is selected. + ### Automatic member removal After a group sync, users who are not members of a mapped SAML group are removed from the group. diff --git a/ee/app/helpers/ee/application_settings_helper.rb b/ee/app/helpers/ee/application_settings_helper.rb index ad2cf38b998f8169891d37e077637c9ead49dfb0..7efa688271cddb287a23664fe2198ecc3741bdee 100644 --- a/ee/app/helpers/ee/application_settings_helper.rb +++ b/ee/app/helpers/ee/application_settings_helper.rb @@ -43,6 +43,7 @@ def visible_attributes :geo_status_timeout, :help_text, :lock_memberships_to_ldap, + :lock_memberships_to_saml, :max_personal_access_token_lifetime, :max_ssh_key_lifetime, :repository_size_limit, diff --git a/ee/app/helpers/ee/projects_helper.rb b/ee/app/helpers/ee/projects_helper.rb index 7c5957fcf2435f875db3561b3e563ecb6b35681e..f69fc59150986701c8cb022f2042a3c47f9d5600 100644 --- a/ee/app/helpers/ee/projects_helper.rb +++ b/ee/app/helpers/ee/projects_helper.rb @@ -163,7 +163,9 @@ def membership_locked? return false unless group - group.membership_lock? || ::Gitlab::CurrentSettings.lock_memberships_to_ldap? + group.membership_lock? || + ::Gitlab::CurrentSettings.lock_memberships_to_ldap? || + ::Gitlab::CurrentSettings.lock_memberships_to_saml? end def group_project_templates_count(group_id) diff --git a/ee/app/models/ee/application_setting.rb b/ee/app/models/ee/application_setting.rb index f5baa5ab3c9043af194ee6936955465c5be449ae..7c3361ca377fef77534c16c881ac95040126877a 100644 --- a/ee/app/models/ee/application_setting.rb +++ b/ee/app/models/ee/application_setting.rb @@ -232,6 +232,7 @@ def defaults globally_allowed_ips: '', license_usage_data_exported: false, lock_memberships_to_ldap: false, + lock_memberships_to_saml: false, maintenance_mode: false, max_personal_access_token_lifetime: nil, max_ssh_key_lifetime: nil, diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb index 0acb7e35ad8ce4bf93b67b898bb183661bc54945..12d37ad852fb05b7b2944f1c8a9ee52070d08dda 100644 --- a/ee/app/models/ee/group.rb +++ b/ee/app/models/ee/group.rb @@ -397,6 +397,10 @@ def group_saml_enabled? saml_provider.persisted? && saml_provider.enabled? end + def saml_group_links_enabled? + group_saml_enabled? && saml_group_links.exists? + end + def global_saml_enabled? ::Gitlab::Auth::Saml::Config.enabled? end diff --git a/ee/app/models/ee/project.rb b/ee/app/models/ee/project.rb index 51e96f3d72e1229b77d06a6aef5f324906e8f757..14da3fb3414c87959fbd1d61c0e55d1c0a61d855 100644 --- a/ee/app/models/ee/project.rb +++ b/ee/app/models/ee/project.rb @@ -604,7 +604,7 @@ def group_ldap_synced? override :allowed_to_share_with_group? def allowed_to_share_with_group? - super && !(group && ::Gitlab::CurrentSettings.lock_memberships_to_ldap?) + super && !(group && lock_memberships_to_ldap_or_saml?) end override :membership_locked? @@ -1075,6 +1075,11 @@ def predefined_push_rule private + def lock_memberships_to_ldap_or_saml? + ::Gitlab::CurrentSettings.lock_memberships_to_ldap? || + ::Gitlab::CurrentSettings.lock_memberships_to_saml? + end + def update_legacy_open_source_license_available project_setting.legacy_open_source_license_available = false end diff --git a/ee/app/policies/ee/group_policy.rb b/ee/app/policies/ee/group_policy.rb index 9befaa9c201df90159f34258ca110ea6bcee99d4..78ee8b7beb0d90d8bcdc2a030fe777792007ad65 100644 --- a/ee/app/policies/ee/group_policy.rb +++ b/ee/app/policies/ee/group_policy.rb @@ -9,6 +9,10 @@ module GroupPolicy include CrudPolicyHelpers condition(:ldap_synced, scope: :subject) { @subject.ldap_synced? } + condition(:saml_group_links_enabled, scope: :subject) do + @subject.root_ancestor.saml_group_links_enabled? + end + condition(:epics_available, scope: :subject) { @subject.feature_available?(:epics) } condition(:iterations_available, scope: :subject) { @subject.feature_available?(:iterations) } condition(:subepics_available, scope: :subject) { @subject.feature_available?(:subepics) } @@ -72,6 +76,10 @@ module GroupPolicy ::Gitlab::CurrentSettings.lock_memberships_to_ldap? end + condition(:memberships_locked_to_saml, scope: :global) do + ::Gitlab::CurrentSettings.lock_memberships_to_saml + end + condition(:owners_bypass_ldap_lock) do ldap_lock_bypassable? end @@ -390,6 +398,10 @@ module GroupPolicy prevent :override_group_member end + rule { memberships_locked_to_saml & saml_group_links_enabled & ~admin }.policy do + prevent :admin_group_member + end + rule { developer }.policy do enable :create_wiki enable :admin_merge_request diff --git a/ee/app/policies/ee/project_policy.rb b/ee/app/policies/ee/project_policy.rb index 273b71b80f6f7602345c4b19370eeb740347f3b8..fb5c8d2030944bd0821ffdfe46e1e6fa9fb37494 100644 --- a/ee/app/policies/ee/project_policy.rb +++ b/ee/app/policies/ee/project_policy.rb @@ -160,7 +160,10 @@ module ProjectPolicy with_scope :subject condition(:membership_locked_via_parent_group) do - @subject.group && (@subject.group.membership_lock? || ::Gitlab::CurrentSettings.lock_memberships_to_ldap?) + @subject.group && ( + @subject.group.membership_lock? || + ::Gitlab::CurrentSettings.lock_memberships_to_ldap? || + ::Gitlab::CurrentSettings.lock_memberships_to_saml) end with_scope :subject diff --git a/ee/app/views/admin/application_settings/_saml_group_locks_setting.html.haml b/ee/app/views/admin/application_settings/_saml_group_locks_setting.html.haml new file mode 100644 index 0000000000000000000000000000000000000000..844dbd1814dd8313f372f5bd9486473dd42384eb --- /dev/null +++ b/ee/app/views/admin/application_settings/_saml_group_locks_setting.html.haml @@ -0,0 +1,11 @@ +- return unless group_saml_enabled? + +- form = local_assigns.fetch(:form) + +.form-group + %h5= _('SAML group membership settings') + - help_text_lock = _('If checked, new group memberships and permissions can only be added via SAML Group Links synchronization') + - help_link_lock = link_to sprite_icon('question-o'), help_page_path('user/group/saml_sso/group_sync.md', anchor: 'global-saml-group-memberships-lock'), title: _('Global SAML group membership lock'), 'aria-label' => _('Global SAML group membership lock') + = form.gitlab_ui_checkbox_component :lock_memberships_to_saml, + _('Lock memberships to SAML Group Links synchronization'), + help_text: '%{help_text} %{help_link}'.html_safe % { help_text: help_text_lock, help_link: help_link_lock } diff --git a/ee/spec/helpers/projects_helper_spec.rb b/ee/spec/helpers/projects_helper_spec.rb index dc7073973184e55f138b15357e0e05751ca60852..2d30d482be8ed41eaa8f1ef1179328f3cb7a8e9d 100644 --- a/ee/spec/helpers/projects_helper_spec.rb +++ b/ee/spec/helpers/projects_helper_spec.rb @@ -120,6 +120,18 @@ end end end + + context 'with SAML membership lock enabled and group membership_lock disabled' do + before do + stub_application_setting(lock_memberships_to_saml: true) + end + + let(:group) { build_stubbed(:group, membership_lock: false) } + + it 'is true' do + expect(helper).to be_membership_locked + end + end end describe '#group_project_templates_count' do diff --git a/ee/spec/models/ee/group_spec.rb b/ee/spec/models/ee/group_spec.rb index 512a91b89d3ac6b5f82933814e561bb5c1ba3527..2a4cbfa74341003dfa8da293f5a190eba5abe060 100644 --- a/ee/spec/models/ee/group_spec.rb +++ b/ee/spec/models/ee/group_spec.rb @@ -1680,6 +1680,32 @@ end end + describe '#saml_group_links_enabled?' do + subject { group.saml_group_links_enabled? } + + context 'with group saml disabled' do + it { is_expected.to eq(false) } + end + + context 'with group saml enabled' do + before do + create(:saml_provider, group: group) + end + + context "without saml group links" do + it { is_expected.to eq(false) } + end + + context 'with saml group links' do + before do + create(:saml_group_link, group: group) + end + + it { is_expected.to eq(true) } + end + end + end + describe "#insights_config" do context 'when group has no Insights project configured' do it 'returns the default config' do diff --git a/ee/spec/models/ee/project_spec.rb b/ee/spec/models/ee/project_spec.rb index 1c91f2d000b4193b5d7f43cb951fd319217d4f8b..d3dfd8103718d3c343cccb3457acf92b6bed665e 100644 --- a/ee/spec/models/ee/project_spec.rb +++ b/ee/spec/models/ee/project_spec.rb @@ -2988,6 +2988,46 @@ it { is_expected.not_to be_allowed_to_share_with_group } end + + context 'with lock_memberships_to_saml group setting enabled' do + let(:group) { build_stubbed(:group) } + + before do + stub_application_setting(lock_memberships_to_saml: true) + end + + context 'with lock for ldap membership disabled' do + it { is_expected.not_to be_allowed_to_share_with_group } + end + + context 'with lock for ldap membership enabled' do + before do + stub_application_setting(lock_memberships_to_ldap: true) + end + + it { is_expected.not_to be_allowed_to_share_with_group } + end + end + + context 'with lock_memberships_to_saml group setting disabled' do + let(:group) { build_stubbed(:group) } + + before do + stub_application_setting(lock_memberships_to_saml: false) + end + + context 'with lock for ldap membership disabled' do + it { is_expected.to be_allowed_to_share_with_group } + end + + context 'with lock for ldap membership enabled' do + before do + stub_application_setting(lock_memberships_to_ldap: true) + end + + it { is_expected.not_to be_allowed_to_share_with_group } + end + end end context 'personal project' do diff --git a/ee/spec/policies/group_policy_spec.rb b/ee/spec/policies/group_policy_spec.rb index 03d2cde26ffe4d81289b2405b056d624e75b9cde..fc0184e225e4520e113276be610b0cb391a38365 100644 --- a/ee/spec/policies/group_policy_spec.rb +++ b/ee/spec/policies/group_policy_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe GroupPolicy do +RSpec.describe GroupPolicy, feature_category: :subgroups do include AdminModeHelper include_context 'GroupPolicy context' @@ -873,6 +873,138 @@ def stub_group_saml_config(enabled) end end + context 'when memberships locked to SAML' do + context 'when group is a root group' do + before do + stub_application_setting(lock_memberships_to_saml: true) + end + + context 'when SAML group link sync is enabled' do + before do + allow(group).to receive(:saml_group_links_enabled?).and_return(true) + end + + context 'admin' do + let(:current_user) { admin } + + context 'when admin mode is enabled', :enable_admin_mode do + it { is_expected.to be_allowed(:admin_group_member) } + end + + context 'when admin mode is disabled' do + it { is_expected.not_to be_allowed(:admin_group_member) } + end + end + + context 'owner' do + let(:current_user) { owner } + + it { is_expected.not_to be_allowed(:admin_group_member) } + end + + context 'maintainer' do + let(:current_user) { maintainer } + + it { is_expected.not_to be_allowed(:admin_group_member) } + end + end + + context 'when no SAML sync is enabled' do + before do + allow(group).to receive(:saml_group_links_enabled?).and_return(false) + end + + context 'admin' do + let(:current_user) { admin } + + it { is_expected.not_to be_allowed(:admin_group_member) } + end + + context 'owner' do + let(:current_user) { owner } + + it { is_expected.to be_allowed(:admin_group_member) } + end + end + end + + context 'when group is not a root group' do + let(:parent_group) { create(:group) } + let(:group) { create(:group, :private, parent: parent_group) } + + before do + group.add_owner(owner) + parent_group.add_owner(owner) + stub_application_setting(lock_memberships_to_saml: true) + end + + context 'when SAML group link sync is enabled' do + before do + allow(group.root_ancestor).to receive(:saml_group_links_enabled?).and_return(true) + end + + context 'admin' do + let(:current_user) { admin } + + context 'when admin mode is enabled', :enable_admin_mode do + it { is_expected.to be_allowed(:admin_group_member) } + end + + context 'when admin mode is disabled' do + it { is_expected.not_to be_allowed(:admin_group_member) } + end + end + + context 'owner' do + let(:current_user) { owner } + + it { is_expected.not_to be_allowed(:admin_group_member) } + end + + context 'maintainer' do + let(:current_user) { maintainer } + + it { is_expected.not_to be_allowed(:admin_group_member) } + end + + context 'when child group has different owner than parent group' do + let(:sub_group_owner) { create(:user) } + let(:current_user) { sub_group_owner } + + before do + group.add_owner(sub_group_owner) + end + + it { is_expected.not_to be_allowed(:admin_group_member) } + end + end + + context 'when no SAML group link sync is enabled' do + before do + allow(group).to receive(:saml_group_links_enabled?).and_return(false) + end + + context 'admin' do + let(:current_user) { admin } + + it { is_expected.to be_disallowed(:admin_group_member) } + end + + context 'owner' do + let(:current_user) { owner } + + it { is_expected.to be_allowed(:admin_group_member) } + end + + context 'maintainer' do + let(:current_user) { maintainer } + + it { is_expected.to be_disallowed(:admin_group_member) } + end + end + end + end + context 'when LDAP sync is enabled' do before do allow(group).to receive(:ldap_synced?).and_return(true) diff --git a/ee/spec/policies/project_policy_spec.rb b/ee/spec/policies/project_policy_spec.rb index 0fc0b9185be18d5c2a3aa9eca896e048c061b43b..5fcd42339f9c3b13ad73665b3d34cddf6b895237 100644 --- a/ee/spec/policies/project_policy_spec.rb +++ b/ee/spec/policies/project_policy_spec.rb @@ -2383,6 +2383,14 @@ def expect_private_project_permissions_as_if_non_member it { is_expected.to be_disallowed(:import_project_members_from_another_project) } end + + context 'via SAML' do + before do + stub_application_setting(lock_memberships_to_saml: true) + end + + it { is_expected.to be_disallowed(:import_project_members_from_another_project) } + end end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 53072c90b9ac420ebb13e611724f50315a1cb409..f6c21a359dd216c959f9ed5c0a100da6aadc7371 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -19488,6 +19488,9 @@ msgstr "" msgid "Given epic is already related to this epic." msgstr "" +msgid "Global SAML group membership lock" +msgstr "" + msgid "Global Search is disabled for this scope" msgstr "" @@ -21557,6 +21560,9 @@ msgstr "" msgid "If checked, new group memberships and permissions can only be added via LDAP synchronization" msgstr "" +msgid "If checked, new group memberships and permissions can only be added via SAML Group Links synchronization" +msgstr "" + msgid "If disabled, a diverged local branch will not be automatically updated with commits from its remote counterpart, to prevent local data loss. If the default branch (%{default_branch}) has diverged and cannot be updated, mirroring will fail. Other diverged branches are silently ignored. %{link_start}Learn more.%{link_end}" msgstr "" @@ -25902,6 +25908,9 @@ msgstr "" msgid "Lock memberships to LDAP synchronization" msgstr "" +msgid "Lock memberships to SAML Group Links synchronization" +msgstr "" + msgid "Lock merge request" msgstr "" @@ -37891,6 +37900,9 @@ msgstr "" msgid "SAML for %{group_name}" msgstr "" +msgid "SAML group membership settings" +msgstr "" + msgid "SAML single sign-on" msgstr ""