From f34151bdb22c8160b0a6eafef20725ebae1768da Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Thu, 18 Nov 2021 18:42:27 +0100 Subject: [PATCH] Move user/org deletion to services (#17673) --- cmd/admin.go | 3 +- models/admin/notice.go | 33 +++------ models/admin/notice_test.go | 3 +- models/consistency.go | 2 +- models/org.go | 64 +++------------- models/org_test.go | 18 ----- models/repo.go | 20 ++--- models/repo_test.go | 2 +- models/user.go | 110 ++++------------------------ models/user/email_address.go | 8 ++ models/user_test.go | 80 -------------------- modules/repository/create_test.go | 3 +- routers/api/v1/admin/user.go | 3 +- routers/api/v1/org/org.go | 3 +- routers/web/admin/users.go | 3 +- routers/web/org/setting.go | 8 +- routers/web/user/setting/account.go | 3 +- services/cron/tasks.go | 6 +- services/cron/tasks_extended.go | 3 +- services/org/org.go | 61 +++++++++++++++ services/org/org_test.go | 37 ++++++++++ services/user/user.go | 106 +++++++++++++++++++++++++++ services/user/user_test.go | 101 +++++++++++++++++++++++++ services/wiki/wiki.go | 3 +- 24 files changed, 382 insertions(+), 301 deletions(-) create mode 100644 services/org/org.go create mode 100644 services/org/org_test.go create mode 100644 services/user/user.go create mode 100644 services/user/user_test.go diff --git a/cmd/admin.go b/cmd/admin.go index 858498ed3..27089a17c 100644 --- a/cmd/admin.go +++ b/cmd/admin.go @@ -26,6 +26,7 @@ import ( auth_service "code.gitea.io/gitea/services/auth" "code.gitea.io/gitea/services/auth/source/oauth2" repo_service "code.gitea.io/gitea/services/repository" + user_service "code.gitea.io/gitea/services/user" "github.com/urfave/cli" ) @@ -534,7 +535,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 models.DeleteUser(user) + return user_service.DeleteUser(user) } func runRepoSyncReleases(_ *cli.Context) error { diff --git a/models/admin/notice.go b/models/admin/notice.go index c41e49ed8..2d71b426d 100644 --- a/models/admin/notice.go +++ b/models/admin/notice.go @@ -43,12 +43,7 @@ func (n *Notice) TrStr() string { } // CreateNotice creates new system notice. -func CreateNotice(tp NoticeType, desc string, args ...interface{}) error { - return CreateNoticeCtx(db.DefaultContext, tp, desc, args...) -} - -// CreateNoticeCtx creates new system notice. -func CreateNoticeCtx(ctx context.Context, tp NoticeType, desc string, args ...interface{}) error { +func CreateNotice(ctx context.Context, tp NoticeType, desc string, args ...interface{}) error { if len(args) > 0 { desc = fmt.Sprintf(desc, args...) } @@ -61,38 +56,28 @@ func CreateNoticeCtx(ctx context.Context, tp NoticeType, desc string, args ...in // CreateRepositoryNotice creates new system notice with type NoticeRepository. func CreateRepositoryNotice(desc string, args ...interface{}) error { - return CreateNoticeCtx(db.DefaultContext, NoticeRepository, desc, args...) + return CreateNotice(db.DefaultContext, NoticeRepository, desc, args...) } // RemoveAllWithNotice removes all directories in given path and // creates a system notice when error occurs. -func RemoveAllWithNotice(title, path string) { - RemoveAllWithNoticeCtx(db.DefaultContext, title, path) -} - -// RemoveStorageWithNotice removes a file from the storage and -// creates a system notice when error occurs. -func RemoveStorageWithNotice(bucket storage.ObjectStorage, title, path string) { - removeStorageWithNotice(db.DefaultContext, bucket, title, path) -} - -func removeStorageWithNotice(ctx context.Context, bucket storage.ObjectStorage, title, path string) { - if err := bucket.Delete(path); err != nil { +func RemoveAllWithNotice(ctx context.Context, title, path string) { + if err := util.RemoveAll(path); err != nil { desc := fmt.Sprintf("%s [%s]: %v", title, path, err) log.Warn(title+" [%s]: %v", path, err) - if err = CreateNoticeCtx(ctx, NoticeRepository, desc); err != nil { + if err = CreateNotice(ctx, NoticeRepository, desc); err != nil { log.Error("CreateRepositoryNotice: %v", err) } } } -// RemoveAllWithNoticeCtx removes all directories in given path and +// RemoveStorageWithNotice removes a file from the storage and // creates a system notice when error occurs. -func RemoveAllWithNoticeCtx(ctx context.Context, title, path string) { - if err := util.RemoveAll(path); err != nil { +func RemoveStorageWithNotice(ctx context.Context, bucket storage.ObjectStorage, title, path string) { + if err := bucket.Delete(path); err != nil { desc := fmt.Sprintf("%s [%s]: %v", title, path, err) log.Warn(title+" [%s]: %v", path, err) - if err = CreateNoticeCtx(ctx, NoticeRepository, desc); err != nil { + if err = CreateNotice(ctx, NoticeRepository, desc); err != nil { log.Error("CreateRepositoryNotice: %v", err) } } diff --git a/models/admin/notice_test.go b/models/admin/notice_test.go index 7272df436..b4613db8e 100644 --- a/models/admin/notice_test.go +++ b/models/admin/notice_test.go @@ -7,6 +7,7 @@ package admin import ( "testing" + "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/unittest" "github.com/stretchr/testify/assert" @@ -28,7 +29,7 @@ func TestCreateNotice(t *testing.T) { Description: "test description", } unittest.AssertNotExistsBean(t, noticeBean) - assert.NoError(t, CreateNotice(noticeBean.Type, noticeBean.Description)) + assert.NoError(t, CreateNotice(db.DefaultContext, noticeBean.Type, noticeBean.Description)) unittest.AssertExistsAndLoadBean(t, noticeBean) } diff --git a/models/consistency.go b/models/consistency.go index 83cfbd0e4..ba669628b 100644 --- a/models/consistency.go +++ b/models/consistency.go @@ -128,7 +128,7 @@ func DeleteOrphanedIssues() error { // Remove issue attachment files. for i := range attachmentPaths { - admin_model.RemoveAllWithNoticeCtx(db.DefaultContext, "Delete issue attachment", attachmentPaths[i]) + admin_model.RemoveAllWithNotice(db.DefaultContext, "Delete issue attachment", attachmentPaths[i]) } return nil } diff --git a/models/org.go b/models/org.go index 1840cae9f..e0b4d2724 100644 --- a/models/org.go +++ b/models/org.go @@ -6,6 +6,7 @@ package models import ( + "context" "fmt" "strings" @@ -14,9 +15,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/storage" "code.gitea.io/gitea/modules/structs" - "code.gitea.io/gitea/modules/util" "xorm.io/builder" "xorm.io/xorm" @@ -254,68 +253,23 @@ func CountOrganizations() int64 { return count } -// DeleteOrganization completely and permanently deletes everything of organization. -func DeleteOrganization(org *User) (err error) { - if !org.IsOrganization() { - return fmt.Errorf("%s is a user not an organization", org.Name) - } - - sess := db.NewSession(db.DefaultContext) - defer sess.Close() - - if err = sess.Begin(); err != nil { - return err - } - - if err = deleteOrg(sess, org); err != nil { - if IsErrUserOwnRepos(err) { - return err - } else if err != nil { - return fmt.Errorf("deleteOrg: %v", err) - } - } - - return sess.Commit() -} - -func deleteOrg(e *xorm.Session, u *User) error { - // Check ownership of repository. - count, err := getRepositoryCount(e, u) - if err != nil { - return fmt.Errorf("GetRepositoryCount: %v", err) - } else if count > 0 { - return ErrUserOwnRepos{UID: u.ID} - } +// DeleteOrganization deletes models associated to an organization. +func DeleteOrganization(ctx context.Context, org *User) error { + e := db.GetEngine(ctx) if err := deleteBeans(e, - &Team{OrgID: u.ID}, - &OrgUser{OrgID: u.ID}, - &TeamUser{OrgID: u.ID}, - &TeamUnit{OrgID: u.ID}, + &Team{OrgID: org.ID}, + &OrgUser{OrgID: org.ID}, + &TeamUser{OrgID: org.ID}, + &TeamUnit{OrgID: org.ID}, ); err != nil { return fmt.Errorf("deleteBeans: %v", err) } - if _, err = e.ID(u.ID).Delete(new(User)); err != nil { + if _, err := e.ID(org.ID).Delete(new(User)); err != nil { return fmt.Errorf("Delete: %v", err) } - // FIXME: system notice - // Note: There are something just cannot be roll back, - // so just keep error logs of those operations. - path := UserPath(u.Name) - - if err := util.RemoveAll(path); err != nil { - return fmt.Errorf("Failed to RemoveAll %s: %v", path, err) - } - - if len(u.Avatar) > 0 { - avatarPath := u.CustomAvatarRelativePath() - if err := storage.Avatars.Delete(avatarPath); err != nil { - return fmt.Errorf("Failed to remove %s: %v", avatarPath, err) - } - } - return nil } diff --git a/models/org_test.go b/models/org_test.go index 2bc4d74fc..8580702a0 100644 --- a/models/org_test.go +++ b/models/org_test.go @@ -261,24 +261,6 @@ func TestCountOrganizations(t *testing.T) { assert.Equal(t, expected, CountOrganizations()) } -func TestDeleteOrganization(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - org := unittest.AssertExistsAndLoadBean(t, &User{ID: 6}).(*User) - assert.NoError(t, DeleteOrganization(org)) - unittest.AssertNotExistsBean(t, &User{ID: 6}) - unittest.AssertNotExistsBean(t, &OrgUser{OrgID: 6}) - unittest.AssertNotExistsBean(t, &Team{OrgID: 6}) - - org = unittest.AssertExistsAndLoadBean(t, &User{ID: 3}).(*User) - err := DeleteOrganization(org) - assert.Error(t, err) - assert.True(t, IsErrUserOwnRepos(err)) - - user := unittest.AssertExistsAndLoadBean(t, &User{ID: 5}).(*User) - assert.Error(t, DeleteOrganization(user)) - unittest.CheckConsistencyFor(t, &User{}, &Team{}) -} - func TestIsOrganizationOwner(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) test := func(orgID, userID int64, expected bool) { diff --git a/models/repo.go b/models/repo.go index 57806a86f..d62b3ec2b 100644 --- a/models/repo.go +++ b/models/repo.go @@ -134,7 +134,7 @@ func NewRepoContext() { loadRepoConfig() unit.LoadUnitConfig() - admin_model.RemoveAllWithNotice("Clean up repository temporary data", filepath.Join(setting.AppDataPath, "tmp")) + admin_model.RemoveAllWithNotice(db.DefaultContext, "Clean up repository temporary data", filepath.Join(setting.AppDataPath, "tmp")) } // RepositoryStatus defines the status of repository @@ -1649,36 +1649,36 @@ func DeleteRepository(doer *User, uid, repoID int64) error { // Remove repository files. repoPath := repo.RepoPath() - admin_model.RemoveAllWithNotice("Delete repository files", repoPath) + admin_model.RemoveAllWithNotice(db.DefaultContext, "Delete repository files", repoPath) // Remove wiki files if repo.HasWiki() { - admin_model.RemoveAllWithNotice("Delete repository wiki", repo.WikiPath()) + admin_model.RemoveAllWithNotice(db.DefaultContext, "Delete repository wiki", repo.WikiPath()) } // Remove archives for i := range archivePaths { - admin_model.RemoveStorageWithNotice(storage.RepoArchives, "Delete repo archive file", archivePaths[i]) + admin_model.RemoveStorageWithNotice(db.DefaultContext, storage.RepoArchives, "Delete repo archive file", archivePaths[i]) } // Remove lfs objects for i := range lfsPaths { - admin_model.RemoveStorageWithNotice(storage.LFS, "Delete orphaned LFS file", lfsPaths[i]) + admin_model.RemoveStorageWithNotice(db.DefaultContext, storage.LFS, "Delete orphaned LFS file", lfsPaths[i]) } // Remove issue attachment files. for i := range attachmentPaths { - admin_model.RemoveStorageWithNotice(storage.Attachments, "Delete issue attachment", attachmentPaths[i]) + admin_model.RemoveStorageWithNotice(db.DefaultContext, storage.Attachments, "Delete issue attachment", attachmentPaths[i]) } // Remove release attachment files. for i := range releaseAttachments { - admin_model.RemoveStorageWithNotice(storage.Attachments, "Delete release attachment", releaseAttachments[i]) + admin_model.RemoveStorageWithNotice(db.DefaultContext, storage.Attachments, "Delete release attachment", releaseAttachments[i]) } // Remove attachment with no issue_id and release_id. for i := range newAttachmentPaths { - admin_model.RemoveStorageWithNotice(storage.Attachments, "Delete issue attachment", attachmentPaths[i]) + admin_model.RemoveStorageWithNotice(db.DefaultContext, storage.Attachments, "Delete issue attachment", attachmentPaths[i]) } if len(repo.Avatar) > 0 { @@ -1803,8 +1803,8 @@ func getPrivateRepositoryCount(e db.Engine, u *User) (int64, error) { } // GetRepositoryCount returns the total number of repositories of user. -func GetRepositoryCount(u *User) (int64, error) { - return getRepositoryCount(db.GetEngine(db.DefaultContext), u) +func GetRepositoryCount(ctx context.Context, u *User) (int64, error) { + return getRepositoryCount(db.GetEngine(ctx), u) } // GetPublicRepositoryCount returns the total number of public repositories of user. diff --git a/models/repo_test.go b/models/repo_test.go index 685570adc..e6f4ea1c3 100644 --- a/models/repo_test.go +++ b/models/repo_test.go @@ -71,7 +71,7 @@ func TestMetas(t *testing.T) { func TestGetRepositoryCount(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) - count, err1 := GetRepositoryCount(&User{ID: int64(10)}) + count, err1 := GetRepositoryCount(db.DefaultContext, &User{ID: int64(10)}) privateCount, err2 := GetPrivateRepositoryCount(&User{ID: int64(10)}) publicCount, err3 := GetPublicRepositoryCount(&User{ID: int64(10)}) assert.NoError(t, err1) diff --git a/models/user.go b/models/user.go index cc6c5a557..1bdb43579 100644 --- a/models/user.go +++ b/models/user.go @@ -22,7 +22,6 @@ import ( _ "image/jpeg" // Needed for jpeg support - admin_model "code.gitea.io/gitea/models/admin" "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/login" "code.gitea.io/gitea/models/unit" @@ -32,7 +31,6 @@ import ( "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" - "code.gitea.io/gitea/modules/storage" "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/util" @@ -542,15 +540,16 @@ func (u *User) IsPublicMember(orgID int64) bool { return isMember } -func (u *User) getOrganizationCount(e db.Engine) (int64, error) { - return e. +// GetOrganizationCount returns count of membership of organization of the user. +func GetOrganizationCount(ctx context.Context, u *User) (int64, error) { + return db.GetEngine(ctx). Where("uid=?", u.ID). Count(new(OrgUser)) } // GetOrganizationCount returns count of membership of organization of user. func (u *User) GetOrganizationCount() (int64, error) { - return u.getOrganizationCount(db.GetEngine(db.DefaultContext)) + return GetOrganizationCount(db.DefaultContext, u) } // GetRepositories returns repositories that user owns, including private repositories. @@ -1149,26 +1148,9 @@ func deleteBeans(e db.Engine, beans ...interface{}) (err error) { return nil } -func deleteUser(ctx context.Context, u *User) error { +// DeleteUser deletes models associated to an user. +func DeleteUser(ctx context.Context, u *User) (err error) { e := db.GetEngine(ctx) - // Note: A user owns any repository or belongs to any organization - // cannot perform delete operation. - - // Check ownership of repository. - count, err := getRepositoryCount(e, u) - if err != nil { - return fmt.Errorf("GetRepositoryCount: %v", err) - } else if count > 0 { - return ErrUserOwnRepos{UID: u.ID} - } - - // Check membership of organization. - count, err = u.getOrganizationCount(e) - if err != nil { - return fmt.Errorf("GetOrganizationCount: %v", err) - } else if count > 0 { - return ErrUserHasOrgs{UID: u.ID} - } // ***** START: Watch ***** watchedRepoIDs := make([]int64, 0, 10) @@ -1301,85 +1283,21 @@ func deleteUser(ctx context.Context, u *User) error { return fmt.Errorf("Delete: %v", err) } - // Note: There are something just cannot be roll back, - // so just keep error logs of those operations. - path := UserPath(u.Name) - if err = util.RemoveAll(path); err != nil { - err = fmt.Errorf("Failed to RemoveAll %s: %v", path, err) - _ = admin_model.CreateNoticeCtx(ctx, admin_model.NoticeTask, fmt.Sprintf("delete user '%s': %v", u.Name, err)) - return err - } - - if len(u.Avatar) > 0 { - avatarPath := u.CustomAvatarRelativePath() - if err = storage.Avatars.Delete(avatarPath); err != nil { - err = fmt.Errorf("Failed to remove %s: %v", avatarPath, err) - _ = admin_model.CreateNoticeCtx(ctx, admin_model.NoticeTask, fmt.Sprintf("delete user '%s': %v", u.Name, err)) - return err - } - } - return nil } -// 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) (err error) { - if u.IsOrganization() { - return fmt.Errorf("%s is an organization not a user", u.Name) - } - - ctx, committer, err := db.TxContext() - if err != nil { - return err - } - defer committer.Close() - - if err = deleteUser(ctx, u); err != nil { - // Note: don't wrapper error here. - return err - } - - return committer.Commit() -} +// GetInactiveUsers gets all inactive users +func GetInactiveUsers(ctx context.Context, olderThan time.Duration) ([]*User, error) { + var cond builder.Cond = builder.Eq{"is_active": false} -// DeleteInactiveUsers deletes all inactive users and email addresses. -func DeleteInactiveUsers(ctx context.Context, olderThan time.Duration) (err error) { - users := make([]*User, 0, 10) if olderThan > 0 { - if err = db.GetEngine(db.DefaultContext). - Where("is_active = ? and created_unix < ?", false, time.Now().Add(-olderThan).Unix()). - Find(&users); err != nil { - return fmt.Errorf("get all inactive users: %v", err) - } - } else { - if err = db.GetEngine(db.DefaultContext). - Where("is_active = ?", false). - Find(&users); err != nil { - return fmt.Errorf("get all inactive users: %v", err) - } - } - // FIXME: should only update authorized_keys file once after all deletions. - for _, u := range users { - select { - case <-ctx.Done(): - return db.ErrCancelledf("Before delete inactive user %s", u.Name) - default: - } - if err = DeleteUser(u); err != nil { - // Ignore users that were set inactive by admin. - if IsErrUserOwnRepos(err) || IsErrUserHasOrgs(err) { - continue - } - return err - } + cond = cond.And(builder.Lt{"created_unix": time.Now().Add(-olderThan).Unix()}) } - _, err = db.GetEngine(db.DefaultContext). - Where("is_activated = ?", false). - Delete(new(user_model.EmailAddress)) - return err + users := make([]*User, 0, 10) + return users, db.GetEngine(ctx). + Where(cond). + Find(&users) } // UserPath returns the path absolute path of user repositories. diff --git a/models/user/email_address.go b/models/user/email_address.go index 74fb71d45..98cd14a6a 100644 --- a/models/user/email_address.go +++ b/models/user/email_address.go @@ -267,3 +267,11 @@ func DeleteEmailAddresses(emails []*EmailAddress) (err error) { return nil } + +// DeleteInactiveEmailAddresses deletes inactive email addresses +func DeleteInactiveEmailAddresses(ctx context.Context) error { + _, err := db.GetEngine(ctx). + Where("is_activated = ?", false). + Delete(new(EmailAddress)) + return err +} diff --git a/models/user_test.go b/models/user_test.go index 4e2e52176..2da5519d7 100644 --- a/models/user_test.go +++ b/models/user_test.go @@ -177,41 +177,6 @@ func TestSearchUsers(t *testing.T) { []int64{24}) } -func TestDeleteUser(t *testing.T) { - test := func(userID int64) { - assert.NoError(t, unittest.PrepareTestDatabase()) - user := unittest.AssertExistsAndLoadBean(t, &User{ID: userID}).(*User) - - ownedRepos := make([]*Repository, 0, 10) - assert.NoError(t, db.GetEngine(db.DefaultContext).Find(&ownedRepos, &Repository{OwnerID: userID})) - if len(ownedRepos) > 0 { - err := DeleteUser(user) - assert.Error(t, err) - assert.True(t, IsErrUserOwnRepos(err)) - return - } - - orgUsers := make([]*OrgUser, 0, 10) - assert.NoError(t, db.GetEngine(db.DefaultContext).Find(&orgUsers, &OrgUser{UID: userID})) - for _, orgUser := range orgUsers { - if err := RemoveOrgUser(orgUser.OrgID, orgUser.UID); err != nil { - assert.True(t, IsErrLastOrgOwner(err)) - return - } - } - assert.NoError(t, DeleteUser(user)) - unittest.AssertNotExistsBean(t, &User{ID: userID}) - unittest.CheckConsistencyFor(t, &User{}, &Repository{}) - } - test(2) - test(4) - test(8) - test(11) - - org := unittest.AssertExistsAndLoadBean(t, &User{ID: 3}).(*User) - assert.Error(t, DeleteUser(org)) -} - func TestEmailNotificationPreferences(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) @@ -333,21 +298,6 @@ func TestDisplayName(t *testing.T) { } } -func TestCreateUser(t *testing.T) { - user := &User{ - Name: "GiteaBot", - Email: "GiteaBot@gitea.io", - Passwd: ";p['////..-++']", - IsAdmin: false, - Theme: setting.UI.DefaultTheme, - MustChangePassword: false, - } - - assert.NoError(t, CreateUser(user)) - - assert.NoError(t, DeleteUser(user)) -} - func TestCreateUserInvalidEmail(t *testing.T) { user := &User{ Name: "GiteaBot", @@ -363,36 +313,6 @@ func TestCreateUserInvalidEmail(t *testing.T) { assert.True(t, user_model.IsErrEmailInvalid(err)) } -func TestCreateUser_Issue5882(t *testing.T) { - // Init settings - _ = setting.Admin - - passwd := ".//.;1;;//.,-=_" - - tt := []struct { - user *User - disableOrgCreation bool - }{ - {&User{Name: "GiteaBot", Email: "GiteaBot@gitea.io", Passwd: passwd, MustChangePassword: false}, false}, - {&User{Name: "GiteaBot2", Email: "GiteaBot2@gitea.io", Passwd: passwd, MustChangePassword: false}, true}, - } - - setting.Service.DefaultAllowCreateOrganization = true - - for _, v := range tt { - setting.Admin.DisableRegularOrgCreation = v.disableOrgCreation - - assert.NoError(t, CreateUser(v.user)) - - u, err := GetUserByEmail(v.user.Email) - assert.NoError(t, err) - - assert.Equal(t, !u.AllowCreateOrganization, v.disableOrgCreation) - - assert.NoError(t, DeleteUser(v.user)) - } -} - func TestGetUserIDsByNames(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) diff --git a/modules/repository/create_test.go b/modules/repository/create_test.go index 2e27ab0d7..55fc51558 100644 --- a/modules/repository/create_test.go +++ b/modules/repository/create_test.go @@ -9,6 +9,7 @@ import ( "testing" "code.gitea.io/gitea/models" + "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/structs" @@ -142,5 +143,5 @@ func TestIncludesAllRepositoriesTeams(t *testing.T) { assert.NoError(t, models.DeleteRepository(user, org.ID, rid), "DeleteRepository %d", i) } } - assert.NoError(t, models.DeleteOrganization(org), "DeleteOrganization") + assert.NoError(t, models.DeleteOrganization(db.DefaultContext, org), "DeleteOrganization") } diff --git a/routers/api/v1/admin/user.go b/routers/api/v1/admin/user.go index 1e4a2851c..95060e7a1 100644 --- a/routers/api/v1/admin/user.go +++ b/routers/api/v1/admin/user.go @@ -22,6 +22,7 @@ import ( "code.gitea.io/gitea/routers/api/v1/user" "code.gitea.io/gitea/routers/api/v1/utils" "code.gitea.io/gitea/services/mailer" + user_service "code.gitea.io/gitea/services/user" ) func parseLoginSource(ctx *context.APIContext, u *models.User, sourceID int64, loginName string) { @@ -289,7 +290,7 @@ func DeleteUser(ctx *context.APIContext) { return } - if err := models.DeleteUser(u); err != nil { + if err := user_service.DeleteUser(u); err != nil { if models.IsErrUserOwnRepos(err) || models.IsErrUserHasOrgs(err) { ctx.Error(http.StatusUnprocessableEntity, "", err) diff --git a/routers/api/v1/org/org.go b/routers/api/v1/org/org.go index d3aa92f46..e82e8552e 100644 --- a/routers/api/v1/org/org.go +++ b/routers/api/v1/org/org.go @@ -16,6 +16,7 @@ import ( "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/routers/api/v1/user" "code.gitea.io/gitea/routers/api/v1/utils" + "code.gitea.io/gitea/services/org" ) func listUserOrgs(ctx *context.APIContext, u *models.User) { @@ -364,7 +365,7 @@ func Delete(ctx *context.APIContext) { // "204": // "$ref": "#/responses/empty" - if err := models.DeleteOrganization(ctx.Org.Organization); err != nil { + if err := org.DeleteOrganization(ctx.Org.Organization); err != nil { ctx.Error(http.StatusInternalServerError, "DeleteOrganization", err) return } diff --git a/routers/web/admin/users.go b/routers/web/admin/users.go index 93e59893e..b23e4cf39 100644 --- a/routers/web/admin/users.go +++ b/routers/web/admin/users.go @@ -26,6 +26,7 @@ import ( router_user_setting "code.gitea.io/gitea/routers/web/user/setting" "code.gitea.io/gitea/services/forms" "code.gitea.io/gitea/services/mailer" + "code.gitea.io/gitea/services/user" ) const ( @@ -377,7 +378,7 @@ func DeleteUser(ctx *context.Context) { return } - if err = models.DeleteUser(u); err != nil { + if err = user.DeleteUser(u); err != nil { switch { case models.IsErrUserOwnRepos(err): ctx.Flash.Error(ctx.Tr("admin.users.still_own_repo")) diff --git a/routers/web/org/setting.go b/routers/web/org/setting.go index 53c31a1c6..f27cb4083 100644 --- a/routers/web/org/setting.go +++ b/routers/web/org/setting.go @@ -20,6 +20,7 @@ import ( "code.gitea.io/gitea/modules/web" userSetting "code.gitea.io/gitea/routers/web/user/setting" "code.gitea.io/gitea/services/forms" + "code.gitea.io/gitea/services/org" ) const ( @@ -156,15 +157,14 @@ func SettingsDelete(ctx *context.Context) { ctx.Data["Title"] = ctx.Tr("org.settings") ctx.Data["PageIsSettingsDelete"] = true - org := ctx.Org.Organization if ctx.Req.Method == "POST" { - if org.Name != ctx.FormString("org_name") { + if ctx.Org.Organization.Name != ctx.FormString("org_name") { ctx.Data["Err_OrgName"] = true ctx.RenderWithErr(ctx.Tr("form.enterred_invalid_org_name"), tplSettingsDelete, nil) return } - if err := models.DeleteOrganization(org); err != nil { + if err := org.DeleteOrganization(ctx.Org.Organization); err != nil { if models.IsErrUserOwnRepos(err) { ctx.Flash.Error(ctx.Tr("form.org_still_own_repo")) ctx.Redirect(ctx.Org.OrgLink + "/settings/delete") @@ -172,7 +172,7 @@ func SettingsDelete(ctx *context.Context) { ctx.ServerError("DeleteOrganization", err) } } else { - log.Trace("Organization deleted: %s", org.Name) + log.Trace("Organization deleted: %s", ctx.Org.Organization.Name) ctx.Redirect(setting.AppSubURL + "/") } return diff --git a/routers/web/user/setting/account.go b/routers/web/user/setting/account.go index 5ef1c3bdc..3362d3806 100644 --- a/routers/web/user/setting/account.go +++ b/routers/web/user/setting/account.go @@ -22,6 +22,7 @@ import ( "code.gitea.io/gitea/services/auth" "code.gitea.io/gitea/services/forms" "code.gitea.io/gitea/services/mailer" + "code.gitea.io/gitea/services/user" ) const ( @@ -241,7 +242,7 @@ func DeleteAccount(ctx *context.Context) { return } - if err := models.DeleteUser(ctx.User); err != nil { + if err := user.DeleteUser(ctx.User); err != nil { switch { case models.IsErrUserOwnRepos(err): ctx.Flash.Error(ctx.Tr("form.still_own_repo")) diff --git a/services/cron/tasks.go b/services/cron/tasks.go index d29bf6d6b..732eead93 100644 --- a/services/cron/tasks.go +++ b/services/cron/tasks.go @@ -90,18 +90,18 @@ func (t *Task) RunWithUser(doer *models.User, config Config) { if err := t.fun(ctx, doer, config); err != nil { if db.IsErrCancelled(err) { message := err.(db.ErrCancelled).Message - if err := admin_model.CreateNotice(admin_model.NoticeTask, config.FormatMessage(t.Name, "aborted", doer, message)); err != nil { + if err := admin_model.CreateNotice(ctx, admin_model.NoticeTask, config.FormatMessage(t.Name, "aborted", doer, message)); err != nil { log.Error("CreateNotice: %v", err) } return } - if err := admin_model.CreateNotice(admin_model.NoticeTask, config.FormatMessage(t.Name, "error", doer, err)); err != nil { + if err := admin_model.CreateNotice(ctx, admin_model.NoticeTask, config.FormatMessage(t.Name, "error", doer, err)); err != nil { log.Error("CreateNotice: %v", err) } return } if config.DoNoticeOnSuccess() { - if err := admin_model.CreateNotice(admin_model.NoticeTask, config.FormatMessage(t.Name, "finished", doer)); err != nil { + if err := admin_model.CreateNotice(ctx, admin_model.NoticeTask, config.FormatMessage(t.Name, "finished", doer)); err != nil { log.Error("CreateNotice: %v", err) } } diff --git a/services/cron/tasks_extended.go b/services/cron/tasks_extended.go index 1f115de43..b0cb5ee92 100644 --- a/services/cron/tasks_extended.go +++ b/services/cron/tasks_extended.go @@ -13,6 +13,7 @@ import ( "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/updatechecker" repo_service "code.gitea.io/gitea/services/repository" + user_service "code.gitea.io/gitea/services/user" ) func registerDeleteInactiveUsers() { @@ -25,7 +26,7 @@ func registerDeleteInactiveUsers() { OlderThan: 0 * time.Second, }, func(ctx context.Context, _ *models.User, config Config) error { olderThanConfig := config.(*OlderThanConfig) - return models.DeleteInactiveUsers(ctx, olderThanConfig.OlderThan) + return user_service.DeleteInactiveUsers(ctx, olderThanConfig.OlderThan) }) } diff --git a/services/org/org.go b/services/org/org.go new file mode 100644 index 000000000..cb1f1eca0 --- /dev/null +++ b/services/org/org.go @@ -0,0 +1,61 @@ +// Copyright 2021 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 org + +import ( + "fmt" + + "code.gitea.io/gitea/models" + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/modules/storage" + "code.gitea.io/gitea/modules/util" +) + +// DeleteOrganization completely and permanently deletes everything of organization. +func DeleteOrganization(org *models.User) error { + if !org.IsOrganization() { + return fmt.Errorf("%s is a user not an organization", org.Name) + } + + ctx, commiter, err := db.TxContext() + if err != nil { + return err + } + defer commiter.Close() + + // Check ownership of repository. + count, err := models.GetRepositoryCount(ctx, org) + if err != nil { + return fmt.Errorf("GetRepositoryCount: %v", err) + } else if count > 0 { + return models.ErrUserOwnRepos{UID: org.ID} + } + + if err := models.DeleteOrganization(ctx, org); err != nil { + return fmt.Errorf("DeleteOrganization: %v", err) + } + + if err := commiter.Commit(); err != nil { + return err + } + + // FIXME: system notice + // Note: There are something just cannot be roll back, + // so just keep error logs of those operations. + path := models.UserPath(org.Name) + + if err := util.RemoveAll(path); err != nil { + return fmt.Errorf("Failed to RemoveAll %s: %v", path, err) + } + + if len(org.Avatar) > 0 { + avatarPath := org.CustomAvatarRelativePath() + if err := storage.Avatars.Delete(avatarPath); err != nil { + return fmt.Errorf("Failed to remove %s: %v", avatarPath, err) + } + } + + return nil +} diff --git a/services/org/org_test.go b/services/org/org_test.go new file mode 100644 index 000000000..5fec086d1 --- /dev/null +++ b/services/org/org_test.go @@ -0,0 +1,37 @@ +// Copyright 2021 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 org + +import ( + "path/filepath" + "testing" + + "code.gitea.io/gitea/models" + "code.gitea.io/gitea/models/unittest" + + "github.com/stretchr/testify/assert" +) + +func TestMain(m *testing.M) { + unittest.MainTest(m, filepath.Join("..", "..")) +} + +func TestDeleteOrganization(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + org := unittest.AssertExistsAndLoadBean(t, &models.User{ID: 6}).(*models.User) + assert.NoError(t, DeleteOrganization(org)) + unittest.AssertNotExistsBean(t, &models.User{ID: 6}) + unittest.AssertNotExistsBean(t, &models.OrgUser{OrgID: 6}) + unittest.AssertNotExistsBean(t, &models.Team{OrgID: 6}) + + org = unittest.AssertExistsAndLoadBean(t, &models.User{ID: 3}).(*models.User) + err := DeleteOrganization(org) + assert.Error(t, err) + assert.True(t, models.IsErrUserOwnRepos(err)) + + user := unittest.AssertExistsAndLoadBean(t, &models.User{ID: 5}).(*models.User) + assert.Error(t, DeleteOrganization(user)) + unittest.CheckConsistencyFor(t, &models.User{}, &models.Team{}) +} diff --git a/services/user/user.go b/services/user/user.go new file mode 100644 index 000000000..a08e40f86 --- /dev/null +++ b/services/user/user.go @@ -0,0 +1,106 @@ +// Copyright 2021 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 user + +import ( + "context" + "fmt" + "time" + + "code.gitea.io/gitea/models" + admin_model "code.gitea.io/gitea/models/admin" + "code.gitea.io/gitea/models/db" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/storage" + "code.gitea.io/gitea/modules/util" +) + +// 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 *models.User) error { + if u.IsOrganization() { + return fmt.Errorf("%s is an organization not a user", u.Name) + } + + ctx, commiter, err := db.TxContext() + if err != nil { + return err + } + defer commiter.Close() + + // Note: A user owns any repository or belongs to any organization + // cannot perform delete operation. + + // Check ownership of repository. + count, err := models.GetRepositoryCount(ctx, u) + if err != nil { + return fmt.Errorf("GetRepositoryCount: %v", err) + } else if count > 0 { + return models.ErrUserOwnRepos{UID: u.ID} + } + + // Check membership of organization. + count, err = models.GetOrganizationCount(ctx, u) + if err != nil { + return fmt.Errorf("GetOrganizationCount: %v", err) + } else if count > 0 { + return models.ErrUserHasOrgs{UID: u.ID} + } + + if err := models.DeleteUser(ctx, u); err != nil { + return fmt.Errorf("DeleteUser: %v", err) + } + + if err := commiter.Commit(); err != nil { + return err + } + + // Note: There are something just cannot be roll back, + // so just keep error logs of those operations. + path := models.UserPath(u.Name) + if err := util.RemoveAll(path); err != nil { + err = fmt.Errorf("Failed to RemoveAll %s: %v", path, err) + _ = admin_model.CreateNotice(db.DefaultContext, admin_model.NoticeTask, fmt.Sprintf("delete user '%s': %v", u.Name, err)) + return err + } + + if u.Avatar != "" { + avatarPath := u.CustomAvatarRelativePath() + if err := storage.Avatars.Delete(avatarPath); err != nil { + err = fmt.Errorf("Failed to remove %s: %v", avatarPath, err) + _ = admin_model.CreateNotice(db.DefaultContext, admin_model.NoticeTask, fmt.Sprintf("delete user '%s': %v", u.Name, err)) + return err + } + } + + return nil +} + +// DeleteInactiveUsers deletes all inactive users and email addresses. +func DeleteInactiveUsers(ctx context.Context, olderThan time.Duration) error { + users, err := models.GetInactiveUsers(ctx, olderThan) + if err != nil { + return err + } + + // FIXME: should only update authorized_keys file once after all deletions. + for _, u := range users { + select { + case <-ctx.Done(): + return db.ErrCancelledf("Before delete inactive user %s", u.Name) + default: + } + if err := DeleteUser(u); err != nil { + // Ignore users that were set inactive by admin. + if models.IsErrUserOwnRepos(err) || models.IsErrUserHasOrgs(err) { + continue + } + return err + } + } + + return user_model.DeleteInactiveEmailAddresses(ctx) +} diff --git a/services/user/user_test.go b/services/user/user_test.go new file mode 100644 index 000000000..9162273fa --- /dev/null +++ b/services/user/user_test.go @@ -0,0 +1,101 @@ +// Copyright 2021 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 user + +import ( + "path/filepath" + "testing" + + "code.gitea.io/gitea/models" + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/models/unittest" + "code.gitea.io/gitea/modules/setting" + + "github.com/stretchr/testify/assert" +) + +func TestMain(m *testing.M) { + unittest.MainTest(m, filepath.Join("..", "..")) +} + +func TestDeleteUser(t *testing.T) { + test := func(userID int64) { + assert.NoError(t, unittest.PrepareTestDatabase()) + user := unittest.AssertExistsAndLoadBean(t, &models.User{ID: userID}).(*models.User) + + ownedRepos := make([]*models.Repository, 0, 10) + assert.NoError(t, db.GetEngine(db.DefaultContext).Find(&ownedRepos, &models.Repository{OwnerID: userID})) + if len(ownedRepos) > 0 { + err := DeleteUser(user) + assert.Error(t, err) + assert.True(t, models.IsErrUserOwnRepos(err)) + return + } + + orgUsers := make([]*models.OrgUser, 0, 10) + assert.NoError(t, db.GetEngine(db.DefaultContext).Find(&orgUsers, &models.OrgUser{UID: userID})) + for _, orgUser := range orgUsers { + if err := models.RemoveOrgUser(orgUser.OrgID, orgUser.UID); err != nil { + assert.True(t, models.IsErrLastOrgOwner(err)) + return + } + } + assert.NoError(t, DeleteUser(user)) + unittest.AssertNotExistsBean(t, &models.User{ID: userID}) + unittest.CheckConsistencyFor(t, &models.User{}, &models.Repository{}) + } + test(2) + test(4) + test(8) + test(11) + + org := unittest.AssertExistsAndLoadBean(t, &models.User{ID: 3}).(*models.User) + assert.Error(t, DeleteUser(org)) +} + +func TestCreateUser(t *testing.T) { + user := &models.User{ + Name: "GiteaBot", + Email: "GiteaBot@gitea.io", + Passwd: ";p['////..-++']", + IsAdmin: false, + Theme: setting.UI.DefaultTheme, + MustChangePassword: false, + } + + assert.NoError(t, models.CreateUser(user)) + + assert.NoError(t, DeleteUser(user)) +} + +func TestCreateUser_Issue5882(t *testing.T) { + // Init settings + _ = setting.Admin + + passwd := ".//.;1;;//.,-=_" + + tt := []struct { + user *models.User + disableOrgCreation bool + }{ + {&models.User{Name: "GiteaBot", Email: "GiteaBot@gitea.io", Passwd: passwd, MustChangePassword: false}, false}, + {&models.User{Name: "GiteaBot2", Email: "GiteaBot2@gitea.io", Passwd: passwd, MustChangePassword: false}, true}, + } + + setting.Service.DefaultAllowCreateOrganization = true + + for _, v := range tt { + setting.Admin.DisableRegularOrgCreation = v.disableOrgCreation + + assert.NoError(t, models.CreateUser(v.user)) + + u, err := models.GetUserByEmail(v.user.Email) + assert.NoError(t, err) + + assert.Equal(t, !u.AllowCreateOrganization, v.disableOrgCreation) + + assert.NoError(t, DeleteUser(v.user)) + } +} diff --git a/services/wiki/wiki.go b/services/wiki/wiki.go index 2db982a50..4c8d7dbc5 100644 --- a/services/wiki/wiki.go +++ b/services/wiki/wiki.go @@ -13,6 +13,7 @@ import ( "code.gitea.io/gitea/models" admin_model "code.gitea.io/gitea/models/admin" + "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/unit" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" @@ -375,6 +376,6 @@ func DeleteWiki(repo *models.Repository) error { return err } - admin_model.RemoveAllWithNotice("Delete repository wiki", repo.WikiPath()) + admin_model.RemoveAllWithNotice(db.DefaultContext, "Delete repository wiki", repo.WikiPath()) return nil }