diff options
-rw-r--r-- | models/git/protected_branch.go | 124 | ||||
-rw-r--r-- | models/migrations/migrations.go | 2 | ||||
-rw-r--r-- | models/migrations/v1_23/v300.go | 17 | ||||
-rw-r--r-- | modules/structs/repo_branch.go | 15 | ||||
-rw-r--r-- | options/locale/locale_en-US.ini | 36 | ||||
-rw-r--r-- | routers/api/v1/repo/branch.go | 82 | ||||
-rw-r--r-- | routers/private/hook_pre_receive.go | 38 | ||||
-rw-r--r-- | routers/web/repo/setting/protected_branch.go | 27 | ||||
-rw-r--r-- | services/convert/convert.go | 7 | ||||
-rw-r--r-- | services/forms/repo_form.go | 4 | ||||
-rw-r--r-- | services/pull/update.go | 30 | ||||
-rw-r--r-- | templates/repo/settings/protected_branch.tmpl | 63 | ||||
-rw-r--r-- | templates/swagger/v1_json.tmpl | 78 | ||||
-rw-r--r-- | tests/integration/git_test.go | 106 |
14 files changed, 528 insertions, 101 deletions
diff --git a/models/git/protected_branch.go b/models/git/protected_branch.go index e0ff4d1542..bde6057375 100644 --- a/models/git/protected_branch.go +++ b/models/git/protected_branch.go @@ -44,6 +44,11 @@ type ProtectedBranch struct { WhitelistDeployKeys bool `xorm:"NOT NULL DEFAULT false"` MergeWhitelistUserIDs []int64 `xorm:"JSON TEXT"` MergeWhitelistTeamIDs []int64 `xorm:"JSON TEXT"` + CanForcePush bool `xorm:"NOT NULL DEFAULT false"` + EnableForcePushAllowlist bool `xorm:"NOT NULL DEFAULT false"` + ForcePushAllowlistUserIDs []int64 `xorm:"JSON TEXT"` + ForcePushAllowlistTeamIDs []int64 `xorm:"JSON TEXT"` + ForcePushAllowlistDeployKeys bool `xorm:"NOT NULL DEFAULT false"` EnableStatusCheck bool `xorm:"NOT NULL DEFAULT false"` StatusCheckContexts []string `xorm:"JSON TEXT"` EnableApprovalsWhitelist bool `xorm:"NOT NULL DEFAULT false"` @@ -143,6 +148,33 @@ func (protectBranch *ProtectedBranch) CanUserPush(ctx context.Context, user *use return in } +// CanUserForcePush returns if some user could force push to this protected branch +// Since force-push extends normal push, we also check if user has regular push access +func (protectBranch *ProtectedBranch) CanUserForcePush(ctx context.Context, user *user_model.User) bool { + if !protectBranch.CanForcePush { + return false + } + + if !protectBranch.EnableForcePushAllowlist { + return protectBranch.CanUserPush(ctx, user) + } + + if slices.Contains(protectBranch.ForcePushAllowlistUserIDs, user.ID) { + return protectBranch.CanUserPush(ctx, user) + } + + if len(protectBranch.ForcePushAllowlistTeamIDs) == 0 { + return false + } + + in, err := organization.IsUserInTeams(ctx, user.ID, protectBranch.ForcePushAllowlistTeamIDs) + if err != nil { + log.Error("IsUserInTeams: %v", err) + return false + } + return in && protectBranch.CanUserPush(ctx, user) +} + // IsUserMergeWhitelisted checks if some user is whitelisted to merge to this branch func IsUserMergeWhitelisted(ctx context.Context, protectBranch *ProtectedBranch, userID int64, permissionInRepo access_model.Permission) bool { if !protectBranch.EnableMergeWhitelist { @@ -301,6 +333,9 @@ type WhitelistOptions struct { UserIDs []int64 TeamIDs []int64 + ForcePushUserIDs []int64 + ForcePushTeamIDs []int64 + MergeUserIDs []int64 MergeTeamIDs []int64 @@ -328,6 +363,12 @@ func UpdateProtectBranch(ctx context.Context, repo *repo_model.Repository, prote } protectBranch.WhitelistUserIDs = whitelist + whitelist, err = updateUserWhitelist(ctx, repo, protectBranch.ForcePushAllowlistUserIDs, opts.ForcePushUserIDs) + if err != nil { + return err + } + protectBranch.ForcePushAllowlistUserIDs = whitelist + whitelist, err = updateUserWhitelist(ctx, repo, protectBranch.MergeWhitelistUserIDs, opts.MergeUserIDs) if err != nil { return err @@ -347,6 +388,12 @@ func UpdateProtectBranch(ctx context.Context, repo *repo_model.Repository, prote } protectBranch.WhitelistTeamIDs = whitelist + whitelist, err = updateTeamWhitelist(ctx, repo, protectBranch.ForcePushAllowlistTeamIDs, opts.ForcePushTeamIDs) + if err != nil { + return err + } + protectBranch.ForcePushAllowlistTeamIDs = whitelist + whitelist, err = updateTeamWhitelist(ctx, repo, protectBranch.MergeWhitelistTeamIDs, opts.MergeTeamIDs) if err != nil { return err @@ -468,43 +515,58 @@ func DeleteProtectedBranch(ctx context.Context, repo *repo_model.Repository, id return nil } -// RemoveUserIDFromProtectedBranch remove all user ids from protected branch options -func RemoveUserIDFromProtectedBranch(ctx context.Context, p *ProtectedBranch, userID int64) error { - lenIDs, lenApprovalIDs, lenMergeIDs := len(p.WhitelistUserIDs), len(p.ApprovalsWhitelistUserIDs), len(p.MergeWhitelistUserIDs) - p.WhitelistUserIDs = util.SliceRemoveAll(p.WhitelistUserIDs, userID) - p.ApprovalsWhitelistUserIDs = util.SliceRemoveAll(p.ApprovalsWhitelistUserIDs, userID) - p.MergeWhitelistUserIDs = util.SliceRemoveAll(p.MergeWhitelistUserIDs, userID) - - if lenIDs != len(p.WhitelistUserIDs) || lenApprovalIDs != len(p.ApprovalsWhitelistUserIDs) || - lenMergeIDs != len(p.MergeWhitelistUserIDs) { - if _, err := db.GetEngine(ctx).ID(p.ID).Cols( - "whitelist_user_i_ds", - "merge_whitelist_user_i_ds", - "approvals_whitelist_user_i_ds", - ).Update(p); err != nil { +// removeIDsFromProtectedBranch is a helper function to remove IDs from protected branch options +func removeIDsFromProtectedBranch(ctx context.Context, p *ProtectedBranch, userID, teamID int64, columnNames []string) error { + lenUserIDs, lenForcePushIDs, lenApprovalIDs, lenMergeIDs := len(p.WhitelistUserIDs), len(p.ForcePushAllowlistUserIDs), len(p.ApprovalsWhitelistUserIDs), len(p.MergeWhitelistUserIDs) + lenTeamIDs, lenForcePushTeamIDs, lenApprovalTeamIDs, lenMergeTeamIDs := len(p.WhitelistTeamIDs), len(p.ForcePushAllowlistTeamIDs), len(p.ApprovalsWhitelistTeamIDs), len(p.MergeWhitelistTeamIDs) + + if userID > 0 { + p.WhitelistUserIDs = util.SliceRemoveAll(p.WhitelistUserIDs, userID) + p.ForcePushAllowlistUserIDs = util.SliceRemoveAll(p.ForcePushAllowlistUserIDs, userID) + p.ApprovalsWhitelistUserIDs = util.SliceRemoveAll(p.ApprovalsWhitelistUserIDs, userID) + p.MergeWhitelistUserIDs = util.SliceRemoveAll(p.MergeWhitelistUserIDs, userID) + } + + if teamID > 0 { + p.WhitelistTeamIDs = util.SliceRemoveAll(p.WhitelistTeamIDs, teamID) + p.ForcePushAllowlistTeamIDs = util.SliceRemoveAll(p.ForcePushAllowlistTeamIDs, teamID) + p.ApprovalsWhitelistTeamIDs = util.SliceRemoveAll(p.ApprovalsWhitelistTeamIDs, teamID) + p.MergeWhitelistTeamIDs = util.SliceRemoveAll(p.MergeWhitelistTeamIDs, teamID) + } + + if (lenUserIDs != len(p.WhitelistUserIDs) || + lenForcePushIDs != len(p.ForcePushAllowlistUserIDs) || + lenApprovalIDs != len(p.ApprovalsWhitelistUserIDs) || + lenMergeIDs != len(p.MergeWhitelistUserIDs)) || + (lenTeamIDs != len(p.WhitelistTeamIDs) || + lenForcePushTeamIDs != len(p.ForcePushAllowlistTeamIDs) || + lenApprovalTeamIDs != len(p.ApprovalsWhitelistTeamIDs) || + lenMergeTeamIDs != len(p.MergeWhitelistTeamIDs)) { + if _, err := db.GetEngine(ctx).ID(p.ID).Cols(columnNames...).Update(p); err != nil { return fmt.Errorf("updateProtectedBranches: %v", err) } } return nil } -// RemoveTeamIDFromProtectedBranch remove all team ids from protected branch options +// RemoveUserIDFromProtectedBranch removes all user ids from protected branch options +func RemoveUserIDFromProtectedBranch(ctx context.Context, p *ProtectedBranch, userID int64) error { + columnNames := []string{ + "whitelist_user_i_ds", + "force_push_allowlist_user_i_ds", + "merge_whitelist_user_i_ds", + "approvals_whitelist_user_i_ds", + } + return removeIDsFromProtectedBranch(ctx, p, userID, 0, columnNames) +} + +// RemoveTeamIDFromProtectedBranch removes all team ids from protected branch options func RemoveTeamIDFromProtectedBranch(ctx context.Context, p *ProtectedBranch, teamID int64) error { - lenIDs, lenApprovalIDs, lenMergeIDs := len(p.WhitelistTeamIDs), len(p.ApprovalsWhitelistTeamIDs), len(p.MergeWhitelistTeamIDs) - p.WhitelistTeamIDs = util.SliceRemoveAll(p.WhitelistTeamIDs, teamID) - p.ApprovalsWhitelistTeamIDs = util.SliceRemoveAll(p.ApprovalsWhitelistTeamIDs, teamID) - p.MergeWhitelistTeamIDs = util.SliceRemoveAll(p.MergeWhitelistTeamIDs, teamID) - - if lenIDs != len(p.WhitelistTeamIDs) || - lenApprovalIDs != len(p.ApprovalsWhitelistTeamIDs) || - lenMergeIDs != len(p.MergeWhitelistTeamIDs) { - if _, err := db.GetEngine(ctx).ID(p.ID).Cols( - "whitelist_team_i_ds", - "merge_whitelist_team_i_ds", - "approvals_whitelist_team_i_ds", - ).Update(p); err != nil { - return fmt.Errorf("updateProtectedBranches: %v", err) - } + columnNames := []string{ + "whitelist_team_i_ds", + "force_push_allowlist_team_i_ds", + "merge_whitelist_team_i_ds", + "approvals_whitelist_team_i_ds", } - return nil + return removeIDsFromProtectedBranch(ctx, p, 0, teamID, columnNames) } diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 08882fb119..88faa6ba8b 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -591,6 +591,8 @@ var migrations = []Migration{ // v299 -> v300 NewMigration("Add content version to issue and comment table", v1_23.AddContentVersionToIssueAndComment), + // v300 -> v301 + NewMigration("Add force-push branch protection support", v1_23.AddForcePushBranchProtection), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v1_23/v300.go b/models/migrations/v1_23/v300.go new file mode 100644 index 0000000000..f1f1cccdbf --- /dev/null +++ b/models/migrations/v1_23/v300.go @@ -0,0 +1,17 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v1_23 //nolint + +import "xorm.io/xorm" + +func AddForcePushBranchProtection(x *xorm.Engine) error { + type ProtectedBranch struct { + CanForcePush bool `xorm:"NOT NULL DEFAULT false"` + EnableForcePushAllowlist bool `xorm:"NOT NULL DEFAULT false"` + ForcePushAllowlistUserIDs []int64 `xorm:"JSON TEXT"` + ForcePushAllowlistTeamIDs []int64 `xorm:"JSON TEXT"` + ForcePushAllowlistDeployKeys bool `xorm:"NOT NULL DEFAULT false"` + } + return x.Sync(new(ProtectedBranch)) +} diff --git a/modules/structs/repo_branch.go b/modules/structs/repo_branch.go index e96d276b29..0f2cf482fd 100644 --- a/modules/structs/repo_branch.go +++ b/modules/structs/repo_branch.go @@ -30,6 +30,11 @@ type BranchProtection struct { PushWhitelistUsernames []string `json:"push_whitelist_usernames"` PushWhitelistTeams []string `json:"push_whitelist_teams"` PushWhitelistDeployKeys bool `json:"push_whitelist_deploy_keys"` + EnableForcePush bool `json:"enable_force_push"` + EnableForcePushAllowlist bool `json:"enable_force_push_allowlist"` + ForcePushAllowlistUsernames []string `json:"force_push_allowlist_usernames"` + ForcePushAllowlistTeams []string `json:"force_push_allowlist_teams"` + ForcePushAllowlistDeployKeys bool `json:"force_push_allowlist_deploy_keys"` EnableMergeWhitelist bool `json:"enable_merge_whitelist"` MergeWhitelistUsernames []string `json:"merge_whitelist_usernames"` MergeWhitelistTeams []string `json:"merge_whitelist_teams"` @@ -63,6 +68,11 @@ type CreateBranchProtectionOption struct { PushWhitelistUsernames []string `json:"push_whitelist_usernames"` PushWhitelistTeams []string `json:"push_whitelist_teams"` PushWhitelistDeployKeys bool `json:"push_whitelist_deploy_keys"` + EnableForcePush bool `json:"enable_force_push"` + EnableForcePushAllowlist bool `json:"enable_force_push_allowlist"` + ForcePushAllowlistUsernames []string `json:"force_push_allowlist_usernames"` + ForcePushAllowlistTeams []string `json:"force_push_allowlist_teams"` + ForcePushAllowlistDeployKeys bool `json:"force_push_allowlist_deploy_keys"` EnableMergeWhitelist bool `json:"enable_merge_whitelist"` MergeWhitelistUsernames []string `json:"merge_whitelist_usernames"` MergeWhitelistTeams []string `json:"merge_whitelist_teams"` @@ -89,6 +99,11 @@ type EditBranchProtectionOption struct { PushWhitelistUsernames []string `json:"push_whitelist_usernames"` PushWhitelistTeams []string `json:"push_whitelist_teams"` PushWhitelistDeployKeys *bool `json:"push_whitelist_deploy_keys"` + EnableForcePush *bool `json:"enable_force_push"` + EnableForcePushAllowlist *bool `json:"enable_force_push_allowlist"` + ForcePushAllowlistUsernames []string `json:"force_push_allowlist_usernames"` + ForcePushAllowlistTeams []string `json:"force_push_allowlist_teams"` + ForcePushAllowlistDeployKeys *bool `json:"force_push_allowlist_deploy_keys"` EnableMergeWhitelist *bool `json:"enable_merge_whitelist"` MergeWhitelistUsernames []string `json:"merge_whitelist_usernames"` MergeWhitelistTeams []string `json:"merge_whitelist_teams"` diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 65ce6c6bc2..e0973fe874 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -2280,6 +2280,7 @@ settings.event_wiki_desc = Wiki page created, renamed, edited or deleted. settings.event_release = Release settings.event_release_desc = Release published, updated or deleted in a repository. settings.event_push = Push +settings.event_force_push = Force Push settings.event_push_desc = Git push to a repository. settings.event_repository = Repository settings.event_repository_desc = Repository created or deleted. @@ -2373,19 +2374,28 @@ settings.protect_this_branch = Enable Branch Protection settings.protect_this_branch_desc = Prevents deletion and restricts Git pushing and merging to the branch. settings.protect_disable_push = Disable Push settings.protect_disable_push_desc = No pushing will be allowed to this branch. +settings.protect_disable_force_push = Disable Force Push +settings.protect_disable_force_push_desc = No force pushing will be allowed to this branch. settings.protect_enable_push = Enable Push settings.protect_enable_push_desc = Anyone with write access will be allowed to push to this branch (but not force push). +settings.protect_enable_force_push_all = Enable Force Push +settings.protect_enable_force_push_all_desc = Anyone with push access will be allowed to force push to this branch. +settings.protect_enable_force_push_allowlist = Allowlist Restricted Force Push +settings.protect_enable_force_push_allowlist_desc = Only allowlisted users or teams with push access will be allowed to force push to this branch. settings.protect_enable_merge = Enable Merge settings.protect_enable_merge_desc = Anyone with write access will be allowed to merge the pull requests into this branch. -settings.protect_whitelist_committers = Whitelist Restricted Push -settings.protect_whitelist_committers_desc = Only whitelisted users or teams will be allowed to push to this branch (but not force push). -settings.protect_whitelist_deploy_keys = Whitelist deploy keys with write access to push. -settings.protect_whitelist_users = Whitelisted users for pushing: -settings.protect_whitelist_teams = Whitelisted teams for pushing: -settings.protect_merge_whitelist_committers = Enable Merge Whitelist -settings.protect_merge_whitelist_committers_desc = Allow only whitelisted users or teams to merge pull requests into this branch. -settings.protect_merge_whitelist_users = Whitelisted users for merging: -settings.protect_merge_whitelist_teams = Whitelisted teams for merging: +settings.protect_whitelist_committers = Allowlist Restricted Push +settings.protect_whitelist_committers_desc = Only allowlisted users or teams will be allowed to push to this branch (but not force push). +settings.protect_whitelist_deploy_keys = Allowlist deploy keys with write access to push. +settings.protect_whitelist_users = Allowlisted users for pushing: +settings.protect_whitelist_teams = Allowlisted teams for pushing: +settings.protect_force_push_allowlist_users = Allowlisted users for force pushing: +settings.protect_force_push_allowlist_teams = Allowlisted teams for force pushing: +settings.protect_force_push_allowlist_deploy_keys = Allowlist deploy keys with push access to force push. +settings.protect_merge_whitelist_committers = Enable Merge Allowlist +settings.protect_merge_whitelist_committers_desc = Allow only allowlisted users or teams to merge pull requests into this branch. +settings.protect_merge_whitelist_users = Allowlisted users for merging: +settings.protect_merge_whitelist_teams = Allowlisted teams for merging: settings.protect_check_status_contexts = Enable Status Check settings.protect_status_check_patterns = Status check patterns: settings.protect_status_check_patterns_desc = Enter patterns to specify which status checks must pass before branches can be merged into a branch that matches this rule. Each line specifies a pattern. Patterns cannot be empty. @@ -2396,10 +2406,10 @@ settings.protect_invalid_status_check_pattern = Invalid status check pattern: "% settings.protect_no_valid_status_check_patterns = No valid status check patterns. settings.protect_required_approvals = Required approvals: settings.protect_required_approvals_desc = Allow only to merge pull request with enough positive reviews. -settings.protect_approvals_whitelist_enabled = Restrict approvals to whitelisted users or teams -settings.protect_approvals_whitelist_enabled_desc = Only reviews from whitelisted users or teams will count to the required approvals. Without approval whitelist, reviews from anyone with write access count to the required approvals. -settings.protect_approvals_whitelist_users = Whitelisted reviewers: -settings.protect_approvals_whitelist_teams = Whitelisted teams for reviews: +settings.protect_approvals_whitelist_enabled = Restrict approvals to allowlisted users or teams +settings.protect_approvals_whitelist_enabled_desc = Only reviews from allowlisted users or teams will count to the required approvals. Without approval allowlist, reviews from anyone with write access count to the required approvals. +settings.protect_approvals_whitelist_users = Allowlisted reviewers: +settings.protect_approvals_whitelist_teams = Allowlisted teams for reviews: settings.dismiss_stale_approvals = Dismiss stale approvals settings.dismiss_stale_approvals_desc = When new commits that change the content of the pull request are pushed to the branch, old approvals will be dismissed. settings.ignore_stale_approvals = Ignore stale approvals diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go index 652fcd9441..63de4b8b6a 100644 --- a/routers/api/v1/repo/branch.go +++ b/routers/api/v1/repo/branch.go @@ -553,6 +553,15 @@ func CreateBranchProtection(ctx *context.APIContext) { ctx.Error(http.StatusInternalServerError, "GetUserIDsByNames", err) return } + forcePushAllowlistUsers, err := user_model.GetUserIDsByNames(ctx, form.ForcePushAllowlistUsernames, false) + if err != nil { + if user_model.IsErrUserNotExist(err) { + ctx.Error(http.StatusUnprocessableEntity, "User does not exist", err) + return + } + ctx.Error(http.StatusInternalServerError, "GetUserIDsByNames", err) + return + } mergeWhitelistUsers, err := user_model.GetUserIDsByNames(ctx, form.MergeWhitelistUsernames, false) if err != nil { if user_model.IsErrUserNotExist(err) { @@ -571,7 +580,7 @@ func CreateBranchProtection(ctx *context.APIContext) { ctx.Error(http.StatusInternalServerError, "GetUserIDsByNames", err) return } - var whitelistTeams, mergeWhitelistTeams, approvalsWhitelistTeams []int64 + var whitelistTeams, forcePushAllowlistTeams, mergeWhitelistTeams, approvalsWhitelistTeams []int64 if repo.Owner.IsOrganization() { whitelistTeams, err = organization.GetTeamIDsByNames(ctx, repo.OwnerID, form.PushWhitelistTeams, false) if err != nil { @@ -582,6 +591,15 @@ func CreateBranchProtection(ctx *context.APIContext) { ctx.Error(http.StatusInternalServerError, "GetTeamIDsByNames", err) return } + forcePushAllowlistTeams, err = organization.GetTeamIDsByNames(ctx, repo.OwnerID, form.ForcePushAllowlistTeams, false) + if err != nil { + if organization.IsErrTeamNotExist(err) { + ctx.Error(http.StatusUnprocessableEntity, "Team does not exist", err) + return + } + ctx.Error(http.StatusInternalServerError, "GetTeamIDsByNames", err) + return + } mergeWhitelistTeams, err = organization.GetTeamIDsByNames(ctx, repo.OwnerID, form.MergeWhitelistTeams, false) if err != nil { if organization.IsErrTeamNotExist(err) { @@ -607,8 +625,11 @@ func CreateBranchProtection(ctx *context.APIContext) { RuleName: ruleName, CanPush: form.EnablePush, EnableWhitelist: form.EnablePush && form.EnablePushWhitelist, - EnableMergeWhitelist: form.EnableMergeWhitelist, WhitelistDeployKeys: form.EnablePush && form.EnablePushWhitelist && form.PushWhitelistDeployKeys, + CanForcePush: form.EnablePush && form.EnableForcePush, + EnableForcePushAllowlist: form.EnablePush && form.EnableForcePush && form.EnableForcePushAllowlist, + ForcePushAllowlistDeployKeys: form.EnablePush && form.EnableForcePush && form.EnableForcePushAllowlist && form.ForcePushAllowlistDeployKeys, + EnableMergeWhitelist: form.EnableMergeWhitelist, EnableStatusCheck: form.EnableStatusCheck, StatusCheckContexts: form.StatusCheckContexts, EnableApprovalsWhitelist: form.EnableApprovalsWhitelist, @@ -626,6 +647,8 @@ func CreateBranchProtection(ctx *context.APIContext) { err = git_model.UpdateProtectBranch(ctx, ctx.Repo.Repository, protectBranch, git_model.WhitelistOptions{ UserIDs: whitelistUsers, TeamIDs: whitelistTeams, + ForcePushUserIDs: forcePushAllowlistUsers, + ForcePushTeamIDs: forcePushAllowlistTeams, MergeUserIDs: mergeWhitelistUsers, MergeTeamIDs: mergeWhitelistTeams, ApprovalsUserIDs: approvalsWhitelistUsers, @@ -756,6 +779,27 @@ func EditBranchProtection(ctx *context.APIContext) { } } + if form.EnableForcePush != nil { + if !*form.EnableForcePush { + protectBranch.CanForcePush = false + protectBranch.EnableForcePushAllowlist = false + protectBranch.ForcePushAllowlistDeployKeys = false + } else { + protectBranch.CanForcePush = true + if form.EnableForcePushAllowlist != nil { + if !*form.EnableForcePushAllowlist { + protectBranch.EnableForcePushAllowlist = false + protectBranch.ForcePushAllowlistDeployKeys = false + } else { + protectBranch.EnableForcePushAllowlist = true + if form.ForcePushAllowlistDeployKeys != nil { + protectBranch.ForcePushAllowlistDeployKeys = *form.ForcePushAllowlistDeployKeys + } + } + } + } + } + if form.EnableMergeWhitelist != nil { protectBranch.EnableMergeWhitelist = *form.EnableMergeWhitelist } @@ -808,7 +852,7 @@ func EditBranchProtection(ctx *context.APIContext) { protectBranch.BlockOnOutdatedBranch = *form.BlockOnOutdatedBranch } - var whitelistUsers []int64 + var whitelistUsers, forcePushAllowlistUsers, mergeWhitelistUsers, approvalsWhitelistUsers []int64 if form.PushWhitelistUsernames != nil { whitelistUsers, err = user_model.GetUserIDsByNames(ctx, form.PushWhitelistUsernames, false) if err != nil { @@ -822,7 +866,19 @@ func EditBranchProtection(ctx *context.APIContext) { } else { whitelistUsers = protectBranch.WhitelistUserIDs } - var mergeWhitelistUsers []int64 + if form.ForcePushAllowlistDeployKeys != nil { + forcePushAllowlistUsers, err = user_model.GetUserIDsByNames(ctx, form.ForcePushAllowlistUsernames, false) + if err != nil { + if user_model.IsErrUserNotExist(err) { + ctx.Error(http.StatusUnprocessableEntity, "User does not exist", err) + return + } + ctx.Error(http.StatusInternalServerError, "GetUserIDsByNames", err) + return + } + } else { + forcePushAllowlistUsers = protectBranch.ForcePushAllowlistUserIDs + } if form.MergeWhitelistUsernames != nil { mergeWhitelistUsers, err = user_model.GetUserIDsByNames(ctx, form.MergeWhitelistUsernames, false) if err != nil { @@ -836,7 +892,6 @@ func EditBranchProtection(ctx *context.APIContext) { } else { mergeWhitelistUsers = protectBranch.MergeWhitelistUserIDs } - var approvalsWhitelistUsers []int64 if form.ApprovalsWhitelistUsernames != nil { approvalsWhitelistUsers, err = user_model.GetUserIDsByNames(ctx, form.ApprovalsWhitelistUsernames, false) if err != nil { @@ -851,7 +906,7 @@ func EditBranchProtection(ctx *context.APIContext) { approvalsWhitelistUsers = protectBranch.ApprovalsWhitelistUserIDs } - var whitelistTeams, mergeWhitelistTeams, approvalsWhitelistTeams []int64 + var whitelistTeams, forcePushAllowlistTeams, mergeWhitelistTeams, approvalsWhitelistTeams []int64 if repo.Owner.IsOrganization() { if form.PushWhitelistTeams != nil { whitelistTeams, err = organization.GetTeamIDsByNames(ctx, repo.OwnerID, form.PushWhitelistTeams, false) @@ -866,6 +921,19 @@ func EditBranchProtection(ctx *context.APIContext) { } else { whitelistTeams = protectBranch.WhitelistTeamIDs } + if form.ForcePushAllowlistTeams != nil { + forcePushAllowlistTeams, err = organization.GetTeamIDsByNames(ctx, repo.OwnerID, form.ForcePushAllowlistTeams, false) + if err != nil { + if organization.IsErrTeamNotExist(err) { + ctx.Error(http.StatusUnprocessableEntity, "Team does not exist", err) + return + } + ctx.Error(http.StatusInternalServerError, "GetTeamIDsByNames", err) + return + } + } else { + forcePushAllowlistTeams = protectBranch.ForcePushAllowlistTeamIDs + } if form.MergeWhitelistTeams != nil { mergeWhitelistTeams, err = organization.GetTeamIDsByNames(ctx, repo.OwnerID, form.MergeWhitelistTeams, false) if err != nil { @@ -897,6 +965,8 @@ func EditBranchProtection(ctx *context.APIContext) { err = git_model.UpdateProtectBranch(ctx, ctx.Repo.Repository, protectBranch, git_model.WhitelistOptions{ UserIDs: whitelistUsers, TeamIDs: whitelistTeams, + ForcePushUserIDs: forcePushAllowlistUsers, + ForcePushTeamIDs: forcePushAllowlistTeams, MergeUserIDs: mergeWhitelistUsers, MergeTeamIDs: mergeWhitelistTeams, ApprovalsUserIDs: approvalsWhitelistUsers, diff --git a/routers/private/hook_pre_receive.go b/routers/private/hook_pre_receive.go index 0a3c8e2559..8853875014 100644 --- a/routers/private/hook_pre_receive.go +++ b/routers/private/hook_pre_receive.go @@ -183,6 +183,8 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r return } + isForcePush := false + // 2. Disallow force pushes to protected branches if oldCommitID != objectFormat.EmptyObjectID().String() { output, _, err := git.NewCommand(ctx, "rev-list", "--max-count=1").AddDynamicArguments(oldCommitID, "^"+newCommitID).RunStdString(&git.RunOpts{Dir: repo.RepoPath(), Env: ctx.env}) @@ -193,11 +195,15 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r }) return } else if len(output) > 0 { - log.Warn("Forbidden: Branch: %s in %-v is protected from force push", branchName, repo) - ctx.JSON(http.StatusForbidden, private.Response{ - UserMsg: fmt.Sprintf("branch %s is protected from force push", branchName), - }) - return + if protectBranch.CanForcePush { + isForcePush = true + } else { + log.Warn("Forbidden: Branch: %s in %-v is protected from force push", branchName, repo) + ctx.JSON(http.StatusForbidden, private.Response{ + UserMsg: fmt.Sprintf("branch %s is protected from force push", branchName), + }) + return + } } } @@ -244,10 +250,15 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r } } - // 5. Check if the doer is allowed to push + // 5. Check if the doer is allowed to push (and force-push if the incoming push is a force-push) var canPush bool if ctx.opts.DeployKeyID != 0 { - canPush = !changedProtectedfiles && protectBranch.CanPush && (!protectBranch.EnableWhitelist || protectBranch.WhitelistDeployKeys) + // This flag is only ever true if protectBranch.CanForcePush is true + if isForcePush { + canPush = !changedProtectedfiles && protectBranch.CanPush && (!protectBranch.EnableForcePushAllowlist || protectBranch.ForcePushAllowlistDeployKeys) + } else { + canPush = !changedProtectedfiles && protectBranch.CanPush && (!protectBranch.EnableWhitelist || protectBranch.WhitelistDeployKeys) + } } else { user, err := user_model.GetUserByID(ctx, ctx.opts.UserID) if err != nil { @@ -257,7 +268,11 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r }) return } - canPush = !changedProtectedfiles && protectBranch.CanUserPush(ctx, user) + if isForcePush { + canPush = !changedProtectedfiles && protectBranch.CanUserForcePush(ctx, user) + } else { + canPush = !changedProtectedfiles && protectBranch.CanUserPush(ctx, user) + } } // 6. If we're not allowed to push directly @@ -293,6 +308,13 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r } // Or we're simply not able to push to this protected branch + if isForcePush { + log.Warn("Forbidden: User %d is not allowed to force-push to protected branch: %s in %-v", ctx.opts.UserID, branchName, repo) + ctx.JSON(http.StatusForbidden, private.Response{ + UserMsg: fmt.Sprintf("Not allowed to force-push to protected branch %s", branchName), + }) + return + } log.Warn("Forbidden: User %d is not allowed to push to protected branch: %s in %-v", ctx.opts.UserID, branchName, repo) ctx.JSON(http.StatusForbidden, private.Response{ UserMsg: fmt.Sprintf("Not allowed to push to protected branch %s", branchName), diff --git a/routers/web/repo/setting/protected_branch.go b/routers/web/repo/setting/protected_branch.go index 400186db67..7d943fd434 100644 --- a/routers/web/repo/setting/protected_branch.go +++ b/routers/web/repo/setting/protected_branch.go @@ -77,6 +77,7 @@ func SettingsProtectedBranch(c *context.Context) { } c.Data["Users"] = users c.Data["whitelist_users"] = strings.Join(base.Int64sToStrings(rule.WhitelistUserIDs), ",") + c.Data["force_push_allowlist_users"] = strings.Join(base.Int64sToStrings(rule.ForcePushAllowlistUserIDs), ",") c.Data["merge_whitelist_users"] = strings.Join(base.Int64sToStrings(rule.MergeWhitelistUserIDs), ",") c.Data["approvals_whitelist_users"] = strings.Join(base.Int64sToStrings(rule.ApprovalsWhitelistUserIDs), ",") c.Data["status_check_contexts"] = strings.Join(rule.StatusCheckContexts, "\n") @@ -91,6 +92,7 @@ func SettingsProtectedBranch(c *context.Context) { } c.Data["Teams"] = teams c.Data["whitelist_teams"] = strings.Join(base.Int64sToStrings(rule.WhitelistTeamIDs), ",") + c.Data["force_push_allowlist_teams"] = strings.Join(base.Int64sToStrings(rule.ForcePushAllowlistTeamIDs), ",") c.Data["merge_whitelist_teams"] = strings.Join(base.Int64sToStrings(rule.MergeWhitelistTeamIDs), ",") c.Data["approvals_whitelist_teams"] = strings.Join(base.Int64sToStrings(rule.ApprovalsWhitelistTeamIDs), ",") } @@ -149,7 +151,7 @@ func SettingsProtectedBranchPost(ctx *context.Context) { } } - var whitelistUsers, whitelistTeams, mergeWhitelistUsers, mergeWhitelistTeams, approvalsWhitelistUsers, approvalsWhitelistTeams []int64 + var whitelistUsers, whitelistTeams, forcePushAllowlistUsers, forcePushAllowlistTeams, mergeWhitelistUsers, mergeWhitelistTeams, approvalsWhitelistUsers, approvalsWhitelistTeams []int64 protectBranch.RuleName = f.RuleName if f.RequiredApprovals < 0 { ctx.Flash.Error(ctx.Tr("repo.settings.protected_branch_required_approvals_min")) @@ -178,6 +180,27 @@ func SettingsProtectedBranchPost(ctx *context.Context) { protectBranch.WhitelistDeployKeys = false } + switch f.EnableForcePush { + case "all": + protectBranch.CanForcePush = true + protectBranch.EnableForcePushAllowlist = false + protectBranch.ForcePushAllowlistDeployKeys = false + case "whitelist": + protectBranch.CanForcePush = true + protectBranch.EnableForcePushAllowlist = true + protectBranch.ForcePushAllowlistDeployKeys = f.ForcePushAllowlistDeployKeys + if strings.TrimSpace(f.ForcePushAllowlistUsers) != "" { + forcePushAllowlistUsers, _ = base.StringsToInt64s(strings.Split(f.ForcePushAllowlistUsers, ",")) + } + if strings.TrimSpace(f.ForcePushAllowlistTeams) != "" { + forcePushAllowlistTeams, _ = base.StringsToInt64s(strings.Split(f.ForcePushAllowlistTeams, ",")) + } + default: + protectBranch.CanForcePush = false + protectBranch.EnableForcePushAllowlist = false + protectBranch.ForcePushAllowlistDeployKeys = false + } + protectBranch.EnableMergeWhitelist = f.EnableMergeWhitelist if f.EnableMergeWhitelist { if strings.TrimSpace(f.MergeWhitelistUsers) != "" { @@ -237,6 +260,8 @@ func SettingsProtectedBranchPost(ctx *context.Context) { err = git_model.UpdateProtectBranch(ctx, ctx.Repo.Repository, protectBranch, git_model.WhitelistOptions{ UserIDs: whitelistUsers, TeamIDs: whitelistTeams, + ForcePushUserIDs: forcePushAllowlistUsers, + ForcePushTeamIDs: forcePushAllowlistTeams, MergeUserIDs: mergeWhitelistUsers, MergeTeamIDs: mergeWhitelistTeams, ApprovalsUserIDs: approvalsWhitelistUsers, diff --git a/services/convert/convert.go b/services/convert/convert.go index 5db33ad85d..3e3fca28ca 100644 --- a/services/convert/convert.go +++ b/services/convert/convert.go @@ -136,6 +136,7 @@ func ToBranchProtection(ctx context.Context, bp *git_model.ProtectedBranch, repo } pushWhitelistUsernames := getWhitelistEntities(readers, bp.WhitelistUserIDs) + forcePushAllowlistUsernames := getWhitelistEntities(readers, bp.ForcePushAllowlistUserIDs) mergeWhitelistUsernames := getWhitelistEntities(readers, bp.MergeWhitelistUserIDs) approvalsWhitelistUsernames := getWhitelistEntities(readers, bp.ApprovalsWhitelistUserIDs) @@ -145,6 +146,7 @@ func ToBranchProtection(ctx context.Context, bp *git_model.ProtectedBranch, repo } pushWhitelistTeams := getWhitelistEntities(teamReaders, bp.WhitelistTeamIDs) + forcePushAllowlistTeams := getWhitelistEntities(teamReaders, bp.ForcePushAllowlistTeamIDs) mergeWhitelistTeams := getWhitelistEntities(teamReaders, bp.MergeWhitelistTeamIDs) approvalsWhitelistTeams := getWhitelistEntities(teamReaders, bp.ApprovalsWhitelistTeamIDs) @@ -161,6 +163,11 @@ func ToBranchProtection(ctx context.Context, bp *git_model.ProtectedBranch, repo PushWhitelistUsernames: pushWhitelistUsernames, PushWhitelistTeams: pushWhitelistTeams, PushWhitelistDeployKeys: bp.WhitelistDeployKeys, + EnableForcePush: bp.CanForcePush, + EnableForcePushAllowlist: bp.EnableForcePushAllowlist, + ForcePushAllowlistUsernames: forcePushAllowlistUsernames, + ForcePushAllowlistTeams: forcePushAllowlistTeams, + ForcePushAllowlistDeployKeys: bp.ForcePushAllowlistDeployKeys, EnableMergeWhitelist: bp.EnableMergeWhitelist, MergeWhitelistUsernames: mergeWhitelistUsernames, MergeWhitelistTeams: mergeWhitelistTeams, diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index 32d96abf4d..a2c2af3fac 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -195,6 +195,10 @@ type ProtectBranchForm struct { WhitelistUsers string WhitelistTeams string WhitelistDeployKeys bool + EnableForcePush string + ForcePushAllowlistUsers string + ForcePushAllowlistTeams string + ForcePushAllowlistDeployKeys bool EnableMergeWhitelist bool MergeWhitelistUsers string MergeWhitelistTeams string diff --git a/services/pull/update.go b/services/pull/update.go index d2c0c2df80..a7fd81421e 100644 --- a/services/pull/update.go +++ b/services/pull/update.go @@ -117,27 +117,25 @@ func IsUserAllowedToUpdate(ctx context.Context, pull *issues_model.PullRequest, return false, false, err } - // can't do rebase on protected branch because need force push - if pb == nil { - if err := pr.LoadBaseRepo(ctx); err != nil { - return false, false, err - } - prUnit, err := pr.BaseRepo.GetUnit(ctx, unit.TypePullRequests) - if err != nil { - if repo_model.IsErrUnitTypeNotExist(err) { - return false, false, nil - } - log.Error("pr.BaseRepo.GetUnit(unit.TypePullRequests): %v", err) - return false, false, err + if err := pr.LoadBaseRepo(ctx); err != nil { + return false, false, err + } + prUnit, err := pr.BaseRepo.GetUnit(ctx, unit.TypePullRequests) + if err != nil { + if repo_model.IsErrUnitTypeNotExist(err) { + return false, false, nil } - rebaseAllowed = prUnit.PullRequestsConfig().AllowRebaseUpdate + log.Error("pr.BaseRepo.GetUnit(unit.TypePullRequests): %v", err) + return false, false, err } - // Update function need push permission + rebaseAllowed = prUnit.PullRequestsConfig().AllowRebaseUpdate + + // If branch protected, disable rebase unless user is whitelisted to force push (which extends regular push) if pb != nil { pb.Repo = pull.BaseRepo - if !pb.CanUserPush(ctx, user) { - return false, false, nil + if !pb.CanUserForcePush(ctx, user) { + rebaseAllowed = false } } diff --git a/templates/repo/settings/protected_branch.tmpl b/templates/repo/settings/protected_branch.tmpl index fec4d7c8d4..364b5ccc55 100644 --- a/templates/repo/settings/protected_branch.tmpl +++ b/templates/repo/settings/protected_branch.tmpl @@ -94,6 +94,69 @@ <p class="help">{{ctx.Locale.Tr "repo.settings.require_signed_commits_desc"}}</p> </div> </div> + <h5 class="ui dividing header">{{ctx.Locale.Tr "repo.settings.event_force_push"}}</h5> + <div class="field"> + <div class="ui radio checkbox"> + <input type="radio" name="enable_force_push" value="none" class="toggle-target-disabled" data-target="#force_push_allowlist_box" {{if not .Rule.CanForcePush}}checked{{end}}> + <label>{{ctx.Locale.Tr "repo.settings.protect_disable_force_push"}}</label> + <p class="help">{{ctx.Locale.Tr "repo.settings.protect_disable_force_push_desc"}}</p> + </div> + </div> + <div class="field"> + <div class="ui radio checkbox"> + <input type="radio" name="enable_force_push" value="all" class="toggle-target-disabled" data-target="#force_push_allowlist_box" {{if and (.Rule.CanForcePush) (not .Rule.EnableForcePushAllowlist)}}checked{{end}}> + <label>{{ctx.Locale.Tr "repo.settings.protect_enable_force_push_all"}}</label> + <p class="help">{{ctx.Locale.Tr "repo.settings.protect_enable_force_push_all_desc"}}</p> + </div> + </div> + <div class="grouped fields"> + <div class="field"> + <div class="ui radio checkbox"> + <input type="radio" name="enable_force_push" value="whitelist" class="toggle-target-enabled" data-target="#force_push_allowlist_box" {{if and (.Rule.CanForcePush) (.Rule.EnableForcePushAllowlist)}}checked{{end}}> + <label>{{ctx.Locale.Tr "repo.settings.protect_enable_force_push_allowlist"}}</label> + <p class="help">{{ctx.Locale.Tr "repo.settings.protect_enable_force_push_allowlist_desc"}}</p> + </div> + </div> + <div id="force_push_allowlist_box" class="grouped fields {{if not .Rule.EnableForcePushAllowlist}}disabled{{end}}"> + <div class="checkbox-sub-item field"> + <label>{{ctx.Locale.Tr "repo.settings.protect_force_push_allowlist_users"}}</label> + <div class="ui multiple search selection dropdown"> + <input type="hidden" name="force_push_allowlist_users" value="{{.force_push_allowlist_users}}"> + <div class="default text">{{ctx.Locale.Tr "search.user_kind"}}</div> + <div class="menu"> + {{range .Users}} + <div class="item" data-value="{{.ID}}"> + {{ctx.AvatarUtils.Avatar . 28 "mini"}}{{template "repo/search_name" .}} + </div> + {{end}} + </div> + </div> + </div> + {{if .Owner.IsOrganization}} + <div class="checkbox-sub-item field"> + <label>{{ctx.Locale.Tr "repo.settings.protect_force_push_allowlist_teams"}}</label> + <div class="ui multiple search selection dropdown"> + <input type="hidden" name="force_push_allowlist_teams" value="{{.force_push_allowlist_teams}}"> + <div class="default text">{{ctx.Locale.Tr "search.team_kind"}}</div> + <div class="menu"> + {{range .Teams}} + <div class="item" data-value="{{.ID}}"> + {{svg "octicon-people"}} + {{.Name}} + </div> + {{end}} + </div> + </div> + </div> + {{end}} + <div class="checkbox-sub-item field"> + <div class="ui checkbox"> + <input type="checkbox" name="force_push_allowlist_deploy_keys" {{if .Rule.ForcePushAllowlistDeployKeys}}checked{{end}}> + <label>{{ctx.Locale.Tr "repo.settings.protect_force_push_allowlist_deploy_keys"}}</label> + </div> + </div> + </div> + </div> <h5 class="ui dividing header">{{ctx.Locale.Tr "repo.settings.event_pull_request_approvals"}}</h5> <div class="field"> <label>{{ctx.Locale.Tr "repo.settings.protect_required_approvals"}}</label> diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 4aa64c5376..5c46c3c9b8 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -18714,6 +18714,14 @@ "type": "boolean", "x-go-name": "EnableApprovalsWhitelist" }, + "enable_force_push": { + "type": "boolean", + "x-go-name": "EnableForcePush" + }, + "enable_force_push_allowlist": { + "type": "boolean", + "x-go-name": "EnableForcePushAllowlist" + }, "enable_merge_whitelist": { "type": "boolean", "x-go-name": "EnableMergeWhitelist" @@ -18730,6 +18738,24 @@ "type": "boolean", "x-go-name": "EnableStatusCheck" }, + "force_push_allowlist_deploy_keys": { + "type": "boolean", + "x-go-name": "ForcePushAllowlistDeployKeys" + }, + "force_push_allowlist_teams": { + "type": "array", + "items": { + "type": "string" + }, + "x-go-name": "ForcePushAllowlistTeams" + }, + "force_push_allowlist_usernames": { + "type": "array", + "items": { + "type": "string" + }, + "x-go-name": "ForcePushAllowlistUsernames" + }, "ignore_stale_approvals": { "type": "boolean", "x-go-name": "IgnoreStaleApprovals" @@ -19378,6 +19404,14 @@ "type": "boolean", "x-go-name": "EnableApprovalsWhitelist" }, + "enable_force_push": { + "type": "boolean", + "x-go-name": "EnableForcePush" + }, + "enable_force_push_allowlist": { + "type": "boolean", + "x-go-name": "EnableForcePushAllowlist" + }, "enable_merge_whitelist": { "type": "boolean", "x-go-name": "EnableMergeWhitelist" @@ -19394,6 +19428,24 @@ "type": "boolean", "x-go-name": "EnableStatusCheck" }, + "force_push_allowlist_deploy_keys": { + "type": "boolean", + "x-go-name": "ForcePushAllowlistDeployKeys" + }, + "force_push_allowlist_teams": { + "type": "array", + "items": { + "type": "string" + }, + "x-go-name": "ForcePushAllowlistTeams" + }, + "force_push_allowlist_usernames": { + "type": "array", + "items": { + "type": "string" + }, + "x-go-name": "ForcePushAllowlistUsernames" + }, "ignore_stale_approvals": { "type": "boolean", "x-go-name": "IgnoreStaleApprovals" @@ -20562,6 +20614,14 @@ "type": "boolean", "x-go-name": "EnableApprovalsWhitelist" }, + "enable_force_push": { + "type": "boolean", + "x-go-name": "EnableForcePush" + }, + "enable_force_push_allowlist": { + "type": "boolean", + "x-go-name": "EnableForcePushAllowlist" + }, "enable_merge_whitelist": { "type": "boolean", "x-go-name": "EnableMergeWhitelist" @@ -20578,6 +20638,24 @@ "type": "boolean", "x-go-name": "EnableStatusCheck" }, + "force_push_allowlist_deploy_keys": { + "type": "boolean", + "x-go-name": "ForcePushAllowlistDeployKeys" + }, + "force_push_allowlist_teams": { + "type": "array", + "items": { + "type": "string" + }, + "x-go-name": "ForcePushAllowlistTeams" + }, + "force_push_allowlist_usernames": { + "type": "array", + "items": { + "type": "string" + }, + "x-go-name": "ForcePushAllowlistUsernames" + }, "ignore_stale_approvals": { "type": "boolean", "x-go-name": "IgnoreStaleApprovals" diff --git a/tests/integration/git_test.go b/tests/integration/git_test.go index 993a3d6799..ac56cffe5e 100644 --- a/tests/integration/git_test.go +++ b/tests/integration/git_test.go @@ -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") + }) + + // 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, "") }) - t.Run("FailToPushToProtectedBranch", doGitPushTestRepositoryFail(dstPath, "origin", "protected")) + + // 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) + 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) - // 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["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) |