From 77d1c7bf2fbdc217cef564417c8fd82612834f6e Mon Sep 17 00:00:00 2001 From: Norwin Date: Tue, 22 Mar 2022 02:09:45 +0100 Subject: [PATCH] Cleanup protected branches when deleting users & teams (#19158) * Clean up protected_branches when deleting user fixes #19094 * Clean up protected_branches when deleting teams * fix issue Co-authored-by: Lauris BH --- models/org_team.go | 42 ++++++++++++++++++++++++++++++++++++++-- models/user.go | 45 +++++++++++++++++++++++++++++++++++++++++++ modules/util/slice.go | 18 +++++++++++++++++ 3 files changed, 103 insertions(+), 2 deletions(-) create mode 100644 modules/util/slice.go diff --git a/models/org_team.go b/models/org_team.go index 17f95bb5b0..faee23f4f8 100644 --- a/models/org_team.go +++ b/models/org_team.go @@ -19,6 +19,7 @@ import ( user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" "xorm.io/builder" ) @@ -776,8 +777,45 @@ func DeleteTeam(t *Team) error { return err } - if err := t.removeAllRepositories(ctx); err != nil { - return err + // update branch protections + { + protections := make([]*ProtectedBranch, 0, 10) + err := sess.In("repo_id", + builder.Select("id").From("repository").Where(builder.Eq{"owner_id": t.OrgID})). + Find(&protections) + if err != nil { + return fmt.Errorf("findProtectedBranches: %v", err) + } + for _, p := range protections { + var matched1, matched2, matched3 bool + if len(p.WhitelistTeamIDs) != 0 { + p.WhitelistTeamIDs, matched1 = util.RemoveIDFromList( + p.WhitelistTeamIDs, t.ID) + } + if len(p.ApprovalsWhitelistTeamIDs) != 0 { + p.ApprovalsWhitelistTeamIDs, matched2 = util.RemoveIDFromList( + p.ApprovalsWhitelistTeamIDs, t.ID) + } + if len(p.MergeWhitelistTeamIDs) != 0 { + p.MergeWhitelistTeamIDs, matched3 = util.RemoveIDFromList( + p.MergeWhitelistTeamIDs, t.ID) + } + if matched1 || matched2 || matched3 { + if _, err = sess.ID(p.ID).Cols( + "whitelist_team_i_ds", + "merge_whitelist_team_i_ds", + "approvals_whitelist_team_i_ds", + ).Update(p); err != nil { + return fmt.Errorf("updateProtectedBranches: %v", err) + } + } + } + } + + if !t.IncludesAllRepositories { + if err := t.removeAllRepositories(ctx); err != nil { + return err + } } // Delete team-user. diff --git a/models/user.go b/models/user.go index 443e0c0c8b..1dbc025155 100644 --- a/models/user.go +++ b/models/user.go @@ -18,6 +18,7 @@ import ( user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/util" "xorm.io/builder" ) @@ -120,6 +121,50 @@ func DeleteUser(ctx context.Context, u *user_model.User) (err error) { } } + // ***** START: Branch Protections ***** + { + const batchSize = 50 + for start := 0; ; start += batchSize { + protections := make([]*ProtectedBranch, 0, batchSize) + // @perf: We can't filter on DB side by u.ID, as those IDs are serialized as JSON strings. + // We could filter down with `WHERE repo_id IN (reposWithPushPermission(u))`, + // though that query will be quite complex and tricky to maintain (compare `getRepoAssignees()`). + // Also, as we didn't update branch protections when removing entries from `access` table, + // it's safer to iterate all protected branches. + if err = e.Limit(batchSize, start).Find(&protections); err != nil { + return fmt.Errorf("findProtectedBranches: %v", err) + } + if len(protections) == 0 { + break + } + for _, p := range protections { + var matched1, matched2, matched3 bool + if len(p.WhitelistUserIDs) != 0 { + p.WhitelistUserIDs, matched1 = util.RemoveIDFromList( + p.WhitelistUserIDs, u.ID) + } + if len(p.ApprovalsWhitelistUserIDs) != 0 { + p.ApprovalsWhitelistUserIDs, matched2 = util.RemoveIDFromList( + p.ApprovalsWhitelistUserIDs, u.ID) + } + if len(p.MergeWhitelistUserIDs) != 0 { + p.MergeWhitelistUserIDs, matched3 = util.RemoveIDFromList( + p.MergeWhitelistUserIDs, u.ID) + } + if matched1 || matched2 || matched3 { + if _, err = e.ID(p.ID).Cols( + "whitelist_user_i_ds", + "merge_whitelist_user_i_ds", + "approvals_whitelist_user_i_ds", + ).Update(p); err != nil { + return fmt.Errorf("updateProtectedBranches: %v", err) + } + } + } + } + } + // ***** END: Branch Protections ***** + // ***** START: PublicKey ***** if _, err = e.Delete(&asymkey_model.PublicKey{OwnerID: u.ID}); err != nil { return fmt.Errorf("deletePublicKeys: %v", err) diff --git a/modules/util/slice.go b/modules/util/slice.go new file mode 100644 index 0000000000..552f5b866a --- /dev/null +++ b/modules/util/slice.go @@ -0,0 +1,18 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package util + +// RemoveIDFromList removes the given ID from the slice, if found. +// It does not preserve order, and assumes the ID is unique. +func RemoveIDFromList(list []int64, id int64) ([]int64, bool) { + n := len(list) - 1 + for i, item := range list { + if item == id { + list[i] = list[n] + return list[:n], true + } + } + return list, false +}