mirror of
https://github.com/prowler-cloud/prowler.git
synced 2026-10-09 21:14:22 +00:00
fix(ui): complete Slack OAuth callback server-side to avoid router race (#12572)
Co-authored-by: alejandrobailo <alejandrobailo94@gmail.com>
This commit is contained in:
Pablo Fernandez Guerra (PFE)
co-authored by
alejandrobailo
alejandrobailo
parent
301edea7ce
commit
a610314eba
@@ -1,156 +0,0 @@
|
||||
/**
|
||||
* The cases `slack-page.integration.test.tsx` cannot express: it runs the Server
|
||||
* Action as a plain function, so there is no client→server transport to reject,
|
||||
* and its handler only answers the contract's shapes. React error boundaries
|
||||
* cannot see a rejection awaited in an effect, so an uncaught one leaves the
|
||||
* user on the spinner with no error and no way out.
|
||||
*/
|
||||
|
||||
import { render, screen } from "@testing-library/react";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
import type { IntegrationProps } from "@/types/integrations";
|
||||
|
||||
import { SlackCallback } from "./slack-callback";
|
||||
|
||||
const COMPLETED_QUERY = "code=slack-code-1f4a&state=st-2f1c9d7a";
|
||||
|
||||
const { exchangeSlackOAuthCode, callbackQuery, routerReplace } = vi.hoisted(
|
||||
() => ({
|
||||
exchangeSlackOAuthCode: vi.fn(),
|
||||
callbackQuery: { value: "" },
|
||||
routerReplace: vi.fn(),
|
||||
}),
|
||||
);
|
||||
|
||||
vi.mock("@/actions/integrations/slack", () => ({ exchangeSlackOAuthCode }));
|
||||
|
||||
// One router across renders, so the redirect off the spent code is assertable.
|
||||
const router = { replace: routerReplace };
|
||||
|
||||
vi.mock("next/navigation", () => ({
|
||||
useRouter: () => router,
|
||||
useSearchParams: () => new URLSearchParams(callbackQuery.value),
|
||||
}));
|
||||
|
||||
beforeEach(() => {
|
||||
callbackQuery.value = COMPLETED_QUERY;
|
||||
routerReplace.mockClear();
|
||||
});
|
||||
|
||||
const SPINNER_COPY = /Connecting your Slack workspace/;
|
||||
|
||||
/**
|
||||
* Literals, not imports: a rename on the component's side has to fail here.
|
||||
* `FAILURE_TITLE` claims nothing was connected, which only holds for outcomes
|
||||
* that happen before the API consumed the code.
|
||||
*/
|
||||
const FAILURE_TITLE = "Slack workspace not connected";
|
||||
const UNCONFIRMED_TITLE = "Slack install not confirmed";
|
||||
|
||||
describe("returning from Slack when the completion answers unexpectedly", () => {
|
||||
it("reports an unconfirmed result instead of spinning forever when the exchange call never comes back", async () => {
|
||||
// The client→server POST itself fails (dropped connection, action id
|
||||
// invalidated by a deploy), so the action's own error handling never runs.
|
||||
exchangeSlackOAuthCode.mockRejectedValue(new TypeError("Failed to fetch"));
|
||||
|
||||
render(<SlackCallback />);
|
||||
|
||||
// The API consumes the single-use code before answering, so the workspace
|
||||
// may well be connected: unknown, not failed.
|
||||
expect(
|
||||
await screen.findByText(/could not confirm whether the workspace/i),
|
||||
).toBeInTheDocument();
|
||||
expect(
|
||||
screen.getByRole("link", { name: /Back to Slack integration/ }),
|
||||
).toHaveAttribute("href", "/integrations/slack");
|
||||
expect(screen.queryByText(SPINNER_COPY)).not.toBeInTheDocument();
|
||||
|
||||
expect(screen.getByText(UNCONFIRMED_TITLE)).toBeInTheDocument();
|
||||
expect(screen.queryByText(FAILURE_TITLE)).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("still reports the workspace as connected when the created integration carries no configuration", async () => {
|
||||
// The install already succeeded; `configuration` only goes missing on the
|
||||
// client, where the callback reads the workspace name off it.
|
||||
exchangeSlackOAuthCode.mockResolvedValue({
|
||||
integration: {
|
||||
type: "integrations",
|
||||
id: "slack-integration-1",
|
||||
attributes: {
|
||||
inserted_at: "2026-08-10T09:00:00Z",
|
||||
updated_at: "2026-08-10T09:00:00Z",
|
||||
enabled: true,
|
||||
connected: null,
|
||||
connection_last_checked_at: null,
|
||||
integration_type: "slack",
|
||||
},
|
||||
links: { self: "/api/v1/integrations/slack-integration-1" },
|
||||
// Cast: the shape is the one the contract rules out.
|
||||
} as unknown as IntegrationProps,
|
||||
});
|
||||
|
||||
render(<SlackCallback />);
|
||||
|
||||
expect(
|
||||
await screen.findByText(/Connected to your Slack workspace/),
|
||||
).toBeInTheDocument();
|
||||
expect(screen.queryByText(SPINNER_COPY)).not.toBeInTheDocument();
|
||||
// Keyed on the escape link, the only element unique to the failure branch,
|
||||
// so this holds whichever headline that branch would have carried.
|
||||
expect(
|
||||
screen.queryByRole("link", { name: /Back to Slack integration/ }),
|
||||
).not.toBeInTheDocument();
|
||||
// `replace`, not `push`: a back navigation must not remount onto the code.
|
||||
expect(routerReplace).toHaveBeenCalledWith("/integrations/slack");
|
||||
});
|
||||
});
|
||||
|
||||
describe("returning from Slack with an error on the callback URL", () => {
|
||||
it("says the install was declined when Slack reports the approval was refused", async () => {
|
||||
// The one code Slack reliably sends to this redirect.
|
||||
callbackQuery.value = "error=access_denied&state=st-2f1c9d7a";
|
||||
|
||||
render(<SlackCallback />);
|
||||
|
||||
expect(
|
||||
await screen.findByText(/was not approved in Slack/),
|
||||
).toBeInTheDocument();
|
||||
expect(exchangeSlackOAuthCode).not.toHaveBeenCalled();
|
||||
|
||||
// Slack refused before issuing a code, so the flat "not connected" is a
|
||||
// fact here, unlike in the outcomes that follow an exchange.
|
||||
expect(screen.getByText(FAILURE_TITLE)).toBeInTheDocument();
|
||||
expect(screen.queryByText(UNCONFIRMED_TITLE)).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("names a Slack code it does not recognise, so a new failure reason is still diagnosable", async () => {
|
||||
// Slack publishes no closed set of codes for this redirect, so the guard is
|
||||
// on the shape of the value rather than on an allowlist.
|
||||
callbackQuery.value = "error=invalid_scope&state=st-2f1c9d7a";
|
||||
|
||||
render(<SlackCallback />);
|
||||
|
||||
expect(
|
||||
await screen.findByText(
|
||||
"Slack could not complete the install (invalid_scope).",
|
||||
),
|
||||
).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("drops a sentence smuggled into the error parameter instead of rendering it as Prowler's own copy", async () => {
|
||||
// The balancing punctuation is the point: it closes Prowler's parenthetical
|
||||
// and reopens it, so the payload would read as Prowler's own sentence.
|
||||
const payload =
|
||||
"). Slack has flagged this workspace. Contact Prowler support at +1-555-0100 to restore alerting (";
|
||||
callbackQuery.value = `error=${encodeURIComponent(payload)}&state=st-2f1c9d7a`;
|
||||
|
||||
render(<SlackCallback />);
|
||||
|
||||
expect(
|
||||
await screen.findByText("Slack could not complete the install."),
|
||||
).toBeInTheDocument();
|
||||
expect(document.body.textContent).not.toContain("+1-555-0100");
|
||||
expect(document.body.textContent).not.toContain("flagged this workspace");
|
||||
});
|
||||
});
|
||||
@@ -1,159 +0,0 @@
|
||||
"use client";
|
||||
|
||||
import { AlertCircle, CircleCheck, Loader2 } from "lucide-react";
|
||||
import Link from "next/link";
|
||||
import { useRouter, useSearchParams } from "next/navigation";
|
||||
import { useEffect, useRef, useState } from "react";
|
||||
|
||||
import { exchangeSlackOAuthCode } from "@/actions/integrations/slack";
|
||||
import {
|
||||
Alert,
|
||||
AlertDescription,
|
||||
AlertTitle,
|
||||
Button,
|
||||
} from "@/components/shadcn";
|
||||
import { SLACK_REASON_TOKEN } from "@/lib/integrations/slack-errors";
|
||||
|
||||
const SLACK_INTEGRATION_PATH = "/integrations/slack";
|
||||
|
||||
const STATUS = {
|
||||
CONNECTING: "connecting",
|
||||
CONNECTED: "connected",
|
||||
FAILED: "failed",
|
||||
} as const;
|
||||
|
||||
type Status = (typeof STATUS)[keyof typeof STATUS];
|
||||
|
||||
const UNCONFIRMED_COMPLETION_MESSAGE =
|
||||
"Prowler could not confirm whether the workspace was connected. Open the Slack integration page to check — if none is listed there, start the install again.";
|
||||
|
||||
const FAILURE_TITLE = "Slack workspace not connected";
|
||||
|
||||
/**
|
||||
* The API consumes the code and upserts the integration before it answers, so an
|
||||
* unreadable or missing answer can still mean a connected workspace. Kept short:
|
||||
* `AlertTitle` clamps to one line.
|
||||
*/
|
||||
const UNCONFIRMED_TITLE = "Slack install not confirmed";
|
||||
|
||||
const describeSlackError = (reason: string): string => {
|
||||
if (reason === "access_denied") {
|
||||
return "The install was not approved in Slack, so no workspace was connected.";
|
||||
}
|
||||
// `error` comes straight off the URL and is interpolated into Prowler's own
|
||||
// copy, so gate on the shape of a code: Slack publishes no closed set.
|
||||
return SLACK_REASON_TOKEN.test(reason)
|
||||
? `Slack could not complete the install (${reason}).`
|
||||
: "Slack could not complete the install.";
|
||||
};
|
||||
|
||||
/**
|
||||
* Slack's `code` is single-use: `hasStarted` holds the exchange to one run per
|
||||
* mount, and `router.replace` (not `push`) keeps a back navigation from
|
||||
* remounting onto a spent code.
|
||||
*/
|
||||
export const SlackCallback = () => {
|
||||
const router = useRouter();
|
||||
const searchParams = useSearchParams();
|
||||
const [status, setStatus] = useState<Status>(STATUS.CONNECTING);
|
||||
const [workspaceName, setWorkspaceName] = useState<string | null>(null);
|
||||
const [failure, setFailure] = useState<string>("");
|
||||
const [failureTitle, setFailureTitle] = useState<string>(FAILURE_TITLE);
|
||||
const hasStarted = useRef(false);
|
||||
|
||||
useEffect(() => {
|
||||
if (hasStarted.current) return;
|
||||
hasStarted.current = true;
|
||||
|
||||
const slackError = searchParams.get("error");
|
||||
const code = searchParams.get("code");
|
||||
const state = searchParams.get("state");
|
||||
|
||||
// Slack answers a declined install with `error` and no code, so there is
|
||||
// nothing to exchange.
|
||||
if (slackError) {
|
||||
setFailure(describeSlackError(slackError));
|
||||
setStatus(STATUS.FAILED);
|
||||
return;
|
||||
}
|
||||
|
||||
if (!code || !state) {
|
||||
setFailure(
|
||||
"Slack sent an incomplete response back, so the install could not be completed.",
|
||||
);
|
||||
setStatus(STATUS.FAILED);
|
||||
return;
|
||||
}
|
||||
|
||||
const complete = async () => {
|
||||
const result = await exchangeSlackOAuthCode({ code, state });
|
||||
|
||||
if ("integration" in result) {
|
||||
setWorkspaceName(
|
||||
result.integration.attributes?.configuration?.team_name ?? null,
|
||||
);
|
||||
setStatus(STATUS.CONNECTED);
|
||||
router.replace(SLACK_INTEGRATION_PATH);
|
||||
return;
|
||||
}
|
||||
|
||||
if ("unavailable" in result) {
|
||||
setFailure("Slack is not available in this environment yet.");
|
||||
} else if ("rateLimited" in result) {
|
||||
setFailure(result.message);
|
||||
} else if ("unconfirmed" in result) {
|
||||
setFailure(result.message);
|
||||
setFailureTitle(UNCONFIRMED_TITLE);
|
||||
} else {
|
||||
setFailure(result.error);
|
||||
}
|
||||
setStatus(STATUS.FAILED);
|
||||
};
|
||||
|
||||
// A rejection here means the call never came back (stale action id after a
|
||||
// deploy, HTML 502): error boundaries cannot see a rejection awaited inside
|
||||
// an effect, and the once-guard blocks a retry, so the page would spin.
|
||||
void complete().catch(() => {
|
||||
setFailure(UNCONFIRMED_COMPLETION_MESSAGE);
|
||||
setFailureTitle(UNCONFIRMED_TITLE);
|
||||
setStatus(STATUS.FAILED);
|
||||
});
|
||||
}, [router, searchParams]);
|
||||
|
||||
if (status === STATUS.CONNECTING) {
|
||||
return (
|
||||
<div className="flex items-center gap-3 text-sm text-gray-600 dark:text-gray-300">
|
||||
<Loader2 className="animate-spin" size={16} />
|
||||
Connecting your Slack workspace...
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
if (status === STATUS.CONNECTED) {
|
||||
return (
|
||||
<Alert variant="success">
|
||||
<CircleCheck />
|
||||
<AlertTitle>
|
||||
Connected to {workspaceName ?? "your Slack workspace"}
|
||||
</AlertTitle>
|
||||
<AlertDescription>
|
||||
Taking you back to the Slack integration, where you can authorize the
|
||||
channels Prowler posts to.
|
||||
</AlertDescription>
|
||||
</Alert>
|
||||
);
|
||||
}
|
||||
|
||||
return (
|
||||
<div className="flex flex-col items-start gap-4">
|
||||
<Alert variant="error">
|
||||
<AlertCircle />
|
||||
<AlertTitle>{failureTitle}</AlertTitle>
|
||||
<AlertDescription>{failure}</AlertDescription>
|
||||
</Alert>
|
||||
<Button asChild variant="outline">
|
||||
<Link href={SLACK_INTEGRATION_PATH}>Back to Slack integration</Link>
|
||||
</Button>
|
||||
</div>
|
||||
);
|
||||
};
|
||||
@@ -0,0 +1,161 @@
|
||||
"use client";
|
||||
|
||||
import { AlertCircle, CircleCheck } from "lucide-react";
|
||||
import { useSearchParams } from "next/navigation";
|
||||
import { useState } from "react";
|
||||
import type { ComponentProps } from "react";
|
||||
|
||||
import { Alert, AlertDescription, AlertTitle } from "@/components/shadcn";
|
||||
import { useMountEffect } from "@/hooks/use-mount-effect";
|
||||
import {
|
||||
readSlackConnectOutcome,
|
||||
SLACK_CONNECT_PARAMS,
|
||||
SLACK_CONNECT_STATUS,
|
||||
} from "@/lib/integrations/slack-connect-status";
|
||||
import type { SlackConnectOutcome } from "@/lib/integrations/slack-connect-status";
|
||||
import {
|
||||
SLACK_GENERIC_ERROR_MESSAGE,
|
||||
SLACK_REASON_TOKEN,
|
||||
slackErrorMessage,
|
||||
slackRateLimitMessage,
|
||||
} from "@/lib/integrations/slack-errors";
|
||||
|
||||
const FAILURE_TITLE = "Slack workspace not connected";
|
||||
|
||||
/**
|
||||
* The API consumes the code and upserts the integration before it answers, so
|
||||
* an unreadable or missing answer can still mean a connected workspace. Kept
|
||||
* short: `AlertTitle` clamps to one line.
|
||||
*/
|
||||
const UNCONFIRMED_TITLE = "Slack install not confirmed";
|
||||
|
||||
const describeSlackError = (reason: string | null): string => {
|
||||
if (reason === "access_denied") {
|
||||
return "The install was not approved in Slack, so no workspace was connected.";
|
||||
}
|
||||
// The reason came off the URL and is interpolated into Prowler's own copy,
|
||||
// so gate on the shape of a code (the contract already did — belt and
|
||||
// braces): Slack publishes no closed set.
|
||||
return reason && SLACK_REASON_TOKEN.test(reason)
|
||||
? `Slack could not complete the install (${reason}).`
|
||||
: "Slack could not complete the install.";
|
||||
};
|
||||
|
||||
interface NoticeContent {
|
||||
variant: ComponentProps<typeof Alert>["variant"];
|
||||
title: string;
|
||||
description: string;
|
||||
}
|
||||
|
||||
const noticeFor = (outcome: SlackConnectOutcome): NoticeContent => {
|
||||
switch (outcome.status) {
|
||||
case SLACK_CONNECT_STATUS.CONNECTED:
|
||||
return {
|
||||
variant: "success",
|
||||
title: "Slack workspace connected",
|
||||
description: "You can now authorize the channels Prowler posts to.",
|
||||
};
|
||||
case SLACK_CONNECT_STATUS.SLACK_ERROR:
|
||||
return {
|
||||
variant: "error",
|
||||
title: FAILURE_TITLE,
|
||||
description: describeSlackError(outcome.reason),
|
||||
};
|
||||
case SLACK_CONNECT_STATUS.INCOMPLETE:
|
||||
return {
|
||||
variant: "error",
|
||||
title: FAILURE_TITLE,
|
||||
description:
|
||||
"Slack sent an incomplete response back, so the install could not be completed.",
|
||||
};
|
||||
case SLACK_CONNECT_STATUS.EXPIRED:
|
||||
return {
|
||||
variant: "error",
|
||||
title: FAILURE_TITLE,
|
||||
// Not "try again in a moment": a spent install link never recovers.
|
||||
description:
|
||||
"The install link from Slack had already been used or expired, so no workspace was connected. Start the install again.",
|
||||
};
|
||||
case SLACK_CONNECT_STATUS.UNAVAILABLE:
|
||||
return {
|
||||
variant: "error",
|
||||
title: FAILURE_TITLE,
|
||||
description: "Slack is not available in this environment yet.",
|
||||
};
|
||||
case SLACK_CONNECT_STATUS.RATE_LIMITED:
|
||||
return {
|
||||
variant: "error",
|
||||
title: FAILURE_TITLE,
|
||||
description: slackRateLimitMessage(outcome.retryAfterSeconds),
|
||||
};
|
||||
case SLACK_CONNECT_STATUS.UNCONFIRMED:
|
||||
return {
|
||||
variant: "error",
|
||||
title: UNCONFIRMED_TITLE,
|
||||
description:
|
||||
"Prowler could not confirm whether the workspace was connected. If none is listed below, start the install again.",
|
||||
};
|
||||
case SLACK_CONNECT_STATUS.ERROR:
|
||||
return {
|
||||
variant: "error",
|
||||
title: FAILURE_TITLE,
|
||||
// Known codes keep Prowler's wording; the URL carries no free text, so
|
||||
// everything else reads as the generic refusal.
|
||||
description: slackErrorMessage(
|
||||
{ code: outcome.code },
|
||||
SLACK_GENERIC_ERROR_MESSAGE,
|
||||
),
|
||||
};
|
||||
}
|
||||
};
|
||||
|
||||
/**
|
||||
* The outcome the OAuth callback route left in the query string, shown once.
|
||||
* The params are stripped via the History API rather than `router.replace`: an
|
||||
* RSC refetch here buys nothing (the exchange already revalidated), and a
|
||||
* client navigation is exactly what the callback stopped relying on.
|
||||
*/
|
||||
interface SlackConnectNoticeProps {
|
||||
hasConnectedWorkspace: boolean;
|
||||
}
|
||||
|
||||
export const SlackConnectNotice = ({
|
||||
hasConnectedWorkspace,
|
||||
}: SlackConnectNoticeProps) => {
|
||||
const searchParams = useSearchParams();
|
||||
// Read once into state: the notice has to survive its own URL cleanup.
|
||||
const [outcome] = useState<SlackConnectOutcome | null>(() =>
|
||||
readSlackConnectOutcome(new URLSearchParams(searchParams.toString())),
|
||||
);
|
||||
|
||||
useMountEffect(() => {
|
||||
if (!outcome) return;
|
||||
const params = new URLSearchParams(window.location.search);
|
||||
SLACK_CONNECT_PARAMS.forEach((param) => params.delete(param));
|
||||
const query = params.toString();
|
||||
const fragment = window.location.hash;
|
||||
window.history.replaceState(
|
||||
null,
|
||||
"",
|
||||
query
|
||||
? `${window.location.pathname}?${query}${fragment}`
|
||||
: `${window.location.pathname}${fragment}`,
|
||||
);
|
||||
});
|
||||
|
||||
if (!outcome) return null;
|
||||
|
||||
const verifiedOutcome =
|
||||
outcome.status === SLACK_CONNECT_STATUS.CONNECTED && !hasConnectedWorkspace
|
||||
? { ...outcome, status: SLACK_CONNECT_STATUS.UNCONFIRMED }
|
||||
: outcome;
|
||||
const notice = noticeFor(verifiedOutcome);
|
||||
|
||||
return (
|
||||
<Alert data-slack-connect-notice variant={notice.variant}>
|
||||
{notice.variant === "success" ? <CircleCheck /> : <AlertCircle />}
|
||||
<AlertTitle>{notice.title}</AlertTitle>
|
||||
<AlertDescription>{notice.description}</AlertDescription>
|
||||
</Alert>
|
||||
);
|
||||
};
|
||||
Reference in New Issue
Block a user