github-forks
/
gitea
mirror of https://github.com/go-gitea/gitea.git
1
0
Fork 0
Code Projects Releases Activity

Fix delete branch perm checking (#32654)

Browse Source
pull/32705/head^2
Lunny Xiao 2 months ago committed by GitHub
parent c9e582c6b6
commit 17053e953f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 128 additions and 81 deletions
Show Stats Download Patch File Download Diff File

5
routers/api/v1/repo/branch.go
Unescape Escape View File

@ -150,11 +150,6 @@ func DeleteBranch(ctx *context.APIContext) {
}
}
if ctx.Repo.Repository.IsMirror {
ctx.Error(http.StatusForbidden, "IsMirrored", fmt.Errorf("can not delete branch of an mirror repository"))
return
}
if err := repo_service.DeleteBranch(ctx, ctx.Doer, ctx.Repo.Repository, ctx.Repo.GitRepo, branchName); err != nil {
switch {
case git.IsErrBranchNotExist(err):

83
routers/api/v1/repo/pull.go
Unescape Escape View File

@ -1057,49 +1057,54 @@ func MergePullRequest(ctx *context.APIContext) {
}
log.Trace("Pull request merged: %d", pr.ID)
if form.DeleteBranchAfterMerge {
// Don't cleanup when there are other PR's that use this branch as head branch.
exist, err := issues_model.HasUnmergedPullRequestsByHeadInfo(ctx, pr.HeadRepoID, pr.HeadBranch)
if err != nil {
ctx.ServerError("HasUnmergedPullRequestsByHeadInfo", err)
return
}
if exist {
ctx.Status(http.StatusOK)
return
}
var headRepo *git.Repository
if ctx.Repo != nil && ctx.Repo.Repository != nil && ctx.Repo.Repository.ID == pr.HeadRepoID && ctx.Repo.GitRepo != nil {
headRepo = ctx.Repo.GitRepo
} else {
headRepo, err = gitrepo.OpenRepository(ctx, pr.HeadRepo)
// for agit flow, we should not delete the agit reference after merge
if form.DeleteBranchAfterMerge && pr.Flow == issues_model.PullRequestFlowGithub {
// check permission even it has been checked in repo_service.DeleteBranch so that we don't need to
// do RetargetChildrenOnMerge
if err := repo_service.CanDeleteBranch(ctx, pr.HeadRepo, pr.HeadBranch, ctx.Doer); err == nil {
// Don't cleanup when there are other PR's that use this branch as head branch.
exist, err := issues_model.HasUnmergedPullRequestsByHeadInfo(ctx, pr.HeadRepoID, pr.HeadBranch)
if err != nil {
ctx.ServerError(fmt.Sprintf("OpenRepository[%s]", pr.HeadRepo.FullName()), err)
ctx.ServerError("HasUnmergedPullRequestsByHeadInfo", err)
return
}
defer headRepo.Close()
}
if err := pull_service.RetargetChildrenOnMerge(ctx, ctx.Doer, pr); err != nil {
ctx.Error(http.StatusInternalServerError, "RetargetChildrenOnMerge", err)
return
}
if err := repo_service.DeleteBranch(ctx, ctx.Doer, pr.HeadRepo, headRepo, pr.HeadBranch); err != nil {
switch {
case git.IsErrBranchNotExist(err):
ctx.NotFound(err)
case errors.Is(err, repo_service.ErrBranchIsDefault):
ctx.Error(http.StatusForbidden, "DefaultBranch", fmt.Errorf("can not delete default branch"))
case errors.Is(err, git_model.ErrBranchIsProtected):
ctx.Error(http.StatusForbidden, "IsProtectedBranch", fmt.Errorf("branch protected"))
default:
ctx.Error(http.StatusInternalServerError, "DeleteBranch", err)
if exist {
ctx.Status(http.StatusOK)
return
}
var headRepo *git.Repository
if ctx.Repo != nil && ctx.Repo.Repository != nil && ctx.Repo.Repository.ID == pr.HeadRepoID && ctx.Repo.GitRepo != nil {
headRepo = ctx.Repo.GitRepo
} else {
headRepo, err = gitrepo.OpenRepository(ctx, pr.HeadRepo)
if err != nil {
ctx.ServerError(fmt.Sprintf("OpenRepository[%s]", pr.HeadRepo.FullName()), err)
return
}
defer headRepo.Close()
}
if err := pull_service.RetargetChildrenOnMerge(ctx, ctx.Doer, pr); err != nil {
ctx.Error(http.StatusInternalServerError, "RetargetChildrenOnMerge", err)
return
}
if err := repo_service.DeleteBranch(ctx, ctx.Doer, pr.HeadRepo, headRepo, pr.HeadBranch); err != nil {
switch {
case git.IsErrBranchNotExist(err):
ctx.NotFound(err)
case errors.Is(err, repo_service.ErrBranchIsDefault):
ctx.Error(http.StatusForbidden, "DefaultBranch", fmt.Errorf("can not delete default branch"))
case errors.Is(err, git_model.ErrBranchIsProtected):
ctx.Error(http.StatusForbidden, "IsProtectedBranch", fmt.Errorf("branch protected"))
default:
ctx.Error(http.StatusInternalServerError, "DeleteBranch", err)
}
return
}
if err := issues_model.AddDeletePRBranchComment(ctx, ctx.Doer, pr.BaseRepo, pr.Issue.ID, pr.HeadBranch); err != nil {
// Do not fail here as branch has already been deleted
log.Error("DeleteBranch: %v", err)
}
return
}
if err := issues_model.AddDeletePRBranchComment(ctx, ctx.Doer, pr.BaseRepo, pr.Issue.ID, pr.HeadBranch); err != nil {
// Do not fail here as branch has already been deleted
log.Error("DeleteBranch: %v", err)
}
}

63
routers/web/repo/pull.go
Unescape Escape View File

@ -1185,32 +1185,34 @@ func MergePullRequest(ctx *context.Context) {
log.Trace("Pull request merged: %d", pr.ID)
if form.DeleteBranchAfterMerge {
// Don't cleanup when other pr use this branch as head branch
exist, err := issues_model.HasUnmergedPullRequestsByHeadInfo(ctx, pr.HeadRepoID, pr.HeadBranch)
if !form.DeleteBranchAfterMerge {
ctx.JSONRedirect(issue.Link())
return
}
// Don't cleanup when other pr use this branch as head branch
exist, err := issues_model.HasUnmergedPullRequestsByHeadInfo(ctx, pr.HeadRepoID, pr.HeadBranch)
if err != nil {
ctx.ServerError("HasUnmergedPullRequestsByHeadInfo", err)
return
}
if exist {
ctx.JSONRedirect(issue.Link())
return
}
var headRepo *git.Repository
if ctx.Repo != nil && ctx.Repo.Repository != nil && pr.HeadRepoID == ctx.Repo.Repository.ID && ctx.Repo.GitRepo != nil {
headRepo = ctx.Repo.GitRepo
} else {
headRepo, err = gitrepo.OpenRepository(ctx, pr.HeadRepo)
if err != nil {
ctx.ServerError("HasUnmergedPullRequestsByHeadInfo", err)
return
}
if exist {
ctx.JSONRedirect(issue.Link())
ctx.ServerError(fmt.Sprintf("OpenRepository[%s]", pr.HeadRepo.FullName()), err)
return
}
var headRepo *git.Repository
if ctx.Repo != nil && ctx.Repo.Repository != nil && pr.HeadRepoID == ctx.Repo.Repository.ID && ctx.Repo.GitRepo != nil {
headRepo = ctx.Repo.GitRepo
} else {
headRepo, err = gitrepo.OpenRepository(ctx, pr.HeadRepo)
if err != nil {
ctx.ServerError(fmt.Sprintf("OpenRepository[%s]", pr.HeadRepo.FullName()), err)
return
}
defer headRepo.Close()
}
deleteBranch(ctx, pr, headRepo)
defer headRepo.Close()
}
deleteBranch(ctx, pr, headRepo)
ctx.JSONRedirect(issue.Link())
}
@ -1403,8 +1405,8 @@ func CleanUpPullRequest(ctx *context.Context) {
pr := issue.PullRequest
// Don't cleanup unmerged and unclosed PRs
if !pr.HasMerged && !issue.IsClosed {
// Don't cleanup unmerged and unclosed PRs and agit PRs
if !pr.HasMerged && !issue.IsClosed && pr.Flow != issues_model.PullRequestFlowGithub {
ctx.NotFound("CleanUpPullRequest", nil)
return
}
@ -1435,13 +1437,12 @@ func CleanUpPullRequest(ctx *context.Context) {
return
}
perm, err := access_model.GetUserRepoPermission(ctx, pr.HeadRepo, ctx.Doer)
if err != nil {
ctx.ServerError("GetUserRepoPermission", err)
return
}
if !perm.CanWrite(unit.TypeCode) {
ctx.NotFound("CleanUpPullRequest", nil)
if err := repo_service.CanDeleteBranch(ctx, pr.HeadRepo, pr.HeadBranch, ctx.Doer); err != nil {
if errors.Is(err, util.ErrPermissionDenied) {
ctx.NotFound("CanDeleteBranch", nil)
} else {
ctx.ServerError("CanDeleteBranch", err)
}
return
}

29
services/repository/branch.go
Unescape Escape View File

@ -14,7 +14,9 @@ import (
"code.gitea.io/gitea/models/db"
git_model "code.gitea.io/gitea/models/git"
issues_model "code.gitea.io/gitea/models/issues"
access_model "code.gitea.io/gitea/models/perm/access"
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unit"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/cache"
"code.gitea.io/gitea/modules/git"
@ -463,15 +465,17 @@ var (
ErrBranchIsDefault = errors.New("branch is default")
)
// DeleteBranch delete branch
func DeleteBranch(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, gitRepo *git.Repository, branchName string) error {
err := repo.MustNotBeArchived()
func CanDeleteBranch(ctx context.Context, repo *repo_model.Repository, branchName string, doer *user_model.User) error {
if branchName == repo.DefaultBranch {
return ErrBranchIsDefault
}
perm, err := access_model.GetUserRepoPermission(ctx, repo, doer)
if err != nil {
return err
}
if branchName == repo.DefaultBranch {
return ErrBranchIsDefault
if !perm.CanWrite(unit.TypeCode) {
return util.NewPermissionDeniedErrorf("permission denied to access repo %d unit %s", repo.ID, unit.TypeCode.LogString())
}
isProtected, err := git_model.IsBranchProtected(ctx, repo.ID, branchName)
@ -481,6 +485,19 @@ func DeleteBranch(ctx context.Context, doer *user_model.User, repo *repo_model.R
if isProtected {
return git_model.ErrBranchIsProtected
}
return nil
}
// DeleteBranch delete branch
func DeleteBranch(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, gitRepo *git.Repository, branchName string) error {
err := repo.MustNotBeArchived()
if err != nil {
return err
}
if err := CanDeleteBranch(ctx, repo, branchName, doer); err != nil {
return err
}
rawBranch, err := git_model.GetBranch(ctx, repo.ID, branchName)
if err != nil && !git_model.IsErrBranchNotExist(err) {

29
tests/integration/pull_merge_test.go
Unescape Escape View File

@ -554,6 +554,10 @@ func TestPullRetargetChildOnBranchDelete(t *testing.T) {
testPullMerge(t, session, elemBasePR[1], elemBasePR[2], elemBasePR[4], repo_model.MergeStyleMerge, true)
repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user2", Name: "repo1"})
branchBasePR := unittest.AssertExistsAndLoadBean(t, &git_model.Branch{RepoID: repo1.ID, Name: "base-pr"})
assert.True(t, branchBasePR.IsDeleted)
// Check child PR
req := NewRequest(t, "GET", test.RedirectURL(respChildPR))
resp := session.MakeRequest(t, req, http.StatusOK)
@ -584,6 +588,10 @@ func TestPullDontRetargetChildOnWrongRepo(t *testing.T) {
testPullMerge(t, session, elemBasePR[1], elemBasePR[2], elemBasePR[4], repo_model.MergeStyleMerge, true)
repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user1", Name: "repo1"})
branchBasePR := unittest.AssertExistsAndLoadBean(t, &git_model.Branch{RepoID: repo1.ID, Name: "base-pr"})
assert.True(t, branchBasePR.IsDeleted)
// Check child PR
req := NewRequest(t, "GET", test.RedirectURL(respChildPR))
resp := session.MakeRequest(t, req, http.StatusOK)
@ -598,6 +606,27 @@ func TestPullDontRetargetChildOnWrongRepo(t *testing.T) {
})
}
func TestPullRequestMergedWithNoPermissionDeleteBranch(t *testing.T) {
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
session := loginUser(t, "user4")
testRepoFork(t, session, "user2", "repo1", "user4", "repo1", "")
testEditFileToNewBranch(t, session, "user4", "repo1", "master", "base-pr", "README.md", "Hello, World\n(Edited - TestPullDontRetargetChildOnWrongRepo - base PR)\n")
respBasePR := testPullCreate(t, session, "user4", "repo1", false, "master", "base-pr", "Base Pull Request")
elemBasePR := strings.Split(test.RedirectURL(respBasePR), "/")
assert.EqualValues(t, "pulls", elemBasePR[3])
// user2 has no permission to delete branch of repo user1/repo1
session2 := loginUser(t, "user2")
testPullMerge(t, session2, elemBasePR[1], elemBasePR[2], elemBasePR[4], repo_model.MergeStyleMerge, true)
repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user4", Name: "repo1"})
branchBasePR := unittest.AssertExistsAndLoadBean(t, &git_model.Branch{RepoID: repo1.ID, Name: "base-pr"})
// branch has not been deleted
assert.False(t, branchBasePR.IsDeleted)
})
}
func TestPullMergeIndexerNotifier(t *testing.T) {
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
// create a pull request

Write Preview
Loading…
Cancel
Save