Allow force push to protected branches (#28086)
Fixes #22722 ### Problem Currently, it is not possible to force push to a branch with branch protection rules in place. There are often times where this is necessary (CI workflows/administrative tasks etc). The current workaround is to rename/remove the branch protection, perform the force push, and then reinstate the protections. ### Solution Provide an additional section in the branch protection rules to allow users to specify which users with push access can also force push to the branch. The default value of the rule will be set to `Disabled`, and the UI is intuitive and very similar to the `Push` section. It is worth noting in this implementation that allowing force push does not override regular push access, and both will need to be enabled for a user to force push. This applies to manual force push to a remote, and also in Gitea UI updating a PR by rebase (which requires force push) This modifies the `BranchProtection` API structs to add: - `enable_force_push bool` - `enable_force_push_whitelist bool` - `force_push_whitelist_usernames string[]` - `force_push_whitelist_teams string[]` - `force_push_whitelist_deploy_keys bool` ### Updated Branch Protection UI: <img width="943" alt="image" src="7491899c-d816-45d5-be84-8512abd156bf"> ### Pull Request `Update branch by Rebase` option enabled with source branch `test` being a protected branch:  <img width="1038" alt="image" src="57ead13e-9006-459f-b83c-7079e6f4c654"> --------- Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
This commit is contained in:
Henry Goodman
GitHub
parent
9c00dda33a
commit
12cb1d2998
14 changed files with 531 additions and 104 deletions
|
|
@ -360,12 +360,62 @@ func doBranchProtectPRMerge(baseCtx *APITestContext, dstPath string) func(t *tes
|
|||
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) {
|
||||
|
||||
// Protect branch without any whitelisting
|
||||
t.Run("ProtectBranchNoWhitelist", func(t *testing.T) {
|
||||
doProtectBranch(ctx, "protected", "", "", "")
|
||||
})
|
||||
|
||||
// Try to push without permissions, which should fail
|
||||
t.Run("TryPushWithoutPermissions", func(t *testing.T) {
|
||||
_, err := generateCommitWithNewData(littleSize, dstPath, "user2@example.com", "User Two", "branch-data-file-")
|
||||
assert.NoError(t, err)
|
||||
doGitPushTestRepositoryFail(dstPath, "origin", "protected")
|
||||
})
|
||||
t.Run("FailToPushToProtectedBranch", doGitPushTestRepositoryFail(dstPath, "origin", "protected"))
|
||||
|
||||
// Set up permissions for normal push but not force push
|
||||
t.Run("SetupNormalPushPermissions", func(t *testing.T) {
|
||||
doProtectBranch(ctx, "protected", baseCtx.Username, "", "")
|
||||
})
|
||||
|
||||
// Normal push should work
|
||||
t.Run("NormalPushWithPermissions", func(t *testing.T) {
|
||||
_, err := generateCommitWithNewData(littleSize, dstPath, "user2@example.com", "User Two", "branch-data-file-")
|
||||
assert.NoError(t, err)
|
||||
doGitPushTestRepository(dstPath, "origin", "protected")
|
||||
})
|
||||
|
||||
// Try to force push without force push permissions, which should fail
|
||||
t.Run("ForcePushWithoutForcePermissions", func(t *testing.T) {
|
||||
t.Run("CreateDivergentHistory", func(t *testing.T) {
|
||||
git.NewCommand(git.DefaultContext, "reset", "--hard", "HEAD~1").Run(&git.RunOpts{Dir: dstPath})
|
||||
_, err := generateCommitWithNewData(littleSize, dstPath, "user2@example.com", "User Two", "branch-data-file-new")
|
||||
assert.NoError(t, err)
|
||||
})
|
||||
doGitPushTestRepositoryFail(dstPath, "-f", "origin", "protected")
|
||||
})
|
||||
|
||||
// Set up permissions for force push but not normal push
|
||||
t.Run("SetupForcePushPermissions", func(t *testing.T) {
|
||||
doProtectBranch(ctx, "protected", "", baseCtx.Username, "")
|
||||
})
|
||||
|
||||
// Try to force push without normal push permissions, which should fail
|
||||
t.Run("ForcePushWithoutNormalPermissions", func(t *testing.T) {
|
||||
doGitPushTestRepositoryFail(dstPath, "-f", "origin", "protected")
|
||||
})
|
||||
|
||||
// Set up permissions for normal and force push (both are required to force push)
|
||||
t.Run("SetupNormalAndForcePushPermissions", func(t *testing.T) {
|
||||
doProtectBranch(ctx, "protected", baseCtx.Username, baseCtx.Username, "")
|
||||
})
|
||||
|
||||
// Force push should now work
|
||||
t.Run("ForcePushWithPermissions", func(t *testing.T) {
|
||||
doGitPushTestRepository(dstPath, "-f", "origin", "protected")
|
||||
})
|
||||
|
||||
t.Run("ProtectProtectedBranchNoWhitelist", doProtectBranch(ctx, "protected", "", "", ""))
|
||||
t.Run("PushToUnprotectedBranch", doGitPushTestRepository(dstPath, "origin", "protected:unprotected"))
|
||||
var pr api.PullRequest
|
||||
var err error
|
||||
|
|
@ -387,14 +437,14 @@ func doBranchProtectPRMerge(baseCtx *APITestContext, dstPath string) func(t *tes
|
|||
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("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("ProtectProtectedBranchWhitelist", doProtectBranch(ctx, "protected", baseCtx.Username, ""))
|
||||
t.Run("ProtectProtectedBranchWhitelist", doProtectBranch(ctx, "protected", baseCtx.Username, "", ""))
|
||||
|
||||
t.Run("CheckoutMaster", doGitCheckoutBranch(dstPath, "master"))
|
||||
t.Run("CreateBranchForced", doGitCreateBranch(dstPath, "toforce"))
|
||||
|
|
@ -409,33 +459,37 @@ func doBranchProtectPRMerge(baseCtx *APITestContext, dstPath string) func(t *tes
|
|||
}
|
||||
}
|
||||
|
||||
func doProtectBranch(ctx APITestContext, branch, userToWhitelist, unprotectedFilePatterns string) func(t *testing.T) {
|
||||
func doProtectBranch(ctx APITestContext, branch, userToWhitelistPush, userToWhitelistForcePush, unprotectedFilePatterns string) func(t *testing.T) {
|
||||
// We are going to just use the owner to set the protection.
|
||||
return func(t *testing.T) {
|
||||
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_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,
|
||||
"enable_push": "whitelist",
|
||||
"enable_whitelist": "on",
|
||||
"whitelist_users": strconv.FormatInt(user.ID, 10),
|
||||
"unprotected_file_patterns": unprotectedFilePatterns,
|
||||
})
|
||||
ctx.Session.MakeRequest(t, req, http.StatusSeeOther)
|
||||
formData := map[string]string{
|
||||
"_csrf": csrf,
|
||||
"rule_name": branch,
|
||||
"unprotected_file_patterns": unprotectedFilePatterns,
|
||||
}
|
||||
|
||||
if userToWhitelistPush != "" {
|
||||
user, err := user_model.GetUserByName(db.DefaultContext, userToWhitelistPush)
|
||||
assert.NoError(t, err)
|
||||
formData["whitelist_users"] = strconv.FormatInt(user.ID, 10)
|
||||
formData["enable_push"] = "whitelist"
|
||||
formData["enable_whitelist"] = "on"
|
||||
}
|
||||
|
||||
if userToWhitelistForcePush != "" {
|
||||
user, err := user_model.GetUserByName(db.DefaultContext, userToWhitelistForcePush)
|
||||
assert.NoError(t, err)
|
||||
formData["force_push_allowlist_users"] = strconv.FormatInt(user.ID, 10)
|
||||
formData["enable_force_push"] = "whitelist"
|
||||
formData["enable_force_push_allowlist"] = "on"
|
||||
}
|
||||
|
||||
// Send the request to update branch protection settings
|
||||
req := NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/settings/branches/edit", url.PathEscape(ctx.Username), url.PathEscape(ctx.Reponame)), formData)
|
||||
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)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue