[MM-33395] Invalidate email tokens (#17069)

* [MM-33395] Invalidate existing verify email tokens when creating a new one

* Update store layers

* Addressing review comments

* Fix linter

Co-authored-by: Mattermod <mattermod@users.noreply.github.com>

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 {

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

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 {

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."

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")

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

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)

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
 }
 

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)

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()