[v11.0.x] User: email verification completion (#86354)
User: email verification completion (#85259)
* TempUser: Include InvitedById in TempUserDTO
* Extract email verfication completion flow to service
(cherry picked from commit 73e426b081)
Co-authored-by: Karl Persson <kalle.persson@grafana.com>
This commit is contained in:
co-authored by
Karl Persson
parent
23a24bc9af
commit
64c17b0ee6
@@ -315,7 +315,14 @@ func (ss *sqlStore) Update(ctx context.Context, cmd *user.UpdateUserCommand) err
|
||||
Updated: time.Now(),
|
||||
}
|
||||
|
||||
if _, err := sess.ID(cmd.UserID).Where(ss.notServiceAccountFilter()).Update(&user); err != nil {
|
||||
q := sess.ID(cmd.UserID).Where(ss.notServiceAccountFilter())
|
||||
|
||||
if cmd.EmailVerified != nil {
|
||||
q.UseBool("email_verified")
|
||||
user.EmailVerified = *cmd.EmailVerified
|
||||
}
|
||||
|
||||
if _, err := q.Update(&user); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
|
||||
@@ -4,26 +4,36 @@ import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/mail"
|
||||
"time"
|
||||
|
||||
"github.com/grafana/grafana/pkg/services/notifications"
|
||||
tempuser "github.com/grafana/grafana/pkg/services/temp_user"
|
||||
"github.com/grafana/grafana/pkg/services/user"
|
||||
"github.com/grafana/grafana/pkg/setting"
|
||||
"github.com/grafana/grafana/pkg/util"
|
||||
"github.com/grafana/grafana/pkg/util/errutil"
|
||||
)
|
||||
|
||||
var (
|
||||
errInvalidCode = errutil.BadRequest("user.code.invalid", errutil.WithPublicMessage("Invalid verification code"))
|
||||
errExpiredCode = errutil.BadRequest("user.code.expired", errutil.WithPublicMessage("Verification code has expired"))
|
||||
)
|
||||
|
||||
var _ user.Verifier = (*Verifier)(nil)
|
||||
|
||||
func ProvideVerifier(us user.Service, ts tempuser.Service, ns notifications.Service) *Verifier {
|
||||
return &Verifier{us, ts, ns}
|
||||
func ProvideVerifier(cfg *setting.Cfg, us user.Service, ts tempuser.Service, ns notifications.Service) *Verifier {
|
||||
return &Verifier{cfg, us, ts, ns}
|
||||
}
|
||||
|
||||
type Verifier struct {
|
||||
us user.Service
|
||||
ts tempuser.Service
|
||||
ns notifications.Service
|
||||
cfg *setting.Cfg
|
||||
us user.Service
|
||||
ts tempuser.Service
|
||||
ns notifications.Service
|
||||
}
|
||||
|
||||
func (s *Verifier) VerifyEmail(ctx context.Context, cmd user.VerifyEmailCommand) error {
|
||||
func (s *Verifier) Start(ctx context.Context, cmd user.StartVerifyEmailCommand) error {
|
||||
usr, err := s.us.GetByLogin(ctx, &user.GetUserByLoginQuery{
|
||||
LoginOrEmail: cmd.Email,
|
||||
})
|
||||
@@ -80,3 +90,60 @@ func (s *Verifier) VerifyEmail(ctx context.Context, cmd user.VerifyEmailCommand)
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
func (s *Verifier) Complete(ctx context.Context, cmd user.CompleteEmailVerifyCommand) error {
|
||||
tmpUsr, err := s.ts.GetTempUserByCode(ctx, &tempuser.GetTempUserByCodeQuery{Code: cmd.Code})
|
||||
if err != nil {
|
||||
return errInvalidCode.Errorf("failed to verify code: %w", err)
|
||||
}
|
||||
|
||||
if tmpUsr.Status != tempuser.TmpUserEmailUpdateStarted {
|
||||
return errInvalidCode.Errorf("wrong status for verification code: %s", tmpUsr.Status)
|
||||
}
|
||||
|
||||
if !tmpUsr.EmailSent {
|
||||
return errInvalidCode.Errorf("email was not marked as sent")
|
||||
}
|
||||
|
||||
if tmpUsr.EmailSentOn.Add(s.cfg.VerificationEmailMaxLifetime).Before(time.Now()) {
|
||||
return errExpiredCode.Errorf("verification code has expired")
|
||||
}
|
||||
|
||||
usr, err := s.us.GetByID(ctx, &user.GetUserByIDQuery{ID: tmpUsr.InvitedByID})
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
verified := true
|
||||
update := &user.UpdateUserCommand{
|
||||
Email: tmpUsr.Email,
|
||||
UserID: tmpUsr.InvitedByID,
|
||||
EmailVerified: &verified,
|
||||
}
|
||||
switch tmpUsr.Name {
|
||||
case string(user.EmailUpdateAction):
|
||||
// User updated the email field
|
||||
if _, err := mail.ParseAddress(usr.Login); err == nil {
|
||||
// If username was also an email, we update it to keep it in sync with the email field
|
||||
update.Login = tmpUsr.Email
|
||||
}
|
||||
case string(user.LoginUpdateAction):
|
||||
// User updated the username field with a new email
|
||||
update.Login = tmpUsr.Email
|
||||
default:
|
||||
return errors.New("trying to update email on unknown field")
|
||||
}
|
||||
|
||||
if err := s.us.Update(ctx, update); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
if err := s.ts.UpdateTempUserStatus(
|
||||
ctx,
|
||||
&tempuser.UpdateTempUserStatusCommand{Code: cmd.Code, Status: tempuser.TmpUserEmailUpdateCompleted},
|
||||
); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -3,6 +3,7 @@ package userimpl
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
|
||||
@@ -11,9 +12,10 @@ import (
|
||||
"github.com/grafana/grafana/pkg/services/temp_user/tempusertest"
|
||||
"github.com/grafana/grafana/pkg/services/user"
|
||||
"github.com/grafana/grafana/pkg/services/user/usertest"
|
||||
"github.com/grafana/grafana/pkg/setting"
|
||||
)
|
||||
|
||||
func TestVerifier_VerifyEmail(t *testing.T) {
|
||||
func TestVerifier_Start(t *testing.T) {
|
||||
ts := &tempusertest.FakeTempUserService{}
|
||||
us := &usertest.FakeUserService{}
|
||||
ns := notifications.MockNotificationService()
|
||||
@@ -24,10 +26,10 @@ func TestVerifier_VerifyEmail(t *testing.T) {
|
||||
updateCalled bool
|
||||
}
|
||||
|
||||
verifier := ProvideVerifier(us, ts, ns)
|
||||
verifier := ProvideVerifier(setting.NewCfg(), us, ts, ns)
|
||||
t.Run("should error if email already exist for other user", func(t *testing.T) {
|
||||
us.ExpectedUser = &user.User{ID: 1}
|
||||
err := verifier.VerifyEmail(context.Background(), user.VerifyEmailCommand{
|
||||
err := verifier.Start(context.Background(), user.StartVerifyEmailCommand{
|
||||
User: user.User{ID: 2},
|
||||
Email: "some@email.com",
|
||||
Action: user.EmailUpdateAction,
|
||||
@@ -59,13 +61,13 @@ func TestVerifier_VerifyEmail(t *testing.T) {
|
||||
c.updateCalled = true
|
||||
return nil
|
||||
}
|
||||
err := verifier.VerifyEmail(context.Background(), user.VerifyEmailCommand{
|
||||
err := verifier.Start(context.Background(), user.StartVerifyEmailCommand{
|
||||
User: user.User{ID: 2},
|
||||
Email: "some@email.com",
|
||||
Action: user.EmailUpdateAction,
|
||||
})
|
||||
|
||||
assert.ErrorIs(t, err, nil)
|
||||
assert.NoError(t, err)
|
||||
assert.True(t, c.expireCalled)
|
||||
assert.True(t, c.createCalled)
|
||||
assert.True(t, c.updateCalled)
|
||||
@@ -94,7 +96,7 @@ func TestVerifier_VerifyEmail(t *testing.T) {
|
||||
c.updateCalled = true
|
||||
return nil
|
||||
}
|
||||
err := verifier.VerifyEmail(context.Background(), user.VerifyEmailCommand{
|
||||
err := verifier.Start(context.Background(), user.StartVerifyEmailCommand{
|
||||
User: user.User{ID: 2},
|
||||
Email: "some@email.com",
|
||||
Action: user.EmailUpdateAction,
|
||||
@@ -106,3 +108,142 @@ func TestVerifier_VerifyEmail(t *testing.T) {
|
||||
assert.True(t, c.updateCalled)
|
||||
})
|
||||
}
|
||||
|
||||
func TestVerifier_Complete(t *testing.T) {
|
||||
ts := &tempusertest.FakeTempUserService{}
|
||||
us := &usertest.FakeUserService{}
|
||||
ns := notifications.MockNotificationService()
|
||||
|
||||
type calls struct {
|
||||
updateCalled bool
|
||||
updateStatusCalled bool
|
||||
}
|
||||
|
||||
cfg := setting.NewCfg()
|
||||
cfg.VerificationEmailMaxLifetime = 1 * time.Hour
|
||||
verifier := ProvideVerifier(cfg, us, ts, ns)
|
||||
t.Run("should return error for invalid code", func(t *testing.T) {
|
||||
ts.GetTempUserByCodeFN = func(ctx context.Context, query *tempuser.GetTempUserByCodeQuery) (*tempuser.TempUserDTO, error) {
|
||||
return nil, tempuser.ErrTempUserNotFound
|
||||
}
|
||||
err := verifier.Complete(context.Background(), user.CompleteEmailVerifyCommand{Code: "some-code"})
|
||||
assert.ErrorIs(t, err, errInvalidCode)
|
||||
})
|
||||
|
||||
t.Run("should return error when verification has wrong status", func(t *testing.T) {
|
||||
ts.GetTempUserByCodeFN = func(ctx context.Context, query *tempuser.GetTempUserByCodeQuery) (*tempuser.TempUserDTO, error) {
|
||||
return &tempuser.TempUserDTO{
|
||||
Status: tempuser.TmpUserEmailUpdateCompleted,
|
||||
}, nil
|
||||
}
|
||||
err := verifier.Complete(context.Background(), user.CompleteEmailVerifyCommand{Code: "some-code"})
|
||||
assert.ErrorIs(t, err, errInvalidCode)
|
||||
})
|
||||
|
||||
t.Run("should return error when verification email was never sent", func(t *testing.T) {
|
||||
ts.GetTempUserByCodeFN = func(ctx context.Context, query *tempuser.GetTempUserByCodeQuery) (*tempuser.TempUserDTO, error) {
|
||||
return &tempuser.TempUserDTO{
|
||||
Status: tempuser.TmpUserEmailUpdateStarted,
|
||||
EmailSent: false,
|
||||
}, nil
|
||||
}
|
||||
err := verifier.Complete(context.Background(), user.CompleteEmailVerifyCommand{Code: "some-code"})
|
||||
assert.ErrorIs(t, err, errInvalidCode)
|
||||
})
|
||||
|
||||
t.Run("should return error when verification code has expired", func(t *testing.T) {
|
||||
ts.GetTempUserByCodeFN = func(ctx context.Context, query *tempuser.GetTempUserByCodeQuery) (*tempuser.TempUserDTO, error) {
|
||||
return &tempuser.TempUserDTO{
|
||||
Status: tempuser.TmpUserEmailUpdateStarted,
|
||||
EmailSent: true,
|
||||
EmailSentOn: time.Now().Add(-10 * time.Hour),
|
||||
}, nil
|
||||
}
|
||||
err := verifier.Complete(context.Background(), user.CompleteEmailVerifyCommand{Code: "some-code"})
|
||||
assert.ErrorIs(t, err, errExpiredCode)
|
||||
})
|
||||
|
||||
t.Run("should return error user connect to code don't exists", func(t *testing.T) {
|
||||
ts.GetTempUserByCodeFN = func(ctx context.Context, query *tempuser.GetTempUserByCodeQuery) (*tempuser.TempUserDTO, error) {
|
||||
return &tempuser.TempUserDTO{
|
||||
Status: tempuser.TmpUserEmailUpdateStarted,
|
||||
EmailSent: true,
|
||||
EmailSentOn: time.Now(),
|
||||
}, nil
|
||||
}
|
||||
us.ExpectedError = user.ErrUserNotFound
|
||||
|
||||
err := verifier.Complete(context.Background(), user.CompleteEmailVerifyCommand{Code: "some-code"})
|
||||
assert.ErrorIs(t, err, user.ErrUserNotFound)
|
||||
})
|
||||
|
||||
t.Run("should update user email on valid code", func(t *testing.T) {
|
||||
var c calls
|
||||
ts.GetTempUserByCodeFN = func(ctx context.Context, query *tempuser.GetTempUserByCodeQuery) (*tempuser.TempUserDTO, error) {
|
||||
return &tempuser.TempUserDTO{
|
||||
Status: tempuser.TmpUserEmailUpdateStarted,
|
||||
Name: string(user.EmailUpdateAction),
|
||||
InvitedByID: 1,
|
||||
Email: "updated@email.com",
|
||||
EmailSent: true,
|
||||
EmailSentOn: time.Now(),
|
||||
}, nil
|
||||
}
|
||||
|
||||
ts.UpdateTempUserStatusFN = func(ctx context.Context, cmd *tempuser.UpdateTempUserStatusCommand) error {
|
||||
c.updateStatusCalled = true
|
||||
return nil
|
||||
}
|
||||
|
||||
us.ExpectedUser = &user.User{Email: "initial@email.com"}
|
||||
us.ExpectedError = nil
|
||||
us.UpdateFn = func(ctx context.Context, cmd *user.UpdateUserCommand) error {
|
||||
c.updateCalled = true
|
||||
assert.True(t, *cmd.EmailVerified)
|
||||
assert.Equal(t, int64(1), cmd.UserID)
|
||||
assert.Equal(t, "", cmd.Login)
|
||||
assert.Equal(t, "updated@email.com", cmd.Email)
|
||||
return nil
|
||||
}
|
||||
|
||||
err := verifier.Complete(context.Background(), user.CompleteEmailVerifyCommand{Code: "some-code"})
|
||||
assert.NoError(t, err)
|
||||
assert.True(t, c.updateCalled)
|
||||
assert.True(t, c.updateStatusCalled)
|
||||
})
|
||||
|
||||
t.Run("should update user email and login if login is an email on valid code", func(t *testing.T) {
|
||||
var c calls
|
||||
ts.GetTempUserByCodeFN = func(ctx context.Context, query *tempuser.GetTempUserByCodeQuery) (*tempuser.TempUserDTO, error) {
|
||||
return &tempuser.TempUserDTO{
|
||||
Status: tempuser.TmpUserEmailUpdateStarted,
|
||||
Name: string(user.EmailUpdateAction),
|
||||
InvitedByID: 1,
|
||||
Email: "updated@email.com",
|
||||
EmailSent: true,
|
||||
EmailSentOn: time.Now(),
|
||||
}, nil
|
||||
}
|
||||
|
||||
ts.UpdateTempUserStatusFN = func(ctx context.Context, cmd *tempuser.UpdateTempUserStatusCommand) error {
|
||||
c.updateStatusCalled = true
|
||||
return nil
|
||||
}
|
||||
|
||||
us.ExpectedUser = &user.User{Email: "initial@email.com", Login: "other@email.com"}
|
||||
us.ExpectedError = nil
|
||||
us.UpdateFn = func(ctx context.Context, cmd *user.UpdateUserCommand) error {
|
||||
c.updateCalled = true
|
||||
assert.True(t, *cmd.EmailVerified)
|
||||
assert.Equal(t, int64(1), cmd.UserID)
|
||||
assert.Equal(t, "updated@email.com", cmd.Email)
|
||||
assert.Equal(t, "updated@email.com", cmd.Login)
|
||||
return nil
|
||||
}
|
||||
|
||||
err := verifier.Complete(context.Background(), user.CompleteEmailVerifyCommand{Code: "some-code"})
|
||||
assert.NoError(t, err)
|
||||
assert.True(t, c.updateCalled)
|
||||
assert.True(t, c.updateStatusCalled)
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user