From ba86dfc5876b354b9d3c20ff45c08ca6f8426149 Mon Sep 17 00:00:00 2001 From: Christopher Speller Date: Fri, 5 Sep 2025 08:06:16 -0700 Subject: [PATCH] Sanatize LastViewedAt and LastUpdateAt for other users on channel member object (#33835) --- server/channels/api4/channel.go | 27 +++++++ server/channels/api4/channel_test.go | 84 ++++++++++++++++++++++ server/channels/api4/user.go | 9 +++ server/public/model/channel_member.go | 11 +++ server/public/model/channel_member_test.go | 71 ++++++++++++++++++ 5 files changed, 202 insertions(+) diff --git a/server/channels/api4/channel.go b/server/channels/api4/channel.go index 6ef374dc570..646e1cc5561 100644 --- a/server/channels/api4/channel.go +++ b/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 { diff --git a/server/channels/api4/channel_test.go b/server/channels/api4/channel_test.go index e1c1951a9e5..e098634ca17 100644 --- a/server/channels/api4/channel_test.go +++ b/server/channels/api4/channel_test.go @@ -6302,3 +6302,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") + }) +} diff --git a/server/channels/api4/user.go b/server/channels/api4/user.go index e4c12d849f6..73150f02d96 100644 --- a/server/channels/api4/user.go +++ b/server/channels/api4/user.go @@ -3090,6 +3090,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)) } @@ -3123,7 +3129,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)) } diff --git a/server/public/model/channel_member.go b/server/public/model/channel_member.go index 5d4f8f33369..da854db0559 100644 --- a/server/public/model/channel_member.go +++ b/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 { diff --git a/server/public/model/channel_member_test.go b/server/public/model/channel_member_test.go index 90c4655357e..26ad8122893 100644 --- a/server/public/model/channel_member_test.go +++ b/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") + }) +}