Only allow admins to rename default/protected branches

[kemzeb]

Currently, anyone with write permissions to a repo are able to rename default or protected branches.

This change follows GitHub's design by only allowing repo/site admins to change these branches. However, it also follows are current design for protected branches and only allows admins to modify branch names == branch protection rule names. Glob-based rules cannot be renamed by anyone (as was already the case, but we now catch ErrBranchIsProtected which we previously did not catch, throwing a 500).

[kemzeb]

I'm iffy about using ErrUserDoesNotHaveAccessToRepo as an error here. Technically the user does have access to the repo, just not the access required to rename default/protected branches. Open to ideas

[kemzeb]

An example of changing default/protected branches in the web UI:

[wxiaoguang]

FYI: Make admins adhere to branch protection rules #32248 : there is a feature to block admins from bypassing the branch protection, maybe it should also be considered (or has it been considered already) ?

[kemzeb]

The current change doesn't consider this. I found a GitHub changelog that seem to match my current implementation. I not familiar with setting up branch protection rules, but from messing around with GitHub's rulesets I don't see a way to disable the ability to rename protected branches

[kemzeb]

Then again I can see how it could be potentially dangerous. An example:
(I have a ruleset of protected/**)

I now renamed the protected branch. GitHub now no longer sees this as a protected branch:

[kemzeb]

Hmm I think we already (in a buggy way) handle this??

Using my changes, when I define a rule without special characters (i.e. protected) I am able to rename it:

But if I introduce wildcard characters (i.e. protected/**) it will attempt and fail to rename because it throws a ErrBranchIsProtected error that we currently don't catch:

Not sure why we don't throw this error for the previous case? In the code we seem to honor the update and reflect it in the protected_branch table, but when I gave wildcard characters to the rule it would be considered a ErrBranchIsProtected error.

[kemzeb]

I think I might understand this design. It looks like if the new branch name == rule name, we treat it as "oh since the user wants to rename the branch I assume they also want to rename the rule". But if is a glob-based rule we don't make this assumption and instead throw this error.

I'll make the necessary changes to follow this design closely.