diff --git a/models/git/protected_branch.go b/models/git/protected_branch.go index 5bc886d7c6f..608355c5124 100644 --- a/models/git/protected_branch.go +++ b/models/git/protected_branch.go @@ -57,6 +57,10 @@ type ProtectedBranchConfig struct { EnableForcePushAllowlist bool `json:"enable_force_push_allowlist,omitzero"` ForcePushAllowlistUserIDs []int64 `json:"force_push_allowlist_user_ids,omitzero"` ForcePushAllowlistTeamIDs []int64 `json:"force_push_allowlist_team_ids,omitzero"` + CanDelete bool `json:"can_delete,omitzero"` + EnableDeletionAllowlist bool `json:"enable_deletion_allowlist,omitzero"` + DeletionAllowlistUserIDs []int64 `json:"deletion_allowlist_user_ids,omitzero"` + DeletionAllowlistTeamIDs []int64 `json:"deletion_allowlist_team_ids,omitzero"` EnableStatusCheck bool `json:"enable_status_check,omitzero"` StatusCheckContexts []string `json:"status_check_contexts,omitzero"` EnableApprovalsWhitelist bool `json:"enable_approvals_whitelist,omitzero"` @@ -180,6 +184,36 @@ func (protectBranch *ProtectedBranch) CanUserForcePush(ctx context.Context, user return in && protectBranch.CanUserPush(ctx, user, permissionInRepo) } +// CanUserDelete returns if some user could delete this protected branch. +// Since deletion can be used to reset a branch, it also requires regular push access. +func (protectBranch *ProtectedBranch) CanUserDelete(ctx context.Context, user *user_model.User, permissionInRepo access_model.Permission) bool { + if user == nil || !permissionInRepo.CanWrite(unit.TypeCode) { + return false + } + if !protectBranch.CanDelete || !protectBranch.CanUserPush(ctx, user, permissionInRepo) { + return false + } + + if !protectBranch.EnableDeletionAllowlist { + return true + } + + if slices.Contains(protectBranch.DeletionAllowlistUserIDs, user.ID) { + return true + } + + if len(protectBranch.DeletionAllowlistTeamIDs) == 0 { + return false + } + + in, err := organization.IsUserInTeams(ctx, user.ID, protectBranch.DeletionAllowlistTeamIDs) + if err != nil { + log.Error("IsUserInTeams: %v", err) + return false + } + return in +} + // 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 { @@ -364,6 +398,9 @@ type WhitelistOptions struct { ForcePushUserIDs []int64 ForcePushTeamIDs []int64 + DeletionUserIDs []int64 + DeletionTeamIDs []int64 + MergeUserIDs []int64 MergeTeamIDs []int64 @@ -400,6 +437,12 @@ func UpdateProtectBranch(ctx context.Context, repo *repo_model.Repository, prote } protectBranch.ForcePushAllowlistUserIDs = whitelist + whitelist, err = updateUserWhitelist(ctx, repo, protectBranch.DeletionAllowlistUserIDs, opts.DeletionUserIDs) + if err != nil { + return err + } + protectBranch.DeletionAllowlistUserIDs = whitelist + whitelist, err = updateUserWhitelist(ctx, repo, protectBranch.MergeWhitelistUserIDs, opts.MergeUserIDs) if err != nil { return err @@ -431,6 +474,12 @@ func UpdateProtectBranch(ctx context.Context, repo *repo_model.Repository, prote } protectBranch.ForcePushAllowlistTeamIDs = whitelist + whitelist, err = updateTeamWhitelist(ctx, repo, protectBranch.DeletionAllowlistTeamIDs, opts.DeletionTeamIDs) + if err != nil { + return err + } + protectBranch.DeletionAllowlistTeamIDs = whitelist + whitelist, err = updateTeamWhitelist(ctx, repo, protectBranch.MergeWhitelistTeamIDs, opts.MergeTeamIDs) if err != nil { return err @@ -597,12 +646,13 @@ func DeleteProtectedBranch(ctx context.Context, repo *repo_model.Repository, id // removeIDsFromProtectedBranch is a helper function to remove IDs from protected branch options func removeIDsFromProtectedBranch(ctx context.Context, p *ProtectedBranch, userID, teamID int64) 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) + lenUserIDs, lenForcePushIDs, lenDeletionIDs, lenApprovalIDs, lenMergeIDs := len(p.WhitelistUserIDs), len(p.ForcePushAllowlistUserIDs), len(p.DeletionAllowlistUserIDs), len(p.ApprovalsWhitelistUserIDs), len(p.MergeWhitelistUserIDs) + lenTeamIDs, lenForcePushTeamIDs, lenDeletionTeamIDs, lenApprovalTeamIDs, lenMergeTeamIDs := len(p.WhitelistTeamIDs), len(p.ForcePushAllowlistTeamIDs), len(p.DeletionAllowlistTeamIDs), len(p.ApprovalsWhitelistTeamIDs), len(p.MergeWhitelistTeamIDs) if userID > 0 { p.WhitelistUserIDs = util.SliceRemoveAll(p.WhitelistUserIDs, userID) p.ForcePushAllowlistUserIDs = util.SliceRemoveAll(p.ForcePushAllowlistUserIDs, userID) + p.DeletionAllowlistUserIDs = util.SliceRemoveAll(p.DeletionAllowlistUserIDs, userID) p.ApprovalsWhitelistUserIDs = util.SliceRemoveAll(p.ApprovalsWhitelistUserIDs, userID) p.MergeWhitelistUserIDs = util.SliceRemoveAll(p.MergeWhitelistUserIDs, userID) } @@ -610,16 +660,19 @@ func removeIDsFromProtectedBranch(ctx context.Context, p *ProtectedBranch, userI if teamID > 0 { p.WhitelistTeamIDs = util.SliceRemoveAll(p.WhitelistTeamIDs, teamID) p.ForcePushAllowlistTeamIDs = util.SliceRemoveAll(p.ForcePushAllowlistTeamIDs, teamID) + p.DeletionAllowlistTeamIDs = util.SliceRemoveAll(p.DeletionAllowlistTeamIDs, teamID) p.ApprovalsWhitelistTeamIDs = util.SliceRemoveAll(p.ApprovalsWhitelistTeamIDs, teamID) p.MergeWhitelistTeamIDs = util.SliceRemoveAll(p.MergeWhitelistTeamIDs, teamID) } if (lenUserIDs != len(p.WhitelistUserIDs) || lenForcePushIDs != len(p.ForcePushAllowlistUserIDs) || + lenDeletionIDs != len(p.DeletionAllowlistUserIDs) || lenApprovalIDs != len(p.ApprovalsWhitelistUserIDs) || lenMergeIDs != len(p.MergeWhitelistUserIDs)) || (lenTeamIDs != len(p.WhitelistTeamIDs) || lenForcePushTeamIDs != len(p.ForcePushAllowlistTeamIDs) || + lenDeletionTeamIDs != len(p.DeletionAllowlistTeamIDs) || lenApprovalTeamIDs != len(p.ApprovalsWhitelistTeamIDs) || lenMergeTeamIDs != len(p.MergeWhitelistTeamIDs)) { if _, err := db.GetEngine(ctx).ID(p.ID).Cols("config").Update(p); err != nil { diff --git a/models/git/protected_branch_test.go b/models/git/protected_branch_test.go index 715f10f0e11..32133860104 100644 --- a/models/git/protected_branch_test.go +++ b/models/git/protected_branch_test.go @@ -7,7 +7,10 @@ import ( "testing" "gitea.dev/models/db" + "gitea.dev/models/perm" + access_model "gitea.dev/models/perm/access" repo_model "gitea.dev/models/repo" + "gitea.dev/models/unit" "gitea.dev/models/unittest" user_model "gitea.dev/models/user" @@ -210,6 +213,8 @@ func TestProtectedBranchConfig(t *testing.T) { pb := &ProtectedBranch{ RepoID: 1, RuleName: "config-test", ProtectedBranchConfig: ProtectedBranchConfig{ + CanDelete: true, EnableDeletionAllowlist: true, + DeletionAllowlistUserIDs: []int64{2, 4, user_model.ActionsUserID, user_model.DeployKeyUserID}, DeletionAllowlistTeamIDs: []int64{1, 2}, CanPush: true, WhitelistUserIDs: []int64{2, 4}, WhitelistTeamIDs: []int64{1, 2}, StatusCheckContexts: []string{}, }, } @@ -222,9 +227,52 @@ func TestProtectedBranchConfig(t *testing.T) { assert.Equal(t, []int64{4}, loaded.WhitelistUserIDs) assert.Equal(t, []int64{2}, loaded.WhitelistTeamIDs) assert.True(t, loaded.CanPush) + assert.True(t, loaded.CanDelete) + assert.True(t, loaded.EnableDeletionAllowlist) + assert.Equal(t, []int64{4, user_model.ActionsUserID, user_model.DeployKeyUserID}, loaded.DeletionAllowlistUserIDs) + assert.Equal(t, []int64{2}, loaded.DeletionAllowlistTeamIDs) loaded.ProtectedBranchConfig = ProtectedBranchConfig{} repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) require.NoError(t, UpdateProtectBranch(ctx, repo, loaded, WhitelistOptions{})) loaded = unittest.AssertExistsAndLoadBean(t, &ProtectedBranch{ID: pb.ID}) assert.Equal(t, ProtectedBranchConfig{}, loaded.ProtectedBranchConfig) } + +func TestProtectedBranchCanUserDelete(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + writePermission := access_model.Permission{} + writePermission.SetUnitsWithDefaultAccessMode([]*repo_model.RepoUnit{{Type: unit.TypeCode}}, perm.AccessModeWrite) + readPermission := access_model.Permission{} + readPermission.SetUnitsWithDefaultAccessMode([]*repo_model.RepoUnit{{Type: unit.TypeCode}}, perm.AccessModeRead) + + for _, user := range []*user_model.User{{ID: 2, Name: "user2"}, user_model.NewActionsUser(), user_model.NewDeployKeyUser()} { + t.Run(user.GetDisplayName(), func(t *testing.T) { + for _, tt := range []struct { + name string + config ProtectedBranchConfig + want bool + }{ + {name: "disabled by default", config: ProtectedBranchConfig{CanPush: true}}, + {name: "push disabled", config: ProtectedBranchConfig{CanDelete: true}}, + {name: "unrestricted", config: ProtectedBranchConfig{CanPush: true, CanDelete: true}, want: true}, + {name: "empty push allowlist", config: ProtectedBranchConfig{CanPush: true, CanDelete: true, EnableWhitelist: true}}, + {name: "not in push allowlist", config: ProtectedBranchConfig{CanPush: true, CanDelete: true, EnableWhitelist: true, WhitelistUserIDs: []int64{1}}}, + {name: "empty deletion allowlist", config: ProtectedBranchConfig{CanPush: true, CanDelete: true, EnableDeletionAllowlist: true}}, + {name: "not in deletion allowlist", config: ProtectedBranchConfig{CanPush: true, CanDelete: true, EnableDeletionAllowlist: true, DeletionAllowlistUserIDs: []int64{1}}}, + {name: "in both allowlists", config: ProtectedBranchConfig{CanPush: true, CanDelete: true, EnableWhitelist: true, WhitelistUserIDs: []int64{user.ID}, EnableDeletionAllowlist: true, DeletionAllowlistUserIDs: []int64{user.ID}}, want: true}, + {name: "deletion allowlist does not grant push", config: ProtectedBranchConfig{CanPush: true, CanDelete: true, EnableWhitelist: true, EnableDeletionAllowlist: true, DeletionAllowlistUserIDs: []int64{user.ID}}}, + {name: "team member", config: ProtectedBranchConfig{CanPush: true, CanDelete: true, EnableDeletionAllowlist: true, DeletionAllowlistTeamIDs: []int64{1}}, want: user.ID == 2}, + {name: "push team member", config: ProtectedBranchConfig{CanPush: true, CanDelete: true, EnableWhitelist: true, WhitelistTeamIDs: []int64{1}}, want: user.ID == 2}, + {name: "not in team", config: ProtectedBranchConfig{CanPush: true, CanDelete: true, EnableDeletionAllowlist: true, DeletionAllowlistTeamIDs: []int64{3}}}, + {name: "user or team", config: ProtectedBranchConfig{CanPush: true, CanDelete: true, EnableDeletionAllowlist: true, DeletionAllowlistUserIDs: []int64{user.ID}, DeletionAllowlistTeamIDs: []int64{3}}, want: true}, + } { + t.Run(tt.name, func(t *testing.T) { + pb := &ProtectedBranch{ProtectedBranchConfig: tt.config} + assert.Equal(t, tt.want, pb.CanUserDelete(t.Context(), user, writePermission)) + assert.False(t, pb.CanUserDelete(t.Context(), user, readPermission)) + assert.False(t, pb.CanUserDelete(t.Context(), nil, writePermission)) + }) + } + }) + } +} diff --git a/modules/structs/repo_branch.go b/modules/structs/repo_branch.go index 2b931c032b9..3585172caff 100644 --- a/modules/structs/repo_branch.go +++ b/modules/structs/repo_branch.go @@ -47,6 +47,10 @@ type BranchProtection struct { ForcePushAllowlistUsernames []string `json:"force_push_allowlist_usernames"` ForcePushAllowlistTeams []string `json:"force_push_allowlist_teams"` ForcePushAllowlistDeployKeys bool `json:"force_push_allowlist_deploy_keys"` + EnableDeletion bool `json:"enable_deletion"` + EnableDeletionAllowlist bool `json:"enable_deletion_allowlist"` + DeletionAllowlistUsernames []string `json:"deletion_allowlist_usernames"` + DeletionAllowlistTeams []string `json:"deletion_allowlist_teams"` EnableMergeWhitelist bool `json:"enable_merge_whitelist"` MergeWhitelistUsernames []string `json:"merge_whitelist_usernames"` MergeWhitelistTeams []string `json:"merge_whitelist_teams"` @@ -91,6 +95,10 @@ type CreateBranchProtectionOption struct { ForcePushAllowlistUsernames []string `json:"force_push_allowlist_usernames"` ForcePushAllowlistTeams []string `json:"force_push_allowlist_teams"` ForcePushAllowlistDeployKeys bool `json:"force_push_allowlist_deploy_keys"` + EnableDeletion bool `json:"enable_deletion"` + EnableDeletionAllowlist bool `json:"enable_deletion_allowlist"` + DeletionAllowlistUsernames []string `json:"deletion_allowlist_usernames"` + DeletionAllowlistTeams []string `json:"deletion_allowlist_teams"` EnableMergeWhitelist bool `json:"enable_merge_whitelist"` MergeWhitelistUsernames []string `json:"merge_whitelist_usernames"` MergeWhitelistTeams []string `json:"merge_whitelist_teams"` @@ -128,6 +136,10 @@ type EditBranchProtectionOption struct { ForcePushAllowlistUsernames []string `json:"force_push_allowlist_usernames"` ForcePushAllowlistTeams []string `json:"force_push_allowlist_teams"` ForcePushAllowlistDeployKeys *bool `json:"force_push_allowlist_deploy_keys"` + EnableDeletion *bool `json:"enable_deletion"` + EnableDeletionAllowlist *bool `json:"enable_deletion_allowlist"` + DeletionAllowlistUsernames []string `json:"deletion_allowlist_usernames"` + DeletionAllowlistTeams []string `json:"deletion_allowlist_teams"` 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.json b/options/locale/locale_en-US.json index 347acdb2b7d..eb0c9888ef2 100644 --- a/options/locale/locale_en-US.json +++ b/options/locale/locale_en-US.json @@ -2299,6 +2299,7 @@ "repo.settings.event_release_desc": "Release published, updated or deleted in a repository.", "repo.settings.event_push": "Push", "repo.settings.event_force_push": "Force Push", + "repo.settings.event_branch_delete": "Branch Deletion", "repo.settings.event_push_desc": "Git push to a repository.", "repo.settings.event_repository": "Repository", "repo.settings.event_repository_desc": "Repository created, deleted, or renamed.", @@ -2410,6 +2411,12 @@ "repo.settings.protect_enable_force_push_all_desc": "Anyone with push access will be allowed to force push to this branch.", "repo.settings.protect_enable_force_push_allowlist": "Allowlist Restricted Force Push", "repo.settings.protect_enable_force_push_allowlist_desc": "Only allowlisted users or teams with push access will be allowed to force push to this branch.", + "repo.settings.protect_disable_deletion": "Disable Branch Deletion", + "repo.settings.protect_disable_deletion_desc": "No branch deletion will be allowed for this branch.", + "repo.settings.protect_enable_deletion_all": "Enable Branch Deletion", + "repo.settings.protect_enable_deletion_all_desc": "Anyone with push access will be allowed to delete this branch.", + "repo.settings.protect_enable_deletion_allowlist": "Allowlist Restricted Branch Deletion", + "repo.settings.protect_enable_deletion_allowlist_desc": "Only allowlisted users or teams with push access will be allowed to delete this branch.", "repo.settings.protect_enable_merge": "Enable Merge", "repo.settings.protect_enable_merge_desc": "Anyone with write access will be allowed to merge the pull requests into this branch.", "repo.settings.protect_whitelist_committers": "Allowlist Restricted Push", @@ -2420,6 +2427,8 @@ "repo.settings.protect_force_push_allowlist_users": "Allowlisted users for force pushing:", "repo.settings.protect_force_push_allowlist_teams": "Allowlisted teams for force pushing:", "repo.settings.protect_force_push_allowlist_deploy_keys": "Allowlist deploy keys with push access to force push.", + "repo.settings.protect_deletion_allowlist_users": "Allowlisted users for deleting:", + "repo.settings.protect_deletion_allowlist_teams": "Allowlisted teams for deleting:", "repo.settings.protect_merge_whitelist_committers": "Enable Merge Allowlist", "repo.settings.protect_merge_whitelist_committers_desc": "Allow only allowlisted users or teams to merge pull requests into this branch.", "repo.settings.protect_merge_whitelist_users": "Allowlisted users for merging:", diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go index f7461df3491..619fb210e0b 100644 --- a/routers/api/v1/repo/branch.go +++ b/routers/api/v1/repo/branch.go @@ -711,6 +711,7 @@ func CreateBranchProtection(ctx *context.APIContext) { whitelistUsers := loadBranchProtectionUserIDs(ctx, form.PushWhitelistUsernames) forcePushAllowlistUsers := loadBranchProtectionUserIDs(ctx, form.ForcePushAllowlistUsernames) + deletionAllowlistUsers := loadBranchProtectionUserIDs(ctx, util.Iif(form.EnablePush && form.EnableDeletion && form.EnableDeletionAllowlist, form.DeletionAllowlistUsernames, nil)) mergeWhitelistUsers := loadBranchProtectionUserIDs(ctx, form.MergeWhitelistUsernames) approvalsWhitelistUsers := loadBranchProtectionUserIDs(ctx, form.ApprovalsWhitelistUsernames) bypassAllowlistUsers := loadBranchProtectionUserIDs(ctx, util.Iif(form.EnableBypassAllowlist, form.BypassAllowlistUsernames, nil)) @@ -718,7 +719,7 @@ func CreateBranchProtection(ctx *context.APIContext) { return } - var whitelistTeams, forcePushAllowlistTeams, mergeWhitelistTeams, approvalsWhitelistTeams, bypassAllowlistTeams []int64 + var whitelistTeams, forcePushAllowlistTeams, deletionAllowlistTeams, mergeWhitelistTeams, approvalsWhitelistTeams, bypassAllowlistTeams []int64 if repo.Owner.IsOrganization() { whitelistTeams, err = organization.GetTeamIDsByNames(ctx, repo.OwnerID, form.PushWhitelistTeams, false) if err != nil { @@ -738,6 +739,17 @@ func CreateBranchProtection(ctx *context.APIContext) { ctx.APIErrorInternal(err) return } + if form.EnablePush && form.EnableDeletion && form.EnableDeletionAllowlist { + deletionAllowlistTeams, err = organization.GetTeamIDsByNames(ctx, repo.OwnerID, form.DeletionAllowlistTeams, false) + if err != nil { + if organization.IsErrTeamNotExist(err) { + ctx.APIError(http.StatusUnprocessableEntity, err.Error()) + return + } + ctx.APIErrorInternal(err) + return + } + } mergeWhitelistTeams, err = organization.GetTeamIDsByNames(ctx, repo.OwnerID, form.MergeWhitelistTeams, false) if err != nil { if organization.IsErrTeamNotExist(err) { @@ -782,6 +794,8 @@ func CreateBranchProtection(ctx *context.APIContext) { EnableWhitelist: form.EnablePush && form.EnablePushWhitelist, CanForcePush: form.EnablePush && form.EnableForcePush, EnableForcePushAllowlist: form.EnablePush && form.EnableForcePush && form.EnableForcePushAllowlist, + CanDelete: form.EnablePush && form.EnableDeletion, + EnableDeletionAllowlist: form.EnablePush && form.EnableDeletion && form.EnableDeletionAllowlist, EnableMergeWhitelist: form.EnableMergeWhitelist, EnableBypassAllowlist: form.EnableBypassAllowlist, EnableStatusCheck: form.EnableStatusCheck, @@ -810,6 +824,8 @@ func CreateBranchProtection(ctx *context.APIContext) { TeamIDs: whitelistTeams, ForcePushUserIDs: forcePushAllowlistUsers, ForcePushTeamIDs: forcePushAllowlistTeams, + DeletionUserIDs: deletionAllowlistUsers, + DeletionTeamIDs: deletionAllowlistTeams, MergeUserIDs: mergeWhitelistUsers, MergeTeamIDs: mergeWhitelistTeams, ApprovalsUserIDs: approvalsWhitelistUsers, @@ -929,6 +945,15 @@ func EditBranchProtection(ctx *context.APIContext) { } } + if form.EnableDeletion != nil { + protectBranch.CanDelete = *form.EnableDeletion + } + if form.EnableDeletionAllowlist != nil { + protectBranch.EnableDeletionAllowlist = *form.EnableDeletionAllowlist + } + protectBranch.CanDelete = protectBranch.CanPush && protectBranch.CanDelete + protectBranch.EnableDeletionAllowlist = protectBranch.CanDelete && protectBranch.EnableDeletionAllowlist + if form.Priority != nil { protectBranch.Priority = *form.Priority } @@ -999,6 +1024,7 @@ func EditBranchProtection(ctx *context.APIContext) { whitelistUsers := loadBranchProtectionUserIDs(ctx, form.PushWhitelistUsernames, protectBranch.WhitelistUserIDs) forcePushAllowlistUsers := loadBranchProtectionUserIDs(ctx, form.ForcePushAllowlistUsernames, protectBranch.ForcePushAllowlistUserIDs) + deletionAllowlistUsers := loadBranchProtectionUserIDs(ctx, form.DeletionAllowlistUsernames, protectBranch.DeletionAllowlistUserIDs) mergeWhitelistUsers := loadBranchProtectionUserIDs(ctx, form.MergeWhitelistUsernames, protectBranch.MergeWhitelistUserIDs) approvalsWhitelistUsers := loadBranchProtectionUserIDs(ctx, form.ApprovalsWhitelistUsernames, protectBranch.ApprovalsWhitelistUserIDs) bypassAllowlistUsers := loadBranchProtectionUserIDs(ctx, form.BypassAllowlistUsernames, protectBranch.BypassAllowlistUserIDs) @@ -1015,7 +1041,7 @@ func EditBranchProtection(ctx *context.APIContext) { whitelistUsers = legacySetDeployKeyAllowlist(whitelistUsers, pushDeployKeys) forcePushAllowlistUsers = legacySetDeployKeyAllowlist(forcePushAllowlistUsers, forcePushDeployKeys) - var whitelistTeams, forcePushAllowlistTeams, mergeWhitelistTeams, approvalsWhitelistTeams, bypassAllowlistTeams []int64 + var whitelistTeams, forcePushAllowlistTeams, deletionAllowlistTeams, mergeWhitelistTeams, approvalsWhitelistTeams, bypassAllowlistTeams []int64 if repo.Owner.IsOrganization() { if form.PushWhitelistTeams != nil { whitelistTeams, err = organization.GetTeamIDsByNames(ctx, repo.OwnerID, form.PushWhitelistTeams, false) @@ -1043,6 +1069,19 @@ func EditBranchProtection(ctx *context.APIContext) { } else { forcePushAllowlistTeams = protectBranch.ForcePushAllowlistTeamIDs } + if form.DeletionAllowlistTeams != nil { + deletionAllowlistTeams, err = organization.GetTeamIDsByNames(ctx, repo.OwnerID, form.DeletionAllowlistTeams, false) + if err != nil { + if organization.IsErrTeamNotExist(err) { + ctx.APIError(http.StatusUnprocessableEntity, err.Error()) + return + } + ctx.APIErrorInternal(err) + return + } + } else { + deletionAllowlistTeams = protectBranch.DeletionAllowlistTeamIDs + } if form.MergeWhitelistTeams != nil { mergeWhitelistTeams, err = organization.GetTeamIDsByNames(ctx, repo.OwnerID, form.MergeWhitelistTeams, false) if err != nil { @@ -1088,11 +1127,18 @@ func EditBranchProtection(ctx *context.APIContext) { bypassAllowlistTeams = nil } + if !protectBranch.EnableDeletionAllowlist { + deletionAllowlistUsers = nil + deletionAllowlistTeams = nil + } + err = git_model.UpdateProtectBranch(ctx, ctx.Repo.Repository, protectBranch, git_model.WhitelistOptions{ UserIDs: whitelistUsers, TeamIDs: whitelistTeams, ForcePushUserIDs: forcePushAllowlistUsers, ForcePushTeamIDs: forcePushAllowlistTeams, + DeletionUserIDs: deletionAllowlistUsers, + DeletionTeamIDs: deletionAllowlistTeams, MergeUserIDs: mergeWhitelistUsers, MergeTeamIDs: mergeWhitelistTeams, ApprovalsUserIDs: approvalsWhitelistUsers, diff --git a/routers/private/hook_pre_receive.go b/routers/private/hook_pre_receive.go index 9d9633957fe..afff02bcac3 100644 --- a/routers/private/hook_pre_receive.go +++ b/routers/private/hook_pre_receive.go @@ -130,15 +130,21 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r repo := ctx.Repo.Repository gitRepo := ctx.Repo.GitRepo objectFormat := ctx.Repo.GetObjectFormat() + isDeletingBranch := newCommitID == objectFormat.EmptyObjectID().String() defaultBranch := repo.DefaultBranch if ctx.opts.IsWiki && repo.DefaultWikiBranch != "" { defaultBranch = repo.DefaultWikiBranch } - if branchName == defaultBranch && newCommitID == objectFormat.EmptyObjectID().String() { + + if isDeletingBranch && branchName == defaultBranch { ctx.PrivateUserErrorf(http.StatusForbidden, "Branch %s is the default branch and cannot be deleted", branchName) return } + if isDeletingBranch && branchName == repo.GetPullRequestTargetBranch(ctx) { + ctx.PrivateUserErrorf(http.StatusForbidden, "Branch %s is protected from deletion", branchName) + return + } protectBranch, err := git_model.GetFirstMatchProtectedBranchRule(ctx, repo.ID, branchName) if err != nil { @@ -156,9 +162,11 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r // // First of all we need to enforce absolutely: // - // 1. Detect and prevent deletion of the branch - if newCommitID == objectFormat.EmptyObjectID().String() { - ctx.PrivateUserErrorf(http.StatusForbidden, "Branch %s is protected from deletion", branchName) + // 1. Enforce branch deletion permissions + if isDeletingBranch { + if !protectBranch.CanUserDelete(ctx, ctx.Doer, ctx.Repo.Permission) { + ctx.PrivateUserErrorf(http.StatusForbidden, "Branch %s is protected from deletion", branchName) + } return } @@ -199,7 +207,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r // Now there are several tests which can be overridden: // // 4. Check protected file patterns - this is overridable from the UI - changedProtectedfiles := false + changedProtectedFiles := false protectedFilePath := "" globs := protectBranch.GetProtectedFilePatterns() @@ -212,7 +220,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r return } - changedProtectedfiles = true + changedProtectedFiles = true protectedFilePath = errFilePathProtected.Path } } @@ -220,9 +228,9 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r // 5. Check if the doer is allowed to push (and force-push if the incoming push is a force-push) var canPush bool if isForcePush { - canPush = !changedProtectedfiles && protectBranch.CanUserForcePush(ctx, ctx.Doer, ctx.Repo.Permission) + canPush = !changedProtectedFiles && protectBranch.CanUserForcePush(ctx, ctx.Doer, ctx.Repo.Permission) } else { - canPush = !changedProtectedfiles && protectBranch.CanUserPush(ctx, ctx.Doer, ctx.Repo.Permission) + canPush = !changedProtectedFiles && protectBranch.CanUserPush(ctx, ctx.Doer, ctx.Repo.Permission) } // 6. If we're not allowed to push directly @@ -232,7 +240,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r // 6a. If we're not merging from the UI/API then there are two ways we got here: // // We are changing a protected file, and we're not allowed to do that - if changedProtectedfiles { + if changedProtectedFiles { ctx.PrivateUserErrorf(http.StatusForbidden, "Branch %s is protected from changing file %s", branchName, protectedFilePath) return } @@ -287,7 +295,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r } // Now if we're not an admin - we can't overwrite protected files so fail now - if changedProtectedfiles { + if changedProtectedFiles { ctx.PrivateUserErrorf(http.StatusForbidden, "Branch %s is protected from changing file %s", branchName, protectedFilePath) return } diff --git a/routers/private/hook_pre_receive_test.go b/routers/private/hook_pre_receive_test.go index 4cfacadf8a0..cb9deb8c435 100644 --- a/routers/private/hook_pre_receive_test.go +++ b/routers/private/hook_pre_receive_test.go @@ -4,12 +4,14 @@ package private import ( + "net/http" "testing" "gitea.dev/models/db" git_model "gitea.dev/models/git" issues_model "gitea.dev/models/issues" repo_model "gitea.dev/models/repo" + "gitea.dev/models/unit" "gitea.dev/models/unittest" user_model "gitea.dev/models/user" "gitea.dev/modules/git" @@ -20,18 +22,27 @@ import ( "github.com/stretchr/testify/require" ) -func TestPreReceiveActionsProtectedBranch(t *testing.T) { +func TestPreReceiveActionsBranch(t *testing.T) { require.NoError(t, unittest.PrepareTestDatabase()) for _, tc := range []struct { - name string - protection git_model.ProtectedBranch - forcePush bool - allowed bool + name string + protection *git_model.ProtectedBranchConfig + forcePush bool + deleteBranch bool + targetBranch bool + defaultBranch bool + allowed bool }{ - {name: "push", protection: git_model.ProtectedBranch{CanPush: true}, allowed: true}, - {name: "push allowlist", protection: git_model.ProtectedBranch{CanPush: true, EnableWhitelist: true}}, - {name: "force push", protection: git_model.ProtectedBranch{CanPush: true, CanForcePush: true}, forcePush: true, allowed: true}, - {name: "force push allowlist", protection: git_model.ProtectedBranch{CanPush: true, CanForcePush: true, EnableForcePushAllowlist: true}, forcePush: true}, + {name: "push", protection: &git_model.ProtectedBranchConfig{CanPush: true}, allowed: true}, + {name: "push allowlist", protection: &git_model.ProtectedBranchConfig{CanPush: true, EnableWhitelist: true}}, + {name: "force push", protection: &git_model.ProtectedBranchConfig{CanPush: true, CanForcePush: true}, forcePush: true, allowed: true}, + {name: "force push allowlist", protection: &git_model.ProtectedBranchConfig{CanPush: true, CanForcePush: true, EnableForcePushAllowlist: true}, forcePush: true}, + {name: "deletion disabled", protection: &git_model.ProtectedBranchConfig{CanPush: true}, deleteBranch: true}, + {name: "deletion allowed", protection: &git_model.ProtectedBranchConfig{CanPush: true, CanDelete: true}, deleteBranch: true, allowed: true}, + {name: "pull request target", protection: &git_model.ProtectedBranchConfig{CanPush: true, CanDelete: true}, deleteBranch: true, targetBranch: true}, + {name: "unprotected deletion", deleteBranch: true, allowed: true}, + {name: "unprotected pull request target", deleteBranch: true, targetBranch: true}, + {name: "default branch", protection: &git_model.ProtectedBranchConfig{CanPush: true, CanDelete: true}, deleteBranch: true, defaultBranch: true}, } { t.Run(tc.name, func(t *testing.T) { mockCtx, resp := contexttest.MockPrivateContext(t, "/") @@ -45,21 +56,34 @@ func TestPreReceiveActionsProtectedBranch(t *testing.T) { doer := user_model.NewActionsUserWithTaskID(53) loadContextDoerPermission(ctx.PrivateContext, doer.ID, doer.ExtDoerData.EncodeToString()) - protection := tc.protection - protection.RepoID = ctx.Repo.Repository.ID - protection.RuleName = "probe" - require.NoError(t, db.Insert(t.Context(), &protection)) - defer func() { - require.NoError(t, git_model.DeleteProtectedBranch(t.Context(), ctx.Repo.Repository, protection.ID)) - }() + if tc.protection != nil { + protection := git_model.ProtectedBranch{ProtectedBranchConfig: *tc.protection} + protection.RepoID = ctx.Repo.Repository.ID + protection.RuleName = "probe" + require.NoError(t, db.Insert(t.Context(), &protection)) + defer func() { + require.NoError(t, git_model.DeleteProtectedBranch(t.Context(), ctx.Repo.Repository, protection.ID)) + }() + } oldCommitID, newCommitID := "205ac761f3326a7ebe416e8673760016450b5cec", "1032bbf17fbc0d9c95bb5418dabe8f8c99278700" if tc.forcePush { oldCommitID, newCommitID = newCommitID, oldCommitID } + if tc.deleteBranch { + newCommitID = ctx.Repo.GetObjectFormat().EmptyObjectID().String() + } + if tc.targetBranch { + ctx.Repo.Repository.Units = append(ctx.Repo.Repository.Units, &repo_model.RepoUnit{Type: unit.TypePullRequests, Config: &repo_model.PullRequestsConfig{DefaultTargetBranch: "probe"}}) + } + if tc.defaultBranch { + ctx.Repo.Repository.DefaultBranch = "probe" + } preReceiveBranch(ctx, oldCommitID, newCommitID, git.RefNameFromBranch("probe")) if tc.allowed { assert.False(t, ctx.Written(), resp.Body.String()) + } else if tc.deleteBranch { + assert.Equal(t, http.StatusForbidden, resp.Code) } else { assert.Contains(t, resp.Body.String(), "Not allowed to") } diff --git a/routers/web/repo/branch.go b/routers/web/repo/branch.go index e2edcc75d65..60893319e11 100644 --- a/routers/web/repo/branch.go +++ b/routers/web/repo/branch.go @@ -50,7 +50,7 @@ func Branches(ctx *context.Context) { kw := ctx.FormString("q") - defaultBranchOptional, branches, branchesCount, err := repo_service.LoadBranches(ctx, ctx.Repo.Repository, ctx.Repo.GitRepo, optional.None[bool](), kw, page, pageSize) + defaultBranchOptional, branches, branchesCount, err := repo_service.LoadBranches(ctx, ctx.Repo.Repository, ctx.Repo.GitRepo, ctx.Doer, ctx.Repo.Permission, optional.None[bool](), kw, page, pageSize) if err != nil { ctx.ServerError("LoadBranches", err) return diff --git a/routers/web/repo/issue_view.go b/routers/web/repo/issue_view.go index f4f57ee1497..6eb14b7f123 100644 --- a/routers/web/repo/issue_view.go +++ b/routers/web/repo/issue_view.go @@ -39,6 +39,7 @@ import ( issue_service "gitea.dev/services/issue" "gitea.dev/services/notifications" pull_service "gitea.dev/services/pull" + repo_service "gitea.dev/services/repository" user_service "gitea.dev/services/user" ) @@ -900,14 +901,15 @@ func (prInfo *pullRequestViewInfo) prepareMergeBox(ctx *context.Context, issue * return } if perm.CanWrite(unit.TypeCode) { - // Check if branch is not protected - if pull.HeadBranch != pull.HeadRepo.DefaultBranch { - if protected, err := git_model.IsBranchProtected(ctx, pull.HeadRepo.ID, pull.HeadBranch); err != nil { - log.Error("IsProtectedBranch: %v", err) - } else if !protected { - canDelete = true - ctx.Data["DeleteBranchLink"] = issue.Link() + "/cleanup" + if err := repo_service.CanDeleteBranchWithPermission(ctx, pull.HeadRepo, pull.HeadBranch, ctx.Doer, perm); err != nil { + if errors.Is(err, util.ErrPermissionDenied) { + log.Trace("CanDeleteBranch: %v", err) + } else { + log.Error("CanDeleteBranch: %v", err) } + } else { + canDelete = true + ctx.Data["DeleteBranchLink"] = issue.Link() + "/cleanup" } canWriteToHeadRepo = true } diff --git a/routers/web/repo/setting/protected_branch.go b/routers/web/repo/setting/protected_branch.go index e71a14a1fdd..beb2cc58382 100644 --- a/routers/web/repo/setting/protected_branch.go +++ b/routers/web/repo/setting/protected_branch.go @@ -89,11 +89,13 @@ func SettingsProtectedBranch(c *context.Context) { c.Data["PushUsers"] = pushUsers c.Data["ForcePushUsers"] = pushUsers + c.Data["DeletionUsers"] = pushUsers c.Data["MergeUsers"] = repoUsers c.Data["BypassCheckUsers"] = repoUsers c.Data["ApprovalUsers"] = repoUsers c.Data["whitelist_users"] = strings.Join(base.Int64sToStrings(rule.WhitelistUserIDs), ",") c.Data["force_push_allowlist_users"] = strings.Join(base.Int64sToStrings(rule.ForcePushAllowlistUserIDs), ",") + c.Data["deletion_allowlist_users"] = strings.Join(base.Int64sToStrings(rule.DeletionAllowlistUserIDs), ",") c.Data["merge_whitelist_users"] = strings.Join(base.Int64sToStrings(rule.MergeWhitelistUserIDs), ",") c.Data["bypass_allowlist_users"] = strings.Join(base.Int64sToStrings(rule.BypassAllowlistUserIDs), ",") c.Data["approvals_whitelist_users"] = strings.Join(base.Int64sToStrings(rule.ApprovalsWhitelistUserIDs), ",") @@ -110,6 +112,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["deletion_allowlist_teams"] = strings.Join(base.Int64sToStrings(rule.DeletionAllowlistTeamIDs), ",") c.Data["merge_whitelist_teams"] = strings.Join(base.Int64sToStrings(rule.MergeWhitelistTeamIDs), ",") c.Data["bypass_allowlist_teams"] = strings.Join(base.Int64sToStrings(rule.BypassAllowlistTeamIDs), ",") c.Data["approvals_whitelist_teams"] = strings.Join(base.Int64sToStrings(rule.ApprovalsWhitelistTeamIDs), ",") @@ -171,7 +174,7 @@ func SettingsProtectedBranchPost(ctx *context.Context) { } } - var whitelistUsers, whitelistTeams, forcePushAllowlistUsers, forcePushAllowlistTeams, mergeWhitelistUsers, mergeWhitelistTeams, approvalsWhitelistUsers, approvalsWhitelistTeams, bypassAllowlistUsers, bypassAllowlistTeams []int64 + var whitelistUsers, whitelistTeams, forcePushAllowlistUsers, forcePushAllowlistTeams, deletionAllowlistUsers, deletionAllowlistTeams, mergeWhitelistUsers, mergeWhitelistTeams, approvalsWhitelistUsers, approvalsWhitelistTeams, bypassAllowlistUsers, bypassAllowlistTeams []int64 protectBranch.RuleName = f.RuleName if f.RequiredApprovals < 0 { ctx.Flash.Error(ctx.Tr("repo.settings.protected_branch_required_approvals_min")) @@ -215,6 +218,29 @@ func SettingsProtectedBranchPost(ctx *context.Context) { protectBranch.EnableForcePushAllowlist = false } + enableDeletion := f.EnableDeletion + if !protectBranch.CanPush { + // Deletion requires push access. + enableDeletion = "" + } + switch enableDeletion { + case "all": + protectBranch.CanDelete = true + protectBranch.EnableDeletionAllowlist = false + case "whitelist": + protectBranch.CanDelete = true + protectBranch.EnableDeletionAllowlist = true + if strings.TrimSpace(f.DeletionAllowlistUsers) != "" { + deletionAllowlistUsers, _ = base.StringsToInt64s(strings.Split(f.DeletionAllowlistUsers, ",")) + } + if strings.TrimSpace(f.DeletionAllowlistTeams) != "" { + deletionAllowlistTeams, _ = base.StringsToInt64s(strings.Split(f.DeletionAllowlistTeams, ",")) + } + default: + protectBranch.CanDelete = false + protectBranch.EnableDeletionAllowlist = false + } + protectBranch.EnableMergeWhitelist = f.EnableMergeWhitelist if f.EnableMergeWhitelist { if strings.TrimSpace(f.MergeWhitelistUsers) != "" { @@ -288,6 +314,8 @@ func SettingsProtectedBranchPost(ctx *context.Context) { TeamIDs: whitelistTeams, ForcePushUserIDs: forcePushAllowlistUsers, ForcePushTeamIDs: forcePushAllowlistTeams, + DeletionUserIDs: deletionAllowlistUsers, + DeletionTeamIDs: deletionAllowlistTeams, MergeUserIDs: mergeWhitelistUsers, MergeTeamIDs: mergeWhitelistTeams, ApprovalsUserIDs: approvalsWhitelistUsers, diff --git a/services/convert/convert.go b/services/convert/convert.go index b403aeee583..f9df55bf429 100644 --- a/services/convert/convert.go +++ b/services/convert/convert.go @@ -156,6 +156,7 @@ func ToBranchProtection(ctx context.Context, bp *git_model.ProtectedBranch, repo pushWhitelistUsernames := getAllowListUserNames(readers, bp.WhitelistUserIDs) forcePushAllowlistUsernames := getAllowListUserNames(readers, bp.ForcePushAllowlistUserIDs) + deletionAllowlistUsernames := getAllowListUserNames(readers, bp.DeletionAllowlistUserIDs) mergeWhitelistUsernames := getAllowListUserNames(readers, bp.MergeWhitelistUserIDs) approvalsWhitelistUsernames := getAllowListUserNames(readers, bp.ApprovalsWhitelistUserIDs) bypassAllowlistUsernames := getAllowListUserNames(readers, bp.BypassAllowlistUserIDs) @@ -167,6 +168,7 @@ func ToBranchProtection(ctx context.Context, bp *git_model.ProtectedBranch, repo pushWhitelistTeams := getAllowListTeamNames(teamReaders, bp.WhitelistTeamIDs) forcePushAllowlistTeams := getAllowListTeamNames(teamReaders, bp.ForcePushAllowlistTeamIDs) + deletionAllowlistTeams := getAllowListTeamNames(teamReaders, bp.DeletionAllowlistTeamIDs) mergeWhitelistTeams := getAllowListTeamNames(teamReaders, bp.MergeWhitelistTeamIDs) approvalsWhitelistTeams := getAllowListTeamNames(teamReaders, bp.ApprovalsWhitelistTeamIDs) bypassAllowlistTeams := getAllowListTeamNames(teamReaders, bp.BypassAllowlistTeamIDs) @@ -190,6 +192,10 @@ func ToBranchProtection(ctx context.Context, bp *git_model.ProtectedBranch, repo ForcePushAllowlistUsernames: forcePushAllowlistUsernames, ForcePushAllowlistTeams: forcePushAllowlistTeams, ForcePushAllowlistDeployKeys: bp.HasDeployKeyForcePush(), + EnableDeletion: bp.CanDelete, + EnableDeletionAllowlist: bp.EnableDeletionAllowlist, + DeletionAllowlistUsernames: deletionAllowlistUsernames, + DeletionAllowlistTeams: deletionAllowlistTeams, EnableMergeWhitelist: bp.EnableMergeWhitelist, MergeWhitelistUsernames: mergeWhitelistUsernames, MergeWhitelistTeams: mergeWhitelistTeams, diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index 38351b6a2fe..241ed59f4e4 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -159,6 +159,9 @@ type ProtectBranchForm struct { EnableForcePush string ForcePushAllowlistUsers string ForcePushAllowlistTeams string + EnableDeletion string + DeletionAllowlistUsers string + DeletionAllowlistTeams string EnableMergeWhitelist bool MergeWhitelistUsers string MergeWhitelistTeams string diff --git a/services/repository/branch.go b/services/repository/branch.go index 0598e7c83a4..8dac9c7b80d 100644 --- a/services/repository/branch.go +++ b/services/repository/branch.go @@ -50,6 +50,7 @@ func CreateNewBranch(ctx context.Context, doer *user_model.User, repo *repo_mode type Branch struct { DBBranch *git_model.Branch IsProtected bool + CanDelete bool IsIncluded bool CommitsAhead int CommitsBehind int @@ -58,7 +59,7 @@ type Branch struct { } // LoadBranches loads branches from the repository limited by page & pageSize. -func LoadBranches(ctx context.Context, repo *repo_model.Repository, gitRepo *git.Repository, isDeletedBranch optional.Option[bool], keyword string, page, pageSize int) (defaultBranchOptional *Branch, _ []*Branch, _ int64, _ error) { +func LoadBranches(ctx context.Context, repo *repo_model.Repository, gitRepo *git.Repository, doer *user_model.User, permissionInRepo access_model.Permission, isDeletedBranch optional.Option[bool], keyword string, page, pageSize int) (defaultBranchOptional *Branch, _ []*Branch, _ int64, _ error) { defaultDBBranchOptional, err := git_model.GetBranchExisting(ctx, repo.ID, repo.DefaultBranch) if err != nil && !errors.Is(err, util.ErrNotExist) { return nil, nil, 0, err @@ -91,16 +92,18 @@ func LoadBranches(ctx context.Context, repo *repo_model.Repository, gitRepo *git if err != nil { return nil, nil, 0, err } + pullRequestTargetBranch := repo.GetPullRequestTargetBranch(ctx) repoIDToRepo := map[int64]*repo_model.Repository{} repoIDToRepo[repo.ID] = repo repoIDToGitRepo := map[int64]*git.Repository{} repoIDToGitRepo[repo.ID] = gitRepo + canDeleteProtectedBranches := make(map[int64]bool, len(rules)) branches := make([]*Branch, 0, len(dbBranches)) for i := range dbBranches { - branch, err := loadOneBranch(ctx, repo, dbBranches[i], &rules, repoIDToRepo, repoIDToGitRepo) + branch, err := loadOneBranch(ctx, repo, dbBranches[i], &rules, repoIDToRepo, repoIDToGitRepo, doer, permissionInRepo, canDeleteProtectedBranches, pullRequestTargetBranch) if err != nil { return nil, nil, 0, fmt.Errorf("loadOneBranch: %v", err) } @@ -109,7 +112,7 @@ func LoadBranches(ctx context.Context, repo *repo_model.Repository, gitRepo *git if defaultDBBranchOptional != nil { // Always add the default branch - defaultBranchOptional, err = loadOneBranch(ctx, repo, defaultDBBranchOptional, &rules, repoIDToRepo, repoIDToGitRepo) + defaultBranchOptional, err = loadOneBranch(ctx, repo, defaultDBBranchOptional, &rules, repoIDToRepo, repoIDToGitRepo, doer, permissionInRepo, canDeleteProtectedBranches, pullRequestTargetBranch) if err != nil { return nil, nil, 0, fmt.Errorf("loadOneBranch: %v", err) } @@ -170,12 +173,26 @@ func DelRepoDivergenceFromCache(ctx context.Context, repoID int64) error { func loadOneBranch(ctx context.Context, repo *repo_model.Repository, dbBranch *git_model.Branch, protectedBranches *git_model.ProtectedBranchRules, repoIDToRepo map[int64]*repo_model.Repository, repoIDToGitRepo map[int64]*git.Repository, + doer *user_model.User, + permissionInRepo access_model.Permission, + canDeleteProtectedBranches map[int64]bool, + pullRequestTargetBranch string, ) (*Branch, error) { log.Trace("loadOneBranch: '%s'", dbBranch.Name) branchName := dbBranch.Name p := protectedBranches.GetFirstMatched(branchName) isProtected := p != nil + canDelete := !dbBranch.IsDeleted && repo.DefaultBranch != branchName && pullRequestTargetBranch != branchName + if isProtected && canDelete { + p.Repo = repo + canUserDelete, ok := canDeleteProtectedBranches[p.ID] + if !ok { + canUserDelete = p.CanUserDelete(ctx, doer, permissionInRepo) + canDeleteProtectedBranches[p.ID] = canUserDelete + } + canDelete = canDelete && canUserDelete + } var divergence *git.DivergeObject @@ -247,6 +264,7 @@ func loadOneBranch(ctx context.Context, repo *repo_model.Repository, dbBranch *g return &Branch{ DBBranch: dbBranch, IsProtected: isProtected, + CanDelete: canDelete, IsIncluded: isIncluded, CommitsAhead: divergence.Ahead, CommitsBehind: divergence.Behind, @@ -545,24 +563,31 @@ func UpdateBranch(ctx context.Context, repo *repo_model.Repository, gitRepo *git var ErrBranchIsDefault = util.ErrorWrap(util.ErrPermissionDenied, "branch is default or pull request target") func CanDeleteBranch(ctx context.Context, repo *repo_model.Repository, branchName string, doer *user_model.User) error { - unitPRConfig := repo.MustGetUnit(ctx, unit.TypePullRequests).PullRequestsConfig() - if branchName == repo.DefaultBranch || branchName == unitPRConfig.DefaultTargetBranch { - return ErrBranchIsDefault - } - perm, err := access_model.GetDoerRepoPermission(ctx, repo, doer) if err != nil { return err } - if !perm.CanWrite(unit.TypeCode) { + return CanDeleteBranchWithPermission(ctx, repo, branchName, doer, perm) +} + +// CanDeleteBranchWithPermission checks whether a branch can be deleted using an already loaded repository permission. +func CanDeleteBranchWithPermission(ctx context.Context, repo *repo_model.Repository, branchName string, doer *user_model.User, permissionInRepo access_model.Permission) error { + if branchName == repo.DefaultBranch || branchName == repo.GetPullRequestTargetBranch(ctx) { + return ErrBranchIsDefault + } + if !permissionInRepo.CanWrite(unit.TypeCode) { return util.NewPermissionDeniedErrorf("permission denied to access repo %d unit %s", repo.ID, unit.TypeCode.LogString()) } - isProtected, err := git_model.IsBranchProtected(ctx, repo.ID, branchName) + protectedBranch, err := git_model.GetFirstMatchProtectedBranchRule(ctx, repo.ID, branchName) if err != nil { return err } - if isProtected { + if protectedBranch != nil { + protectedBranch.Repo = repo + if protectedBranch.CanUserDelete(ctx, doer, permissionInRepo) { + return nil + } return git_model.ErrBranchIsProtected } return nil diff --git a/services/repository/branch_test.go b/services/repository/branch_test.go new file mode 100644 index 00000000000..95314521465 --- /dev/null +++ b/services/repository/branch_test.go @@ -0,0 +1,44 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package repository + +import ( + "testing" + + "gitea.dev/models/db" + git_model "gitea.dev/models/git" + repo_model "gitea.dev/models/repo" + "gitea.dev/models/unit" + "gitea.dev/models/unittest" + user_model "gitea.dev/models/user" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestCanDeleteBranchDefaultTargets(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + ctx := t.Context() + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) + + rule := &git_model.ProtectedBranch{ + RepoID: repo.ID, RuleName: "*", + ProtectedBranchConfig: git_model.ProtectedBranchConfig{CanPush: true, CanDelete: true}, + } + require.NoError(t, db.Insert(ctx, rule)) + assert.NoError(t, CanDeleteBranch(ctx, repo, "branch2", owner)) + assert.ErrorIs(t, CanDeleteBranch(ctx, repo, repo.DefaultBranch, owner), ErrBranchIsDefault) + + pullRequestUnit, err := repo.GetUnit(ctx, unit.TypePullRequests) + assert.NoError(t, err) + pullRequestConfig := pullRequestUnit.PullRequestsConfig() + pullRequestConfig.DefaultTargetBranch = "branch2" + pullRequestUnit.Config = pullRequestConfig + assert.NoError(t, repo_model.UpdateRepoUnitConfig(ctx, pullRequestUnit)) + repo.Units = nil + + assert.ErrorIs(t, CanDeleteBranch(ctx, repo, "branch2", owner), ErrBranchIsDefault) +} diff --git a/templates/repo/branch/list.tmpl b/templates/repo/branch/list.tmpl index 9ac6ad873bd..b150a1e57ce 100644 --- a/templates/repo/branch/list.tmpl +++ b/templates/repo/branch/list.tmpl @@ -192,14 +192,16 @@ {{svg "octicon-pencil"}} {{end}} - {{if and $.IsWriter $.Repository.CanContentChange (not .IsProtected)}} + {{if and $.IsWriter $.Repository.CanContentChange}} {{if .DBBranch.IsDeleted}} - - {{else}} + {{if not .IsProtected}} + + {{end}} + {{else if .CanDelete}} diff --git a/templates/repo/settings/protected_branch.tmpl b/templates/repo/settings/protected_branch.tmpl index 42872ecbfee..46a57e6f7bb 100644 --- a/templates/repo/settings/protected_branch.tmpl +++ b/templates/repo/settings/protected_branch.tmpl @@ -106,6 +106,42 @@ {{end}} +
{{ctx.Locale.Tr "repo.settings.event_branch_delete"}}
+
+
+ + +

{{ctx.Locale.Tr "repo.settings.protect_disable_deletion_desc"}}

+
+
+
+
+ + +

{{ctx.Locale.Tr "repo.settings.protect_enable_deletion_all_desc"}}

+
+
+
+
+
+ + +

{{ctx.Locale.Tr "repo.settings.protect_enable_deletion_allowlist_desc"}}

+
+
+
+
+ + {{template "repo/settings/user_dropdown" (dict "Name" "deletion_allowlist_users" "Value" .deletion_allowlist_users "Users" .DeletionUsers)}} +
+ {{if .Owner.IsOrganization}} +
+ + {{template "repo/settings/team_dropdown" (dict "Name" "deletion_allowlist_teams" "Value" .deletion_allowlist_teams "Teams" .Teams)}} +
+ {{end}} +
+
{{ctx.Locale.Tr "repo.settings.event_pull_request_approvals"}}
diff --git a/templates/swagger/v1-openapi3.generated.json b/templates/swagger/v1-openapi3.generated.json index 46fcfccaeab..8ef255a225d 100644 --- a/templates/swagger/v1-openapi3.generated.json +++ b/templates/swagger/v1-openapi3.generated.json @@ -2917,6 +2917,20 @@ "type": "string", "x-go-name": "Created" }, + "deletion_allowlist_teams": { + "items": { + "type": "string" + }, + "type": "array", + "x-go-name": "DeletionAllowlistTeams" + }, + "deletion_allowlist_usernames": { + "items": { + "type": "string" + }, + "type": "array", + "x-go-name": "DeletionAllowlistUsernames" + }, "dismiss_stale_approvals": { "type": "boolean", "x-go-name": "DismissStaleApprovals" @@ -2929,6 +2943,14 @@ "type": "boolean", "x-go-name": "EnableBypassAllowlist" }, + "enable_deletion": { + "type": "boolean", + "x-go-name": "EnableDeletion" + }, + "enable_deletion_allowlist": { + "type": "boolean", + "x-go-name": "EnableDeletionAllowlist" + }, "enable_force_push": { "type": "boolean", "x-go-name": "EnableForcePush" @@ -3817,6 +3839,20 @@ "type": "array", "x-go-name": "BypassAllowlistUsernames" }, + "deletion_allowlist_teams": { + "items": { + "type": "string" + }, + "type": "array", + "x-go-name": "DeletionAllowlistTeams" + }, + "deletion_allowlist_usernames": { + "items": { + "type": "string" + }, + "type": "array", + "x-go-name": "DeletionAllowlistUsernames" + }, "dismiss_stale_approvals": { "type": "boolean", "x-go-name": "DismissStaleApprovals" @@ -3829,6 +3865,14 @@ "type": "boolean", "x-go-name": "EnableBypassAllowlist" }, + "enable_deletion": { + "type": "boolean", + "x-go-name": "EnableDeletion" + }, + "enable_deletion_allowlist": { + "type": "boolean", + "x-go-name": "EnableDeletionAllowlist" + }, "enable_force_push": { "type": "boolean", "x-go-name": "EnableForcePush" @@ -5372,6 +5416,20 @@ "type": "array", "x-go-name": "BypassAllowlistUsernames" }, + "deletion_allowlist_teams": { + "items": { + "type": "string" + }, + "type": "array", + "x-go-name": "DeletionAllowlistTeams" + }, + "deletion_allowlist_usernames": { + "items": { + "type": "string" + }, + "type": "array", + "x-go-name": "DeletionAllowlistUsernames" + }, "dismiss_stale_approvals": { "type": "boolean", "x-go-name": "DismissStaleApprovals" @@ -5384,6 +5442,14 @@ "type": "boolean", "x-go-name": "EnableBypassAllowlist" }, + "enable_deletion": { + "type": "boolean", + "x-go-name": "EnableDeletion" + }, + "enable_deletion_allowlist": { + "type": "boolean", + "x-go-name": "EnableDeletionAllowlist" + }, "enable_force_push": { "type": "boolean", "x-go-name": "EnableForcePush" diff --git a/templates/swagger/v1-swagger.generated.json b/templates/swagger/v1-swagger.generated.json index 7ffc1ebd418..9f6f28bef01 100644 --- a/templates/swagger/v1-swagger.generated.json +++ b/templates/swagger/v1-swagger.generated.json @@ -25987,6 +25987,20 @@ "format": "date-time", "x-go-name": "Created" }, + "deletion_allowlist_teams": { + "type": "array", + "items": { + "type": "string" + }, + "x-go-name": "DeletionAllowlistTeams" + }, + "deletion_allowlist_usernames": { + "type": "array", + "items": { + "type": "string" + }, + "x-go-name": "DeletionAllowlistUsernames" + }, "dismiss_stale_approvals": { "type": "boolean", "x-go-name": "DismissStaleApprovals" @@ -25999,6 +26013,14 @@ "type": "boolean", "x-go-name": "EnableBypassAllowlist" }, + "enable_deletion": { + "type": "boolean", + "x-go-name": "EnableDeletion" + }, + "enable_deletion_allowlist": { + "type": "boolean", + "x-go-name": "EnableDeletionAllowlist" + }, "enable_force_push": { "type": "boolean", "x-go-name": "EnableForcePush" @@ -26870,6 +26892,20 @@ }, "x-go-name": "BypassAllowlistUsernames" }, + "deletion_allowlist_teams": { + "type": "array", + "items": { + "type": "string" + }, + "x-go-name": "DeletionAllowlistTeams" + }, + "deletion_allowlist_usernames": { + "type": "array", + "items": { + "type": "string" + }, + "x-go-name": "DeletionAllowlistUsernames" + }, "dismiss_stale_approvals": { "type": "boolean", "x-go-name": "DismissStaleApprovals" @@ -26882,6 +26918,14 @@ "type": "boolean", "x-go-name": "EnableBypassAllowlist" }, + "enable_deletion": { + "type": "boolean", + "x-go-name": "EnableDeletion" + }, + "enable_deletion_allowlist": { + "type": "boolean", + "x-go-name": "EnableDeletionAllowlist" + }, "enable_force_push": { "type": "boolean", "x-go-name": "EnableForcePush" @@ -28451,6 +28495,20 @@ }, "x-go-name": "BypassAllowlistUsernames" }, + "deletion_allowlist_teams": { + "type": "array", + "items": { + "type": "string" + }, + "x-go-name": "DeletionAllowlistTeams" + }, + "deletion_allowlist_usernames": { + "type": "array", + "items": { + "type": "string" + }, + "x-go-name": "DeletionAllowlistUsernames" + }, "dismiss_stale_approvals": { "type": "boolean", "x-go-name": "DismissStaleApprovals" @@ -28463,6 +28521,14 @@ "type": "boolean", "x-go-name": "EnableBypassAllowlist" }, + "enable_deletion": { + "type": "boolean", + "x-go-name": "EnableDeletion" + }, + "enable_deletion_allowlist": { + "type": "boolean", + "x-go-name": "EnableDeletionAllowlist" + }, "enable_force_push": { "type": "boolean", "x-go-name": "EnableForcePush" diff --git a/tests/integration/api_branch_test.go b/tests/integration/api_branch_test.go index 431ab4890f8..22f6472e0be 100644 --- a/tests/integration/api_branch_test.go +++ b/tests/integration/api_branch_test.go @@ -65,7 +65,7 @@ func testAPICreateBranchProtection(t *testing.T, branchName string, expectedPrio } } -func testAPIEditBranchProtection(t *testing.T, branchName string, body *api.BranchProtection, expectedHTTPStatus int) { +func testAPIEditBranchProtection(t *testing.T, branchName string, body any, expectedHTTPStatus int) { token := getUserToken(t, "user2", auth_model.AccessTokenScopeWriteRepository) req := NewRequestWithJSON(t, "PATCH", "/api/v1/repos/user2/repo1/branch_protections/"+branchName, body). AddTokenAuth(token) @@ -406,6 +406,15 @@ func testAPIBranchProtectionBasic(t *testing.T) { // Test branch deletion testAPIDeleteBranch(t, "master", http.StatusForbidden) + testAPICreateBranchProtection(t, "branch2", 3, http.StatusCreated) + testAPIDeleteBranch(t, "branch2", http.StatusForbidden) + testAPIEditBranchProtection(t, "branch2", &api.EditBranchProtectionOption{ + EnablePush: new(true), EnableDeletion: new(true), EnableDeletionAllowlist: new(true), + DeletionAllowlistUsernames: []string{"user2"}, + }, http.StatusOK) + bp = testAPIGetBranchProtection(t, "branch2", http.StatusOK) + assert.True(t, bp.EnableDeletion) + assert.Equal(t, []string{"user2"}, bp.DeletionAllowlistUsernames) testAPIDeleteBranch(t, "branch2", http.StatusNoContent) testAPIDeleteBranch(t, "branch2", http.StatusNotFound) // deleted branch, there is a record in DB with IsDelete=true testAPIDeleteBranch(t, "no-such-branch", http.StatusNotFound) // non-existing branch, not exist in git or DB diff --git a/tests/integration/branches_test.go b/tests/integration/branches_test.go index 1a647001d91..a6f18964181 100644 --- a/tests/integration/branches_test.go +++ b/tests/integration/branches_test.go @@ -11,6 +11,7 @@ import ( "gitea.dev/models/db" repo_model "gitea.dev/models/repo" "gitea.dev/models/unittest" + api "gitea.dev/modules/structs" "gitea.dev/modules/translation" "gitea.dev/tests" @@ -70,3 +71,39 @@ func TestUndoDeleteBranch(t *testing.T) { ) }) } + +func TestDeleteProtectedBranchWeb(t *testing.T) { + onGiteaRun(t, func(t *testing.T, _ *url.URL) { + session := loginUser(t, "user2") + const branchURL = "/user2/repo1/branches/delete?name=branch2" + const deleteButton = `.delete-branch-button[data-modal-name="branch2"]` + testAPICreateBranchProtection(t, "branch2", 1, http.StatusCreated) + + resp := session.MakeRequest(t, NewRequest(t, "GET", "/user2/repo1/branches"), http.StatusOK) + AssertHTMLElement(t, NewHTMLParser(t, resp.Body), deleteButton, 0) + session.MakeRequest(t, NewRequest(t, "POST", branchURL), http.StatusBadRequest) + MakeRequest(t, NewRequest(t, "GET", "/api/v1/repos/user2/repo1/branches/branch2"), http.StatusOK) + + testAPIEditBranchProtection(t, "branch2", &api.EditBranchProtectionOption{ + EnablePush: new(true), + EnableDeletion: new(true), + EnableDeletionAllowlist: new(true), + DeletionAllowlistUsernames: []string{"user2"}, + }, http.StatusOK) + resp = session.MakeRequest(t, NewRequest(t, "GET", "/user2/repo1/branches"), http.StatusOK) + htmlDoc := NewHTMLParser(t, resp.Body) + AssertHTMLElement(t, htmlDoc, deleteButton, 1) + button := htmlDoc.doc.Find(deleteButton) + assert.Equal(t, "button", button.AttrOr("type", "")) + link, exists := button.Attr("data-modal-form.url") + require.True(t, exists) + resp = session.MakeRequest(t, NewRequest(t, "POST", link), http.StatusOK) + assert.JSONEq(t, `{"redirect":""}`, resp.Body.String()) + testAPIGetBranch(t, "branch2", false) + + resp = session.MakeRequest(t, NewRequest(t, "GET", "/user2/repo1/branches"), http.StatusOK) + htmlDoc = NewHTMLParser(t, resp.Body) + AssertHTMLElement(t, htmlDoc, deleteButton, 0) + AssertHTMLElement(t, htmlDoc, `.restore-branch-button[data-url*="name=branch2"]`, 0) + }) +} diff --git a/tests/integration/git_general_test.go b/tests/integration/git_general_test.go index 6d2ec28b422..3ba7c138063 100644 --- a/tests/integration/git_general_test.go +++ b/tests/integration/git_general_test.go @@ -486,6 +486,16 @@ func doBranchProtectPRMerge(baseCtx *APITestContext, dstPath string) func(t *tes t.Run("MergeProtectedToToforce", doGitMerge(dstPath, "protected")) t.Run("PushToProtectedBranch", doGitPushTestRepository(dstPath, "origin", "toforce:protected")) t.Run("CheckoutMasterAgain", doGitCheckoutBranch(dstPath, "master")) + + t.Run("CreateBranchForProtectedDeletion", doGitCreateBranch(dstPath, "protected-delete")) + t.Run("PushBranchForProtectedDeletion", doGitPushTestRepository(dstPath, "origin", "protected-delete")) + t.Run("ProtectBranchWithoutDeletion", doProtectBranch(ctx, "protected-delete", baseCtx.Username, "", "", "")) + t.Run("DeleteProtectedBranchDenied", doGitPushTestRepositoryFail(dstPath, "origin", "--delete", "protected-delete")) + t.Run("ProtectBranchWithDeletionAllowlist", doProtectBranchExt(ctx, "protected-delete", doProtectBranchOptions{ + UserToWhitelistPush: baseCtx.Username, + UserToWhitelistDelete: baseCtx.Username, + })) + t.Run("DeleteProtectedBranchAllowed", doGitPushTestRepository(dstPath, "origin", "--delete", "protected-delete")) } } @@ -499,7 +509,7 @@ func doProtectBranch(ctx APITestContext, branch, userToWhitelistPush, userToWhit } type doProtectBranchOptions struct { - UserToWhitelistPush, UserToWhitelistForcePush, UnprotectedFilePatterns, ProtectedFilePatterns string + UserToWhitelistPush, UserToWhitelistForcePush, UserToWhitelistDelete, UnprotectedFilePatterns, ProtectedFilePatterns string StatusCheckPatterns []string } @@ -529,6 +539,13 @@ func doProtectBranchExt(ctx APITestContext, ruleName string, opts doProtectBranc formData["enable_force_push_allowlist"] = "on" } + if opts.UserToWhitelistDelete != "" { + user, err := user_model.GetUserByName(t.Context(), opts.UserToWhitelistDelete) + assert.NoError(t, err) + formData["deletion_allowlist_users"] = strconv.FormatInt(user.ID, 10) + formData["enable_deletion"] = "whitelist" + } + if len(opts.StatusCheckPatterns) > 0 { formData["enable_status_check"] = "on" formData["status_check_contexts"] = strings.Join(opts.StatusCheckPatterns, "\n")