From bffa30302070b594a1c40cdc56264b9731036fb3 Mon Sep 17 00:00:00 2001 From: zeripath Date: Thu, 14 Jul 2022 08:22:09 +0100 Subject: [PATCH] Add option to purge users (#18064) Add the ability to purge users when deleting them. Close #15588 Signed-off-by: Andrew Thornton --- cmd/admin.go | 6 +- integrations/admin_user_test.go | 2 +- integrations/integration_test.go | 9 ++- models/packages/package_version.go | 9 ++- models/project/project.go | 37 +++++++++ models/repo.go | 12 +-- models/user.go | 6 +- options/locale/locale_en-US.ini | 2 + routers/api/v1/admin/user.go | 2 +- routers/web/admin/users.go | 22 ++--- routers/web/user/setting/account.go | 2 +- services/packages/container/cleanup.go | 2 +- services/packages/packages.go | 28 +++++++ services/user/user.go | 106 ++++++++++++++++++++++++- services/user/user_test.go | 10 +-- templates/admin/user/edit.tmpl | 17 +++- 16 files changed, 221 insertions(+), 51 deletions(-) diff --git a/cmd/admin.go b/cmd/admin.go index 32f9a95a6658..524cc3056388 100644 --- a/cmd/admin.go +++ b/cmd/admin.go @@ -157,6 +157,10 @@ var ( Name: "email,e", Usage: "Email of the user to delete", }, + cli.BoolFlag{ + Name: "purge", + Usage: "Purge user, all their repositories, organizations and comments", + }, }, Action: runDeleteUser, } @@ -675,7 +679,7 @@ func runDeleteUser(c *cli.Context) error { return fmt.Errorf("The user %s does not match the provided id %d", user.Name, c.Int64("id")) } - return user_service.DeleteUser(user) + return user_service.DeleteUser(ctx, user, c.Bool("purge")) } func runGenerateAccessToken(c *cli.Context) error { diff --git a/integrations/admin_user_test.go b/integrations/admin_user_test.go index 59adac7ecc65..a2020652b747 100644 --- a/integrations/admin_user_test.go +++ b/integrations/admin_user_test.go @@ -76,7 +76,7 @@ func TestAdminDeleteUser(t *testing.T) { req := NewRequestWithValues(t, "POST", "/admin/users/8/delete", map[string]string{ "_csrf": csrf, }) - session.MakeRequest(t, req, http.StatusOK) + session.MakeRequest(t, req, http.StatusSeeOther) assertUserDeleted(t, 8) unittest.CheckConsistencyFor(t, &user_model.User{}) diff --git a/integrations/integration_test.go b/integrations/integration_test.go index 8a43de7c45fa..230f780175c9 100644 --- a/integrations/integration_test.go +++ b/integrations/integration_test.go @@ -188,8 +188,13 @@ func initIntegrationTest() { switch { case setting.Database.UseMySQL: - db, err := sql.Open("mysql", fmt.Sprintf("%s:%s@tcp(%s)/", - setting.Database.User, setting.Database.Passwd, setting.Database.Host)) + connType := "tcp" + if len(setting.Database.Host) > 0 && setting.Database.Host[0] == '/' { // looks like a unix socket + connType = "unix" + } + + db, err := sql.Open("mysql", fmt.Sprintf("%s:%s@%s(%s)/", + setting.Database.User, setting.Database.Passwd, connType, setting.Database.Host)) defer db.Close() if err != nil { log.Fatal("sql.Open: %v", err) diff --git a/models/packages/package_version.go b/models/packages/package_version.go index 583f832e5eec..83c2fdb67489 100644 --- a/models/packages/package_version.go +++ b/models/packages/package_version.go @@ -107,7 +107,7 @@ func getVersionByNameAndVersion(ctx context.Context, ownerID int64, packageType ExactMatch: true, Value: version, }, - IsInternal: isInternal, + IsInternal: util.OptionalBoolOf(isInternal), Paginator: db.NewAbsoluteListOptions(0, 1), }) if err != nil { @@ -171,7 +171,7 @@ type PackageSearchOptions struct { Name SearchValue // only results with the specific name are found Version SearchValue // only results with the specific version are found Properties map[string]string // only results are found which contain all listed version properties with the specific value - IsInternal bool + IsInternal util.OptionalBool HasFileWithName string // only results are found which are associated with a file with the specific name HasFiles util.OptionalBool // only results are found which have associated files Sort string @@ -179,7 +179,10 @@ type PackageSearchOptions struct { } func (opts *PackageSearchOptions) toConds() builder.Cond { - var cond builder.Cond = builder.Eq{"package_version.is_internal": opts.IsInternal} + cond := builder.NewCond() + if !opts.IsInternal.IsNone() { + cond = builder.Eq{"package_version.is_internal": opts.IsInternal.IsTrue()} + } if opts.OwnerID != 0 { cond = cond.And(builder.Eq{"package.owner_id": opts.OwnerID}) diff --git a/models/project/project.go b/models/project/project.go index 0aa37cc5c907..86a77947d883 100644 --- a/models/project/project.go +++ b/models/project/project.go @@ -330,3 +330,40 @@ func DeleteProjectByIDCtx(ctx context.Context, id int64) error { return updateRepositoryProjectCount(ctx, p.RepoID) } + +func DeleteProjectByRepoIDCtx(ctx context.Context, repoID int64) error { + switch { + case setting.Database.UseSQLite3: + if _, err := db.GetEngine(ctx).Exec("DELETE FROM project_issue WHERE project_issue.id IN (SELECT project_issue.id FROM project_issue INNER JOIN project WHERE project.id = project_issue.project_id AND project.repo_id = ?)", repoID); err != nil { + return err + } + if _, err := db.GetEngine(ctx).Exec("DELETE FROM project_board WHERE project_board.id IN (SELECT project_board.id FROM project_board INNER JOIN project WHERE project.id = project_board.project_id AND project.repo_id = ?)", repoID); err != nil { + return err + } + if _, err := db.GetEngine(ctx).Table("project").Where("repo_id = ? ", repoID).Delete(&Project{}); err != nil { + return err + } + case setting.Database.UsePostgreSQL: + if _, err := db.GetEngine(ctx).Exec("DELETE FROM project_issue USING project WHERE project.id = project_issue.project_id AND project.repo_id = ? ", repoID); err != nil { + return err + } + if _, err := db.GetEngine(ctx).Exec("DELETE FROM project_board USING project WHERE project.id = project_board.project_id AND project.repo_id = ? ", repoID); err != nil { + return err + } + if _, err := db.GetEngine(ctx).Table("project").Where("repo_id = ? ", repoID).Delete(&Project{}); err != nil { + return err + } + default: + if _, err := db.GetEngine(ctx).Exec("DELETE project_issue FROM project_issue INNER JOIN project ON project.id = project_issue.project_id WHERE project.repo_id = ? ", repoID); err != nil { + return err + } + if _, err := db.GetEngine(ctx).Exec("DELETE project_board FROM project_board INNER JOIN project ON project.id = project_board.project_id WHERE project.repo_id = ? ", repoID); err != nil { + return err + } + if _, err := db.GetEngine(ctx).Table("project").Where("repo_id = ? ", repoID).Delete(&Project{}); err != nil { + return err + } + } + + return updateRepositoryProjectCount(ctx, repoID) +} diff --git a/models/repo.go b/models/repo.go index ca83b03e42ca..66ef51473950 100644 --- a/models/repo.go +++ b/models/repo.go @@ -342,16 +342,8 @@ func DeleteRepository(doer *user_model.User, uid, repoID int64) error { } } - projects, _, err := project_model.GetProjects(ctx, project_model.SearchOptions{ - RepoID: repoID, - }) - if err != nil { - return fmt.Errorf("get projects: %v", err) - } - for i := range projects { - if err := project_model.DeleteProjectByIDCtx(ctx, projects[i].ID); err != nil { - return fmt.Errorf("delete project [%d]: %v", projects[i].ID, err) - } + if err := project_model.DeleteProjectByRepoIDCtx(ctx, repoID); err != nil { + return fmt.Errorf("unable to delete projects for repo[%d]: %v", repoID, err) } // Remove LFS objects diff --git a/models/user.go b/models/user.go index 49374014aa7d..86a714e746bb 100644 --- a/models/user.go +++ b/models/user.go @@ -27,7 +27,7 @@ import ( ) // DeleteUser deletes models associated to an user. -func DeleteUser(ctx context.Context, u *user_model.User) (err error) { +func DeleteUser(ctx context.Context, u *user_model.User, purge bool) (err error) { e := db.GetEngine(ctx) // ***** START: Watch ***** @@ -95,8 +95,8 @@ func DeleteUser(ctx context.Context, u *user_model.User) (err error) { return err } - if setting.Service.UserDeleteWithCommentsMaxTime != 0 && - u.CreatedUnix.AsTime().Add(setting.Service.UserDeleteWithCommentsMaxTime).After(time.Now()) { + if purge || (setting.Service.UserDeleteWithCommentsMaxTime != 0 && + u.CreatedUnix.AsTime().Add(setting.Service.UserDeleteWithCommentsMaxTime).After(time.Now())) { // Delete Comments const batchSize = 50 diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 9e8a0303393b..167d2cc1f20c 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -2540,6 +2540,8 @@ users.delete_account = Delete User Account users.cannot_delete_self = "You cannot delete yourself" users.still_own_repo = This user still owns one or more repositories. Delete or transfer these repositories first. users.still_has_org = This user is a member of an organization. Remove the user from any organizations first. +users.purge = Purge User +users.purge_help = Forcibly delete user and any repositories, organizations, and packages owned by the user. All comments will be deleted too. users.still_own_packages = This user still owns one or more packages. Delete these packages first. users.deletion_success = The user account has been deleted. users.reset_2fa = Reset 2FA diff --git a/routers/api/v1/admin/user.go b/routers/api/v1/admin/user.go index 71932136b1f9..1a4b02001133 100644 --- a/routers/api/v1/admin/user.go +++ b/routers/api/v1/admin/user.go @@ -316,7 +316,7 @@ func DeleteUser(ctx *context.APIContext) { return } - if err := user_service.DeleteUser(ctx.ContextUser); err != nil { + if err := user_service.DeleteUser(ctx, ctx.ContextUser, ctx.FormBool("purge")); err != nil { if models.IsErrUserOwnRepos(err) || models.IsErrUserHasOrgs(err) || models.IsErrUserOwnPackages(err) { diff --git a/routers/web/admin/users.go b/routers/web/admin/users.go index c37ecfd71ea9..aab633ec84b2 100644 --- a/routers/web/admin/users.go +++ b/routers/web/admin/users.go @@ -419,29 +419,21 @@ func DeleteUser(ctx *context.Context) { // admin should not delete themself if u.ID == ctx.Doer.ID { ctx.Flash.Error(ctx.Tr("admin.users.cannot_delete_self")) - ctx.JSON(http.StatusOK, map[string]interface{}{ - "redirect": setting.AppSubURL + "/admin/users/" + url.PathEscape(ctx.Params(":userid")), - }) + ctx.Redirect(setting.AppSubURL + "/admin/users/" + url.PathEscape(ctx.Params(":userid"))) return } - if err = user_service.DeleteUser(u); err != nil { + if err = user_service.DeleteUser(ctx, u, ctx.FormBool("purge")); err != nil { switch { case models.IsErrUserOwnRepos(err): ctx.Flash.Error(ctx.Tr("admin.users.still_own_repo")) - ctx.JSON(http.StatusOK, map[string]interface{}{ - "redirect": setting.AppSubURL + "/admin/users/" + url.PathEscape(ctx.Params(":userid")), - }) + ctx.Redirect(setting.AppSubURL + "/admin/users/" + url.PathEscape(ctx.Params(":userid"))) case models.IsErrUserHasOrgs(err): ctx.Flash.Error(ctx.Tr("admin.users.still_has_org")) - ctx.JSON(http.StatusOK, map[string]interface{}{ - "redirect": setting.AppSubURL + "/admin/users/" + url.PathEscape(ctx.Params(":userid")), - }) + ctx.Redirect(setting.AppSubURL + "/admin/users/" + url.PathEscape(ctx.Params(":userid"))) case models.IsErrUserOwnPackages(err): ctx.Flash.Error(ctx.Tr("admin.users.still_own_packages")) - ctx.JSON(http.StatusOK, map[string]interface{}{ - "redirect": setting.AppSubURL + "/admin/users/" + ctx.Params(":userid"), - }) + ctx.Redirect(setting.AppSubURL + "/admin/users/" + ctx.Params(":userid")) default: ctx.ServerError("DeleteUser", err) } @@ -450,9 +442,7 @@ func DeleteUser(ctx *context.Context) { log.Trace("Account deleted by admin (%s): %s", ctx.Doer.Name, u.Name) ctx.Flash.Success(ctx.Tr("admin.users.deletion_success")) - ctx.JSON(http.StatusOK, map[string]interface{}{ - "redirect": setting.AppSubURL + "/admin/users", - }) + ctx.Redirect(setting.AppSubURL + "/admin/users") } // AvatarPost response for change user's avatar request diff --git a/routers/web/user/setting/account.go b/routers/web/user/setting/account.go index dfade13a1ce4..cdb24c606674 100644 --- a/routers/web/user/setting/account.go +++ b/routers/web/user/setting/account.go @@ -248,7 +248,7 @@ func DeleteAccount(ctx *context.Context) { return } - if err := user.DeleteUser(ctx.Doer); err != nil { + if err := user.DeleteUser(ctx, ctx.Doer, false); err != nil { switch { case models.IsErrUserOwnRepos(err): ctx.Flash.Error(ctx.Tr("form.still_own_repo")) diff --git a/services/packages/container/cleanup.go b/services/packages/container/cleanup.go index 390a0b7b052d..3e44f9aa1a0f 100644 --- a/services/packages/container/cleanup.go +++ b/services/packages/container/cleanup.go @@ -59,7 +59,7 @@ func cleanupExpiredUploadedBlobs(ctx context.Context, olderThan time.Duration) e ExactMatch: true, Value: container_model.UploadVersion, }, - IsInternal: true, + IsInternal: util.OptionalBoolTrue, HasFiles: util.OptionalBoolFalse, }) if err != nil { diff --git a/services/packages/packages.go b/services/packages/packages.go index 7f25fce5b85c..0ebf6e7df0cd 100644 --- a/services/packages/packages.go +++ b/services/packages/packages.go @@ -13,6 +13,7 @@ import ( "code.gitea.io/gitea/models/db" packages_model "code.gitea.io/gitea/models/packages" + repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/log" @@ -451,3 +452,30 @@ func GetPackageFileStream(ctx context.Context, pf *packages_model.PackageFile) ( } return s, pf, err } + +// RemoveAllPackages for User +func RemoveAllPackages(ctx context.Context, userID int64) (int, error) { + count := 0 + for { + pkgVersions, _, err := packages_model.SearchVersions(ctx, &packages_model.PackageSearchOptions{ + Paginator: &db.ListOptions{ + PageSize: repo_model.RepositoryListDefaultPageSize, + Page: 1, + }, + OwnerID: userID, + }) + if err != nil { + return count, fmt.Errorf("GetOwnedPackages[%d]: %w", userID, err) + } + if len(pkgVersions) == 0 { + break + } + for _, pv := range pkgVersions { + if err := DeletePackageVersionAndReferences(ctx, pv); err != nil { + return count, fmt.Errorf("unable to delete package %d:%s[%d]. Error: %w", pv.PackageID, pv.Version, pv.ID, err) + } + count++ + } + } + return count, nil +} diff --git a/services/user/user.go b/services/user/user.go index 4db4d7ca17f1..448b7c2daf8d 100644 --- a/services/user/user.go +++ b/services/user/user.go @@ -21,19 +21,116 @@ import ( repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/avatar" + "code.gitea.io/gitea/modules/eventsource" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/storage" "code.gitea.io/gitea/modules/util" + "code.gitea.io/gitea/services/packages" ) // DeleteUser completely and permanently deletes everything of a user, // but issues/comments/pulls will be kept and shown as someone has been deleted, // unless the user is younger than USER_DELETE_WITH_COMMENTS_MAX_DAYS. -func DeleteUser(u *user_model.User) error { +func DeleteUser(ctx context.Context, u *user_model.User, purge bool) error { if u.IsOrganization() { return fmt.Errorf("%s is an organization not a user", u.Name) } + if purge { + // Disable the user first + // NOTE: This is deliberately not within a transaction as it must disable the user immediately to prevent any further action by the user to be purged. + if err := user_model.UpdateUserCols(ctx, &user_model.User{ + ID: u.ID, + IsActive: false, + IsRestricted: true, + IsAdmin: false, + ProhibitLogin: true, + Passwd: "", + Salt: "", + PasswdHashAlgo: "", + MaxRepoCreation: 0, + }, "is_active", "is_restricted", "is_admin", "prohibit_login", "max_repo_creation", "passwd", "salt", "passwd_hash_algo"); err != nil { + return fmt.Errorf("unable to disable user: %s[%d] prior to purge. UpdateUserCols: %w", u.Name, u.ID, err) + } + + // Force any logged in sessions to log out + // FIXME: We also need to tell the session manager to log them out too. + eventsource.GetManager().SendMessage(u.ID, &eventsource.Event{ + Name: "logout", + }) + + // Delete all repos belonging to this user + // Now this is not within a transaction because there are internal transactions within the DeleteRepository + // BUT: the db will still be consistent even if a number of repos have already been deleted. + // And in fact we want to capture any repositories that are being created in other transactions in the meantime + // + // An alternative option here would be write a DeleteAllRepositoriesForUserID function which would delete all of the repos + // but such a function would likely get out of date + for { + repos, _, err := repo_model.GetUserRepositories(&repo_model.SearchRepoOptions{ + ListOptions: db.ListOptions{ + PageSize: repo_model.RepositoryListDefaultPageSize, + Page: 1, + }, + Private: true, + OwnerID: u.ID, + }) + if err != nil { + return fmt.Errorf("SearchRepositoryByName: %v", err) + } + if len(repos) == 0 { + break + } + for _, repo := range repos { + if err := models.DeleteRepository(u, u.ID, repo.ID); err != nil { + return fmt.Errorf("unable to delete repository %s for %s[%d]. Error: %v", repo.Name, u.Name, u.ID, err) + } + } + } + + // Remove from Organizations and delete last owner organizations + // Now this is not within a transaction because there are internal transactions within the DeleteOrganization + // BUT: the db will still be consistent even if a number of organizations memberships and organizations have already been deleted + // And in fact we want to capture any organization additions that are being created in other transactions in the meantime + // + // An alternative option here would be write a function which would delete all organizations but it seems + // but such a function would likely get out of date + for { + orgs, err := organization.FindOrgs(organization.FindOrgOptions{ + ListOptions: db.ListOptions{ + PageSize: repo_model.RepositoryListDefaultPageSize, + Page: 1, + }, + UserID: u.ID, + IncludePrivate: true, + }) + if err != nil { + return fmt.Errorf("unable to find org list for %s[%d]. Error: %v", u.Name, u.ID, err) + } + if len(orgs) == 0 { + break + } + for _, org := range orgs { + if err := models.RemoveOrgUser(org.ID, u.ID); err != nil { + if organization.IsErrLastOrgOwner(err) { + err = organization.DeleteOrganization(ctx, org) + } + if err != nil { + return fmt.Errorf("unable to remove user %s[%d] from org %s[%d]. Error: %v", u.Name, u.ID, org.Name, org.ID, err) + } + } + } + } + + // Delete Packages + if setting.Packages.Enabled { + if _, err := packages.RemoveAllPackages(ctx, u.ID); err != nil { + return err + } + } + } + ctx, committer, err := db.TxContext() if err != nil { return err @@ -41,7 +138,8 @@ func DeleteUser(u *user_model.User) error { defer committer.Close() // Note: A user owns any repository or belongs to any organization - // cannot perform delete operation. + // cannot perform delete operation. This causes a race with the purge above + // however consistency requires that we ensure that this is the case // Check ownership of repository. count, err := repo_model.CountRepositories(ctx, repo_model.CountRepositoryOptions{OwnerID: u.ID}) @@ -66,7 +164,7 @@ func DeleteUser(u *user_model.User) error { return models.ErrUserOwnPackages{UID: u.ID} } - if err := models.DeleteUser(ctx, u); err != nil { + if err := models.DeleteUser(ctx, u, purge); err != nil { return fmt.Errorf("DeleteUser: %v", err) } @@ -117,7 +215,7 @@ func DeleteInactiveUsers(ctx context.Context, olderThan time.Duration) error { return db.ErrCancelledf("Before delete inactive user %s", u.Name) default: } - if err := DeleteUser(u); err != nil { + if err := DeleteUser(ctx, u, false); err != nil { // Ignore users that were set inactive by admin. if models.IsErrUserOwnRepos(err) || models.IsErrUserHasOrgs(err) || models.IsErrUserOwnPackages(err) { continue diff --git a/services/user/user_test.go b/services/user/user_test.go index cfa02b003311..aefbcd9ecb49 100644 --- a/services/user/user_test.go +++ b/services/user/user_test.go @@ -33,7 +33,7 @@ func TestDeleteUser(t *testing.T) { ownedRepos := make([]*repo_model.Repository, 0, 10) assert.NoError(t, db.GetEngine(db.DefaultContext).Find(&ownedRepos, &repo_model.Repository{OwnerID: userID})) if len(ownedRepos) > 0 { - err := DeleteUser(user) + err := DeleteUser(db.DefaultContext, user, false) assert.Error(t, err) assert.True(t, models.IsErrUserOwnRepos(err)) return @@ -47,7 +47,7 @@ func TestDeleteUser(t *testing.T) { return } } - assert.NoError(t, DeleteUser(user)) + assert.NoError(t, DeleteUser(db.DefaultContext, user, false)) unittest.AssertNotExistsBean(t, &user_model.User{ID: userID}) unittest.CheckConsistencyFor(t, &user_model.User{}, &repo_model.Repository{}) } @@ -57,7 +57,7 @@ func TestDeleteUser(t *testing.T) { test(11) org := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3}).(*user_model.User) - assert.Error(t, DeleteUser(org)) + assert.Error(t, DeleteUser(db.DefaultContext, org, false)) } func TestCreateUser(t *testing.T) { @@ -72,7 +72,7 @@ func TestCreateUser(t *testing.T) { assert.NoError(t, user_model.CreateUser(user)) - assert.NoError(t, DeleteUser(user)) + assert.NoError(t, DeleteUser(db.DefaultContext, user, false)) } func TestCreateUser_Issue5882(t *testing.T) { @@ -101,6 +101,6 @@ func TestCreateUser_Issue5882(t *testing.T) { assert.Equal(t, !u.AllowCreateOrganization, v.disableOrgCreation) - assert.NoError(t, DeleteUser(v.user)) + assert.NoError(t, DeleteUser(db.DefaultContext, v.user, false)) } } diff --git a/templates/admin/user/edit.tmpl b/templates/admin/user/edit.tmpl index 29dcaa127af2..17cbef9f108a 100644 --- a/templates/admin/user/edit.tmpl +++ b/templates/admin/user/edit.tmpl @@ -151,7 +151,7 @@
-
{{.locale.Tr "admin.users.delete_account"}}
+
{{.locale.Tr "admin.users.delete_account"}}
@@ -196,7 +196,7 @@ -
+