From 9737d557c85c992a91f23174e33f18f6f4e57bf3 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 26 Nov 2024 20:00:57 -0800 Subject: [PATCH 1/6] Fix delete branch perm checking --- models/perm/access/repo_permission.go | 10 ++++ routers/api/v1/repo/pull.go | 80 ++++++++++++++------------- services/repository/branch.go | 30 ++++++++++ 3 files changed, 81 insertions(+), 39 deletions(-) diff --git a/models/perm/access/repo_permission.go b/models/perm/access/repo_permission.go index 0ed116a132465..cdb715e6031c3 100644 --- a/models/perm/access/repo_permission.go +++ b/models/perm/access/repo_permission.go @@ -18,6 +18,16 @@ import ( "code.gitea.io/gitea/modules/util" ) +type ErrNoPermission struct { + RepoID int64 + Unit unit.Type + Perm perm_model.AccessMode +} + +func (e ErrNoPermission) Error() string { + return fmt.Sprintf("no permission to access repo %d unit %s with mode %s", e.RepoID, e.Unit, e.Perm) +} + // Permission contains all the permissions related variables to a repository for a user type Permission struct { AccessMode perm_model.AccessMode diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 28d7379f07b8a..c02ba68a51f45 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -1057,49 +1057,51 @@ 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) + if form.DeleteBranchAfterMerge && pr.Flow == issues_model.PullRequestFlowGithub { + 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) } } diff --git a/services/repository/branch.go b/services/repository/branch.go index 600ba96e92e17..c2d6f8387109e 100644 --- a/services/repository/branch.go +++ b/services/repository/branch.go @@ -14,7 +14,10 @@ import ( "code.gitea.io/gitea/models/db" git_model "code.gitea.io/gitea/models/git" issues_model "code.gitea.io/gitea/models/issues" + perm_model "code.gitea.io/gitea/models/perm" + 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,6 +466,33 @@ var ( ErrBranchIsDefault = errors.New("branch is default") ) +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 !perm.CanWrite(unit.TypeCode) { + return access_model.ErrNoPermission{ + RepoID: repo.ID, + Unit: unit.TypeCode, + Perm: perm_model.AccessModeWrite, + } + } + + isProtected, err := git_model.IsBranchProtected(ctx, repo.ID, branchName) + if err != nil { + return err + } + 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() From 070ff9bf2f4702388fbff7ab23ea3294049fd5e5 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 26 Nov 2024 20:13:22 -0800 Subject: [PATCH 2/6] Add more checks --- models/perm/access/repo_permission.go | 2 +- routers/api/v1/repo/branch.go | 5 ----- routers/api/v1/repo/pull.go | 2 ++ routers/web/repo/pull.go | 15 +++++++-------- services/repository/branch.go | 10 +--------- 5 files changed, 11 insertions(+), 23 deletions(-) diff --git a/models/perm/access/repo_permission.go b/models/perm/access/repo_permission.go index cdb715e6031c3..a1f9dc98a3065 100644 --- a/models/perm/access/repo_permission.go +++ b/models/perm/access/repo_permission.go @@ -25,7 +25,7 @@ type ErrNoPermission struct { } func (e ErrNoPermission) Error() string { - return fmt.Sprintf("no permission to access repo %d unit %s with mode %s", e.RepoID, e.Unit, e.Perm) + return fmt.Sprintf("no permission to access repo %d unit %s with mode %s", e.RepoID, e.Unit.LogString(), e.Perm.LogString()) } // Permission contains all the permissions related variables to a repository for a user diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go index 1cea7d8c72e2e..3805d597b6922 100644 --- a/routers/api/v1/repo/branch.go +++ b/routers/api/v1/repo/branch.go @@ -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): diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index c02ba68a51f45..331e69ebe25fa 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -1058,6 +1058,8 @@ func MergePullRequest(ctx *context.APIContext) { log.Trace("Pull request merged: %d", pr.ID) 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) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index cd20c0b18e889..4992857b6e0f9 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1404,7 +1404,7 @@ func CleanUpPullRequest(ctx *context.Context) { pr := issue.PullRequest // Don't cleanup unmerged and unclosed PRs - if !pr.HasMerged && !issue.IsClosed { + if !pr.HasMerged && !issue.IsClosed && pr.Flow != issues_model.PullRequestFlowGithub { ctx.NotFound("CleanUpPullRequest", nil) return } @@ -1435,13 +1435,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, access_model.ErrNoPermission{}) { + ctx.NotFound("CleanUpPullRequest", nil) + } else { + ctx.ServerError("GetUserRepoPermission", err) + } return } diff --git a/services/repository/branch.go b/services/repository/branch.go index c2d6f8387109e..7cf794cbbbeac 100644 --- a/services/repository/branch.go +++ b/services/repository/branch.go @@ -500,17 +500,9 @@ func DeleteBranch(ctx context.Context, doer *user_model.User, repo *repo_model.R return err } - if branchName == repo.DefaultBranch { - return ErrBranchIsDefault - } - - isProtected, err := git_model.IsBranchProtected(ctx, repo.ID, branchName) - if err != nil { + if err := CanDeleteBranch(ctx, repo, branchName, doer); err != nil { return err } - if isProtected { - return git_model.ErrBranchIsProtected - } rawBranch, err := git_model.GetBranch(ctx, repo.ID, branchName) if err != nil && !git_model.IsErrBranchNotExist(err) { From b3582d0b2e7b2dce12a09e2a5fe0a9b3de13b2cd Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 28 Nov 2024 10:46:39 -0800 Subject: [PATCH 3/6] rename the error name and add detection method --- models/perm/access/repo_permission.go | 11 ++++++++--- routers/web/repo/pull.go | 6 +++--- services/repository/branch.go | 2 +- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/models/perm/access/repo_permission.go b/models/perm/access/repo_permission.go index a1f9dc98a3065..a17734dd976ac 100644 --- a/models/perm/access/repo_permission.go +++ b/models/perm/access/repo_permission.go @@ -18,14 +18,19 @@ import ( "code.gitea.io/gitea/modules/util" ) -type ErrNoPermission struct { +type ErrPermissionDenied struct { RepoID int64 Unit unit.Type Perm perm_model.AccessMode } -func (e ErrNoPermission) Error() string { - return fmt.Sprintf("no permission to access repo %d unit %s with mode %s", e.RepoID, e.Unit.LogString(), e.Perm.LogString()) +func (e ErrPermissionDenied) Error() string { + return fmt.Sprintf("permission denied to access repo %d unit %s with mode %s", e.RepoID, e.Unit.LogString(), e.Perm.LogString()) +} + +func IsErrPermissionDenied(err error) bool { + _, ok := err.(ErrPermissionDenied) + return ok } // Permission contains all the permissions related variables to a repository for a user diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 4992857b6e0f9..08073c148a775 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1436,10 +1436,10 @@ func CleanUpPullRequest(ctx *context.Context) { } if err := repo_service.CanDeleteBranch(ctx, pr.HeadRepo, pr.HeadBranch, ctx.Doer); err != nil { - if errors.Is(err, access_model.ErrNoPermission{}) { - ctx.NotFound("CleanUpPullRequest", nil) + if access_model.IsErrPermissionDenied(err) { + ctx.NotFound("CanDeleteBranch", nil) } else { - ctx.ServerError("GetUserRepoPermission", err) + ctx.ServerError("CanDeleteBranch", err) } return } diff --git a/services/repository/branch.go b/services/repository/branch.go index 7cf794cbbbeac..e0995806f7643 100644 --- a/services/repository/branch.go +++ b/services/repository/branch.go @@ -476,7 +476,7 @@ func CanDeleteBranch(ctx context.Context, repo *repo_model.Repository, branchNam return err } if !perm.CanWrite(unit.TypeCode) { - return access_model.ErrNoPermission{ + return access_model.ErrPermissionDenied{ RepoID: repo.ID, Unit: unit.TypeCode, Perm: perm_model.AccessModeWrite, From cf1d71a8b6b73ca30704d34f599c9143a5744ed4 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 28 Nov 2024 18:43:19 -0800 Subject: [PATCH 4/6] Use exist permission denied error --- models/perm/access/repo_permission.go | 15 --------------- routers/web/repo/pull.go | 2 +- services/repository/branch.go | 7 +------ 3 files changed, 2 insertions(+), 22 deletions(-) diff --git a/models/perm/access/repo_permission.go b/models/perm/access/repo_permission.go index a17734dd976ac..0ed116a132465 100644 --- a/models/perm/access/repo_permission.go +++ b/models/perm/access/repo_permission.go @@ -18,21 +18,6 @@ import ( "code.gitea.io/gitea/modules/util" ) -type ErrPermissionDenied struct { - RepoID int64 - Unit unit.Type - Perm perm_model.AccessMode -} - -func (e ErrPermissionDenied) Error() string { - return fmt.Sprintf("permission denied to access repo %d unit %s with mode %s", e.RepoID, e.Unit.LogString(), e.Perm.LogString()) -} - -func IsErrPermissionDenied(err error) bool { - _, ok := err.(ErrPermissionDenied) - return ok -} - // Permission contains all the permissions related variables to a repository for a user type Permission struct { AccessMode perm_model.AccessMode diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 08073c148a775..a47aab6ce4464 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1436,7 +1436,7 @@ func CleanUpPullRequest(ctx *context.Context) { } if err := repo_service.CanDeleteBranch(ctx, pr.HeadRepo, pr.HeadBranch, ctx.Doer); err != nil { - if access_model.IsErrPermissionDenied(err) { + if errors.Is(err, util.ErrPermissionDenied) { ctx.NotFound("CanDeleteBranch", nil) } else { ctx.ServerError("CanDeleteBranch", err) diff --git a/services/repository/branch.go b/services/repository/branch.go index e0995806f7643..508817c83e6c9 100644 --- a/services/repository/branch.go +++ b/services/repository/branch.go @@ -14,7 +14,6 @@ import ( "code.gitea.io/gitea/models/db" git_model "code.gitea.io/gitea/models/git" issues_model "code.gitea.io/gitea/models/issues" - perm_model "code.gitea.io/gitea/models/perm" access_model "code.gitea.io/gitea/models/perm/access" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unit" @@ -476,11 +475,7 @@ func CanDeleteBranch(ctx context.Context, repo *repo_model.Repository, branchNam return err } if !perm.CanWrite(unit.TypeCode) { - return access_model.ErrPermissionDenied{ - RepoID: repo.ID, - Unit: unit.TypeCode, - Perm: perm_model.AccessModeWrite, - } + 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) From 7b44f35cc167661f53eaa593677d8b86c6e3230e Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 29 Nov 2024 10:22:23 -0800 Subject: [PATCH 5/6] Add some comments --- routers/api/v1/repo/pull.go | 1 + routers/web/repo/pull.go | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 331e69ebe25fa..1116a4e9b1d25 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -1057,6 +1057,7 @@ func MergePullRequest(ctx *context.APIContext) { } log.Trace("Pull request merged: %d", pr.ID) + // 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 diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index a47aab6ce4464..766ab8f2ff708 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1403,7 +1403,7 @@ func CleanUpPullRequest(ctx *context.Context) { pr := issue.PullRequest - // Don't cleanup unmerged and unclosed PRs + // 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 From 6dbaefee1a86f419ed233ad3062573627bbc5362 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 3 Dec 2024 18:14:06 -0800 Subject: [PATCH 6/6] Add test for merging with no permission to delete branch --- routers/web/repo/pull.go | 46 +++++++++++++++------------- tests/integration/pull_merge_test.go | 29 ++++++++++++++++++ 2 files changed, 53 insertions(+), 22 deletions(-) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 766ab8f2ff708..e3b329d01d851 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -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()) } diff --git a/tests/integration/pull_merge_test.go b/tests/integration/pull_merge_test.go index bbd99f7aab0af..eb3743bc17640 100644 --- a/tests/integration/pull_merge_test.go +++ b/tests/integration/pull_merge_test.go @@ -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