IntenseWebs/grafana
3
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2025-02-25 18:55:37 -06:00
Code Issues Packages Projects Releases Wiki Activity

Login: Replace command dispatch by explicit call (#32088)

Browse Source 
* Fix LoginService.UpsertUser user creation

* Fix API AdminCreateUser user creation

* Add missing underscore import

* Fix API CompleteInvite user creation

* Fix API SignUpStep2 user creation
This commit is contained in:
Joan López de la Franca Beltran
2021-03-18 17:16:56 +01:00
committed by GitHub
parent dfd4eccc7c
commit b2e82a4f37
11 changed files with 325 additions and 338 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
pkg/api/admin_users.go
View File

@@ -12,7 +12,7 @@ import (
"github.com/grafana/grafana/pkg/util"
)
func AdminCreateUser(c *models.ReqContext, form dtos.AdminCreateUserForm) response.Response {
func (hs *HTTPServer) AdminCreateUser(c *models.ReqContext, form dtos.AdminCreateUserForm) response.Response {
cmd := models.CreateUserCommand{
Login: form.Login,
Email: form.Email,
@@ -32,7 +32,8 @@ func AdminCreateUser(c *models.ReqContext, form dtos.AdminCreateUserForm) respon
return response.Error(400, "Password is missing or too short", nil)
}
if err := bus.Dispatch(&cmd); err != nil {
user, err := hs.Login.CreateUser(cmd)
if err != nil {
if errors.Is(err, models.ErrOrgNotFound) {
return response.Error(400, err.Error(), nil)
}
@@ -46,8 +47,6 @@ func AdminCreateUser(c *models.ReqContext, form dtos.AdminCreateUserForm) respon
metrics.MApiAdminUserCreate.Inc()
user := cmd.Result
result := models.UserIdDTO{
Message: "User created",
Id: user.Id,

99
pkg/api/admin_users_test.go
View File

@@ -1,6 +1,7 @@
package api
import (
"errors"
"fmt"
"testing"
@@ -11,14 +12,16 @@ import (
"github.com/grafana/grafana/pkg/components/simplejson"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/auth"
"github.com/grafana/grafana/pkg/services/login"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
const (
testLogin = "test@example.com"
testPassword = "password"
nonExistingOrgID = 1000
testLogin = "test@example.com"
testPassword = "password"
nonExistingOrgID = 1000
existingTestLogin = "existing@example.com"
)
func TestAdminAPIEndpoint(t *testing.T) {
@@ -223,21 +226,6 @@ func TestAdminAPIEndpoint(t *testing.T) {
adminCreateUserScenario(t, "Should create the user", "/api/admin/users", "/api/admin/users", createCmd, func(sc *scenarioContext) {
bus.ClearBusHandlers()
var userLogin string
var orgID int64
bus.AddHandler("test", func(cmd *models.CreateUserCommand) error {
userLogin = cmd.Login
orgID = cmd.OrgId
if orgID == nonExistingOrgID {
return models.ErrOrgNotFound
}
cmd.Result = models.User{Id: testUserID}
return nil
})
sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec()
assert.Equal(t, 200, sc.resp.Code)
@@ -245,10 +233,6 @@ func TestAdminAPIEndpoint(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, testUserID, respJSON.Get("id").MustInt64())
assert.Equal(t, "User created", respJSON.Get("message").MustString())
// Verify that userLogin and orgID were transmitted correctly to the handler
assert.Equal(t, testLogin, userLogin)
assert.Equal(t, int64(0), orgID)
})
})
@@ -262,21 +246,6 @@ func TestAdminAPIEndpoint(t *testing.T) {
adminCreateUserScenario(t, "Should create the user", "/api/admin/users", "/api/admin/users", createCmd, func(sc *scenarioContext) {
bus.ClearBusHandlers()
var userLogin string
var orgID int64
bus.AddHandler("test", func(cmd *models.CreateUserCommand) error {
userLogin = cmd.Login
orgID = cmd.OrgId
if orgID == nonExistingOrgID {
return models.ErrOrgNotFound
}
cmd.Result = models.User{Id: testUserID}
return nil
})
sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec()
assert.Equal(t, 200, sc.resp.Code)
@@ -284,9 +253,6 @@ func TestAdminAPIEndpoint(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, testUserID, respJSON.Get("id").MustInt64())
assert.Equal(t, "User created", respJSON.Get("message").MustString())
assert.Equal(t, testLogin, userLogin)
assert.Equal(t, testOrgID, orgID)
})
})
@@ -300,47 +266,25 @@ func TestAdminAPIEndpoint(t *testing.T) {
adminCreateUserScenario(t, "Should create the user", "/api/admin/users", "/api/admin/users", createCmd, func(sc *scenarioContext) {
bus.ClearBusHandlers()
var userLogin string
var orgID int64
bus.AddHandler("test", func(cmd *models.CreateUserCommand) error {
userLogin = cmd.Login
orgID = cmd.OrgId
if orgID == nonExistingOrgID {
return models.ErrOrgNotFound
}
cmd.Result = models.User{Id: testUserID}
return nil
})
sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec()
assert.Equal(t, 400, sc.resp.Code)
respJSON, err := simplejson.NewJson(sc.resp.Body.Bytes())
require.NoError(t, err)
assert.Equal(t, "organization not found", respJSON.Get("message").MustString())
assert.Equal(t, testLogin, userLogin)
assert.Equal(t, int64(1000), orgID)
})
})
})
t.Run("When a server admin attempts to create a user with an already existing email/login", func(t *testing.T) {
createCmd := dtos.AdminCreateUserForm{
Login: testLogin,
Login: existingTestLogin,
Password: testPassword,
}
adminCreateUserScenario(t, "Should return an error", "/api/admin/users", "/api/admin/users", createCmd, func(sc *scenarioContext) {
bus.ClearBusHandlers()
bus.AddHandler("test", func(cmd *models.CreateUserCommand) error {
return models.ErrUserAlreadyExists
})
sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec()
assert.Equal(t, 412, sc.resp.Code)
@@ -506,12 +450,17 @@ func adminCreateUserScenario(t *testing.T, desc string, url string, routePattern
t.Run(fmt.Sprintf("%s %s", desc, url), func(t *testing.T) {
t.Cleanup(bus.ClearBusHandlers)
hs := HTTPServer{
Bus: bus.GetBus(),
Login: fakeLoginService{expected: cmd},
}
sc := setupScenarioContext(t, url)
sc.defaultHandler = routing.Wrap(func(c *models.ReqContext) response.Response {
sc.context = c
sc.context.UserId = testUserID
return AdminCreateUser(c, cmd)
return hs.AdminCreateUser(c, cmd)
})
sc.m.Post(routePattern, sc.defaultHandler)
@@ -519,3 +468,25 @@ func adminCreateUserScenario(t *testing.T, desc string, url string, routePattern
fn(sc)
})
}
type fakeLoginService struct {
login.Service
expected dtos.AdminCreateUserForm
}
func (s fakeLoginService) CreateUser(cmd models.CreateUserCommand) (*models.User, error) {
if cmd.OrgId == nonExistingOrgID {
return nil, models.ErrOrgNotFound
}
if cmd.Login == existingTestLogin {
return nil, models.ErrUserAlreadyExists
}
if s.expected.Login == cmd.Login && s.expected.Email == cmd.Email &&
s.expected.Password == cmd.Password && s.expected.Name == cmd.Name && s.expected.OrgId == cmd.OrgId {
return &models.User{Id: testUserID}, nil
}
return nil, errors.New("unexpected cmd")
}

2
pkg/api/api.go
View File

@@ -404,7 +404,7 @@ func (hs *HTTPServer) registerRoutes() {
// admin api
r.Group("/api/admin", func(adminRoute routing.RouteRegister) {
adminRoute.Get("/settings", routing.Wrap(AdminGetSettings))
adminRoute.Post("/users", bind(dtos.AdminCreateUserForm{}), routing.Wrap(AdminCreateUser))
adminRoute.Post("/users", bind(dtos.AdminCreateUserForm{}), routing.Wrap(hs.AdminCreateUser))
adminRoute.Put("/users/:id/password", bind(dtos.AdminUpdateUserPasswordForm{}), routing.Wrap(AdminUpdateUserPassword))
adminRoute.Put("/users/:id/permissions", bind(dtos.AdminUpdateUserPermissionsForm{}), routing.Wrap(AdminUpdateUserPermissions))
adminRoute.Delete("/users/:id", routing.Wrap(AdminDeleteUser))

2
pkg/api/http_server.go
View File

@@ -76,7 +76,7 @@ type HTTPServer struct {
QuotaService *quota.QuotaService `inject:""`
RemoteCacheService *remotecache.RemoteCache `inject:""`
ProvisioningService provisioning.ProvisioningService `inject:""`
Login *login.LoginService `inject:""`
Login login.Service `inject:""`
License models.Licensing `inject:""`
BackendPluginManager backendplugin.Manager `inject:""`
DataProxy *datasourceproxy.DatasourceProxyService `inject:""`

7
pkg/api/org_invite.go
View File

@@ -186,7 +186,8 @@ func (hs *HTTPServer) CompleteInvite(c *models.ReqContext, completeInvite dtos.C
SkipOrgSetup: true,
}
if err := bus.Dispatch(&cmd); err != nil {
user, err := hs.Login.CreateUser(cmd)
if err != nil {
if errors.Is(err, models.ErrUserAlreadyExists) {
return response.Error(412, fmt.Sprintf("User with email '%s' or username '%s' already exists", completeInvite.Email, completeInvite.Username), err)
}
@@ -194,8 +195,6 @@ func (hs *HTTPServer) CompleteInvite(c *models.ReqContext, completeInvite dtos.C
return response.Error(500, "failed to create user", err)
}
user := &cmd.Result
if err := bus.Publish(&events.SignUpCompleted{
Name: user.NameOrFallback(),
Email: user.Email,
@@ -207,7 +206,7 @@ func (hs *HTTPServer) CompleteInvite(c *models.ReqContext, completeInvite dtos.C
return rsp
}
err := hs.loginUserWithUser(user, c)
err = hs.loginUserWithUser(user, c)
if err != nil {
return response.Error(500, "failed to accept invite", err)
}

7
pkg/api/signup.go
View File

@@ -81,8 +81,8 @@ func (hs *HTTPServer) SignUpStep2(c *models.ReqContext, form dtos.SignUpStep2For
createUserCmd.EmailVerified = true
}
// dispatch create command
if err := bus.Dispatch(&createUserCmd); err != nil {
user, err := hs.Login.CreateUser(createUserCmd)
if err != nil {
if errors.Is(err, models.ErrUserAlreadyExists) {
return response.Error(401, "User with same email address already exists", nil)
}
@@ -91,7 +91,6 @@ func (hs *HTTPServer) SignUpStep2(c *models.ReqContext, form dtos.SignUpStep2For
}
// publish signup event
user := &createUserCmd.Result
if err := bus.Publish(&events.SignUpCompleted{
Email: user.Email,
Name: user.NameOrFallback(),
@@ -118,7 +117,7 @@ func (hs *HTTPServer) SignUpStep2(c *models.ReqContext, form dtos.SignUpStep2For
apiResponse["code"] = "redirect-to-select-org"
}
err := hs.loginUserWithUser(user, c)
err = hs.loginUserWithUser(user, c)
if err != nil {
return response.Error(500, "failed to login user", err)
}