From 4511a22c2dd8018b88c2bc85cb747adf074fb99e Mon Sep 17 00:00:00 2001 From: Thibault Le Ouay Date: Tue, 29 Sep 2026 14:53:03 +0200 Subject: [PATCH] slack: members-only access (#2791) Co-authored-by: Claude Opus 5.5 --- .../settings/integrations/slack-card.tsx | 45 ++++- .../integrations/slack/link/client.tsx | 152 +++++++++++++++ .../settings/integrations/slack/link/page.tsx | 13 ++ .../integrations/slack/link/search-params.tsx | 7 + apps/dashboard/src/lib/workspace-cookie.ts | 4 +- apps/server/src/routes/slack/agent.ts | 21 +-- apps/server/src/routes/slack/blocks.ts | 30 +++ apps/server/src/routes/slack/commands.test.ts | 94 ++++++++++ apps/server/src/routes/slack/commands.ts | 68 +++++-- apps/server/src/routes/slack/handler.test.ts | 154 ++++++++++++++-- apps/server/src/routes/slack/handler.ts | 174 ++++++++++++++---- apps/server/src/routes/slack/home.ts | 13 ++ apps/server/src/routes/slack/index.test.ts | 2 +- .../src/routes/slack/interactions.test.ts | 64 ++++++- apps/server/src/routes/slack/interactions.ts | 91 +++++---- apps/server/src/routes/slack/oauth.test.ts | 17 +- apps/server/src/routes/slack/oauth.ts | 30 ++- .../src/routes/slack/require-slack-member.ts | 70 +++++++ .../routes/slack/resolve-slack-user.test.ts | 100 ++++++---- .../src/routes/slack/resolve-slack-user.ts | 68 +++---- .../src/routes/slack/service-adapter.ts | 13 +- packages/api/src/lambda.ts | 2 + packages/api/src/router/slackUser.test.ts | 71 +++++++ packages/api/src/router/slackUser.ts | 95 ++++++++++ .../src/auth/__tests__/require-scope.test.ts | 1 + packages/services/src/context.ts | 11 +- .../__tests__/install-slack-agent.test.ts | 1 + .../maintenance/__tests__/maintenance.test.ts | 1 + .../slack-user/__tests__/slack-user.test.ts | 71 +++++++ packages/services/src/slack-user/index.ts | 6 + .../services/src/slack-user/link-token.ts | 87 +++++++++ packages/services/src/slack-user/list.ts | 23 +++ .../__tests__/status-report.test.ts | 1 + packages/services/test/helpers.ts | 2 +- 34 files changed, 1369 insertions(+), 233 deletions(-) create mode 100644 apps/dashboard/src/app/(dashboard)/settings/integrations/slack/link/client.tsx create mode 100644 apps/dashboard/src/app/(dashboard)/settings/integrations/slack/link/page.tsx create mode 100644 apps/dashboard/src/app/(dashboard)/settings/integrations/slack/link/search-params.tsx create mode 100644 apps/server/src/routes/slack/commands.test.ts create mode 100644 apps/server/src/routes/slack/require-slack-member.ts create mode 100644 packages/api/src/router/slackUser.test.ts create mode 100644 packages/api/src/router/slackUser.ts create mode 100644 packages/services/src/slack-user/link-token.ts create mode 100644 packages/services/src/slack-user/list.ts diff --git a/apps/dashboard/src/app/(dashboard)/settings/integrations/slack-card.tsx b/apps/dashboard/src/app/(dashboard)/settings/integrations/slack-card.tsx index 5fe0096e..d276f461 100644 --- a/apps/dashboard/src/app/(dashboard)/settings/integrations/slack-card.tsx +++ b/apps/dashboard/src/app/(dashboard)/settings/integrations/slack-card.tsx @@ -3,7 +3,7 @@ import { Lock } from "@openstatus/icons"; import { Badge } from "@openstatus/ui/components/ui/badge"; import { Button } from "@openstatus/ui/components/ui/button"; -import { useMutation, useQueryClient } from "@tanstack/react-query"; +import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import { useRouter } from "next/navigation"; import { Link } from "@/components/common/link"; @@ -61,6 +61,12 @@ export function SlackIntegrationCard({ }), ); + const linkedAccountsQuery = useQuery({ + ...trpc.slackUser.list.queryOptions(), + enabled: isConnected, + }); + const linkedAccounts = linkedAccountsQuery.data; + const handleInstall = () => { generateToken.mutate(); }; @@ -85,10 +91,39 @@ export function SlackIntegrationCard({ {isConnected ? ( -

- Connected to{" "} - {integration.data?.teamName ?? "Slack workspace"} -

+
+

+ Connected to{" "} + {integration.data?.teamName ?? "Slack workspace"} + . Only members with a linked Slack account can use it. +

+ {linkedAccountsQuery.isPending ? ( +

+ Loading linked accounts… +

+ ) : linkedAccountsQuery.isError ? ( +

+ Could not load linked accounts. Reload the page to try again. +

+ ) : linkedAccounts?.length ? ( +
    + {linkedAccounts.map((account) => ( +
  • + Linked Slack user {account.slackUserId} +
  • + ))} +
+ ) : ( +

+ Your Slack account links itself the first time you use + openstatus in Slack, when its email matches your openstatus + email. +

+ )} +
) : (

Connect your Slack workspace to get started. diff --git a/apps/dashboard/src/app/(dashboard)/settings/integrations/slack/link/client.tsx b/apps/dashboard/src/app/(dashboard)/settings/integrations/slack/link/client.tsx new file mode 100644 index 00000000..7cbcaf5c --- /dev/null +++ b/apps/dashboard/src/app/(dashboard)/settings/integrations/slack/link/client.tsx @@ -0,0 +1,152 @@ +"use client"; + +import { Button } from "@openstatus/ui/components/ui/button"; +import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; +import { useQueryStates } from "nuqs"; + +import { Link } from "@/components/common/link"; +import { + EmptyStateContainer, + EmptyStateDescription, + EmptyStateTitle, +} from "@/components/content/empty-state"; +import { + Section, + SectionDescription, + SectionGroup, + SectionHeader, + SectionTitle, +} from "@/components/content/section"; +import { useTRPC } from "@/lib/trpc/client"; +import { switchWorkspace } from "@/lib/workspace-cookie"; + +import { searchParamsParsers } from "./search-params"; + +function Message({ + title, + description, + children, +}: { + title: string; + description: string; + children?: React.ReactNode; +}) { + return ( + + {title} + {description} +

+ {children} + +
+ + ); +} + +export function Client() { + const trpc = useTRPC(); + const queryClient = useQueryClient(); + const [{ token }] = useQueryStates(searchParamsParsers); + const previewQuery = useQuery({ + ...trpc.slackUser.previewLink.queryOptions({ token }), + retry: false, + }); + const preview = previewQuery.data; + const link = useMutation( + trpc.slackUser.link.mutationOptions({ + onSuccess: () => + queryClient.invalidateQueries({ + queryKey: trpc.slackUser.list.queryKey(), + }), + }), + ); + + const body = (() => { + if (previewQuery.isPending) { + return

Checking link…

; + } + if (previewQuery.isError || !preview) { + return ( + + ); + } + if (link.isSuccess) { + return ( + + ); + } + switch (preview.status) { + case "invalid": + return ( + + ); + case "not-member": + return ( + + ); + case "wrong-workspace": + return ( + + + + ); + case "ready": + return ( + + + + ); + } + })(); + + return ( + +
+ + Slack + + Only members of this workspace can use openstatus in Slack. + + + {link.error ? ( + + ) : ( + body + )} +
+
+ ); +} diff --git a/apps/dashboard/src/app/(dashboard)/settings/integrations/slack/link/page.tsx b/apps/dashboard/src/app/(dashboard)/settings/integrations/slack/link/page.tsx new file mode 100644 index 00000000..9b91b924 --- /dev/null +++ b/apps/dashboard/src/app/(dashboard)/settings/integrations/slack/link/page.tsx @@ -0,0 +1,13 @@ +import { redirect } from "next/navigation"; +import type { SearchParams } from "nuqs"; + +import { Client } from "./client"; +import { searchParamsCache } from "./search-params"; + +export default async function SlackLinkPage(props: { + searchParams: Promise; +}) { + const { token } = await searchParamsCache.parse(props.searchParams); + if (!token) return redirect("/settings/integrations"); + return ; +} diff --git a/apps/dashboard/src/app/(dashboard)/settings/integrations/slack/link/search-params.tsx b/apps/dashboard/src/app/(dashboard)/settings/integrations/slack/link/search-params.tsx new file mode 100644 index 00000000..e6db52ec --- /dev/null +++ b/apps/dashboard/src/app/(dashboard)/settings/integrations/slack/link/search-params.tsx @@ -0,0 +1,7 @@ +import { createSearchParamsCache, parseAsString } from "nuqs/server"; + +export const searchParamsParsers = { + token: parseAsString.withDefault(""), +}; + +export const searchParamsCache = createSearchParamsCache(searchParamsParsers); diff --git a/apps/dashboard/src/lib/workspace-cookie.ts b/apps/dashboard/src/lib/workspace-cookie.ts index 89ab3ed2..a271e921 100644 --- a/apps/dashboard/src/lib/workspace-cookie.ts +++ b/apps/dashboard/src/lib/workspace-cookie.ts @@ -2,7 +2,7 @@ export const WORKSPACE_SLUG_COOKIE = "workspace-slug"; // Client-side workspace switch: set the cookie the proxy reads, then hard-reload // so every server component re-resolves against the new workspace. -export function switchWorkspace(slug: string) { +export function switchWorkspace(slug: string, href = "/overview") { document.cookie = `${WORKSPACE_SLUG_COOKIE}=${slug}; path=/;`; - window.location.href = "/overview"; + window.location.href = href; } diff --git a/apps/server/src/routes/slack/agent.ts b/apps/server/src/routes/slack/agent.ts index 9e8da209..dceeb448 100644 --- a/apps/server/src/routes/slack/agent.ts +++ b/apps/server/src/routes/slack/agent.ts @@ -6,6 +6,7 @@ import type { ModelMessage, Tool } from "ai"; import { tb } from "@/libs/clients"; import { buildSlackTools } from "./registry-runner"; +import type { SlackActor } from "./require-slack-member"; import { buildSystemPrompt } from "./system-prompt"; // Vercel AI Gateway model id (`anthropic/`). Override via @@ -83,25 +84,11 @@ export async function runAgent( workspace: Workspace, thread: SlackThreadMessage[], botUserId: string, - userText?: string, - origin?: { - slackUserId: string; - teamId: string | undefined; - /** Member matched by Slack email; reads run without it, approvals require it. */ - userId?: number; - }, + userText: string | undefined, + actor: SlackActor, options?: AgentOptions, ): Promise { - const ctx: ServiceContext = { - workspace, - actor: { - type: "slack", - teamId: origin?.teamId ?? "", - slackUserId: origin?.slackUserId ?? "", - userId: origin?.userId, - }, - tb, - }; + const ctx: ServiceContext = { workspace, actor, tb }; const tools = buildSlackTools(ctx, options?.tools); let messages = convertThreadToMessages(thread, botUserId); diff --git a/apps/server/src/routes/slack/blocks.ts b/apps/server/src/routes/slack/blocks.ts index 8b1584d1..0701cfca 100644 --- a/apps/server/src/routes/slack/blocks.ts +++ b/apps/server/src/routes/slack/blocks.ts @@ -46,6 +46,7 @@ interface ButtonElement { text: TextObject; action_id: string; value?: string; + url?: string; style?: "primary" | "danger"; } @@ -85,6 +86,35 @@ function capMessageText(text: string): string { return `${text.slice(0, kept)}${TRUNCATION_NOTICE}`; } +export const LINK_ACCOUNT_ACTION_ID = "link_account"; + +export const LINK_ACCOUNT_TEXT = + "Link your openstatus account to use openstatus in Slack."; + +export function buildLinkAccountBlocks(url: string): Block[] { + return [ + { + type: "section", + text: { + type: "mrkdwn", + text: "*Link your openstatus account*\nOnly members of this openstatus workspace can use openstatus in Slack. Link your account to continue — the link is valid for 10 minutes.", + }, + }, + { + type: "actions", + elements: [ + { + type: "button", + text: { type: "plain_text", text: "Link account" }, + action_id: LINK_ACCOUNT_ACTION_ID, + url, + style: "primary", + }, + ], + }, + ]; +} + /** * Action-id encoding. We need to round-trip both the pending action's id * and (when the tool declares one) the user's extraFlag choice. The diff --git a/apps/server/src/routes/slack/commands.test.ts b/apps/server/src/routes/slack/commands.test.ts new file mode 100644 index 00000000..81e97e97 --- /dev/null +++ b/apps/server/src/routes/slack/commands.test.ts @@ -0,0 +1,94 @@ +import crypto from "node:crypto"; + +import { beforeEach, describe, expect, test } from "@openstatus/test-utils"; +import { Hono } from "hono"; + +import { slackTestState } from "@/libs/test/doubles/slack-test-state"; +import { + TEST_SIGNING_SECRET as SIGNING_SECRET, + withSlackConfig, +} from "@/libs/test/slack-config"; + +import { handleSlackCommand } from "./commands"; +import type { SlackEnv } from "./config"; +import { verifySlackSignature } from "./verify"; + +function createTestApp() { + const app = withSlackConfig(new Hono()); + app.post("/slack/commands", verifySlackSignature, handleSlackCommand); + return app; +} + +function post( + app: ReturnType, + text: string, + user: string, +) { + const body = new URLSearchParams({ + text, + team_id: "T_KNOWN", + user_id: user, + channel_id: "C1", + }).toString(); + const timestamp = Math.floor(Date.now() / 1000); + const sig = crypto + .createHmac("sha256", SIGNING_SECRET) + .update(`v0:${timestamp}:${body}`) + .digest("hex"); + return app.request("/slack/commands", { + method: "POST", + headers: { + "Content-Type": "application/x-www-form-urlencoded", + "x-slack-request-timestamp": String(timestamp), + "x-slack-signature": `v0=${sig}`, + }, + body, + }); +} + +describe("handleSlackCommand (members only)", () => { + const app = createTestApp(); + + beforeEach(() => { + slackTestState.calls = []; + slackTestState.resolveWorkspace = (teamId: string) => + teamId === "T_KNOWN" + ? Promise.resolve({ + workspace: { id: 1 }, + botToken: "xoxb-test", + botUserId: "UBOT", + }) + : Promise.resolve(null); + }); + + test("help needs no link", async () => { + const res = await post(app, "help", `U_${crypto.randomUUID()}`); + const json = (await res.json()) as { text: string }; + expect(json.text).toContain("/openstatus subscribe"); + }); + + test("an unlinked user gets the link card instead of running the command", async () => { + slackTestState.usersInfoImpl = () => + Promise.resolve({ ok: true, user: { profile: {} } }); + const res = await post(app, "subscriptions", `U_${crypto.randomUUID()}`); + const json = (await res.json()) as { + text: string; + blocks?: { type: string }[]; + }; + expect(json.text).toContain("Link your openstatus account"); + expect(json.blocks?.some((b) => b.type === "actions")).toBe(true); + }); + + test("a linked member runs the command", async () => { + slackTestState.usersInfoImpl = () => + Promise.resolve({ + ok: true, + user: { profile: { email: "ping@openstatus.dev" } }, + }); + const res = await post(app, "subscriptions", `U_${crypto.randomUUID()}`); + const json = (await res.json()) as { text: string; blocks?: unknown }; + // The `subscriptions` reply itself, not the link card or the error text. + expect(json.text).toContain("subscribed to"); + expect(json.blocks).toBeUndefined(); + }); +}); diff --git a/apps/server/src/routes/slack/commands.ts b/apps/server/src/routes/slack/commands.ts index 753c7111..91966fdd 100644 --- a/apps/server/src/routes/slack/commands.ts +++ b/apps/server/src/routes/slack/commands.ts @@ -10,6 +10,13 @@ import type { Context } from "hono"; import { z } from "zod"; import { runInBackground } from "./background"; +import { + type Block, + buildLinkAccountBlocks, + LINK_ACCOUNT_TEXT, +} from "./blocks"; +import type { SlackConfig, SlackEnv } from "./config"; +import { linkAccountUrl, requireSlackMember } from "./require-slack-member"; import { resolvePageFromUrl } from "./resolve-page"; import { resolveWorkspace } from "./workspace-resolver"; @@ -18,6 +25,7 @@ const logger = getLogger("api-server"); const slashCommandSchema = z.object({ text: z.string().optional().default(""), team_id: z.string(), + user_id: z.string(), channel_id: z.string(), channel_name: z.string().optional(), response_url: z.string().optional(), @@ -32,16 +40,21 @@ const HELP = [ "• `/openstatus subscriptions` — show this channel's subscriptions", ].join("\n"); -function ephemeral(c: Context, text: string) { - return c.json({ response_type: "ephemeral", text }); +type CommandReply = { text: string; blocks?: Block[] }; + +function ephemeral(c: Context, reply: CommandReply) { + return c.json({ response_type: "ephemeral", ...reply }); } /** Deliver a reply after the ack, via the command's single-use response URL. */ -async function respondLater(responseUrl: string, text: string): Promise { +async function respondLater( + responseUrl: string, + reply: CommandReply, +): Promise { const res = await fetch(responseUrl, { method: "POST", headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ response_type: "ephemeral", text }), + body: JSON.stringify({ response_type: "ephemeral", ...reply }), }); if (!res.ok) { logger.error("slack response_url delivery failed", { @@ -63,10 +76,11 @@ async function joinChannel(teamId: string, channelId: string): Promise { } } -export function handleSlackCommand(c: Context) { +export function handleSlackCommand(c: Context) { + const config = c.get("slackConfig"); const parsed = slashCommandSchema.safeParse(c.get("slackBody")); if (!parsed.success) { - return ephemeral(c, "Could not read the command."); + return ephemeral(c, { text: "Could not read the command." }); } const command = parsed.data; const sub = subcommand(command); @@ -74,7 +88,7 @@ export function handleSlackCommand(c: Context) { // `help` — and anything unrecognised, which falls through to it — needs no // I/O, so it is answered in the ack itself. if (sub !== "subscribe" && sub !== "unsubscribe" && sub !== "subscriptions") { - return ephemeral(c, HELP); + return ephemeral(c, { text: HELP }); } // The rest resolve a page, write to the DB and call the Slack API, which can @@ -84,7 +98,9 @@ export function handleSlackCommand(c: Context) { if (!responseUrl) { // Slack always sends one; without it there is nowhere to deliver a late // reply, so fall back to answering inline. - return runCommand(command).then((text) => ephemeral(c, text)); + return runMemberCommand(command, config).then((reply) => + ephemeral(c, reply), + ); } runInBackground( @@ -92,15 +108,15 @@ export function handleSlackCommand(c: Context) { async () => { // The 200 above is the only other thing the user gets: without this the // command fails silently on their side. - const text = await runCommand(command).catch((err: unknown) => { + const reply = await runMemberCommand(command, config).catch((error) => { logger.error("slack command failed", { - error: err, + error, teamId: command.team_id, channelId: command.channel_id, }); - return ":x: Something went wrong. Please try again."; + return { text: ":x: Something went wrong. Please try again." }; }); - await respondLater(responseUrl, text); + await respondLater(responseUrl, reply); }, { teamId: command.team_id, channelId: command.channel_id }, ); @@ -117,6 +133,34 @@ function argument(command: SlashCommand): string | undefined { return command.text.trim().split(/\s+/).filter(Boolean)[1]; } +/** Only linked members of the connected workspace may run commands. */ +async function runMemberCommand( + command: SlashCommand, + config: SlackConfig, +): Promise { + const resolved = await resolveWorkspace(command.team_id); + if (!resolved) { + return { + text: "openstatus isn't connected to this Slack workspace. Connect it from the openstatus dashboard.", + }; + } + const actor = await requireSlackMember({ + workspace: resolved.workspace, + teamId: command.team_id, + slackUserId: command.user_id, + slack: new WebClient(resolved.botToken), + }); + if (!actor) { + const url = await linkAccountUrl(config, { + workspaceId: resolved.workspace.id, + teamId: command.team_id, + slackUserId: command.user_id, + }); + return { text: LINK_ACCOUNT_TEXT, blocks: buildLinkAccountBlocks(url) }; + } + return { text: await runCommand(command) }; +} + /** Runs the subcommand and returns the message to show the user. */ async function runCommand(command: SlashCommand): Promise { const { diff --git a/apps/server/src/routes/slack/handler.test.ts b/apps/server/src/routes/slack/handler.test.ts index 42d8c26a..457418bc 100644 --- a/apps/server/src/routes/slack/handler.test.ts +++ b/apps/server/src/routes/slack/handler.test.ts @@ -11,6 +11,7 @@ import { withSlackConfig, } from "@/libs/test/slack-config"; +import { settleBackgroundTasks } from "./background"; import type { SlackEnv } from "./config"; import { handleSlackEvent, @@ -50,8 +51,25 @@ function signAndPost( }); } +// Generous: the member gate hits the DB, which is slow under `--parallel`. +async function waitForCall(method: string, timeoutMs = 5000) { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + const call = slackTestState.calls.find((m) => m.method === method); + if (call) return call; + await new Promise((r) => setTimeout(r, 10)); + } + return undefined; +} + function resetSlackTestState() { slackTestState.calls = []; + // The seeded member of workspace 1, so every test user is linked. + slackTestState.usersInfoImpl = () => + Promise.resolve({ + ok: true, + user: { profile: { email: "ping@openstatus.dev" } }, + }); slackTestState.postMessageOverride = null; slackTestState.updateOverride = null; slackTestState.postEphemeralOverride = null; @@ -154,17 +172,16 @@ describe("handleSlackEvent", () => { }); expect(res.status).toBe(200); - await new Promise((r) => setTimeout(r, 50)); - - const publish = slackTestState.calls.find( - (m) => m.method === "views.publish", - ); + const publish = await waitForCall("views.publish"); expect(publish).toBeDefined(); expect((publish?.args.view as { type: string }).type).toBe("home"); expect(publish?.args.user_id).toBe("U1"); }); test("ignores app_home_opened for the messages tab", async () => { + // Earlier tests' events may still be posting; only this one's calls count. + await settleBackgroundTasks(); + slackTestState.calls = []; const res = await signAndPost(app, { type: "event_callback", team_id: "T_KNOWN", @@ -177,7 +194,7 @@ describe("handleSlackEvent", () => { }); expect(res.status).toBe(200); - await new Promise((r) => setTimeout(r, 50)); + await settleBackgroundTasks(); expect(slackTestState.calls.length).toBe(0); }); @@ -1412,7 +1429,7 @@ describe("greeting on first contact", () => { test("greets when the Messages tab is opened", async () => { await homeOpened("messages"); - await new Promise((r) => setTimeout(r, 50)); + await settleBackgroundTasks(); expect(welcomes()).toHaveLength(1); // Top-level in the DM: the agent experience has no thread to greet into. @@ -1421,25 +1438,25 @@ describe("greeting on first contact", () => { test("greets a person once, however often they open it", async () => { await homeOpened("messages"); - await new Promise((r) => setTimeout(r, 50)); + await settleBackgroundTasks(); await homeOpened("messages"); - await new Promise((r) => setTimeout(r, 50)); + await settleBackgroundTasks(); expect(welcomes()).toHaveLength(1); }); test("greets each person separately", async () => { await homeOpened("messages", "U1"); - await new Promise((r) => setTimeout(r, 50)); + await settleBackgroundTasks(); await homeOpened("messages", "U2"); - await new Promise((r) => setTimeout(r, 50)); + await settleBackgroundTasks(); expect(welcomes()).toHaveLength(2); }); test("publishes the home view on the Home tab without greeting", async () => { await homeOpened("home"); - await new Promise((r) => setTimeout(r, 50)); + await settleBackgroundTasks(); expect(slackTestState.calls.some((m) => m.method === "views.publish")).toBe( true, @@ -1452,7 +1469,7 @@ describe("greeting on first contact", () => { Promise.reject(new Error("channel_not_found")); await homeOpened("messages"); - await new Promise((r) => setTimeout(r, 50)); + await settleBackgroundTasks(); // Otherwise a transient failure costs them the greeting permanently. expect(redisStore.has("slack:greeted:T_KNOWN:U1")).toBe(false); @@ -1655,3 +1672,114 @@ describe("confirmation cards", () => { ).toBeGreaterThanOrEqual(2); }); }); + +describe("members only", () => { + const app = createTestApp(); + const redisStore = (globalThis as Record) + .__testRedisStore as Map; + + beforeEach(() => { + resetSlackTestState(); + slackTestState.usersInfoImpl = () => + Promise.resolve({ ok: true, user: { profile: {} } }); + }); + + const unlinked = () => `U_UNLINKED_${crypto.randomUUID()}`; + let agentRuns = 0; + + test("a mention from an unlinked user gets a link card, not an answer", async () => { + agentRuns = 0; + slackTestState.runAgentOverride = () => { + agentRuns++; + return Promise.resolve({ + text: "should not run", + toolResults: [], + finishReason: "stop", + stepCount: 1, + hitStepLimit: false, + aborted: false, + }); + }; + const user = unlinked(); + await signAndPost(app, { + type: "event_callback", + team_id: "T_KNOWN", + event_id: `evt_unlinked_${Date.now()}`, + event: { + type: "app_mention", + text: "<@UBOT> what is down?", + user, + channel: "C1", + channel_type: "channel", + ts: `${Date.now()}.71`, + }, + }); + const card = await waitForCall("postEphemeral"); + expect(card?.args.user).toBe(user); + expect(String(card?.args.text)).toContain("Link your openstatus account"); + const blocks = card?.args.blocks as { + type: string; + elements?: { url?: string }[]; + }[]; + const url = blocks.find((b) => b.type === "actions")?.elements?.[0] + ?.url as string; + expect(url).toContain("/settings/integrations/slack/link?token="); + expect(agentRuns).toBe(0); + }); + + test("the link card is sent once per window on passive surfaces", async () => { + const user = unlinked(); + for (const suffix of ["81", "82"]) { + await signAndPost(app, { + type: "event_callback", + team_id: "T_KNOWN", + event_id: `evt_unlinked_twice_${suffix}_${Date.now()}`, + event: { + type: "app_mention", + text: "<@UBOT> hello", + user, + channel: "C1", + channel_type: "channel", + ts: `${Date.now()}.${suffix}`, + }, + }); + await settleBackgroundTasks(); + } + expect( + slackTestState.calls.filter((c) => c.method === "postEphemeral"), + ).toHaveLength(1); + expect(redisStore.has(`slack:linkcard:T_KNOWN:${user}`)).toBe(true); + }); + + test("an unlinked user in the agent pane gets the card in the thread", async () => { + const user = unlinked(); + await signAndPost(app, { + type: "event_callback", + team_id: "T_KNOWN", + event_id: `evt_unlinked_im_${Date.now()}`, + event: { + type: "message", + text: "hi", + user, + channel: "D1", + channel_type: "im", + ts: `${Date.now()}.91`, + }, + }); + const card = await waitForCall("postMessage"); + expect(String(card?.args.text)).toContain("Link your openstatus account"); + expect(card?.args.channel).toBe("D1"); + }); + + test("the home tab shows the link view to an unlinked user", async () => { + await signAndPost(app, { + type: "event_callback", + team_id: "T_KNOWN", + event_id: `evt_unlinked_home_${Date.now()}`, + event: { type: "app_home_opened", tab: "home", user: unlinked() }, + }); + const publish = await waitForCall("views.publish"); + const view = publish?.args.view as { blocks: { type: string }[] }; + expect(view.blocks.some((b) => b.type === "actions")).toBe(true); + }); +}); diff --git a/apps/server/src/routes/slack/handler.ts b/apps/server/src/routes/slack/handler.ts index c71b886c..c5cfb531 100644 --- a/apps/server/src/routes/slack/handler.ts +++ b/apps/server/src/routes/slack/handler.ts @@ -7,11 +7,14 @@ import { z } from "zod"; import { type AgentEvents, runAgent } from "./agent"; import { greetOnce, setAssistantStatus, setSessionStatus } from "./assistant"; +import { runInBackground } from "./background"; import { type Block, buildAnswerMessage, buildConfirmationBlocks, + buildLinkAccountBlocks, getConfirmationText, + LINK_ACCOUNT_TEXT, type RefResolvers, } from "./blocks"; import { @@ -22,9 +25,10 @@ import { recallContext, rememberContext, } from "./channel-context"; +import type { SlackConfig, SlackEnv } from "./config"; import { draftKey, findByThread, replace, store } from "./confirmation-store"; import type { PendingPayload } from "./confirmation-store"; -import { publishHomeView } from "./home"; +import { publishHomeView, publishLinkAccountView } from "./home"; import { getComponentNames, getPageDashboardLink, @@ -35,7 +39,12 @@ import { isSlackToolDraft, type SlackToolDraft, } from "./registry-runner"; -import { resolveSlackUserId } from "./resolve-slack-user"; +import { + claimLinkCardWindow, + linkAccountUrl, + releaseLinkCardWindow, + requireSlackMember, +} from "./require-slack-member"; import { abortTurn, endTurn, startTurn } from "./running-turns"; import { buildThreadTitle, @@ -212,8 +221,9 @@ export function isAnswerToAgent( return starter?.user === message.user; } -export async function handleSlackEvent(c: Context) { +export async function handleSlackEvent(c: Context) { const body = c.get("slackBody") as SlackEvent; + const config = c.get("slackConfig"); if (body.type === "url_verification") { return c.json({ challenge: body.challenge }); @@ -227,19 +237,50 @@ export async function handleSlackEvent(c: Context) { return c.json({ ok: true }); } - const promise = processEvent(body); - promise.catch((err) => - logger.error("slack event processing error", { - error: err, - teamId: body.team_id, - eventId: body.event_id, - }), - ); + runInBackground("event", () => processEvent(body, config), { + teamId: body.team_id, + eventId: body.event_id, + }); return c.json({ ok: true }); } -async function processEvent(body: SlackEvent) { +/** + * Tells an unlinked Slack user how to link their account. Passive surfaces + * (mentions, DMs) send it at most once per window, so a chatty user isn't + * flooded with cards. + */ +async function sendLinkCard(args: { + config: SlackConfig; + workspaceId: number; + teamId: string; + slackUserId: string; + post: (message: { + text: string; + blocks: Block[]; + }) => Promise<{ ok?: boolean }>; +}): Promise { + const { config, workspaceId, teamId, slackUserId, post } = args; + if (!(await claimLinkCardWindow(teamId, slackUserId))) return; + try { + const url = await linkAccountUrl(config, { + workspaceId, + teamId, + slackUserId, + }); + await post({ + text: LINK_ACCOUNT_TEXT, + blocks: buildLinkAccountBlocks(url), + }); + } catch (err) { + // Otherwise a transient failure silences the card for the whole window. + await releaseLinkCardWindow(teamId, slackUserId).catch(() => undefined); + throw err; + } + logger.info("slack link card sent", { teamId, slackUserId }); +} + +async function processEvent(body: SlackEvent, config: SlackConfig) { const event = body.event; if (!event) return; @@ -281,15 +322,33 @@ async function processEvent(body: SlackEvent) { const resolved = await resolveWorkspace(teamId); if (!resolved) return; const slack = new WebClient(resolved.botToken); + const member = { workspace: resolved.workspace, teamId, slack }; if (tab === "messages") { - if (!event.channel) return; + const channel = event.channel; + if (!channel) return; + const actor = await requireSlackMember({ + ...member, + slackUserId: userId, + }); + if (!actor) { + await sendLinkCard({ + config, + workspaceId: resolved.workspace.id, + teamId, + slackUserId: userId, + post: (message) => slack.chat.postMessage({ channel, ...message }), + }).catch((error) => + logger.error("slack failed to send link card", { error, teamId }), + ); + return; + } try { await greetOnce({ slack, teamId, userId, - channel: event.channel, + channel, }); } catch (err) { logger.error("slack failed to greet user", { error: err, teamId }); @@ -298,7 +357,20 @@ async function processEvent(body: SlackEvent) { } try { - await publishHomeView(slack, userId); + const actor = await requireSlackMember({ + ...member, + slackUserId: userId, + }); + if (actor) { + await publishHomeView(slack, userId); + } else { + const url = await linkAccountUrl(config, { + workspaceId: resolved.workspace.id, + teamId, + slackUserId: userId, + }); + await publishLinkAccountView(slack, userId, url); + } } catch (err) { logger.error("slack failed to publish home view", { error: err, teamId }); } @@ -314,11 +386,34 @@ async function processEvent(body: SlackEvent) { if (!teamId || !thread?.user_id) return; const resolved = await resolveWorkspace(teamId); if (!resolved) return; + const slack = new WebClient(resolved.botToken); + const slackUserId = thread.user_id; try { + const actor = await requireSlackMember({ + workspace: resolved.workspace, + teamId, + slackUserId, + slack, + }); + if (!actor) { + await sendLinkCard({ + config, + workspaceId: resolved.workspace.id, + teamId, + slackUserId, + post: (message) => + slack.chat.postMessage({ + channel: thread.channel_id, + thread_ts: thread.thread_ts, + ...message, + }), + }); + return; + } await greetOnce({ - slack: new WebClient(resolved.botToken), + slack, teamId, - userId: thread.user_id, + userId: slackUserId, channel: thread.channel_id, threadTs: thread.thread_ts, }); @@ -476,6 +571,36 @@ async function processEvent(body: SlackEvent) { agentThread: isAgentThread, }); + const channel = event.channel; + const slackUserId = event.user; + if (!slackUserId) return; + const actor = await requireSlackMember({ + workspace: resolved.workspace, + teamId, + slackUserId, + slack, + }); + if (!actor) { + await sendLinkCard({ + config, + workspaceId: resolved.workspace.id, + teamId, + slackUserId, + post: (message) => + isAgentThread + ? slack.chat.postMessage({ channel, thread_ts: threadTs, ...message }) + : slack.chat.postEphemeral({ + channel, + user: slackUserId, + thread_ts: event.thread_ts, + ...message, + }), + }).catch((error) => + logger.error("slack failed to send link card", { error, teamId }), + ); + return; + } + // Registered before the session is marked `processing`: a stop landing in // that window has to find a controller, or the turn runs on unstoppable. const turn = startTurn(event.channel, threadTs); @@ -502,15 +627,6 @@ async function processEvent(body: SlackEvent) { return; } - // Only once we answer: an ignored event must not cost a Slack call. - // Overlaps the thread fetch; never rejects, so an early return can drop it. - const slackMember = resolveSlackUserId({ - workspace: resolved.workspace, - teamId, - slackUserId: event.user ?? "", - slack, - }); - let thread: ThreadMessage[] = []; if (prefetchedThread) { thread = prefetchedThread; @@ -545,11 +661,7 @@ async function processEvent(body: SlackEvent) { thread, botUserId, event.text, - { - slackUserId: event.user ?? "", - teamId, - userId: (await slackMember) ?? undefined, - }, + actor, { events: reply.progress, signal: turn.signal, diff --git a/apps/server/src/routes/slack/home.ts b/apps/server/src/routes/slack/home.ts index 39a20591..824df333 100644 --- a/apps/server/src/routes/slack/home.ts +++ b/apps/server/src/routes/slack/home.ts @@ -1,6 +1,8 @@ import type { WebClient } from "@slack/web-api"; import type { KnownBlock } from "@slack/web-api"; +import { buildLinkAccountBlocks } from "./blocks"; + export const DOCS_URL = "https://www.openstatus.dev/docs"; export function buildHomeBlocks(): KnownBlock[] { @@ -52,3 +54,14 @@ export async function publishHomeView( view: { type: "home", blocks: buildHomeBlocks() }, }); } + +export async function publishLinkAccountView( + slack: WebClient, + userId: string, + url: string, +): Promise { + await slack.views.publish({ + user_id: userId, + view: { type: "home", blocks: buildLinkAccountBlocks(url) }, + }); +} diff --git a/apps/server/src/routes/slack/index.test.ts b/apps/server/src/routes/slack/index.test.ts index cadf7d6a..6cb69fe9 100644 --- a/apps/server/src/routes/slack/index.test.ts +++ b/apps/server/src/routes/slack/index.test.ts @@ -19,7 +19,7 @@ function signRequest(body: string, timestamp: number): string { } function makeInstallToken(workspaceId: number): string { - const payload = JSON.stringify({ workspaceId, ts: Date.now() }); + const payload = JSON.stringify({ workspaceId, userId: 1, ts: Date.now() }); const sig = crypto .createHmac("sha256", SIGNING_SECRET) .update(payload) diff --git a/apps/server/src/routes/slack/interactions.test.ts b/apps/server/src/routes/slack/interactions.test.ts index 06ae3b13..75c0e503 100644 --- a/apps/server/src/routes/slack/interactions.test.ts +++ b/apps/server/src/routes/slack/interactions.test.ts @@ -22,7 +22,6 @@ import { import { settleBackgroundTasks } from "./background"; import type { SlackEnv } from "./config"; import { handleSlackInteraction } from "./interactions"; -import { resetSlackUserCache } from "./resolve-slack-user"; import { verifySlackSignature } from "./verify"; const redisStore = (globalThis as Record) @@ -41,9 +40,12 @@ const basePending = { function configureSlackDoubles() { slackTestState.calls = []; - resetSlackUserCache(); + // The seeded member of workspace 1, so the clicking user is linked. slackTestState.usersInfoImpl = () => - Promise.resolve({ ok: true, user: { profile: {} } }); + Promise.resolve({ + ok: true, + user: { profile: { email: "ping@openstatus.dev" } }, + }); slackTestState.resolveWorkspace = (teamId: string) => teamId === "T_KNOWN" ? Promise.resolve({ botToken: "xoxb-fallback", workspace: { id: 1 } }) @@ -320,6 +322,62 @@ describe("handleSlackInteraction (dispatch)", () => { }); }); +describe("handleSlackInteraction (members only)", () => { + const app = createTestApp(); + + beforeEach(() => { + configureSlackDoubles(); + redisStore.clear(); + slackTestState.usersInfoImpl = () => + Promise.resolve({ ok: true, user: { profile: {} } }); + }); + + function seedUnlinked(id: string, slackUserId: string) { + const data = { ...seedCreateMaintenance(id), userId: slackUserId }; + redisStore.set(`slack:action:${id}`, JSON.stringify(data)); + } + + test("an unlinked approver gets the link card and the draft stays live", async () => { + const slackUserId = `U_UNLINKED_${crypto.randomUUID()}`; + seedUnlinked("maint-unlinked", slackUserId); + const res = await signAndPost(app, { + type: "block_actions", + user: { id: slackUserId }, + channel: { id: "C1" }, + message: { ts: "2.2" }, + team: { id: "T_KNOWN" }, + actions: [{ action_id: "approve_maint-unlinked" }], + }); + expect(res.status).toBe(200); + expect(redisStore.has("slack:action:maint-unlinked")).toBe(true); + const ephemeral = slackTestState.calls.find( + (c) => c.method === "postEphemeral", + ); + expect(ephemeral?.args.text).toContain("Link your openstatus account"); + expect(slackTestState.calls.some((c) => c.method === "update")).toBe(false); + }); + + test("an unlinked initiator can still cancel their own draft", async () => { + const slackUserId = `U_UNLINKED_${crypto.randomUUID()}`; + seedUnlinked("maint-unlinked-cancel", slackUserId); + const res = await signAndPost(app, { + type: "block_actions", + user: { id: slackUserId }, + channel: { id: "C1" }, + message: { ts: "2.2" }, + team: { id: "T_KNOWN" }, + actions: [{ action_id: "cancel_maint-unlinked-cancel" }], + }); + expect(res.status).toBe(200); + expect(redisStore.has("slack:action:maint-unlinked-cancel")).toBe(false); + const cancelled = slackTestState.calls.find( + (c) => + c.method === "update" && c.args.text === ":no_entry_sign: Cancelled.", + ); + expect(cancelled).toBeDefined(); + }); +}); + describe("registry-runner execution paths", () => { const app = createTestApp(); diff --git a/apps/server/src/routes/slack/interactions.ts b/apps/server/src/routes/slack/interactions.ts index 650078f8..1398a87e 100644 --- a/apps/server/src/routes/slack/interactions.ts +++ b/apps/server/src/routes/slack/interactions.ts @@ -4,12 +4,22 @@ import { WebClient } from "@slack/web-api"; import type { Context } from "hono"; import { runInBackground } from "./background"; -import { type ParsedActionId, parseActionId } from "./blocks"; +import { + buildLinkAccountBlocks, + LINK_ACCOUNT_TEXT, + type ParsedActionId, + parseActionId, +} from "./blocks"; +import type { SlackConfig, SlackEnv } from "./config"; import { consume, get } from "./confirmation-store"; import type { PendingAction } from "./confirmation-store"; import { renderToolResult } from "./presenters"; import { executeRegistryAction, getRegistryTool } from "./registry-runner"; -import { resolveSlackUserId } from "./resolve-slack-user"; +import { + linkAccountUrl, + requireSlackMember, + type SlackActor, +} from "./require-slack-member"; import { toServiceCtx } from "./service-adapter"; import { resolveWorkspace } from "./workspace-resolver"; @@ -24,8 +34,9 @@ interface SlackInteractionPayload { actions: Array<{ action_id: string; value?: string }>; } -export function handleSlackInteraction(c: Context) { +export function handleSlackInteraction(c: Context) { const payload = c.get("slackBody") as SlackInteractionPayload; + const config = c.get("slackConfig"); if (payload.type !== "block_actions" || !payload.actions?.length) { return c.json({ ok: true }); @@ -38,10 +49,14 @@ export function handleSlackInteraction(c: Context) { // the status page — well past Slack's 3s ack window, which would mark the // click as failed even though it worked. Ack now; the card is updated with // the outcome when the work finishes. - runInBackground("interaction", () => processInteraction(parsed, payload), { - actionId: payload.actions[0].action_id, - teamId: payload.team?.id, - }); + runInBackground( + "interaction", + () => processInteraction(parsed, payload, config), + { + actionId: payload.actions[0].action_id, + teamId: payload.team?.id, + }, + ); return c.json({ ok: true }); } @@ -49,6 +64,7 @@ export function handleSlackInteraction(c: Context) { async function processInteraction( parsed: ParsedActionId, payload: SlackInteractionPayload, + config: SlackConfig, ) { const channelId = payload.channel.id; const messageTs = payload.message.ts; @@ -106,16 +122,38 @@ async function processInteraction( return; } - // TODO: refuse an unmapped approver here, before `consume`, with an ephemeral - // "link your openstatus account" so the card stays live. Every Slack - // mutation must resolve to a member (decision 2026-09-28); Cancel is exempt. - // + // Checked before `consume` so the card stays live while they link. Cancel is + // exempt: the initiator can always dismiss their own draft. + let actor: SlackActor | null = null; + if (parsed.kind !== "cancel") { + actor = await requireSlackMember({ + workspace: resolved.workspace, + teamId: workspaceTeamId, + slackUserId: userId, + slack, + }); + if (!actor) { + const url = await linkAccountUrl(config, { + workspaceId: resolved.workspace.id, + teamId: workspaceTeamId, + slackUserId: userId, + }); + await slack.chat.postEphemeral({ + channel: channelId, + user: userId, + text: LINK_ACCOUNT_TEXT, + blocks: buildLinkAccountBlocks(url), + }); + return; + } + } + // Atomic consume — prevents double execution from concurrent requests // (e.g. double-click). If another request already won, return. const consumed = await consume(parsed.pendingId); if (!consumed) return; - if (parsed.kind === "cancel") { + if (parsed.kind === "cancel" || !actor) { await slack.chat.update({ channel: channelId, ts: messageTs, @@ -125,14 +163,6 @@ async function processInteraction( return; } - // Attribution only until the gate above lands; moves up with it. - const memberId = await resolveSlackUserId({ - workspace: resolved.workspace, - teamId: workspaceTeamId, - slackUserId: userId, - slack, - }); - try { await runAndPresent({ pending: consumed, @@ -140,9 +170,7 @@ async function processInteraction( slack, channelId, messageTs, - slackUserId: userId, - teamId: workspaceTeamId, - userId: memberId ?? undefined, + actor, }); } catch (err) { logger.error("slack action execution error", { @@ -166,20 +194,9 @@ async function runAndPresent(args: { slack: WebClient; channelId: string; messageTs: string; - slackUserId: string; - teamId: string; - userId?: number; + actor: SlackActor; }) { - const { - pending, - flag, - slack, - channelId, - messageTs, - slackUserId, - teamId, - userId, - } = args; + const { pending, flag, slack, channelId, messageTs, actor } = args; const tool = getRegistryTool(pending.payload.toolName); if (!tool) { throw new Error( @@ -187,7 +204,7 @@ async function runAndPresent(args: { ); } - const ctx = await toServiceCtx({ pending, slackUserId, teamId, userId }); + const ctx = await toServiceCtx({ pending, actor }); const flagId = tool.approval?.extraFlags?.[0]?.id; const flags: Record = flagId ? { [flagId]: flag } : {}; diff --git a/apps/server/src/routes/slack/oauth.test.ts b/apps/server/src/routes/slack/oauth.test.ts index e8b68de2..27e8a331 100644 --- a/apps/server/src/routes/slack/oauth.test.ts +++ b/apps/server/src/routes/slack/oauth.test.ts @@ -19,7 +19,11 @@ function createTestApp() { return app; } -function signToken(data: { workspaceId: number; ts: number }): string { +function signToken(data: { + workspaceId: number; + userId?: number; + ts: number; +}): string { const payload = JSON.stringify(data); const signature = crypto .createHmac("sha256", SIGNING_SECRET) @@ -29,11 +33,11 @@ function signToken(data: { workspaceId: number; ts: number }): string { } function encodeState(state: { workspaceId: number; ts: number }): string { - return signToken(state); + return signToken({ userId: 1, ...state }); } function makeInstallToken(workspaceId: number): string { - return signToken({ workspaceId, ts: Date.now() }); + return signToken({ workspaceId, userId: 1, ts: Date.now() }); } describe("handleSlackInstall", () => { @@ -72,6 +76,7 @@ describe("handleSlackInstall", () => { test("returns 403 for expired token", async () => { const expired = signToken({ workspaceId: 1, + userId: 1, ts: Date.now() - 10 * 60 * 1000, }); const res = await app.request(`/slack/install?token=${expired}`); @@ -164,7 +169,11 @@ describe("handleSlackOAuthCallback", () => { }); test("returns 400 for tampered state", async () => { - const payload = JSON.stringify({ workspaceId: 1, ts: Date.now() }); + const payload = JSON.stringify({ + workspaceId: 1, + userId: 1, + ts: Date.now(), + }); const tamperedState = Buffer.from(`${payload}.invalidsignature`).toString( "base64url", ); diff --git a/apps/server/src/routes/slack/oauth.ts b/apps/server/src/routes/slack/oauth.ts index a7c9218c..a877d0e6 100644 --- a/apps/server/src/routes/slack/oauth.ts +++ b/apps/server/src/routes/slack/oauth.ts @@ -8,6 +8,7 @@ import { } from "@openstatus/db/src/schema"; import { installSlackAgent } from "@openstatus/services/integration"; import type { Context } from "hono"; +import { z } from "zod"; import type { SlackConfig, SlackEnv } from "./config"; @@ -32,13 +33,12 @@ const BOT_SCOPES = [ "users:read.email", ].join(","); -interface OAuthState { - workspaceId: number; - // The openstatus user who initiated the install. Optional so in-flight - // installs that started before this field was added still parse. - userId?: number; - ts: number; -} +const oauthStateSchema = z.object({ + workspaceId: z.number().int(), + userId: z.number().int(), + ts: z.number(), +}); +type OAuthState = z.infer; interface SlackOAuthResponse { ok: boolean; @@ -219,7 +219,8 @@ function decodeState(config: SlackConfig, encoded: string): OAuthState | null { if (!verifyHmac(config, payload, signature)) return null; - return JSON.parse(payload) as OAuthState; + const parsed = oauthStateSchema.safeParse(JSON.parse(payload)); + return parsed.success ? parsed.data : null; } catch { return null; } @@ -246,7 +247,7 @@ const INSTALL_TOKEN_TTL_MS = 5 * 60 * 1000; function verifyInstallToken( config: SlackConfig, token: string, -): { workspaceId: number; userId?: number } | null { +): { workspaceId: number; userId: number } | null { try { const decoded = Buffer.from(token, "base64url").toString(); const dotIdx = decoded.lastIndexOf("."); @@ -257,14 +258,11 @@ function verifyInstallToken( if (!verifyHmac(config, payload, signature)) return null; - const data = JSON.parse(payload) as { - workspaceId: number; - userId?: number; - ts: number; - }; - if (Date.now() - data.ts > INSTALL_TOKEN_TTL_MS) return null; + const parsed = oauthStateSchema.safeParse(JSON.parse(payload)); + if (!parsed.success) return null; + if (Date.now() - parsed.data.ts > INSTALL_TOKEN_TTL_MS) return null; - return { workspaceId: data.workspaceId, userId: data.userId }; + return { workspaceId: parsed.data.workspaceId, userId: parsed.data.userId }; } catch { return null; } diff --git a/apps/server/src/routes/slack/require-slack-member.ts b/apps/server/src/routes/slack/require-slack-member.ts new file mode 100644 index 00000000..a42ccf7f --- /dev/null +++ b/apps/server/src/routes/slack/require-slack-member.ts @@ -0,0 +1,70 @@ +import type { Workspace } from "@openstatus/db/src/schema/workspaces/validation"; +import { signSlackLinkToken } from "@openstatus/services/slack-user"; +import type { WebClient } from "@slack/web-api"; + +import { redis } from "@/libs/clients"; + +import type { SlackConfig } from "./config"; +import { resolveSlackMember } from "./resolve-slack-user"; + +export type SlackActor = { + type: "slack"; + teamId: string; + slackUserId: string; + userId: number; +}; + +const LINK_CARD_WINDOW_SECONDS = 10 * 60; + +/** The Slack actor for a linked member, or `null` for anyone else. */ +export async function requireSlackMember(args: { + workspace: Workspace; + teamId: string; + slackUserId: string; + slack: WebClient; +}): Promise { + const userId = await resolveSlackMember(args); + if (userId === null) return null; + return { + type: "slack", + teamId: args.teamId, + slackUserId: args.slackUserId, + userId, + }; +} + +export async function linkAccountUrl( + config: SlackConfig, + input: { workspaceId: number; teamId: string; slackUserId: string }, +): Promise { + if (!config.signingSecret) { + throw new Error("Slack signing secret not configured"); + } + const token = await signSlackLinkToken(config.signingSecret, input); + const params = new URLSearchParams({ token }); + return `${config.dashboardUrl}/settings/integrations/slack/link?${params.toString()}`; +} + +function linkCardKey(teamId: string, slackUserId: string): string { + return `slack:linkcard:${teamId}:${slackUserId}`; +} + +/** Claims the per-user link-card window; `false` means one was sent recently. */ +export async function claimLinkCardWindow( + teamId: string, + slackUserId: string, +): Promise { + const claimed = await redis.set(linkCardKey(teamId, slackUserId), "1", { + nx: true, + ex: LINK_CARD_WINDOW_SECONDS, + }); + return claimed !== null; +} + +/** Frees the window again when the card never made it out. */ +export async function releaseLinkCardWindow( + teamId: string, + slackUserId: string, +): Promise { + await redis.del(linkCardKey(teamId, slackUserId)); +} diff --git a/apps/server/src/routes/slack/resolve-slack-user.test.ts b/apps/server/src/routes/slack/resolve-slack-user.test.ts index 58540342..08855ee3 100644 --- a/apps/server/src/routes/slack/resolve-slack-user.test.ts +++ b/apps/server/src/routes/slack/resolve-slack-user.test.ts @@ -1,6 +1,11 @@ -import { selectWorkspaceSchema } from "@openstatus/db/src/schema"; +import { and, db, eq } from "@openstatus/db"; +import { + selectWorkspaceSchema, + usersToWorkspaces, +} from "@openstatus/db/src/schema"; import { addUserToWorkspace, + createSlackUser, createTestWorkspace, createUser, } from "@openstatus/db/src/test/factories"; @@ -12,7 +17,7 @@ import { beforeAll, beforeEach, describe, test } from "@std/testing/bdd"; // slackTestState.usersInfoImpl. import { slackTestState } from "@/libs/test/doubles/slack-test-state"; -import { resetSlackUserCache, resolveSlackUserId } from "./resolve-slack-user"; +import { resolveSlackMember } from "./resolve-slack-user"; let workspace: ReturnType; let memberId: number; @@ -25,6 +30,8 @@ const withEmail = (email?: string) => () => const infoCalls = () => slackTestState.calls.filter((c) => c.method === "users.info").length; +const slackId = () => `U_${crypto.randomUUID()}`; + beforeAll(async () => { const fixture = await createTestWorkspace(); workspace = selectWorkspaceSchema.parse(fixture.workspace); @@ -37,29 +44,34 @@ beforeAll(async () => { beforeEach(() => { slackTestState.calls = []; slackTestState.usersInfoImpl = withEmail(undefined); - resetSlackUserCache(); }); -describe("resolveSlackUserId", () => { - test("matches the member by email and caches the hit", async () => { +describe("resolveSlackMember", () => { + test("links the member by email once, then reads the stored link", async () => { slackTestState.usersInfoImpl = withEmail(memberEmail.toUpperCase()); - const args = { workspace, teamId: "T1", slackUserId: "U_A", slack }; - expect(await resolveSlackUserId(args)).toBe(memberId); - expect(await resolveSlackUserId(args)).toBe(memberId); + const args = { workspace, teamId: "T1", slackUserId: slackId(), slack }; + expect(await resolveSlackMember(args)).toBe(memberId); + expect(await resolveSlackMember(args)).toBe(memberId); expect(infoCalls()).toBe(1); }); - test("a miss is not retried until the cache is cleared", async () => { - const args = { workspace, teamId: "T1", slackUserId: "U_B", slack }; - expect(await resolveSlackUserId(args)).toBeNull(); - expect(infoCalls()).toBe(1); + test("an existing link wins without calling Slack", async () => { + const slackUserId = slackId(); + await createSlackUser(workspace.id, memberId, { + slackTeamId: "T1", + slackUserId, + }); + expect( + await resolveSlackMember({ workspace, teamId: "T1", slackUserId, slack }), + ).toBe(memberId); + expect(infoCalls()).toBe(0); + }); + test("a miss is retried on the next interaction", async () => { + const args = { workspace, teamId: "T1", slackUserId: slackId(), slack }; + expect(await resolveSlackMember(args)).toBeNull(); slackTestState.usersInfoImpl = withEmail(memberEmail); - expect(await resolveSlackUserId(args)).toBeNull(); - expect(infoCalls()).toBe(1); - - resetSlackUserCache(); - expect(await resolveSlackUserId(args)).toBe(memberId); + expect(await resolveSlackMember(args)).toBe(memberId); expect(infoCalls()).toBe(2); }); @@ -67,34 +79,56 @@ describe("resolveSlackUserId", () => { const outsider = await createUser(); slackTestState.usersInfoImpl = withEmail(outsider.email as string); expect( - await resolveSlackUserId({ + await resolveSlackMember({ workspace, teamId: "T1", - slackUserId: "U_C", + slackUserId: slackId(), slack, }), ).toBeNull(); }); - test("swallows Slack errors such as missing_scope without caching them", async () => { - slackTestState.usersInfoImpl = () => - Promise.reject( - Object.assign(new Error("An API error occurred: missing_scope"), { - data: { ok: false, error: "missing_scope" }, - }), + test("a removed member's link no longer resolves", async () => { + const leaver = await createUser(); + await addUserToWorkspace(leaver.id, workspace.id, "member"); + const slackUserId = slackId(); + await createSlackUser(workspace.id, leaver.id, { + slackTeamId: "T1", + slackUserId, + }); + expect( + await resolveSlackMember({ workspace, teamId: "T1", slackUserId, slack }), + ).toBe(leaver.id); + + await db + .delete(usersToWorkspaces) + .where( + and( + eq(usersToWorkspaces.userId, leaver.id), + eq(usersToWorkspaces.workspaceId, workspace.id), + ), ); - const args = { workspace, teamId: "T1", slackUserId: "U_D", slack }; - expect(await resolveSlackUserId(args)).toBeNull(); - expect(infoCalls()).toBe(1); + expect( + await resolveSlackMember({ workspace, teamId: "T1", slackUserId, slack }), + ).toBeNull(); + }); - slackTestState.usersInfoImpl = withEmail(memberEmail); - expect(await resolveSlackUserId(args)).toBe(memberId); - expect(infoCalls()).toBe(2); + test("swallows Slack errors such as missing_scope", async () => { + slackTestState.usersInfoImpl = () => + Promise.reject(new Error("An API error occurred: missing_scope")); + expect( + await resolveSlackMember({ + workspace, + teamId: "T1", + slackUserId: slackId(), + slack, + }), + ).toBeNull(); }); test("skips resolution entirely without a team or user id", async () => { expect( - await resolveSlackUserId({ + await resolveSlackMember({ workspace, teamId: "", slackUserId: "U_E", @@ -102,7 +136,7 @@ describe("resolveSlackUserId", () => { }), ).toBeNull(); expect( - await resolveSlackUserId({ + await resolveSlackMember({ workspace, teamId: "T1", slackUserId: "", diff --git a/apps/server/src/routes/slack/resolve-slack-user.ts b/apps/server/src/routes/slack/resolve-slack-user.ts index 33287e99..c65d58b7 100644 --- a/apps/server/src/routes/slack/resolve-slack-user.ts +++ b/apps/server/src/routes/slack/resolve-slack-user.ts @@ -2,35 +2,19 @@ import { getLogger } from "@logtape/logtape"; import type { Workspace } from "@openstatus/db/src/schema/workspaces/validation"; import type { ServiceContext } from "@openstatus/services"; import { findMemberIdByEmail } from "@openstatus/services/member"; +import { + createSlackUserMapping, + getSlackUserMapping, +} from "@openstatus/services/slack-user"; import type { WebClient } from "@slack/web-api"; const logger = getLogger(["api-server", "slack", "resolve-user"]); -// Per process, like the event dedup in handler.ts. Nothing is persisted: the -// audit row and the `*_by` columns record the resolved id where it mattered. -// Misses expire sooner so a freshly invited member is picked up quickly. -const HIT_TTL_MS = 60 * 60_000; -const MISS_TTL_MS = 10 * 60_000; -const cache = new Map(); - -export function resetSlackUserCache() { - cache.clear(); -} - -function readCache(key: string): number | null | undefined { - const now = Date.now(); - for (const [k, entry] of cache) { - if (entry.expiresAt <= now) cache.delete(k); - } - return cache.get(key)?.userId; -} - /** - * openstatus member behind a Slack user, matched by verified profile email. - * Attribution only, so every failure (missing `users:read.email` scope, no - * email, no unique member) is `null`. + * The openstatus member linked to a Slack user, or `null`. A missing link is + * created on the spot when the Slack profile email matches exactly one member. */ -export async function resolveSlackUserId(args: { +export async function resolveSlackMember(args: { workspace: Workspace; teamId: string; slackUserId: string; @@ -39,33 +23,33 @@ export async function resolveSlackUserId(args: { const { workspace, teamId, slackUserId, slack } = args; if (!teamId || !slackUserId) return null; - const key = `${workspace.id}:${teamId}:${slackUserId}`; - const hit = readCache(key); - if (hit !== undefined) return hit; - - let userId: number | null = null; + const ctx: ServiceContext = { + workspace, + actor: { type: "system", job: "slack-user-automap" }, + }; try { + const linked = await getSlackUserMapping({ + ctx, + input: { teamId, slackUserId }, + }); + if (linked !== null) return linked; + const info = await slack.users.info({ user: slackUserId }); const email = info.user?.profile?.email; - if (email) { - const ctx: ServiceContext = { - workspace, - actor: { type: "system", job: "slack-user-automap" }, - }; - userId = await findMemberIdByEmail({ ctx, input: { email } }); - } - // Only a completed lookup is cached; a thrown call may be transient. - cache.set(key, { - userId, - expiresAt: Date.now() + (userId === null ? MISS_TTL_MS : HIT_TTL_MS), + if (!email) return null; + const userId = await findMemberIdByEmail({ ctx, input: { email } }); + if (userId === null) return null; + await createSlackUserMapping({ + ctx, + input: { teamId, slackUserId, userId }, }); + return userId; } catch (err) { - // `missing_scope` until the workspace reinstalls with users:read.email. logger.warn("slack user resolution failed", { workspaceId: workspace.id, teamId, - error: (err as { data?: { error?: string } })?.data?.error ?? String(err), + error: err instanceof Error ? err.message : String(err), }); + return null; } - return userId; } diff --git a/apps/server/src/routes/slack/service-adapter.ts b/apps/server/src/routes/slack/service-adapter.ts index 25e867d0..c48a71d4 100644 --- a/apps/server/src/routes/slack/service-adapter.ts +++ b/apps/server/src/routes/slack/service-adapter.ts @@ -6,18 +6,16 @@ import { import type { ServiceContext } from "@openstatus/services"; import type { PendingAction } from "./confirmation-store"; +import type { SlackActor } from "./require-slack-member"; /** * Build a `ServiceContext` for a Slack-originated action. Loads the * workspace fresh since `PendingAction` only stores its id, and we want * services to see the latest plan/limits state at execution time. - * `userId` is the member the caller resolved from the Slack email, if any. */ export async function toServiceCtx(args: { pending: PendingAction; - slackUserId: string; - teamId: string | undefined; - userId?: number; + actor: SlackActor; requestId?: string; }): Promise { const row = await db @@ -32,12 +30,7 @@ export async function toServiceCtx(args: { } return { workspace: selectWorkspaceSchema.parse(row), - actor: { - type: "slack", - teamId: args.teamId ?? "", - slackUserId: args.slackUserId, - userId: args.userId, - }, + actor: args.actor, requestId: args.requestId, }; } diff --git a/packages/api/src/lambda.ts b/packages/api/src/lambda.ts index 4a5cc87f..caab8039 100644 --- a/packages/api/src/lambda.ts +++ b/packages/api/src/lambda.ts @@ -2,6 +2,7 @@ import { apiKeyRouter } from "./router/apiKey"; import { blobRouter } from "./router/blob"; import { emailRouter } from "./router/email"; import { integrationRouter } from "./router/integration"; +import { slackUserRouter } from "./router/slackUser"; import { ssoRouter } from "./router/sso"; import { stripeRouter } from "./router/stripe"; import { subscriberNotificationRouter } from "./router/subscriber-notification"; @@ -12,6 +13,7 @@ export const lambdaRouter = createTRPCRouter({ emailRouter: emailRouter, apiKey: apiKeyRouter, integrationRouter: integrationRouter, + slackUser: slackUserRouter, blob: blobRouter, subscriberNotification: subscriberNotificationRouter, sso: ssoRouter, diff --git a/packages/api/src/router/slackUser.test.ts b/packages/api/src/router/slackUser.test.ts new file mode 100644 index 00000000..0d605175 --- /dev/null +++ b/packages/api/src/router/slackUser.test.ts @@ -0,0 +1,71 @@ +import { selectWorkspaceSchema } from "@openstatus/db/src/schema"; +import { + addUserToWorkspace, + createSlackUser, + createUser, + createWorkspace, +} from "@openstatus/db/src/test/factories"; +import { signSlackLinkToken } from "@openstatus/services/slack-user"; +import { expect } from "@std/expect"; +import { beforeAll, test } from "@std/testing/bdd"; +import { TRPCError } from "@trpc/server"; + +import { lambdaRouter } from "../lambda"; +import { createInnerTRPCContext } from "../trpc"; + +const SECRET = "test-slack-signing-secret"; + +let workspace: ReturnType; +let ownerId: number; +let otherId: number; + +// Passing both `user` and `workspace` trips the NODE_ENV=test escape hatch in +// the authed middleware, so the caller is scoped to this suite's workspace. +function callerFor(userId: number) { + const ctx = createInnerTRPCContext({ + req: undefined, + session: { user: { id: String(userId) } }, + // @ts-expect-error - minimal user for test + user: { id: userId }, + workspace, + }); + return lambdaRouter.createCaller(ctx); +} + +beforeAll(async () => { + process.env.SLACK_SIGNING_SECRET = SECRET; + workspace = selectWorkspaceSchema.parse(await createWorkspace()); + ownerId = (await createUser()).id; + otherId = (await createUser()).id; + await addUserToWorkspace(ownerId, workspace.id, "member"); + await addUserToWorkspace(otherId, workspace.id, "member"); +}); + +test("link refuses to move a Slack account linked to another member", async () => { + const linked = await createSlackUser(workspace.id, ownerId); + const token = await signSlackLinkToken(SECRET, { + workspaceId: workspace.id, + teamId: linked.slackTeamId, + slackUserId: linked.slackUserId, + }); + + const err = await callerFor(otherId) + .slackUser.link({ token }) + .catch((e: unknown) => e); + expect(err).toBeInstanceOf(TRPCError); + expect((err as TRPCError).code).toBe("CONFLICT"); + + // The owner of the link can still redeem it. + const row = await callerFor(ownerId).slackUser.link({ token }); + expect(row?.userId).toBe(ownerId); +}); + +test("link maps an unlinked Slack account to the caller", async () => { + const token = await signSlackLinkToken(SECRET, { + workspaceId: workspace.id, + teamId: "T_ROUTER", + slackUserId: `U_${crypto.randomUUID()}`, + }); + const row = await callerFor(otherId).slackUser.link({ token }); + expect(row?.userId).toBe(otherId); +}); diff --git a/packages/api/src/router/slackUser.ts b/packages/api/src/router/slackUser.ts new file mode 100644 index 00000000..456e8e87 --- /dev/null +++ b/packages/api/src/router/slackUser.ts @@ -0,0 +1,95 @@ +import { + createSlackUserMapping, + getSlackUserMapping, + listSlackUserMappings, + verifySlackLinkToken, +} from "@openstatus/services/slack-user"; +import { TRPCError } from "@trpc/server"; +import { z } from "zod"; + +import { toServiceCtx, toTRPCError } from "../service-adapter"; +import { createTRPCRouter, protectedProcedure } from "../trpc"; + +async function readLinkToken(token: string) { + const secret = process.env.SLACK_SIGNING_SECRET; + if (!secret) { + throw new TRPCError({ + code: "PRECONDITION_FAILED", + message: "Slack not configured", + }); + } + return verifySlackLinkToken(secret, token); +} + +export const slackUserRouter = createTRPCRouter({ + previewLink: protectedProcedure + .input(z.object({ token: z.string() })) + .query(async ({ ctx, input }) => { + const payload = await readLinkToken(input.token); + if (!payload) return { status: "invalid" as const }; + if (payload.workspaceId === ctx.workspace.id) { + return { status: "ready" as const, workspaceName: ctx.workspace.name }; + } + const target = ctx.workspaces.find((w) => w.id === payload.workspaceId); + if (target) { + return { + status: "wrong-workspace" as const, + workspaceName: target.name, + workspaceSlug: target.slug, + }; + } + return { status: "not-member" as const }; + }), + + link: protectedProcedure + .input(z.object({ token: z.string() })) + .mutation(async ({ ctx, input }) => { + const payload = await readLinkToken(input.token); + if (!payload) { + throw new TRPCError({ + code: "BAD_REQUEST", + message: "This link is invalid or has expired.", + }); + } + if (payload.workspaceId !== ctx.workspace.id) { + throw new TRPCError({ + code: "FORBIDDEN", + message: "This link belongs to another workspace.", + }); + } + try { + const serviceCtx = toServiceCtx(ctx); + // The token only proves someone saw the card, so it must not move a + // Slack account that already belongs to another member. + const linkedTo = await getSlackUserMapping({ + ctx: serviceCtx, + input: { teamId: payload.teamId, slackUserId: payload.slackUserId }, + }); + if (linkedTo !== null && linkedTo !== ctx.user.id) { + throw new TRPCError({ + code: "CONFLICT", + message: + "This Slack account is already linked to another member of this workspace.", + }); + } + return await createSlackUserMapping({ + ctx: serviceCtx, + input: { + teamId: payload.teamId, + slackUserId: payload.slackUserId, + userId: ctx.user.id, + }, + }); + } catch (err) { + toTRPCError(err); + } + }), + + list: protectedProcedure.query(async ({ ctx }) => { + try { + return await listSlackUserMappings({ ctx: toServiceCtx(ctx) }); + } catch (err) { + toTRPCError(err); + } + }), +}); diff --git a/packages/services/src/auth/__tests__/require-scope.test.ts b/packages/services/src/auth/__tests__/require-scope.test.ts index cd46a176..b2c708d8 100644 --- a/packages/services/src/auth/__tests__/require-scope.test.ts +++ b/packages/services/src/auth/__tests__/require-scope.test.ts @@ -80,6 +80,7 @@ describe("requireScope", () => { type: "slack", teamId: "T1", slackUserId: "U1", + userId: 1, }); expect(() => requireScope(ctx, "write")).not.toThrow(); }); diff --git a/packages/services/src/context.ts b/packages/services/src/context.ts index 44025989..fa350ce4 100644 --- a/packages/services/src/context.ts +++ b/packages/services/src/context.ts @@ -17,9 +17,9 @@ export type Actor = | { type: "user"; userId: number } | { type: "apiKey"; keyId: string; userId?: number; scopes: Scope[] } | { type: "mcp"; keyId: string; userId?: number; scopes: Scope[] } - // `userId`: the member whose email matches the Slack profile (or the - // installing user during OAuth). Adapters resolve it before any write. - | { type: "slack"; teamId: string; slackUserId: string; userId?: number } + // `userId`: the linked member (`slack_user`), or the installing user during + // OAuth. Unlinked Slack users never reach a service call. + | { type: "slack"; teamId: string; slackUserId: string; userId: number } | { type: "system"; job: string } | { type: "webhook"; source: string; externalId?: string } | { type: "subscriber"; subscriberId: number }; @@ -119,16 +119,15 @@ export function extractActorId(actor: Actor): string { /** * Return the openstatus `user.id` attributable to this actor, or `null` * when none is available. Used by mutations that stamp a `*_by` column. - * `slack` and `apiKey` actors may carry an optional linked userId once - * the corresponding mapping layers exist. */ export function tryGetActorUserId(actor: Actor): number | null { switch (actor.type) { case "user": return actor.userId; + case "slack": + return actor.userId; case "apiKey": case "mcp": - case "slack": return actor.userId ?? null; case "system": case "webhook": diff --git a/packages/services/src/integration/__tests__/install-slack-agent.test.ts b/packages/services/src/integration/__tests__/install-slack-agent.test.ts index 83420f68..fd9e3e9a 100644 --- a/packages/services/src/integration/__tests__/install-slack-agent.test.ts +++ b/packages/services/src/integration/__tests__/install-slack-agent.test.ts @@ -25,6 +25,7 @@ beforeAll(async () => { ctx = makeSlackCtx(team.workspace, { teamId: TEAM_ID, slackUserId: SLACK_USER_ID, + userId: team.userId, }); }); diff --git a/packages/services/src/maintenance/__tests__/maintenance.test.ts b/packages/services/src/maintenance/__tests__/maintenance.test.ts index 7b6d96dc..fbb99242 100644 --- a/packages/services/src/maintenance/__tests__/maintenance.test.ts +++ b/packages/services/src/maintenance/__tests__/maintenance.test.ts @@ -554,6 +554,7 @@ describe("slack actor path", () => { ...makeSlackCtx(teamCtx.workspace, { teamId: "T123", slackUserId: "U123", + userId: 1, }), db: tx, }; diff --git a/packages/services/src/slack-user/__tests__/slack-user.test.ts b/packages/services/src/slack-user/__tests__/slack-user.test.ts index 9c781fbf..166f043b 100644 --- a/packages/services/src/slack-user/__tests__/slack-user.test.ts +++ b/packages/services/src/slack-user/__tests__/slack-user.test.ts @@ -20,6 +20,9 @@ import { createSlackUserMapping, deleteSlackUserMappings, getSlackUserMapping, + listSlackUserMappings, + signSlackLinkToken, + verifySlackLinkToken, } from "../index"; let workspace: Workspace; @@ -146,3 +149,71 @@ describe("slack user mapping", () => { }); }); }); + +describe("link token", () => { + const secret = "test-secret"; + const input = { workspaceId: 1, teamId: "T1", slackUserId: "U1" }; + + test("round-trips", async () => { + const token = await signSlackLinkToken(secret, input); + expect(await verifySlackLinkToken(secret, token)).toMatchObject(input); + }); + + test("rejects a wrong secret", async () => { + const token = await signSlackLinkToken(secret, input); + expect(await verifySlackLinkToken("other", token)).toBeNull(); + }); + + test("rejects a tampered payload", async () => { + const token = await signSlackLinkToken(secret, input); + const [, sig] = token.split("."); + const forged = btoa( + JSON.stringify({ + kind: "slack-link", + ...input, + workspaceId: 2, + ts: Date.now(), + }), + ) + .replace(/\+/g, "-") + .replace(/\//g, "_") + .replace(/=+$/, ""); + expect(await verifySlackLinkToken(secret, `${forged}.${sig}`)).toBeNull(); + }); + + test("expires after 10 minutes", async () => { + const issued = Date.now() - 11 * 60 * 1000; + const token = await signSlackLinkToken(secret, input, issued); + expect(await verifySlackLinkToken(secret, token)).toBeNull(); + }); + + test("tolerates a signer clock slightly ahead, not far ahead", async () => { + const now = Date.now(); + const ahead = await signSlackLinkToken(secret, input, now + 5_000); + expect(await verifySlackLinkToken(secret, ahead, now)).toMatchObject(input); + const farAhead = await signSlackLinkToken(secret, input, now + 60_000); + expect(await verifySlackLinkToken(secret, farAhead, now)).toBeNull(); + }); +}); + +describe("list", () => { + test("a user lists their own linked accounts, not others'", async () => { + const other = await createUser(); + await addUserToWorkspace(other.id, workspace.id, "member"); + await withTestTransaction(async (tx) => { + const system = { ...makeSystemCtx(workspace, { job: "test" }), db: tx }; + const mine = await createSlackUserMapping({ + ctx: system, + input: { teamId: "T6", slackUserId: "U6", userId: memberId }, + }); + const theirs = await createSlackUserMapping({ + ctx: system, + input: { teamId: "T6", slackUserId: "U7", userId: other.id }, + }); + const ctx = { ...makeUserCtx(workspace, { userId: memberId }), db: tx }; + const ids = (await listSlackUserMappings({ ctx })).map((r) => r.id); + expect(ids).toContain(mine.id); + expect(ids).not.toContain(theirs.id); + }); + }); +}); diff --git a/packages/services/src/slack-user/index.ts b/packages/services/src/slack-user/index.ts index e83b422e..a288bcdd 100644 --- a/packages/services/src/slack-user/index.ts +++ b/packages/services/src/slack-user/index.ts @@ -1,6 +1,12 @@ export { createSlackUserMapping } from "./create"; export { getSlackUserMapping } from "./get"; export { deleteSlackUserMappings } from "./internal"; +export { + type SlackLinkTokenPayload, + signSlackLinkToken, + verifySlackLinkToken, +} from "./link-token"; +export { listSlackUserMappings } from "./list"; export { CreateSlackUserMappingInput, GetSlackUserMappingInput, diff --git a/packages/services/src/slack-user/link-token.ts b/packages/services/src/slack-user/link-token.ts new file mode 100644 index 00000000..b0d3e34e --- /dev/null +++ b/packages/services/src/slack-user/link-token.ts @@ -0,0 +1,87 @@ +import { z } from "zod"; + +const LINK_TOKEN_TTL_MS = 10 * 60 * 1000; +// The server signs and the dashboard verifies, so their clocks can disagree. +const LINK_TOKEN_CLOCK_SKEW_MS = 30 * 1000; + +const linkTokenPayload = z.object({ + kind: z.literal("slack-link"), + workspaceId: z.number().int(), + teamId: z.string().min(1), + slackUserId: z.string().min(1), + ts: z.number(), +}); +export type SlackLinkTokenPayload = z.infer; + +const encoder = new TextEncoder(); + +function toBase64Url(bytes: Uint8Array): string { + let binary = ""; + for (const b of bytes) binary += String.fromCharCode(b); + return btoa(binary) + .replace(/\+/g, "-") + .replace(/\//g, "_") + .replace(/=+$/, ""); +} + +function fromBase64Url(value: string): Uint8Array { + const padded = value.replace(/-/g, "+").replace(/_/g, "/"); + const binary = atob(padded + "=".repeat((4 - (padded.length % 4)) % 4)); + return Uint8Array.from(binary, (c) => c.charCodeAt(0)); +} + +async function hmac(secret: string, payload: string): Promise { + const key = await crypto.subtle.importKey( + "raw", + encoder.encode(secret), + { name: "HMAC", hash: "SHA-256" }, + false, + ["sign"], + ); + return new Uint8Array( + await crypto.subtle.sign("HMAC", key, encoder.encode(payload)), + ); +} + +function timingSafeEqual(a: Uint8Array, b: Uint8Array): boolean { + if (a.length !== b.length) return false; + let diff = 0; + for (let i = 0; i < a.length; i++) diff |= a[i] ^ b[i]; + return diff === 0; +} + +export async function signSlackLinkToken( + secret: string, + input: Omit, + now: number = Date.now(), +): Promise { + const payload = JSON.stringify({ kind: "slack-link", ...input, ts: now }); + const sig = toBase64Url(await hmac(secret, payload)); + return `${toBase64Url(encoder.encode(payload))}.${sig}`; +} + +/** The token's payload, or `null` when it is malformed, forged or expired. */ +export async function verifySlackLinkToken( + secret: string, + token: string, + now: number = Date.now(), +): Promise { + const [encodedPayload, sig] = token.split("."); + if (!encodedPayload || !sig) return null; + try { + const payload = new TextDecoder().decode(fromBase64Url(encodedPayload)); + const expected = await hmac(secret, payload); + if (!timingSafeEqual(expected, fromBase64Url(sig))) return null; + const parsed = linkTokenPayload.safeParse(JSON.parse(payload)); + if (!parsed.success) return null; + if ( + now - parsed.data.ts > LINK_TOKEN_TTL_MS || + parsed.data.ts - now > LINK_TOKEN_CLOCK_SKEW_MS + ) { + return null; + } + return parsed.data; + } catch { + return null; + } +} diff --git a/packages/services/src/slack-user/list.ts b/packages/services/src/slack-user/list.ts new file mode 100644 index 00000000..027172d8 --- /dev/null +++ b/packages/services/src/slack-user/list.ts @@ -0,0 +1,23 @@ +import { and, eq } from "@openstatus/db"; +import { type SlackUser, slackUser } from "@openstatus/db/src/schema"; + +import { type ServiceContext, getReadDb, tryGetActorUserId } from "../context"; + +/** The Slack accounts linked to the calling user in this workspace. */ +export async function listSlackUserMappings(args: { + ctx: ServiceContext; +}): Promise { + const { ctx } = args; + const userId = tryGetActorUserId(ctx.actor); + if (userId === null) return []; + return getReadDb(ctx) + .select() + .from(slackUser) + .where( + and( + eq(slackUser.workspaceId, ctx.workspace.id), + eq(slackUser.userId, userId), + ), + ) + .all(); +} diff --git a/packages/services/src/status-report/__tests__/status-report.test.ts b/packages/services/src/status-report/__tests__/status-report.test.ts index 26ea6ec3..2708634e 100644 --- a/packages/services/src/status-report/__tests__/status-report.test.ts +++ b/packages/services/src/status-report/__tests__/status-report.test.ts @@ -759,6 +759,7 @@ describe("slack actor path", () => { ...makeSlackCtx(teamCtx.workspace, { teamId: "T123", slackUserId: "U123", + userId: 1, }), db: tx, }; diff --git a/packages/services/test/helpers.ts b/packages/services/test/helpers.ts index 171697bd..319c430d 100644 --- a/packages/services/test/helpers.ts +++ b/packages/services/test/helpers.ts @@ -175,7 +175,7 @@ export function makeSlackCtx( opts: { teamId: string; slackUserId: string; - userId?: number; + userId: number; requestId?: string; }, ): ServiceContext { -- 2.51.2