IntenseWebs/grafana
3
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2026-08-18 17:15:08 -05:00
Code Issues Packages Projects Releases Wiki Activity

Chore: Move user errors to user service (#52460)

Browse Source 
* Move user not found err to user service

* User ErrCaseInsensitive from user pkg

* User ErrUserAlreadyExists from user pkg

* User ErrLastGrafanaAdmin from user pkg

* Remove errors from model
This commit is contained in:
idafurjes
2022-07-20 14:50:06 +02:00
committed by GitHub
parent 78f26a079c
commit d3d8fdd878
37 changed files with 237 additions and 230 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/api/admin_users.go
+13 -13
View File
@@ -39,13 +39,13 @@ func (hs *HTTPServer) AdminCreateUser(c *models.ReqContext) response.Response {
return response.Error(400, "Password is missing or too short", nil)
}
user, err := hs.Login.CreateUser(cmd)
usr, err := hs.Login.CreateUser(cmd)
if err != nil {
if errors.Is(err, models.ErrOrgNotFound) {
return response.Error(400, err.Error(), nil)
}
if errors.Is(err, models.ErrUserAlreadyExists) {
if errors.Is(err, user.ErrUserAlreadyExists) {
return response.Error(412, fmt.Sprintf("User with email '%s' or username '%s' already exists", form.Email, form.Login), err)
}
@@ -56,7 +56,7 @@ func (hs *HTTPServer) AdminCreateUser(c *models.ReqContext) response.Response {
result := models.UserIdDTO{
Message: "User created",
Id: user.ID,
Id: usr.ID,
}
return response.JSON(http.StatusOK, result)
@@ -113,8 +113,8 @@ func (hs *HTTPServer) AdminUpdateUserPermissions(c *models.ReqContext) response.
err = hs.SQLStore.UpdateUserPermissions(userID, form.IsGrafanaAdmin)
if err != nil {
if errors.Is(err, models.ErrLastGrafanaAdmin) {
return response.Error(400, models.ErrLastGrafanaAdmin.Error(), nil)
if errors.Is(err, user.ErrLastGrafanaAdmin) {
return response.Error(400, user.ErrLastGrafanaAdmin.Error(), nil)
}
return response.Error(500, "Failed to update user permissions", err)
@@ -132,8 +132,8 @@ func (hs *HTTPServer) AdminDeleteUser(c *models.ReqContext) response.Response {
cmd := models.DeleteUserCommand{UserId: userID}
if err := hs.SQLStore.DeleteUser(c.Req.Context(), &cmd); err != nil {
if errors.Is(err, models.ErrUserNotFound) {
return response.Error(404, models.ErrUserNotFound.Error(), nil)
if errors.Is(err, user.ErrUserNotFound) {
return response.Error(404, user.ErrUserNotFound.Error(), nil)
}
return response.Error(500, "Failed to delete user", err)
}
@@ -150,14 +150,14 @@ func (hs *HTTPServer) AdminDisableUser(c *models.ReqContext) response.Response {
// External users shouldn't be disabled from API
authInfoQuery := &models.GetAuthInfoQuery{UserId: userID}
if err := hs.authInfoService.GetAuthInfo(c.Req.Context(), authInfoQuery); !errors.Is(err, models.ErrUserNotFound) {
if err := hs.authInfoService.GetAuthInfo(c.Req.Context(), authInfoQuery); !errors.Is(err, user.ErrUserNotFound) {
return response.Error(500, "Could not disable external user", nil)
}
disableCmd := models.DisableUserCommand{UserId: userID, IsDisabled: true}
if err := hs.SQLStore.DisableUser(c.Req.Context(), &disableCmd); err != nil {
if errors.Is(err, models.ErrUserNotFound) {
return response.Error(404, models.ErrUserNotFound.Error(), nil)
if errors.Is(err, user.ErrUserNotFound) {
return response.Error(404, user.ErrUserNotFound.Error(), nil)
}
return response.Error(500, "Failed to disable user", err)
}
@@ -179,14 +179,14 @@ func (hs *HTTPServer) AdminEnableUser(c *models.ReqContext) response.Response {
// External users shouldn't be disabled from API
authInfoQuery := &models.GetAuthInfoQuery{UserId: userID}
if err := hs.authInfoService.GetAuthInfo(c.Req.Context(), authInfoQuery); !errors.Is(err, models.ErrUserNotFound) {
if err := hs.authInfoService.GetAuthInfo(c.Req.Context(), authInfoQuery); !errors.Is(err, user.ErrUserNotFound) {
return response.Error(500, "Could not enable external user", nil)
}
disableCmd := models.DisableUserCommand{UserId: userID, IsDisabled: false}
if err := hs.SQLStore.DisableUser(c.Req.Context(), &disableCmd); err != nil {
if errors.Is(err, models.ErrUserNotFound) {
return response.Error(404, models.ErrUserNotFound.Error(), nil)
if errors.Is(err, user.ErrUserNotFound) {
return response.Error(404, user.ErrUserNotFound.Error(), nil)
}
return response.Error(500, "Failed to enable user", err)
}
pkg/api/admin_users_test.go
+11 -10
View File
@@ -14,6 +14,7 @@ import (
"github.com/grafana/grafana/pkg/services/login/logintest"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/sqlstore/mockstore"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/setting"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@@ -34,7 +35,7 @@ func TestAdminAPIEndpoint(t *testing.T) {
IsGrafanaAdmin: false,
}
mock := &mockstore.SQLStoreMock{
ExpectedError: models.ErrLastGrafanaAdmin,
ExpectedError: user.ErrLastGrafanaAdmin,
}
putAdminScenario(t, "When calling PUT on", "/api/admin/users/1/permissions",
"/api/admin/users/:id/permissions", role, updateCmd, func(sc *scenarioContext) {
@@ -54,7 +55,7 @@ func TestAdminAPIEndpoint(t *testing.T) {
t.Run("When a server admin attempts to logout a non-existing user from all devices", func(t *testing.T) {
mock := &mockstore.SQLStoreMock{
ExpectedError: models.ErrUserNotFound,
ExpectedError: user.ErrUserNotFound,
}
adminLogoutUserScenario(t, "Should return not found when calling POST on", "/api/admin/users/200/logout",
"/api/admin/users/:id/logout", func(sc *scenarioContext) {
@@ -66,7 +67,7 @@ func TestAdminAPIEndpoint(t *testing.T) {
t.Run("When a server admin attempts to revoke an auth token for a non-existing user", func(t *testing.T) {
cmd := models.RevokeAuthTokenCmd{AuthTokenId: 2}
mock := &mockstore.SQLStoreMock{
ExpectedError: models.ErrUserNotFound,
ExpectedError: user.ErrUserNotFound,
}
adminRevokeUserAuthTokenScenario(t, "Should return not found when calling POST on",
"/api/admin/users/200/revoke-auth-token", "/api/admin/users/:id/revoke-auth-token", cmd, func(sc *scenarioContext) {
@@ -77,7 +78,7 @@ func TestAdminAPIEndpoint(t *testing.T) {
t.Run("When a server admin gets auth tokens for a non-existing user", func(t *testing.T) {
mock := &mockstore.SQLStoreMock{
ExpectedError: models.ErrUserNotFound,
ExpectedError: user.ErrUserNotFound,
}
adminGetUserAuthTokensScenario(t, "Should return not found when calling GET on",
"/api/admin/users/200/auth-tokens", "/api/admin/users/:id/auth-tokens", func(sc *scenarioContext) {
@@ -90,8 +91,8 @@ func TestAdminAPIEndpoint(t *testing.T) {
adminDisableUserScenario(t, "Should return user not found on a POST request", "enable",
"/api/admin/users/42/enable", "/api/admin/users/:id/enable", func(sc *scenarioContext) {
store := sc.sqlStore.(*mockstore.SQLStoreMock)
sc.authInfoService.ExpectedError = models.ErrUserNotFound
store.ExpectedError = models.ErrUserNotFound
sc.authInfoService.ExpectedError = user.ErrUserNotFound
store.ExpectedError = user.ErrUserNotFound
sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec()
@@ -107,8 +108,8 @@ func TestAdminAPIEndpoint(t *testing.T) {
adminDisableUserScenario(t, "Should return user not found on a POST request", "disable",
"/api/admin/users/42/disable", "/api/admin/users/:id/disable", func(sc *scenarioContext) {
store := sc.sqlStore.(*mockstore.SQLStoreMock)
sc.authInfoService.ExpectedError = models.ErrUserNotFound
store.ExpectedError = models.ErrUserNotFound
sc.authInfoService.ExpectedError = user.ErrUserNotFound
store.ExpectedError = user.ErrUserNotFound
sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec()
@@ -152,8 +153,8 @@ func TestAdminAPIEndpoint(t *testing.T) {
t.Run("When a server admin attempts to delete a nonexistent user", func(t *testing.T) {
adminDeleteUserScenario(t, "Should return user not found error", "/api/admin/users/42",
"/api/admin/users/:id", func(sc *scenarioContext) {
sc.sqlStore.(*mockstore.SQLStoreMock).ExpectedError = models.ErrUserNotFound
sc.authInfoService.ExpectedError = models.ErrUserNotFound
sc.sqlStore.(*mockstore.SQLStoreMock).ExpectedError = user.ErrUserNotFound
sc.authInfoService.ExpectedError = user.ErrUserNotFound
sc.fakeReqWithParams("DELETE", sc.url, map[string]string{}).exec()
userID := sc.sqlStore.(*mockstore.SQLStoreMock).LatestUserId
pkg/api/ldap_debug.go
+5 -4
View File
@@ -14,6 +14,7 @@ import (
"github.com/grafana/grafana/pkg/services/ldap"
"github.com/grafana/grafana/pkg/services/multildap"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/util"
"github.com/grafana/grafana/pkg/web"
)
@@ -172,8 +173,8 @@ func (hs *HTTPServer) PostSyncUserWithLDAP(c *models.ReqContext) response.Respon
query := models.GetUserByIdQuery{Id: userId}
if err := hs.SQLStore.GetUserById(c.Req.Context(), &query); err != nil { // validate the userId exists
if errors.Is(err, models.ErrUserNotFound) {
return response.Error(404, models.ErrUserNotFound.Error(), nil)
if errors.Is(err, user.ErrUserNotFound) {
return response.Error(404, user.ErrUserNotFound.Error(), nil)
}
return response.Error(500, "Failed to get user", err)
@@ -181,8 +182,8 @@ func (hs *HTTPServer) PostSyncUserWithLDAP(c *models.ReqContext) response.Respon
authModuleQuery := &models.GetAuthInfoQuery{UserId: query.Result.ID, AuthModule: models.AuthModuleLDAP}
if err := hs.authInfoService.GetAuthInfo(c.Req.Context(), authModuleQuery); err != nil { // validate the userId comes from LDAP
if errors.Is(err, models.ErrUserNotFound) {
return response.Error(404, models.ErrUserNotFound.Error(), nil)
if errors.Is(err, user.ErrUserNotFound) {
return response.Error(404, user.ErrUserNotFound.Error(), nil)
}
return response.Error(500, "Failed to get user", err)
pkg/api/ldap_debug_test.go
+1 -1
View File
@@ -431,7 +431,7 @@ func TestPostSyncUserWithLDAPAPIEndpoint_Success(t *testing.T) {
}
func TestPostSyncUserWithLDAPAPIEndpoint_WhenUserNotFound(t *testing.T) {
sqlstoremock := mockstore.SQLStoreMock{ExpectedError: models.ErrUserNotFound}
sqlstoremock := mockstore.SQLStoreMock{ExpectedError: user.ErrUserNotFound}
sc := postSyncUserWithLDAPContext(t, "/api/admin/ldap/sync/34", func(t *testing.T, sc *scenarioContext) {
getLDAPConfig = func(*setting.Cfg) (*ldap.Config, error) {
return &ldap.Config{}, nil
pkg/api/login.go
+5 -5
View File
@@ -180,7 +180,7 @@ func (hs *HTTPServer) LoginPost(c *models.ReqContext) response.Response {
return response.Error(http.StatusBadRequest, "bad login data", err)
}
authModule := ""
var user *user.User
var usr *user.User
var resp *response.NormalResponse
defer func() {
@@ -190,7 +190,7 @@ func (hs *HTTPServer) LoginPost(c *models.ReqContext) response.Response {
}
hs.HooksService.RunLoginHook(&models.LoginInfo{
AuthModule: authModule,
User: user,
User: usr,
LoginUsername: cmd.User,
HTTPStatus: resp.Status(),
Error: err,
@@ -215,7 +215,7 @@ func (hs *HTTPServer) LoginPost(c *models.ReqContext) response.Response {
if err != nil {
resp = response.Error(401, "Invalid username or password", err)
if errors.Is(err, login.ErrInvalidCredentials) || errors.Is(err, login.ErrTooManyLoginAttempts) || errors.Is(err,
models.ErrUserNotFound) {
user.ErrUserNotFound) {
return resp
}
@@ -230,9 +230,9 @@ func (hs *HTTPServer) LoginPost(c *models.ReqContext) response.Response {
return resp
}
user = authQuery.User
usr = authQuery.User
err = hs.loginUserWithUser(user, c)
err = hs.loginUserWithUser(usr, c)
if err != nil {
var createTokenErr *models.CreateTokenErr
if errors.As(err, &createTokenErr) {
pkg/api/org_invite.go
+2 -2
View File
@@ -48,7 +48,7 @@ func (hs *HTTPServer) AddOrgInvite(c *models.ReqContext) response.Response {
// first try get existing user
userQuery := models.GetUserByLoginQuery{LoginOrEmail: inviteDto.LoginOrEmail}
if err := hs.SQLStore.GetUserByLogin(c.Req.Context(), &userQuery); err != nil {
if !errors.Is(err, models.ErrUserNotFound) {
if !errors.Is(err, user.ErrUserNotFound) {
return response.Error(500, "Failed to query db for existing user check", err)
}
} else {
@@ -221,7 +221,7 @@ func (hs *HTTPServer) CompleteInvite(c *models.ReqContext) response.Response {
usr, err := hs.Login.CreateUser(cmd)
if err != nil {
if errors.Is(err, models.ErrUserAlreadyExists) {
if errors.Is(err, user.ErrUserAlreadyExists) {
return response.Error(412, fmt.Sprintf("User with email '%s' or username '%s' already exists", completeInvite.Email, completeInvite.Username), err)
}
pkg/api/signup.go
+1 -1
View File
@@ -94,7 +94,7 @@ func (hs *HTTPServer) SignUpStep2(c *models.ReqContext) response.Response {
usr, err := hs.Login.CreateUser(createUserCmd)
if err != nil {
if errors.Is(err, models.ErrUserAlreadyExists) {
if errors.Is(err, user.ErrUserAlreadyExists) {
return response.Error(401, "User with same email address already exists", nil)
}
pkg/api/user.go
+6 -5
View File
@@ -9,6 +9,7 @@ import (
"github.com/grafana/grafana/pkg/api/dtos"
"github.com/grafana/grafana/pkg/api/response"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/util"
"github.com/grafana/grafana/pkg/web"
@@ -32,8 +33,8 @@ func (hs *HTTPServer) getUserUserProfile(c *models.ReqContext, userID int64) res
query := models.GetUserProfileQuery{UserId: userID}
if err := hs.SQLStore.GetUserProfile(c.Req.Context(), &query); err != nil {
if errors.Is(err, models.ErrUserNotFound) {
return response.Error(404, models.ErrUserNotFound.Error(), nil)
if errors.Is(err, user.ErrUserNotFound) {
return response.Error(404, user.ErrUserNotFound.Error(), nil)
}
return response.Error(500, "Failed to get user", err)
}
@@ -56,8 +57,8 @@ func (hs *HTTPServer) getUserUserProfile(c *models.ReqContext, userID int64) res
func (hs *HTTPServer) GetUserByLoginOrEmail(c *models.ReqContext) response.Response {
query := models.GetUserByLoginQuery{LoginOrEmail: c.Query("loginOrEmail")}
if err := hs.SQLStore.GetUserByLogin(c.Req.Context(), &query); err != nil {
if errors.Is(err, models.ErrUserNotFound) {
return response.Error(404, models.ErrUserNotFound.Error(), nil)
if errors.Is(err, user.ErrUserNotFound) {
return response.Error(404, user.ErrUserNotFound.Error(), nil)
}
return response.Error(500, "Failed to get user", err)
}
@@ -141,7 +142,7 @@ func (hs *HTTPServer) handleUpdateUser(ctx context.Context, cmd models.UpdateUse
}
if err := hs.SQLStore.UpdateUser(ctx, &cmd); err != nil {
if errors.Is(err, models.ErrCaseInsensitive) {
if errors.Is(err, user.ErrCaseInsensitive) {
return response.Error(http.StatusConflict, "Update would result in user login conflict", err)
}
return response.Error(http.StatusInternalServerError, "Failed to update user", err)
pkg/api/user_token.go
+5 -4
View File
@@ -9,6 +9,7 @@ import (
"github.com/grafana/grafana/pkg/api/dtos"
"github.com/grafana/grafana/pkg/api/response"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/util"
"github.com/grafana/grafana/pkg/web"
"github.com/ua-parser/uap-go/uaparser"
@@ -32,7 +33,7 @@ func (hs *HTTPServer) logoutUserFromAllDevicesInternal(ctx context.Context, user
userQuery := models.GetUserByIdQuery{Id: userID}
if err := hs.SQLStore.GetUserById(ctx, &userQuery); err != nil {
if errors.Is(err, models.ErrUserNotFound) {
if errors.Is(err, user.ErrUserNotFound) {
return response.Error(404, "User not found", err)
}
return response.Error(500, "Could not read user from database", err)
@@ -52,9 +53,9 @@ func (hs *HTTPServer) getUserAuthTokensInternal(c *models.ReqContext, userID int
userQuery := models.GetUserByIdQuery{Id: userID}
if err := hs.SQLStore.GetUserById(c.Req.Context(), &userQuery); err != nil {
if errors.Is(err, models.ErrUserNotFound) {
if errors.Is(err, user.ErrUserNotFound) {
return response.Error(http.StatusNotFound, "User not found", err)
} else if errors.Is(err, models.ErrCaseInsensitive) {
} else if errors.Is(err, user.ErrCaseInsensitive) {
return response.Error(http.StatusConflict,
"User has conflicting login or email with another user. Please contact server admin", err)
}
@@ -122,7 +123,7 @@ func (hs *HTTPServer) getUserAuthTokensInternal(c *models.ReqContext, userID int
func (hs *HTTPServer) revokeUserAuthTokenInternal(c *models.ReqContext, userID int64, cmd models.RevokeAuthTokenCmd) response.Response {
userQuery := models.GetUserByIdQuery{Id: userID}
if err := hs.SQLStore.GetUserById(c.Req.Context(), &userQuery); err != nil {
if errors.Is(err, models.ErrUserNotFound) {
if errors.Is(err, user.ErrUserNotFound) {
return response.Error(404, "User not found", err)
}
return response.Error(500, "Failed to get user", err)
pkg/api/user_token_test.go
+3 -3
View File
@@ -20,7 +20,7 @@ func TestUserTokenAPIEndpoint(t *testing.T) {
mock := mockstore.NewSQLStoreMock()
t.Run("When current user attempts to revoke an auth token for a non-existing user", func(t *testing.T) {
cmd := models.RevokeAuthTokenCmd{AuthTokenId: 2}
mock.ExpectedError = models.ErrUserNotFound
mock.ExpectedError = user.ErrUserNotFound
revokeUserAuthTokenScenario(t, "Should return not found when calling POST on", "/api/user/revoke-auth-token",
"/api/user/revoke-auth-token", cmd, 200, func(sc *scenarioContext) {
sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec()
@@ -31,7 +31,7 @@ func TestUserTokenAPIEndpoint(t *testing.T) {
t.Run("When current user gets auth tokens for a non-existing user", func(t *testing.T) {
mock := &mockstore.SQLStoreMock{
ExpectedUser: &user.User{ID: 200},
ExpectedError: models.ErrUserNotFound,
ExpectedError: user.ErrUserNotFound,
}
getUserAuthTokensScenario(t, "Should return not found when calling GET on", "/api/user/auth-tokens", "/api/user/auth-tokens", 200, func(sc *scenarioContext) {
sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec()
@@ -51,7 +51,7 @@ func TestUserTokenAPIEndpoint(t *testing.T) {
t.Run("When logout a non-existing user from all devices", func(t *testing.T) {
logoutUserFromAllDevicesInternalScenario(t, "Should return not found", testUserID, func(sc *scenarioContext) {
mock.ExpectedError = models.ErrUserNotFound
mock.ExpectedError = user.ErrUserNotFound
sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec()
assert.Equal(t, 404, sc.resp.Code)