diff --git a/server/channels/api4/channel.go b/server/channels/api4/channel.go index e9aaca1e64a..cf7932038aa 100644 --- a/server/channels/api4/channel.go +++ b/server/channels/api4/channel.go @@ -28,6 +28,24 @@ func rejectBoardChannelByID(c *Context, channelId string) bool { return false } +// rejectSpaceChannelByID returns true and sets c.Err if the channel ID belongs +// to a space channel. Space channels must use the spaces API, not /channels. +// Use this on write endpoints to give a clear error instead of a 404. +// It reads from the primary so a freshly created space cannot slip through on +// replica lag, and fails closed by rejecting on any error other than not-found. +func rejectSpaceChannelByID(c *Context, channelId string) bool { + _, err := c.App.GetChannelOfType(c.AppContext.With(app.RequestContextWithMaster), channelId, model.ChannelTypeSpace) + if err == nil { + c.Err = model.NewAppError("", "api.channel.space_channel.app_error", nil, "space channels cannot be accessed via /channels endpoints", http.StatusBadRequest) + return true + } + if err.StatusCode != http.StatusNotFound { + c.Err = err + return true + } + return false +} + func (api *API) InitChannel() { api.BaseRoutes.Channels.Handle("", api.APISessionRequired(getAllChannels)).Methods(http.MethodGet) api.BaseRoutes.Channels.Handle("", api.APISessionRequired(createChannel)).Methods(http.MethodPost) @@ -117,6 +135,11 @@ func createChannel(c *Context, w http.ResponseWriter, r *http.Request) { return } + if channel.IsSpace() { + c.SetInvalidParamWithDetails("type", "cannot create space channels via /channels endpoint") + return + } + license := c.App.Channels().License() if !channel.IsGroupOrDirect() && model.SafeDereference(c.App.Config().PrivacySettings.UseAnonymousURLs) && model.MinimumEnterpriseAdvancedLicense(license) { channel.Name = model.NewId() @@ -2121,6 +2144,10 @@ func updateChannelMemberRoles(c *Context, w http.ResponseWriter, r *http.Request return } + if rejectSpaceChannelByID(c, c.Params.ChannelId) { + return + } + props := model.MapFromJSON(r.Body) newRoles := props["roles"] @@ -2159,6 +2186,10 @@ func updateChannelMemberSchemeRoles(c *Context, w http.ResponseWriter, r *http.R return } + if rejectSpaceChannelByID(c, c.Params.ChannelId) { + return + } + var schemeRoles model.SchemeRoles if jsonErr := json.NewDecoder(r.Body).Decode(&schemeRoles); jsonErr != nil { c.SetInvalidParamWithErr("scheme_roles", jsonErr) @@ -2195,6 +2226,10 @@ func updateChannelMemberNotifyProps(c *Context, w http.ResponseWriter, r *http.R return } + if rejectSpaceChannelByID(c, c.Params.ChannelId) { + return + } + props := model.MapFromJSON(r.Body) if props == nil { c.SetInvalidParam("notify_props") @@ -2241,6 +2276,10 @@ func updateChannelMemberAutotranslation(c *Context, w http.ResponseWriter, r *ht return } + if rejectSpaceChannelByID(c, c.Params.ChannelId) { + return + } + props := UpdateChannelMemberAutotranslationProps{} if err := json.NewDecoder(r.Body).Decode(&props); err != nil { c.SetInvalidParamWithErr("autotranslation_disabled", err) @@ -2840,6 +2879,10 @@ func channelMembersMinusGroupMembers(c *Context, w http.ResponseWriter, r *http. return } + if rejectSpaceChannelByID(c, c.Params.ChannelId) { + return + } + groupIDsParam := groupIDsQueryParamRegex.ReplaceAllString(c.Params.GroupIDs, "") if len(groupIDsParam) < 26 { @@ -3145,6 +3188,10 @@ func convertGroupMessageToChannel(c *Context, w http.ResponseWriter, r *http.Req return } + if rejectSpaceChannelByID(c, c.Params.ChannelId) { + return + } + var gmConversionRequest *model.GroupMessageConversionRequestBody if err := json.NewDecoder(r.Body).Decode(&gmConversionRequest); err != nil || gmConversionRequest == nil { c.SetInvalidParamWithErr("body", err) diff --git a/server/channels/api4/channel_test.go b/server/channels/api4/channel_test.go index ba8340b4faa..a23b79a84ba 100644 --- a/server/channels/api4/channel_test.go +++ b/server/channels/api4/channel_test.go @@ -333,6 +333,28 @@ func TestCreateChannel(t *testing.T) { }) } +func TestCreateChannelRejectsSpaceType(t *testing.T) { + mainHelper.Parallel(t) + + for _, enableDocs := range []bool{false, true} { + t.Run(fmt.Sprintf("EnableDocs=%v", enableDocs), func(t *testing.T) { + th := SetupConfig(t, func(cfg *model.Config) { + cfg.FeatureFlags.EnableDocs = enableDocs + }).InitBasic(t) + + space := &model.Channel{ + DisplayName: "Space", + Name: "space-" + model.NewId(), + Type: model.ChannelTypeSpace, + TeamId: th.BasicTeam.Id, + } + _, resp, err := th.Client.CreateChannel(context.Background(), space) + require.Error(t, err) + CheckBadRequestStatus(t, resp) + }) + } +} + func TestCreateChannelManagedCategory(t *testing.T) { mainHelper.Parallel(t) th := SetupConfig(t, func(cfg *model.Config) { @@ -7795,6 +7817,382 @@ func TestChannelEndpointsExcludeBoards(t *testing.T) { }) } +// createTestSpaceChannel creates a ChannelTypeSpace ("S") backing channel directly through the +// store, since spaces have no dedicated REST creation endpoint in this slice. +func createTestSpaceChannel(t *testing.T, th *TestHelper) *model.Channel { + t.Helper() + space := &model.Channel{ + TeamId: th.BasicTeam.Id, + DisplayName: "Space", + Name: "space-" + model.NewId(), + Type: model.ChannelTypeSpace, + } + space, err := th.App.Srv().Store().Channel().Save(th.Context, space, -1) + require.NoError(t, err) + return space +} + +// addUserToSpaceChannel makes a user a member of a space backing channel. Membership is written +// directly through the store, since this slice has no app/REST path for adding members to a +// space, and the per-user authorization cache is invalidated so SessionHasPermissionToChannel +// sees the new membership. +func addUserToSpaceChannel(t *testing.T, th *TestHelper, space *model.Channel, userID string) { + t.Helper() + _, err := th.App.Srv().Store().Channel().SaveMember(th.Context, &model.ChannelMember{ + ChannelId: space.Id, + UserId: userID, + NotifyProps: model.GetDefaultChannelNotifyProps(), + SchemeUser: true, + }) + require.NoError(t, err) + th.App.Srv().Store().Channel().InvalidateAllChannelMembersForUser(userID) +} + +// TestChannelEndpointsExcludeSpaces mirrors TestChannelEndpointsExcludeBoards for the space ("S") +// backing-channel type: it 404s on every /channels endpoint and never leaks into any list, search, +// or by-name surface, except where rejectSpaceChannelByID guards a member-mutation endpoint or +// view-state marking no-ops through the store-level type filter. +func TestChannelEndpointsExcludeSpaces(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + client := th.Client + ctx := context.Background() + + space := createTestSpaceChannel(t, th) + + // BasicUser is a member of the space backing channel (e.g. a page collaborator). + addUserToSpaceChannel(t, th, space, th.BasicUser.Id) + + assertNoSpacesInList := func(t *testing.T, channels []*model.Channel) { + t.Helper() + for _, ch := range channels { + assert.NotEqual(t, model.ChannelTypeSpace, ch.Type, "space channel %s should not appear in channel list", ch.Id) + } + } + + // --- Space backing channels never leak into list/search surfaces --- + + t.Run("getPublicChannelsForTeam excludes spaces", func(t *testing.T) { + channels, _, err := client.GetPublicChannelsForTeam(ctx, th.BasicTeam.Id, 0, 100, "") + require.NoError(t, err) + assertNoSpacesInList(t, channels) + }) + + t.Run("getPrivateChannelsForTeam excludes spaces", func(t *testing.T) { + channels, _, err := th.SystemAdminClient.GetPrivateChannelsForTeam(ctx, th.BasicTeam.Id, 0, 100, "") + require.NoError(t, err) + assertNoSpacesInList(t, channels) + }) + + t.Run("getDeletedChannelsForTeam excludes spaces", func(t *testing.T) { + deletedSpace := createTestSpaceChannel(t, th) + nErr := th.App.Srv().Store().Channel().Delete(deletedSpace.Id, model.GetMillis()) + require.NoError(t, nErr) + + channels, _, err := th.SystemAdminClient.GetDeletedChannelsForTeam(ctx, th.BasicTeam.Id, 0, 100, "") + require.NoError(t, err) + assertNoSpacesInList(t, channels) + }) + + t.Run("searchChannels excludes spaces", func(t *testing.T) { + channels, _, err := client.SearchChannels(ctx, th.BasicTeam.Id, &model.ChannelSearch{Term: "space"}) + require.NoError(t, err) + assertNoSpacesInList(t, channels) + }) + + t.Run("autocompleteChannelsForTeam excludes spaces", func(t *testing.T) { + channels, _, err := client.AutocompleteChannelsForTeam(ctx, th.BasicTeam.Id, "space") + require.NoError(t, err) + assertNoSpacesInList(t, []*model.Channel(channels)) + }) + + t.Run("searchAllChannels excludes spaces", func(t *testing.T) { + channels, _, err := th.SystemAdminClient.SearchAllChannels(ctx, &model.ChannelSearch{Term: "space"}) + require.NoError(t, err) + for _, ch := range channels { + assert.NotEqual(t, model.ChannelTypeSpace, ch.Type, "space channel %s should not appear in searchAllChannels results", ch.Id) + } + }) + + t.Run("getAllChannels excludes spaces", func(t *testing.T) { + channels, _, err := th.SystemAdminClient.GetAllChannels(ctx, 0, 100, "") + require.NoError(t, err) + for _, ch := range channels { + assert.NotEqual(t, model.ChannelTypeSpace, ch.Type, "space channel %s should not appear in getAllChannels results", ch.Id) + } + }) + + t.Run("getChannelsForTeamForUser excludes spaces", func(t *testing.T) { + channels, _, err := client.GetChannelsForTeamForUser(ctx, th.BasicTeam.Id, th.BasicUser.Id, false, "") + require.NoError(t, err) + assertNoSpacesInList(t, channels) + }) + + t.Run("getChannelsForUser excludes spaces even for a member", func(t *testing.T) { + channels, _, err := client.GetChannelsForUserWithLastDeleteAt(ctx, th.BasicUser.Id, 0) + require.NoError(t, err) + assertNoSpacesInList(t, channels) + }) + + t.Run("getChannelsMemberCount excludes spaces", func(t *testing.T) { + counts, _, err := client.GetChannelsMemberCount(ctx, []string{th.BasicChannel.Id, space.Id}) + require.NoError(t, err) + _, hasRegular := counts[th.BasicChannel.Id] + assert.True(t, hasRegular, "regular channel should be in member count results") + _, hasSpace := counts[space.Id] + assert.False(t, hasSpace, "space backing channel should not be in member count results") + }) + + t.Run("getChannelByName 404s for space", func(t *testing.T) { + _, resp, err := client.GetChannelByName(ctx, space.Name, th.BasicTeam.Id, "") + require.Error(t, err) + CheckNotFoundStatus(t, resp) + }) + + t.Run("getChannelByNameForTeamName 404s for space", func(t *testing.T) { + _, resp, err := client.GetChannelByNameForTeamName(ctx, space.Name, th.BasicTeam.Name, "") + require.Error(t, err) + CheckNotFoundStatus(t, resp) + }) + + t.Run("searchAllChannelsForUser excludes spaces", func(t *testing.T) { + channels, _, err := client.SearchAllChannelsForUser(ctx, "space") + require.NoError(t, err) + for _, ch := range channels { + assert.NotEqual(t, model.ChannelTypeSpace, ch.Type, "space channel %s should not appear in searchAllChannelsForUser results", ch.Id) + } + }) + + // --- Like boards, a space backing channel is excluded from the generic getChannel and + // --- 404s for everyone; it resolves only through the dedicated space APIs. --- + + t.Run("getChannel 404s a space for a member", func(t *testing.T) { + _, resp, err := client.GetChannel(ctx, space.Id) + require.Error(t, err) + CheckNotFoundStatus(t, resp) + }) + + t.Run("getChannel 404s a space for system admin", func(t *testing.T) { + _, resp, err := th.SystemAdminClient.GetChannel(ctx, space.Id) + require.Error(t, err) + CheckNotFoundStatus(t, resp) + }) + + // --- Generic destructive/conversion endpoints 404 (Get-resolving) or reject (member-mutation) spaces --- + + t.Run("createChannel rejects a space", func(t *testing.T) { + _, resp, err := client.CreateChannel(ctx, &model.Channel{ + TeamId: th.BasicTeam.Id, + DisplayName: "Space", + Name: "space-" + model.NewId(), + Type: model.ChannelTypeSpace, + }) + require.Error(t, err) + CheckBadRequestStatus(t, resp) + }) + + t.Run("deleteChannel 404s a space", func(t *testing.T) { + resp, err := th.SystemAdminClient.DeleteChannel(ctx, space.Id) + require.Error(t, err) + CheckNotFoundStatus(t, resp) + }) + + t.Run("updateChannelPrivacy 404s a space", func(t *testing.T) { + _, resp, err := th.SystemAdminClient.UpdateChannelPrivacy(ctx, space.Id, model.ChannelTypeOpen) + require.Error(t, err) + CheckNotFoundStatus(t, resp) + }) + + t.Run("restoreChannel 404s a space", func(t *testing.T) { + _, resp, err := th.SystemAdminClient.RestoreChannel(ctx, space.Id) + require.Error(t, err) + CheckNotFoundStatus(t, resp) + }) + + t.Run("updateChannelScheme rejects a space", func(t *testing.T) { + originalLicense := th.App.Srv().License() + th.App.Srv().SetLicense(model.NewTestLicense("")) + defer th.App.Srv().SetLicense(originalLicense) + + err := th.App.SetPhase2PermissionsMigrationStatus(true) + require.NoError(t, err) + + channelScheme, _, err := th.SystemAdminClient.CreateScheme(ctx, &model.Scheme{ + DisplayName: "DisplayName", + Name: model.NewId(), + Description: "Some description", + Scope: model.SchemeScopeChannel, + }) + require.NoError(t, err) + + resp, err := th.SystemAdminClient.UpdateChannelScheme(ctx, space.Id, channelScheme.Id) + require.Error(t, err) + CheckNotFoundStatus(t, resp) + }) + + // --- Generic member/view-state mutation endpoints reject or no-op spaces (managed by the spaces feature) --- + + t.Run("viewChannel silently no-ops a space", func(t *testing.T) { + // Spaces carry no chat read-state: GetChannelsWithUnreadsAndWithMentions filters + // them out, so viewing one succeeds without marking anything viewed — the same + // behavior as a nonexistent channel ID. + viewResp, resp, err := client.ViewChannel(ctx, th.BasicUser.Id, &model.ChannelView{ChannelId: space.Id}) + require.NoError(t, err) + CheckOKStatus(t, resp) + require.NotContains(t, viewResp.LastViewedAtTimes, space.Id) + }) + + t.Run("updateChannelMemberRoles rejects a space", func(t *testing.T) { + resp, err := th.SystemAdminClient.UpdateChannelRoles(ctx, space.Id, th.BasicUser.Id, "channel_user") + require.Error(t, err) + CheckBadRequestStatus(t, resp) + }) + + t.Run("updateChannelMemberSchemeRoles rejects a space", func(t *testing.T) { + resp, err := th.SystemAdminClient.UpdateChannelMemberSchemeRoles(ctx, space.Id, th.BasicUser.Id, &model.SchemeRoles{SchemeUser: true}) + require.Error(t, err) + CheckBadRequestStatus(t, resp) + }) + + t.Run("updateChannelMemberNotifyProps rejects a space", func(t *testing.T) { + resp, err := client.UpdateChannelNotifyProps(ctx, space.Id, th.BasicUser.Id, map[string]string{model.MarkUnreadNotifyProp: model.ChannelMarkUnreadAll}) + require.Error(t, err) + CheckBadRequestStatus(t, resp) + }) + + t.Run("updateChannelMemberAutotranslation rejects a space", func(t *testing.T) { + mockAutotranslation := &einterfacesmocks.AutoTranslationInterface{} + mockAutotranslation.On("IsFeatureAvailable").Return(true) + originalAutoTranslation := th.Server.AutoTranslation + th.Server.AutoTranslation = mockAutotranslation + defer func() { + th.Server.AutoTranslation = originalAutoTranslation + }() + + resp, err := client.UpdateChannelMemberAutotranslation(ctx, space.Id, th.BasicUser.Id, true) + require.Error(t, err) + CheckBadRequestStatus(t, resp) + }) + + t.Run("addChannelMember 404s a space", func(t *testing.T) { + _, resp, err := th.SystemAdminClient.AddChannelMember(ctx, space.Id, th.BasicUser2.Id) + require.Error(t, err) + CheckNotFoundStatus(t, resp) + }) + + t.Run("setChannelMembers 404s a space", func(t *testing.T) { + _, resp, err := th.SystemAdminClient.SetChannelMembers(ctx, space.Id, &model.SetChannelMembersRequest{Members: []string{th.BasicUser2.Id}}, 0, 0) + require.Error(t, err) + CheckNotFoundStatus(t, resp) + }) + + t.Run("moveChannel 404s a space", func(t *testing.T) { + _, resp, err := th.SystemAdminClient.MoveChannel(ctx, space.Id, th.BasicTeam.Id, false) + require.Error(t, err) + CheckNotFoundStatus(t, resp) + }) + + // --- Generic read endpoints that resolve the channel via GetChannel return 403 for spaces, + // --- because GetChannel returns not-found for space channels, which SessionHasPermissionToChannel + // --- treats as no permission. No explicit space guard is needed here. --- + + t.Run("getChannelStats rejects a space", func(t *testing.T) { + _, resp, err := client.GetChannelStats(ctx, space.Id, "", false) + require.Error(t, err) + CheckForbiddenStatus(t, resp) + }) + + t.Run("getChannelMembers rejects a space", func(t *testing.T) { + _, resp, err := client.GetChannelMembers(ctx, space.Id, 0, 100, "") + require.Error(t, err) + CheckForbiddenStatus(t, resp) + }) + + t.Run("getChannelMembersTimezones rejects a space", func(t *testing.T) { + _, resp, err := client.GetChannelMembersTimezones(ctx, space.Id) + require.Error(t, err) + CheckForbiddenStatus(t, resp) + }) + + t.Run("getChannelMembersByIds rejects a space", func(t *testing.T) { + _, resp, err := client.GetChannelMembersByIds(ctx, space.Id, []string{th.BasicUser.Id}) + require.Error(t, err) + CheckForbiddenStatus(t, resp) + }) + + t.Run("getChannelMember rejects a space", func(t *testing.T) { + _, resp, err := client.GetChannelMember(ctx, space.Id, th.BasicUser.Id, "") + require.Error(t, err) + CheckForbiddenStatus(t, resp) + }) + + t.Run("channelMemberCountsByGroup rejects a space", func(t *testing.T) { + originalLicense := th.App.Srv().License() + th.App.Srv().SetLicense(model.NewTestLicense()) + defer th.App.Srv().SetLicense(originalLicense) + + _, resp, err := client.GetChannelMemberCountsByGroup(ctx, space.Id, false, "") + require.Error(t, err) + CheckForbiddenStatus(t, resp) + }) + + t.Run("channelMembersMinusGroupMembers rejects a space", func(t *testing.T) { + _, _, resp, err := th.SystemAdminClient.ChannelMembersMinusGroupMembers(ctx, space.Id, []string{model.NewId()}, 0, 100, "") + require.Error(t, err) + CheckBadRequestStatus(t, resp) + }) + + t.Run("getChannelAccessControlAttributes rejects a space", func(t *testing.T) { + r, err := client.DoAPIGet(ctx, "/channels/"+space.Id+"/access_control/attributes", "") + require.Error(t, err) + require.Equal(t, http.StatusForbidden, r.StatusCode) + }) + + t.Run("convertGroupMessageToChannel rejects a space", func(t *testing.T) { + resp, err := client.DoAPIPost(ctx, "/channels/"+space.Id+"/convert_to_channel", "{}") + require.Error(t, err) + require.Equal(t, http.StatusBadRequest, resp.StatusCode) + }) + + // --- rejectSpaceChannelByID passes through a not-found result (a nonexistent id is definitely + // --- not a space), so the endpoint's own not-found/permission handling surfaces normally --- + + t.Run("updateChannelMemberRoles on a nonexistent channel behaves as before", func(t *testing.T) { + resp, err := th.SystemAdminClient.UpdateChannelRoles(ctx, model.NewId(), th.BasicUser.Id, "channel_user") + require.Error(t, err) + CheckForbiddenStatus(t, resp) + }) +} + +// TestLocalChannelEndpoints404Spaces verifies the local (admin socket) channel endpoints 404 a +// space, the same as /api/v4: space channels are excluded from the generic GetChannel these +// handlers resolve through, so they never reach the mutation. +func TestLocalChannelEndpoints404Spaces(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + ctx := context.Background() + + space := createTestSpaceChannel(t, th) + + t.Run("local deleteChannel 404s a space", func(t *testing.T) { + resp, err := th.LocalClient.DeleteChannel(ctx, space.Id) + require.Error(t, err) + CheckNotFoundStatus(t, resp) + }) + + t.Run("local restoreChannel 404s a space", func(t *testing.T) { + _, resp, err := th.LocalClient.RestoreChannel(ctx, space.Id) + require.Error(t, err) + CheckNotFoundStatus(t, resp) + }) + + t.Run("local addChannelMember 404s a space", func(t *testing.T) { + _, resp, err := th.LocalClient.AddChannelMember(ctx, space.Id, th.BasicUser2.Id) + require.Error(t, err) + CheckNotFoundStatus(t, resp) + }) +} + func TestSetChannelMembers(t *testing.T) { mainHelper.Parallel(t) th := Setup(t).InitBasic(t) diff --git a/server/channels/app/channel.go b/server/channels/app/channel.go index 551b1912a7d..c80952254d7 100644 --- a/server/channels/app/channel.go +++ b/server/channels/app/channel.go @@ -165,6 +165,10 @@ func (a *App) CreateChannelWithUser(rctx request.CTX, channel *model.Channel, us return nil, model.NewAppError("CreateChannelWithUser", "app.channel.create_channel.board_type.app_error", nil, "use CreateBoardChannel instead", http.StatusBadRequest) } + if channel.IsSpace() { + return nil, model.NewAppError("CreateChannelWithUser", "app.channel.create_channel.space_type.app_error", nil, "use CreateChannel instead", http.StatusBadRequest) + } + if channel.TeamId == "" { return nil, model.NewAppError("CreateChannelWithUser", "app.channel.create_channel.no_team_id.app_error", nil, "", http.StatusBadRequest) } @@ -237,6 +241,10 @@ func (a *App) CreateChannel(rctx request.CTX, channel *model.Channel, addMember return nil, model.NewAppError("CreateChannel", "app.channel.create_channel.board_type.app_error", nil, "use CreateBoardChannel instead", http.StatusBadRequest) } + if channel.IsSpace() && !a.Config().FeatureFlags.EnableDocs { + return nil, model.NewAppError("CreateChannel", "app.channel.create_channel.spaces_not_enabled.app_error", nil, "", http.StatusForbidden) + } + channel.DisplayName = strings.TrimSpace(channel.DisplayName) channel.DefaultCategoryName = strings.TrimSpace(channel.DefaultCategoryName) channel.ManagedCategoryName = strings.TrimSpace(channel.ManagedCategoryName) @@ -326,6 +334,10 @@ func (a *App) CreateChannel(rctx request.CTX, channel *model.Channel, addMember } } + if sc.IsSpace() { + return sc, nil + } + a.Srv().Go(func() { pluginContext := pluginContext(rctx) a.ch.RunMultiHook(func(hooks plugin.Hooks, _ *model.Manifest) bool { @@ -737,7 +749,18 @@ func (a *App) GetGroupChannel(rctx request.CTX, userIDs []string) (*model.Channe // UpdateChannel updates a given channel by its Id. It also publishes the CHANNEL_UPDATED event. func (a *App) UpdateChannel(rctx request.CTX, channel *model.Channel) (*model.Channel, *model.AppError) { - oldChannel, getErr := a.Srv().Store().Channel().Get(channel.Id, true) + // The generic Get excludes spaces, so fetch a space by its exact type instead; otherwise + // UpdateChannel can't load the existing channel and a rename or header edit would fail with + // a not-found before it reaches the store. + // Read from master: spaces are uncached, so an update right after create would otherwise + // miss against a lagging replica. + var oldChannel *model.Channel + var getErr error + if channel.IsSpace() { + oldChannel, getErr = a.Srv().Store().Channel().GetChannelOfType(RequestContextWithMaster(rctx), channel.Id, model.ChannelTypeSpace) + } else { + oldChannel, getErr = a.Srv().Store().Channel().Get(channel.Id, true) + } if getErr != nil { errCtx := map[string]any{"channel_id": channel.Id} var nfErr *store.ErrNotFound @@ -749,34 +772,38 @@ func (a *App) UpdateChannel(rctx request.CTX, channel *model.Channel) (*model.Ch } } - enforced, appErr := a.ChannelAccessControlled(rctx, channel.Id) - if appErr != nil { - return nil, appErr - } - if enforced { - if channel.Type != model.ChannelTypePrivate && channel.Type != model.ChannelTypeOpen { - return nil, model.NewAppError("UpdateChannel", "api.channel.update_channel.not_allowed.app_error", nil, "", http.StatusForbidden) + // Space backing channels are internal: skip ABAC enforcement and the plugin + // ChannelWillBeUpdated hook, matching the other space lifecycle paths. + if !channel.IsSpace() { + enforced, appErr := a.ChannelAccessControlled(rctx, channel.Id) + if appErr != nil { + return nil, appErr + } + if enforced { + if channel.Type != model.ChannelTypePrivate && channel.Type != model.ChannelTypeOpen { + return nil, model.NewAppError("UpdateChannel", "api.channel.update_channel.not_allowed.app_error", nil, "", http.StatusForbidden) + } + + // Block public ↔ private conversion while an ABAC policy is attached. + // Public-channel and private-channel ABAC have asymmetric semantics + // (advisory recommend/auto-add vs hard-gate with member removal); a + // silent type flip would change what the existing policy actually + // does to members. The admin must remove the policy first and + // re-apply it after the conversion if they still want it. + if oldChannel.Type != channel.Type { + return nil, model.NewAppError("UpdateChannel", + "api.channel.update_channel.policy_enforced_type_conversion.app_error", + nil, "channel has an active ABAC policy; remove the policy before converting between public and private", http.StatusBadRequest) + } } - // Block public ↔ private conversion while an ABAC policy is attached. - // Public-channel and private-channel ABAC have asymmetric semantics - // (advisory recommend/auto-add vs hard-gate with member removal); a - // silent type flip would change what the existing policy actually - // does to members. The admin must remove the policy first and - // re-apply it after the conversion if they still want it. - if oldChannel.Type != channel.Type { - return nil, model.NewAppError("UpdateChannel", - "api.channel.update_channel.policy_enforced_type_conversion.app_error", - nil, "channel has an active ABAC policy; remove the policy before converting between public and private", http.StatusBadRequest) + var channelErr *model.AppError + channel, channelErr = a.runGuardedChannelWillBeUpdated(rctx, channel, oldChannel) + if channelErr != nil { + return nil, channelErr } } - var channelErr *model.AppError - channel, channelErr = a.runGuardedChannelWillBeUpdated(rctx, channel, oldChannel) - if channelErr != nil { - return nil, channelErr - } - _, err := a.Srv().Store().Channel().Update(rctx, channel) if err != nil { var appErr *model.AppError @@ -796,6 +823,11 @@ func (a *App) UpdateChannel(rctx request.CTX, channel *model.Channel) (*model.Ch a.Srv().Platform().InvalidateCacheForChannel(channel) + // Space backing channels are internal: skip the channel_updated broadcast. + if channel.IsSpace() { + return channel, nil + } + messageWs := model.NewWebSocketEvent(model.WebsocketEventChannelUpdated, "", channel.Id, "", nil, "") channelJSON, jsonErr := json.Marshal(channel) if jsonErr != nil { @@ -849,6 +881,10 @@ func (a *App) UpdateChannelScheme(rctx request.CTX, channel *model.Channel) (*mo } func (a *App) UpdateChannelPrivacy(rctx request.CTX, oldChannel *model.Channel, user *model.User) (*model.Channel, *model.AppError) { + if oldChannel.IsSpace() { + return nil, model.NewAppError("UpdateChannelPrivacy", "app.channel.update_channel_privacy.space.app_error", nil, "", http.StatusBadRequest) + } + wasDiscoverable := oldChannel.Discoverable // Public channels are inherently joinable; the discoverable flag only // has meaning for private channels. Clear it eagerly so callers reading @@ -946,8 +982,11 @@ func (a *App) RestoreChannel(rctx request.CTX, channel *model.Channel, userID st return nil, model.NewAppError("restoreChannel", "api.channel.restore_channel.restored.app_error", nil, "", http.StatusBadRequest) } - if appErr := a.runGuardedChannelWillBeRestored(rctx, channel); appErr != nil { - return nil, appErr + // Space backing channels are internal; plugin ChannelWillBeRestored hooks are skipped. + if !channel.IsSpace() { + if appErr := a.runGuardedChannelWillBeRestored(rctx, channel); appErr != nil { + return nil, appErr + } } if err := a.Srv().Store().Channel().Restore(channel.Id, model.GetMillis()); err != nil { @@ -956,6 +995,11 @@ func (a *App) RestoreChannel(rctx request.CTX, channel *model.Channel, userID st channel.DeleteAt = 0 a.Srv().Platform().InvalidateCacheForChannel(channel) + // Space backing channels are internal: skip the channel_restored chat event and system post. + if channel.IsSpace() { + return channel, nil + } + var message *model.WebSocketEvent if channel.Type == model.ChannelTypeOpen { message = model.NewWebSocketEvent(model.WebsocketEventChannelRestored, channel.TeamId, "", "", nil, "") @@ -1693,16 +1737,19 @@ func (a *App) DeleteChannel(rctx request.CTX, channel *model.Channel, userID str return err } - var archiveRejectionReason string - pluginContext := pluginContext(rctx) - a.ch.RunMultiHook(func(hooks plugin.Hooks, _ *model.Manifest) bool { - archiveRejectionReason = hooks.ChannelWillBeArchived(pluginContext, channel) - return archiveRejectionReason == "" - }, plugin.ChannelWillBeArchivedID) + // Space backing channels are internal; plugin ChannelWillBeArchived hooks are skipped. + if !channel.IsSpace() { + var archiveRejectionReason string + pluginContext := pluginContext(rctx) + a.ch.RunMultiHook(func(hooks plugin.Hooks, _ *model.Manifest) bool { + archiveRejectionReason = hooks.ChannelWillBeArchived(pluginContext, channel) + return archiveRejectionReason == "" + }, plugin.ChannelWillBeArchivedID) - if archiveRejectionReason != "" { - return model.NewAppError("DeleteChannel", "app.channel.delete_channel.rejected_by_plugin", - map[string]any{"Reason": archiveRejectionReason}, "", http.StatusBadRequest) + if archiveRejectionReason != "" { + return model.NewAppError("DeleteChannel", "app.channel.delete_channel.rejected_by_plugin", + map[string]any{"Reason": archiveRejectionReason}, "", http.StatusBadRequest) + } } deleteAt := model.GetMillis() @@ -1711,40 +1758,43 @@ func (a *App) DeleteChannel(rctx request.CTX, channel *model.Channel, userID str return model.NewAppError("DeleteChannel", "app.channel.delete.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } - if user != nil { - T := i18n.GetUserTranslations(user.Locale) + // Space backing channels are internal and skip the archive system post. + if !channel.IsSpace() { + if user != nil { + T := i18n.GetUserTranslations(user.Locale) - post := &model.Post{ - ChannelId: channel.Id, - Message: fmt.Sprintf(T("api.channel.delete_channel.archived"), user.Username), - Type: model.PostTypeChannelDeleted, - UserId: userID, - Props: model.StringInterface{ - "username": user.Username, - }, - } - - if _, _, err := a.CreatePost(rctx, post, channel, model.CreatePostFlags{SetOnline: true}); err != nil { - rctx.Logger().Warn("Failed to post archive message", mlog.Err(err)) - } - } else { - systemBot, err := a.GetSystemBot(rctx) - if err != nil { - rctx.Logger().Warn("Failed to post archive message", mlog.Err(err)) - } else { post := &model.Post{ ChannelId: channel.Id, - Message: fmt.Sprintf(i18n.T("api.channel.delete_channel.archived"), systemBot.Username), + Message: fmt.Sprintf(T("api.channel.delete_channel.archived"), user.Username), Type: model.PostTypeChannelDeleted, - UserId: systemBot.UserId, + UserId: userID, Props: model.StringInterface{ - "username": systemBot.Username, + "username": user.Username, }, } if _, _, err := a.CreatePost(rctx, post, channel, model.CreatePostFlags{SetOnline: true}); err != nil { rctx.Logger().Warn("Failed to post archive message", mlog.Err(err)) } + } else { + systemBot, err := a.GetSystemBot(rctx) + if err != nil { + rctx.Logger().Warn("Failed to post archive message", mlog.Err(err)) + } else { + post := &model.Post{ + ChannelId: channel.Id, + Message: fmt.Sprintf(i18n.T("api.channel.delete_channel.archived"), systemBot.Username), + Type: model.PostTypeChannelDeleted, + UserId: systemBot.UserId, + Props: model.StringInterface{ + "username": systemBot.Username, + }, + } + + if _, _, err := a.CreatePost(rctx, post, channel, model.CreatePostFlags{SetOnline: true}); err != nil { + rctx.Logger().Warn("Failed to post archive message", mlog.Err(err)) + } + } } } @@ -1774,6 +1824,10 @@ func (a *App) DeleteChannel(rctx request.CTX, channel *model.Channel, userID str a.Srv().Platform().InvalidateCacheForChannel(channel) + if channel.IsSpace() { + return nil + } + var message *model.WebSocketEvent if channel.Type == model.ChannelTypeOpen { message = model.NewWebSocketEvent(model.WebsocketEventChannelDeleted, channel.TeamId, "", "", nil, "") @@ -1788,7 +1842,7 @@ func (a *App) DeleteChannel(rctx request.CTX, channel *model.Channel, userID str } func (a *App) addUserToChannel(rctx request.CTX, user *model.User, channel *model.Channel) (*model.ChannelMember, *model.AppError) { - if channel.Type != model.ChannelTypeOpen && channel.Type != model.ChannelTypePrivate { + if channel.Type != model.ChannelTypeOpen && channel.Type != model.ChannelTypePrivate && !channel.IsSpace() { return nil, model.NewAppError("AddUserToChannel", "api.channel.add_user_to_channel.type.app_error", nil, "", http.StatusBadRequest) } @@ -1858,9 +1912,11 @@ func (a *App) addUserToChannel(rctx request.CTX, user *model.User, channel *mode } var channelMemberErr *model.AppError - newMember, channelMemberErr = a.runGuardedChannelMemberWillBeAdded(rctx, channel.Id, newMember) - if channelMemberErr != nil { - return nil, channelMemberErr + if !channel.IsSpace() { + newMember, channelMemberErr = a.runGuardedChannelMemberWillBeAdded(rctx, channel.Id, newMember) + if channelMemberErr != nil { + return nil, channelMemberErr + } } newMember, nErr = a.Srv().Store().Channel().SaveMember(rctx, newMember) @@ -1910,6 +1966,10 @@ func (a *App) AddUserToChannel(rctx request.CTX, user *model.User, channel *mode return nil, err } + if channel.IsSpace() { + return newMember, nil + } + a.addChannelToDefaultCategory(rctx, user.Id, channel) // We are sending separate websocket events to the user added and to the channel @@ -1971,6 +2031,10 @@ func (a *App) AddChannelMember(rctx request.CTX, userID string, channel *model.C return nil, err } + if channel.IsSpace() { + return cm, nil + } + a.Srv().Go(func() { pluginContext := pluginContext(rctx) a.ch.RunMultiHook(func(hooks plugin.Hooks, _ *model.Manifest) bool { @@ -2187,6 +2251,23 @@ func (a *App) GetBoardChannel(rctx request.CTX, channelID string) (*model.Channe return channel, nil } +// GetChannelOfType resolves a channel by ID, requiring it to be of the given type. Generic +// channel access goes through GetChannel, which excludes opaque backing channel types (e.g. +// space); callers that legitimately need such a channel ask for it by its exact type here. +func (a *App) GetChannelOfType(rctx request.CTX, channelID string, channelType model.ChannelType) (*model.Channel, *model.AppError) { + channel, err := a.Srv().Store().Channel().GetChannelOfType(rctx, channelID, channelType) + if err != nil { + var nfErr *store.ErrNotFound + switch { + case errors.As(err, &nfErr): + return nil, model.NewAppError("GetChannelOfType", "app.channel.get.existing.app_error", map[string]any{"channel_id": channelID}, "", http.StatusNotFound).Wrap(err) + default: + return nil, model.NewAppError("GetChannelOfType", "app.channel.get.find.app_error", map[string]any{"channel_id": channelID}, "", http.StatusInternalServerError).Wrap(err) + } + } + return channel, nil +} + func (s *Server) getChannel(rctx request.CTX, channelID string) (*model.Channel, *model.AppError) { channel, err := s.Store().Channel().Get(channelID, true) if err != nil { @@ -2958,18 +3039,26 @@ func (a *App) removeUserFromChannel(rctx request.CTX, userIDToRemove string, rem if err != nil { return err } + // GetChannelMembersForUser excludes space backing channels. A guest still in a space + // belongs to the team, so count space memberships before evicting them from the team. if len(currentMembers) == 0 { - teamMember, err := a.GetTeamMember(rctx, channel.TeamId, userIDToRemove) - if err != nil { - return model.NewAppError("removeUserFromChannel", "api.team.remove_user_from_team.missing.app_error", nil, "", http.StatusBadRequest).Wrap(err) + spaceChannels, sErr := a.Srv().Store().Channel().GetTeamSpaceChannelsForUser(channel.TeamId, userIDToRemove) + if sErr != nil { + return model.NewAppError("removeUserFromChannel", "app.channel.get_channels.get.app_error", nil, "", http.StatusInternalServerError).Wrap(sErr) } + if len(spaceChannels) == 0 { + teamMember, err := a.GetTeamMember(rctx, channel.TeamId, userIDToRemove) + if err != nil { + return model.NewAppError("removeUserFromChannel", "api.team.remove_user_from_team.missing.app_error", nil, "", http.StatusBadRequest).Wrap(err) + } - if err := a.ch.srv.teamService.RemoveTeamMember(rctx, teamMember); err != nil { - return model.NewAppError("removeUserFromChannel", "api.team.remove_user_from_team.missing.app_error", nil, "", http.StatusBadRequest).Wrap(err) - } + if err := a.ch.srv.teamService.RemoveTeamMember(rctx, teamMember); err != nil { + return model.NewAppError("removeUserFromChannel", "api.team.remove_user_from_team.missing.app_error", nil, "", http.StatusBadRequest).Wrap(err) + } - if err = a.postProcessTeamMemberLeave(rctx, teamMember, removerUserId); err != nil { - return err + if err = a.postProcessTeamMemberLeave(rctx, teamMember, removerUserId); err != nil { + return err + } } } } @@ -2979,6 +3068,11 @@ func (a *App) removeUserFromChannel(rctx request.CTX, userIDToRemove string, rem a.Srv().Store().AutoTranslation().InvalidateUserAutoTranslation(userIDToRemove, channel.Id) a.Srv().Store().AutoTranslation().InvalidateUserLocaleCache(userIDToRemove) + // Space backing channels are internal: skip the user_removed chat events and plugin hook. + if channel.IsSpace() { + return nil + } + var actorUser *model.User if removerUserId != "" { actorUser, _ = a.GetUser(removerUserId) @@ -3020,6 +3114,11 @@ func (a *App) RemoveUserFromChannel(rctx request.CTX, userIDToRemove string, rem return err } + // Space backing channels are internal: skip the leave/remove system post. + if channel.IsSpace() { + return nil + } + var user *model.User if user, err = a.GetUser(userIDToRemove); err != nil { return err @@ -3690,6 +3789,10 @@ func (a *App) PermanentDeleteChannel(rctx request.CTX, channel *model.Channel) * a.Srv().Platform().InvalidateCacheForChannel(channel) + if channel.IsSpace() { + return nil + } + var message *model.WebSocketEvent if channel.Type == model.ChannelTypeOpen { message = model.NewWebSocketEvent(model.WebsocketEventChannelDeleted, channel.TeamId, "", "", nil, "") @@ -3715,6 +3818,10 @@ func (a *App) RemoveAllDeactivatedMembersFromChannel(rctx request.CTX, channel * // MoveChannel method is prone to data races if someone joins to channel during the move process. However this // function is only exposed to sysadmins and the possibility of this edge case is relatively small. func (a *App) MoveChannel(rctx request.CTX, team *model.Team, channel *model.Channel, user *model.User) *model.AppError { + if channel.IsSpace() { + return model.NewAppError("MoveChannel", "app.channel.move_channel.space.app_error", nil, "", http.StatusForbidden) + } + // Check that all channel members are in the destination team. channelMembers, err := a.GetChannelMembersPage(rctx, channel.Id, 0, 10000000) if err != nil { diff --git a/server/channels/app/channel_test.go b/server/channels/app/channel_test.go index 5238b2f16bd..787e4cee00f 100644 --- a/server/channels/app/channel_test.go +++ b/server/channels/app/channel_test.go @@ -505,6 +505,57 @@ func TestCreateChannelDisplayNameTrimsWhitespace(t *testing.T) { require.Equal(t, channel.DisplayName, "Public 1") } +func TestCreateChannelSpaceRequiresEnableDocs(t *testing.T) { + mainHelper.Parallel(t) + + newSpace := func(teamID string) *model.Channel { + return &model.Channel{ + DisplayName: "Space", + Name: "space-" + model.NewId(), + Type: model.ChannelTypeSpace, + TeamId: teamID, + } + } + + t.Run("CreateChannel rejects a space channel when EnableDocs is off", func(t *testing.T) { + th := SetupConfig(t, func(cfg *model.Config) { + cfg.FeatureFlags.EnableDocs = false + }).InitBasic(t) + + _, appErr := th.App.CreateChannel(th.Context, newSpace(th.BasicTeam.Id), false) + require.NotNil(t, appErr) + assert.Equal(t, "app.channel.create_channel.spaces_not_enabled.app_error", appErr.Id) + assert.Equal(t, http.StatusForbidden, appErr.StatusCode) + }) + + t.Run("CreateChannel allows a space channel when EnableDocs is on", func(t *testing.T) { + th := SetupConfig(t, func(cfg *model.Config) { + cfg.FeatureFlags.EnableDocs = true + }).InitBasic(t) + + channel, appErr := th.App.CreateChannel(th.Context, newSpace(th.BasicTeam.Id), false) + require.Nil(t, appErr) + defer func() { + require.NoError(t, th.App.Srv().Store().Channel().PermanentDelete(th.Context, channel.Id)) + }() + assert.Equal(t, model.ChannelTypeSpace, channel.Type) + }) + + t.Run("CreateChannelWithUser rejects a space channel", func(t *testing.T) { + // Space backing channels are created through CreateChannel (the docs plugin path), + // never CreateChannelWithUser, which applies chat semantics (sidebar category, join + // post, channel_created event) that do not belong on an internal backing channel. + th := SetupConfig(t, func(cfg *model.Config) { + cfg.FeatureFlags.EnableDocs = true + }).InitBasic(t) + + _, appErr := th.App.CreateChannelWithUser(th.Context, newSpace(th.BasicTeam.Id), th.BasicUser.Id) + require.NotNil(t, appErr) + assert.Equal(t, "app.channel.create_channel.space_type.app_error", appErr.Id) + assert.Equal(t, http.StatusBadRequest, appErr.StatusCode) + }) +} + func TestUpdateChannelPrivacy(t *testing.T) { mainHelper.Parallel(t) th := Setup(t).InitBasic(t) @@ -518,6 +569,24 @@ func TestUpdateChannelPrivacy(t *testing.T) { assert.Equal(t, publicChannel.Type, model.ChannelTypeOpen) } +func TestUpdateChannelPrivacyRejectsSpace(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + space, err := th.GetSqlStore().Channel().Save(th.Context, &model.Channel{ + TeamId: th.BasicTeam.Id, + DisplayName: "Space", + Name: "space-" + model.NewId(), + Type: model.ChannelTypeSpace, + }, -1) + require.NoError(t, err) + + _, appErr := th.App.UpdateChannelPrivacy(th.Context, space, th.BasicUser) + require.NotNil(t, appErr) + assert.Equal(t, "app.channel.update_channel_privacy.space.app_error", appErr.Id) + assert.Equal(t, http.StatusBadRequest, appErr.StatusCode) +} + func TestUpdateChannelPrivacyWebSocketEvent(t *testing.T) { mainHelper.Parallel(t) th := Setup(t).InitBasic(t) @@ -1016,6 +1085,44 @@ func TestLeaveLastChannel(t *testing.T) { }) } +func TestLeaveLastChannelGuestStillInSpace(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + guest := th.CreateGuest(t) + th.LinkUserToTeam(t, guest, th.BasicTeam) + + townSquare, appErr := th.App.GetChannelByName(th.Context, "town-square", th.BasicTeam.Id, false) + require.Nil(t, appErr) + th.AddUserToChannel(t, guest, townSquare) + th.AddUserToChannel(t, guest, th.BasicChannel) + + // The guest also belongs to a space backing channel, which GetChannelMembersForUser excludes. + // Leaving every chat channel must not evict them from the team while a space membership remains. + space, nErr := th.App.Srv().Store().Channel().Save(th.Context, &model.Channel{ + TeamId: th.BasicTeam.Id, + DisplayName: "Space", + Name: "space-" + model.NewId(), + Type: model.ChannelTypeSpace, + }, -1) + require.NoError(t, nErr) + _, nErr = th.App.Srv().Store().Channel().SaveMember(th.Context, &model.ChannelMember{ + ChannelId: space.Id, + UserId: guest.Id, + NotifyProps: model.GetDefaultChannelNotifyProps(), + SchemeGuest: true, + }) + require.NoError(t, nErr) + + appErr = th.App.LeaveChannel(th.Context, townSquare.Id, guest.Id) + require.Nil(t, appErr) + appErr = th.App.LeaveChannel(th.Context, th.BasicChannel.Id, guest.Id) + require.Nil(t, appErr) + + _, appErr = th.App.GetTeamMember(th.Context, th.BasicTeam.Id, guest.Id) + assert.Nil(t, appErr, "guest still in a space should keep the team membership") +} + func TestAddChannelMemberNoUserRequestor(t *testing.T) { mainHelper.Parallel(t) th := Setup(t).InitBasic(t) diff --git a/server/channels/app/plugin_api.go b/server/channels/app/plugin_api.go index 2c709a06875..2c39b95dac9 100644 --- a/server/channels/app/plugin_api.go +++ b/server/channels/app/plugin_api.go @@ -467,7 +467,8 @@ func (api *PluginAPI) GetLDAPUserAttributes(userID string, attributes []string) func (api *PluginAPI) CreateChannel(channel *model.Channel) (*model.Channel, *model.AppError) { UseAnonymousURLs := model.SafeDereference(api.app.Config().PrivacySettings.UseAnonymousURLs) && model.MinimumEnterpriseAdvancedLicense(api.app.License()) - if !channel.IsGroupOrDirect() && UseAnonymousURLs { + // Space backing channels have system-assigned names, not user-visible URLs — anonymous-URL scrambling does not apply. + if !channel.IsGroupOrDirect() && !channel.IsSpace() && UseAnonymousURLs { channel.Name = model.NewId() } @@ -475,13 +476,22 @@ func (api *PluginAPI) CreateChannel(channel *model.Channel) (*model.Channel, *mo } func (api *PluginAPI) DeleteChannel(channelID string) *model.AppError { - channel, err := api.app.GetChannel(api.ctx, channelID) + channel, err := api.resolveChannel(channelID) if err != nil { return err } return api.app.DeleteChannel(api.ctx, channel, "") } +func (api *PluginAPI) RestoreChannel(channelID string) *model.AppError { + channel, err := api.resolveChannel(channelID) + if err != nil { + return err + } + _, err = api.app.RestoreChannel(api.ctx, channel, "") + return err +} + func (api *PluginAPI) GetPublicChannelsForTeam(teamID string, page, perPage int) ([]*model.Channel, *model.AppError) { channels, err := api.app.GetPublicChannelsForTeam(api.ctx, teamID, page*perPage, perPage) if err != nil { @@ -494,6 +504,58 @@ func (api *PluginAPI) GetChannel(channelID string) (*model.Channel, *model.AppEr return api.app.GetChannel(api.ctx, channelID) } +// GetChannelOfType resolves a channel by ID, requiring it to be of the given type. Generic +// GetChannel excludes opaque backing channel types (e.g. space); plugins that manage such a +// channel resolve it by its exact type here. +func (api *PluginAPI) GetChannelOfType(channelID string, channelType model.ChannelType) (*model.Channel, *model.AppError) { + ctx := api.ctx + // Opaque backing types (e.g. space) are uncached and created without waiting for replica + // replication, so a plugin resolving one immediately after creating it could miss it on a + // lagging replica; read those from master. Cached message-bearing types stay on the replica. + if channelType == model.ChannelTypeSpace { + ctx = RequestContextWithMaster(api.ctx) + } + return api.app.GetChannelOfType(ctx, channelID, channelType) +} + +// resolveChannel fetches a channel by ID for plugin API mutation methods. Generic channel +// lookups exclude opaque backing channel types (e.g. space); on 404 this retries the ID +// against each known backing type so plugins can manage backing channels through the standard +// API without a separate resolution step. The retry uses master context to avoid +// read-after-write misses on lagging replicas. +func (api *PluginAPI) resolveChannel(channelID string) (*model.Channel, *model.AppError) { + return resolveChannelByID( + api.ctx, + channelID, + api.app.GetChannel, + api.app.GetChannelOfType, + ) +} + +// resolveChannelByID is the testable core of resolveChannel. +// It is a package-level function so tests can inject controlled fetch functions. +func resolveChannelByID( + rctx request.CTX, + channelID string, + getChannel func(request.CTX, string) (*model.Channel, *model.AppError), + getChannelOfType func(request.CTX, string, model.ChannelType) (*model.Channel, *model.AppError), +) (*model.Channel, *model.AppError) { + channel, err := getChannel(rctx, channelID) + if err == nil { + return channel, nil + } + if err.StatusCode != http.StatusNotFound { + return nil, err + } + // Generic lookup excludes backing channel types; retry as each known backing type. + if spaceChannel, spErr := getChannelOfType(RequestContextWithMaster(rctx), channelID, model.ChannelTypeSpace); spErr == nil { + return spaceChannel, nil + } else if spErr.StatusCode != http.StatusNotFound { + return nil, spErr + } + return nil, err +} + func (api *PluginAPI) GetChannelByName(teamID, name string, includeDeleted bool) (*model.Channel, *model.AppError) { return api.app.GetChannelByName(api.ctx, name, teamID, includeDeleted) } @@ -621,12 +683,16 @@ func (api *PluginAPI) SearchPostsInTeamForUser(teamID string, userID string, sea } func (api *PluginAPI) AddChannelMember(channelID, userID string) (*model.ChannelMember, *model.AppError) { - channel, err := api.GetChannel(channelID) + channel, err := api.resolveChannel(channelID) if err != nil { return nil, err } + ctx := api.ctx + if channel.IsSpace() { + ctx = RequestContextWithMaster(api.ctx) + } - return api.app.AddChannelMember(api.ctx, userID, channel, ChannelMemberOpts{ + return api.app.AddChannelMember(ctx, userID, channel, ChannelMemberOpts{ // For now, don't allow overriding these via the plugin API. UserRequestorID: "", PostRootID: "", @@ -634,12 +700,16 @@ func (api *PluginAPI) AddChannelMember(channelID, userID string) (*model.Channel } func (api *PluginAPI) AddUserToChannel(channelID, userID, asUserID string) (*model.ChannelMember, *model.AppError) { - channel, err := api.GetChannel(channelID) + channel, err := api.resolveChannel(channelID) if err != nil { return nil, err } + ctx := api.ctx + if channel.IsSpace() { + ctx = RequestContextWithMaster(api.ctx) + } - return api.app.AddChannelMember(api.ctx, userID, channel, ChannelMemberOpts{ + return api.app.AddChannelMember(ctx, userID, channel, ChannelMemberOpts{ UserRequestorID: asUserID, }) } @@ -667,15 +737,46 @@ func (api *PluginAPI) UpdateChannelMemberRoles(channelID, userID, newRoles strin } func (api *PluginAPI) UpdateChannelMemberNotifications(channelID, userID string, notifications map[string]string) (*model.ChannelMember, *model.AppError) { + if appErr := api.rejectSpaceChannel(channelID); appErr != nil { + return nil, appErr + } return api.app.UpdateChannelMemberNotifyProps(api.ctx, notifications, channelID, userID) } func (api *PluginAPI) PatchChannelMembersNotifications(members []*model.ChannelMemberIdentifier, notifications map[string]string) *model.AppError { + for _, member := range members { + if appErr := api.rejectSpaceChannel(member.ChannelId); appErr != nil { + return appErr + } + } _, err := api.app.PatchChannelMembersNotifyProps(api.ctx, members, notifications) return err } +// rejectSpaceChannel returns a bad-request AppError when channelID is a space backing channel. +// Notify-prop mutations carry chat semantics (they emit a channel_member_updated event) that do +// not belong on an internal space backing channel, so they are rejected here. It fails closed by +// propagating any error other than not-found, so a lookup failure cannot let a space through. +func (api *PluginAPI) rejectSpaceChannel(channelID string) *model.AppError { + _, err := api.app.GetChannelOfType(RequestContextWithMaster(api.ctx), channelID, model.ChannelTypeSpace) + if err == nil { + return model.NewAppError("PluginAPI.rejectSpaceChannel", "plugin_api.channel.space_notify_props.app_error", nil, "", http.StatusBadRequest) + } + if err.StatusCode != http.StatusNotFound { + return err + } + return nil +} + func (api *PluginAPI) DeleteChannelMember(channelID, userID string) *model.AppError { + channel, err := api.resolveChannel(channelID) + if err != nil { + return err + } + if channel.IsSpace() { + // Space backing channels resolve outside LeaveChannel's generic GetChannel; remove directly. + return api.app.RemoveUserFromChannel(RequestContextWithMaster(api.ctx), userID, userID, channel) + } return api.app.LeaveChannel(api.ctx, channelID, userID) } diff --git a/server/channels/app/plugin_api_test.go b/server/channels/app/plugin_api_test.go index f5ecf5e8009..a1be73a4933 100644 --- a/server/channels/app/plugin_api_test.go +++ b/server/channels/app/plugin_api_test.go @@ -3799,6 +3799,366 @@ func TestPluginAPICreateChannelManagedCategory(t *testing.T) { assert.Equal(t, categoryName, mappings[createdChannel.Id]) } +func TestPluginAPICreateSpaceRequiresEnableDocs(t *testing.T) { + mainHelper.Parallel(t) + + th := Setup(t).InitBasic(t) + th.ConfigStore.SetReadOnlyFF(false) + t.Cleanup(func() { th.ConfigStore.SetReadOnlyFF(true) }) + api := th.SetupPluginAPI() + + newSpace := func() *model.Channel { + return &model.Channel{ + TeamId: th.BasicTeam.Id, + DisplayName: "Space", + Name: "space-" + model.NewId(), + Type: model.ChannelTypeSpace, + CreatorId: th.BasicUser.Id, + } + } + + t.Run("EnableDocs off: CreateChannel rejects space type", func(t *testing.T) { + th.App.UpdateConfig(func(cfg *model.Config) { cfg.FeatureFlags.EnableDocs = false }) + + _, appErr := api.CreateChannel(newSpace()) + require.NotNil(t, appErr) + assert.Equal(t, http.StatusForbidden, appErr.StatusCode) + }) + + t.Run("EnableDocs on: CreateChannel allows space type", func(t *testing.T) { + th.App.UpdateConfig(func(cfg *model.Config) { cfg.FeatureFlags.EnableDocs = true }) + defer th.App.UpdateConfig(func(cfg *model.Config) { cfg.FeatureFlags.EnableDocs = false }) + + space, appErr := api.CreateChannel(newSpace()) + require.Nil(t, appErr) + require.Equal(t, model.ChannelTypeSpace, space.Type) + t.Cleanup(func() { + require.NoError(t, th.App.Srv().Store().Channel().PermanentDelete(th.Context, space.Id)) + }) + }) +} + +func TestPluginAPICreateSpaceAndAddMember(t *testing.T) { + mainHelper.Parallel(t) + + th := Setup(t).InitBasic(t) + th.ConfigStore.SetReadOnlyFF(false) + t.Cleanup(func() { + th.ConfigStore.SetReadOnlyFF(true) + }) + th.App.UpdateConfig(func(cfg *model.Config) { cfg.FeatureFlags.EnableDocs = true }) + defer th.App.UpdateConfig(func(cfg *model.Config) { cfg.FeatureFlags.EnableDocs = false }) + + api := th.SetupPluginAPI() + + // Reproduces the docs plugin's CreateSpace flow: create the space backing channel, + // then add the creator as a member. Both must succeed through the plugin API. + space := &model.Channel{ + TeamId: th.BasicTeam.Id, + DisplayName: "Space", + Name: "space-" + model.NewId(), + Type: model.ChannelTypeSpace, + CreatorId: th.BasicUser.Id, + } + created, appErr := api.CreateChannel(space) + require.Nil(t, appErr) + require.Equal(t, model.ChannelTypeSpace, created.Type) + + member, appErr := api.AddChannelMember(created.Id, th.BasicUser.Id) + require.Nil(t, appErr) + require.Equal(t, created.Id, member.ChannelId) + require.Equal(t, th.BasicUser.Id, member.UserId) + + // AddUserToChannel skips the sidebar default-category assignment for a space backing channel, + // so the space must not appear in the member's sidebar categories. + categories, appErr := th.App.GetSidebarCategories(th.Context, th.BasicUser.Id, th.BasicTeam.Id) + require.Nil(t, appErr) + for _, category := range categories.Categories { + require.NotContains(t, category.Channels, created.Id, "space backing channel must not appear in the sidebar") + } + + // AddUserToChannel resolves the space backing channel through the same 404 fallback. + added, appErr := api.AddUserToChannel(created.Id, th.BasicUser2.Id, th.BasicUser.Id) + require.Nil(t, appErr) + require.Equal(t, created.Id, added.ChannelId) + require.Equal(t, th.BasicUser2.Id, added.UserId) + + // DeleteChannelMember resolves the space backing channel and removes the member. + appErr = api.DeleteChannelMember(created.Id, th.BasicUser2.Id) + require.Nil(t, appErr) + + _, appErr = api.GetChannelMember(created.Id, th.BasicUser2.Id) + require.NotNil(t, appErr) + require.Equal(t, http.StatusNotFound, appErr.StatusCode) +} + +func TestPluginAPIChannelMemberNotificationsRejectSpace(t *testing.T) { + mainHelper.Parallel(t) + + th := Setup(t).InitBasic(t) + api := th.SetupPluginAPI() + + space := &model.Channel{ + TeamId: th.BasicTeam.Id, + DisplayName: "Space", + Name: "space-" + model.NewId(), + Type: model.ChannelTypeSpace, + } + space, nErr := th.App.Srv().Store().Channel().Save(th.Context, space, -1) + require.NoError(t, nErr) + _, nErr = th.App.Srv().Store().Channel().SaveMember(th.Context, &model.ChannelMember{ + ChannelId: space.Id, + UserId: th.BasicUser.Id, + NotifyProps: model.GetDefaultChannelNotifyProps(), + SchemeUser: true, + }) + require.NoError(t, nErr) + + notifications := map[string]string{model.MarkUnreadNotifyProp: model.ChannelMarkUnreadMention} + + // Notify-prop mutations carry chat semantics (a channel_member_updated event) that do not + // belong on an internal space backing channel, so the plugin API rejects them like /channels. + _, appErr := api.UpdateChannelMemberNotifications(space.Id, th.BasicUser.Id, notifications) + require.NotNil(t, appErr) + require.Equal(t, http.StatusBadRequest, appErr.StatusCode) + + appErr = api.PatchChannelMembersNotifications( + []*model.ChannelMemberIdentifier{{ChannelId: space.Id, UserId: th.BasicUser.Id}}, + notifications, + ) + require.NotNil(t, appErr) + require.Equal(t, http.StatusBadRequest, appErr.StatusCode) + + // A regular channel is unaffected. + _, appErr = api.UpdateChannelMemberNotifications(th.BasicChannel.Id, th.BasicUser.Id, notifications) + require.Nil(t, appErr) +} + +func TestPluginAPIUpdateSpaceBackingChannel(t *testing.T) { + mainHelper.Parallel(t) + + th := Setup(t).InitBasic(t) + th.ConfigStore.SetReadOnlyFF(false) + t.Cleanup(func() { + th.ConfigStore.SetReadOnlyFF(true) + }) + th.App.UpdateConfig(func(cfg *model.Config) { cfg.FeatureFlags.EnableDocs = true }) + defer th.App.UpdateConfig(func(cfg *model.Config) { cfg.FeatureFlags.EnableDocs = false }) + + api := th.SetupPluginAPI() + + space := &model.Channel{ + TeamId: th.BasicTeam.Id, + DisplayName: "Space", + Name: "space-" + model.NewId(), + Type: model.ChannelTypeSpace, + CreatorId: th.BasicUser.Id, + } + created, appErr := api.CreateChannel(space) + require.Nil(t, appErr) + + // Reproduces the docs plugin's rename/metadata sync: fetch the backing channel, edit its + // user-visible fields, and Update it. UpdateChannel must resolve the space through the + // dedicated path instead of 404ing on the space-excluding generic Get. + created.DisplayName = "Renamed Space" + created.Header = "New header" + updated, appErr := api.UpdateChannel(created) + require.Nil(t, appErr) + require.Equal(t, "Renamed Space", updated.DisplayName) + require.Equal(t, "New header", updated.Header) + + got, appErr := api.GetChannelOfType(created.Id, model.ChannelTypeSpace) + require.Nil(t, appErr) + require.Equal(t, "Renamed Space", got.DisplayName) + require.Equal(t, "New header", got.Header) +} + +func TestPluginAPISpaceLifecycleSkipsChatSideEffects(t *testing.T) { + mainHelper.Parallel(t) + + th := Setup(t).InitBasic(t) + th.ConfigStore.SetReadOnlyFF(false) + t.Cleanup(func() { + th.ConfigStore.SetReadOnlyFF(true) + }) + th.App.UpdateConfig(func(cfg *model.Config) { cfg.FeatureFlags.EnableDocs = true }) + defer th.App.UpdateConfig(func(cfg *model.Config) { cfg.FeatureFlags.EnableDocs = false }) + + api := th.SetupPluginAPI() + + space := &model.Channel{ + TeamId: th.BasicTeam.Id, + DisplayName: "Space", + Name: "space-" + model.NewId(), + Type: model.ChannelTypeSpace, + CreatorId: th.BasicUser.Id, + } + created, appErr := api.CreateChannel(space) + require.Nil(t, appErr) + + _, appErr = api.AddChannelMember(created.Id, th.BasicUser.Id) + require.Nil(t, appErr) + + appErr = api.DeleteChannel(created.Id) + require.Nil(t, appErr) + + appErr = api.RestoreChannel(created.Id) + require.Nil(t, appErr) + + // Internal backing channels get none of the chat-UI side effects: no join system post on + // member add, no archive post on delete, no unarchive post on restore. + posts, appErr := th.App.GetPosts(th.Context, created.Id, 0, 60) + require.Nil(t, appErr) + assert.Empty(t, posts.Order, "space backing channel should carry no join/archive/restore system posts") +} + +func TestPluginAPIDeleteAndRestoreChannelAllowSpace(t *testing.T) { + mainHelper.Parallel(t) + + th := Setup(t).InitBasic(t) + api := th.SetupPluginAPI() + + space := &model.Channel{ + TeamId: th.BasicTeam.Id, + DisplayName: "Space", + Name: "space-" + model.NewId(), + Type: model.ChannelTypeSpace, + } + space, nErr := th.App.Srv().Store().Channel().Save(th.Context, space, -1) + require.NoError(t, nErr) + + // The docs plugin manages the space backing channel lifecycle through these plugin APIs, + // so archive and restore must succeed on a space channel. + appErr := api.DeleteChannel(space.Id) + require.Nil(t, appErr) + + appErr = api.RestoreChannel(space.Id) + require.Nil(t, appErr) +} + +func TestPluginAPIGetChannelOfType(t *testing.T) { + mainHelper.Parallel(t) + + th := Setup(t).InitBasic(t) + api := th.SetupPluginAPI() + + space := &model.Channel{ + TeamId: th.BasicTeam.Id, + DisplayName: "Space", + Name: "space-" + model.NewId(), + Type: model.ChannelTypeSpace, + } + space, nErr := th.App.Srv().Store().Channel().Save(th.Context, space, -1) + require.NoError(t, nErr) + + // Resolves an opaque backing channel type (space) that generic GetChannel excludes. + got, appErr := api.GetChannelOfType(space.Id, model.ChannelTypeSpace) + require.Nil(t, appErr) + require.Equal(t, space.Id, got.Id) + require.Equal(t, model.ChannelTypeSpace, got.Type) + + // Also resolves a non-opaque type by ID + type. + got, appErr = api.GetChannelOfType(th.BasicChannel.Id, model.ChannelTypeOpen) + require.Nil(t, appErr) + require.Equal(t, th.BasicChannel.Id, got.Id) + + // A type mismatch (space ID asked for as an open channel) returns not-found. + _, appErr = api.GetChannelOfType(space.Id, model.ChannelTypeOpen) + require.NotNil(t, appErr) + require.Equal(t, http.StatusNotFound, appErr.StatusCode) + + // A regular channel ID is not a space, so this returns not-found. + _, appErr = api.GetChannelOfType(th.BasicChannel.Id, model.ChannelTypeSpace) + require.NotNil(t, appErr) + require.Equal(t, http.StatusNotFound, appErr.StatusCode) +} + +func TestPluginAPIResolveSpaceChannelNotFound(t *testing.T) { + mainHelper.Parallel(t) + + th := Setup(t).InitBasic(t) + api := th.SetupPluginAPI() + + nonExistentID := model.NewId() + + // A genuinely non-existent ID (neither regular channel nor space) should return a not-found error. + appErr := api.DeleteChannel(nonExistentID) + require.NotNil(t, appErr) + require.Equal(t, http.StatusNotFound, appErr.StatusCode) + + appErr = api.RestoreChannel(nonExistentID) + require.NotNil(t, appErr) + require.Equal(t, http.StatusNotFound, appErr.StatusCode) +} + +func TestResolveChannelByID(t *testing.T) { + // resolveChannelByID has three branches that must be covered: + // 1. GetChannel succeeds → return that channel (no space lookup). + // 2. GetChannel returns 404 AND GetChannelOfType succeeds → return the space. + // 3. GetChannel returns a non-404 error → return that error immediately, skip the space lookup. + // Branches 1 and 2 are exercised by the integration tests above. + // Branch 3 cannot be triggered through a live DB (a SELECT by ID either finds the row or + // returns ErrNotFound), so it is covered here by calling the extracted function directly + // with controlled stubs. + + th := Setup(t) + ctx := th.Context + + t.Run("non-404 from GetChannel is propagated without attempting space lookup", func(t *testing.T) { + dbErr := model.NewAppError("store.Get", "store.sql_channel.get.app_error", nil, "", http.StatusInternalServerError) + + spaceWasCalled := false + getChannel := func(_ request.CTX, _ string) (*model.Channel, *model.AppError) { + return nil, dbErr + } + getChannelOfType := func(_ request.CTX, _ string, _ model.ChannelType) (*model.Channel, *model.AppError) { + spaceWasCalled = true + return nil, model.NewAppError("store.GetChannelOfType", "not_found", nil, "", http.StatusNotFound) + } + + got, appErr := resolveChannelByID(ctx, model.NewId(), getChannel, getChannelOfType) + require.NotNil(t, appErr) + require.Equal(t, http.StatusInternalServerError, appErr.StatusCode) + require.Nil(t, got) + require.False(t, spaceWasCalled, "space lookup must not be attempted when GetChannel returns a non-404 error") + }) + + t.Run("non-404 from GetChannelOfType is propagated", func(t *testing.T) { + notFoundErr := model.NewAppError("store.Get", "not_found", nil, "", http.StatusNotFound) + spaceErr := model.NewAppError("store.GetChannelOfType", "store.sql_channel.get.app_error", nil, "", http.StatusInternalServerError) + + getChannel := func(_ request.CTX, _ string) (*model.Channel, *model.AppError) { + return nil, notFoundErr + } + getChannelOfType := func(_ request.CTX, _ string, _ model.ChannelType) (*model.Channel, *model.AppError) { + return nil, spaceErr + } + + got, appErr := resolveChannelByID(ctx, model.NewId(), getChannel, getChannelOfType) + require.NotNil(t, appErr) + require.Equal(t, http.StatusInternalServerError, appErr.StatusCode) + require.Nil(t, got) + }) + + t.Run("404 from both returns the original GetChannel 404", func(t *testing.T) { + notFoundErr := model.NewAppError("store.Get", "not_found", nil, "", http.StatusNotFound) + spaceNotFoundErr := model.NewAppError("store.GetChannelOfType", "not_found", nil, "", http.StatusNotFound) + + getChannel := func(_ request.CTX, _ string) (*model.Channel, *model.AppError) { + return nil, notFoundErr + } + getChannelOfType := func(_ request.CTX, _ string, _ model.ChannelType) (*model.Channel, *model.AppError) { + return nil, spaceNotFoundErr + } + + got, appErr := resolveChannelByID(ctx, model.NewId(), getChannel, getChannelOfType) + require.NotNil(t, appErr) + require.Equal(t, http.StatusNotFound, appErr.StatusCode) + require.Equal(t, notFoundErr, appErr, "should return the original GetChannel error, not the space error") + require.Nil(t, got) + }) +} + func TestPluginAPICreateChannelAnonymousURLs(t *testing.T) { mainHelper.Parallel(t) @@ -3858,6 +4218,39 @@ func TestPluginAPICreateChannelAnonymousURLs(t *testing.T) { assert.True(t, model.IsValidId(createdChannel.Name), "channel name should be a valid server-generated ID") }) + t.Run("should preserve space backing channel name when UseAnonymousURLs is enabled", func(t *testing.T) { + th.ConfigStore.SetReadOnlyFF(false) + defer th.ConfigStore.SetReadOnlyFF(true) + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.PrivacySettings.UseAnonymousURLs = true + cfg.FeatureFlags.EnableDocs = true + }) + defer th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.PrivacySettings.UseAnonymousURLs = false + cfg.FeatureFlags.EnableDocs = false + }) + + th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuEnterpriseAdvanced)) + defer func() { + appErr := th.App.Srv().RemoveLicense() + require.Nil(t, appErr) + }() + + originalName := "space-" + model.NewId() + channel := &model.Channel{ + DisplayName: "Space", + Name: originalName, + Type: model.ChannelTypeSpace, + TeamId: th.BasicTeam.Id, + } + + createdChannel, appErr := api.CreateChannel(channel) + require.Nil(t, appErr) + require.NotNil(t, createdChannel) + + assert.Equal(t, originalName, createdChannel.Name, "space backing channel name should not be rewritten") + }) + t.Run("should preserve channel name when UseAnonymousURLs is disabled", func(t *testing.T) { th.App.UpdateConfig(func(cfg *model.Config) { *cfg.PrivacySettings.UseAnonymousURLs = false }) diff --git a/server/channels/app/team.go b/server/channels/app/team.go index be5ba75b577..43ad2a5e34c 100644 --- a/server/channels/app/team.go +++ b/server/channels/app/team.go @@ -1389,6 +1389,20 @@ func (a *App) LeaveTeam(rctx request.CTX, team *model.Team, user *model.User, re } } + // Space backing channels are excluded from GetChannels, so their membership rows survive a + // plain team leave and keep authorizing space-scoped WebSocket delivery to a former member. + // Remove them explicitly. + spaceChannels, sErr := a.Srv().Store().Channel().GetTeamSpaceChannelsForUser(team.Id, user.Id) + if sErr != nil { + return model.NewAppError("LeaveTeam", "app.channel.get_channels.get.app_error", nil, "", http.StatusInternalServerError).Wrap(sErr) + } + for _, channel := range spaceChannels { + a.invalidateCacheForChannelMembers(channel.Id) + if appErr := a.removeChannelMembership(rctx, user.Id, channel.Id, "LeaveTeam"); appErr != nil { + return appErr + } + } + if *a.Config().ServiceSettings.ExperimentalEnableDefaultChannelLeaveJoinMessages { channel, cErr := a.Srv().Store().Channel().GetByName(team.Id, model.DefaultChannelName, false) if cErr != nil { @@ -1924,6 +1938,18 @@ func (a *App) PermanentDeleteTeam(rctx request.CTX, team *model.Team) *model.App } } + // Space backing channels are excluded from GetTeamChannels, so tear them down explicitly to + // avoid leaving hidden channels, members, and posts behind with a dead TeamId. + spaceChannels, spaceErr := a.Srv().Store().Channel().GetTeamSpaceChannels(team.Id) + if spaceErr != nil { + return model.NewAppError("PermanentDeleteTeam", "app.channel.get_channels.get.app_error", nil, "", http.StatusInternalServerError).Wrap(spaceErr) + } + for _, ch := range spaceChannels { + if err := a.PermanentDeleteChannel(rctx, ch); err != nil { + rctx.Logger().Warn("Error permanently deleting space channel during team deletion", mlog.String("channel_id", ch.Id), mlog.String("team_id", team.Id), mlog.Err(err)) + } + } + if err := a.Srv().Store().Team().RemoveAllMembersByTeam(team.Id); err != nil { return model.NewAppError("PermanentDeleteTeam", "app.team.remove_member.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } diff --git a/server/channels/app/team_test.go b/server/channels/app/team_test.go index f928d7b5cac..24527b8de27 100644 --- a/server/channels/app/team_test.go +++ b/server/channels/app/team_test.go @@ -803,6 +803,75 @@ func TestPermanentDeleteTeam(t *testing.T) { } } +func TestPermanentDeleteTeamRemovesSpaceChannels(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + team := th.CreateTeam(t) + + // Space backing channels are excluded from GetTeamChannels, so team teardown must clean them + // up through the dedicated path or they orphan with a dead TeamId. + space, nErr := th.App.Srv().Store().Channel().Save(th.Context, &model.Channel{ + TeamId: team.Id, + DisplayName: "Space", + Name: "space-" + model.NewId(), + Type: model.ChannelTypeSpace, + }, -1) + require.NoError(t, nErr) + + _, nErr = th.App.Srv().Store().Channel().SaveMember(th.Context, &model.ChannelMember{ + ChannelId: space.Id, + UserId: th.BasicUser.Id, + NotifyProps: model.GetDefaultChannelNotifyProps(), + SchemeUser: true, + }) + require.NoError(t, nErr) + + // Sanity: the space backing channel resolves through the typed getter before deletion. + _, appErr := th.App.GetChannelOfType(th.Context, space.Id, model.ChannelTypeSpace) + require.Nil(t, appErr) + + appErr = th.App.PermanentDeleteTeam(th.Context, team) + require.Nil(t, appErr) + + // Assert through the typed getter: generic Get already excludes spaces, so it returns + // not-found whether or not the row was deleted and would pass vacuously. + _, getErr := th.App.GetChannelOfType(th.Context, space.Id, model.ChannelTypeSpace) + require.NotNil(t, getErr, "space backing channel should be permanently deleted with its team") + + _, memErr := th.App.Srv().Store().Channel().GetMember(th.Context, space.Id, th.BasicUser.Id) + require.Error(t, memErr, "space channel membership should be removed with its team") +} + +func TestLeaveTeamRemovesSpaceMemberships(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + // Space backing channels are excluded from GetChannels, so a plain team leave leaves their + // membership rows behind unless LeaveTeam removes them explicitly. + space, nErr := th.App.Srv().Store().Channel().Save(th.Context, &model.Channel{ + TeamId: th.BasicTeam.Id, + DisplayName: "Space", + Name: "space-" + model.NewId(), + Type: model.ChannelTypeSpace, + }, -1) + require.NoError(t, nErr) + + _, nErr = th.App.Srv().Store().Channel().SaveMember(th.Context, &model.ChannelMember{ + ChannelId: space.Id, + UserId: th.BasicUser.Id, + NotifyProps: model.GetDefaultChannelNotifyProps(), + SchemeUser: true, + }) + require.NoError(t, nErr) + + appErr := th.App.LeaveTeam(th.Context, th.BasicTeam, th.BasicUser, th.BasicUser.Id) + require.Nil(t, appErr) + + _, memErr := th.App.Srv().Store().Channel().GetMember(th.Context, space.Id, th.BasicUser.Id) + require.Error(t, memErr, "space channel membership should be removed when the user leaves the team") +} + func TestSanitizeTeam(t *testing.T) { mainHelper.Parallel(t) th := Setup(t) @@ -1152,6 +1221,7 @@ func TestLeaveTeamPanic(t *testing.T) { }, }, nil) mockChannelStore.On("GetChannels", "myteam", "userID", mock.Anything).Return(model.ChannelList{}, nil) + mockChannelStore.On("GetTeamSpaceChannelsForUser", "myteam", "userID").Return(model.ChannelList{}, nil) var err error th.App.ch.srv.userService, err = users.New(users.ServiceConfig{ diff --git a/server/channels/db/migrations/migrations.list b/server/channels/db/migrations/migrations.list index c9980270a1c..bfc9a4b1274 100644 --- a/server/channels/db/migrations/migrations.list +++ b/server/channels/db/migrations/migrations.list @@ -401,3 +401,5 @@ channels/db/migrations/postgres/000202_create_property_values_groupid_updateat_i channels/db/migrations/postgres/000202_create_property_values_groupid_updateat_id_index.up.sql channels/db/migrations/postgres/000203_add_lastnotifiedat_to_user_access_tokens.down.sql channels/db/migrations/postgres/000203_add_lastnotifiedat_to_user_access_tokens.up.sql +channels/db/migrations/postgres/000204_add_channel_type_space_enum.down.sql +channels/db/migrations/postgres/000204_add_channel_type_space_enum.up.sql diff --git a/server/channels/db/migrations/postgres/000204_add_channel_type_space_enum.down.sql b/server/channels/db/migrations/postgres/000204_add_channel_type_space_enum.down.sql new file mode 100644 index 00000000000..b9546210f8d --- /dev/null +++ b/server/channels/db/migrations/postgres/000204_add_channel_type_space_enum.down.sql @@ -0,0 +1,4 @@ +-- Removing the space channel type. Channels with type S should be deleted +-- before running this migration. Postgres cannot drop a value from an existing +-- enum in place, so the 'S' value remains. +SELECT 1; diff --git a/server/channels/db/migrations/postgres/000204_add_channel_type_space_enum.up.sql b/server/channels/db/migrations/postgres/000204_add_channel_type_space_enum.up.sql new file mode 100644 index 00000000000..dd7d264bd7f --- /dev/null +++ b/server/channels/db/migrations/postgres/000204_add_channel_type_space_enum.up.sql @@ -0,0 +1 @@ +ALTER TYPE channel_type ADD VALUE IF NOT EXISTS 'S'; diff --git a/server/channels/store/localcachelayer/channel_layer.go b/server/channels/store/localcachelayer/channel_layer.go index 2429909cd7b..0c1fa4d2890 100644 --- a/server/channels/store/localcachelayer/channel_layer.go +++ b/server/channels/store/localcachelayer/channel_layer.go @@ -224,7 +224,10 @@ func (s LocalCacheChannelStore) GetPinnedPostCount(channelId string, allowFromCa func (s LocalCacheChannelStore) Save(rctx request.CTX, channel *model.Channel, maxChannelsPerTeam int64, channelOptions ...model.ChannelOption) (*model.Channel, error) { newChannel, err := s.ChannelStore.Save(rctx, channel, maxChannelsPerTeam, channelOptions...) - if err == nil { + // Space backing channels are excluded from the generic by-id Get/GetMany (SQL) and resolve + // only through GetChannelOfType; caching them here would let the generic cached lookups + // return them and defeat that exclusion. + if err == nil && !newChannel.IsSpace() { s.rootStore.doStandardAddToCache(s.rootStore.channelByIdCache, newChannel.Id, newChannel) } return newChannel, err diff --git a/server/channels/store/retrylayer/retrylayer.go b/server/channels/store/retrylayer/retrylayer.go index 2a89e47bbac..f8d7d108c58 100644 --- a/server/channels/store/retrylayer/retrylayer.go +++ b/server/channels/store/retrylayer/retrylayer.go @@ -2097,6 +2097,27 @@ func (s *RetryLayerChannelStore) GetChannelMembersTimezones(channelID string) ([ } +func (s *RetryLayerChannelStore) GetChannelOfType(rctx request.CTX, id string, channelType model.ChannelType) (*model.Channel, error) { + + tries := 0 + for { + result, err := s.ChannelStore.GetChannelOfType(rctx, id, channelType) + if err == nil { + return result, nil + } + if !isRepeatableError(err) { + return result, err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return result, err + } + timepkg.Sleep(100 * timepkg.Millisecond) + } + +} + func (s *RetryLayerChannelStore) GetChannelUnread(channelID string, userID string) (*model.ChannelUnread, error) { tries := 0 @@ -2985,6 +3006,48 @@ func (s *RetryLayerChannelStore) GetTeamMembersForChannel(rctx request.CTX, chan } +func (s *RetryLayerChannelStore) GetTeamSpaceChannels(teamID string) (model.ChannelList, error) { + + tries := 0 + for { + result, err := s.ChannelStore.GetTeamSpaceChannels(teamID) + if err == nil { + return result, nil + } + if !isRepeatableError(err) { + return result, err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return result, err + } + timepkg.Sleep(100 * timepkg.Millisecond) + } + +} + +func (s *RetryLayerChannelStore) GetTeamSpaceChannelsForUser(teamID string, userID string) (model.ChannelList, error) { + + tries := 0 + for { + result, err := s.ChannelStore.GetTeamSpaceChannelsForUser(teamID, userID) + if err == nil { + return result, nil + } + if !isRepeatableError(err) { + return result, err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return result, err + } + timepkg.Sleep(100 * timepkg.Millisecond) + } + +} + func (s *RetryLayerChannelStore) GroupSyncedChannelCount() (int64, error) { tries := 0 diff --git a/server/channels/store/searchlayer/channel_layer.go b/server/channels/store/searchlayer/channel_layer.go index 0ff966e068f..370ed8fd9f9 100644 --- a/server/channels/store/searchlayer/channel_layer.go +++ b/server/channels/store/searchlayer/channel_layer.go @@ -37,6 +37,11 @@ func (c *SearchChannelStore) deleteChannelIndex(rctx request.CTX, channel *model } func (c *SearchChannelStore) indexChannel(rctx request.CTX, channel *model.Channel) { + // Space backing channels are internal and never surface in channel search. + if channel.IsSpace() { + return + } + var userIDs, teamMemberIDs []string var err error if channel.Type == model.ChannelTypePrivate { @@ -175,8 +180,11 @@ func (c *SearchChannelStore) RemoveMember(rctx request.CTX, channelID, userIdToR c.rootStore.indexUserFromID(rctx, userIdToRemove) } - channel, err := c.ChannelStore.Get(channelID, true) - if err == nil { + // return the removal result, not the re-index Get's — that Get returns + // not-found for space backing channels (excluded from the generic Get) + // and must not mask a successful removal. + channel, getErr := c.ChannelStore.Get(channelID, true) + if getErr == nil { c.indexChannel(rctx, channel) } diff --git a/server/channels/store/sqlstore/channel_store.go b/server/channels/store/sqlstore/channel_store.go index 7e4ecdbd527..4d8faaa191c 100644 --- a/server/channels/store/sqlstore/channel_store.go +++ b/server/channels/store/sqlstore/channel_store.go @@ -43,6 +43,28 @@ var messageChannelTypes = []model.ChannelType{ model.ChannelTypeGroup, } +// nonMessageBackingChannelTypes is the deny-list for queries that filter by channel ID or +// team+user rather than by type, so the messageChannelTypes allow-list does not apply to them. +// A backing channel type must be added here when it writes real posts to the backing channel; +// otherwise TotalMsgCount grows and the channel generates unread badges and push notifications +// in the chat UI. Backing channel types that never write posts are already invisible in these +// queries without an explicit filter and do not need to be listed here. +var nonMessageBackingChannelTypes = []model.ChannelType{ + model.ChannelTypeSpace, +} + +// nonMessageBackingChannelTypesNotIn returns a "NOT IN (...)" SQL clause and its args for use +// in raw SQL queries, keeping those callers in sync with the deny-list. +func nonMessageBackingChannelTypesNotIn() (string, []any) { + placeholders := make([]string, len(nonMessageBackingChannelTypes)) + args := make([]any, len(nonMessageBackingChannelTypes)) + for i, t := range nonMessageBackingChannelTypes { + placeholders[i] = "?" + args[i] = string(t) + } + return "NOT IN (" + strings.Join(placeholders, ",") + ")", args +} + // teamMessageChannelTypes is messageChannelTypes minus direct channels, used // for team-scoped queries where direct channels don't belong. var teamMessageChannelTypes = []model.ChannelType{ @@ -774,7 +796,8 @@ func (s SqlChannelStore) saveChannelT(transaction *sqlxTxWrapper, channel *model return nil, err // we just pass through the error as-is for now. } - if channel.Type != model.ChannelTypeDirect && channel.Type != model.ChannelTypeGroup && maxChannelsPerTeam >= 0 { + // Space channels are exempt from the per-team channel limit. + if channel.Type != model.ChannelTypeDirect && channel.Type != model.ChannelTypeGroup && channel.Type != model.ChannelTypeSpace && maxChannelsPerTeam >= 0 { var count int64 if err := transaction.Get(&count, "SELECT COUNT(0) FROM Channels WHERE TeamId = ? AND DeleteAt = 0 AND (Type = ? OR Type = ?)", channel.TeamId, model.ChannelTypeOpen, model.ChannelTypePrivate); err != nil { return nil, errors.Wrapf(err, "save_channel_count: teamId=%s", channel.TeamId) @@ -995,6 +1018,27 @@ func (s SqlChannelStore) GetBoardChannel(id string) (*model.Channel, error) { return &ch, nil } +// GetChannelOfType fetches a channel by ID, requiring it to be of the given type. Unlike Get(), +// it resolves opaque backing channel types (e.g. space) that Get() excludes; callers that need +// such a channel ask for it by its exact type. +func (s SqlChannelStore) GetChannelOfType(rctx request.CTX, id string, channelType model.ChannelType) (*model.Channel, error) { + ch := model.Channel{} + query := s.tableSelectQuery.Where(sq.And{ + sq.Eq{"Id": id}, + sq.Eq{"Type": channelType}, + }) + + err := s.DBXFromContext(rctx.Context()).GetBuilder(&ch, query) + if err != nil { + if err == sql.ErrNoRows { + return nil, store.NewErrNotFound("Channel", id) + } + return nil, errors.Wrapf(err, "failed to find channel with id = %s and type = %s", id, channelType) + } + + return &ch, nil +} + //nolint:unparam func (s SqlChannelStore) GetMany(ids []string, allowFromCache bool) (model.ChannelList, error) { query := s.getQueryBuilder(). @@ -1540,6 +1584,49 @@ func (s SqlChannelStore) GetTeamChannels(teamId string) (model.ChannelList, erro return data, nil } +// GetTeamSpaceChannels returns all space (S) channels for a team, including archived ones, so +// team teardown can remove them. GetTeamChannels/GetAll exclude spaces, hence this dedicated +// enumerator. Returns an empty list (not ErrNotFound) when the team has no spaces. It reads from +// the primary because its callers (team teardown, member cleanup) act on the result destructively +// and must not miss a just-created space due to replica lag. +func (s SqlChannelStore) GetTeamSpaceChannels(teamId string) (model.ChannelList, error) { + data := model.ChannelList{} + query := s.tableSelectQuery.Where(sq.And{ + sq.Eq{"TeamId": teamId}, + sq.Eq{"Type": model.ChannelTypeSpace}, + }).OrderBy("Id") + + if err := s.GetMaster().SelectBuilder(&data, query); err != nil { + return nil, errors.Wrapf(err, "failed to find space Channels with teamId=%s", teamId) + } + + return data, nil +} + +// GetTeamSpaceChannelsForUser returns the team's space (S) channels, including archived ones, +// that the user is a member of. Space memberships are excluded from GetChannels and +// GetChannelMembersForUser, hence this dedicated lookup. It reads from the primary because its +// callers (team leave, guest eviction) act on the result destructively and must not miss a +// just-created membership due to replica lag. +func (s SqlChannelStore) GetTeamSpaceChannelsForUser(teamId string, userId string) (model.ChannelList, error) { + data := model.ChannelList{} + query := s.getQueryBuilder(). + Select(channelSliceColumns(true, "Channels")...). + From("Channels"). + InnerJoin("ChannelMembers ON (Channels.Id = ChannelMembers.ChannelId)"). + Where(sq.And{ + sq.Eq{"Channels.TeamId": teamId}, + sq.Eq{"Channels.Type": model.ChannelTypeSpace}, + sq.Eq{"ChannelMembers.UserId": userId}, + }).OrderBy("Channels.Id") + + if err := s.GetMaster().SelectBuilder(&data, query); err != nil { + return nil, errors.Wrapf(err, "failed to find space Channels with teamId=%s and userId=%s", teamId, userId) + } + + return data, nil +} + func (s SqlChannelStore) GetByNamesIncludeDeleted(teamId string, names []string, allowFromCache bool) ([]*model.Channel, error) { return s.getByNames(teamId, names, allowFromCache, true) } @@ -2158,7 +2245,9 @@ func (s SqlChannelStore) GetChannelsWithUnreadsAndWithMentions(_ request.CTX, ch Where(sq.Eq{ "ChannelMembers.ChannelId": channelIDs, "ChannelMembers.UserId": userID, - }) + }). + // Space backing channels are internal and carry no chat read-state. + Where(sq.NotEq{"Channels.Type": nonMessageBackingChannelTypes}) queryString, args, err := query.ToSql() if err != nil { @@ -2231,7 +2320,9 @@ func (s SqlChannelStore) GetTeamChannelsWithUnreadAndMentions(rctx request.CTX, Where(sq.Eq{ "Channels.TeamId": teamID, "ChannelMembers.UserId": userID, - }) + }). + // Space backing channels are internal and carry no chat read-state. + Where(sq.NotEq{"Channels.Type": nonMessageBackingChannelTypes}) var channels []struct { Id string @@ -3144,6 +3235,7 @@ func (s SqlChannelStore) AnalyticsCountAll(teamId string) (map[model.ChannelType if teamId != "" { query = query.Where(sq.Eq{"TeamId": teamId}) } + query = query.Where(sq.NotEq{"Type": nonMessageBackingChannelTypes}) sqlStr, args, err := query.ToSql() if err != nil { @@ -3175,6 +3267,7 @@ func (s SqlChannelStore) GetMembersForUser(teamID string, userID string) (model. sq.Eq{"Teams.Id": ""}, sq.Eq{"Teams.Id": nil}, }, + sq.NotEq{"Channels.Type": nonMessageBackingChannelTypes}, }).ToSql() if err != nil { return nil, errors.Wrapf(err, "GetMembersForUser_ToSql teamID=%s userID=%s", teamID, userID) @@ -3192,7 +3285,10 @@ func (s SqlChannelStore) GetMembersForUser(teamID string, userID string) (model. func (s SqlChannelStore) GetMembersForUserWithPagination(userId string, page, perPage int) (model.ChannelMembersWithTeamData, error) { dbMembers := channelMemberWithTeamWithSchemeRolesList{} offset := page * perPage - err := s.GetReplica().Select(&dbMembers, channelMembersWithSchemeSelectQuery+"WHERE ChannelMembers.UserId = ? ORDER BY ChannelId ASC Limit ? Offset ?", userId, perPage, offset) + notInClause, notInArgs := nonMessageBackingChannelTypesNotIn() + queryArgs := append([]any{userId}, notInArgs...) + queryArgs = append(queryArgs, perPage, offset) + err := s.GetReplica().Select(&dbMembers, channelMembersWithSchemeSelectQuery+"WHERE ChannelMembers.UserId = ? AND Channels.Type "+notInClause+" ORDER BY ChannelId ASC Limit ? Offset ?", queryArgs...) if err != nil { return nil, errors.Wrapf(err, "failed to find ChannelMembers data with and userId=%s", userId) } @@ -3202,7 +3298,10 @@ func (s SqlChannelStore) GetMembersForUserWithPagination(userId string, page, pe func (s SqlChannelStore) GetMembersForUserWithCursorPagination(userId string, perPage int, fromChannelID string) (model.ChannelMembersWithTeamData, error) { dbMembers := channelMemberWithTeamWithSchemeRolesList{} - err := s.GetReplica().Select(&dbMembers, channelMembersWithSchemeSelectQuery+"WHERE ChannelMembers.UserId = ? AND ChannelId > ? ORDER BY ChannelId ASC Limit ?", userId, fromChannelID, perPage) + notInClause, notInArgs := nonMessageBackingChannelTypesNotIn() + queryArgs := append([]any{userId, fromChannelID}, notInArgs...) + queryArgs = append(queryArgs, perPage) + err := s.GetReplica().Select(&dbMembers, channelMembersWithSchemeSelectQuery+"WHERE ChannelMembers.UserId = ? AND ChannelId > ? AND Channels.Type "+notInClause+" ORDER BY ChannelId ASC Limit ?", queryArgs...) if err != nil { return nil, errors.Wrapf(err, "failed to find ChannelMembers data with and userId=%s", userId) } @@ -3971,7 +4070,7 @@ func (s SqlChannelStore) GetMembersInfoByChannelIds(channelIDs []string) (map[st func (s SqlChannelStore) GetChannelsByScheme(schemeId string, offset int, limit int) (model.ChannelList, error) { channels := model.ChannelList{} - query := s.tableSelectQuery.Where(sq.Eq{"SchemeId": schemeId}).OrderBy("DisplayName").Limit(uint64(limit)).Offset(uint64(offset)) + query := s.tableSelectQuery.Where(sq.Eq{"SchemeId": schemeId}).Where(sq.NotEq{"Type": nonMessageBackingChannelTypes}).OrderBy("DisplayName").Limit(uint64(limit)).Offset(uint64(offset)) if err := s.GetReplica().SelectBuilder(&channels, query); err != nil { return nil, errors.Wrapf(err, "failed to find Channels with schemeId=%s", schemeId) @@ -4247,7 +4346,10 @@ func (s SqlChannelStore) GetChannelMembersForExport(userId string, teamId string if !includeArchivedChannel { q += " AND Channels.DeleteAt = 0" } - err := s.GetReplica().Select(&members, q, userId, teamId) + notInClause, notInArgs := nonMessageBackingChannelTypesNotIn() + q += " AND Channels.Type " + notInClause + queryArgs := append([]any{userId, teamId}, notInArgs...) + err := s.GetReplica().Select(&members, q, queryArgs...) if err != nil { return nil, errors.Wrap(err, "failed to find Channels for export") } diff --git a/server/channels/store/sqlstore/team_store.go b/server/channels/store/sqlstore/team_store.go index 0b0bb936329..4cc40c1c925 100644 --- a/server/channels/store/sqlstore/team_store.go +++ b/server/channels/store/sqlstore/team_store.go @@ -1227,7 +1227,9 @@ func (s SqlTeamStore) GetChannelUnreadsForAllTeams(excludeTeamId, userId string) From("Channels"). Join("ChannelMembers ON Id = ChannelId"). Where(sq.Eq{"UserId": userId, "DeleteAt": 0}). - Where(sq.NotEq{"TeamId": excludeTeamId}).ToSql() + Where(sq.NotEq{"TeamId": excludeTeamId}). + // Space backing channels are internal and carry no chat read-state. + Where(sq.NotEq{"Channels.Type": nonMessageBackingChannelTypes}).ToSql() if err != nil { return nil, errors.Wrap(err, "team_tosql") } @@ -1246,7 +1248,9 @@ func (s SqlTeamStore) GetChannelUnreadsForTeam(teamId, userId string) ([]*model. Select("Channels.TeamId TeamId", "Channels.Id ChannelId", "(Channels.TotalMsgCount - ChannelMembers.MsgCount) MsgCount", "(Channels.TotalMsgCountRoot - ChannelMembers.MsgCountRoot) MsgCountRoot", "ChannelMembers.MentionCount MentionCount", "ChannelMembers.MentionCountRoot MentionCountRoot", "ChannelMembers.NotifyProps NotifyProps"). From("Channels"). Join("ChannelMembers ON Id = ChannelId"). - Where(sq.Eq{"UserId": userId, "TeamId": teamId, "DeleteAt": 0}).ToSql() + Where(sq.Eq{"UserId": userId, "TeamId": teamId, "DeleteAt": 0}). + // Space backing channels are internal and carry no chat read-state. + Where(sq.NotEq{"Channels.Type": nonMessageBackingChannelTypes}).ToSql() if err != nil { return nil, errors.Wrap(err, "team_tosql") } diff --git a/server/channels/store/sqlstore/user_store.go b/server/channels/store/sqlstore/user_store.go index 5d01f3a1072..4511e175aea 100644 --- a/server/channels/store/sqlstore/user_store.go +++ b/server/channels/store/sqlstore/user_store.go @@ -1587,6 +1587,8 @@ func (us SqlUserStore) GetUnreadCount(userId string, isCRTEnabled bool) (int64, mentionCountColumn = "cm.MentionCountRoot" } + // Space backing channels are internal and carry no chat read-state. + typeClause, typeArgs := nonMessageBackingChannelTypesNotIn() query := ` SELECT SUM(` + mentionCountColumn + `) FROM Channels c @@ -1594,10 +1596,11 @@ func (us SqlUserStore) GetUnreadCount(userId string, isCRTEnabled bool) (int64, ON cm.ChannelId = c.Id AND cm.UserId = ? AND c.DeleteAt = 0 + WHERE c.Type ` + typeClause + ` ` var count int64 - err := us.GetReplica().Get(&count, query, userId) + err := us.GetReplica().Get(&count, query, append([]any{userId}, typeArgs...)...) if err != nil { return count, errors.Wrapf(err, "failed to count unread Channels for userId=%s", userId) } diff --git a/server/channels/store/store.go b/server/channels/store/store.go index ad81b4dccb9..79874e97064 100644 --- a/server/channels/store/store.go +++ b/server/channels/store/store.go @@ -206,6 +206,7 @@ type ChannelStore interface { ClearSidebarOnTeamLeave(userID, teamID string) error Get(id string, allowFromCache bool) (*model.Channel, error) GetBoardChannel(id string) (*model.Channel, error) + GetChannelOfType(rctx request.CTX, id string, channelType model.ChannelType) (*model.Channel, error) GetMany(ids []string, allowFromCache bool) (model.ChannelList, error) InvalidateChannel(id string) InvalidateChannelByName(teamID, name string) @@ -230,6 +231,8 @@ type ChannelStore interface { GetPublicChannelsForTeam(teamID string, offset int, limit int) (model.ChannelList, error) GetPublicChannelsByIdsForTeam(teamID string, channelIds []string) (model.ChannelList, error) GetTeamChannels(teamID string) (model.ChannelList, error) + GetTeamSpaceChannels(teamID string) (model.ChannelList, error) + GetTeamSpaceChannelsForUser(teamID string, userID string) (model.ChannelList, error) GetAll(teamID string) ([]*model.Channel, error) GetChannelsByIds(channelIds []string, includeDeleted bool) ([]*model.Channel, error) GetChannelsWithTeamDataByIds(channelIds []string, includeDeleted bool) ([]*model.ChannelWithTeamData, error) diff --git a/server/channels/store/storetest/channel_store.go b/server/channels/store/storetest/channel_store.go index a1c6e25bfe3..b10a9ef512f 100644 --- a/server/channels/store/storetest/channel_store.go +++ b/server/channels/store/storetest/channel_store.go @@ -8,6 +8,7 @@ import ( "encoding/json" "errors" "fmt" + "slices" "sort" "strconv" "strings" @@ -81,6 +82,9 @@ func TestChannelStore(t *testing.T, rctx request.CTX, ss store.Store, s SqlStore t.Cleanup(func() { cleanupChannelStoreData(t, s) }) t.Run("Save", func(t *testing.T) { testChannelStoreSave(t, rctx, ss) }) + t.Run("SpaceExclusion", func(t *testing.T) { testChannelStoreSpaceExclusion(t, rctx, ss) }) + t.Run("GetTeamSpaceChannels", func(t *testing.T) { testChannelStoreGetTeamSpaceChannels(t, rctx, ss) }) + t.Run("GetTeamSpaceChannelsForUser", func(t *testing.T) { testChannelStoreGetTeamSpaceChannelsForUser(t, rctx, ss) }) t.Run("SaveDirectChannel", func(t *testing.T) { testChannelStoreSaveDirectChannel(t, rctx, ss, s) }) t.Run("CreateDirectChannel", func(t *testing.T) { testChannelStoreCreateDirectChannel(t, rctx, ss) }) t.Run("GetMembersWithCursorPagination", func(t *testing.T) { testChannelStoreGetMembersWithCursorPagination(t, rctx, ss) }) @@ -9594,3 +9598,262 @@ func testGetTeamChannelsWithUnreadAndMentions(t *testing.T, rctx request.CTX, ss require.Equal(t, o4.LastPostAt, times[o4.Id]) }) } + +// testChannelStoreSpaceExclusion verifies the ChannelTypeSpace ("S") backing-channel contract: +// - opaque to the generic by-id reads (Get/GetMany); docs/spaces code resolves it only through +// GetChannelOfType with the space type; +// - resolvable in the per-user authorization membership map (GetAllChannelMembersForUser) so a +// space member is authorized for the backing channel; +// - excluded from aggregate/analytics and client-facing member listings so it never leaks into +// chat surfaces. +func testChannelStoreSpaceExclusion(t *testing.T, rctx request.CTX, ss store.Store) { + teamID := model.NewId() + + open := &model.Channel{TeamId: teamID, DisplayName: "Open", Name: "open-" + model.NewId(), Type: model.ChannelTypeOpen} + _, err := ss.Channel().Save(rctx, open, -1) + require.NoError(t, err) + + space := &model.Channel{TeamId: teamID, DisplayName: "Space", Name: "space-" + model.NewId(), Type: model.ChannelTypeSpace} + _, err = ss.Channel().Save(rctx, space, -1) + require.NoError(t, err) + + // Space backing channels are opaque to the generic by-id reads and resolve only through + // GetChannelOfType with the space type. + _, err = ss.Channel().Get(space.Id, false) + var nfErr *store.ErrNotFound + require.True(t, errors.As(err, &nfErr), "Get must exclude space channels") + + got, err := ss.Channel().GetChannelOfType(rctx, space.Id, model.ChannelTypeSpace) + require.NoError(t, err) + require.Equal(t, model.ChannelTypeSpace, got.Type) + + many, err := ss.Channel().GetMany([]string{open.Id, space.Id}, false) + require.NoError(t, err) + require.Len(t, many, 1, "GetMany must exclude space channels") + require.Equal(t, open.Id, many[0].Id) + + // GetChannelsByIds is restricted to message-bearing types, so it excludes space channels. + byIds, err := ss.Channel().GetChannelsByIds([]string{open.Id, space.Id}, false) + require.NoError(t, err) + require.Len(t, byIds, 1, "GetChannelsByIds must exclude space channels") + require.Equal(t, open.Id, byIds[0].Id) + + // Aggregate reads exclude space backing channels. + counts, err := ss.Channel().AnalyticsCountAll(teamID) + require.NoError(t, err) + require.Zero(t, counts[model.ChannelTypeSpace], "space backing channels must be excluded from analytics counts") + require.EqualValues(t, 1, counts[model.ChannelTypeOpen]) + + // A user who is a member of both channels. + userID := model.NewId() + _, err = ss.Channel().SaveMember(rctx, &model.ChannelMember{ChannelId: open.Id, UserId: userID, NotifyProps: model.GetDefaultChannelNotifyProps()}) + require.NoError(t, err) + _, err = ss.Channel().SaveMember(rctx, &model.ChannelMember{ChannelId: space.Id, UserId: userID, NotifyProps: model.GetDefaultChannelNotifyProps()}) + require.NoError(t, err) + + // The authorization membership map MUST resolve the space channel so its members are + // authorized for the backing channel (same as board channels). + authMembers, err := ss.Channel().GetAllChannelMembersForUser(rctx, userID, false, false) + require.NoError(t, err) + require.Contains(t, authMembers, open.Id) + require.Contains(t, authMembers, space.Id, "space backing channels must resolve in the authorization membership map") + + // Client-facing member listings exclude space backing channels. + hasChannel := func(channelID string) func(m model.ChannelMemberWithTeamData) bool { + return func(m model.ChannelMemberWithTeamData) bool { return m.ChannelId == channelID } + } + + paged, err := ss.Channel().GetMembersForUserWithPagination(userID, 0, 100) + require.NoError(t, err) + require.False(t, slices.ContainsFunc(paged, hasChannel(space.Id)), "GetMembersForUserWithPagination must exclude space channels") + require.True(t, slices.ContainsFunc(paged, hasChannel(open.Id))) + + cursored, err := ss.Channel().GetMembersForUserWithCursorPagination(userID, 100, "") + require.NoError(t, err) + require.False(t, slices.ContainsFunc(cursored, hasChannel(space.Id)), "GetMembersForUserWithCursorPagination must exclude space channels") + require.True(t, slices.ContainsFunc(cursored, hasChannel(open.Id))) + + exported, err := ss.Channel().GetChannelMembersForExport(userID, teamID, false) + require.NoError(t, err) + var exportedSpace, exportedOpen bool + for _, m := range exported { + switch m.ChannelId { + case space.Id: + exportedSpace = true + case open.Id: + exportedOpen = true + } + } + require.False(t, exportedSpace, "GetChannelMembersForExport must exclude space channels") + require.True(t, exportedOpen) + + // The team-scoped membership listing excludes space backing channels. + teamMembers, err := ss.Channel().GetMembersForUser(teamID, userID) + require.NoError(t, err) + require.False(t, slices.ContainsFunc(teamMembers, func(m model.ChannelMember) bool { return m.ChannelId == space.Id }), "GetMembersForUser must exclude space channels") + require.True(t, slices.ContainsFunc(teamMembers, func(m model.ChannelMember) bool { return m.ChannelId == open.Id })) + + // Scheme-scoped listing excludes space backing channels even when a space shares a scheme. + scheme := &model.Scheme{DisplayName: model.NewId(), Name: model.NewId(), Description: model.NewId(), Scope: model.SchemeScopeChannel} + scheme, err = ss.Scheme().Save(scheme) + require.NoError(t, err) + schemedOpen := &model.Channel{TeamId: teamID, DisplayName: "SchemedOpen", Name: "schemed-open-" + model.NewId(), Type: model.ChannelTypeOpen, SchemeId: &scheme.Id} + _, err = ss.Channel().Save(rctx, schemedOpen, -1) + require.NoError(t, err) + schemedSpace := &model.Channel{TeamId: teamID, DisplayName: "SchemedSpace", Name: "schemed-space-" + model.NewId(), Type: model.ChannelTypeSpace, SchemeId: &scheme.Id} + _, err = ss.Channel().Save(rctx, schemedSpace, -1) + require.NoError(t, err) + byScheme, err := ss.Channel().GetChannelsByScheme(scheme.Id, 0, 100) + require.NoError(t, err) + require.Len(t, byScheme, 1, "GetChannelsByScheme must exclude the space backing channel") + require.Equal(t, schemedOpen.Id, byScheme[0].Id) + + // Unread-aggregate queries exclude space backing channels even when the member has unreads + // (TotalMsgCount ahead of the member's MsgCount). + unreadTeamID := model.NewId() + unreadUserID := model.NewId() + unreadOpen := &model.Channel{TeamId: unreadTeamID, DisplayName: "UnreadOpen", Name: "unread-open-" + model.NewId(), Type: model.ChannelTypeOpen, TotalMsgCount: 25, LastPostAt: 12345, LastRootPostAt: 12345} + _, err = ss.Channel().Save(rctx, unreadOpen, -1) + require.NoError(t, err) + unreadSpace := &model.Channel{TeamId: unreadTeamID, DisplayName: "UnreadSpace", Name: "unread-space-" + model.NewId(), Type: model.ChannelTypeSpace, TotalMsgCount: 25, LastPostAt: 12345, LastRootPostAt: 12345} + _, err = ss.Channel().Save(rctx, unreadSpace, -1) + require.NoError(t, err) + for _, ch := range []*model.Channel{unreadOpen, unreadSpace} { + _, err = ss.Channel().SaveMember(rctx, &model.ChannelMember{ChannelId: ch.Id, UserId: unreadUserID, NotifyProps: model.GetDefaultChannelNotifyProps()}) + require.NoError(t, err) + } + notifyProps := model.GetDefaultChannelNotifyProps() + + unreadIDs, _, _, err := ss.Channel().GetChannelsWithUnreadsAndWithMentions(rctx, []string{unreadOpen.Id, unreadSpace.Id}, unreadUserID, notifyProps) + require.NoError(t, err) + require.Contains(t, unreadIDs, unreadOpen.Id) + require.NotContains(t, unreadIDs, unreadSpace.Id, "GetChannelsWithUnreadsAndWithMentions must exclude space channels") + + teamUnreadIDs, _, _, err := ss.Channel().GetTeamChannelsWithUnreadAndMentions(rctx, unreadTeamID, unreadUserID, notifyProps) + require.NoError(t, err) + require.Contains(t, teamUnreadIDs, unreadOpen.Id) + require.NotContains(t, teamUnreadIDs, unreadSpace.Id, "GetTeamChannelsWithUnreadAndMentions must exclude space channels") + + // Space channels are exempt from the per-team channel limit (like direct/group channels): a + // team already at its limit can still create a space backing channel. + limitedTeamID := model.NewId() + atLimit := &model.Channel{TeamId: limitedTeamID, DisplayName: "AtLimit", Name: "atlimit-" + model.NewId(), Type: model.ChannelTypeOpen} + _, err = ss.Channel().Save(rctx, atLimit, 1) + require.NoError(t, err) + + overLimit := &model.Channel{TeamId: limitedTeamID, DisplayName: "OverLimit", Name: "overlimit-" + model.NewId(), Type: model.ChannelTypeOpen} + _, err = ss.Channel().Save(rctx, overLimit, 1) + require.Error(t, err, "a non-space channel beyond the team limit must be rejected") + + limitedSpace := &model.Channel{TeamId: limitedTeamID, DisplayName: "LimitedSpace", Name: "limitedspace-" + model.NewId(), Type: model.ChannelTypeSpace} + _, err = ss.Channel().Save(rctx, limitedSpace, 1) + require.NoError(t, err, "space backing channels must be exempt from the per-team channel limit") +} + +func testChannelStoreGetTeamSpaceChannels(t *testing.T, rctx request.CTX, ss store.Store) { + teamID := model.NewId() + otherTeamID := model.NewId() + + space1 := &model.Channel{TeamId: teamID, DisplayName: "Space1", Name: "space1-" + model.NewId(), Type: model.ChannelTypeSpace} + _, err := ss.Channel().Save(rctx, space1, -1) + require.NoError(t, err) + + space2 := &model.Channel{TeamId: teamID, DisplayName: "Space2", Name: "space2-" + model.NewId(), Type: model.ChannelTypeSpace} + _, err = ss.Channel().Save(rctx, space2, -1) + require.NoError(t, err) + + open := &model.Channel{TeamId: teamID, DisplayName: "Open", Name: "open-" + model.NewId(), Type: model.ChannelTypeOpen} + _, err = ss.Channel().Save(rctx, open, -1) + require.NoError(t, err) + + otherTeamSpace := &model.Channel{TeamId: otherTeamID, DisplayName: "OtherSpace", Name: "other-space-" + model.NewId(), Type: model.ChannelTypeSpace} + _, err = ss.Channel().Save(rctx, otherTeamSpace, -1) + require.NoError(t, err) + + channelIDs := func(list model.ChannelList) []string { + ids := make([]string, len(list)) + for i, ch := range list { + ids[i] = ch.Id + } + return ids + } + + // Returns all space channels for the team, excluding non-space and other-team channels. + channels, err := ss.Channel().GetTeamSpaceChannels(teamID) + require.NoError(t, err) + ids := channelIDs(channels) + require.Contains(t, ids, space1.Id) + require.Contains(t, ids, space2.Id) + require.NotContains(t, ids, open.Id, "must not include non-space channels") + require.NotContains(t, ids, otherTeamSpace.Id, "must not include spaces from other teams") + + // Archived spaces are included so team teardown can remove them. + err = ss.Channel().Delete(space1.Id, model.GetMillis()) + require.NoError(t, err) + channels, err = ss.Channel().GetTeamSpaceChannels(teamID) + require.NoError(t, err) + require.Contains(t, channelIDs(channels), space1.Id, "archived space channels must be included") + + // Team with no spaces returns an empty list, not an error. + empty, err := ss.Channel().GetTeamSpaceChannels(model.NewId()) + require.NoError(t, err) + require.Empty(t, empty) +} + +func testChannelStoreGetTeamSpaceChannelsForUser(t *testing.T, rctx request.CTX, ss store.Store) { + teamID := model.NewId() + otherTeamID := model.NewId() + userID := model.NewId() + otherUserID := model.NewId() + + space1 := &model.Channel{TeamId: teamID, DisplayName: "Space1", Name: "space1-" + model.NewId(), Type: model.ChannelTypeSpace} + _, err := ss.Channel().Save(rctx, space1, -1) + require.NoError(t, err) + + space2 := &model.Channel{TeamId: teamID, DisplayName: "Space2", Name: "space2-" + model.NewId(), Type: model.ChannelTypeSpace} + _, err = ss.Channel().Save(rctx, space2, -1) + require.NoError(t, err) + + open := &model.Channel{TeamId: teamID, DisplayName: "Open", Name: "open-" + model.NewId(), Type: model.ChannelTypeOpen} + _, err = ss.Channel().Save(rctx, open, -1) + require.NoError(t, err) + + otherTeamSpace := &model.Channel{TeamId: otherTeamID, DisplayName: "OtherSpace", Name: "other-space-" + model.NewId(), Type: model.ChannelTypeSpace} + _, err = ss.Channel().Save(rctx, otherTeamSpace, -1) + require.NoError(t, err) + + for _, channelID := range []string{space1.Id, open.Id, otherTeamSpace.Id} { + _, err = ss.Channel().SaveMember(rctx, &model.ChannelMember{ + ChannelId: channelID, + UserId: userID, + NotifyProps: model.GetDefaultChannelNotifyProps(), + }) + require.NoError(t, err) + } + _, err = ss.Channel().SaveMember(rctx, &model.ChannelMember{ + ChannelId: space2.Id, + UserId: otherUserID, + NotifyProps: model.GetDefaultChannelNotifyProps(), + }) + require.NoError(t, err) + + // Returns only the team's spaces the user is a member of: not space2 (other user's + // membership), not the open channel, not the other team's space. + channels, err := ss.Channel().GetTeamSpaceChannelsForUser(teamID, userID) + require.NoError(t, err) + require.Len(t, channels, 1) + require.Equal(t, space1.Id, channels[0].Id) + + // Archived spaces are included so membership cleanup can still find them. + err = ss.Channel().Delete(space1.Id, model.GetMillis()) + require.NoError(t, err) + channels, err = ss.Channel().GetTeamSpaceChannelsForUser(teamID, userID) + require.NoError(t, err) + require.Len(t, channels, 1) + require.Equal(t, space1.Id, channels[0].Id) + + // User with no space memberships in the team returns an empty list, not an error. + empty, err := ss.Channel().GetTeamSpaceChannelsForUser(teamID, model.NewId()) + require.NoError(t, err) + require.Empty(t, empty) +} diff --git a/server/channels/store/storetest/mocks/ChannelStore.go b/server/channels/store/storetest/mocks/ChannelStore.go index 7e63eb319e2..1f401469080 100644 --- a/server/channels/store/storetest/mocks/ChannelStore.go +++ b/server/channels/store/storetest/mocks/ChannelStore.go @@ -978,6 +978,36 @@ func (_m *ChannelStore) GetChannelMembersTimezones(channelID string) ([]model.St return r0, r1 } +// GetChannelOfType provides a mock function with given fields: rctx, id, channelType +func (_m *ChannelStore) GetChannelOfType(rctx request.CTX, id string, channelType model.ChannelType) (*model.Channel, error) { + ret := _m.Called(rctx, id, channelType) + + if len(ret) == 0 { + panic("no return value specified for GetChannelOfType") + } + + var r0 *model.Channel + var r1 error + if rf, ok := ret.Get(0).(func(request.CTX, string, model.ChannelType) (*model.Channel, error)); ok { + return rf(rctx, id, channelType) + } + if rf, ok := ret.Get(0).(func(request.CTX, string, model.ChannelType) *model.Channel); ok { + r0 = rf(rctx, id, channelType) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*model.Channel) + } + } + + if rf, ok := ret.Get(1).(func(request.CTX, string, model.ChannelType) error); ok { + r1 = rf(rctx, id, channelType) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // GetChannelUnread provides a mock function with given fields: channelID, userID func (_m *ChannelStore) GetChannelUnread(channelID string, userID string) (*model.ChannelUnread, error) { ret := _m.Called(channelID, userID) @@ -2300,6 +2330,66 @@ func (_m *ChannelStore) GetTeamMembersForChannel(rctx request.CTX, channelID str return r0, r1 } +// GetTeamSpaceChannels provides a mock function with given fields: teamID +func (_m *ChannelStore) GetTeamSpaceChannels(teamID string) (model.ChannelList, error) { + ret := _m.Called(teamID) + + if len(ret) == 0 { + panic("no return value specified for GetTeamSpaceChannels") + } + + var r0 model.ChannelList + var r1 error + if rf, ok := ret.Get(0).(func(string) (model.ChannelList, error)); ok { + return rf(teamID) + } + if rf, ok := ret.Get(0).(func(string) model.ChannelList); ok { + r0 = rf(teamID) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(model.ChannelList) + } + } + + if rf, ok := ret.Get(1).(func(string) error); ok { + r1 = rf(teamID) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// GetTeamSpaceChannelsForUser provides a mock function with given fields: teamID, userID +func (_m *ChannelStore) GetTeamSpaceChannelsForUser(teamID string, userID string) (model.ChannelList, error) { + ret := _m.Called(teamID, userID) + + if len(ret) == 0 { + panic("no return value specified for GetTeamSpaceChannelsForUser") + } + + var r0 model.ChannelList + var r1 error + if rf, ok := ret.Get(0).(func(string, string) (model.ChannelList, error)); ok { + return rf(teamID, userID) + } + if rf, ok := ret.Get(0).(func(string, string) model.ChannelList); ok { + r0 = rf(teamID, userID) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(model.ChannelList) + } + } + + if rf, ok := ret.Get(1).(func(string, string) error); ok { + r1 = rf(teamID, userID) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // GroupSyncedChannelCount provides a mock function with no fields func (_m *ChannelStore) GroupSyncedChannelCount() (int64, error) { ret := _m.Called() diff --git a/server/channels/store/storetest/team_store.go b/server/channels/store/storetest/team_store.go index b4ea1e24022..ee9e3c07ac4 100644 --- a/server/channels/store/storetest/team_store.go +++ b/server/channels/store/storetest/team_store.go @@ -3247,10 +3247,19 @@ func testGetChannelUnreadsForAllTeams(t *testing.T, rctx request.CTX, ss store.S _, err = ss.Channel().SaveMember(rctx, cm2) require.NoError(t, err) + // A space backing channel with non-zero counters must not surface as an unread. + cSpace := &model.Channel{TeamId: m1.TeamId, Name: model.NewId(), DisplayName: "Space", Type: model.ChannelTypeSpace, TotalMsgCount: 100} + _, nErr = ss.Channel().Save(rctx, cSpace, -1) + require.NoError(t, nErr) + cmSpace := &model.ChannelMember{ChannelId: cSpace.Id, UserId: uid, NotifyProps: model.GetDefaultChannelNotifyProps(), MsgCount: 90, MentionCount: 5} + _, err = ss.Channel().SaveMember(rctx, cmSpace) + require.NoError(t, err) + ms1, nErr := ss.Team().GetChannelUnreadsForAllTeams("", uid) require.NoError(t, nErr) membersMap := make(map[string]bool) for i := range ms1 { + require.NotEqual(t, cSpace.Id, ms1[i].ChannelId, "space backing channel must not contribute to unreads") id := ms1[i].TeamId if _, ok := membersMap[id]; !ok { membersMap[id] = true @@ -3301,9 +3310,20 @@ func testGetChannelUnreadsForTeam(t *testing.T, rctx request.CTX, ss store.Store _, nErr = ss.Channel().SaveMember(rctx, cm2) require.NoError(t, nErr) + // A space backing channel with non-zero counters must not surface as an unread. + cSpace := &model.Channel{TeamId: m1.TeamId, Name: model.NewId(), DisplayName: "Space", Type: model.ChannelTypeSpace, TotalMsgCount: 100} + _, nErr = ss.Channel().Save(rctx, cSpace, -1) + require.NoError(t, nErr) + cmSpace := &model.ChannelMember{ChannelId: cSpace.Id, UserId: m1.UserId, NotifyProps: model.GetDefaultChannelNotifyProps(), MsgCount: 90, MentionCount: 5} + _, nErr = ss.Channel().SaveMember(rctx, cmSpace) + require.NoError(t, nErr) + ms, err := ss.Team().GetChannelUnreadsForTeam(m1.TeamId, m1.UserId) require.NoError(t, err) require.Len(t, ms, 2, "wrong length") + for i := range ms { + require.NotEqual(t, cSpace.Id, ms[i].ChannelId, "space backing channel must not contribute to unreads") + } require.Equal(t, 10, int(ms[0].MsgCount), "subtraction failed") } diff --git a/server/channels/store/storetest/user_store.go b/server/channels/store/storetest/user_store.go index 7956b7a0b05..92a82cdd41f 100644 --- a/server/channels/store/storetest/user_store.go +++ b/server/channels/store/storetest/user_store.go @@ -2824,6 +2824,14 @@ func testUserUnreadCount(t *testing.T, rctx request.CTX, ss store.Store) { nErr = ss.Channel().IncrementMentionCount(c2.Id, []string{u2.Id}, false, false) require.NoError(t, nErr) + // A space backing channel carrying a mention for u3 must not inflate the badge count. + cSpace := model.Channel{TeamId: teamID, DisplayName: "Space", Name: "space-" + model.NewId(), Type: model.ChannelTypeSpace} + _, nErr = ss.Channel().Save(rctx, &cSpace, -1) + require.NoError(t, nErr) + mSpace := model.ChannelMember{ChannelId: cSpace.Id, UserId: u3.Id, NotifyProps: model.GetDefaultChannelNotifyProps(), MentionCount: 5, MentionCountRoot: 5} + _, nErr = ss.Channel().SaveMember(rctx, &mSpace) + require.NoError(t, nErr) + badge, unreadCountErr := ss.User().GetUnreadCount(u2.Id, false) require.NoError(t, unreadCountErr) require.Equal(t, int64(3), badge, "should have 3 unread messages") diff --git a/server/channels/store/timerlayer/timerlayer.go b/server/channels/store/timerlayer/timerlayer.go index 4b44c262adf..1aabf85929e 100644 --- a/server/channels/store/timerlayer/timerlayer.go +++ b/server/channels/store/timerlayer/timerlayer.go @@ -1799,6 +1799,22 @@ func (s *TimerLayerChannelStore) GetChannelMembersTimezones(channelID string) ([ return result, err } +func (s *TimerLayerChannelStore) GetChannelOfType(rctx request.CTX, id string, channelType model.ChannelType) (*model.Channel, error) { + start := time.Now() + + result, err := s.ChannelStore.GetChannelOfType(rctx, id, channelType) + + elapsed := float64(time.Since(start)) / float64(time.Second) + if s.Root.Metrics != nil { + success := "false" + if err == nil { + success = "true" + } + s.Root.Metrics.ObserveStoreMethodDuration("ChannelStore.GetChannelOfType", success, elapsed) + } + return result, err +} + func (s *TimerLayerChannelStore) GetChannelUnread(channelID string, userID string) (*model.ChannelUnread, error) { start := time.Now() @@ -2487,6 +2503,38 @@ func (s *TimerLayerChannelStore) GetTeamMembersForChannel(rctx request.CTX, chan return result, err } +func (s *TimerLayerChannelStore) GetTeamSpaceChannels(teamID string) (model.ChannelList, error) { + start := time.Now() + + result, err := s.ChannelStore.GetTeamSpaceChannels(teamID) + + elapsed := float64(time.Since(start)) / float64(time.Second) + if s.Root.Metrics != nil { + success := "false" + if err == nil { + success = "true" + } + s.Root.Metrics.ObserveStoreMethodDuration("ChannelStore.GetTeamSpaceChannels", success, elapsed) + } + return result, err +} + +func (s *TimerLayerChannelStore) GetTeamSpaceChannelsForUser(teamID string, userID string) (model.ChannelList, error) { + start := time.Now() + + result, err := s.ChannelStore.GetTeamSpaceChannelsForUser(teamID, userID) + + elapsed := float64(time.Since(start)) / float64(time.Second) + if s.Root.Metrics != nil { + success := "false" + if err == nil { + success = "true" + } + s.Root.Metrics.ObserveStoreMethodDuration("ChannelStore.GetTeamSpaceChannelsForUser", success, elapsed) + } + return result, err +} + func (s *TimerLayerChannelStore) GroupSyncedChannelCount() (int64, error) { start := time.Now() diff --git a/server/i18n/en.json b/server/i18n/en.json index c2315d2bb17..581fc4252aa 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -687,6 +687,10 @@ "id": "api.channel.set_members.type.app_error", "translation": "Cannot set members on this channel type." }, + { + "id": "api.channel.space_channel.app_error", + "translation": "Space channels cannot be accessed via /channels endpoints. Use the spaces API instead." + }, { "id": "api.channel.update_channel.banner_info.channel_type.not_allowed", "translation": "Channel banner can only be configured on Public and Private channels." @@ -5626,6 +5630,14 @@ "id": "app.channel.create_channel.no_team_id.app_error", "translation": "Must specify the team ID to create a channel." }, + { + "id": "app.channel.create_channel.space_type.app_error", + "translation": "Space channels cannot be created via this endpoint." + }, + { + "id": "app.channel.create_channel.spaces_not_enabled.app_error", + "translation": "The Docs feature is not enabled." + }, { "id": "app.channel.create_direct_channel.internal_error", "translation": "Unable to save direct channel." @@ -5834,6 +5846,10 @@ "id": "app.channel.move_channel.members_do_not_match.error", "translation": "Unable to move a channel unless all its members are already members of the destination team." }, + { + "id": "app.channel.move_channel.space.app_error", + "translation": "Spaces cannot be moved through this endpoint." + }, { "id": "app.channel.patch_channel_members_notify_props.app_error", "translation": "Unable to update channel members' notify props." @@ -5946,6 +5962,10 @@ "id": "app.channel.update_channel.rejected_by_plugin", "translation": "Channel update rejected by plugin: {{.Reason}}" }, + { + "id": "app.channel.update_channel_privacy.space.app_error", + "translation": "Spaces cannot have their privacy changed through this endpoint." + }, { "id": "app.channel.update_last_viewed_at.app_error", "translation": "Unable to update the last viewed at time." @@ -13422,6 +13442,10 @@ "id": "plugin_api.bot_cant_create_bot", "translation": "Bot user cannot create bot user." }, + { + "id": "plugin_api.channel.space_notify_props.app_error", + "translation": "Notify props cannot be set on space channels." + }, { "id": "plugin_api.get_file_link.disabled.app_error", "translation": "Public links have been disabled." diff --git a/server/public/model/channel.go b/server/public/model/channel.go index 23e24c6cdeb..989370f0257 100644 --- a/server/public/model/channel.go +++ b/server/public/model/channel.go @@ -29,6 +29,7 @@ const ( ChannelTypePrivate ChannelType = "P" ChannelTypeDirect ChannelType = "D" ChannelTypeGroup ChannelType = "G" + ChannelTypeSpace ChannelType = "S" ChannelTypeOpenBoard ChannelType = "BO" ChannelTypePrivateBoard ChannelType = "BP" @@ -329,7 +330,7 @@ func (o *Channel) IsValid() *AppError { return NewAppError("Channel.IsValid", "model.channel.is_valid.1_or_more.app_error", nil, "id="+o.Id, http.StatusBadRequest) } - if !(o.Type == ChannelTypeOpen || o.Type == ChannelTypePrivate || o.Type == ChannelTypeDirect || o.Type == ChannelTypeGroup || o.Type == ChannelTypeOpenBoard || o.Type == ChannelTypePrivateBoard) { + if !(o.Type == ChannelTypeOpen || o.Type == ChannelTypePrivate || o.Type == ChannelTypeDirect || o.Type == ChannelTypeGroup || o.Type == ChannelTypeSpace || o.Type == ChannelTypeOpenBoard || o.Type == ChannelTypePrivateBoard) { return NewAppError("Channel.IsValid", "model.channel.is_valid.type.app_error", nil, "id="+o.Id, http.StatusBadRequest) } @@ -439,6 +440,10 @@ func (o *Channel) IsBoard() bool { return o.Type == ChannelTypeOpenBoard || o.Type == ChannelTypePrivateBoard } +func (o *Channel) IsSpace() bool { + return o.Type == ChannelTypeSpace +} + // IsMessageChannel reports whether the channel is one of the message-bearing // types (open, private, direct, or group). Returns false for boards and any // future non-message channel types. diff --git a/server/public/model/channel_test.go b/server/public/model/channel_test.go index 53c3872142b..0f290ff3b6a 100644 --- a/server/public/model/channel_test.go +++ b/server/public/model/channel_test.go @@ -194,6 +194,21 @@ func TestChannelIsValid(t *testing.T) { require.NotNil(t, o.IsValid()) } +func TestChannelTypeSpace(t *testing.T) { + t.Run("IsValid accepts a space backing channel", func(t *testing.T) { + o := Channel{ + Id: NewId(), + CreateAt: GetMillis(), + UpdateAt: GetMillis(), + TeamId: NewId(), + DisplayName: "Space", + Name: "space-" + NewId(), + Type: ChannelTypeSpace, + } + require.Nil(t, o.IsValid()) + }) +} + func TestChannelIsValidBoard(t *testing.T) { t.Run("rejects non-board type", func(t *testing.T) { c := &Channel{Type: ChannelTypeOpen, TeamId: NewId(), DisplayName: "Board"} diff --git a/server/public/model/feature_flags.go b/server/public/model/feature_flags.go index 04c95c5a5be..dd5a91bf159 100644 --- a/server/public/model/feature_flags.go +++ b/server/public/model/feature_flags.go @@ -100,6 +100,10 @@ type FeatureFlags struct { // Enable the Integrated Boards feature within Mattermost channels IntegratedBoards bool + // FEATURE_FLAG_REMOVAL: EnableDocs - Remove this when GA is released + // Enable the Docs (spaces and pages) feature within Mattermost channels + EnableDocs bool + // Enable LIKE-based CJK (Chinese, Japanese, Korean) search for PostgreSQL CJKSearch bool @@ -182,6 +186,8 @@ func (f *FeatureFlags) SetDefaults() { f.IntegratedBoards = false + f.EnableDocs = false + f.CJKSearch = true f.AggregatePluginMetrics = false diff --git a/server/public/plugin/api.go b/server/public/plugin/api.go index 328d49ecb77..1cf9044a790 100644 --- a/server/public/plugin/api.go +++ b/server/public/plugin/api.go @@ -447,6 +447,20 @@ type API interface { // Minimum server version: 5.2 DeleteChannel(channelId string) *model.AppError + // RestoreChannel restores a previously deleted (archived) channel. + // + // @tag Channel + // Minimum server version: 11.10 + RestoreChannel(channelId string) *model.AppError + + // GetChannelOfType resolves a channel by ID, requiring it to be of the given type. Unlike + // GetChannel, it resolves opaque backing channel types (e.g. space) that GetChannel excludes; + // a caller that needs such a channel asks for it by its exact type. + // + // @tag Channel + // Minimum server version: 11.10 + GetChannelOfType(channelId string, channelType model.ChannelType) (*model.Channel, *model.AppError) + // GetPublicChannelsForTeam gets a list of all channels. // // @tag Channel diff --git a/server/public/plugin/api_timer_layer_generated.go b/server/public/plugin/api_timer_layer_generated.go index c4301202d4e..c33e7b9aea4 100644 --- a/server/public/plugin/api_timer_layer_generated.go +++ b/server/public/plugin/api_timer_layer_generated.go @@ -497,6 +497,20 @@ func (api *apiTimerLayer) DeleteChannel(channelId string) *model.AppError { return _returnsA } +func (api *apiTimerLayer) RestoreChannel(channelId string) *model.AppError { + startTime := timePkg.Now() + _returnsA := api.apiImpl.RestoreChannel(channelId) + api.recordTime(startTime, "RestoreChannel", _returnsA == nil) + return _returnsA +} + +func (api *apiTimerLayer) GetChannelOfType(channelId string, channelType model.ChannelType) (*model.Channel, *model.AppError) { + startTime := timePkg.Now() + _returnsA, _returnsB := api.apiImpl.GetChannelOfType(channelId, channelType) + api.recordTime(startTime, "GetChannelOfType", _returnsB == nil) + return _returnsA, _returnsB +} + func (api *apiTimerLayer) GetPublicChannelsForTeam(teamID string, page, perPage int) ([]*model.Channel, *model.AppError) { startTime := timePkg.Now() _returnsA, _returnsB := api.apiImpl.GetPublicChannelsForTeam(teamID, page, perPage) diff --git a/server/public/plugin/client_rpc_generated.go b/server/public/plugin/client_rpc_generated.go index 412646599d8..f72acccc72f 100644 --- a/server/public/plugin/client_rpc_generated.go +++ b/server/public/plugin/client_rpc_generated.go @@ -4194,6 +4194,64 @@ func (s *apiRPCServer) DeleteChannel(args *Z_DeleteChannelArgs, returns *Z_Delet return nil } +type Z_RestoreChannelArgs struct { + A string +} + +type Z_RestoreChannelReturns struct { + A *model.AppError +} + +func (g *apiRPCClient) RestoreChannel(channelId string) *model.AppError { + _args := &Z_RestoreChannelArgs{channelId} + _returns := &Z_RestoreChannelReturns{} + if err := g.client.Call("Plugin.RestoreChannel", _args, _returns); err != nil { + log.Printf("RPC call to RestoreChannel API failed: %s", err.Error()) + } + return _returns.A +} + +func (s *apiRPCServer) RestoreChannel(args *Z_RestoreChannelArgs, returns *Z_RestoreChannelReturns) error { + if hook, ok := s.impl.(interface { + RestoreChannel(channelId string) *model.AppError + }); ok { + returns.A = hook.RestoreChannel(args.A) + } else { + return encodableError(fmt.Errorf("API RestoreChannel called but not implemented.")) + } + return nil +} + +type Z_GetChannelOfTypeArgs struct { + A string + B model.ChannelType +} + +type Z_GetChannelOfTypeReturns struct { + A *model.Channel + B *model.AppError +} + +func (g *apiRPCClient) GetChannelOfType(channelId string, channelType model.ChannelType) (*model.Channel, *model.AppError) { + _args := &Z_GetChannelOfTypeArgs{channelId, channelType} + _returns := &Z_GetChannelOfTypeReturns{} + if err := g.client.Call("Plugin.GetChannelOfType", _args, _returns); err != nil { + log.Printf("RPC call to GetChannelOfType API failed: %s", err.Error()) + } + return _returns.A, _returns.B +} + +func (s *apiRPCServer) GetChannelOfType(args *Z_GetChannelOfTypeArgs, returns *Z_GetChannelOfTypeReturns) error { + if hook, ok := s.impl.(interface { + GetChannelOfType(channelId string, channelType model.ChannelType) (*model.Channel, *model.AppError) + }); ok { + returns.A, returns.B = hook.GetChannelOfType(args.A, args.B) + } else { + return encodableError(fmt.Errorf("API GetChannelOfType called but not implemented.")) + } + return nil +} + type Z_GetPublicChannelsForTeamArgs struct { A string B int diff --git a/server/public/plugin/plugintest/api.go b/server/public/plugin/plugintest/api.go index e37102f60e9..b2e9c90b0ca 100644 --- a/server/public/plugin/plugintest/api.go +++ b/server/public/plugin/plugintest/api.go @@ -1563,6 +1563,38 @@ func (_m *API) GetChannelMembersForUser(teamID string, userID string, page int, return r0, r1 } +// GetChannelOfType provides a mock function with given fields: channelId, channelType +func (_m *API) GetChannelOfType(channelId string, channelType model.ChannelType) (*model.Channel, *model.AppError) { + ret := _m.Called(channelId, channelType) + + if len(ret) == 0 { + panic("no return value specified for GetChannelOfType") + } + + var r0 *model.Channel + var r1 *model.AppError + if rf, ok := ret.Get(0).(func(string, model.ChannelType) (*model.Channel, *model.AppError)); ok { + return rf(channelId, channelType) + } + if rf, ok := ret.Get(0).(func(string, model.ChannelType) *model.Channel); ok { + r0 = rf(channelId, channelType) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*model.Channel) + } + } + + if rf, ok := ret.Get(1).(func(string, model.ChannelType) *model.AppError); ok { + r1 = rf(channelId, channelType) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 +} + // GetChannelSidebarCategories provides a mock function with given fields: userID, teamID func (_m *API) GetChannelSidebarCategories(userID string, teamID string) (*model.OrderedSidebarCategories, *model.AppError) { ret := _m.Called(userID, teamID) @@ -4887,6 +4919,26 @@ func (_m *API) RequestTrialLicense(requesterID string, users int, termsAccepted return r0 } +// RestoreChannel provides a mock function with given fields: channelId +func (_m *API) RestoreChannel(channelId string) *model.AppError { + ret := _m.Called(channelId) + + if len(ret) == 0 { + panic("no return value specified for RestoreChannel") + } + + var r0 *model.AppError + if rf, ok := ret.Get(0).(func(string) *model.AppError); ok { + r0 = rf(channelId) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*model.AppError) + } + } + + return r0 +} + // RestoreGroup provides a mock function with given fields: groupID func (_m *API) RestoreGroup(groupID string) (*model.Group, *model.AppError) { ret := _m.Called(groupID) diff --git a/server/public/pluginapi/channel.go b/server/public/pluginapi/channel.go index 405e22f25c4..7a4e2acafd0 100644 --- a/server/public/pluginapi/channel.go +++ b/server/public/pluginapi/channel.go @@ -102,6 +102,13 @@ func (c *ChannelService) Create(channel *model.Channel) error { *channel = *createdChannel + // The replica wait polls the generic GetChannel, which excludes opaque backing channel + // types, so it could never observe a space channel. Space channels are also resolved via + // GetChannelOfType, which reads from the master DB, so there is no replica lag to wait out. + if channel.IsSpace() { + return nil + } + return c.waitForChannelCreation(channel.Id) } @@ -126,6 +133,24 @@ func (c *ChannelService) Delete(channelID string) error { return normalizeAppErr(c.api.DeleteChannel(channelID)) } +// Restore restores a previously deleted (archived) channel. +// +// Minimum server version: 11.10 +func (c *ChannelService) Restore(channelID string) error { + return normalizeAppErr(c.api.RestoreChannel(channelID)) +} + +// GetChannelOfType resolves a channel by ID, requiring it to be of the given type. The generic +// Get excludes opaque backing channel types (e.g. space); a plugin that manages such a channel +// resolves it by its exact type here. +// +// Minimum server version: 11.10 +func (c *ChannelService) GetChannelOfType(channelID string, channelType model.ChannelType) (*model.Channel, error) { + channel, appErr := c.api.GetChannelOfType(channelID, channelType) + + return channel, normalizeAppErr(appErr) +} + // GetChannelStats gets statistics for a channel. // // Minimum server version: 5.6 diff --git a/server/public/pluginapi/channel_test.go b/server/public/pluginapi/channel_test.go index 2e6803ca1f0..88209926bb7 100644 --- a/server/public/pluginapi/channel_test.go +++ b/server/public/pluginapi/channel_test.go @@ -25,6 +25,56 @@ func TestGetMembers(t *testing.T) { }) } +func TestRestoreChannel(t *testing.T) { + t.Run("success", func(t *testing.T) { + api := &plugintest.API{} + defer api.AssertExpectations(t) + client := pluginapi.NewClient(api, &plugintest.Driver{}) + + api.On("RestoreChannel", "channelID").Return(nil) + + err := client.Channel.Restore("channelID") + require.NoError(t, err) + }) + + t.Run("failure", func(t *testing.T) { + api := &plugintest.API{} + defer api.AssertExpectations(t) + client := pluginapi.NewClient(api, &plugintest.Driver{}) + + api.On("RestoreChannel", "channelID").Return(newAppError()) + + err := client.Channel.Restore("channelID") + require.EqualError(t, err, "here: id, an error occurred") + }) +} + +func TestGetChannelOfType(t *testing.T) { + t.Run("success", func(t *testing.T) { + api := &plugintest.API{} + defer api.AssertExpectations(t) + client := pluginapi.NewClient(api, &plugintest.Driver{}) + + expected := &model.Channel{Id: "channelID", Type: model.ChannelTypeSpace} + api.On("GetChannelOfType", "channelID", model.ChannelTypeSpace).Return(expected, nil) + + channel, err := client.Channel.GetChannelOfType("channelID", model.ChannelTypeSpace) + require.NoError(t, err) + require.Equal(t, expected, channel) + }) + + t.Run("failure", func(t *testing.T) { + api := &plugintest.API{} + defer api.AssertExpectations(t) + client := pluginapi.NewClient(api, &plugintest.Driver{}) + + api.On("GetChannelOfType", "channelID", model.ChannelTypeSpace).Return(nil, newAppError()) + + _, err := client.Channel.GetChannelOfType("channelID", model.ChannelTypeSpace) + require.EqualError(t, err, "here: id, an error occurred") + }) +} + func TestGetTeamChannelByName(t *testing.T) { t.Run("success", func(t *testing.T) { api := &plugintest.API{} @@ -131,6 +181,26 @@ func TestCreateChannel(t *testing.T) { require.NoError(t, err) }) + t.Run("create space channel with replicas skips the wait", func(t *testing.T) { + api := &plugintest.API{} + defer api.AssertExpectations(t) + client := pluginapi.NewClient(api, &plugintest.Driver{}) + + c := &model.Channel{ + Id: model.NewId(), + Name: "name", + DisplayName: "displayname", + Type: model.ChannelTypeSpace, + } + // The generic GetChannel the replica wait polls excludes space channels, so waiting + // would always time out; Create must return without consulting the config or polling. + api.On("CreateChannel", c).Return(c, nil).Once() + + err := client.Channel.Create(c) + require.NoError(t, err) + api.AssertNotCalled(t, "GetChannel", c.Id) + }) + t.Run("create channel and wait once", func(t *testing.T) { api := &plugintest.API{} defer api.AssertExpectations(t)