From 39c85ffb77af32881348713f90b97c200d4b792f Mon Sep 17 00:00:00 2001 From: "Pablo Fernandez Guerra (PFE)" <148432447+pfe-nazaries@users.noreply.github.com> Date: Wed, 26 Aug 2026 08:52:56 +0200 Subject: [PATCH] feat(ui): authorize multiple Slack destination channels (#12491) --- ui/__tests__/msw/handlers/slack.fixtures.ts | 115 +++-- ui/__tests__/msw/handlers/slack.test.ts | 121 ++++++ ui/__tests__/msw/handlers/slack.ts | 200 ++++++--- ui/actions/integrations/integrations.ts | 11 +- ui/actions/integrations/slack.test.ts | 67 ++- ui/actions/integrations/slack.ts | 44 +- ui/app/(prowler)/integrations/slack/page.tsx | 3 +- .../slack/slack-integration.harness.ts | 281 +++++++++---- .../slack/slack-page.integration.test.tsx | 394 ++++++++++++++---- .../slack-authorized-channels.added.md | 1 + .../integrations/slack/slack-callback.tsx | 4 +- .../slack/slack-channel-multi-select.tsx | 162 +++++++ .../slack/slack-channel-selector.tsx | 195 --------- .../slack/slack-integration-card.tsx | 4 +- .../slack/slack-integration-manager.test.tsx | 2 +- .../slack/slack-integration-manager.tsx | 313 +++++++++----- ui/types/integrations.ts | 26 +- 17 files changed, 1371 insertions(+), 572 deletions(-) create mode 100644 ui/__tests__/msw/handlers/slack.test.ts create mode 100644 ui/changelog.d/slack-authorized-channels.added.md create mode 100644 ui/components/integrations/slack/slack-channel-multi-select.tsx delete mode 100644 ui/components/integrations/slack/slack-channel-selector.tsx diff --git a/ui/__tests__/msw/handlers/slack.fixtures.ts b/ui/__tests__/msw/handlers/slack.fixtures.ts index e29f3e4b21..5ad60df276 100644 --- a/ui/__tests__/msw/handlers/slack.fixtures.ts +++ b/ui/__tests__/msw/handlers/slack.fixtures.ts @@ -1,26 +1,36 @@ /** - * Fixture data for the Slack handlers. Shapes follow the API contract in - * `openspec/changes/add-slack-integration/design.md`. + * Fixture data for the Slack handlers. Shapes follow the signed API contract in + * `openspec/changes/add-slack-alert-channels/contract/slack-alerts-api.md`. */ export interface SlackWorkspaceFixture { teamId: string; teamName: string; botUserId: string; - /** - * Absent from the serialized configuration until a channel is chosen: the API - * omits the keys rather than sending nulls. - */ - channelId?: string; - channelName?: string; + /** An empty array, never an omitted key (contract, OAuth and reads). */ + authorizedChannels?: SlackAuthorizedChannelFixture[]; } +/** Every field is null until a check is queued (contract, OAuth and reads). */ +export interface SlackVerificationFixture { + taskId: string | null; + startedAt: string | null; + finishedAt: string | null; +} + +export const NO_VERIFICATION: SlackVerificationFixture = { + taskId: null, + startedAt: null, + finishedAt: null, +}; + export interface SlackInstallFixture { id: string; /** `null` until the first connection check runs. */ connected: boolean | null; connectionLastCheckedAt: string | null; workspace: SlackWorkspaceFixture; + verification?: SlackVerificationFixture; } export const SLACK_EXCHANGE_OUTCOME = { @@ -46,6 +56,11 @@ export type SlackExchangeOutcome = export interface SlackConnectionFixture { connected: boolean; error: string | null; + /** + * Set only for a channel-level failure; absent for a credential-level one, + * which is about the workspace as a whole (contract, Connection). + */ + failedChannelId?: string | null; } /** A channel the listing endpoint offers for the picker. */ @@ -56,6 +71,14 @@ export interface SlackChannelFixture { isPrivate: boolean; } +/** + * `confirmationSentAt` is null until a check posts there, and a check never + * posts to a channel that already has one (contract, Connection). + */ +export interface SlackAuthorizedChannelFixture extends SlackChannelFixture { + confirmationSentAt: string | null; +} + /** * A refusal as the API sends one: the machine-readable reason in `code`, human * copy in `detail`, and — for a `429` — the wait in `Retry-After`. @@ -351,12 +374,6 @@ export const SLACK_CHANNELS: SlackChannelFixture[] = [ /** Two channels per page, so `SLACK_CHANNELS` spans exactly two pages. */ export const SLACK_CHANNELS_PAGE_SIZE = 2; -/** - * The first channel the picker offers, so an install seeded with it always - * points at a channel the listing really has. - */ -export const SLACK_DEFAULT_CHANNEL = SLACK_PUBLIC_CHANNEL; - const PROWLER_HQ: SlackWorkspaceFixture = { teamId: "T01PROWLER", teamName: "Prowler HQ", @@ -397,34 +414,76 @@ export const connectedSlackFixture = ( id: SLACK_INTEGRATION_ID, connected: null, connectionLastCheckedAt: null, - workspace: { ...PROWLER_HQ }, + workspace: { ...PROWLER_HQ, authorizedChannels: [] }, + verification: { ...NO_VERIFICATION }, }, exchangeOutcome: SLACK_EXCHANGE_OUTCOME.REINSTALLED, ...overrides, }); +export const SLACK_CONFIRMED_AT = "2026-08-10T09:30:00Z"; + +const SETTLED_VERIFICATION: SlackVerificationFixture = { + taskId: "5d408881-3e9e-4195-a281-a8d1be849472", + startedAt: "2026-08-10T09:29:58Z", + finishedAt: SLACK_CONFIRMED_AT, +}; + +/** + * Confirmed by default: an install reporting itself connected has had a check + * post to every channel on it. + */ +export const authorizedChannel = ( + channel: SlackChannelFixture, + confirmationSentAt: string | null = SLACK_CONFIRMED_AT, +): SlackAuthorizedChannelFixture => ({ ...channel, confirmationSentAt }); + const configuredInstall = ( - channel: SlackChannelFixture = SLACK_DEFAULT_CHANNEL, + channels: SlackChannelFixture[] = [SLACK_PUBLIC_CHANNEL], ): SlackInstallFixture => ({ id: SLACK_INTEGRATION_ID, connected: true, - connectionLastCheckedAt: "2026-08-10T09:30:00Z", + connectionLastCheckedAt: SLACK_CONFIRMED_AT, workspace: { ...PROWLER_HQ, - channelId: channel.id, - channelName: channel.name, + authorizedChannels: channels.map((channel) => authorizedChannel(channel)), }, + verification: { ...SETTLED_VERIFICATION }, }); /** - * The same tenant with a destination channel already on record: the state a - * second visit starts from. + * The same tenant with destination channels already authorized and confirmed: + * the state a second visit starts from. */ -export const slackFixtureWithDefaultChannel = ( - channel: SlackChannelFixture = SLACK_PUBLIC_CHANNEL, +export const slackFixtureWithAuthorizedChannels = ( + channels: SlackChannelFixture[] = [SLACK_PUBLIC_CHANNEL], overrides: Partial = {}, ): SlackFixture => - connectedSlackFixture({ install: configuredInstall(channel), ...overrides }); + connectedSlackFixture({ install: configuredInstall(channels), ...overrides }); + +/** + * What a same-workspace reinstall leaves behind: the set kept, every + * confirmation and the verification reset (contract, OAuth and reads). + */ +export const unconfirmedChannelsSlackFixture = ( + channels: SlackChannelFixture[] = [SLACK_PUBLIC_CHANNEL], + overrides: Partial = {}, +): SlackFixture => + connectedSlackFixture({ + install: { + id: SLACK_INTEGRATION_ID, + connected: null, + connectionLastCheckedAt: null, + workspace: { + ...PROWLER_HQ, + authorizedChannels: channels.map((channel) => + authorizedChannel(channel, null), + ), + }, + verification: { ...NO_VERIFICATION }, + }, + ...overrides, + }); /** * The same finished setup, with a check time no parser can read: a zero date @@ -446,20 +505,20 @@ export const unreadableCheckTimeSlackFixture = (): SlackFixture => export const partiallyReadSlackFixture = ( overrides: Partial = {}, ): SlackFixture => - slackFixtureWithDefaultChannel(SLACK_PUBLIC_CHANNEL, { + slackFixtureWithAuthorizedChannels([SLACK_PUBLIC_CHANNEL], { channelsRefusal: SLACK_RATE_LIMITED_REFUSAL, channelsRefusalFromCursor: SLACK_CHANNELS_PAGE_SIZE, ...overrides, }); /** - * A workspace connected *and* a channel on record. Anything the API refuses - * until a channel exists (the connection check) needs this fixture. + * A workspace connected *and* channels authorized. Anything the API refuses + * while the set is empty (the connection check) needs this fixture. */ export const configuredSlackFixture = ( overrides: Partial = {}, ): SlackFixture => - slackFixtureWithDefaultChannel(SLACK_DEFAULT_CHANNEL, overrides); + slackFixtureWithAuthorizedChannels([SLACK_PUBLIC_CHANNEL], overrides); /** * A connected tenant whose disconnect removes the row but cannot revoke at diff --git a/ui/__tests__/msw/handlers/slack.test.ts b/ui/__tests__/msw/handlers/slack.test.ts new file mode 100644 index 0000000000..29819206a3 --- /dev/null +++ b/ui/__tests__/msw/handlers/slack.test.ts @@ -0,0 +1,121 @@ +/** + * The Slack double's own writes, driven over HTTP because the page cannot + * navigate to every state a connection check leaves behind. A double serving a + * state the API cannot produce lets a UI test prove copy no deployment shows. + */ + +import { setupServer } from "msw/node"; +import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest"; + +import { handlersForSlack } from "./slack"; +import { + SLACK_INTEGRATION_ID, + SLACK_NOT_IN_CHANNEL_CODE, + SLACK_PRIVATE_CHANNEL, + SLACK_PUBLIC_CHANNEL, + SLACK_TOKEN_REVOKED_CODE, + unconfirmedChannelsSlackFixture, +} from "./slack.fixtures"; +import type { SlackConnectionFixture } from "./slack.fixtures"; + +const API = process.env.UI_API_BASE_URL; + +/** Re-declared, not imported from product code, so a rename fails here. */ +interface ChannelBody { + id: string; + confirmation_sent_at: string | null; +} + +const server = setupServer(); + +beforeAll(() => server.listen({ onUnhandledRequest: "error" })); +afterEach(() => server.resetHandlers()); +afterAll(() => server.close()); + +/** The poll is what writes, so the read has to follow it. */ +const runConnectionCheck = async (connection: SlackConnectionFixture) => { + server.use( + ...handlersForSlack( + unconfirmedChannelsSlackFixture( + [SLACK_PUBLIC_CHANNEL, SLACK_PRIVATE_CHANNEL], + { connection }, + ), + ), + ); + + const queued = await fetch( + `${API}/integrations/${SLACK_INTEGRATION_ID}/connection`, + { method: "POST" }, + ); + expect(queued.status).toBe(202); + const { data: task } = (await queued.json()) as { data: { id: string } }; + + const polled = await fetch(`${API}/tasks/${task.id}`); + expect(polled.status).toBe(200); + + const listed = await fetch( + `${API}/integrations?filter[integration_type]=slack`, + ); + const { data } = (await listed.json()) as { + data: { + attributes: { + connected: boolean | null; + configuration: { channels: ChannelBody[] }; + }; + }[]; + }; + + const { connected: recorded, configuration } = data[0].attributes; + return { + connected: recorded, + confirmedAt: new Map( + configuration.channels.map((channel) => [ + channel.id, + channel.confirmation_sent_at, + ]), + ), + }; +}; + +describe("the Slack double's connection check", () => { + it("confirms no channel at all when the failure is about the workspace", async () => { + // Given/When — a revoked grant, which `auth.test` finds before the + // per-channel loop, so the failure names no channel. + const { connected, confirmedAt } = await runConnectionCheck({ + connected: false, + error: SLACK_TOKEN_REVOKED_CODE, + }); + + // Then + expect(connected).toBe(false); + expect(confirmedAt.get(SLACK_PUBLIC_CHANNEL.id)).toBeNull(); + expect(confirmedAt.get(SLACK_PRIVATE_CHANNEL.id)).toBeNull(); + }); + + it("confirms every channel but the one a channel-level failure names", async () => { + // Given/When — the token is fine and Slack refuses one channel of the set. + const { connected, confirmedAt } = await runConnectionCheck({ + connected: false, + error: SLACK_NOT_IN_CHANNEL_CODE, + failedChannelId: SLACK_PRIVATE_CHANNEL.id, + }); + + // Then — the refused channel is the retry's remaining work. + expect(connected).toBe(false); + expect(confirmedAt.get(SLACK_PUBLIC_CHANNEL.id)).not.toBeNull(); + expect(confirmedAt.get(SLACK_PRIVATE_CHANNEL.id)).toBeNull(); + }); + + it("confirms the whole set when the check succeeds", async () => { + // Given/When + const { connected, confirmedAt } = await runConnectionCheck({ + connected: true, + error: null, + }); + + // Then + expect(connected).toBe(true); + expect(confirmedAt.get(SLACK_PUBLIC_CHANNEL.id)).not.toBeNull(); + expect(confirmedAt.get(SLACK_PRIVATE_CHANNEL.id)).not.toBeNull(); + }); +}); diff --git a/ui/__tests__/msw/handlers/slack.ts b/ui/__tests__/msw/handlers/slack.ts index 4dbe0476d5..9cfa1bde13 100644 --- a/ui/__tests__/msw/handlers/slack.ts +++ b/ui/__tests__/msw/handlers/slack.ts @@ -1,8 +1,9 @@ /** - * MSW handlers for the Slack integration, derived from the API contract in - * `openspec/changes/add-slack-integration/design.md` (the API itself lives in - * the cloud repository). State is per-call: an exchange creates the install the - * subsequent `GET /integrations` returns. + * MSW handlers for the Slack integration, derived from the signed API contract + * in `openspec/changes/add-slack-alert-channels/contract/slack-alerts-api.md` + * (the API itself lives in the cloud repository). State is per-call: an + * exchange creates the install the subsequent `GET /integrations` returns, and + * a save or a connection check writes to it. * * Wire them per test via `worker.use(...handlersForSlack(fx))`. */ @@ -11,6 +12,7 @@ import { http, HttpResponse } from "msw"; import { INTEGRATIONS_SERVER_ERROR_DETAIL, + NO_VERIFICATION, PROXY_CHALLENGE_PAGE, SLACK_AUTHORIZE_URL, SLACK_DIFFERENT_WORKSPACE_DETAIL, @@ -27,6 +29,7 @@ import { SLACK_WORKSPACE_CONFLICT_CODE, } from "./slack.fixtures"; import type { + SlackAuthorizedChannelFixture, SlackExchangeOutcome, SlackFixture, SlackInstallFixture, @@ -35,9 +38,15 @@ import type { const API = process.env.UI_API_BASE_URL; const TS = "2026-08-10T09:00:00Z"; +/** The time a check records, distinct from `TS` so a test can tell one ran. */ +const CHECK_TS = "2026-08-10T10:15:00Z"; const CONNECTION_TASK_PREFIX = "slack-conn-task-"; +/** Order-insensitive: a reorder is not a changed set (contract, PATCH). */ +const sameChannelIds = (a: string[], b: string[]) => + a.length === b.length && new Set([...a, ...b]).size === a.length; + /** Opaque to the UI, which only ever follows `links.next` (design D6). */ const CHANNEL_CURSOR_PARAM = "page[cursor]"; @@ -71,14 +80,26 @@ const refuse = (refusal: SlackRefusalFixture) => }, ); -const configuration = (workspace: SlackInstallFixture["workspace"]) => ({ - team_id: workspace.teamId, - team_name: workspace.teamName, - bot_user_id: workspace.botUserId, - // The API omits these keys until a channel is chosen, never sending nulls. - ...(workspace.channelId ? { channel_id: workspace.channelId } : {}), - ...(workspace.channelName ? { channel_name: workspace.channelName } : {}), -}); +const configuration = (install: SlackInstallFixture) => { + const verification = install.verification ?? NO_VERIFICATION; + return { + team_id: install.workspace.teamId, + team_name: install.workspace.teamName, + bot_user_id: install.workspace.botUserId, + // Always an array, empty rather than omitted (contract, OAuth and reads). + channels: (install.workspace.authorizedChannels ?? []).map((channel) => ({ + id: channel.id, + name: channel.name, + is_private: channel.isPrivate, + confirmation_sent_at: channel.confirmationSentAt, + })), + verification: { + task_id: verification.taskId, + started_at: verification.startedAt, + finished_at: verification.finishedAt, + }, + }; +}; const integrationResource = (install: SlackInstallFixture) => ({ id: install.id, @@ -91,7 +112,7 @@ const integrationResource = (install: SlackInstallFixture) => ({ connection_last_checked_at: install.connectionLastCheckedAt, integration_type: "slack", // No credentials: the bot token is encrypted at rest and never serialized. - configuration: configuration(install.workspace), + configuration: configuration(install), }, links: { self: `${API}/integrations/${install.id}` }, }); @@ -112,6 +133,18 @@ const taskResource = (id: string, state: string, result: unknown) => ({ data: { id, type: "tasks", attributes: { state, result } }, }); +/** Stamps only channels this run posted to, never one already confirmed. */ +const confirmedByThisRun = + (connected: boolean, failedChannelId: string | null) => + (channel: SlackAuthorizedChannelFixture): SlackAuthorizedChannelFixture => + // A failure naming no channel posted nothing: `auth.test` runs once, before + // the per-channel loop (contract, Connection). + (connected || failedChannelId !== null) && + channel.confirmationSentAt === null && + channel.id !== failedChannelId + ? { ...channel, confirmationSentAt: CHECK_TS } + : channel; + /** * All three are `2xx`: the first two make `response.json()` throw, the third * parses into a body that names no resource. @@ -133,6 +166,14 @@ export const handlersForSlack = (fx: SlackFixture) => { ? { ...fx.install, workspace: { ...fx.install.workspace } } : null; + const freshInstall = (id?: string): SlackInstallFixture => ({ + id: id ?? SLACK_INTEGRATION_ID, + connected: null, + connectionLastCheckedAt: null, + workspace: { ...fx.exchangeWorkspace, authorizedChannels: [] }, + verification: { ...NO_VERIFICATION }, + }); + const unconfigured = () => HttpResponse.json(errorBody(SLACK_UNCONFIGURED_DETAIL, 503), { status: 503, @@ -189,28 +230,23 @@ export const handlersForSlack = (fx: SlackFixture) => { case SLACK_EXCHANGE_OUTCOME.UNREADABLE_HTML: case SLACK_EXCHANGE_OUTCOME.UNREADABLE_NO_DATA: // The install still happened: the API upserts before it answers. - install = { - id: SLACK_INTEGRATION_ID, - connected: null, - connectionLastCheckedAt: null, - workspace: { ...fx.exchangeWorkspace }, - }; + install = freshInstall(); return unreadableExchange(fx.exchangeOutcome); case SLACK_EXCHANGE_OUTCOME.REINSTALLED: install = { - id: install?.id ?? SLACK_INTEGRATION_ID, - connected: null, - connectionLastCheckedAt: null, - workspace: { ...fx.exchangeWorkspace }, + ...freshInstall(install?.id), + workspace: { + ...fx.exchangeWorkspace, + // A reinstall keeps the set and resets every confirmation, with the + // connection and verification state (contract, OAuth and reads). + authorizedChannels: ( + install?.workspace.authorizedChannels ?? [] + ).map((channel) => ({ ...channel, confirmationSentAt: null })), + }, }; return HttpResponse.json({ data: integrationResource(install) }); default: - install = { - id: SLACK_INTEGRATION_ID, - connected: null, - connectionLastCheckedAt: null, - workspace: { ...fx.exchangeWorkspace }, - }; + install = freshInstall(); return HttpResponse.json( { data: integrationResource(install) }, { status: 201 }, @@ -237,32 +273,55 @@ export const handlersForSlack = (fx: SlackFixture) => { http.post<{ id: string }>( `${API}/integrations/:id/connection`, ({ params }) => { - // The check posts to the channel, so the API refuses until one exists. - if (!install?.workspace.channelId) { + // A check posts to every authorized channel, so the API requires at least + // one (contract, Connection). + if (!install?.workspace.authorizedChannels?.length) { return HttpResponse.json(errorBody(SLACK_NO_CHANNEL_DETAIL, 400), { status: 400, }); } - return HttpResponse.json( - taskResource( - `${CONNECTION_TASK_PREFIX}${params.id}`, - "executing", - null, - ), - { status: 202 }, - ); + // Id stored before the task is published; the worker stamps `started_at`. + const taskId = `${CONNECTION_TASK_PREFIX}${params.id}`; + install.verification = { + taskId, + startedAt: null, + finishedAt: null, + }; + + return HttpResponse.json(taskResource(taskId, "executing", null), { + status: 202, + }); }, ), http.get<{ taskId: string }>(`${API}/tasks/:taskId`, ({ params }) => { - const { connected, error } = fx.connection; - if (install && params.taskId.startsWith(CONNECTION_TASK_PREFIX)) { + const { connected, error, failedChannelId } = fx.connection; + // Only the task whose id still matches may write: a late poll must not + // overwrite a newer check (contract, Connection). + if ( + install && + params.taskId.startsWith(CONNECTION_TASK_PREFIX) && + install.verification?.taskId === params.taskId + ) { install.connected = connected; - install.connectionLastCheckedAt = TS; + install.connectionLastCheckedAt = CHECK_TS; + install.verification = { + taskId: params.taskId, + startedAt: CHECK_TS, + finishedAt: CHECK_TS, + }; + install.workspace.authorizedChannels = ( + install.workspace.authorizedChannels ?? [] + ).map(confirmedByThisRun(connected, failedChannelId ?? null)); } return HttpResponse.json( - taskResource(params.taskId, "completed", { connected, error }), + // `channel` carries the failing channel's id, not an object. + taskResource(params.taskId, "completed", { + connected, + error, + channel: failedChannelId ?? null, + }), ); }), @@ -307,8 +366,8 @@ export const handlersForSlack = (fx: SlackFixture) => { ), /** - * The generic PATCH. The UI submits only `channel_id`; the name is derived - * from it here, as the API derives it from Slack (design D6). + * The generic PATCH. The write names only `id`; name and privacy are derived + * here, as the API derives them from Slack (contract, PATCH). */ http.patch(`${API}/integrations/:id`, async ({ request }) => { const body = (await request.json().catch(() => null)) as { @@ -316,10 +375,9 @@ export const handlersForSlack = (fx: SlackFixture) => { } | null; const attributes = body?.data?.attributes ?? {}; const configurationPatch = attributes.configuration as - | { channel_id?: string } + | { channels?: { id: string }[] } | undefined; - const channelId = configurationPatch?.channel_id; - const channel = fx.channels.find((c) => c.id === channelId); + const requested = configurationPatch?.channels; if (!install) { return HttpResponse.json(errorBody("Not found.", 404), { status: 404 }); @@ -338,17 +396,53 @@ export const handlersForSlack = (fx: SlackFixture) => { status: 400, }); } - // Checked before the id lookup: the picker did offer this channel, and - // Slack refused it anyway when the API validated it. + // Omitted leaves the set alone; empty clears it (contract, PATCH). + if (requested === undefined) { + return HttpResponse.json({ data: integrationResource(install) }); + } + + const channelIds = Array.from( + new Set(requested.map((channel) => channel.id)), + ); + const matched = channelIds + .map((channelId) => + fx.channels.find((channel) => channel.id === channelId), + ) + .filter((channel) => channel !== undefined); + + // Checked before the id lookup: the picker did offer these channels, and + // Slack refused one anyway when the API validated the set. if (fx.channelSaveRefusal) return refuse(fx.channelSaveRefusal); - if (!channel) { + // One unknown id refuses the whole save: the set is validated together. + if (matched.length !== channelIds.length) { return HttpResponse.json(errorBody(SLACK_UNKNOWN_CHANNEL_DETAIL, 400), { status: 400, }); } - install.workspace.channelId = channel.id; - install.workspace.channelName = channel.name; + const previous = install.workspace.authorizedChannels ?? []; + const confirmedAt = new Map( + previous.map((channel) => [channel.id, channel.confirmationSentAt]), + ); + install.workspace.authorizedChannels = matched.map((channel) => ({ + ...channel, + // Retained ids keep their confirmation; new ones start without one. + confirmationSentAt: confirmedAt.get(channel.id) ?? null, + })); + + // A changed id set resets the connection and verification state, so the + // record never claims a check covered a channel it never saw. + if ( + !sameChannelIds( + previous.map((channel) => channel.id), + channelIds, + ) + ) { + install.connected = null; + install.connectionLastCheckedAt = null; + install.verification = { ...NO_VERIFICATION }; + } + return HttpResponse.json({ data: integrationResource(install) }); }), diff --git a/ui/actions/integrations/integrations.ts b/ui/actions/integrations/integrations.ts index f93ef93baa..9a62452748 100644 --- a/ui/actions/integrations/integrations.ts +++ b/ui/actions/integrations/integrations.ts @@ -18,6 +18,8 @@ type TestConnectionResponse = { taskId?: string; data?: TaskStartResponse; error?: string; + /** The id of the channel a channel-level failure named, when it named one. */ + failedChannelId?: string | null; }; export const getIntegrations = async (searchParams?: URLSearchParams) => { @@ -265,7 +267,12 @@ export const deleteIntegration = async ( } }; -type ConnectionTaskResult = { connected?: boolean; error?: string | null }; +type ConnectionTaskResult = { + connected?: boolean; + error?: string | null; + // The failing channel's id, or null when the failure names no channel. + channel?: string | null; +}; type PollConnectionResult = | { @@ -358,6 +365,7 @@ export const testIntegrationConnection = async ( return { success: false, error: pollResult.message || "Connection test failed.", + failedChannelId: pollResult.result?.channel ?? null, }; } } else { @@ -404,6 +412,7 @@ export const pollConnectionTestStatus = async ( return { success: false, error: pollResult.message || "Connection test failed.", + failedChannelId: pollResult.result?.channel ?? null, }; } } catch (_error) { diff --git a/ui/actions/integrations/slack.test.ts b/ui/actions/integrations/slack.test.ts index c49f2cc2de..ba00e6b87b 100644 --- a/ui/actions/integrations/slack.test.ts +++ b/ui/actions/integrations/slack.test.ts @@ -63,7 +63,7 @@ import { exchangeSlackOAuthCode, getSlackAuthorizeUrl, getSlackChannels, - setSlackDefaultChannel, + setSlackAuthorizedChannels, } from "./slack"; /** The status the contract reserves for an upstream Slack failure. */ @@ -510,10 +510,10 @@ const expectNoParserProse = (result: unknown) => { const INTEGRATION_URL = `https://api.test/api/v1/integrations/${SLACK_INTEGRATION_ID}`; -const saveChannel = () => - setSlackDefaultChannel(SLACK_INTEGRATION_ID, FIRST_CHANNEL.id); +const saveChannels = () => + setSlackAuthorizedChannels(SLACK_INTEGRATION_ID, [FIRST_CHANNEL.id]); -/** The save as the API answers it: the channel's name derived server-side. */ +/** The save as the API answers it: the channels' names derived server-side. */ const savedIntegration = () => new Response( JSON.stringify({ @@ -523,8 +523,14 @@ const savedIntegration = () => attributes: { integration_type: "slack", configuration: { - channel_id: FIRST_CHANNEL.id, - channel_name: FIRST_CHANNEL.name, + channels: [ + { + id: FIRST_CHANNEL.id, + name: FIRST_CHANNEL.name, + is_private: false, + confirmation_sent_at: null, + }, + ], }, }, }, @@ -542,16 +548,20 @@ const expectIntegrationsRevalidated = () => { ]); }; -describe("setSlackDefaultChannel", () => { +describe("setSlackAuthorizedChannels", () => { it("returns the saved integration and revalidates the pages listing it", async () => { fetchMock.mockResolvedValueOnce(savedIntegration()); - const result = await saveChannel(); + const result = await saveChannels(); expect(requestedUrls()).toEqual([INTEGRATION_URL]); expect(result).toMatchObject({ integration: { - attributes: { configuration: { channel_name: FIRST_CHANNEL.name } }, + attributes: { + configuration: { + channels: [expect.objectContaining({ name: FIRST_CHANNEL.name })], + }, + }, }, }); expectIntegrationsRevalidated(); @@ -560,16 +570,35 @@ describe("setSlackDefaultChannel", () => { // The write serializer names whatever it will not take and refuses the whole // save, so a body that also carried the integration's own (immutable) type // came back as `Invalid fields: {'integration_type'}` and recorded nothing. - it("submits the channel as the save's only attribute", async () => { + it("submits the channels as the save's only attribute, naming nothing but their ids", async () => { fetchMock.mockResolvedValueOnce(savedIntegration()); - await saveChannel(); + await saveChannels(); expect(sentBody()).toEqual({ data: { type: "integrations", id: SLACK_INTEGRATION_ID, - attributes: { configuration: { channel_id: FIRST_CHANNEL.id } }, + attributes: { + configuration: { channels: [{ id: FIRST_CHANNEL.id }] }, + }, + }, + }); + }); + + it("submits a channel once, however many times the caller named it", async () => { + fetchMock.mockResolvedValueOnce(savedIntegration()); + + await setSlackAuthorizedChannels(SLACK_INTEGRATION_ID, [ + FIRST_CHANNEL.id, + FIRST_CHANNEL.id, + ]); + + expect(sentBody()).toMatchObject({ + data: { + attributes: { + configuration: { channels: [{ id: FIRST_CHANNEL.id }] }, + }, }, }); }); @@ -582,11 +611,11 @@ describe("setSlackDefaultChannel", () => { async ({ body }) => { fetchMock.mockResolvedValueOnce(unreadableOk(body)); - const result = await saveChannel(); + const result = await saveChannels(); expect(result).toEqual({ error: SLACK_UNREADABLE_RESULT_MESSAGE }); expectNoParserProse(result); - // The API recorded the channel before answering, so both pages refresh. + // The API recorded the channels before answering, so both pages refresh. expectIntegrationsRevalidated(); }, ); @@ -607,7 +636,7 @@ describe("setSlackDefaultChannel", () => { }), ); - const result = await saveChannel(); + const result = await saveChannels(); expect(result).toEqual({ error: SLACK_UNREADABLE_RESULT_MESSAGE }); expectNoParserProse(result); @@ -623,8 +652,8 @@ const COPY_ONLY_ACTIONS = [ call: (id: string) => getSlackChannels(id), }, { - name: "setSlackDefaultChannel", - call: (id: string) => setSlackDefaultChannel(id, FIRST_CHANNEL.id), + name: "setSlackAuthorizedChannels", + call: (id: string) => setSlackAuthorizedChannels(id, [FIRST_CHANNEL.id]), }, { name: "disconnectSlackIntegration", @@ -687,8 +716,8 @@ describe.each(COPY_ONLY_ACTIONS)("$name", ({ call }) => { { status: 400, why: "a refusal the API meant to give", - response: () => errorResponse(400, "No default channel is set."), - expected: "No default channel is set.", + response: () => errorResponse(400, "The integration is not connected."), + expected: "The integration is not connected.", }, ])("reports nothing for a $status: that is $why", async (refusal) => { fetchMock.mockResolvedValue(refusal.response()); diff --git a/ui/actions/integrations/slack.ts b/ui/actions/integrations/slack.ts index 4761c3e446..b22746f995 100644 --- a/ui/actions/integrations/slack.ts +++ b/ui/actions/integrations/slack.ts @@ -330,11 +330,11 @@ const MAX_CHANNEL_PAGES = 20; * Every channel Prowler can post to in the connected workspace — the picker's * options. * - * The durable primitive, not the channel stored on the integration (design D6): - * a consumer needing a per-rule channel reads the same endpoint. `links.next` - * is followed opaquely — the contract does not pin the cursor parameter naming, - * so the UI never builds one of its own. An early stop that still read - * something reports through `incomplete`, not as a failure. + * The durable primitive, not the channels recorded on the integration + * (design D6): a consumer needing a per-rule channel reads the same endpoint. + * `links.next` is followed opaquely — the contract does not pin the cursor + * parameter naming, so the UI never builds one of its own. An early stop that + * still read something reports through `incomplete`, not as a failure. */ export const getSlackChannels = async ( integrationId: string, @@ -422,26 +422,27 @@ export const getSlackChannels = async ( } }; -interface SlackDefaultChannelSuccess { +interface SlackAuthorizedChannelsSuccess { integration: IntegrationProps; } -export type SlackDefaultChannelResult = - | SlackDefaultChannelSuccess +export type SlackAuthorizedChannelsResult = + | SlackAuthorizedChannelsSuccess | SlackActionError; /** - * Record the channel Prowler posts to, on the generic integration endpoint. + * Record the set of channels Prowler is authorized to post to, on the generic + * integration endpoint. The list replaces the set; an empty one clears it. * * A Slack action despite the generic `PATCH`: `channel_not_found` and * `not_in_channel` carry the same `detail`, so only `code` tells them apart, - * and the generic action reads `detail` alone. Only `channel_id` travels — the - * API derives `channel_name` server-side (design D6). + * and the generic action reads `detail` alone. Only ids travel — the API + * derives each name and its privacy server-side. */ -export const setSlackDefaultChannel = async ( +export const setSlackAuthorizedChannels = async ( integrationId: string, - channelId: string, -): Promise => { + channelIds: string[], +): Promise => { const id = parseIntegrationId(integrationId); if (!id) return { error: SLACK_GENERIC_ERROR_MESSAGE }; @@ -460,7 +461,14 @@ export const setSlackDefaultChannel = async ( // serializer refuses whatever it does not accept, so naming the // integration's own (immutable) type is answered with a 400, // "Invalid fields: {'integration_type'}". - attributes: { configuration: { channel_id: channelId } }, + attributes: { + configuration: { + // Ids wrapped as objects, never a bare array (contract, PATCH). + channels: Array.from(new Set(channelIds), (channelId) => ({ + id: channelId, + })), + }, + }, }, }), }); @@ -470,18 +478,18 @@ export const setSlackDefaultChannel = async ( // this `catch`. return await refusalFrom( response, - `Unable to save the destination channel: ${response.statusText}`, + `Unable to save the destination channels: ${response.statusText}`, ); } const body = await response.json().catch(() => null); // Before the guard and on both paths: the save happened, so a cache still - // holding the previous channel would keep showing it. + // holding the previous channels would keep showing them. revalidatePath("/integrations"); revalidatePath("/integrations/slack"); - // Guarded as deep as the caller reads: it names the saved channel from + // Guarded as deep as the caller reads: it names the saved channels from // `attributes.configuration`. if (!body?.data?.attributes?.configuration) { return { error: SLACK_UNREADABLE_RESULT_MESSAGE }; diff --git a/ui/app/(prowler)/integrations/slack/page.tsx b/ui/app/(prowler)/integrations/slack/page.tsx index 7f1aea2b65..f46c43ed5d 100644 --- a/ui/app/(prowler)/integrations/slack/page.tsx +++ b/ui/app/(prowler)/integrations/slack/page.tsx @@ -16,7 +16,8 @@ export default async function SlackIntegrationPage() {

- Connect a Slack workspace so Prowler can post to one of its channels. + Connect a Slack workspace so Prowler can post to the channels you + authorize.

diff --git a/ui/app/(prowler)/integrations/slack/slack-integration.harness.ts b/ui/app/(prowler)/integrations/slack/slack-integration.harness.ts index 4edb8ebbde..ee22452dd1 100644 --- a/ui/app/(prowler)/integrations/slack/slack-integration.harness.ts +++ b/ui/app/(prowler)/integrations/slack/slack-integration.harness.ts @@ -14,7 +14,7 @@ import { handlersForSlack } from "@/__tests__/msw/handlers/slack"; import type { SlackFixture } from "@/__tests__/msw/handlers/slack.fixtures"; import { worker } from "@/__tests__/msw/worker"; import { render } from "@/__tests__/render-browser"; -import { setSlackDefaultChannel } from "@/actions/integrations/slack"; +import { setSlackAuthorizedChannels } from "@/actions/integrations/slack"; import { SlackCallback } from "@/components/integrations/slack/slack-callback"; import { IntegrationsContent } from "../integrations-content"; @@ -29,8 +29,13 @@ export const CONNECTION_OUTCOME = { export type ConnectionOutcome = (typeof CONNECTION_OUTCOME)[keyof typeof CONNECTION_OUTCOME]; -/** Sentinel: the page settled on "no channel recorded", rather than not yet. */ -const NO_DEFAULT_CHANNEL = ""; +/** Sentinel: the page settled on "no channels authorized", rather than not yet. */ +const NO_AUTHORIZED_CHANNELS = ""; + +interface ChannelChip { + name: string; + isPrivate: boolean; +} export const REVOCATION_OUTCOME = { REVOKED: "revoked", @@ -125,6 +130,8 @@ export class SlackIntegrationHarness extends BrowserHarness { } async mountCallback({ code, state, error }: CallbackParams): Promise { + // Unmount first, or two copies of the page answer every query. + (await this.mounted)?.unmount(); const params = new URLSearchParams(); if (code) params.set("code", code); if (state) params.set("state", state); @@ -136,7 +143,8 @@ export class SlackIntegrationHarness extends BrowserHarness { ); this.wireHandlers(); - render(createElement(SlackCallback)); + // Held so the `revisit()` that follows a reinstall can take it down first. + this.mounted = render(createElement(SlackCallback)); } /** Mount the integrations catalogue. No handlers: every card there is static. */ @@ -294,11 +302,24 @@ export class SlackIntegrationHarness extends BrowserHarness { return !button.disabled; } + /** The same copy, waited for: it follows data that lands after the action. */ + async connectionCheckHintMatching(pattern: RegExp): Promise { + return this.waitFor( + () => { + const hint = this.connectionCheckHint(); + return hint && pattern.test(hint) ? hint : null; + }, + 10000, + `the connection check hint to match ${pattern}`, + ); + } + /** - * Why the check cannot run, read from the copy the button points at: a reason - * found anywhere else on the page never reaches whoever sees the control. + * What the check does — or why it cannot run — read from the copy the button + * points at: a reason found anywhere else on the page never reaches whoever + * sees the control. */ - connectionCheckBlockedReason(): string | null { + connectionCheckHint(): string | null { const button = this.buttonByText(/Test connection/); const describedBy = button?.getAttribute("aria-describedby"); if (!describedBy) return null; @@ -307,6 +328,14 @@ export class SlackIntegrationHarness extends BrowserHarness { return reason ? (reason.textContent ?? "").trim() : null; } + async connectionFailureToast(): Promise { + return this.waitFor( + () => this.toastText(/Connection test failed/), + 15000, + "the connection failure toast", + ); + } + /** * The "last checked" line as rendered, or null when the page shows none — * which is what a workspace whose connection was never checked shows. @@ -421,24 +450,30 @@ export class SlackIntegrationHarness extends BrowserHarness { ); } + /** + * Scoped to the popover: the multi-select keeps a hidden mirror of its items + * that exists with the listing closed, so a bare `[role="option"]` matches it. + */ + private openPickerOptions(): HTMLElement[] | null { + const options = Array.from( + document.querySelectorAll( + '[data-slot="multiselect-content"] [role="option"]', + ), + ); + return options.length > 0 ? options : null; + } + /** * Open the picker and hand back its options. A re-render landing mid-gesture * makes Radix drop the open state, so re-open from the keyboard when nothing * mounted at all. */ private async openChannelPicker(): Promise { - const mounted = (): HTMLElement[] | null => { - const options = Array.from( - document.querySelectorAll('[role="option"]'), - ); - return options.length > 0 ? options : null; - }; - - const alreadyOpen = mounted(); + const alreadyOpen = this.openPickerOptions(); if (alreadyOpen) return alreadyOpen; const trigger = await this.waitFor( - () => this.q("#slack-channel"), + () => this.q("#slack-channels"), 10000, "the channel picker", ); @@ -446,13 +481,17 @@ export class SlackIntegrationHarness extends BrowserHarness { await this.clickElement(trigger, { fallbackToDomClick: true }); let options = await this.waitForOrNull( - mounted, + () => this.openPickerOptions(), 2000, "the channel options", ); if (!options) { await this.user.keyboard("{Enter}"); - options = await this.waitForOrNull(mounted, 8000, "the channel options"); + options = await this.waitForOrNull( + () => this.openPickerOptions(), + 8000, + "the channel options", + ); } if (!options) { @@ -535,97 +574,155 @@ export class SlackIntegrationHarness extends BrowserHarness { // would never settle — the tests only narrow. await this.waitFor( () => - document.querySelectorAll('[role="option"]').length !== all.length || - document.querySelector("[cmdk-empty]") !== null || + (this.openPickerOptions() ?? []).length !== all.length || + document.querySelector( + '[data-slot="multiselect-content"] [cmdk-empty]', + ) !== null || null, 5000, "the search to narrow the channels", ); - const offered = Array.from( - document.querySelectorAll('[role="option"]'), - ).map((option) => option.getAttribute("data-channel") ?? ""); + const offered = (this.openPickerOptions() ?? []).map( + (option) => option.getAttribute("data-channel") ?? "", + ); const emptyNote = - document.querySelector("[cmdk-empty]")?.textContent ?? null; + document.querySelector( + '[data-slot="multiselect-content"] [cmdk-empty]', + )?.textContent ?? null; await this.closeChannelPicker(); return { offered, emptyNote }; } - private async pickAndSave(name: string): Promise { - const options = await this.openChannelPicker(); - const option = options.find( - (element) => element.getAttribute("data-channel") === name, - ); + private async pickChannels(names: string[]): Promise { + for (const name of names) { + const options = await this.openChannelPicker(); + const option = options.find( + (element) => element.getAttribute("data-channel") === name, + ); - if (!option) { - throw new Error(`pickAndSave: no channel named "${name}" is offered`); + if (!option) { + throw new Error(`pickChannels: no channel named "${name}" is offered`); + } + + await this.user.click(option); + await this.waitForTransition(); } - - await this.user.click(option); - await this.waitForTransition(); - await this.clickButton(/Save channel/); + await this.closeChannelPicker(); } - /** Pick a channel, save it, and wait for it to be recorded as the destination. */ - async chooseChannel(name: string): Promise { - await this.pickAndSave(name); + /** Toggle the named channels without saving: the picks stay buffered. */ + async chooseChannels(names: string[]): Promise { + await this.pickChannels(names); + } + + async saveChannels(): Promise { + await this.clickButton(/Save channels/); + } + + /** What the page says before a save that drops channels (design D11). */ + deauthorizationWarning(): string | null { + const warning = this.q("[data-deauthorize-warning]"); + return warning + ? (warning.textContent ?? "").replace(/\s+/g, " ").trim() + : null; + } + + /** Drop the named channels, save, and wait until each is off the record. */ + async deauthorizeChannels(names: string[]): Promise { + await this.pickChannels(names); + await this.saveChannels(); await this.waitFor( - () => this.defaultChannelName() === name, + () => { + const authorized = this.authorizedChannelNames(); + const settled = + authorized ?? + (this.containsText(/No destination channels authorized yet/) + ? [] + : null); + return settled && names.every((name) => !settled.includes(name)) + ? true + : null; + }, 15000, - `#${name} to be recorded as the destination`, + `${names.map((name) => `#${name}`).join(", ")} to be de-authorized`, + ); + } + + /** Toggle the named channels, save, and wait until each is recorded. */ + async authorizeChannels(names: string[]): Promise { + await this.pickChannels(names); + await this.saveChannels(); + await this.waitFor( + () => { + const authorized = this.authorizedChannelNames(); + return authorized && names.every((name) => authorized.includes(name)) + ? true + : null; + }, + 15000, + `${names.map((name) => `#${name}`).join(", ")} to be authorized`, ); } /** - * Record a different destination away from this page — a second tab, or someone + * Authorize a different set away from this page — a second tab, or someone * else in the tenant. Goes through the same call the page makes, leaving this * page's own copy of it untouched. */ - async channelRecordedElsewhere(name: string): Promise { - const channel = this.fixture.channels.find((c) => c.name === name); - if (!channel) { - throw new Error( - `channelRecordedElsewhere: no channel named "${name}" is offered`, - ); - } + async channelsRecordedElsewhere(names: string[]): Promise { + const channels = names.map((name) => { + const channel = this.fixture.channels.find((c) => c.name === name); + if (!channel) { + throw new Error( + `channelsRecordedElsewhere: no channel named "${name}" is offered`, + ); + } + return channel; + }); const integrationId = this.fixture.install?.id; if (!integrationId) { - throw new Error("channelRecordedElsewhere: no workspace is connected"); + throw new Error("channelsRecordedElsewhere: no workspace is connected"); } - const result = await setSlackDefaultChannel(integrationId, channel.id); + const result = await setSlackAuthorizedChannels( + integrationId, + channels.map((channel) => channel.id), + ); if ("error" in result) { - throw new Error(`channelRecordedElsewhere: ${result.error}`); + throw new Error(`channelsRecordedElsewhere: ${result.error}`); } } - /** Whether the picked channel can be saved — false when there is nothing new to save. */ - offersChannelSave(): boolean { - const button = this.buttonByText(/Save channel/); + /** Whether the picked channels can be saved — false when there is nothing new to save. */ + offersChannelsSave(): boolean { + const button = this.buttonByText(/Save channels/); return button !== null && !button.disabled; } /** - * Try to save a channel the API refuses and hand back what the user is told. A - * save that succeeds fails the test rather than timing out. + * Try to save channels the API refuses and hand back what the user is told. + * A save that succeeds fails the test rather than timing out. */ - async refusedChannelSave(name: string): Promise { - await this.pickAndSave(name); + async refusedChannelsSave(names: string[]): Promise { + await this.pickChannels(names); + await this.saveChannels(); return this.waitFor( () => { - if (this.defaultChannelName() === name) { + const authorized = this.authorizedChannelNames() ?? []; + if (names.every((name) => authorized.includes(name))) { throw new Error( - `refusedChannelSave: #${name} was recorded, not refused`, + `refusedChannelsSave: ${names.join(", ")} were recorded, not refused`, ); } - return this.toastText(/Could not save the destination channel/); + return this.toastText(/Could not save the destination channels/); }, 15000, - "the refused channel save", + "the refused channels save", ); } @@ -640,26 +737,58 @@ export class SlackIntegrationHarness extends BrowserHarness { return toast ? (toast.textContent ?? "").replace(/\s+/g, " ").trim() : null; } - private defaultChannelName(): string | null { - return ( - /Prowler posts to #(\S+?)\./.exec( - this.container.textContent ?? "", - )?.[1] ?? null - ); + /** The names the "Prowler posts to …" summary carries, or null while unsettled. */ + private authorizedChannelNames(): string[] | null { + // The keyed element, not a text search: the card's subtitle also starts + // with "Prowler posts to". + const summary = this.q("[data-authorized-channels]"); + const text = (summary?.textContent ?? "").trim(); + if (!/^Prowler posts to /.test(text)) return null; + + return Array.from(text.matchAll(/#([^\s,.]+)/g), (match) => match[1]); } - /** The channel recorded as the integration's destination, if any. */ - async defaultChannel(): Promise { - const settled = await this.waitFor( + /** The channels recorded as the integration's authorized destinations. */ + async authorizedChannels(): Promise { + const settled = await this.waitFor< + string[] | typeof NO_AUTHORIZED_CHANNELS + >( () => - this.defaultChannelName() ?? - (this.containsText(/No destination channel recorded yet/) - ? NO_DEFAULT_CHANNEL + this.authorizedChannelNames() ?? + (this.containsText(/No destination channels authorized yet/) + ? NO_AUTHORIZED_CHANNELS : null), 10000, - "the recorded destination channel", + "the authorized destination channels", ); - return settled === NO_DEFAULT_CHANNEL ? null : settled; + return settled === NO_AUTHORIZED_CHANNELS ? [] : settled; + } + + /** + * What the closed picker shows: a chip per selected channel, each carrying + * whether the chip itself marks the channel private. + */ + async authorizedChannelChips(): Promise { + const chips = await this.waitFor( + () => { + const found = Array.from( + this.container.querySelectorAll( + '[data-slot="multiselect-value"] [data-selected-item]', + ), + ); + return found.length > 0 ? found : null; + }, + 10000, + "the authorized channel chips", + ); + + return chips.map((chip) => { + const text = (chip.textContent ?? "").trim(); + return { + name: text.replace(/^Private/, "").replace(/^#/, ""), + isPrivate: /^Private/.test(text), + }; + }); } /** What the user is told when the workspace exposes no channel at all. */ diff --git a/ui/app/(prowler)/integrations/slack/slack-page.integration.test.tsx b/ui/app/(prowler)/integrations/slack/slack-page.integration.test.tsx index e201b388ce..dc5ac48681 100644 --- a/ui/app/(prowler)/integrations/slack/slack-page.integration.test.tsx +++ b/ui/app/(prowler)/integrations/slack/slack-page.integration.test.tsx @@ -22,6 +22,8 @@ import { SLACK_MISSING_SCOPE_REFUSAL, SLACK_NOT_IN_CHANNEL_CODE, SLACK_NOT_IN_CHANNEL_REFUSAL, + SLACK_OAUTH_CODE, + SLACK_OAUTH_STATE, SLACK_PRIVATE_CHANNEL, SLACK_PUBLIC_CHANNEL, SLACK_RATE_LIMITED_REFUSAL, @@ -32,7 +34,7 @@ import { SLACK_UNKNOWN_CHANNEL_DETAIL, SLACK_UPSTREAM_REFUSAL, slackFixture, - slackFixtureWithDefaultChannel, + slackFixtureWithAuthorizedChannels, unreadableCheckTimeSlackFixture, unreportedRevocationSlackFixture, } from "@/__tests__/msw/handlers/slack.fixtures"; @@ -43,7 +45,7 @@ import { SlackIntegrationHarness, } from "./slack-integration.harness"; -/** The shape the channel save is asserted against — only the id travels. */ +/** The shape the channels save is asserted against — only ids travel. */ interface PatchIntegrationBody { data: PatchIntegrationData; } @@ -57,12 +59,15 @@ interface PatchIntegrationAttributes { } interface PatchChannelConfiguration { - channel_id: string; + channels: { id: string }[]; } /** The workspace the fixtures connect. */ const WORKSPACE_NAME = "Prowler HQ"; +/** The tail guard keeps `#security` from matching `#security-alerts`. */ +const channelMention = (name: string) => new RegExp(`#${name}(?![\\w-])`); + /** The only scopes Prowler asks a workspace for (design D2). */ const REQUIRED_SCOPES = [ "chat:write", @@ -248,30 +253,28 @@ describe("a connected workspace", () => { }, 30000); it("does not offer a connection check the API is bound to refuse", async () => { - // Given — a workspace connected and no destination channel recorded. + // Given — a workspace connected and no channels authorized yet. const harness = new SlackIntegrationHarness(connectedSlackFixture()); await harness.mount(); - // The check posts to the destination channel, so with none recorded the API - // answers 400 rather than `connected: false`. + // The check posts to the authorized channels, so with none the API answers + // 400 rather than `connected: false`. expect(await harness.offersConnectionTest()).toBe(false); // And — the control itself says what unblocks it. - expect(harness.connectionCheckBlockedReason()).toMatch( - /destination channel/i, - ); + expect(harness.connectionCheckHint()).toMatch(/destination channel/i); }, 30000); }); -describe("choosing a destination channel", () => { - it("offers the workspace's channels and remembers the one chosen", async () => { +describe("authorizing destination channels", () => { + it("offers the workspace's channels and remembers the several authorized", async () => { // Given — a connected tenant whose channels span two cursor pages. const harness = new SlackIntegrationHarness(connectedSlackFixture()); await harness.mount(); // Then — every channel is offered, so the picker followed `links.next` - // rather than stopping at the first page (design D6). Alphabetically: the - // picker sorts, so the API's page order is not the offered order. + // rather than stopping at the first page. Alphabetically: the picker + // sorts, so the API's page order is not the offered order. expect(await harness.channelOptions()).toEqual([ SLACK_SECOND_PUBLIC_CHANNEL.name, SLACK_PUBLIC_CHANNEL.name, @@ -279,23 +282,60 @@ describe("choosing a destination channel", () => { ]); expect(harness.channelListCallCount).toBe(2); - // When - await harness.chooseChannel(SLACK_PUBLIC_CHANNEL.name); + // When — more than one channel is authorized in a single save. + await harness.authorizeChannels([ + SLACK_PUBLIC_CHANNEL.name, + SLACK_PRIVATE_CHANNEL.name, + ]); - // Then — only the id is submitted: the API derives the name from it. + // Then — objects naming nothing but the id: the API derives each name and + // its privacy from Slack itself. const saved = await harness.lastRequestBody( "PATCH", "/integrations/", ); - expect(saved?.data.attributes.configuration).toEqual({ - channel_id: SLACK_PUBLIC_CHANNEL.id, - }); + const written = saved?.data.attributes.configuration.channels ?? []; + expect(written).toHaveLength(2); + expect(written).toEqual( + expect.arrayContaining([ + { id: SLACK_PUBLIC_CHANNEL.id }, + { id: SLACK_PRIVATE_CHANNEL.id }, + ]), + ); - // And — a later visit shows it, under the name the API derived from the id. + // And — a later visit shows the set, under the names the API derived. await harness.revisit(); - expect(await harness.defaultChannel()).toBe(SLACK_PUBLIC_CHANNEL.name); + expect(await harness.authorizedChannels()).toEqual([ + SLACK_PUBLIC_CHANNEL.name, + SLACK_PRIVATE_CHANNEL.name, + ]); }, 60000); + it("keeps a private channel identified on its chip with the listing closed", async () => { + // Given — an authorized set holding a private channel next to a public + // one, as a later visit reads it back. + const harness = new SlackIntegrationHarness( + slackFixtureWithAuthorizedChannels([ + SLACK_PUBLIC_CHANNEL, + SLACK_PRIVATE_CHANNEL, + ]), + ); + + // When — nothing but opening the page: the listing stays closed. + await harness.mount(); + + // Then — the chips carry the marking, not only the open listing's rows. + const chips = await harness.authorizedChannelChips(); + expect(chips).toContainEqual({ + name: SLACK_PRIVATE_CHANNEL.name, + isPrivate: true, + }); + expect(chips).toContainEqual({ + name: SLACK_PUBLIC_CHANNEL.name, + isPrivate: false, + }); + }, 30000); + it("narrows the offered channels as the user types", async () => { // Given — a connected workspace whose channels were read. const harness = new SlackIntegrationHarness(connectedSlackFixture()); @@ -313,7 +353,7 @@ describe("choosing a destination channel", () => { expect(none.emptyNote).toMatch(/No channel matches/); }, 60000); - it("offers a private channel the app was invited to, marked as private, and saves it", async () => { + it("offers a private channel the app was invited to, marked as private, and authorizes it", async () => { // Given — `@Prowler` was invited to one private channel; `groups:read` is // membership-gated (D2). const harness = new SlackIntegrationHarness(connectedSlackFixture()); @@ -331,10 +371,12 @@ describe("choosing a destination channel", () => { ).toBe(false); // When - await harness.chooseChannel(SLACK_PRIVATE_CHANNEL.name); + await harness.authorizeChannels([SLACK_PRIVATE_CHANNEL.name]); // Then - expect(await harness.defaultChannel()).toBe(SLACK_PRIVATE_CHANNEL.name); + expect(await harness.authorizedChannels()).toEqual([ + SLACK_PRIVATE_CHANNEL.name, + ]); }, 60000); it("offers a private channel once @Prowler is invited to it and the list is refreshed", async () => { @@ -380,23 +422,21 @@ describe("choosing a destination channel", () => { const message = await harness.channelPickerMessage(); expect(message).toMatch(/No channels available yet/); expect(message).toMatch(/invite @Prowler/); - expect(await harness.defaultChannel()).toBeNull(); + expect(await harness.authorizedChannels()).toEqual([]); expect(await harness.offersConnectionTest()).toBe(false); }, 30000); - it("checks the connection itself as soon as the destination is saved", async () => { - // Given — connected with nothing recorded: the check posts to the - // destination, so it is not offered yet. + it("checks the connection itself as soon as the channels are saved", async () => { + // Given — connected with nothing authorized: the check posts to the set, + // so it is not offered yet. const harness = new SlackIntegrationHarness(connectedSlackFixture()); await harness.mount(); expect(await harness.offersConnectionTest()).toBe(false); - expect(harness.connectionCheckBlockedReason()).toMatch( - /destination channel/i, - ); + expect(harness.connectionCheckHint()).toMatch(/destination channel/i); expect(harness.connectionCheckCallCount).toBe(0); // When - await harness.chooseChannel(SLACK_PUBLIC_CHANNEL.name); + await harness.authorizeChannels([SLACK_PUBLIC_CHANNEL.name]); // Then expect(await harness.connectionOutcome()).toBe(CONNECTION_OUTCOME.SUCCESS); @@ -404,55 +444,81 @@ describe("choosing a destination channel", () => { // And — everything waiting on a destination moves with the save, in the // same paint: no reload to find the check on offer for later. expect(await harness.offersConnectionTest()).toBe(true); - expect(harness.connectionCheckBlockedReason()).toBeNull(); + expect(harness.connectionCheckHint()).toMatch(/every authorized channel/); + // And — one confirmation per channel (design D7), not a broadcast test message. + expect(harness.connectionCheckHint()).not.toMatch(/test message/i); }, 60000); - it("reports a saved destination the check cannot reach, without losing the save", async () => { - // Given — a channel the API records, then refuses to reach: the bot is not - // in it, which only the check finds out. + it("reports a saved destination the check cannot reach, naming the channel, without losing the save", async () => { + // Given — channels the API records, then refuses to reach one of: the bot is + // not in it, and only the check can say which channel that was (design D7). const harness = new SlackIntegrationHarness( connectedSlackFixture({ - connection: { connected: false, error: SLACK_NOT_IN_CHANNEL_CODE }, + connection: { + connected: false, + error: SLACK_NOT_IN_CHANNEL_CODE, + failedChannelId: SLACK_PRIVATE_CHANNEL.id, + }, }), ); await harness.mount(); // When - await harness.chooseChannel(SLACK_PUBLIC_CHANNEL.name); + await harness.authorizeChannels([ + SLACK_PUBLIC_CHANNEL.name, + SLACK_PRIVATE_CHANNEL.name, + ]); - // Then — only the check failed, so the destination stays on record. + // Then — only the check failed, so the destinations stay on record, and + // the failure names the one channel Slack refused. expect(await harness.connectionOutcome()).toBe(CONNECTION_OUTCOME.FAILURE); - expect(await harness.defaultChannel()).toBe(SLACK_PUBLIC_CHANNEL.name); + expect(await harness.connectionFailureToast()).toMatch( + new RegExp(`Slack refused #${SLACK_PRIVATE_CHANNEL.name}`), + ); + expect(await harness.authorizedChannels()).toEqual([ + SLACK_PUBLIC_CHANNEL.name, + SLACK_PRIVATE_CHANNEL.name, + ]); expect(await harness.offersConnectionTest()).toBe(true); + + // And — the confirmation is stamped only where Slack accepted it, so the + // next check has the refused channel alone left to confirm. + await harness.refreshPageData(); + const hint = await harness.connectionCheckHintMatching( + channelMention(SLACK_PRIVATE_CHANNEL.name), + ); + expect(hint).not.toMatch(channelMention(SLACK_PUBLIC_CHANNEL.name)); }, 60000); - it("follows the destination recorded elsewhere when the page's data refreshes under it", async () => { + it("follows the set recorded elsewhere when the page's data refreshes under it", async () => { // Given — a finished setup, open on screen. const harness = new SlackIntegrationHarness(configuredSlackFixture()); await harness.mount(); - expect(await harness.defaultChannel()).toBe(SLACK_PUBLIC_CHANNEL.name); + expect(await harness.authorizedChannels()).toEqual([ + SLACK_PUBLIC_CHANNEL.name, + ]); - // When — the destination changes elsewhere (a second tab, another user) and - // this page's server data refreshes under the open card, as - // `revalidatePath` does after an action. - await harness.channelRecordedElsewhere(SLACK_SECOND_PUBLIC_CHANNEL.name); + // When — the set changes elsewhere (a second tab, another user) and this + // page's server data refreshes under the open card, as `revalidatePath` + // does after an action. + await harness.channelsRecordedElsewhere([SLACK_SECOND_PUBLIC_CHANNEL.name]); await harness.refreshPageData(); // Then — the card reports what is on record, not the copy it took at mount. - expect(await harness.defaultChannel()).toBe( + expect(await harness.authorizedChannels()).toEqual([ SLACK_SECOND_PUBLIC_CHANNEL.name, - ); + ]); expect(await harness.offersConnectionTest()).toBe(true); - // And — the picker followed too: the superseded destination is not left one - // click from being saved back. - expect(harness.offersChannelSave()).toBe(false); + // And — the picker followed too: the superseded set is not left one click + // from being saved back. + expect(harness.offersChannelsSave()).toBe(false); }, 60000); - it("says which permission is missing when Slack refuses the channel listing, leaving the recorded channel alone", async () => { - // Given — a recorded destination, and an install missing a scope the listing + it("says which permission is missing when Slack refuses the channel listing, leaving the authorized set alone", async () => { + // Given — an authorized set, and an install missing a scope the listing // needs. The API names it in `code` (contract, Errors), not in `detail`. const harness = new SlackIntegrationHarness( - slackFixtureWithDefaultChannel(SLACK_PUBLIC_CHANNEL, { + slackFixtureWithAuthorizedChannels([SLACK_PUBLIC_CHANNEL], { channelsRefusal: SLACK_MISSING_SCOPE_REFUSAL, }), ); @@ -470,9 +536,11 @@ describe("choosing a destination channel", () => { expect(message).not.toMatch(SLACK_MISSING_SCOPE_CODE); expect(harness.channelInviteHint()).toMatch(/invites @Prowler/); - // And — a listing Prowler could not read says nothing about the channel - // already recorded. - expect(await harness.defaultChannel()).toBe(SLACK_PUBLIC_CHANNEL.name); + // And — a listing Prowler could not read says nothing about the channels + // already authorized. + expect(await harness.authorizedChannels()).toEqual([ + SLACK_PUBLIC_CHANNEL.name, + ]); expect(await harness.offersConnectionTest()).toBe(true); }, 30000); @@ -480,7 +548,7 @@ describe("choosing a destination channel", () => { // Given — `conversations.list` is Slack tier 2 and paginated (contract, // Errors); the `429` carries the wait in `Retry-After`. const harness = new SlackIntegrationHarness( - slackFixtureWithDefaultChannel(SLACK_PUBLIC_CHANNEL, { + slackFixtureWithAuthorizedChannels([SLACK_PUBLIC_CHANNEL], { channelsRefusal: SLACK_RATE_LIMITED_REFUSAL, }), ); @@ -496,7 +564,9 @@ describe("choosing a destination channel", () => { // And — waiting is the fix, so nothing is said about permissions. expect(message).not.toMatch(/permission/); - expect(await harness.defaultChannel()).toBe(SLACK_PUBLIC_CHANNEL.name); + expect(await harness.authorizedChannels()).toEqual([ + SLACK_PUBLIC_CHANNEL.name, + ]); }, 30000); it("keeps the channels it did read on offer when Slack refuses a later page", async () => { @@ -521,8 +591,10 @@ describe("choosing a destination channel", () => { expect(notice).toMatch(/rate limiting/); expect(notice).toMatch(/about 30 seconds/); - // And — a partial read says nothing about the destination already recorded. - expect(await harness.defaultChannel()).toBe(SLACK_PUBLIC_CHANNEL.name); + // And — a partial read says nothing about the channels already authorized. + expect(await harness.authorizedChannels()).toEqual([ + SLACK_PUBLIC_CHANNEL.name, + ]); expect(await harness.offersConnectionTest()).toBe(true); }, 60000); @@ -541,7 +613,7 @@ describe("choosing a destination channel", () => { // Given — a `502`, which names no `code` because there is nothing to act on // (contract, Errors). const harness = new SlackIntegrationHarness( - slackFixtureWithDefaultChannel(SLACK_PUBLIC_CHANNEL, { + slackFixtureWithAuthorizedChannels([SLACK_PUBLIC_CHANNEL], { channelsRefusal: SLACK_UPSTREAM_REFUSAL, }), ); @@ -553,12 +625,14 @@ describe("choosing a destination channel", () => { const message = await harness.channelPickerMessage(); expect(message).toMatch(/Slack is temporarily unavailable/); expect(message).not.toMatch(/rate limiting/); - expect(await harness.defaultChannel()).toBe(SLACK_PUBLIC_CHANNEL.name); + expect(await harness.authorizedChannels()).toEqual([ + SLACK_PUBLIC_CHANNEL.name, + ]); }, 30000); - it("says to invite @Prowler when Slack refuses the channel because the app is not in it", async () => { + it("says to invite @Prowler when Slack refuses a channel because the app is not in it", async () => { // Given — a private channel the app was removed from. The API validates the - // channel against Slack on the way in and refuses with `not_in_channel`. + // set against Slack on the way in and refuses with `not_in_channel`. const harness = new SlackIntegrationHarness( connectedSlackFixture({ channelSaveRefusal: SLACK_NOT_IN_CHANNEL_REFUSAL, @@ -567,9 +641,9 @@ describe("choosing a destination channel", () => { await harness.mount(); // When - const refusal = await harness.refusedChannelSave( + const refusal = await harness.refusedChannelsSave([ SLACK_PRIVATE_CHANNEL.name, - ); + ]); // Then — the one fix the user can carry out themselves, in Slack. expect(refusal).toMatch(/Prowler is not in that channel/); @@ -577,7 +651,7 @@ describe("choosing a destination channel", () => { expect(refusal).not.toMatch(SLACK_NOT_IN_CHANNEL_CODE); // And — nothing was recorded, so there is still nothing to check against. - expect(await harness.defaultChannel()).toBeNull(); + expect(await harness.authorizedChannels()).toEqual([]); expect(await harness.offersConnectionTest()).toBe(false); }, 60000); @@ -593,7 +667,9 @@ describe("choosing a destination channel", () => { await harness.mount(); // When - const refusal = await harness.refusedChannelSave(SLACK_PUBLIC_CHANNEL.name); + const refusal = await harness.refusedChannelsSave([ + SLACK_PUBLIC_CHANNEL.name, + ]); // Then — a different problem, so different copy: nothing to invite to a // channel that no longer exists. @@ -601,7 +677,179 @@ describe("choosing a destination channel", () => { expect(refusal).toMatch(/Choose another one/); expect(refusal).not.toMatch(/Invite @Prowler/); expect(refusal).not.toMatch(SLACK_UNKNOWN_CHANNEL_DETAIL); - expect(await harness.defaultChannel()).toBeNull(); + expect(await harness.authorizedChannels()).toEqual([]); + }, 60000); + + it("promises the confirmation only to the channels that have not had one", async () => { + // Given — one authorized channel, and a check that already confirmed it. + const harness = new SlackIntegrationHarness(configuredSlackFixture()); + await harness.mount(); + + // Then — checking again posts nothing: the confirmation is one-time + // (design D7), not a fresh message every run. + expect(harness.connectionCheckHint()).toMatch(/nothing is posted/); + expect(harness.connectionCheckHint()).not.toMatch(/test message/i); + + // When — a second channel is authorized. + await harness.authorizeChannels([SLACK_SECOND_PUBLIC_CHANNEL.name]); + + // Then — the copy names the newly authorized channel alone, and says what + // will land in it. + const hint = await harness.connectionCheckHintMatching( + channelMention(SLACK_SECOND_PUBLIC_CHANNEL.name), + ); + expect(hint).toMatch(channelMention(SLACK_SECOND_PUBLIC_CHANNEL.name)); + expect(hint).not.toMatch(channelMention(SLACK_PUBLIC_CHANNEL.name)); + expect(hint).toMatch( + /✅ Prowler connection verified\. Notifications will be delivered to this channel\./, + ); + + // And — once the check the save ran has landed, there is nothing left to + // confirm at all. + expect(await harness.connectionOutcome()).toBe(CONNECTION_OUTCOME.SUCCESS); + await harness.refreshPageData(); + expect( + await harness.connectionCheckHintMatching(/nothing is posted/), + ).toMatch(/every authorized channel/); + }, 60000); + + it("warns that dropping a channel drops it from the alert rules too, before saving", async () => { + // Given — two channels authorized, either of which an alert rule may target. + const harness = new SlackIntegrationHarness( + slackFixtureWithAuthorizedChannels([ + SLACK_PUBLIC_CHANNEL, + SLACK_PRIVATE_CHANNEL, + ]), + ); + await harness.mount(); + expect(harness.deauthorizationWarning()).toBeNull(); + + // When — one is deselected and nothing is saved yet. + await harness.chooseChannels([SLACK_PRIVATE_CHANNEL.name]); + + // Then — what the save would do server-side, said where it is decided + // (design D11): the rules lose the channel, delivery stops, history stays. + const warning = harness.deauthorizationWarning() ?? ""; + expect(warning).toMatch(channelMention(SLACK_PRIVATE_CHANNEL.name)); + expect(warning).not.toMatch(channelMention(SLACK_PUBLIC_CHANNEL.name)); + expect(warning).toMatch(/alert rule/i); + expect(warning).toMatch(/stops delivering/); + expect(warning).toMatch(/already delivered stay in Slack/); + + // And — it belongs to the pending selection, not to the record: putting the + // channel back takes it away. + await harness.chooseChannels([SLACK_PRIVATE_CHANNEL.name]); + expect(harness.deauthorizationWarning()).toBeNull(); + + // When — the removal is saved after all. + await harness.deauthorizeChannels([SLACK_PRIVATE_CHANNEL.name]); + + // Then — the record follows and the warning has nothing left to say. + expect(await harness.authorizedChannels()).toEqual([ + SLACK_PUBLIC_CHANNEL.name, + ]); + expect(harness.deauthorizationWarning()).toBeNull(); + }, 60000); + + it("keeps a pick made after a save when the page's data refreshes under it", async () => { + // Given — one authorized channel, and a second authorized from this page. + const harness = new SlackIntegrationHarness(configuredSlackFixture()); + await harness.mount(); + await harness.authorizeChannels([SLACK_SECOND_PUBLIC_CHANNEL.name]); + + // When — the user changes their mind about the channel just saved, and the + // save's revalidation lands while that new pick is still unsaved. + await harness.chooseChannels([SLACK_SECOND_PUBLIC_CHANNEL.name]); + await harness.refreshPageData(); + + // Then — the refresh carries the record this page just saved, so it is no + // reason to overwrite the pick taken since: the drop and its warning stand. + expect( + (await harness.authorizedChannelChips()).map((chip) => chip.name), + ).toEqual([SLACK_PUBLIC_CHANNEL.name]); + const warning = harness.deauthorizationWarning() ?? ""; + expect(warning).toMatch(channelMention(SLACK_SECOND_PUBLIC_CHANNEL.name)); + }, 60000); + + it("clears the authorized set when the last channel is dropped", async () => { + // Given — a single authorized channel. + const harness = new SlackIntegrationHarness(configuredSlackFixture()); + await harness.mount(); + + // When + await harness.deauthorizeChannels([SLACK_PUBLIC_CHANNEL.name]); + + // Then — an empty list clears the set, where an omitted one would leave it + // untouched (contract, PATCH). + const saved = await harness.lastRequestBody( + "PATCH", + "/integrations/", + ); + expect(saved?.data.attributes.configuration.channels).toEqual([]); + expect(await harness.authorizedChannels()).toEqual([]); + + // And — with nothing to post to, the check is not offered and says why. + expect(await harness.offersConnectionTest()).toBe(false); + expect(harness.connectionCheckHint()).toMatch( + /Authorize at least one destination channel/, + ); + }, 60000); + + it("keeps the connection when the same channels are recorded in another order", async () => { + // Given — two channels authorized and a connection checked against them. + const harness = new SlackIntegrationHarness( + slackFixtureWithAuthorizedChannels([ + SLACK_PUBLIC_CHANNEL, + SLACK_SECOND_PUBLIC_CHANNEL, + ]), + ); + await harness.mount(); + expect(await harness.connectionBadge()).toBe("Connected"); + + // When — the same ids are written back in the other order. + await harness.channelsRecordedElsewhere([ + SLACK_SECOND_PUBLIC_CHANNEL.name, + SLACK_PUBLIC_CHANNEL.name, + ]); + await harness.refreshPageData(); + + // Then — a reorder is not a changed set, so the check that ran still + // covers it (contract, PATCH). + expect(await harness.connectionBadge()).toBe("Connected"); + + // When — the set itself changes. + await harness.channelsRecordedElsewhere([SLACK_PUBLIC_CHANNEL.name]); + await harness.refreshPageData(); + + // Then — no check has covered this set, so the card claims no connection. + expect(await harness.connectionBadge()).toBe("Not checked yet"); + }, 60000); + + it("keeps the authorized channels through a reinstall, with their confirmations to run again", async () => { + // Given — a finished setup whose channel an earlier check confirmed. + const harness = new SlackIntegrationHarness(configuredSlackFixture()); + await harness.mount(); + expect(harness.connectionCheckHint()).toMatch(/nothing is posted/); + + // When — the same workspace is approved again. + await harness.mountCallback({ + code: SLACK_OAUTH_CODE, + state: SLACK_OAUTH_STATE, + }); + expect(await harness.completedInstall()).toBe(true); + await harness.revisit(); + + // Then — a same-workspace reinstall keeps the channels and resets every + // confirmation with the connection state (contract, OAuth and reads). + expect(await harness.authorizedChannels()).toEqual([ + SLACK_PUBLIC_CHANNEL.name, + ]); + expect(await harness.connectionBadge()).toBe("Not checked yet"); + expect( + await harness.connectionCheckHintMatching( + channelMention(SLACK_PUBLIC_CHANNEL.name), + ), + ).toMatch(/Prowler connection verified/); }, 60000); }); @@ -808,10 +1056,10 @@ describe("a credential Slack no longer accepts", () => { expect(harness.showsRevokedCredentialNotice()).toBe(true); expect(await harness.connectionBadge()).toBe("Disconnected"); - // When — the access is approved again in Slack and the user saves a - // destination: both the save and the check it runs answer for the grant. + // When — the access is approved again in Slack and the user saves a wider + // set: both the save and the check it runs answer for the grant. harness.fixture.connection = { connected: true, error: null }; - await harness.chooseChannel(SLACK_SECOND_PUBLIC_CHANNEL.name); + await harness.authorizeChannels([SLACK_SECOND_PUBLIC_CHANNEL.name]); // Then — Slack answered, so the notice about a credential it no longer // accepts goes, and the card is back to what it reported on arrival. diff --git a/ui/changelog.d/slack-authorized-channels.added.md b/ui/changelog.d/slack-authorized-channels.added.md new file mode 100644 index 0000000000..b1f68dfa96 --- /dev/null +++ b/ui/changelog.d/slack-authorized-channels.added.md @@ -0,0 +1 @@ +Slack integration: authorize several destination channels at once — the connection check confirms each authorized channel with a one-time message and names the one Slack refuses diff --git a/ui/components/integrations/slack/slack-callback.tsx b/ui/components/integrations/slack/slack-callback.tsx index fa3ed36a0d..b20f5c82f8 100644 --- a/ui/components/integrations/slack/slack-callback.tsx +++ b/ui/components/integrations/slack/slack-callback.tsx @@ -137,8 +137,8 @@ export const SlackCallback = () => { Connected to {workspaceName ?? "your Slack workspace"} - Taking you back to the Slack integration, where you can choose the - channel Prowler posts to. + Taking you back to the Slack integration, where you can authorize the + channels Prowler posts to. ); diff --git a/ui/components/integrations/slack/slack-channel-multi-select.tsx b/ui/components/integrations/slack/slack-channel-multi-select.tsx new file mode 100644 index 0000000000..2abfcb0d25 --- /dev/null +++ b/ui/components/integrations/slack/slack-channel-multi-select.tsx @@ -0,0 +1,162 @@ +"use client"; + +import { Lock, RefreshCw } from "lucide-react"; + +import { + Alert, + AlertDescription, + AlertTitle, + Badge, + Button, + Label, +} from "@/components/shadcn"; +import { + MultiSelect, + MultiSelectContent, + MultiSelectItem, + MultiSelectTrigger, + MultiSelectValue, +} from "@/components/shadcn/select/multiselect"; +import type { SlackChannelOption } from "@/types/integrations"; + +const INVITE_HINT = + "A private channel only appears here after someone invites @Prowler to it in Slack. Invite it, then refresh."; + +interface SlackChannelMultiSelectProps { + options: SlackChannelOption[]; + values: string[]; + onChange: (channelIds: string[]) => void; + isLoading?: boolean; + /** Why the channels could not be read — Slack's own reason, when it gave one. */ + error?: string | null; + /** Why the list is partial. Shown with the picker, not instead of it. */ + incompleteNotice?: string | null; + onRefresh?: () => void; + disabled?: boolean; + id?: string; +} + +const chipLabel = (option: SlackChannelOption) => ( + + {option.is_private && ( + <> + +); + +/** Driven entirely by props (design D1) so any consumer can reuse it. */ +export const SlackChannelMultiSelect = ({ + options, + values, + onChange, + isLoading = false, + error = null, + incompleteNotice = null, + onRefresh, + disabled = false, + id = "slack-channels", +}: SlackChannelMultiSelectProps) => { + const isEmpty = !isLoading && !error && options.length === 0; + // `htmlFor` may only name an element that exists, and the trigger is only + // rendered in the picker branch. + const hasPicker = !error && !isEmpty; + + // A copy: the list belongs to the caller. Sorted here rather than upstream so + // every consumer of the picker offers the same order. + const listed = [...options].sort((left, right) => + left.name.localeCompare(right.name, undefined, { + sensitivity: "base", + numeric: true, + }), + ); + + return ( +
+
+ + {onRefresh && ( + + )} +
+ + {error ? ( + + Could not read the workspace's channels + {error} + + ) : isEmpty ? ( + + No channels available yet + + Prowler cannot see a single channel in this workspace. Create a + public channel, or invite @Prowler to a private one in Slack with + /invite @Prowler, then + refresh. + + + ) : ( + <> + {incompleteNotice && ( + + Not every channel is listed + {incompleteNotice} + + )} + + + + + + {listed.map((option) => ( + + #{option.name} + {option.is_private && ( + + Private + + )} + + ))} + + + + )} + +

{INVITE_HINT}

+
+ ); +}; diff --git a/ui/components/integrations/slack/slack-channel-selector.tsx b/ui/components/integrations/slack/slack-channel-selector.tsx deleted file mode 100644 index cfa326cc79..0000000000 --- a/ui/components/integrations/slack/slack-channel-selector.tsx +++ /dev/null @@ -1,195 +0,0 @@ -"use client"; - -import { ChevronDown, RefreshCw } from "lucide-react"; -import { useState } from "react"; - -import { - Alert, - AlertDescription, - AlertTitle, - Badge, - Button, - Label, -} from "@/components/shadcn"; -import { - Command, - CommandEmpty, - CommandGroup, - CommandInput, - CommandItem, - CommandList, -} from "@/components/shadcn/command"; -import { - Popover, - PopoverContent, - PopoverTrigger, -} from "@/components/shadcn/popover"; -import type { SlackChannelOption } from "@/types/integrations"; - -const INVITE_HINT = - "A private channel only appears here after someone invites @Prowler to it in Slack. Invite it, then refresh."; - -interface SlackChannelSelectorProps { - options: SlackChannelOption[]; - value: string | null; - onChange: (channelId: string) => void; - isLoading?: boolean; - /** Why the channels could not be read — Slack's own reason, when it gave one. */ - error?: string | null; - /** Why the list is partial. Shown with the picker, not instead of it. */ - incompleteNotice?: string | null; - onRefresh?: () => void; - disabled?: boolean; -} - -/** Driven entirely by props (design D13) so the alert-rule form can reuse it. */ -export const SlackChannelSelector = ({ - options, - value, - onChange, - isLoading = false, - error = null, - incompleteNotice = null, - onRefresh, - disabled = false, -}: SlackChannelSelectorProps) => { - const [isOpen, setIsOpen] = useState(false); - const [query, setQuery] = useState(""); - - const isEmpty = !isLoading && !error && options.length === 0; - // `htmlFor` may only name an element that exists, and the trigger is only - // rendered in the picker branch. - const hasPicker = !error && !isEmpty; - - // A copy: the list belongs to the caller. Sorted here rather than upstream so - // every consumer of the picker offers the same order. - const listed = [...options].sort((left, right) => - left.name.localeCompare(right.name, undefined, { - sensitivity: "base", - numeric: true, - }), - ); - const selected = options.find((option) => option.id === value) ?? null; - - const handleOpenChange = (open: boolean) => { - setIsOpen(open); - // Drop the search with the popover, so re-opening it never starts filtered. - if (!open) setQuery(""); - }; - - const handleSelect = (channelId: string) => { - onChange(channelId); - handleOpenChange(false); - }; - - return ( -
-
- - {onRefresh && ( - - )} -
- - {error ? ( - - Could not read the workspace's channels - {error} - - ) : isEmpty ? ( - - No channels available yet - - Prowler cannot see a single channel in this workspace. Create a - public channel, or invite @Prowler to a private one in Slack with - /invite @Prowler, then - refresh. - - - ) : ( - <> - {incompleteNotice && ( - - Not every channel is listed - {incompleteNotice} - - )} - - - - - - - - - {/* A workspace with no channels at all is a different - situation, answered by the alert above. */} - No channel matches that search. - - {listed.map((option) => ( - handleSelect(option.id)} - // Name hook: the rendered label mixes it with a - // "Private" badge. - data-channel={option.name} - > - #{option.name} - {option.is_private && ( - - Private - - )} - - ))} - - - - - - - )} - -

{INVITE_HINT}

-
- ); -}; diff --git a/ui/components/integrations/slack/slack-integration-card.tsx b/ui/components/integrations/slack/slack-integration-card.tsx index ba21c82170..a4e8397afb 100644 --- a/ui/components/integrations/slack/slack-integration-card.tsx +++ b/ui/components/integrations/slack/slack-integration-card.tsx @@ -45,8 +45,8 @@ export const SlackIntegrationCard = () => {

- Connect a Slack workspace and pick the channel Prowler posts to, so - your team gets security updates where it already works. + Connect a Slack workspace and authorize the channels Prowler posts to, + so your team gets security updates where it already works.

diff --git a/ui/components/integrations/slack/slack-integration-manager.test.tsx b/ui/components/integrations/slack/slack-integration-manager.test.tsx index b5f011ae75..57ed73973c 100644 --- a/ui/components/integrations/slack/slack-integration-manager.test.tsx +++ b/ui/components/integrations/slack/slack-integration-manager.test.tsx @@ -14,7 +14,7 @@ import { SlackIntegrationManager } from "./slack-integration-manager"; vi.mock("@/actions/integrations/slack", () => ({ getSlackChannels: vi.fn(), - setSlackDefaultChannel: vi.fn(), + setSlackAuthorizedChannels: vi.fn(), })); vi.mock("@/actions/integrations/integrations", () => ({ diff --git a/ui/components/integrations/slack/slack-integration-manager.tsx b/ui/components/integrations/slack/slack-integration-manager.tsx index 9424105c1d..66c6e9c9f7 100644 --- a/ui/components/integrations/slack/slack-integration-manager.tsx +++ b/ui/components/integrations/slack/slack-integration-manager.tsx @@ -9,11 +9,11 @@ import { disconnectSlackIntegration, getSlackAuthorizeUrl, getSlackChannels, - setSlackDefaultChannel, + setSlackAuthorizedChannels, } from "@/actions/integrations/slack"; import { SlackIcon } from "@/components/icons/services/IconServices"; import { IntegrationCardHeader } from "@/components/integrations/shared"; -import { SlackChannelSelector } from "@/components/integrations/slack/slack-channel-selector"; +import { SlackChannelMultiSelect } from "@/components/integrations/slack/slack-channel-multi-select"; import { Alert, AlertDescription, @@ -33,6 +33,7 @@ import { import type { SlackTokenErrorCode } from "@/lib/integrations/slack-errors"; import type { IntegrationProps, + SlackAuthorizedChannel, SlackChannelOption, } from "@/types/integrations"; @@ -60,7 +61,11 @@ interface ChannelsLoaded { type ChannelsState = ChannelsLoading | ChannelsFailed | ChannelsLoaded; -const CHECK_BLOCKED_REASON_ID = "slack-connection-check-blocked"; +const CHECK_HINT_ID = "slack-connection-check-hint"; + +/** The exact text the check posts (contract, Connection); the hint quotes it. */ +const CONFIRMATION_MESSAGE = + "✅ Prowler connection verified. Notifications will be delivered to this channel."; /** * A disconnect that removed the row without Slack confirming the revocation. @@ -71,16 +76,63 @@ interface UnconfirmedRevocation { workspace: string | null; } -// The name may be missing: the id decides what the UI can do with it. -interface SlackChannelRef { - id: string; - name: string | null; -} +/** Order-insensitive: the mirror must not re-seed on a mere reordering. */ +const sameChannelIds = (a: string[], b: string[]) => + a.length === b.length && new Set([...a, ...b]).size === a.length; -const channelRefEquals = ( - a: SlackChannelRef | null, - b: SlackChannelRef | null, -) => a?.id === b?.id && a?.name === b?.name; +/** Confirmation is part of the record: a confirm-only change must re-seed. */ +const sameChannelSets = ( + a: SlackAuthorizedChannel[], + b: SlackAuthorizedChannel[], +) => { + if (a.length !== b.length) return false; + const byId = new Map(a.map((channel) => [channel.id, channel])); + return b.every((channel) => { + const other = byId.get(channel.id); + return ( + other?.name === channel.name && + other?.confirmation_sent_at === channel.confirmation_sent_at + ); + }); +}; + +/** + * A stored channel the listing no longer carries stays selectable: dropping it + * would deselect it behind the user's back. + */ +const mergeChannelOptions = ( + listed: SlackChannelOption[], + stored: SlackAuthorizedChannel[], +): SlackChannelOption[] => { + const merged = new Map(listed.map((channel) => [channel.id, channel])); + stored.forEach(({ id, name, is_private }) => { + if (!merged.has(id)) merged.set(id, { id, name, is_private }); + }); + return Array.from(merged.values()); +}; + +/** + * Fallback for a save that answered without channels. Retained ids keep their + * confirmation, new ones start without one (contract, PATCH). + */ +const recordedFromSelection = ( + channelIds: string[], + options: SlackChannelOption[], + previous: SlackAuthorizedChannel[], +): SlackAuthorizedChannel[] => + channelIds.flatMap((channelId) => { + const option = options.find((channel) => channel.id === channelId); + if (!option) return []; + const stored = previous.find((channel) => channel.id === channelId); + return [ + { ...option, confirmation_sent_at: stored?.confirmation_sent_at ?? null }, + ]; + }); + +const channelList = (names: string[]): string => + new Intl.ListFormat("en", { style: "long", type: "conjunction" }).format( + names.map((name) => `#${name}`), + ); /** * Slack's own reason, when the string is one: the connection check reports a @@ -129,14 +181,8 @@ export const SlackIntegrationManager = ({ const { toast } = useToast(); const integrationId = integration?.id ?? null; - const recordedChannelId = - integration?.attributes.configuration.channel_id ?? null; - const recordedChannelName = - integration?.attributes.configuration.channel_name ?? null; - - const recordedChannel: SlackChannelRef | null = recordedChannelId - ? { id: recordedChannelId, name: recordedChannelName } - : null; + const recordedChannels: SlackAuthorizedChannel[] = + integration?.attributes.configuration.channels ?? []; // Seeded `loading`, not by the effect: the effect never runs on the server, // so anything else would server-render a "no channels" picker until @@ -148,26 +194,29 @@ export const SlackIntegrationManager = ({ ); // Bumped by refresh: a channel invited after load only shows on a re-read. const [channelReloads, setChannelReloads] = useState(0); - // Local state needed: the pick is buffered until the user saves it. - const [selectedChannelId, setSelectedChannelId] = useState( - recordedChannelId, + // Local state needed: the picks are buffered until the user saves them. + const [selectedChannelIds, setSelectedChannelIds] = useState( + recordedChannels.map((channel) => channel.id), ); // Mirrored in state, not read from the prop, so channel-gated affordances // move on save instead of waiting for the revalidation. - const [defaultChannel, setDefaultChannel] = useState(recordedChannel); + const [authorizedChannels, setAuthorizedChannels] = + useState(recordedChannels); // The prop the mirror was last taken from: the card never unmounts, so a // mirror seeded only at mount would go stale when the record changes. - const [syncedChannel, setSyncedChannel] = useState(recordedChannel); - const [isSavingChannel, setIsSavingChannel] = useState(false); + const [syncedChannels, setSyncedChannels] = useState(recordedChannels); + const [isSavingChannels, setIsSavingChannels] = useState(false); - if (!channelRefEquals(recordedChannel, syncedChannel)) { - const previousSyncedId = syncedChannel?.id ?? null; - setSyncedChannel(recordedChannel); - setDefaultChannel(recordedChannel); - // Follow the record only while the buffered pick still matches it: an + if (!sameChannelSets(recordedChannels, syncedChannels)) { + // The baseline is `authorizedChannels`, not `syncedChannels`: a save + // advances the mirror first, so the prop reads a later pick as none. + const shownIds = authorizedChannels.map((channel) => channel.id); + setSyncedChannels(recordedChannels); + setAuthorizedChannels(recordedChannels); + // Follow the record only while the buffered picks still match it: an // unsaved pick is the user's, not ours to overwrite mid-edit. - if (selectedChannelId === previousSyncedId) { - setSelectedChannelId(recordedChannel?.id ?? null); + if (sameChannelIds(selectedChannelIds, shownIds)) { + setSelectedChannelIds(recordedChannels.map((channel) => channel.id)); } } @@ -250,68 +299,102 @@ export const SlackIntegrationManager = ({ }; }, [integrationId, channelReloads]); - const channels = + const listedChannels = channelsState.status === CHANNELS_STATUS.LOADED ? channelsState.channels : []; + const channelOptions = mergeChannelOptions( + listedChannels, + authorizedChannels, + ); - const handleSaveChannel = async () => { - if (!integrationId || !selectedChannelId) return; + // The check posts only where no confirmation has landed (contract, Connection). + const unconfirmedChannels = authorizedChannels.filter( + (channel) => channel.confirmation_sent_at === null, + ); + // Dropping a channel cascades into the alert rules targeting it (design D11). + const droppedChannels = authorizedChannels.filter( + (channel) => !selectedChannelIds.includes(channel.id), + ); - let saved = false; - setIsSavingChannel(true); + const checkHint = (): string => { + if (authorizedChannels.length === 0) { + return "Authorize at least one destination channel below to enable this check."; + } + return unconfirmedChannels.length > 0 + ? `Checks every authorized channel and posts “${CONFIRMATION_MESSAGE}” once to ${channelList( + unconfirmedChannels.map((channel) => channel.name), + )}.` + : "Checks every authorized channel. Each was confirmed once already, so nothing is posted."; + }; + + const handleSaveChannels = async () => { + if (!integrationId) return; + + let savedChannels: SlackAuthorizedChannel[] = []; + setIsSavingChannels(true); try { - // Only the id travels — the API validates it and derives the name - // (design D6). - const result = await setSlackDefaultChannel( + // Only ids travel — the API validates them and derives the names. + const result = await setSlackAuthorizedChannels( integrationId, - selectedChannelId, + selectedChannelIds, ); if ("error" in result) { - // The API validates the channel against Slack, so the save can + // The API validates the channels against Slack, so the save can // discover the credential is gone. recordRefusal(result.code); toast({ variant: "destructive", - title: "Could not save the destination channel", + title: "Could not save the destination channels", description: result.error, }); return; } - // Prefer the API's derived name: a channel renamed in Slack since the - // list was read would otherwise show its old name. - const savedName = - result.integration.attributes.configuration.channel_name ?? - channels.find((channel) => channel.id === selectedChannelId)?.name ?? - null; + // Prefer the API's own record: a channel renamed in Slack since the list + // was read would otherwise show its old name. + savedChannels = + result.integration.attributes.configuration.channels ?? + recordedFromSelection( + selectedChannelIds, + channelOptions, + authorizedChannels, + ); provedCredentialAlive(); - setDefaultChannel({ id: selectedChannelId, name: savedName }); - saved = true; + setAuthorizedChannels(savedChannels); + setSelectedChannelIds(savedChannels.map((channel) => channel.id)); toast({ - title: "Destination channel saved", - description: savedName - ? `Prowler will post to #${savedName}.` - : "Prowler will post to the channel you chose.", + title: "Destination channels saved", + description: + savedChannels.length > 0 + ? `Prowler will post to ${channelList( + savedChannels.map((channel) => channel.name), + )}.` + : "No destination channels are authorized any more.", }); } catch (_error) { toast({ variant: "destructive", - title: "Could not save the destination channel", + title: "Could not save the destination channels", description: "Something went wrong. Please try again.", }); } finally { - setIsSavingChannel(false); + setIsSavingChannels(false); } - // Recording a destination is what makes a check possible (design D7), and - // the save alone only proves the API took the id. - if (saved) await handleTestConnection(integrationId); + // Recording destinations is what makes a check possible (design D7), and + // the save alone only proves the API took the ids. + if (savedChannels.length > 0) + await handleTestConnection(integrationId, savedChannels); }; - const handleTestConnection = async (id: string) => { + const handleTestConnection = async ( + id: string, + // Passed in by a chained check: it runs before the save's state lands. + channels: SlackAuthorizedChannel[] = authorizedChannels, + ) => { setIsTesting(true); try { const result = await testIntegrationConnection(id); @@ -321,7 +404,8 @@ export const SlackIntegrationManager = ({ toast({ title: "Connection test successful!", description: - result.message || "Prowler can reach your Slack workspace.", + result.message || + "Prowler can reach your Slack workspace and every authorized channel.", }); } else { // A dead credential named here is not a failure checking again can @@ -330,12 +414,22 @@ export const SlackIntegrationManager = ({ recordRefusal(asReasonCode(reason)); + const explanation = reason + ? slackErrorMessage({ code: reason, detail: reason }) + : "Failed to reach your Slack workspace."; + + // The failure names its channel by id (design D7); an id the set no + // longer holds falls back to workspace-wide, never a raw Slack id. + const refusedChannel = result.failedChannelId + ? channels.find((channel) => channel.id === result.failedChannelId) + : undefined; + toast({ variant: "destructive", title: "Connection test failed", - description: reason - ? slackErrorMessage({ code: reason, detail: reason }) - : "Failed to reach your Slack workspace.", + description: refusedChannel + ? `Slack refused #${refusedChannel.name}: ${explanation}` + : explanation, }); } } catch (_error) { @@ -540,17 +634,19 @@ export const SlackIntegrationManager = ({
- {/* The check posts to the destination channel: the API answers - 400 when none is recorded yet. */} + {/* The check reaches the authorized channels: the API + answers 400 while the set is empty. */}
- {!defaultChannel && ( -

- Choose a destination channel below to enable this check. -

- )} +

+ {checkHint()} +

- setChannelReloads((reloads) => reloads + 1)} - disabled={isSavingChannel} + disabled={isSavingChannels} /> + {droppedChannels.length > 0 && ( + + + Dropped channels leave your alert rules too + + + Saving this selection drops{" "} + {channelList( + droppedChannels.map((channel) => channel.name), + )} + . Every alert rule targeting a dropped channel stops + targeting it, and Prowler stops delivering there. + Notifications already delivered stay in Slack. + + + )} +
-

- {/* The id decides, not the name: a missing name would deny - a destination the check runs against. */} - {defaultChannel - ? defaultChannel.name - ? `Prowler posts to #${defaultChannel.name}.` - : "Prowler posts to the channel you saved." - : "No destination channel recorded yet."} + {/* Keyed: the card's subtitle also starts "Prowler posts to". */} +

+ {authorizedChannels.length > 0 + ? `Prowler posts to ${channelList( + authorizedChannels.map((channel) => channel.name), + )}.` + : "No destination channels authorized yet."}

diff --git a/ui/types/integrations.ts b/ui/types/integrations.ts index a1efe220cb..bbd37595c5 100644 --- a/ui/types/integrations.ts +++ b/ui/types/integrations.ts @@ -98,13 +98,13 @@ export interface IntegrationProps { domain?: string; projects?: { [key: string]: string }; issue_types?: { [key: string]: string[] }; - // Slack specific configuration, server-owned. The channel keys are absent - // until one is chosen, not present and null: read them with `?? null`. + // Slack specific configuration, server-owned. The keys are optional here + // because the shape is shared with every integration: read with `?? []`. team_id?: string; team_name?: string; bot_user_id?: string; - channel_id?: string; - channel_name?: string; + channels?: SlackAuthorizedChannel[]; + verification?: SlackVerification; [key: string]: unknown; }; url?: string; @@ -123,6 +123,24 @@ export interface SlackChannelOption { is_private: boolean; } +/** + * `confirmation_sent_at` is null until a connection check posts one, and null + * again after a same-workspace reinstall (contract, OAuth and reads). + */ +export interface SlackAuthorizedChannel extends SlackChannelOption { + confirmation_sent_at: string | null; +} + +/** + * The contract's `verification` block in full, though nothing reads it yet + * (contract, OAuth and reads). + */ +export interface SlackVerification { + task_id: string | null; + started_at: string | null; + finished_at: string | null; +} + // Jira dispatch types export interface JiraDispatchRequest { data: {