From 24b74c2d94fc599459c727feb92dce3257c1b400 Mon Sep 17 00:00:00 2001 From: viktorstrate Date: Tue, 11 May 2021 20:40:18 +0200 Subject: [PATCH] Add check that prevents deletion of sole admin user --- api/graphql/models/actions/actions_test.go | 12 +++ api/graphql/models/actions/user_actions.go | 70 ++++++++++++++++ .../models/actions/user_actions_test.go | 82 +++++++++++++++++++ api/graphql/resolvers/user.go | 49 +---------- 4 files changed, 166 insertions(+), 47 deletions(-) create mode 100644 api/graphql/models/actions/actions_test.go create mode 100644 api/graphql/models/actions/user_actions.go create mode 100644 api/graphql/models/actions/user_actions_test.go diff --git a/api/graphql/models/actions/actions_test.go b/api/graphql/models/actions/actions_test.go new file mode 100644 index 00000000..7e6f46b2 --- /dev/null +++ b/api/graphql/models/actions/actions_test.go @@ -0,0 +1,12 @@ +package actions_test + +import ( + "os" + "testing" + + "github.com/photoview/photoview/api/test_utils" +) + +func TestMain(m *testing.M) { + os.Exit(test_utils.IntegrationTestRun(m)) +} diff --git a/api/graphql/models/actions/user_actions.go b/api/graphql/models/actions/user_actions.go new file mode 100644 index 00000000..55d23dc0 --- /dev/null +++ b/api/graphql/models/actions/user_actions.go @@ -0,0 +1,70 @@ +package actions + +import ( + "errors" + "os" + "path" + "strconv" + + "github.com/photoview/photoview/api/graphql/models" + "github.com/photoview/photoview/api/utils" + "gorm.io/gorm" +) + +func DeleteUser(db *gorm.DB, userID int) (*models.User, error) { + + // make sure the last admin user is not deleted + var adminUsers []*models.User + db.Model(&models.User{}).Where("admin = true").Limit(2).Find(&adminUsers) + if len(adminUsers) == 1 && adminUsers[0].ID == userID { + return nil, errors.New("deleting sole admin user is not allowed") + } + + var user models.User + deletedAlbumIDs := make([]int, 0) + + err := db.Transaction(func(tx *gorm.DB) error { + if err := tx.First(&user, userID).Error; err != nil { + return err + } + + userAlbums := user.Albums + if err := tx.Model(&user).Association("Albums").Find(&userAlbums); err != nil { + return err + } + + if err := tx.Model(&user).Association("Albums").Clear(); err != nil { + return err + } + + for _, album := range userAlbums { + var associatedUsers = tx.Model(album).Association("Owners").Count() + + if associatedUsers == 0 { + deletedAlbumIDs = append(deletedAlbumIDs, album.ID) + if err := tx.Delete(album).Error; err != nil { + return err + } + } + } + + if err := tx.Delete(&user).Error; err != nil { + return err + } + + return nil + }) + + if err != nil { + return nil, err + } + + // If there is only one associated user, clean up the cache folder and delete the album row + for _, deletedAlbumID := range deletedAlbumIDs { + cachePath := path.Join(utils.MediaCachePath(), strconv.Itoa(int(deletedAlbumID))) + if err := os.RemoveAll(cachePath); err != nil { + return &user, err + } + } + return &user, nil +} diff --git a/api/graphql/models/actions/user_actions_test.go b/api/graphql/models/actions/user_actions_test.go new file mode 100644 index 00000000..6ab69528 --- /dev/null +++ b/api/graphql/models/actions/user_actions_test.go @@ -0,0 +1,82 @@ +package actions_test + +import ( + "testing" + + "github.com/photoview/photoview/api/graphql/models" + "github.com/photoview/photoview/api/graphql/models/actions" + "github.com/photoview/photoview/api/test_utils" + "github.com/stretchr/testify/assert" +) + +func TestDeleteUser(t *testing.T) { + t.Run("Delete regular user", func(t *testing.T) { + db := test_utils.DatabaseTest(t) + + adminUser, err := models.RegisterUser(db, "admin", nil, true) + assert.NoError(t, err) + + regularUser, err := models.RegisterUser(db, "regular", nil, false) + assert.NoError(t, err) + + var dbUsers []*models.User + err = db.Model(models.User{}).Find(&dbUsers).Error + assert.NoError(t, err) + assert.Len(t, dbUsers, 2) + + deletedUser, err := actions.DeleteUser(db, regularUser.ID) + assert.NoError(t, err) + assert.Equal(t, regularUser.ID, deletedUser.ID) + + err = db.Model(models.User{}).Find(&dbUsers).Error + assert.NoError(t, err) + assert.Len(t, dbUsers, 1) + assert.Equal(t, adminUser.ID, dbUsers[0].ID) + }) + + t.Run("Try to delete sole admin user", func(t *testing.T) { + db := test_utils.DatabaseTest(t) + + adminUser, err := models.RegisterUser(db, "admin", nil, true) + assert.NoError(t, err) + + _, err = models.RegisterUser(db, "regular", nil, false) + assert.NoError(t, err) + + var dbUsers []*models.User + err = db.Model(models.User{}).Find(&dbUsers).Error + assert.NoError(t, err) + assert.Len(t, dbUsers, 2) + + _, err = actions.DeleteUser(db, adminUser.ID) + assert.Error(t, err) + + err = db.Model(models.User{}).Find(&dbUsers).Error + assert.NoError(t, err) + assert.Len(t, dbUsers, 2) + }) + + t.Run("Delete admin user when multiple admins exist", func(t *testing.T) { + db := test_utils.DatabaseTest(t) + + adminUser1, err := models.RegisterUser(db, "admin", nil, true) + assert.NoError(t, err) + + adminUser2, err := models.RegisterUser(db, "another_admin", nil, true) + assert.NoError(t, err) + + var dbUsers []*models.User + err = db.Model(models.User{}).Find(&dbUsers).Error + assert.NoError(t, err) + assert.Len(t, dbUsers, 2) + + deletedUser, err := actions.DeleteUser(db, adminUser1.ID) + assert.NoError(t, err) + assert.Equal(t, adminUser1.ID, deletedUser.ID) + + err = db.Model(models.User{}).Find(&dbUsers).Error + assert.NoError(t, err) + assert.Len(t, dbUsers, 1) + assert.Equal(t, adminUser2.ID, dbUsers[0].ID) + }) +} diff --git a/api/graphql/resolvers/user.go b/api/graphql/resolvers/user.go index 578baef7..23bf3be3 100644 --- a/api/graphql/resolvers/user.go +++ b/api/graphql/resolvers/user.go @@ -9,6 +9,7 @@ import ( api "github.com/photoview/photoview/api/graphql" "github.com/photoview/photoview/api/graphql/auth" "github.com/photoview/photoview/api/graphql/models" + "github.com/photoview/photoview/api/graphql/models/actions" "github.com/photoview/photoview/api/scanner" "github.com/photoview/photoview/api/scanner/face_detection" "github.com/photoview/photoview/api/utils" @@ -254,53 +255,7 @@ func (r *mutationResolver) CreateUser(ctx context.Context, username string, pass } func (r *mutationResolver) DeleteUser(ctx context.Context, id int) (*models.User, error) { - var user models.User - deletedAlbumIDs := make([]int, 0) - - err := r.Database.Transaction(func(tx *gorm.DB) error { - if err := tx.First(&user, id).Error; err != nil { - return err - } - - userAlbums := user.Albums - if err := tx.Model(&user).Association("Albums").Find(&userAlbums); err != nil { - return err - } - - if err := tx.Model(&user).Association("Albums").Clear(); err != nil { - return err - } - - for _, album := range userAlbums { - var associatedUsers = tx.Model(album).Association("Owners").Count() - - if associatedUsers == 0 { - deletedAlbumIDs = append(deletedAlbumIDs, album.ID) - if err := tx.Delete(album).Error; err != nil { - return err - } - } - } - - if err := tx.Delete(&user).Error; err != nil { - return err - } - - return nil - }) - - if err != nil { - return nil, err - } - - // If there is only one associated user, clean up the cache folder and delete the album row - for _, deletedAlbumID := range deletedAlbumIDs { - cachePath := path.Join(utils.MediaCachePath(), strconv.Itoa(int(deletedAlbumID))) - if err := os.RemoveAll(cachePath); err != nil { - return &user, err - } - } - return &user, nil + return actions.DeleteUser(r.Database, id) } func (r *mutationResolver) UserAddRootPath(ctx context.Context, id int, rootPath string) (*models.Album, error) {