ApiUser: Fix response when enabling, disabling or deleting a nonexistent user (#21391)

* ApiUser: Fix response when enabling, disabling or deleting a nonexistent user

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)
 	}
 

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)
+	})
+}

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 = ?",