diff --git a/app/validators/json_schemas/member_role_permissions.json b/app/validators/json_schemas/member_role_permissions.json index a4b6109da101dfbb2451d1a1280cd9fda7588e06..b2c50bb9ad4a2880ff371a4f14be3740eb361340 100644 --- a/app/validators/json_schemas/member_role_permissions.json +++ b/app/validators/json_schemas/member_role_permissions.json @@ -16,6 +16,9 @@ "admin_push_rules": { "type": "boolean" }, + "manage_security_policy_link": { + "type": "boolean" + }, "admin_terraform_state": { "type": "boolean" }, diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 53d7c081ad8ad02d5d3806d69784e152265e02e8..3302231de3e6146747f0acad203f9e5347b1ac12 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -32444,6 +32444,7 @@ Member role permission. | `ARCHIVE_PROJECT` | Allows archiving of projects. | | `MANAGE_GROUP_ACCESS_TOKENS` | Create, read, update, and delete group access tokens. When creating a token, users with this custom permission must select a role for that token that has the same or fewer permissions as the default role used as the base for the custom role. | | `MANAGE_PROJECT_ACCESS_TOKENS` | Create, read, update, and delete project access tokens. When creating a token, users with this custom permission must select a role for that token that has the same or fewer permissions as the default role used as the base for the custom role. | +| `MANAGE_SECURITY_POLICY_LINK` | Allows assigning security policy projects. | | `READ_CODE` | Allows read-only access to the source code. | | `READ_DEPENDENCY` | Allows read-only access to the dependencies and licenses. | | `READ_VULNERABILITY` | Read vulnerability reports and security dashboards. | diff --git a/doc/user/custom_roles/abilities.md b/doc/user/custom_roles/abilities.md index 9919b543f1dc1b66aa841371374638bc430bb1c5..e49f3ec807cefac918d4471d48edb9937a5fcd93 100644 --- a/doc/user/custom_roles/abilities.md +++ b/doc/user/custom_roles/abilities.md @@ -56,6 +56,12 @@ These requirements are documented in the `Required permission` column in the fol |:-----|:------------|:------------------|:---------|:--------------|:---------| | [`admin_cicd_variables`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/143369) | | Create, read, update, and delete CI/CD variables. | GitLab [16.10](https://gitlab.com/gitlab-org/gitlab/-/issues/437947) | | | +## Security policy management + +| Name | Required permission | Description | Introduced in | Feature flag | Enabled in | +|:-----|:------------|:------------------|:---------|:--------------|:---------| +| [`manage_security_policy_link`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/148371) | | Allows assigning security policy projects. | GitLab [16.11](https://gitlab.com/gitlab-org/gitlab/-/issues/440226) | | | + ## Source code management | Name | Required permission | Description | Introduced in | Feature flag | Enabled in | diff --git a/ee/app/policies/ee/group_policy.rb b/ee/app/policies/ee/group_policy.rb index b28135a7303c1c89fc5d70f1ab7f4a411d350f82..7b9a55d6db5f58620b068dacd5a056b5bea6d83d 100644 --- a/ee/app/policies/ee/group_policy.rb +++ b/ee/app/policies/ee/group_policy.rb @@ -274,6 +274,15 @@ module GroupPolicy ).has_ability? end + desc "Custom role on group that enables managing security policy links" + condition(:custom_roles_enables_manage_security_policy_link) do + ::Auth::MemberRoleAbilityLoader.new( + user: @user, + resource: @subject, + ability: :manage_security_policy_link + ).has_ability? + end + rule { owner & unique_project_download_limit_enabled }.policy do enable :ban_group_member end @@ -534,6 +543,12 @@ module GroupPolicy enable :modify_security_policy end + rule { custom_roles_allowed & security_orchestration_policies_enabled & custom_roles_enables_manage_security_policy_link }.policy do + enable :read_security_orchestration_policies + enable :read_security_orchestration_policy_project + enable :update_security_orchestration_policy_project + end + rule { security_dashboard_enabled & developer }.policy do enable :read_group_security_dashboard end diff --git a/ee/app/policies/ee/project_policy.rb b/ee/app/policies/ee/project_policy.rb index 2440bcde14fe1fe21496563486ce36bf638328aa..bbaf37caf826f6d65c132a3f85b7abf6d939c582 100644 --- a/ee/app/policies/ee/project_policy.rb +++ b/ee/app/policies/ee/project_policy.rb @@ -495,6 +495,13 @@ module ProjectPolicy enable :modify_security_policy end + rule { custom_roles_allowed & security_orchestration_policies_enabled & custom_roles_enables_manage_security_policy_link }.policy do + enable :read_security_orchestration_policies + enable :read_security_orchestration_policy_project + enable :update_security_orchestration_policy_project + enable :access_security_and_compliance + end + rule { security_dashboard_enabled & can?(:developer_access) }.policy do enable :read_security_resource end @@ -740,6 +747,15 @@ module ProjectPolicy ).has_ability? end + desc "Custom role on project that enables managing security policy links" + condition(:custom_roles_enables_manage_security_policy_link) do + ::Auth::MemberRoleAbilityLoader.new( + user: @user, + resource: @subject, + ability: :manage_security_policy_link + ).has_ability? + end + rule { custom_roles_allowed & custom_role_enables_remove_projects }.policy do enable :remove_project end diff --git a/ee/app/services/security/orchestration/assign_service.rb b/ee/app/services/security/orchestration/assign_service.rb index 54a10a5af52f8d0efebfa757a6cc07122db94bc6..42e7e4f3085bebb63a18f82daa44f35bbc711970 100644 --- a/ee/app/services/security/orchestration/assign_service.rb +++ b/ee/app/services/security/orchestration/assign_service.rb @@ -25,7 +25,7 @@ def execute private def validate_access! - return if current_user.can?(:modify_security_policy, container) + return if current_user.can?(:update_security_orchestration_policy_project, container) raise Gitlab::Access::AccessDeniedError end diff --git a/ee/config/custom_abilities/manage_security_policy_link.yml b/ee/config/custom_abilities/manage_security_policy_link.yml new file mode 100644 index 0000000000000000000000000000000000000000..0e786a08d5605a5f5217d3e4a1934850cb4e245c --- /dev/null +++ b/ee/config/custom_abilities/manage_security_policy_link.yml @@ -0,0 +1,10 @@ +--- +name: manage_security_policy_link +description: Allows assigning security policy projects. +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/440226 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/148371 +feature_category: security_policy_management +milestone: '16.11' +group_ability: true +project_ability: true +available_from_access_level: 50 diff --git a/ee/spec/models/members/member_role_spec.rb b/ee/spec/models/members/member_role_spec.rb index 709b029faa37f5e8a0d4171481c5b0eef3ac50aa..dcd5c8c7bd99988268c54ae47b8a9bb8b743805d 100644 --- a/ee/spec/models/members/member_role_spec.rb +++ b/ee/spec/models/members/member_role_spec.rb @@ -407,17 +407,19 @@ with_them do before do stub_member_access_level(object, role => user) - stub_licensed_features(security_dashboard: true) + stub_licensed_features(security_dashboard: true, security_orchestration_policies: true) end let(:policy) { policy_class.new(user, object) } it 'gives access from the specified access level' do abilities.each do |ability| + granted_permission = exceptions[ability[:name].to_sym] || ability[:name] + if ability[:available_from_access_level] > level - expect(policy).to be_disallowed(ability[:name]) + expect(policy).to be_disallowed(granted_permission) else - expect(policy).to be_allowed(ability[:name]) + expect(policy).to be_allowed(granted_permission) end end end @@ -426,6 +428,11 @@ describe 'available_from_access_level for abilities' do let_it_be(:user) { build_stubbed(:user) } + let_it_be(:exceptions) do + { + manage_security_policy_link: :update_security_orchestration_policy_project + } + end context 'for group abilities' do let_it_be(:object) { build_stubbed(:group) } diff --git a/ee/spec/policies/group_policy_spec.rb b/ee/spec/policies/group_policy_spec.rb index ca8d475b8b6a85e744f16179fbec0ff55afddc0d..d774a08ce13cc4da9589a43503fb0f4ed70993fe 100644 --- a/ee/spec/policies/group_policy_spec.rb +++ b/ee/spec/policies/group_policy_spec.rb @@ -3131,6 +3131,8 @@ def expect_private_group_permissions_as_if_non_member let(:member_role_abilities) { {} } let(:allowed_abilities) { [] } + let(:disallowed_abilities) { [] } + let(:licensed_features) { {} } let(:current_user) { guest } def create_member_role(member, abilities = member_role_abilities) @@ -3148,7 +3150,7 @@ def create_member_role(member, abilities = member_role_abilities) before do create_member_role(group_member_guest) - stub_licensed_features(custom_roles: false) + stub_licensed_features(licensed_features.merge(custom_roles: false)) end it { is_expected.to be_disallowed(*allowed_abilities) } @@ -3156,7 +3158,7 @@ def create_member_role(member, abilities = member_role_abilities) context 'with custom_roles license enabled' do before do - stub_licensed_features(custom_roles: true) + stub_licensed_features(licensed_features.merge(custom_roles: true)) end context 'custom role for parent group membership' do @@ -3166,6 +3168,7 @@ def create_member_role(member, abilities = member_role_abilities) end it { is_expected.to be_allowed(*allowed_abilities) } + it { is_expected.to be_disallowed(*disallowed_abilities) } end context 'when a role does not enable the abilities' do @@ -3180,6 +3183,7 @@ def create_member_role(member, abilities = member_role_abilities) end it { is_expected.to be_allowed(*allowed_abilities) } + it { is_expected.to be_disallowed(*disallowed_abilities) } end context 'when a role does not enable the abilities' do @@ -3321,6 +3325,22 @@ def create_member_role(member, abilities = member_role_abilities) it_behaves_like 'custom roles abilities' end + + context 'for a custom role with the `manage_security_policy_link` ability' do + let(:member_role_abilities) { { manage_security_policy_link: true } } + let(:licensed_features) { { security_orchestration_policies: true } } + + let(:allowed_abilities) do + [:read_security_orchestration_policies, :read_security_orchestration_policy_project, + :update_security_orchestration_policy_project] + end + + let(:disallowed_abilities) do + [:modify_security_policy] + end + + it_behaves_like 'custom roles abilities' + end end context 'for :read_limit_alert' do diff --git a/ee/spec/policies/project_policy_spec.rb b/ee/spec/policies/project_policy_spec.rb index 17a0224c9ad8b2d6bc6c7dcd8ae2cc4b0929026b..8fb9e82b358d7f2d73223aa02ecca9dd6b4d334a 100644 --- a/ee/spec/policies/project_policy_spec.rb +++ b/ee/spec/policies/project_policy_spec.rb @@ -2604,6 +2604,7 @@ def expect_private_project_permissions_as_if_non_member let(:member_role_abilities) { {} } let(:allowed_abilities) { [] } + let(:disallowed_abilities) { [] } let(:current_user) { guest } let(:licensed_features) { {} } @@ -2618,11 +2619,11 @@ def create_member_role(member, abilities = member_role_abilities) end shared_examples 'custom roles abilities' do - context 'without custom_roles license enabled' do + context 'with custom_roles license disabled' do before do create_member_role(group_member_guest) - stub_licensed_features(custom_roles: false) + stub_licensed_features(licensed_features.merge(custom_roles: false)) end it { is_expected.to be_disallowed(*allowed_abilities) } @@ -2640,6 +2641,7 @@ def create_member_role(member, abilities = member_role_abilities) end it { is_expected.to be_allowed(*allowed_abilities) } + it { is_expected.to be_disallowed(*disallowed_abilities) } end context 'when a role does not enable the abilities' do @@ -2654,6 +2656,7 @@ def create_member_role(member, abilities = member_role_abilities) end it { is_expected.to be_allowed(*allowed_abilities) } + it { is_expected.to be_disallowed(*disallowed_abilities) } end context 'when a role does not enable the abilities' do @@ -2765,6 +2768,21 @@ def create_member_role(member, abilities = member_role_abilities) it_behaves_like 'custom roles abilities' end + context 'for a member role with `manage_security_policy_link` true' do + let(:member_role_abilities) { { manage_security_policy_link: true } } + let(:licensed_features) { { security_orchestration_policies: true } } + let(:allowed_abilities) do + [:read_security_orchestration_policies, :update_security_orchestration_policy_project, + :access_security_and_compliance] + end + + let(:disallowed_abilities) do + [:modify_security_policy] + end + + it_behaves_like 'custom roles abilities' + end + context 'when a user is assigned to custom roles in both group and project' do before do stub_licensed_features(custom_roles: true, dependency_scanning: true) diff --git a/ee/spec/requests/custom_roles/manage_security_policy_link/request_spec.rb b/ee/spec/requests/custom_roles/manage_security_policy_link/request_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..c6042c86c48d46177b60dc31e0700ef7e635c32b --- /dev/null +++ b/ee/spec/requests/custom_roles/manage_security_policy_link/request_spec.rb @@ -0,0 +1,147 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'User with manage_security_policy_link custom role', feature_category: :security_policy_management do + let_it_be(:user) { create(:user) } + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, group: group) } + + let_it_be_with_reload(:role) { create(:member_role, :guest, manage_security_policy_link: true, namespace: group) } + let_it_be(:member) { create(:group_member, :guest, user: user, source: group, member_role: role) } + + let_it_be(:policy_management_project) { create(:project, :repository, namespace: group) } + let_it_be(:project_policy_configuration) do + create(:security_orchestration_policy_configuration, + security_policy_management_project: policy_management_project, project: project) + end + + let_it_be(:group_policy_configuration) do + create(:security_orchestration_policy_configuration, :namespace, + security_policy_management_project: policy_management_project, namespace: group) + end + + before do + stub_licensed_features(custom_roles: true, security_orchestration_policies: true) + + allow_next_instance_of(Repository) do |repository| + allow(repository).to receive(:blob_data_at).and_return({ scan_execution_policy: [policy] }.to_yaml) + end + + sign_in(user) + end + + describe 'Controllers endpoints' do + let_it_be(:policy) { build(:scan_execution_policy) } + let_it_be(:type) { 'scan_execution_policy' } + let_it_be(:policy_id) { policy[:name] } + + describe Projects::Security::PoliciesController do + describe "#index" do + it 'user has access via a custom role' do + get project_security_policies_path(project) + + expect(response).to have_gitlab_http_status(:ok) + end + end + end + + describe Groups::Security::PoliciesController do + describe "#index" do + it 'user has access via a custom role' do + get group_security_policies_path(group) + + expect(response).to have_gitlab_http_status(:ok) + end + end + end + end + + describe 'GraphQL mutations' do + include GraphqlHelpers + + let_it_be_with_refind(:policy_project) { create(:project, group: group) } + let(:full_path) { project.full_path } + let(:base_params) do + { + full_path: full_path + + } + end + + let(:addtional_params) { {} } + let(:input) { base_params.merge(addtional_params) } + let(:fields) do + <<~FIELDS + errors + FIELDS + end + + let(:mutation) { graphql_mutation(mutation_name, input, fields) } + let_it_be_with_refind(:policy_project_id) { GitlabSchema.id_from_object(policy_project).to_s } + let(:mutation_name) { nil } + + subject(:execute_mutation) { post_graphql_mutation(mutation, current_user: user) } + + describe Mutations::SecurityPolicy::AssignSecurityPolicyProject do + let(:mutation_name) { :security_policy_project_assign } + let(:addtional_params) { { security_policy_project_id: policy_project_id } } + + context 'for project' do + it 'has access via a custom role' do + execute_mutation + + expect(response).to have_gitlab_http_status(:success) + mutation_response = graphql_mutation_response(:security_policy_project_assign) + expect(mutation_response).to be_present + + expect(mutation_response['errors']).to be_empty + end + end + + context 'for group' do + let(:full_path) { group.full_path } + + it 'has access via a custom role' do + execute_mutation + + expect(response).to have_gitlab_http_status(:success) + mutation_response = graphql_mutation_response(:security_policy_project_assign) + expect(mutation_response).to be_present + + expect(mutation_response['errors']).to be_empty + end + end + end + + describe Mutations::SecurityPolicy::UnassignSecurityPolicyProject do + let(:mutation_name) { :security_policy_project_unassign } + + context 'for project' do + it 'has access via a custom role' do + execute_mutation + + expect(response).to have_gitlab_http_status(:success) + mutation_response = graphql_mutation_response(:security_policy_project_unassign) + expect(mutation_response).to be_present + + expect(mutation_response['errors']).to be_empty + end + end + + context 'for group' do + let(:full_path) { group.full_path } + + it 'has access via a custom role' do + execute_mutation + + expect(response).to have_gitlab_http_status(:success) + mutation_response = graphql_mutation_response(:security_policy_project_unassign) + expect(mutation_response).to be_present + + expect(mutation_response['errors']).to be_empty + end + end + end + end +end diff --git a/ee/spec/services/security/orchestration/assign_service_spec.rb b/ee/spec/services/security/orchestration/assign_service_spec.rb index 7e71e4e1b6022ba0a5e3c7b2dba8e8b916628c4e..3c7572ec64b96b283dba9f95418dee4ed7206d1f 100644 --- a/ee/spec/services/security/orchestration/assign_service_spec.rb +++ b/ee/spec/services/security/orchestration/assign_service_spec.rb @@ -213,7 +213,7 @@ root_ancestor: instance_double(Group, delete_redundant_policy_projects?: true) ) - allow(current_user).to receive(:can?).with(:modify_security_policy, dbl).and_return(true) + allow(current_user).to receive(:can?).with(:update_security_orchestration_policy_project, dbl).and_return(true) allow(dbl_error).to receive(:security_policy_management_project).and_return(policy_project) allow(dbl_error).to receive(:transaction).and_yield allow(dbl_error).to receive(:delete_scan_finding_rules).and_return(nil)