diff --git a/app/validators/json_schemas/member_role_permissions.json b/app/validators/json_schemas/member_role_permissions.json index b2c50bb9ad4a2880ff371a4f14be3740eb361340..2257ec489342e4b4f6bb5a1d82cc08f20cd7acc5 100644 --- a/app/validators/json_schemas/member_role_permissions.json +++ b/app/validators/json_schemas/member_role_permissions.json @@ -22,6 +22,9 @@ "admin_terraform_state": { "type": "boolean" }, + "admin_compliance_framework": { + "type": "boolean" + }, "admin_vulnerability": { "type": "boolean" }, diff --git a/app/views/groups/edit.html.haml b/app/views/groups/edit.html.haml index 1c1388013afe5005cbef04bc94aa3b0ae2ff615c..afdb4cc1c550611e365f758ccc353278ea2cdaa5 100644 --- a/app/views/groups/edit.html.haml +++ b/app/views/groups/edit.html.haml @@ -45,7 +45,10 @@ .settings-content = render 'shared/badges/badge_settings' +- if can?(current_user, :admin_compliance_framework, @group) = render_if_exists 'groups/compliance_frameworks', expanded: expanded + +- if can?(current_user, :admin_group, @group) = render_if_exists 'groups/custom_project_templates_setting' = render_if_exists 'groups/templates_setting', expanded: expanded = render_if_exists 'shared/groups/max_pages_size_setting' @@ -60,6 +63,7 @@ = _('Perform advanced options such as changing path, transferring, exporting, or removing the group.') .settings-content = render 'groups/settings/advanced' + - elsif can?(current_user, :remove_group, @group) = render 'groups/settings/remove', group: @group, remove_form_id: 'js-remove-group-form' = render_if_exists 'groups/settings/restore', group: @group diff --git a/app/views/projects/edit.html.haml b/app/views/projects/edit.html.haml index 90837a1a2918f5a618dce6059ff181c58563496a..d185ef2f1309d440c5fc7dbbe3e7758cf3f4ae3c 100644 --- a/app/views/projects/edit.html.haml +++ b/app/views/projects/edit.html.haml @@ -51,8 +51,10 @@ .settings-content = render 'shared/badges/badge_settings' +- if can?(current_user, :admin_compliance_framework, @project) = render_if_exists 'compliance_management/compliance_framework/project_settings', expanded: expanded +- if can?(current_user, :admin_project, @project) = render_if_exists 'projects/settings/default_issue_template' = render 'projects/service_desk_settings' diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 9b9013d64fb7d075df7a8e3bf56fec61bec8ad8f..52982dfb1209cb43a0f97051cad17a07e919313e 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -33106,6 +33106,7 @@ Member role permission. | Value | Description | | ----- | ----------- | | `ADMIN_CICD_VARIABLES` | Create, read, update, and delete CI/CD variables. | +| `ADMIN_COMPLIANCE_FRAMEWORK` | Enables administrator access to the compliance framework. | | `ADMIN_GROUP_MEMBER` | Add or remove users in a group, and assign roles to users. When assigning a role, users with this custom permission must select a role that has the same or fewer permissions as the default role used as the base for their custom role. | | `ADMIN_MERGE_REQUEST` | Allows approval of merge requests. | | `ADMIN_PUSH_RULES` | Configure push rules for repositories at the group or project level. | diff --git a/doc/user/custom_roles/abilities.md b/doc/user/custom_roles/abilities.md index e49f3ec807cefac918d4471d48edb9937a5fcd93..7dcede3beabb932e1f113bf500a66ce1a9878dad 100644 --- a/doc/user/custom_roles/abilities.md +++ b/doc/user/custom_roles/abilities.md @@ -30,6 +30,12 @@ These requirements are documented in the `Required permission` column in the fol | [`admin_merge_request`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/128302) | | Allows approval of merge requests. | GitLab [16.4](https://gitlab.com/gitlab-org/gitlab/-/issues/412708) | | | | [`read_code`](https://gitlab.com/gitlab-org/gitlab/-/issues/376180) | | Allows read-only access to the source code. | GitLab [15.7](https://gitlab.com/gitlab-org/gitlab/-/issues/20277) | `customizable_roles` | GitLab [15.9](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/110810) | +## Compliance management + +| Name | Required permission | Description | Introduced in | Feature flag | Enabled in | +|:-----|:------------|:------------------|:---------|:--------------|:---------| +| [`admin_compliance_framework`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/144183) | | Enables administrator access to the compliance framework. | GitLab [17.0](https://gitlab.com/gitlab-org/gitlab/-/issues/411502) | | | + ## Group and projects | Name | Required permission | Description | Introduced in | Feature flag | Enabled in | diff --git a/ee/app/controllers/ee/groups_controller.rb b/ee/app/controllers/ee/groups_controller.rb index 1f0d486eb4b4d0d49bd5abe865c5fccedc1dab18..b9f880646e490912838b1d4dc4eb71d84d984224 100644 --- a/ee/app/controllers/ee/groups_controller.rb +++ b/ee/app/controllers/ee/groups_controller.rb @@ -15,6 +15,10 @@ module GroupsController before_action :authorize_remove_group!, only: [:destroy, :restore] before_action :check_subscription!, only: [:destroy] + # for general settings certain features can be enabled via custom roles + skip_before_action :authorize_admin_group!, only: [:edit, :update] + before_action :authorize_view_edit_page!, only: [:edit, :update] + before_action do push_frontend_feature_flag(:saas_user_caps_auto_approve_pending_users_on_cap_increase, @group) end diff --git a/ee/app/graphql/mutations/compliance_management/frameworks/create.rb b/ee/app/graphql/mutations/compliance_management/frameworks/create.rb index 742f4bb540645cdf3b3844666ab0915df5bf6740..473f928c98493466d04fc6efed09e9ea23ff4044 100644 --- a/ee/app/graphql/mutations/compliance_management/frameworks/create.rb +++ b/ee/app/graphql/mutations/compliance_management/frameworks/create.rb @@ -20,7 +20,11 @@ class Create < BaseMutation description: 'Parameters to update the compliance framework with.' def resolve(**args) - service = ::ComplianceManagement::Frameworks::CreateService.new(namespace: namespace(args[:namespace_path]), + group = namespace(args[:namespace_path]) + + raise_resource_not_available_error! unless group + + service = ::ComplianceManagement::Frameworks::CreateService.new(namespace: group, params: args[:params].to_h, current_user: current_user).execute diff --git a/ee/app/policies/compliance_management/framework_policy.rb b/ee/app/policies/compliance_management/framework_policy.rb index 815caf6fcad1310afb7eceb9b407225b10550946..b8cc0f821ec62a3ea976592210ee4f0de86ccaf2 100644 --- a/ee/app/policies/compliance_management/framework_policy.rb +++ b/ee/app/policies/compliance_management/framework_policy.rb @@ -16,6 +16,23 @@ class FrameworkPolicy < BasePolicy @user.can?(:read_group, @subject.namespace.root_ancestor) end + condition(:custom_roles_allowed, scope: :subject) do + @subject.namespace.custom_roles_enabled? + end + + desc "Custom role on group that enables managing compliance framework" + condition(:role_enables_admin_compliance_framework) do + ::Auth::MemberRoleAbilityLoader.new( + user: @user, + resource: @subject.namespace, + ability: :admin_compliance_framework + ).has_ability? + end + + condition(:custom_ability_compliance_enabled) do + custom_roles_allowed? && role_enables_admin_compliance_framework? + end + rule { can?(:owner_access) & custom_compliance_frameworks_enabled }.policy do enable :admin_compliance_framework enable :read_compliance_framework @@ -28,5 +45,14 @@ class FrameworkPolicy < BasePolicy rule { can?(:owner_access) & group_level_compliance_pipeline_enabled }.policy do enable :admin_compliance_pipeline_configuration end + + rule { custom_ability_compliance_enabled & custom_compliance_frameworks_enabled }.policy do + enable :admin_compliance_framework + enable :read_compliance_framework + end + + rule { custom_ability_compliance_enabled & group_level_compliance_pipeline_enabled }.policy do + enable :admin_compliance_pipeline_configuration + end end end diff --git a/ee/app/policies/ee/group_policy.rb b/ee/app/policies/ee/group_policy.rb index 9427d209c130b12d197263447c890de4e6660209..86f9c61350297d8bfe8be32017b1a27afbcc6428 100644 --- a/ee/app/policies/ee/group_policy.rb +++ b/ee/app/policies/ee/group_policy.rb @@ -284,6 +284,15 @@ module GroupPolicy ).has_ability? end + desc "Custom role on group that enables managing compliance framework" + condition(:role_enables_admin_compliance_framework) do + ::Auth::MemberRoleAbilityLoader.new( + user: @user, + resource: @subject, + ability: :admin_compliance_framework + ).has_ability? + end + rule { owner & unique_project_download_limit_enabled }.policy do enable :ban_group_member end @@ -607,6 +616,12 @@ module GroupPolicy enable :admin_cicd_variables end + rule { custom_roles_allowed & role_enables_admin_compliance_framework & compliance_framework_available }.policy do + enable :admin_compliance_framework + enable :admin_compliance_pipeline_configuration + enable :read_group_compliance_dashboard + end + rule { custom_roles_allowed & role_enables_remove_group & has_parent }.policy do enable :remove_group end @@ -615,6 +630,10 @@ module GroupPolicy enable :admin_push_rules end + rule { can?(:admin_group) | can?(:admin_compliance_framework) }.policy do + enable :view_edit_page + end + rule { can?(:read_group_security_dashboard) }.policy do enable :create_vulnerability_export enable :read_security_resource diff --git a/ee/app/policies/ee/project_policy.rb b/ee/app/policies/ee/project_policy.rb index 8d3998cba1f89507daea9a861eb4e9049e284f9c..9cf959abdb749ec8d4a6b6b429e1d6da25ef4a1f 100644 --- a/ee/app/policies/ee/project_policy.rb +++ b/ee/app/policies/ee/project_policy.rb @@ -313,6 +313,15 @@ module ProjectPolicy ).has_ability? end + desc "Custom role on project that enables managing compliance framework" + condition(:role_enables_admin_compliance_framework) do + ::Auth::MemberRoleAbilityLoader.new( + user: @user, + resource: @subject, + ability: :admin_compliance_framework + ).has_ability? + end + condition(:developer_access_to_admin_vulnerability) do ::Feature.disabled?(:disable_developer_access_to_admin_vulnerability, subject&.root_namespace) && can?(:developer_access) @@ -760,7 +769,7 @@ module ProjectPolicy enable :remove_project end - rule { can?(:admin_project) | can?(:archive_project) | can?(:remove_project) }.policy do + rule { can?(:admin_project) | can?(:archive_project) | can?(:remove_project) | can?(:admin_compliance_framework) }.policy do enable :view_edit_page end @@ -874,6 +883,10 @@ module ProjectPolicy enable :read_dependency end + rule { custom_roles_allowed & role_enables_admin_compliance_framework & compliance_framework_available }.policy do + enable :admin_compliance_framework + end + rule { can?(:create_issue) & okrs_enabled }.policy do enable :create_objective enable :create_key_result diff --git a/ee/config/custom_abilities/admin_compliance_framework.yml b/ee/config/custom_abilities/admin_compliance_framework.yml new file mode 100644 index 0000000000000000000000000000000000000000..57186cdaed96dd5e660f00a828e6a2ec69583d7c --- /dev/null +++ b/ee/config/custom_abilities/admin_compliance_framework.yml @@ -0,0 +1,10 @@ +--- +name: admin_compliance_framework +description: Enables administrator access to the compliance framework. +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/411502 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/144183 +feature_category: compliance_management +milestone: '17.0' +group_ability: true +project_ability: true +requirement: '' diff --git a/ee/lib/ee/sidebars/groups/menus/settings_menu.rb b/ee/lib/ee/sidebars/groups/menus/settings_menu.rb index e7b1d7ad99b6da5f5bb3ed985bc9494089703752..b54413686c0a62b69b312adc7b067064f3dc06a0 100644 --- a/ee/lib/ee/sidebars/groups/menus/settings_menu.rb +++ b/ee/lib/ee/sidebars/groups/menus/settings_menu.rb @@ -21,18 +21,22 @@ def configure_menu_items add_item(billing_menu_item) add_item(reporting_menu_item) else - add_menu_item_for_ability(general_menu_item, :remove_group) - add_menu_item_for_ability(access_tokens_menu_item, :read_resource_access_tokens) - add_menu_item_for_ability(repository_menu_item, :change_push_rules) - add_menu_item_for_ability(ci_cd_menu_item, :admin_cicd_variables) - add_menu_item_for_ability(billing_menu_item, :read_billing) + add_menu_item_for_abilities(general_menu_item, [:remove_group, :admin_compliance_framework]) + add_menu_item_for_abilities(access_tokens_menu_item, :read_resource_access_tokens) + add_menu_item_for_abilities(repository_menu_item, :change_push_rules) + add_menu_item_for_abilities(ci_cd_menu_item, :admin_cicd_variables) + add_menu_item_for_abilities(billing_menu_item, :read_billing) end end private - def add_menu_item_for_ability(menu_item, ability) - add_item(menu_item) if can?(context.current_user, ability, context.group) + def add_menu_item_for_abilities(menu_item, abilities) + add_item(menu_item) if allowed_any_ability?(abilities) + end + + def allowed_any_ability?(abilities) + Array(abilities).any? { |ability| can?(context.current_user, ability, context.group) } end def roles_and_permissions_menu_item diff --git a/ee/spec/graphql/mutations/compliance_management/frameworks/create_spec.rb b/ee/spec/graphql/mutations/compliance_management/frameworks/create_spec.rb index b64c8444bcd78752b0f6e98860e258e1425674b8..16068b46fe0685f96974b6152ef0fcf9bb66e310 100644 --- a/ee/spec/graphql/mutations/compliance_management/frameworks/create_spec.rb +++ b/ee/spec/graphql/mutations/compliance_management/frameworks/create_spec.rb @@ -65,8 +65,8 @@ context 'namespace does not exist' do let(:params) { valid_params.merge(namespace_path: 'not_a_path') } - it 'returns useful error messages' do - expect(subject[:errors]).to include 'Not permitted to create framework' + it 'raises an error' do + expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) end end diff --git a/ee/spec/lib/ee/sidebars/groups/menus/settings_menu_spec.rb b/ee/spec/lib/ee/sidebars/groups/menus/settings_menu_spec.rb index 72e5e5037462295932541863a86ee22b8f1819f1..a72ffa60b810cf2cfc3cf9c871c938ba69442245 100644 --- a/ee/spec/lib/ee/sidebars/groups/menus/settings_menu_spec.rb +++ b/ee/spec/lib/ee/sidebars/groups/menus/settings_menu_spec.rb @@ -335,5 +335,27 @@ end end end + + context 'when the user is not an owner but has `admin_compliance_framework` custom ability', feature_category: :compliance_management do + let_it_be(:user) { create(:user) } + + subject { menu.renderable_items.find { |e| e.item_id == item_id } } + + before do + allow(Ability).to receive(:allowed?).and_call_original + allow(Ability).to receive(:allowed?).with(user, :admin_group, group).and_return(false) + allow(Ability).to receive(:allowed?).with(user, :admin_compliance_framework, group).and_return(true) + end + + describe 'General menu item' do + let(:item_id) { :general } + + it { is_expected.to be_present } + + it 'does not show any other menu items' do + expect(menu.renderable_items.length).to equal(1) + end + end + end end end diff --git a/ee/spec/policies/compliance_management/framework_policy_spec.rb b/ee/spec/policies/compliance_management/framework_policy_spec.rb index 7a35b35ecb2a2e76ce64055254599291f3432458..c9f86374f1d0ac04b154d211eb331f7c9011d8e2 100644 --- a/ee/spec/policies/compliance_management/framework_policy_spec.rb +++ b/ee/spec/policies/compliance_management/framework_policy_spec.rb @@ -2,12 +2,16 @@ require 'spec_helper' -RSpec.describe ComplianceManagement::FrameworkPolicy do +RSpec.describe ComplianceManagement::FrameworkPolicy, feature_category: :compliance_management do let_it_be(:user) { create(:user) } let_it_be(:group) { create(:group, :private) } let_it_be_with_refind(:framework) { create(:compliance_framework, namespace: group) } + let(:licensed_compliance_features) do + { custom_compliance_frameworks: true, evaluate_group_level_compliance_pipeline: true } + end + subject { described_class.new(user, framework) } shared_examples 'full access to compliance framework administration' do @@ -24,7 +28,7 @@ context 'feature is licensed' do before do - stub_licensed_features(custom_compliance_frameworks: true, evaluate_group_level_compliance_pipeline: true) + stub_licensed_features(licensed_compliance_features) end context 'user is group owner' do @@ -47,6 +51,26 @@ it_behaves_like 'full access to compliance framework administration' end + context 'when user is a guest with admin_compliance_framework custom permission' do + let_it_be(:user) { create(:user) } + let_it_be(:member_role) { create(:member_role, :guest, namespace: group, admin_compliance_framework: true) } + let_it_be(:member) { create(:group_member, :guest, group: group, user: user, member_role: member_role) } + + context 'when custom_roles feature is disabled' do + it { is_expected.to be_allowed(:read_compliance_framework) } + it { is_expected.to be_disallowed(:admin_compliance_framework) } + it { is_expected.to be_disallowed(:admin_compliance_pipeline_configuration) } + end + + context 'when custom_roles feature is enabled' do + before do + stub_licensed_features(licensed_compliance_features.merge(custom_roles: true)) + end + + it_behaves_like 'full access to compliance framework administration' + end + end + context 'user is subgroup member but not the owner of the root namespace' do let_it_be(:user) { create(:user) } diff --git a/ee/spec/policies/group_policy_spec.rb b/ee/spec/policies/group_policy_spec.rb index 790219fbe749ef95cd91fa66bdf9e279db01498b..db576e1706017b2744e83089fb75574cbeaea1cc 100644 --- a/ee/spec/policies/group_policy_spec.rb +++ b/ee/spec/policies/group_policy_spec.rb @@ -3431,6 +3431,28 @@ def create_member_role(member, abilities = member_role_abilities) it_behaves_like 'custom roles abilities' end + context 'for a member role with admin_compliance_framework true' do + let(:member_role_abilities) { { admin_compliance_framework: true } } + let(:allowed_abilities) do + [:admin_compliance_framework, :admin_compliance_pipeline_configuration, :read_group_compliance_dashboard] + end + + context 'when compliance framework feature is available' do + let(:licensed_features) { { custom_compliance_frameworks: true } } + + it_behaves_like 'custom roles abilities' + end + + context 'when compliance framework feature is unavailable' do + before do + create_member_role(group_member_guest) + stub_licensed_features(custom_roles: true, custom_compliance_frameworks: false) + end + + it { is_expected.to be_disallowed(*allowed_abilities) } + end + end + context 'for a custom role with the `remove_group` ability' do let(:member_role_abilities) { { remove_group: true } } let(:allowed_abilities) { [:remove_group, :view_edit_page] } diff --git a/ee/spec/policies/project_policy_spec.rb b/ee/spec/policies/project_policy_spec.rb index 5855b8fce75613b328a9336a37fb4b305906e199..3cca53e07cb9a180f090ff10bb55856bd7a6d633 100644 --- a/ee/spec/policies/project_policy_spec.rb +++ b/ee/spec/policies/project_policy_spec.rb @@ -2839,6 +2839,14 @@ def create_member_role(member, abilities = member_role_abilities) it_behaves_like 'custom roles abilities' end + + context 'for a member role with `custom_compliance_frameworks` true' do + let(:licensed_features) { { compliance_framework: true } } + let(:member_role_abilities) { { admin_compliance_framework: true } } + let(:allowed_abilities) { [:admin_compliance_framework] } + + it_behaves_like 'custom roles abilities' + end end describe 'permissions for suggested reviewers bot', :saas do diff --git a/ee/spec/requests/custom_roles/admin_compliance_framework/request_spec.rb b/ee/spec/requests/custom_roles/admin_compliance_framework/request_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..eccf496aa480fab496a223238cb6470b43b282e3 --- /dev/null +++ b/ee/spec/requests/custom_roles/admin_compliance_framework/request_spec.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'User with admin_compliance_framework custom role', feature_category: :compliance_management do + let_it_be(:current_user) { create(:user) } + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, group: group) } + let_it_be(:role) { create(:member_role, :guest, namespace: group, admin_compliance_framework: true) } + let_it_be(:membership) { create(:group_member, :guest, member_role: role, user: current_user, group: group) } + let_it_be(:framework) { create(:compliance_framework, namespace: group) } + let(:compliance_features) do + { custom_compliance_frameworks: true, evaluate_group_level_compliance_pipeline: true, + group_level_compliance_dashboard: true, compliance_framework: true } + end + + before do + stub_licensed_features(compliance_features.merge(custom_roles: true)) + + sign_in(current_user) + end + + describe GroupsController do + it 'user can see edit a group page via a custom role' do + get edit_group_path(group) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:edit) + end + end + + describe ProjectsController do + it 'user can see edit a project page via a custom role' do + get edit_project_path(project) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:edit) + end + end + + describe Projects::ComplianceFrameworksController do + it 'user can assign a compliance framework to a project via a custom role' do + params = { framework: framework.id } + + post project_compliance_frameworks_path(project), params: params + + expect(response).to have_gitlab_http_status(:redirect) + expect(flash[:notice]).to eq("Project '#{project.name}' was successfully updated.") + end + end +end diff --git a/ee/spec/requests/projects/compliance_frameworks_controller_spec.rb b/ee/spec/requests/projects/compliance_frameworks_controller_spec.rb index 8f03db9da83c0acf052c22cc7968d60208c8abe7..87bff1cfb069b5d7dc3e5a9f15cd64f38e34362d 100644 --- a/ee/spec/requests/projects/compliance_frameworks_controller_spec.rb +++ b/ee/spec/requests/projects/compliance_frameworks_controller_spec.rb @@ -9,7 +9,7 @@ let_it_be(:framework) { create(:compliance_framework, namespace: group) } before do - stub_licensed_features(compliance_framework: true, custom_compliance_frameworks: true) + stub_licensed_features(compliance_framework: true, custom_compliance_frameworks: true, custom_roles: true) login_as(user) end @@ -17,22 +17,41 @@ describe 'POST #create' do let(:params) { { framework: framework.id } } - context 'when user has permissions to update the framework' do - before_all do - project.add_owner(user) - end + subject(:assign_framework) { post project_compliance_frameworks_path(project), params: params } + shared_examples 'setting compliance framework' do it 'redirects with notice message' do - post project_compliance_frameworks_path(project), params: params + assign_framework expect(response).to redirect_to(edit_project_path(project, anchor: 'js-general-project-settings')) expect(flash[:notice]).to eq("Project '#{project.name}' was successfully updated.") end + + it 'sets the compliance framework' do + assign_framework + + expect(project.reload.compliance_framework_setting.compliance_management_framework).to eq(framework) + end + end + + context 'when user is a project owner' do + before_all do + project.add_owner(user) + end + + it_behaves_like 'setting compliance framework' + end + + context 'when user has the permission because of a custom role' do + let_it_be(:role) { create(:member_role, :guest, namespace: group, admin_compliance_framework: true) } + let_it_be(:membership) { create(:group_member, :guest, member_role: role, user: user, group: group) } + + it_behaves_like 'setting compliance framework' end context 'when user does not have permissions to update the framework' do it 'returns a 404' do - post project_compliance_frameworks_path(project), params: params + assign_framework expect(response).to have_gitlab_http_status(:not_found) end