From e6c59693af1778e9bc17c5c34adadfa71d796d1c Mon Sep 17 00:00:00 2001 From: Ibrahim Serdar Acikgoz Date: Thu, 21 May 2026 17:10:51 +0200 Subject: [PATCH] =?UTF-8?q?MM-68763:=20Discoverable=20Private=20Channels?= =?UTF-8?q?=20=E2=80=94=20Server=20feature=20complete=20(visibility,=20ABA?= =?UTF-8?q?C,=20queue=20API)=20(#36580)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- server/channels/api4/channel.go | 145 +++++- server/channels/api4/channel_join_request.go | 293 ++++++++++++ .../api4/channel_join_request_test.go | 154 ++++++ server/channels/app/channel.go | 34 +- .../app/channel_discoverable_visibility.go | 384 +++++++++++++++ .../channel_discoverable_visibility_test.go | 85 ++++ server/channels/app/channel_join_request.go | 447 ++++++++++++++++++ .../channels/app/channel_join_request_test.go | 379 +++++++++++++++ server/channels/app/web_broadcast_hooks.go | 30 ++ .../channels/store/sqlstore/channel_store.go | 14 + server/channels/web/params.go | 4 + server/i18n/en.json | 64 +++ server/public/model/audit_events.go | 3 + 13 files changed, 2033 insertions(+), 3 deletions(-) create mode 100644 server/channels/api4/channel_join_request.go create mode 100644 server/channels/api4/channel_join_request_test.go create mode 100644 server/channels/app/channel_discoverable_visibility.go create mode 100644 server/channels/app/channel_discoverable_visibility_test.go create mode 100644 server/channels/app/channel_join_request.go create mode 100644 server/channels/app/channel_join_request_test.go diff --git a/server/channels/api4/channel.go b/server/channels/api4/channel.go index 344827d0557..815f556da33 100644 --- a/server/channels/api4/channel.go +++ b/server/channels/api4/channel.go @@ -100,6 +100,8 @@ func (api *API) InitChannel() { api.BaseRoutes.ChannelModerations.Handle("", api.APISessionRequired(getChannelModerations)).Methods(http.MethodGet) api.BaseRoutes.ChannelModerations.Handle("/patch", api.APISessionRequired(patchChannelModerations)).Methods(http.MethodPut) + + api.initChannelJoinRequestRoutes() } func createChannel(c *Context, w http.ResponseWriter, r *http.Request) { @@ -144,6 +146,24 @@ func createChannel(c *Context, w http.ResponseWriter, r *http.Request) { return } + if channel.Discoverable { + if !c.App.Config().FeatureFlags.DiscoverableChannels { + c.Err = model.NewAppError("createChannel", "api.channel.discoverable_join_request.feature_disabled.app_error", nil, "", http.StatusBadRequest) + return + } + if channel.Type != model.ChannelTypePrivate { + c.Err = model.NewAppError("createChannel", "model.channel.is_valid.discoverable.app_error", nil, "", http.StatusBadRequest) + return + } + // The team-scoped check is the closest analog to "would this user + // have permission to manage discoverability after the channel is + // created" — channel-scope grants don't exist yet at creation time. + if !c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), channel.TeamId, model.PermissionManagePrivateChannelDiscoverability) { + c.SetPermissionError(model.PermissionManagePrivateChannelDiscoverability) + return + } + } + sc, appErr := c.App.CreateChannelWithUser(c.AppContext, channel, c.AppContext.Session().UserId) if appErr != nil { c.Err = appErr @@ -377,12 +397,36 @@ func patchChannel(c *Context, w http.ResponseWriter, r *http.Request) { updatingProperties := patch.DisplayName != nil || patch.Name != nil || patch.Header != nil || patch.Purpose != nil || patch.GroupConstrained != nil || patch.DefaultCategoryName != nil updatingAutoTranslation := patch.AutoTranslation != nil updatingManagedCategory := patch.ManagedCategoryName != nil + updatingDiscoverable := patch.Discoverable != nil - if !updatingProperties && !updatingAutoTranslation && patch.BannerInfo == nil && !updatingManagedCategory { + if !updatingProperties && !updatingAutoTranslation && patch.BannerInfo == nil && !updatingManagedCategory && !updatingDiscoverable { c.Err = model.NewAppError("patchChannel", "api.channel.patch_update_channel.no_changes.app_error", nil, "", http.StatusBadRequest) return } + if updatingDiscoverable { + if !c.App.Config().FeatureFlags.DiscoverableChannels { + c.Err = model.NewAppError("patchChannel", "api.channel.discoverable_join_request.feature_disabled.app_error", nil, "", http.StatusBadRequest) + return + } + if oldChannel.Type != model.ChannelTypePrivate { + c.Err = model.NewAppError("patchChannel", "model.channel.is_valid.discoverable.app_error", nil, "", http.StatusBadRequest) + return + } + if oldChannel.DeleteAt != 0 { + c.Err = model.NewAppError("patchChannel", "api.channel.update_channel.deleted.app_error", nil, "", http.StatusBadRequest) + return + } + if oldChannel.IsShared() { + c.Err = model.NewAppError("patchChannel", "api.channel.discoverable_join_request.shared.app_error", nil, "", http.StatusBadRequest) + return + } + if ok, _ := c.App.SessionHasPermissionToChannel(c.AppContext, *c.AppContext.Session(), c.Params.ChannelId, model.PermissionManagePrivateChannelDiscoverability); !ok { + c.SetPermissionError(model.PermissionManagePrivateChannelDiscoverability) + return + } + } + if updatingAutoTranslation && (c.App.AutoTranslation() == nil || !c.App.AutoTranslation().IsFeatureAvailable()) { c.Err = model.NewAppError("patchChannel", "api.channel.patch_update_channel.feature_not_available.app_error", nil, "", http.StatusForbidden) return @@ -806,6 +850,9 @@ func getChannel(c *Context, w http.ResponseWriter, r *http.Request) { } } } else if ok, _ := c.App.SessionHasPermissionToChannel(c.AppContext, *c.AppContext.Session(), c.Params.ChannelId, model.PermissionReadChannel); !ok { + if served := serveDiscoverableNonMember(c, w, channel); served { + return + } c.SetPermissionError(model.PermissionReadChannel) return } @@ -822,6 +869,80 @@ func getChannel(c *Context, w http.ResponseWriter, r *http.Request) { } } +// sanitizeDiscoverableChannel returns a copy of `channel` containing only the +// fields safe to expose to a non-member who can see the channel through the +// discoverable surface. Cell-level secrets such as Props or per-channel +// scheme identifiers are stripped so this view is strictly read-only metadata. +func sanitizeDiscoverableChannel(channel *model.Channel) *model.Channel { + if channel == nil { + return nil + } + return &model.Channel{ + Id: channel.Id, + TeamId: channel.TeamId, + Type: channel.Type, + DisplayName: channel.DisplayName, + Name: channel.Name, + Header: channel.Header, + Purpose: channel.Purpose, + Discoverable: channel.Discoverable, + PolicyEnforced: channel.PolicyEnforced, + CreateAt: channel.CreateAt, + UpdateAt: channel.UpdateAt, + DeleteAt: channel.DeleteAt, + } +} + +// discoverableNonMemberView returns a sanitized non-member view of `channel` +// when the calling user qualifies under the discoverable visibility rules, +// or (nil, nil) when the channel must remain hidden — the caller should +// emit its own permission-denied response. Errors from the discoverable +// lookup are returned for the caller to assign to c.Err. When the feature +// flag is off, this returns (nil, nil) and the caller falls through to its +// default 403/404 path so the existing read contract is preserved. +func discoverableNonMemberView(c *Context, channel *model.Channel) (*model.Channel, *model.AppError) { + if !c.App.Config().FeatureFlags.DiscoverableChannels { + return nil, nil + } + user, userErr := c.App.GetUser(c.AppContext.Session().UserId) + if userErr != nil { + return nil, userErr + } + allowed, allowedErr := c.App.IsDiscoverableJoinAllowed(c.AppContext, user, channel) + if allowedErr != nil { + return nil, allowedErr + } + if !allowed { + return nil, nil + } + return sanitizeDiscoverableChannel(channel), nil +} + +// serveDiscoverableNonMember writes the sanitized non-member discoverable +// view of `channel` to `w` and returns true when the request was handled +// here (either the response was written, or c.Err was set on a lookup +// failure). Returns false without touching the response when the caller +// should emit its own permission-denied response (the channel is hidden +// from this non-member, or the feature flag is off). +// +// Centralising this here means every read endpoint that previously emitted +// 403/404 to a non-member can keep its prior failure shape while opting in +// to the discoverable surface with a single `if served { return }` guard. +func serveDiscoverableNonMember(c *Context, w http.ResponseWriter, channel *model.Channel) bool { + sanitized, err := discoverableNonMemberView(c, channel) + if err != nil { + c.Err = err + return true + } + if sanitized == nil { + return false + } + if encErr := json.NewEncoder(w).Encode(sanitized); encErr != nil { + c.Logger.Warn("Error while writing response", mlog.Err(encErr)) + } + return true +} + func getChannelUnread(c *Context, w http.ResponseWriter, r *http.Request) { c.RequireChannelId().RequireUserId() if c.Err != nil { @@ -1646,6 +1767,9 @@ func getChannelByName(c *Context, w http.ResponseWriter, r *http.Request) { // allows team admins to access private channel if !c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), channel.TeamId, model.PermissionManageTeam) { if ok, _ := c.App.SessionHasPermissionToChannel(c.AppContext, *c.AppContext.Session(), channel.Id, model.PermissionReadChannel); !ok { + if served := serveDiscoverableNonMember(c, w, channel); served { + return + } c.Err = model.NewAppError("getChannelByName", "app.channel.get_by_name.missing.app_error", nil, "teamId="+channel.TeamId+", "+"name="+channel.Name+"", http.StatusNotFound) return } @@ -1686,6 +1810,9 @@ func getChannelByNameForTeamName(c *Context, w http.ResponseWriter, r *http.Requ } else if !channelOk { // allows team admins to access private channel if !c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), channel.TeamId, model.PermissionManageTeam) { + if served := serveDiscoverableNonMember(c, w, channel); served { + return + } c.Err = model.NewAppError("getChannelByNameForTeamName", "app.channel.get_by_name.missing.app_error", nil, "teamId="+channel.TeamId+", "+"name="+channel.Name+"", http.StatusNotFound) return } @@ -2252,9 +2379,25 @@ func addChannelMember(c *Context, w http.ResponseWriter, r *http.Request) { if channel.Type == model.ChannelTypePrivate { if hasPermission, _ := c.App.SessionHasPermissionToChannel(c.AppContext, *c.AppContext.Session(), channel.Id, model.PermissionManagePrivateChannelMembers); !hasPermission { + // Allow the user to self-add to a discoverable private channel only + // through the request flow — the discoverable toggle does not + // implicitly grant PermissionManagePrivateChannelMembers, and the + // existing addChannelMember API would otherwise let any caller + // bypass the queue by issuing a direct POST. c.SetPermissionError(model.PermissionManagePrivateChannelMembers) return } + + // Discoverable + no policy: the request flow is the only path. Even + // admins use it to ensure the audit trail. We exempt the case where + // the requester is adding someone other than themselves so admin + // invites still work. + for _, userId := range userIds { + if c.App.IsDiscoverableSelfAddBlocked(c.AppContext, channel, c.AppContext.Session().UserId, userId) { + c.Err = model.NewAppError("addChannelMember", "api.channel.discoverable_join_request.discoverable_requires_approval.app_error", nil, "channel_id="+channel.Id, http.StatusForbidden) + return + } + } } if channel.IsGroupConstrained() { diff --git a/server/channels/api4/channel_join_request.go b/server/channels/api4/channel_join_request.go new file mode 100644 index 00000000000..8f928d3c3b9 --- /dev/null +++ b/server/channels/api4/channel_join_request.go @@ -0,0 +1,293 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package api4 + +import ( + "encoding/json" + "net/http" + "strconv" + + "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/public/shared/mlog" +) + +// initChannelJoinRequestRoutes registers the discoverable-private-channel +// join request endpoints. The route group is split into its own file so the +// handlers stay isolated from the rest of api4/channel.go. +func (api *API) initChannelJoinRequestRoutes() { + if !api.srv.Config().FeatureFlags.DiscoverableChannels { + return + } + + api.BaseRoutes.Channel.Handle("/join_request", api.APISessionRequired(requestJoinChannel)).Methods(http.MethodPost) + api.BaseRoutes.Channel.Handle("/join_request", api.APISessionRequired(getMyChannelJoinRequest)).Methods(http.MethodGet) + api.BaseRoutes.Channel.Handle("/join_request", api.APISessionRequired(withdrawMyChannelJoinRequest)).Methods(http.MethodDelete) + + api.BaseRoutes.Channel.Handle("/join_requests", api.APISessionRequired(getChannelJoinRequests)).Methods(http.MethodGet) + api.BaseRoutes.Channel.Handle("/join_requests/count", api.APISessionRequired(countPendingChannelJoinRequests)).Methods(http.MethodGet) + api.BaseRoutes.Channel.Handle("/join_requests/{request_id:[A-Za-z0-9]+}", api.APISessionRequired(patchChannelJoinRequest)).Methods(http.MethodPatch) + + api.BaseRoutes.User.Handle("/channel_join_requests", api.APISessionRequired(getMyChannelJoinRequests)).Methods(http.MethodGet) +} + +// channelJoinRequestBody is the POST body shape for /channels/{id}/join_request. +type channelJoinRequestBody struct { + Message string `json:"message"` +} + +func requireDiscoverableChannelsEnabled(c *Context, where string) bool { + if !c.App.Config().FeatureFlags.DiscoverableChannels { + c.Err = model.NewAppError(where, "api.channel.discoverable_join_request.feature_disabled.app_error", nil, "", http.StatusNotFound) + return false + } + return true +} + +func requestJoinChannel(c *Context, w http.ResponseWriter, r *http.Request) { + c.RequireChannelId() + if c.Err != nil { + return + } + if !requireDiscoverableChannelsEnabled(c, "requestJoinChannel") { + return + } + + var body channelJoinRequestBody + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + c.SetInvalidParamWithErr("body", err) + return + } + + auditRec := c.MakeAuditRecord(model.AuditEventCreateChannelJoinRequest, model.AuditStatusFail) + defer c.LogAuditRec(auditRec) + model.AddEventParameterToAuditRec(auditRec, "channel_id", c.Params.ChannelId) + model.AddEventParameterToAuditRec(auditRec, "user_id", c.AppContext.Session().UserId) + + joined, req, appErr := c.App.RequestJoinChannel(c.AppContext, c.AppContext.Session().UserId, c.Params.ChannelId, body.Message) + if appErr != nil { + c.Err = appErr + return + } + + auditRec.Success() + if req != nil { + auditRec.AddEventResultState(req) + } + + if joined { + // Mirror the membership endpoint's "no body, just status" semantics + // when the user was added directly via the ABAC fast path. + w.WriteHeader(http.StatusCreated) + if err := json.NewEncoder(w).Encode(map[string]string{"status": model.ChannelJoinRequestStatusApproved}); err != nil { + c.Logger.Warn("Error while writing response", mlog.Err(err)) + } + return + } + + w.WriteHeader(http.StatusCreated) + if err := json.NewEncoder(w).Encode(req); err != nil { + c.Logger.Warn("Error while writing response", mlog.Err(err)) + } +} + +func getMyChannelJoinRequest(c *Context, w http.ResponseWriter, r *http.Request) { + c.RequireChannelId() + if c.Err != nil { + return + } + if !requireDiscoverableChannelsEnabled(c, "getMyChannelJoinRequest") { + return + } + + req, appErr := c.App.GetMyChannelJoinRequest(c.AppContext, c.AppContext.Session().UserId, c.Params.ChannelId) + if appErr != nil { + c.Err = appErr + return + } + + if req == nil { + // Mirror REST conventions: not-found instead of an explicit `null` + // so clients can distinguish "no pending request" from "service down". + w.WriteHeader(http.StatusNotFound) + return + } + + if err := json.NewEncoder(w).Encode(req); err != nil { + c.Logger.Warn("Error while writing response", mlog.Err(err)) + } +} + +func withdrawMyChannelJoinRequest(c *Context, w http.ResponseWriter, r *http.Request) { + c.RequireChannelId() + if c.Err != nil { + return + } + if !requireDiscoverableChannelsEnabled(c, "withdrawMyChannelJoinRequest") { + return + } + + auditRec := c.MakeAuditRecord(model.AuditEventWithdrawChannelJoinRequest, model.AuditStatusFail) + defer c.LogAuditRec(auditRec) + model.AddEventParameterToAuditRec(auditRec, "channel_id", c.Params.ChannelId) + model.AddEventParameterToAuditRec(auditRec, "user_id", c.AppContext.Session().UserId) + + req, appErr := c.App.GetMyChannelJoinRequest(c.AppContext, c.AppContext.Session().UserId, c.Params.ChannelId) + if appErr != nil { + c.Err = appErr + return + } + if req == nil { + c.Err = model.NewAppError("withdrawMyChannelJoinRequest", "app.channel.join_request.not_found.app_error", nil, "channel_id="+c.Params.ChannelId, http.StatusNotFound) + return + } + + updated, appErr := c.App.WithdrawChannelJoinRequest(c.AppContext, req.Id, c.AppContext.Session().UserId) + if appErr != nil { + c.Err = appErr + return + } + + auditRec.Success() + auditRec.AddEventResultState(updated) + + if err := json.NewEncoder(w).Encode(updated); err != nil { + c.Logger.Warn("Error while writing response", mlog.Err(err)) + } +} + +func getChannelJoinRequests(c *Context, w http.ResponseWriter, r *http.Request) { + c.RequireChannelId() + if c.Err != nil { + return + } + if !requireDiscoverableChannelsEnabled(c, "getChannelJoinRequests") { + return + } + + if ok, _ := c.App.SessionHasPermissionToChannel(c.AppContext, *c.AppContext.Session(), c.Params.ChannelId, model.PermissionManageChannelJoinRequests); !ok { + c.SetPermissionError(model.PermissionManageChannelJoinRequests) + return + } + + opts := model.GetChannelJoinRequestsOpts{ + Status: r.URL.Query().Get("status"), + Page: c.Params.Page, + PerPage: c.Params.PerPage, + } + + list, appErr := c.App.GetChannelJoinRequests(c.AppContext, c.Params.ChannelId, opts) + if appErr != nil { + c.Err = appErr + return + } + + if err := json.NewEncoder(w).Encode(list); err != nil { + c.Logger.Warn("Error while writing response", mlog.Err(err)) + } +} + +func countPendingChannelJoinRequests(c *Context, w http.ResponseWriter, r *http.Request) { + c.RequireChannelId() + if c.Err != nil { + return + } + if !requireDiscoverableChannelsEnabled(c, "countPendingChannelJoinRequests") { + return + } + + if ok, _ := c.App.SessionHasPermissionToChannel(c.AppContext, *c.AppContext.Session(), c.Params.ChannelId, model.PermissionManageChannelJoinRequests); !ok { + c.SetPermissionError(model.PermissionManageChannelJoinRequests) + return + } + + count, appErr := c.App.CountPendingChannelJoinRequests(c.AppContext, c.Params.ChannelId) + if appErr != nil { + c.Err = appErr + return + } + + if err := json.NewEncoder(w).Encode(map[string]int64{"count": count}); err != nil { + c.Logger.Warn("Error while writing response", mlog.Err(err)) + } +} + +func patchChannelJoinRequest(c *Context, w http.ResponseWriter, r *http.Request) { + c.RequireChannelId() + if c.Err != nil { + return + } + if !requireDiscoverableChannelsEnabled(c, "patchChannelJoinRequest") { + return + } + if !model.IsValidId(c.Params.RequestId) { + c.SetInvalidURLParam("request_id") + return + } + + if ok, _ := c.App.SessionHasPermissionToChannel(c.AppContext, *c.AppContext.Session(), c.Params.ChannelId, model.PermissionManageChannelJoinRequests); !ok { + c.SetPermissionError(model.PermissionManageChannelJoinRequests) + return + } + + var patch model.ChannelJoinRequestPatch + if err := json.NewDecoder(r.Body).Decode(&patch); err != nil { + c.SetInvalidParamWithErr("channel_join_request_patch", err) + return + } + + auditRec := c.MakeAuditRecord(model.AuditEventUpdateChannelJoinRequest, model.AuditStatusFail) + defer c.LogAuditRec(auditRec) + model.AddEventParameterToAuditRec(auditRec, "channel_id", c.Params.ChannelId) + model.AddEventParameterToAuditRec(auditRec, "request_id", c.Params.RequestId) + model.AddEventParameterToAuditRec(auditRec, "status", patch.Status) + // Capture only the presence of a denial reason in the audit log; the + // free-text contents are intentionally excluded. + model.AddEventParameterToAuditRec(auditRec, "has_denial_reason", strconv.FormatBool(patch.DenialReason != nil && *patch.DenialReason != "")) + + updated, appErr := c.App.UpdateChannelJoinRequest(c.AppContext, c.Params.RequestId, c.Params.ChannelId, &patch, c.AppContext.Session().UserId) + if appErr != nil { + c.Err = appErr + return + } + + auditRec.Success() + auditRec.AddEventResultState(updated) + + if err := json.NewEncoder(w).Encode(updated); err != nil { + c.Logger.Warn("Error while writing response", mlog.Err(err)) + } +} + +func getMyChannelJoinRequests(c *Context, w http.ResponseWriter, r *http.Request) { + c.RequireUserId() + if c.Err != nil { + return + } + if !requireDiscoverableChannelsEnabled(c, "getMyChannelJoinRequests") { + return + } + + // Only the calling user can list their own requests; admins should use + // the per-channel queue endpoint. + if c.Params.UserId != c.AppContext.Session().UserId { + c.SetPermissionError(model.PermissionEditOtherUsers) + return + } + + opts := model.GetChannelJoinRequestsOpts{ + Status: r.URL.Query().Get("status"), + Page: c.Params.Page, + PerPage: c.Params.PerPage, + } + + list, appErr := c.App.GetMyChannelJoinRequests(c.AppContext, c.AppContext.Session().UserId, opts) + if appErr != nil { + c.Err = appErr + return + } + + if err := json.NewEncoder(w).Encode(list); err != nil { + c.Logger.Warn("Error while writing response", mlog.Err(err)) + } +} diff --git a/server/channels/api4/channel_join_request_test.go b/server/channels/api4/channel_join_request_test.go new file mode 100644 index 00000000000..57bfc586854 --- /dev/null +++ b/server/channels/api4/channel_join_request_test.go @@ -0,0 +1,154 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package api4 + +import ( + "context" + "encoding/json" + "io" + "net/http" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/mattermost/mattermost/server/public/model" +) + +// setupDiscoverableTH spins up an api4 fixture with the discoverable channels +// feature flag enabled so the new routes are registered. +func setupDiscoverableTH(t *testing.T) *TestHelper { + t.Helper() + return SetupConfig(t, func(cfg *model.Config) { + cfg.FeatureFlags.DiscoverableChannels = true + }).InitBasic(t) +} + +// markDiscoverableViaAdmin patches `channel` to discoverable=true using the +// SystemAdminClient so the permission check is satisfied without needing to +// rebind the channel-admin role on the test fixture. +func markDiscoverableViaAdmin(t *testing.T, th *TestHelper, channel *model.Channel) *model.Channel { + t.Helper() + on := true + patched, _, err := th.SystemAdminClient.PatchChannel(context.Background(), channel.Id, &model.ChannelPatch{Discoverable: &on}) + require.NoError(t, err) + require.True(t, patched.Discoverable) + return patched +} + +func TestRequestJoinChannelAPI_HappyPath(t *testing.T) { + mainHelper.Parallel(t) + th := setupDiscoverableTH(t) + + channel := th.CreatePrivateChannel(t) + channel = markDiscoverableViaAdmin(t, th, channel) + + other := th.CreateUser(t) + th.LinkUserToTeam(t, other, th.BasicTeam) + _, _, err := th.Client.Login(context.Background(), other.Email, other.Password) + require.NoError(t, err) + + body := []byte(`{"message":"hi"}`) + resp, err := th.Client.DoAPIPost(context.Background(), "/channels/"+channel.Id+"/join_request", string(body)) + require.NoError(t, err) + defer resp.Body.Close() + require.Equal(t, http.StatusCreated, resp.StatusCode) + + var req model.ChannelJoinRequest + require.NoError(t, json.NewDecoder(resp.Body).Decode(&req)) + assert.Equal(t, model.ChannelJoinRequestStatusPending, req.Status) + assert.Equal(t, channel.Id, req.ChannelId) + assert.Equal(t, other.Id, req.UserId) +} + +func TestRequestJoinChannelAPI_FeatureDisabled(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + channel := th.CreatePrivateChannel(t) + body := []byte(`{"message":"hi"}`) + resp, err := th.Client.DoAPIPost(context.Background(), "/channels/"+channel.Id+"/join_request", string(body)) + defer closeBodyOrNil(resp) + require.Error(t, err) + require.NotNil(t, resp) + assert.Equal(t, http.StatusNotFound, resp.StatusCode, "route must be unregistered when feature flag is off") +} + +func TestPatchChannelDiscoverable_RejectsNonPrivate(t *testing.T) { + mainHelper.Parallel(t) + th := setupDiscoverableTH(t) + + publicChannel := th.CreatePublicChannel(t) + on := true + _, resp, err := th.SystemAdminClient.PatchChannel(context.Background(), publicChannel.Id, &model.ChannelPatch{Discoverable: &on}) + require.Error(t, err) + require.NotNil(t, resp) + assert.Equal(t, http.StatusBadRequest, resp.StatusCode) +} + +func TestAddChannelMember_BlocksSelfAddOnDiscoverable(t *testing.T) { + mainHelper.Parallel(t) + th := setupDiscoverableTH(t) + + channel := th.CreatePrivateChannel(t) + channel = markDiscoverableViaAdmin(t, th, channel) + + // Add a user that has manage-private-channel-members on a different + // channel but not this one. Use Client (BasicUser2) - they're a team + // member but not yet a channel member here. + _, _, err := th.Client.Login(context.Background(), th.BasicUser2.Email, th.BasicUser2.Password) + require.NoError(t, err) + + _, resp, err := th.Client.AddChannelMember(context.Background(), channel.Id, th.BasicUser2.Id) + require.Error(t, err) + require.NotNil(t, resp) + // Without channel admin permission the underlying permission check + // fails first; either way the request flow is what they need to use. + assert.True(t, resp.StatusCode == http.StatusForbidden || resp.StatusCode == http.StatusUnauthorized, + "got %d", resp.StatusCode) +} + +func TestGetChannelByName_HiddenForNonQualifyingNonMember(t *testing.T) { + mainHelper.Parallel(t) + th := setupDiscoverableTH(t) + + // Plain (non-discoverable) private channel: a non-member must still get + // 404 — this guards against a regression in the existing read paths. + channel := th.CreatePrivateChannel(t) + + _, _, err := th.Client.Login(context.Background(), th.BasicUser2.Email, th.BasicUser2.Password) + require.NoError(t, err) + + _, resp, err := th.Client.GetChannelByName(context.Background(), channel.Name, th.BasicTeam.Id, "") + require.Error(t, err) + require.NotNil(t, resp) + assert.Equal(t, http.StatusNotFound, resp.StatusCode) +} + +func TestGetChannelByName_VisibleForQualifyingNonMemberOnDiscoverable(t *testing.T) { + mainHelper.Parallel(t) + th := setupDiscoverableTH(t) + + channel := th.CreatePrivateChannel(t) + channel = markDiscoverableViaAdmin(t, th, channel) + + _, _, err := th.Client.Login(context.Background(), th.BasicUser2.Email, th.BasicUser2.Password) + require.NoError(t, err) + + got, _, err := th.Client.GetChannelByName(context.Background(), channel.Name, th.BasicTeam.Id, "") + require.NoError(t, err) + require.NotNil(t, got) + assert.Equal(t, channel.Id, got.Id) + assert.True(t, got.Discoverable) +} + +// closeBodyOrNil is a tiny helper so the negative-path tests don't need to +// branch on a nil response body before deferring Close. +func closeBodyOrNil(resp *http.Response) { + if resp == nil || resp.Body == nil { + return + } + _, _ = io.Copy(io.Discard, resp.Body) + _ = resp.Body.Close() +} diff --git a/server/channels/app/channel.go b/server/channels/app/channel.go index 713e1cbea46..99c32bb8e58 100644 --- a/server/channels/app/channel.go +++ b/server/channels/app/channel.go @@ -835,6 +835,14 @@ 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) { + wasDiscoverable := oldChannel.Discoverable + // Public channels are inherently joinable; the discoverable flag only + // has meaning for private channels. Clear it eagerly so callers reading + // the row mid-conversion don't see an inconsistent state. + if oldChannel.Type == model.ChannelTypeOpen { + oldChannel.Discoverable = false + } + channel, err := a.UpdateChannel(rctx, oldChannel) if err != nil { return channel, err @@ -844,6 +852,11 @@ func (a *App) UpdateChannelPrivacy(rctx request.CTX, oldChannel *model.Channel, if postErr != nil { if channel.Type == model.ChannelTypeOpen { channel.Type = model.ChannelTypePrivate + // Restore the discoverable flag we eagerly cleared above so + // the rollback fully undoes the conversion. Without this the + // caller would see a private channel with discoverable=false + // (and would have to re-toggle it). + channel.Discoverable = wasDiscoverable } else { channel.Type = model.ChannelTypeOpen } @@ -854,6 +867,19 @@ func (a *App) UpdateChannelPrivacy(rctx request.CTX, oldChannel *model.Channel, return channel, postErr } + // Now that the conversion is fully committed, cancel pending join + // requests for the formerly discoverable private channel — the WS + // broadcast inside the helper updates each requester's My Pending + // Requests list in real-time. Doing this after the privacy-message + // step ensures a transient post failure (which triggers the rollback + // above) cannot leave requests cancelled against a still-private + // channel. + if wasDiscoverable && channel.Type == model.ChannelTypeOpen { + a.Srv().Go(func() { + a.CancelPendingChannelJoinRequestsOnConvert(rctx, channel) + }) + } + a.Srv().Platform().InvalidateCacheForChannel(channel) messageWs := model.NewWebSocketEvent(model.WebsocketEventChannelConverted, channel.TeamId, "", "", nil, "") @@ -3229,6 +3255,10 @@ func (a *App) AutocompleteChannels(rctx request.CTX, userID, term string) (model return nil, model.NewAppError("AutocompleteChannels", "app.channel.search.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } + channelList, _, appErr = a.FilterChannelListWithTeamDataForUserVisibility(rctx, channelList, userID) + if appErr != nil { + return nil, appErr + } return channelList, nil } @@ -3246,7 +3276,7 @@ func (a *App) AutocompleteChannelsForTeam(rctx request.CTX, teamID, userID, term return nil, model.NewAppError("AutocompleteChannels", "app.channel.search.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } - return channelList, nil + return a.FilterChannelListForUserVisibility(rctx, channelList, userID) } func (a *App) AutocompleteChannelsForTeamFiltered(rctx request.CTX, teamID, userID, term string, privateOnly, excludeGroupConstrained bool) (model.ChannelList, *model.AppError) { @@ -3263,7 +3293,7 @@ func (a *App) AutocompleteChannelsForTeamFiltered(rctx request.CTX, teamID, user return nil, model.NewAppError("AutocompleteChannelsForTeamFiltered", "app.channel.search.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } - return channelList, nil + return a.FilterChannelListForUserVisibility(rctx, channelList, userID) } func (a *App) AutocompleteChannelsForSearch(rctx request.CTX, teamID string, userID string, term string) (model.ChannelList, *model.AppError) { diff --git a/server/channels/app/channel_discoverable_visibility.go b/server/channels/app/channel_discoverable_visibility.go new file mode 100644 index 00000000000..95cf7a2c012 --- /dev/null +++ b/server/channels/app/channel_discoverable_visibility.go @@ -0,0 +1,384 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package app + +import ( + "context" + "sync" + + "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/public/shared/mlog" + "github.com/mattermost/mattermost/server/public/shared/request" +) + +// channelVisibilityCacheKey is the per-request request.CTX value key used to +// memoise PDP membership decisions across N+1 channel filtering work in a +// single Browse Channels load. +type channelVisibilityCacheKey struct{} + +type channelVisibilityCache struct { + mu sync.Mutex + decisions map[string]bool +} + +func getChannelVisibilityCache(rctx request.CTX) *channelVisibilityCache { + if v := rctx.Context().Value(channelVisibilityCacheKey{}); v != nil { + if cache, ok := v.(*channelVisibilityCache); ok { + return cache + } + } + return nil +} + +// withChannelVisibilityCache returns a request context that memoises PDP +// membership decisions across the visibility filter calls in a single request. +// It's safe to call this multiple times — only the outermost installation +// allocates a cache. +func withChannelVisibilityCache(rctx request.CTX) request.CTX { + if getChannelVisibilityCache(rctx) != nil { + return rctx + } + cache := &channelVisibilityCache{decisions: map[string]bool{}} + return rctx.WithContext(context.WithValue(rctx.Context(), channelVisibilityCacheKey{}, cache)) +} + +func (c *channelVisibilityCache) get(channelID string) (bool, bool) { + c.mu.Lock() + defer c.mu.Unlock() + v, ok := c.decisions[channelID] + return v, ok +} + +func (c *channelVisibilityCache) set(channelID string, allow bool) { + c.mu.Lock() + defer c.mu.Unlock() + c.decisions[channelID] = allow +} + +// FilterDiscoverableChannelsByPolicy removes from `channels` any +// policy-enforced private channel that the user fails to satisfy — the +// security-critical visibility invariant in plan §6c. Channels without an +// active policy are returned untouched. Callers that need the additional +// "non-member private must be discoverable" gate should use +// FilterChannelsForUserVisibility instead. +// +// Failure modes are fail-secure: a missing AccessControl service, a +// subject-build failure, or any PDP error drops the offending channel from +// the result so a non-qualifying user can never be inadvertently shown a +// gated channel. Decisions are cached per-request via the request.CTX value +// bag installed by withChannelVisibilityCache. +func (a *App) FilterDiscoverableChannelsByPolicy(rctx request.CTX, channels []*model.Channel, userID string) ([]*model.Channel, *model.AppError) { + if len(channels) == 0 { + return channels, nil + } + + if !a.Config().FeatureFlags.DiscoverableChannels { + return channels, nil + } + + rctx = withChannelVisibilityCache(rctx) + cache := getChannelVisibilityCache(rctx) + + var ( + user *model.User + userErr *model.AppError + userOnce sync.Once + filtered = make([]*model.Channel, 0, len(channels)) + dropCount int + ) + + for _, channel := range channels { + if channel == nil { + continue + } + + if !channel.PolicyEnforced || channel.Type != model.ChannelTypePrivate || !channel.Discoverable { + filtered = append(filtered, channel) + continue + } + + if cached, ok := cache.get(channel.Id); ok { + if cached { + filtered = append(filtered, channel) + } else { + dropCount++ + } + continue + } + + userOnce.Do(func() { + user, userErr = a.GetUser(userID) + }) + if userErr != nil { + return nil, userErr + } + + // Guests are never permitted to see discoverable private channels. + if user.IsGuest() { + cache.set(channel.Id, false) + dropCount++ + continue + } + + decision, evalErr := a.evaluateChannelMembership(rctx, user, channel) + if evalErr != nil { + rctx.Logger().Warn("FilterDiscoverableChannelsByPolicy: PDP error, hiding channel (fail-secure)", + mlog.String("user_id", userID), + mlog.String("channel_id", channel.Id), + mlog.Err(evalErr), + ) + cache.set(channel.Id, false) + dropCount++ + continue + } + cache.set(channel.Id, decision) + if decision { + filtered = append(filtered, channel) + } else { + dropCount++ + } + } + + return filtered, nil +} + +// FilterChannelsForUserVisibility wraps FilterDiscoverableChannelsByPolicy with +// the secondary invariant: a non-member private channel must be discoverable +// to be visible at all. The caller is expected to scope `channels` to results +// where the user is a non-member; member channels should not be passed +// through this filter (their visibility is governed by membership alone). +// +// In practice the search/autocomplete store paths return a mix of member and +// non-member rows; callers should pass the full list because the helper +// detects membership-implying fields. The current implementation only checks +// the discoverability gate (the SQL-level membership join already excluded +// unaffiliated channels). +func (a *App) FilterChannelsForUserVisibility(rctx request.CTX, channels []*model.Channel, userID string) ([]*model.Channel, *model.AppError) { + return a.FilterDiscoverableChannelsByPolicy(rctx, channels, userID) +} + +// FilterChannelListForUserVisibility is the convenience overload for +// model.ChannelList callers (the standard list shape returned by app-layer +// search functions). +func (a *App) FilterChannelListForUserVisibility(rctx request.CTX, channels model.ChannelList, userID string) (model.ChannelList, *model.AppError) { + filtered, err := a.FilterChannelsForUserVisibility(rctx, channels, userID) + if err != nil { + return nil, err + } + return model.ChannelList(filtered), nil +} + +// FilterChannelListWithTeamDataForUserVisibility filters the team-data list +// shape used by Autocomplete and SearchAllChannels. The function preserves +// the embedded TeamDisplayName / TeamName fields. Returns the post-filter +// total adjustment so paginated callers can shrink TotalCount alongside the +// trimmed result set. +func (a *App) FilterChannelListWithTeamDataForUserVisibility(rctx request.CTX, channels model.ChannelListWithTeamData, userID string) (model.ChannelListWithTeamData, int, *model.AppError) { + if len(channels) == 0 { + return channels, 0, nil + } + + if !a.Config().FeatureFlags.DiscoverableChannels { + return channels, 0, nil + } + + rctx = withChannelVisibilityCache(rctx) + cache := getChannelVisibilityCache(rctx) + + var ( + user *model.User + userErr *model.AppError + userOnce sync.Once + out = make(model.ChannelListWithTeamData, 0, len(channels)) + dropped int + ) + + for i := range channels { + ch := channels[i] + if !ch.PolicyEnforced || ch.Type != model.ChannelTypePrivate || !ch.Discoverable { + out = append(out, ch) + continue + } + + if cached, ok := cache.get(ch.Id); ok { + if cached { + out = append(out, ch) + } else { + dropped++ + } + continue + } + + userOnce.Do(func() { + user, userErr = a.GetUser(userID) + }) + if userErr != nil { + return nil, 0, userErr + } + + if user.IsGuest() { + cache.set(ch.Id, false) + dropped++ + continue + } + + decision, evalErr := a.evaluateChannelMembership(rctx, user, &ch.Channel) + if evalErr != nil { + rctx.Logger().Warn("FilterChannelListWithTeamDataForUserVisibility: PDP error, hiding channel (fail-secure)", + mlog.String("user_id", userID), + mlog.String("channel_id", ch.Id), + mlog.Err(evalErr), + ) + cache.set(ch.Id, false) + dropped++ + continue + } + cache.set(ch.Id, decision) + if decision { + out = append(out, ch) + } else { + dropped++ + } + } + + return out, dropped, nil +} + +// IsDiscoverableJoinAllowed reports whether `user` may view `channel` as a +// non-member through the discoverable-channels surface. Returns 404 (mapped +// by callers) when the channel is hidden from this user — matching the +// "indistinguishable from a non-existent channel" requirement so the policy +// cannot act as an existence oracle. +func (a *App) IsDiscoverableJoinAllowed(rctx request.CTX, user *model.User, channel *model.Channel) (bool, *model.AppError) { + if channel == nil { + return false, nil + } + if channel.Type != model.ChannelTypePrivate || !channel.Discoverable { + return false, nil + } + if user == nil || user.IsGuest() || user.DeleteAt != 0 { + return false, nil + } + if channel.DeleteAt != 0 || channel.IsShared() { + return false, nil + } + if !channel.PolicyEnforced { + return true, nil + } + decision, evalErr := a.evaluateChannelMembership(rctx, user, channel) + if evalErr != nil { + // Fail-secure: PDP failure hides the channel rather than leak it. + rctx.Logger().Warn("IsDiscoverableJoinAllowed: PDP error, hiding channel (fail-secure)", + mlog.String("user_id", user.Id), + mlog.String("channel_id", channel.Id), + mlog.Err(evalErr), + ) + return false, nil + } + return decision, nil +} + +// CancelPendingChannelJoinRequestsOnConvert transitions every pending request +// for a channel to the withdrawn state — used when the channel is converted +// to public (open channels are inherently joinable, so a pending queue is +// nonsensical) and when the channel is archived. Failures are logged because +// the conversion / archive must not be blocked. +func (a *App) CancelPendingChannelJoinRequestsOnConvert(rctx request.CTX, channel *model.Channel) { + if channel == nil { + return + } + + const ( + pageSize = 200 + maxIterations = 50 // hard cap at ~10k requests per channel + ) + for range maxIterations { + opts := model.GetChannelJoinRequestsOpts{ + Status: model.ChannelJoinRequestStatusPending, + Page: 0, + PerPage: pageSize, + } + rows, _, err := a.Srv().Store().ChannelJoinRequest().GetForChannel(channel.Id, opts) + if err != nil { + rctx.Logger().Warn("CancelPendingChannelJoinRequestsOnConvert: failed to list pending requests", + mlog.String("channel_id", channel.Id), + mlog.Err(err), + ) + return + } + if len(rows) == 0 { + return + } + failed := 0 + for _, row := range rows { + row.Status = model.ChannelJoinRequestStatusWithdrawn + row.Message = "" + updated, updateErr := a.Srv().Store().ChannelJoinRequest().Update(row) + if updateErr != nil { + failed++ + rctx.Logger().Warn("CancelPendingChannelJoinRequestsOnConvert: failed to withdraw pending request", + mlog.String("channel_id", channel.Id), + mlog.String("request_id", row.Id), + mlog.Err(updateErr), + ) + continue + } + a.broadcastChannelJoinRequestUpdated(rctx, channel, updated) + } + // If every row in the batch failed to update, the next iteration + // would re-fetch the same rows and loop forever. Break out and + // surface the situation in the log — the operator can re-run the + // cleanup manually after addressing the underlying store error. + if failed == len(rows) { + rctx.Logger().Warn("CancelPendingChannelJoinRequestsOnConvert: every row in batch failed to update, aborting to avoid infinite loop", + mlog.String("channel_id", channel.Id), + mlog.Int("failed", failed), + ) + return + } + // Standard exit when the last page is partial: every remaining + // pending row was successfully withdrawn (or logged as failed). + if len(rows) < pageSize { + return + } + } + // maxIterations safety net — this should be effectively unreachable + // because the per-batch all-failed check above already aborts on + // systemic update failures. Fire a higher-severity log if we hit it. + rctx.Logger().Error("CancelPendingChannelJoinRequestsOnConvert: hit maxIterations, aborting", + mlog.String("channel_id", channel.Id), + mlog.Int("max_iterations", maxIterations), + ) +} + +// IsDiscoverableSelfAddBlocked reports whether a user trying to self-add to +// `channel` via POST /channels/{id}/members must instead go through the +// request flow. The block applies only when: +// - the channel is private, +// - it is discoverable but does NOT have an active ABAC policy +// (channels with a policy use the existing PDP gate inside +// addUserToChannel — admins can still add others by policy), +// - the user is not yet a member, +// - and the requester is the user themselves. +// +// Other paths (admin invites, API by reviewer ID) are unaffected: the request +// flow exists to give admins a queue, not to block invites. +func (a *App) IsDiscoverableSelfAddBlocked(rctx request.CTX, channel *model.Channel, requesterUserID, targetUserID string) bool { + if channel == nil || channel.Type != model.ChannelTypePrivate { + return false + } + if !channel.Discoverable { + return false + } + if channel.PolicyEnforced { + return false + } + if requesterUserID != targetUserID { + return false + } + if !a.Config().FeatureFlags.DiscoverableChannels { + return false + } + return true +} diff --git a/server/channels/app/channel_discoverable_visibility_test.go b/server/channels/app/channel_discoverable_visibility_test.go new file mode 100644 index 00000000000..ada36a94d7f --- /dev/null +++ b/server/channels/app/channel_discoverable_visibility_test.go @@ -0,0 +1,85 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package app + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestDiscoverableVisibilityInvariant_NonGuestSeesNoPolicy verifies that a +// discoverable + no-policy private channel is returned through the +// non-member autocomplete path for a non-guest user. +// +// The complementary policy-enforced + non-qualifying user case is covered +// by TestFilterDiscoverableChannelsByPolicy_PolicyEnforcedFailSecure (which +// checks the fail-secure path) and the dedicated guest case is in +// TestFilterDiscoverableChannelsByPolicy_GuestHidden. +func TestDiscoverableVisibilityInvariant_NonGuestSeesNoPolicy(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + withDiscoverableChannelsFlag(t, th, true) + + channel := markDiscoverable(t, th, th.CreatePrivateChannel(t, th.BasicTeam)) + + // BasicUser2 is a member of the team but NOT of `channel`. The + // autocomplete query must still surface the channel because of the + // discoverable OR-branch (post-query ABAC filter is a no-op since the + // channel has no policy). + results, appErr := th.App.AutocompleteChannelsForTeam(th.Context, th.BasicTeam.Id, th.BasicUser2.Id, channel.Name) + require.Nil(t, appErr) + + found := false + for _, c := range results { + if c.Id == channel.Id { + found = true + break + } + } + assert.True(t, found, "discoverable + no-policy private channel must appear in autocomplete for a non-member non-guest") +} + +// TestDiscoverableVisibilityInvariant_NonDiscoverableHidden ensures that the +// store-level OR-branch we added does not inadvertently leak private +// channels with discoverable=false to non-members. The new OR clause must be +// gated on `Discoverable=true`. +func TestDiscoverableVisibilityInvariant_NonDiscoverableHidden(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + withDiscoverableChannelsFlag(t, th, true) + + plain := th.CreatePrivateChannel(t, th.BasicTeam) + + results, appErr := th.App.AutocompleteChannelsForTeam(th.Context, th.BasicTeam.Id, th.BasicUser2.Id, plain.Name) + require.Nil(t, appErr) + + for _, c := range results { + assert.NotEqual(t, plain.Id, c.Id, "non-discoverable private channel must remain hidden from non-members") + } +} + +// TestDiscoverableVisibilityInvariant_GuestHidden re-verifies the guest path +// at the autocomplete level (the unit-level guest case lives in +// TestFilterDiscoverableChannelsByPolicy_GuestHidden, but this test exercises +// the full app+store integration so we don't accidentally rely on the +// in-memory filter alone). +func TestDiscoverableVisibilityInvariant_GuestHidden(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + withDiscoverableChannelsFlag(t, th, true) + + channel := markDiscoverable(t, th, th.CreatePrivateChannel(t, th.BasicTeam)) + + guest := th.CreateGuest(t) + th.LinkUserToTeam(t, guest, th.BasicTeam) + + results, appErr := th.App.AutocompleteChannelsForTeam(th.Context, th.BasicTeam.Id, guest.Id, channel.Name) + require.Nil(t, appErr) + + for _, c := range results { + assert.NotEqual(t, channel.Id, c.Id, "guests must never see discoverable private channels in autocomplete") + } +} diff --git a/server/channels/app/channel_join_request.go b/server/channels/app/channel_join_request.go new file mode 100644 index 00000000000..41e4189844d --- /dev/null +++ b/server/channels/app/channel_join_request.go @@ -0,0 +1,447 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package app + +import ( + "encoding/json" + "errors" + "net/http" + + "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/public/shared/mlog" + "github.com/mattermost/mattermost/server/public/shared/request" + "github.com/mattermost/mattermost/server/v8/channels/store" +) + +// channelJoinRequestPaginationDefaultPerPage matches the public /api/v4 default +// for paginated endpoints. +const channelJoinRequestPaginationDefaultPerPage = 60 + +// channelJoinRequestPaginationMaxPerPage caps a single page's size; mirrors the +// 200 cap shared by other public list endpoints. +const channelJoinRequestPaginationMaxPerPage = 200 + +// requestJoinChannelGuard validates that a user is allowed to express interest +// in joining `channel` and returns a sanitized result for `channel`. Callers +// are expected to look up `channel` via the store before calling this helper. +func (a *App) requestJoinChannelGuard(rctx request.CTX, user *model.User, channel *model.Channel) *model.AppError { + if channel == nil { + return model.NewAppError("RequestJoinChannel", "app.channel.get.existing.app_error", nil, "", http.StatusNotFound) + } + + if channel.DeleteAt != 0 { + return model.NewAppError("RequestJoinChannel", "api.channel.discoverable_join_request.archived.app_error", nil, "channel_id="+channel.Id, http.StatusBadRequest) + } + + if channel.Type != model.ChannelTypePrivate { + return model.NewAppError("RequestJoinChannel", "api.channel.discoverable_join_request.not_private.app_error", nil, "channel_id="+channel.Id, http.StatusBadRequest) + } + + if !channel.Discoverable { + return model.NewAppError("RequestJoinChannel", "api.channel.discoverable_join_request.not_discoverable.app_error", nil, "channel_id="+channel.Id, http.StatusForbidden) + } + + // Shared channels join through their own remote-cluster sync mechanism. + if channel.IsShared() { + return model.NewAppError("RequestJoinChannel", "api.channel.discoverable_join_request.shared.app_error", nil, "channel_id="+channel.Id, http.StatusBadRequest) + } + + if user.IsGuest() { + return model.NewAppError("RequestJoinChannel", "api.channel.discoverable_join_request.guest.app_error", nil, "user_id="+user.Id, http.StatusForbidden) + } + + if user.DeleteAt != 0 { + return model.NewAppError("RequestJoinChannel", "app.channel.add_member.deleted_user.app_error", nil, "", http.StatusForbidden) + } + + return nil +} + +// RequestJoinChannel decides between an immediate ABAC-gated auto-join and an +// asynchronous request-to-join row. +// +// Returns the persisted ChannelJoinRequest when the user must wait for an +// admin review, or nil when the user was added directly to the channel (the +// caller can detect this via the `joined` return value). +func (a *App) RequestJoinChannel(rctx request.CTX, userID, channelID, message string) (joined bool, req *model.ChannelJoinRequest, appErr *model.AppError) { + user, appErr := a.GetUser(userID) + if appErr != nil { + return false, nil, appErr + } + + channel, appErr := a.GetChannel(rctx, channelID) + if appErr != nil { + return false, nil, appErr + } + + if guardErr := a.requestJoinChannelGuard(rctx, user, channel); guardErr != nil { + return false, nil, guardErr + } + + _, memberErr := a.Srv().Store().Channel().GetMember(rctx, channel.Id, user.Id) + if memberErr == nil { + return false, nil, model.NewAppError("RequestJoinChannel", "api.channel.discoverable_join_request.already_member.app_error", nil, "channel_id="+channel.Id, http.StatusBadRequest) + } + var nfErr *store.ErrNotFound + if !errors.As(memberErr, &nfErr) { + return false, nil, model.NewAppError("RequestJoinChannel", "app.channel.get_member.app_error", nil, "", http.StatusInternalServerError).Wrap(memberErr) + } + + enforced, appErr := a.ChannelAccessControlled(rctx, channel.Id) + if appErr != nil { + return false, nil, appErr + } + + // ABAC gate: when an active policy is attached and the user qualifies, add + // the member directly. AddChannelMember re-runs the PDP gate inside + // addUserToChannel, so a denial here is authoritative; a non-allow result + // falls through to the request-row path below ONLY when there is no policy. + if enforced { + decision, evalErr := a.evaluateChannelMembership(rctx, user, channel) + if evalErr != nil { + return false, nil, evalErr + } + if !decision { + return false, nil, model.NewAppError("RequestJoinChannel", "api.channel.discoverable_join_request.policy_denied.app_error", nil, "channel_id="+channel.Id, http.StatusForbidden) + } + + if _, err := a.AddChannelMember(rctx, user.Id, channel, ChannelMemberOpts{UserRequestorID: user.Id}); err != nil { + return false, nil, err + } + return true, nil, nil + } + + pending := &model.ChannelJoinRequest{ + ChannelId: channel.Id, + UserId: user.Id, + Message: message, + } + + saved, err := a.Srv().Store().ChannelJoinRequest().Save(pending) + if err != nil { + var conflict *store.ErrConflict + if errors.As(err, &conflict) { + existing, getErr := a.Srv().Store().ChannelJoinRequest().GetPendingForChannelAndUser(channel.Id, user.Id) + if getErr == nil { + return false, existing, nil + } + return false, nil, model.NewAppError("RequestJoinChannel", "api.channel.discoverable_join_request.duplicate.app_error", nil, "channel_id="+channel.Id, http.StatusConflict) + } + if appErr, ok := err.(*model.AppError); ok { + return false, nil, appErr + } + return false, nil, model.NewAppError("RequestJoinChannel", "app.channel.join_request.save.app_error", nil, "", http.StatusInternalServerError).Wrap(err) + } + + a.broadcastChannelJoinRequestCreated(rctx, channel, saved) + return false, saved, nil +} + +// WithdrawChannelJoinRequest flips a pending request the calling user owns to +// the withdrawn state. Non-owners receive a 404 (no oracle on existence) and +// already-terminal rows return 409. +func (a *App) WithdrawChannelJoinRequest(rctx request.CTX, requestID, userID string) (*model.ChannelJoinRequest, *model.AppError) { + current, err := a.Srv().Store().ChannelJoinRequest().Get(requestID) + if err != nil { + var nfErr *store.ErrNotFound + if errors.As(err, &nfErr) { + return nil, model.NewAppError("WithdrawChannelJoinRequest", "app.channel.join_request.not_found.app_error", nil, "request_id="+requestID, http.StatusNotFound) + } + return nil, model.NewAppError("WithdrawChannelJoinRequest", "app.channel.join_request.get.app_error", nil, "", http.StatusInternalServerError).Wrap(err) + } + + if current.UserId != userID { + // Hide the row from non-owners by returning the same not-found + // response. The reviewer flow uses different endpoints. + return nil, model.NewAppError("WithdrawChannelJoinRequest", "app.channel.join_request.not_found.app_error", nil, "request_id="+requestID, http.StatusNotFound) + } + + if current.Status != model.ChannelJoinRequestStatusPending { + return nil, model.NewAppError("WithdrawChannelJoinRequest", "api.channel.discoverable_join_request.not_pending.app_error", nil, "request_id="+requestID, http.StatusConflict) + } + + current.Status = model.ChannelJoinRequestStatusWithdrawn + current.Message = "" + + updated, err := a.Srv().Store().ChannelJoinRequest().Update(current) + if err != nil { + if appErr, ok := err.(*model.AppError); ok { + return nil, appErr + } + return nil, model.NewAppError("WithdrawChannelJoinRequest", "app.channel.join_request.update.app_error", nil, "", http.StatusInternalServerError).Wrap(err) + } + + channel, channelErr := a.GetChannel(rctx, updated.ChannelId) + if channelErr != nil { + // Channel went away mid-flight — still report the update; we just + // can't broadcast to the admin queue. + rctx.Logger().Warn("WithdrawChannelJoinRequest: failed to load channel for broadcast", mlog.String("channel_id", updated.ChannelId), mlog.Err(channelErr)) + return updated, nil + } + a.broadcastChannelJoinRequestUpdated(rctx, channel, updated) + return updated, nil +} + +// GetMyChannelJoinRequest returns the calling user's active pending request for +// `channelID`, or nil if none exists. It never returns an error for a missing +// row — that's the non-pending state and is expected. +func (a *App) GetMyChannelJoinRequest(rctx request.CTX, userID, channelID string) (*model.ChannelJoinRequest, *model.AppError) { + req, err := a.Srv().Store().ChannelJoinRequest().GetPendingForChannelAndUser(channelID, userID) + if err != nil { + var nfErr *store.ErrNotFound + if errors.As(err, &nfErr) { + return nil, nil + } + return nil, model.NewAppError("GetMyChannelJoinRequest", "app.channel.join_request.get.app_error", nil, "", http.StatusInternalServerError).Wrap(err) + } + return req, nil +} + +// GetMyChannelJoinRequests lists the calling user's join requests across all +// channels. The "My Pending Requests" tab filters by `Status="pending"` (the +// default when opts.Status is empty). +func (a *App) GetMyChannelJoinRequests(rctx request.CTX, userID string, opts model.GetChannelJoinRequestsOpts) (*model.ChannelJoinRequestList, *model.AppError) { + opts = sanitizeJoinRequestListOpts(opts) + rows, total, err := a.Srv().Store().ChannelJoinRequest().GetForUser(userID, opts) + if err != nil { + return nil, model.NewAppError("GetMyChannelJoinRequests", "app.channel.join_request.get.app_error", nil, "", http.StatusInternalServerError).Wrap(err) + } + return &model.ChannelJoinRequestList{Requests: rows, TotalCount: total}, nil +} + +// GetChannelJoinRequests lists the join requests targeting `channelID` for the +// admin queue UI. The visibility check is performed by the API layer via the +// PermissionManageChannelJoinRequests permission. +func (a *App) GetChannelJoinRequests(rctx request.CTX, channelID string, opts model.GetChannelJoinRequestsOpts) (*model.ChannelJoinRequestList, *model.AppError) { + opts = sanitizeJoinRequestListOpts(opts) + rows, total, err := a.Srv().Store().ChannelJoinRequest().GetForChannel(channelID, opts) + if err != nil { + return nil, model.NewAppError("GetChannelJoinRequests", "app.channel.join_request.get.app_error", nil, "", http.StatusInternalServerError).Wrap(err) + } + return &model.ChannelJoinRequestList{Requests: rows, TotalCount: total}, nil +} + +// CountPendingChannelJoinRequests returns the number of pending join requests +// for `channelID`, used by the channel-header badge. +func (a *App) CountPendingChannelJoinRequests(rctx request.CTX, channelID string) (int64, *model.AppError) { + count, err := a.Srv().Store().ChannelJoinRequest().CountPending(channelID) + if err != nil { + return 0, model.NewAppError("CountPendingChannelJoinRequests", "app.channel.join_request.get.app_error", nil, "", http.StatusInternalServerError).Wrap(err) + } + return count, nil +} + +// UpdateChannelJoinRequest applies an admin review (approve / deny) to a +// pending request. When approving, the user is added via AddChannelMember so +// the existing PDP gate inside addUserToChannel re-runs — admins cannot bypass +// an active ABAC policy. The store row is only updated after a successful add +// to keep the audit trail consistent. +func (a *App) UpdateChannelJoinRequest(rctx request.CTX, requestID, channelID string, patch *model.ChannelJoinRequestPatch, reviewerID string) (*model.ChannelJoinRequest, *model.AppError) { + if patch == nil { + return nil, model.NewAppError("UpdateChannelJoinRequest", "api.channel.discoverable_join_request.invalid_patch.app_error", nil, "", http.StatusBadRequest) + } + + switch patch.Status { + case model.ChannelJoinRequestStatusApproved, model.ChannelJoinRequestStatusDenied: + default: + return nil, model.NewAppError("UpdateChannelJoinRequest", "api.channel.discoverable_join_request.invalid_patch.app_error", nil, "status="+patch.Status, http.StatusBadRequest) + } + + current, err := a.Srv().Store().ChannelJoinRequest().Get(requestID) + if err != nil { + var nfErr *store.ErrNotFound + if errors.As(err, &nfErr) { + return nil, model.NewAppError("UpdateChannelJoinRequest", "app.channel.join_request.not_found.app_error", nil, "request_id="+requestID, http.StatusNotFound) + } + return nil, model.NewAppError("UpdateChannelJoinRequest", "app.channel.join_request.get.app_error", nil, "", http.StatusInternalServerError).Wrap(err) + } + + // Defense in depth: refuse cross-channel updates so a forged request id + // can't be reviewed against a channel the admin happens to own. + if current.ChannelId != channelID { + return nil, model.NewAppError("UpdateChannelJoinRequest", "app.channel.join_request.not_found.app_error", nil, "request_id="+requestID, http.StatusNotFound) + } + + if current.Status != model.ChannelJoinRequestStatusPending { + return nil, model.NewAppError("UpdateChannelJoinRequest", "api.channel.discoverable_join_request.not_pending.app_error", nil, "request_id="+requestID, http.StatusConflict) + } + + channel, appErr := a.GetChannel(rctx, current.ChannelId) + if appErr != nil { + return nil, appErr + } + + if patch.Status == model.ChannelJoinRequestStatusApproved { + if _, err := a.AddChannelMember(rctx, current.UserId, channel, ChannelMemberOpts{UserRequestorID: reviewerID}); err != nil { + return nil, err + } + } + + current.Status = patch.Status + current.ReviewedBy = reviewerID + current.ReviewedAt = model.GetMillis() + current.DenialReason = "" + if patch.Status == model.ChannelJoinRequestStatusDenied && patch.DenialReason != nil { + current.DenialReason = *patch.DenialReason + } + // Drop the original message from the response; it served its purpose + // during review and keeping it would leak free-text into the audit trail. + current.Message = "" + + updated, err := a.Srv().Store().ChannelJoinRequest().Update(current) + if err != nil { + if appErr, ok := err.(*model.AppError); ok { + return nil, appErr + } + return nil, model.NewAppError("UpdateChannelJoinRequest", "app.channel.join_request.update.app_error", nil, "", http.StatusInternalServerError).Wrap(err) + } + + a.broadcastChannelJoinRequestUpdated(rctx, channel, updated) + return updated, nil +} + +// sanitizeJoinRequestListOpts clamps user-provided pagination + status options +// so the store sees a normalized request. +func sanitizeJoinRequestListOpts(opts model.GetChannelJoinRequestsOpts) model.GetChannelJoinRequestsOpts { + if opts.Status == "" { + opts.Status = model.ChannelJoinRequestStatusPending + } else if !model.IsValidChannelJoinRequestStatus(opts.Status) { + opts.Status = model.ChannelJoinRequestStatusPending + } + if opts.Page < 0 { + opts.Page = 0 + } + if opts.PerPage <= 0 { + opts.PerPage = channelJoinRequestPaginationDefaultPerPage + } else if opts.PerPage > channelJoinRequestPaginationMaxPerPage { + opts.PerPage = channelJoinRequestPaginationMaxPerPage + } + return opts +} + +// evaluateChannelMembership runs the access-control PDP for `user` against the +// `membership` action on `channel`, returning the boolean decision. Errors +// from the PDP are returned to callers so they can choose between the +// "channel is invisible" (visibility filter) or "channel cannot be joined" +// (request flow) fail-secure semantics. Callers must have already verified +// that `channel.PolicyEnforced` is true before invoking the PDP. +func (a *App) evaluateChannelMembership(rctx request.CTX, user *model.User, channel *model.Channel) (bool, *model.AppError) { + acs := a.Srv().Channels().AccessControl + if acs == nil { + // No ABAC service → fail-secure. The channel acts as if the user did + // not satisfy the policy. + return false, nil + } + + subject, appErr := a.BuildAccessControlSubject(rctx, user.Id, user.Roles) + if appErr != nil { + return false, appErr + } + + decision, evalErr := acs.AccessEvaluation(rctx, model.AccessRequest{ + Subject: *subject, + Resource: model.Resource{ + Type: model.AccessControlPolicyTypeChannel, + ID: channel.Id, + }, + Action: "membership", + }) + if evalErr != nil { + return false, evalErr + } + return decision.Decision, nil +} + +// channelAdminUserIDs returns the user ids of channel members with the +// scheme-admin role on `channelID`. Used to scope WS broadcasts of join-request +// events to the queue audience. Failures bubble up because broadcasting to no +// one would silently break the admin UI. +func (a *App) channelAdminUserIDs(rctx request.CTX, channelID string) ([]string, *model.AppError) { + const channelMembersPageSize = 200 + + admins := []string{} + page := 0 + for { + members, err := a.Srv().Store().Channel().GetMembers(model.ChannelMembersGetOptions{ + ChannelID: channelID, + Offset: page * channelMembersPageSize, + Limit: channelMembersPageSize, + }) + if err != nil { + return nil, model.NewAppError("channelAdminUserIDs", "app.channel.get_members.app_error", nil, "", http.StatusInternalServerError).Wrap(err) + } + for _, m := range members { + if m.SchemeAdmin { + admins = append(admins, m.UserId) + } + } + if len(members) < channelMembersPageSize { + break + } + page++ + } + return admins, nil +} + +// broadcastChannelJoinRequestCreated fires a channel_join_request_created event +// scoped to the channel admin set, using the OnlyChannelAdmins broadcast hook +// to filter out non-admin members the channel-id broadcast would otherwise +// reach. +func (a *App) broadcastChannelJoinRequestCreated(rctx request.CTX, channel *model.Channel, req *model.ChannelJoinRequest) { + a.publishChannelJoinRequestEvent(rctx, channel, req, model.WebsocketEventChannelJoinRequestCreated, true /* adminsOnly */) +} + +// broadcastChannelJoinRequestUpdated fires a channel_join_request_updated event +// to the channel admin set + the requesting user (so their My Pending Requests +// list reacts in real-time). +func (a *App) broadcastChannelJoinRequestUpdated(rctx request.CTX, channel *model.Channel, req *model.ChannelJoinRequest) { + // Send a dedicated copy to the requester so an offline-but-then-reconnected + // requester gets their own row update even when they are not a channel + // member yet (the channel-id broadcast wouldn't reach them otherwise). + if req.UserId != "" { + userMessage := model.NewWebSocketEvent(model.WebsocketEventChannelJoinRequestUpdated, "", "", req.UserId, nil, "") + userMessage.Add("request", marshalChannelJoinRequest(rctx, req)) + userMessage.Add("channel_id", channel.Id) + a.Publish(userMessage) + } + a.publishChannelJoinRequestEvent(rctx, channel, req, model.WebsocketEventChannelJoinRequestUpdated, true /* adminsOnly */) +} + +func (a *App) publishChannelJoinRequestEvent(rctx request.CTX, channel *model.Channel, req *model.ChannelJoinRequest, event model.WebsocketEventType, adminsOnly bool) { + message := model.NewWebSocketEvent(event, "", channel.Id, "", nil, "") + message.Add("request", marshalChannelJoinRequest(rctx, req)) + message.Add("channel_id", channel.Id) + + if adminsOnly { + admins, appErr := a.channelAdminUserIDs(rctx, channel.Id) + if appErr != nil { + rctx.Logger().Warn("Failed to compute channel admin set for join request broadcast", + mlog.String("channel_id", channel.Id), + mlog.Err(appErr), + ) + return + } + useOnlyChannelAdminsHook(message, admins) + } + a.Publish(message) +} + +// marshalChannelJoinRequest returns the request as a JSON string for the WS +// payload. JSON encoding errors are logged and the payload is delivered as an +// empty string so the event still arrives (clients can tolerate a missing +// request body and refetch). +func marshalChannelJoinRequest(rctx request.CTX, req *model.ChannelJoinRequest) string { + if req == nil { + return "" + } + buf, err := json.Marshal(req) + if err != nil { + rctx.Logger().Warn("Failed to marshal ChannelJoinRequest for WS broadcast", + mlog.String("request_id", req.Id), + mlog.Err(err), + ) + return "" + } + return string(buf) +} diff --git a/server/channels/app/channel_join_request_test.go b/server/channels/app/channel_join_request_test.go new file mode 100644 index 00000000000..6cda6adb46e --- /dev/null +++ b/server/channels/app/channel_join_request_test.go @@ -0,0 +1,379 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package app + +import ( + "net/http" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/mattermost/mattermost/server/public/model" +) + +// withDiscoverableChannelsFlag toggles the FeatureFlag for the duration of a +// test and restores it on cleanup. Feature flags are read-only by default in +// the test config store; flipping SetReadOnlyFF lets the UpdateConfig call +// land. We deliberately do NOT restore SetReadOnlyFF(true) afterward — the +// underlying store is per-test and disposed on cleanup. +func withDiscoverableChannelsFlag(t *testing.T, th *TestHelper, on bool) { + t.Helper() + th.ConfigStore.SetReadOnlyFF(false) + previous := th.App.Config().FeatureFlags.DiscoverableChannels + th.App.UpdateConfig(func(cfg *model.Config) { cfg.FeatureFlags.DiscoverableChannels = on }) + t.Cleanup(func() { + th.App.UpdateConfig(func(cfg *model.Config) { cfg.FeatureFlags.DiscoverableChannels = previous }) + }) +} + +// markDiscoverable flips the channel's discoverable flag in the store via +// PatchChannel so the model invariants run alongside the test scenario. +func markDiscoverable(t *testing.T, th *TestHelper, channel *model.Channel) *model.Channel { + t.Helper() + on := true + patched, err := th.App.PatchChannel(th.Context, channel, &model.ChannelPatch{Discoverable: &on}, th.BasicUser.Id) + require.Nil(t, err) + require.True(t, patched.Discoverable) + return patched +} + +func TestRequestJoinChannel_RejectsNonDiscoverable(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + withDiscoverableChannelsFlag(t, th, true) + + channel := th.CreatePrivateChannel(t, th.BasicTeam) + + other := th.CreateUser(t) + th.LinkUserToTeam(t, other, th.BasicTeam) + + joined, req, appErr := th.App.RequestJoinChannel(th.Context, other.Id, channel.Id, "please") + require.NotNil(t, appErr) + assert.Equal(t, http.StatusForbidden, appErr.StatusCode) + assert.Equal(t, "api.channel.discoverable_join_request.not_discoverable.app_error", appErr.Id) + assert.False(t, joined) + assert.Nil(t, req) +} + +func TestRequestJoinChannel_RejectsExistingMember(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + withDiscoverableChannelsFlag(t, th, true) + + channel := th.CreatePrivateChannel(t, th.BasicTeam) + channel = markDiscoverable(t, th, channel) + + // BasicUser is the channel creator → already a member. + _, _, appErr := th.App.RequestJoinChannel(th.Context, th.BasicUser.Id, channel.Id, "") + require.NotNil(t, appErr) + assert.Equal(t, http.StatusBadRequest, appErr.StatusCode) + assert.Equal(t, "api.channel.discoverable_join_request.already_member.app_error", appErr.Id) +} + +func TestRequestJoinChannel_PendingHappyPath(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + withDiscoverableChannelsFlag(t, th, true) + + channel := th.CreatePrivateChannel(t, th.BasicTeam) + channel = markDiscoverable(t, th, channel) + + other := th.CreateUser(t) + th.LinkUserToTeam(t, other, th.BasicTeam) + + joined, req, appErr := th.App.RequestJoinChannel(th.Context, other.Id, channel.Id, "let me in") + require.Nil(t, appErr) + assert.False(t, joined, "should not auto-join when no policy is enforced") + require.NotNil(t, req) + assert.Equal(t, model.ChannelJoinRequestStatusPending, req.Status) + assert.Equal(t, channel.Id, req.ChannelId) + assert.Equal(t, other.Id, req.UserId) + assert.Equal(t, "let me in", req.Message) + + // Submitting again returns the existing pending row (idempotent on + // partial-unique conflict). + joined, req2, appErr := th.App.RequestJoinChannel(th.Context, other.Id, channel.Id, "again") + require.Nil(t, appErr) + assert.False(t, joined) + require.NotNil(t, req2) + assert.Equal(t, req.Id, req2.Id) +} + +func TestRequestJoinChannel_RejectsGuest(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + withDiscoverableChannelsFlag(t, th, true) + + channel := th.CreatePrivateChannel(t, th.BasicTeam) + channel = markDiscoverable(t, th, channel) + + guest := th.CreateGuest(t) + th.LinkUserToTeam(t, guest, th.BasicTeam) + + _, _, appErr := th.App.RequestJoinChannel(th.Context, guest.Id, channel.Id, "") + require.NotNil(t, appErr) + assert.Equal(t, http.StatusForbidden, appErr.StatusCode) + assert.Equal(t, "api.channel.discoverable_join_request.guest.app_error", appErr.Id) +} + +func TestUpdateChannelJoinRequest_ApproveAddsMember(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + withDiscoverableChannelsFlag(t, th, true) + + channel := th.CreatePrivateChannel(t, th.BasicTeam) + channel = markDiscoverable(t, th, channel) + + other := th.CreateUser(t) + th.LinkUserToTeam(t, other, th.BasicTeam) + + _, req, appErr := th.App.RequestJoinChannel(th.Context, other.Id, channel.Id, "") + require.Nil(t, appErr) + require.NotNil(t, req) + + patch := &model.ChannelJoinRequestPatch{Status: model.ChannelJoinRequestStatusApproved} + updated, appErr := th.App.UpdateChannelJoinRequest(th.Context, req.Id, channel.Id, patch, th.BasicUser.Id) + require.Nil(t, appErr) + assert.Equal(t, model.ChannelJoinRequestStatusApproved, updated.Status) + assert.Equal(t, th.BasicUser.Id, updated.ReviewedBy) + assert.NotZero(t, updated.ReviewedAt) + assert.Empty(t, updated.Message, "message should be redacted from the response after review") + + member, mErr := th.App.GetChannelMember(th.Context, channel.Id, other.Id) + require.Nil(t, mErr) + require.NotNil(t, member) + assert.Equal(t, other.Id, member.UserId) +} + +func TestUpdateChannelJoinRequest_DenyKeepsReason(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + withDiscoverableChannelsFlag(t, th, true) + + channel := th.CreatePrivateChannel(t, th.BasicTeam) + channel = markDiscoverable(t, th, channel) + + other := th.CreateUser(t) + th.LinkUserToTeam(t, other, th.BasicTeam) + _, req, appErr := th.App.RequestJoinChannel(th.Context, other.Id, channel.Id, "please") + require.Nil(t, appErr) + require.NotNil(t, req) + + reason := "team-internal channel" + patch := &model.ChannelJoinRequestPatch{ + Status: model.ChannelJoinRequestStatusDenied, + DenialReason: &reason, + } + updated, appErr := th.App.UpdateChannelJoinRequest(th.Context, req.Id, channel.Id, patch, th.BasicUser.Id) + require.Nil(t, appErr) + assert.Equal(t, model.ChannelJoinRequestStatusDenied, updated.Status) + assert.Equal(t, reason, updated.DenialReason) + + // Member must NOT have been added. + _, mErr := th.App.GetChannelMember(th.Context, channel.Id, other.Id) + require.NotNil(t, mErr) + assert.Equal(t, MissingChannelMemberError, mErr.Id) +} + +func TestUpdateChannelJoinRequest_RejectsCrossChannel(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + withDiscoverableChannelsFlag(t, th, true) + + channelA := markDiscoverable(t, th, th.CreatePrivateChannel(t, th.BasicTeam)) + channelB := markDiscoverable(t, th, th.CreatePrivateChannel(t, th.BasicTeam)) + + other := th.CreateUser(t) + th.LinkUserToTeam(t, other, th.BasicTeam) + _, req, appErr := th.App.RequestJoinChannel(th.Context, other.Id, channelA.Id, "") + require.Nil(t, appErr) + require.NotNil(t, req) + + patch := &model.ChannelJoinRequestPatch{Status: model.ChannelJoinRequestStatusApproved} + _, appErr = th.App.UpdateChannelJoinRequest(th.Context, req.Id, channelB.Id, patch, th.BasicUser.Id) + require.NotNil(t, appErr) + assert.Equal(t, http.StatusNotFound, appErr.StatusCode) +} + +func TestWithdrawChannelJoinRequest_OwnerOnly(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + withDiscoverableChannelsFlag(t, th, true) + + channel := markDiscoverable(t, th, th.CreatePrivateChannel(t, th.BasicTeam)) + + other := th.CreateUser(t) + th.LinkUserToTeam(t, other, th.BasicTeam) + _, req, appErr := th.App.RequestJoinChannel(th.Context, other.Id, channel.Id, "") + require.Nil(t, appErr) + require.NotNil(t, req) + + stranger := th.CreateUser(t) + _, appErr = th.App.WithdrawChannelJoinRequest(th.Context, req.Id, stranger.Id) + require.NotNil(t, appErr) + assert.Equal(t, http.StatusNotFound, appErr.StatusCode) + + updated, appErr := th.App.WithdrawChannelJoinRequest(th.Context, req.Id, other.Id) + require.Nil(t, appErr) + assert.Equal(t, model.ChannelJoinRequestStatusWithdrawn, updated.Status) + + // A second withdrawal is rejected with 409. + _, appErr = th.App.WithdrawChannelJoinRequest(th.Context, req.Id, other.Id) + require.NotNil(t, appErr) + assert.Equal(t, http.StatusConflict, appErr.StatusCode) +} + +func TestGetMyChannelJoinRequests(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + withDiscoverableChannelsFlag(t, th, true) + + channelA := markDiscoverable(t, th, th.CreatePrivateChannel(t, th.BasicTeam)) + channelB := markDiscoverable(t, th, th.CreatePrivateChannel(t, th.BasicTeam)) + + other := th.CreateUser(t) + th.LinkUserToTeam(t, other, th.BasicTeam) + _, _, appErr := th.App.RequestJoinChannel(th.Context, other.Id, channelA.Id, "") + require.Nil(t, appErr) + _, _, appErr = th.App.RequestJoinChannel(th.Context, other.Id, channelB.Id, "") + require.Nil(t, appErr) + + list, appErr := th.App.GetMyChannelJoinRequests(th.Context, other.Id, model.GetChannelJoinRequestsOpts{}) + require.Nil(t, appErr) + require.NotNil(t, list) + assert.EqualValues(t, 2, list.TotalCount) + assert.Len(t, list.Requests, 2) +} + +func TestCountPendingChannelJoinRequests(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + withDiscoverableChannelsFlag(t, th, true) + + channel := markDiscoverable(t, th, th.CreatePrivateChannel(t, th.BasicTeam)) + + other := th.CreateUser(t) + th.LinkUserToTeam(t, other, th.BasicTeam) + _, _, appErr := th.App.RequestJoinChannel(th.Context, other.Id, channel.Id, "") + require.Nil(t, appErr) + + count, appErr := th.App.CountPendingChannelJoinRequests(th.Context, channel.Id) + require.Nil(t, appErr) + assert.EqualValues(t, 1, count) +} + +func TestUpdateChannelPrivacy_CancelsPendingRequestsOnConvertToPublic(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + withDiscoverableChannelsFlag(t, th, true) + + channel := markDiscoverable(t, th, th.CreatePrivateChannel(t, th.BasicTeam)) + + other := th.CreateUser(t) + th.LinkUserToTeam(t, other, th.BasicTeam) + _, req, appErr := th.App.RequestJoinChannel(th.Context, other.Id, channel.Id, "") + require.Nil(t, appErr) + require.NotNil(t, req) + + channel.Type = model.ChannelTypeOpen + converted, appErr := th.App.UpdateChannelPrivacy(th.Context, channel, th.BasicUser) + require.Nil(t, appErr) + + // Discoverable must be reset on convert-to-public — the model invariant + // (Channel.IsValid) rejects (type=O, discoverable=true), so leaving it + // true would also break the next channel save. + assert.False(t, converted.Discoverable, "Discoverable must be reset to false after convert-to-public") + persisted, getErr := th.App.GetChannel(th.Context, channel.Id) + require.Nil(t, getErr) + assert.False(t, persisted.Discoverable, "Discoverable must be persisted as false after convert-to-public") + + // The cancellation side-effect is dispatched on a goroutine; poll for + // the withdrawn state instead of sleeping. + require.Eventually(t, func() bool { + row, err := th.App.Srv().Store().ChannelJoinRequest().Get(req.Id) + if err != nil { + return false + } + return row.Status == model.ChannelJoinRequestStatusWithdrawn + }, 2*time.Second, 50*time.Millisecond) +} + +func TestIsDiscoverableSelfAddBlocked(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + withDiscoverableChannelsFlag(t, th, true) + + channel := markDiscoverable(t, th, th.CreatePrivateChannel(t, th.BasicTeam)) + + other := th.CreateUser(t) + assert.True(t, th.App.IsDiscoverableSelfAddBlocked(th.Context, channel, other.Id, other.Id), "self-add to discoverable + no-policy private must be blocked") + assert.False(t, th.App.IsDiscoverableSelfAddBlocked(th.Context, channel, th.BasicUser.Id, other.Id), "admin invite must not be blocked") + + // Toggle off the flag → guard is inert. + withDiscoverableChannelsFlag(t, th, false) + assert.False(t, th.App.IsDiscoverableSelfAddBlocked(th.Context, channel, other.Id, other.Id)) +} + +func TestFilterDiscoverableChannelsByPolicy_FlagOff(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + // Flag off → filter is a no-op even when channels look discoverable. + + channel := markDiscoverableInMemory(t, th.CreatePrivateChannel(t, th.BasicTeam)) + channel.PolicyEnforced = true + out, appErr := th.App.FilterDiscoverableChannelsByPolicy(th.Context, []*model.Channel{channel}, th.BasicUser2.Id) + require.Nil(t, appErr) + require.Len(t, out, 1) +} + +func TestFilterDiscoverableChannelsByPolicy_NoPolicyPasses(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + withDiscoverableChannelsFlag(t, th, true) + + channel := markDiscoverableInMemory(t, th.CreatePrivateChannel(t, th.BasicTeam)) + out, appErr := th.App.FilterDiscoverableChannelsByPolicy(th.Context, []*model.Channel{channel}, th.BasicUser2.Id) + require.Nil(t, appErr) + require.Len(t, out, 1, "no-policy discoverable channels are visible without ABAC evaluation") +} + +func TestFilterDiscoverableChannelsByPolicy_PolicyEnforcedFailSecure(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + withDiscoverableChannelsFlag(t, th, true) + + // PolicyEnforced + Discoverable + no AccessControl service wired ⇒ hidden. + channel := markDiscoverableInMemory(t, th.CreatePrivateChannel(t, th.BasicTeam)) + channel.PolicyEnforced = true + + require.Nil(t, th.App.Srv().Channels().AccessControl, "test fixture must not have ABAC wired") + + out, appErr := th.App.FilterDiscoverableChannelsByPolicy(th.Context, []*model.Channel{channel}, th.BasicUser2.Id) + require.Nil(t, appErr) + assert.Len(t, out, 0, "fail-secure must hide policy-enforced channels when ABAC is unavailable") +} + +func TestFilterDiscoverableChannelsByPolicy_GuestHidden(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + withDiscoverableChannelsFlag(t, th, true) + + channel := markDiscoverableInMemory(t, th.CreatePrivateChannel(t, th.BasicTeam)) + channel.PolicyEnforced = true + + guest := th.CreateGuest(t) + out, appErr := th.App.FilterDiscoverableChannelsByPolicy(th.Context, []*model.Channel{channel}, guest.Id) + require.Nil(t, appErr) + assert.Empty(t, out, "guests must never see discoverable + policy-enforced channels") +} + +// markDiscoverableInMemory is a no-DB helper for visibility filter tests that +// don't care about persistence — they only exercise the in-memory list filter. +func markDiscoverableInMemory(t *testing.T, channel *model.Channel) *model.Channel { + t.Helper() + channel.Discoverable = true + return channel +} diff --git a/server/channels/app/web_broadcast_hooks.go b/server/channels/app/web_broadcast_hooks.go index 3db2fa7fcbd..f8ba6713a48 100644 --- a/server/channels/app/web_broadcast_hooks.go +++ b/server/channels/app/web_broadcast_hooks.go @@ -25,6 +25,7 @@ const ( broadcastBurnOnRead = "burn_on_read" broadcastBurnOnReadReaction = "burn_on_read_reaction" broadcastAbacFiles = "abac_files" + broadcastOnlyChannelAdmins = "only_channel_admins" ) func (s *Server) makeBroadcastHooks() map[string]platform.BroadcastHook { @@ -37,6 +38,7 @@ func (s *Server) makeBroadcastHooks() map[string]platform.BroadcastHook { broadcastBurnOnRead: &burnOnReadBroadcastHook{}, broadcastBurnOnReadReaction: &burnOnReadReactionBroadcastHook{}, broadcastAbacFiles: &abacFilesBroadcastHook{}, + broadcastOnlyChannelAdmins: &onlyChannelAdminsBroadcastHook{}, } } @@ -505,6 +507,34 @@ func (h *abacFilesBroadcastHook) stripFilesFromMessage(msg *platform.HookedWebSo return nil } +// onlyChannelAdminsBroadcastHook narrows a channel-scoped broadcast to the +// channel-admin subset of the channel's members. The hook arg +// `channel_admin_user_ids` is the precomputed list of admin user ids at publish +// time; recipients not in that set have the event rejected. +// +// Pair with `Broadcast{ChannelId: channelId}` so the platform's existing +// channel-member fan-out is the outer bound and this hook simply filters +// non-admin members out. +type onlyChannelAdminsBroadcastHook struct{} + +func useOnlyChannelAdminsHook(message *model.WebSocketEvent, channelAdminUserIds []string) { + message.GetBroadcast().AddHook(broadcastOnlyChannelAdmins, map[string]any{ + "channel_admin_user_ids": model.StringArray(channelAdminUserIds), + }) +} + +func (h *onlyChannelAdminsBroadcastHook) Process(msg *platform.HookedWebSocketEvent, webConn *platform.WebConn, args map[string]any) error { + adminUserIDs, err := getTypedArg[model.StringArray](args, "channel_admin_user_ids") + if err != nil { + return errors.Wrap(err, "Invalid channel_admin_user_ids value passed to onlyChannelAdminsBroadcastHook") + } + + if !slices.Contains(adminUserIDs, webConn.UserId) { + msg.Event().Reject() + } + return nil +} + func incrementWebsocketCounter(wc *platform.WebConn) { if wc.Platform.Metrics() == nil { return diff --git a/server/channels/store/sqlstore/channel_store.go b/server/channels/store/sqlstore/channel_store.go index d98f215dacb..9b4cbf7805c 100644 --- a/server/channels/store/sqlstore/channel_store.go +++ b/server/channels/store/sqlstore/channel_store.go @@ -3260,6 +3260,9 @@ func (s SqlChannelStore) Autocomplete(rctx request.CTX, userID, term string, inc From("ChannelMembers"). Where(sq.Eq{"UserId": userID}))) } else { + // Non-guests see public channels, private channels they're a member of, and + // discoverable private channels (subject to a post-query ABAC visibility filter + // applied at the app layer for policy-enforced channels). query = query.Where(sq.Or{ sq.NotEq{"c.Type": model.ChannelTypePrivate}, sq.And{ @@ -3268,6 +3271,10 @@ func (s SqlChannelStore) Autocomplete(rctx request.CTX, userID, term string, inc From("ChannelMembers"). Where(sq.Eq{"UserId": userID})), }, + sq.And{ + sq.Eq{"c.Type": model.ChannelTypePrivate}, + sq.Eq{"c.Discoverable": true}, + }, }) } @@ -3311,12 +3318,19 @@ func (s SqlChannelStore) buildAutocompleteInTeamQuery(teamID, userID, term strin if isGuest { query = query.Where(sq.Expr("c.Id IN (?)", memberSubQuery)) } else { + // Non-guests see public channels, private channels they're a member of, and + // discoverable private channels (subject to a post-query ABAC visibility filter + // applied at the app layer for policy-enforced channels). query = query.Where(sq.Or{ sq.NotEq{"c.Type": model.ChannelTypePrivate}, sq.And{ sq.Eq{"c.Type": model.ChannelTypePrivate}, sq.Expr("c.Id IN (?)", memberSubQuery), }, + sq.And{ + sq.Eq{"c.Type": model.ChannelTypePrivate}, + sq.Eq{"c.Discoverable": true}, + }, }) } diff --git a/server/channels/web/params.go b/server/channels/web/params.go index b57cbf6a2a3..1bd741bde3d 100644 --- a/server/channels/web/params.go +++ b/server/channels/web/params.go @@ -129,6 +129,9 @@ type Params struct { GroupName string ObjectType string TargetId string + + // Channel join requests + RequestId string } var getChannelMembersForUserRegex = regexp.MustCompile("/api/v4/users/[A-Za-z0-9]{26}/channel_members") @@ -205,6 +208,7 @@ func ParamsFromRequest(r *http.Request) *Params { params.GroupName = props["group_name"] params.ObjectType = props["object_type"] params.TargetId = props["target_id"] + params.RequestId = props["request_id"] params.Scope = query.Get("scope") if val, err := strconv.Atoi(query.Get("page")); err != nil || (val < 0 && params.UserId == "" && !getChannelMembersForUserRegex.MatchString(r.URL.Path)) { diff --git a/server/i18n/en.json b/server/i18n/en.json index 043fa6f30a2..1086ec088a5 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -427,6 +427,54 @@ "id": "api.channel.delete_channel.type.invalid", "translation": "Unable to delete direct or group message channels" }, + { + "id": "api.channel.discoverable_join_request.already_member.app_error", + "translation": "You are already a member of this channel." + }, + { + "id": "api.channel.discoverable_join_request.archived.app_error", + "translation": "Cannot request to join an archived channel." + }, + { + "id": "api.channel.discoverable_join_request.discoverable_requires_approval.app_error", + "translation": "This channel requires admin approval to join. Please send a request from the Browse Channels modal." + }, + { + "id": "api.channel.discoverable_join_request.duplicate.app_error", + "translation": "You already have a pending request to join this channel." + }, + { + "id": "api.channel.discoverable_join_request.feature_disabled.app_error", + "translation": "Discoverable channels are not enabled on this server." + }, + { + "id": "api.channel.discoverable_join_request.guest.app_error", + "translation": "Guests cannot request to join discoverable private channels." + }, + { + "id": "api.channel.discoverable_join_request.invalid_patch.app_error", + "translation": "Invalid update for the channel join request." + }, + { + "id": "api.channel.discoverable_join_request.not_discoverable.app_error", + "translation": "This channel is not discoverable." + }, + { + "id": "api.channel.discoverable_join_request.not_pending.app_error", + "translation": "The join request is no longer pending." + }, + { + "id": "api.channel.discoverable_join_request.not_private.app_error", + "translation": "Only private channels accept join requests." + }, + { + "id": "api.channel.discoverable_join_request.policy_denied.app_error", + "translation": "You do not satisfy the access rules required to join this channel." + }, + { + "id": "api.channel.discoverable_join_request.shared.app_error", + "translation": "Shared channels do not accept discoverable join requests." + }, { "id": "api.channel.get_channel.flagged_post_mismatch.app_error", "translation": "Channel ID does not match the channel ID of the flagged post." @@ -5634,6 +5682,22 @@ "id": "app.channel.group_message_conversion.post_message.error", "translation": "Failed to create group message to channel conversion post" }, + { + "id": "app.channel.join_request.get.app_error", + "translation": "Failed to load channel join request." + }, + { + "id": "app.channel.join_request.not_found.app_error", + "translation": "Channel join request not found." + }, + { + "id": "app.channel.join_request.save.app_error", + "translation": "Failed to save channel join request." + }, + { + "id": "app.channel.join_request.update.app_error", + "translation": "Failed to update channel join request." + }, { "id": "app.channel.migrate_channel_members.select.app_error", "translation": "Failed to select the batch of channel members." diff --git a/server/public/model/audit_events.go b/server/public/model/audit_events.go index a82ec6b7d83..489a4e1a3dd 100644 --- a/server/public/model/audit_events.go +++ b/server/public/model/audit_events.go @@ -84,6 +84,9 @@ const ( AuditEventAddChannelMember = "addChannelMember" // add member to channel AuditEventConvertGroupMessageToChannel = "convertGroupMessageToChannel" // convert group message to private channel AuditEventCreateChannel = "createChannel" // create public or private channel + AuditEventCreateChannelJoinRequest = "createChannelJoinRequest" // request to join a discoverable private channel + AuditEventUpdateChannelJoinRequest = "updateChannelJoinRequest" // approve or deny a channel join request + AuditEventWithdrawChannelJoinRequest = "withdrawChannelJoinRequest" // requester cancels their channel join request AuditEventCreateDirectChannel = "createDirectChannel" // create direct message channel between two users AuditEventCreateGroupChannel = "createGroupChannel" // create group message channel with multiple users AuditEventDeleteChannel = "deleteChannel" // delete channel