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

Security fix for privilege escalation (#598)

Browse Source 
* Prevent external users to update their information

* 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
This commit is contained in:
Misi
2022-10-24 20:53:02 +02:00
committed by GitHub
parent 11ab2c8b1b
commit 7221a95d3e
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
}
pkg/services/login/logintest/logintest.go
+2
View File
@@ -22,6 +22,7 @@ func (l *LoginServiceFake) SetTeamSyncFunc(login.TeamSyncFunc) {}
type AuthInfoServiceFake struct {
LatestUserID int64
ExpectedUserAuth *models.UserAuth
ExpectedUser *models.User
ExpectedExternalUser *models.ExternalUserInfo
ExpectedError error
@@ -38,6 +39,7 @@ func (a *AuthInfoServiceFake) LookupAndUpdate(ctx context.Context, query *models
func (a *AuthInfoServiceFake) GetAuthInfo(ctx context.Context, query *models.GetAuthInfoQuery) error {
a.LatestUserID = query.UserId
query.Result = a.ExpectedUserAuth
return a.ExpectedError
}
public/app/core/components/Signup/SignupPage.tsx
+2 -1
View File
@@ -5,6 +5,7 @@ import { Form, Field, Input, Button, HorizontalGroup, LinkButton, FormAPI } from
import { getConfig } from 'app/core/config';
import { useAppNotification } from 'app/core/copy/appNotification';
import { GrafanaRouteComponentProps } from 'app/core/navigation/types';
import { w3cStandardEmailValidator } from 'app/features/admin/utils';
import { InnerBox, LoginLayout } from '../Login/LoginLayout';
import { PasswordField } from '../PasswordField/PasswordField';
@@ -74,7 +75,7 @@ export const SignupPage: FC<Props> = (props) => {
{...register('email', {
required: 'Email is required',
pattern: {
value: /^\S+@\S+$/,
value: w3cStandardEmailValidator,
message: 'Email is invalid',
},
})}
public/app/core/components/Signup/VerifyEmail.tsx
+2 -1
View File
@@ -4,6 +4,7 @@ import { getBackendSrv } from '@grafana/runtime';
import { Form, Field, Input, Button, Legend, Container, HorizontalGroup, LinkButton } from '@grafana/ui';
import { getConfig } from 'app/core/config';
import { useAppNotification } from 'app/core/copy/appNotification';
import { w3cStandardEmailValidator } from 'app/features/admin/utils';
interface EmailDTO {
email: string;
@@ -53,7 +54,7 @@ export const VerifyEmail: FC = () => {
{...register('email', {
required: 'Email is required',
pattern: {
value: /^\S+@\S+$/,
value: w3cStandardEmailValidator,
message: 'Email is invalid',
},
})}
public/app/features/admin/UserProfile.tsx
+6 -2
View File
@@ -225,7 +225,9 @@ export class UserProfileRow extends PureComponent<UserProfileRowProps, UserProfi
return;
}
this.setState({ value: event.target.value });
this.setState({
value: event.target.value,
});
};
onInputBlur = (event: React.FocusEvent<HTMLInputElement>, status?: LegacyInputStatus) => {
@@ -233,7 +235,9 @@ export class UserProfileRow extends PureComponent<UserProfileRowProps, UserProfi
return;
}
this.setState({ value: event.target.value });
this.setState({
value: event.target.value,
});
};
focusInput = () => {
public/app/features/admin/utils.ts
+4
View File
@@ -1,5 +1,9 @@
import { config } from '@grafana/runtime/src';
// https://html.spec.whatwg.org/multipage/input.html#valid-e-mail-address
export const w3cStandardEmailValidator =
/^[a-zA-Z0-9.!#$%&'*+\/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$/;
export function isTrial() {
const expiry = config.licenseInfo?.trialExpiry;
return !!(expiry && expiry > 0);
public/app/features/invites/SignupInvited.tsx
+3 -1
View File
@@ -8,6 +8,8 @@ import { getConfig } from 'app/core/config';
import { contextSrv } from 'app/core/core';
import { GrafanaRouteComponentProps } from 'app/core/navigation/types';
import { w3cStandardEmailValidator } from '../admin/utils';
interface FormModel {
email: string;
name?: string;
@@ -77,7 +79,7 @@ export const SignupInvitedPage: FC<Props> = ({ match }) => {
{...register('email', {
required: 'Email is required',
pattern: {
value: /^\S+@\S+$/,
value: w3cStandardEmailValidator,
message: 'Email is invalid',
},
})}