Skip to content

Commit 8843b8b

Browse files
committed
Only allow admins to rename default/protected branches
1 parent ecd463c commit 8843b8b

File tree

6 files changed

+64
-8
lines changed

6 files changed

+64
-8
lines changed

models/fixtures/access.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,3 +171,9 @@
171171
user_id: 40
172172
repo_id: 61
173173
mode: 4
174+
175+
-
176+
id: 30
177+
user_id: 4
178+
repo_id: 1
179+
mode: 2

options/locale/locale_en-US.ini

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2713,6 +2713,7 @@ branch.create_branch_operation = Create branch
27132713
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.
2716+
branch.rename_default_or_protected_branch_error = Only admins can rename default or protected branches.
27162717
27172718
tag.create_tag = Create tag %s
27182719
tag.create_tag_operation = Create tag

routers/api/v1/repo/branch.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"code.gitea.io/gitea/models/db"
1313
git_model "code.gitea.io/gitea/models/git"
1414
"code.gitea.io/gitea/models/organization"
15+
repo_model "code.gitea.io/gitea/models/repo"
1516
user_model "code.gitea.io/gitea/models/user"
1617
"code.gitea.io/gitea/modules/git"
1718
"code.gitea.io/gitea/modules/gitrepo"
@@ -443,7 +444,12 @@ func UpdateBranch(ctx *context.APIContext) {
443444

444445
msg, err := repo_service.RenameBranch(ctx, repo, ctx.Doer, ctx.Repo.GitRepo, oldName, opt.Name)
445446
if err != nil {
446-
ctx.Error(http.StatusInternalServerError, "RenameBranch", err)
447+
switch {
448+
case repo_model.IsErrUserDoesNotHaveAccessToRepo(err):
449+
ctx.Error(http.StatusForbidden, "", "User must be a repo or site admin to rename default or protected branches.")
450+
default:
451+
ctx.Error(http.StatusInternalServerError, "RenameBranch", err)
452+
}
447453
return
448454
}
449455
if msg == "target_exist" {

routers/web/repo/setting/protected_branch.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"code.gitea.io/gitea/models/organization"
1515
"code.gitea.io/gitea/models/perm"
1616
access_model "code.gitea.io/gitea/models/perm/access"
17+
repo_model "code.gitea.io/gitea/models/repo"
1718
"code.gitea.io/gitea/modules/base"
1819
"code.gitea.io/gitea/modules/templates"
1920
"code.gitea.io/gitea/modules/web"
@@ -351,6 +352,9 @@ func RenameBranchPost(ctx *context.Context) {
351352
msg, err := repository.RenameBranch(ctx, ctx.Repo.Repository, ctx.Doer, ctx.Repo.GitRepo, form.From, form.To)
352353
if err != nil {
353354
switch {
355+
case repo_model.IsErrUserDoesNotHaveAccessToRepo(err):
356+
ctx.Flash.Error(ctx.Tr("repo.branch.rename_default_or_protected_branch_error"))
357+
ctx.Redirect(fmt.Sprintf("%s/branches", ctx.Repo.RepoLink))
354358
case git_model.IsErrBranchAlreadyExists(err):
355359
ctx.Flash.Error(ctx.Tr("repo.branch.branch_already_exists", form.To))
356360
ctx.Redirect(fmt.Sprintf("%s/branches", ctx.Repo.RepoLink))

services/repository/branch.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,30 @@ func RenameBranch(ctx context.Context, repo *repo_model.Repository, doer *user_m
416416
return "from_not_exist", nil
417417
}
418418

419+
perm, err := access_model.GetUserRepoPermission(ctx, repo, doer)
420+
if err != nil {
421+
return "", err
422+
}
423+
424+
isDefault := from == repo.DefaultBranch
425+
if isDefault && !perm.IsAdmin() {
426+
return "", repo_model.ErrUserDoesNotHaveAccessToRepo{
427+
UserID: doer.ID,
428+
RepoName: repo.LowerName,
429+
}
430+
}
431+
432+
isProtected, err := git_model.IsBranchProtected(ctx, repo.ID, from)
433+
if err != nil {
434+
return "", err
435+
}
436+
if isProtected && !perm.IsAdmin() {
437+
return "", repo_model.ErrUserDoesNotHaveAccessToRepo{
438+
UserID: doer.ID,
439+
RepoName: repo.LowerName,
440+
}
441+
}
442+
419443
if err := git_model.RenameBranch(ctx, repo, from, to, func(ctx context.Context, isDefault bool) error {
420444
err2 := gitRepo.RenameBranch(from, to)
421445
if err2 != nil {

tests/integration/api_branch_test.go

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -190,28 +190,43 @@ func testAPICreateBranch(t testing.TB, session *TestSession, user, repo, oldBran
190190
func TestAPIUpdateBranch(t *testing.T) {
191191
onGiteaRun(t, func(t *testing.T, _ *url.URL) {
192192
t.Run("UpdateBranchWithEmptyRepo", func(t *testing.T) {
193-
testAPIUpdateBranch(t, "user10", "repo6", "master", "test", http.StatusNotFound)
193+
testAPIUpdateBranch(t, "user10", "user10", "repo6", "master", "test", http.StatusNotFound)
194194
})
195195
t.Run("UpdateBranchWithSameBranchNames", func(t *testing.T) {
196-
resp := testAPIUpdateBranch(t, "user2", "repo1", "master", "master", http.StatusUnprocessableEntity)
196+
resp := testAPIUpdateBranch(t, "user2", "user2", "repo1", "master", "master", http.StatusUnprocessableEntity)
197197
assert.Contains(t, resp.Body.String(), "Cannot rename a branch using the same name or rename to a branch that already exists.")
198198
})
199199
t.Run("UpdateBranchThatAlreadyExists", func(t *testing.T) {
200-
resp := testAPIUpdateBranch(t, "user2", "repo1", "master", "branch2", http.StatusUnprocessableEntity)
200+
resp := testAPIUpdateBranch(t, "user2", "user2", "repo1", "master", "branch2", http.StatusUnprocessableEntity)
201201
assert.Contains(t, resp.Body.String(), "Cannot rename a branch using the same name or rename to a branch that already exists.")
202202
})
203203
t.Run("UpdateBranchWithNonExistentBranch", func(t *testing.T) {
204-
resp := testAPIUpdateBranch(t, "user2", "repo1", "i-dont-exist", "new-branch-name", http.StatusNotFound)
204+
resp := testAPIUpdateBranch(t, "user2", "user2", "repo1", "i-dont-exist", "new-branch-name", http.StatusNotFound)
205205
assert.Contains(t, resp.Body.String(), "Branch doesn't exist.")
206206
})
207+
t.Run("UpdateBranchWithNonAdminDoer", func(t *testing.T) {
208+
// don't allow default branch renaming
209+
resp := testAPIUpdateBranch(t, "user4", "user2", "repo1", "master", "new-branch-name", http.StatusForbidden)
210+
assert.Contains(t, resp.Body.String(), "User must be a repo or site admin to rename default or protected branches.")
211+
212+
// don't allow protected branch renaming
213+
token := getUserToken(t, "user2", auth_model.AccessTokenScopeWriteRepository)
214+
req := NewRequestWithJSON(t, "POST", "/api/v1/repos/user2/repo1/branches", &api.CreateBranchRepoOption{
215+
BranchName: "protected-branch",
216+
}).AddTokenAuth(token)
217+
MakeRequest(t, req, http.StatusCreated)
218+
testAPICreateBranchProtection(t, "protected-branch", 1, http.StatusCreated)
219+
resp = testAPIUpdateBranch(t, "user4", "user2", "repo1", "protected-branch", "new-branch-name", http.StatusForbidden)
220+
assert.Contains(t, resp.Body.String(), "User must be a repo or site admin to rename default or protected branches.")
221+
})
207222
t.Run("RenameBranchNormalScenario", func(t *testing.T) {
208-
testAPIUpdateBranch(t, "user2", "repo1", "branch2", "new-branch-name", http.StatusNoContent)
223+
testAPIUpdateBranch(t, "user2", "user2", "repo1", "branch2", "new-branch-name", http.StatusNoContent)
209224
})
210225
})
211226
}
212227

213-
func testAPIUpdateBranch(t *testing.T, ownerName, repoName, from, to string, expectedHTTPStatus int) *httptest.ResponseRecorder {
214-
token := getUserToken(t, ownerName, auth_model.AccessTokenScopeWriteRepository)
228+
func testAPIUpdateBranch(t *testing.T, doerName, ownerName, repoName, from, to string, expectedHTTPStatus int) *httptest.ResponseRecorder {
229+
token := getUserToken(t, doerName, auth_model.AccessTokenScopeWriteRepository)
215230
req := NewRequestWithJSON(t, "PATCH", "api/v1/repos/"+ownerName+"/"+repoName+"/branches/"+from, &api.UpdateBranchRepoOption{
216231
Name: to,
217232
}).AddTokenAuth(token)

0 commit comments

Comments
 (0)