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

This commit is contained in:
Christopher Speller 2025-09-05 08:06:16 -07:00 committed by GitHub
parent 2c12d3a11c
commit ba86dfc587
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 202 additions and 0 deletions

View file

@ -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 {

View file

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

View file

@ -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))
}

View file

@ -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 {

View file

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