Email: trigger email verification flow (#85587)

* Add email and email_verified to id token if identity is a user

* Add endpoint to trigger email verification for user

* Add function to clear stored id tokens and use it when email verification is completed
This commit is contained in:
Karl Persson 2024-04-05 12:05:46 +02:00 committed by GitHub
parent 661aaf352e
commit ba41954854
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
14 changed files with 122 additions and 19 deletions

View File

@ -191,6 +191,7 @@ func (hs *HTTPServer) registerRoutes() {
// update user email // update user email
if hs.Cfg.Smtp.Enabled && hs.Cfg.VerifyEmailEnabled { if hs.Cfg.Smtp.Enabled && hs.Cfg.VerifyEmailEnabled {
r.Get("/user/email/update", reqSignedInNoAnonymous, routing.Wrap(hs.UpdateUserEmail)) r.Get("/user/email/update", reqSignedInNoAnonymous, routing.Wrap(hs.UpdateUserEmail))
r.Post("/api/user/email/start-verify", reqSignedInNoAnonymous, routing.Wrap(hs.StartEmailVerificaton))
} }
// invited // invited

View File

@ -264,12 +264,12 @@ func (hs *HTTPServer) handleUpdateUser(ctx context.Context, cmd user.UpdateUserC
if err != nil { if err != nil {
return response.Error(http.StatusBadRequest, "Invalid email address", err) return response.Error(http.StatusBadRequest, "Invalid email address", err)
} }
return hs.verifyEmailUpdate(ctx, normalized, user.EmailUpdateAction, usr) return hs.startEmailVerification(ctx, normalized, user.EmailUpdateAction, usr)
} }
if len(cmd.Login) != 0 && usr.Login != cmd.Login { if len(cmd.Login) != 0 && usr.Login != cmd.Login {
normalized, err := ValidateAndNormalizeEmail(cmd.Login) normalized, err := ValidateAndNormalizeEmail(cmd.Login)
if err == nil && usr.Email != normalized { if err == nil && usr.Email != normalized {
return hs.verifyEmailUpdate(ctx, cmd.Login, user.LoginUpdateAction, usr) return hs.startEmailVerification(ctx, cmd.Login, user.LoginUpdateAction, usr)
} }
} }
} }
@ -284,7 +284,34 @@ func (hs *HTTPServer) handleUpdateUser(ctx context.Context, cmd user.UpdateUserC
return response.Success("User updated") return response.Success("User updated")
} }
func (hs *HTTPServer) verifyEmailUpdate(ctx context.Context, email string, field user.UpdateEmailActionType, usr *user.User) response.Response { func (hs *HTTPServer) StartEmailVerificaton(c *contextmodel.ReqContext) response.Response {
namespace, id := c.SignedInUser.GetNamespacedID()
if !identity.IsNamespace(namespace, identity.NamespaceUser) {
return response.Error(http.StatusBadRequest, "Only users can verify their email", nil)
}
if c.SignedInUser.IsEmailVerified() {
// email is already verified so we don't need to trigger the flow.
return response.Respond(http.StatusNotModified, nil)
}
userID, err := strconv.ParseInt(id, 10, 64)
if err != nil {
return response.Error(http.StatusInternalServerError, "Got invalid user id", err)
}
usr, err := hs.userService.GetByID(c.Req.Context(), &user.GetUserByIDQuery{
ID: userID,
})
if err != nil {
return response.ErrOrFallback(http.StatusInternalServerError, "Failed to fetch user", err)
}
return hs.startEmailVerification(c.Req.Context(), usr.Email, user.EmailUpdateAction, usr)
}
func (hs *HTTPServer) startEmailVerification(ctx context.Context, email string, field user.UpdateEmailActionType, usr *user.User) response.Response {
if err := hs.userVerifier.Start(ctx, user.StartVerifyEmailCommand{ if err := hs.userVerifier.Start(ctx, user.StartVerifyEmailCommand{
User: *usr, User: *usr,
Email: email, Email: email,

View File

@ -10,13 +10,6 @@ import (
"testing" "testing"
"time" "time"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/notifications"
"github.com/grafana/grafana/pkg/services/secrets/fakes"
tempuser "github.com/grafana/grafana/pkg/services/temp_user"
"github.com/grafana/grafana/pkg/services/temp_user/tempuserimpl"
"github.com/grafana/grafana/pkg/web/webtest"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"golang.org/x/oauth2" "golang.org/x/oauth2"
@ -30,23 +23,30 @@ import (
"github.com/grafana/grafana/pkg/infra/remotecache" "github.com/grafana/grafana/pkg/infra/remotecache"
"github.com/grafana/grafana/pkg/login/social" "github.com/grafana/grafana/pkg/login/social"
"github.com/grafana/grafana/pkg/login/social/socialtest" "github.com/grafana/grafana/pkg/login/social/socialtest"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/accesscontrol/acimpl" "github.com/grafana/grafana/pkg/services/accesscontrol/acimpl"
acmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock" acmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
"github.com/grafana/grafana/pkg/services/auth/idtest"
contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
"github.com/grafana/grafana/pkg/services/login" "github.com/grafana/grafana/pkg/services/login"
"github.com/grafana/grafana/pkg/services/login/authinfoimpl" "github.com/grafana/grafana/pkg/services/login/authinfoimpl"
"github.com/grafana/grafana/pkg/services/login/authinfotest" "github.com/grafana/grafana/pkg/services/login/authinfotest"
"github.com/grafana/grafana/pkg/services/notifications"
"github.com/grafana/grafana/pkg/services/org/orgimpl" "github.com/grafana/grafana/pkg/services/org/orgimpl"
"github.com/grafana/grafana/pkg/services/quota/quotatest" "github.com/grafana/grafana/pkg/services/quota/quotatest"
"github.com/grafana/grafana/pkg/services/searchusers" "github.com/grafana/grafana/pkg/services/searchusers"
"github.com/grafana/grafana/pkg/services/searchusers/filters" "github.com/grafana/grafana/pkg/services/searchusers/filters"
"github.com/grafana/grafana/pkg/services/secrets/database" "github.com/grafana/grafana/pkg/services/secrets/database"
"github.com/grafana/grafana/pkg/services/secrets/fakes"
secretsManager "github.com/grafana/grafana/pkg/services/secrets/manager" secretsManager "github.com/grafana/grafana/pkg/services/secrets/manager"
"github.com/grafana/grafana/pkg/services/supportbundles/supportbundlestest" "github.com/grafana/grafana/pkg/services/supportbundles/supportbundlestest"
tempuser "github.com/grafana/grafana/pkg/services/temp_user"
"github.com/grafana/grafana/pkg/services/temp_user/tempuserimpl"
"github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/services/user/userimpl" "github.com/grafana/grafana/pkg/services/user/userimpl"
"github.com/grafana/grafana/pkg/services/user/usertest" "github.com/grafana/grafana/pkg/services/user/usertest"
"github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/web/webtest"
) )
const newEmail = "newemail@localhost" const newEmail = "newemail@localhost"
@ -397,7 +397,7 @@ func setupUpdateEmailTests(t *testing.T, cfg *setting.Cfg) (*user.User, *HTTPSer
require.NoError(t, err) require.NoError(t, err)
nsMock := notifications.MockNotificationService() nsMock := notifications.MockNotificationService()
verifier := userimpl.ProvideVerifier(cfg, userSvc, tempUserService, nsMock) verifier := userimpl.ProvideVerifier(cfg, userSvc, tempUserService, nsMock, &idtest.MockService{})
hs := &HTTPServer{ hs := &HTTPServer{
Cfg: cfg, Cfg: cfg,
@ -620,7 +620,7 @@ func TestUser_UpdateEmail(t *testing.T) {
hs.tempUserService = tempUserSvc hs.tempUserService = tempUserSvc
hs.NotificationService = nsMock hs.NotificationService = nsMock
hs.SecretsService = fakes.NewFakeSecretsService() hs.SecretsService = fakes.NewFakeSecretsService()
hs.userVerifier = userimpl.ProvideVerifier(settings, userSvc, tempUserSvc, nsMock) hs.userVerifier = userimpl.ProvideVerifier(settings, userSvc, tempUserSvc, nsMock, &idtest.MockService{})
// User is internal // User is internal
hs.authInfoService = &authinfotest.FakeService{ExpectedError: user.ErrUserNotFound} hs.authInfoService = &authinfotest.FakeService{ExpectedError: user.ErrUserNotFound}
}) })

View File

@ -11,6 +11,9 @@ import (
type IDService interface { type IDService interface {
// SignIdentity signs a id token for provided identity that can be forwarded to plugins and external services // SignIdentity signs a id token for provided identity that can be forwarded to plugins and external services
SignIdentity(ctx context.Context, identity identity.Requester) (string, error) SignIdentity(ctx context.Context, identity identity.Requester) (string, error)
// RemoveIDToken removes any locally stored id tokens for key
RemoveIDToken(ctx context.Context, identity identity.Requester) error
} }
type IDSigner interface { type IDSigner interface {
@ -19,5 +22,7 @@ type IDSigner interface {
type IDClaims struct { type IDClaims struct {
jwt.Claims jwt.Claims
Email string `json:"email"`
EmailVerified bool `json:"email_verified"`
AuthenticatedBy string `json:"authenticatedBy,omitempty"` AuthenticatedBy string `json:"authenticatedBy,omitempty"`
} }

View File

@ -32,6 +32,8 @@ type Requester interface {
// GetEmail returns the email of the active entity. // GetEmail returns the email of the active entity.
// Can be empty. // Can be empty.
GetEmail() string GetEmail() string
// IsEmailVerified returns if email is verified for entity.
IsEmailVerified() bool
// GetIsGrafanaAdmin returns true if the user is a server admin // GetIsGrafanaAdmin returns true if the user is a server admin
GetIsGrafanaAdmin() bool GetIsGrafanaAdmin() bool
// GetLogin returns the login of the active entity // GetLogin returns the login of the active entity

View File

@ -90,7 +90,7 @@ func (s *Service) SignIdentity(ctx context.Context, id identity.Requester) (stri
} }
if identity.IsNamespace(namespace, identity.NamespaceUser) { if identity.IsNamespace(namespace, identity.NamespaceUser) {
if err := s.setUserClaims(ctx, identifier, claims); err != nil { if err := s.setUserClaims(ctx, id, identifier, claims); err != nil {
return "", err return "", err
} }
} }
@ -130,7 +130,11 @@ func (s *Service) SignIdentity(ctx context.Context, id identity.Requester) (stri
return result.(string), nil return result.(string), nil
} }
func (s *Service) setUserClaims(ctx context.Context, identifier string, claims *auth.IDClaims) error { func (s *Service) RemoveIDToken(ctx context.Context, id identity.Requester) error {
return s.cache.Delete(ctx, prefixCacheKey(id.GetCacheKey()))
}
func (s *Service) setUserClaims(ctx context.Context, ident identity.Requester, identifier string, claims *auth.IDClaims) error {
id, err := strconv.ParseInt(identifier, 10, 64) id, err := strconv.ParseInt(identifier, 10, 64)
if err != nil { if err != nil {
return err return err
@ -140,6 +144,9 @@ func (s *Service) setUserClaims(ctx context.Context, identifier string, claims *
return nil return nil
} }
claims.Email = ident.GetEmail()
claims.EmailVerified = ident.IsEmailVerified()
info, err := s.authInfoService.GetAuthInfo(ctx, &login.GetAuthInfoQuery{UserId: id}) info, err := s.authInfoService.GetAuthInfo(ctx, &login.GetAuthInfoQuery{UserId: id})
if err != nil { if err != nil {
// we ignore errors when a user don't have external user auth // we ignore errors when a user don't have external user auth

View File

@ -4,8 +4,30 @@ import (
"context" "context"
"github.com/grafana/grafana/pkg/services/auth" "github.com/grafana/grafana/pkg/services/auth"
"github.com/grafana/grafana/pkg/services/auth/identity"
) )
var _ auth.IDService = (*MockService)(nil)
type MockService struct {
SignIdentityFn func(ctx context.Context, identity identity.Requester) (string, error)
RemoveIDTokenFn func(ctx context.Context, identity identity.Requester) error
}
func (m *MockService) SignIdentity(ctx context.Context, identity identity.Requester) (string, error) {
if m.SignIdentityFn != nil {
return m.SignIdentityFn(ctx, identity)
}
return "", nil
}
func (m *MockService) RemoveIDToken(ctx context.Context, identity identity.Requester) error {
if m.RemoveIDTokenFn != nil {
return m.RemoveIDTokenFn(ctx, identity)
}
return nil
}
type MockSigner struct { type MockSigner struct {
SignIDTokenFn func(ctx context.Context, claims *auth.IDClaims) (string, error) SignIDTokenFn func(ctx context.Context, claims *auth.IDClaims) (string, error)
} }

View File

@ -407,4 +407,5 @@ func syncSignedInUserToIdentity(usr *user.SignedInUser, identity *authn.Identity
identity.LastSeenAt = usr.LastSeenAt identity.LastSeenAt = usr.LastSeenAt
identity.IsDisabled = usr.IsDisabled identity.IsDisabled = usr.IsDisabled
identity.IsGrafanaAdmin = &usr.IsGrafanaAdmin identity.IsGrafanaAdmin = &usr.IsGrafanaAdmin
identity.EmailVerified = usr.EmailVerified
} }

View File

@ -55,6 +55,8 @@ type Identity struct {
Name string Name string
// Email is the email address of the entity. Should be unique. // Email is the email address of the entity. Should be unique.
Email string Email string
// EmailVerified is true if entity has verified their email with grafana.
EmailVerified bool
// IsGrafanaAdmin is true if the entity is a Grafana admin. // IsGrafanaAdmin is true if the entity is a Grafana admin.
IsGrafanaAdmin *bool IsGrafanaAdmin *bool
// AuthenticatedBy is the name of the authentication client that was used to authenticate the current Identity. // AuthenticatedBy is the name of the authentication client that was used to authenticate the current Identity.
@ -123,6 +125,10 @@ func (i *Identity) GetEmail() string {
return i.Email return i.Email
} }
func (i *Identity) IsEmailVerified() bool {
return i.EmailVerified
}
func (i *Identity) GetIDToken() string { func (i *Identity) GetIDToken() string {
return i.IDToken return i.IDToken
} }

View File

@ -22,6 +22,7 @@ type SignedInUser struct {
Login string Login string
Name string Name string
Email string Email string
EmailVerified bool
AuthenticatedBy string AuthenticatedBy string
ApiKeyID int64 `xorm:"api_key_id"` ApiKeyID int64 `xorm:"api_key_id"`
IsServiceAccount bool `xorm:"is_service_account"` IsServiceAccount bool `xorm:"is_service_account"`
@ -241,6 +242,10 @@ func (u *SignedInUser) GetEmail() string {
return u.Email return u.Email
} }
func (u *SignedInUser) IsEmailVerified() bool {
return u.EmailVerified
}
// GetDisplayName returns the display name of the active entity // GetDisplayName returns the display name of the active entity
// The display name is the name if it is set, otherwise the login or email // The display name is the name if it is set, otherwise the login or email
func (u *SignedInUser) GetDisplayName() string { func (u *SignedInUser) GetDisplayName() string {

View File

@ -228,6 +228,7 @@ type StartVerifyEmailCommand struct {
} }
type CompleteEmailVerifyCommand struct { type CompleteEmailVerifyCommand struct {
User identity.Requester
Code string Code string
} }

View File

@ -382,6 +382,7 @@ func (ss *sqlStore) GetSignedInUser(ctx context.Context, query *user.GetSignedIn
u.uid as user_uid, u.uid as user_uid,
u.is_admin as is_grafana_admin, u.is_admin as is_grafana_admin,
u.email as email, u.email as email,
u.email_verified as email_verified,
u.login as login, u.login as login,
u.name as name, u.name as name,
u.is_disabled as is_disabled, u.is_disabled as is_disabled,

View File

@ -7,6 +7,8 @@ import (
"net/mail" "net/mail"
"time" "time"
"github.com/grafana/grafana/pkg/services/auth"
"github.com/grafana/grafana/pkg/services/authn"
"github.com/grafana/grafana/pkg/services/notifications" "github.com/grafana/grafana/pkg/services/notifications"
tempuser "github.com/grafana/grafana/pkg/services/temp_user" tempuser "github.com/grafana/grafana/pkg/services/temp_user"
"github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/services/user"
@ -22,8 +24,8 @@ var (
var _ user.Verifier = (*Verifier)(nil) var _ user.Verifier = (*Verifier)(nil)
func ProvideVerifier(cfg *setting.Cfg, us user.Service, ts tempuser.Service, ns notifications.Service) *Verifier { func ProvideVerifier(cfg *setting.Cfg, us user.Service, ts tempuser.Service, ns notifications.Service, is auth.IDService) *Verifier {
return &Verifier{cfg, us, ts, ns} return &Verifier{cfg, us, ts, ns, is}
} }
type Verifier struct { type Verifier struct {
@ -31,6 +33,7 @@ type Verifier struct {
us user.Service us user.Service
ts tempuser.Service ts tempuser.Service
ns notifications.Service ns notifications.Service
is auth.IDService
} }
func (s *Verifier) Start(ctx context.Context, cmd user.StartVerifyEmailCommand) error { func (s *Verifier) Start(ctx context.Context, cmd user.StartVerifyEmailCommand) error {
@ -145,5 +148,10 @@ func (s *Verifier) Complete(ctx context.Context, cmd user.CompleteEmailVerifyCom
return err return err
} }
return nil // We store email and email verified in id tokens. So whenever we perform and update / confirmation we need to
// remove the current token, so a new one can be generated with correct values.
return s.is.RemoveIDToken(
ctx,
&user.SignedInUser{UserID: usr.ID, OrgID: usr.OrgID, NamespacedID: authn.NamespacedID(authn.NamespaceUser, usr.ID)},
)
} }

View File

@ -7,6 +7,8 @@ import (
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/grafana/grafana/pkg/services/auth/identity"
"github.com/grafana/grafana/pkg/services/auth/idtest"
"github.com/grafana/grafana/pkg/services/notifications" "github.com/grafana/grafana/pkg/services/notifications"
tempuser "github.com/grafana/grafana/pkg/services/temp_user" tempuser "github.com/grafana/grafana/pkg/services/temp_user"
"github.com/grafana/grafana/pkg/services/temp_user/tempusertest" "github.com/grafana/grafana/pkg/services/temp_user/tempusertest"
@ -19,6 +21,7 @@ func TestVerifier_Start(t *testing.T) {
ts := &tempusertest.FakeTempUserService{} ts := &tempusertest.FakeTempUserService{}
us := &usertest.FakeUserService{} us := &usertest.FakeUserService{}
ns := notifications.MockNotificationService() ns := notifications.MockNotificationService()
is := &idtest.MockService{}
type calls struct { type calls struct {
expireCalled bool expireCalled bool
@ -26,7 +29,7 @@ func TestVerifier_Start(t *testing.T) {
updateCalled bool updateCalled bool
} }
verifier := ProvideVerifier(setting.NewCfg(), us, ts, ns) verifier := ProvideVerifier(setting.NewCfg(), us, ts, ns, is)
t.Run("should error if email already exist for other user", func(t *testing.T) { t.Run("should error if email already exist for other user", func(t *testing.T) {
us.ExpectedUser = &user.User{ID: 1} us.ExpectedUser = &user.User{ID: 1}
err := verifier.Start(context.Background(), user.StartVerifyEmailCommand{ err := verifier.Start(context.Background(), user.StartVerifyEmailCommand{
@ -113,15 +116,17 @@ func TestVerifier_Complete(t *testing.T) {
ts := &tempusertest.FakeTempUserService{} ts := &tempusertest.FakeTempUserService{}
us := &usertest.FakeUserService{} us := &usertest.FakeUserService{}
ns := notifications.MockNotificationService() ns := notifications.MockNotificationService()
is := &idtest.MockService{}
type calls struct { type calls struct {
updateCalled bool updateCalled bool
updateStatusCalled bool updateStatusCalled bool
removeTokenCalled bool
} }
cfg := setting.NewCfg() cfg := setting.NewCfg()
cfg.VerificationEmailMaxLifetime = 1 * time.Hour cfg.VerificationEmailMaxLifetime = 1 * time.Hour
verifier := ProvideVerifier(cfg, us, ts, ns) verifier := ProvideVerifier(cfg, us, ts, ns, is)
t.Run("should return error for invalid code", func(t *testing.T) { t.Run("should return error for invalid code", func(t *testing.T) {
ts.GetTempUserByCodeFN = func(ctx context.Context, query *tempuser.GetTempUserByCodeQuery) (*tempuser.TempUserDTO, error) { ts.GetTempUserByCodeFN = func(ctx context.Context, query *tempuser.GetTempUserByCodeQuery) (*tempuser.TempUserDTO, error) {
return nil, tempuser.ErrTempUserNotFound return nil, tempuser.ErrTempUserNotFound
@ -195,6 +200,11 @@ func TestVerifier_Complete(t *testing.T) {
return nil return nil
} }
is.RemoveIDTokenFn = func(ctx context.Context, identity identity.Requester) error {
c.removeTokenCalled = true
return nil
}
us.ExpectedUser = &user.User{Email: "initial@email.com"} us.ExpectedUser = &user.User{Email: "initial@email.com"}
us.ExpectedError = nil us.ExpectedError = nil
us.UpdateFn = func(ctx context.Context, cmd *user.UpdateUserCommand) error { us.UpdateFn = func(ctx context.Context, cmd *user.UpdateUserCommand) error {
@ -210,6 +220,7 @@ func TestVerifier_Complete(t *testing.T) {
assert.NoError(t, err) assert.NoError(t, err)
assert.True(t, c.updateCalled) assert.True(t, c.updateCalled)
assert.True(t, c.updateStatusCalled) assert.True(t, c.updateStatusCalled)
assert.True(t, c.removeTokenCalled)
}) })
t.Run("should update user email and login if login is an email on valid code", func(t *testing.T) { t.Run("should update user email and login if login is an email on valid code", func(t *testing.T) {
@ -230,6 +241,11 @@ func TestVerifier_Complete(t *testing.T) {
return nil return nil
} }
is.RemoveIDTokenFn = func(ctx context.Context, identity identity.Requester) error {
c.removeTokenCalled = true
return nil
}
us.ExpectedUser = &user.User{Email: "initial@email.com", Login: "other@email.com"} us.ExpectedUser = &user.User{Email: "initial@email.com", Login: "other@email.com"}
us.ExpectedError = nil us.ExpectedError = nil
us.UpdateFn = func(ctx context.Context, cmd *user.UpdateUserCommand) error { us.UpdateFn = func(ctx context.Context, cmd *user.UpdateUserCommand) error {
@ -245,5 +261,6 @@ func TestVerifier_Complete(t *testing.T) {
assert.NoError(t, err) assert.NoError(t, err)
assert.True(t, c.updateCalled) assert.True(t, c.updateCalled)
assert.True(t, c.updateStatusCalled) assert.True(t, c.updateStatusCalled)
assert.True(t, c.removeTokenCalled)
}) })
} }