From beaa59db5457599949927e1c37f3c129b267b8c5 Mon Sep 17 00:00:00 2001 From: Maria A Nunez Date: Fri, 5 Jun 2026 03:29:59 -0400 Subject: [PATCH] [release-10.11] Restrict group_constrained to channels that support group sync (#36916) Automatic Merge --- server/channels/api4/channel.go | 5 +++ server/channels/api4/channel_test.go | 30 ++++++++++++++ server/channels/store/sqlstore/group_store.go | 2 + server/i18n/en.json | 8 ++++ server/public/model/channel.go | 9 ++++ server/public/model/channel_test.go | 41 +++++++++++++++++++ 6 files changed, 95 insertions(+) diff --git a/server/channels/api4/channel.go b/server/channels/api4/channel.go index df3fa423c5..326de5040e 100644 --- a/server/channels/api4/channel.go +++ b/server/channels/api4/channel.go @@ -345,6 +345,11 @@ func patchChannel(c *Context, w http.ResponseWriter, r *http.Request) { model.AddEventParameterAuditableToAuditRec(auditRec, "channel", patch) auditRec.AddEventPriorState(oldChannel) + if patch.GroupConstrained != nil && !oldChannel.SupportsGroupSync() { + c.Err = model.NewAppError("patchChannel", "api.channel.patch_update_channel.group_constrained_not_allowed.app_error", nil, "", http.StatusBadRequest) + return + } + switch oldChannel.Type { case model.ChannelTypeOpen: if ok, _ := c.App.SessionHasPermissionToChannel(c.AppContext, *c.AppContext.Session(), c.Params.ChannelId, model.PermissionManagePublicChannelProperties); !ok { diff --git a/server/channels/api4/channel_test.go b/server/channels/api4/channel_test.go index 4ac462aae9..d62bc1b1f7 100644 --- a/server/channels/api4/channel_test.go +++ b/server/channels/api4/channel_test.go @@ -872,6 +872,36 @@ func TestPatchChannel(t *testing.T) { CheckBadRequestStatus(t, resp) }) + t.Run("Should block setting group_constrained on group and direct messages", func(t *testing.T) { + user1 := th.CreateUser() + user2 := th.CreateUser() + user3 := th.CreateUser() + + _, err := client.Logout(context.Background()) + require.NoError(t, err) + _, _, err = client.Login(context.Background(), user1.Email, user1.Password) + require.NoError(t, err) + + groupChannel, _, err := client.CreateGroupChannel(context.Background(), []string{user1.Id, user2.Id, user3.Id}) + require.NoError(t, err) + + patch := &model.ChannelPatch{GroupConstrained: model.NewPointer(true)} + _, resp, err := client.PatchChannel(context.Background(), groupChannel.Id, patch) + require.Error(t, err) + CheckBadRequestStatus(t, resp) + + stats, _, err := client.GetChannelStats(context.Background(), groupChannel.Id, "", false) + require.NoError(t, err) + require.Equal(t, int64(3), stats.MemberCount) + + directChannel, _, err := client.CreateDirectChannel(context.Background(), user1.Id, user2.Id) + require.NoError(t, err) + + _, resp, err = client.PatchChannel(context.Background(), directChannel.Id, patch) + require.Error(t, err) + CheckBadRequestStatus(t, resp) + }) + t.Run("Should not be able to configure channel banner without a license", func(t *testing.T) { _, err := client.Logout(context.Background()) require.NoError(t, err) diff --git a/server/channels/store/sqlstore/group_store.go b/server/channels/store/sqlstore/group_store.go index 93bea7d7fe..f529acf1a9 100644 --- a/server/channels/store/sqlstore/group_store.go +++ b/server/channels/store/sqlstore/group_store.go @@ -1268,6 +1268,8 @@ func (s *SqlGroupStore) ChannelMembersToRemove(channelID *string) ([]*model.Chan Join("Channels ON Channels.Id = ChannelMembers.ChannelId"). LeftJoin("Bots ON Bots.UserId = ChannelMembers.UserId"). Where(sq.Eq{"Channels.DeleteAt": 0, "Channels.GroupConstrained": true, "Bots.UserId": nil}). + // Only public/private channels support group sync; never treat other channel members as removable. + Where(sq.Eq{"Channels.Type": []model.ChannelType{model.ChannelTypeOpen, model.ChannelTypePrivate}}). Where(whereStmt) if channelID != nil { diff --git a/server/i18n/en.json b/server/i18n/en.json index 375c7f1a0d..28c5132f5c 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -455,6 +455,10 @@ "id": "api.channel.patch_update_channel.forbidden.app_error", "translation": "Failed to update the channel." }, + { + "id": "api.channel.patch_update_channel.group_constrained_not_allowed.app_error", + "translation": "You are not allowed to set group_constrained on this channel type." + }, { "id": "api.channel.patch_update_channel.update_direct_or_group_messages_not_allowed.app_error", "translation": "You are not allowed to update the name, display_name, and purpose of direct or group messages." @@ -8980,6 +8984,10 @@ "id": "model.channel.is_valid.display_name.app_error", "translation": "Invalid display name." }, + { + "id": "model.channel.is_valid.group_constrained.app_error", + "translation": "Only public and private channels can be group constrained." + }, { "id": "model.channel.is_valid.header.app_error", "translation": "Invalid header." diff --git a/server/public/model/channel.go b/server/public/model/channel.go index 615bbc75cf..cbf9210caf 100644 --- a/server/public/model/channel.go +++ b/server/public/model/channel.go @@ -329,6 +329,10 @@ func (o *Channel) IsValid() *AppError { } } + if o.IsGroupConstrained() && !o.SupportsGroupSync() { + return NewAppError("Channel.IsValid", "model.channel.is_valid.group_constrained.app_error", nil, "id="+o.Id, http.StatusBadRequest) + } + return nil } @@ -356,6 +360,11 @@ func (o *Channel) IsGroupOrDirect() bool { return o.Type == ChannelTypeDirect || o.Type == ChannelTypeGroup } +// SupportsGroupSync reports whether group_constrained is meaningful for the channel type. +func (o *Channel) SupportsGroupSync() bool { + return o.Type == ChannelTypeOpen || o.Type == ChannelTypePrivate +} + func (o *Channel) IsOpen() bool { return o.Type == ChannelTypeOpen } diff --git a/server/public/model/channel_test.go b/server/public/model/channel_test.go index 36758bba6d..64de115550 100644 --- a/server/public/model/channel_test.go +++ b/server/public/model/channel_test.go @@ -35,6 +35,47 @@ func TestChannelPatch(t *testing.T) { require.Equal(t, *p.GroupConstrained, *o.GroupConstrained) } +func TestChannelSupportsGroupSync(t *testing.T) { + require.True(t, (&Channel{Type: ChannelTypeOpen}).SupportsGroupSync()) + require.True(t, (&Channel{Type: ChannelTypePrivate}).SupportsGroupSync()) + require.False(t, (&Channel{Type: ChannelTypeDirect}).SupportsGroupSync()) + require.False(t, (&Channel{Type: ChannelTypeGroup}).SupportsGroupSync()) +} + +func TestChannelIsValidGroupConstrained(t *testing.T) { + base := Channel{ + Id: NewId(), + CreateAt: GetMillis(), + UpdateAt: GetMillis(), + DisplayName: "x", + Name: "valid-name", + Header: "h", + Purpose: "p", + } + + t.Run("group_constrained is allowed on public and private channels", func(t *testing.T) { + c := base + c.GroupConstrained = NewPointer(true) + + c.Type = ChannelTypeOpen + require.Nil(t, c.IsValid()) + + c.Type = ChannelTypePrivate + require.Nil(t, c.IsValid()) + }) + + t.Run("group_constrained is rejected on direct and group channels", func(t *testing.T) { + c := base + c.GroupConstrained = NewPointer(true) + + c.Type = ChannelTypeDirect + require.NotNil(t, c.IsValid()) + + c.Type = ChannelTypeGroup + require.NotNil(t, c.IsValid()) + }) +} + func TestChannelIsValid(t *testing.T) { o := Channel{}