diff --git a/db/migrate/20230522210320_add_admin_vulnerability_to_member_roles.rb b/db/migrate/20230522210320_add_admin_vulnerability_to_member_roles.rb new file mode 100644 index 0000000000000000000000000000000000000000..f3f68f007dd75e13d499b710b8ebf2e8a99d8f35 --- /dev/null +++ b/db/migrate/20230522210320_add_admin_vulnerability_to_member_roles.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddAdminVulnerabilityToMemberRoles < Gitlab::Database::Migration[2.1] + def change + add_column :member_roles, :admin_vulnerability, :boolean, default: false, null: false + end +end diff --git a/db/schema_migrations/20230522210320 b/db/schema_migrations/20230522210320 new file mode 100644 index 0000000000000000000000000000000000000000..74a32da1d1a5720eb6429415a5acf2e5384f10a6 --- /dev/null +++ b/db/schema_migrations/20230522210320 @@ -0,0 +1 @@ +6c287edd3102743f83a94b3f961074eb2d85c63c7f7789c63cb51a11ef4c04c1 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 3dfcb3ef38ed1a1cbf5cf79016ad01c795d2a77a..e761bcb557504c68657c457d8d0411365b635a78 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -17843,7 +17843,8 @@ CREATE TABLE member_roles ( updated_at timestamp with time zone NOT NULL, base_access_level integer NOT NULL, read_code boolean DEFAULT false, - read_vulnerability boolean DEFAULT false NOT NULL + read_vulnerability boolean DEFAULT false NOT NULL, + admin_vulnerability boolean DEFAULT false NOT NULL ); CREATE SEQUENCE member_roles_id_seq diff --git a/ee/app/models/members/member_role.rb b/ee/app/models/members/member_role.rb index f2fcb40310eb537e9547c3214dc24399a43f7b5e..05b454e71e1d31e1edc106f4a72c34265470b624 100644 --- a/ee/app/models/members/member_role.rb +++ b/ee/app/models/members/member_role.rb @@ -9,7 +9,12 @@ class MemberRole < ApplicationRecord # rubocop:disable Gitlab/NamespacedClass read_code: { description: 'Permission to read code', minimal_level: Gitlab::Access::GUEST }, read_vulnerability: - { descripition: 'Permission to read vulnerability', minimal_level: Gitlab::Access::GUEST } + { descripition: 'Permission to read vulnerability', minimal_level: Gitlab::Access::GUEST }, + admin_vulnerability: { + descripition: 'Permission to admin vulnerability', + minimal_level: Gitlab::Access::GUEST, + requirement: :read_vulnerability + } }.freeze CUSTOMIZABLE_PERMISSIONS_EXEMPT_FROM_CONSUMING_SEAT = [:read_code].freeze NON_PERMISSION_COLUMNS = [:id, :namespace_id, :created_at, :updated_at, :base_access_level].freeze @@ -26,6 +31,9 @@ class MemberRole < ApplicationRecord # rubocop:disable Gitlab/NamespacedClass validate :validate_minimal_base_access_level, if: ->(member_role) do Feature.enabled?(:custom_roles_vulnerability, member_role.namespace&.root_ancestor) end + validate :validate_requirements, if: ->(member_role) do + Feature.enabled?(:custom_roles_vulnerability, member_role.namespace&.root_ancestor) + end validates_associated :members @@ -85,6 +93,21 @@ def validate_minimal_base_access_level end end + def validate_requirements + ALL_CUSTOMIZABLE_PERMISSIONS.each do |permission, params| + requirement = params[:requirement] + + next unless self[permission] # skipping permissions not set for the object + next unless requirement # skipping permissions that have no requirement + next if self[requirement] # the requierement is met + + errors.add(permission, + format(s_("MemberRole|%{requirement} has to be enabled in order to enable %{permission}."), + requirement: requirement, permission: permission) + ) + end + end + def attributes_locked_after_member_associated return unless members.present? diff --git a/ee/app/policies/ee/project_policy.rb b/ee/app/policies/ee/project_policy.rb index 6dda1d176ee5d42394726361c2d9c661d9c09806..21caa86bea7c11cfdbed1e11c9574a7829b94be5 100644 --- a/ee/app/policies/ee/project_policy.rb +++ b/ee/app/policies/ee/project_policy.rb @@ -225,6 +225,13 @@ module ProjectPolicy @user.custom_permission_for?(project, :read_vulnerability) end + desc "Custom role on project that enables admin vulnerability" + condition(:role_enables_admin_vulnerability) do + next unless @user.is_a?(User) + + @user.custom_permission_for?(project, :admin_vulnerability) + end + with_scope :subject condition(:suggested_reviewers_available) do @subject.can_suggest_reviewers? @@ -565,6 +572,9 @@ module ProjectPolicy enable :read_security_resource enable :create_vulnerability_export end + rule { custom_roles_allowed & custom_roles_vulnerabilities_allowed & role_enables_admin_vulnerability }.policy do + enable :admin_vulnerability + end rule { can?(:create_issue) & okrs_enabled }.policy do enable :create_objective diff --git a/ee/spec/lib/ee/api/entities/member_role_spec.rb b/ee/spec/lib/ee/api/entities/member_role_spec.rb index 87bae88d76595054443c53b6d3a91e2d6fb9aabc..8972656071cba320dcca7e00cefd01aec243a185 100644 --- a/ee/spec/lib/ee/api/entities/member_role_spec.rb +++ b/ee/spec/lib/ee/api/entities/member_role_spec.rb @@ -17,6 +17,7 @@ expect(subject[:base_access_level]).to eq member_role.base_access_level expect(subject[:read_code]).to eq member_role.read_code expect(subject[:read_vulnerability]).to eq member_role.read_vulnerability + expect(subject[:admin_vulnerability]).to eq member_role.admin_vulnerability expect(subject[:group_id]).to eq(member_role.namespace.id) end end diff --git a/ee/spec/models/members/member_role_spec.rb b/ee/spec/models/members/member_role_spec.rb index e28df6c1bfb8a73ef0d4a2f694e3a3feecaa0024..c30d6eb19674396eb1c98db1016547bed2c2f5c6 100644 --- a/ee/spec/models/members/member_role_spec.rb +++ b/ee/spec/models/members/member_role_spec.rb @@ -126,6 +126,32 @@ end end end + + context 'when requirement is not met' do + context 'when custom_roles_vulnerability FF is enabled' do + it 'creates a validation error' do + member_role.base_access_level = Gitlab::Access::GUEST + member_role.admin_vulnerability = true + + expect(member_role).not_to be_valid + expect(member_role.errors[:admin_vulnerability]) + .to include(s_("MemberRole|read_vulnerability has to be enabled in order to enable admin_vulnerability.")) + end + end + + context 'when custom_roles_vulnerability FF is disabled' do + before do + stub_feature_flags(custom_roles_vulnerability: false) + end + + it 'is valid' do + member_role.base_access_level = Gitlab::Access::GUEST + member_role.admin_vulnerability = true + + expect(member_role).to be_valid + end + end + end end end diff --git a/ee/spec/models/preloaders/user_member_roles_in_projects_preloader_spec.rb b/ee/spec/models/preloaders/user_member_roles_in_projects_preloader_spec.rb index 0db84fb0101a3d49846c47c4b22649a81a7e8d4f..93fe43a4326cb40264a372ff40016088ef601bb3 100644 --- a/ee/spec/models/preloaders/user_member_roles_in_projects_preloader_spec.rb +++ b/ee/spec/models/preloaders/user_member_roles_in_projects_preloader_spec.rb @@ -11,15 +11,27 @@ subject(:result) { described_class.new(projects: project_list, user: user).execute } + def ability_requirement(ability) + ability_definition = MemberRole::ALL_CUSTOMIZABLE_PERMISSIONS[ability] + ability_definition[:requirement] + end + + def create_member_role(ability, member) + create(:member_role, :guest, namespace: project.group).tap do |record| + record[ability] = true + record[ability_requirement(ability)] = true if ability_requirement(ability) + record.save! + record.members << member + end + end + shared_examples 'custom roles' do |ability| + let(:expected_abilities) { [ability, ability_requirement(ability)].compact } + context 'when custom_roles license is not enabled on project root ancestor' do it 'skips preload' do stub_licensed_features(custom_roles: false) - create(:member_role, :guest, namespace: project.group).tap do |record| - record[ability] = true - record.save! - record.members << project_member - end + create_member_role(ability, project_member) expect(result).to eq({}) end @@ -32,17 +44,13 @@ context 'when project has custom role' do let_it_be(:member_role) do - create(:member_role, :guest, namespace: project.group).tap do |record| - record[ability] = true - record.save! - record.members << project_member - end + create_member_role(ability, project_member) end context 'when custom role has ability: true' do context 'when Array of project passed' do it 'returns the project_id with a value array that includes the ability' do - expect(result).to eq({ project.id => [ability] }) + expect(result[project.id]).to match_array(expected_abilities) end end @@ -50,7 +58,7 @@ let(:project_list) { Project.where(id: project.id) } it 'returns the project_id with a value array that includes the ability' do - expect(result).to eq({ project.id => [ability] }) + expect(result[project.id]).to match_array(expected_abilities) end end end @@ -59,15 +67,11 @@ context 'when project namespace has a custom role with ability: true' do let_it_be(:group_member) { create(:group_member, :guest, user: user, source: project.namespace) } let_it_be(:member_role) do - create(:member_role, :guest, namespace: project.group).tap do |record| - record[ability] = true - record.save! - record.members << group_member - end + create_member_role(ability, group_member) end it 'returns the project_id with a value array that includes the ability' do - expect(result).to eq({ project.id => [ability] }) + expect(result[project.id]).to match_array(expected_abilities) end end @@ -75,18 +79,14 @@ let_it_be(:group_member) { create(:group_member, :guest, user: user, source: project.group) } it 'project value array includes the ability' do + create_member_role(ability, group_member) create(:member_role, :guest, namespace: project.group).tap do |record| record[ability] = false record.save! record.members << project_member end - create(:member_role, :guest, namespace: project.group).tap do |record| - record[ability] = true - record.save! - record.members << project_member - end - expect(result[project.id]).to match_array([ability]) + expect(result[project.id]).to match_array(expected_abilities) end end @@ -123,11 +123,7 @@ # subgroup is within parent group of project but not above project subgroup = create(:group, parent: project.group) subgroup_member = create(:group_member, :guest, user: user, source: subgroup) - _custom_role_outside_hierarchy = create(:member_role, :guest, namespace: project.group).tap do |record| - record[ability] = false - record.save! - record.members << subgroup_member - end + create_member_role(ability, subgroup_member) expect(result).to eq({ project.id => [] }) end @@ -137,4 +133,5 @@ it_behaves_like 'custom roles', :read_code it_behaves_like 'custom roles', :read_vulnerability + it_behaves_like 'custom roles', :admin_vulnerability end diff --git a/ee/spec/policies/project_policy_spec.rb b/ee/spec/policies/project_policy_spec.rb index 51f46747700b5790006be951d89339b42aa6b8c3..79f9c25d89037c9f2ee1a46f56df44fa65003830 100644 --- a/ee/spec/policies/project_policy_spec.rb +++ b/ee/spec/policies/project_policy_spec.rb @@ -2545,6 +2545,10 @@ def create_member_role(member, abilities = member_role_abilities) end it_behaves_like 'custom roles abilities' + + it 'does not enable to admin_vulnerability' do + expect(subject).to be_disallowed(:admin_vulnerability) + end end context 'with custom_roles_vulnerability FF disabled' do @@ -2560,6 +2564,30 @@ def create_member_role(member, abilities = member_role_abilities) it { is_expected.to be_disallowed(*disallowed_abilities) } end end + + context 'for a member role with admin_vulnerability true' do + context 'with custom_roles_vulnerability FF enabled' do + before do + stub_feature_flags(custom_roles_vulnerability: [project.group]) + end + + let(:member_role_abilities) { { read_vulnerability: true, admin_vulnerability: true } } + let(:allowed_abilities) { [:read_vulnerability, :admin_vulnerability] } + + it_behaves_like 'custom roles abilities' + end + + context 'with custom_roles_vulnerability FF disabled' do + before do + stub_feature_flags(custom_roles_vulnerability: false) + create_member_role(group_member_guest) + end + + let(:disallowed_abilities) { [:admin_vulnerability] } + + it { is_expected.to be_disallowed(*disallowed_abilities) } + end + end end describe 'permissions for suggested reviewers bot', :saas do diff --git a/ee/spec/requests/api/member_roles_spec.rb b/ee/spec/requests/api/member_roles_spec.rb index bb255c8a8eb640331deed8181f4b04f5d284f938..8d1b6a99408b69b7ab44292d0f51578ee37d6a41 100644 --- a/ee/spec/requests/api/member_roles_spec.rb +++ b/ee/spec/requests/api/member_roles_spec.rb @@ -96,6 +96,7 @@ "base_access_level" => ::Gitlab::Access::REPORTER, "read_code" => false, "read_vulnerability" => true, + "admin_vulnerability" => false, "group_id" => group_id }, { @@ -103,6 +104,7 @@ "base_access_level" => ::Gitlab::Access::REPORTER, "read_code" => true, "read_vulnerability" => false, + "admin_vulnerability" => false, "group_id" => group_id } ] diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 8ec3ee9bfe7519b4f1677ac2841a62a1baacf354..dde0745e8987722fae11a1aed120957601c7f000 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -27911,6 +27911,9 @@ msgstr "" msgid "MemberInviteEmail|Invitation to join the %{project_or_group} %{project_or_group_name}" msgstr "" +msgid "MemberRole|%{requirement} has to be enabled in order to enable %{permission}." +msgstr "" + msgid "MemberRole|%{role} - custom" msgstr ""