From 86669be84b3d33425b3b6e2607d735a9f5504358 Mon Sep 17 00:00:00 2001 From: Marvin Zhang Date: Wed, 13 Nov 2024 17:02:51 +0800 Subject: [PATCH] fix: disallow deleting for root admin users --- core/controllers/router.go | 10 +++ core/controllers/user.go | 76 ++++++++++++++++++ core/controllers/user_test.go | 144 +++++++++++++++++++++++++++++++++- 3 files changed, 226 insertions(+), 4 deletions(-) diff --git a/core/controllers/router.go b/core/controllers/router.go index 0836043b..4f13e9bb 100644 --- a/core/controllers/router.go +++ b/core/controllers/router.go @@ -255,6 +255,16 @@ func InitRoutes(app *gin.Engine) (err error) { Path: "/:id/change-password", HandlerFunc: PostUserChangePassword, }, + { + Method: http.MethodDelete, + Path: "/:id", + HandlerFunc: DeleteUserById, + }, + { + Method: http.MethodDelete, + Path: "", + HandlerFunc: DeleteUserList, + }, { Method: http.MethodGet, Path: "/me", diff --git a/core/controllers/user.go b/core/controllers/user.go index 37a41266..cc789b91 100644 --- a/core/controllers/user.go +++ b/core/controllers/user.go @@ -162,6 +162,82 @@ func PostUserChangePassword(c *gin.Context) { postUserChangePassword(id, c) } +func DeleteUserById(c *gin.Context) { + id, err := primitive.ObjectIDFromHex(c.Param("id")) + if err != nil { + HandleErrorBadRequest(c, err) + return + } + + user, err := service.NewModelService[models.User]().GetById(id) + if err != nil { + HandleErrorInternalServerError(c, err) + return + } + if user.RootAdmin { + HandleErrorBadRequest(c, errors.New("root admin cannot be deleted")) + return + } + + if err := service.NewModelService[models.User]().DeleteById(id); err != nil { + HandleErrorInternalServerError(c, err) + return + } + + HandleSuccess(c) +} + +func DeleteUserList(c *gin.Context) { + type Payload struct { + Ids []string `json:"ids"` + } + + var payload Payload + if err := c.ShouldBindJSON(&payload); err != nil { + HandleErrorBadRequest(c, err) + return + } + + // Convert string IDs to ObjectIDs + var ids []primitive.ObjectID + for _, id := range payload.Ids { + objectId, err := primitive.ObjectIDFromHex(id) + if err != nil { + HandleErrorBadRequest(c, err) + return + } + ids = append(ids, objectId) + } + + // Check if root admin is in the list + _, err := service.NewModelService[models.User]().GetOne(bson.M{ + "_id": bson.M{ + "$in": ids, + }, + "root_admin": true, + }, nil) + if err == nil { + HandleErrorBadRequest(c, errors.New("root admin cannot be deleted")) + return + } + if !errors.Is(err, mongo2.ErrNoDocuments) { + HandleErrorInternalServerError(c, err) + return + } + + // Delete users + if err := service.NewModelService[models.User]().DeleteMany(bson.M{ + "_id": bson.M{ + "$in": ids, + }, + }); err != nil { + HandleErrorInternalServerError(c, err) + return + } + + HandleSuccess(c) +} + func GetUserMe(c *gin.Context) { u := GetUserFromContext(c) getUserById(u.Id, c) diff --git a/core/controllers/user_test.go b/core/controllers/user_test.go index ebe6fa1f..94fa93c0 100644 --- a/core/controllers/user_test.go +++ b/core/controllers/user_test.go @@ -1,6 +1,12 @@ package controllers_test import ( + "fmt" + "net/http" + "net/http/httptest" + "strings" + "testing" + "github.com/crawlab-team/crawlab/core/controllers" "github.com/crawlab-team/crawlab/core/middlewares" "github.com/crawlab-team/crawlab/core/models/models" @@ -10,10 +16,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.mongodb.org/mongo-driver/bson" - "net/http" - "net/http/httptest" - "strings" - "testing" + "go.mongodb.org/mongo-driver/bson/primitive" "github.com/crawlab-team/crawlab/core/utils" ) @@ -369,3 +372,136 @@ func TestPostUserMeChangePassword_Success(t *testing.T) { router.ServeHTTP(w, req) assert.Equal(t, http.StatusBadRequest, w.Code) } + +func TestDeleteUserById_Success(t *testing.T) { + SetupTestDB() + defer CleanupTestDB() + + // Create test user + modelSvc := service.NewModelService[models.User]() + u := models.User{ + Username: "testuser", + Email: "test@example.com", + Password: utils.EncryptMd5("testpassword"), + } + id, err := modelSvc.InsertOne(u) + require.Nil(t, err) + u.SetId(id) + + router := gin.Default() + router.Use(middlewares.AuthorizationMiddleware()) + router.DELETE("/users/:id", controllers.DeleteUserById) + + // Test deleting normal user + req, err := http.NewRequest(http.MethodDelete, "/users/"+id.Hex(), nil) + assert.Nil(t, err) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Authorization", TestToken) + + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + assert.Equal(t, http.StatusOK, w.Code) + + // Verify user was deleted + _, err = modelSvc.GetById(id) + assert.NotNil(t, err) + + // Test deleting root admin user + rootAdmin := models.User{ + Username: "rootadmin", + Email: "root@example.com", + Password: utils.EncryptMd5("rootpass"), + RootAdmin: true, + } + rootId, err := modelSvc.InsertOne(rootAdmin) + require.Nil(t, err) + + req, err = http.NewRequest(http.MethodDelete, "/users/"+rootId.Hex(), nil) + assert.Nil(t, err) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Authorization", TestToken) + + w = httptest.NewRecorder() + router.ServeHTTP(w, req) + assert.Equal(t, http.StatusBadRequest, w.Code) + + // Test deleting with invalid ID + req, err = http.NewRequest(http.MethodDelete, "/users/invalid-id", nil) + assert.Nil(t, err) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Authorization", TestToken) + + w = httptest.NewRecorder() + router.ServeHTTP(w, req) + assert.Equal(t, http.StatusBadRequest, w.Code) +} + +func TestDeleteUserList_Success(t *testing.T) { + SetupTestDB() + defer CleanupTestDB() + + modelSvc := service.NewModelService[models.User]() + + // Create test users + users := []models.User{ + {Username: "user1", Email: "user1@example.com", Password: utils.EncryptMd5("pass1")}, + {Username: "user2", Email: "user2@example.com", Password: utils.EncryptMd5("pass2")}, + {Username: "rootadmin", Email: "root@example.com", Password: utils.EncryptMd5("rootpass"), RootAdmin: true}, + } + + var userIds []primitive.ObjectID + var normalUserIds []primitive.ObjectID + for _, user := range users { + id, err := modelSvc.InsertOne(user) + require.Nil(t, err) + userIds = append(userIds, id) + if !user.RootAdmin { + normalUserIds = append(normalUserIds, id) + } + } + + router := gin.Default() + router.Use(middlewares.AuthorizationMiddleware()) + router.DELETE("/users", controllers.DeleteUserList) + + // Test deleting normal users + reqBody := strings.NewReader(fmt.Sprintf(`{"ids":["%s","%s"]}`, + normalUserIds[0].Hex(), + normalUserIds[1].Hex())) + req, err := http.NewRequest(http.MethodDelete, "/users", reqBody) + assert.Nil(t, err) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Authorization", TestToken) + + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + assert.Equal(t, http.StatusOK, w.Code) + + // Verify users were deleted + for _, id := range normalUserIds { + _, err = modelSvc.GetById(id) + assert.NotNil(t, err) + } + + // Test attempting to delete list including root admin + reqBody = strings.NewReader(fmt.Sprintf(`{"ids":["%s"]}`, userIds[2].Hex())) + req, err = http.NewRequest(http.MethodDelete, "/users", reqBody) + assert.Nil(t, err) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Authorization", TestToken) + + w = httptest.NewRecorder() + router.ServeHTTP(w, req) + assert.Equal(t, http.StatusBadRequest, w.Code) + + // Test with mix of valid and invalid ids + reqBody = strings.NewReader(fmt.Sprintf(`{"ids":["%s","invalid-id"]}`, normalUserIds[0].Hex())) + req, err = http.NewRequest(http.MethodDelete, "/users", reqBody) + assert.Nil(t, err) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Authorization", TestToken) + + w = httptest.NewRecorder() + router.ServeHTTP(w, req) + assert.Equal(t, http.StatusBadRequest, w.Code) +}