diff --git a/server/channels/api4/channel_write_access_test.go b/server/channels/api4/channel_write_access_test.go index dd4ae4c210c..8f071b04f8a 100644 --- a/server/channels/api4/channel_write_access_test.go +++ b/server/channels/api4/channel_write_access_test.go @@ -395,6 +395,56 @@ func TestChannelAccessDoesNotGateReviewerAssignment(t *testing.T) { require.Equal(t, http.StatusOK, resp.StatusCode) } +// An incoming webhook posts into its channel with no session to evaluate later, so +// setting one up there -- creating it, or moving an existing hook in -- is the write. +func TestChannelWriteAccessGatesIncomingWebhooks(t *testing.T) { + setup := func(t *testing.T, allow bool) *channelWriteAccessFixture { + t.Helper() + f, _ := setupChannelWriteAccessAPI(t, allow) + f.th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.EnableIncomingWebhooks = true + }) + defaultRolePermissions := f.th.SaveDefaultRolePermissions(t) + t.Cleanup(func() { f.th.RestoreDefaultRolePermissions(t, defaultRolePermissions) }) + addIncomingWebhookPermissions(t, f.th, model.TeamUserRoleId) + return f + } + + create := func(f *channelWriteAccessFixture) error { + _, _, err := f.th.Client.CreateIncomingWebhook(context.Background(), &model.IncomingWebhook{ChannelId: f.th.BasicChannel.Id}) + return err + } + + // The hook starts in BasicChannel2, set up through the app so the gate under test + // is only the move into BasicChannel. + move := func(t *testing.T, f *channelWriteAccessFixture) error { + hook, appErr := f.th.App.CreateIncomingWebhookForChannel(f.th.BasicUser.Id, f.th.BasicChannel2, &model.IncomingWebhook{ + ChannelId: f.th.BasicChannel2.Id, + }) + require.Nil(t, appErr) + hook.ChannelId = f.th.BasicChannel.Id + _, _, err := f.th.Client.UpdateIncomingWebhook(context.Background(), hook) + return err + } + + t.Run("denied", func(t *testing.T) { + f := setup(t, false) + + err := create(f) + require.True(t, isChannelReadAccessDenial(err, abacWriteDeniedErrorID), "creating a hook: expected the write denial, got %v", err) + + err = move(t, f) + require.True(t, isChannelReadAccessDenial(err, abacWriteDeniedErrorID), "moving a hook in: expected the write denial, got %v", err) + }) + + t.Run("allowed", func(t *testing.T) { + f := setup(t, true) + + require.NoError(t, create(f)) + require.NoError(t, move(t, f)) + }) +} + // The gate is inert until a channel_write_access policy governs the channel: a // deployment that only restricts reading must not start refusing posts. This is // the reason TestChannelReadAccessDoesNotGateWrites is still true. diff --git a/server/channels/api4/webhook.go b/server/channels/api4/webhook.go index 0caa107212d..c6093a76c4d 100644 --- a/server/channels/api4/webhook.go +++ b/server/channels/api4/webhook.go @@ -56,6 +56,13 @@ func createIncomingHook(c *Context, w http.ResponseWriter, r *http.Request) { return } + // The hook posts into this channel without a session to evaluate later, so a user + // the channel's write rules deny must not be able to set one up. + if !requireChannelWriteAccess(c, channel) { + c.LogAudit("fail - channel write access denied") + return + } + userId := c.AppContext.Session().UserId if hook.UserId != "" && hook.UserId != userId { if !c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), channel.TeamId, model.PermissionManageOthersIncomingWebhooks) { @@ -176,6 +183,10 @@ func updateIncomingHook(c *Context, w http.ResponseWriter, r *http.Request) { // Moving the hook must not attribute its owner's posts to a channel they cannot access. if updatedHook.ChannelId != oldHook.ChannelId { + if !requireChannelWriteAccess(c, channel) { + c.LogAudit("fail - channel write access denied") + return + } if appErr := c.App.ValidateIncomingWebhookUserChannelAccess(c.AppContext, oldHook.UserId, channel); appErr != nil { c.LogAudit("fail - invalid webhook user") c.Err = appErr