public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix: Email and username trimming and invitation validation (#58449)

Browse Source 
* Trim leading and trailing whitespaces from email and username on signup

* Check whether the provided email address is the same as where the invitation sent

Co-authored-by: Mihaly Gyongyosi <mgyongyosi@users.noreply.github.com>
This commit is contained in:
Jo
2022-11-14 13:13:06 +01:00
committed by GitHub
co-authored by Mihaly Gyongyosi
parent 5facdc6510
commit 4e4f4218d1
12 changed files with 229 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/api/admin_users.go
+5
View File
@@ -5,6 +5,7 @@ import (
"fmt"
"net/http"
"strconv"
"strings"
"github.com/grafana/grafana/pkg/api/dtos"
"github.com/grafana/grafana/pkg/api/response"
@@ -19,6 +20,10 @@ func (hs *HTTPServer) AdminCreateUser(c *models.ReqContext) response.Response {
if err := web.Bind(c.Req, &form); err != nil {
return response.Error(http.StatusBadRequest, "bad request data", err)
}
form.Email = strings.TrimSpace(form.Email)
form.Login = strings.TrimSpace(form.Login)
cmd := models.CreateUserCommand{
Login: form.Login,
Email: form.Email,
pkg/api/org_invite.go
+22 -5
View File
@@ -6,6 +6,7 @@ import (
"fmt"
"net/http"
"strconv"
"strings"
"github.com/grafana/grafana/pkg/api/dtos"
"github.com/grafana/grafana/pkg/api/response"
@@ -186,21 +187,37 @@ func (hs *HTTPServer) GetInviteInfoByCode(c *models.ReqContext) response.Respons
func (hs *HTTPServer) CompleteInvite(c *models.ReqContext) response.Response {
completeInvite := dtos.CompleteInviteForm{}
if err := web.Bind(c.Req, &completeInvite); err != nil {
var err error
if err = web.Bind(c.Req, &completeInvite); err != nil {
return response.Error(http.StatusBadRequest, "bad request data", err)
}
query := models.GetTempUserByCodeQuery{Code: completeInvite.InviteCode}
completeInvite.Email, err = ValidateAndNormalizeEmail(completeInvite.Email)
if err != nil {
return response.Error(http.StatusBadRequest, "Invalid email address provided", nil)
}
completeInvite.Username = strings.TrimSpace(completeInvite.Username)
query := models.GetTempUserByCodeQuery{Code: completeInvite.InviteCode}
if err := hs.SQLStore.GetTempUserByCode(c.Req.Context(), &query); err != nil {
if errors.Is(err, models.ErrTempUserNotFound) {
return response.Error(404, "Invite not found", nil)
return response.Error(http.StatusNotFound, "Invite not found", nil)
}
return response.Error(500, "Failed to get invite", err)
return response.Error(http.StatusInternalServerError, "Failed to get invite", err)
}
invite := query.Result
if invite.Status != models.TmpUserInvitePending {
return response.Error(412, fmt.Sprintf("Invite cannot be used in status %s", invite.Status), nil)
return response.Error(http.StatusPreconditionFailed, fmt.Sprintf("Invite cannot be used in status %s", invite.Status), nil)
}
// In case the user is invited by email address
if inviteMail, err := ValidateAndNormalizeEmail(invite.Email); err == nil {
// Make sure that the email address is not amended
if completeInvite.Email != inviteMail {
return response.Error(http.StatusBadRequest, "The provided email is different from the address that is found in the invite", nil)
}
}
cmd := models.CreateUserCommand{
pkg/api/signup.go
+12 -3
View File
@@ -4,6 +4,7 @@ import (
"context"
"errors"
"net/http"
"strings"
"github.com/grafana/grafana/pkg/api/dtos"
"github.com/grafana/grafana/pkg/api/response"
@@ -27,15 +28,21 @@ func GetSignUpOptions(c *models.ReqContext) response.Response {
// POST /api/user/signup
func (hs *HTTPServer) SignUp(c *models.ReqContext) response.Response {
form := dtos.SignUpForm{}
if err := web.Bind(c.Req, &form); err != nil {
var err error
if err = web.Bind(c.Req, &form); err != nil {
return response.Error(http.StatusBadRequest, "bad request data", err)
}
if !setting.AllowUserSignUp {
return response.Error(401, "User signup is disabled", nil)
}
form.Email, err = ValidateAndNormalizeEmail(form.Email)
if err != nil {
return response.Error(http.StatusBadRequest, "Invalid email address", nil)
}
existing := models.GetUserByLoginQuery{LoginOrEmail: form.Email}
if err := hs.SQLStore.GetUserByLogin(c.Req.Context(), &existing); err == nil {
if err = hs.SQLStore.GetUserByLogin(c.Req.Context(), &existing); err == nil {
return response.Error(422, "User with same email address already exists", nil)
}
@@ -44,7 +51,6 @@ func (hs *HTTPServer) SignUp(c *models.ReqContext) response.Response {
cmd.Email = form.Email
cmd.Status = models.TmpUserSignUpStarted
cmd.InvitedByUserId = c.UserId
var err error
cmd.Code, err = util.GetRandomString(20)
if err != nil {
return response.Error(500, "Failed to generate random string", err)
@@ -76,6 +82,9 @@ func (hs *HTTPServer) SignUpStep2(c *models.ReqContext) response.Response {
return response.Error(401, "User signup is disabled", nil)
}
form.Email = strings.TrimSpace(form.Email)
form.Username = strings.TrimSpace(form.Username)
createUserCmd := models.CreateUserCommand{
Email: form.Email,
Login: form.Username,
pkg/api/user.go
+37 -2
View File
@@ -5,6 +5,7 @@ import (
"errors"
"net/http"
"strconv"
"strings"
"github.com/grafana/grafana/pkg/api/dtos"
"github.com/grafana/grafana/pkg/api/response"
@@ -79,9 +80,14 @@ func (hs *HTTPServer) GetUserByLoginOrEmail(c *models.ReqContext) response.Respo
// POST /api/user
func (hs *HTTPServer) UpdateSignedInUser(c *models.ReqContext) response.Response {
cmd := models.UpdateUserCommand{}
if err := web.Bind(c.Req, &cmd); err != nil {
var err error
if err = web.Bind(c.Req, &cmd); err != nil {
return response.Error(http.StatusBadRequest, "bad request data", err)
}
cmd.Email = strings.TrimSpace(cmd.Email)
cmd.Login = strings.TrimSpace(cmd.Login)
if setting.AuthProxyEnabled {
if setting.AuthProxyHeaderProperty == "email" && cmd.Email != c.Email {
return response.Error(400, "Not allowed to change email when auth proxy is using email property", nil)
@@ -98,13 +104,18 @@ func (hs *HTTPServer) UpdateSignedInUser(c *models.ReqContext) response.Response
func (hs *HTTPServer) UpdateUser(c *models.ReqContext) response.Response {
cmd := models.UpdateUserCommand{}
var err error
if err := web.Bind(c.Req, &cmd); err != nil {
if err = web.Bind(c.Req, &cmd); err != nil {
return response.Error(http.StatusBadRequest, "bad request data", err)
}
cmd.Email = strings.TrimSpace(cmd.Email)
cmd.Login = strings.TrimSpace(cmd.Login)
cmd.UserId, err = strconv.ParseInt(web.Params(c.Req)[":id"], 10, 64)
if err != nil {
return response.Error(http.StatusBadRequest, "id is invalid", err)
}
return hs.handleUpdateUser(c.Req.Context(), cmd)
}
@@ -133,6 +144,16 @@ func (hs *HTTPServer) UpdateUserActiveOrg(c *models.ReqContext) response.Respons
}
func (hs *HTTPServer) handleUpdateUser(ctx context.Context, cmd models.UpdateUserCommand) response.Response {
// external user -> user data cannot be updated
isExternal, err := hs.isExternalUser(ctx, cmd.UserId)
if err != nil {
return response.Error(http.StatusInternalServerError, "Failed to validate User", err)
}
if isExternal {
return response.Error(http.StatusForbidden, "User info cannot be updated for external Users", nil)
}
if len(cmd.Login) == 0 {
cmd.Login = cmd.Email
if len(cmd.Login) == 0 {
@@ -147,6 +168,20 @@ func (hs *HTTPServer) handleUpdateUser(ctx context.Context, cmd models.UpdateUse
return response.Success("User updated")
}
func (hs *HTTPServer) isExternalUser(ctx context.Context, userID int64) (bool, error) {
getAuthQuery := models.GetAuthInfoQuery{UserId: userID}
var err error
if err = hs.authInfoService.GetAuthInfo(ctx, &getAuthQuery); err == nil {
return true, nil
}
if errors.Is(err, models.ErrUserNotFound) {
return false, nil
}
return false, err
}
// GET /api/user/orgs
func (hs *HTTPServer) GetSignedInUserOrgList(c *models.ReqContext) response.Response {
return hs.getUserOrgList(c.Req.Context(), c.UserId)
pkg/api/user_test.go
+117
View File
@@ -13,11 +13,14 @@ import (
"golang.org/x/oauth2"
"github.com/grafana/grafana/pkg/api/dtos"
"github.com/grafana/grafana/pkg/api/response"
"github.com/grafana/grafana/pkg/api/routing"
"github.com/grafana/grafana/pkg/components/simplejson"
"github.com/grafana/grafana/pkg/models"
acmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
"github.com/grafana/grafana/pkg/services/login/authinfoservice"
authinfostore "github.com/grafana/grafana/pkg/services/login/authinfoservice/database"
"github.com/grafana/grafana/pkg/services/login/logintest"
"github.com/grafana/grafana/pkg/services/searchusers"
"github.com/grafana/grafana/pkg/services/searchusers/filters"
"github.com/grafana/grafana/pkg/services/secrets/database"
@@ -184,3 +187,117 @@ func TestUserAPIEndpoint_userLoggedIn(t *testing.T) {
assert.Equal(t, 10, respJSON.Get("perPage").MustInt())
}, mock)
}
func TestHTTPServer_UpdateUser(t *testing.T) {
settings := setting.NewCfg()
sqlStore := sqlstore.InitTestDB(t)
hs := &HTTPServer{
Cfg: settings,
SQLStore: sqlStore,
AccessControl: acmock.New(),
}
updateUserCommand := models.UpdateUserCommand{
Email: fmt.Sprint("admin", "@test.com"),
Name: "admin",
Login: "admin",
UserId: 1,
}
updateUserScenario(t, updateUserContext{
desc: "Should return 403 when the current User is an external user",
url: "/api/users/1",
routePattern: "/api/users/:id",
cmd: updateUserCommand,
fn: func(sc *scenarioContext) {
sc.authInfoService.ExpectedUserAuth = &models.UserAuth{}
sc.fakeReqWithParams("PUT", sc.url, map[string]string{"id": "1"}).exec()
assert.Equal(t, 403, sc.resp.Code)
},
}, hs)
}
type updateUserContext struct {
desc string
url string
routePattern string
cmd models.UpdateUserCommand
fn scenarioFunc
}
func updateUserScenario(t *testing.T, ctx updateUserContext, hs *HTTPServer) {
t.Run(fmt.Sprintf("%s %s", ctx.desc, ctx.url), func(t *testing.T) {
sc := setupScenarioContext(t, ctx.url)
sc.authInfoService = &logintest.AuthInfoServiceFake{}
hs.authInfoService = sc.authInfoService
sc.defaultHandler = routing.Wrap(func(c *models.ReqContext) response.Response {
c.Req.Body = mockRequestBody(ctx.cmd)
c.Req.Header.Add("Content-Type", "application/json")
sc.context = c
sc.context.OrgId = testOrgID
sc.context.UserId = testUserID
return hs.UpdateUser(c)
})
sc.m.Put(ctx.routePattern, sc.defaultHandler)
ctx.fn(sc)
})
}
func TestHTTPServer_UpdateSignedInUser(t *testing.T) {
settings := setting.NewCfg()
sqlStore := sqlstore.InitTestDB(t)
hs := &HTTPServer{
Cfg: settings,
SQLStore: sqlStore,
AccessControl: acmock.New(),
}
updateUserCommand := models.UpdateUserCommand{
Email: fmt.Sprint("admin", "@test.com"),
Name: "admin",
Login: "admin",
UserId: 1,
}
updateSignedInUserScenario(t, updateUserContext{
desc: "Should return 403 when the current User is an external user",
url: "/api/users/",
routePattern: "/api/users/",
cmd: updateUserCommand,
fn: func(sc *scenarioContext) {
sc.authInfoService.ExpectedUserAuth = &models.UserAuth{}
sc.fakeReqWithParams("PUT", sc.url, map[string]string{"id": "1"}).exec()
assert.Equal(t, 403, sc.resp.Code)
},
}, hs)
}
func updateSignedInUserScenario(t *testing.T, ctx updateUserContext, hs *HTTPServer) {
t.Run(fmt.Sprintf("%s %s", ctx.desc, ctx.url), func(t *testing.T) {
sc := setupScenarioContext(t, ctx.url)
sc.authInfoService = &logintest.AuthInfoServiceFake{}
hs.authInfoService = sc.authInfoService
sc.defaultHandler = routing.Wrap(func(c *models.ReqContext) response.Response {
c.Req.Body = mockRequestBody(ctx.cmd)
c.Req.Header.Add("Content-Type", "application/json")
sc.context = c
sc.context.OrgId = testOrgID
sc.context.UserId = testUserID
return hs.UpdateSignedInUser(c)
})
sc.m.Put(ctx.routePattern, sc.defaultHandler)
ctx.fn(sc)
})
}
pkg/api/utils.go
+17 -1
View File
@@ -1,9 +1,25 @@
package api
import "encoding/json"
import (
"encoding/json"
"net/mail"
)
func jsonMap(data []byte) (map[string]string, error) {
jsonMap := make(map[string]string)
err := json.Unmarshal(data, &jsonMap)
return jsonMap, err
}
func ValidateAndNormalizeEmail(email string) (string, error) {
if email == "" {
return "", nil
}
e, err := mail.ParseAddress(email)
if err != nil {
return "", err
}
return e.Address, nil
}