mirror of
https://github.com/go-gitea/gitea.git
synced 2026-10-19 21:35:30 +02:00
feat(repo): allow deleting protected branches (#39243)
This adds a Branch Deletion setting with three modes: disabled by default, anyone with push access, or a restricted user/team/deploy-key allowlist. The policy applies to Git pushes, API/UI deletion, and post-merge cleanup; the default branch and the pull request target branch remain undeletable. Closes https://github.com/go-gitea/gitea/issues/33305 --------- Co-authored-by: Enoque Duarte <12515551+enoquefcd@users.noreply.github.com> Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
This commit is contained in:
21 files changed
+600
-59
No files matched your search
@@ -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 {
|
||||
|
||||
@@ -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))
|
||||
})
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user