`Gitlab::Checks::ChangeAccess` looks for user permissions that don't exist
Gitlab::Checks::ChangeAccess, we check for the
:remove_protected_branchespermissions. These symbols are not defined anywhere else in the codebase, except in the spec for this module.
if forced_push? && user_access.cannot_do_action?(:force_push_code_to_protected_branches) return "You are not allowed to force push code to a protected branch on this project." elsif Gitlab::Git.blank_ref?(@newrev) && user_access.cannot_do_action?(:remove_protected_branches) return "You are not allowed to delete protected branches from this project." end
After investigating, it looks like the
:remove_protected_branchespermissions are always disallowed, regardless of the user's level of access. Because of this, we don't need to define them in the ability/policy files, which act as a whitelist, and only require a list of permissions that should be allowed.
I created this issue to make sure everyone is on the same page here, and to decide if the status quo is alright, or if we need to change things up.
user.can?for permissions that don't exist in the policy file. This would make things less confusing, but would also make it harder to change the behavior of these always-disallowed permissions. For example, right now, if we need to allow admin to delete protected branches (for example), we just need to add
:remove_protected_branchesto the relevant ability/policy file, with no other changes required.
List these permissions in the policy/ability files using the
cannot!declaration. After taking a quick look at the code, it seems like this is used to blacklist permissions that were previously whitelisted, but I don't see any reason why we can't use it for our purposes here.