MM-66757: Improve WebSocket user update events (#34600) (#34856)

* improve TestUserUpdateEvents

* improve CheckUserSanitization

* check user sanitization in TestUserUpdateEvents

* minimally sanitize user sent to event creator

server/channels/api4/apitestlib.go

@@ -1160,9 +1160,9 @@ func GenerateTestID() string {
 func CheckUserSanitization(tb testing.TB, user *model.User) {
 	tb.Helper()
 
-	require.Equal(tb, "", user.Password, "password wasn't blank")
+	require.Empty(tb, user.Password, "password wasn't blank")
 	require.Empty(tb, user.AuthData, "auth data wasn't blank")
-	require.Equal(tb, "", user.MfaSecret, "mfa secret wasn't blank")
+	require.Empty(tb, user.MfaSecret, "mfa secret wasn't blank")
 }
 
 func CheckEtag(tb testing.TB, data any, resp *model.Response) {

server/channels/api4/user_test.go

@@ -8564,40 +8564,118 @@ func TestUserUpdateEvents(t *testing.T) {
 
 	client1 := th.CreateClient()
 	th.LoginBasicWithClient(client1)
-	WebSocketClient := th.CreateConnectedWebSocketClientWithClient(t, client1)
+	wsClient1 := th.CreateConnectedWebSocketClientWithClient(t, client1)
-	resp := <-WebSocketClient.ResponseChannel
-	require.Equal(t, resp.Status, model.StatusOk)
 
 	client2 := th.CreateClient()
 	th.LoginBasic2WithClient(client2)
-	WebSocketClient2 := th.CreateConnectedWebSocketClientWithClient(t, client2)
+	wsClient2 := th.CreateConnectedWebSocketClientWithClient(t, client2)
-	resp = <-WebSocketClient2.ResponseChannel
-	require.Equal(t, resp.Status, model.StatusOk)
 
-	time.Sleep(1000 * time.Millisecond)
+	t.Run("nickname", func(t *testing.T) {
+		assertUpdated := func(t *testing.T, event *model.WebSocketEvent, expectedNickname string) *model.User {
+			eventUser, ok := event.GetData()["user"].(*model.User)
+			require.True(t, ok, "expected user")
+			assert.Equal(t, th.BasicUser.Id, eventUser.Id)
+			assert.Equal(t, expectedNickname, eventUser.Nickname)
 
+			// Some fields must always be sanitized
+			CheckUserSanitization(t, eventUser)
+
+			return eventUser
+		}
+
+		t.Run("update", func(t *testing.T) {
 			th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) {
-		// trigger user update for onlineUser2
+				newNickname := model.NewUsername()
-		th.BasicUser.Nickname = "something_else"
+				th.BasicUser.Nickname = newNickname
-		ruser, _, err := client1.UpdateUser(context.Background(), th.BasicUser)
+				_, _, err := client1.UpdateUser(context.Background(), th.BasicUser)
 				require.NoError(t, err)
-		CheckUserSanitization(t, ruser)
 
-		assertExpectedWebsocketEvent(t, WebSocketClient, model.WebsocketEventUserUpdated, func(event *model.WebSocketEvent) {
+				assertExpectedWebsocketEvent(t, wsClient1, model.WebsocketEventUserUpdated, func(event *model.WebSocketEvent) {
-			eventUser, ok := event.GetData()["user"].(*model.User)
+					eventUser := assertUpdated(t, event, newNickname)
-			require.True(t, ok, "expected user")
+					assert.NotEmpty(t, eventUser.NotifyProps, "source user should keep notify_props")
-			// assert eventUser.Id is same as th.BasicUser.Id
-			assert.Equal(t, eventUser.Id, th.BasicUser.Id)
-			// assert eventUser.NotifyProps isn't empty
-			require.NotEmpty(t, eventUser.NotifyProps, "user event for source user should not be sanitized")
 				})
-		assertExpectedWebsocketEvent(t, WebSocketClient2, model.WebsocketEventUserUpdated, func(event *model.WebSocketEvent) {
+
+				assertExpectedWebsocketEvent(t, wsClient2, model.WebsocketEventUserUpdated, func(event *model.WebSocketEvent) {
+					eventUser := assertUpdated(t, event, newNickname)
+					assert.Empty(t, eventUser.NotifyProps, "non-source users should have sanitized notify_props")
+				})
+			})
+		})
+
+		t.Run("patch", func(t *testing.T) {
+			th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) {
+				newNickname := model.NewUsername()
+
+				_, _, err := client1.PatchUser(context.Background(), th.BasicUser.Id, &model.UserPatch{
+					Nickname: &newNickname,
+				})
+				require.NoError(t, err)
+
+				assertExpectedWebsocketEvent(t, wsClient1, model.WebsocketEventUserUpdated, func(event *model.WebSocketEvent) {
+					eventUser := assertUpdated(t, event, newNickname)
+					assert.NotEmpty(t, eventUser.NotifyProps, "source user should keep notify_props")
+				})
+
+				assertExpectedWebsocketEvent(t, wsClient2, model.WebsocketEventUserUpdated, func(event *model.WebSocketEvent) {
+					eventUser := assertUpdated(t, event, newNickname)
+					assert.Empty(t, eventUser.NotifyProps, "non-source users should have sanitized notify_props")
+				})
+			})
+		})
+	})
+
+	t.Run("username", func(t *testing.T) {
+		assertUpdated := func(t *testing.T, event *model.WebSocketEvent, expectedUsername string) *model.User {
 			eventUser, ok := event.GetData()["user"].(*model.User)
 			require.True(t, ok, "expected user")
-			// assert eventUser.Id is same as th.BasicUser.Id
+			assert.Equal(t, th.BasicUser.Id, eventUser.Id)
-			assert.Equal(t, eventUser.Id, th.BasicUser.Id)
+			assert.Equal(t, expectedUsername, eventUser.Username)
-			// assert eventUser.NotifyProps is an empty map
+
-			require.Empty(t, eventUser.NotifyProps, "user event for non-source users should be sanitized")
+			// Some fields must always be sanitized
+			CheckUserSanitization(t, eventUser)
+
+			return eventUser
+		}
+
+		t.Run("update", func(t *testing.T) {
+			th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) {
+				newUsername := model.NewUsername()
+
+				th.BasicUser.Username = newUsername
+				_, _, err := client1.UpdateUser(context.Background(), th.BasicUser)
+				require.NoError(t, err)
+
+				assertExpectedWebsocketEvent(t, wsClient1, model.WebsocketEventUserUpdated, func(event *model.WebSocketEvent) {
+					eventUser := assertUpdated(t, event, newUsername)
+					assert.NotEmpty(t, eventUser.NotifyProps, "source user should keep notify_props")
+				})
+
+				assertExpectedWebsocketEvent(t, wsClient2, model.WebsocketEventUserUpdated, func(event *model.WebSocketEvent) {
+					eventUser := assertUpdated(t, event, newUsername)
+					assert.Empty(t, eventUser.NotifyProps, "non-source users should have sanitized notify_props")
+				})
+			})
+		})
+
+		t.Run("patch", func(t *testing.T) {
+			th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) {
+				newUsername := model.NewUsername()
+
+				_, _, err := client1.PatchUser(context.Background(), th.BasicUser.Id, &model.UserPatch{
+					Username: &newUsername,
+				})
+				require.NoError(t, err)
+
+				assertExpectedWebsocketEvent(t, wsClient1, model.WebsocketEventUserUpdated, func(event *model.WebSocketEvent) {
+					eventUser := assertUpdated(t, event, newUsername)
+					assert.NotEmpty(t, eventUser.NotifyProps, "source user should keep notify_props")
+				})
+
+				assertExpectedWebsocketEvent(t, wsClient2, model.WebsocketEventUserUpdated, func(event *model.WebSocketEvent) {
+					eventUser := assertUpdated(t, event, newUsername)
+					assert.Empty(t, eventUser.NotifyProps, "non-source users should have sanitized notify_props")
+				})
+			})
 		})
 	})
 }

server/channels/app/user.go

@@ -1262,9 +1262,11 @@ func (a *App) sendUpdatedUserEvent(user *model.User) {
 	// First, creating a base copy to avoid race conditions
 	// from setting the binaryParamKey in userstore.Update.
 	user = user.DeepCopy()
-	// declare admin and unsanitized copy of user
+	// Create copies for different sanitization levels:
+	// - adminCopyOfUser: moderately sanitized for admins
+	// - sourceUserCopyOfUser: minimally sanitized (keeps NotifyProps) for event creator
 	adminCopyOfUser := user.DeepCopy()
-	unsanitizedCopyOfUser := user.DeepCopy()
+	sourceUserCopyOfUser := user.DeepCopy()
 
 	a.SanitizeProfile(adminCopyOfUser, true)
 	adminMessage := model.NewWebSocketEvent(model.WebsocketEventUserUpdated, "", "", "", omitUsers, "")
@@ -1278,9 +1280,9 @@ func (a *App) sendUpdatedUserEvent(user *model.User) {
 	message.GetBroadcast().ContainsSanitizedData = true
 	a.Publish(message)
 
-	// send unsanitized user to event creator
+	sourceUserCopyOfUser.Sanitize(nil)
-	sourceUserMessage := model.NewWebSocketEvent(model.WebsocketEventUserUpdated, "", "", unsanitizedCopyOfUser.Id, nil, "")
+	sourceUserMessage := model.NewWebSocketEvent(model.WebsocketEventUserUpdated, "", "", sourceUserCopyOfUser.Id, nil, "")
-	sourceUserMessage.Add("user", unsanitizedCopyOfUser)
+	sourceUserMessage.Add("user", sourceUserCopyOfUser)
 	a.Publish(sourceUserMessage)
 }