diff --git a/app/assets/javascripts/projects/settings/components/access_dropdown.vue b/app/assets/javascripts/projects/settings/components/access_dropdown.vue index 135e926cedf6660fa2578f14adcfe230b6484216..0e75054a50d6674a0b18bfb50a12206555847161 100644 --- a/app/assets/javascripts/projects/settings/components/access_dropdown.vue +++ b/app/assets/javascripts/projects/settings/components/access_dropdown.vue @@ -9,6 +9,7 @@ import { GlSprintf, } from '@gitlab/ui'; import { debounce, intersectionWith, groupBy, differenceBy, intersectionBy } from 'lodash'; +import glAbilitiesMixin from '~/vue_shared/mixins/gl_abilities_mixin'; import { createAlert } from '~/alert'; import { __, s__, n__ } from '~/locale'; import { getUsers, getGroups, getDeployKeys } from '../api/access_dropdown_api'; @@ -35,6 +36,7 @@ export default { GlAvatar, GlSprintf, }, + mixins: [glAbilitiesMixin()], props: { accessLevelsData: { type: Array, @@ -187,6 +189,9 @@ export default { ...this.getDataForSave(LEVEL_TYPES.DEPLOY_KEY, 'deploy_key_id'), ]; }, + canAdminContainer() { + return this.glAbilities.adminProject || this.glAbilities.adminGroup; + }, }, watch: { query: debounce(function debouncedSearch() { @@ -226,29 +231,45 @@ export default { focusInput() { this.$refs.search?.focusInput(); }, + getGroups() { + return this.groups.length + ? Promise.resolve({ data: this.groups }) + : getGroups({ withProjectAccess: this.groupsWithProjectAccess }); + }, getData({ initial = false } = {}) { this.initialLoading = initial; this.loading = true; if (this.hasLicense) { - Promise.all([ - getDeployKeys(this.query), - getUsers(this.query), - this.groups.length - ? Promise.resolve({ data: this.groups }) - : getGroups({ withProjectAccess: this.groupsWithProjectAccess }), - ]) - .then(([deployKeysResponse, usersResponse, groupsResponse]) => { - this.consolidateData(deployKeysResponse.data, usersResponse.data, groupsResponse.data); - this.setSelected({ initial }); - }) - .catch(() => - createAlert({ message: __('Failed to load groups, users and deploy keys.') }), - ) - .finally(() => { - this.initialLoading = false; - this.loading = false; - }); + if (this.canAdminContainer) { + Promise.all([getDeployKeys(this.query), getUsers(this.query), this.getGroups()]) + .then(([deployKeysResponse, usersResponse, groupsResponse]) => { + this.consolidateData( + deployKeysResponse.data, + usersResponse.data, + groupsResponse.data, + ); + this.setSelected({ initial }); + }) + .catch(() => + createAlert({ message: __('Failed to load groups, users and deploy keys.') }), + ) + .finally(() => { + this.initialLoading = false; + this.loading = false; + }); + } else if (this.glAbilities.adminProtectedBranch) { + Promise.all([getUsers(this.query), this.getGroups()]) + .then(([usersResponse, groupsResponse]) => { + this.consolidateData(null, usersResponse.data, groupsResponse.data); + this.setSelected({ initial }); + }) + .catch(() => createAlert({ message: __('Failed to load groups and users.') })) + .finally(() => { + this.initialLoading = false; + this.loading = false; + }); + } } else { getDeployKeys(this.query) .then((deployKeysResponse) => { @@ -284,27 +305,31 @@ export default { } } - this.deployKeys = deployKeysResponse.map((response) => { - const { - id, - fingerprint, - fingerprint_sha256: fingerprintSha256, - title, - owner: { avatar_url, name, username }, - } = response; + if (this.canAdminContainer) { + this.deployKeys = deployKeysResponse.map((response) => { + const { + id, + fingerprint, + fingerprint_sha256: fingerprintSha256, + title, + owner: { avatar_url, name, username }, + } = response; - const availableFingerprint = fingerprintSha256 || fingerprint; - const shortFingerprint = `(${availableFingerprint.substring(0, 14)}...)`; + const availableFingerprint = fingerprintSha256 || fingerprint; + const shortFingerprint = `(${availableFingerprint.substring(0, 14)}...)`; - return { - id, - title: title.concat(' ', shortFingerprint), - avatar_url, - fullname: name, - username, - type: LEVEL_TYPES.DEPLOY_KEY, - }; - }); + return { + id, + title: title.concat(' ', shortFingerprint), + avatar_url, + fullname: name, + username, + type: LEVEL_TYPES.DEPLOY_KEY, + }; + }); + } else { + this.deployKeys = []; + } }, setSelected({ initial } = {}) { if (initial) { diff --git a/app/assets/javascripts/protected_branches/protected_branch_create.js b/app/assets/javascripts/protected_branches/protected_branch_create.js index 612f801300e5f100a936fbd12e396a430296ff42..f74256425826e2476849fbb41837d3fbb094ed41 100644 --- a/app/assets/javascripts/protected_branches/protected_branch_create.js +++ b/app/assets/javascripts/protected_branches/protected_branch_create.js @@ -143,13 +143,26 @@ export default class ProtectedBranchCreate { }); } + createLimitedSuccessAlert() { + this.alert = createAlert({ + variant: VARIANT_SUCCESS, + containerSelector: '.js-alert-protected-branch-created-container', + message: s__('ProtectedBranch|Protected branch was sucessfully created'), + }); + } + showSuccessAlertIfNeeded() { if (!this.hasProtectedBranchSuccessAlert()) { return; } this.expandAndScroll(PROTECTED_BRANCHES_ANCHOR); - this.createSuccessAlert(); + if (gon.abilities.adminProject || gon.abilities.adminGroup) { + this.createSuccessAlert(); + } else { + this.createLimitedSuccessAlert(); + } + localStorage.removeItem(IS_PROTECTED_BRANCH_CREATED); } diff --git a/app/controllers/groups/settings/repository_controller.rb b/app/controllers/groups/settings/repository_controller.rb index ecd5d814fb613dcbeff676e41f7ddc53ab71e990..6729ea1a01409bb6313bb6fa1ba255789847f5e4 100644 --- a/app/controllers/groups/settings/repository_controller.rb +++ b/app/controllers/groups/settings/repository_controller.rb @@ -9,6 +9,10 @@ class RepositoryController < Groups::ApplicationController before_action :authorize_access!, only: :show before_action :define_deploy_token_variables, if: -> { can?(current_user, :create_deploy_token, @group) } + before_action do + push_frontend_ability(ability: :admin_group, resource: @group, user: current_user) + end + feature_category :continuous_delivery urgency :low diff --git a/app/controllers/projects/settings/ci_cd_controller.rb b/app/controllers/projects/settings/ci_cd_controller.rb index 21a8da551c0398965ae9378c369b73795bbeccfc..3157cfa89349bbb114f05da83ff49407212c9e0d 100644 --- a/app/controllers/projects/settings/ci_cd_controller.rb +++ b/app/controllers/projects/settings/ci_cd_controller.rb @@ -17,6 +17,8 @@ class CiCdController < Projects::ApplicationController push_frontend_feature_flag(:ci_variables_pages, current_user) push_frontend_feature_flag(:allow_push_repository_for_job_token, @project) push_frontend_feature_flag(:ci_hidden_variables, @project.root_ancestor) + + push_frontend_ability(ability: :admin_project, resource: @project, user: current_user) end helper_method :highlight_badge diff --git a/app/controllers/projects/settings/repository_controller.rb b/app/controllers/projects/settings/repository_controller.rb index a06858cf9c1d5da715545369fa0634395224d7a8..ac57e1e59fa86d79815428ac8edbeca479168e1e 100644 --- a/app/controllers/projects/settings/repository_controller.rb +++ b/app/controllers/projects/settings/repository_controller.rb @@ -9,6 +9,8 @@ class RepositoryController < Projects::ApplicationController before_action do push_frontend_feature_flag(:edit_branch_rules, @project) + push_frontend_ability(ability: :admin_project, resource: @project, user: current_user) + push_frontend_ability(ability: :admin_protected_branch, resource: @project, user: current_user) end feature_category :source_code_management, [:show, :cleanup, :update] diff --git a/app/policies/projects/branch_rule_policy.rb b/app/policies/projects/branch_rule_policy.rb index d1a6b66eebc1a6d9ce84f55ad1612520fd8c995c..0094da6699256e31f6fb9a7d015bbce91d157958 100644 --- a/app/policies/projects/branch_rule_policy.rb +++ b/app/policies/projects/branch_rule_policy.rb @@ -2,10 +2,12 @@ module Projects class BranchRulePolicy < ::ProtectedBranchPolicy - rule { can?(:read_protected_branch) }.enable :read_branch_rule - rule { can?(:create_protected_branch) }.enable :create_branch_rule - rule { can?(:update_protected_branch) }.enable :update_branch_rule - rule { can?(:destroy_protected_branch) }.enable :destroy_branch_rule + rule { can?(:admin_project) }.policy do + enable :read_branch_rule + enable :create_branch_rule + enable :update_branch_rule + enable :destroy_branch_rule + end end end diff --git a/app/validators/json_schemas/member_role_permissions.json b/app/validators/json_schemas/member_role_permissions.json index 86abb3ce9ceac83bf9b4c2cf33c829df0d2e6d74..2504dfc5df25995f6318050e1b6ef04d696b6ec1 100644 --- a/app/validators/json_schemas/member_role_permissions.json +++ b/app/validators/json_schemas/member_role_permissions.json @@ -19,6 +19,9 @@ "admin_merge_request": { "type": "boolean" }, + "admin_protected_branch": { + "type": "boolean" + }, "admin_push_rules": { "type": "boolean" }, diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 3195920dd075005a1c65b30de62a4f75553abaa9..30d86852ed87eae441a75c26eb1717d698c14370 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -36691,6 +36691,7 @@ Member role permission. | `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_PROTECTED_BRANCH` | Create, read, update, and delete protected branches for a project. | | `ADMIN_PUSH_RULES` | Configure push rules for repositories at the group or project level. | | `ADMIN_RUNNERS` | Create, view, edit, and delete group or project Runners. Includes configuring Runner settings. | | `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 21157a17d1ddc588b0adc2286530a58d3f1765c8..a9ffbd4252bc41d755619781ae7696756b58c2a8 100644 --- a/doc/user/custom_roles/abilities.md +++ b/doc/user/custom_roles/abilities.md @@ -86,6 +86,7 @@ These requirements are documented in the `Required permission` column in the fol | Name | Required permission | Description | Introduced in | Feature flag | Enabled in | |:-----|:------------|:------------------|:---------|:--------------|:---------| | [`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) | | | +| [`admin_protected_branch`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/162208) | | Create, read, update, and delete protected branches for a project. | GitLab [17.4](https://gitlab.com/gitlab-org/gitlab/-/issues/448823) | | | | [`admin_push_rules`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/147872) | | Configure push rules for repositories at the group or project level. | GitLab [16.11](https://gitlab.com/gitlab-org/gitlab/-/issues/421786) | `custom_ability_admin_push_rules` | | | [`read_code`](https://gitlab.com/gitlab-org/gitlab/-/issues/376180) | | Allows read-only access to the source code in the user interface. Does not allow users to edit or download repository archives, clone or pull repositories, view source code in an IDE, or view merge requests for private projects. You can download individual files because read-only access inherently grants the ability to make a local copy of the file. | 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) | diff --git a/ee/app/controllers/ee/projects/protected_branches_controller.rb b/ee/app/controllers/ee/projects/protected_branches_controller.rb index f1964210371557990855b35b35da268c291ed0ff..6cdcf49fcec86a32970d87a678fbb930d97ded9d 100644 --- a/ee/app/controllers/ee/projects/protected_branches_controller.rb +++ b/ee/app/controllers/ee/projects/protected_branches_controller.rb @@ -15,6 +15,10 @@ def protected_ref_params(*attrs) params_hash end + + def authorize_admin_protected_refs! + authorize_admin_protected_branch! + end end end end diff --git a/ee/app/controllers/ee/projects/settings/repository_controller.rb b/ee/app/controllers/ee/projects/settings/repository_controller.rb index 86a7fcefffa0c372685827d49a1b607b6ab199c7..06903678837160e8317073e5af6b5eb12fc3ad27 100644 --- a/ee/app/controllers/ee/projects/settings/repository_controller.rb +++ b/ee/app/controllers/ee/projects/settings/repository_controller.rb @@ -95,7 +95,8 @@ def allow_protected_branches_for_group?(group) def authorize_view_repository_settings! return if can?(current_user, :admin_push_rules, project) || - can?(current_user, :manage_deploy_tokens, project) + can?(current_user, :manage_deploy_tokens, project) || + can?(current_user, :admin_protected_branch, project) authorize_admin_project! end diff --git a/ee/app/policies/ee/project_policy.rb b/ee/app/policies/ee/project_policy.rb index a4de461735be259c0a35fb0a50a6260b86acced9..14c10fec5d932271fb9c1b6cba4d0698ac72d506 100644 --- a/ee/app/policies/ee/project_policy.rb +++ b/ee/app/policies/ee/project_policy.rb @@ -768,6 +768,14 @@ module ProjectPolicy enable :destroy_deploy_token end + rule { custom_role_enables_admin_protected_branch }.policy do + enable :read_protected_branch + enable :create_protected_branch + enable :update_protected_branch + enable :destroy_protected_branch + enable :admin_protected_branch + end + rule { can?(:create_issue) & okrs_enabled }.policy do enable :create_objective enable :create_key_result diff --git a/ee/config/custom_abilities/admin_protected_branch.yml b/ee/config/custom_abilities/admin_protected_branch.yml new file mode 100644 index 0000000000000000000000000000000000000000..0274ff48f684a25584a216433c42c5d864e7472c --- /dev/null +++ b/ee/config/custom_abilities/admin_protected_branch.yml @@ -0,0 +1,11 @@ +--- +name: admin_protected_branch +description: Create, read, update, and delete protected branches for a project. +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/448823 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/162208 +feature_category: source_code_management +milestone: '17.4' +group_ability: false +project_ability: true +requirements: [] +available_from_access_level: 40 diff --git a/ee/lib/ee/sidebars/projects/menus/settings_menu.rb b/ee/lib/ee/sidebars/projects/menus/settings_menu.rb index 02adfce3bbec13d564f9b0872bf799159b34e9f4..e5675363aa8b7af1e1c6e49e5f4ca418b1af0f46 100644 --- a/ee/lib/ee/sidebars/projects/menus/settings_menu.rb +++ b/ee/lib/ee/sidebars/projects/menus/settings_menu.rb @@ -16,7 +16,8 @@ module SettingsMenu ], repository_menu_item: [ :admin_push_rules, - :manage_deploy_tokens + :manage_deploy_tokens, + :admin_protected_branch ], merge_requests_menu_item: [ :manage_merge_request_settings diff --git a/ee/spec/controllers/ee/projects/protected_branches_controller_spec.rb b/ee/spec/controllers/ee/projects/protected_branches_controller_spec.rb index b8ab963723650c2143c8fd558593a4434688204e..6be04f91095ed1a4167536ec17985c143d6160eb 100644 --- a/ee/spec/controllers/ee/projects/protected_branches_controller_spec.rb +++ b/ee/spec/controllers/ee/projects/protected_branches_controller_spec.rb @@ -11,6 +11,62 @@ project.add_maintainer(user) end + context 'when using custom roles' do + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, :repository, group: group) } + let(:base_params) { project_params.merge(id: protected_branch.id) } + + let_it_be(:protected_branch) { create(:protected_branch, project: project) } + let_it_be(:another_user) { create(:user) } + + let(:maintainer_access_level) { [{ access_level: Gitlab::Access::MAINTAINER }] } + let(:access_level_params) do + { merge_access_levels_attributes: maintainer_access_level, + push_access_levels_attributes: maintainer_access_level } + end + + let(:create_params) do + attributes_for(:protected_branch).merge(access_level_params) + end + + let(:update_params) { { name: 'new_name' } } + + before do + sign_in(another_user) + end + + context 'when a user has custom roles with `admin_protected_branch` assigned' do + let_it_be(:role) { create(:member_role, :guest, namespace: group, admin_protected_branch: true) } + let_it_be(:membership) { create(:group_member, :guest, member_role: role, user: another_user, group: group) } + + context 'when custom_roles feature is available' do + before do + stub_licensed_features(custom_roles: true) + end + + describe "POST #create" do + subject(:create_request) { post(:create, params: project_params.merge(protected_branch: create_params)) } + + it 'creates a protected branch' do + expect { create_request }.to change { ProtectedBranch.count }.by(1) + + expect(response).to have_gitlab_http_status(:found) + end + end + + describe "PUT #update" do + subject(:update_request) { put(:update, params: base_params.merge(protected_branch: update_params)) } + + it 'creates a protected branch' do + expect { update_request }.to change { protected_branch.reload.name } + + expect(response).to have_gitlab_http_status(:ok) + end + end + end + end + end + describe "POST #create" do shared_examples "protected branch with code owner approvals feature" do |boolean| it "sets code owner approvals to #{boolean} when protecting the branch" do diff --git a/ee/spec/controllers/projects/settings/repository_controller_spec.rb b/ee/spec/controllers/projects/settings/repository_controller_spec.rb index 17fa491eba5693646ba5ad3539d24338ae28a380..358cf1bfedf51e94566b8390801295a771f42551 100644 --- a/ee/spec/controllers/projects/settings/repository_controller_spec.rb +++ b/ee/spec/controllers/projects/settings/repository_controller_spec.rb @@ -198,5 +198,39 @@ .to contain_exactly(protected_branch_from_deletion) end end + + context 'when accessing through custom ability' do + let_it_be(:another_user) { create(:user) } + let_it_be(:role) { create(:member_role, :guest, namespace: group, admin_protected_branch: true) } + let_it_be(:membership) { create(:group_member, :guest, member_role: role, user: another_user, group: group) } + + before do + sign_in(another_user) + end + + context 'with custom_roles feature enabled' do + before do + stub_licensed_features(custom_roles: true) + end + + it 'allows access' do + get :show, params: { namespace_id: group, project_id: project } + + expect(response).to have_gitlab_http_status(:ok) + end + end + + context 'with custom_roles feature disabled' do + before do + stub_licensed_features(custom_roles: false) + end + + it 'does not allow access' do + get :show, params: { namespace_id: group, project_id: project } + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end end end diff --git a/ee/spec/features/projects/protected_branches_spec.rb b/ee/spec/features/projects/protected_branches_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..997157c61ae7b97643e4ca3303affac50f2b670f --- /dev/null +++ b/ee/spec/features/projects/protected_branches_spec.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Protected Branches', :js, feature_category: :source_code_management do + include ProtectedBranchHelpers + + context 'when a guest has custom roles with `admin_protected_branch` assigned' do + let_it_be(:user) { create(:user) } + let_it_be(:admin) { create(:admin) } + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, :repository, group: group) } + let_it_be(:role) { create(:member_role, :guest, namespace: group, admin_protected_branch: true) } + let_it_be(:membership) { create(:group_member, :guest, member_role: role, user: user, group: group) } + + let(:success_message) { s_('ProtectedBranch|Protected branch was sucessfully created') } + + before do + stub_licensed_features(custom_roles: true) + + sign_in(user) + end + + it_behaves_like 'setting project protected branches' + end +end diff --git a/ee/spec/features/projects/settings/ee/protected_branches_spec.rb b/ee/spec/features/projects/settings/ee/protected_branches_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..5f365b6ae36db71dad0836b09e919b1c4bffefec --- /dev/null +++ b/ee/spec/features/projects/settings/ee/protected_branches_spec.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Projects > Settings > Repository settings using custom role', :js, feature_category: :source_code_management do + include ProtectedBranchHelpers + + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, :repository, group: group) } + + let_it_be(:current_user) { create(:user) } + let_it_be(:admin) { create(:admin) } + let_it_be(:role) { create(:member_role, :guest, namespace: group, admin_protected_branch: true) } + let_it_be(:membership) { create(:group_member, :guest, member_role: role, user: current_user, group: group) } + + let(:success_message) { s_('ProtectedBranch|Protected branch was sucessfully created') } + + context 'when user is a guest with custom roles that enables handling protected branches' do + before do + stub_licensed_features(custom_roles: true) + + sign_in(current_user) + end + + it_behaves_like 'setting project protected branches' + + it 'does not show sections not allowed by the custom role', :aggregate_failures do + expect(page).not_to have_content('Branch defaults') + expect(page).not_to have_content('Push rules') + expect(page).not_to have_content('Mirroring repositories') + expect(page).not_to have_content('Protected tags') + expect(page).not_to have_content('Deploy tokens') + expect(page).not_to have_content('Deploy keys') + expect(page).not_to have_content('Repository maintenance') + end + end +end diff --git a/ee/spec/frontend/protected_environments/add_approvers_spec.js b/ee/spec/frontend/protected_environments/add_approvers_spec.js index 2d3459d705c1df0a5830f26c4806593525c1515a..89fe5a36b3b5d31ee771ea7b1e5c39996a551f4a 100644 --- a/ee/spec/frontend/protected_environments/add_approvers_spec.js +++ b/ee/spec/frontend/protected_environments/add_approvers_spec.js @@ -74,6 +74,7 @@ describe('ee/protected_environments/add_approvers.vue', () => { deploy_access_levels: { roles: [], }, + abilities: { adminProject: true }, }; mockAxios = new MockAdapter(axios); }); diff --git a/ee/spec/frontend/protected_environments/create_protected_environment_spec.js b/ee/spec/frontend/protected_environments/create_protected_environment_spec.js index 5fb170ba1c10a2f07a961181fe20238484822491..dd43acdaeb3a9bb791896c5d6b04d59219785bf7 100644 --- a/ee/spec/frontend/protected_environments/create_protected_environment_spec.js +++ b/ee/spec/frontend/protected_environments/create_protected_environment_spec.js @@ -57,6 +57,7 @@ describe('ee/protected_environments/create_protected_environment.vue', () => { deploy_access_levels: { roles: [], }, + abilities: { adminProject: true }, }; mockAxios = new MockAdapter(axios); }); diff --git a/ee/spec/frontend/protected_environments/edit_protected_environments_list_spec.js b/ee/spec/frontend/protected_environments/edit_protected_environments_list_spec.js index 125204fce5de8db10ae268f9c36714b893c84166..54e722544376f8d064583ce1fbc859d68a963efe 100644 --- a/ee/spec/frontend/protected_environments/edit_protected_environments_list_spec.js +++ b/ee/spec/frontend/protected_environments/edit_protected_environments_list_spec.js @@ -135,7 +135,10 @@ describe('ee/protected_environments/edit_protected_environments_list.vue', () => describe('on the project level', () => { beforeEach(() => { mock = new MockAdapter(axios); - window.gon = { api_version: 'v4' }; + window.gon = { + api_version: 'v4', + abilities: { adminProject: true }, + }; mock .onGet('/api/v4/projects/8/protected_environments/') .reply(HTTP_STATUS_OK, DEFAULT_ENVIRONMENTS); diff --git a/ee/spec/frontend/protected_environments/protected_environment_edit_spec.js b/ee/spec/frontend/protected_environments/protected_environment_edit_spec.js index 95c56b9d1c87e943331cfed3f55b7ce48d0a4463..28a3a16991d0ad76d68c0115e3d322e7d90ee4c7 100644 --- a/ee/spec/frontend/protected_environments/protected_environment_edit_spec.js +++ b/ee/spec/frontend/protected_environments/protected_environment_edit_spec.js @@ -27,6 +27,7 @@ describe('Protected Environment Edit', () => { deploy_access_levels: { roles: [], }, + abilities: { adminProject: true }, }; mockAxios = new MockAdapter(axios); }); 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 06835533292d2c5009fad35f92990e01b024a9d6..1c10ec7d7c742ce1d83b345b7f3725615f7dec1b 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 @@ -114,6 +114,7 @@ where(:ability, :menu_item) do :admin_cicd_variables | 'CI/CD' :admin_push_rules | 'Repository' + :admin_protected_branch | 'Repository' :admin_runners | 'CI/CD' :manage_deploy_tokens | 'Repository' :manage_merge_request_settings | 'Merge requests' diff --git a/ee/spec/policies/project_policy_spec.rb b/ee/spec/policies/project_policy_spec.rb index e9a34196faad7d371b6bc8012e5d5cd4e2ee7a7c..15640f2dabb0d63ab4de7a368e70d1a8d862ec2d 100644 --- a/ee/spec/policies/project_policy_spec.rb +++ b/ee/spec/policies/project_policy_spec.rb @@ -3017,6 +3017,16 @@ def create_member_role(member, abilities = member_role_abilities) it_behaves_like 'custom roles abilities' end + + context 'for a member role with `admin_protected_branch` true' do + let(:member_role_abilities) { { admin_protected_branch: true } } + let(:allowed_abilities) do + [:admin_protected_branch, :read_protected_branch, :create_protected_branch, + :update_protected_branch, :destroy_protected_branch] + end + + 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_protected_branch/request_spec.rb b/ee/spec/requests/custom_roles/admin_protected_branch/request_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..f5a30ba7836d0bf0748855363a31dacf0072f152 --- /dev/null +++ b/ee/spec/requests/custom_roles/admin_protected_branch/request_spec.rb @@ -0,0 +1,112 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'User with admin_protected_branch custom role', feature_category: :source_code_management do + include ApiHelpers + + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, group: group) } + let_it_be(:protected_branch) { create(:protected_branch, project: project) } + + let_it_be(:current_user) { create(:user) } + let_it_be(:role) { create(:member_role, :guest, namespace: group, admin_protected_branch: true) } + let_it_be(:membership) { create(:group_member, :guest, member_role: role, user: current_user, group: group) } + + before do + stub_licensed_features(custom_roles: true) + + sign_in(current_user) + end + + describe Projects::ProtectedBranchesController do + describe 'POST #create' do + include_context 'with correct create params' + + it 'user can create protected branch via a custom role' do + post project_protected_branches_path(project, params: { protected_branch: create_params }) + + expect(response).to have_gitlab_http_status(:found) + end + end + + describe 'PUT #update' do + include_context 'with correct update params' + + it 'user can update the protected branch via a custom role' do + put project_protected_branch_path(project, id: protected_branch, params: { protected_branch: update_params }) + + expect(response).to have_gitlab_http_status(:ok) + end + end + + describe 'DELETE #destroy' do + it 'user can destroy the protected branch via a custom role' do + delete project_protected_branch_path(project, id: protected_branch) + + expect(response).to have_gitlab_http_status(:found) + end + end + end + + describe Projects::Settings::RepositoryController do + describe 'GET show' do + it 'user can see project repository settings page via a custom role' do + get namespace_project_settings_repository_path(namespace_id: group, project_id: project) + + expect(response).to have_gitlab_http_status(:ok) + end + end + end + + describe API::ProtectedBranches do + describe "GET /projects/:id/protected_branches" do + it 'user can see a protected branches list via a custom role' do + get api("/projects/#{project.id}/protected_branches", current_user) + + expect(response).to have_gitlab_http_status(:ok) + end + end + + describe "GET /projects/:id/protected_branches/:branch" do + it 'user can see a protected branch detail via a custom role' do + get api("/projects/#{project.id}/protected_branches/#{protected_branch.name}", current_user) + + expect(response).to have_gitlab_http_status(:ok) + end + end + + describe "PATCH /projects/:id/protected_branches/:branch" do + let(:params) do + { access_level_param: + [ + { + access_level: Gitlab::Access::MAINTAINER + } + ] } + end + + it 'user can update a protected branch via a custom role' do + patch api("/projects/#{project.id}/protected_branches/#{protected_branch.name}", current_user), params: params + + expect(response).to have_gitlab_http_status(:ok) + end + end + + describe 'POST /projects/:id/protected_branches' do + it 'user can create a protected branch via a custom role' do + post api("/projects/#{project.id}/protected_branches", current_user), params: { name: 'branch_name' } + + expect(response).to have_gitlab_http_status(:created) + end + end + + describe 'DELETE /projects/:id/protected_branches/:name' do + it 'user can create a protected branch via a custom role' do + delete api("/projects/#{project.id}/protected_branches/#{protected_branch.name}", current_user) + + expect(response).to have_gitlab_http_status(:no_content) + end + end + end +end diff --git a/ee/spec/requests/ee/projects/protected_branches_controller_spec.rb b/ee/spec/requests/ee/projects/protected_branches_controller_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..e231484b8d8541886b53a63a4ef5167b128db62c --- /dev/null +++ b/ee/spec/requests/ee/projects/protected_branches_controller_spec.rb @@ -0,0 +1,95 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Projects::ProtectedBranchesController, feature_category: :source_code_management do + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, :repository, group: group) } + let_it_be(:protected_branch) { create(:protected_branch, project: project) } + + let_it_be(:user) { create(:user) } + let_it_be(:role) { create(:member_role, :guest, namespace: group, admin_protected_branch: true) } + let_it_be(:membership) { create(:group_member, :guest, member_role: role, user: user, group: group) } + + let(:create_request) do + post project_protected_branches_path(project, params: { protected_branch: create_params }) + end + + let(:update_request) do + put project_protected_branch_path(project, id: protected_branch, params: { protected_branch: update_params }) + end + + subject(:delete_request) { delete project_protected_branch_path(project, id: protected_branch) } + + before do + sign_in(user) + end + + context 'with custom_roles feature enabled' do + before do + stub_licensed_features(custom_roles: true) + end + + describe "POST #create" do + include_context 'with correct create params' + + it 'creates a protected branch' do + expect { create_request }.to change { ProtectedBranch.count }.by(1) + + expect(response).to have_gitlab_http_status(:found) + end + end + + describe "PUT #update" do + include_context 'with correct update params' + + it 'updates the protected branch' do + expect { update_request }.to change { protected_branch.reload.name }.to('new name') + + expect(response).to have_gitlab_http_status(:ok) + end + end + + describe "DELETE #destroy" do + it 'destroys the protected branch' do + expect { delete_request }.to change { ProtectedBranch.count }.by(-1) + + expect(response).to have_gitlab_http_status(:found) + end + end + end + + context 'with custom_roles feature disabled' do + before do + stub_licensed_features(custom_roles: false) + end + + describe "POST #create" do + include_context 'with correct create params' + + it 'does not create a protected branch' do + expect { create_request }.not_to change { ProtectedBranch.count } + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + describe "PUT #update" do + include_context 'with correct update params' + + it 'does not update the protected branch' do + expect { update_request }.not_to change { protected_branch.reload.name } + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + describe "DELETE #destroy" do + it 'does not destroy the protected branch' do + expect { delete_request }.not_to change { ProtectedBranch.count } + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end +end diff --git a/ee/spec/support/shared_contexts/repository_settings_shared_contexts.rb b/ee/spec/support/shared_contexts/repository_settings_shared_contexts.rb new file mode 100644 index 0000000000000000000000000000000000000000..f43bab74705ff14f6d995bda9d2955a1441a0fc9 --- /dev/null +++ b/ee/spec/support/shared_contexts/repository_settings_shared_contexts.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +RSpec.shared_context 'with correct create params' do + let(:maintainer_access_level) { [{ access_level: Gitlab::Access::MAINTAINER }] } + let(:access_level_params) do + { + merge_access_levels_attributes: maintainer_access_level, + push_access_levels_attributes: maintainer_access_level + } + end + + let(:create_params) { attributes_for(:protected_branch).merge(access_level_params) } +end + +RSpec.shared_context 'with correct update params' do + let(:update_params) { { id: protected_branch, name: 'new name' } } +end diff --git a/lib/api/helpers/protected_branches_helpers.rb b/lib/api/helpers/protected_branches_helpers.rb index 4a968ad1d6064ac0bf5ee1128612ea2748e72ba5..c53c398a84c4bf00fa3a741cc7a098601e559c63 100644 --- a/lib/api/helpers/protected_branches_helpers.rb +++ b/lib/api/helpers/protected_branches_helpers.rb @@ -6,6 +6,18 @@ module ProtectedBranchesHelpers extend ActiveSupport::Concern extend Grape::API::Helpers + def authorize_create_protected_branch! + authorize!(:create_protected_branch, user_project) + end + + def authorize_update_protected_branch!(protected_branch) + authorize!(:update_protected_branch, protected_branch) + end + + def authorize_destroy_protected_branch!(protected_branch) + authorize!(:read_protected_branch, protected_branch) + end + params :optional_params_ee do end end diff --git a/lib/api/protected_branches.rb b/lib/api/protected_branches.rb index 60ce5ddc4f70f584b167cbdac8b2204d137c530e..bfe496ca1c4122d67cc07e90da0aea18f8f3eec2 100644 --- a/lib/api/protected_branches.rb +++ b/lib/api/protected_branches.rb @@ -88,7 +88,7 @@ class ProtectedBranches < ::API::Base end # rubocop: disable CodeReuse/ActiveRecord post ':id/protected_branches' do - authorize_admin_project + authorize_create_protected_branch! protected_branch = user_project.protected_branches.find_by(name: params[:name]) @@ -127,10 +127,10 @@ class ProtectedBranches < ::API::Base end # rubocop: disable CodeReuse/ActiveRecord patch ':id/protected_branches/:name', requirements: BRANCH_ENDPOINT_REQUIREMENTS do - authorize_admin_project - protected_branch = user_project.protected_branches.find_by!(name: params[:name]) + authorize_update_protected_branch!(protected_branch) + declared_params = declared_params(include_missing: false) api_service = ::ProtectedBranches::ApiService.new(user_project, current_user, declared_params) protected_branch = api_service.update(protected_branch) @@ -156,10 +156,10 @@ class ProtectedBranches < ::API::Base end # rubocop: disable CodeReuse/ActiveRecord delete ':id/protected_branches/:name', requirements: BRANCH_ENDPOINT_REQUIREMENTS, urgency: :low do - authorize_admin_project - protected_branch = user_project.protected_branches.find_by!(name: params[:name]) + authorize_destroy_protected_branch!(protected_branch) + destroy_conditionally!(protected_branch) do destroy_service = ::ProtectedBranches::DestroyService.new(user_project, current_user) destroy_service.execute(protected_branch) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 67e5a792ea26441b6eae6ab5ceaac75afb47949e..e32fb4fc701c501a74ad051332f29a144d2a6f2e 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -43647,6 +43647,9 @@ msgstr "" msgid "ProtectedBranch|Protect a branch" msgstr "" +msgid "ProtectedBranch|Protected branch was sucessfully created" +msgstr "" + msgid "ProtectedBranch|Protected branches" msgstr "" diff --git a/spec/features/protected_branches_spec.rb b/spec/features/protected_branches_spec.rb index 517bdada675f0e6a0d0c366c54dd6144a3447187..b90277d2e0cab331826bbf046828745d622b3d70 100644 --- a/spec/features/protected_branches_spec.rb +++ b/spec/features/protected_branches_spec.rb @@ -33,53 +33,14 @@ end context 'logged in as maintainer' do + let(:success_message) { s_('ProtectedBranch|View protected branches as branch rules') } + before do project.add_maintainer(user) sign_in(user) end - it 'allows to create a protected branch with name containing HTML tags' do - visit project_protected_branches_path(project) - - show_add_form - set_defaults - set_protected_branch_name('foobar<\b>') - click_on "Protect" - - within(".protected-branches-list") { expect(page).to have_content('foobar<\b>') } - expect(ProtectedBranch.count).to eq(1) - expect(ProtectedBranch.last.name).to eq('foobar<\b>') - end - - describe 'Delete protected branch' do - before do - create(:protected_branch, project: project, name: 'fix') - expect(ProtectedBranch.count).to eq(1) - end - - it 'removes branch after modal confirmation' do - visit project_branches_path(project) - - find('input[data-testid="branch-search"]').set('fix') - find('input[data-testid="branch-search"]').native.send_keys(:enter) - - expect(page).to have_content('fix') - expect(find('.all-branches')).to have_selector('li', count: 1) - - within_testid('branch-more-actions') do - find('button').click - end - - wait_for_requests - expect(page).to have_button('Delete protected branch', disabled: false) - - find_by_testid('delete-branch-button').click - fill_in 'delete_branch_input', with: 'fix' - click_button 'Yes, delete protected branch' - - expect(page).to have_content('No branches to show') - end - end + it_behaves_like 'setting project protected branches' end context 'logged in as admin' do @@ -88,114 +49,6 @@ enable_admin_mode!(admin) end - describe "explicit protected branches" do - it "allows creating explicit protected branches" do - visit project_protected_branches_path(project) - - show_add_form - set_defaults - set_protected_branch_name('some->branch') - click_on "Protect" - - within(".protected-branches-list") { expect(page).to have_content('some->branch') } - expect(ProtectedBranch.count).to eq(1) - expect(ProtectedBranch.last.name).to eq('some->branch') - end - - it "shows success alert once protected branch is created" do - visit project_protected_branches_path(project) - - show_add_form - set_defaults - set_protected_branch_name('some->branch') - click_on "Protect" - wait_for_requests - expect(page).to have_content(s_('ProtectedBranch|View protected branches as branch rules')) - end - - it "displays the last commit on the matching branch if it exists" do - commit = create(:commit, project: project) - project.repository.add_branch(admin, 'some-branch', commit.id) - - visit project_protected_branches_path(project) - - show_add_form - set_defaults - set_protected_branch_name('some-branch') - click_on "Protect" - - within(".protected-branches-list") do - expect(page).not_to have_content("matching") - expect(page).not_to have_content("was deleted") - end - end - - it "displays an error message if the named branch does not exist" do - visit project_protected_branches_path(project) - - show_add_form - set_defaults - set_protected_branch_name('some-branch') - click_on "Protect" - - within(".protected-branches-list") { expect(page).to have_content('Branch does not exist') } - end - end - - describe "wildcard protected branches" do - it "allows creating protected branches with a wildcard" do - visit project_protected_branches_path(project) - - show_add_form - set_defaults - set_protected_branch_name('*-stable') - click_on "Protect" - - within(".protected-branches-list") { expect(page).to have_content('*-stable') } - expect(ProtectedBranch.count).to eq(1) - expect(ProtectedBranch.last.name).to eq('*-stable') - end - - it "displays the number of matching branches", - quarantine: 'https://gitlab.com/gitlab-org/quality/engineering-productivity/flaky-tests/-/issues/3459' do - project.repository.add_branch(admin, 'production-stable', 'master') - project.repository.add_branch(admin, 'staging-stable', 'master') - - visit project_protected_branches_path(project) - - show_add_form - set_defaults - set_protected_branch_name('*-stable') - click_on "Protect" - - within(".protected-branches-list") do - expect(page).to have_content("2 matching branches") - end - end - - it "displays all the branches matching the wildcard" do - project.repository.add_branch(admin, 'production-stable', 'master') - project.repository.add_branch(admin, 'staging-stable', 'master') - project.repository.add_branch(admin, 'development', 'master') - - visit project_protected_branches_path(project) - - show_add_form - set_protected_branch_name('*-stable') - set_defaults - click_on "Protect" - - visit project_protected_branches_path(project) - click_on "2 matching branches" - - within(".protected-branches-list") do - expect(page).to have_content("production-stable") - expect(page).to have_content("staging-stable") - expect(page).not_to have_content("development") - end - end - end - describe "access control" do before do stub_licensed_features(protected_refs_for_users: false) diff --git a/spec/frontend/projects/settings/components/access_dropdown_spec.js b/spec/frontend/projects/settings/components/access_dropdown_spec.js index bbc87462777be82252ddffe8a5a11ca402d9452c..266593907e985000ed4e454ed38e1edf715943d9 100644 --- a/spec/frontend/projects/settings/components/access_dropdown_spec.js +++ b/spec/frontend/projects/settings/components/access_dropdown_spec.js @@ -50,6 +50,10 @@ jest.mock('~/projects/settings/api/access_dropdown_api', () => ({ })); describe('Access Level Dropdown', () => { + beforeEach(() => { + window.gon = { abilities: { adminProject: true } }; + }); + let wrapper; const defaultToggleClass = '!gl-text-gray-500'; const mockAccessLevelsData = [ @@ -71,9 +75,14 @@ describe('Access Level Dropdown', () => { }, ]; + const abilities = { + adminProject: true, + adminProtectedBranch: false, + }; const createComponent = ({ accessLevelsData = mockAccessLevelsData, accessLevel = ACCESS_LEVELS.PUSH, + glAbilities = abilities, stubs = {}, ...optionalProps } = {}) => { @@ -83,6 +92,9 @@ describe('Access Level Dropdown', () => { accessLevel, ...optionalProps, }, + provide: { + glAbilities, + }, stubs: { GlSprintf, GlDropdown, @@ -121,6 +133,18 @@ describe('Access Level Dropdown', () => { }); }); + describe('withProtectedBranchesAccess', () => { + it('should make an api call for users && groups when user has a license', () => { + createComponent({ + groupsWithProjectAccess: true, + glAbilities: { adminProject: false, adminProtectedBranch: true }, + }); + expect(getUsers).toHaveBeenCalled(); + expect(getGroups).toHaveBeenCalledWith({ withProjectAccess: true }); + expect(getDeployKeys).not.toHaveBeenCalled(); + }); + }); + it('should make an api call for deployKeys but not for users or groups when user does not have a license', () => { createComponent({ hasLicense: false }); expect(getUsers).not.toHaveBeenCalled(); diff --git a/spec/frontend/protected_branches/protected_branch_create_spec.js b/spec/frontend/protected_branches/protected_branch_create_spec.js index e2a0f02e0cf775305bcc6d35f21ef882981c408f..a7e0dcc6597f02b4d828c2b86a1093f515564bdb 100644 --- a/spec/frontend/protected_branches/protected_branch_create_spec.js +++ b/spec/frontend/protected_branches/protected_branch_create_spec.js @@ -17,6 +17,7 @@ describe('ProtectedBranchCreate', () => { window.gon = { merge_access_levels: { roles: [] }, push_access_levels: { roles: [] }, + abilities: { adminProject: true }, }; }); diff --git a/spec/frontend/protected_branches/protected_branch_edit_spec.js b/spec/frontend/protected_branches/protected_branch_edit_spec.js index 463354ab7e1eb3e898bfe9728378ff6900758ef6..edcc91e07f2bc18bfab6dff5337e1054f284ec33 100644 --- a/spec/frontend/protected_branches/protected_branch_edit_spec.js +++ b/spec/frontend/protected_branches/protected_branch_edit_spec.js @@ -143,6 +143,7 @@ describe('ProtectedBranchEdit', () => { current_project_id: 1, merge_access_levels: { roles: accessLevels }, push_access_levels: { roles: accessLevels }, + abilities: { adminProject: true }, }; jest.spyOn(ProtectedBranchEdit.prototype, 'initToggles').mockImplementation(); diff --git a/spec/support/shared_examples/projects/protected_branches_shared_examples.rb b/spec/support/shared_examples/projects/protected_branches_shared_examples.rb new file mode 100644 index 0000000000000000000000000000000000000000..e6ed34026d2721d56c99a72ec7c9be8eeb598886 --- /dev/null +++ b/spec/support/shared_examples/projects/protected_branches_shared_examples.rb @@ -0,0 +1,111 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'setting project protected branches' do + describe "explicit protected branches" do + it "allows creating explicit protected branches" do + visit project_protected_branches_path(project) + + show_add_form + set_defaults + set_protected_branch_name('some->branch') + click_on "Protect" + + within(".protected-branches-list") { expect(page).to have_content('some->branch') } + expect(ProtectedBranch.count).to eq(1) + expect(ProtectedBranch.last.name).to eq('some->branch') + end + + it "shows success alert once protected branch is created" do + visit project_protected_branches_path(project) + + show_add_form + set_defaults + set_protected_branch_name('some->branch') + click_on "Protect" + wait_for_requests + expect(page).to have_content(success_message) + end + + it "displays the last commit on the matching branch if it exists" do + commit = create(:commit, project: project) + project.repository.add_branch(admin, 'some-branch', commit.id) + + visit project_protected_branches_path(project) + + show_add_form + set_defaults + set_protected_branch_name('some-branch') + click_on "Protect" + + within(".protected-branches-list") do + expect(page).not_to have_content("matching") + expect(page).not_to have_content("was deleted") + end + end + + it "displays an error message if the named branch does not exist" do + visit project_protected_branches_path(project) + + show_add_form + set_defaults + set_protected_branch_name('some-unexisting-branch') + click_on "Protect" + + within(".protected-branches-list") { expect(page).to have_content('Branch does not exist') } + end + end + + describe "wildcard protected branches" do + it "allows creating protected branches with a wildcard" do + visit project_protected_branches_path(project) + + show_add_form + set_defaults + set_protected_branch_name('*-stable') + click_on "Protect" + + within(".protected-branches-list") { expect(page).to have_content('*-stable') } + expect(ProtectedBranch.count).to eq(1) + expect(ProtectedBranch.last.name).to eq('*-stable') + end + + it "displays the number of matching branches", + quarantine: 'https://gitlab.com/gitlab-org/quality/engineering-productivity/flaky-tests/-/issues/3459' do + project.repository.add_branch(admin, 'production-stable', 'master') + project.repository.add_branch(admin, 'staging-stable', 'master') + + visit project_protected_branches_path(project) + + show_add_form + set_defaults + set_protected_branch_name('*-stable') + click_on "Protect" + + within(".protected-branches-list") do + expect(page).to have_content("2 matching branches") + end + end + + it "displays all the branches matching the wildcard" do + project.repository.add_branch(admin, 'production-stable', 'master') + project.repository.add_branch(admin, 'staging-stable', 'master') + project.repository.add_branch(admin, 'development', 'master') + + visit project_protected_branches_path(project) + + show_add_form + set_protected_branch_name('*-stable') + set_defaults + click_on "Protect" + + visit project_protected_branches_path(project) + click_on "2 matching branches" + + within(".protected-branches-list") do + expect(page).to have_content("production-stable") + expect(page).to have_content("staging-stable") + expect(page).not_to have_content("development") + end + end + end +end