From 431c9f7392b282686470ef6e1073d7df62c28348 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jarka=20Ko=C5=A1anov=C3=A1?= Date: Wed, 29 May 2024 18:17:15 +0200 Subject: [PATCH] Add admin integrations custom permission - added to project and group policies - changed permissions checks for the controllers - allowed users with this custom permission to access the menu Changelog: added EE: true --- .../settings/integrations_controller.rb | 2 +- .../settings/integrations_controller.rb | 2 +- app/policies/group_policy.rb | 1 + app/policies/project_policy.rb | 1 + .../json_schemas/member_role_permissions.json | 3 + doc/api/graphql/reference/index.md | 1 + doc/user/custom_roles/abilities.md | 6 + ee/app/policies/ee/group_policy.rb | 4 + ee/app/policies/ee/project_policy.rb | 4 + .../custom_abilities/admin_integrations.yml | 10 ++ .../ee/sidebars/groups/menus/settings_menu.rb | 1 + .../sidebars/projects/menus/settings_menu.rb | 3 + .../groups/menus/settings_menu_spec.rb | 1 + .../projects/menus/settings_menu_spec.rb | 1 + ee/spec/policies/group_policy_spec.rb | 10 ++ ee/spec/policies/project_policy_spec.rb | 7 + .../admin_integrations/request_spec.rb | 130 ++++++++++++++++++ .../policies/group_policy_shared_context.rb | 1 + .../policies/project_policy_shared_context.rb | 1 + 19 files changed, 187 insertions(+), 2 deletions(-) create mode 100644 ee/config/custom_abilities/admin_integrations.yml create mode 100644 ee/spec/requests/custom_roles/admin_integrations/request_spec.rb diff --git a/app/controllers/groups/settings/integrations_controller.rb b/app/controllers/groups/settings/integrations_controller.rb index 59b24e8103de30..9715fad056d62a 100644 --- a/app/controllers/groups/settings/integrations_controller.rb +++ b/app/controllers/groups/settings/integrations_controller.rb @@ -5,7 +5,7 @@ module Settings class IntegrationsController < Groups::ApplicationController include ::Integrations::Actions - before_action :authorize_admin_group! + before_action :authorize_admin_integrations! feature_category :integrations diff --git a/app/controllers/projects/settings/integrations_controller.rb b/app/controllers/projects/settings/integrations_controller.rb index ad8ddf182b7138..fb304a5b77cd99 100644 --- a/app/controllers/projects/settings/integrations_controller.rb +++ b/app/controllers/projects/settings/integrations_controller.rb @@ -6,7 +6,7 @@ class IntegrationsController < Projects::ApplicationController include ::Integrations::Params include ::InternalRedirect - before_action :authorize_admin_project! + before_action :authorize_admin_integrations! before_action :ensure_integration_enabled, only: [:edit, :update, :test] before_action :integration, only: [:edit, :update, :test] before_action :default_integration, only: [:edit, :update] diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb index ce6f286a95f4c2..554bdf97dfce1e 100644 --- a/app/policies/group_policy.rb +++ b/app/policies/group_policy.rb @@ -267,6 +267,7 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy enable :admin_namespace enable :admin_group_member enable :admin_package + enable :admin_integrations enable :change_visibility_level enable :read_usage_quotas diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index f47861ca9148f9..cca2750d1fd96a 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -569,6 +569,7 @@ class ProjectPolicy < BasePolicy enable :admin_note enable :admin_wiki enable :admin_project + enable :admin_integrations enable :admin_commit_status enable :admin_build enable :admin_container_image diff --git a/app/validators/json_schemas/member_role_permissions.json b/app/validators/json_schemas/member_role_permissions.json index 191cbdb8b39622..e56df0c903036a 100644 --- a/app/validators/json_schemas/member_role_permissions.json +++ b/app/validators/json_schemas/member_role_permissions.json @@ -13,6 +13,9 @@ "admin_group_member": { "type": "boolean" }, + "admin_integrations": { + "type": "boolean" + }, "admin_merge_request": { "type": "boolean" }, diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 684b12a920f753..0f0a3e67f77bab 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -34325,6 +34325,7 @@ Member role permission. | `ADMIN_CICD_VARIABLES` | Create, read, update, and delete CI/CD variables. | | `ADMIN_COMPLIANCE_FRAMEWORK` | Create, read, update, and delete compliance frameworks. Users with this permission can also assign a compliance framework label to a project, and set the default framework of a group. | | `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_INTEGRATIONS` | Create, read, update, and delete integrations with external applications. | | `ADMIN_MERGE_REQUEST` | Allows approval of merge requests. | | `ADMIN_PUSH_RULES` | Configure push rules for repositories at the group or project level. | | `ADMIN_TERRAFORM_STATE` | Execute terraform commands, lock/unlock terraform state files, and remove file versions. | diff --git a/doc/user/custom_roles/abilities.md b/doc/user/custom_roles/abilities.md index 7e9544587a9c7d..e9e5f33682115d 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_terraform_state`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/140759) | | Execute terraform commands, lock/unlock terraform state files, and remove file versions. | GitLab [16.8](https://gitlab.com/gitlab-org/gitlab/-/issues/421789) | | | +## Integrations + +| Name | Required permission | Description | Introduced in | Feature flag | Enabled in | +|:-----|:------------|:------------------|:---------|:--------------|:---------| +| [`admin_integrations`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/TODO) | | Create, read, update, and delete integrations with external applications. | GitLab [17.1](https://gitlab.com/gitlab-org/gitlab/-/issues/460522) | | | + ## Secrets 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 f68aea3688e5c7..9f094c25d8c190 100644 --- a/ee/app/policies/ee/group_policy.rb +++ b/ee/app/policies/ee/group_policy.rb @@ -549,6 +549,10 @@ module GroupPolicy enable :admin_push_rules end + rule { custom_role_enables_admin_integrations }.policy do + enable :admin_integrations + end + rule { can?(:admin_group) | can?(:admin_compliance_framework) | can?(:manage_deploy_tokens) | can?(:manage_merge_request_settings) }.policy do enable :view_edit_page end diff --git a/ee/app/policies/ee/project_policy.rb b/ee/app/policies/ee/project_policy.rb index fd263748cec2a3..6466e0c9b06884 100644 --- a/ee/app/policies/ee/project_policy.rb +++ b/ee/app/policies/ee/project_policy.rb @@ -314,6 +314,10 @@ module ProjectPolicy enable :admin_push_rules end + rule { custom_role_enables_admin_integrations }.policy do + enable :admin_integrations + end + condition(:ci_cancellation_maintainers_only, scope: :subject) do project.ci_cancellation_restriction.maintainers_only_allowed? end diff --git a/ee/config/custom_abilities/admin_integrations.yml b/ee/config/custom_abilities/admin_integrations.yml new file mode 100644 index 00000000000000..28263484ada588 --- /dev/null +++ b/ee/config/custom_abilities/admin_integrations.yml @@ -0,0 +1,10 @@ +--- +name: admin_integrations +description: Create, read, update, and delete integrations with external applications. +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/460522 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/TODO +feature_category: integrations +milestone: '17.1' +group_ability: true +project_ability: true +requirements: [] diff --git a/ee/lib/ee/sidebars/groups/menus/settings_menu.rb b/ee/lib/ee/sidebars/groups/menus/settings_menu.rb index 350cee524ea382..4eb03cc003d7b1 100644 --- a/ee/lib/ee/sidebars/groups/menus/settings_menu.rb +++ b/ee/lib/ee/sidebars/groups/menus/settings_menu.rb @@ -26,6 +26,7 @@ def configure_menu_items else add_menu_item_for_abilities(general_menu_item, [:remove_group, :admin_compliance_framework, :manage_merge_request_settings]) + add_menu_item_for_abilities(integrations_menu_item, :admin_integrations) add_menu_item_for_abilities(access_tokens_menu_item, :read_resource_access_tokens) add_menu_item_for_abilities(repository_menu_item, [:admin_push_rules, :manage_deploy_tokens]) add_menu_item_for_abilities(ci_cd_menu_item, :admin_cicd_variables) diff --git a/ee/lib/ee/sidebars/projects/menus/settings_menu.rb b/ee/lib/ee/sidebars/projects/menus/settings_menu.rb index d34933d170aaed..00a068da2280a3 100644 --- a/ee/lib/ee/sidebars/projects/menus/settings_menu.rb +++ b/ee/lib/ee/sidebars/projects/menus/settings_menu.rb @@ -23,6 +23,9 @@ module SettingsMenu ], ci_cd_menu_item: [ :admin_cicd_variables + ], + integrations_menu_item: [ + :admin_integrations ] }.freeze 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 f16fa33d1502e8..4a1ee1a491bd3e 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 @@ -318,6 +318,7 @@ :manage_group_access_tokens | 'Access Tokens' :manage_merge_request_settings | 'General' :remove_group | 'General' + :admin_integrations | 'Integrations' end with_them do diff --git a/ee/spec/lib/ee/sidebars/projects/menus/settings_menu_spec.rb b/ee/spec/lib/ee/sidebars/projects/menus/settings_menu_spec.rb index d82a4b1f645edf..9047663cec545b 100644 --- a/ee/spec/lib/ee/sidebars/projects/menus/settings_menu_spec.rb +++ b/ee/spec/lib/ee/sidebars/projects/menus/settings_menu_spec.rb @@ -121,6 +121,7 @@ :admin_push_rules | 'Repository' :manage_merge_request_settings | 'Merge requests' :manage_project_access_tokens | 'Access Tokens' + :admin_integrations | 'Integrations' end with_them do diff --git a/ee/spec/policies/group_policy_spec.rb b/ee/spec/policies/group_policy_spec.rb index 13607845cb98da..3fb94033f5dea4 100644 --- a/ee/spec/policies/group_policy_spec.rb +++ b/ee/spec/policies/group_policy_spec.rb @@ -3670,6 +3670,16 @@ def create_member_role(member, abilities = member_role_abilities) it { is_expected.to be_allowed(:admin_merge_request_approval_settings) } end end + + context 'for a custom role with the `admin_integrations` permission' do + let(:member_role_abilities) { { admin_integrations: true } } + + let(:allowed_abilities) do + [:admin_integrations] + 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 82791962f287b9..a7bf4704f02545 100644 --- a/ee/spec/policies/project_policy_spec.rb +++ b/ee/spec/policies/project_policy_spec.rb @@ -3014,6 +3014,13 @@ def create_member_role(member, abilities = member_role_abilities) it_behaves_like 'custom roles abilities' end end + + context 'for a custom role with the `admin_integrations` ability' do + let(:member_role_abilities) { { admin_integrations: true } } + let(:allowed_abilities) { [:admin_integrations] } + + 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_integrations/request_spec.rb b/ee/spec/requests/custom_roles/admin_integrations/request_spec.rb new file mode 100644 index 00000000000000..d9b7cc7535fcd0 --- /dev/null +++ b/ee/spec/requests/custom_roles/admin_integrations/request_spec.rb @@ -0,0 +1,130 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'User with admin_integrations custom role', feature_category: :integrations do + let_it_be(: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_integrations: true) } + let_it_be(:membership) { create(:group_member, :guest, member_role: role, user: user, group: group) } + + before do + stub_licensed_features(custom_roles: true) + + sign_in(user) + end + + describe Groups::Settings::IntegrationsController do + let_it_be(:jira_integration) { create(:jira_integration, :group, group: group) } + + describe '#index' do + it 'user can access the page via a custom role' do + get group_settings_integrations_path(group_id: group) + + expect(response).to have_gitlab_http_status(:ok) + end + end + + describe '#edit' do + it 'user can access the page via a custom role' do + get edit_group_settings_integration_path(group_id: group, id: jira_integration.to_param) + + expect(response).to have_gitlab_http_status(:ok) + end + end + + describe '#update' do + include JiraIntegrationHelpers + + let(:params) { { url: 'https://jira.gitlab-example.com', password: 'password' } } + + before do + stub_jira_integration_test + end + + it 'user can access the page via a custom role' do + put group_settings_integration_path(group_id: group, id: jira_integration.to_param, service: params) + + expect(response).to have_gitlab_http_status(:found) + end + end + + describe '#reset' do + let_it_be(:inheriting_integration) { create(:jira_integration, inherit_from_id: jira_integration.id) } + + it 'user can access the page via a custom role' do + post reset_group_settings_integration_path(group_id: group, id: jira_integration.to_param) + + expect(response).to have_gitlab_http_status(:ok) + end + end + + describe '#test' do + include JiraIntegrationHelpers + + before do + stub_jira_integration_test + end + + it 'user can access the page via a custom role' do + put test_group_settings_integration_path(group_id: group, id: jira_integration.to_param) + + expect(response).to have_gitlab_http_status(:ok) + end + end + end + + describe Projects::Settings::IntegrationsController do + let_it_be(:jira_integration) { create(:jira_integration, project: project) } + + describe '#index' do + it 'user can access the page via a custom role' do + get namespace_project_settings_integrations_path(namespace_id: group, project_id: project) + + expect(response).to have_gitlab_http_status(:ok) + end + end + + describe '#edit' do + it 'user can access the page via a custom role' do + get edit_namespace_project_settings_integration_path(namespace_id: group, project_id: project, + id: jira_integration.to_param) + + expect(response).to have_gitlab_http_status(:ok) + end + end + + describe '#update' do + include JiraIntegrationHelpers + + let(:params) { { url: 'https://jira.gitlab-example.com', password: 'password' } } + + before do + stub_jira_integration_test + end + + it 'user can access the page via a custom role' do + put namespace_project_settings_integration_path(namespace_id: group, project_id: project, + id: jira_integration.to_param, service: params) + + expect(response).to have_gitlab_http_status(:found) + end + end + + describe '#test' do + include JiraIntegrationHelpers + + before do + stub_jira_integration_test + end + + it 'user can access the page via a custom role' do + put test_namespace_project_settings_integration_path(namespace_id: group, project_id: project, + id: jira_integration.to_param) + + expect(response).to have_gitlab_http_status(:ok) + end + end + end +end diff --git a/spec/support/shared_contexts/policies/group_policy_shared_context.rb b/spec/support/shared_contexts/policies/group_policy_shared_context.rb index e399122cc96a59..241ca8cf6fd174 100644 --- a/spec/support/shared_contexts/policies/group_policy_shared_context.rb +++ b/spec/support/shared_contexts/policies/group_policy_shared_context.rb @@ -92,6 +92,7 @@ remove_group view_edit_page manage_merge_request_settings + admin_integrations ] end diff --git a/spec/support/shared_contexts/policies/project_policy_shared_context.rb b/spec/support/shared_contexts/policies/project_policy_shared_context.rb index fc9aa9275f9fa7..984584f72a8b92 100644 --- a/spec/support/shared_contexts/policies/project_policy_shared_context.rb +++ b/spec/support/shared_contexts/policies/project_policy_shared_context.rb @@ -71,6 +71,7 @@ admin_wiki create_deploy_token destroy_deploy_token manage_deploy_tokens push_to_delete_protected_branch read_deploy_token update_snippet destroy_upload admin_member_access_request rename_project manage_merge_request_settings + admin_integrations ] end -- GitLab