Sanatize LastViewedAt and LastUpdateAt for other users on channel member object (#33835) (#33905)

Automatic Merge

server/channels/api4/channel.go

@@ -1516,6 +1516,12 @@ func getChannelMembers(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// Sanitize members for current user
+	currentUserId := c.AppContext.Session().UserId
+	for i := range members {
+		members[i].SanitizeForCurrentUser(currentUserId)
+	}
+
 	if err := json.NewEncoder(w).Encode(members); err != nil {
 		c.Logger.Warn("Error while writing response", mlog.Err(err))
 	}
@@ -1569,6 +1575,12 @@ func getChannelMembersByIds(c *Context, w http.ResponseWriter, r *http.Request)
 		return
 	}
 
+	// Sanitize members for current user
+	currentUserId := c.AppContext.Session().UserId
+	for i := range members {
+		members[i].SanitizeForCurrentUser(currentUserId)
+	}
+
 	if err := json.NewEncoder(w).Encode(members); err != nil {
 		c.Logger.Warn("Error while writing response", mlog.Err(err))
 	}
@@ -1592,6 +1604,9 @@ func getChannelMember(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// Sanitize member for current user
+	member.SanitizeForCurrentUser(c.AppContext.Session().UserId)
+
 	if err := json.NewEncoder(w).Encode(member); err != nil {
 		c.Logger.Warn("Error while writing response", mlog.Err(err))
 	}
@@ -1619,6 +1634,12 @@ func getChannelMembersForTeamForUser(c *Context, w http.ResponseWriter, r *http.
 		return
 	}
 
+	// Sanitize members for current user
+	currentUserId := c.AppContext.Session().UserId
+	for i := range members {
+		members[i].SanitizeForCurrentUser(currentUserId)
+	}
+
 	if err := json.NewEncoder(w).Encode(members); err != nil {
 		c.Logger.Warn("Error while writing response", mlog.Err(err))
 	}
@@ -2005,6 +2026,12 @@ func addChannelMember(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// Sanitize the returned members
+	currentUserId := c.AppContext.Session().UserId
+	for i := range newChannelMembers {
+		newChannelMembers[i].SanitizeForCurrentUser(currentUserId)
+	}
+
 	w.WriteHeader(http.StatusCreated)
 	userId, ok := props["user_id"]
 	if ok && len(newChannelMembers) == 1 && newChannelMembers[0].UserId == userId {

server/channels/api4/channel_test.go

@@ -6318,3 +6318,87 @@ func TestViewChannelWithoutCollapsedThreads(t *testing.T) {
 	require.NoError(t, err)
 	require.Zero(t, threads.TotalUnreadMentions)
 }
+
+func TestChannelMemberSanitization(t *testing.T) {
+	mainHelper.Parallel(t)
+	th := Setup(t).InitBasic()
+	defer th.TearDown()
+
+	client := th.Client
+	user := th.BasicUser
+	user2 := th.BasicUser2
+	channel := th.CreatePublicChannel()
+
+	// Add second user to channel
+	_, _, err := client.AddChannelMember(context.Background(), channel.Id, user2.Id)
+	require.NoError(t, err)
+
+	t.Run("getChannelMembers sanitizes LastViewedAt and LastUpdateAt for other users", func(t *testing.T) {
+		members, _, err := client.GetChannelMembers(context.Background(), channel.Id, 0, 60, "")
+		require.NoError(t, err)
+
+		for _, member := range members {
+			if member.UserId == user.Id {
+				// Current user should see their own timestamps
+				assert.NotEqual(t, int64(-1), member.LastViewedAt, "Current user should see their LastViewedAt")
+				assert.NotEqual(t, int64(-1), member.LastUpdateAt, "Current user should see their LastUpdateAt")
+			} else {
+				// Other users' timestamps should be sanitized
+				assert.Equal(t, int64(-1), member.LastViewedAt, "Other users' LastViewedAt should be sanitized")
+				assert.Equal(t, int64(-1), member.LastUpdateAt, "Other users' LastUpdateAt should be sanitized")
+			}
+		}
+	})
+
+	t.Run("getChannelMember sanitizes LastViewedAt and LastUpdateAt for other users", func(t *testing.T) {
+		// Get other user's membership data
+		member, _, err := client.GetChannelMember(context.Background(), channel.Id, user2.Id, "")
+		require.NoError(t, err)
+
+		// Should be sanitized since it's not the current user
+		assert.Equal(t, int64(-1), member.LastViewedAt, "Other user's LastViewedAt should be sanitized")
+		assert.Equal(t, int64(-1), member.LastUpdateAt, "Other user's LastUpdateAt should be sanitized")
+
+		// Get current user's membership data
+		currentMember, _, err := client.GetChannelMember(context.Background(), channel.Id, user.Id, "")
+		require.NoError(t, err)
+
+		// Should not be sanitized since it's the current user
+		assert.NotEqual(t, int64(-1), currentMember.LastViewedAt, "Current user should see their LastViewedAt")
+		assert.NotEqual(t, int64(-1), currentMember.LastUpdateAt, "Current user should see their LastUpdateAt")
+	})
+
+	t.Run("getChannelMembersByIds sanitizes data appropriately", func(t *testing.T) {
+		userIds := []string{user.Id, user2.Id}
+		members, _, err := client.GetChannelMembersByIds(context.Background(), channel.Id, userIds)
+		require.NoError(t, err)
+		require.Len(t, members, 2)
+
+		for _, member := range members {
+			if member.UserId == user.Id {
+				// Current user should see their own timestamps
+				assert.NotEqual(t, int64(-1), member.LastViewedAt, "Current user should see their LastViewedAt")
+				assert.NotEqual(t, int64(-1), member.LastUpdateAt, "Current user should see their LastUpdateAt")
+			} else {
+				// Other users' timestamps should be sanitized
+				assert.Equal(t, int64(-1), member.LastViewedAt, "Other users' LastViewedAt should be sanitized")
+				assert.Equal(t, int64(-1), member.LastUpdateAt, "Other users' LastUpdateAt should be sanitized")
+			}
+		}
+	})
+
+	t.Run("addChannelMember sanitizes returned member data", func(t *testing.T) {
+		newUser := th.CreateUser()
+		th.LinkUserToTeam(newUser, th.BasicTeam)
+
+		// Add new user and check returned member data
+		returnedMember, _, err := client.AddChannelMember(context.Background(), channel.Id, newUser.Id)
+		require.NoError(t, err)
+
+		// The returned member should be sanitized since it's not the current user
+		assert.Equal(t, int64(-1), returnedMember.LastViewedAt, "Returned member LastViewedAt should be sanitized")
+		assert.Equal(t, int64(-1), returnedMember.LastUpdateAt, "Returned member LastUpdateAt should be sanitized")
+		assert.Equal(t, newUser.Id, returnedMember.UserId, "UserId should be preserved")
+		assert.Equal(t, channel.Id, returnedMember.ChannelId, "ChannelId should be preserved")
+	})
+}

server/channels/api4/user.go

@@ -3128,6 +3128,12 @@ func getChannelMembersForUser(c *Context, w http.ResponseWriter, r *http.Request
 			return
 		}
 
+		// Sanitize members for current user
+		currentUserId := c.AppContext.Session().UserId
+		for i := range members {
+			members[i].SanitizeForCurrentUser(currentUserId)
+		}
+
 		if err := json.NewEncoder(w).Encode(members); err != nil {
 			c.Logger.Warn("Error while writing response", mlog.Err(err))
 		}
@@ -3161,7 +3167,10 @@ func getChannelMembersForUser(c *Context, w http.ResponseWriter, r *http.Request
 			return
 		}
 
+		currentUserId := c.AppContext.Session().UserId
 		for _, member := range members {
+			// Sanitize each member before encoding in the stream
+			member.SanitizeForCurrentUser(currentUserId)
 			if err := enc.Encode(member); err != nil {
 				c.Logger.Warn("Error while writing response", mlog.Err(err))
 			}

server/public/model/channel_member.go

@@ -89,6 +89,17 @@ func (o *ChannelMember) Auditable() map[string]any {
 	}
 }
 
+// SanitizeForCurrentUser sanitizes channel member data based on whether
+// it's the current user's own membership or another user's membership
+func (o *ChannelMember) SanitizeForCurrentUser(currentUserId string) {
+	// If this is not the current user's own membership,
+	// sanitize sensitive timestamp fields
+	if o.UserId != currentUserId {
+		o.LastViewedAt = -1
+		o.LastUpdateAt = -1
+	}
+}
+
 // ChannelMemberWithTeamData contains ChannelMember appended with extra team information
 // as well.
 type ChannelMemberWithTeamData struct {

server/public/model/channel_member_test.go

@@ -58,3 +58,74 @@ func TestIsChannelMemberNotifyPropsValid(t *testing.T) {
 		assert.Nil(t, err)
 	})
 }
+
+func TestChannelMemberSanitizeForCurrentUser(t *testing.T) {
+	currentUserId := NewId()
+	otherUserId := NewId()
+	channelId := NewId()
+
+	t.Run("should not sanitize current user's own membership", func(t *testing.T) {
+		member := &ChannelMember{
+			ChannelId:    channelId,
+			UserId:       currentUserId,
+			LastViewedAt: 1234567890000,
+			LastUpdateAt: 1234567890000,
+			NotifyProps:  GetDefaultChannelNotifyProps(),
+		}
+
+		originalLastViewedAt := member.LastViewedAt
+		originalLastUpdateAt := member.LastUpdateAt
+
+		member.SanitizeForCurrentUser(currentUserId)
+
+		assert.Equal(t, originalLastViewedAt, member.LastViewedAt, "LastViewedAt should not be sanitized for current user")
+		assert.Equal(t, originalLastUpdateAt, member.LastUpdateAt, "LastUpdateAt should not be sanitized for current user")
+	})
+
+	t.Run("should sanitize other users' membership data", func(t *testing.T) {
+		member := &ChannelMember{
+			ChannelId:    channelId,
+			UserId:       otherUserId,
+			LastViewedAt: 1234567890000,
+			LastUpdateAt: 1234567890000,
+			NotifyProps:  GetDefaultChannelNotifyProps(),
+		}
+
+		member.SanitizeForCurrentUser(currentUserId)
+
+		assert.Equal(t, int64(-1), member.LastViewedAt, "LastViewedAt should be sanitized for other users")
+		assert.Equal(t, int64(-1), member.LastUpdateAt, "LastUpdateAt should be sanitized for other users")
+	})
+
+	t.Run("should preserve other fields when sanitizing", func(t *testing.T) {
+		member := &ChannelMember{
+			ChannelId:     channelId,
+			UserId:        otherUserId,
+			Roles:         "channel_user",
+			LastViewedAt:  1234567890000,
+			LastUpdateAt:  1234567890000,
+			MsgCount:      100,
+			MentionCount:  5,
+			NotifyProps:   GetDefaultChannelNotifyProps(),
+			SchemeUser:    true,
+			SchemeAdmin:   false,
+			ExplicitRoles: "",
+		}
+
+		originalRoles := member.Roles
+		originalMsgCount := member.MsgCount
+		originalMentionCount := member.MentionCount
+		originalSchemeUser := member.SchemeUser
+		originalSchemeAdmin := member.SchemeAdmin
+
+		member.SanitizeForCurrentUser(currentUserId)
+
+		assert.Equal(t, int64(-1), member.LastViewedAt, "LastViewedAt should be sanitized")
+		assert.Equal(t, int64(-1), member.LastUpdateAt, "LastUpdateAt should be sanitized")
+		assert.Equal(t, originalRoles, member.Roles, "Roles should be preserved")
+		assert.Equal(t, originalMsgCount, member.MsgCount, "MsgCount should be preserved")
+		assert.Equal(t, originalMentionCount, member.MentionCount, "MentionCount should be preserved")
+		assert.Equal(t, originalSchemeUser, member.SchemeUser, "SchemeUser should be preserved")
+		assert.Equal(t, originalSchemeAdmin, member.SchemeAdmin, "SchemeAdmin should be preserved")
+	})
+}