diff --git a/server/channels/api4/user_test.go b/server/channels/api4/user_test.go index 80aaa75047..af258c85ad 100644 --- a/server/channels/api4/user_test.go +++ b/server/channels/api4/user_test.go @@ -7711,6 +7711,63 @@ func TestGetThreadsForUser(t *testing.T) { }) } +func TestGetThreadsForUser_AfterTeamRemovalAndReinvite(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic() + + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.ThreadAutoFollow = true + *cfg.ServiceSettings.CollapsedThreads = model.CollapsedThreadsDefaultOn + }) + th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuProfessional)) + + admin := th.BasicUser + victim := th.BasicUser2 + + privateChannel := th.CreatePrivateChannel() + th.AddUserToChannel(victim, privateChannel) + + defer func() { + require.NoError(t, th.App.Srv().Store().Post().PermanentDeleteByUser(th.Context, admin.Id)) + require.NoError(t, th.App.Srv().Store().Post().PermanentDeleteByUser(th.Context, victim.Id)) + }() + + rootPost, _, err := th.Client.CreatePost(context.Background(), &model.Post{ + ChannelId: privateChannel.Id, + Message: "private team secret", + }) + require.NoError(t, err) + + victimClient := th.CreateClient() + th.LoginBasic2WithClient(victimClient) + + _, _, err = victimClient.CreatePost(context.Background(), &model.Post{ + ChannelId: privateChannel.Id, + RootId: rootPost.Id, + Message: "victim reply", + }) + require.NoError(t, err) + + uss, _, err := victimClient.GetUserThreads(context.Background(), victim.Id, th.BasicTeam.Id, model.GetUserThreadsOpts{Extended: true}) + require.NoError(t, err) + require.Len(t, uss.Threads, 1, "sanity: victim should see their own thread before team removal") + + _, err = th.SystemAdminClient.RemoveTeamMember(context.Background(), th.BasicTeam.Id, victim.Id) + require.NoError(t, err) + + _, _, err = th.SystemAdminClient.AddTeamMember(context.Background(), th.BasicTeam.Id, victim.Id) + require.NoError(t, err) + + th.LoginBasic2WithClient(victimClient) + + uss, _, err = victimClient.GetUserThreads(context.Background(), victim.Id, th.BasicTeam.Id, model.GetUserThreadsOpts{Extended: true}) + require.NoError(t, err) + for _, thr := range uss.Threads { + require.NotEqual(t, rootPost.Id, thr.PostId, "private-channel thread must not leak to re-invited user") + } + require.Len(t, uss.Threads, 0, "re-invited user must not receive any threads from private channels they no longer belong to") +} + func TestThreadSocketEvents(t *testing.T) { mainHelper.Parallel(t) th := Setup(t).InitBasic() diff --git a/server/channels/app/channel.go b/server/channels/app/channel.go index 4d72f81b8b..cacdd83ef1 100644 --- a/server/channels/app/channel.go +++ b/server/channels/app/channel.go @@ -2668,6 +2668,19 @@ func (a *App) postRemoveFromChannelMessage(c request.CTX, removerUserId string, return nil } +// removeChannelMembership strips a user's channel membership and the associated +// thread memberships. Keeping these together ensures channel access cannot be +// revoked without also dropping the thread state that depends on it. +func (a *App) removeChannelMembership(rctx request.CTX, userID, channelID, caller string) *model.AppError { + if err := a.Srv().Store().Channel().RemoveMember(rctx, channelID, userID); err != nil { + return model.NewAppError(caller, "app.channel.remove_member.app_error", nil, "", http.StatusInternalServerError).Wrap(err) + } + if err := a.Srv().Store().Thread().DeleteMembershipsForChannel(userID, channelID); err != nil { + return model.NewAppError(caller, model.NoTranslation, nil, "failed to delete threadmemberships upon leaving channel", http.StatusInternalServerError).Wrap(err) + } + return nil +} + func (a *App) removeUserFromChannel(c request.CTX, userIDToRemove string, removerUserId string, channel *model.Channel) *model.AppError { user, nErr := a.Srv().Store().User().Get(context.Background(), userIDToRemove) if nErr != nil { @@ -2702,15 +2715,12 @@ func (a *App) removeUserFromChannel(c request.CTX, userIDToRemove string, remove return err } - if err := a.Srv().Store().Channel().RemoveMember(c, channel.Id, userIDToRemove); err != nil { - return model.NewAppError("removeUserFromChannel", "app.channel.remove_member.app_error", nil, "", http.StatusInternalServerError).Wrap(err) + if appErr := a.removeChannelMembership(c, userIDToRemove, channel.Id, "removeUserFromChannel"); appErr != nil { + return appErr } if err := a.Srv().Store().ChannelMemberHistory().LogLeaveEvent(userIDToRemove, channel.Id, model.GetMillis()); err != nil { return model.NewAppError("removeUserFromChannel", "app.channel_member_history.log_leave_event.internal_error", nil, "", http.StatusInternalServerError).Wrap(err) } - if err := a.Srv().Store().Thread().DeleteMembershipsForChannel(userIDToRemove, channel.Id); err != nil { - return model.NewAppError("removeUserFromChannel", model.NoTranslation, nil, "failed to delete threadmemberships upon leaving channel", http.StatusInternalServerError).Wrap(err) - } if isGuest { currentMembers, err := a.GetChannelMembersForUser(c, channel.TeamId, userIDToRemove) diff --git a/server/channels/app/team.go b/server/channels/app/team.go index b1060e3ea9..e8bf0b259c 100644 --- a/server/channels/app/team.go +++ b/server/channels/app/team.go @@ -1237,8 +1237,8 @@ func (a *App) LeaveTeam(c request.CTX, team *model.Team, user *model.User, reque for _, channel := range channelList { if !channel.IsGroupOrDirect() { a.invalidateCacheForChannelMembers(channel.Id) - if nErr = a.Srv().Store().Channel().RemoveMember(c, channel.Id, user.Id); nErr != nil { - return model.NewAppError("LeaveTeam", "app.channel.remove_member.app_error", nil, "", http.StatusInternalServerError).Wrap(nErr) + if appErr := a.removeChannelMembership(c, user.Id, channel.Id, "LeaveTeam"); appErr != nil { + return appErr } } } diff --git a/server/channels/app/team_test.go b/server/channels/app/team_test.go index e47d691214..8a63242a61 100644 --- a/server/channels/app/team_test.go +++ b/server/channels/app/team_test.go @@ -1139,6 +1139,256 @@ func TestLeaveTeamPanic(t *testing.T) { }, "unexpected panic from LeaveTeam") } +func TestLeaveTeamCleansUpThreadMemberships(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic() + + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.ThreadAutoFollow = true + *cfg.ServiceSettings.CollapsedThreads = model.CollapsedThreadsDefaultOn + }) + th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuProfessional)) + + admin := th.BasicUser + victim := th.BasicUser2 + + privateChannel := th.CreatePrivateChannel(th.Context, th.BasicTeam) + th.AddUserToChannel(victim, privateChannel) + + rootPost, _, appErr := th.App.CreatePost(th.Context, &model.Post{ + UserId: admin.Id, + ChannelId: privateChannel.Id, + Message: "private team secret", + }, privateChannel, model.CreatePostFlags{SetOnline: true}) + require.Nil(t, appErr) + defer func() { + require.NoError(t, th.App.Srv().Store().Post().PermanentDeleteByUser(th.Context, admin.Id)) + }() + + _, _, appErr = th.App.CreatePost(th.Context, &model.Post{ + UserId: victim.Id, + ChannelId: privateChannel.Id, + RootId: rootPost.Id, + Message: "victim reply", + }, privateChannel, model.CreatePostFlags{SetOnline: true}) + require.Nil(t, appErr) + defer func() { + require.NoError(t, th.App.Srv().Store().Post().PermanentDeleteByUser(th.Context, victim.Id)) + }() + + _, sErr := th.App.Srv().Store().Thread().GetMembershipForUser(victim.Id, rootPost.Id) + require.NoError(t, sErr, "victim should follow the thread after replying") + + appErr = th.App.LeaveTeam(th.Context, th.BasicTeam, victim, victim.Id) + require.Nil(t, appErr) + + _, gErr := th.App.Srv().Store().Thread().GetMembershipForUser(victim.Id, rootPost.Id) + var errNotFound *store.ErrNotFound + require.ErrorAs(t, gErr, &errNotFound, "thread membership must be deleted when user leaves the team") +} + +func TestLeaveTeamCleansUpThreadMembershipsAcrossChannels(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic() + + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.ThreadAutoFollow = true + *cfg.ServiceSettings.CollapsedThreads = model.CollapsedThreadsDefaultOn + }) + th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuProfessional)) + + admin := th.BasicUser + victim := th.BasicUser2 + + privateA := th.CreatePrivateChannel(th.Context, th.BasicTeam) + privateB := th.CreatePrivateChannel(th.Context, th.BasicTeam) + openC := th.CreateChannel(th.Context, th.BasicTeam) + th.AddUserToChannel(victim, privateA) + th.AddUserToChannel(victim, privateB) + th.AddUserToChannel(victim, openC) + + rootIDs := make([]string, 0, 3) + for _, ch := range []*model.Channel{privateA, privateB, openC} { + root, _, appErr := th.App.CreatePost(th.Context, &model.Post{ + UserId: admin.Id, + ChannelId: ch.Id, + Message: "root in " + ch.Id, + }, ch, model.CreatePostFlags{SetOnline: true}) + require.Nil(t, appErr) + _, _, appErr = th.App.CreatePost(th.Context, &model.Post{ + UserId: victim.Id, + ChannelId: ch.Id, + RootId: root.Id, + Message: "reply", + }, ch, model.CreatePostFlags{SetOnline: true}) + require.Nil(t, appErr) + rootIDs = append(rootIDs, root.Id) + } + defer func() { + require.NoError(t, th.App.Srv().Store().Post().PermanentDeleteByUser(th.Context, admin.Id)) + require.NoError(t, th.App.Srv().Store().Post().PermanentDeleteByUser(th.Context, victim.Id)) + }() + + for _, rid := range rootIDs { + _, sErr := th.App.Srv().Store().Thread().GetMembershipForUser(victim.Id, rid) + require.NoError(t, sErr, "sanity: victim should follow each thread") + } + + appErr := th.App.LeaveTeam(th.Context, th.BasicTeam, victim, victim.Id) + require.Nil(t, appErr) + + var errNotFound *store.ErrNotFound + for _, rid := range rootIDs { + _, gErr := th.App.Srv().Store().Thread().GetMembershipForUser(victim.Id, rid) + require.ErrorAs(t, gErr, &errNotFound, "thread membership for %s must be deleted on team leave", rid) + } +} + +func TestLeaveTeamPreservesDMThreadMemberships(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic() + + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.ThreadAutoFollow = true + *cfg.ServiceSettings.CollapsedThreads = model.CollapsedThreadsDefaultOn + }) + th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuProfessional)) + + admin := th.BasicUser + victim := th.BasicUser2 + + dmChannel, appErr := th.App.GetOrCreateDirectChannel(th.Context, admin.Id, victim.Id) + require.Nil(t, appErr) + + dmRoot, _, appErr := th.App.CreatePost(th.Context, &model.Post{ + UserId: admin.Id, + ChannelId: dmChannel.Id, + Message: "dm root", + }, dmChannel, model.CreatePostFlags{SetOnline: true}) + require.Nil(t, appErr) + _, _, appErr = th.App.CreatePost(th.Context, &model.Post{ + UserId: victim.Id, + ChannelId: dmChannel.Id, + RootId: dmRoot.Id, + Message: "dm reply", + }, dmChannel, model.CreatePostFlags{SetOnline: true}) + require.Nil(t, appErr) + defer func() { + require.NoError(t, th.App.Srv().Store().Post().PermanentDeleteByUser(th.Context, admin.Id)) + require.NoError(t, th.App.Srv().Store().Post().PermanentDeleteByUser(th.Context, victim.Id)) + }() + + _, sErr := th.App.Srv().Store().Thread().GetMembershipForUser(victim.Id, dmRoot.Id) + require.NoError(t, sErr, "sanity: victim should follow the DM thread") + + appErr = th.App.LeaveTeam(th.Context, th.BasicTeam, victim, victim.Id) + require.Nil(t, appErr) + + _, gErr := th.App.Srv().Store().Thread().GetMembershipForUser(victim.Id, dmRoot.Id) + require.NoError(t, gErr, "DM thread membership must survive leaving an unrelated team") +} + +func TestGetThreadsForUser_ReadPathRejectsOrphanThreadMembership(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic() + + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.ThreadAutoFollow = true + *cfg.ServiceSettings.CollapsedThreads = model.CollapsedThreadsDefaultOn + }) + th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuProfessional)) + + admin := th.BasicUser + victim := th.BasicUser2 + + privateChannel := th.CreatePrivateChannel(th.Context, th.BasicTeam) + th.AddUserToChannel(victim, privateChannel) + + rootPost, _, appErr := th.App.CreatePost(th.Context, &model.Post{ + UserId: admin.Id, + ChannelId: privateChannel.Id, + Message: "private team secret", + }, privateChannel, model.CreatePostFlags{SetOnline: true}) + require.Nil(t, appErr) + defer func() { + require.NoError(t, th.App.Srv().Store().Post().PermanentDeleteByUser(th.Context, admin.Id)) + require.NoError(t, th.App.Srv().Store().Post().PermanentDeleteByUser(th.Context, victim.Id)) + }() + + _, _, appErr = th.App.CreatePost(th.Context, &model.Post{ + UserId: victim.Id, + ChannelId: privateChannel.Id, + RootId: rootPost.Id, + Message: "victim reply", + }, privateChannel, model.CreatePostFlags{SetOnline: true}) + require.Nil(t, appErr) + + _, sErr := th.App.Srv().Store().Thread().GetMembershipForUser(victim.Id, rootPost.Id) + require.NoError(t, sErr, "sanity: victim should follow the thread after replying") + + require.NoError(t, th.App.Srv().Store().Channel().RemoveMember(th.Context, privateChannel.Id, victim.Id)) + + _, sErr2 := th.App.Srv().Store().Thread().GetMembershipForUser(victim.Id, rootPost.Id) + require.NoError(t, sErr2, "sanity: synthetic orphan ThreadMembership must remain") + + threads, gErr := th.App.Srv().Store().Thread().GetThreadsForUser(victim.Id, th.BasicTeam.Id, model.GetUserThreadsOpts{}) + require.NoError(t, gErr) + for _, thr := range threads { + require.NotEqual(t, rootPost.Id, thr.PostId, "read path must not surface threads from channels the user no longer belongs to") + } + require.Empty(t, threads, "GetThreadsForUser must filter out orphan ThreadMembership rows") + + totalThreads, gErr := th.App.Srv().Store().Thread().GetTotalThreads(victim.Id, th.BasicTeam.Id, model.GetUserThreadsOpts{}) + require.NoError(t, gErr) + require.Zero(t, totalThreads, "GetTotalThreads must not count orphan ThreadMembership rows") + + totalUnread, gErr := th.App.Srv().Store().Thread().GetTotalUnreadThreads(victim.Id, th.BasicTeam.Id, model.GetUserThreadsOpts{}) + require.NoError(t, gErr) + require.Zero(t, totalUnread, "GetTotalUnreadThreads must not count orphan ThreadMembership rows") +} + +func TestPermanentDeleteChannelRemovesThreadMemberships(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic() + + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.ThreadAutoFollow = true + *cfg.ServiceSettings.CollapsedThreads = model.CollapsedThreadsDefaultOn + }) + th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuProfessional)) + + admin := th.BasicUser + victim := th.BasicUser2 + + privateChannel := th.CreatePrivateChannel(th.Context, th.BasicTeam) + th.AddUserToChannel(victim, privateChannel) + + rootPost, _, appErr := th.App.CreatePost(th.Context, &model.Post{ + UserId: admin.Id, + ChannelId: privateChannel.Id, + Message: "doomed root", + }, privateChannel, model.CreatePostFlags{SetOnline: true}) + require.Nil(t, appErr) + + _, _, appErr = th.App.CreatePost(th.Context, &model.Post{ + UserId: victim.Id, + ChannelId: privateChannel.Id, + RootId: rootPost.Id, + Message: "doomed reply", + }, privateChannel, model.CreatePostFlags{SetOnline: true}) + require.Nil(t, appErr) + + _, sErr := th.App.Srv().Store().Thread().GetMembershipForUser(victim.Id, rootPost.Id) + require.NoError(t, sErr, "victim should follow the thread after replying") + + appErr = th.App.PermanentDeleteChannel(th.Context, privateChannel) + require.Nil(t, appErr) + + _, gErr := th.App.Srv().Store().Thread().GetMembershipForUser(victim.Id, rootPost.Id) + var errNotFound *store.ErrNotFound + require.ErrorAs(t, gErr, &errNotFound, "thread membership must be deleted with the channel") +} + func TestAppUpdateTeamScheme(t *testing.T) { mainHelper.Parallel(t) th := Setup(t).InitBasic() diff --git a/server/channels/db/migrations/migrations.list b/server/channels/db/migrations/migrations.list index 2a85e8eace..c24ac1f980 100644 --- a/server/channels/db/migrations/migrations.list +++ b/server/channels/db/migrations/migrations.list @@ -285,6 +285,8 @@ channels/db/migrations/mysql/000143_backfill_roles_schemeid.down.sql channels/db/migrations/mysql/000143_backfill_roles_schemeid.up.sql channels/db/migrations/mysql/000144_add_roles_schemeid_index.down.sql channels/db/migrations/mysql/000144_add_roles_schemeid_index.up.sql +channels/db/migrations/mysql/000195_threadmemberships_cleanup_v2.down.sql +channels/db/migrations/mysql/000195_threadmemberships_cleanup_v2.up.sql channels/db/migrations/postgres/000001_create_teams.down.sql channels/db/migrations/postgres/000001_create_teams.up.sql channels/db/migrations/postgres/000002_create_team_members.down.sql @@ -571,3 +573,5 @@ channels/db/migrations/postgres/000143_backfill_roles_schemeid.down.sql channels/db/migrations/postgres/000143_backfill_roles_schemeid.up.sql channels/db/migrations/postgres/000144_add_roles_schemeid_index.down.sql channels/db/migrations/postgres/000144_add_roles_schemeid_index.up.sql +channels/db/migrations/postgres/000195_threadmemberships_cleanup_v2.down.sql +channels/db/migrations/postgres/000195_threadmemberships_cleanup_v2.up.sql diff --git a/server/channels/db/migrations/mysql/000195_threadmemberships_cleanup_v2.down.sql b/server/channels/db/migrations/mysql/000195_threadmemberships_cleanup_v2.down.sql new file mode 100644 index 0000000000..44724bc5de --- /dev/null +++ b/server/channels/db/migrations/mysql/000195_threadmemberships_cleanup_v2.down.sql @@ -0,0 +1 @@ +-- Skipping it because the forward migration is destructive diff --git a/server/channels/db/migrations/mysql/000195_threadmemberships_cleanup_v2.up.sql b/server/channels/db/migrations/mysql/000195_threadmemberships_cleanup_v2.up.sql new file mode 100644 index 0000000000..14eedbe516 --- /dev/null +++ b/server/channels/db/migrations/mysql/000195_threadmemberships_cleanup_v2.up.sql @@ -0,0 +1,7 @@ +-- Drop ThreadMembership rows whose user is no longer a member of the thread's channel. +DELETE tm +FROM ThreadMemberships AS tm +JOIN Threads ON Threads.PostId = tm.PostId +LEFT JOIN ChannelMembers ON ChannelMembers.UserId = tm.UserId + AND Threads.ChannelId = ChannelMembers.ChannelId +WHERE ChannelMembers.ChannelId IS NULL; diff --git a/server/channels/db/migrations/postgres/000195_threadmemberships_cleanup_v2.down.sql b/server/channels/db/migrations/postgres/000195_threadmemberships_cleanup_v2.down.sql new file mode 100644 index 0000000000..44724bc5de --- /dev/null +++ b/server/channels/db/migrations/postgres/000195_threadmemberships_cleanup_v2.down.sql @@ -0,0 +1 @@ +-- Skipping it because the forward migration is destructive diff --git a/server/channels/db/migrations/postgres/000195_threadmemberships_cleanup_v2.up.sql b/server/channels/db/migrations/postgres/000195_threadmemberships_cleanup_v2.up.sql new file mode 100644 index 0000000000..7d30131137 --- /dev/null +++ b/server/channels/db/migrations/postgres/000195_threadmemberships_cleanup_v2.up.sql @@ -0,0 +1,13 @@ +-- Drop ThreadMembership rows whose user is no longer a member of the thread's channel. +DELETE FROM threadmemberships WHERE (postid, userid) IN ( + SELECT + threadmemberships.postid, + threadmemberships.userid + FROM + threadmemberships + JOIN threads ON threads.postid = threadmemberships.postid + LEFT JOIN channelmembers ON channelmembers.userid = threadmemberships.userid + AND threads.channelid = channelmembers.channelid + WHERE + channelmembers.channelid IS NULL +); diff --git a/server/channels/store/sqlstore/thread_store.go b/server/channels/store/sqlstore/thread_store.go index de3c421450..b1c0af7e24 100644 --- a/server/channels/store/sqlstore/thread_store.go +++ b/server/channels/store/sqlstore/thread_store.go @@ -70,6 +70,17 @@ type SqlThreadStore struct { func (s *SqlThreadStore) ClearCaches() { } +// channelMembershipPredicate filters out ThreadMemberships whose user is no +// longer a member of the thread's channel. DM/GM threads have an empty +// ThreadTeamId and are exempt because their access is intrinsic to the +// channel members. +func channelMembershipPredicate() sq.Sqlizer { + return sq.Or{ + sq.Eq{"Threads.ThreadTeamId": ""}, + sq.Expr("EXISTS (SELECT 1 FROM ChannelMembers WHERE ChannelMembers.ChannelId = Threads.ChannelId AND ChannelMembers.UserId = ThreadMemberships.UserId)"), + } +} + func newSqlThreadStore(sqlStore *SqlStore) store.ThreadStore { s := SqlThreadStore{ SqlStore: sqlStore, @@ -131,7 +142,8 @@ func (s *SqlThreadStore) getTotalThreadsQuery(userId, teamId string, opts model. Where(sq.Eq{ "ThreadMemberships.UserId": userId, "ThreadMemberships.Following": true, - }) + }). + Where(channelMembershipPredicate()) if teamId != "" { if opts.ExcludeDirect { @@ -198,7 +210,8 @@ func (s *SqlThreadStore) GetTotalUnreadMentions(userId, teamId string, opts mode Where(sq.Eq{ "ThreadMemberships.UserId": userId, "ThreadMemberships.Following": true, - }) + }). + Where(channelMembershipPredicate()) if teamId != "" { if opts.ExcludeDirect { @@ -234,15 +247,13 @@ func (s *SqlThreadStore) GetTotalUnreadUrgentMentions(userId, teamId string, opt Select("COALESCE(SUM(ThreadMemberships.UnreadMentions),0)"). From("ThreadMemberships"). Join("PostsPriority ON PostsPriority.PostId = ThreadMemberships.PostId"). + Join("Threads ON Threads.PostId = ThreadMemberships.PostId"). Where(sq.Eq{ "ThreadMemberships.UserId": userId, "ThreadMemberships.Following": true, "PostsPriority.Priority": model.PostPriorityUrgent, - }) - - if teamId != "" || !opts.Deleted { - query = query.Join("Threads ON Threads.PostId = ThreadMemberships.PostId") - } + }). + Where(channelMembershipPredicate()) if teamId != "" { if opts.ExcludeDirect { @@ -298,7 +309,8 @@ func (s *SqlThreadStore) GetThreadsForUser(userId, teamId string, opts model.Get query = query. Where(sq.Eq{"ThreadMemberships.UserId": userId}). - Where(sq.Eq{"ThreadMemberships.Following": true}) + Where(sq.Eq{"ThreadMemberships.Following": true}). + Where(channelMembershipPredicate()) if opts.IncludeIsUrgent { urgencyCase := sq. @@ -405,6 +417,7 @@ func (s *SqlThreadStore) GetTeamsUnreadForUser(userID string, teamIDs []string, sq.Eq{"ThreadMemberships.Following": true}, sq.Eq{"Threads.ThreadTeamId": teamIDs}, sq.Eq{"COALESCE(Threads.ThreadDeleteAt, 0)": 0}, + channelMembershipPredicate(), } var eg errgroup.Group diff --git a/server/channels/store/storetest/thread_store.go b/server/channels/store/storetest/thread_store.go index 3a682f04d6..fe29f94d81 100644 --- a/server/channels/store/storetest/thread_store.go +++ b/server/channels/store/storetest/thread_store.go @@ -717,6 +717,12 @@ func testGetTeamsUnreadForUser(t *testing.T, rctx request.CTX, ss store.Store) { Type: model.ChannelTypeOpen, }, -1) require.NoError(t, err) + _, err = ss.Channel().SaveMember(rctx, &model.ChannelMember{ + ChannelId: channel1.Id, + UserId: userID, + NotifyProps: model.GetDefaultChannelNotifyProps(), + }) + require.NoError(t, err) post, err := ss.Post().Save(rctx, &model.Post{ ChannelId: channel1.Id, UserId: userID, @@ -759,6 +765,12 @@ func testGetTeamsUnreadForUser(t *testing.T, rctx request.CTX, ss store.Store) { Type: model.ChannelTypeOpen, }, -1) require.NoError(t, err) + _, err = ss.Channel().SaveMember(rctx, &model.ChannelMember{ + ChannelId: channel2.Id, + UserId: userID, + NotifyProps: model.GetDefaultChannelNotifyProps(), + }) + require.NoError(t, err) post2, err := ss.Post().Save(rctx, &model.Post{ ChannelId: channel2.Id, @@ -869,6 +881,12 @@ func testVarious(t *testing.T, rctx request.CTX, ss store.Store) { Type: model.ChannelTypeOpen, }, -1) require.NoError(t, err) + _, err = ss.Channel().SaveMember(rctx, &model.ChannelMember{ + ChannelId: team1channel1.Id, + UserId: user1ID, + NotifyProps: model.GetDefaultChannelNotifyProps(), + }) + require.NoError(t, err) team2channel1, err := ss.Channel().Save(rctx, &model.Channel{ TeamId: team2.Id, @@ -877,6 +895,12 @@ func testVarious(t *testing.T, rctx request.CTX, ss store.Store) { Type: model.ChannelTypeOpen, }, -1) require.NoError(t, err) + _, err = ss.Channel().SaveMember(rctx, &model.ChannelMember{ + ChannelId: team2channel1.Id, + UserId: user1ID, + NotifyProps: model.GetDefaultChannelNotifyProps(), + }) + require.NoError(t, err) dm1, err := ss.Channel().CreateDirectChannel(rctx, &model.User{Id: user1ID}, &model.User{Id: user2ID}) require.NoError(t, err) @@ -1345,6 +1369,17 @@ func testMarkAllAsReadByChannels(t *testing.T, rctx request.CTX, ss store.Store) }, -1) require.NoError(t, err) + for _, ch := range []*model.Channel{channel1, channel2} { + for _, uid := range []string{userAID, userBID} { + _, err = ss.Channel().SaveMember(rctx, &model.ChannelMember{ + ChannelId: ch.Id, + UserId: uid, + NotifyProps: model.GetDefaultChannelNotifyProps(), + }) + require.NoError(t, err) + } + } + createThreadMembership := func(userID, postID string) { t.Helper() opts := store.ThreadMembershipOpts{ @@ -1535,6 +1570,17 @@ func testMarkAllAsReadByTeam(t *testing.T, rctx request.CTX, ss store.Store) { }, -1) require.NoError(t, err) + for _, ch := range []*model.Channel{team1channel1, team1channel2, team2channel1, team2channel2} { + for _, uid := range []string{userAID, userBID} { + _, err = ss.Channel().SaveMember(rctx, &model.ChannelMember{ + ChannelId: ch.Id, + UserId: uid, + NotifyProps: model.GetDefaultChannelNotifyProps(), + }) + require.NoError(t, err) + } + } + team1channel1post1, err := ss.Post().Save(rctx, &model.Post{ ChannelId: team1channel1.Id, UserId: postingUserId, @@ -2110,6 +2156,13 @@ func testUpdateTeamIdForChannelThreads(t *testing.T, rctx request.CTX, ss store. }) require.NoError(t, err) + _, err = ss.Channel().SaveMember(rctx, &model.ChannelMember{ + ChannelId: channel1.Id, + UserId: userA.Id, + NotifyProps: model.GetDefaultChannelNotifyProps(), + }) + require.NoError(t, err) + _, clean := createThreadMembership(userA.Id, rootPost1.Id, true) defer clean() @@ -2134,6 +2187,13 @@ func testUpdateTeamIdForChannelThreads(t *testing.T, rctx request.CTX, ss store. }) require.NoError(t, err) + _, err = ss.Channel().SaveMember(rctx, &model.ChannelMember{ + ChannelId: channel1.Id, + UserId: userA.Id, + NotifyProps: model.GetDefaultChannelNotifyProps(), + }) + require.NoError(t, err) + newTeamID := model.NewId() _, clean := createThreadMembership(userA.Id, rootPost1.Id, true)