Skip to content

Commit 070ff9b

Browse files
committed
Add more checks
1 parent 9737d55 commit 070ff9b

File tree

5 files changed

+11
-23
lines changed

5 files changed

+11
-23
lines changed

models/perm/access/repo_permission.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ type ErrNoPermission struct {
2525
}
2626

2727
func (e ErrNoPermission) Error() string {
28-
return fmt.Sprintf("no permission to access repo %d unit %s with mode %s", e.RepoID, e.Unit, e.Perm)
28+
return fmt.Sprintf("no permission to access repo %d unit %s with mode %s", e.RepoID, e.Unit.LogString(), e.Perm.LogString())
2929
}
3030

3131
// Permission contains all the permissions related variables to a repository for a user

routers/api/v1/repo/branch.go

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -150,11 +150,6 @@ func DeleteBranch(ctx *context.APIContext) {
150150
}
151151
}
152152

153-
if ctx.Repo.Repository.IsMirror {
154-
ctx.Error(http.StatusForbidden, "IsMirrored", fmt.Errorf("can not delete branch of an mirror repository"))
155-
return
156-
}
157-
158153
if err := repo_service.DeleteBranch(ctx, ctx.Doer, ctx.Repo.Repository, ctx.Repo.GitRepo, branchName); err != nil {
159154
switch {
160155
case git.IsErrBranchNotExist(err):

routers/api/v1/repo/pull.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1058,6 +1058,8 @@ func MergePullRequest(ctx *context.APIContext) {
10581058
log.Trace("Pull request merged: %d", pr.ID)
10591059

10601060
if form.DeleteBranchAfterMerge && pr.Flow == issues_model.PullRequestFlowGithub {
1061+
// check permission even it has been checked in repo_service.DeleteBranch so that we don't need to
1062+
// do RetargetChildrenOnMerge
10611063
if err := repo_service.CanDeleteBranch(ctx, pr.HeadRepo, pr.HeadBranch, ctx.Doer); err == nil {
10621064
// Don't cleanup when there are other PR's that use this branch as head branch.
10631065
exist, err := issues_model.HasUnmergedPullRequestsByHeadInfo(ctx, pr.HeadRepoID, pr.HeadBranch)

routers/web/repo/pull.go

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1404,7 +1404,7 @@ func CleanUpPullRequest(ctx *context.Context) {
14041404
pr := issue.PullRequest
14051405

14061406
// Don't cleanup unmerged and unclosed PRs
1407-
if !pr.HasMerged && !issue.IsClosed {
1407+
if !pr.HasMerged && !issue.IsClosed && pr.Flow != issues_model.PullRequestFlowGithub {
14081408
ctx.NotFound("CleanUpPullRequest", nil)
14091409
return
14101410
}
@@ -1435,13 +1435,12 @@ func CleanUpPullRequest(ctx *context.Context) {
14351435
return
14361436
}
14371437

1438-
perm, err := access_model.GetUserRepoPermission(ctx, pr.HeadRepo, ctx.Doer)
1439-
if err != nil {
1440-
ctx.ServerError("GetUserRepoPermission", err)
1441-
return
1442-
}
1443-
if !perm.CanWrite(unit.TypeCode) {
1444-
ctx.NotFound("CleanUpPullRequest", nil)
1438+
if err := repo_service.CanDeleteBranch(ctx, pr.HeadRepo, pr.HeadBranch, ctx.Doer); err != nil {
1439+
if errors.Is(err, access_model.ErrNoPermission{}) {
1440+
ctx.NotFound("CleanUpPullRequest", nil)
1441+
} else {
1442+
ctx.ServerError("GetUserRepoPermission", err)
1443+
}
14451444
return
14461445
}
14471446

services/repository/branch.go

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -500,17 +500,9 @@ func DeleteBranch(ctx context.Context, doer *user_model.User, repo *repo_model.R
500500
return err
501501
}
502502

503-
if branchName == repo.DefaultBranch {
504-
return ErrBranchIsDefault
505-
}
506-
507-
isProtected, err := git_model.IsBranchProtected(ctx, repo.ID, branchName)
508-
if err != nil {
503+
if err := CanDeleteBranch(ctx, repo, branchName, doer); err != nil {
509504
return err
510505
}
511-
if isProtected {
512-
return git_model.ErrBranchIsProtected
513-
}
514506

515507
rawBranch, err := git_model.GetBranch(ctx, repo.ID, branchName)
516508
if err != nil && !git_model.IsErrBranchNotExist(err) {

0 commit comments

Comments
 (0)