Skip to content

Commit 22e1ee5

Browse files
committed
Account for protected branches design
1 parent 5064d83 commit 22e1ee5

File tree

5 files changed

+34
-8
lines changed

5 files changed

+34
-8
lines changed

options/locale/locale_en-US.ini

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2714,6 +2714,7 @@ branch.new_branch = Create new branch
27142714
branch.new_branch_from = Create new branch from "%s"
27152715
branch.renamed = Branch %s was renamed to %s.
27162716
branch.rename_default_or_protected_branch_error = Only admins can rename default or protected branches.
2717+
branch.rename_protected_branch_failed = This branch is protected by glob-based protection rules.
27172718
27182719
tag.create_tag = Create tag %s
27192720
tag.create_tag_operation = Create tag

routers/api/v1/repo/branch.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -447,6 +447,8 @@ func UpdateBranch(ctx *context.APIContext) {
447447
switch {
448448
case repo_model.IsErrUserDoesNotHaveAccessToRepo(err):
449449
ctx.Error(http.StatusForbidden, "", "User must be a repo or site admin to rename default or protected branches.")
450+
case errors.Is(err, git_model.ErrBranchIsProtected):
451+
ctx.Error(http.StatusForbidden, "", "Branch is protected by glob-based protection rules.")
450452
default:
451453
ctx.Error(http.StatusInternalServerError, "RenameBranch", err)
452454
}

routers/web/repo/setting/protected_branch.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
package setting
55

66
import (
7+
"errors"
78
"fmt"
89
"net/http"
910
"net/url"
@@ -358,6 +359,9 @@ func RenameBranchPost(ctx *context.Context) {
358359
case git_model.IsErrBranchAlreadyExists(err):
359360
ctx.Flash.Error(ctx.Tr("repo.branch.branch_already_exists", form.To))
360361
ctx.Redirect(fmt.Sprintf("%s/branches", ctx.Repo.RepoLink))
362+
case errors.Is(err, git_model.ErrBranchIsProtected):
363+
ctx.Flash.Error(ctx.Tr("repo.branch.rename_protected_branch_failed"))
364+
ctx.Redirect(fmt.Sprintf("%s/branches", ctx.Repo.RepoLink))
361365
default:
362366
ctx.ServerError("RenameBranch", err)
363367
}

services/repository/branch.go

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -429,14 +429,15 @@ func RenameBranch(ctx context.Context, repo *repo_model.Repository, doer *user_m
429429
}
430430
}
431431

432-
isProtected, err := git_model.IsBranchProtected(ctx, repo.ID, from)
433-
if err != nil {
432+
// If from == rule name, admins are allowed to modify them.
433+
if protectedBranch, err := git_model.GetProtectedBranchRuleByName(ctx, repo.ID, from); err != nil {
434434
return "", err
435-
}
436-
if isProtected && !perm.IsAdmin() {
437-
return "", repo_model.ErrUserDoesNotHaveAccessToRepo{
438-
UserID: doer.ID,
439-
RepoName: repo.LowerName,
435+
} else {
436+
if protectedBranch != nil && !perm.IsAdmin() {
437+
return "", repo_model.ErrUserDoesNotHaveAccessToRepo{
438+
UserID: doer.ID,
439+
RepoName: repo.LowerName,
440+
}
440441
}
441442
}
442443

tests/integration/api_branch_test.go

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,25 @@ func TestAPIUpdateBranch(t *testing.T) {
219219
resp = testAPIUpdateBranch(t, "user40", "user2", "repo1", "protected-branch", "new-branch-name", http.StatusForbidden)
220220
assert.Contains(t, resp.Body.String(), "User must be a repo or site admin to rename default or protected branches.")
221221
})
222-
t.Run("RenameBranchNormalScenario", func(t *testing.T) {
222+
t.Run("UpdateBranchWithGlobedBasedProtectionRulesAndAdminAccess", func(t *testing.T) {
223+
// don't allow branch that falls under glob-based protection rules to be renamed
224+
token := getUserToken(t, "user2", auth_model.AccessTokenScopeWriteRepository)
225+
req := NewRequestWithJSON(t, "POST", "/api/v1/repos/user2/repo1/branch_protections", &api.BranchProtection{
226+
RuleName: "protected/**",
227+
EnablePush: true,
228+
}).AddTokenAuth(token)
229+
MakeRequest(t, req, http.StatusCreated)
230+
231+
from := "protected/1"
232+
req = NewRequestWithJSON(t, "POST", "/api/v1/repos/user2/repo1/branches", &api.CreateBranchRepoOption{
233+
BranchName: from,
234+
}).AddTokenAuth(token)
235+
MakeRequest(t, req, http.StatusCreated)
236+
237+
resp := testAPIUpdateBranch(t, "user2", "user2", "repo1", from, "new-branch-name", http.StatusForbidden)
238+
assert.Contains(t, resp.Body.String(), "Branch is protected by glob-based protection rules.")
239+
})
240+
t.Run("UpdateBranchNormalScenario", func(t *testing.T) {
223241
testAPIUpdateBranch(t, "user2", "user2", "repo1", "branch2", "new-branch-name", http.StatusNoContent)
224242
})
225243
})

0 commit comments

Comments
 (0)