diff --git a/pkg/api/admin_users.go b/pkg/api/admin_users.go index 6570e69f0d9..dff36c19084 100644 --- a/pkg/api/admin_users.go +++ b/pkg/api/admin_users.go @@ -108,6 +108,10 @@ func AdminDeleteUser(c *models.ReqContext) { cmd := models.DeleteUserCommand{UserId: userID} if err := bus.Dispatch(&cmd); err != nil { + if err == models.ErrUserNotFound { + c.JsonApiErr(404, models.ErrUserNotFound.Error(), nil) + return + } c.JsonApiErr(500, "Failed to delete user", err) return } @@ -127,6 +131,9 @@ func (server *HTTPServer) AdminDisableUser(c *models.ReqContext) Response { disableCmd := models.DisableUserCommand{UserId: userID, IsDisabled: true} if err := bus.Dispatch(&disableCmd); err != nil { + if err == models.ErrUserNotFound { + return Error(404, models.ErrUserNotFound.Error(), nil) + } return Error(500, "Failed to disable user", err) } @@ -150,6 +157,9 @@ func AdminEnableUser(c *models.ReqContext) Response { disableCmd := models.DisableUserCommand{UserId: userID, IsDisabled: false} if err := bus.Dispatch(&disableCmd); err != nil { + if err == models.ErrUserNotFound { + return Error(404, models.ErrUserNotFound.Error(), nil) + } return Error(500, "Failed to enable user", err) } diff --git a/pkg/api/admin_users_test.go b/pkg/api/admin_users_test.go index 4d858183545..75f0be3c072 100644 --- a/pkg/api/admin_users_test.go +++ b/pkg/api/admin_users_test.go @@ -86,6 +86,46 @@ func TestAdminApiEndpoint(t *testing.T) { }) }) + Convey("When a server admin attempts to enable/disable a nonexistent user", t, func() { + var userId int64 + isDisabled := false + bus.AddHandler("test", func(cmd *m.GetAuthInfoQuery) error { + return m.ErrUserNotFound + }) + + bus.AddHandler("test", func(cmd *m.DisableUserCommand) error { + userId = cmd.UserId + isDisabled = cmd.IsDisabled + return m.ErrUserNotFound + }) + + adminDisableUserScenario("Should return user not found on a POST request", "enable", "/api/admin/users/42/enable", "/api/admin/users/:id/enable", func(sc *scenarioContext) { + sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec() + + So(sc.resp.Code, ShouldEqual, 404) + respJSON, err := simplejson.NewJson(sc.resp.Body.Bytes()) + So(err, ShouldBeNil) + + So(respJSON.Get("message").MustString(), ShouldEqual, "User not found") + + So(userId, ShouldEqual, 42) + So(isDisabled, ShouldEqual, false) + }) + + adminDisableUserScenario("Should return user not found on a POST request", "disable", "/api/admin/users/42/disable", "/api/admin/users/:id/disable", func(sc *scenarioContext) { + sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec() + + So(sc.resp.Code, ShouldEqual, 404) + respJSON, err := simplejson.NewJson(sc.resp.Body.Bytes()) + So(err, ShouldBeNil) + + So(respJSON.Get("message").MustString(), ShouldEqual, "User not found") + + So(userId, ShouldEqual, 42) + So(isDisabled, ShouldEqual, true) + }) + }) + Convey("When a server admin attempts to disable/enable external user", t, func() { userId := int64(0) bus.AddHandler("test", func(cmd *m.GetAuthInfoQuery) error { @@ -115,6 +155,26 @@ func TestAdminApiEndpoint(t *testing.T) { So(userId, ShouldEqual, 42) }) }) + + Convey("When a server admin attempts to delete a nonexistent user", t, func() { + var userId int64 + bus.AddHandler("test", func(cmd *m.DeleteUserCommand) error { + userId = cmd.UserId + return m.ErrUserNotFound + }) + + adminDeleteUserScenario("Should return user not found error", "/api/admin/users/42", "/api/admin/users/:id", func(sc *scenarioContext) { + sc.fakeReqWithParams("DELETE", sc.url, map[string]string{}).exec() + + So(sc.resp.Code, ShouldEqual, 404) + + respJSON, err := simplejson.NewJson(sc.resp.Body.Bytes()) + So(err, ShouldBeNil) + So(respJSON.Get("message").MustString(), ShouldEqual, "User not found") + + So(userId, ShouldEqual, 42) + }) + }) } func putAdminScenario(desc string, url string, routePattern string, role m.RoleType, cmd dtos.AdminUpdateUserPermissionsForm, fn scenarioFunc) { @@ -246,3 +306,21 @@ func adminDisableUserScenario(desc string, action string, url string, routePatte fn(sc) }) } + +func adminDeleteUserScenario(desc string, url string, routePattern string, fn scenarioFunc) { + Convey(desc+" "+url, func() { + defer bus.ClearBusHandlers() + + sc := setupScenarioContext(url) + sc.defaultHandler = Wrap(func(c *m.ReqContext) { + sc.context = c + sc.context.UserId = TestUserID + + AdminDeleteUser(c) + }) + + sc.m.Delete(routePattern, sc.defaultHandler) + + fn(sc) + }) +} diff --git a/pkg/services/sqlstore/user.go b/pkg/services/sqlstore/user.go index dc0264e3787..c401ef18bbe 100644 --- a/pkg/services/sqlstore/user.go +++ b/pkg/services/sqlstore/user.go @@ -480,8 +480,11 @@ func SearchUsers(query *models.SearchUsersQuery) error { func DisableUser(cmd *models.DisableUserCommand) error { user := models.User{} sess := x.Table("user") - if _, err := sess.ID(cmd.UserId).Get(&user); err != nil { + + if has, err := sess.ID(cmd.UserId).Get(&user); err != nil { return err + } else if !has { + return models.ErrUserNotFound } user.IsDisabled = cmd.IsDisabled @@ -523,6 +526,16 @@ func DeleteUser(cmd *models.DeleteUserCommand) error { } func deleteUserInTransaction(sess *DBSession, cmd *models.DeleteUserCommand) error { + //Check if user exists + user := models.User{Id: cmd.UserId} + has, err := sess.Get(&user) + if err != nil { + return err + } + if !has { + return models.ErrUserNotFound + } + deletes := []string{ "DELETE FROM star WHERE user_id = ?", "DELETE FROM " + dialect.Quote("user") + " WHERE id = ?",