From c6e25135705bb947ccc9a8eec3b1e769eda6d970 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 13 Oct 2026 08:40:21 +0800 Subject: [PATCH] refactor: migrate protected branch fields into one json column and support system users (#39735) 1. migrate protected branch fields into one json column 2. use the dropdown list for push/force-push user selection, and Actions user can be added into the list now. --- modelmigration/base/db.go | 17 ++ modelmigration/migrations.go | 1 + modelmigration/v29/v360.go | 112 ++++++++ modelmigration/v29/v360_test.go | 116 ++++++++ models/db/iterate.go | 14 +- models/git/protected_branch.go | 117 ++++---- models/git/protected_branch_test.go | 26 ++ models/issues/assignees.go | 2 +- models/issues/review_test.go | 2 +- models/user/user.go | 66 +++-- models/user/user_system.go | 12 +- models/user/user_test.go | 16 +- routers/api/v1/repo/branch.go | 249 +++++++----------- routers/api/v1/repo/issue.go | 4 +- routers/api/v1/repo/issue_assignee.go | 10 +- routers/api/v1/repo/pull.go | 2 +- routers/api/v1/repo/tag.go | 17 +- routers/private/hook_pre_receive.go | 16 +- routers/web/repo/issue_new.go | 7 +- routers/web/repo/setting/protected_branch.go | 20 +- services/convert/convert.go | 65 +++-- services/forms/repo_form.go | 2 - templates/repo/settings/protected_branch.tmpl | 22 +- tests/integration/api_branch_test.go | 76 ++++++ tests/integration/api_issue_test.go | 2 +- 25 files changed, 659 insertions(+), 334 deletions(-) create mode 100644 modelmigration/v29/v360.go create mode 100644 modelmigration/v29/v360_test.go diff --git a/modelmigration/base/db.go b/modelmigration/base/db.go index 3158aa3da47..6456057135d 100644 --- a/modelmigration/base/db.go +++ b/modelmigration/base/db.go @@ -518,3 +518,20 @@ func ModifyColumn(ctx context.Context, x EngineMigration, tableName string, col func Iterate[Bean any](ctx context.Context, cond builder.Cond, f func(ctx context.Context, bean *Bean) error) error { return db.Iterate(ctx, cond, f) } + +func EngineTransaction(x EngineMigration, f func(Session) error) error { + sess := x.NewSession() + defer sess.Close() + if err := sess.Begin(); err != nil { + return err + } + err := f(sess) + if err != nil { + return err + } + return sess.Commit() +} + +func IterateTableByColumn[Bean any](ctx context.Context, x EngineMigration, sess Session, colName string, cond builder.Cond, f func(ctx context.Context, bean *Bean) error) error { + return db.IterateTableByColumn(ctx, x, sess, colName, cond, f) +} diff --git a/modelmigration/migrations.go b/modelmigration/migrations.go index 9701fb6326f..c160e7f1938 100644 --- a/modelmigration/migrations.go +++ b/modelmigration/migrations.go @@ -436,6 +436,7 @@ func prepareMigrationTasks() []*migration { newMigration(357, "Normalize legacy team authorize values", v29.NormalizeLegacyTeamAuthorize), newMigration(358, "Track previous remember-me token hashes", v29.AddPreviousAuthTokenHash), newMigration(359, "Add stage and number columns to action_task_step", v29.AddStageAndNumberToActionTaskStep), + newMigration(360, "Move branch protection settings to JSON", v29.MoveBranchProtectionConfigToJSON), } return preparedMigrations } diff --git a/modelmigration/v29/v360.go b/modelmigration/v29/v360.go new file mode 100644 index 00000000000..e1076515b38 --- /dev/null +++ b/modelmigration/v29/v360.go @@ -0,0 +1,112 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v29 + +import ( + "context" + + "gitea.dev/modelmigration/base" + "gitea.dev/modules/json" + "gitea.dev/modules/timeutil" + "gitea.dev/modules/util" + + "xorm.io/xorm" +) + +type protectedBranchConfigV358 struct { + CanPush bool `xorm:"NOT NULL DEFAULT false" json:"can_push,omitzero"` + EnableWhitelist bool `json:"enable_whitelist,omitzero"` + WhitelistUserIDs []int64 `xorm:"JSON TEXT" json:"whitelist_user_ids,omitzero"` + WhitelistTeamIDs []int64 `xorm:"JSON TEXT" json:"whitelist_team_ids,omitzero"` + EnableMergeWhitelist bool `xorm:"NOT NULL DEFAULT false" json:"enable_merge_whitelist,omitzero"` + WhitelistDeployKeys bool `xorm:"NOT NULL DEFAULT false" json:"-"` + MergeWhitelistUserIDs []int64 `xorm:"JSON TEXT" json:"merge_whitelist_user_ids,omitzero"` + MergeWhitelistTeamIDs []int64 `xorm:"JSON TEXT" json:"merge_whitelist_team_ids,omitzero"` + EnableBypassAllowlist bool `xorm:"NOT NULL DEFAULT false" json:"enable_bypass_allowlist,omitzero"` + BypassAllowlistUserIDs []int64 `xorm:"JSON TEXT" json:"bypass_allowlist_user_ids,omitzero"` + BypassAllowlistTeamIDs []int64 `xorm:"JSON TEXT" json:"bypass_allowlist_team_ids,omitzero"` + CanForcePush bool `xorm:"NOT NULL DEFAULT false" json:"can_force_push,omitzero"` + EnableForcePushAllowlist bool `xorm:"NOT NULL DEFAULT false" json:"enable_force_push_allowlist,omitzero"` + ForcePushAllowlistUserIDs []int64 `xorm:"JSON TEXT" json:"force_push_allowlist_user_ids,omitzero"` + ForcePushAllowlistTeamIDs []int64 `xorm:"JSON TEXT" json:"force_push_allowlist_team_ids,omitzero"` + ForcePushAllowlistDeployKeys bool `xorm:"NOT NULL DEFAULT false" json:"-"` + EnableStatusCheck bool `xorm:"NOT NULL DEFAULT false" json:"enable_status_check,omitzero"` + StatusCheckContexts []string `xorm:"JSON TEXT" json:"status_check_contexts,omitzero"` + EnableApprovalsWhitelist bool `xorm:"NOT NULL DEFAULT false" json:"enable_approvals_whitelist,omitzero"` + ApprovalsWhitelistUserIDs []int64 `xorm:"JSON TEXT" json:"approvals_whitelist_user_ids,omitzero"` + ApprovalsWhitelistTeamIDs []int64 `xorm:"JSON TEXT" json:"approvals_whitelist_team_ids,omitzero"` + RequiredApprovals int64 `xorm:"NOT NULL DEFAULT 0" json:"required_approvals,omitzero"` + BlockOnRejectedReviews bool `xorm:"NOT NULL DEFAULT false" json:"block_on_rejected_reviews,omitzero"` + BlockOnOfficialReviewRequests bool `xorm:"NOT NULL DEFAULT false" json:"block_on_official_review_requests,omitzero"` + BlockOnCodeownerReviews bool `xorm:"NOT NULL DEFAULT false" json:"block_on_codeowner_reviews,omitzero"` + BlockOnOutdatedBranch bool `xorm:"NOT NULL DEFAULT false" json:"block_on_outdated_branch,omitzero"` + DismissStaleApprovals bool `xorm:"NOT NULL DEFAULT false" json:"dismiss_stale_approvals,omitzero"` + IgnoreStaleApprovals bool `xorm:"NOT NULL DEFAULT false" json:"ignore_stale_approvals,omitzero"` + RequireSignedCommits bool `xorm:"NOT NULL DEFAULT false" json:"require_signed_commits,omitzero"` + ProtectedFilePatterns string `xorm:"TEXT" json:"protected_file_patterns,omitzero"` + UnprotectedFilePatterns string `xorm:"TEXT" json:"unprotected_file_patterns,omitzero"` + BlockAdminMergeOverride bool `xorm:"NOT NULL DEFAULT false" json:"block_admin_merge_override,omitzero"` +} + +type protectedBranchV358 struct { + ID int64 `xorm:"pk autoincr"` + RepoID int64 `xorm:"UNIQUE(s)"` + BranchName string `xorm:"UNIQUE(s)"` + Priority int64 `xorm:"NOT NULL DEFAULT 0"` + Config string `xorm:"LONGTEXT"` + CreatedUnix timeutil.TimeStamp + UpdatedUnix timeutil.TimeStamp +} + +func (protectedBranchV358) TableName() string { return "protected_branch" } + +type intermediateProtectedBranch struct { + ID int64 `xorm:"pk autoincr"` + NewConfig string `xorm:"LONGTEXT config"` + OldConfig protectedBranchConfigV358 `xorm:"extends"` +} + +func (intermediateProtectedBranch) TableName() string { return "protected_branch" } + +func MoveBranchProtectionConfigToJSON(ctx context.Context, x base.EngineMigration) error { + if _, err := x.SyncWithOptions(xorm.SyncOptions{IgnoreDropIndices: true}, new(protectedBranchV358)); err != nil { + return err + } + exists, err := x.Dialect().IsColumnExist(x.DB(), ctx, "protected_branch", "can_push") + if err != nil || !exists { + return err + } + + err = base.EngineTransaction(x, func(sess base.Session) error { + err := base.IterateTableByColumn(ctx, x, sess, "id", nil, func(ctx context.Context, pb *intermediateProtectedBranch) error { + if pb.NewConfig != "" { + return nil // skip the already migrated fields + } + const deployKeyUserID int64 = -3 // frozen user.DeployKeyUserID + if pb.OldConfig.WhitelistDeployKeys { + pb.OldConfig.WhitelistUserIDs = append(pb.OldConfig.WhitelistUserIDs, deployKeyUserID) + } + if pb.OldConfig.ForcePushAllowlistDeployKeys { + pb.OldConfig.ForcePushAllowlistUserIDs = append(pb.OldConfig.ForcePushAllowlistUserIDs, deployKeyUserID) + } + configJson, err := json.Marshal(pb.OldConfig) + util.MustNoError(err) + _, err = sess.ID(pb.ID).Cols("config").Update(&protectedBranchV358{Config: string(configJson)}) + return err + }) + if err != nil { + return err + } + legacyTableInfo, err := x.TableInfo(new(protectedBranchConfigV358)) + if err != nil { + return err + } + return base.DropTableColumns(sess, "protected_branch", legacyTableInfo.ColumnsSeq()...) + }) + if err != nil { + return err + } + // SQLite rebuilds the table when dropping columns, so restore its unique index. + return x.Sync(new(protectedBranchV358)) +} diff --git a/modelmigration/v29/v360_test.go b/modelmigration/v29/v360_test.go new file mode 100644 index 00000000000..c562a0944ea --- /dev/null +++ b/modelmigration/v29/v360_test.go @@ -0,0 +1,116 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v29 + +import ( + "fmt" + "testing" + + "gitea.dev/modelmigration/migrationtest" + "gitea.dev/modules/json" + "gitea.dev/modules/timeutil" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type protectedBranchBeforeV358 struct { + ID int64 `xorm:"pk autoincr"` + RepoID int64 `xorm:"UNIQUE(s)"` + BranchName string `xorm:"UNIQUE(s)"` + Priority int64 + CreatedUnix timeutil.TimeStamp + UpdatedUnix timeutil.TimeStamp + Config protectedBranchConfigV358 `xorm:"extends"` +} + +func (protectedBranchBeforeV358) TableName() string { return "protected_branch" } + +func TestMoveBranchProtectionConfigToJSON(t *testing.T) { + x, deferrable := migrationtest.PrepareTestEnv(t, 0, new(protectedBranchBeforeV358)) + defer deferrable() + if x == nil || t.Failed() { + return + } + + const configJSON = `{ +"can_push":true, +"enable_whitelist":true, +"whitelist_user_ids":[2,4,-3], +"whitelist_team_ids":[1,2], +"enable_merge_whitelist":true, +"merge_whitelist_user_ids":[3,5], +"merge_whitelist_team_ids":[3,4], +"enable_bypass_allowlist":true, +"bypass_allowlist_user_ids":[6,7], +"bypass_allowlist_team_ids":[5,6], +"can_force_push":true, +"enable_force_push_allowlist":true, +"force_push_allowlist_user_ids":[8,9,-3], +"force_push_allowlist_team_ids":[7,8], +"enable_status_check":true, +"status_check_contexts":["ci/*","lint"], +"enable_approvals_whitelist":true, +"approvals_whitelist_user_ids":[10,11], +"approvals_whitelist_team_ids":[9,10], +"required_approvals":2, +"block_on_rejected_reviews":true, +"block_on_official_review_requests":true, +"block_on_codeowner_reviews":true, +"block_on_outdated_branch":true, +"dismiss_stale_approvals":true, +"ignore_stale_approvals":true, +"require_signed_commits":true, +"protected_file_patterns":"*.go;*.md", +"unprotected_file_patterns":"docs/**", +"block_admin_merge_override":true +}` + var config protectedBranchConfigV358 + require.NoError(t, json.Unmarshal([]byte(configJSON), &config)) + config.WhitelistUserIDs = []int64{2, 4} + config.ForcePushAllowlistUserIDs = []int64{8, 9} + config.WhitelistDeployKeys = true + config.ForcePushAllowlistDeployKeys = true + for i := range 101 { + branch := protectedBranchBeforeV358{RepoID: 1, BranchName: fmt.Sprintf("release-%d/*", i), Priority: int64(i), CreatedUnix: 123, UpdatedUnix: 456} + if i%2 == 0 { + branch.Config = config + } else if i == 1 { + branch.Config.StatusCheckContexts = []string{} + } + _, err := x.Insert(&branch) + require.NoError(t, err) + } + require.NoError(t, MoveBranchProtectionConfigToJSON(t.Context(), x)) + // A retry must preserve the migrated configuration and its indexes. + require.NoError(t, MoveBranchProtectionConfigToJSON(t.Context(), x)) + + var branches []protectedBranchV358 + require.NoError(t, x.Asc("id").Find(&branches)) + require.Len(t, branches, 101) + for i, branch := range branches { + assert.EqualValues(t, i+1, branch.ID) + assert.EqualValues(t, 1, branch.RepoID) + assert.Equal(t, fmt.Sprintf("release-%d/*", i), branch.BranchName) + assert.EqualValues(t, i, branch.Priority) + assert.EqualValues(t, 123, branch.CreatedUnix) + assert.EqualValues(t, 456, branch.UpdatedUnix) + if i%2 == 0 { + assert.JSONEq(t, configJSON, branch.Config) + } else if i == 1 { + assert.JSONEq(t, `{"status_check_contexts":[]}`, branch.Config) + } else { + assert.JSONEq(t, "{}", branch.Config) + } + } + tables, err := x.DBMetas() + require.NoError(t, err) + for _, table := range tables { + if table.Name == "protected_branch" { + assert.ElementsMatch(t, []string{"id", "repo_id", "branch_name", "priority", "config", "created_unix", "updated_unix"}, table.ColumnsSeq()) + } + } + _, err = x.Insert(&protectedBranchV358{RepoID: 1, BranchName: branches[0].BranchName}) + require.Error(t, err, "the repository/rule unique constraint must survive the migration") +} diff --git a/models/db/iterate.go b/models/db/iterate.go index bb83ffdd178..c8bcf890c29 100644 --- a/models/db/iterate.go +++ b/models/db/iterate.go @@ -13,8 +13,12 @@ import ( "xorm.io/xorm/schemas" ) -func iterateTableByColumn[Bean any](ctx context.Context, colName string, cond builder.Cond, f func(ctx context.Context, bean *Bean) error) error { - table, err := xormEngine.TableInfo(new(Bean)) +type sqlTableInfoProvider interface { + TableInfo(bean any) (*schemas.Table, error) +} + +func IterateTableByColumn[Bean any](ctx context.Context, tableInfo sqlTableInfoProvider, sess SQLSession, colName string, cond builder.Cond, f func(ctx context.Context, bean *Bean) error) error { + table, err := tableInfo.TableInfo(new(Bean)) if err != nil { return err } @@ -36,7 +40,7 @@ func iterateTableByColumn[Bean any](ctx context.Context, colName string, cond bu } beans := make([]*Bean, 0, batchSize) - query := GetEngine(ctx).Table(table.Name).Asc(colName) + query := sess.Table(table.Name).Asc(colName) batchCond := cond if lastColValue != nil { @@ -68,9 +72,9 @@ func iterateTableByColumn[Bean any](ctx context.Context, colName string, cond bu } func IterateByColumn[Bean any](ctx context.Context, colName string, cond builder.Cond, f func(ctx context.Context, bean *Bean) error) error { - return iterateTableByColumn(ctx, colName, cond, f) + return IterateTableByColumn(ctx, xormEngine, GetEngine(ctx), colName, cond, f) } func Iterate[Bean any](ctx context.Context, cond builder.Cond, f func(ctx context.Context, bean *Bean) error) error { - return iterateTableByColumn(ctx, "", cond, f) + return IterateTableByColumn(ctx, xormEngine, GetEngine(ctx), "", cond, f) } diff --git a/models/git/protected_branch.go b/models/git/protected_branch.go index b11da1bddbc..5bc886d7c6f 100644 --- a/models/git/protected_branch.go +++ b/models/git/protected_branch.go @@ -28,50 +28,53 @@ var ErrBranchIsProtected = util.ErrorWrap(util.ErrPermissionDenied, "branch is p // ProtectedBranch struct type ProtectedBranch struct { - ID int64 `xorm:"pk autoincr"` - RepoID int64 `xorm:"UNIQUE(s)"` - Repo *repo_model.Repository `xorm:"-"` - RuleName string `xorm:"'branch_name' UNIQUE(s)"` // a branch name or a glob match to branch name - Priority int64 `xorm:"NOT NULL DEFAULT 0"` - globRule glob.Glob `xorm:"-"` - isPlainName bool `xorm:"-"` - CanPush bool `xorm:"NOT NULL DEFAULT false"` - EnableWhitelist bool - WhitelistUserIDs []int64 `xorm:"JSON TEXT"` - WhitelistTeamIDs []int64 `xorm:"JSON TEXT"` - EnableMergeWhitelist bool `xorm:"NOT NULL DEFAULT false"` - WhitelistDeployKeys bool `xorm:"NOT NULL DEFAULT false"` - MergeWhitelistUserIDs []int64 `xorm:"JSON TEXT"` - MergeWhitelistTeamIDs []int64 `xorm:"JSON TEXT"` - EnableBypassAllowlist bool `xorm:"NOT NULL DEFAULT false"` - BypassAllowlistUserIDs []int64 `xorm:"JSON TEXT"` - BypassAllowlistTeamIDs []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"` - ApprovalsWhitelistUserIDs []int64 `xorm:"JSON TEXT"` - ApprovalsWhitelistTeamIDs []int64 `xorm:"JSON TEXT"` - RequiredApprovals int64 `xorm:"NOT NULL DEFAULT 0"` - BlockOnRejectedReviews bool `xorm:"NOT NULL DEFAULT false"` - BlockOnOfficialReviewRequests bool `xorm:"NOT NULL DEFAULT false"` - BlockOnCodeownerReviews bool `xorm:"NOT NULL DEFAULT false"` - BlockOnOutdatedBranch bool `xorm:"NOT NULL DEFAULT false"` - DismissStaleApprovals bool `xorm:"NOT NULL DEFAULT false"` - IgnoreStaleApprovals bool `xorm:"NOT NULL DEFAULT false"` - RequireSignedCommits bool `xorm:"NOT NULL DEFAULT false"` - ProtectedFilePatterns string `xorm:"TEXT"` - UnprotectedFilePatterns string `xorm:"TEXT"` - BlockAdminMergeOverride bool `xorm:"NOT NULL DEFAULT false"` + ID int64 `xorm:"pk autoincr"` + RepoID int64 `xorm:"UNIQUE(s)"` + Repo *repo_model.Repository `xorm:"-"` + RuleName string `xorm:"'branch_name' UNIQUE(s)"` // a branch name or a glob match to branch name + Priority int64 `xorm:"NOT NULL DEFAULT 0"` + globRule glob.Glob `xorm:"-"` + isPlainName bool `xorm:"-"` + ProtectedBranchConfig `xorm:"'config' JSON LONGTEXT"` CreatedUnix timeutil.TimeStamp `xorm:"created"` UpdatedUnix timeutil.TimeStamp `xorm:"updated"` } +// ProtectedBranchConfig stores protection settings in a single JSON column. +type ProtectedBranchConfig struct { + CanPush bool `json:"can_push,omitzero"` + EnableWhitelist bool `json:"enable_whitelist,omitzero"` + WhitelistUserIDs []int64 `json:"whitelist_user_ids,omitzero"` + WhitelistTeamIDs []int64 `json:"whitelist_team_ids,omitzero"` + EnableMergeWhitelist bool `json:"enable_merge_whitelist,omitzero"` + MergeWhitelistUserIDs []int64 `json:"merge_whitelist_user_ids,omitzero"` + MergeWhitelistTeamIDs []int64 `json:"merge_whitelist_team_ids,omitzero"` + EnableBypassAllowlist bool `json:"enable_bypass_allowlist,omitzero"` + BypassAllowlistUserIDs []int64 `json:"bypass_allowlist_user_ids,omitzero"` + BypassAllowlistTeamIDs []int64 `json:"bypass_allowlist_team_ids,omitzero"` + CanForcePush bool `json:"can_force_push,omitzero"` + 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"` + EnableStatusCheck bool `json:"enable_status_check,omitzero"` + StatusCheckContexts []string `json:"status_check_contexts,omitzero"` + EnableApprovalsWhitelist bool `json:"enable_approvals_whitelist,omitzero"` + ApprovalsWhitelistUserIDs []int64 `json:"approvals_whitelist_user_ids,omitzero"` + ApprovalsWhitelistTeamIDs []int64 `json:"approvals_whitelist_team_ids,omitzero"` + RequiredApprovals int64 `json:"required_approvals,omitzero"` + BlockOnRejectedReviews bool `json:"block_on_rejected_reviews,omitzero"` + BlockOnOfficialReviewRequests bool `json:"block_on_official_review_requests,omitzero"` + BlockOnCodeownerReviews bool `json:"block_on_codeowner_reviews,omitzero"` + BlockOnOutdatedBranch bool `json:"block_on_outdated_branch,omitzero"` + DismissStaleApprovals bool `json:"dismiss_stale_approvals,omitzero"` + IgnoreStaleApprovals bool `json:"ignore_stale_approvals,omitzero"` + RequireSignedCommits bool `json:"require_signed_commits,omitzero"` + ProtectedFilePatterns string `json:"protected_file_patterns,omitzero"` + UnprotectedFilePatterns string `json:"unprotected_file_patterns,omitzero"` + BlockAdminMergeOverride bool `json:"block_admin_merge_override,omitzero"` +} + func init() { db.RegisterModel(new(ProtectedBranch)) } @@ -525,13 +528,17 @@ func updateUserWhitelist(ctx context.Context, repo *repo_model.Repository, curre whitelist = make([]int64, 0, len(newWhitelist)) for _, userID := range newWhitelist { + if userID == user_model.ActionsUserID || userID == user_model.DeployKeyUserID { + whitelist = append(whitelist, userID) + continue + } user, err := user_model.GetUserByID(ctx, userID) if err != nil { - return nil, fmt.Errorf("GetUserByID [user_id: %d, repo_id: %d]: %v", userID, repo.ID, err) + return nil, fmt.Errorf("GetUserByID [user_id: %d, repo_id: %d]: %w", userID, repo.ID, err) } perm, err := access_model.GetIndividualUserRepoPermission(ctx, repo, user) if err != nil { - return nil, fmt.Errorf("GetIndividualUserRepoPermission [user_id: %d, repo_id: %d]: %v", userID, repo.ID, err) + return nil, fmt.Errorf("GetIndividualUserRepoPermission [user_id: %d, repo_id: %d]: %w", userID, repo.ID, err) } if !perm.CanWrite(unit.TypeCode) { @@ -589,7 +596,7 @@ 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, columnNames []string) error { +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) @@ -615,7 +622,7 @@ func removeIDsFromProtectedBranch(ctx context.Context, p *ProtectedBranch, userI 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 { + if _, err := db.GetEngine(ctx).ID(p.ID).Cols("config").Update(p); err != nil { return fmt.Errorf("updateProtectedBranches: %v", err) } } @@ -624,22 +631,18 @@ func removeIDsFromProtectedBranch(ctx context.Context, p *ProtectedBranch, userI // 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) + return removeIDsFromProtectedBranch(ctx, p, userID, 0) } // RemoveTeamIDFromProtectedBranch removes all team ids from protected branch options func RemoveTeamIDFromProtectedBranch(ctx context.Context, p *ProtectedBranch, teamID int64) error { - columnNames := []string{ - "whitelist_team_i_ds", - "force_push_allowlist_team_i_ds", - "merge_whitelist_team_i_ds", - "approvals_whitelist_team_i_ds", - } - return removeIDsFromProtectedBranch(ctx, p, 0, teamID, columnNames) + return removeIDsFromProtectedBranch(ctx, p, 0, teamID) +} + +func (protectBranch *ProtectedBranch) HasDeployKeyPush() bool { + return slices.Contains(protectBranch.WhitelistUserIDs, user_model.DeployKeyUserID) +} + +func (protectBranch *ProtectedBranch) HasDeployKeyForcePush() bool { + return slices.Contains(protectBranch.ForcePushAllowlistUserIDs, user_model.DeployKeyUserID) } diff --git a/models/git/protected_branch_test.go b/models/git/protected_branch_test.go index da642d6aad7..715f10f0e11 100644 --- a/models/git/protected_branch_test.go +++ b/models/git/protected_branch_test.go @@ -12,6 +12,7 @@ import ( user_model "gitea.dev/models/user" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestBranchRuleMatch(t *testing.T) { @@ -202,3 +203,28 @@ func TestCanBypassBranchProtection(t *testing.T) { // User does not bypass when not in allowlisted teams. testBypass(t, false, pb, user, false) } + +func TestProtectedBranchConfig(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + ctx := t.Context() + pb := &ProtectedBranch{ + RepoID: 1, RuleName: "config-test", + ProtectedBranchConfig: ProtectedBranchConfig{ + CanPush: true, WhitelistUserIDs: []int64{2, 4}, WhitelistTeamIDs: []int64{1, 2}, StatusCheckContexts: []string{}, + }, + } + require.NoError(t, db.Insert(ctx, pb)) + loaded := unittest.AssertExistsAndLoadBean(t, &ProtectedBranch{ID: pb.ID}) + assert.Equal(t, pb.ProtectedBranchConfig, loaded.ProtectedBranchConfig) + require.NoError(t, RemoveUserIDFromProtectedBranch(ctx, loaded, 2)) + require.NoError(t, RemoveTeamIDFromProtectedBranch(ctx, loaded, 1)) + loaded = unittest.AssertExistsAndLoadBean(t, &ProtectedBranch{ID: pb.ID}) + assert.Equal(t, []int64{4}, loaded.WhitelistUserIDs) + assert.Equal(t, []int64{2}, loaded.WhitelistTeamIDs) + assert.True(t, loaded.CanPush) + 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) +} diff --git a/models/issues/assignees.go b/models/issues/assignees.go index 622df6ac34d..994e9f0ce91 100644 --- a/models/issues/assignees.go +++ b/models/issues/assignees.go @@ -188,5 +188,5 @@ func MakeIDsFromAPIAssigneesToAdd(ctx context.Context, oneAssignee string, multi } // Get the IDs of all assignees - return user_model.GetUserIDsByNames(ctx, requestAssignees, false) + return user_model.GetDoerUserIDsByNames(ctx, requestAssignees) } diff --git a/models/issues/review_test.go b/models/issues/review_test.go index 653081c8ad0..0f074f7edb0 100644 --- a/models/issues/review_test.go +++ b/models/issues/review_test.go @@ -433,7 +433,7 @@ func TestRecalculateReviewsOfficial(t *testing.T) { // Once the reviewer is whitelisted, re-evaluating restores the official flag. rule.ApprovalsWhitelistUserIDs = []int64{2, reviewer.ID} - _, err = db.GetEngine(t.Context()).ID(rule.ID).Cols("approvals_whitelist_user_i_ds").Update(rule) + _, err = db.GetEngine(t.Context()).ID(rule.ID).Cols("config").Update(rule) assert.NoError(t, err) assert.NoError(t, issues_model.RecalculateReviewsOfficial(t.Context(), issue)) diff --git a/models/user/user.go b/models/user/user.go index a9bb8e0f5b0..674195400d0 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -560,13 +560,13 @@ func GetUserSalt() (string, error) { } type globalVarsStruct struct { - customCharsReplacement *strings.Replacer - removeCharsRE *regexp.Regexp - transformDiacritics transform.Transformer - replaceCharsHyphenRE *regexp.Regexp - emailToReplacer *strings.Replacer - systemUserNewFuncs map[int64]func() *User - systemUserNameIdMap map[string]int64 + customCharsReplacement *strings.Replacer + removeCharsRE *regexp.Regexp + transformDiacritics transform.Transformer + replaceCharsHyphenRE *regexp.Regexp + emailToReplacer *strings.Replacer + systemUserNewFuncs map[int64]func() *User + systemUserLowerNameIdMap map[string]int64 } var globalVars = sync.OnceValue(func() *globalVarsStruct { @@ -592,11 +592,11 @@ var globalVars = sync.OnceValue(func() *globalVarsStruct { userFuncs := []func() *User{NewGhostUser, NewActionsUser, NewDeployKeyUser, NewCliUser, NewAuthSourceUser} ret.systemUserNewFuncs = map[int64]func() *User{} - ret.systemUserNameIdMap = map[string]int64{} + ret.systemUserLowerNameIdMap = map[string]int64{} for _, fn := range userFuncs { u := fn() ret.systemUserNewFuncs[u.ID] = fn - ret.systemUserNameIdMap[u.LowerName] = u.ID + ret.systemUserLowerNameIdMap[u.LowerName] = u.ID } return ret }) @@ -1148,20 +1148,50 @@ func GetMailableUsersByIDs(ctx context.Context, ids []int64, isMention bool) ([] Find(&ous) } -// GetUserIDsByNames returns a slice of ids corresponds to names. -func GetUserIDsByNames(ctx context.Context, names []string, ignoreNonExistent bool) ([]int64, error) { - ids := make([]int64, 0, len(names)) +func getDoerUserIDsByNames(ctx context.Context, names []string, allowSystemDoer bool) ([]int64, error) { + namesLower := make([]string, 0, len(names)) + namesMap := make(map[string]*User, len(names)) for _, name := range names { - u, err := GetUserByName(ctx, name) - if err != nil { - if ignoreNonExistent { - continue + nameLower := strings.ToLower(name) + namesLower = append(namesLower, nameLower) + namesMap[nameLower] = nil + if allowSystemDoer { + if id, ok := globalVars().systemUserLowerNameIdMap[nameLower]; ok && id != GhostUserID { + namesMap[nameLower] = globalVars().systemUserNewFuncs[id]() } - return nil, err + } + } + + var users []*User + err := db.GetEngine(ctx).In("lower_name", namesLower).Where("`type` IN (?,?)", UserTypeIndividual, UserTypeBot).Find(&users) + if err != nil { + return nil, err + } + for _, user := range users { + namesMap[user.LowerName] = user + } + + ids := make([]int64, 0, len(names)) + var errFinal error + for _, nameLower := range namesLower { + u := namesMap[nameLower] + if u == nil { + errFinal = ErrUserNotExist{Name: nameLower} + continue } ids = append(ids, u.ID) } - return ids, nil + return ids, errFinal +} + +func GetDoerUserIDsByNamesWithSystem(ctx context.Context, names []string) ([]int64, error) { + return getDoerUserIDsByNames(ctx, names, true) +} + +// GetDoerUserIDsByNames returns a slice of ids corresponds to names. +// It always returns found user IDs even if some names are not found, but returns an error if any name is not found. +func GetDoerUserIDsByNames(ctx context.Context, names []string) ([]int64, error) { + return getDoerUserIDsByNames(ctx, names, false) } // GetUsersBySource returns a list of Users for a login source diff --git a/models/user/user_system.go b/models/user/user_system.go index 7e1b49d265e..55b8e58e0a4 100644 --- a/models/user/user_system.go +++ b/models/user/user_system.go @@ -73,7 +73,7 @@ func NewActionsUserWithTaskID(id int64) *User { } func NewDeployKeyUser() *User { - return newSystemUser(DeployKeyUserID, "(deploy-key)", "Deploy Key") + return newSystemUser(DeployKeyUserID, "gitea:deploy-key", "Deploy Key") } func GetDeployKeyUserDeployKeyID(u *User) (int64, bool) { @@ -94,17 +94,21 @@ func NewDeployKeyUserWithKeyID(id int64) *User { func NewCliUser() *User { // for audit log only - return newSystemUser(CliUserID, "(gitea-cli)", "Gitea CLI") + return newSystemUser(CliUserID, "gitea:cli", "Gitea CLI") } func NewAuthSourceUser() *User { // for audit log only - return newSystemUser(AuthSourceUserID, "(gitea-auth-source)", "Gitea Auth Source") + return newSystemUser(AuthSourceUserID, "gitea:auth-source", "Gitea Auth Source") } func GetSystemUserByName(name string) *User { lowerName := strings.ToLower(name) - uid := globalVars().systemUserNameIdMap[lowerName] + uid := globalVars().systemUserLowerNameIdMap[lowerName] + return GetSystemUserByID(uid) +} + +func GetSystemUserByID(uid int64) *User { if fn := globalVars().systemUserNewFuncs[uid]; fn != nil { return fn() } diff --git a/models/user/user_test.go b/models/user/user_test.go index 49eec8e5deb..cca2d0192a1 100644 --- a/models/user/user_test.go +++ b/models/user/user_test.go @@ -21,6 +21,7 @@ import ( "gitea.dev/modules/structs" "gitea.dev/modules/test" "gitea.dev/modules/timeutil" + "gitea.dev/modules/util" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -363,18 +364,15 @@ func TestCreateUserWithoutCustomTimestamps(t *testing.T) { assert.LessOrEqual(t, fetched.UpdatedUnix, timestampEnd) } -func TestGetUserIDsByNames(t *testing.T) { +func TestGetDoerUserIDsByNames(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) + ids, err := user_model.GetDoerUserIDsByNames(t.Context(), []string{"user1", "user2", "none_existing_user"}) + assert.ErrorIs(t, err, util.ErrNotExist) + assert.Equal(t, []int64{1, 2}, ids) - // ignore non existing - IDs, err := user_model.GetUserIDsByNames(t.Context(), []string{"user1", "user2", "none_existing_user"}, true) + ids, err = user_model.GetDoerUserIDsByNamesWithSystem(t.Context(), []string{"gitea-actions", "gitea:deploy-key"}) assert.NoError(t, err) - assert.Equal(t, []int64{1, 2}, IDs) - - // ignore non existing - IDs, err = user_model.GetUserIDsByNames(t.Context(), []string{"user1", "do_not_exist"}, false) - assert.Error(t, err) - assert.Equal(t, []int64(nil), IDs) + assert.Equal(t, []int64{-2, -3}, ids) } func TestGetMaileableUsersByIDs(t *testing.T) { diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go index 38d5f5e1907..f7461df3491 100644 --- a/routers/api/v1/repo/branch.go +++ b/routers/api/v1/repo/branch.go @@ -7,6 +7,7 @@ package repo import ( "errors" "net/http" + "slices" "gitea.dev/models/db" git_model "gitea.dev/models/git" @@ -618,6 +619,34 @@ func ListBranchProtections(ctx *context.APIContext) { ctx.JSON(http.StatusOK, apiBps) } +func legacySetDeployKeyAllowlist(userIDs []int64, enabled bool) []int64 { + if !enabled { + return util.SliceRemoveAll(userIDs, user_model.DeployKeyUserID) + } + if !slices.Contains(userIDs, user_model.DeployKeyUserID) { + userIDs = append(userIDs, user_model.DeployKeyUserID) + } + return userIDs +} + +func loadBranchProtectionUserIDs(ctx *context.APIContext, names []string, def ...[]int64) []int64 { + if names == nil { + return util.OptionalArg(def) + } + if ctx.Written() { + return nil + } + ids, err := user_model.GetDoerUserIDsByNamesWithSystem(ctx, names) + if err != nil { + if errors.Is(err, util.ErrNotExist) { + ctx.APIError(http.StatusUnprocessableEntity, err.Error()) + } else { + ctx.APIErrorInternal(err) + } + } + return ids +} + // CreateBranchProtection creates a branch protection for a repo func CreateBranchProtection(ctx *context.APIContext) { // swagger:operation POST /repos/{owner}/{repo}/branch_protections repository repoCreateBranchProtection @@ -680,54 +709,15 @@ func CreateBranchProtection(ctx *context.APIContext) { requiredApprovals = form.RequiredApprovals } - whitelistUsers, err := user_model.GetUserIDsByNames(ctx, form.PushWhitelistUsernames, false) - if err != nil { - if user_model.IsErrUserNotExist(err) { - ctx.APIError(http.StatusUnprocessableEntity, err.Error()) - return - } - ctx.APIErrorInternal(err) + whitelistUsers := loadBranchProtectionUserIDs(ctx, form.PushWhitelistUsernames) + forcePushAllowlistUsers := loadBranchProtectionUserIDs(ctx, form.ForcePushAllowlistUsernames) + mergeWhitelistUsers := loadBranchProtectionUserIDs(ctx, form.MergeWhitelistUsernames) + approvalsWhitelistUsers := loadBranchProtectionUserIDs(ctx, form.ApprovalsWhitelistUsernames) + bypassAllowlistUsers := loadBranchProtectionUserIDs(ctx, util.Iif(form.EnableBypassAllowlist, form.BypassAllowlistUsernames, nil)) + if ctx.Written() { return } - forcePushAllowlistUsers, err := user_model.GetUserIDsByNames(ctx, form.ForcePushAllowlistUsernames, false) - if err != nil { - if user_model.IsErrUserNotExist(err) { - ctx.APIError(http.StatusUnprocessableEntity, err.Error()) - return - } - ctx.APIErrorInternal(err) - return - } - mergeWhitelistUsers, err := user_model.GetUserIDsByNames(ctx, form.MergeWhitelistUsernames, false) - if err != nil { - if user_model.IsErrUserNotExist(err) { - ctx.APIError(http.StatusUnprocessableEntity, err.Error()) - return - } - ctx.APIErrorInternal(err) - return - } - approvalsWhitelistUsers, err := user_model.GetUserIDsByNames(ctx, form.ApprovalsWhitelistUsernames, false) - if err != nil { - if user_model.IsErrUserNotExist(err) { - ctx.APIError(http.StatusUnprocessableEntity, err.Error()) - return - } - ctx.APIErrorInternal(err) - return - } - var bypassAllowlistUsers []int64 - if form.EnableBypassAllowlist { - bypassAllowlistUsers, err = user_model.GetUserIDsByNames(ctx, form.BypassAllowlistUsernames, false) - if err != nil { - if user_model.IsErrUserNotExist(err) { - ctx.APIError(http.StatusUnprocessableEntity, err.Error()) - return - } - ctx.APIErrorInternal(err) - return - } - } + var whitelistTeams, forcePushAllowlistTeams, mergeWhitelistTeams, approvalsWhitelistTeams, bypassAllowlistTeams []int64 if repo.Owner.IsOrganization() { whitelistTeams, err = organization.GetTeamIDsByNames(ctx, repo.OwnerID, form.PushWhitelistTeams, false) @@ -779,32 +769,40 @@ func CreateBranchProtection(ctx *context.APIContext) { } } + if form.EnablePush && form.EnablePushWhitelist && form.PushWhitelistDeployKeys { + whitelistUsers = legacySetDeployKeyAllowlist(whitelistUsers, true) + } + if form.EnablePush && form.EnableForcePush && form.EnableForcePushAllowlist && form.ForcePushAllowlistDeployKeys { + forcePushAllowlistUsers = legacySetDeployKeyAllowlist(forcePushAllowlistUsers, true) + } + protectBranch = &git_model.ProtectedBranch{ - RepoID: ctx.Repo.Repository.ID, - RuleName: ruleName, - Priority: form.Priority, - CanPush: form.EnablePush, - EnableWhitelist: form.EnablePush && form.EnablePushWhitelist, - 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, - EnableBypassAllowlist: form.EnableBypassAllowlist, - EnableStatusCheck: form.EnableStatusCheck, - StatusCheckContexts: form.StatusCheckContexts, - EnableApprovalsWhitelist: form.EnableApprovalsWhitelist, - RequiredApprovals: requiredApprovals, - BlockOnRejectedReviews: form.BlockOnRejectedReviews, - BlockOnOfficialReviewRequests: form.BlockOnOfficialReviewRequests, - BlockOnCodeownerReviews: form.BlockOnCodeownerReviews, - DismissStaleApprovals: form.DismissStaleApprovals, - IgnoreStaleApprovals: form.IgnoreStaleApprovals, - RequireSignedCommits: form.RequireSignedCommits, - ProtectedFilePatterns: form.ProtectedFilePatterns, - UnprotectedFilePatterns: form.UnprotectedFilePatterns, - BlockOnOutdatedBranch: form.BlockOnOutdatedBranch, - BlockAdminMergeOverride: form.BlockAdminMergeOverride, + ProtectedBranchConfig: git_model.ProtectedBranchConfig{ + CanPush: form.EnablePush, + EnableWhitelist: form.EnablePush && form.EnablePushWhitelist, + CanForcePush: form.EnablePush && form.EnableForcePush, + EnableForcePushAllowlist: form.EnablePush && form.EnableForcePush && form.EnableForcePushAllowlist, + EnableMergeWhitelist: form.EnableMergeWhitelist, + EnableBypassAllowlist: form.EnableBypassAllowlist, + EnableStatusCheck: form.EnableStatusCheck, + StatusCheckContexts: form.StatusCheckContexts, + EnableApprovalsWhitelist: form.EnableApprovalsWhitelist, + RequiredApprovals: requiredApprovals, + BlockOnRejectedReviews: form.BlockOnRejectedReviews, + BlockOnOfficialReviewRequests: form.BlockOnOfficialReviewRequests, + BlockOnCodeownerReviews: form.BlockOnCodeownerReviews, + DismissStaleApprovals: form.DismissStaleApprovals, + IgnoreStaleApprovals: form.IgnoreStaleApprovals, + RequireSignedCommits: form.RequireSignedCommits, + ProtectedFilePatterns: form.ProtectedFilePatterns, + UnprotectedFilePatterns: form.UnprotectedFilePatterns, + BlockOnOutdatedBranch: form.BlockOnOutdatedBranch, + BlockAdminMergeOverride: form.BlockAdminMergeOverride, + }, + + RepoID: ctx.Repo.Repository.ID, + RuleName: ruleName, + Priority: form.Priority, } if err := pull_service.CreateOrUpdateProtectedBranch(ctx, ctx.Repo.Repository, protectBranch, git_model.WhitelistOptions{ @@ -819,18 +817,17 @@ func CreateBranchProtection(ctx *context.APIContext) { BypassUserIDs: bypassAllowlistUsers, BypassTeamIDs: bypassAllowlistTeams, }); err != nil { - ctx.APIErrorInternal(err) + ctx.APIErrorAuto(err) return } // Reload from db to get all whitelists bp, err := git_model.GetProtectedBranchRuleByName(ctx, ctx.Repo.Repository.ID, ruleName) if err != nil { - ctx.APIErrorInternal(err) + ctx.APIErrorAuto(err) return - } - if bp == nil || bp.RepoID != ctx.Repo.Repository.ID { - ctx.APIErrorInternal(err) + } else if bp == nil { + ctx.APIErrorNotFound() return } @@ -888,21 +885,23 @@ func EditBranchProtection(ctx *context.APIContext) { return } + pushDeployKeys := protectBranch.HasDeployKeyPush() + forcePushDeployKeys := protectBranch.HasDeployKeyForcePush() if form.EnablePush != nil { if !*form.EnablePush { protectBranch.CanPush = false protectBranch.EnableWhitelist = false - protectBranch.WhitelistDeployKeys = false + pushDeployKeys = false } else { protectBranch.CanPush = true if form.EnablePushWhitelist != nil { if !*form.EnablePushWhitelist { protectBranch.EnableWhitelist = false - protectBranch.WhitelistDeployKeys = false + pushDeployKeys = false } else { protectBranch.EnableWhitelist = true if form.PushWhitelistDeployKeys != nil { - protectBranch.WhitelistDeployKeys = *form.PushWhitelistDeployKeys + pushDeployKeys = *form.PushWhitelistDeployKeys } } } @@ -913,17 +912,17 @@ func EditBranchProtection(ctx *context.APIContext) { if !*form.EnableForcePush { protectBranch.CanForcePush = false protectBranch.EnableForcePushAllowlist = false - protectBranch.ForcePushAllowlistDeployKeys = false + forcePushDeployKeys = false } else { protectBranch.CanForcePush = true if form.EnableForcePushAllowlist != nil { if !*form.EnableForcePushAllowlist { protectBranch.EnableForcePushAllowlist = false - protectBranch.ForcePushAllowlistDeployKeys = false + forcePushDeployKeys = false } else { protectBranch.EnableForcePushAllowlist = true if form.ForcePushAllowlistDeployKeys != nil { - protectBranch.ForcePushAllowlistDeployKeys = *form.ForcePushAllowlistDeployKeys + forcePushDeployKeys = *form.ForcePushAllowlistDeployKeys } } } @@ -998,72 +997,23 @@ func EditBranchProtection(ctx *context.APIContext) { protectBranch.BlockAdminMergeOverride = *form.BlockAdminMergeOverride } - var whitelistUsers, forcePushAllowlistUsers, mergeWhitelistUsers, approvalsWhitelistUsers, bypassAllowlistUsers []int64 - if form.PushWhitelistUsernames != nil { - whitelistUsers, err = user_model.GetUserIDsByNames(ctx, form.PushWhitelistUsernames, false) - if err != nil { - if user_model.IsErrUserNotExist(err) { - ctx.APIError(http.StatusUnprocessableEntity, err.Error()) - return - } - ctx.APIErrorInternal(err) - return - } - } else { - whitelistUsers = protectBranch.WhitelistUserIDs + whitelistUsers := loadBranchProtectionUserIDs(ctx, form.PushWhitelistUsernames, protectBranch.WhitelistUserIDs) + forcePushAllowlistUsers := loadBranchProtectionUserIDs(ctx, form.ForcePushAllowlistUsernames, protectBranch.ForcePushAllowlistUserIDs) + mergeWhitelistUsers := loadBranchProtectionUserIDs(ctx, form.MergeWhitelistUsernames, protectBranch.MergeWhitelistUserIDs) + approvalsWhitelistUsers := loadBranchProtectionUserIDs(ctx, form.ApprovalsWhitelistUsernames, protectBranch.ApprovalsWhitelistUserIDs) + bypassAllowlistUsers := loadBranchProtectionUserIDs(ctx, form.BypassAllowlistUsernames, protectBranch.BypassAllowlistUserIDs) + if ctx.Written() { + return } - if form.ForcePushAllowlistUsernames != nil { - forcePushAllowlistUsers, err = user_model.GetUserIDsByNames(ctx, form.ForcePushAllowlistUsernames, false) - if err != nil { - if user_model.IsErrUserNotExist(err) { - ctx.APIError(http.StatusUnprocessableEntity, err.Error()) - return - } - ctx.APIErrorInternal(err) - return - } - } else { - forcePushAllowlistUsers = protectBranch.ForcePushAllowlistUserIDs + + if form.PushWhitelistUsernames != nil && form.PushWhitelistDeployKeys == nil && slices.Contains(whitelistUsers, user_model.DeployKeyUserID) { + pushDeployKeys = true } - if form.MergeWhitelistUsernames != nil { - mergeWhitelistUsers, err = user_model.GetUserIDsByNames(ctx, form.MergeWhitelistUsernames, false) - if err != nil { - if user_model.IsErrUserNotExist(err) { - ctx.APIError(http.StatusUnprocessableEntity, err.Error()) - return - } - ctx.APIErrorInternal(err) - return - } - } else { - mergeWhitelistUsers = protectBranch.MergeWhitelistUserIDs - } - if form.ApprovalsWhitelistUsernames != nil { - approvalsWhitelistUsers, err = user_model.GetUserIDsByNames(ctx, form.ApprovalsWhitelistUsernames, false) - if err != nil { - if user_model.IsErrUserNotExist(err) { - ctx.APIError(http.StatusUnprocessableEntity, err.Error()) - return - } - ctx.APIErrorInternal(err) - return - } - } else { - approvalsWhitelistUsers = protectBranch.ApprovalsWhitelistUserIDs - } - if form.BypassAllowlistUsernames != nil { - bypassAllowlistUsers, err = user_model.GetUserIDsByNames(ctx, form.BypassAllowlistUsernames, false) - if err != nil { - if user_model.IsErrUserNotExist(err) { - ctx.APIError(http.StatusUnprocessableEntity, err.Error()) - return - } - ctx.APIErrorInternal(err) - return - } - } else { - bypassAllowlistUsers = protectBranch.BypassAllowlistUserIDs + if form.ForcePushAllowlistUsernames != nil && form.ForcePushAllowlistDeployKeys == nil && slices.Contains(forcePushAllowlistUsers, user_model.DeployKeyUserID) { + forcePushDeployKeys = true } + whitelistUsers = legacySetDeployKeyAllowlist(whitelistUsers, pushDeployKeys) + forcePushAllowlistUsers = legacySetDeployKeyAllowlist(forcePushAllowlistUsers, forcePushDeployKeys) var whitelistTeams, forcePushAllowlistTeams, mergeWhitelistTeams, approvalsWhitelistTeams, bypassAllowlistTeams []int64 if repo.Owner.IsOrganization() { @@ -1151,7 +1101,7 @@ func EditBranchProtection(ctx *context.APIContext) { BypassTeamIDs: bypassAllowlistTeams, }) if err != nil { - ctx.APIErrorInternal(err) + ctx.APIErrorAuto(err) return } @@ -1199,11 +1149,10 @@ func EditBranchProtection(ctx *context.APIContext) { // Reload from db to ensure get all whitelists bp, err := git_model.GetProtectedBranchRuleByName(ctx, repo.ID, bpName) if err != nil { - ctx.APIErrorInternal(err) + ctx.APIErrorAuto(err) return - } - if bp == nil || bp.RepoID != ctx.Repo.Repository.ID { - ctx.APIErrorInternal(err) + } else if bp == nil { + ctx.APIErrorNotFound() return } diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go index a8a6bb1ddf2..dd951232eef 100644 --- a/routers/api/v1/repo/issue.go +++ b/routers/api/v1/repo/issue.go @@ -584,8 +584,8 @@ func CreateIssue(ctx *context.APIContext) { issue.MilestoneID = form.Milestone assigneeIDs, err = issues_model.MakeIDsFromAPIAssigneesToAdd(ctx, form.Assignee, form.Assignees) if err != nil { - if user_model.IsErrUserNotExist(err) { - ctx.APIError(http.StatusUnprocessableEntity, fmt.Sprintf("Assignee does not exist: [name: %s]", err)) + if errors.Is(err, util.ErrNotExist) { + ctx.APIError(http.StatusUnprocessableEntity, fmt.Sprintf("Assignee does not exist: %s", err)) } else { ctx.APIErrorInternal(err) } diff --git a/routers/api/v1/repo/issue_assignee.go b/routers/api/v1/repo/issue_assignee.go index adc0240eee1..e37190dd6c4 100644 --- a/routers/api/v1/repo/issue_assignee.go +++ b/routers/api/v1/repo/issue_assignee.go @@ -4,6 +4,7 @@ package repo import ( + "errors" "net/http" issues_model "gitea.dev/models/issues" @@ -11,6 +12,7 @@ import ( repo_model "gitea.dev/models/repo" user_model "gitea.dev/models/user" api "gitea.dev/modules/structs" + "gitea.dev/modules/util" "gitea.dev/modules/web" "gitea.dev/services/context" "gitea.dev/services/convert" @@ -203,13 +205,13 @@ func updateIssueAssignees(ctx *context.APIContext, opts api.IssueAssigneesOption return } - assigneeIDs, err := user_model.GetUserIDsByNames(ctx, opts.Assignees, false) + assigneeIDs, err := user_model.GetDoerUserIDsByNames(ctx, opts.Assignees) if err != nil { - if user_model.IsErrUserNotExist(err) { + if errors.Is(err, util.ErrNotExist) { ctx.APIError(http.StatusUnprocessableEntity, err.Error()) - return + } else { + ctx.APIErrorAuto(err) } - ctx.APIErrorAuto(err) return } diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 4cda175700d..61b6367d3e1 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -521,7 +521,7 @@ func CreatePullRequest(ctx *context.APIContext) { // Get all assignee IDs assigneeIDs, err := issues_model.MakeIDsFromAPIAssigneesToAdd(ctx, form.Assignee, form.Assignees) if err != nil { - if user_model.IsErrUserNotExist(err) { + if errors.Is(err, util.ErrNotExist) { ctx.APIError(http.StatusUnprocessableEntity, fmt.Sprintf("Assignee does not exist: [name: %s]", err)) } else { ctx.APIErrorInternal(err) diff --git a/routers/api/v1/repo/tag.go b/routers/api/v1/repo/tag.go index 0d4d4553b4f..c4853f41388 100644 --- a/routers/api/v1/repo/tag.go +++ b/routers/api/v1/repo/tag.go @@ -14,6 +14,7 @@ import ( repo_model "gitea.dev/models/repo" user_model "gitea.dev/models/user" api "gitea.dev/modules/structs" + "gitea.dev/modules/util" "gitea.dev/modules/web" "gitea.dev/routers/api/v1/utils" "gitea.dev/services/context" @@ -435,13 +436,13 @@ func CreateTagProtection(ctx *context.APIContext) { } var whitelistUsers, whitelistTeams []int64 - whitelistUsers, err = user_model.GetUserIDsByNames(ctx, form.WhitelistUsernames, false) + whitelistUsers, err = user_model.GetDoerUserIDsByNames(ctx, form.WhitelistUsernames) if err != nil { - if user_model.IsErrUserNotExist(err) { + if errors.Is(err, util.ErrNotExist) { ctx.APIError(http.StatusUnprocessableEntity, err.Error()) - return + } else { + ctx.APIErrorInternal(err) } - ctx.APIErrorInternal(err) return } @@ -557,13 +558,13 @@ func EditTagProtection(ctx *context.APIContext) { } if form.WhitelistUsernames != nil { - whitelistUsers, err = user_model.GetUserIDsByNames(ctx, form.WhitelistUsernames, false) + whitelistUsers, err = user_model.GetDoerUserIDsByNames(ctx, form.WhitelistUsernames) if err != nil { - if user_model.IsErrUserNotExist(err) { + if errors.Is(err, util.ErrNotExist) { ctx.APIError(http.StatusUnprocessableEntity, err.Error()) - return + } else { + ctx.APIErrorInternal(err) } - ctx.APIErrorInternal(err) return } pt.AllowlistUserIDs = whitelistUsers diff --git a/routers/private/hook_pre_receive.go b/routers/private/hook_pre_receive.go index c4ce50c787e..9d9633957fe 100644 --- a/routers/private/hook_pre_receive.go +++ b/routers/private/hook_pre_receive.go @@ -11,7 +11,6 @@ import ( git_model "gitea.dev/models/git" issues_model "gitea.dev/models/issues" "gitea.dev/models/unit" - user_model "gitea.dev/models/user" "gitea.dev/modules/git" "gitea.dev/modules/git/gitcmd" "gitea.dev/modules/private" @@ -220,19 +219,10 @@ 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 ctx.opts.UserID == user_model.DeployKeyUserID { - // 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) - } + if isForcePush { + canPush = !changedProtectedfiles && protectBranch.CanUserForcePush(ctx, ctx.Doer, ctx.Repo.Permission) } else { - if isForcePush { - 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 diff --git a/routers/web/repo/issue_new.go b/routers/web/repo/issue_new.go index efbf6354bc1..d3c401df16d 100644 --- a/routers/web/repo/issue_new.go +++ b/routers/web/repo/issue_new.go @@ -78,10 +78,9 @@ func setTemplateIfExists(ctx *context.Context, ctxDataKey string, possibleFiles metaData.LabelsData.SetSelectedLabelNames(template.Labels) selectedAssigneeIDStrings := make([]string, 0, len(template.Assignees)) - if userIDs, err := user_model.GetUserIDsByNames(ctx, template.Assignees, true); err == nil { - for _, userID := range userIDs { - selectedAssigneeIDStrings = append(selectedAssigneeIDStrings, strconv.FormatInt(userID, 10)) - } + selectedAssigneeIDs, _ := user_model.GetDoerUserIDsByNames(ctx, template.Assignees) + for _, userID := range selectedAssigneeIDs { + selectedAssigneeIDStrings = append(selectedAssigneeIDStrings, strconv.FormatInt(userID, 10)) } metaData.AssigneesData.SelectedAssigneeIDs = strings.Join(selectedAssigneeIDStrings, ",") diff --git a/routers/web/repo/setting/protected_branch.go b/routers/web/repo/setting/protected_branch.go index 045513a321c..e71a14a1fdd 100644 --- a/routers/web/repo/setting/protected_branch.go +++ b/routers/web/repo/setting/protected_branch.go @@ -19,6 +19,7 @@ import ( access_model "gitea.dev/models/perm/access" repo_model "gitea.dev/models/repo" "gitea.dev/models/unit" + user_model "gitea.dev/models/user" "gitea.dev/modules/base" "gitea.dev/modules/glob" "gitea.dev/modules/json" @@ -76,12 +77,21 @@ func SettingsProtectedBranch(c *context.Context) { c.Data["PageIsSettingsBranches"] = true c.Data["Title"] = c.Locale.TrString("repo.settings.protected_branch") + " - " + rule.RuleName - users, err := access_model.GetUsersWithAnyUnitAccess(c, c.Repo.Repository, perm.AccessModeRead, unit.TypeCode, unit.TypePullRequests) + + repoUsers, err := access_model.GetUsersWithAnyUnitAccess(c, c.Repo.Repository, perm.AccessModeRead, unit.TypeCode, unit.TypePullRequests) if err != nil { c.ServerError("GetUsersWithUnitAccess", err) return } - c.Data["Users"] = users + + pushUsers := []*user_model.User{user_model.NewActionsUser(), user_model.NewDeployKeyUser()} + pushUsers = append(pushUsers, repoUsers...) + + c.Data["PushUsers"] = pushUsers + c.Data["ForcePushUsers"] = 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["merge_whitelist_users"] = strings.Join(base.Int64sToStrings(rule.MergeWhitelistUserIDs), ",") @@ -173,11 +183,9 @@ func SettingsProtectedBranchPost(ctx *context.Context) { case "all": protectBranch.CanPush = true protectBranch.EnableWhitelist = false - protectBranch.WhitelistDeployKeys = false case "whitelist": protectBranch.CanPush = true protectBranch.EnableWhitelist = true - protectBranch.WhitelistDeployKeys = f.WhitelistDeployKeys if strings.TrimSpace(f.WhitelistUsers) != "" { whitelistUsers, _ = base.StringsToInt64s(strings.Split(f.WhitelistUsers, ",")) } @@ -187,18 +195,15 @@ func SettingsProtectedBranchPost(ctx *context.Context) { default: protectBranch.CanPush = false protectBranch.EnableWhitelist = false - 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, ",")) } @@ -208,7 +213,6 @@ func SettingsProtectedBranchPost(ctx *context.Context) { default: protectBranch.CanForcePush = false protectBranch.EnableForcePushAllowlist = false - protectBranch.ForcePushAllowlistDeployKeys = false } protectBranch.EnableMergeWhitelist = f.EnableMergeWhitelist diff --git a/services/convert/convert.go b/services/convert/convert.go index e1b848a5f3e..b403aeee583 100644 --- a/services/convert/convert.go +++ b/services/convert/convert.go @@ -120,24 +120,31 @@ func ToBranch(ctx context.Context, repo *repo_model.Repository, branchName strin return branch, nil } -// getWhitelistEntities returns the names of the entities that are in the whitelist -func getWhitelistEntities[T *user_model.User | *organization.Team](entities []T, whitelistIDs []int64) []string { - whitelistUserIDsSet := container.SetOf(whitelistIDs...) - whitelistNames := make([]string, 0) - for _, entity := range entities { - switch v := any(entity).(type) { - case *user_model.User: - if whitelistUserIDsSet.Contains(v.ID) { - whitelistNames = append(whitelistNames, v.Name) - } - case *organization.Team: - if whitelistUserIDsSet.Contains(v.ID) { - whitelistNames = append(whitelistNames, v.Name) - } +func getAllowListUserNames(users []*user_model.User, ids []int64) []string { + userIdMap := map[int64]string{} + for _, user := range users { + userIdMap[user.ID] = user.Name + } + names := make([]string, 0, len(ids)) + for _, id := range ids { + if name, ok := userIdMap[id]; ok { + names = append(names, name) + } else if u := user_model.GetSystemUserByID(id); u != nil { + names = append(names, u.Name) } } + return names +} - return whitelistNames +func getAllowListTeamNames(teams []*organization.Team, ids []int64) []string { + idSet := container.SetOf(ids...) + names := make([]string, 0, len(ids)) + for _, team := range teams { + if idSet.Contains(team.ID) { + names = append(names, team.Name) + } + } + return names } // ToBranchProtection convert a ProtectedBranch to api.BranchProtection @@ -147,22 +154,22 @@ func ToBranchProtection(ctx context.Context, bp *git_model.ProtectedBranch, repo log.Error("GetRepoReaders: %v", err) } - pushWhitelistUsernames := getWhitelistEntities(readers, bp.WhitelistUserIDs) - forcePushAllowlistUsernames := getWhitelistEntities(readers, bp.ForcePushAllowlistUserIDs) - mergeWhitelistUsernames := getWhitelistEntities(readers, bp.MergeWhitelistUserIDs) - approvalsWhitelistUsernames := getWhitelistEntities(readers, bp.ApprovalsWhitelistUserIDs) - bypassAllowlistUsernames := getWhitelistEntities(readers, bp.BypassAllowlistUserIDs) + pushWhitelistUsernames := getAllowListUserNames(readers, bp.WhitelistUserIDs) + forcePushAllowlistUsernames := getAllowListUserNames(readers, bp.ForcePushAllowlistUserIDs) + mergeWhitelistUsernames := getAllowListUserNames(readers, bp.MergeWhitelistUserIDs) + approvalsWhitelistUsernames := getAllowListUserNames(readers, bp.ApprovalsWhitelistUserIDs) + bypassAllowlistUsernames := getAllowListUserNames(readers, bp.BypassAllowlistUserIDs) teamReaders, err := organization.GetTeamsWithAccessToAnyRepoUnit(ctx, repo.Owner.ID, repo.ID, perm.AccessModeRead, unit.TypeCode, unit.TypePullRequests) if err != nil { log.Error("Repo.Owner.TeamsWithAccessToRepo: %v", err) } - pushWhitelistTeams := getWhitelistEntities(teamReaders, bp.WhitelistTeamIDs) - forcePushAllowlistTeams := getWhitelistEntities(teamReaders, bp.ForcePushAllowlistTeamIDs) - mergeWhitelistTeams := getWhitelistEntities(teamReaders, bp.MergeWhitelistTeamIDs) - approvalsWhitelistTeams := getWhitelistEntities(teamReaders, bp.ApprovalsWhitelistTeamIDs) - bypassAllowlistTeams := getWhitelistEntities(teamReaders, bp.BypassAllowlistTeamIDs) + pushWhitelistTeams := getAllowListTeamNames(teamReaders, bp.WhitelistTeamIDs) + forcePushAllowlistTeams := getAllowListTeamNames(teamReaders, bp.ForcePushAllowlistTeamIDs) + mergeWhitelistTeams := getAllowListTeamNames(teamReaders, bp.MergeWhitelistTeamIDs) + approvalsWhitelistTeams := getAllowListTeamNames(teamReaders, bp.ApprovalsWhitelistTeamIDs) + bypassAllowlistTeams := getAllowListTeamNames(teamReaders, bp.BypassAllowlistTeamIDs) branchName := "" if !git_model.IsRuleNameSpecial(bp.RuleName) { @@ -177,12 +184,12 @@ func ToBranchProtection(ctx context.Context, bp *git_model.ProtectedBranch, repo EnablePushWhitelist: bp.EnableWhitelist, PushWhitelistUsernames: pushWhitelistUsernames, PushWhitelistTeams: pushWhitelistTeams, - PushWhitelistDeployKeys: bp.WhitelistDeployKeys, + PushWhitelistDeployKeys: bp.HasDeployKeyPush(), EnableForcePush: bp.CanForcePush, EnableForcePushAllowlist: bp.EnableForcePushAllowlist, ForcePushAllowlistUsernames: forcePushAllowlistUsernames, ForcePushAllowlistTeams: forcePushAllowlistTeams, - ForcePushAllowlistDeployKeys: bp.ForcePushAllowlistDeployKeys, + ForcePushAllowlistDeployKeys: bp.HasDeployKeyForcePush(), EnableMergeWhitelist: bp.EnableMergeWhitelist, MergeWhitelistUsernames: mergeWhitelistUsernames, MergeWhitelistTeams: mergeWhitelistTeams, @@ -970,14 +977,14 @@ func ToTagProtection(ctx context.Context, pt *git_model.ProtectedTag, repo *repo log.Error("GetRepoReaders: %v", err) } - whitelistUsernames := getWhitelistEntities(readers, pt.AllowlistUserIDs) + whitelistUsernames := getAllowListUserNames(readers, pt.AllowlistUserIDs) teamReaders, err := organization.GetTeamsWithAccessToAnyRepoUnit(ctx, repo.Owner.ID, repo.ID, perm.AccessModeRead, unit.TypeCode, unit.TypePullRequests) if err != nil { log.Error("Repo.Owner.TeamsWithAccessToRepo: %v", err) } - whitelistTeams := getWhitelistEntities(teamReaders, pt.AllowlistTeamIDs) + whitelistTeams := getAllowListTeamNames(teamReaders, pt.AllowlistTeamIDs) return &api.TagProtection{ ID: pt.ID, diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index 49f64bea63c..38351b6a2fe 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -156,11 +156,9 @@ type ProtectBranchForm struct { EnablePush string WhitelistUsers string WhitelistTeams string - WhitelistDeployKeys bool EnableForcePush string ForcePushAllowlistUsers string ForcePushAllowlistTeams string - ForcePushAllowlistDeployKeys bool EnableMergeWhitelist bool MergeWhitelistUsers string MergeWhitelistTeams string diff --git a/templates/repo/settings/protected_branch.tmpl b/templates/repo/settings/protected_branch.tmpl index 715e3a2a79c..42872ecbfee 100644 --- a/templates/repo/settings/protected_branch.tmpl +++ b/templates/repo/settings/protected_branch.tmpl @@ -53,7 +53,7 @@
- {{template "repo/settings/user_dropdown" (dict "Name" "whitelist_users" "Value" .whitelist_users "Users" .Users)}} + {{template "repo/settings/user_dropdown" (dict "Name" "whitelist_users" "Value" .whitelist_users "Users" .PushUsers)}}
{{if .Owner.IsOrganization}}
@@ -61,12 +61,6 @@ {{template "repo/settings/team_dropdown" (dict "Name" "whitelist_teams" "Value" .whitelist_teams "Teams" .Teams)}}
{{end}} -
-
- - -
-
@@ -102,7 +96,7 @@
- {{template "repo/settings/user_dropdown" (dict "Name" "force_push_allowlist_users" "Value" .force_push_allowlist_users "Users" .Users)}} + {{template "repo/settings/user_dropdown" (dict "Name" "force_push_allowlist_users" "Value" .force_push_allowlist_users "Users" .ForcePushUsers)}}
{{if .Owner.IsOrganization}}
@@ -110,12 +104,6 @@ {{template "repo/settings/team_dropdown" (dict "Name" "force_push_allowlist_teams" "Value" .force_push_allowlist_teams "Teams" .Teams)}}
{{end}} -
-
- - -
-
{{ctx.Locale.Tr "repo.settings.event_pull_request_approvals"}}
@@ -135,7 +123,7 @@
- {{template "repo/settings/user_dropdown" (dict "Name" "approvals_whitelist_users" "Value" .approvals_whitelist_users "Users" .Users)}} + {{template "repo/settings/user_dropdown" (dict "Name" "approvals_whitelist_users" "Value" .approvals_whitelist_users "Users" .ApprovalUsers)}}
{{if .Owner.IsOrganization}}
@@ -211,7 +199,7 @@
- {{template "repo/settings/user_dropdown" (dict "Name" "merge_whitelist_users" "Value" .merge_whitelist_users "Users" .Users)}} + {{template "repo/settings/user_dropdown" (dict "Name" "merge_whitelist_users" "Value" .merge_whitelist_users "Users" .MergeUsers)}}
{{if .Owner.IsOrganization}}
@@ -233,7 +221,7 @@
- {{template "repo/settings/user_dropdown" (dict "Name" "bypass_allowlist_users" "Value" .bypass_allowlist_users "Users" .Users)}} + {{template "repo/settings/user_dropdown" (dict "Name" "bypass_allowlist_users" "Value" .bypass_allowlist_users "Users" .BypassCheckUsers)}}
{{if .Owner.IsOrganization}}
diff --git a/tests/integration/api_branch_test.go b/tests/integration/api_branch_test.go index c4946f05e37..431ab4890f8 100644 --- a/tests/integration/api_branch_test.go +++ b/tests/integration/api_branch_test.go @@ -18,6 +18,7 @@ import ( "gitea.dev/tests" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func testAPIGetBranch(t *testing.T, branchName string, exists bool) { @@ -472,3 +473,78 @@ func TestAPICreateBranchWithSyncBranches(t *testing.T) { assert.NoError(t, err) assert.Len(t, branches, 1) } + +func TestAPIBranchProtectionDeployKeyAllowlists(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + token := getUserToken(t, "user2", auth_model.AccessTokenScopeWriteRepository) + path := "/api/v1/repos/user2/repo1/branch_protections" + createProtection := func(t *testing.T, ruleName string, deployKeys bool) *api.BranchProtection { + t.Helper() + resp := MakeRequest(t, NewRequestWithJSON(t, "POST", path, &api.CreateBranchProtectionOption{ + RuleName: ruleName, EnablePush: true, EnablePushWhitelist: true, PushWhitelistDeployKeys: deployKeys, + EnableForcePush: true, EnableForcePushAllowlist: true, ForcePushAllowlistDeployKeys: deployKeys, + }).AddTokenAuth(token), http.StatusCreated) + return DecodeJSON(t, resp, &api.BranchProtection{}) + } + + t.Run("CreateWithLegacyFlags", func(t *testing.T) { + bp := createProtection(t, "legacy-flags", true) + assert.True(t, bp.PushWhitelistDeployKeys) + assert.True(t, bp.ForcePushAllowlistDeployKeys) + }) + + t.Run("PreserveDeployKeysWhenUpdatingUsernames", func(t *testing.T) { + createProtection(t, "update-usernames", true) + resp := MakeRequest(t, NewRequestWithJSON(t, "PATCH", path+"/update-usernames", map[string]any{ + "push_whitelist_usernames": []string{"user2"}, "force_push_allowlist_usernames": []string{}, + }).AddTokenAuth(token), http.StatusOK) + bp := DecodeJSON(t, resp, &api.BranchProtection{}) + assert.True(t, bp.PushWhitelistDeployKeys) + assert.True(t, bp.ForcePushAllowlistDeployKeys) + assert.Equal(t, []string{"user2", "gitea:deploy-key"}, bp.PushWhitelistUsernames) + }) + + t.Run("LegacyFlagsDisableDeployKeys", func(t *testing.T) { + createProtection(t, "disable-deploy-keys", true) + resp := MakeRequest(t, NewRequestWithJSON(t, "PATCH", path+"/disable-deploy-keys", map[string]any{ + "enable_push": true, "enable_push_whitelist": true, "push_whitelist_deploy_keys": false, + "enable_force_push": true, "enable_force_push_allowlist": true, "force_push_allowlist_deploy_keys": false, + "push_whitelist_usernames": []string{"user2", "gitea:deploy-key"}, "force_push_allowlist_usernames": []string{"gitea:deploy-key"}, + }).AddTokenAuth(token), http.StatusOK) + bp := DecodeJSON(t, resp, &api.BranchProtection{}) + assert.False(t, bp.PushWhitelistDeployKeys) + assert.False(t, bp.ForcePushAllowlistDeployKeys) + assert.Equal(t, []string{"user2"}, bp.PushWhitelistUsernames) + rule, err := git_model.GetProtectedBranchRuleByName(t.Context(), 1, "disable-deploy-keys") + require.NoError(t, err) + require.NotNil(t, rule) + assert.Equal(t, []int64{2}, rule.WhitelistUserIDs) + assert.Empty(t, rule.ForcePushAllowlistUserIDs) + }) + + t.Run("AddDeployKeysByUsername", func(t *testing.T) { + createProtection(t, "add-deploy-keys", false) + resp := MakeRequest(t, NewRequestWithJSON(t, "PATCH", path+"/add-deploy-keys", map[string]any{ + "push_whitelist_usernames": []string{"user2", "gitea:deploy-key"}, "force_push_allowlist_usernames": []string{"gitea:deploy-key"}, + }).AddTokenAuth(token), http.StatusOK) + bp := DecodeJSON(t, resp, &api.BranchProtection{}) + assert.True(t, bp.PushWhitelistDeployKeys) + assert.True(t, bp.ForcePushAllowlistDeployKeys) + assert.Equal(t, []string{"user2", "gitea:deploy-key"}, bp.PushWhitelistUsernames) + assert.Equal(t, []string{"gitea:deploy-key"}, bp.ForcePushAllowlistUsernames) + }) + + t.Run("CreateWithDeployKeyUsernames", func(t *testing.T) { + resp := MakeRequest(t, NewRequestWithJSON(t, "POST", path, &api.CreateBranchProtectionOption{ + RuleName: "deploy-key-usernames", EnablePush: true, EnablePushWhitelist: true, + PushWhitelistUsernames: []string{"gitea:deploy-key"}, EnableForcePush: true, EnableForcePushAllowlist: true, + ForcePushAllowlistUsernames: []string{"gitea:deploy-key"}, + }).AddTokenAuth(token), http.StatusCreated) + bp := DecodeJSON(t, resp, &api.BranchProtection{}) + assert.True(t, bp.PushWhitelistDeployKeys) + assert.True(t, bp.ForcePushAllowlistDeployKeys) + assert.Equal(t, []string{"gitea:deploy-key"}, bp.PushWhitelistUsernames) + assert.Equal(t, []string{"gitea:deploy-key"}, bp.ForcePushAllowlistUsernames) + }) +} diff --git a/tests/integration/api_issue_test.go b/tests/integration/api_issue_test.go index 69c5c412cfe..28c5a477aec 100644 --- a/tests/integration/api_issue_test.go +++ b/tests/integration/api_issue_test.go @@ -537,7 +537,7 @@ func testAPIIssueAssignees(t *testing.T) { t.Run("OrganizationAssignee", func(t *testing.T) { req := NewRequestWithJSON(t, "POST", urlStr, &api.IssueAssigneesOption{Assignees: []string{"org3"}}). AddTokenAuth(token) - MakeRequest(t, req, http.StatusBadRequest) + MakeRequest(t, req, http.StatusUnprocessableEntity) checkReq := NewRequest(t, "GET", fmt.Sprintf("%s/%s", urlStr, "org3")).AddTokenAuth(token) MakeRequest(t, checkReq, http.StatusBadRequest)