[go: up one dir, main page]

Custom role with admin_protected_branch doesn't provide protected branch deletion option

Summary

The custom role option admin_protected_branch, introduced in 17.4, was as described in the original issue, designed to provide a way for Developers or other base roles to manage protected branches.

The issue specifically mentions:

The permission actions for admin_protected_branches includes creating, reading, updating, and deleting protected branches along with properties associated

In the current implementation, a Developer with admin_protected_branches has the ability to see some things within the Settings → Repository → Protected Branches page, and can "un-protect" a branch. They can not delete a protected branch via the UI while it is still protected, as the option is not presented in Repository → Branches. See below for an example.

Steps to reproduce

  1. Create a custom role with admin_protected_branch either at the instance or group level.
  2. Assign a Guest/Reporter/Developer user to a project, then assign them the custom role you created.
  3. Create a protected branch in the project
  4. As the logged-in user with the custom role:
    1. Navigate to Code → Branches
      • Look for the delete option in the branch actions menu. This is not shown as a policy permission is missing for the user account.

What is the current bug behavior?

Some components of the application still rely on a different policy permission. From a review of the code from frontend → backend:

app/views/projects/branches/_branch.html.haml defines can_delete_branch when calling the Vue object branchmoreActions. This is defined by the call:

user_access(@project).can_delete_branch?(branch.name).to_s`

The origin of this is in lib/gitlab/user_access.rb with:

   user.can?(:push_to_delete_protected_branch, container)

When checking against the project policy in app/policies/project_policy.rb, this is only provided for Maintainer/Owner access:

rule { can?(:maintainer_access) }.policy do
    ...
    enable :push_to_delete_protected_branch

If we perform a manual check against a user with the custom role, given the admin_protected_branch permissions, we'll see that they don't get this permission from the policy:

user = User.find_by(username: "reported_user_jesus_aufderhar")
project = Project.find_by_full_path('flightjs/flight')
protected_branch = project.protected_branches.find_by(name: 'protected-branch')
...

Ability.allowed?(user, :push_to_delete_protected_branch, protected_branch)
...
=> false

What is the expected correct behavior?

As defined in the original issue, a user given this custom role should have full CRUD permissions on protected branches, including being able to delete the protected branch via the UI.

Relevant logs and/or screenshots

Developer with current custom role Project owner
image image

Output of checks

This bug happens on GitLab.com

reproduced on GitLab.com

Possible fixes

We can expand custom_role_enables_admin_protected_branch with:

      rule { custom_role_enables_admin_protected_branch }.policy do
        # Existing actions
        enable :read_protected_branch
        enable :create_protected_branch
        enable :update_protected_branch
        enable :destroy_protected_branch
        enable :admin_protected_branch
        # New action
        enable :push_to_delete_protected_branch
      end

For example, with this change implemented:

Developer with custom role:

image

Screenshot shows the UI option is now available:

image

Edited by Ben King