diff --git a/.dev.vars.example b/.dev.vars.example new file mode 100644 index 0000000..212fe9f --- /dev/null +++ b/.dev.vars.example @@ -0,0 +1,19 @@ +# Local development settings for `wrangler dev`. Copy to `.dev.vars` (which is +# git-ignored) and fill in. Deployed environments set these as Worker secrets +# instead — never as [vars] in wrangler.toml. + +# WorkOS AuthKit domain (bare host or https origin). 1Password: nyuchi/workos, +# field WORKOS_AUTHKIT_DOMAIN. Leave unset to use the localhost-only opt-out +# below. +AUTHKIT_DOMAIN= + +# WorkOS Connect public client ID for the site login. 1Password: +# nyuchi/workos, field WORKOS_INTERNAL_TOOLS_CLIENT_ID. +WORKOS_CLIENT_ID= + +# Signs the site session cookie. Any long random string locally. +SESSION_SECRET= + +# Local development only: lets /mcp run without a bearer token when +# AUTHKIT_DOMAIN is unset and the request is to localhost. Never deployed. +ALLOW_UNAUTHENTICATED_DEV=true diff --git a/CHANGELOG.md b/CHANGELOG.md index 19b8af1..a4c3091 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Bundu theme and official logos** in `nyuchi-imaging`: a `bundu` theme in copper and terracotta, signed with the published Bundu Foundation logo. +### Security + +- **The site login reads its WorkOS client ID from configuration.** `/login` + and `/callback` now use the `WORKOS_CLIENT_ID` Worker secret instead of a + compiled-in client ID. With it unset, `/login` answers `503` + "WORKOS_CLIENT_ID is not configured" and `/callback` denies without + attempting a token exchange; there is no fallback client. +- **`/mcp` fails closed when `AUTHKIT_DOMAIN` is not configured.** It used to + run without its bearer-token gate ("open mode"), so a missing secret served + every tool unauthenticated. It now answers `503` "AUTHKIT_DOMAIN is not + configured", as does the protected-resource metadata. Local development can + still opt out explicitly with `ALLOW_UNAUTHENTICATED_DEV=true`, honoured only + for requests to `localhost` and never set in a deployed environment. + ### Changed - **Vite+ for the root checks and Worker tests** (tooling only). `vite-plus` diff --git a/SECURITY.md b/SECURITY.md index 0a1150d..9483fa1 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -62,7 +62,7 @@ The web/MCP surface (`tools.nyuchi.com` / `tools.nyuchi.dev`, source in `mcp/src ### Authentication -- **Site pages** sit behind a site-wide login gate: Authorization Code + PKCE against WorkOS AuthKit (`identity.nyuchi.com`), session held in an HMAC-signed cookie. The gate **fails closed** — a missing `SESSION_SECRET` means "no valid session", never "run open". +- **Site pages** sit behind a site-wide login gate: Authorization Code + PKCE against WorkOS AuthKit (the `AUTHKIT_DOMAIN` and `WORKOS_CLIENT_ID` set per environment — never hardcoded), session held in an HMAC-signed cookie. The gate **fails closed** — a missing `SESSION_SECRET` means "no valid session", never "run open". - **`/mcp`** requires WorkOS-issued bearer JWTs (OAuth 2.1, dynamic client registration) whenever `AUTHKIT_DOMAIN` is set. Audience is pinned to the registered resource (`MCP_RESOURCE`). ### Secrets diff --git a/mcp/README.md b/mcp/README.md index aff783a..f973ecd 100644 --- a/mcp/README.md +++ b/mcp/README.md @@ -31,13 +31,13 @@ comment in `../wrangler.toml` for why `/mcp` lives on a separate hostname): JSON-RPC. - `/auth.md` — human/agent-readable description of the OAuth architecture (`text/markdown`). Documents that this server is a resource server only — - the authorization server is identity.nyuchi.com (WorkOS Connect), outside - this repo. + the authorization server is WorkOS Connect on the configured + `AUTHKIT_DOMAIN`, outside this repo. - `/.well-known/oauth-protected-resource` — protected-resource metadata (see `auth.ts`); JSON 404 when auth is off. - `/.well-known/oauth-authorization-server` and `/.well-known/openid-configuration` — read-only mirrors of - identity.nyuchi.com's own discovery documents, fetched and passed through + the `AUTHKIT_DOMAIN` authorization server's own discovery documents, fetched and passed through (never fabricated) when auth is on; JSON 404 when auth is off. Responses are cached in-memory per-isolate for ~5 minutes as a soft optimization. - `/login`, `/callback`, `/logout` — the **site-wide login gate** (see @@ -53,17 +53,18 @@ comment in `../wrangler.toml` for why `/mcp` lives on a separate hostname): Every human-facing page sits behind a session cookie (`nyuchi_session`), a compact HS256 JWT signed with the `SESSION_SECRET` secret. The flow (see `site-auth.ts`) is Authorization Code + PKCE against the same WorkOS Connect -app that already protects `/mcp` (`client_01KVTX0V2K1VM3PSC0DJ9VZWTV`, -authorization server `identity.nyuchi.com`), used here as a public client +app that already protects `/mcp` (client ID = the configured `WORKOS_CLIENT_ID`, +authorization server = the configured `AUTHKIT_DOMAIN`), used here as a public client (`token_endpoint_auth_method=none` — no client secret is ever sent): - `GET /login` — validates `?return_to=` as a same-origin relative path (rejects absolute/protocol-relative URLs, falling back to `/`), generates PKCE `state`/`code_verifier`/`code_challenge`, stashes them in a short-lived `nyuchi_oauth` cookie, and 302s to - `https://identity.nyuchi.com/oauth2/authorize`. Returns 500 (fails closed) - if `SESSION_SECRET` isn't configured, rather than starting an OAuth round - trip that could never succeed. + `/oauth2/authorize`. Returns 500 (fails closed) if + `SESSION_SECRET` isn't configured, and 503 (naming the missing setting) if + `AUTHKIT_DOMAIN` or `WORKOS_CLIENT_ID` isn't, rather than starting an OAuth round trip that + could never succeed. - `GET /callback` — reads the `nyuchi_oauth` cookie, verifies `state` matches, exchanges `code` for an access token, verifies that token with the exact same JWKS/issuer/audience logic `/mcp`'s bearer-token gate uses @@ -93,6 +94,15 @@ authorization server `identity.nyuchi.com`), used here as a public client just a fixed path list) and `[assets]` has an explicit `binding = "ASSETS"` so `c.env.ASSETS.fetch(...)` can serve the static build from inside the Worker after the gate passes. +- **`AUTHKIT_DOMAIN` is required** and set per environment as a Worker secret + (the owner script reads it from 1Password, `nyuchi/workos`). It is never + committed — not in code, not in `wrangler.toml` `[vars]` — and there is no + default. +- **`WORKOS_CLIENT_ID` is required** too: the site login's public client ID, + set per environment as a Worker secret (the owner script reads it from + 1Password, `nyuchi/workos`, field `WORKOS_INTERNAL_TOOLS_CLIENT_ID`). There + is no compiled-in client. For `wrangler dev`, copy `.dev.vars.example` at + the repo root to `.dev.vars`. ## Where things live diff --git a/mcp/src/as-metadata-proxy.ts b/mcp/src/as-metadata-proxy.ts index fd1eb94..ac86b57 100644 --- a/mcp/src/as-metadata-proxy.ts +++ b/mcp/src/as-metadata-proxy.ts @@ -5,7 +5,7 @@ * protected-resource metadata still find it. * * This proxies (fetches + passes through) the real upstream documents from - * identity.nyuchi.com — it never fabricates metadata. Only wired up when + * the configured AUTHKIT_DOMAIN — it never fabricates metadata. Only wired up when * `authConfigured(env)` is true; see index.ts. */ diff --git a/mcp/src/auth-md.ts b/mcp/src/auth-md.ts index f2e2c76..8f3e067 100644 --- a/mcp/src/auth-md.ts +++ b/mcp/src/auth-md.ts @@ -3,7 +3,7 @@ * architecture, served at GET /auth.md (see index.ts). This is descriptive * documentation, not protocol metadata: the machine-readable metadata lives * at /.well-known/oauth-protected-resource (this resource server) and at - * identity.nyuchi.com's own /.well-known/oauth-authorization-server (the + * the configured AUTHKIT_DOMAIN's own /.well-known/oauth-authorization-server (the * WorkOS Connect authorization server, mirrored read-only at the same path * on this domain — see auth.ts / index.ts). * @@ -14,15 +14,25 @@ * whichever hostname actually served the request. */ -import { type AuthEnv, resourceOrigin, resourceUrl } from "./auth"; - -// Fixed, not derived from env.AUTHKIT_DOMAIN: this describes the canonical -// architecture (which authorization server this resource server trusts) -// even in open mode, when AUTHKIT_DOMAIN is unset — see "When auth is not -// required" below. Only the resource-server side (this domain) varies. -const ISSUER = "https://identity.nyuchi.com"; +import { type AuthEnv, authConfigured, issuerUrl, resourceOrigin, resourceUrl } from "./auth"; export function authMd(env: AuthEnv): string { + // The issuer comes only from configuration (AUTHKIT_DOMAIN) — never a + // compiled-in host. When it is unset this page says so rather than naming one. + // An unusable value is described as such, never echoed. + let issuer: string | null = null; + let unsetReason = "AUTHKIT_DOMAIN is unset"; + if (authConfigured(env)) { + try { + issuer = issuerUrl(env); + } catch { + unsetReason = "AUTHKIT_DOMAIN is not an https origin"; + } + } + const issuerLabel = issuer ?? `not configured (${unsetReason})`; + const issuerMetadata = issuer + ? new URL("/.well-known/oauth-authorization-server", issuer).href + : `not available (${unsetReason})`; const resource = resourceUrl(env); const origin = resourceOrigin(env); const host = new URL(origin).hostname; @@ -35,7 +45,7 @@ ${resource}. ${host} is a resource server only — it verifies bearer tokens but never issues them and runs no authorization flow itself. The authorization -server is identity.nyuchi.com (WorkOS Connect), outside this repo; this page +server is WorkOS Connect (${issuerLabel}), outside this repo; this page describes our side of the handshake only and does not restate or invent authorization-server metadata on its behalf. @@ -46,8 +56,8 @@ authorization-server metadata on its behalf. ## Authorization server -- Issuer: ${ISSUER} (WorkOS Connect) -- Metadata: ${ISSUER}/.well-known/oauth-authorization-server +- Issuer: ${issuerLabel} (WorkOS Connect) +- Metadata: ${issuerMetadata} (mirrored at /.well-known/oauth-authorization-server on this domain) - Flow: OAuth 2.1 Authorization Code + PKCE - Client registration: Dynamic Client Registration (RFC 7591) at the @@ -62,11 +72,11 @@ Send the access token as \`Authorization: Bearer \` on requests to \`/mcp\`. Unauthenticated requests receive \`401\` with a \`WWW-Authenticate\` header pointing back at the protected-resource metadata. -## When auth is not required +## When auth is not configured -This server can run in an open mode (no \`AUTHKIT_DOMAIN\` configured) for -local development; the protected-resource metadata endpoint returns \`404\` -in that mode to signal no authorization is needed, and \`/mcp\` accepts -requests without a bearer token. +\`/mcp\` fails closed: if \`AUTHKIT_DOMAIN\` is not configured it answers +\`503\` and serves no tools. The only exception is local development, with +\`ALLOW_UNAUTHENTICATED_DEV=true\` set explicitly and requests addressed to +\`localhost\`; that setting is never present in a deployed environment. `; } diff --git a/mcp/src/auth.ts b/mcp/src/auth.ts index a26ca2a..aa112c7 100644 --- a/mcp/src/auth.ts +++ b/mcp/src/auth.ts @@ -1,9 +1,11 @@ /** * Optional WorkOS Connect (OAuth 2.1) protection for the MCP endpoint. * - * Config comes from Worker vars (wrangler.toml [vars] or dashboard secrets): + * Config comes from the Worker's settings — never from committed config: * AUTHKIT_DOMAIN — the WorkOS AuthKit/Connect domain for the environment, - * e.g. "your-workspace.authkit.app". When UNSET, the MCP + * a bare host or an https origin. Set per environment as a + * Worker secret (never a wrangler.toml [vars] entry, and + * there is no default in code). When UNSET, the MCP * server runs open (no auth) and OAuth discovery endpoints * return 404s that tell clients no sign-in is needed. * MCP_RESOURCE — this server's canonical resource URL @@ -15,7 +17,7 @@ * - Unauthenticated /mcp requests get 401 + WWW-Authenticate pointing at * that metadata, which is how MCP clients discover the OAuth flow. * - Bearer tokens are JWTs verified against the WorkOS JWKS - * (https:///oauth2/jwks) with issuer + audience checks. + * (/oauth2/jwks) with issuer + audience checks. * * Client registration (CIMD / dynamic client registration) is handled by * WorkOS itself — enable it in the WorkOS dashboard under @@ -27,22 +29,85 @@ import { createRemoteJWKSet, jwtVerify } from "jose"; export interface AuthEnv { AUTHKIT_DOMAIN?: string; MCP_RESOURCE?: string; + /** + * Local development only: `"true"` lets `/mcp` run without a bearer token + * when AUTHKIT_DOMAIN is unset AND the request is to localhost. Never set in + * any deployed environment (it is not in wrangler.toml); put it in + * `.dev.vars` for `wrangler dev`. + */ + ALLOW_UNAUTHENTICATED_DEV?: string; } // Must match the "AuthKit OAuth resource" registered in the WorkOS dashboard // for this integration — see the MCP_RESOURCE comment in wrangler.toml. export const DEFAULT_RESOURCE = "https://tools.nyuchi.dev/mcp"; +/** Message used whenever a flow needs AUTHKIT_DOMAIN and it is unset. */ +export const AUTHKIT_DOMAIN_MISSING = "AUTHKIT_DOMAIN is not configured"; + +/** + * True when AUTHKIT_DOMAIN is set at all. Deliberately presence-only: this + * switches auth ON, and an unusable value must not switch it OFF (that would + * fail open). With auth on and an invalid value, `issuerUrl` throws, so bearer + * verification returns null (401) and the OAuth surfaces error — fail closed. + */ export function authConfigured(env: AuthEnv): boolean { - return typeof env.AUTHKIT_DOMAIN === "string" && env.AUTHKIT_DOMAIN.length > 0; + return typeof env.AUTHKIT_DOMAIN === "string" && env.AUTHKIT_DOMAIN.trim().length > 0; +} + +const LOCAL_HOSTNAMES = new Set(["localhost", "127.0.0.1", "[::1]"]); + +/** + * True only for the explicit local-development opt-out: AUTHKIT_DOMAIN unset, + * `ALLOW_UNAUTHENTICATED_DEV` exactly `"true"`, and the request addressed to + * localhost. Everywhere else an unset AUTHKIT_DOMAIN fails closed — `/mcp` + * never serves tools unauthenticated by default. + */ +export function unauthenticatedDevAllowed(env: AuthEnv, requestUrl: string): boolean { + if (authConfigured(env)) return false; + if (env.ALLOW_UNAUTHENTICATED_DEV !== "true") return false; + try { + return LOCAL_HOSTNAMES.has(new URL(requestUrl).hostname); + } catch { + return false; + } } export function resourceUrl(env: AuthEnv): string { return env.MCP_RESOURCE || DEFAULT_RESOURCE; } +/** + * Parse — never concatenate — a configured AuthKit domain into an https origin. + * + * Accepts a bare host or an https origin, in any case. Any path, query or + * fragment is dropped. A blank value, `http:`, any other scheme, embedded + * credentials and anything `URL` cannot parse all throw an error whose message + * starts with `AUTHKIT_DOMAIN_MISSING`. The result is `URL.origin`. + */ +export function normaliseAuthkitDomain(value: string | undefined): string { + const raw = value?.trim(); + if (!raw) throw new Error(AUTHKIT_DOMAIN_MISSING); + let url: URL; + try { + url = new URL(/^[a-z][a-z0-9+.-]*:\/\//i.test(raw) ? raw : `https://${raw}`); + } catch { + throw new Error(`${AUTHKIT_DOMAIN_MISSING} (not a valid host or URL)`); + } + if (url.protocol !== "https:" || url.username || url.password) { + throw new Error(`${AUTHKIT_DOMAIN_MISSING} (must be an https origin)`); + } + return url.origin; +} + +/** + * The AuthKit issuer origin, from configuration only, parsed by + * `normaliseAuthkitDomain` (the `iss` check is an exact string match against + * it). Throws when AUTHKIT_DOMAIN is unset or is not a bare host / https + * origin — callers check `authConfigured` first; there is no fallback host. + */ export function issuerUrl(env: AuthEnv): string { - return `https://${env.AUTHKIT_DOMAIN}`; + return normaliseAuthkitDomain(env.AUTHKIT_DOMAIN); } /** @@ -77,11 +142,11 @@ export function wwwAuthenticateHeader(env: AuthEnv): string { /* JWKS instances are cached per AuthKit domain for the isolate's lifetime. */ const jwksCache = new Map>(); -function jwksFor(domain: string): ReturnType { - let jwks = jwksCache.get(domain); +function jwksFor(issuer: string): ReturnType { + let jwks = jwksCache.get(issuer); if (!jwks) { - jwks = createRemoteJWKSet(new URL(`https://${domain}/oauth2/jwks`)); - jwksCache.set(domain, jwks); + jwks = createRemoteJWKSet(new URL("/oauth2/jwks", issuer)); + jwksCache.set(issuer, jwks); } return jwks; } @@ -117,10 +182,11 @@ export async function verifyJwt( token: string | undefined, expectedAudience?: string, ): Promise { - if (!token || !env.AUTHKIT_DOMAIN) return null; + if (!token || !authConfigured(env)) return null; try { - const { payload } = await jwtVerify(token, jwksFor(env.AUTHKIT_DOMAIN), { - issuer: issuerUrl(env), + const issuer = issuerUrl(env); + const { payload } = await jwtVerify(token, jwksFor(issuer), { + issuer, audience: expectedAudience ?? resourceUrl(env), }); const scopes = diff --git a/mcp/src/index.ts b/mcp/src/index.ts index 1916dad..8b7a091 100644 --- a/mcp/src/index.ts +++ b/mcp/src/index.ts @@ -14,7 +14,7 @@ * for MCP clients and agents. See auth.ts. * - `/login`, `/callback`, `/logout` — the site-wide login gate for human * visitors, an Authorization Code + PKCE flow against the Hosted AuthKit - * UI at identity.nyuchi.com. See site-auth.ts. + * UI on the configured AUTHKIT_DOMAIN. See site-auth.ts. * - everything else — the built signature-generator Astro site, served as * static assets via `c.env.ASSETS.fetch(...)`, but only once the * site-wide login gate above has let the request through. @@ -37,9 +37,11 @@ import { z } from "zod"; import { deleteCookie, getCookie, setCookie } from "hono/cookie"; import { type AuthEnv, + AUTHKIT_DOMAIN_MISSING, authConfigured, issuerUrl, protectedResourceMetadata, + unauthenticatedDevAllowed, verifyBearer, verifyJwt, wwwAuthenticateHeader, @@ -63,7 +65,9 @@ import { SESSION_COOKIE_NAME, sessionCookieOptions, type SessionClaims, - SITE_CLIENT_ID, + siteClientConfigured, + siteClientId, + WORKOS_CLIENT_ID_MISSING, type SiteAuthEnv, verifySessionCookie, } from "./site-auth.js"; @@ -664,7 +668,7 @@ function buildServer(env: Env): McpServer { * - Auth OFF (no AUTHKIT_DOMAIN): there is no authorization server to * mirror, so this keeps returning the same JSON 404 as before. * - Auth ON: fetches (never fabricates) the real document from - * identity.nyuchi.com and passes it through; a fetch failure or a + * the configured AUTHKIT_DOMAIN and passes it through; a fetch failure or a * non-200 upstream response becomes a 502 — never fake metadata. */ function authorizationServerMetadataHandler(wellKnownPath: "oauth-authorization-server" | "openid-configuration") { @@ -678,7 +682,7 @@ function authorizationServerMetadataHandler(wellKnownPath: "oauth-authorization- 404, ); } - const upstream = `${issuerUrl(c.env)}/.well-known/${wellKnownPath}`; + const upstream = new URL(`/.well-known/${wellKnownPath}`, issuerUrl(c.env)).href; const result = await fetchMetadata(upstream); if (!result.ok) { return c.json({ error: "upstream_fetch_failed", detail: result.message }, 502); @@ -769,6 +773,14 @@ app.get("/login", async (c) => { // the misconfiguration instead of silently granting or looping. return c.text("Site authentication is not configured.", 500); } + if (!authConfigured(c.env)) { + // Fail CLOSED: the authorization server comes only from configuration. + return c.text(`Service Unavailable: ${AUTHKIT_DOMAIN_MISSING}`, 503); + } + if (!siteClientConfigured(c.env)) { + // Fail CLOSED: the client ID comes only from configuration too. + return c.text(`Service Unavailable: ${WORKOS_CLIENT_ID_MISSING}`, 503); + } const returnTo = sanitizeReturnTo(c.req.query("return_to")); const state = generateState(); const codeVerifier = generateCodeVerifier(); @@ -796,14 +808,16 @@ app.get(CALLBACK_PATH, async (c) => { if (!oauthPayload || !code || !state || state !== oauthPayload.state) { return denyLogin(); } - if (!c.env.SESSION_SECRET) { - // Fail CLOSED: never mint a session without a configured secret. + if (!c.env.SESSION_SECRET || !authConfigured(c.env) || !siteClientConfigured(c.env)) { + // Fail CLOSED: never mint a session without a configured secret, and + // never exchange a code without a configured authorization server and + // client ID. return denyLogin(); } let idToken: string | undefined; try { - const tokenResponse = await exchangeCode(code, oauthPayload.codeVerifier); + const tokenResponse = await exchangeCode(c.env, code, oauthPayload.codeVerifier); idToken = tokenResponse.id_token; } catch { return denyLogin(); @@ -817,7 +831,7 @@ app.get(CALLBACK_PATH, async (c) => { // Verify the id_token, not the access_token: an id_token's `aud` is the // OAuth client_id per OIDC Core (§2), which has nothing to do with the // /mcp resource indicator `verifyJwt` defaults to for bearer-token calls. - const verified = await verifyJwt(c.env, idToken, SITE_CLIENT_ID); + const verified = await verifyJwt(c.env, idToken, siteClientId(c.env)); if (!verified) { return denyLogin(); } @@ -839,36 +853,40 @@ app.get("/logout", (c) => { return c.redirect("/", 302); }); -// OAuth surface. Behavior is driven by the AUTHKIT_DOMAIN Worker var: +// OAuth surface. Behavior is driven by the AUTHKIT_DOMAIN Worker secret: // -// unset → the MCP server is OPEN. Discovery endpoints return JSON 404s so -// MCP clients (e.g. claude.ai connectors) conclude "no sign-in needed" -// instead of hitting the SPA fallback's 200 text/html and inventing a -// broken sign-in service. +// unset → FAIL CLOSED. `/mcp` answers 503 "AUTHKIT_DOMAIN is not +// configured" and the protected-resource metadata says the same, so no +// client is ever served tools unauthenticated. The one exception is the +// explicit local-development opt-out (`ALLOW_UNAUTHENTICATED_DEV=true`, +// only on localhost — see `unauthenticatedDevAllowed`), where discovery +// returns JSON 404s so a local MCP client concludes "no sign-in needed". // // set → WorkOS Connect protects /mcp. The protected-resource metadata // advertises the WorkOS authorization server; client registration and // the actual OAuth flow happen on WorkOS, not here. The authorization // server's own discovery documents (oauth-authorization-server, -// openid-configuration) are mirrored — fetched from identity.nyuchi.com +// openid-configuration) are mirrored — fetched from AUTHKIT_DOMAIN // and passed through, never fabricated — onto this domain's // `.well-known` paths so agents that only probe the resource server // still find them. // // These paths reach the Worker via assets.run_worker_first in wrangler.toml. -app.all("/.well-known/oauth-protected-resource", (c) => { +function protectedResourceResponse(c: Context<{ Bindings: Env; Variables: Variables }>) { if (authConfigured(c.env)) return c.json(protectedResourceMetadata(c.env)); - return c.json( - { error: "no_protected_resource_metadata", detail: "This MCP server does not require authentication." }, - 404, - ); + if (unauthenticatedDevAllowed(c.env, c.req.url)) { + return c.json( + { error: "no_protected_resource_metadata", detail: "Local development: this MCP server does not require authentication." }, + 404, + ); + } + return c.json({ error: "authorization_not_configured", detail: AUTHKIT_DOMAIN_MISSING }, 503); +} +app.all("/.well-known/oauth-protected-resource", (c) => { + return protectedResourceResponse(c); }); app.all("/.well-known/oauth-protected-resource/*", (c) => { - if (authConfigured(c.env)) return c.json(protectedResourceMetadata(c.env)); - return c.json( - { error: "no_protected_resource_metadata", detail: "This MCP server does not require authentication." }, - 404, - ); + return protectedResourceResponse(c); }); app.all("/.well-known/oauth-authorization-server", authorizationServerMetadataHandler("oauth-authorization-server")); app.all("/.well-known/oauth-authorization-server/*", authorizationServerMetadataHandler("oauth-authorization-server")); @@ -887,9 +905,18 @@ app.get("/.well-known/mcp/server-card.json", (c) => c.json(buildServerCard(SERVE // wrangler.toml's assets.run_worker_first. app.get("/auth.md", (c) => c.text(authMd(c.env), 200, { "Content-Type": "text/markdown; charset=utf-8" })); -// Bearer-token gate for /mcp — no-op until AUTHKIT_DOMAIN is configured. +// Bearer-token gate for /mcp. Fails CLOSED: with AUTHKIT_DOMAIN unset, /mcp +// answers 503 and never serves tools unauthenticated — except the explicit +// localhost-only development opt-out (`unauthenticatedDevAllowed`). With it set +// but unusable, bearer verification fails and every request is 401. app.use("/mcp", async (c, next) => { - if (!authConfigured(c.env)) return next(); + if (!authConfigured(c.env)) { + if (unauthenticatedDevAllowed(c.env, c.req.url)) return next(); + return c.json( + { error: "authorization_not_configured", detail: `Service Unavailable: ${AUTHKIT_DOMAIN_MISSING}` }, + 503, + ); + } const verified = await verifyBearer(c.env, c.req.header("Authorization")); if (!verified) { return c.json( diff --git a/mcp/src/site-auth.ts b/mcp/src/site-auth.ts index 91b8f27..74eb028 100644 --- a/mcp/src/site-auth.ts +++ b/mcp/src/site-auth.ts @@ -9,8 +9,8 @@ * that flow succeeds. * * This reuses the SAME WorkOS Connect app that already protects /mcp - * (client_01KVTX0V2K1VM3PSC0DJ9VZWTV, authorization server - * identity.nyuchi.com — see auth.ts) as a public client (PKCE, + * (client ID = the configured WORKOS_CLIENT_ID; authorization server = the + * configured AUTHKIT_DOMAIN — see auth.ts) as a public client (PKCE, * token_endpoint_auth_method=none, no client_secret). The access token * returned by the token exchange is verified with the exact same * JWKS/issuer/audience logic /mcp uses for bearer tokens (`verifyJwt` in @@ -31,14 +31,36 @@ */ import { SignJWT, base64url, jwtVerify } from "jose"; -import type { AuthEnv } from "./auth.js"; +import { type AuthEnv, issuerUrl } from "./auth.js"; export interface SiteAuthEnv extends AuthEnv { SESSION_SECRET?: string; + /** + * The WorkOS Connect public client ID for the site login (the "Nyuchi + * Internal Tools" app). Public, but environment-specific: set per + * environment as a Worker secret, never compiled in. Without it /login + * answers 503 and /callback denies. + */ + WORKOS_CLIENT_ID?: string; } -/** The public client already provisioned in WorkOS for tools.nyuchi.com. */ -export const SITE_CLIENT_ID = "client_01KVTX0V2K1VM3PSC0DJ9VZWTV"; +/** Message used whenever the login flow needs WORKOS_CLIENT_ID and it is unset. */ +export const WORKOS_CLIENT_ID_MISSING = "WORKOS_CLIENT_ID is not configured"; + +/** True when WORKOS_CLIENT_ID is set to a non-blank value. */ +export function siteClientConfigured(env: SiteAuthEnv): boolean { + return typeof env.WORKOS_CLIENT_ID === "string" && env.WORKOS_CLIENT_ID.trim().length > 0; +} + +/** + * The configured site client ID. Throws (fails closed) when WORKOS_CLIENT_ID + * is unset — there is no fallback client; callers check + * `siteClientConfigured` first. + */ +export function siteClientId(env: SiteAuthEnv): string { + if (!siteClientConfigured(env)) throw new Error(WORKOS_CLIENT_ID_MISSING); + return (env.WORKOS_CLIENT_ID as string).trim(); +} export const CALLBACK_PATH = "/callback"; export const SESSION_COOKIE_NAME = "nyuchi_session"; @@ -52,8 +74,15 @@ export const OAUTH_COOKIE_NAME = "nyuchi_oauth"; const SITE_ORIGIN = "https://tools.nyuchi.com"; const REDIRECT_URI = `${SITE_ORIGIN}${CALLBACK_PATH}`; -const AUTHORIZE_ENDPOINT = "https://identity.nyuchi.com/oauth2/authorize"; -const TOKEN_ENDPOINT = "https://identity.nyuchi.com/oauth2/token"; +// The authorize and token endpoints live on the configured AuthKit domain +// (AUTHKIT_DOMAIN) — never a compiled-in host. `issuerUrl` throws when it is +// unset; /login checks `authConfigured` first and answers 503. +function authorizeEndpoint(env: AuthEnv): string { + return new URL("/oauth2/authorize", issuerUrl(env)).href; +} +function tokenEndpoint(env: AuthEnv): string { + return new URL("/oauth2/token", issuerUrl(env)).href; +} /** Session cookie lifetime: 7 days. */ const SESSION_TTL_SECONDS = 7 * 24 * 60 * 60; @@ -143,11 +172,10 @@ export function buildAuthorizeUrl( codeChallenge: string, returnTo: string, ): string { - void env; void returnTo; - const url = new URL(AUTHORIZE_ENDPOINT); + const url = new URL(authorizeEndpoint(env)); url.searchParams.set("response_type", "code"); - url.searchParams.set("client_id", SITE_CLIENT_ID); + url.searchParams.set("client_id", siteClientId(env)); url.searchParams.set("redirect_uri", REDIRECT_URI); url.searchParams.set("code_challenge", codeChallenge); url.searchParams.set("code_challenge_method", "S256"); @@ -171,15 +199,19 @@ export interface TokenResponse { * Throws on any non-2xx response or a malformed body; never returns a * partial/fabricated token. */ -export async function exchangeCode(code: string, codeVerifier: string): Promise { +export async function exchangeCode( + env: SiteAuthEnv, + code: string, + codeVerifier: string, +): Promise { const body = new URLSearchParams({ grant_type: "authorization_code", code, redirect_uri: REDIRECT_URI, - client_id: SITE_CLIENT_ID, + client_id: siteClientId(env), code_verifier: codeVerifier, }); - const response = await fetch(TOKEN_ENDPOINT, { + const response = await fetch(tokenEndpoint(env), { method: "POST", headers: { "Content-Type": "application/x-www-form-urlencoded" }, body: body.toString(), diff --git a/mcp/tests/worker.test.ts b/mcp/tests/worker.test.ts index 16a67bb..ba84c45 100644 --- a/mcp/tests/worker.test.ts +++ b/mcp/tests/worker.test.ts @@ -13,6 +13,7 @@ import { join } from 'node:path' import { afterEach, describe, expect, it, vi } from 'vite-plus/test' import { SignJWT, base64url } from 'jose' import worker from '../src/index' +import { AUTHKIT_DOMAIN_MISSING, issuerUrl, normaliseAuthkitDomain, verifyJwt } from '../src/auth' import { CALLBACK_PATH, decodeOauthCookie, @@ -20,7 +21,8 @@ import { mintSessionCookie, OAUTH_COOKIE_NAME, SESSION_COOKIE_NAME, - SITE_CLIENT_ID, + siteClientId, + WORKOS_CLIENT_ID_MISSING, verifySessionCookie, } from '../src/site-auth' import { @@ -42,8 +44,13 @@ import { buildSignatureHtml } from '../../signature-generator/src/engines/signat const BASE = 'https://tools.nyuchi.com' -/** No auth configured: the MCP server runs open. */ -const OPEN_ENV = {} +/** Nothing configured. With AUTHKIT_DOMAIN unset, /mcp fails closed (503). */ +const NO_CONFIG_ENV = {} + +/** The explicit local-development opt-out: /mcp runs without a bearer token, + * but only for requests to localhost (see LOCAL_BASE). Never set in production. */ +const LOCAL_DEV_ENV = { ALLOW_UNAUTHENTICATED_DEV: 'true' } +const LOCAL_BASE = 'http://localhost:8787' /** Auth configured: WorkOS AuthKit protects /mcp. */ const AUTH_ENV = { AUTHKIT_DOMAIN: 'x.authkit.app' } @@ -51,6 +58,10 @@ const AUTH_ENV = { AUTHKIT_DOMAIN: 'x.authkit.app' } /** Site-wide login gate configured with a known, throwaway test secret. */ const TEST_SESSION_SECRET = 'test-secret-do-not-use-in-prod' const SITE_ENV = { SESSION_SECRET: TEST_SESSION_SECRET } +/** Throwaway site client ID (test fixture) — the real one is configuration only. */ +const TEST_CLIENT_ID = 'client_test_site_login' +/** SITE_ENV plus an AuthKit domain and client ID — the login flow needs all three (test fixture). */ +const LOGIN_ENV = { ...SITE_ENV, AUTHKIT_DOMAIN: 'identity.example.test', WORKOS_CLIENT_ID: TEST_CLIENT_ID } /** Stub ASSETS binding: the real one only exists in the real Workers * runtime, so tests that need a request to reach the post-auth catch-all @@ -89,6 +100,7 @@ const FONT_ASSETS_STUB = { /** Env with Cloudflare Images + GitHub feedback configured (values fake; * the corresponding fetches are mocked per test). */ const UPLOAD_ENV = { + ...LOCAL_DEV_ENV, ASSETS: FONT_ASSETS_STUB, CF_IMAGES_ACCOUNT_ID: 'acct123', CF_IMAGES_TOKEN: 'tok123', @@ -99,14 +111,20 @@ const PNG_MAGIC = [0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a] type Env = Record -function get(path: string, env: Env = OPEN_ENV, headers?: HeadersInit): Promise { - return Promise.resolve(worker.fetch(new Request(`${BASE}${path}`, { headers }), env)) +/** Requests under the local-development opt-out go to localhost, as they would + * under `wrangler dev`; everything else to the production host. */ +function baseFor(env: Env): string { + return env.ALLOW_UNAUTHENTICATED_DEV === 'true' ? LOCAL_BASE : BASE +} + +function get(path: string, env: Env = LOCAL_DEV_ENV, headers?: HeadersInit): Promise { + return Promise.resolve(worker.fetch(new Request(`${baseFor(env)}${path}`, { headers }), env)) } -function post(path: string, body: unknown, env: Env = OPEN_ENV): Promise { +function post(path: string, body: unknown, env: Env = LOCAL_DEV_ENV): Promise { return Promise.resolve( worker.fetch( - new Request(`${BASE}${path}`, { + new Request(`${baseFor(env)}${path}`, { method: 'POST', headers: { 'content-type': 'application/json' }, body: typeof body === 'string' ? body : JSON.stringify(body), @@ -476,7 +494,7 @@ describe('POST /mcp — JSON-RPC', () => { }, 18, ), - { ...OPEN_ENV, ASSETS: ICON_ASSETS_STUB }, + { ...LOCAL_DEV_ENV, ASSETS: ICON_ASSETS_STUB }, ) expect(res.status).toBe(200) const body = (await res.json()) as JsonRpcResponse @@ -606,7 +624,7 @@ describe('nyuchi_generate_studio_card — returnFormat / upload', () => { }, 42, ), - { ...OPEN_ENV, ASSETS: FONT_ASSETS_STUB }, + { ...LOCAL_DEV_ENV, ASSETS: FONT_ASSETS_STUB }, ) expect(res.status).toBe(200) const body = (await res.json()) as JsonRpcResponse @@ -684,7 +702,7 @@ describe('nyuchi_generate_studio_card — returnFormat / upload', () => { }, 43, ), - { ...OPEN_ENV, ASSETS: FONT_ASSETS_STUB }, + { ...LOCAL_DEV_ENV, ASSETS: FONT_ASSETS_STUB }, ) const body = (await res.json()) as JsonRpcResponse const result = body.result as { isError: boolean; content: { text: string }[] } @@ -875,7 +893,7 @@ describe('nyuchi_upload_asset', () => { const res = await post( '/mcp', rpc('tools/call', { name: 'nyuchi_upload_asset', arguments: { pngBase64: TINY_PNG_B64 } }, 54), - { ...OPEN_ENV, ASSETS: FONT_ASSETS_STUB }, + { ...LOCAL_DEV_ENV, ASSETS: FONT_ASSETS_STUB }, ) const body = (await res.json()) as JsonRpcResponse const result = body.result as { isError: boolean; content: { text: string }[] } @@ -985,18 +1003,72 @@ describe('nyuchi_report_issue', () => { }) }) -describe('OAuth discovery — auth OFF (no AUTHKIT_DOMAIN)', () => { - it('protected-resource metadata is a JSON 404 (open server)', async () => { - const res = await get('/.well-known/oauth-protected-resource') - expect(res.status).toBe(404) - const body = (await res.json()) as { error: string } - expect(body.error).toBe('no_protected_resource_metadata') +describe('OAuth discovery — auth OFF (no AUTHKIT_DOMAIN): fails closed', () => { + it('protected-resource metadata is a 503 naming AUTHKIT_DOMAIN', async () => { + for (const path of ['/.well-known/oauth-protected-resource', '/.well-known/oauth-protected-resource/mcp']) { + const res = await get(path, NO_CONFIG_ENV) + expect(res.status, path).toBe(503) + const body = (await res.json()) as { error: string; detail: string } + expect(body.error).toBe('authorization_not_configured') + expect(body.detail).toContain('AUTHKIT_DOMAIN is not configured') + } + }) + + it('GET /mcp is 503, never open', async () => { + const res = await get('/mcp', NO_CONFIG_ENV) + expect(res.status).toBe(503) + const body = (await res.json()) as { error: string; detail: string } + expect(body.error).toBe('authorization_not_configured') + expect(body.detail).toContain('AUTHKIT_DOMAIN is not configured') + }) + + it('POST /mcp serves no tools without auth', async () => { + const res = await post('/mcp', rpc('tools/list'), NO_CONFIG_ENV) + expect(res.status).toBe(503) + expect(await res.text()).not.toContain('"tools"') + }) + + it('the dev opt-out does not apply to a non-localhost host', async () => { + const res = await Promise.resolve( + worker.fetch( + new Request(`${BASE}/mcp`, { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify(rpc('tools/list')), + }), + LOCAL_DEV_ENV, + ), + ) + expect(res.status).toBe(503) }) + it('localhost alone does not open /mcp — the opt-out must be exactly "true"', async () => { + for (const env of [NO_CONFIG_ENV, { ALLOW_UNAUTHENTICATED_DEV: '1' }, { ALLOW_UNAUTHENTICATED_DEV: 'TRUE' }]) { + const res = await Promise.resolve(worker.fetch(new Request(`${LOCAL_BASE}/mcp`), env)) + expect(res.status).toBe(503) + } + }) + + it('the dev opt-out never overrides a configured AUTHKIT_DOMAIN', async () => { + const res = await Promise.resolve( + worker.fetch(new Request(`${LOCAL_BASE}/mcp`), { ...LOCAL_DEV_ENV, ...AUTH_ENV }), + ) + expect(res.status).toBe(401) + }) +}) + +describe('Local development opt-out (ALLOW_UNAUTHENTICATED_DEV=true on localhost)', () => { it('/mcp requires no token', async () => { - const res = await get('/mcp') + const res = await get('/mcp', LOCAL_DEV_ENV) expect(res.status).toBe(200) }) + + it('protected-resource metadata is a JSON 404 (no sign-in needed locally)', async () => { + const res = await get('/.well-known/oauth-protected-resource', LOCAL_DEV_ENV) + expect(res.status).toBe(404) + const body = (await res.json()) as { error: string } + expect(body.error).toBe('no_protected_resource_metadata') + }) }) describe('OAuth discovery — auth ON (AUTHKIT_DOMAIN set)', () => { @@ -1090,14 +1162,20 @@ describe('GET /auth.md', () => { }) it('describes the real architecture without fabricating an agent_auth block', async () => { - const res = await get('/auth.md') + const res = await get('/auth.md', { AUTHKIT_DOMAIN: 'identity.example.test' }) const body = await res.text() - expect(body).toContain('https://identity.nyuchi.com') + expect(body).toContain('https://identity.example.test') expect(body).toContain('/.well-known/oauth-protected-resource') expect(body).toContain('tools.nyuchi.dev is a resource server') expect(body).not.toContain('agent_auth') }) + it('names no authorization server when AUTHKIT_DOMAIN is unset — never a compiled-in host', async () => { + const body = await (await get('/auth.md')).text() + expect(body).toContain('AUTHKIT_DOMAIN is unset') + expect(body).not.toMatch(/https:\/\/[^\s]*(identity|accounts|authkit)/) + }) + it('is reachable the same way in auth-on mode too', async () => { const res = await get('/auth.md', AUTH_ENV) expect(res.status).toBe(200) @@ -1181,19 +1259,22 @@ describe('Authorization server metadata mirror', () => { }) describe('Site-wide login gate — exempt paths still work with zero cookies', () => { + // AUTH_ENV puts /mcp behind its own bearer gate: the answer is that gate's + // 401 (or the metadata), never the site gate's redirect to /login. it('/mcp (GET discovery) needs no session cookie', async () => { - const res = await get('/mcp', SITE_ENV) - expect(res.status).toBe(200) + const res = await get('/mcp', { ...SITE_ENV, ...AUTH_ENV }) + expect(res.status).toBe(401) + expect(res.headers.get('WWW-Authenticate')).toContain('Bearer') }) it('/mcp (POST JSON-RPC) needs no session cookie', async () => { - const res = await post('/mcp', rpc('tools/list', {}, 1), SITE_ENV) + const res = await post('/mcp', rpc('tools/list', {}, 1), { ...SITE_ENV, ...LOCAL_DEV_ENV }) expect(res.status).toBe(200) }) it('/.well-known/oauth-protected-resource needs no session cookie', async () => { - const res = await get('/.well-known/oauth-protected-resource', SITE_ENV) - expect(res.status).toBe(404) // auth not configured for /mcp in this env — still reachable, not redirected + const res = await get('/.well-known/oauth-protected-resource', { ...SITE_ENV, ...AUTH_ENV }) + expect(res.status).toBe(200) // the metadata itself — reachable, not redirected }) it('/.well-known/mcp/server-card.json needs no session cookie', async () => { @@ -1228,7 +1309,7 @@ describe('Site-wide login gate — protected paths', () => { it('denies access (redirects, never passes through) when SESSION_SECRET is unset entirely', async () => { // Fail CLOSED: no SESSION_SECRET at all must behave exactly like "no // valid session", never like "auth is off". - const res = await get('/', OPEN_ENV) + const res = await get('/', NO_CONFIG_ENV) expect(res.status).toBe(302) expect(res.headers.get('Location')).toContain('/login?return_to=') }) @@ -1251,20 +1332,41 @@ describe('Site-wide login gate — protected paths', () => { describe('GET /login', () => { it('returns 500 (fails closed) when SESSION_SECRET is not configured', async () => { - const res = await get('/login', OPEN_ENV) + const res = await get('/login', NO_CONFIG_ENV) expect(res.status).toBe(500) }) - it('sets the oauth cookie and redirects to the authorize endpoint with the right params', async () => { + it('returns 503 (fails closed) when AUTHKIT_DOMAIN is not configured — no default host', async () => { const res = await get('/login', SITE_ENV) + expect(res.status).toBe(503) + expect(await res.text()).toContain('AUTHKIT_DOMAIN is not configured') + }) + + it('returns 503 (fails closed) when WORKOS_CLIENT_ID is not configured — no compiled-in client', async () => { + for (const WORKOS_CLIENT_ID of [undefined, '', ' ']) { + const res = await get('/login', { ...LOGIN_ENV, WORKOS_CLIENT_ID }) + expect(res.status).toBe(503) + expect(await res.text()).toContain('WORKOS_CLIENT_ID is not configured') + expect(res.headers.get('Set-Cookie')).toBeNull() + } + }) + + it('siteClientId reads only configuration and throws when it is unset', () => { + expect(siteClientId({ WORKOS_CLIENT_ID: ' client_abc ' })).toBe('client_abc') + expect(() => siteClientId({})).toThrow(WORKOS_CLIENT_ID_MISSING) + expect(() => siteClientId({ WORKOS_CLIENT_ID: ' ' })).toThrow(WORKOS_CLIENT_ID_MISSING) + }) + + it('sets the oauth cookie and redirects to the authorize endpoint with the right params', async () => { + const res = await get('/login', LOGIN_ENV) expect(res.status).toBe(302) const location = res.headers.get('Location') expect(location).toBeTruthy() const url = new URL(location!) - expect(url.origin + url.pathname).toBe('https://identity.nyuchi.com/oauth2/authorize') + expect(url.origin + url.pathname).toBe('https://identity.example.test/oauth2/authorize') expect(url.searchParams.get('response_type')).toBe('code') - expect(url.searchParams.get('client_id')).toBe(SITE_CLIENT_ID) + expect(url.searchParams.get('client_id')).toBe(TEST_CLIENT_ID) expect(url.searchParams.get('redirect_uri')).toBe('https://tools.nyuchi.com/callback') expect(url.searchParams.get('code_challenge_method')).toBe('S256') expect(url.searchParams.get('scope')).toBe('openid profile email') @@ -1285,7 +1387,7 @@ describe('GET /login', () => { }) it('rejects an absolute-URL return_to and stores "/" instead', async () => { - const res = await get(`/login?return_to=${encodeURIComponent('https://evil.com')}`, SITE_ENV) + const res = await get(`/login?return_to=${encodeURIComponent('https://evil.com')}`, LOGIN_ENV) expect(res.status).toBe(302) const oauthValue = cookieValueFrom(res.headers.get('Set-Cookie'), OAUTH_COOKIE_NAME) const payload = decodeOauthCookie(oauthValue ?? undefined) @@ -1293,14 +1395,14 @@ describe('GET /login', () => { }) it('rejects a protocol-relative return_to ("//evil.com") and stores "/" instead', async () => { - const res = await get(`/login?return_to=${encodeURIComponent('//evil.com')}`, SITE_ENV) + const res = await get(`/login?return_to=${encodeURIComponent('//evil.com')}`, LOGIN_ENV) const oauthValue = cookieValueFrom(res.headers.get('Set-Cookie'), OAUTH_COOKIE_NAME) const payload = decodeOauthCookie(oauthValue ?? undefined) expect(payload?.returnTo).toBe('/') }) it('rejects a return_to containing a CRLF (header-injection attempt) and stores "/" instead', async () => { - const res = await get(`/login?return_to=${encodeURIComponent('/studio\r\nSet-Cookie: evil=1')}`, SITE_ENV) + const res = await get(`/login?return_to=${encodeURIComponent('/studio\r\nSet-Cookie: evil=1')}`, LOGIN_ENV) expect(res.status).toBe(302) const oauthValue = cookieValueFrom(res.headers.get('Set-Cookie'), OAUTH_COOKIE_NAME) const payload = decodeOauthCookie(oauthValue ?? undefined) @@ -1308,7 +1410,7 @@ describe('GET /login', () => { }) it('accepts a legitimate same-origin relative return_to', async () => { - const res = await get(`/login?return_to=${encodeURIComponent('/studio')}`, SITE_ENV) + const res = await get(`/login?return_to=${encodeURIComponent('/studio')}`, LOGIN_ENV) const oauthValue = cookieValueFrom(res.headers.get('Set-Cookie'), OAUTH_COOKIE_NAME) const payload = decodeOauthCookie(oauthValue ?? undefined) expect(payload?.returnTo).toBe('/studio') @@ -1346,15 +1448,33 @@ describe('GET /callback', () => { it('redirects to /login (never crashes) when SESSION_SECRET is unset, even with matching state', async () => { const oauthCookie = encodeOauthCookie({ state: 'expected-state', codeVerifier: 'verifier', returnTo: '/' }) - const res = await get(`${CALLBACK_PATH}?code=abc&state=expected-state`, OPEN_ENV, { + const res = await get(`${CALLBACK_PATH}?code=abc&state=expected-state`, NO_CONFIG_ENV, { + Cookie: `${OAUTH_COOKIE_NAME}=${oauthCookie}`, + }) + expect(res.status).toBe(302) + expect(res.headers.get('Location')).toBe('/login?error=1') + }) + + it('redirects to /login without attempting the exchange when WORKOS_CLIENT_ID is unset', async () => { + const fetchSpy = vi.spyOn(globalThis, 'fetch') + const oauthCookie = encodeOauthCookie({ state: 'expected-state', codeVerifier: 'verifier', returnTo: '/' }) + const { WORKOS_CLIENT_ID: _omit, ...noClientEnv } = LOGIN_ENV + void _omit + const res = await get(`${CALLBACK_PATH}?code=abc&state=expected-state`, noClientEnv, { Cookie: `${OAUTH_COOKIE_NAME}=${oauthCookie}`, }) expect(res.status).toBe(302) expect(res.headers.get('Location')).toBe('/login?error=1') + expect(fetchSpy).not.toHaveBeenCalled() + fetchSpy.mockRestore() }) describe('successful exchange (mocked token + JWKS endpoints)', () => { - const CALLBACK_ENV = { SESSION_SECRET: TEST_SESSION_SECRET, AUTHKIT_DOMAIN: 'identity.nyuchi.com' } + const CALLBACK_ENV = { + SESSION_SECRET: TEST_SESSION_SECRET, + AUTHKIT_DOMAIN: 'identity.example.test', + WORKOS_CLIENT_ID: TEST_CLIENT_ID, + } afterEach(() => { vi.restoreAllMocks() @@ -1372,21 +1492,21 @@ describe('GET /callback', () => { const idToken = await new SignJWT({ email: claims.email }) .setProtectedHeader({ alg: 'RS256', kid }) .setIssuedAt() - .setIssuer('https://identity.nyuchi.com') - .setAudience(SITE_CLIENT_ID) + .setIssuer('https://identity.example.test') + .setAudience(TEST_CLIENT_ID) .setSubject(claims.sub) .setExpirationTime('5m') .sign(privateKey) vi.spyOn(globalThis, 'fetch').mockImplementation(async (input: RequestInfo | URL) => { const url = typeof input === 'string' ? input : input instanceof URL ? input.href : input.url - if (url === 'https://identity.nyuchi.com/oauth2/token') { + if (url === 'https://identity.example.test/oauth2/token') { return new Response( JSON.stringify({ access_token: 'unused-access-token', id_token: idToken, token_type: 'Bearer', expires_in: 300 }), { status: 200, headers: { 'content-type': 'application/json' } }, ) } - if (url === 'https://identity.nyuchi.com/oauth2/jwks') { + if (url === 'https://identity.example.test/oauth2/jwks') { return new Response(JSON.stringify({ keys: [{ ...publicJwk, kid, alg: 'RS256', use: 'sig' }] }), { status: 200, headers: { 'content-type': 'application/json' }, @@ -1415,7 +1535,7 @@ describe('GET /callback', () => { it('denies login when the id_token audience is wrong (e.g. an access token used by mistake)', async () => { // Same as above, but sign the token with aud=resourceUrl (the /mcp - // resource indicator) instead of aud=SITE_CLIENT_ID — the exact bug + // resource indicator) instead of aud=WORKOS_CLIENT_ID — the exact bug // this test guards against regressing to. const { generateKeyPair, exportJWK, SignJWT } = await import('jose') const { publicKey, privateKey } = await generateKeyPair('RS256') @@ -1424,7 +1544,7 @@ describe('GET /callback', () => { const wrongAudienceToken = await new SignJWT({ email: 'bryan@nyuchi.com' }) .setProtectedHeader({ alg: 'RS256', kid }) .setIssuedAt() - .setIssuer('https://identity.nyuchi.com') + .setIssuer('https://identity.example.test') .setAudience('https://tools.nyuchi.dev/mcp') .setSubject('user_123') .setExpirationTime('5m') @@ -1432,13 +1552,13 @@ describe('GET /callback', () => { vi.spyOn(globalThis, 'fetch').mockImplementation(async (input: RequestInfo | URL) => { const url = typeof input === 'string' ? input : input instanceof URL ? input.href : input.url - if (url === 'https://identity.nyuchi.com/oauth2/token') { + if (url === 'https://identity.example.test/oauth2/token') { return new Response( JSON.stringify({ access_token: 'unused', id_token: wrongAudienceToken, token_type: 'Bearer', expires_in: 300 }), { status: 200, headers: { 'content-type': 'application/json' } }, ) } - if (url === 'https://identity.nyuchi.com/oauth2/jwks') { + if (url === 'https://identity.example.test/oauth2/jwks') { return new Response(JSON.stringify({ keys: [{ ...publicJwk, kid, alg: 'RS256', use: 'sig' }] }), { status: 200, headers: { 'content-type': 'application/json' }, @@ -1462,7 +1582,7 @@ describe('GET /callback', () => { it('denies login when the token response has no id_token', async () => { vi.spyOn(globalThis, 'fetch').mockImplementation(async (input: RequestInfo | URL) => { const url = typeof input === 'string' ? input : input instanceof URL ? input.href : input.url - if (url === 'https://identity.nyuchi.com/oauth2/token') { + if (url === 'https://identity.example.test/oauth2/token') { return new Response(JSON.stringify({ access_token: 'unused', token_type: 'Bearer', expires_in: 300 }), { status: 200, headers: { 'content-type': 'application/json' }, @@ -2702,3 +2822,83 @@ describe('POST /mcp — resources & prompts (catalog)', () => { expect(text).toContain('layout 5') }) }) + +describe('AUTHKIT_DOMAIN is parsed into an https origin — never concatenated', () => { + afterEach(() => { + vi.restoreAllMocks() + }) + + it('fails closed when unset or blank', () => { + expect(() => normaliseAuthkitDomain(undefined)).toThrow(AUTHKIT_DOMAIN_MISSING) + expect(() => normaliseAuthkitDomain(' ')).toThrow(AUTHKIT_DOMAIN_MISSING) + expect(() => issuerUrl({})).toThrow(AUTHKIT_DOMAIN_MISSING) + }) + + it('accepts a bare host or an https origin in any case, and returns the origin', () => { + expect(issuerUrl({ AUTHKIT_DOMAIN: 'https://identity.example.test' })).toBe('https://identity.example.test') + expect(issuerUrl({ AUTHKIT_DOMAIN: 'identity.example.test' })).toBe('https://identity.example.test') + expect(issuerUrl({ AUTHKIT_DOMAIN: 'https://identity.example.test/' })).toBe('https://identity.example.test') + expect(issuerUrl({ AUTHKIT_DOMAIN: 'HTTPS://Identity.Example.Test' })).toBe('https://identity.example.test') + }) + + it('drops any path, query or fragment', () => { + expect(issuerUrl({ AUTHKIT_DOMAIN: 'https://identity.example.test/x/y?z=1#f' })).toBe( + 'https://identity.example.test', + ) + }) + + it('rejects anything that is not an https origin', () => { + for (const bad of [ + 'http://identity.example.test', + 'javascript://identity.example.test', + 'https://user:pass@identity.example.test', + 'user@identity.example.test', + 'https://', + ]) { + expect(() => issuerUrl({ AUTHKIT_DOMAIN: bad }), bad).toThrow(AUTHKIT_DOMAIN_MISSING) + } + }) + + it('an unusable value keeps auth ON and fails closed — /mcp is 401, never open', async () => { + const res = await get('/mcp', { AUTHKIT_DOMAIN: 'http://identity.example.test' }) + expect(res.status).toBe(401) + }) + + it('binds iss to the normalised origin by exact match, never by prefix', async () => { + const { generateKeyPair, exportJWK } = await import('jose') + const { publicKey, privateKey } = await generateKeyPair('RS256') + const publicJwk = await exportJWK(publicKey) + const fetched: string[] = [] + vi.spyOn(globalThis, 'fetch').mockImplementation(async (input: RequestInfo | URL) => { + const url = typeof input === 'string' ? input : input instanceof URL ? input.href : input.url + fetched.push(url) + return new Response(JSON.stringify({ keys: [{ ...publicJwk, kid: 'k1', alg: 'RS256', use: 'sig' }] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }) + }) + const sign = (iss: string) => + new SignJWT({}) + .setProtectedHeader({ alg: 'RS256', kid: 'k1' }) + .setIssuedAt() + .setIssuer(iss) + .setAudience('aud-test') + .setSubject('user_123') + .setExpirationTime('5m') + .sign(privateKey) + // A host of its own: the JWKS set is cached per issuer for the isolate. + const env = { AUTHKIT_DOMAIN: 'HTTPS://Iss-Bind.Example.Test/some/path' } + + expect(await verifyJwt(env, await sign('https://iss-bind.example.test'), 'aud-test')).toMatchObject({ + sub: 'user_123', + }) + expect(fetched).toEqual(['https://iss-bind.example.test/oauth2/jwks']) + for (const iss of [ + 'https://iss-bind.example.test.evil.test', + 'https://iss-bind.example.test/', + 'http://iss-bind.example.test', + ]) { + expect(await verifyJwt(env, await sign(iss), 'aud-test'), iss).toBeNull() + } + }) +}) diff --git a/package-lock.json b/package-lock.json index 155e6a4..4f22d7d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -783,22 +783,6 @@ } } }, - "node_modules/@google/clasp/node_modules/typescript": { - "version": "5.9.3", - "resolved": "https://registry.npmjs.org/typescript/-/typescript-5.9.3.tgz", - "integrity": "sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw==", - "dev": true, - "license": "Apache-2.0", - "optional": true, - "peer": true, - "bin": { - "tsc": "bin/tsc", - "tsserver": "bin/tsserver" - }, - "engines": { - "node": ">=14.17" - } - }, "node_modules/@google/clasp/node_modules/zod": { "version": "3.25.76", "resolved": "https://registry.npmjs.org/zod/-/zod-3.25.76.tgz", @@ -6854,9 +6838,9 @@ } }, "node_modules/proxy-addr": { - "version": "2.0.7", - "resolved": "https://registry.npmjs.org/proxy-addr/-/proxy-addr-2.0.7.tgz", - "integrity": "sha512-llQsMLSUDUPT44jdrU/O37qlnifitDP+ZwrmmZcoSKyLKvtZxpyV0n2/bD/N4tBAAZ/gJEdZU7KMraoK1+XYAg==", + "version": "2.0.8", + "resolved": "https://registry.npmjs.org/proxy-addr/-/proxy-addr-2.0.8.tgz", + "integrity": "sha512-5nnx0yGyVUcY6t9RnWcARWtwT9F1D8O9rt08htPvnd49W1IgZtmLkhu9WfMzQj1cFxjHIO6connUNVW5k7AVyQ==", "license": "MIT", "dependencies": { "forwarded": "0.2.0", @@ -6864,6 +6848,10 @@ }, "engines": { "node": ">= 0.10" + }, + "funding": { + "type": "opencollective", + "url": "https://opencollective.com/express" } }, "node_modules/qs": { diff --git a/wrangler.toml b/wrangler.toml index 61c6af8..ad4f2d2 100644 --- a/wrangler.toml +++ b/wrangler.toml @@ -62,11 +62,13 @@ custom_domain = true # longer the advertised resource — MCP_RESOURCE below is what discovery # metadata and audience checks use. MCP_RESOURCE = "https://tools.nyuchi.dev/mcp" -# WorkOS Connect hosted auth (Connect app client_01KVTX0V2K1VM3PSC0DJ9VZWTV). -# /mcp requires bearer JWTs issued by this authorization server; MCP clients -# discover it via /.well-known/oauth-protected-resource and self-register -# through its dynamic client registration. Unset to run the MCP open. -AUTHKIT_DOMAIN = "identity.nyuchi.com" +# AUTHKIT_DOMAIN — the WorkOS Connect / AuthKit domain — and WORKOS_CLIENT_ID — +# the Connect app's public client ID — are deliberately NOT declared here. It is +# REQUIRED and set per environment as a Worker secret (see the secrets note +# below); it is never committed and there is no default in code. /mcp requires +# bearer JWTs issued by this authorization server; MCP clients discover it via +# /.well-known/oauth-protected-resource and self-register through its dynamic +# client registration. # Cloudflare Images account for the MCP upload tools (nyuchi_generate_studio_card's # returnFormat 'url' / nyuchi_upload_asset). Same account as account_id above. The # matching CF_IMAGES_TOKEN secret (an API token with Cloudflare Images edit @@ -87,6 +89,21 @@ GOOGLE_WORKSPACE_DOMAIN = "nyuchi.com" # Secrets are deliberately NOT set here — they must never be committed to # git in plaintext. Provision each out-of-band with `wrangler secret put`: # +# AUTHKIT_DOMAIN — the WorkOS AuthKit domain (bare host or https +# origin). REQUIRED; set per environment by the owner script from +# 1Password (nyuchi/workos, field WORKOS_AUTHKIT_DOMAIN). Never a [vars] +# entry: a var of the same name wins on every deploy. Without it /login +# answers 503 "AUTHKIT_DOMAIN is not configured" and so does /mcp — +# it never serves tools unauthenticated. Set it before deploying. +# WORKOS_CLIENT_ID — the WorkOS Connect public client ID for the +# site login ("Nyuchi Internal Tools"). Public but per environment; +# REQUIRED. Set by the owner script from 1Password (nyuchi/workos, field +# WORKOS_INTERNAL_TOOLS_CLIENT_ID). Never a [vars] entry and no default +# in code: without it /login answers 503 "WORKOS_CLIENT_ID is not +# configured" and /callback denies. +# ALLOW_UNAUTHENTICATED_DEV — NEVER set here or as a secret. Local +# development only (`.dev.vars`): "true" lets /mcp run without a bearer +# token when AUTHKIT_DOMAIN is unset and the request is to localhost. # SESSION_SECRET — site-wide login gate (mcp/src/site-auth.ts). # Fails CLOSED without it: every session-cookie verification treats a # missing SESSION_SECRET as "no valid session" (redirect to /login), and