diff --git a/app/graphql/types/member_access_level_enum.rb b/app/graphql/types/member_access_level_enum.rb index b6e7b90d380965c9eabaeda35f959ec372ab5d60..3366ecc6cfc7dffca08c079c457c086bbb82ab85 100644 --- a/app/graphql/types/member_access_level_enum.rb +++ b/app/graphql/types/member_access_level_enum.rb @@ -10,7 +10,7 @@ def self.descriptions end Gitlab::Access.options_with_owner.each do |key, value| - value key.upcase, value: value, description: descriptions[value] + value key.upcase.tr(' ', '_'), value: value, description: descriptions[value] end end end diff --git a/ee/spec/graphql/resolvers/members/standard_roles_resolver_spec.rb b/ee/spec/graphql/resolvers/members/standard_roles_resolver_spec.rb index 87038caa31974aadb60ffd0603ff891d6a869fb1..8d74a4c8177e0307df800640a149e3b04bf79095 100644 --- a/ee/spec/graphql/resolvers/members/standard_roles_resolver_spec.rb +++ b/ee/spec/graphql/resolvers/members/standard_roles_resolver_spec.rb @@ -24,7 +24,7 @@ it 'returns the totals for each standard role' do expect(result).to be_present - expect(result.count).to eq(7) + expect(result.count).to eq(8) roles_with_members = [::Gitlab::Access::MAINTAINER, ::Gitlab::Access::DEVELOPER] diff --git a/ee/spec/lib/ee/gitlab/access_spec.rb b/ee/spec/lib/ee/gitlab/access_spec.rb index 511fb9b360b1c0b09778449768629e37ed597e6c..3564efbf5ef81244b07b8d0656ede3b9a3e1514b 100644 --- a/ee/spec/lib/ee/gitlab/access_spec.rb +++ b/ee/spec/lib/ee/gitlab/access_spec.rb @@ -86,7 +86,8 @@ describe '.options_with_minimal_access' do it 'returns the hash of roles with Owner' do expected_result = { - "Guest" => 10, "Planner" => 15, "Reporter" => 20, "Developer" => 30, "Maintainer" => 40, 'Owner' => 50, + "Guest" => 10, "Planner" => 15, "Reporter" => 20, "Security Manager" => 25, + "Developer" => 30, "Maintainer" => 40, 'Owner' => 50, "Minimal Access" => 5 } @@ -97,7 +98,8 @@ describe '.options_for_custom_roles' do it 'returns the hash of roles without Owner' do expected_result = { - "Guest" => 10, "Planner" => 15, "Reporter" => 20, "Developer" => 30, "Maintainer" => 40, "Minimal Access" => 5 + "Guest" => 10, "Planner" => 15, "Reporter" => 20, "Security Manager" => 25, + "Developer" => 30, "Maintainer" => 40, "Minimal Access" => 5 } expect(described_class.options_for_custom_roles).to eq(expected_result) diff --git a/ee/spec/models/ee/group_spec.rb b/ee/spec/models/ee/group_spec.rb index bc62020bc07ab144d915dc90a5adfb1a2e59cbf6..e03276bf67a4a76504a1d43fa5cbebe562f3b1e9 100644 --- a/ee/spec/models/ee/group_spec.rb +++ b/ee/spec/models/ee/group_spec.rb @@ -3527,6 +3527,7 @@ def webhook_headers "Guest" => 10, "Planner" => 15, "Reporter" => 20, + "Security Manager" => 25, "Developer" => 30, "Maintainer" => 40, "Owner" => 50 diff --git a/ee/spec/models/instance_security_dashboard_spec.rb b/ee/spec/models/instance_security_dashboard_spec.rb index 4c40ae5bee0fd37a4cc5426bb9d62df5a75e8f5d..3e2f8cfd243f22efc50259f7e8cd55efa25b6867 100644 --- a/ee/spec/models/instance_security_dashboard_spec.rb +++ b/ee/spec/models/instance_security_dashboard_spec.rb @@ -98,7 +98,7 @@ it_behaves_like 'project permissions' end - all_roles = Gitlab::Access.sym_options.keys + all_roles = RolesHelpers.testable_roles permitted_roles = %i[developer maintainer].freeze unpermitted_roles = all_roles - permitted_roles diff --git a/ee/spec/models/members/member_role_spec.rb b/ee/spec/models/members/member_role_spec.rb index 82aa2726d4fbb82ea62ab3f26a0ec5235d58c0ea..15eecb44399653b939daa843a2adaf090857cc26 100644 --- a/ee/spec/models/members/member_role_spec.rb +++ b/ee/spec/models/members/member_role_spec.rb @@ -546,7 +546,8 @@ describe '.levels_sentence' do it 'returns the list of access levels with names' do expect(described_class.levels_sentence).to eq( - "10 (Guest), 15 (Planner), 20 (Reporter), 30 (Developer), 40 (Maintainer), and 50 (Owner)" + "10 (Guest), 15 (Planner), 20 (Reporter), 25 (Security Manager), 30 (Developer), " \ + "40 (Maintainer), and 50 (Owner)" ) end end diff --git a/ee/spec/policies/group_policy_spec.rb b/ee/spec/policies/group_policy_spec.rb index 23dcb2a00faf64b1d1dc9ade43e9946bd6ed5e96..a8e716055841147a1d17145c1aa5e488d4b5e20b 100644 --- a/ee/spec/policies/group_policy_spec.rb +++ b/ee/spec/policies/group_policy_spec.rb @@ -3157,7 +3157,7 @@ def set_access_level(access_level) subject { described_class.new(user, private_group) } where(:role) do - Gitlab::Access.sym_options_with_owner.keys.map(&:to_sym) + RolesHelpers.testable_roles(include_owner: true) end with_them do @@ -3172,7 +3172,7 @@ def set_access_level(access_level) let_it_be(:project) { create(:project, :private, namespace: private_group) } where(:role) do - Gitlab::Access.sym_options_with_owner.keys.map(&:to_sym) + RolesHelpers.testable_roles(include_owner: true) end with_them do @@ -3191,7 +3191,7 @@ def set_access_level(access_level) subject { described_class.new(user, public_group) } where(:role) do - Gitlab::Access.sym_options_with_owner.keys.map(&:to_sym) + RolesHelpers.testable_roles(include_owner: true) end with_them do @@ -3220,7 +3220,7 @@ def set_access_level(access_level) end where(:role) do - Gitlab::Access.sym_options_with_owner.keys.map(&:to_sym) + RolesHelpers.testable_roles(include_owner: true) end with_them do diff --git a/ee/spec/policies/project_policy_spec.rb b/ee/spec/policies/project_policy_spec.rb index d3f2d27fa3c85059b285cf1320d7361283c25a99..ed4244031c4ba9a25242011eceadf95416201308 100644 --- a/ee/spec/policies/project_policy_spec.rb +++ b/ee/spec/policies/project_policy_spec.rb @@ -2365,7 +2365,7 @@ let_it_be(:project) { create(:project, :private, public_builds: false) } where(:role) do - Gitlab::Access.sym_options.keys.map(&:to_sym) + RolesHelpers.testable_roles end with_them do @@ -2385,7 +2385,7 @@ end where(:role) do - Gitlab::Access.sym_options_with_owner.keys.map(&:to_sym) + RolesHelpers.testable_roles(include_owner: true) end with_them do @@ -2406,7 +2406,7 @@ end where(:role) do - Gitlab::Access.sym_options_with_owner.keys.map(&:to_sym) + RolesHelpers.testable_roles(include_owner: true) end with_them do @@ -2423,7 +2423,7 @@ let_it_be(:project) { create(:project, :private, public_builds: false, namespace: subgroup) } where(:role) do - Gitlab::Access.sym_options_with_owner.keys.map(&:to_sym) + RolesHelpers.testable_roles(include_owner: true) end with_them do diff --git a/ee/spec/presenters/group_member_presenter_spec.rb b/ee/spec/presenters/group_member_presenter_spec.rb index 0d244ea165cb07ceb45924559c774e2b71000e81..00c4ea54f586233af48647cc9ea9aeee4841e959 100644 --- a/ee/spec/presenters/group_member_presenter_spec.rb +++ b/ee/spec/presenters/group_member_presenter_spec.rb @@ -108,7 +108,7 @@ context 'with minimal access role feature switched off' do it_behaves_like '#valid_level_roles', :group do - let(:expected_roles) { { 'Developer' => 30, 'Maintainer' => 40, 'Owner' => 50, 'Reporter' => 20 } } + let(:expected_roles) { { 'Developer' => 30, 'Maintainer' => 40, 'Owner' => 50, 'Reporter' => 20, 'Security Manager' => 25 } } before do entity.update!(parent: group) diff --git a/ee/spec/requests/api/graphql/group/standard_roles_spec.rb b/ee/spec/requests/api/graphql/group/standard_roles_spec.rb index 9217a721a485767b81a47c5f1d9d7b547b0749ca..36f709e1e2259dffb762c8d9833a2559c1f2afbf 100644 --- a/ee/spec/requests/api/graphql/group/standard_roles_spec.rb +++ b/ee/spec/requests/api/graphql/group/standard_roles_spec.rb @@ -63,6 +63,7 @@ def run_query(query) [::Gitlab::Access::GUEST, 0], [::Gitlab::Access::PLANNER, 0], [::Gitlab::Access::REPORTER, 0], + [::Gitlab::Access::SECURITY_MANAGER, 0], [::Gitlab::Access::DEVELOPER, 2], [::Gitlab::Access::MAINTAINER, 2], [::Gitlab::Access::OWNER, 1 + 2] @@ -93,6 +94,7 @@ def run_query(query) [::Gitlab::Access::GUEST, 0], [::Gitlab::Access::PLANNER, 0], [::Gitlab::Access::REPORTER, 0], + [::Gitlab::Access::SECURITY_MANAGER, 0], [::Gitlab::Access::DEVELOPER, 2], [::Gitlab::Access::MAINTAINER, 2], [::Gitlab::Access::OWNER, 1 + 2] @@ -126,6 +128,7 @@ def run_query(query) [::Gitlab::Access::GUEST, 0], [::Gitlab::Access::PLANNER, 0], [::Gitlab::Access::REPORTER, 0], + [::Gitlab::Access::SECURITY_MANAGER, 0], [::Gitlab::Access::DEVELOPER, 2 + 3], [::Gitlab::Access::MAINTAINER, 2 + 3], [::Gitlab::Access::OWNER, 1 + 2 + 3] diff --git a/ee/spec/requests/api/graphql/members/group_standard_roles_spec.rb b/ee/spec/requests/api/graphql/members/group_standard_roles_spec.rb index 3655203885e2363164062f3806ddf90d4b675b41..1e7a06af9bed48e5f7ad2e1e6a2825e023da57ff 100644 --- a/ee/spec/requests/api/graphql/members/group_standard_roles_spec.rb +++ b/ee/spec/requests/api/graphql/members/group_standard_roles_spec.rb @@ -75,6 +75,13 @@ def standard_roles_query 'usersCount' => 0, 'detailsPath' => '/admin/application_settings/roles_and_permissions/REPORTER' }, + { + 'accessLevel' => 25, + 'name' => 'Security Manager', + 'membersCount' => 0, + 'usersCount' => 0, + 'detailsPath' => '/admin/application_settings/roles_and_permissions/SECURITY_MANAGER' + }, { 'accessLevel' => 30, 'name' => 'Developer', diff --git a/ee/spec/requests/api/graphql/members/instance_standard_roles_spec.rb b/ee/spec/requests/api/graphql/members/instance_standard_roles_spec.rb index 785b3ac7134a672bd4d7dbe145cf1a26b59a6d2e..5425dd68ba48f1dff410fb8527c04f957aa2ab33 100644 --- a/ee/spec/requests/api/graphql/members/instance_standard_roles_spec.rb +++ b/ee/spec/requests/api/graphql/members/instance_standard_roles_spec.rb @@ -74,6 +74,13 @@ def standard_roles_query 'usersCount' => 0, 'detailsPath' => '/admin/application_settings/roles_and_permissions/REPORTER' }, + { + 'accessLevel' => 25, + 'name' => 'Security Manager', + 'membersCount' => 0, + 'usersCount' => 0, + 'detailsPath' => '/admin/application_settings/roles_and_permissions/SECURITY_MANAGER' + }, { 'accessLevel' => 30, 'name' => 'Developer', diff --git a/ee/spec/services/ee/clusters/agents/authorize_proxy_user_service_spec.rb b/ee/spec/services/ee/clusters/agents/authorize_proxy_user_service_spec.rb index 3c37e53f29fd76e7ea70a0f90f9e1d9b483b1997..388e722af362e25088ca624eaf4aaf393c2c21b1 100644 --- a/ee/spec/services/ee/clusters/agents/authorize_proxy_user_service_spec.rb +++ b/ee/spec/services/ee/clusters/agents/authorize_proxy_user_service_spec.rb @@ -43,8 +43,8 @@ expect(service_response).to be_success expect(service_response.payload[:access_as]).to eq({ user: { - projects: [{ id: deployment_project.id, roles: %i[guest planner reporter developer] }], - groups: [{ id: deployment_group.id, roles: %i[guest planner reporter developer maintainer] }] + projects: [{ id: deployment_project.id, roles: %i[guest planner reporter security_manager developer] }], + groups: [{ id: deployment_group.id, roles: %i[guest planner reporter security_manager developer maintainer] }] } }) end diff --git a/ee/spec/support/shared_examples/models/secrets_management/secrets_permission_examples.rb b/ee/spec/support/shared_examples/models/secrets_management/secrets_permission_examples.rb index 64440bbde6a5a85e38acf683fa8b938fb6e67581..238ac83b0bdaf3087e7344f21f559fe88c872c50 100644 --- a/ee/spec/support/shared_examples/models/secrets_management/secrets_permission_examples.rb +++ b/ee/spec/support/shared_examples/models/secrets_management/secrets_permission_examples.rb @@ -101,7 +101,7 @@ permission.principal_id = 999 # Invalid role ID expect(permission).not_to be_valid expect(permission.errors[:principal_id][0]) - .to match(/must be one of: reporter \(20\), developer \(30\), maintainer \(40\)/) + .to match(/must be one of: reporter \(20\), security_manager \(25\), developer \(30\), maintainer \(40\)/) end end diff --git a/spec/lib/gitlab/access_spec.rb b/spec/lib/gitlab/access_spec.rb index 92c321da84050cb6726e3aca4d2e7a467adfcc24..01e9781601129a7cefea6f1af5216c382591888e 100644 --- a/spec/lib/gitlab/access_spec.rb +++ b/spec/lib/gitlab/access_spec.rb @@ -53,61 +53,49 @@ def level_encompasses?(current_level, level_to_assign) end describe '.options' do - it 'returns correct role options in correct order' do - expect(described_class.options.keys).to eq(%w[Guest Planner Reporter Developer Maintainer]) + it 'returns roles in correct order' do + expect(described_class.options.keys).to eq(["Guest", "Planner", "Reporter", "Security Manager", "Developer", + "Maintainer"]) end - context 'when security manager role is enabled' do - before do - allow(Gitlab::Security::SecurityManagerConfig).to receive(:enabled?).and_return(true) - end - - it 'returns roles in correct order between Reporter and Developer' do - expect(described_class.options.keys).to eq(["Guest", "Planner", "Reporter", "Security Manager", "Developer", - "Maintainer"]) - end + it 'includes Security Manager with correct access level' do + expect(described_class.options['Security Manager']).to eq(25) + end - it 'includes Security Manager with correct access level' do - expect(described_class.options['Security Manager']).to eq(25) + context 'when the security manager role is disabled', :disable_security_manager do + it 'does not include Security Manager' do + expect(described_class.options.keys).to eq(%w[Guest Planner Reporter Developer Maintainer]) end end end describe '.option_descriptions' do - it 'does not include Security Manager description' do - expect(described_class.option_descriptions).not_to have_key(25) + it 'includes Security Manager description' do + expect(described_class.option_descriptions).to have_key(25) + expect(described_class.option_descriptions[25]).to include('Security Manager') end - context 'when security manager role is enabled' do - before do - allow(Gitlab::Security::SecurityManagerConfig).to receive(:enabled?).and_return(true) - end - - it 'includes Security Manager description' do - expect(described_class.option_descriptions).to have_key(25) - expect(described_class.option_descriptions[25]).to include('Security Manager') + context 'when the security manager role is disabled', :disable_security_manager do + it 'does not include Security Manager description' do + expect(described_class.option_descriptions).not_to have_key(25) end end end describe '.sym_options' do it 'returns roles in correct order' do - expect(described_class.sym_options.keys).to eq([:guest, :planner, :reporter, :developer, :maintainer]) + expect(described_class.sym_options.keys).to eq( + [:guest, :planner, :reporter, :security_manager, :developer, :maintainer] + ) end - context 'when security manager role is enabled' do - before do - allow(Gitlab::Security::SecurityManagerConfig).to receive(:enabled?).and_return(true) - end - - it 'returns roles in correct order between reporter and developer' do - expect(described_class.sym_options.keys).to eq( - [:guest, :planner, :reporter, :security_manager, :developer, :maintainer] - ) - end + it 'includes security_manager with correct access level' do + expect(described_class.sym_options[:security_manager]).to eq(25) + end - it 'includes security_manager with correct access level' do - expect(described_class.sym_options[:security_manager]).to eq(25) + context 'when the security manager role is disabled', :disable_security_manager do + it 'does not include security_manager' do + expect(described_class.sym_options.keys).to eq([:guest, :planner, :reporter, :developer, :maintainer]) end end end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 5baa2bb5daab58b531bc4de21e871a510732ce5f..52c99203400d84c54244f86434304742d03f586e 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -3587,6 +3587,7 @@ def define_cache_expectations(cache_key) 'Guest' => 10, 'Planner' => 15, 'Reporter' => 20, + 'Security Manager' => 25, 'Developer' => 30, 'Maintainer' => 40, 'Owner' => 50 diff --git a/spec/models/user_highest_role_spec.rb b/spec/models/user_highest_role_spec.rb index 0894c3b20276361bd507a72f2100ba584fe9cce5..234327caac908960420af5c9dc65fdbe22db7f9c 100644 --- a/spec/models/user_highest_role_spec.rb +++ b/spec/models/user_highest_role_spec.rb @@ -33,6 +33,7 @@ Gitlab::Access::GUEST, Gitlab::Access::PLANNER, Gitlab::Access::REPORTER, + Gitlab::Access::SECURITY_MANAGER, Gitlab::Access::DEVELOPER, Gitlab::Access::MAINTAINER, Gitlab::Access::OWNER diff --git a/spec/presenters/group_member_presenter_spec.rb b/spec/presenters/group_member_presenter_spec.rb index 259928711604cfb21aa90cddf6b3dcdebfa5675a..56dec74ebf5e5ad847ad9e35b57a6e18b42812b0 100644 --- a/spec/presenters/group_member_presenter_spec.rb +++ b/spec/presenters/group_member_presenter_spec.rb @@ -157,10 +157,24 @@ end it_behaves_like '#valid_level_roles', :group do - let(:expected_roles) { { 'Developer' => 30, 'Maintainer' => 40, 'Owner' => 50, 'Reporter' => 20 } } + let(:expected_roles) do + { 'Developer' => 30, 'Maintainer' => 40, 'Owner' => 50, 'Reporter' => 20, 'Security Manager' => 25 } + end before do entity.update!(parent: group) end end + + context 'when security manager is disabled', :disable_security_manager do + it_behaves_like '#valid_level_roles', :group do + let(:expected_roles) do + { 'Developer' => 30, 'Maintainer' => 40, 'Owner' => 50, 'Reporter' => 20 } + end + + before do + entity.update!(parent: group) + end + end + end end diff --git a/spec/presenters/member_presenter_spec.rb b/spec/presenters/member_presenter_spec.rb index ebf2e847a0874526bb02da43555b93936099732b..b620bcf4da9d28ac5b0926f834bccdb9495a81ab 100644 --- a/spec/presenters/member_presenter_spec.rb +++ b/spec/presenters/member_presenter_spec.rb @@ -39,6 +39,7 @@ it 'does not return levels lower than user highest membership in the hierarchy' do expect(described_class.new(subgroup_member).valid_level_roles).to eq( 'Reporter' => Gitlab::Access::REPORTER, + 'Security Manager' => Gitlab::Access::SECURITY_MANAGER, 'Developer' => Gitlab::Access::DEVELOPER, 'Maintainer' => Gitlab::Access::MAINTAINER, 'Owner' => Gitlab::Access::OWNER @@ -50,6 +51,7 @@ 'Guest' => Gitlab::Access::GUEST, 'Planner' => Gitlab::Access::PLANNER, 'Reporter' => Gitlab::Access::REPORTER, + 'Security Manager' => Gitlab::Access::SECURITY_MANAGER, 'Developer' => Gitlab::Access::DEVELOPER, 'Maintainer' => Gitlab::Access::MAINTAINER, 'Owner' => Gitlab::Access::OWNER diff --git a/spec/requests/api/ci/jobs_spec.rb b/spec/requests/api/ci/jobs_spec.rb index 6d71d288d618086651538a686337b2f8e472c118..337cd4aecc36065537c4a41f64636e8da813e69d 100644 --- a/spec/requests/api/ci/jobs_spec.rb +++ b/spec/requests/api/ci/jobs_spec.rb @@ -303,7 +303,7 @@ def perform_request expect(json_response.dig('project', 'groups')).to match_array([{ 'id' => group.id }]) expect(json_response.dig('user', 'id')).to eq(api_user.id) expect(json_response.dig('user', 'username')).to eq(api_user.username) - expect(json_response.dig('user', 'roles_in_project')).to match_array %w[guest planner reporter developer] + expect(json_response.dig('user', 'roles_in_project')).to match_array %w[guest planner reporter developer security_manager] expect(json_response).not_to include('environment') end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 3f3ca8ce92f6c31785be593bd3fe8afe8d9a9b45..2aa4c1eeef314086d6302b165d7f23a871bac0a6 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -23,6 +23,7 @@ ENV["RAILS_ENV"] = 'test' ENV["IN_MEMORY_APPLICATION_SETTINGS"] = 'true' +ENV["GITLAB_SECURITY_MANAGER_ROLE"] = 'true' require_relative '../config/environment' @@ -443,6 +444,12 @@ end end + # Security manager is enabled by default via ENV var + # Tests can opt out with :disable_security_manager tag + if example.metadata[:disable_security_manager] + stub_env('GITLAB_SECURITY_MANAGER_ROLE', 'false') + end + # Make sure specs test by default admin mode setting on, unless forced to the opposite stub_application_setting(admin_mode: true) unless example.metadata[:do_not_mock_admin_mode_setting] diff --git a/spec/support/helpers/roles_helpers.rb b/spec/support/helpers/roles_helpers.rb index f0eddc74eea5936206b0b16d83c0bd75aad76c1d..05918ac27a8ec0566de68002c7f403a16edd090c 100644 --- a/spec/support/helpers/roles_helpers.rb +++ b/spec/support/helpers/roles_helpers.rb @@ -5,9 +5,10 @@ module RolesHelpers def assignable_roles { - owner: [:owner, :maintainer, :planner, :developer, :reporter, :guest], + owner: [:owner, :maintainer, :planner, :developer, :security_manager, :reporter, :guest], maintainer: [:maintainer, :developer, :reporter, :guest], developer: [:developer, :reporter, :guest], + security_manager: [:security_manager, :reporter, :guest], reporter: [:reporter, :guest], planner: [:planner, :guest], guest: [:guest] @@ -17,4 +18,10 @@ def assignable_roles def access_level_value(name) Gitlab::Access.sym_options_with_owner[name] end + + def testable_roles(include_owner: false) + roles = include_owner ? Gitlab::Access.sym_options_with_owner.keys : Gitlab::Access.sym_options.keys + ## TODO: remove this hack in the MR that added the add_security_manager method (https://gitlab.com/gitlab-org/gitlab/-/merge_requests/215015) + roles.map(&:to_sym).reject { |role| role == :security_manager } + end end diff --git a/spec/support/known_rspec_metadata_keys.yml b/spec/support/known_rspec_metadata_keys.yml index 866c28daf707a7434feed8560a0aae121a8bd01b..491d124077683b64fa54bda408de977254f0e6a8 100644 --- a/spec/support/known_rspec_metadata_keys.yml +++ b/spec/support/known_rspec_metadata_keys.yml @@ -175,3 +175,4 @@ - :zoekt - :zoekt_cache_disabled - :zoekt_settings_enabled +- :disable_security_manager