diff --git a/pkg/api/admin_users.go b/pkg/api/admin_users.go index e5febcb0fe5..56265711893 100644 --- a/pkg/api/admin_users.go +++ b/pkg/api/admin_users.go @@ -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) } diff --git a/pkg/api/admin_users_test.go b/pkg/api/admin_users_test.go index 1ab6c368b38..71a5d6bbf55 100644 --- a/pkg/api/admin_users_test.go +++ b/pkg/api/admin_users_test.go @@ -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 diff --git a/pkg/api/ldap_debug.go b/pkg/api/ldap_debug.go index 48015002395..6468d9998a8 100644 --- a/pkg/api/ldap_debug.go +++ b/pkg/api/ldap_debug.go @@ -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) diff --git a/pkg/api/ldap_debug_test.go b/pkg/api/ldap_debug_test.go index 074ee98f695..509e3e684dd 100644 --- a/pkg/api/ldap_debug_test.go +++ b/pkg/api/ldap_debug_test.go @@ -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 diff --git a/pkg/api/login.go b/pkg/api/login.go index 1fc39b82f94..cea06ee2340 100644 --- a/pkg/api/login.go +++ b/pkg/api/login.go @@ -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) { diff --git a/pkg/api/org_invite.go b/pkg/api/org_invite.go index c5ae8e49139..c543150530c 100644 --- a/pkg/api/org_invite.go +++ b/pkg/api/org_invite.go @@ -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) } diff --git a/pkg/api/signup.go b/pkg/api/signup.go index 323dada1d49..aa8648ef1de 100644 --- a/pkg/api/signup.go +++ b/pkg/api/signup.go @@ -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) } diff --git a/pkg/api/user.go b/pkg/api/user.go index 4cde69bb04b..da36550cc73 100644 --- a/pkg/api/user.go +++ b/pkg/api/user.go @@ -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) diff --git a/pkg/api/user_token.go b/pkg/api/user_token.go index ee73259ca9f..0546a5c76c6 100644 --- a/pkg/api/user_token.go +++ b/pkg/api/user_token.go @@ -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) diff --git a/pkg/api/user_token_test.go b/pkg/api/user_token_test.go index d44f362d870..b560f9f5ee1 100644 --- a/pkg/api/user_token_test.go +++ b/pkg/api/user_token_test.go @@ -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) diff --git a/pkg/login/auth.go b/pkg/login/auth.go index d75b1cd8ad2..53d247a7e2e 100644 --- a/pkg/login/auth.go +++ b/pkg/login/auth.go @@ -9,6 +9,7 @@ import ( "github.com/grafana/grafana/pkg/services/ldap" "github.com/grafana/grafana/pkg/services/login" "github.com/grafana/grafana/pkg/services/sqlstore" + "github.com/grafana/grafana/pkg/services/user" ) var ( @@ -54,7 +55,7 @@ func (a *AuthenticatorService) AuthenticateUser(ctx context.Context, query *mode } err := loginUsingGrafanaDB(ctx, query, a.store) - if err == nil || (!errors.Is(err, models.ErrUserNotFound) && !errors.Is(err, ErrInvalidCredentials) && + if err == nil || (!errors.Is(err, user.ErrUserNotFound) && !errors.Is(err, ErrInvalidCredentials) && !errors.Is(err, ErrUserDisabled)) { query.AuthModule = "grafana" return err diff --git a/pkg/login/auth_test.go b/pkg/login/auth_test.go index 2dad43286c4..fecb8dfd086 100644 --- a/pkg/login/auth_test.go +++ b/pkg/login/auth_test.go @@ -11,6 +11,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/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -88,14 +89,14 @@ func TestAuthenticateUser(t *testing.T) { authScenario(t, "When a non-existing grafana user authenticate and ldap disabled", func(sc *authScenarioContext) { mockLoginAttemptValidation(nil, sc) - mockLoginUsingGrafanaDB(models.ErrUserNotFound, sc) + mockLoginUsingGrafanaDB(user.ErrUserNotFound, sc) mockLoginUsingLDAP(false, nil, sc) mockSaveInvalidLoginAttempt(sc) a := AuthenticatorService{store: mockstore.NewSQLStoreMock(), loginService: &logintest.LoginServiceFake{}} err := a.AuthenticateUser(context.Background(), sc.loginUserQuery) - require.EqualError(t, err, models.ErrUserNotFound.Error()) + require.EqualError(t, err, user.ErrUserNotFound.Error()) assert.True(t, sc.loginAttemptValidationWasCalled) assert.True(t, sc.grafanaLoginWasCalled) assert.True(t, sc.ldapLoginWasCalled) @@ -105,7 +106,7 @@ func TestAuthenticateUser(t *testing.T) { authScenario(t, "When a non-existing grafana user authenticate and invalid ldap credentials", func(sc *authScenarioContext) { mockLoginAttemptValidation(nil, sc) - mockLoginUsingGrafanaDB(models.ErrUserNotFound, sc) + mockLoginUsingGrafanaDB(user.ErrUserNotFound, sc) mockLoginUsingLDAP(true, ldap.ErrInvalidCredentials, sc) mockSaveInvalidLoginAttempt(sc) @@ -122,7 +123,7 @@ func TestAuthenticateUser(t *testing.T) { authScenario(t, "When a non-existing grafana user authenticate and valid ldap credentials", func(sc *authScenarioContext) { mockLoginAttemptValidation(nil, sc) - mockLoginUsingGrafanaDB(models.ErrUserNotFound, sc) + mockLoginUsingGrafanaDB(user.ErrUserNotFound, sc) mockLoginUsingLDAP(true, nil, sc) mockSaveInvalidLoginAttempt(sc) @@ -140,7 +141,7 @@ func TestAuthenticateUser(t *testing.T) { authScenario(t, "When a non-existing grafana user authenticate and ldap returns unexpected error", func(sc *authScenarioContext) { customErr := errors.New("custom") mockLoginAttemptValidation(nil, sc) - mockLoginUsingGrafanaDB(models.ErrUserNotFound, sc) + mockLoginUsingGrafanaDB(user.ErrUserNotFound, sc) mockLoginUsingLDAP(true, customErr, sc) mockSaveInvalidLoginAttempt(sc) diff --git a/pkg/login/grafana_login_test.go b/pkg/login/grafana_login_test.go index 175e0b0f3af..ba1042d214a 100644 --- a/pkg/login/grafana_login_test.go +++ b/pkg/login/grafana_login_test.go @@ -15,7 +15,7 @@ func TestLoginUsingGrafanaDB(t *testing.T) { grafanaLoginScenario(t, "When login with non-existing user", func(sc *grafanaLoginScenarioContext) { sc.withNonExistingUser() err := loginUsingGrafanaDB(context.Background(), sc.loginUserQuery, sc.store) - require.EqualError(t, err, models.ErrUserNotFound.Error()) + require.EqualError(t, err, user.ErrUserNotFound.Error()) assert.False(t, sc.validatePasswordCalled) assert.Nil(t, sc.loginUserQuery.User) @@ -97,10 +97,10 @@ func mockPasswordValidation(valid bool, sc *grafanaLoginScenarioContext) { } } -func (sc *grafanaLoginScenarioContext) getUserByLoginQueryReturns(user *user.User) { - sc.store.ExpectedUser = user - if user == nil { - sc.store.ExpectedError = models.ErrUserNotFound +func (sc *grafanaLoginScenarioContext) getUserByLoginQueryReturns(usr *user.User) { + sc.store.ExpectedUser = usr + if usr == nil { + sc.store.ExpectedError = user.ErrUserNotFound } } diff --git a/pkg/middleware/middleware_basic_auth_test.go b/pkg/middleware/middleware_basic_auth_test.go index 442df59b11b..bcefdcdfa88 100644 --- a/pkg/middleware/middleware_basic_auth_test.go +++ b/pkg/middleware/middleware_basic_auth_test.go @@ -73,7 +73,7 @@ func TestMiddlewareBasicAuth(t *testing.T) { }, configure) middlewareScenario(t, "Should return error if user is not found", func(t *testing.T, sc *scenarioContext) { - sc.mockSQLStore.ExpectedError = models.ErrUserNotFound + sc.mockSQLStore.ExpectedError = user.ErrUserNotFound sc.fakeReq("GET", "/") sc.req.SetBasicAuth("user", "password") sc.exec() @@ -86,7 +86,7 @@ func TestMiddlewareBasicAuth(t *testing.T) { }, configure) middlewareScenario(t, "Should return error if user & password do not match", func(t *testing.T, sc *scenarioContext) { - sc.mockSQLStore.ExpectedError = models.ErrUserNotFound + sc.mockSQLStore.ExpectedError = user.ErrUserNotFound sc.fakeReq("GET", "/") sc.req.SetBasicAuth("killa", "gorilla") sc.exec() diff --git a/pkg/middleware/middleware_jwt_auth_test.go b/pkg/middleware/middleware_jwt_auth_test.go index 31403464d69..c94f4698f78 100644 --- a/pkg/middleware/middleware_jwt_auth_test.go +++ b/pkg/middleware/middleware_jwt_auth_test.go @@ -9,6 +9,7 @@ import ( "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/contexthandler" + "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/setting" ) @@ -88,7 +89,7 @@ func TestMiddlewareJWTAuth(t *testing.T) { "foo-email": myEmail, }, nil } - sc.mockSQLStore.ExpectedError = models.ErrUserNotFound + sc.mockSQLStore.ExpectedError = user.ErrUserNotFound sc.fakeReq("GET", "/").withJWTAuthHeader(token).exec() assert.Equal(t, verifiedToken, token) diff --git a/pkg/models/user.go b/pkg/models/user.go index 14d075e65ff..7b6905bd9d8 100644 --- a/pkg/models/user.go +++ b/pkg/models/user.go @@ -1,21 +1,11 @@ package models import ( - "errors" "time" "github.com/grafana/grafana/pkg/services/user" ) -// Typed errors -var ( - ErrCaseInsensitive = errors.New("case insensitive conflict") - ErrUserNotFound = errors.New("user not found") - ErrUserAlreadyExists = errors.New("user already exists") - ErrLastGrafanaAdmin = errors.New("cannot remove last grafana admin") - ErrProtectedUser = errors.New("cannot adopt protected user") -) - type Password string func (p Password) IsWeak() bool { diff --git a/pkg/services/accesscontrol/database/resource_permissions.go b/pkg/services/accesscontrol/database/resource_permissions.go index 0a39bb2663a..38b6a8e474a 100644 --- a/pkg/services/accesscontrol/database/resource_permissions.go +++ b/pkg/services/accesscontrol/database/resource_permissions.go @@ -11,6 +11,7 @@ import ( "github.com/grafana/grafana/pkg/services/accesscontrol/resourcepermissions/types" "github.com/grafana/grafana/pkg/services/serviceaccounts" "github.com/grafana/grafana/pkg/services/sqlstore" + "github.com/grafana/grafana/pkg/services/user" ) type flatResourcePermission struct { @@ -39,18 +40,18 @@ func (p *flatResourcePermission) IsInherited(scope string) bool { } func (s *AccessControlStore) SetUserResourcePermission( - ctx context.Context, orgID int64, user accesscontrol.User, + ctx context.Context, orgID int64, usr accesscontrol.User, cmd types.SetResourcePermissionCommand, hook types.UserResourceHookFunc, ) (*accesscontrol.ResourcePermission, error) { - if user.ID == 0 { - return nil, models.ErrUserNotFound + if usr.ID == 0 { + return nil, user.ErrUserNotFound } var err error var permission *accesscontrol.ResourcePermission err = s.sql.WithTransactionalDbSession(ctx, func(sess *sqlstore.DBSession) error { - permission, err = s.setUserResourcePermission(sess, orgID, user, cmd, hook) + permission, err = s.setUserResourcePermission(sess, orgID, usr, cmd, hook) return err }) diff --git a/pkg/services/contexthandler/auth_jwt.go b/pkg/services/contexthandler/auth_jwt.go index 1b1856426ba..f1b847b3814 100644 --- a/pkg/services/contexthandler/auth_jwt.go +++ b/pkg/services/contexthandler/auth_jwt.go @@ -6,6 +6,7 @@ import ( "github.com/grafana/grafana/pkg/login" "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/user" ) const InvalidJWT = "Invalid JWT" @@ -79,7 +80,7 @@ func (h *ContextHandler) initContextWithJWT(ctx *models.ReqContext, orgId int64) } if err := h.SQLStore.GetSignedInUserWithCacheCtx(ctx.Req.Context(), &query); err != nil { - if errors.Is(err, models.ErrUserNotFound) { + if errors.Is(err, user.ErrUserNotFound) { ctx.Logger.Debug( "Failed to find user using JWT claims", "email_claim", query.Email, diff --git a/pkg/services/contexthandler/auth_proxy_test.go b/pkg/services/contexthandler/auth_proxy_test.go index d7fddc239c1..2e02aa81812 100644 --- a/pkg/services/contexthandler/auth_proxy_test.go +++ b/pkg/services/contexthandler/auth_proxy_test.go @@ -95,7 +95,7 @@ type FakeGetSignUserStore struct { func (f *FakeGetSignUserStore) GetSignedInUser(ctx context.Context, query *models.GetSignedInUserQuery) error { if query.UserId != userID { - return models.ErrUserNotFound + return user.ErrUserNotFound } query.Result = &models.SignedInUser{ diff --git a/pkg/services/contexthandler/contexthandler.go b/pkg/services/contexthandler/contexthandler.go index 94759f169b2..c8bd9470da2 100644 --- a/pkg/services/contexthandler/contexthandler.go +++ b/pkg/services/contexthandler/contexthandler.go @@ -24,6 +24,7 @@ import ( "github.com/grafana/grafana/pkg/services/login" "github.com/grafana/grafana/pkg/services/rendering" "github.com/grafana/grafana/pkg/services/sqlstore" + "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" @@ -346,7 +347,7 @@ func (h *ContextHandler) initContextWithBasicAuth(reqContext *models.ReqContext, "err", err, ) - if errors.Is(err, models.ErrUserNotFound) { + if errors.Is(err, user.ErrUserNotFound) { err = login.ErrInvalidCredentials } reqContext.JsonApiErr(401, InvalidUsernamePassword, err) diff --git a/pkg/services/login/authinfoservice/database/database.go b/pkg/services/login/authinfoservice/database/database.go index e4bc09e7cbd..a2049c1a1ac 100644 --- a/pkg/services/login/authinfoservice/database/database.go +++ b/pkg/services/login/authinfoservice/database/database.go @@ -55,7 +55,7 @@ func (s *AuthInfoStore) GetExternalUserInfoByLogin(ctx context.Context, query *m func (s *AuthInfoStore) GetAuthInfo(ctx context.Context, query *models.GetAuthInfoQuery) error { if query.UserId == 0 && query.AuthId == "" { - return models.ErrUserNotFound + return user.ErrUserNotFound } userAuth := &models.UserAuth{ @@ -76,7 +76,7 @@ func (s *AuthInfoStore) GetAuthInfo(ctx context.Context, query *models.GetAuthIn } if !has { - return models.ErrUserNotFound + return user.ErrUserNotFound } secretAccessToken, err := s.decodeAndDecrypt(userAuth.OAuthAccessToken) diff --git a/pkg/services/login/authinfoservice/service.go b/pkg/services/login/authinfoservice/service.go index ddcc1385352..23926920219 100644 --- a/pkg/services/login/authinfoservice/service.go +++ b/pkg/services/login/authinfoservice/service.go @@ -38,7 +38,7 @@ func (s *Implementation) LookupAndFix(ctx context.Context, query *models.GetUser authQuery.AuthId = query.AuthId err := s.authInfoStore.GetAuthInfo(ctx, authQuery) - if !errors.Is(err, models.ErrUserNotFound) { + if !errors.Is(err, user.ErrUserNotFound) { if err != nil { return false, nil, nil, err } @@ -53,11 +53,11 @@ func (s *Implementation) LookupAndFix(ctx context.Context, query *models.GetUser s.logger.Error("Error removing user_auth entry", "error", err) } - return false, nil, nil, models.ErrUserNotFound + return false, nil, nil, user.ErrUserNotFound } else { - user, err := s.authInfoStore.GetUserById(ctx, authQuery.Result.UserId) + usr, err := s.authInfoStore.GetUserById(ctx, authQuery.Result.UserId) if err != nil { - if errors.Is(err, models.ErrUserNotFound) { + if errors.Is(err, user.ErrUserNotFound) { // if the user has been deleted then remove the entry if errDel := s.authInfoStore.DeleteAuthInfo(ctx, &models.DeleteAuthInfoCommand{ UserAuth: authQuery.Result, @@ -65,53 +65,53 @@ func (s *Implementation) LookupAndFix(ctx context.Context, query *models.GetUser s.logger.Error("Error removing user_auth entry", "error", errDel) } - return false, nil, nil, models.ErrUserNotFound + return false, nil, nil, user.ErrUserNotFound } return false, nil, nil, err } - return true, user, authQuery.Result, nil + return true, usr, authQuery.Result, nil } } } - return false, nil, nil, models.ErrUserNotFound + return false, nil, nil, user.ErrUserNotFound } func (s *Implementation) LookupByOneOf(ctx context.Context, params *models.UserLookupParams) (*user.User, error) { - var user *user.User + var usr *user.User var err error // If not found, try to find the user by id if params.UserID != nil && *params.UserID != 0 { - user, err = s.authInfoStore.GetUserById(ctx, *params.UserID) - if err != nil && !errors.Is(err, models.ErrUserNotFound) { + usr, err = s.authInfoStore.GetUserById(ctx, *params.UserID) + if err != nil && !errors.Is(err, user.ErrUserNotFound) { return nil, err } } // If not found, try to find the user by email address - if user == nil && params.Email != nil && *params.Email != "" { - user, err = s.authInfoStore.GetUserByEmail(ctx, *params.Email) - if err != nil && !errors.Is(err, models.ErrUserNotFound) { + if usr == nil && params.Email != nil && *params.Email != "" { + usr, err = s.authInfoStore.GetUserByEmail(ctx, *params.Email) + if err != nil && !errors.Is(err, user.ErrUserNotFound) { return nil, err } } // If not found, try to find the user by login - if user == nil && params.Login != nil && *params.Login != "" { - user, err = s.authInfoStore.GetUserByLogin(ctx, *params.Login) - if err != nil && !errors.Is(err, models.ErrUserNotFound) { + if usr == nil && params.Login != nil && *params.Login != "" { + usr, err = s.authInfoStore.GetUserByLogin(ctx, *params.Login) + if err != nil && !errors.Is(err, user.ErrUserNotFound) { return nil, err } } - if user == nil { - return nil, models.ErrUserNotFound + if usr == nil { + return nil, user.ErrUserNotFound } - return user, nil + return usr, nil } func (s *Implementation) GenericOAuthLookup(ctx context.Context, authModule string, authId string, userID int64) (*models.UserAuth, error) { @@ -133,26 +133,26 @@ func (s *Implementation) GenericOAuthLookup(ctx context.Context, authModule stri func (s *Implementation) LookupAndUpdate(ctx context.Context, query *models.GetUserByAuthInfoQuery) (*user.User, error) { // 1. LookupAndFix = auth info, user, error // TODO: Not a big fan of the fact that we are deleting auth info here, might want to move that - foundUser, user, authInfo, err := s.LookupAndFix(ctx, query) - if err != nil && !errors.Is(err, models.ErrUserNotFound) { + foundUser, usr, authInfo, err := s.LookupAndFix(ctx, query) + if err != nil && !errors.Is(err, user.ErrUserNotFound) { return nil, err } // 2. FindByUserDetails if !foundUser { - user, err = s.LookupByOneOf(ctx, &query.UserLookupParams) + usr, err = s.LookupByOneOf(ctx, &query.UserLookupParams) if err != nil { return nil, err } } - if err := s.UserProtectionService.AllowUserMapping(user, query.AuthModule); err != nil { + if err := s.UserProtectionService.AllowUserMapping(usr, query.AuthModule); err != nil { return nil, err } // Special case for generic oauth duplicates - ai, err := s.GenericOAuthLookup(ctx, query.AuthModule, query.AuthId, user.ID) - if !errors.Is(err, models.ErrUserNotFound) { + ai, err := s.GenericOAuthLookup(ctx, query.AuthModule, query.AuthId, usr.ID) + if !errors.Is(err, user.ErrUserNotFound) { if err != nil { return nil, err } @@ -164,7 +164,7 @@ func (s *Implementation) LookupAndUpdate(ctx context.Context, query *models.GetU if query.AuthModule != "" { if authInfo == nil { cmd := &models.SetAuthInfoCommand{ - UserId: user.ID, + UserId: usr.ID, AuthModule: query.AuthModule, AuthId: query.AuthId, } @@ -178,7 +178,7 @@ func (s *Implementation) LookupAndUpdate(ctx context.Context, query *models.GetU } } - return user, nil + return usr, nil } func (s *Implementation) GetAuthInfo(ctx context.Context, query *models.GetAuthInfoQuery) error { diff --git a/pkg/services/login/authinfoservice/user_auth_test.go b/pkg/services/login/authinfoservice/user_auth_test.go index c67ea0f5b4d..a00e36522b7 100644 --- a/pkg/services/login/authinfoservice/user_auth_test.go +++ b/pkg/services/login/authinfoservice/user_auth_test.go @@ -44,95 +44,95 @@ func TestUserAuth(t *testing.T) { login := "loginuser0" query := &models.GetUserByAuthInfoQuery{UserLookupParams: models.UserLookupParams{Login: &login}} - user, err := srv.LookupAndUpdate(context.Background(), query) + usr, err := srv.LookupAndUpdate(context.Background(), query) require.Nil(t, err) - require.Equal(t, user.Login, login) + require.Equal(t, usr.Login, login) // By ID - id := user.ID + id := usr.ID - user, err = srv.LookupByOneOf(context.Background(), &models.UserLookupParams{ + usr, err = srv.LookupByOneOf(context.Background(), &models.UserLookupParams{ UserID: &id, }) require.Nil(t, err) - require.Equal(t, user.ID, id) + require.Equal(t, usr.ID, id) // By Email email := "user1@test.com" - user, err = srv.LookupByOneOf(context.Background(), &models.UserLookupParams{ + usr, err = srv.LookupByOneOf(context.Background(), &models.UserLookupParams{ Email: &email, }) require.Nil(t, err) - require.Equal(t, user.Email, email) + require.Equal(t, usr.Email, email) // Don't find nonexistent user email = "nonexistent@test.com" - user, err = srv.LookupByOneOf(context.Background(), &models.UserLookupParams{ + usr, err = srv.LookupByOneOf(context.Background(), &models.UserLookupParams{ Email: &email, }) - require.Equal(t, models.ErrUserNotFound, err) - require.Nil(t, user) + require.Equal(t, user.ErrUserNotFound, err) + require.Nil(t, usr) }) t.Run("Can set & locate by AuthModule and AuthId", func(t *testing.T) { // get nonexistent user_auth entry query := &models.GetUserByAuthInfoQuery{AuthModule: "test", AuthId: "test"} - user, err := srv.LookupAndUpdate(context.Background(), query) + usr, err := srv.LookupAndUpdate(context.Background(), query) - require.Equal(t, models.ErrUserNotFound, err) - require.Nil(t, user) + require.Equal(t, user.ErrUserNotFound, err) + require.Nil(t, usr) // create user_auth entry login := "loginuser0" query.UserLookupParams.Login = &login - user, err = srv.LookupAndUpdate(context.Background(), query) + usr, err = srv.LookupAndUpdate(context.Background(), query) require.Nil(t, err) - require.Equal(t, user.Login, login) + require.Equal(t, usr.Login, login) // get via user_auth query = &models.GetUserByAuthInfoQuery{AuthModule: "test", AuthId: "test"} - user, err = srv.LookupAndUpdate(context.Background(), query) + usr, err = srv.LookupAndUpdate(context.Background(), query) require.Nil(t, err) - require.Equal(t, user.Login, login) + require.Equal(t, usr.Login, login) // get with non-matching id - idPlusOne := user.ID + 1 + idPlusOne := usr.ID + 1 query.UserLookupParams.UserID = &idPlusOne - user, err = srv.LookupAndUpdate(context.Background(), query) + usr, err = srv.LookupAndUpdate(context.Background(), query) require.Nil(t, err) - require.Equal(t, user.Login, "loginuser1") + require.Equal(t, usr.Login, "loginuser1") // get via user_auth query = &models.GetUserByAuthInfoQuery{AuthModule: "test", AuthId: "test"} - user, err = srv.LookupAndUpdate(context.Background(), query) + usr, err = srv.LookupAndUpdate(context.Background(), query) require.Nil(t, err) - require.Equal(t, user.Login, "loginuser1") + require.Equal(t, usr.Login, "loginuser1") // remove user err = sqlStore.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error { - _, err := sess.Exec("DELETE FROM "+sqlStore.Dialect.Quote("user")+" WHERE id=?", user.ID) + _, err := sess.Exec("DELETE FROM "+sqlStore.Dialect.Quote("user")+" WHERE id=?", usr.ID) return err }) require.NoError(t, err) // get via user_auth for deleted user query = &models.GetUserByAuthInfoQuery{AuthModule: "test", AuthId: "test"} - user, err = srv.LookupAndUpdate(context.Background(), query) + usr, err = srv.LookupAndUpdate(context.Background(), query) - require.Equal(t, err, models.ErrUserNotFound) - require.Nil(t, user) + require.Equal(t, err, user.ErrUserNotFound) + require.Nil(t, usr) }) t.Run("Can set & retrieve oauth token information", func(t *testing.T) { diff --git a/pkg/services/login/loginservice/loginservice.go b/pkg/services/login/loginservice/loginservice.go index e86fbad9a4e..76bf618964b 100644 --- a/pkg/services/login/loginservice/loginservice.go +++ b/pkg/services/login/loginservice/loginservice.go @@ -54,7 +54,7 @@ func (ls *Implementation) UpsertUser(ctx context.Context, cmd *models.UpsertUser UserLookupParams: cmd.UserLookupParams, }) if err != nil { - if !errors.Is(err, models.ErrUserNotFound) { + if !errors.Is(err, user.ErrUserNotFound) { return err } if !cmd.SignupAllowed { diff --git a/pkg/services/login/loginservice/loginservice_mock.go b/pkg/services/login/loginservice/loginservice_mock.go index 077f2738733..99348babc41 100644 --- a/pkg/services/login/loginservice/loginservice_mock.go +++ b/pkg/services/login/loginservice/loginservice_mock.go @@ -27,7 +27,7 @@ func (s LoginServiceMock) CreateUser(cmd user.CreateUserCommand) (*user.User, er } if cmd.Login == s.AlreadyExitingLogin { - return nil, models.ErrUserAlreadyExists + return nil, user.ErrUserAlreadyExists } if s.ExpectedUserForm.Login == cmd.Login && s.ExpectedUserForm.Email == cmd.Email && diff --git a/pkg/services/oauthtoken/oauth_token.go b/pkg/services/oauthtoken/oauth_token.go index 6447b8e019e..07772864626 100644 --- a/pkg/services/oauthtoken/oauth_token.go +++ b/pkg/services/oauthtoken/oauth_token.go @@ -11,6 +11,7 @@ import ( "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/datasources" "github.com/grafana/grafana/pkg/services/login" + "github.com/grafana/grafana/pkg/services/user" ) var ( @@ -35,19 +36,19 @@ func ProvideService(socialService social.Service, authInfoService login.AuthInfo } // GetCurrentOAuthToken returns the OAuth token, if any, for the authenticated user. Will try to refresh the token if it has expired. -func (o *Service) GetCurrentOAuthToken(ctx context.Context, user *models.SignedInUser) *oauth2.Token { - if user == nil { +func (o *Service) GetCurrentOAuthToken(ctx context.Context, usr *models.SignedInUser) *oauth2.Token { + if usr == nil { // No user, therefore no token return nil } - authInfoQuery := &models.GetAuthInfoQuery{UserId: user.UserId} + authInfoQuery := &models.GetAuthInfoQuery{UserId: usr.UserId} if err := o.AuthInfoService.GetAuthInfo(ctx, authInfoQuery); err != nil { - if errors.Is(err, models.ErrUserNotFound) { + if errors.Is(err, user.ErrUserNotFound) { // Not necessarily an error. User may be logged in another way. - logger.Debug("no OAuth token for user found", "userId", user.UserId, "username", user.Login) + logger.Debug("no OAuth token for user found", "userId", usr.UserId, "username", usr.Login) } else { - logger.Error("failed to get OAuth token for user", "userId", user.UserId, "username", user.Login, "error", err) + logger.Error("failed to get OAuth token for user", "userId", usr.UserId, "username", usr.Login, "error", err) } return nil } @@ -80,7 +81,7 @@ func (o *Service) GetCurrentOAuthToken(ctx context.Context, user *models.SignedI // TokenSource handles refreshing the token if it has expired token, err := connect.TokenSource(ctx, persistedToken).Token() if err != nil { - logger.Error("failed to retrieve OAuth access token", "provider", authInfoQuery.Result.AuthModule, "userId", user.UserId, "username", user.Login, "error", err) + logger.Error("failed to retrieve OAuth access token", "provider", authInfoQuery.Result.AuthModule, "userId", usr.UserId, "username", usr.Login, "error", err) return nil } @@ -93,10 +94,10 @@ func (o *Service) GetCurrentOAuthToken(ctx context.Context, user *models.SignedI OAuthToken: token, } if err := o.AuthInfoService.UpdateAuthInfo(ctx, updateAuthCommand); err != nil { - logger.Error("failed to update auth info during token refresh", "userId", user.UserId, "username", user.Login, "error", err) + logger.Error("failed to update auth info during token refresh", "userId", usr.UserId, "username", usr.Login, "error", err) return nil } - logger.Debug("updated OAuth info for user", "userId", user.UserId, "username", user.Login) + logger.Debug("updated OAuth info for user", "userId", usr.UserId, "username", usr.Login) } return token } diff --git a/pkg/services/serviceaccounts/database/database.go b/pkg/services/serviceaccounts/database/database.go index 279401d6d7d..9c7e732b921 100644 --- a/pkg/services/serviceaccounts/database/database.go +++ b/pkg/services/serviceaccounts/database/database.go @@ -72,7 +72,7 @@ func (s *ServiceAccountsStoreImpl) CreateServiceAccount(ctx context.Context, org }) if createErr != nil { - if errors.Is(createErr, models.ErrUserAlreadyExists) { + if errors.Is(createErr, user.ErrUserAlreadyExists) { return nil, ErrServiceAccountAlreadyExists } diff --git a/pkg/services/sqlstore/org_test.go b/pkg/services/sqlstore/org_test.go index 6070c65b602..250c275da30 100644 --- a/pkg/services/sqlstore/org_test.go +++ b/pkg/services/sqlstore/org_test.go @@ -331,7 +331,7 @@ func TestIntegrationAccountDataAccess(t *testing.T) { require.True(t, remCmd.UserWasDeleted) err = sqlStore.GetSignedInUser(context.Background(), &models.GetSignedInUserQuery{UserId: ac2.ID}) - require.Equal(t, err, models.ErrUserNotFound) + require.Equal(t, err, user.ErrUserNotFound) }) t.Run("Cannot delete last admin org user", func(t *testing.T) { diff --git a/pkg/services/sqlstore/org_users.go b/pkg/services/sqlstore/org_users.go index f1fa782b18b..0a46a90b96f 100644 --- a/pkg/services/sqlstore/org_users.go +++ b/pkg/services/sqlstore/org_users.go @@ -15,19 +15,19 @@ import ( func (ss *SQLStore) AddOrgUser(ctx context.Context, cmd *models.AddOrgUserCommand) error { return ss.WithTransactionalDbSession(ctx, func(sess *DBSession) error { // check if user exists - var user user.User + var usr user.User session := sess.ID(cmd.UserId) if !cmd.AllowAddingServiceAccount { session = session.Where(notServiceAccountFilter(ss)) } - if exists, err := session.Get(&user); err != nil { + if exists, err := session.Get(&usr); err != nil { return err } else if !exists { - return models.ErrUserNotFound + return user.ErrUserNotFound } - if res, err := sess.Query("SELECT 1 from org_user WHERE org_id=? and user_id=?", cmd.OrgId, user.ID); err != nil { + if res, err := sess.Query("SELECT 1 from org_user WHERE org_id=? and user_id=?", cmd.OrgId, usr.ID); err != nil { return err } else if len(res) == 1 { return models.ErrOrgUserAlreadyAdded @@ -55,7 +55,7 @@ func (ss *SQLStore) AddOrgUser(ctx context.Context, cmd *models.AddOrgUserComman var userOrgs []*models.UserOrgDTO sess.Table("org_user") sess.Join("INNER", "org", "org_user.org_id=org.id") - sess.Where("org_user.user_id=? AND org_user.org_id=?", user.ID, user.OrgID) + sess.Where("org_user.user_id=? AND org_user.org_id=?", usr.ID, usr.OrgID) sess.Cols("org.name", "org_user.role", "org_user.org_id") err = sess.Find(&userOrgs) @@ -64,7 +64,7 @@ func (ss *SQLStore) AddOrgUser(ctx context.Context, cmd *models.AddOrgUserComman } if len(userOrgs) == 0 { - return setUsingOrgInTransaction(sess, user.ID, cmd.OrgId) + return setUsingOrgInTransaction(sess, usr.ID, cmd.OrgId) } return nil @@ -249,11 +249,11 @@ func (ss *SQLStore) SearchOrgUsers(ctx context.Context, query *models.SearchOrgU func (ss *SQLStore) RemoveOrgUser(ctx context.Context, cmd *models.RemoveOrgUserCommand) error { return ss.WithTransactionalDbSession(ctx, func(sess *DBSession) error { // check if user exists - var user user.User - if exists, err := sess.ID(cmd.UserId).Where(notServiceAccountFilter(ss)).Get(&user); err != nil { + var usr user.User + if exists, err := sess.ID(cmd.UserId).Where(notServiceAccountFilter(ss)).Get(&usr); err != nil { return err } else if !exists { - return models.ErrUserNotFound + return user.ErrUserNotFound } deletes := []string{ @@ -279,7 +279,7 @@ func (ss *SQLStore) RemoveOrgUser(ctx context.Context, cmd *models.RemoveOrgUser var userOrgs []*models.UserOrgDTO sess.Table("org_user") sess.Join("INNER", "org", "org_user.org_id=org.id") - sess.Where("org_user.user_id=?", user.ID) + sess.Where("org_user.user_id=?", usr.ID) sess.Cols("org.name", "org_user.role", "org_user.org_id") err := sess.Find(&userOrgs) @@ -290,28 +290,28 @@ func (ss *SQLStore) RemoveOrgUser(ctx context.Context, cmd *models.RemoveOrgUser if len(userOrgs) > 0 { hasCurrentOrgSet := false for _, userOrg := range userOrgs { - if user.OrgID == userOrg.OrgId { + if usr.OrgID == userOrg.OrgId { hasCurrentOrgSet = true break } } if !hasCurrentOrgSet { - err = setUsingOrgInTransaction(sess, user.ID, userOrgs[0].OrgId) + err = setUsingOrgInTransaction(sess, usr.ID, userOrgs[0].OrgId) if err != nil { return err } } } else if cmd.ShouldDeleteOrphanedUser { // no other orgs, delete the full user - if err := deleteUserInTransaction(ss, sess, &models.DeleteUserCommand{UserId: user.ID}); err != nil { + if err := deleteUserInTransaction(ss, sess, &models.DeleteUserCommand{UserId: usr.ID}); err != nil { return err } cmd.UserWasDeleted = true } else { // no orgs, but keep the user -> clean up orgId - err = removeUserOrg(sess, user.ID) + err = removeUserOrg(sess, usr.ID) if err != nil { return err } diff --git a/pkg/services/sqlstore/org_users_test.go b/pkg/services/sqlstore/org_users_test.go index 2b9e4ad9d3b..1e253a8ef29 100644 --- a/pkg/services/sqlstore/org_users_test.go +++ b/pkg/services/sqlstore/org_users_test.go @@ -194,7 +194,7 @@ func TestSQLStore_AddOrgUser(t *testing.T) { if err != nil { return err } else if !has { - return models.ErrUserNotFound + return user.ErrUserNotFound } return nil }) diff --git a/pkg/services/sqlstore/user.go b/pkg/services/sqlstore/user.go index 9b1cbddc47a..7b0e9ba8939 100644 --- a/pkg/services/sqlstore/user.go +++ b/pkg/services/sqlstore/user.go @@ -20,7 +20,7 @@ type ErrCaseInsensitiveLoginConflict struct { } func (e *ErrCaseInsensitiveLoginConflict) Unwrap() error { - return models.ErrCaseInsensitive + return user.ErrCaseInsensitive } func (e *ErrCaseInsensitiveLoginConflict) Error() string { @@ -100,7 +100,7 @@ func (ss *SQLStore) createUser(ctx context.Context, sess *DBSession, args user.C return usr, err } if exists { - return usr, models.ErrUserAlreadyExists + return usr, user.ErrUserAlreadyExists } // create user @@ -196,25 +196,25 @@ func notServiceAccountFilter(ss *SQLStore) string { func (ss *SQLStore) GetUserById(ctx context.Context, query *models.GetUserByIdQuery) error { return ss.WithDbSession(ctx, func(sess *DBSession) error { - user := new(user.User) + usr := new(user.User) has, err := sess.ID(query.Id). Where(notServiceAccountFilter(ss)). - Get(user) + Get(usr) if err != nil { return err } else if !has { - return models.ErrUserNotFound + return user.ErrUserNotFound } if ss.Cfg.CaseInsensitiveLogin { - if err := ss.userCaseInsensitiveLoginConflict(ctx, sess, user.Login, user.Email); err != nil { + if err := ss.userCaseInsensitiveLoginConflict(ctx, sess, usr.Login, usr.Email); err != nil { return err } } - query.Result = user + query.Result = usr return nil }) @@ -223,7 +223,7 @@ func (ss *SQLStore) GetUserById(ctx context.Context, query *models.GetUserByIdQu func (ss *SQLStore) GetUserByLogin(ctx context.Context, query *models.GetUserByLoginQuery) error { return ss.WithDbSession(ctx, func(sess *DBSession) error { if query.LoginOrEmail == "" { - return models.ErrUserNotFound + return user.ErrUserNotFound } // Try and find the user by login first. @@ -254,7 +254,7 @@ func (ss *SQLStore) GetUserByLogin(ctx context.Context, query *models.GetUserByL if err != nil { return err } else if !has { - return models.ErrUserNotFound + return user.ErrUserNotFound } if ss.Cfg.CaseInsensitiveLogin { @@ -272,30 +272,30 @@ func (ss *SQLStore) GetUserByLogin(ctx context.Context, query *models.GetUserByL func (ss *SQLStore) GetUserByEmail(ctx context.Context, query *models.GetUserByEmailQuery) error { return ss.WithDbSession(ctx, func(sess *DBSession) error { if query.Email == "" { - return models.ErrUserNotFound + return user.ErrUserNotFound } - user := &user.User{} + usr := &user.User{} where := "email=?" if ss.Cfg.CaseInsensitiveLogin { where = "LOWER(email)=LOWER(?)" } - has, err := sess.Where(notServiceAccountFilter(ss)).Where(where, query.Email).Get(user) + has, err := sess.Where(notServiceAccountFilter(ss)).Where(where, query.Email).Get(usr) if err != nil { return err } else if !has { - return models.ErrUserNotFound + return user.ErrUserNotFound } if ss.Cfg.CaseInsensitiveLogin { - if err := ss.userCaseInsensitiveLoginConflict(ctx, sess, user.Login, user.Email); err != nil { + if err := ss.userCaseInsensitiveLoginConflict(ctx, sess, usr.Login, usr.Email); err != nil { return err } } - query.Result = user + query.Result = usr return nil }) @@ -405,26 +405,26 @@ func removeUserOrg(sess *DBSession, userID int64) error { func (ss *SQLStore) GetUserProfile(ctx context.Context, query *models.GetUserProfileQuery) error { return ss.WithDbSession(ctx, func(sess *DBSession) error { - var user user.User - has, err := sess.ID(query.UserId).Where(notServiceAccountFilter(ss)).Get(&user) + var usr user.User + has, err := sess.ID(query.UserId).Where(notServiceAccountFilter(ss)).Get(&usr) if err != nil { return err } else if !has { - return models.ErrUserNotFound + return user.ErrUserNotFound } query.Result = models.UserProfileDTO{ - Id: user.ID, - Name: user.Name, - Email: user.Email, - Login: user.Login, - Theme: user.Theme, - IsGrafanaAdmin: user.IsAdmin, - IsDisabled: user.IsDisabled, - OrgId: user.OrgID, - UpdatedAt: user.Updated, - CreatedAt: user.Created, + Id: usr.ID, + Name: usr.Name, + Email: usr.Email, + Login: usr.Login, + Theme: usr.Theme, + IsGrafanaAdmin: usr.IsAdmin, + IsDisabled: usr.IsDisabled, + OrgId: usr.OrgID, + UpdatedAt: usr.Updated, + CreatedAt: usr.Created, } return err @@ -536,35 +536,35 @@ func (ss *SQLStore) GetSignedInUser(ctx context.Context, query *models.GetSigned } } - var user models.SignedInUser - has, err := sess.Get(&user) + var usr models.SignedInUser + has, err := sess.Get(&usr) if err != nil { return err } else if !has { - return models.ErrUserNotFound + return user.ErrUserNotFound } - if user.OrgRole == "" { - user.OrgId = -1 - user.OrgName = "Org missing" + if usr.OrgRole == "" { + usr.OrgId = -1 + usr.OrgName = "Org missing" } - if user.ExternalAuthModule != "oauth_grafana_com" { - user.ExternalAuthId = "" + if usr.ExternalAuthModule != "oauth_grafana_com" { + usr.ExternalAuthId = "" } // tempUser is used to retrieve the teams for the signed in user for internal use. tempUser := &models.SignedInUser{ - OrgId: user.OrgId, + OrgId: usr.OrgId, Permissions: map[int64]map[string][]string{ - user.OrgId: { + usr.OrgId: { ac.ActionTeamsRead: {ac.ScopeTeamsAll}, }, }, } getTeamsByUserQuery := &models.GetTeamsByUserQuery{ - OrgId: user.OrgId, - UserId: user.UserId, + OrgId: usr.OrgId, + UserId: usr.UserId, SignedInUser: tempUser, } err = ss.GetTeamsByUser(ctx, getTeamsByUserQuery) @@ -572,12 +572,12 @@ func (ss *SQLStore) GetSignedInUser(ctx context.Context, query *models.GetSigned return err } - user.Teams = make([]int64, len(getTeamsByUserQuery.Result)) + usr.Teams = make([]int64, len(getTeamsByUserQuery.Result)) for i, t := range getTeamsByUserQuery.Result { - user.Teams[i] = t.Id + usr.Teams[i] = t.Id } - query.Result = &user + query.Result = &usr return err }) } @@ -699,19 +699,19 @@ func (ss *SQLStore) SearchUsers(ctx context.Context, query *models.SearchUsersQu func (ss *SQLStore) DisableUser(ctx context.Context, cmd *models.DisableUserCommand) error { return ss.WithDbSession(ctx, func(dbSess *DBSession) error { - user := user.User{} + usr := user.User{} sess := dbSess.Table("user") - if has, err := sess.ID(cmd.UserId).Where(notServiceAccountFilter(ss)).Get(&user); err != nil { + if has, err := sess.ID(cmd.UserId).Where(notServiceAccountFilter(ss)).Get(&usr); err != nil { return err } else if !has { - return models.ErrUserNotFound + return user.ErrUserNotFound } - user.IsDisabled = cmd.IsDisabled + usr.IsDisabled = cmd.IsDisabled sess.UseBool("is_disabled") - _, err := sess.ID(cmd.UserId).Update(&user) + _, err := sess.ID(cmd.UserId).Update(&usr) return err }) } @@ -745,13 +745,13 @@ func (ss *SQLStore) DeleteUser(ctx context.Context, cmd *models.DeleteUserComman func deleteUserInTransaction(ss *SQLStore, sess *DBSession, cmd *models.DeleteUserCommand) error { // Check if user exists - user := user.User{ID: cmd.UserId} - has, err := sess.Where(notServiceAccountFilter(ss)).Get(&user) + usr := user.User{ID: cmd.UserId} + has, err := sess.Where(notServiceAccountFilter(ss)).Get(&usr) if err != nil { return err } if !has { - return models.ErrUserNotFound + return user.ErrUserNotFound } for _, sql := range UserDeletions() { _, err := sess.Exec(sql, cmd.UserId) @@ -864,7 +864,7 @@ func validateOneAdminLeft(sess *DBSession) error { } if count == 0 { - return models.ErrLastGrafanaAdmin + return user.ErrLastGrafanaAdmin } return nil diff --git a/pkg/services/sqlstore/user_test.go b/pkg/services/sqlstore/user_test.go index 47da354fadb..ab48f73c907 100644 --- a/pkg/services/sqlstore/user_test.go +++ b/pkg/services/sqlstore/user_test.go @@ -662,7 +662,7 @@ func TestIntegrationUserDataAccess(t *testing.T) { // Cannot make themselves a non-admin updatePermsError := ss.UpdateUserPermissions(usr.ID, false) - require.Equal(t, updatePermsError, models.ErrLastGrafanaAdmin) + require.Equal(t, updatePermsError, user.ErrLastGrafanaAdmin) query := models.GetUserByIdQuery{Id: usr.ID} getUserError := ss.GetUserById(context.Background(), &query) @@ -689,7 +689,7 @@ func TestIntegrationUserDataAccess(t *testing.T) { SkipOrgSetup: true, } _, err = ss.CreateUser(context.Background(), createUserCmd) - require.Equal(t, err, models.ErrUserAlreadyExists) + require.Equal(t, err, user.ErrUserAlreadyExists) // When trying to create a new user with the same login, an error is returned createUserCmd = user.CreateUserCommand{ @@ -699,7 +699,7 @@ func TestIntegrationUserDataAccess(t *testing.T) { SkipOrgSetup: true, } _, err = ss.CreateUser(context.Background(), createUserCmd) - require.Equal(t, err, models.ErrUserAlreadyExists) + require.Equal(t, err, user.ErrUserAlreadyExists) }) } diff --git a/pkg/services/user/model.go b/pkg/services/user/model.go index e7425d69b9d..d44c0fdc3e8 100644 --- a/pkg/services/user/model.go +++ b/pkg/services/user/model.go @@ -1,11 +1,21 @@ package user import ( + "errors" "time" ) type HelpFlags1 uint64 +// Typed errors +var ( + ErrCaseInsensitive = errors.New("case insensitive conflict") + ErrUserNotFound = errors.New("user not found") + ErrUserAlreadyExists = errors.New("user already exists") + ErrLastGrafanaAdmin = errors.New("cannot remove last grafana admin") + ErrProtectedUser = errors.New("cannot adopt protected user") +) + type User struct { ID int64 `xorm:"pk autoincr 'id'"` Version int diff --git a/pkg/services/user/userimpl/store.go b/pkg/services/user/userimpl/store.go index 9dabdf39cb6..3f19fa3b866 100644 --- a/pkg/services/user/userimpl/store.go +++ b/pkg/services/user/userimpl/store.go @@ -5,7 +5,6 @@ import ( "fmt" "github.com/grafana/grafana/pkg/events" - "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/services/sqlstore/db" "github.com/grafana/grafana/pkg/services/sqlstore/migrator" @@ -52,7 +51,7 @@ func (ss *sqlStore) Get(ctx context.Context, usr *user.User) (*user.User, error) err := ss.db.WithDbSession(ctx, func(sess *sqlstore.DBSession) error { exists, err := sess.Where("email=? OR login=?", usr.Email, usr.Login).Get(usr) if !exists { - return models.ErrUserNotFound + return user.ErrUserNotFound } if err != nil { return err @@ -78,18 +77,18 @@ func (ss *sqlStore) Delete(ctx context.Context, userID int64) error { } func (ss *sqlStore) GetNotServiceAccount(ctx context.Context, userID int64) (*user.User, error) { - user := user.User{ID: userID} + usr := user.User{ID: userID} err := ss.db.WithDbSession(ctx, func(sess *sqlstore.DBSession) error { - has, err := sess.Where(ss.notServiceAccountFilter()).Get(&user) + has, err := sess.Where(ss.notServiceAccountFilter()).Get(&usr) if err != nil { return err } if !has { - return models.ErrUserNotFound + return user.ErrUserNotFound } return nil }) - return &user, err + return &usr, err } func (ss *sqlStore) notServiceAccountFilter() string { diff --git a/pkg/services/user/userimpl/store_test.go b/pkg/services/user/userimpl/store_test.go index c158f6fd38b..8b1bbe7e7e7 100644 --- a/pkg/services/user/userimpl/store_test.go +++ b/pkg/services/user/userimpl/store_test.go @@ -5,7 +5,6 @@ import ( "testing" "time" - "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/services/user" @@ -28,7 +27,7 @@ func TestIntegrationUserDataAccess(t *testing.T) { Login: "test1", }, ) - require.Error(t, err, models.ErrUserNotFound) + require.Error(t, err, user.ErrUserNotFound) }) t.Run("insert user", func(t *testing.T) { diff --git a/pkg/services/user/userimpl/user.go b/pkg/services/user/userimpl/user.go index cac3f4e83e1..c5c1223e88e 100644 --- a/pkg/services/user/userimpl/user.go +++ b/pkg/services/user/userimpl/user.go @@ -6,7 +6,6 @@ import ( "fmt" "time" - "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/org" @@ -84,7 +83,7 @@ func (s *Service) Create(ctx context.Context, cmd *user.CreateUserCommand) (*use Email: cmd.Email, } usr, err = s.store.Get(ctx, usr) - if err != nil && !errors.Is(err, models.ErrUserNotFound) { + if err != nil && !errors.Is(err, user.ErrUserNotFound) { return usr, err } diff --git a/pkg/services/user/userimpl/user_test.go b/pkg/services/user/userimpl/user_test.go index 7e982e5bd38..5cb6c164290 100644 --- a/pkg/services/user/userimpl/user_test.go +++ b/pkg/services/user/userimpl/user_test.go @@ -5,7 +5,6 @@ import ( "errors" "testing" - "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/accesscontrol/mock" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/org/orgtest" @@ -46,12 +45,12 @@ func TestUserService(t *testing.T) { }) t.Run("delete user store returns error", func(t *testing.T) { - userStore.ExpectedDeleteUserError = models.ErrUserNotFound + userStore.ExpectedDeleteUserError = user.ErrUserNotFound t.Cleanup(func() { userStore.ExpectedDeleteUserError = nil }) err := userService.Delete(context.Background(), &user.DeleteUserCommand{UserID: 1}) - require.Error(t, err, models.ErrUserNotFound) + require.Error(t, err, user.ErrUserNotFound) }) t.Run("delete user returns from team", func(t *testing.T) {