diff --git a/pkg/api/admin_users.go b/pkg/api/admin_users.go index 58719149ca1..8ab17970e96 100644 --- a/pkg/api/admin_users.go +++ b/pkg/api/admin_users.go @@ -1,6 +1,9 @@ package api import ( + "errors" + "fmt" + "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/infra/metrics" @@ -29,8 +32,12 @@ func AdminCreateUser(c *models.ReqContext, form dtos.AdminCreateUserForm) Respon } if err := bus.Dispatch(&cmd); err != nil { - if err == models.ErrOrgNotFound { - return Error(400, models.ErrOrgNotFound.Error(), nil) + if errors.Is(err, models.ErrOrgNotFound) { + return Error(400, err.Error(), nil) + } + + if errors.Is(err, models.ErrUserAlreadyExists) { + return Error(412, fmt.Sprintf("User with email '%s' or username '%s' already exists", form.Email, form.Login), err) } return Error(500, "failed to create user", err) diff --git a/pkg/api/admin_users_test.go b/pkg/api/admin_users_test.go index cb876f7b9a2..99416091f2f 100644 --- a/pkg/api/admin_users_test.go +++ b/pkg/api/admin_users_test.go @@ -259,6 +259,26 @@ func TestAdminApiEndpoint(t *testing.T) { }) }) }) + + Convey("When a server admin attempts to create a user with an already existing email/login", t, func() { + bus.AddHandler("test", func(cmd *models.CreateUserCommand) error { + return models.ErrUserAlreadyExists + }) + + createCmd := dtos.AdminCreateUserForm{ + Login: TestLogin, + Password: TestPassword, + } + + adminCreateUserScenario("Should return an error", "/api/admin/users", "/api/admin/users", createCmd, func(sc *scenarioContext) { + sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec() + So(sc.resp.Code, ShouldEqual, 412) + + respJSON, err := simplejson.NewJson(sc.resp.Body.Bytes()) + So(err, ShouldBeNil) + So(respJSON.Get("error").MustString(), ShouldEqual, "User already exists") + }) + }) } func putAdminScenario(desc string, url string, routePattern string, role models.RoleType, cmd dtos.AdminUpdateUserPermissionsForm, fn scenarioFunc) { diff --git a/pkg/api/org_invite.go b/pkg/api/org_invite.go index 595ec3249fc..077db58b952 100644 --- a/pkg/api/org_invite.go +++ b/pkg/api/org_invite.go @@ -1,6 +1,7 @@ package api import ( + "errors" "fmt" "github.com/grafana/grafana/pkg/api/dtos" @@ -81,6 +82,7 @@ func AddOrgInvite(c *models.ReqContext, inviteDto dtos.AddInviteForm) Response { if err == models.ErrSmtpNotEnabled { return Error(412, err.Error(), err) } + return Error(500, "Failed to send email invite", err) } @@ -181,6 +183,10 @@ func (hs *HTTPServer) CompleteInvite(c *models.ReqContext, completeInvite dtos.C } if err := bus.Dispatch(&cmd); err != nil { + if errors.Is(err, models.ErrUserAlreadyExists) { + return Error(412, fmt.Sprintf("User with email '%s' or username '%s' already exists", completeInvite.Email, completeInvite.Username), err) + } + return Error(500, "failed to create user", err) } diff --git a/pkg/api/signup.go b/pkg/api/signup.go index 5adfb7050ad..7db8ee421bb 100644 --- a/pkg/api/signup.go +++ b/pkg/api/signup.go @@ -1,6 +1,8 @@ package api import ( + "errors" + "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/events" @@ -78,14 +80,12 @@ func (hs *HTTPServer) SignUpStep2(c *models.ReqContext, form dtos.SignUpStep2For createUserCmd.EmailVerified = true } - // check if user exists - existing := models.GetUserByLoginQuery{LoginOrEmail: form.Email} - if err := bus.Dispatch(&existing); err == nil { - return Error(401, "User with same email address already exists", nil) - } - // dispatch create command if err := bus.Dispatch(&createUserCmd); err != nil { + if errors.Is(err, models.ErrUserAlreadyExists) { + return Error(401, "User with same email address already exists", nil) + } + return Error(500, "Failed to create user", err) } diff --git a/pkg/models/user.go b/pkg/models/user.go index afada014366..ce71ebc668e 100644 --- a/pkg/models/user.go +++ b/pkg/models/user.go @@ -7,8 +7,9 @@ import ( // Typed errors var ( - ErrUserNotFound = errors.New("User not found") - ErrLastGrafanaAdmin = errors.New("Cannot remove last grafana admin") + ErrUserNotFound = errors.New("User not found") + ErrUserAlreadyExists = errors.New("User already exists") + ErrLastGrafanaAdmin = errors.New("Cannot remove last grafana admin") ) type Password string diff --git a/pkg/services/sqlstore/user.go b/pkg/services/sqlstore/user.go index 52eeb03d303..07338407d1d 100644 --- a/pkg/services/sqlstore/user.go +++ b/pkg/services/sqlstore/user.go @@ -68,6 +68,11 @@ func CreateUser(ctx context.Context, cmd *models.CreateUserCommand) error { cmd.Email = cmd.Login } + exists, _ := sess.Where("email=? OR login=?", cmd.Email, cmd.Login).Get(&models.User{}) + if exists { + return models.ErrUserAlreadyExists + } + // create user user := models.User{ Email: cmd.Email, diff --git a/pkg/services/sqlstore/user_test.go b/pkg/services/sqlstore/user_test.go index 43427014be8..df539497dc1 100644 --- a/pkg/services/sqlstore/user_test.go +++ b/pkg/services/sqlstore/user_test.go @@ -568,6 +568,40 @@ func TestUserDataAccess(t *testing.T) { So(query.Result.IsAdmin, ShouldEqual, true) }) }) + + Convey("Given one user", func() { + const email = "user@test.com" + const username = "user" + createUserCmd := &models.CreateUserCommand{ + Email: email, + Name: "user", + Login: username, + } + err := CreateUser(context.Background(), createUserCmd) + So(err, ShouldBeNil) + + Convey("When trying to create a new user with the same email, an error is returned", func() { + createUserCmd := &models.CreateUserCommand{ + Email: email, + Name: "user2", + Login: "user2", + SkipOrgSetup: true, + } + err := CreateUser(context.Background(), createUserCmd) + So(err, ShouldEqual, models.ErrUserAlreadyExists) + }) + + Convey("When trying to create a new user with the same login, an error is returned", func() { + createUserCmd := &models.CreateUserCommand{ + Email: "user2@test.com", + Name: "user2", + Login: username, + SkipOrgSetup: true, + } + err := CreateUser(context.Background(), createUserCmd) + So(err, ShouldEqual, models.ErrUserAlreadyExists) + }) + }) }) }