From 001c79e06c4cbc8d58108c1e0fab067ecd751c89 Mon Sep 17 00:00:00 2001 From: Ty Tremblay Date: Sat, 22 Aug 2026 16:05:46 -0400 Subject: [PATCH] Add a web landing page and a Slack-signed-in configuration page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit bot. now serves a branded landing page at / and a settings editor at /config — the same settings, validation, and usergroup handle→id resolution as /hawkbot config, through one shared write path (slack/settingsWrite.ts, extracted from commands/config.ts). Sign in with Slack (OpenID Connect, `openid profile`) establishes who the browser is; the existing HawkBot Admin check decides what they may do, live on every page load. The OIDC token is used for one userInfo call and discarded — never stored, never logged. Sessions are stateless HMAC-signed cookies (12h, SLACK_STATE_SECRET); CSRF is SameSite=Lax plus an Origin check on every POST. ADR-0015 records why identity-only OIDC scopes are the one deliberate exception to "no user scopes", and CLAUDE.md rule 1 is sharpened to match. Deploying needs the manifest's new /auth/slack/callback redirect URL and the openid/profile user scopes; no bot reinstall. Co-Authored-By: Claude Fable 5 --- .env.example | 9 +- CLAUDE.md | 24 +- CONTEXT.md | 3 + ...nfig-page-authenticates-with-slack-oidc.md | 12 + docs/slack-app-manifest.yaml | 14 +- src/commands/config.ts | 59 +-- src/slack/app.ts | 5 +- src/slack/settingsWrite.ts | 81 +++ src/web/pages.ts | 207 ++++++++ src/web/routes.ts | 461 ++++++++++++++++++ src/web/session.ts | 147 ++++++ test/webPages.test.ts | 88 ++++ test/webSession.test.ts | 107 ++++ 13 files changed, 1172 insertions(+), 45 deletions(-) create mode 100644 docs/adr/0015-web-config-page-authenticates-with-slack-oidc.md create mode 100644 src/slack/settingsWrite.ts create mode 100644 src/web/pages.ts create mode 100644 src/web/routes.ts create mode 100644 src/web/session.ts create mode 100644 test/webPages.test.ts create mode 100644 test/webSession.test.ts diff --git a/.env.example b/.env.example index 234dfd0..3f85bac 100644 --- a/.env.example +++ b/.env.example @@ -2,13 +2,16 @@ SLACK_SIGNING_SECRET= SLACK_CLIENT_ID= SLACK_CLIENT_SECRET= -# Any long random string; signs the OAuth state param. +# Any long random string; signs the OAuth state param, and the web +# configuration page's session cookies (src/web/session.ts). Rotating it +# signs everyone out of the web page, nothing more. # openssl rand -hex 32 SLACK_STATE_SECRET= # Public HTTPS base URL this app is reachable at. Slack posts slash commands -# and events to $PUBLIC_URL/slack/events and redirects OAuth to -# $PUBLIC_URL/slack/oauth_redirect. It must match the app manifest exactly. +# and events to $PUBLIC_URL/slack/events, redirects install OAuth to +# $PUBLIC_URL/slack/oauth_redirect, and redirects web sign-in to +# $PUBLIC_URL/auth/slack/callback. It must match the app manifest exactly. PUBLIC_URL=https://hawk-bot.example.org PORT=3000 diff --git a/CLAUDE.md b/CLAUDE.md index c257f4c..5a5c08c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -59,6 +59,15 @@ src/slack/commands.ts the slash-command router src/slack/events.ts app_home_opened, app_mention, app_uninstalled src/slack/authz.ts "is this person a workspace admin", cached 5 minutes src/slack/home.ts the App Home view, rendered from the command registry +src/slack/settingsWrite.ts + the one write path for settings, shared by the config + command and the web configuration page +src/web/routes.ts the web surface at bot.: a landing page, and a + configuration page behind Sign in with Slack (OIDC, + identity only — see ADR-0015) plus the HawkBot Admin + check +src/web/session.ts stateless HMAC-signed session cookies; pure, tested +src/web/pages.ts HTML rendering; pure, tested ``` ### The command registry is the extension point @@ -111,9 +120,14 @@ nothing in the suite — Caddy fronts it. ## Rules that are load-bearing -1. **No user scopes.** Ever. This app is in a workspace shared with minors and - its defensibility rests on being unable to read anything a person could not - see it read. Adding a user scope changes what this app _is_. +1. **No user token scopes.** Ever. This app is in a workspace shared with + minors and its defensibility rests on being unable to read anything a + person could not see it read. Adding a scope that reads or acts as a + person changes what this app _is_. The one deliberate exception: the + OpenID Connect identity scopes (`openid`, `profile`) behind the web + configuration page's Sign in with Slack, which answer "who is this + person" and grant nothing else — the token is used for that single + lookup and discarded, never stored. See ADR-0015. 2. **Never log message text or a token.** `logger.ts` writes JSON to stderr and that is not an auditable place. 3. **Migrations that have shipped are never edited.** Add another file. @@ -122,7 +136,9 @@ nothing in the suite — Caddy fronts it. `domain/settings.ts`. Nothing team-specific is ever baked into the image. 5. **Keep `BOT_SCOPES` and `docs/slack-app-manifest.yaml` identical.** Slack grants what the manifest says; Bolt asks for what the code says. When they - disagree the symptom is a command that silently does nothing. + disagree the symptom is a command that silently does nothing. The same + goes for the manifest's OIDC user scopes and `OIDC_SCOPES` in + `src/web/routes.ts`, and for the `/auth/slack/callback` redirect URL. 6. **`.env.example` documents every variable `config.ts` reads.** A missing one reads as "the feature is off". diff --git a/CONTEXT.md b/CONTEXT.md index 08b874d..78197aa 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -77,6 +77,9 @@ No link out to another surface — the detail lives entirely inside the thread r **HawkBot Admin**: Replaces "Slack workspace Owner/Admin" as the authorization model for every admin-gated capability (`/hawkbot config`, `/hawkbot event create`, the season CSV export, and the Reaction Cutoff Verification failure DM) — full replacement, not an additional either/or check. Defined by membership in a dedicated Slack User Group — not Slack's built-in Admins group — so a team can grant bot-admin duties (mentors, team leads) without handing out full Slack workspace administration. An admin configures it by handle (e.g. `hawkbot-admins`); the bot resolves that to the group's Slack-internal id once, at set-time, and checks membership against the id from then on. Slack workspace Owners always retain HawkBot Admin rights regardless of the group's state, as a bootstrap/lockout safety net — otherwise an empty or unconfigured group would mean nobody could run the command needed to fix it. Group membership is cached (same 5-minute-TTL shape as the old per-user admin cache), rather than looked up per check. +**Web Configuration Page**: +The settings editor served at `bot./config` (routing by hawk_suite) — the same settings, validation, and usergroup handle→id resolution as `/hawkbot config`, through one shared write path (`slack/settingsWrite.ts`). Gated twice: Sign in with Slack (OpenID Connect, identity-only — see ADR-0015) establishes who the browser is, then the ordinary HawkBot Admin check decides whether they may edit. A landing page at `/` fronts it. No new authorization model and no new stored state beyond a stateless signed session cookie. + **Weekly Summary Post**: A single Slack message posted to the announcements channel at an admin-configurable day/time (default Sunday, noon), listing every Event scheduled for the 7 days starting the day after that post — a rolling window, not aligned to any fixed Monday–Sunday calendar week (see ADR-0011) — a full planning digest, including Hourly, All-Day, and Multi-Day Events alike, regardless of whether an Event already has (or will soon have) its own Event Check-in Post. A new Weekly Summary Post goes out every cycle without deleting the previous one — see ADR-0014; the channel accumulates one post per week, each a snapshot of what was upcoming as of that post's date, rather than there ever being exactly one "current" post. A Multi-Day Event appears here exactly once, as its own date-range line — its Multi-Day Child Events are never listed individually, so the same competition is never shown more than once. _Avoid_: "this week" or "calendar week" implying Monday–Sunday alignment — "This Week" is the Slack-facing label, but the underlying window floats with whatever day the post lands on (see ADR-0011). diff --git a/docs/adr/0015-web-config-page-authenticates-with-slack-oidc.md b/docs/adr/0015-web-config-page-authenticates-with-slack-oidc.md new file mode 100644 index 0000000..137152f --- /dev/null +++ b/docs/adr/0015-web-config-page-authenticates-with-slack-oidc.md @@ -0,0 +1,12 @@ +# The web configuration page authenticates with Sign in with Slack (OIDC), and OIDC identity scopes are not "user scopes" + +Hawk Bot now serves a landing page at `/` and a configuration page at `/config` (`bot.`, routed by hawk_suite). The configuration page edits the exact same settings as `/hawkbot config` — same `domain/settings.ts` validation, same usergroup handle→id resolution, one shared write path (`slack/settingsWrite.ts`) — and is gated the same way: sign in, then the HawkBot Admin check from `slack/authz.ts` (ADR-0004). + +Sign-in is Sign in with Slack, Slack's OpenID Connect flow, with the identity scopes `openid` and `profile`. That deserves a careful reading against the repo's first load-bearing rule, "no user scopes, ever", because in a Slack app manifest OIDC scopes are listed under `oauth_config.scopes.user`. The rule's _reason_ is what governs: the app's defensibility rests on being unable to read anything a person could not watch it read. OIDC identity scopes grant no such ability. The token Slack returns from the sign-in exchange can call exactly one method, `openid.connect.userInfo` — "who is this person" — and nothing else: no messages, no channels, no acting on anyone's behalf. Hawk Bot uses it for that single call and discards it; it is never stored (there is nothing it could be used for later) and never logged (rule 2). The rule's wording in `CLAUDE.md` is sharpened accordingly: no user _token_ scopes — nothing that lets the app read or do anything as a person; identity-only OIDC scopes for sign-in are the one deliberate exception. + +Alternatives considered: + +- **A magic link DM'd via a slash command** (`/hawkbot weblogin`) would have needed no manifest change at all, but invents a bespoke login flow — with its own token lifetime, single-use, and phishing questions — to avoid a standard one whose entire grant is "name and id". Slack's own hosted sign-in screen is more legible to a mentor than a bot DM containing a login link, which is exactly the shape phishing training teaches people to distrust. +- **A separate password or allow-list** would violate the standing decision that authorization lives in Slack (ADR-0004): a stored credential list is precisely the drift — someone keeping access after they've left — that model exists to prevent. + +The web session is a stateless HMAC-signed cookie (12 h, signed with `SLACK_STATE_SECRET`), so the database stores nothing new: a session merely caches "Slack said this is user U in team T", and every page load still runs the live HawkBot Admin check, so removal from the admin group takes effect within its 5-minute cache like everywhere else. diff --git a/docs/slack-app-manifest.yaml b/docs/slack-app-manifest.yaml index f6d7e3d..3c85f90 100644 --- a/docs/slack-app-manifest.yaml +++ b/docs/slack-app-manifest.yaml @@ -36,6 +36,9 @@ features: oauth_config: redirect_urls: - https://hawk-bot.example.org/slack/oauth_redirect + # Sign in with Slack (OpenID Connect) for the web configuration page — + # see src/web/routes.ts and ADR-0015. + - https://hawk-bot.example.org/auth/slack/callback scopes: # Keep identical to BOT_SCOPES in src/slack/app.ts. bot: @@ -69,7 +72,16 @@ oauth_config: # reading that group's membership — the other half of the # authorization model, alongside the Owner check above. - usergroups:read - # Deliberately none. Hawk Bot never acts on a person's behalf. + # These are Sign in with Slack's OpenID Connect identity scopes, NOT user + # token scopes — the deliberate, sole exception to "no user scopes". The + # token they yield can call openid.connect.userInfo ("who is this + # person") and nothing else: no messages, no acting on anyone's behalf. + # It is used for that one call during web sign-in and discarded, never + # stored. See ADR-0015. Keep identical to OIDC_SCOPES in + # src/web/routes.ts. + user: + - openid + - profile settings: interactivity: diff --git a/src/commands/config.ts b/src/commands/config.ts index a878797..e9125f8 100644 --- a/src/commands/config.ts +++ b/src/commands/config.ts @@ -1,9 +1,13 @@ import type { WebClient } from "@slack/web-api"; import { SLASH_COMMAND } from "../brand.js"; -import { clearSetting, getSetting, setSetting } from "../db/repo.js"; +import { getSetting } from "../db/repo.js"; import { formatResolvedValue } from "../domain/configDisplay.js"; -import { SETTINGS, checkSetting, findSetting } from "../domain/settings.js"; +import { SETTINGS } from "../domain/settings.js"; import { resolveName } from "../slack/nameResolution.js"; +import { + applySettingInput, + applySettingUnset, +} from "../slack/settingsWrite.js"; import type { Command } from "./types.js"; async function describeCurrentValue( @@ -56,16 +60,12 @@ export const config: Command = { text: `Usage: \`${SLASH_COMMAND} config unset \``, }; } - const setting = findSetting(key); - if (!setting) { - const known = SETTINGS.map((s) => s.key).join(", "); - return { text: `Unknown setting \`${key}\`. Known: ${known}` }; - } - const removed = clearSetting(setting.key); + const outcome = applySettingUnset(key); + if (!outcome.ok) return { text: outcome.reason }; return { - text: removed - ? `Unset \`${setting.key}\`.` - : `\`${setting.key}\` was already unset.`, + text: outcome.removed + ? `Unset \`${outcome.key}\`.` + : `\`${outcome.key}\` was already unset.`, }; } @@ -81,33 +81,20 @@ export const config: Command = { }; } - const checked = checkSetting(key, valueParts.join(" ")); - if (!checked.ok) return { text: checked.reason }; - - // All three usergroup settings are set by handle, but membership checks - // need the Slack-internal group id — resolved here, once, rather than on - // every check. See CONTEXT.md, HawkBot Admin. - if ( - checked.key === "admin_usergroup" || - checked.key === "student_usergroup" || - checked.key === "mentor_usergroup" - ) { - const groups = await ctx.client.usergroups.list({}); - const match = groups.usergroups?.find( - (g) => g.handle?.toLowerCase() === checked.value.toLowerCase() - ); - if (!match?.id) { - return { - text: `No Slack User Group found with handle \`${checked.value}\`. Check the handle and try again.`, - }; - } - setSetting(checked.key, match.id, ctx.userId); + // Validation and the usergroup handle→id resolution live in + // slack/settingsWrite.ts, shared with the web configuration page. + const outcome = await applySettingInput( + ctx.client, + key, + valueParts.join(" "), + ctx.userId + ); + if (!outcome.ok) return { text: outcome.reason }; + if (outcome.groupHandle) { return { - text: `Set \`${checked.key}\` to the group \`@${checked.value}\` (\`${match.id}\`).`, + text: `Set \`${outcome.key}\` to the group \`@${outcome.groupHandle}\` (\`${outcome.storedValue}\`).`, }; } - - setSetting(checked.key, checked.value, ctx.userId); - return { text: `Set \`${checked.key}\` to \`${checked.value}\`.` }; + return { text: `Set \`${outcome.key}\` to \`${outcome.storedValue}\`.` }; }, }; diff --git a/src/slack/app.ts b/src/slack/app.ts index c088855..78da1fc 100644 --- a/src/slack/app.ts +++ b/src/slack/app.ts @@ -3,6 +3,7 @@ import { APP_NAME, BRAND, ICON_SVG, SLASH_COMMAND } from "../brand.js"; import { config } from "../config.js"; import { healthHandler } from "../health.js"; import { log } from "../logger.js"; +import { webRoutes } from "../web/routes.js"; import { registerAttendanceEvents } from "./attendanceEvents.js"; import { registerCommands } from "./commands.js"; import { registerEvents } from "./events.js"; @@ -65,10 +66,12 @@ export function createApp(): App { stateSecret: cfg.SLACK_STATE_SECRET, scopes: BOT_SCOPES, installationStore, - // Bolt owns the HTTP server, so the container's health endpoint has to be + // Bolt owns the HTTP server, so the container's health endpoint and the + // web pages (landing + configuration; see web/routes.ts) have to be // registered through it rather than served alongside. customRoutes: [ { path: "/health", method: ["GET"], handler: healthHandler }, + ...webRoutes(), ], redirectUri: `${cfg.PUBLIC_URL}/slack/oauth_redirect`, installerOptions: { diff --git a/src/slack/settingsWrite.ts b/src/slack/settingsWrite.ts new file mode 100644 index 0000000..68dc28a --- /dev/null +++ b/src/slack/settingsWrite.ts @@ -0,0 +1,81 @@ +import type { WebClient } from "@slack/web-api"; +import { clearSetting, setSetting } from "../db/repo.js"; +import type { SettingKey } from "../domain/settings.js"; +import { checkSetting, findSetting, SETTINGS } from "../domain/settings.js"; + +/** + * The one write path for workspace settings, shared by `/hawkbot config set` + * and the web configuration page so the two surfaces cannot drift: same + * validation (domain/settings.ts), same usergroup handle→id resolution, same + * stored value. Callers format the outcome for their own medium — Slack + * markdown or HTML — which is why this returns data, not a message. + */ + +export type SetOutcome = + | { + ok: true; + key: SettingKey; + /** What actually went into the settings table. */ + storedValue: string; + /** Present when a usergroup handle was resolved to a group id. */ + groupHandle?: string; + } + | { ok: false; reason: string }; + +export async function applySettingInput( + client: WebClient, + rawKey: string, + rawValue: string, + setBy: string +): Promise { + const checked = checkSetting(rawKey, rawValue); + if (!checked.ok) return { ok: false, reason: checked.reason }; + + // All three usergroup settings are set by handle, but membership checks + // need the Slack-internal group id — resolved here, once, rather than on + // every check. See CONTEXT.md, HawkBot Admin. Deliberately a fresh list, + // not nameResolution.ts's 5-minute cache: a group created moments ago must + // be settable immediately. + if ( + checked.key === "admin_usergroup" || + checked.key === "student_usergroup" || + checked.key === "mentor_usergroup" + ) { + const groups = await client.usergroups.list({}); + const match = groups.usergroups?.find( + (g) => g.handle?.toLowerCase() === checked.value.toLowerCase() + ); + if (!match?.id) { + return { + ok: false, + reason: `No Slack User Group found with handle \`${checked.value}\`. Check the handle and try again.`, + }; + } + setSetting(checked.key, match.id, setBy); + return { + ok: true, + key: checked.key, + storedValue: match.id, + groupHandle: checked.value, + }; + } + + setSetting(checked.key, checked.value, setBy); + return { ok: true, key: checked.key, storedValue: checked.value }; +} + +export type UnsetOutcome = + | { ok: true; key: SettingKey; removed: boolean } + | { ok: false; reason: string }; + +export function applySettingUnset(rawKey: string): UnsetOutcome { + const setting = findSetting(rawKey); + if (!setting) { + const known = SETTINGS.map((s) => s.key).join(", "); + return { + ok: false, + reason: `Unknown setting \`${rawKey}\`. Known: ${known}`, + }; + } + return { ok: true, key: setting.key, removed: clearSetting(setting.key) }; +} diff --git a/src/web/pages.ts b/src/web/pages.ts new file mode 100644 index 0000000..844d8a2 --- /dev/null +++ b/src/web/pages.ts @@ -0,0 +1,207 @@ +import { APP_NAME, BRAND, ICON_SVG, SLASH_COMMAND } from "../brand.js"; + +/** + * Every page the web surface renders, as pure functions from data to HTML — + * sibling in spirit to the post-install page in slack/app.ts, and styled to + * match it. No Slack client, no database, no config: web/routes.ts gathers + * the data, this turns it into markup, and a test can call any of these + * without an environment. + */ + +export function escapeHtml(value: string): string { + return value + .replaceAll("&", "&") + .replaceAll("<", "<") + .replaceAll(">", ">") + .replaceAll('"', """) + .replaceAll("'", "'"); +} + +function layout(title: string, body: string): string { + return ` + + + +${escapeHtml(title)} + +
+${body} +
+`; +} + +function header(heading: string): string { + return `${ICON_SVG} +

${escapeHtml(heading)}

+

${escapeHtml(APP_NAME)}

`; +} + +/** `bot.`'s front door — for the curious, not a console. */ +export function landingPage(): string { + return layout( + APP_NAME, + `${header(APP_NAME)} +

${escapeHtml(APP_NAME)} is Red Hawk Robotics' team assistant in Slack. It +posts meeting check-ins and weekly schedules, tracks attendance from +reactions, and answers ${escapeHtml(SLASH_COMMAND)} help.

+

It acts only as itself and only where it is invited — it holds no +permission to read anyone's messages on their behalf.

+

+ + For HawkBot Admins — sign in with Slack required. +

` + ); +} + +/** Shown at /config when there is no (valid) session yet. */ +export function signInPage(): string { + return layout( + `Sign in — ${APP_NAME}`, + `${header("Configuration")} +

Workspace settings live behind Slack sign-in, and changing them is limited +to HawkBot Admins.

+

+

Signing in only tells ${escapeHtml(APP_NAME)} who you are. +It grants no access to your messages, and no token is kept.

` + ); +} + +/** Signed in fine, but not a HawkBot Admin. */ +export function forbiddenPage(name: string): string { + return layout( + `Not authorized — ${APP_NAME}`, + `${header("Not authorized")} +

You're signed in as ${escapeHtml(name)}, but changing +workspace settings needs HawkBot Admin: membership in the admin User Group +(or being a workspace Owner). Ask a coach to add you, then reload.

+${signOutForm()}` + ); +} + +/** Sign-in attempted before any workspace has installed the app. */ +export function notInstalledPage(): string { + return layout( + `Not installed — ${APP_NAME}`, + `${header("Not installed yet")} +

${escapeHtml(APP_NAME)} hasn't been added to a Slack workspace yet, so +there is nothing to configure. A workspace admin can +install it first.

` + ); +} + +export function errorPage(message: string): string { + return layout( + `Something went wrong — ${APP_NAME}`, + `${header("Something went wrong")} +

${escapeHtml(message)}

+

Back to configuration

` + ); +} + +function signOutForm(): string { + return `
+ +
`; +} + +export type SettingView = { + key: string; + summary: string; + expects: string; + /** The stored value, absent when unset. */ + value?: string; + /** + * The stored value with its live-resolved name annotation (see + * domain/configDisplay.ts), when the setting is a channel or usergroup. + */ + display?: string; +}; + +export type Flash = { kind: "ok" | "err"; text: string }; + +/** The settings editor — one card per declared setting. */ +export function configPage(args: { + settings: readonly SettingView[]; + signedInAs: string; + flash?: Flash; +}): string { + const flash = args.flash + ? `
${escapeHtml(args.flash.text)}
` + : ""; + const cards = args.settings + .map((s) => { + const current = s.value + ? `

Currently ${escapeHtml(s.display ?? s.value)}

` + : `

Not set

`; + return `
+
${escapeHtml(s.key)}
+

${escapeHtml(s.summary)}

+${current} +
+ +
+ + +
+
+${ + s.value + ? `
+ + +
` + : "" +} +
`; + }) + .join("\n"); + return layout( + `Configuration — ${APP_NAME}`, + `${header("Configuration")} +
+

Signed in as ${escapeHtml(args.signedInAs)}

+${signOutForm()} +
+${flash} +

The same settings as ${escapeHtml(SLASH_COMMAND)} config, with +the same validation. Changes take effect immediately.

+${cards}` + ); +} diff --git a/src/web/routes.ts b/src/web/routes.ts new file mode 100644 index 0000000..ec09d43 --- /dev/null +++ b/src/web/routes.ts @@ -0,0 +1,461 @@ +import { randomBytes } from "node:crypto"; +import type { IncomingMessage, ServerResponse } from "node:http"; +import type { CustomRoute } from "@slack/bolt"; +import { WebClient } from "@slack/web-api"; +import { config } from "../config.js"; +import { anyInstallation, listSettings } from "../db/repo.js"; +import { formatResolvedValue } from "../domain/configDisplay.js"; +import { SETTINGS } from "../domain/settings.js"; +import { log } from "../logger.js"; +import { isHawkBotAdmin } from "../slack/authz.js"; +import { resolveName } from "../slack/nameResolution.js"; +import { + applySettingInput, + applySettingUnset, +} from "../slack/settingsWrite.js"; +import { + configPage, + errorPage, + forbiddenPage, + landingPage, + notInstalledPage, + signInPage, + type Flash, + type SettingView, +} from "./pages.js"; +import { + parseCookies, + serializeCookie, + signSession, + signState, + verifySession, + verifyState, + type WebSession, +} from "./session.js"; + +/** + * The web surface behind `bot.`: a landing page, and a configuration + * page for the same workspace settings as `/hawkbot config`. + * + * Sign in with Slack (OpenID Connect) answers "who is this browser" — and + * that is *all* it answers. The scopes are `openid profile`, which grant + * identity, not access: the token Slack hands back can call + * `openid.connect.userInfo` and nothing else, is used for exactly that one + * call, and is never stored or logged. What a signed-in person may *do* is + * then decided by the same HawkBot Admin check every admin command goes + * through (slack/authz.ts). See ADR-0015. + * + * CSRF: session cookies are SameSite=Lax, and every state-changing POST also + * requires an Origin header matching PUBLIC_URL. + */ + +export const OIDC_SCOPES = "openid profile"; + +const SESSION_COOKIE = "hawk_bot_session"; +const STATE_COOKIE = "hawk_bot_oauth_state"; +const SESSION_TTL_MS = 12 * 60 * 60 * 1000; +const STATE_TTL_MS = 10 * 60 * 1000; + +function isSecure(): boolean { + return new URL(config().PUBLIC_URL).protocol === "https:"; +} + +function sendHtml(res: ServerResponse, code: number, body: string): void { + res.writeHead(code, { + "content-type": "text/html; charset=utf-8", + "cache-control": "no-store", + }); + res.end(body); +} + +function redirect( + res: ServerResponse, + location: string, + cookies: string[] = [] +): void { + res.writeHead(303, { + location, + "cache-control": "no-store", + ...(cookies.length ? { "set-cookie": cookies } : {}), + }); + res.end(); +} + +function currentSession(req: IncomingMessage): WebSession | undefined { + const token = parseCookies(req.headers.cookie).get(SESSION_COOKIE); + if (!token) return undefined; + return verifySession(token, config().SLACK_STATE_SECRET, Date.now()); +} + +/** The installed workspace's bot client — the same pattern as scheduler.ts. */ +function installedWorkspace(): + { client: WebClient; teamId: string } | undefined { + const installation = anyInstallation(); + if (!installation) return undefined; + const payload = installation.payload as { bot?: { token?: string } }; + const token = payload.bot?.token; + if (!token) return undefined; + return { client: new WebClient(token), teamId: installation.teamId }; +} + +/** + * The session-and-authorization gate in front of the configuration page and + * both of its POSTs. Writes the appropriate page and returns undefined when + * the caller should stop; the session's team must be the installed team, so + * a sign-in from some other Slack workspace proves nothing here. + */ +async function requireAdmin( + req: IncomingMessage, + res: ServerResponse +): Promise<{ session: WebSession; client: WebClient } | undefined> { + const session = currentSession(req); + if (!session) { + sendHtml(res, 200, signInPage()); + return undefined; + } + const installed = installedWorkspace(); + if (!installed) { + sendHtml(res, 200, notInstalledPage()); + return undefined; + } + if (session.teamId !== installed.teamId) { + sendHtml(res, 403, forbiddenPage(session.name)); + return undefined; + } + if (!(await isHawkBotAdmin(installed.client, session.userId))) { + sendHtml(res, 403, forbiddenPage(session.name)); + return undefined; + } + return { session, client: installed.client }; +} + +/** + * SameSite=Lax stops a cross-site form from carrying the session cookie, but + * only in browsers that enforce it — this check is the belt to that + * suspender. Same-origin POSTs always carry an Origin header, so a missing + * one is rejected too. + */ +function isSameOrigin(req: IncomingMessage): boolean { + return req.headers.origin === new URL(config().PUBLIC_URL).origin; +} + +/** A small urlencoded form — anything over 32 KiB is not one of our forms. */ +function readForm(req: IncomingMessage): Promise { + return new Promise((resolve, reject) => { + const chunks: Buffer[] = []; + let size = 0; + req.on("data", (chunk: Buffer) => { + size += chunk.length; + if (size > 32 * 1024) { + reject(new Error("form body too large")); + req.destroy(); + return; + } + chunks.push(chunk); + }); + req.on("end", () => + resolve(new URLSearchParams(Buffer.concat(chunks).toString("utf8"))) + ); + req.on("error", reject); + }); +} + +/** + * Slack-markdown reason strings (from domain/settings.ts) carry backticks + * that mean nothing in a URL-borne flash message — dropped, and the result + * bounded because it rides in a redirect's query string. + */ +function asFlashText(reason: string): string { + const plain = reason.replaceAll("`", ""); + return plain.length > 400 ? `${plain.slice(0, 400)}…` : plain; +} + +function flashFromQuery(url: URL): Flash | undefined { + const ok = url.searchParams.get("ok"); + if (ok) return { kind: "ok", text: ok.slice(0, 500) }; + const err = url.searchParams.get("err"); + if (err) return { kind: "err", text: err.slice(0, 500) }; + return undefined; +} + +function requestUrl(req: IncomingMessage): URL { + return new URL(req.url ?? "/", config().PUBLIC_URL); +} + +/* ---------------------------------------------------------------- handlers */ + +function handleLanding(_req: IncomingMessage, res: ServerResponse): void { + res.writeHead(200, { "content-type": "text/html; charset=utf-8" }); + res.end(landingPage()); +} + +async function handleConfigPage( + req: IncomingMessage, + res: ServerResponse +): Promise { + const authed = await requireAdmin(req, res); + if (!authed) return; + + const stored = new Map(listSettings().map((r) => [r.key, r.value])); + const settings: SettingView[] = await Promise.all( + SETTINGS.map(async (s) => { + const value = stored.get(s.key); + const view: SettingView = { + key: s.key, + summary: s.summary, + expects: s.expects, + value, + }; + if (value && s.resolveAs) { + const resolution = await resolveName(authed.client, s.resolveAs, value); + view.display = formatResolvedValue(value, s.resolveAs, resolution); + } + return view; + }) + ); + sendHtml( + res, + 200, + configPage({ + settings, + signedInAs: authed.session.name, + flash: flashFromQuery(requestUrl(req)), + }) + ); +} + +async function handleConfigSet( + req: IncomingMessage, + res: ServerResponse +): Promise { + if (!isSameOrigin(req)) { + sendHtml(res, 403, errorPage("Cross-origin request refused.")); + return; + } + const authed = await requireAdmin(req, res); + if (!authed) return; + const form = await readForm(req); + const outcome = await applySettingInput( + authed.client, + form.get("key") ?? "", + form.get("value") ?? "", + authed.session.userId + ); + if (!outcome.ok) { + redirect( + res, + `/config?err=${encodeURIComponent(asFlashText(outcome.reason))}` + ); + return; + } + const detail = outcome.groupHandle + ? `the group @${outcome.groupHandle} (${outcome.storedValue})` + : outcome.storedValue; + redirect( + res, + `/config?ok=${encodeURIComponent(`Set ${outcome.key} to ${detail}.`)}` + ); +} + +async function handleConfigUnset( + req: IncomingMessage, + res: ServerResponse +): Promise { + if (!isSameOrigin(req)) { + sendHtml(res, 403, errorPage("Cross-origin request refused.")); + return; + } + const authed = await requireAdmin(req, res); + if (!authed) return; + const form = await readForm(req); + const outcome = applySettingUnset(form.get("key") ?? ""); + if (!outcome.ok) { + redirect( + res, + `/config?err=${encodeURIComponent(asFlashText(outcome.reason))}` + ); + return; + } + const text = outcome.removed + ? `Unset ${outcome.key}.` + : `${outcome.key} was already unset.`; + redirect(res, `/config?ok=${encodeURIComponent(text)}`); +} + +function handleSignIn(_req: IncomingMessage, res: ServerResponse): void { + const cfg = config(); + const state = randomBytes(16).toString("base64url"); + const stateToken = signState( + state, + Date.now() + STATE_TTL_MS, + cfg.SLACK_STATE_SECRET + ); + + const authorize = new URL("https://slack.com/openid/connect/authorize"); + authorize.searchParams.set("response_type", "code"); + authorize.searchParams.set("scope", OIDC_SCOPES); + authorize.searchParams.set("client_id", cfg.SLACK_CLIENT_ID); + authorize.searchParams.set("state", state); + authorize.searchParams.set( + "redirect_uri", + `${cfg.PUBLIC_URL}/auth/slack/callback` + ); + // Skips Slack's workspace picker when we already know the one workspace + // this app serves. + const installed = installedWorkspace(); + if (installed) authorize.searchParams.set("team", installed.teamId); + + redirect(res, authorize.toString(), [ + serializeCookie(STATE_COOKIE, stateToken, { + maxAgeSeconds: STATE_TTL_MS / 1000, + secure: isSecure(), + path: "/auth", + }), + ]); +} + +async function handleOAuthCallback( + req: IncomingMessage, + res: ServerResponse +): Promise { + const cfg = config(); + const url = requestUrl(req); + const clearState = serializeCookie(STATE_COOKIE, "", { + maxAgeSeconds: 0, + secure: isSecure(), + path: "/auth", + }); + + const code = url.searchParams.get("code"); + const state = url.searchParams.get("state"); + const stateToken = parseCookies(req.headers.cookie).get(STATE_COOKIE); + const expectedState = stateToken + ? verifyState(stateToken, cfg.SLACK_STATE_SECRET, Date.now()) + : undefined; + if (!code || !state || !expectedState || state !== expectedState) { + res.setHeader("set-cookie", clearState); + sendHtml( + res, + 400, + errorPage( + "Sign-in didn't complete — the browser's sign-in attempt expired or didn't match. Start again from the configuration page." + ) + ); + return; + } + + // The OIDC token is used for exactly one userInfo call, right here, and + // then dropped. Nothing about it is stored or logged (rule 2). + const exchange = await new WebClient().openid.connect.token({ + client_id: cfg.SLACK_CLIENT_ID, + client_secret: cfg.SLACK_CLIENT_SECRET, + code, + redirect_uri: `${cfg.PUBLIC_URL}/auth/slack/callback`, + }); + const identity = (await new WebClient( + exchange.access_token + ).openid.connect.userInfo()) as { + ok?: boolean; + name?: string; + "https://slack.com/user_id"?: string; + "https://slack.com/team_id"?: string; + }; + const userId = identity["https://slack.com/user_id"]; + const teamId = identity["https://slack.com/team_id"]; + if (!identity.ok || !userId || !teamId) { + res.setHeader("set-cookie", clearState); + sendHtml(res, 502, errorPage("Slack didn't say who you are. Try again.")); + return; + } + + const session: WebSession = { + userId, + teamId, + name: identity.name ?? userId, + exp: Date.now() + SESSION_TTL_MS, + }; + log.info("web sign-in", { userId, teamId }); + redirect(res, "/config", [ + clearState, + serializeCookie( + SESSION_COOKIE, + signSession(session, cfg.SLACK_STATE_SECRET), + { maxAgeSeconds: SESSION_TTL_MS / 1000, secure: isSecure() } + ), + ]); +} + +function handleSignOut(req: IncomingMessage, res: ServerResponse): void { + if (!isSameOrigin(req)) { + sendHtml(res, 403, errorPage("Cross-origin request refused.")); + return; + } + redirect(res, "/", [ + serializeCookie(SESSION_COOKIE, "", { + maxAgeSeconds: 0, + secure: isSecure(), + }), + ]); +} + +/* ----------------------------------------------------------------- routes */ + +type Handler = ( + req: IncomingMessage, + res: ServerResponse +) => void | Promise; + +/** + * Bolt swallows a rejected handler promise; without this, a Slack API error + * during sign-in would leave the browser hanging on a request that never + * finishes. + */ +function guarded(handler: Handler): Handler { + return async (req, res) => { + try { + await handler(req, res); + } catch (error) { + log.error("web route failed", { + path: req.url?.split("?")[0], + error: String(error), + }); + if (!res.headersSent) { + sendHtml( + res, + 500, + errorPage("Something went wrong on our side. Try again.") + ); + } else { + res.end(); + } + } + }; +} + +export function webRoutes(): CustomRoute[] { + return [ + { path: "/", method: ["GET"], handler: guarded(handleLanding) }, + { path: "/config", method: ["GET"], handler: guarded(handleConfigPage) }, + { + path: "/config/set", + method: ["POST"], + handler: guarded(handleConfigSet), + }, + { + path: "/config/unset", + method: ["POST"], + handler: guarded(handleConfigUnset), + }, + { path: "/auth/slack", method: ["GET"], handler: guarded(handleSignIn) }, + { + path: "/auth/slack/callback", + method: ["GET"], + handler: guarded(handleOAuthCallback), + }, + { + path: "/auth/sign-out", + method: ["POST"], + handler: guarded(handleSignOut), + }, + ]; +} diff --git a/src/web/session.ts b/src/web/session.ts new file mode 100644 index 0000000..1a972ae --- /dev/null +++ b/src/web/session.ts @@ -0,0 +1,147 @@ +import { createHmac, timingSafeEqual } from "node:crypto"; + +/** + * Signed tokens for the web configuration page: the session cookie an admin + * carries after Sign in with Slack, and the short-lived state token that ties + * an OAuth callback to the browser that started it. + * + * Stateless on purpose — the token *is* the session, HMAC-signed with + * SLACK_STATE_SECRET (passed in, never read here, so tests need no + * environment). Nothing about a web session is ever written to the database: + * what a session proves is "Slack said this is user U in team T until time + * exp", and that claim expires on its own. + * + * Each token kind signs a distinct purpose string, so a state token can never + * be replayed as a session cookie even though both are minted from the same + * secret. + */ + +export type WebSession = { + userId: string; + teamId: string; + /** Display name from Sign in with Slack, for the "signed in as" line. */ + name: string; + /** Unix milliseconds. */ + exp: number; +}; + +function hmac(purpose: string, payload: string, secret: string): Buffer { + return createHmac("sha256", secret).update(`${purpose}.${payload}`).digest(); +} + +function sign(purpose: string, value: unknown, secret: string): string { + const payload = Buffer.from(JSON.stringify(value)).toString("base64url"); + const mac = hmac(purpose, payload, secret).toString("base64url"); + return `${payload}.${mac}`; +} + +function verify( + purpose: string, + token: string, + secret: string +): unknown | undefined { + const dot = token.lastIndexOf("."); + if (dot <= 0) return undefined; + const payload = token.slice(0, dot); + const mac = Buffer.from(token.slice(dot + 1), "base64url"); + const expected = hmac(purpose, payload, secret); + if (mac.length !== expected.length || !timingSafeEqual(mac, expected)) { + return undefined; + } + try { + return JSON.parse(Buffer.from(payload, "base64url").toString("utf8")); + } catch { + return undefined; + } +} + +export function signSession(session: WebSession, secret: string): string { + return sign("session", session, secret); +} + +export function verifySession( + token: string, + secret: string, + nowMs: number +): WebSession | undefined { + const value = verify("session", token, secret) as WebSession | undefined; + if (!value || typeof value !== "object") return undefined; + const { userId, teamId, name, exp } = value; + if ( + typeof userId !== "string" || + typeof teamId !== "string" || + typeof name !== "string" || + typeof exp !== "number" || + exp <= nowMs + ) { + return undefined; + } + return { userId, teamId, name, exp }; +} + +/** + * The OAuth state round-trip: minted into a cookie when the sign-in redirect + * leaves, required back — via both the cookie and Slack's `state` query + * parameter — when the callback returns. `value` is caller-supplied + * randomness; the signature and expiry are what make it unforgeable and + * short-lived. + */ +export function signState( + value: string, + expMs: number, + secret: string +): string { + return sign("oauth-state", { value, exp: expMs }, secret); +} + +export function verifyState( + token: string, + secret: string, + nowMs: number +): string | undefined { + const parsed = verify("oauth-state", token, secret) as + { value?: unknown; exp?: unknown } | undefined; + if (!parsed || typeof parsed !== "object") return undefined; + if (typeof parsed.value !== "string" || typeof parsed.exp !== "number") { + return undefined; + } + return parsed.exp > nowMs ? parsed.value : undefined; +} + +/* ---------------------------------------------------------------- cookies */ + +/** `cookie` header → name/value map. Malformed pairs are simply skipped. */ +export function parseCookies(header: string | undefined): Map { + const cookies = new Map(); + if (!header) return cookies; + for (const part of header.split(";")) { + const eq = part.indexOf("="); + if (eq <= 0) continue; + const name = part.slice(0, eq).trim(); + const value = part.slice(eq + 1).trim(); + if (name) cookies.set(name, value); + } + return cookies; +} + +/** + * A Set-Cookie value. HttpOnly and SameSite=Lax always: no script ever needs + * these cookies, and Lax is half of the CSRF story (the Origin check in + * web/routes.ts is the other half). `secure` follows PUBLIC_URL's scheme so + * local HTTP development still works. `maxAgeSeconds: 0` deletes. + */ +export function serializeCookie( + name: string, + value: string, + opts: { maxAgeSeconds: number; secure: boolean; path?: string } +): string { + const parts = [ + `${name}=${value}`, + `Path=${opts.path ?? "/"}`, + `Max-Age=${opts.maxAgeSeconds}`, + "HttpOnly", + "SameSite=Lax", + ]; + if (opts.secure) parts.push("Secure"); + return parts.join("; "); +} diff --git a/test/webPages.test.ts b/test/webPages.test.ts new file mode 100644 index 0000000..1ee38fc --- /dev/null +++ b/test/webPages.test.ts @@ -0,0 +1,88 @@ +import assert from "node:assert/strict"; +import { describe, it } from "node:test"; +import { + configPage, + escapeHtml, + forbiddenPage, + landingPage, + signInPage, +} from "../src/web/pages.js"; + +describe("escapeHtml", () => { + it("escapes the five HTML metacharacters", () => { + assert.equal( + escapeHtml(`&`), + "<a href="x" onclick='y'>&" + ); + }); + + it("passes ordinary text through", () => { + assert.equal( + escapeHtml("team@group.calendar.google.com"), + "team@group.calendar.google.com" + ); + }); +}); + +describe("pages", () => { + it("landing page names the app and links to configuration", () => { + const html = landingPage(); + assert.match(html, /Hawk Bot/); + assert.match(html, /href="\/config"/); + }); + + it("sign-in page links the Slack OAuth entry point", () => { + assert.match(signInPage(), /href="\/auth\/slack"/); + }); + + it("forbidden page escapes the signed-in name", () => { + const html = forbiddenPage(``); + assert.ok(!html.includes(" { + const html = configPage({ + signedInAs: "Ty", + settings: [ + { + key: "announce_channel", + summary: "Channel Hawk Bot posts to", + expects: "a channel id", + value: `C0123">`, + }, + { + key: "home_note", + summary: "Free text", + expects: "any text", + }, + ], + flash: { kind: "err", text: `not valid` }, + }); + assert.match(html, /announce_channel/); + assert.match(html, /home_note/); + assert.match(html, /Not set/); + assert.ok(!html.includes("