From bad8e72bcd4e044fe3b05b4fc1a9152ad7aac7a2 Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Sat, 1 Jun 2024 10:58:46 +0200 Subject: [PATCH] tests(integration): add TestPullMergeBranchProtect Verify variations of branch protection that are in play when merging a pull request as: * instance admin * repository admin / owner * user with write permissions on the repository In all cases the result is expected to be the same when merging the pull request via: * API * web Although the implementations are different. (cherry picked from commit 793421bf5990d1b28d79ea78d310d8a6e15bf562) Conflicts: tests/integration/pull_merge_test.go trivial context conflict --- .../user5/repo4.git/hooks/post-receive | 7 + .../repo4.git/hooks/post-receive.d/gitea | 2 + .../user5/repo4.git/hooks/pre-receive | 7 + .../user5/repo4.git/hooks/pre-receive.d/gitea | 2 + .../user5/repo4.git/hooks/update | 7 + .../user5/repo4.git/hooks/update.d/gitea | 2 + tests/integration/pull_merge_test.go | 147 ++++++++++++++++++ 7 files changed, 174 insertions(+) create mode 100755 tests/gitea-repositories-meta/user5/repo4.git/hooks/post-receive create mode 100755 tests/gitea-repositories-meta/user5/repo4.git/hooks/post-receive.d/gitea create mode 100755 tests/gitea-repositories-meta/user5/repo4.git/hooks/pre-receive create mode 100755 tests/gitea-repositories-meta/user5/repo4.git/hooks/pre-receive.d/gitea create mode 100755 tests/gitea-repositories-meta/user5/repo4.git/hooks/update create mode 100755 tests/gitea-repositories-meta/user5/repo4.git/hooks/update.d/gitea diff --git a/tests/gitea-repositories-meta/user5/repo4.git/hooks/post-receive b/tests/gitea-repositories-meta/user5/repo4.git/hooks/post-receive new file mode 100755 index 0000000000..4b3d452abc --- /dev/null +++ b/tests/gitea-repositories-meta/user5/repo4.git/hooks/post-receive @@ -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 \ No newline at end of file diff --git a/tests/gitea-repositories-meta/user5/repo4.git/hooks/post-receive.d/gitea b/tests/gitea-repositories-meta/user5/repo4.git/hooks/post-receive.d/gitea new file mode 100755 index 0000000000..43a948da3a --- /dev/null +++ b/tests/gitea-repositories-meta/user5/repo4.git/hooks/post-receive.d/gitea @@ -0,0 +1,2 @@ +#!/usr/bin/env bash +"$GITEA_ROOT/gitea" hook --config="$GITEA_ROOT/$GITEA_CONF" post-receive diff --git a/tests/gitea-repositories-meta/user5/repo4.git/hooks/pre-receive b/tests/gitea-repositories-meta/user5/repo4.git/hooks/pre-receive new file mode 100755 index 0000000000..4127013053 --- /dev/null +++ b/tests/gitea-repositories-meta/user5/repo4.git/hooks/pre-receive @@ -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 \ No newline at end of file diff --git a/tests/gitea-repositories-meta/user5/repo4.git/hooks/pre-receive.d/gitea b/tests/gitea-repositories-meta/user5/repo4.git/hooks/pre-receive.d/gitea new file mode 100755 index 0000000000..49d0940636 --- /dev/null +++ b/tests/gitea-repositories-meta/user5/repo4.git/hooks/pre-receive.d/gitea @@ -0,0 +1,2 @@ +#!/usr/bin/env bash +"$GITEA_ROOT/gitea" hook --config="$GITEA_ROOT/$GITEA_CONF" pre-receive diff --git a/tests/gitea-repositories-meta/user5/repo4.git/hooks/update b/tests/gitea-repositories-meta/user5/repo4.git/hooks/update new file mode 100755 index 0000000000..c186fe4a18 --- /dev/null +++ b/tests/gitea-repositories-meta/user5/repo4.git/hooks/update @@ -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 \ No newline at end of file diff --git a/tests/gitea-repositories-meta/user5/repo4.git/hooks/update.d/gitea b/tests/gitea-repositories-meta/user5/repo4.git/hooks/update.d/gitea new file mode 100755 index 0000000000..38101c2426 --- /dev/null +++ b/tests/gitea-repositories-meta/user5/repo4.git/hooks/update.d/gitea @@ -0,0 +1,2 @@ +#!/usr/bin/env bash +"$GITEA_ROOT/gitea" hook --config="$GITEA_ROOT/$GITEA_CONF" update $1 $2 $3 diff --git a/tests/integration/pull_merge_test.go b/tests/integration/pull_merge_test.go index 0c9e85cd24..49f2317b0e 100644 --- a/tests/integration/pull_merge_test.go +++ b/tests/integration/pull_merge_test.go @@ -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" @@ -674,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) + }) + } + } + }) +}