Set EmailVerified to true for all remote users during migration (#594) (

#596) * set emailverified to true for all remote users during migration * change tests accordingly * prevent panic if remoteid is nil * add EmailVerified true to synthetic user created from getters
mattermost · Apr 12, 2024 · 9ed29fb · 9ed29fb
1 parent 3d33d70
commit 9ed29fb
Show file tree

Hide file tree

Showing 10 changed files with 35 additions and 42 deletions.
diff --git a/server/command.go b/server/command.go
@@ -597,7 +597,6 @@ func (p *Plugin) executePromoteUserCommand(args *model.CommandArgs, parameters [
 
 	user.RemoteId = nil
 	user.Username = newUsername
-	user.EmailVerified = true
 	_, appErr = p.API.UpdateUser(user)
 	if appErr != nil {
 		p.sendBotEphemeralPost(args.UserId, args.ChannelId, "Error: Unable to promote account "+username)

diff --git a/server/command_test.go b/server/command_test.go
@@ -1382,7 +1382,7 @@ func TestExecutePromoteCommand(t *testing.T) {
 				api.On("HasPermissionTo", testutils.GetUserID(), model.PermissionManageSystem).Return(true).Times(1)
 				api.On("GetUserByUsername", "valid-user").Return(&model.User{Id: "test", Username: "valid-user", RemoteId: model.NewString("remote-id")}, nil).Once()
 				api.On("GetUserByUsername", "new-user").Return(nil, &model.AppError{}).Once()
-				api.On("UpdateUser", &model.User{Id: "test", Username: "new-user", RemoteId: nil, EmailVerified: true}).Return(&model.User{Id: "test", Username: "new-user", RemoteId: nil, EmailVerified: true}, nil).Once()
+				api.On("UpdateUser", &model.User{Id: "test", Username: "new-user", RemoteId: nil}).Return(&model.User{Id: "test", Username: "new-user", RemoteId: nil}, nil).Once()
 				api.On("SendEphemeralPost", testutils.GetUserID(), testutils.GetEphemeralPost(p.userID, testutils.GetChannelID(), "Account valid-user has been promoted and updated the username to new-user")).Return(testutils.GetPost(testutils.GetChannelID(), testutils.GetUserID(), time.Now().UnixMicro())).Once()
 			},
 			setupStore: func(s *mockStore.Store) {
@@ -1396,7 +1396,7 @@ func TestExecutePromoteCommand(t *testing.T) {
 				api.On("HasPermissionTo", testutils.GetUserID(), model.PermissionManageSystem).Return(true).Times(1)
 				api.On("GetUserByUsername", "valid-user").Return(&model.User{Id: "test", Username: "valid-user", RemoteId: model.NewString("remote-id")}, nil).Once()
 				api.On("GetUserByUsername", "new-user").Return(nil, &model.AppError{}).Once()
-				api.On("UpdateUser", &model.User{Id: "test", Username: "new-user", RemoteId: nil, EmailVerified: true}).Return(&model.User{Id: "test", Username: "new-user", RemoteId: nil}, nil).Once()
+				api.On("UpdateUser", &model.User{Id: "test", Username: "new-user", RemoteId: nil}).Return(&model.User{Id: "test", Username: "new-user", RemoteId: nil}, nil).Once()
 				api.On("SendEphemeralPost", testutils.GetUserID(), testutils.GetEphemeralPost(p.userID, testutils.GetChannelID(), "Account valid-user has been promoted and updated the username to new-user")).Return(testutils.GetPost(testutils.GetChannelID(), testutils.GetUserID(), time.Now().UnixMicro())).Once()
 			},
 			setupStore: func(s *mockStore.Store) {
@@ -1409,7 +1409,7 @@ func TestExecutePromoteCommand(t *testing.T) {
 			setupAPI: func(api *plugintest.API) {
 				api.On("HasPermissionTo", testutils.GetUserID(), model.PermissionManageSystem).Return(true).Times(1)
 				api.On("GetUserByUsername", "valid-user").Return(&model.User{Id: "test", Username: "valid-user", RemoteId: model.NewString("remote-id")}, nil).Times(2)
-				api.On("UpdateUser", &model.User{Id: "test", Username: "valid-user", RemoteId: nil, EmailVerified: true}).Return(&model.User{Id: "test", Username: "valid-user", RemoteId: nil, EmailVerified: true}, nil).Times(1)
+				api.On("UpdateUser", &model.User{Id: "test", Username: "valid-user", RemoteId: nil}).Return(&model.User{Id: "test", Username: "valid-user", RemoteId: nil}, nil).Times(1)
 				api.On("SendEphemeralPost", testutils.GetUserID(), testutils.GetEphemeralPost(p.userID, testutils.GetChannelID(), "Account valid-user has been promoted and updated the username to valid-user")).Return(testutils.GetPost(testutils.GetChannelID(), testutils.GetUserID(), time.Now().UnixMicro())).Once()
 			},
 			setupStore: func(s *mockStore.Store) {
@@ -1422,7 +1422,7 @@ func TestExecutePromoteCommand(t *testing.T) {
 			setupAPI: func(api *plugintest.API) {
 				api.On("HasPermissionTo", testutils.GetUserID(), model.PermissionManageSystem).Return(true).Times(1)
 				api.On("GetUserByUsername", "valid-user").Return(&model.User{Id: "test", Username: "valid-user", RemoteId: model.NewString("remote-id")}, nil).Times(2)
-				api.On("UpdateUser", &model.User{Id: "test", Username: "valid-user", RemoteId: nil, EmailVerified: true}).Return(&model.User{Id: "test", Username: "valid-user", RemoteId: nil}, nil).Times(1)
+				api.On("UpdateUser", &model.User{Id: "test", Username: "valid-user", RemoteId: nil}).Return(&model.User{Id: "test", Username: "valid-user", RemoteId: nil}, nil).Times(1)
 				api.On("SendEphemeralPost", testutils.GetUserID(), testutils.GetEphemeralPost(p.userID, testutils.GetChannelID(), "Account valid-user has been promoted and updated the username to valid-user")).Return(testutils.GetPost(testutils.GetChannelID(), testutils.GetUserID(), time.Now().UnixMicro())).Once()
 			},
 			setupStore: func(s *mockStore.Store) {

diff --git a/server/handlers/getters.go b/server/handlers/getters.go
@@ -115,11 +115,12 @@ func (ah *ActivityHandler) getOrCreateSyntheticUser(user *clientmodels.User, cre
 		username := "msteams_" + slug.Make(userDisplayName)
 
 		newMMUser := &model.User{
-			Username:  username,
-			FirstName: userDisplayName,
-			Email:     user.Mail,
-			Password:  ah.plugin.GenerateRandomPassword(),
-			RemoteId:  &shortUserID,
+			Username:      username,
+			FirstName:     userDisplayName,
+			Email:         user.Mail,
+			Password:      ah.plugin.GenerateRandomPassword(),
+			RemoteId:      &shortUserID,
+			EmailVerified: true,
 		}
 		newMMUser.SetDefaultNotifications()
 		newMMUser.NotifyProps[model.EmailNotifyProp] = "false"

diff --git a/server/handlers/getters_test.go b/server/handlers/getters_test.go
@@ -120,6 +120,9 @@ func TestGetOrCreateSyntheticUser(t *testing.T) {
 					if user.Email != testutils.GetTestEmail() {
 						return false
 					}
+					if !user.EmailVerified {
+						return false
+					}
 					return true
 				})).Return(&model.User{Id: "new-user-id"}, nil).Times(1)
 				api.On("UpdatePreferencesForUser", "new-user-id", mock.MatchedBy(func(preferences model.Preferences) bool {

diff --git a/server/message_hooks.go b/server/message_hooks.go
@@ -26,7 +26,6 @@ import (
 func (p *Plugin) UserWillLogIn(_ *plugin.Context, user *model.User) string {
 	if p.IsRemoteUser(user) && p.getConfiguration().AutomaticallyPromoteSyntheticUsers {
 		*user.RemoteId = ""
-		user.EmailVerified = true
 		if _, appErr := p.API.UpdateUser(user); appErr != nil {
 			p.API.LogWarn("Unable to promote synthetic user", "user_id", user.Id, "error", appErr.Error())
 			return "Unable to promote synthetic user"

diff --git a/server/message_hooks_test.go b/server/message_hooks_test.go
@@ -2819,22 +2819,13 @@ func TestUserWillLogin(t *testing.T) {
 		{
 			Name: "Autopromotion works",
 			User: &model.User{
-				Id:            testutils.GetID(),
-				EmailVerified: false,
+				Id: testutils.GetID(),
 			},
 			UserIsRemote:                       true,
 			AutomaticallyPromoteSyntheticUsers: true,
 			SetupAPI: func(api *plugintest.API) {
 				api.On("UpdateUser", mock.MatchedBy(func(user *model.User) bool {
-					if user.EmailVerified == false {
-						return false
-					}
-
-					if *user.RemoteId != "" {
-						return false
-					}
-
-					return true
+					return !user.IsRemote()
 				})).Once().Return(nil, nil)
 				api.On("LogInfo", "Promoted synthetic user", "user_id", testutils.GetID()).Once()
 			},
@@ -2847,22 +2838,13 @@ func TestUserWillLogin(t *testing.T) {
 		{
 			Name: "UpdateUser failed during autopromotion",
 			User: &model.User{
-				Id:            testutils.GetID(),
-				EmailVerified: false,
+				Id: testutils.GetID(),
 			},
 			UserIsRemote:                       true,
 			AutomaticallyPromoteSyntheticUsers: true,
 			SetupAPI: func(api *plugintest.API) {
 				api.On("UpdateUser", mock.MatchedBy(func(user *model.User) bool {
-					if user.EmailVerified == false {
-						return false
-					}
-
-					if *user.RemoteId != "" {
-						return false
-					}
-
-					return true
+					return !user.IsRemote()
 				})).Once().Return(nil, model.NewAppError("UpdateUser", "err from test", nil, "", 500))
 				api.On("LogWarn", "Failed to promote synthetic user", "user_id", testutils.GetID(), "err", "err from test").Once()
 			},
@@ -2873,8 +2855,7 @@ func TestUserWillLogin(t *testing.T) {
 		{
 			Name: "No autopromotion",
 			User: &model.User{
-				Id:            testutils.GetID(),
-				EmailVerified: false,
+				Id: testutils.GetID(),
 			},
 			UserIsRemote:                       true,
 			AutomaticallyPromoteSyntheticUsers: false,

diff --git a/server/plugin.go b/server/plugin.go
@@ -737,11 +737,6 @@ func (p *Plugin) syncUsers() {
 					shouldUpdate = true
 				}
 
-				if !mmUser.EmailVerified {
-					mmUser.EmailVerified = true
-					shouldUpdate = true
-				}
-
 				if shouldUpdate {
 					for {
 						p.API.LogInfo("Updating user profile", "user_id", mmUser.Id, "teams_user_id", msUser.ID)

diff --git a/server/plugin_test.go b/server/plugin_test.go
@@ -372,8 +372,7 @@ func TestSyncUsers(t *testing.T) {
 				}, nil).Times(1)
 				api.On("GetUser", testutils.GetUserID()).Return(testutils.GetRemoteUser(model.SystemAdminRoleId, "[email protected]", "remote-id"), nil).Once()
 				api.On("UpdateUser", mock.MatchedBy(func(u *model.User) bool {
-					return u.EmailVerified == true &&
-						u.FirstName == "mockDisplayName" &&
+					return u.FirstName == "mockDisplayName" &&
 						u.Username == "msteams_mockdisplayname"
 				})).Return(&model.User{
 					Id: testutils.GetID(),

diff --git a/server/store/sqlstore/migrations.go b/server/store/sqlstore/migrations.go
@@ -13,3 +13,15 @@ func (s *SQLStore) runMigrationRemoteID(remoteID string) error {
 	}).Exec()
 	return err
 }
+
+func (s *SQLStore) runSetEmailVerifiedToTrueForRemoteUsers(remoteID string) error {
+	_, err := s.getQueryBuilder().
+		Update("Users").
+		Set("EmailVerified", true).
+		Where(sq.And{
+			sq.Eq{"RemoteID": remoteID},
+			sq.Eq{"EmailVerified": false},
+		}).Exec()
+
+	return err
+}
diff --git a/server/store/sqlstore/store.go b/server/store/sqlstore/store.go
@@ -165,6 +165,10 @@ func (s *SQLStore) Init(remoteID string) error {
 		if err := s.runMigrationRemoteID(remoteID); err != nil {
 			return err
 		}
+
+		if err := s.runSetEmailVerifiedToTrueForRemoteUsers(remoteID); err != nil {
+			return err
+		}
 	}
 	return nil
 }