From 03b0e5892bfc8d05dfdf3d650f075c9db51c66ff Mon Sep 17 00:00:00 2001 From: Ben King Date: Mon, 20 Oct 2025 00:40:47 +0000 Subject: [PATCH 1/6] Add initial policy change --- ee/app/policies/ee/project_policy.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/ee/app/policies/ee/project_policy.rb b/ee/app/policies/ee/project_policy.rb index 009fa9d76361a5..4791eb38228969 100644 --- a/ee/app/policies/ee/project_policy.rb +++ b/ee/app/policies/ee/project_policy.rb @@ -1113,6 +1113,7 @@ module ProjectPolicy enable :update_protected_branch enable :destroy_protected_branch enable :admin_protected_branch + enable :push_to_delete_protected_branch end rule { can?(:create_issue) & okrs_enabled }.policy do -- GitLab From dd3ed2809f1ece4372297e6aa9ace44abefd097d Mon Sep 17 00:00:00 2001 From: Ben King Date: Mon, 20 Oct 2025 21:33:18 +0000 Subject: [PATCH 2/6] Update existing spec test --- .../projects/protected_branches_spec.rb | 39 +++++++++++++++---- 1 file changed, 32 insertions(+), 7 deletions(-) diff --git a/ee/spec/features/projects/protected_branches_spec.rb b/ee/spec/features/projects/protected_branches_spec.rb index da5e010651fbe2..27f2a4afb08d34 100644 --- a/ee/spec/features/projects/protected_branches_spec.rb +++ b/ee/spec/features/projects/protected_branches_spec.rb @@ -5,21 +5,46 @@ RSpec.describe 'Protected Branches', :js, feature_category: :source_code_management do include ProtectedBranchHelpers + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, :repository, group: group) } + + before do + stub_licensed_features(custom_roles: true) + sign_in(user) + end + 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, :admin_protected_branch, namespace: group) } let_it_be(:membership) { create(:group_member, :guest, member_role: role, user: user, group: group) } let(:success_message) { s_('ProtectedBranch|Protected branch was successfully created') } - before do - stub_licensed_features(custom_roles: true) - sign_in(user) - end - it_behaves_like 'setting project protected branches' end + + context 'when a developer has custom roles with `admin_protected_branch` assigned' do + let_it_be(:user) { create(:user) } + let_it_be(:role) { create(:member_role, :developer, :admin_protected_branch, namespace: group) } + let_it_be(:membership) { create(:group_member, :developer, member_role: role, user: user, group: group) } + + it 'shows delete button for protected branches' do + project.repository.add_branch(project.creator, 'protected', 'HEAD') + create(:protected_branch, project: project, name: 'protected') + + visit project_branches_path(project) + + find('input[data-testid="branch-search"]').set('protected') + find('input[data-testid="branch-search"]').native.send_keys(:enter) + + within('[data-name="protected"]') do + within_testid('branch-more-actions') do + find('.gl-new-dropdown-toggle').click + end + end + + expect(page).to have_button('Delete protected branch') + end + end end -- GitLab From 0e668c4a64f8b65459a232b71964ee3fdfe9ef76 Mon Sep 17 00:00:00 2001 From: Ben King Date: Tue, 21 Oct 2025 01:05:18 +0000 Subject: [PATCH 3/6] Update custom role docs --- doc/user/custom_roles/abilities.md | 1 + tooling/custom_roles/docs/templates/custom_abilities.md.erb | 1 + 2 files changed, 2 insertions(+) diff --git a/doc/user/custom_roles/abilities.md b/doc/user/custom_roles/abilities.md index 5c33d9ec1df2de..6f47b53e2c8878 100644 --- a/doc/user/custom_roles/abilities.md +++ b/doc/user/custom_roles/abilities.md @@ -20,6 +20,7 @@ description: Configure granular permissions with specific abilities for fine-gra - Custom admin roles [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/181346) in GitLab 17.9 [with a flag](../../administration/feature_flags/_index.md) named `custom_admin_roles`. Disabled by default. - Custom admin roles [generally available](https://gitlab.com/groups/gitlab-org/-/epics/15957) in GitLab 18.3. Feature flag `custom_admin_roles` enabled by default. +- `admin_protected_branch` [updated](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/209412) in GitLab 18.6 to enable protected branch deletion in the UI. {{< /history >}} diff --git a/tooling/custom_roles/docs/templates/custom_abilities.md.erb b/tooling/custom_roles/docs/templates/custom_abilities.md.erb index a27b5fcc57f954..255ac7c16d89fd 100644 --- a/tooling/custom_roles/docs/templates/custom_abilities.md.erb +++ b/tooling/custom_roles/docs/templates/custom_abilities.md.erb @@ -48,6 +48,7 @@ description: Configure granular permissions with specific abilities for fine-gra - Custom admin roles [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/181346) in GitLab 17.9 [with a flag](../../administration/feature_flags/_index.md) named `custom_admin_roles`. Disabled by default. - Custom admin roles [generally available](https://gitlab.com/groups/gitlab-org/-/epics/15957) in GitLab 18.3. Feature flag `custom_admin_roles` enabled by default. +- `admin_protected_branch` [updated](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/209412) in GitLab 18.6 to enable protected branch deletion in the UI. {{< /history >}} -- GitLab From 9354228ef88b0dfe7583cfdea803fcdcc52b1031 Mon Sep 17 00:00:00 2001 From: Ben King Date: Mon, 27 Oct 2025 04:50:07 +0000 Subject: [PATCH 4/6] Apply suggested changes --- doc/user/custom_roles/abilities.md | 1 - ee/app/policies/ee/project_policy.rb | 6 ++++-- lib/gitlab/user_access.rb | 2 +- tooling/custom_roles/docs/templates/custom_abilities.md.erb | 1 - 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/doc/user/custom_roles/abilities.md b/doc/user/custom_roles/abilities.md index 6f47b53e2c8878..5c33d9ec1df2de 100644 --- a/doc/user/custom_roles/abilities.md +++ b/doc/user/custom_roles/abilities.md @@ -20,7 +20,6 @@ description: Configure granular permissions with specific abilities for fine-gra - Custom admin roles [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/181346) in GitLab 17.9 [with a flag](../../administration/feature_flags/_index.md) named `custom_admin_roles`. Disabled by default. - Custom admin roles [generally available](https://gitlab.com/groups/gitlab-org/-/epics/15957) in GitLab 18.3. Feature flag `custom_admin_roles` enabled by default. -- `admin_protected_branch` [updated](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/209412) in GitLab 18.6 to enable protected branch deletion in the UI. {{< /history >}} diff --git a/ee/app/policies/ee/project_policy.rb b/ee/app/policies/ee/project_policy.rb index 4791eb38228969..168f9580af54aa 100644 --- a/ee/app/policies/ee/project_policy.rb +++ b/ee/app/policies/ee/project_policy.rb @@ -1108,12 +1108,14 @@ module ProjectPolicy end rule { custom_role_enables_admin_protected_branch }.policy do + enable :admin_protected_branch + end + + rule { can?(: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 - enable :push_to_delete_protected_branch end rule { can?(:create_issue) & okrs_enabled }.policy do diff --git a/lib/gitlab/user_access.rb b/lib/gitlab/user_access.rb index a777c727a4aa27..2bde78db022c66 100644 --- a/lib/gitlab/user_access.rb +++ b/lib/gitlab/user_access.rb @@ -55,7 +55,7 @@ def allowed? return false unless can_access_git? if protected?(ProtectedBranch, ref) - user.can?(:push_to_delete_protected_branch, container) + user.can?(:destroy_protected_branch, container) else can_push? end diff --git a/tooling/custom_roles/docs/templates/custom_abilities.md.erb b/tooling/custom_roles/docs/templates/custom_abilities.md.erb index 255ac7c16d89fd..a27b5fcc57f954 100644 --- a/tooling/custom_roles/docs/templates/custom_abilities.md.erb +++ b/tooling/custom_roles/docs/templates/custom_abilities.md.erb @@ -48,7 +48,6 @@ description: Configure granular permissions with specific abilities for fine-gra - Custom admin roles [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/181346) in GitLab 17.9 [with a flag](../../administration/feature_flags/_index.md) named `custom_admin_roles`. Disabled by default. - Custom admin roles [generally available](https://gitlab.com/groups/gitlab-org/-/epics/15957) in GitLab 18.3. Feature flag `custom_admin_roles` enabled by default. -- `admin_protected_branch` [updated](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/209412) in GitLab 18.6 to enable protected branch deletion in the UI. {{< /history >}} -- GitLab From 3c88fbabaebab7746ca558dc4fb56fa016f4a520 Mon Sep 17 00:00:00 2001 From: Ben King Date: Thu, 6 Nov 2025 02:22:05 +0000 Subject: [PATCH 5/6] Add unit test --- ee/spec/lib/gitlab/user_access_spec.rb | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/ee/spec/lib/gitlab/user_access_spec.rb b/ee/spec/lib/gitlab/user_access_spec.rb index a9869d7f5551cb..945bc5be884b00 100644 --- a/ee/spec/lib/gitlab/user_access_spec.rb +++ b/ee/spec/lib/gitlab/user_access_spec.rb @@ -20,4 +20,23 @@ end end end + + describe '#can_delete_branch?' do + context 'when user has custom role with admin_protected_branch' do + let(:group) { create(:group) } + let(:project) { create(:project, :repository, group: group) } + let(:protected_branch) { create(:protected_branch, project: project, name: 'protected') } + let(:role) { create(:member_role, :developer, :admin_protected_branch, namespace: group) } + let(:user) { create(:user) } + + before do + stub_licensed_features(custom_roles: true) + create(:group_member, :developer, member_role: role, user: user, group: group) + end + + it 'returns true for protected branches' do + expect(access.can_delete_branch?(protected_branch.name)).to be true + end + end + end end -- GitLab From a3dce774a1803d6c236e15f73c79efb153fb11b0 Mon Sep 17 00:00:00 2001 From: Ben King Date: Wed, 17 Dec 2025 04:03:30 +0000 Subject: [PATCH 6/6] Apply suggested spec fixes --- .../projects/protected_branches_spec.rb | 37 ++++++++------ ee/spec/lib/gitlab/user_access_spec.rb | 49 +++++++++++++------ 2 files changed, 55 insertions(+), 31 deletions(-) diff --git a/ee/spec/features/projects/protected_branches_spec.rb b/ee/spec/features/projects/protected_branches_spec.rb index 27f2a4afb08d34..f7c98e41102316 100644 --- a/ee/spec/features/projects/protected_branches_spec.rb +++ b/ee/spec/features/projects/protected_branches_spec.rb @@ -5,34 +5,41 @@ RSpec.describe 'Protected Branches', :js, feature_category: :source_code_management do include ProtectedBranchHelpers + let_it_be(:admin) { create(:admin) } let_it_be(:group) { create(:group) } - let_it_be(:project) { create(:project, :repository, group: group) } + let_it_be(:project) { create(:project, :repository, group: group, create_branch: 'protected') } - before do - stub_licensed_features(custom_roles: true) - sign_in(user) - end + context 'when a guest has custom role with `admin_protected_branch` assigned' do + let_it_be(:guest) { create(:user) } - 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(:role) { create(:member_role, :guest, :admin_protected_branch, namespace: group) } - let_it_be(:membership) { create(:group_member, :guest, member_role: role, user: user, group: group) } + let_it_be(:membership) { create(:group_member, :guest, member_role: role, user: guest, group: group) } let(:success_message) { s_('ProtectedBranch|Protected branch was successfully created') } + before do + stub_licensed_features(custom_roles: true) + sign_in(guest) + end + it_behaves_like 'setting project protected branches' end - context 'when a developer has custom roles with `admin_protected_branch` assigned' do - let_it_be(:user) { create(:user) } + context 'when a developer has custom role with `admin_protected_branch` assigned' do + # Only Developer+ roles can access the project branches page + let_it_be(:developer) { create(:user) } + let_it_be(:role) { create(:member_role, :developer, :admin_protected_branch, namespace: group) } - let_it_be(:membership) { create(:group_member, :developer, member_role: role, user: user, group: group) } + let_it_be(:membership) { create(:group_member, :developer, member_role: role, user: developer, group: group) } + + let_it_be(:branch) { create(:protected_branch, project: project, name: 'protected') } - it 'shows delete button for protected branches' do - project.repository.add_branch(project.creator, 'protected', 'HEAD') - create(:protected_branch, project: project, name: 'protected') + before do + stub_licensed_features(custom_roles: true) + sign_in(developer) + end + it 'allows developer to remove protected branch' do visit project_branches_path(project) find('input[data-testid="branch-search"]').set('protected') diff --git a/ee/spec/lib/gitlab/user_access_spec.rb b/ee/spec/lib/gitlab/user_access_spec.rb index 945bc5be884b00..3f8d38160cb619 100644 --- a/ee/spec/lib/gitlab/user_access_spec.rb +++ b/ee/spec/lib/gitlab/user_access_spec.rb @@ -2,15 +2,16 @@ require 'spec_helper' -RSpec.describe Gitlab::UserAccess do +RSpec.describe Gitlab::UserAccess, feature_category: :permissions do include ExternalAuthorizationServiceHelpers - let(:user) { create(:user) } - let(:access) { described_class.new(user, container: project) } + let_it_be_with_reload(:user) { create(:user) } + + subject(:access) { described_class.new(user, container: project) } describe '#can_push_to_branch?' do describe 'push to empty project' do - let(:project) { create(:project_empty_repo) } + let_it_be(:project) { create(:project_empty_repo) } it 'returns false when the external service denies access' do project.add_maintainer(user) @@ -22,20 +23,36 @@ end describe '#can_delete_branch?' do - context 'when user has custom role with admin_protected_branch' do - let(:group) { create(:group) } - let(:project) { create(:project, :repository, group: group) } - let(:protected_branch) { create(:protected_branch, project: project, name: 'protected') } - let(:role) { create(:member_role, :developer, :admin_protected_branch, namespace: group) } - let(:user) { create(:user) } - - before do - stub_licensed_features(custom_roles: true) - create(:group_member, :developer, member_role: role, user: user, group: group) + context 'when a user has custom roles with `admin_protected_branch` assigned' do + let_it_be(:project) { create(:project, :repository, :in_group) } + + let_it_be(:role) { create(:member_role, :developer, :admin_protected_branch, namespace: project.group) } + let_it_be(:project_member) do + create(:project_member, :developer, member_role: role, user: user, project: project) end - it 'returns true for protected branches' do - expect(access.can_delete_branch?(protected_branch.name)).to be true + describe 'delete protected branch' do + let_it_be(:branch) { create(:protected_branch, project: project, name: "test") } + + context 'when custom roles is enabled' do + before do + stub_licensed_features(custom_roles: true) + end + + it 'returns true' do + expect(access.can_delete_branch?(branch.name)).to be(true) + end + end + + context 'when custom roles is disabled' do + before do + stub_licensed_features(custom_roles: false) + end + + it 'returns false' do + expect(access.can_delete_branch?(branch.name)).to be(false) + end + end end end end -- GitLab