Login: Remove CreateUser from LoginService (#59464)

* LoginService: remove create user and use user.Service instead

* Fix tests
This commit is contained in:
Karl Persson 2022-11-29 10:20:44 +01:00 committed by GitHub
parent 454c8841a4
commit a4a5307722
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 22 additions and 65 deletions

View File

@ -65,7 +65,7 @@ func (hs *HTTPServer) AdminCreateUser(c *models.ReqContext) response.Response {
return response.Error(400, "Password is missing or too short", nil)
}
usr, err := hs.Login.CreateUser(cmd)
usr, err := hs.userService.Create(c.Req.Context(), &cmd)
if err != nil {
if errors.Is(err, models.ErrOrgNotFound) {
return response.Error(400, err.Error(), nil)

View File

@ -14,7 +14,6 @@ import (
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/auth"
"github.com/grafana/grafana/pkg/services/auth/authtest"
"github.com/grafana/grafana/pkg/services/login/loginservice"
"github.com/grafana/grafana/pkg/services/login/logintest"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/sqlstore"
@ -168,8 +167,8 @@ func TestAdminAPIEndpoint(t *testing.T) {
Login: testLogin,
Password: testPassword,
}
adminCreateUserScenario(t, "Should create the user", "/api/admin/users", "/api/admin/users", createCmd, func(sc *scenarioContext) {
usrSvc := &usertest.FakeUserService{ExpectedUser: &user.User{ID: testUserID}}
adminCreateUserScenario(t, "Should create the user", "/api/admin/users", "/api/admin/users", createCmd, usrSvc, func(sc *scenarioContext) {
sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec()
assert.Equal(t, 200, sc.resp.Code)
@ -186,8 +185,8 @@ func TestAdminAPIEndpoint(t *testing.T) {
Password: testPassword,
OrgId: testOrgID,
}
adminCreateUserScenario(t, "Should create the user", "/api/admin/users", "/api/admin/users", createCmd, func(sc *scenarioContext) {
usrSvc := &usertest.FakeUserService{ExpectedUser: &user.User{ID: testUserID}}
adminCreateUserScenario(t, "Should create the user", "/api/admin/users", "/api/admin/users", createCmd, usrSvc, func(sc *scenarioContext) {
sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec()
assert.Equal(t, 200, sc.resp.Code)
@ -204,8 +203,8 @@ func TestAdminAPIEndpoint(t *testing.T) {
Password: testPassword,
OrgId: nonExistingOrgID,
}
adminCreateUserScenario(t, "Should create the user", "/api/admin/users", "/api/admin/users", createCmd, func(sc *scenarioContext) {
usrSvc := &usertest.FakeUserService{ExpectedError: models.ErrOrgNotFound}
adminCreateUserScenario(t, "Should create the user", "/api/admin/users", "/api/admin/users", createCmd, usrSvc, func(sc *scenarioContext) {
sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec()
assert.Equal(t, 400, sc.resp.Code)
@ -221,8 +220,8 @@ func TestAdminAPIEndpoint(t *testing.T) {
Login: existingTestLogin,
Password: testPassword,
}
adminCreateUserScenario(t, "Should return an error", "/api/admin/users", "/api/admin/users", createCmd, func(sc *scenarioContext) {
usrSvc := &usertest.FakeUserService{ExpectedError: user.ErrUserAlreadyExists}
adminCreateUserScenario(t, "Should return an error", "/api/admin/users", "/api/admin/users", createCmd, usrSvc, func(sc *scenarioContext) {
sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec()
assert.Equal(t, 412, sc.resp.Code)
@ -397,15 +396,10 @@ func adminDeleteUserScenario(t *testing.T, desc string, url string, routePattern
})
}
func adminCreateUserScenario(t *testing.T, desc string, url string, routePattern string, cmd dtos.AdminCreateUserForm, fn scenarioFunc) {
func adminCreateUserScenario(t *testing.T, desc string, url string, routePattern string, cmd dtos.AdminCreateUserForm, svc *usertest.FakeUserService, fn scenarioFunc) {
t.Run(fmt.Sprintf("%s %s", desc, url), func(t *testing.T) {
hs := HTTPServer{
Login: loginservice.LoginServiceMock{
ExpectedUserForm: cmd,
NoExistingOrgId: nonExistingOrgID,
AlreadyExitingLogin: existingTestLogin,
GeneratedUserId: testUserID,
},
userService: svc,
}
sc := setupScenarioContext(t, url)

View File

@ -259,7 +259,7 @@ func (hs *HTTPServer) CompleteInvite(c *models.ReqContext) response.Response {
SkipOrgSetup: true,
}
usr, err := hs.Login.CreateUser(cmd)
usr, err := hs.userService.Create(c.Req.Context(), &cmd)
if err != nil {
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)

View File

@ -102,7 +102,7 @@ func (hs *HTTPServer) SignUpStep2(c *models.ReqContext) response.Response {
createUserCmd.EmailVerified = true
}
usr, err := hs.Login.CreateUser(createUserCmd)
usr, err := hs.userService.Create(c.Req.Context(), &createUserCmd)
if err != nil {
if errors.Is(err, user.ErrUserAlreadyExists) {
return response.Error(401, "User with same email address already exists", nil)

View File

@ -18,7 +18,6 @@ var (
type TeamSyncFunc func(user *user.User, externalUser *models.ExternalUserInfo) error
type Service interface {
CreateUser(cmd user.CreateUserCommand) (*user.User, error)
UpsertUser(ctx context.Context, cmd *models.UpsertUserCommand) error
DisableExternalUser(ctx context.Context, username string) error
SetTeamSyncFunc(TeamSyncFunc)

View File

@ -43,11 +43,6 @@ type Implementation struct {
orgService org.Service
}
// CreateUser creates inserts a new one.
func (ls *Implementation) CreateUser(cmd user.CreateUserCommand) (*user.User, error) {
return ls.userService.Create(context.Background(), &cmd)
}
// UpsertUser updates an existing user, or if it doesn't exist, inserts a new one.
func (ls *Implementation) UpsertUser(ctx context.Context, cmd *models.UpsertUserCommand) error {
extUser := cmd.ExternalUser
@ -80,7 +75,12 @@ func (ls *Implementation) UpsertUser(ctx context.Context, cmd *models.UpsertUser
}
}
result, errCreateUser := ls.createUser(extUser)
result, errCreateUser := ls.userService.Create(ctx, &user.CreateUserCommand{
Login: extUser.Login,
Email: extUser.Email,
Name: extUser.Name,
SkipOrgSetup: len(extUser.OrgRoles) > 0,
})
if errCreateUser != nil {
return errCreateUser
}
@ -207,16 +207,6 @@ func (ls *Implementation) SetTeamSyncFunc(teamSyncFunc login.TeamSyncFunc) {
ls.TeamSync = teamSyncFunc
}
func (ls *Implementation) createUser(extUser *models.ExternalUserInfo) (*user.User, error) {
cmd := user.CreateUserCommand{
Login: extUser.Login,
Email: extUser.Email,
Name: extUser.Name,
SkipOrgSetup: len(extUser.OrgRoles) > 0,
}
return ls.CreateUser(cmd)
}
func (ls *Implementation) updateUser(ctx context.Context, usr *user.User, extUser *models.ExternalUserInfo) error {
// sync user info
updateCmd := &user.UpdateUserCommand{

View File

@ -2,9 +2,7 @@ package loginservice
import (
"context"
"errors"
"github.com/grafana/grafana/pkg/api/dtos"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/login"
"github.com/grafana/grafana/pkg/services/user"
@ -12,30 +10,9 @@ import (
type LoginServiceMock struct {
login.Service
ExpectedUserForm dtos.AdminCreateUserForm
NoExistingOrgId int64
AlreadyExitingLogin string
GeneratedUserId int64
ExpectedUser *user.User
ExpectedUserFunc func(cmd *models.UpsertUserCommand) *user.User
ExpectedError error
}
func (s LoginServiceMock) CreateUser(cmd user.CreateUserCommand) (*user.User, error) {
if cmd.OrgID == s.NoExistingOrgId {
return nil, models.ErrOrgNotFound
}
if cmd.Login == s.AlreadyExitingLogin {
return nil, user.ErrUserAlreadyExists
}
if s.ExpectedUserForm.Login == cmd.Login && s.ExpectedUserForm.Email == cmd.Email &&
s.ExpectedUserForm.Password == cmd.Password && s.ExpectedUserForm.Name == cmd.Name && s.ExpectedUserForm.OrgId == cmd.OrgID {
return &user.User{ID: s.GeneratedUserId}, nil
}
return nil, errors.New("unexpected cmd")
ExpectedUser *user.User
ExpectedUserFunc func(cmd *models.UpsertUserCommand) *user.User
ExpectedError error
}
func (s LoginServiceMock) UpsertUser(ctx context.Context, cmd *models.UpsertUserCommand) error {

View File

@ -10,9 +10,6 @@ import (
type LoginServiceFake struct{}
func (l *LoginServiceFake) CreateUser(cmd user.CreateUserCommand) (*user.User, error) {
return nil, nil
}
func (l *LoginServiceFake) UpsertUser(ctx context.Context, cmd *models.UpsertUserCommand) error {
return nil
}