diff --git a/app/email.go b/app/email.go index f2230d2b08..06e54558ed 100644 --- a/app/email.go +++ b/app/email.go @@ -759,6 +759,35 @@ func (es *EmailService) sendMailWithEmbeddedFiles(to, subject, htmlBody string, return mail.SendMailWithEmbeddedFilesUsingConfig(to, subject, htmlBody, embeddedFiles, mailConfig, license != nil && *license.Features.Compliance, "") } +func (es *EmailService) InvalidateVerifyEmailTokensForUser(userID string) *model.AppError { + tokens, err := es.srv.Store.Token().GetAllTokensByType(TokenTypeVerifyEmail) + if err != nil { + return model.NewAppError("InvalidateVerifyEmailTokensForUser", "api.user.invalidate_verify_email_tokens.error", nil, err.Error(), http.StatusInternalServerError) + } + + var appErr *model.AppError = nil + for _, token := range tokens { + tokenExtra := struct { + UserId string + Email string + }{} + if err := json.Unmarshal([]byte(token.Extra), &tokenExtra); err != nil { + appErr = model.NewAppError("InvalidateVerifyEmailTokensForUser", "api.user.invalidate_verify_email_tokens_parse.error", nil, err.Error(), http.StatusInternalServerError) + continue + } + + if tokenExtra.UserId != userID { + continue + } + + if err := es.srv.Store.Token().Delete(token.Token); err != nil { + appErr = model.NewAppError("InvalidateVerifyEmailTokensForUser", "api.user.invalidate_verify_email_tokens_delete.error", nil, err.Error(), http.StatusInternalServerError) + } + } + + return appErr +} + func (es *EmailService) CreateVerifyEmailToken(userID string, newEmail string) (*model.Token, *model.AppError) { tokenExtra := struct { UserId string @@ -775,6 +804,10 @@ func (es *EmailService) CreateVerifyEmailToken(userID string, newEmail string) ( token := model.NewToken(TokenTypeVerifyEmail, string(jsonData)) + if err := es.InvalidateVerifyEmailTokensForUser(userID); err != nil { + return nil, err + } + if err = es.srv.Store.Token().Save(token); err != nil { var appErr *model.AppError switch { diff --git a/app/user.go b/app/user.go index a733f65cb6..2fbc427f17 100644 --- a/app/user.go +++ b/app/user.go @@ -1301,7 +1301,6 @@ func (a *App) SendPasswordReset(email string, siteURL string) (bool, *model.AppE } func (a *App) CreatePasswordRecoveryToken(userID, email string) (*model.Token, *model.AppError) { - tokenExtra := struct { UserId string Email string diff --git a/app/user_test.go b/app/user_test.go index cf0665591f..cbf087583e 100644 --- a/app/user_test.go +++ b/app/user_test.go @@ -488,6 +488,48 @@ func TestUpdateUserEmail(t *testing.T) { assert.Equal(t, err.Id, "app.user.save.email_exists.app_error") assert.Nil(t, user3) }) + + t.Run("Only the last token works if verification is required", func(t *testing.T) { + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.EmailSettings.RequireEmailVerification = true + }) + + // we update the email a first time and update. The first + // token is sent with the email + user.Email = th.MakeEmail() + _, appErr := th.App.UpdateUser(user, true) + require.Nil(t, appErr) + + tokens := []*model.Token{} + require.Eventually(t, func() bool { + var err error + tokens, err = th.App.Srv().Store.Token().GetAllTokensByType(TokenTypeVerifyEmail) + return err == nil && len(tokens) == 1 + }, 100*time.Millisecond, 10*time.Millisecond) + + firstToken := tokens[0] + + // without using the first token, we update the email a second + // time and another token gets sent. The first one should not + // work anymore and the second should work properly + user.Email = th.MakeEmail() + _, appErr = th.App.UpdateUser(user, true) + require.Nil(t, appErr) + + require.Eventually(t, func() bool { + var err error + tokens, err = th.App.Srv().Store.Token().GetAllTokensByType(TokenTypeVerifyEmail) + return err == nil && len(tokens) == 1 + }, 100*time.Millisecond, 10*time.Millisecond) + secondToken := tokens[0] + + _, err := th.App.Srv().Store.Token().GetByToken(firstToken.Token) + require.Error(t, err) + + require.NotNil(t, th.App.VerifyEmailFromToken(firstToken.Token)) + require.Nil(t, th.App.VerifyEmailFromToken(secondToken.Token)) + require.NotNil(t, th.App.VerifyEmailFromToken(firstToken.Token)) + }) } func getUserFromDB(a *App, id string, t *testing.T) *model.User { diff --git a/i18n/en.json b/i18n/en.json index 1db27db311..a65a171728 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -3982,6 +3982,18 @@ "id": "api.user.get_user_by_email.permissions.app_error", "translation": "Unable to get user by email." }, + { + "id": "api.user.invalidate_verify_email_tokens.error", + "translation": "Unable to get tokens by type when invalidating email verification tokens" + }, + { + "id": "api.user.invalidate_verify_email_tokens_delete.error", + "translation": "Unable to remove token when invalidating email verification tokens" + }, + { + "id": "api.user.invalidate_verify_email_tokens_parse.error", + "translation": "Unable to parse token when invalidating email verification tokens" + }, { "id": "api.user.ldap_to_email.not_available.app_error", "translation": "AD/LDAP not available on this server." diff --git a/store/opentracinglayer/opentracinglayer.go b/store/opentracinglayer/opentracinglayer.go index 70731b9df0..843ae53841 100644 --- a/store/opentracinglayer/opentracinglayer.go +++ b/store/opentracinglayer/opentracinglayer.go @@ -9329,6 +9329,24 @@ func (s *OpenTracingLayerTokenStore) Delete(token string) error { return err } +func (s *OpenTracingLayerTokenStore) GetAllTokensByType(tokenType string) ([]*model.Token, error) { + origCtx := s.Root.Store.Context() + span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "TokenStore.GetAllTokensByType") + s.Root.Store.SetContext(newCtx) + defer func() { + s.Root.Store.SetContext(origCtx) + }() + + defer span.Finish() + result, err := s.TokenStore.GetAllTokensByType(tokenType) + if err != nil { + span.LogFields(spanlog.Error(err)) + ext.Error.Set(span, true) + } + + return result, err +} + func (s *OpenTracingLayerTokenStore) GetByToken(token string) (*model.Token, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "TokenStore.GetByToken") diff --git a/store/retrylayer/retrylayer.go b/store/retrylayer/retrylayer.go index e23b821fcf..15f6fceab2 100644 --- a/store/retrylayer/retrylayer.go +++ b/store/retrylayer/retrylayer.go @@ -10154,6 +10154,26 @@ func (s *RetryLayerTokenStore) Delete(token string) error { } +func (s *RetryLayerTokenStore) GetAllTokensByType(tokenType string) ([]*model.Token, error) { + + tries := 0 + for { + result, err := s.TokenStore.GetAllTokensByType(tokenType) + if err == nil { + return result, nil + } + if !isRepeatableError(err) { + return result, err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return result, err + } + } + +} + func (s *RetryLayerTokenStore) GetByToken(token string) (*model.Token, error) { tries := 0 diff --git a/store/sqlstore/tokens_store.go b/store/sqlstore/tokens_store.go index 80c40873c1..0d7ebdf1ac 100644 --- a/store/sqlstore/tokens_store.go +++ b/store/sqlstore/tokens_store.go @@ -7,6 +7,7 @@ import ( "database/sql" "fmt" + sq "github.com/Masterminds/squirrel" "github.com/pkg/errors" "github.com/mattermost/mattermost-server/v5/model" @@ -74,6 +75,19 @@ func (s SqlTokenStore) Cleanup() { } } +func (s SqlTokenStore) GetAllTokensByType(tokenType string) ([]*model.Token, error) { + tokens := []*model.Token{} + query, args, err := s.getQueryBuilder().Select("*").From("Tokens").Where(sq.Eq{"Type": tokenType}).ToSql() + if err != nil { + return nil, errors.Wrap(err, "could not build sql query to get all tokens by type") + } + + if _, err := s.GetReplica().Select(&tokens, query, args...); err != nil { + return nil, errors.Wrapf(err, "failed to get all tokens of Type=%s", tokenType) + } + return tokens, nil +} + func (s SqlTokenStore) RemoveAllTokensByType(tokenType string) error { if _, err := s.GetMaster().Exec("DELETE FROM Tokens WHERE Type = :TokenType", map[string]interface{}{"TokenType": tokenType}); err != nil { return errors.Wrapf(err, "failed to remove all Tokens with Type=%s", tokenType) diff --git a/store/store.go b/store/store.go index d6f53f7e21..83a487410a 100644 --- a/store/store.go +++ b/store/store.go @@ -595,6 +595,7 @@ type TokenStore interface { Delete(token string) error GetByToken(token string) (*model.Token, error) Cleanup() + GetAllTokensByType(tokenType string) ([]*model.Token, error) RemoveAllTokensByType(tokenType string) error } diff --git a/store/storetest/mocks/TokenStore.go b/store/storetest/mocks/TokenStore.go index e205e2157e..8bc2ce33ba 100644 --- a/store/storetest/mocks/TokenStore.go +++ b/store/storetest/mocks/TokenStore.go @@ -33,6 +33,29 @@ func (_m *TokenStore) Delete(token string) error { return r0 } +// GetAllTokensByType provides a mock function with given fields: tokenType +func (_m *TokenStore) GetAllTokensByType(tokenType string) ([]*model.Token, error) { + ret := _m.Called(tokenType) + + var r0 []*model.Token + if rf, ok := ret.Get(0).(func(string) []*model.Token); ok { + r0 = rf(tokenType) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]*model.Token) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(string) error); ok { + r1 = rf(tokenType) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // GetByToken provides a mock function with given fields: token func (_m *TokenStore) GetByToken(token string) (*model.Token, error) { ret := _m.Called(token) diff --git a/store/timerlayer/timerlayer.go b/store/timerlayer/timerlayer.go index 3bae3c6031..40cccb3038 100644 --- a/store/timerlayer/timerlayer.go +++ b/store/timerlayer/timerlayer.go @@ -8405,6 +8405,22 @@ func (s *TimerLayerTokenStore) Delete(token string) error { return err } +func (s *TimerLayerTokenStore) GetAllTokensByType(tokenType string) ([]*model.Token, error) { + start := timemodule.Now() + + result, err := s.TokenStore.GetAllTokensByType(tokenType) + + elapsed := float64(timemodule.Since(start)) / float64(timemodule.Second) + if s.Root.Metrics != nil { + success := "false" + if err == nil { + success = "true" + } + s.Root.Metrics.ObserveStoreMethodDuration("TokenStore.GetAllTokensByType", success, elapsed) + } + return result, err +} + func (s *TimerLayerTokenStore) GetByToken(token string) (*model.Token, error) { start := timemodule.Now()