troldspejlet/forgejo
troldspejlet/forgejo
1
0
Fork 0
mirror of https://codeberg.org/forgejo/forgejo.git synced 2024-11-14 22:59:29 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions 13

Merge pull request '[v7.0/forgejo] fix(hook): repo admins are wrongly denied the right to force merge' (#3997) from earl-warren/forgejo:wip-7.0-admin-protection into v7.0/forgejo

Browse source 
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/3997
Reviewed-by: Victoria <efertone@noreply.codeberg.org>
Reviewed-by: Otto <otto@codeberg.org>
This commit is contained in:
Earl Warren 2024-06-03 04:51:53 +00:00
commit 4d0a5ea317
13 changed files with 349 additions and 127 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
release-notes/8.0.0/fix/3976.md Normal file
View file

@ -0,0 +1 @@
- repository admins are always denied the right to force merge and instance admins are subject to restrictions to merge that must only apply to repository admins

9
routers/private/hook_pre_receive.go
View file

@ -398,9 +398,14 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r
return
}
// If we're an admin for the instance, we can ignore checks
if ctx.user.IsAdmin {
return
}
// It's not allowed t overwrite protected files. Unless if the user is an
// admin and the protected branch rule doesn't apply to admins.
if changedProtectedfiles && (!ctx.user.IsAdmin || protectBranch.ApplyToAdmins) {
if changedProtectedfiles && (!ctx.userPerm.IsAdmin() || protectBranch.ApplyToAdmins) {
log.Warn("Forbidden: Branch: %s in %-v is protected from changing file %s", branchName, repo, protectedFilePath)
ctx.JSON(http.StatusForbidden, private.Response{
UserMsg: fmt.Sprintf("branch %s is protected from changing file %s", branchName, protectedFilePath),
@ -412,7 +417,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r
if pb, err := pull_service.CheckPullBranchProtections(ctx, pr, true); err != nil {
if models.IsErrDisallowedToMerge(err) {
// Allow this if the rule doesn't apply to admins and the user is an admin.
if ctx.user.IsAdmin && !pb.ApplyToAdmins {
if ctx.userPerm.IsAdmin() && !pb.ApplyToAdmins {
return
}
log.Warn("Forbidden: User %d is not allowed push to protected branch %s in %-v and pr #%d is not ready to be merged: %s", ctx.opts.UserID, branchName, repo, pr.Index, err.Error())

16
services/pull/check.go
View file

@ -119,12 +119,16 @@ func CheckPullMergable(stdCtx context.Context, doer *user_model.User, perm *acce
// * if the doer is admin, they could skip the branch protection check,
// if that's allowed by the protected branch rule.
if adminSkipProtectionCheck && !pb.ApplyToAdmins {
if isRepoAdmin, errCheckAdmin := access_model.IsUserRepoAdmin(ctx, pr.BaseRepo, doer); errCheckAdmin != nil {
log.Error("Unable to check if %-v is a repo admin in %-v: %v", doer, pr.BaseRepo, errCheckAdmin)
return errCheckAdmin
} else if isRepoAdmin {
err = nil // repo admin can skip the check, so clear the error
if adminSkipProtectionCheck {
if doer.IsAdmin {
err = nil // instance admin can skip the check, so clear the error
} else if !pb.ApplyToAdmins {
if isRepoAdmin, errCheckAdmin := access_model.IsUserRepoAdmin(ctx, pr.BaseRepo, doer); errCheckAdmin != nil {
log.Error("Unable to check if %-v is a repo admin in %-v: %v", doer, pr.BaseRepo, errCheckAdmin)
return errCheckAdmin
} else if isRepoAdmin {
err = nil // repo admin can skip the check, so clear the error
}
}
}

7
tests/gitea-repositories-meta/user5/repo4.git/hooks/post-receive Executable file
View file

@ -0,0 +1,7 @@
#!/usr/bin/env bash
ORI_DIR=`pwd`
SHELL_FOLDER=$(cd "$(dirname "$0")";pwd)
cd "$ORI_DIR"
for i in `ls "$SHELL_FOLDER/post-receive.d"`; do
sh "$SHELL_FOLDER/post-receive.d/$i"
done

2
tests/gitea-repositories-meta/user5/repo4.git/hooks/post-receive.d/gitea Executable file
View file

@ -0,0 +1,2 @@
#!/usr/bin/env bash
"$GITEA_ROOT/gitea" hook --config="$GITEA_ROOT/$GITEA_CONF" post-receive

7
tests/gitea-repositories-meta/user5/repo4.git/hooks/pre-receive Executable file
View file

@ -0,0 +1,7 @@
#!/usr/bin/env bash
ORI_DIR=`pwd`
SHELL_FOLDER=$(cd "$(dirname "$0")";pwd)
cd "$ORI_DIR"
for i in `ls "$SHELL_FOLDER/pre-receive.d"`; do
sh "$SHELL_FOLDER/pre-receive.d/$i"
done

2
tests/gitea-repositories-meta/user5/repo4.git/hooks/pre-receive.d/gitea Executable file
View file

@ -0,0 +1,2 @@
#!/usr/bin/env bash
"$GITEA_ROOT/gitea" hook --config="$GITEA_ROOT/$GITEA_CONF" pre-receive

7
tests/gitea-repositories-meta/user5/repo4.git/hooks/update Executable file
View file

@ -0,0 +1,7 @@
#!/usr/bin/env bash
ORI_DIR=`pwd`
SHELL_FOLDER=$(cd "$(dirname "$0")";pwd)
cd "$ORI_DIR"
for i in `ls "$SHELL_FOLDER/update.d"`; do
sh "$SHELL_FOLDER/update.d/$i" $1 $2 $3
done

2
tests/gitea-repositories-meta/user5/repo4.git/hooks/update.d/gitea Executable file
View file

@ -0,0 +1,2 @@
#!/usr/bin/env bash
"$GITEA_ROOT/gitea" hook --config="$GITEA_ROOT/$GITEA_CONF" update $1 $2 $3

74
tests/integration/api_helper_for_declarative_test.go
View file

@ -257,41 +257,51 @@ func doAPIGetPullRequest(ctx APITestContext, owner, repo string, index int64) fu
func doAPIMergePullRequest(ctx APITestContext, owner, repo string, index int64) func(*testing.T) {
return func(t *testing.T) {
urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/merge", owner, repo, index)
var req *RequestWrapper
var resp *httptest.ResponseRecorder
for i := 0; i < 6; i++ {
req = NewRequestWithJSON(t, http.MethodPost, urlStr, &forms.MergePullRequestForm{
MergeMessageField: "doAPIMergePullRequest Merge",
Do: string(repo_model.MergeStyleMerge),
}).AddTokenAuth(ctx.Token)
resp = ctx.Session.MakeRequest(t, req, NoExpectedStatus)
if resp.Code != http.StatusMethodNotAllowed {
break
}
err := api.APIError{}
DecodeJSON(t, resp, &err)
assert.EqualValues(t, "Please try again later", err.Message)
queue.GetManager().FlushAll(context.Background(), 5*time.Second)
<-time.After(1 * time.Second)
}
expected := ctx.ExpectedCode
if expected == 0 {
expected = http.StatusOK
}
if !assert.EqualValues(t, expected, resp.Code,
"Request: %s %s", req.Method, req.URL.String()) {
logUnexpectedResponse(t, resp)
}
t.Helper()
doAPIMergePullRequestForm(t, ctx, owner, repo, index, &forms.MergePullRequestForm{
MergeMessageField: "doAPIMergePullRequest Merge",
Do: string(repo_model.MergeStyleMerge),
})
}
}
func doAPIMergePullRequestForm(t *testing.T, ctx APITestContext, owner, repo string, index int64, merge *forms.MergePullRequestForm) *httptest.ResponseRecorder {
t.Helper()
urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/merge", owner, repo, index)
var req *RequestWrapper
var resp *httptest.ResponseRecorder
for i := 0; i < 6; i++ {
req = NewRequestWithJSON(t, http.MethodPost, urlStr, merge).AddTokenAuth(ctx.Token)
resp = ctx.Session.MakeRequest(t, req, NoExpectedStatus)
if resp.Code != http.StatusMethodNotAllowed {
break
}
err := api.APIError{}
DecodeJSON(t, resp, &err)
if err.Message != "Please try again later" {
break
}
queue.GetManager().FlushAll(context.Background(), 5*time.Second)
<-time.After(1 * time.Second)
}
expected := ctx.ExpectedCode
if expected == 0 {
expected = http.StatusOK
}
if !assert.EqualValues(t, expected, resp.Code,
"Request: %s %s", req.Method, req.URL.String()) {
logUnexpectedResponse(t, resp)
}
return resp
}
func doAPIManuallyMergePullRequest(ctx APITestContext, owner, repo, commitID string, index int64) func(*testing.T) {
return func(t *testing.T) {
urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/merge", owner, repo, index)

16
tests/integration/git_helper_for_declarative_test.go
View file

@ -89,6 +89,7 @@ func onGiteaRun[T testing.TB](t T, callback func(T, *url.URL)) {
func doGitClone(dstLocalPath string, u *url.URL) func(*testing.T) {
return func(t *testing.T) {
t.Helper()
assert.NoError(t, git.CloneWithArgs(context.Background(), git.AllowLFSFiltersArgs(), u.String(), dstLocalPath, git.CloneRepoOptions{}))
exist, err := util.IsExist(filepath.Join(dstLocalPath, "README.md"))
assert.NoError(t, err)
@ -98,6 +99,7 @@ func doGitClone(dstLocalPath string, u *url.URL) func(*testing.T) {
func doPartialGitClone(dstLocalPath string, u *url.URL) func(*testing.T) {
return func(t *testing.T) {
t.Helper()
assert.NoError(t, git.CloneWithArgs(context.Background(), git.AllowLFSFiltersArgs(), u.String(), dstLocalPath, git.CloneRepoOptions{
Filter: "blob:none",
}))
@ -109,6 +111,7 @@ func doPartialGitClone(dstLocalPath string, u *url.URL) func(*testing.T) {
func doGitCloneFail(u *url.URL) func(*testing.T) {
return func(t *testing.T) {
t.Helper()
tmpDir := t.TempDir()
assert.Error(t, git.Clone(git.DefaultContext, u.String(), tmpDir, git.CloneRepoOptions{}))
exist, err := util.IsExist(filepath.Join(tmpDir, "README.md"))
@ -119,6 +122,7 @@ func doGitCloneFail(u *url.URL) func(*testing.T) {
func doGitInitTestRepository(dstPath string, objectFormat git.ObjectFormat) func(*testing.T) {
return func(t *testing.T) {
t.Helper()
// Init repository in dstPath
assert.NoError(t, git.InitRepository(git.DefaultContext, dstPath, false, objectFormat.Name()))
// forcibly set default branch to master
@ -141,6 +145,7 @@ func doGitInitTestRepository(dstPath string, objectFormat git.ObjectFormat) func
func doGitAddRemote(dstPath, remoteName string, u *url.URL) func(*testing.T) {
return func(t *testing.T) {
t.Helper()
_, _, err := git.NewCommand(git.DefaultContext, "remote", "add").AddDynamicArguments(remoteName, u.String()).RunStdString(&git.RunOpts{Dir: dstPath})
assert.NoError(t, err)
}
@ -156,6 +161,7 @@ func doGitPushTestRepository(dstPath string, args ...string) func(*testing.T) {
func doGitPushTestRepositoryFail(dstPath string, args ...string) func(*testing.T) {
return func(t *testing.T) {
t.Helper()
_, _, err := git.NewCommand(git.DefaultContext, "push").AddArguments(git.ToTrustedCmdArgs(args)...).RunStdString(&git.RunOpts{Dir: dstPath})
assert.Error(t, err)
}
@ -163,6 +169,7 @@ func doGitPushTestRepositoryFail(dstPath string, args ...string) func(*testing.T
func doGitCreateBranch(dstPath, branch string) func(*testing.T) {
return func(t *testing.T) {
t.Helper()
_, _, err := git.NewCommand(git.DefaultContext, "checkout", "-b").AddDynamicArguments(branch).RunStdString(&git.RunOpts{Dir: dstPath})
assert.NoError(t, err)
}
@ -170,20 +177,15 @@ func doGitCreateBranch(dstPath, branch string) func(*testing.T) {
func doGitCheckoutBranch(dstPath string, args ...string) func(*testing.T) {
return func(t *testing.T) {
t.Helper()
_, _, err := git.NewCommandContextNoGlobals(git.DefaultContext, git.AllowLFSFiltersArgs()...).AddArguments("checkout").AddArguments(git.ToTrustedCmdArgs(args)...).RunStdString(&git.RunOpts{Dir: dstPath})
assert.NoError(t, err)
}
}
func doGitMerge(dstPath string, args ...string) func(*testing.T) {
return func(t *testing.T) {
_, _, err := git.NewCommand(git.DefaultContext, "merge").AddArguments(git.ToTrustedCmdArgs(args)...).RunStdString(&git.RunOpts{Dir: dstPath})
assert.NoError(t, err)
}
}
func doGitPull(dstPath string, args ...string) func(*testing.T) {
return func(t *testing.T) {
t.Helper()
_, _, err := git.NewCommandContextNoGlobals(git.DefaultContext, git.AllowLFSFiltersArgs()...).AddArguments("pull").AddArguments(git.ToTrustedCmdArgs(args)...).RunStdString(&git.RunOpts{Dir: dstPath})
assert.NoError(t, err)
}

153
tests/integration/git_test.go
View file

@ -85,7 +85,7 @@ func testGit(t *testing.T, u *url.URL) {
t.Run("CreateAgitFlowPull", doCreateAgitFlowPull(dstPath, &httpContext, "master", "test/head"))
t.Run("InternalReferences", doInternalReferences(&httpContext, dstPath))
t.Run("BranchProtectMerge", doBranchProtectPRMerge(&httpContext, dstPath))
t.Run("BranchProtectMerge", doBranchProtect(&httpContext, dstPath))
t.Run("AutoMerge", doAutoPRMerge(&httpContext, dstPath))
t.Run("CreatePRAndSetManuallyMerged", doCreatePRAndSetManuallyMerged(httpContext, httpContext, dstPath, "master", "test-manually-merge"))
t.Run("MergeFork", func(t *testing.T) {
@ -127,7 +127,7 @@ func testGit(t *testing.T, u *url.URL) {
t.Run("CreateAgitFlowPull", doCreateAgitFlowPull(dstPath, &sshContext, "master", "test/head2"))
t.Run("InternalReferences", doInternalReferences(&sshContext, dstPath))
t.Run("BranchProtectMerge", doBranchProtectPRMerge(&sshContext, dstPath))
t.Run("BranchProtectMerge", doBranchProtect(&sshContext, dstPath))
t.Run("MergeFork", func(t *testing.T) {
defer tests.PrintCurrentTest(t)()
t.Run("CreatePRAndMerge", doMergeFork(sshContext, forkedUserCtx, "master", sshContext.Username+":master"))
@ -360,63 +360,86 @@ func generateCommitWithNewData(size int, repoPath, email, fullName, prefix strin
return filepath.Base(tmpFile.Name()), err
}
func doBranchProtectPRMerge(baseCtx *APITestContext, dstPath string) func(t *testing.T) {
func doBranchProtect(baseCtx *APITestContext, dstPath string) func(t *testing.T) {
return func(t *testing.T) {
defer tests.PrintCurrentTest(t)()
t.Run("CreateBranchProtected", doGitCreateBranch(dstPath, "protected"))
t.Run("PushProtectedBranch", doGitPushTestRepository(dstPath, "origin", "protected"))
ctx := NewAPITestContext(t, baseCtx.Username, baseCtx.Reponame, auth_model.AccessTokenScopeWriteRepository)
t.Run("ProtectProtectedBranchNoWhitelist", doProtectBranch(ctx, "protected", "", ""))
t.Run("GenerateCommit", func(t *testing.T) {
_, err := generateCommitWithNewData(littleSize, dstPath, "user2@example.com", "User Two", "branch-data-file-")
assert.NoError(t, err)
})
t.Run("FailToPushToProtectedBranch", doGitPushTestRepositoryFail(dstPath, "origin", "protected"))
t.Run("PushToUnprotectedBranch", doGitPushTestRepository(dstPath, "origin", "protected:unprotected"))
var pr api.PullRequest
var err error
t.Run("CreatePullRequest", func(t *testing.T) {
pr, err = doAPICreatePullRequest(ctx, baseCtx.Username, baseCtx.Reponame, "protected", "unprotected")(t)
assert.NoError(t, err)
})
t.Run("GenerateCommit", func(t *testing.T) {
_, err := generateCommitWithNewData(littleSize, dstPath, "user2@example.com", "User Two", "branch-data-file-")
assert.NoError(t, err)
})
t.Run("PushToUnprotectedBranch", doGitPushTestRepository(dstPath, "origin", "protected:unprotected-2"))
var pr2 api.PullRequest
t.Run("CreatePullRequest", func(t *testing.T) {
pr2, err = doAPICreatePullRequest(ctx, baseCtx.Username, baseCtx.Reponame, "unprotected", "unprotected-2")(t)
assert.NoError(t, err)
})
t.Run("MergePR2", doAPIMergePullRequest(ctx, baseCtx.Username, baseCtx.Reponame, pr2.Index))
t.Run("MergePR", doAPIMergePullRequest(ctx, baseCtx.Username, baseCtx.Reponame, pr.Index))
t.Run("PullProtected", doGitPull(dstPath, "origin", "protected"))
t.Run("ProtectProtectedBranchUnprotectedFilePaths", doProtectBranch(ctx, "protected", "", "unprotected-file-*"))
t.Run("GenerateCommit", func(t *testing.T) {
_, err := generateCommitWithNewData(littleSize, dstPath, "user2@example.com", "User Two", "unprotected-file-")
assert.NoError(t, err)
})
t.Run("PushUnprotectedFilesToProtectedBranch", doGitPushTestRepository(dstPath, "origin", "protected"))
t.Run("FailToPushToProtectedBranch", func(t *testing.T) {
t.Run("ProtectProtectedBranch", doProtectBranch(ctx, "protected"))
t.Run("Create modified-protected-branch", doGitCheckoutBranch(dstPath, "-b", "modified-protected-branch", "protected"))
t.Run("GenerateCommit", func(t *testing.T) {
_, err := generateCommitWithNewData(littleSize, dstPath, "user2@example.com", "User Two", "branch-data-file-")
assert.NoError(t, err)
})
t.Run("ProtectProtectedBranchWhitelist", doProtectBranch(ctx, "protected", baseCtx.Username, ""))
t.Run("CheckoutMaster", doGitCheckoutBranch(dstPath, "master"))
t.Run("CreateBranchForced", doGitCreateBranch(dstPath, "toforce"))
t.Run("GenerateCommit", func(t *testing.T) {
_, err := generateCommitWithNewData(littleSize, dstPath, "user2@example.com", "User Two", "branch-data-file-")
assert.NoError(t, err)
doGitPushTestRepositoryFail(dstPath, "origin", "modified-protected-branch:protected")(t)
})
t.Run("PushToUnprotectedBranch", doGitPushTestRepository(dstPath, "origin", "modified-protected-branch:unprotected"))
t.Run("FailToPushProtectedFilesToProtectedBranch", func(t *testing.T) {
t.Run("Create modified-protected-file-protected-branch", doGitCheckoutBranch(dstPath, "-b", "modified-protected-file-protected-branch", "protected"))
t.Run("GenerateCommit", func(t *testing.T) {
_, err := generateCommitWithNewData(littleSize, dstPath, "user2@example.com", "User Two", "protected-file-")
assert.NoError(t, err)
})
t.Run("ProtectedFilePathsApplyToAdmins", doProtectBranch(ctx, "protected"))
doGitPushTestRepositoryFail(dstPath, "origin", "modified-protected-file-protected-branch:protected")(t)
doGitCheckoutBranch(dstPath, "protected")(t)
doGitPull(dstPath, "origin", "protected")(t)
})
t.Run("PushUnprotectedFilesToProtectedBranch", func(t *testing.T) {
t.Run("Create modified-unprotected-file-protected-branch", doGitCheckoutBranch(dstPath, "-b", "modified-unprotected-file-protected-branch", "protected"))
t.Run("UnprotectedFilePaths", doProtectBranch(ctx, "protected", parameterProtectBranch{
"unprotected_file_patterns": "unprotected-file-*",
}))
t.Run("GenerateCommit", func(t *testing.T) {
_, err := generateCommitWithNewData(littleSize, dstPath, "user2@example.com", "User Two", "unprotected-file-")
assert.NoError(t, err)
})
doGitPushTestRepository(dstPath, "origin", "modified-unprotected-file-protected-branch:protected")(t)
doGitCheckoutBranch(dstPath, "protected")(t)
doGitPull(dstPath, "origin", "protected")(t)
})
user, err := user_model.GetUserByName(db.DefaultContext, baseCtx.Username)
assert.NoError(t, err)
t.Run("WhitelistUsers", doProtectBranch(ctx, "protected", parameterProtectBranch{
"enable_push": "whitelist",
"enable_whitelist": "on",
"whitelist_users": strconv.FormatInt(user.ID, 10),
}))
t.Run("WhitelistedUserFailToForcePushToProtectedBranch", func(t *testing.T) {
t.Run("Create toforce", doGitCheckoutBranch(dstPath, "-b", "toforce", "master"))
t.Run("GenerateCommit", func(t *testing.T) {
_, err := generateCommitWithNewData(littleSize, dstPath, "user2@example.com", "User Two", "branch-data-file-")
assert.NoError(t, err)
})
doGitPushTestRepositoryFail(dstPath, "-f", "origin", "toforce:protected")(t)
})
t.Run("WhitelistedUserPushToProtectedBranch", func(t *testing.T) {
t.Run("Create topush", doGitCheckoutBranch(dstPath, "-b", "topush", "protected"))
t.Run("GenerateCommit", func(t *testing.T) {
_, err := generateCommitWithNewData(littleSize, dstPath, "user2@example.com", "User Two", "branch-data-file-")
assert.NoError(t, err)
})
doGitPushTestRepository(dstPath, "origin", "topush:protected")(t)
})
t.Run("FailToForcePushToProtectedBranch", doGitPushTestRepositoryFail(dstPath, "-f", "origin", "toforce:protected"))
t.Run("MergeProtectedToToforce", doGitMerge(dstPath, "protected"))
t.Run("PushToProtectedBranch", doGitPushTestRepository(dstPath, "origin", "toforce:protected"))
t.Run("CheckoutMasterAgain", doGitCheckoutBranch(dstPath, "master"))
}
}
func doProtectBranch(ctx APITestContext, branch, userToWhitelist, unprotectedFilePatterns string) func(t *testing.T) {
type parameterProtectBranch map[string]string
func doProtectBranch(ctx APITestContext, branch string, addParameter ...parameterProtectBranch) func(t *testing.T) {
// We are going to just use the owner to set the protection.
return func(t *testing.T) {
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{Name: ctx.Reponame, OwnerName: ctx.Username})
@ -425,30 +448,20 @@ func doProtectBranch(ctx APITestContext, branch, userToWhitelist, unprotectedFil
csrf := GetCSRF(t, ctx.Session, fmt.Sprintf("/%s/%s/settings/branches", url.PathEscape(ctx.Username), url.PathEscape(ctx.Reponame)))
if userToWhitelist == "" {
// Change branch to protected
req := NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/settings/branches/edit", url.PathEscape(ctx.Username), url.PathEscape(ctx.Reponame)), map[string]string{
"_csrf": csrf,
"rule_id": strconv.FormatInt(rule.ID, 10),
"rule_name": branch,
"unprotected_file_patterns": unprotectedFilePatterns,
})
ctx.Session.MakeRequest(t, req, http.StatusSeeOther)
} else {
user, err := user_model.GetUserByName(db.DefaultContext, userToWhitelist)
assert.NoError(t, err)
// Change branch to protected
req := NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/settings/branches/edit", url.PathEscape(ctx.Username), url.PathEscape(ctx.Reponame)), map[string]string{
"_csrf": csrf,
"rule_name": branch,
"rule_id": strconv.FormatInt(rule.ID, 10),
"enable_push": "whitelist",
"enable_whitelist": "on",
"whitelist_users": strconv.FormatInt(user.ID, 10),
"unprotected_file_patterns": unprotectedFilePatterns,
})
ctx.Session.MakeRequest(t, req, http.StatusSeeOther)
parameter := parameterProtectBranch{
"_csrf": csrf,
"rule_id": strconv.FormatInt(rule.ID, 10),
"rule_name": branch,
}
if len(addParameter) > 0 {
for k, v := range addParameter[0] {
parameter[k] = v
}
}
// Change branch to protected
req := NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/settings/branches/edit", url.PathEscape(ctx.Username), url.PathEscape(ctx.Reponame)), parameter)
ctx.Session.MakeRequest(t, req, http.StatusSeeOther)
// Check if master branch has been locked successfully
flashCookie := ctx.Session.GetCookie(gitea_context.CookieNameFlash)
assert.NotNil(t, flashCookie)

180
tests/integration/pull_merge_test.go
View file

@ -32,6 +32,7 @@ import (
api "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/test"
"code.gitea.io/gitea/modules/translation"
"code.gitea.io/gitea/services/forms"
"code.gitea.io/gitea/services/pull"
files_service "code.gitea.io/gitea/services/repository/files"
webhook_service "code.gitea.io/gitea/services/webhook"
@ -39,7 +40,20 @@ import (
"github.com/stretchr/testify/assert"
)
type optionsPullMerge map[string]string
func testPullMerge(t *testing.T, session *TestSession, user, repo, pullnum string, mergeStyle repo_model.MergeStyle, deleteBranch bool) *httptest.ResponseRecorder {
options := optionsPullMerge{
"do": string(mergeStyle),
}
if deleteBranch {
options["delete_branch_after_merge"] = "on"
}
return testPullMergeForm(t, session, http.StatusOK, user, repo, pullnum, options)
}
func testPullMergeForm(t *testing.T, session *TestSession, expectedCode int, user, repo, pullnum string, addOptions optionsPullMerge) *httptest.ResponseRecorder {
req := NewRequest(t, "GET", path.Join(user, repo, "pulls", pullnum))
resp := session.MakeRequest(t, req, http.StatusOK)
@ -48,22 +62,22 @@ func testPullMerge(t *testing.T, session *TestSession, user, repo, pullnum strin
options := map[string]string{
"_csrf": htmlDoc.GetCSRF(),
"do": string(mergeStyle),
}
if deleteBranch {
options["delete_branch_after_merge"] = "on"
for k, v := range addOptions {
options[k] = v
}
req = NewRequestWithValues(t, "POST", link, options)
resp = session.MakeRequest(t, req, http.StatusOK)
resp = session.MakeRequest(t, req, expectedCode)
respJSON := struct {
Redirect string
}{}
DecodeJSON(t, resp, &respJSON)
if expectedCode == http.StatusOK {
respJSON := struct {
Redirect string
}{}
DecodeJSON(t, resp, &respJSON)
assert.EqualValues(t, fmt.Sprintf("/%s/%s/pulls/%s", user, repo, pullnum), respJSON.Redirect)
assert.EqualValues(t, fmt.Sprintf("/%s/%s/pulls/%s", user, repo, pullnum), respJSON.Redirect)
}
return resp
}
@ -661,3 +675,149 @@ func TestPullMergeIndexerNotifier(t *testing.T) {
}
})
}
func TestPullMergeBranchProtect(t *testing.T) {
onGiteaRun(t, func(t *testing.T, u *url.URL) {
admin := "user1"
owner := "user5"
notOwner := "user4"
repo := "repo4"
dstPath := t.TempDir()
u.Path = fmt.Sprintf("%s/%s.git", owner, repo)
u.User = url.UserPassword(owner, userPassword)
t.Run("Clone", doGitClone(dstPath, u))
for _, testCase := range []struct {
name string
doer string
expectedCode map[string]int
filename string
protectBranch parameterProtectBranch
}{
{
name: "SuccessAdminNotEnoughMergeRequiredApprovals",
doer: admin,
expectedCode: map[string]int{"api": http.StatusOK, "web": http.StatusOK},
filename: "branch-data-file-",
protectBranch: parameterProtectBranch{
"required_approvals": "1",
"apply_to_admins": "true",
},
},
{
name: "FailOwnerProtectedFile",
doer: owner,
expectedCode: map[string]int{"api": http.StatusMethodNotAllowed, "web": http.StatusBadRequest},
filename: "protected-file-",
protectBranch: parameterProtectBranch{
"protected_file_patterns": "protected-file-*",
"apply_to_admins": "true",
},
},
{
name: "OwnerProtectedFile",
doer: owner,
expectedCode: map[string]int{"api": http.StatusOK, "web": http.StatusOK},
filename: "protected-file-",
protectBranch: parameterProtectBranch{
"protected_file_patterns": "protected-file-*",
"apply_to_admins": "false",
},
},
{
name: "FailNotOwnerProtectedFile",
doer: notOwner,
expectedCode: map[string]int{"api": http.StatusMethodNotAllowed, "web": http.StatusBadRequest},
filename: "protected-file-",
protectBranch: parameterProtectBranch{
"protected_file_patterns": "protected-file-*",
},
},
{
name: "FailOwnerNotEnoughMergeRequiredApprovals",
doer: owner,
expectedCode: map[string]int{"api": http.StatusMethodNotAllowed, "web": http.StatusBadRequest},
filename: "branch-data-file-",
protectBranch: parameterProtectBranch{
"required_approvals": "1",
"apply_to_admins": "true",
},
},
{
name: "SuccessOwnerNotEnoughMergeRequiredApprovals",
doer: owner,
expectedCode: map[string]int{"api": http.StatusOK, "web": http.StatusOK},
filename: "branch-data-file-",
protectBranch: parameterProtectBranch{
"required_approvals": "1",
"apply_to_admins": "false",
},
},
{
name: "FailNotOwnerNotEnoughMergeRequiredApprovals",
doer: notOwner,
expectedCode: map[string]int{"api": http.StatusMethodNotAllowed, "web": http.StatusBadRequest},
filename: "branch-data-file-",
protectBranch: parameterProtectBranch{
"required_approvals": "1",
"apply_to_admins": "false",
},
},
{
name: "SuccessNotOwner",
doer: notOwner,
expectedCode: map[string]int{"api": http.StatusOK, "web": http.StatusOK},
filename: "branch-data-file-",
protectBranch: parameterProtectBranch{
"required_approvals": "0",
},
},
} {
mergeWith := func(t *testing.T, ctx APITestContext, apiOrWeb string, expectedCode int, pr int64) {
switch apiOrWeb {
case "api":
ctx.ExpectedCode = expectedCode
doAPIMergePullRequestForm(t, ctx, owner, repo, pr,
&forms.MergePullRequestForm{
MergeMessageField: "doAPIMergePullRequest Merge",
Do: string(repo_model.MergeStyleMerge),
ForceMerge: true,
})
ctx.ExpectedCode = 0
case "web":
testPullMergeForm(t, ctx.Session, expectedCode, owner, repo, fmt.Sprintf("%d", pr), optionsPullMerge{
"do": string(repo_model.MergeStyleMerge),
"force_merge": "true",
})
default:
panic(apiOrWeb)
}
}
for _, withAPIOrWeb := range []string{"api", "web"} {
t.Run(testCase.name+" "+withAPIOrWeb, func(t *testing.T) {
branch := testCase.name + "-" + withAPIOrWeb
unprotected := branch + "-unprotected"
doGitCheckoutBranch(dstPath, "master")(t)
doGitCreateBranch(dstPath, branch)(t)
doGitPushTestRepository(dstPath, "origin", branch)(t)
ctx := NewAPITestContext(t, owner, repo, auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser)
doProtectBranch(ctx, branch, testCase.protectBranch)(t)
ctx = NewAPITestContext(t, testCase.doer, "not used", auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser)
ctx.Username = owner
ctx.Reponame = repo
_, err := generateCommitWithNewData(littleSize, dstPath, "user2@example.com", "User Two", testCase.filename)
assert.NoError(t, err)
doGitPushTestRepository(dstPath, "origin", branch+":"+unprotected)(t)
pr, err := doAPICreatePullRequest(ctx, owner, repo, branch, unprotected)(t)
assert.NoError(t, err)
mergeWith(t, ctx, withAPIOrWeb, testCase.expectedCode[withAPIOrWeb], pr.Index)
})
}
}
})
}