From 3e000faa3109230b748e4418a13c11c9e59f5cfb Mon Sep 17 00:00:00 2001 From: "Pablo Fernandez Guerra (PFE)" <148432447+pfe-nazaries@users.noreply.github.com> Date: Fri, 21 Aug 2026 12:42:47 +0200 Subject: [PATCH] feat(ui): add Slack disconnect and revoked-credential recovery (#12437) --- ui/__tests__/msw/handlers/slack.fixtures.ts | 116 +++++++ ui/__tests__/msw/handlers/slack.ts | 17 ++ ui/actions/integrations/slack.test.ts | 34 ++- ui/actions/integrations/slack.ts | 131 ++++++-- .../slack/slack-integration.harness.ts | 196 ++++++++++++ .../slack/slack-page.integration.test.tsx | 225 ++++++++++++++ .../slack/slack-integration-manager.tsx | 286 ++++++++++++++++-- 7 files changed, 961 insertions(+), 44 deletions(-) diff --git a/ui/__tests__/msw/handlers/slack.fixtures.ts b/ui/__tests__/msw/handlers/slack.fixtures.ts index 3d063c5ee8..e29f3e4b21 100644 --- a/ui/__tests__/msw/handlers/slack.fixtures.ts +++ b/ui/__tests__/msw/handlers/slack.fixtures.ts @@ -69,6 +69,24 @@ export interface SlackRefusalFixture { retryAfterSeconds: number | null; } +/** + * What `DELETE /integrations/{id}` reports about revoking the token at Slack. + * Revocation is best-effort: the row goes either way, and the outcome travels in + * JSON:API `meta`. + * + * One boolean is the whole of it: the API sends no reason for a revocation that + * did not happen, so modelling one would let a test prove copy the real + * deployment can never produce. + */ +export interface SlackRevocationFixture { + /** + * Slack confirmed the token no longer grants Prowler anything. `null` when the + * answer reports nothing at all — the plain `204` a deployment without a + * `destroy` override sends, which is what the UI meets today. + */ + revoked: boolean | null; +} + export interface SlackFixture { /** * The deployment has `SLACK_CLIENT_ID` / `SLACK_CLIENT_SECRET` / @@ -111,6 +129,13 @@ export interface SlackFixture { * listing itself answered fine. */ channelSaveRefusal: SlackRefusalFixture | null; + /** + * The API refused the disconnect itself, so the row is still there + * afterwards. Distinct from `revocation`, which is about a removal that + * happened. + */ + disconnectRefusal: SlackRefusalFixture | null; + revocation: SlackRevocationFixture; } /** @@ -173,6 +198,12 @@ export const SLACK_RATE_LIMITED_DETAIL = export const INTEGRATIONS_SERVER_ERROR_DETAIL = "A server error occurred."; export const SLACK_MISSING_SCOPE_DETAIL = "Slack refused the request: missing_scope."; +/** + * Names the raw reason, as the missing-scope wording does: what lets a test tell + * copy the UI mapped from `code` apart from an echoed `detail`. + */ +export const SLACK_TOKEN_EXPIRED_DETAIL = + "Slack refused the request: token_expired."; /** * The same sentence for "it is gone" and "the app was removed from it": only * `code` separates them, which is why a client must read `code`. @@ -180,6 +211,14 @@ export const SLACK_MISSING_SCOPE_DETAIL = export const SLACK_UNKNOWN_CHANNEL_DETAIL = "That channel is not one Prowler can post to."; +/** + * A `403` from the shared destroy route: the role can read the integration but + * not remove it. Names no Slack reason, so the API's own wording is what the + * user is shown. + */ +export const SLACK_DISCONNECT_FORBIDDEN_DETAIL = + "You do not have permission to disconnect this integration."; + /** * A `200` challenge page from a proxy or WAF that took the call instead of the * API. V8 truncates the parser message for this body before the word `html`, so @@ -205,6 +244,13 @@ export const SLACK_NOT_IN_CHANNEL_CODE = "not_in_channel"; * open-ended, so having no copy for one is the ordinary case. */ export const SLACK_UNMAPPED_REASON_CODE = "is_archived"; +/** + * Two of the four dead-grant codes the contract lists. Whichever call surfaces + * one, the integration is disconnected and the only way out is connecting the + * workspace again (contract, Cross-cutting). + */ +export const SLACK_TOKEN_REVOKED_CODE = "token_revoked"; +export const SLACK_TOKEN_EXPIRED_CODE = "token_expired"; export const SLACK_RETRY_AFTER_SECONDS = 30; @@ -227,6 +273,18 @@ export const SLACK_RATE_LIMITED_REFUSAL: SlackRefusalFixture = { retryAfterSeconds: SLACK_RETRY_AFTER_SECONDS, }; +/** + * The stored grant is no longer usable: a `400` like any other actionable + * refusal, deliberately not the `401` that would read as an expired Prowler + * session (contract, Errors). + */ +export const SLACK_TOKEN_EXPIRED_REFUSAL: SlackRefusalFixture = { + status: 400, + code: SLACK_TOKEN_EXPIRED_CODE, + detail: SLACK_TOKEN_EXPIRED_DETAIL, + retryAfterSeconds: null, +}; + /** Slack-side or transport failure — a `502` naming no reason at all. */ export const SLACK_UPSTREAM_REFUSAL: SlackRefusalFixture = { status: 502, @@ -254,6 +312,14 @@ export const SLACK_NOT_IN_CHANNEL_REFUSAL: SlackRefusalFixture = { retryAfterSeconds: null, }; +/** The disconnect is refused before Slack is asked anything: a `403`. */ +export const SLACK_DISCONNECT_FORBIDDEN_REFUSAL: SlackRefusalFixture = { + status: 403, + code: null, + detail: SLACK_DISCONNECT_FORBIDDEN_DETAIL, + retryAfterSeconds: null, +}; + /** * Two public channels and one private the Prowler app was invited to, ordered * so the private one lands on the second cursor page. @@ -313,6 +379,8 @@ export const slackFixture = ( channelsPageSize: SLACK_CHANNELS_PAGE_SIZE, channelsRefusal: null, channelSaveRefusal: null, + disconnectRefusal: null, + revocation: { revoked: true }, ...overrides, }); @@ -392,3 +460,51 @@ export const configuredSlackFixture = ( overrides: Partial = {}, ): SlackFixture => slackFixtureWithDefaultChannel(SLACK_DEFAULT_CHANNEL, overrides); + +/** + * A connected tenant whose disconnect removes the row but cannot revoke at + * Slack — the outcome the user has to finish by hand in the workspace. + */ +export const revokeFailureSlackFixture = ( + overrides: Partial = {}, +): SlackFixture => + connectedSlackFixture({ + revocation: { revoked: false }, + ...overrides, + }); + +/** + * A connected tenant whose disconnect answers a plain `204` with no body: the + * row is gone and the revocation is unreported. + */ +export const unreportedRevocationSlackFixture = ( + overrides: Partial = {}, +): SlackFixture => + connectedSlackFixture({ + revocation: { revoked: null }, + ...overrides, + }); + +/** + * A connected tenant whose disconnect the API refuses: nothing is removed and + * nothing is revoked, so the workspace is still connected afterwards. + */ +export const disconnectRefusalSlackFixture = ( + overrides: Partial = {}, +): SlackFixture => + configuredSlackFixture({ + disconnectRefusal: SLACK_DISCONNECT_FORBIDDEN_REFUSAL, + ...overrides, + }); + +/** + * A connected tenant whose token has been revoked at Slack: the row still says + * connected until a check runs, and the check is what surfaces it. + */ +export const revokedTokenSlackFixture = ( + overrides: Partial = {}, +): SlackFixture => + configuredSlackFixture({ + connection: { connected: false, error: SLACK_TOKEN_REVOKED_CODE }, + ...overrides, + }); diff --git a/ui/__tests__/msw/handlers/slack.ts b/ui/__tests__/msw/handlers/slack.ts index 60d0908fbf..4dbe0476d5 100644 --- a/ui/__tests__/msw/handlers/slack.ts +++ b/ui/__tests__/msw/handlers/slack.ts @@ -351,5 +351,22 @@ export const handlersForSlack = (fx: SlackFixture) => { install.workspace.channelName = channel.name; return HttpResponse.json({ data: integrationResource(install) }); }), + + // Disconnect. Revocation at Slack is best-effort: the row is removed either + // way and the outcome travels in `meta` — or nowhere at all, in the plain + // `204` a deployment with no `destroy` override sends. + http.delete<{ id: string }>(`${API}/integrations/:id`, ({ params }) => { + if (!install || install.id !== params.id) { + return HttpResponse.json(errorBody("Not found.", 404), { status: 404 }); + } + // Checked first: a refused disconnect removes nothing. + if (fx.disconnectRefusal) return refuse(fx.disconnectRefusal); + + install = null; + if (fx.revocation.revoked === null) { + return new HttpResponse(null, { status: 204 }); + } + return HttpResponse.json({ meta: { revoked: fx.revocation.revoked } }); + }), ]; }; diff --git a/ui/actions/integrations/slack.test.ts b/ui/actions/integrations/slack.test.ts index c790a161eb..c49f2cc2de 100644 --- a/ui/actions/integrations/slack.test.ts +++ b/ui/actions/integrations/slack.test.ts @@ -59,6 +59,7 @@ vi.mock("@/lib", () => ({ })); import { + disconnectSlackIntegration, exchangeSlackOAuthCode, getSlackAuthorizeUrl, getSlackChannels, @@ -351,6 +352,12 @@ const channelOptions = (count: number) => const RATE_LIMITED_MESSAGE = "Slack is rate limiting Prowler right now. Try again in about 30 seconds."; +/** A dead grant as the API reports it: reason in `code`, prose in `detail`. */ +const TOKEN_EXPIRED_CODE = "token_expired"; +const TOKEN_EXPIRED_DETAIL = "Slack refused the request: token_expired."; +const TOKEN_EXPIRED_MESSAGE = + "Prowler's Slack credential has expired. Connect the workspace again to restore access."; + describe("getSlackChannels", () => { it("follows a cursor-only `next` on the listing's own URL, not on the API root", async () => { // The link is opaque (design D6), so the API may answer with the cursor @@ -445,9 +452,27 @@ describe("getSlackChannels", () => { const result = await getSlackChannels(SLACK_INTEGRATION_ID); + // A rate limit says nothing about the grant, so the truncation names none. expect(result).toEqual({ channels: [channelOption(FIRST_CHANNEL)], incomplete: RATE_LIMITED_MESSAGE, + code: null, + }); + }); + + it("names the reason a later page was refused, not only the wording", async () => { + fetchMock + .mockResolvedValueOnce(channelPage(FIRST_CHANNEL, "?page[cursor]=2")) + .mockResolvedValueOnce( + errorResponse(400, TOKEN_EXPIRED_DETAIL, TOKEN_EXPIRED_CODE), + ); + + const result = await getSlackChannels(SLACK_INTEGRATION_ID); + + expect(result).toEqual({ + channels: [channelOption(FIRST_CHANNEL)], + incomplete: TOKEN_EXPIRED_MESSAGE, + code: TOKEN_EXPIRED_CODE, }); }); @@ -456,7 +481,7 @@ describe("getSlackChannels", () => { const result = await getSlackChannels(SLACK_INTEGRATION_ID); - expect(result).toEqual({ error: RATE_LIMITED_MESSAGE }); + expect(result).toEqual({ error: RATE_LIMITED_MESSAGE, code: null }); }); }); @@ -601,6 +626,10 @@ const COPY_ONLY_ACTIONS = [ name: "setSlackDefaultChannel", call: (id: string) => setSlackDefaultChannel(id, FIRST_CHANNEL.id), }, + { + name: "disconnectSlackIntegration", + call: (id: string) => disconnectSlackIntegration(id), + }, ]; const rateLimitedResponse = () => @@ -668,7 +697,8 @@ describe.each(COPY_ONLY_ACTIONS)("$name", ({ call }) => { expect(captureExceptionMock).not.toHaveBeenCalled(); expect(captureMessageMock).not.toHaveBeenCalled(); - expect(result).toEqual({ error: refusal.expected }); + // None of these refusals names a `code`. + expect(result).toEqual({ error: refusal.expected, code: null }); }); }); diff --git a/ui/actions/integrations/slack.ts b/ui/actions/integrations/slack.ts index e29031b99b..4761c3e446 100644 --- a/ui/actions/integrations/slack.ts +++ b/ui/actions/integrations/slack.ts @@ -40,6 +40,14 @@ interface SlackUnconfirmed { interface SlackActionError { error: string; + /** + * The refusal's `code`, when it named one, alongside the copy. A caller reads + * it to recognise a class of failure the wording cannot be pattern-matched + * for — a Slack grant that has stopped working, which the contract allows + * from any of these calls (Cross-cutting) and is recovered from by + * reconnecting rather than by retrying. + */ + code?: string | null; } interface SlackAuthorizeUrl { @@ -169,19 +177,19 @@ const failureFrom = async ( }; } - return { error: slackErrorMessage(failure, fallback) }; + return { error: slackErrorMessage(failure, fallback), code: failure.code }; }; /** - * `failureFrom` flattened to one line of copy, for the calls whose only - * outcome is "it did not work". Rate limiting keeps its own wording: - * `conversations.list` is Slack tier 2, so a `429` shows up here (contract, - * Errors) and the wait it names is the useful part. + * `failureFrom` flattened to one refusal, for the calls whose only outcome is + * "it did not work". Rate limiting keeps its own wording: `conversations.list` + * is Slack tier 2, so a `429` shows up here (contract, Errors) and the wait it + * names is the useful part. */ -const errorMessageFrom = async ( +const refusalFrom = async ( response: Response, fallback: string, -): Promise => { +): Promise => { // Same 5xx handling as `failureFrom`, `503` excepted: here too it means Slack // is unavailable. Must run before `readSlackFailure`: a body can only be read // once. @@ -191,9 +199,13 @@ const errorMessageFrom = async ( const failure = await readSlackFailure(response); - return failure.status === RATE_LIMITED_STATUS - ? slackRateLimitMessage(failure.retryAfterSeconds) - : slackErrorMessage(failure, fallback); + return { + error: + failure.status === RATE_LIMITED_STATUS + ? slackRateLimitMessage(failure.retryAfterSeconds) + : slackErrorMessage(failure, fallback), + code: failure.code, + }; }; /** Mint an OAuth state and get the consent URL. Creates no integration. */ @@ -296,6 +308,13 @@ interface SlackChannelsSuccess { * the picker *and* the reason. */ incomplete?: string; + /** + * The `code` of the refusal that cut the read short, when it named one. A + * grant that has stopped working refuses the second cursor page exactly as it + * refuses the first, and a caller reading only the failure path would never + * hear about it. + */ + code?: string | null; } export type SlackChannelsResult = SlackChannelsSuccess | SlackActionError; @@ -339,9 +358,9 @@ export const getSlackChannels = async ( }); if (!response.ok) { - let message: string; + let refusal: SlackActionError; try { - message = await errorMessageFrom( + refusal = await refusalFrom( response, `Unable to read the workspace's channels: ${response.statusText}`, ); @@ -353,8 +372,8 @@ export const getSlackChannels = async ( } return channels.length > 0 - ? { channels, incomplete: message } - : { error: message }; + ? { channels, incomplete: refusal.error, code: refusal.code } + : refusal; } // A page that is not JSON reads as no channels, rather than throwing a @@ -447,12 +466,12 @@ export const setSlackDefaultChannel = async ( }); if (!response.ok) { - return { - error: await errorMessageFrom( - response, - `Unable to save the destination channel: ${response.statusText}`, - ), - }; + // Awaited inside the `try`: unawaited, a 5xx's rejection would skip + // this `catch`. + return await refusalFrom( + response, + `Unable to save the destination channel: ${response.statusText}`, + ); } const body = await response.json().catch(() => null); @@ -473,3 +492,75 @@ export const setSlackDefaultChannel = async ( return handleApiError(error); } }; + +/** + * What the API reports about revoking Prowler's token at Slack: one boolean in + * `meta`, and nothing else — it sends no reason for a revocation that did not + * happen, so there is no field here to hold one. + */ +export interface SlackRevocation { + /** + * Whether Slack confirmed the token no longer grants Prowler anything, or + * `null` when the response carried no outcome. The contract says the outcome + * is always reported, so `null` means the response is wrong, not the + * revocation. + */ + revoked: boolean | null; +} + +interface SlackDisconnectSuccess { + /** The integration is gone from Prowler, whatever Slack answered. */ + disconnected: true; + revocation: SlackRevocation; +} + +export type SlackDisconnectResult = SlackDisconnectSuccess | SlackActionError; + +/** + * Disconnect the workspace: `DELETE /integrations/{id}`. + * + * The generic `deleteIntegration` cannot serve this: it discards the response + * body, and the body is the whole point. Revocation at Slack is best-effort — + * the row is removed either way and the outcome travels in JSON:API `meta` — so + * a caller has to tell "gone and revoked" from "gone, but still installed in + * Slack". + * + * A body without the field (an empty `204`, say) yields `null`, not `false`: an + * unreported outcome must not send the user off to clean up Slack, nor be shown + * as access revoked. + */ +export const disconnectSlackIntegration = async ( + integrationId: string, +): Promise => { + const id = parseIntegrationId(integrationId); + if (!id) return { error: SLACK_GENERIC_ERROR_MESSAGE }; + + const headers = await getAuthHeaders({ contentType: true }); + const url = new URL(`${apiBaseUrl}/integrations/${id}`); + + try { + const response = await fetch(url.toString(), { method: "DELETE", headers }); + + if (!response.ok) { + return await refusalFrom( + response, + `Unable to disconnect the Slack workspace: ${response.statusText}`, + ); + } + + const body = await response.json().catch(() => ({})); + const meta = body?.meta ?? {}; + + revalidatePath("/integrations"); + revalidatePath("/integrations/slack"); + + return { + disconnected: true, + revocation: { + revoked: typeof meta.revoked === "boolean" ? meta.revoked : null, + }, + }; + } catch (error) { + return handleApiError(error); + } +}; diff --git a/ui/app/(prowler)/integrations/slack/slack-integration.harness.ts b/ui/app/(prowler)/integrations/slack/slack-integration.harness.ts index 095d9a7660..4edb8ebbde 100644 --- a/ui/app/(prowler)/integrations/slack/slack-integration.harness.ts +++ b/ui/app/(prowler)/integrations/slack/slack-integration.harness.ts @@ -32,6 +32,22 @@ export type ConnectionOutcome = /** Sentinel: the page settled on "no channel recorded", rather than not yet. */ const NO_DEFAULT_CHANNEL = ""; +export const REVOCATION_OUTCOME = { + REVOKED: "revoked", + NOT_REVOKED: "not-revoked", + /** The answer said nothing either way, so the page claims neither. */ + UNREPORTED: "unreported", +} as const; + +export type RevocationOutcome = + (typeof REVOCATION_OUTCOME)[keyof typeof REVOCATION_OUTCOME]; + +/** The alert shown when Slack never confirmed the revocation. */ +const REVOCATION_NOTICE = /revocation/i; + +/** The alert shown when Slack has stopped accepting the credential. */ +const REVOKED_CREDENTIAL_NOTICE = /no longer accepts Prowler's access/; + interface CallbackParams { code?: string; state?: string; @@ -686,4 +702,184 @@ export class SlackIntegrationHarness extends BrowserHarness { ).find((element) => /invites? @Prowler/.test(element.textContent ?? "")); return hint ? (hint.textContent ?? "").trim() : null; } + + // --- Disconnecting ------------------------------------------------------ + + get disconnectCallCount(): number { + return this.countRequests("DELETE", "/integrations/"); + } + + /** + * Disconnects the workspace, confirming the way a user has to, and reports + * what the page says about the revocation. The outcomes are mutually + * exclusive, so asking for one also checks the others are absent. + * + * The revoked and unreported outcomes share a toast title, so each is read + * from its own description: a title match would agree with either. + */ + async disconnect(): Promise { + await this.openDisconnectConfirmation(); + return this.confirmDisconnect(); + } + + /** + * Opens the confirmation and reports what it asks the user to accept, before + * anything is accepted. + */ + async openDisconnectConfirmation(): Promise { + // The dialog's own button carries the noun too, hence the exact match on + // the card's action. + await this.clickButton(/^\s*Disconnect\s*$/); + + const description = await this.waitFor( + () => this.q('[data-slot="dialog-description"]'), + 10000, + "the disconnect confirmation", + ); + return (description.textContent ?? "").replace(/\s+/g, " ").trim(); + } + + /** Accepts the open confirmation and reports the revocation outcome. */ + async confirmDisconnect(): Promise { + await this.clickButton(/Disconnect workspace/); + + return this.waitFor( + () => { + const outcomes = [ + this.alertMatching(REVOCATION_NOTICE) + ? REVOCATION_OUTCOME.NOT_REVOKED + : null, + this.containsText(/has been revoked/) + ? REVOCATION_OUTCOME.REVOKED + : null, + this.containsText(/is no longer connected to Prowler/) + ? REVOCATION_OUTCOME.UNREPORTED + : null, + ].filter((outcome): outcome is RevocationOutcome => outcome !== null); + + if (outcomes.length > 1) { + throw new Error( + `confirmDisconnect: the page shows ${outcomes.join(" and ")} at once`, + ); + } + return outcomes[0] ?? null; + }, + 15000, + "the disconnect outcome", + ); + } + + /** + * Try a disconnect the API refuses and hand back what the user is told. One + * that goes through fails the test rather than timing out. + */ + async refusedDisconnect(): Promise { + await this.openDisconnectConfirmation(); + await this.clickButton(/Disconnect workspace/); + + return this.waitFor( + () => { + if (this.containsText(/No workspace connected/)) { + throw new Error( + "refusedDisconnect: the workspace was disconnected, not refused", + ); + } + return this.toastText(/Disconnect failed/); + }, + 15000, + "the refused disconnect", + ); + } + + /** + * Whether the page is back to offering an install with no workspace + * connected. The consent URL is minted after the disconnect, so the install + * affordance appears a beat after the copy does. + */ + async returnedToUnconnectedState(): Promise { + await this.waitForText(/No workspace connected/, 10000); + return ( + (await this.waitForOrNull( + () => this.offersInstall(), + 5000, + "the install to be offered again", + )) ?? false + ); + } + + /** Whether the page is asking the user to remove the access in Slack. */ + showsRevocationNotice(): boolean { + return this.alertMatching(REVOCATION_NOTICE) !== null; + } + + /** + * What the user is told when the row was removed but Slack never confirmed + * the revocation. + */ + async revocationNotice(): Promise { + const notice = await this.waitFor( + () => this.alertMatching(REVOCATION_NOTICE), + 10000, + "the revocation notice", + ); + return (notice.textContent ?? "").trim(); + } + + // --- A credential Slack no longer accepts -------------------------------- + + /** What the user is told when Slack has stopped accepting the credential. */ + async revokedCredentialNotice(): Promise { + const notice = await this.waitFor( + () => this.alertMatching(REVOKED_CREDENTIAL_NOTICE), + 10000, + "the revoked-credential notice", + ); + return (notice.textContent ?? "").replace(/\s+/g, " ").trim(); + } + + /** Whether the page is saying Slack has stopped accepting the credential. */ + showsRevokedCredentialNotice(): boolean { + return this.alertMatching(REVOKED_CREDENTIAL_NOTICE) !== null; + } + + private reconnectLink(): HTMLAnchorElement | null { + return ( + Array.from(this.container.querySelectorAll("a")).find((anchor) => + /Reconnect to Slack/.test(anchor.textContent ?? ""), + ) ?? null + ); + } + + /** Whether the page offers to approve Prowler in the workspace again. */ + offersReconnect(): boolean { + return this.reconnectLink() !== null; + } + + /** + * Waits, unlike `offersReconnect`: the affordance needs a consent URL the + * page mints only once it knows it needs one, so it lands a beat after the + * notice that explains it. + */ + async waitForReconnect(): Promise { + await this.waitFor(() => this.reconnectLink(), 10000, "the reconnect link"); + } + + /** The consent URL the reconnect affordance points at, once it is offered. */ + async reconnectUrl(): Promise { + const link = await this.waitFor( + () => this.reconnectLink(), + 10000, + "the reconnect link", + ); + return link.href; + } + + /** The alert whose text matches, of however many the page is showing. */ + private alertMatching(pattern: RegExp): HTMLElement | null { + return ( + Array.from( + this.container.querySelectorAll('[data-slot="alert"]'), + ).find((alert) => pattern.test(alert.textContent ?? "")) ?? null + ); + } } 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 0ca1a28eea..e201b388ce 100644 --- a/ui/app/(prowler)/integrations/slack/slack-page.integration.test.tsx +++ b/ui/app/(prowler)/integrations/slack/slack-page.integration.test.tsx @@ -11,9 +11,13 @@ import { it } from "@/__tests__/fixtures"; import { configuredSlackFixture, connectedSlackFixture, + disconnectRefusalSlackFixture, INTEGRATIONS_SERVER_ERROR_DETAIL, partiallyReadSlackFixture, + revokedTokenSlackFixture, + revokeFailureSlackFixture, SLACK_CHANNEL_NOT_FOUND_REFUSAL, + SLACK_DISCONNECT_FORBIDDEN_DETAIL, SLACK_MISSING_SCOPE_CODE, SLACK_MISSING_SCOPE_REFUSAL, SLACK_NOT_IN_CHANNEL_CODE, @@ -22,15 +26,20 @@ import { SLACK_PUBLIC_CHANNEL, SLACK_RATE_LIMITED_REFUSAL, SLACK_SECOND_PUBLIC_CHANNEL, + SLACK_TOKEN_EXPIRED_CODE, + SLACK_TOKEN_EXPIRED_REFUSAL, + SLACK_TOKEN_REVOKED_CODE, SLACK_UNKNOWN_CHANNEL_DETAIL, SLACK_UPSTREAM_REFUSAL, slackFixture, slackFixtureWithDefaultChannel, unreadableCheckTimeSlackFixture, + unreportedRevocationSlackFixture, } from "@/__tests__/msw/handlers/slack.fixtures"; import { CONNECTION_OUTCOME, + REVOCATION_OUTCOME, SlackIntegrationHarness, } from "./slack-integration.harness"; @@ -595,3 +604,219 @@ describe("choosing a destination channel", () => { expect(await harness.defaultChannel()).toBeNull(); }, 60000); }); + +describe("disconnecting a workspace", () => { + it("removes the integration and returns the card to its unconnected state", async () => { + // Given — a tenant with a workspace connected. + const harness = new SlackIntegrationHarness(connectedSlackFixture()); + await harness.mount(); + + // When — the user opens the confirmation. Only the removal is Prowler's to + // promise: the revocation happens at Slack, which can refuse it or report + // nothing at all. + expect(await harness.openDisconnectConfirmation()).toBe( + "Prowler will remove the integration, stop posting to Prowler HQ, and " + + "attempt to revoke its access at Slack. Connecting again means " + + "approving Prowler in Slack.", + ); + + // And — the user confirms; Slack confirms the revocation. + expect(await harness.confirmDisconnect()).toBe(REVOCATION_OUTCOME.REVOKED); + + expect(harness.disconnectCallCount).toBe(1); + expect(await harness.returnedToUnconnectedState()).toBe(true); + }, 30000); + + it("still removes the integration when the revocation fails, and says access may need removing by hand", async () => { + // Given — Slack will not accept the revocation; the row goes either way. + const harness = new SlackIntegrationHarness(revokeFailureSlackFixture()); + await harness.mount(); + + // When + expect(await harness.disconnect()).toBe(REVOCATION_OUTCOME.NOT_REVOKED); + + // And — the disconnect revalidates, so the copy below is read from props + // that no longer carry an integration at all. + await harness.refreshPageData(); + + // Then — what is true of both sides: nothing is left in Prowler to retry, + // and the app may still be installed at Slack. + const notice = await harness.revocationNotice(); + expect(notice).toMatch(/gone from Prowler/); + expect(notice).toMatch(/nothing to retry here/); + expect(notice).toMatch(/may still be installed in Prowler HQ/); + expect(notice).toMatch( + /remove it from that workspace's Slack app settings/, + ); + expect(await harness.returnedToUnconnectedState()).toBe(true); + }, 30000); + + it("says only that the workspace is no longer connected when nothing reports the revocation", async () => { + // Given — the plain `204` a deployment that overrides nothing answers: no + // body, so no `meta` to read the outcome from. The case users really meet. + const harness = new SlackIntegrationHarness( + unreportedRevocationSlackFixture(), + ); + await harness.mount(); + + // When + expect(await harness.disconnect()).toBe(REVOCATION_OUTCOME.UNREPORTED); + + // Then — nothing sends the user to Slack to finish a job no answer said + // was unfinished. + expect(harness.showsRevocationNotice()).toBe(false); + expect(await harness.returnedToUnconnectedState()).toBe(true); + }, 30000); + + it("says why the disconnect failed and leaves the workspace connected", async () => { + // Given — a finished setup whose disconnect the API refuses outright, so + // nothing is removed and Slack is never asked to revoke anything. + const harness = new SlackIntegrationHarness( + disconnectRefusalSlackFixture(), + ); + await harness.mount(); + + // When + const refusal = await harness.refusedDisconnect(); + + // Then — the API's own reason reaches the user, and nothing claims a + // revocation that never ran. + expect(refusal).toMatch(SLACK_DISCONNECT_FORBIDDEN_DETAIL); + expect(harness.showsRevocationNotice()).toBe(false); + // And — the card is untouched: the integration the user still has is the + // one to try again on. + expect(await harness.connectedWorkspaceName()).toBe(WORKSPACE_NAME); + expect(await harness.connectionBadge()).toBe("Connected"); + }, 30000); +}); + +describe("a credential Slack no longer accepts", () => { + it("says the connection check found a dead credential, and offers to connect the workspace again", async () => { + // Given — the token was revoked at Slack, so the row still reads connected + // until a check runs (contract, Cross-cutting). + const harness = new SlackIntegrationHarness(revokedTokenSlackFixture()); + await harness.mount(); + + // When + expect(await harness.testConnection()).toBe(CONNECTION_OUTCOME.FAILURE); + + // Then — a way forward rather than only an error: a revoked token is fixed + // by approving Prowler again, not by checking a second time. + const notice = await harness.revokedCredentialNotice(); + expect(notice).toMatch(/no longer accepts Prowler's access to Prowler HQ/); + expect(notice).toMatch(/Prowler's access to Slack was revoked/); + expect(notice).toMatch(/Connect the workspace again to restore access/); + // Slack's reason is a protocol token: it is what the UI switched on, never + // what it showed. + expect(notice).not.toMatch(new RegExp(SLACK_TOKEN_REVOKED_CODE)); + + const consentScreen = new URL(await harness.reconnectUrl()); + expect(`${consentScreen.origin}${consentScreen.pathname}`).toBe( + "https://slack.com/oauth/v2/authorize", + ); + await harness.waitForReconnect(); + }, 30000); + + it("offers the same recovery when the channel listing is what finds the credential dead", async () => { + // Given — a finished setup whose credential expired. The listing runs on + // arrival, so it meets Slack before any check does, and the contract says + // any call can be the one that surfaces this. + const harness = new SlackIntegrationHarness( + configuredSlackFixture({ channelsRefusal: SLACK_TOKEN_EXPIRED_REFUSAL }), + ); + + // When — nothing but opening the page. + await harness.mount(); + + // Then — the same answer the connection check gives, worded for how this + // credential died rather than left as a channel problem. + const notice = await harness.revokedCredentialNotice(); + expect(notice).toMatch(/Prowler's Slack credential has expired/); + expect(notice).toMatch(/Connect the workspace again to restore access/); + await harness.waitForReconnect(); + + // And — the picker says the same, in the same words: `detail` names the raw + // reason, and it is `code` the UI answered from. + const message = await harness.channelPickerMessage(); + expect(message).toMatch(/Prowler's Slack credential has expired/); + expect(message).not.toMatch(new RegExp(SLACK_TOKEN_EXPIRED_CODE)); + }, 30000); + + it("offers it too when only a later cursor page is what Slack refuses", async () => { + // Given — a two-page workspace whose second page is refused by a credential + // Slack no longer accepts: the read stops short rather than failing. + const harness = new SlackIntegrationHarness( + partiallyReadSlackFixture({ + channelsRefusal: SLACK_TOKEN_EXPIRED_REFUSAL, + }), + ); + + // When — nothing but opening the page. + await harness.mount(); + + // Then — what was read stays on offer, as it does for any short list. + // Alphabetically, as the picker sorts what it offers. + expect(await harness.channelOptions()).toEqual([ + SLACK_SECOND_PUBLIC_CHANNEL.name, + SLACK_PUBLIC_CHANNEL.name, + ]); + + // And — the dead credential is reported all the same: a picker that still + // works is no reason to leave the user without the one fix there is. + const notice = await harness.revokedCredentialNotice(); + expect(notice).toMatch(/Prowler's Slack credential has expired/); + await harness.waitForReconnect(); + expect(await harness.connectionBadge()).toBe("Disconnected"); + }, 60000); + + it("keeps saying so when a later check fails without Slack naming a reason", async () => { + // Given — the listing found the credential dead on arrival, and a later + // check that fails naming no reason at all. + const harness = new SlackIntegrationHarness( + configuredSlackFixture({ + channelsRefusal: SLACK_TOKEN_EXPIRED_REFUSAL, + connection: { connected: false, error: null }, + }), + ); + await harness.mount(); + expect(await harness.revokedCredentialNotice()).toMatch( + /Prowler's Slack credential has expired/, + ); + + // When + expect(await harness.testConnection()).toBe(CONNECTION_OUTCOME.FAILURE); + + // Then — a failure Slack never answered is no evidence the grant works + // again, so the dead credential is still what the page reports. + expect(await harness.revokedCredentialNotice()).toMatch( + /Prowler's Slack credential has expired/, + ); + await harness.waitForReconnect(); + expect(await harness.connectionBadge()).toBe("Disconnected"); + }, 60000); + + it("stops saying so once a save Slack validated goes through", async () => { + // Given — a finished setup whose connection check found the grant revoked. + const harness = new SlackIntegrationHarness( + configuredSlackFixture({ + connection: { connected: false, error: SLACK_TOKEN_REVOKED_CODE }, + }), + ); + await harness.mount(); + expect(await harness.connectionBadge()).toBe("Connected"); + expect(await harness.testConnection()).toBe(CONNECTION_OUTCOME.FAILURE); + 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. + harness.fixture.connection = { connected: true, error: null }; + await harness.chooseChannel(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. + expect(harness.showsRevokedCredentialNotice()).toBe(false); + expect(harness.offersReconnect()).toBe(false); + expect(await harness.connectionBadge()).toBe("Connected"); + }, 60000); +}); diff --git a/ui/components/integrations/slack/slack-integration-manager.tsx b/ui/components/integrations/slack/slack-integration-manager.tsx index fa43d1c7c2..9424105c1d 100644 --- a/ui/components/integrations/slack/slack-integration-manager.tsx +++ b/ui/components/integrations/slack/slack-integration-manager.tsx @@ -1,11 +1,13 @@ "use client"; import { format, isValid, parseISO } from "date-fns"; -import { TestTube } from "lucide-react"; +import { TestTube, Unplug } from "lucide-react"; import { useEffect, useState } from "react"; import { testIntegrationConnection } from "@/actions/integrations/integrations"; import { + disconnectSlackIntegration, + getSlackAuthorizeUrl, getSlackChannels, setSlackDefaultChannel, } from "@/actions/integrations/slack"; @@ -22,6 +24,13 @@ import { CardHeader, useToast, } from "@/components/shadcn"; +import { Modal } from "@/components/shadcn/modal"; +import { + isSlackTokenErrorCode, + SLACK_REASON_TOKEN, + slackErrorMessage, +} from "@/lib/integrations/slack-errors"; +import type { SlackTokenErrorCode } from "@/lib/integrations/slack-errors"; import type { IntegrationProps, SlackChannelOption, @@ -53,6 +62,15 @@ type ChannelsState = ChannelsLoading | ChannelsFailed | ChannelsLoaded; const CHECK_BLOCKED_REASON_ID = "slack-connection-check-blocked"; +/** + * A disconnect that removed the row without Slack confirming the revocation. + * The workspace name travels with it: the notice exists to name the workspace + * to clean up, and the record is gone by the time revalidation lands. + */ +interface UnconfirmedRevocation { + workspace: string | null; +} + // The name may be missing: the id decides what the UI can do with it. interface SlackChannelRef { id: string; @@ -64,6 +82,14 @@ const channelRefEquals = ( b: SlackChannelRef | null, ) => a?.id === b?.id && a?.name === b?.name; +/** + * Slack's own reason, when the string is one: the connection check reports a + * reason and its own prose in the same field, and only a reason is an answer + * from Slack about the credential. + */ +const asReasonCode = (reason: string | null): string | null => + reason && SLACK_REASON_TOKEN.test(reason) ? reason : null; + interface SlackIntegrationManagerProps { /** At most one exists per tenant (one workspace). */ integration: IntegrationProps | null; @@ -82,6 +108,24 @@ export const SlackIntegrationManager = ({ loadError, }: SlackIntegrationManagerProps) => { const [isTesting, setIsTesting] = useState(false); + const [isDisconnectOpen, setIsDisconnectOpen] = useState(false); + const [isDisconnecting, setIsDisconnecting] = useState(false); + // The row is gone the moment the API says so; the server component's + // revalidation only catches up on the next navigation. + const [disconnected, setDisconnected] = useState(false); + const [unconfirmedRevocation, setUnconfirmedRevocation] = + useState(null); + /** + * The `code` of the last refusal any Slack-backed call ran into, or `null` + * when the last answer was not a refusal. A dead grant can surface from any + * of them (contract, Cross-cutting), so every call reports here instead of + * deciding on its own. + */ + const [lastRefusalCode, setLastRefusalCode] = useState(null); + // A connected workspace arrives with no consent URL, since no install is left + // to start (design D10), so one is minted only if a reconnect turns out to be + // the way out. + const [mintedInstallUrl, setMintedInstallUrl] = useState(null); const { toast } = useToast(); const integrationId = integration?.id ?? null; @@ -127,6 +171,47 @@ export const SlackIntegrationManager = ({ } } + // Only an answer from Slack moves the bus: a call that never got one proves + // nothing and leaves the last answer standing. + const provedCredentialAlive = () => setLastRefusalCode(null); + + const recordRefusal = (code: string | null | undefined) => { + if (code) setLastRefusalCode(code); + }; + + /** + * Whether the last refusal proves the grant itself is dead, rather than a + * channel unreachable or Slack busy. Derived, not stored, so it self-clears: + * a later call Slack answered at all (even to refuse a channel) is proof the + * credential works again, and the notice goes with it. + */ + const credentialFailure: SlackTokenErrorCode | null = isSlackTokenErrorCode( + lastRefusalCode, + ) + ? lastRefusalCode + : null; + + const needsInstallUrl = disconnected || credentialFailure !== null; + + useEffect(() => { + if (!needsInstallUrl) return; + + let cancelled = false; + + getSlackAuthorizeUrl() + .then((result) => { + if (cancelled || !("authorizeUrl" in result)) return; + setMintedInstallUrl(result.authorizeUrl); + }) + .catch(() => { + // Nothing to say: the page loses a shortcut, not a way to reconnect. + }); + + return () => { + cancelled = true; + }; + }, [needsInstallUrl]); + useEffect(() => { if (!integrationId) return; @@ -145,6 +230,12 @@ export const SlackIntegrationManager = ({ notice: result.incomplete ?? null, }, ); + // The listing runs on arrival, so it is where a dead credential shows + // up first. A read cut short still names its refusal's code, so a grant + // that died on a later cursor page is heard too; a truncation naming + // none was Slack busy, not refusing. + if ("error" in result || result.code) recordRefusal(result.code); + else provedCredentialAlive(); }) .catch(() => { if (cancelled) return; @@ -178,6 +269,9 @@ export const SlackIntegrationManager = ({ ); if ("error" in result) { + // The API validates the channel against Slack, so the save can + // discover the credential is gone. + recordRefusal(result.code); toast({ variant: "destructive", title: "Could not save the destination channel", @@ -193,6 +287,7 @@ export const SlackIntegrationManager = ({ channels.find((channel) => channel.id === selectedChannelId)?.name ?? null; + provedCredentialAlive(); setDefaultChannel({ id: selectedChannelId, name: savedName }); saved = true; toast({ @@ -222,16 +317,25 @@ export const SlackIntegrationManager = ({ const result = await testIntegrationConnection(id); if (result.success) { + provedCredentialAlive(); toast({ title: "Connection test successful!", description: result.message || "Prowler can reach your Slack workspace.", }); } else { + // A dead credential named here is not a failure checking again can + // fix, so the reason is recorded and not only reported. + const reason = result.error?.trim() || null; + + recordRefusal(asReasonCode(reason)); + toast({ variant: "destructive", title: "Connection test failed", - description: result.error || "Failed to reach your Slack workspace.", + description: reason + ? slackErrorMessage({ code: reason, detail: reason }) + : "Failed to reach your Slack workspace.", }); } } catch (_error) { @@ -245,7 +349,61 @@ export const SlackIntegrationManager = ({ } }; + const handleDisconnect = async (id: string) => { + const recordedWorkspace = + integration?.attributes.configuration.team_name ?? null; + const workspace = recordedWorkspace ?? "your Slack workspace"; + + setIsDisconnecting(true); + try { + const result = await disconnectSlackIntegration(id); + + if ("error" in result) { + toast({ + variant: "destructive", + title: "Disconnect failed", + description: result.error, + }); + return; + } + + const { revoked } = result.revocation; + + // The row is gone whatever Slack answered, so the page goes back to its + // unconnected state either way, and a dead credential is moot once the + // row it belonged to is gone. + setDisconnected(true); + setLastRefusalCode(null); + // Only an explicit `false` sends the user to finish the job in Slack: an + // unreported outcome is neither a failed revocation nor a confirmed one, + // so it claims neither. + setUnconfirmedRevocation( + revoked === false ? { workspace: recordedWorkspace } : null, + ); + + if (revoked !== false) { + toast({ + title: "Slack workspace disconnected", + description: + revoked === true + ? `Prowler's access to ${workspace} has been revoked.` + : `${workspace} is no longer connected to Prowler.`, + }); + } + } catch (_error) { + toast({ + variant: "destructive", + title: "Error", + description: "Failed to disconnect Slack. Please try again.", + }); + } finally { + setIsDisconnecting(false); + setIsDisconnectOpen(false); + } + }; + const workspaceName = integration?.attributes.configuration.team_name; + const installUrl = mintedInstallUrl ?? authorizeUrl; const checkedAt = integration?.attributes.connection_last_checked_at; const checkedOn = checkedAt ? parseISO(checkedAt) : null; @@ -264,6 +422,36 @@ export const SlackIntegrationManager = ({ )} + +
+ + + +
+ + {loadError && ( Could not load your Slack integration @@ -271,6 +459,45 @@ export const SlackIntegrationManager = ({ )} + {unconfirmedRevocation && ( + + + Slack disconnected — remove Prowler's access in Slack + + + The integration and the token Prowler had stored are gone from + Prowler, so there is nothing to retry here. Slack did not confirm + the revocation, so the Prowler app may still be installed in{" "} + {unconfirmedRevocation.workspace ?? "the workspace"} — remove it + from that workspace's Slack app settings. + + + )} + + {credentialFailure && ( + + + Slack no longer accepts Prowler's access to{" "} + {workspaceName ?? "this workspace"} + + {/* Each mapped sentence already ends in the thing that fixes it. */} + + {slackErrorMessage({ code: credentialFailure })} Until then, nothing + Prowler sends will reach the workspace. + + {installUrl && ( + + )} + + )} + {/* Replaces the cards, not the whole page: an early return here would swallow the rate-limit and load-error notices above. */} {unavailable ? ( @@ -284,7 +511,7 @@ export const SlackIntegrationManager = ({ soon as it is. - ) : integration ? ( + ) : integration && !disconnected ? ( @@ -308,22 +539,33 @@ export const SlackIntegrationManager = ({ )}
- {/* The check posts to the destination channel: the API answers - 400 when none is recorded yet. */} - +
+ {/* The check posts to the destination channel: the API answers + 400 when none is recorded yet. */} + + +
{!defaultChannel && (

- {authorizeUrl ? ( + {installUrl ? (