diff --git a/.changeset/check-token-type.md b/.changeset/check-token-type.md new file mode 100644 index 0000000..dd89590 --- /dev/null +++ b/.changeset/check-token-type.md @@ -0,0 +1,13 @@ +--- +"@seamless-auth/core": patch +"@seamless-auth/express": patch +"@seamless-auth/fastify": patch +"@seamless-auth/nextjs": patch +--- + +Check the token type before trusting a token. `ensureCookies` now refuses an access cookie whose +inner token is not an access token (`typ: "access"`), as `requireAuth` already did, so a signed +pre-auth cookie copied into the access slot is answered 401. `verifyUpstreamSession` refuses a +response whose token has the wrong `typ`: an access token for session flows, and an ephemeral +token for `/login` and the manifest's pre-auth and registration routes (new optional +`expectedType` argument). Core also exports `holdsAccessToken`. diff --git a/.changeset/console-security-headers.md b/.changeset/console-security-headers.md new file mode 100644 index 0000000..dec360a --- /dev/null +++ b/.changeset/console-security-headers.md @@ -0,0 +1,11 @@ +--- +"@seamless-auth/express": patch +"@seamless-auth/fastify": patch +"@seamless-auth/nextjs": patch +--- + +Keep the API's framing and content-type headers on the console proxy. The proxy now forwards +`Content-Security-Policy`, `X-Frame-Options` and `X-Content-Type-Options` from the API, and when +the API sends neither of the last two it sets `X-Frame-Options: DENY` and +`X-Content-Type-Options: nosniff` (unless the application already set them), so the admin console +served on the app origin cannot be framed. diff --git a/.changeset/no-upstream-redirects.md b/.changeset/no-upstream-redirects.md new file mode 100644 index 0000000..95d5c76 --- /dev/null +++ b/.changeset/no-upstream-redirects.md @@ -0,0 +1,12 @@ +--- +"@seamless-auth/core": patch +"@seamless-auth/express": patch +"@seamless-auth/fastify": patch +"@seamless-auth/nextjs": patch +--- + +Do not follow redirects on upstream requests. `authFetch` now sends every auth API request with +`redirect: "manual"` and answers a 3xx as a 502 with `{ "error": "upstream_redirect" }`, so a +misconfigured proxy in front of the API can no longer get the service token and forwarded client +headers resent to another origin. The adapter manifest fetch no longer follows redirects either, +and falls back to the bundled copy instead. diff --git a/.changeset/origin-check-same-site.md b/.changeset/origin-check-same-site.md new file mode 100644 index 0000000..10ba77f --- /dev/null +++ b/.changeset/origin-check-same-site.md @@ -0,0 +1,16 @@ +--- +"@seamless-auth/core": minor +"@seamless-auth/express": minor +"@seamless-auth/fastify": minor +"@seamless-auth/nextjs": minor +--- + +Check `Origin` against `allowedOrigins` even when `Sec-Fetch-Site` is present. A `same-site` +request also covers sibling subdomains, so with `SameSite=None` cookies the header alone let any +subdomain drive a state change. When `allowedOrigins` is configured, a state-changing `same-site` +request must now carry an `Origin` on the list (a missing `Origin` is refused). `same-origin` +requests always pass and `cross-site` requests are always refused. + +This tightens behavior for adopters who set `allowedOrigins`: list every origin that calls the +adapter from a sibling subdomain. With `allowedOrigins` unset the default is unchanged, and a +`same-site` request passes as before. diff --git a/.changeset/refuse-dot-segment-params.md b/.changeset/refuse-dot-segment-params.md new file mode 100644 index 0000000..39bd97c --- /dev/null +++ b/.changeset/refuse-dot-segment-params.md @@ -0,0 +1,11 @@ +--- +"@seamless-auth/core": patch +"@seamless-auth/express": patch +"@seamless-auth/fastify": patch +"@seamless-auth/nextjs": patch +--- + +Refuse empty, `.` and `..` route params on the hand-written passthroughs and core handlers with +a 400 `bad_request`, as the manifest proxy already did. A decoded `%2e%2e` param previously stayed +literal after encoding and moved the upstream path. Core exports `encodePathParam` and +`InvalidPathParamError` for the shared rule. diff --git a/.changeset/refuse-placeholder-secrets.md b/.changeset/refuse-placeholder-secrets.md new file mode 100644 index 0000000..4076db5 --- /dev/null +++ b/.changeset/refuse-placeholder-secrets.md @@ -0,0 +1,19 @@ +--- +"@seamless-auth/core": minor +"@seamless-auth/express": minor +"@seamless-auth/fastify": minor +"@seamless-auth/nextjs": minor +--- + +Refuse known placeholder secrets at startup. `assertSecretStrength` now refuses +`dev-only-cookie-signing-key-replace-me`, `GRAB_FROM_SEAMLESS_AUTH_PORTAL_DEV`, and any value +containing `replace-me`, which were long enough to pass the length check. Every adapter refuses one +for `cookieSecret` or `serviceSecret` when it starts, unless it runs in its insecure local mode +(`cookieSecure: false`). `assertSecretStrength` takes a new `{ allowPlaceholder }` option and +`assertSecrets` reads `cookieSecure`; core also exports `isPlaceholderSecret` and +`KNOWN_PLACEHOLDER_SECRETS`. + +`requireAuth` (and `authenticateRequest`) configured with `authServerUrl` and `audience` now also +verifies the access token inside the cookie against the auth API's JWKS, and takes the session's +roles from it, so a cookie forged with the cookie secret alone cannot carry claims of its own. +Without that configuration the guard decodes the inner token as before. diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 0000000..1d1a824 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,11 @@ +version: 2 +updates: + # Workflow actions are pinned to commit SHAs, so updates have to come from here. + - package-ecosystem: github-actions + directory: / + schedule: + interval: weekly + groups: + github-actions: + patterns: + - "*" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7efce16..5e333bd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,6 +6,9 @@ on: - dev - main +permissions: + contents: read + jobs: test: name: Run Tests (Node ${{ matrix.node-version }}) @@ -19,13 +22,13 @@ jobs: node-version: [22, 24, latest] steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 - - uses: pnpm/action-setup@v4 + - uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v4.4.0 with: version: 10.33.0 - - uses: actions/setup-node@v4 + - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 with: node-version: ${{ matrix.node-version }} cache: pnpm diff --git a/.github/workflows/conformance.yml b/.github/workflows/conformance.yml index 0e4a9cb..cb8b8f9 100644 --- a/.github/workflows/conformance.yml +++ b/.github/workflows/conformance.yml @@ -5,6 +5,9 @@ name: conformance on: pull_request: +permissions: + contents: read + jobs: verify: uses: fells-code/seamless-cli/.github/workflows/verify-conformance.yml@main diff --git a/.github/workflows/prerelease.yml b/.github/workflows/prerelease.yml index 82ec663..fec584d 100644 --- a/.github/workflows/prerelease.yml +++ b/.github/workflows/prerelease.yml @@ -17,6 +17,9 @@ on: - beta - rc +permissions: + contents: read + jobs: prerelease: name: Publish Prerelease Snapshot @@ -27,15 +30,15 @@ jobs: id-token: write steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 with: fetch-depth: 0 - - uses: pnpm/action-setup@v4 + - uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v4.4.0 with: version: 10.33.0 - - uses: actions/setup-node@v4 + - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 with: node-version-file: '.nvmrc' cache: pnpm diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 89796fe..43f3792 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -6,6 +6,9 @@ on: - main workflow_dispatch: +permissions: + contents: read + jobs: release: name: Version or Publish Stable Packages @@ -17,15 +20,15 @@ jobs: id-token: write steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 with: fetch-depth: 0 - - uses: pnpm/action-setup@v4 + - uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v4.4.0 with: version: 10.33.0 - - uses: actions/setup-node@v4 + - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 with: node-version-file: '.nvmrc' cache: pnpm @@ -38,7 +41,7 @@ jobs: run: pnpm test - name: Version or publish packages - uses: changesets/action@v1 + uses: changesets/action@a45c4d594aa4e2c509dc14a9f2b3b67ba3780d0d # v1.9.0 with: version: pnpm version-packages publish: pnpm release:stable diff --git a/packages/core/src/authFetch.ts b/packages/core/src/authFetch.ts index a4f24a9..91bfa48 100644 --- a/packages/core/src/authFetch.ts +++ b/packages/core/src/authFetch.ts @@ -2,6 +2,8 @@ * Copyright © 2026 Fells Code, LLC * Licensed under the Apache License, Version 2.0 */ +import { getSeamlessLogger } from "./logger.js"; + export interface AuthFetchOptions { method: "GET" | "POST" | "PUT" | "PATCH" | "DELETE"; headers?: Record; @@ -31,15 +33,49 @@ export async function authFetch( : {}), }; + // Following a redirect would resend the service token and the forwarded + // client headers to wherever a misconfigured proxy in front of the API points. const response = await fetch(url, { method: options.method, headers, body: options.body ? JSON.stringify(options.body) : undefined, + redirect: "manual", }); + if (isRedirect(response)) { + getSeamlessLogger().error( + `[SeamlessAuth] The auth API at ${originOf(url)} answered a ${options.method} with a redirect (HTTP ${response.status}), which is not followed.`, + ); + return makeJsonTolerant( + new Response(JSON.stringify({ error: UPSTREAM_REDIRECT_ERROR_CODE }), { + status: 502, + headers: { "Content-Type": "application/json" }, + }), + ); + } + return makeJsonTolerant(response); } +/** The error code a request gets when the auth API answered it with a redirect. */ +export const UPSTREAM_REDIRECT_ERROR_CODE = "upstream_redirect"; + +function isRedirect(response: Response): boolean { + return ( + response.type === "opaqueredirect" || + (response.status >= 300 && response.status < 400 && response.status !== 304) + ); +} + +// The path can carry a credential, such as a magic link token. +function originOf(url: string): string { + try { + return new URL(url).origin; + } catch { + return ""; + } +} + // Upstream responses aren't always JSON, and native Response.json() throws on the ones // that aren't, which would crash the 24 call sites that parse a body before checking // the status. Make json() tolerant instead: parsed JSON, { message: text } for a diff --git a/packages/core/src/createServiceToken.ts b/packages/core/src/createServiceToken.ts index 29dcaa8..08d4484 100644 --- a/packages/core/src/createServiceToken.ts +++ b/packages/core/src/createServiceToken.ts @@ -13,7 +13,9 @@ export interface ServiceTokenOptions { } export function createServiceToken(opts: ServiceTokenOptions): string { - assertSecretStrength("serviceSecret", opts.serviceSecret); + assertSecretStrength("serviceSecret", opts.serviceSecret, { + allowPlaceholder: true, + }); return jwt.sign( { diff --git a/packages/core/src/ensureCookies.ts b/packages/core/src/ensureCookies.ts index 4181be9..f676c2d 100644 --- a/packages/core/src/ensureCookies.ts +++ b/packages/core/src/ensureCookies.ts @@ -1,3 +1,4 @@ +import { holdsAccessToken } from "./guards.js"; import { verifyCookieJwt } from "./verifyCookieJwt.js"; import type { ResultFailure } from "./result.js"; import { refreshAccessToken } from "./refreshAccessToken.js"; @@ -349,7 +350,9 @@ export async function ensureCookies( input: EnsureCookiesInput, opts: EnsureCookiesOptions, ): Promise { - assertSecrets(opts); + // The adapter refused a placeholder at startup, where it knows whether it + // runs in local mode; these options do not say. + assertSecrets(opts, { allowPlaceholder: true }); const requirement = cookieRequirement(input); @@ -414,7 +417,13 @@ export async function ensureCookies( }; } - const token = typeof payload.token === "string" ? payload.token : undefined; + // Every adapter cookie is signed with the same secret, so the signature + // does not tell a pre-auth cookie from a session. The token type does. + const token = + typeof payload.token === "string" && + (cookieName !== opts.accessCookieName || holdsAccessToken(payload.token)) + ? payload.token + : undefined; if (required && !token && cookieName === opts.accessCookieName) { const refreshed = await refreshRequiredCookie( diff --git a/packages/core/src/getSeamlessUser.ts b/packages/core/src/getSeamlessUser.ts index c133217..f419362 100644 --- a/packages/core/src/getSeamlessUser.ts +++ b/packages/core/src/getSeamlessUser.ts @@ -60,7 +60,9 @@ export async function getSeamlessUser( cookies: Record, opts: GetSeamlessUserOptions, ): Promise { - assertSecretStrength("cookieSecret", opts.cookieSecret); + assertSecretStrength("cookieSecret", opts.cookieSecret, { + allowPlaceholder: true, + }); const authorization = await resolveUpstreamAuthorization(cookies, opts); if (authorization === null) return null; diff --git a/packages/core/src/guards.ts b/packages/core/src/guards.ts index 4b34766..71fd59f 100644 --- a/packages/core/src/guards.ts +++ b/packages/core/src/guards.ts @@ -56,9 +56,12 @@ function normalizeOrigin(value: string): string { * `Strict` cookie is not sent on one, so the check is inert there. * * `Sec-Fetch-Site` is the primary signal: current browsers send it and page - * JavaScript cannot forge it. When it is absent the `Origin` is matched against - * `allowedOrigins`, but only when the adopter opted in, so nothing regresses for - * callers that predate the guard. + * JavaScript cannot forge it. `cross-site` is refused and `same-origin` passes. + * `same-site` also covers a sibling subdomain, so when the adopter configured + * `allowedOrigins` the `Origin` must be on it as well; without the allowlist a + * same-site request passes, as it always has. When `Sec-Fetch-Site` is absent + * the `Origin` is matched against `allowedOrigins`, again only when the adopter + * opted in, so nothing regresses for callers that predate the guard. */ export function checkOrigin( input: OriginCheckInput, @@ -75,9 +78,20 @@ export function checkOrigin( }; if (input.secFetchSite !== undefined) { - return input.secFetchSite.toLowerCase() === "cross-site" - ? rejection - : undefined; + const site = input.secFetchSite.toLowerCase(); + + if (site === "cross-site") { + return rejection; + } + + if (site !== "same-site" || !input.allowedOrigins) { + return undefined; + } + + return input.origin !== undefined && + isAllowedOrigin(input.origin, input.allowedOrigins) + ? undefined + : rejection; } // No `Origin` and no `Sec-Fetch-Site` is a same-origin or non-browser @@ -98,11 +112,15 @@ export function checkOrigin( return undefined; } - const allowed = new Set( - input.allowedOrigins.map(normalizeOrigin).filter(Boolean), - ); + return isAllowedOrigin(input.origin, input.allowedOrigins) + ? undefined + : rejection; +} + +function isAllowedOrigin(origin: string, allowedOrigins: string[]): boolean { + const allowed = new Set(allowedOrigins.map(normalizeOrigin).filter(Boolean)); - return allowed.has(normalizeOrigin(input.origin)) ? undefined : rejection; + return allowed.has(normalizeOrigin(origin)); } export interface CookieAuthInput { @@ -147,7 +165,7 @@ export type CookieAuthResult = * against the auth API's key set before the adapter signed it into the cookie, * so its claims are read here without verifying it again. */ -function holdsAccessToken(token: unknown): boolean { +export function holdsAccessToken(token: unknown): boolean { if (typeof token !== "string") return false; try { @@ -165,7 +183,9 @@ function holdsAccessToken(token: unknown): boolean { * there. */ export function authenticateCookie(input: CookieAuthInput): CookieAuthResult { - assertSecretStrength("requireAuth: cookieSecret", input.cookieSecret); + assertSecretStrength("requireAuth: cookieSecret", input.cookieSecret, { + allowPlaceholder: true, + }); if (!input.token) { return { @@ -296,15 +316,24 @@ export interface RequestAuthInput extends CookieAuthInput { * stale cookie should see the session rejected rather than fall through to a * header it never meant to send. Bearer is only consulted when there is no * cookie at all and the adopter has opted in. + * + * With `bearer` configured, a cookie's inner access token is also verified + * against the auth API's key set, and its roles come from that token. */ export async function authenticateRequest( input: RequestAuthInput, ): Promise { if (input.token || !input.bearer) { - return authenticateCookie(input); + const result = authenticateCookie(input); + + return result.user && input.bearer + ? verifyCookieAccessToken(result.user, input.bearer) + : result; } - assertSecretStrength("requireAuth: cookieSecret", input.cookieSecret); + assertSecretStrength("requireAuth: cookieSecret", input.cookieSecret, { + allowPlaceholder: true, + }); if (!extractBearerToken(input.authorization)) { return { @@ -321,6 +350,33 @@ export async function authenticateRequest( }); } +// The cookie secret alone can sign a cookie. Holding the inner token to the +// auth API's key set means a forged cookie cannot carry claims of its own. +async function verifyCookieAccessToken( + user: SeamlessAuthUser, + verify: NonNullable, +): Promise { + const claims = await verifyAccessToken( + user.token as string, + verify.authServerUrl, + verify.audience, + verify.authServerIssuer, + ); + + if (!claims || claims.sub !== user.id) { + return { + rejection: { status: 401, errorCode: "Invalid or expired session" }, + }; + } + + return { + user: { + ...user, + roles: Array.isArray(claims.roles) ? claims.roles : user.roles, + }, + }; +} + /** * Authorization only, against a session a guard has already authenticated. * diff --git a/packages/core/src/handlers/admin.ts b/packages/core/src/handlers/admin.ts index 870d163..436b271 100644 --- a/packages/core/src/handlers/admin.ts +++ b/packages/core/src/handlers/admin.ts @@ -1,3 +1,4 @@ +import { encodePathParam } from "../pathParam.js"; import { authFetch } from "../authFetch.js"; import { buildUpstreamUrl, type QueryInput } from "../proxyRequest.js"; import type { ResultFailure } from "../result.js"; @@ -66,13 +67,13 @@ export const deleteUserHandler = (opts: WithBody) => request("DELETE", "/admin/users", opts); export const updateUserHandler = (userId: string, opts: WithBody) => - request("PATCH", `/admin/users/${encodeURIComponent(userId)}`, opts); + request("PATCH", `/admin/users/${encodePathParam(userId)}`, opts); export const getUserDetailHandler = (userId: string, opts: BaseOpts) => - request("GET", `/admin/users/${encodeURIComponent(userId)}`, opts); + request("GET", `/admin/users/${encodePathParam(userId)}`, opts); export const getUserAnomaliesHandler = (userId: string, opts: BaseOpts) => - request("GET", `/admin/users/${encodeURIComponent(userId)}/anomalies`, opts); + request("GET", `/admin/users/${encodePathParam(userId)}/anomalies`, opts); export const getAuthEventsHandler = (opts: WithQuery) => request("GET", "/admin/auth-events", opts); @@ -84,15 +85,15 @@ export const listAllSessionsHandler = (opts: WithQuery) => request("GET", "/admin/sessions", opts); export const listUserSessionsHandler = (userId: string, opts: BaseOpts) => - request("GET", `/admin/sessions/${encodeURIComponent(userId)}`, opts); + request("GET", `/admin/sessions/${encodePathParam(userId)}`, opts); export const revokeUserSessionHandler = (id: string, opts: BaseOpts) => - request("DELETE", `/admin/sessions/by-id/${encodeURIComponent(id)}`, opts); + request("DELETE", `/admin/sessions/by-id/${encodePathParam(id)}`, opts); export const revokeAllUserSessionsHandler = (userId: string, opts: BaseOpts) => request( "DELETE", - `/admin/sessions/${encodeURIComponent(userId)}/revoke-all`, + `/admin/sessions/${encodePathParam(userId)}/revoke-all`, opts, ); @@ -102,6 +103,6 @@ export const recoverUserForDeviceReplacementHandler = ( ) => request( "POST", - `/admin/users/${encodeURIComponent(userId)}/recovery/device-replacement`, + `/admin/users/${encodePathParam(userId)}/recovery/device-replacement`, opts, ); diff --git a/packages/core/src/handlers/login.ts b/packages/core/src/handlers/login.ts index 6eed3bd..543dcec 100644 --- a/packages/core/src/handlers/login.ts +++ b/packages/core/src/handlers/login.ts @@ -73,6 +73,7 @@ export async function loginHandler( opts.authServerUrl, opts.audience, opts.authServerIssuer, + "ephemeral", ); // The ephemeral token is what carries the flow forward, and a bearer client diff --git a/packages/core/src/handlers/oauthHandlers.ts b/packages/core/src/handlers/oauthHandlers.ts index df8e1e8..1ccf0d5 100644 --- a/packages/core/src/handlers/oauthHandlers.ts +++ b/packages/core/src/handlers/oauthHandlers.ts @@ -1,3 +1,4 @@ +import { encodePathParam } from "../pathParam.js"; import type { AuthServerIssuerOption } from "../authServerIssuer.js"; import { authFetch } from "../authFetch.js"; import { sessionResult } from "../upstreamSession.js"; @@ -54,7 +55,7 @@ export async function startOAuthLoginHandler( opts: Pick, ) { const up = await authFetch( - `${opts.authServerUrl}/oauth/${encodeURIComponent(input.providerId!)}/start`, + `${opts.authServerUrl}/oauth/${encodePathParam(input.providerId!)}/start`, { method: "POST", body: input.body, @@ -77,7 +78,7 @@ export async function finishOAuthLoginHandler( opts: OAuthHandlerOptions, ): Promise { const up = await authFetch( - `${opts.authServerUrl}/oauth/${encodeURIComponent(input.providerId!)}/callback`, + `${opts.authServerUrl}/oauth/${encodePathParam(input.providerId!)}/callback`, { method: "POST", body: input.body, diff --git a/packages/core/src/handlers/sessions.ts b/packages/core/src/handlers/sessions.ts index d1f1f92..f597f12 100644 --- a/packages/core/src/handlers/sessions.ts +++ b/packages/core/src/handlers/sessions.ts @@ -1,3 +1,4 @@ +import { encodePathParam } from "../pathParam.js"; import { authFetch } from "../authFetch.js"; import type { ResultFailure } from "../result.js"; import { readUpstreamFailure } from "../upstreamError.js"; @@ -47,7 +48,7 @@ export const listSessionsHandler = (opts: BaseOpts) => request("GET", "/sessions", opts); export const revokeSessionHandler = (id: string, opts: BaseOpts) => - request("DELETE", `/sessions/${encodeURIComponent(id)}`, opts); + request("DELETE", `/sessions/${encodePathParam(id)}`, opts); export const revokeAllSessionsHandler = (opts: BaseOpts) => request("DELETE", "/sessions", opts); diff --git a/packages/core/src/handlers/switchOrganizationHandler.ts b/packages/core/src/handlers/switchOrganizationHandler.ts index bf3efda..5c3d255 100644 --- a/packages/core/src/handlers/switchOrganizationHandler.ts +++ b/packages/core/src/handlers/switchOrganizationHandler.ts @@ -1,3 +1,4 @@ +import { encodePathParam } from "../pathParam.js"; import type { AuthServerIssuerOption } from "../authServerIssuer.js"; import { authFetch } from "../authFetch.js"; import { sessionResult } from "../upstreamSession.js"; @@ -38,7 +39,7 @@ export async function switchOrganizationHandler( opts: SwitchOrganizationOptions, ): Promise { const up = await authFetch( - `${opts.authServerUrl}/organizations/${encodeURIComponent(input.organizationId)}/switch`, + `${opts.authServerUrl}/organizations/${encodePathParam(input.organizationId)}/switch`, { method: "POST", authorization: input.authorization, diff --git a/packages/core/src/handlers/verifyMagicLinkHandler.ts b/packages/core/src/handlers/verifyMagicLinkHandler.ts index e6de4a6..6c04551 100644 --- a/packages/core/src/handlers/verifyMagicLinkHandler.ts +++ b/packages/core/src/handlers/verifyMagicLinkHandler.ts @@ -1,3 +1,4 @@ +import { encodePathParam } from "../pathParam.js"; import { authFetch } from "../authFetch.js"; import { readPassthroughFailure } from "../upstreamError.js"; import type { ResultFailure } from "../result.js"; @@ -68,7 +69,7 @@ export async function verifyMagicLinkHandler( opts: VerifyMagicLinkOptions, ): Promise { const up = await authFetch( - `${opts.authServerUrl}/magic-link/verify/${encodeURIComponent(input.token)}`, + `${opts.authServerUrl}/magic-link/verify/${encodePathParam(input.token)}`, { method: "GET", authorization: input.authorization, diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index a2206d4..042910e 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -26,6 +26,7 @@ export { export * from "./apiContract.js"; export * from "./transport.js"; export * from "./applyResult.js"; +export * from "./pathParam.js"; export * from "./proxyRequest.js"; export * from "./result.js"; export * from "./upstreamSession.js"; diff --git a/packages/core/src/manifest/adapterManifest.ts b/packages/core/src/manifest/adapterManifest.ts index 99339d3..d9b1a95 100644 --- a/packages/core/src/manifest/adapterManifest.ts +++ b/packages/core/src/manifest/adapterManifest.ts @@ -1,4 +1,5 @@ import { getSeamlessLogger } from "../logger.js"; +import { isDotSegment } from "../pathParam.js"; import { BUNDLED_ADAPTER_MANIFEST } from "./bundledManifest.js"; /** @@ -152,7 +153,7 @@ export function matchManifestRoute( // Re-encoded, a dot segment survives as a literal `..` that fetch then // resolves, sending the held token to a different upstream path. - if (value === "." || value === "..") { + if (isDotSegment(value)) { matched = false; break; } @@ -230,6 +231,7 @@ export function createAdapterManifestSource( try { const response = await fetch(`${opts.authServerUrl}${ADAPTER_MANIFEST_PATH}`, { headers: { accept: "application/json" }, + redirect: "manual", signal: AbortSignal.timeout(opts.timeoutMs ?? DEFAULT_TIMEOUT_MS), }); diff --git a/packages/core/src/manifestProxy.ts b/packages/core/src/manifestProxy.ts index c376d57..48cb4c4 100644 --- a/packages/core/src/manifestProxy.ts +++ b/packages/core/src/manifestProxy.ts @@ -110,6 +110,7 @@ async function cookiesFor( opts.authServerUrl, opts.audience, opts.authServerIssuer, + "ephemeral", ); return [ @@ -312,6 +313,9 @@ export async function handleManifestRoute( opts.authServerUrl, opts.audience, opts.authServerIssuer, + route.issues === "preAuth" || route.issues === "registration" + ? "ephemeral" + : "access", ); } diff --git a/packages/core/src/pathParam.ts b/packages/core/src/pathParam.ts new file mode 100644 index 0000000..d8e755b --- /dev/null +++ b/packages/core/src/pathParam.ts @@ -0,0 +1,36 @@ +/* + * Copyright © 2026 Fells Code, LLC + * Licensed under the Apache License, Version 2.0 + */ + +/** Thrown for a path parameter that cannot stay inside one upstream segment. */ +export class InvalidPathParamError extends Error { + readonly status = 400; + + constructor(readonly param?: string) { + super( + param ? `Invalid route parameter "${param}"` : "Invalid route parameter", + ); + this.name = "InvalidPathParamError"; + } +} + +/** + * `.` and `..` survive encodeURIComponent unchanged, and fetch then resolves + * them, moving the upstream request (and the token it carries) to another path. + */ +export function isDotSegment(value: string): boolean { + return value === "." || value === ".."; +} + +/** + * Encodes a decoded route parameter as a single upstream path segment, refusing + * empty and dot-segment values. + */ +export function encodePathParam(value: string, param?: string): string { + if (typeof value !== "string" || value === "" || isDotSegment(value)) { + throw new InvalidPathParamError(param); + } + + return encodeURIComponent(value); +} diff --git a/packages/core/src/refreshAccessToken.ts b/packages/core/src/refreshAccessToken.ts index e38bfb7..90745db 100644 --- a/packages/core/src/refreshAccessToken.ts +++ b/packages/core/src/refreshAccessToken.ts @@ -106,7 +106,7 @@ export async function refreshAccessToken( refreshCookie: string, opts: RefreshAccessTokenOptions, ): Promise { - assertSecrets(opts); + assertSecrets(opts, { allowPlaceholder: true }); return dedupeRefresh(`cookie:${refreshCookie}`, async () => { const payload = verifyRefreshCookie(refreshCookie, opts.cookieSecret); diff --git a/packages/core/src/upstreamSession.ts b/packages/core/src/upstreamSession.ts index b34e9c9..81f9998 100644 --- a/packages/core/src/upstreamSession.ts +++ b/packages/core/src/upstreamSession.ts @@ -47,6 +47,9 @@ export function withoutSessionMaterial(data: return rest; } +/** The `typ` claim the auth API signs into the tokens it returns. */ +export type UpstreamTokenType = "access" | "ephemeral"; + export interface VerifiedUpstreamSession { /** The `sid` claim, when the access token carries one. */ sessionId?: string; @@ -65,6 +68,8 @@ export async function verifyUpstreamSession( authServerUrl: string, audience: string, authServerIssuer?: string, + /** `ephemeral` for the pre-auth and registration tokens a sign-in flow carries. */ + expectedType: UpstreamTokenType = "access", ): Promise { const verified = await verifySignedAuthResponse( data.token, @@ -77,6 +82,14 @@ export async function verifyUpstreamSession( throw new Error("Invalid signed response from Auth Server"); } + // Access and ephemeral tokens share the key, issuer and audience, so only + // `typ` stops an ephemeral token from becoming a session cookie. + if (verified.typ !== expectedType) { + throw new Error( + `Auth Server response did not carry an ${expectedType} token`, + ); + } + if (verified.sub !== data.sub) { throw new Error("Signature mismatch with data payload"); } diff --git a/packages/core/src/validateSecrets.ts b/packages/core/src/validateSecrets.ts index 90604e0..8fc7a20 100644 --- a/packages/core/src/validateSecrets.ts +++ b/packages/core/src/validateSecrets.ts @@ -1,6 +1,34 @@ export const MIN_SECRET_LENGTH = 32; -export function assertSecretStrength(name: string, value: unknown): void { +/** + * Example values that have shipped in starters and docs. Each is long enough to + * pass the length check, so a deployment that kept one would otherwise boot. + */ +export const KNOWN_PLACEHOLDER_SECRETS: readonly string[] = [ + "dev-only-cookie-signing-key-replace-me", + "GRAB_FROM_SEAMLESS_AUTH_PORTAL_DEV", +]; + +export interface SecretStrengthOptions { + /** + * Accept a known placeholder value. Only the adapter's insecure local mode + * (`cookieSecure: false`) sets this. + */ + allowPlaceholder?: boolean; +} + +export function isPlaceholderSecret(value: string): boolean { + return ( + KNOWN_PLACEHOLDER_SECRETS.includes(value) || + value.toLowerCase().includes("replace-me") + ); +} + +export function assertSecretStrength( + name: string, + value: unknown, + options: SecretStrengthOptions = {}, +): void { if (typeof value !== "string" || value.length === 0) { throw new Error(`Missing ${name}`); } @@ -10,12 +38,30 @@ export function assertSecretStrength(name: string, value: unknown): void { `${name} must be at least ${MIN_SECRET_LENGTH} characters. A short secret can be brute forced offline, letting an attacker forge cookie sessions and service tokens. Generate one with: openssl rand -base64 48`, ); } + + if (!options.allowPlaceholder && isPlaceholderSecret(value)) { + throw new Error( + `${name} is a published placeholder value. Anyone who has read the example can forge cookie sessions and service tokens with it. Generate one with: openssl rand -base64 48 (a placeholder is accepted only with cookieSecure: false, for local HTTP development)`, + ); + } } -export function assertSecrets(opts: { - cookieSecret: unknown; - serviceSecret: unknown; -}): void { - assertSecretStrength("cookieSecret", opts.cookieSecret); - assertSecretStrength("serviceSecret", opts.serviceSecret); +/** + * Checks both adapter secrets. A placeholder is refused unless the adapter runs + * in its insecure local mode, `cookieSecure: false`, or `options` allows it. + */ +export function assertSecrets( + opts: { + cookieSecret: unknown; + serviceSecret: unknown; + cookieSecure?: boolean; + }, + options: SecretStrengthOptions = {}, +): void { + const strength = { + allowPlaceholder: options.allowPlaceholder || opts.cookieSecure === false, + }; + + assertSecretStrength("cookieSecret", opts.cookieSecret, strength); + assertSecretStrength("serviceSecret", opts.serviceSecret, strength); } diff --git a/packages/core/tests/adapterManifest.test.js b/packages/core/tests/adapterManifest.test.js index 0cc6332..64ba08f 100644 --- a/packages/core/tests/adapterManifest.test.js +++ b/packages/core/tests/adapterManifest.test.js @@ -172,6 +172,20 @@ describe("createAdapterManifestSource", () => { expect(global.fetch).toHaveBeenCalledTimes(1); }); + it("does not follow a redirect to another manifest", async () => { + global.fetch = jest.fn( + async () => + new Response(null, { + status: 302, + headers: { location: "https://elsewhere.test/manifest.json" }, + }), + ); + const source = createAdapterManifestSource({ authServerUrl: "https://auth.test" }); + + await expect(source.get()).resolves.toBe(BUNDLED_ADAPTER_MANIFEST); + expect(global.fetch.mock.calls[0][1].redirect).toBe("manual"); + }); + it("retries after the wait", async () => { global.fetch = jest .fn() diff --git a/packages/core/tests/authFetch.test.js b/packages/core/tests/authFetch.test.js index e598afa..2c20569 100644 --- a/packages/core/tests/authFetch.test.js +++ b/packages/core/tests/authFetch.test.js @@ -1,4 +1,5 @@ import { jest } from "@jest/globals"; +import http from "node:http"; describe("authFetch", () => { const originalFetch = global.fetch; @@ -115,4 +116,56 @@ describe("authFetch", () => { await expect(res.json()).resolves.toBeUndefined(); }); + + it("asks fetch not to follow redirects", async () => { + const { authFetch } = await import("../dist/authFetch.js"); + + await authFetch("https://auth.example.com/users/me", { method: "GET" }); + + expect(global.fetch.mock.calls[0][1].redirect).toBe("manual"); + }); +}); + +describe("authFetch against a redirecting upstream", () => { + const servers = []; + + async function listen(handler) { + const server = http.createServer(handler); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + servers.push(server); + return `http://127.0.0.1:${server.address().port}`; + } + + afterEach(async () => { + await Promise.all( + servers.splice(0).map((s) => new Promise((resolve) => s.close(resolve))), + ); + }); + + it.each([301, 302, 307, 308])( + "answers %i as a 502 without resending the credentials", + async (status) => { + const { authFetch, UPSTREAM_REDIRECT_ERROR_CODE } = await import( + "../dist/authFetch.js" + ); + const elsewhere = jest.fn((_req, res) => res.end("{}")); + const target = await listen(elsewhere); + const api = await listen((_req, res) => { + res.writeHead(status, { location: `${target}/stolen` }); + res.end(); + }); + + const response = await authFetch(`${api}/admin/users`, { + method: "POST", + body: { userId: "u1" }, + authorization: "Bearer user-access-token", + serviceAuthorization: "service-token", + }); + + expect(response.status).toBe(502); + expect(response.ok).toBe(false); + expect(await response.json()).toEqual({ error: UPSTREAM_REDIRECT_ERROR_CODE }); + expect(elsewhere).not.toHaveBeenCalled(); + }, + ); }); diff --git a/packages/core/tests/ensureCookies.test.js b/packages/core/tests/ensureCookies.test.js index 79968a0..f3e9011 100644 --- a/packages/core/tests/ensureCookies.test.js +++ b/packages/core/tests/ensureCookies.test.js @@ -16,6 +16,9 @@ jest.unstable_mockModule("../dist/verifySignedAuthResponse.js", () => ({ verifySignedAuthResponse: verifySignedAuthResponseMock, })); +// Unsigned: the adapter only reads the inner token's type, the auth API verifies it. +const ACCESS_TOKEN = `e30.${Buffer.from(JSON.stringify({ typ: "access" })).toString("base64url")}.sig`; + const BASE_OPTS = { authServerUrl: "https://auth.example.com", cookieDomain: "example.com", @@ -53,7 +56,7 @@ describe("ensureCookies", () => { verifyCookieJwtMock.mockReturnValue({ sub: "user-123", - token: "access-token", + token: ACCESS_TOKEN, roles: ["user"], }); @@ -68,7 +71,7 @@ describe("ensureCookies", () => { expect(result.type).toBe("ok"); expect(result.user).toEqual({ sub: "user-123", - token: "access-token", + token: ACCESS_TOKEN, roles: ["user"], }); }); @@ -122,6 +125,7 @@ describe("ensureCookies", () => { const { ensureCookies } = await import("../dist/ensureCookies.js"); verifySignedAuthResponseMock.mockResolvedValue({ + typ: "access", sub: "user-123", sid: "session-123", }); @@ -193,6 +197,46 @@ describe("ensureCookies", () => { }, ); + // Every adapter cookie is signed with the same secret, so a pre-auth cookie + // copied into the access slot verifies. Its token type gives it away. + it.each([ + ["an ephemeral token", `e30.${Buffer.from(JSON.stringify({ typ: "ephemeral" })).toString("base64url")}.sig`], + ["a token with no type", `e30.${Buffer.from("{}").toString("base64url")}.sig`], + ["an opaque string", "access-token"], + ])("refuses an access cookie holding %s", async (_label, token) => { + const { ensureCookies } = await import("../dist/ensureCookies.js"); + + verifyCookieJwtMock.mockReturnValue({ sub: "user-123", token }); + + const result = await ensureCookies( + { path: "/users/me", cookies: { access: "signed.cookie.jwt" } }, + BASE_OPTS, + ); + + expect(result).toEqual({ + type: "error", + status: 401, + errorCode: "Invalid or expired access cookie", + clearCookies: ["access"], + }); + expect(refreshAccessTokenMock).not.toHaveBeenCalled(); + }); + + it("still accepts an ephemeral token in the pre-auth cookie", async () => { + const { ensureCookies } = await import("../dist/ensureCookies.js"); + const token = `e30.${Buffer.from(JSON.stringify({ typ: "ephemeral" })).toString("base64url")}.sig`; + + verifyCookieJwtMock.mockReturnValue({ sub: "user-123", token }); + + const result = await ensureCookies( + { path: "/webAuthn/login/start", cookies: { preauth: "signed.cookie.jwt" } }, + BASE_OPTS, + ); + + expect(result.type).toBe("ok"); + expect(result.user.token).toBe(token); + }); + it("refreshes old access cookies that do not contain a stored auth token", async () => { const { ensureCookies } = await import("../dist/ensureCookies.js"); @@ -202,6 +246,7 @@ describe("ensureCookies", () => { roles: ["user"], }); verifySignedAuthResponseMock.mockResolvedValue({ + typ: "access", sub: "user-123", sid: "session-456", }); @@ -272,7 +317,7 @@ describe("ensureCookies", () => { }); it("issues no cookie when the token names a different subject than the body", async () => { - verifySignedAuthResponseMock.mockResolvedValue({ sub: "someone-else" }); + verifySignedAuthResponseMock.mockResolvedValue({ typ: "access", sub: "someone-else" }); const result = await silentRefresh(); @@ -281,7 +326,7 @@ describe("ensureCookies", () => { }); it("verifies against the access token audience, not the service token's", async () => { - verifySignedAuthResponseMock.mockResolvedValue({ sub: "user-123" }); + verifySignedAuthResponseMock.mockResolvedValue({ typ: "access", sub: "user-123" }); await silentRefresh({ ...BASE_OPTS, @@ -299,7 +344,7 @@ describe("ensureCookies", () => { }); it("defaults the access token audience to the auth server URL", async () => { - verifySignedAuthResponseMock.mockResolvedValue({ sub: "user-123" }); + verifySignedAuthResponseMock.mockResolvedValue({ typ: "access", sub: "user-123" }); await silentRefresh({ ...BASE_OPTS, audience: "seamless-auth" }); @@ -312,7 +357,7 @@ describe("ensureCookies", () => { }); it("verifies against the configured auth server issuer", async () => { - verifySignedAuthResponseMock.mockResolvedValue({ sub: "user-123" }); + verifySignedAuthResponseMock.mockResolvedValue({ typ: "access", sub: "user-123" }); await silentRefresh({ ...BASE_OPTS, @@ -331,6 +376,7 @@ describe("ensureCookies", () => { it("takes the session id from the signed token rather than the body", async () => { verifySignedAuthResponseMock.mockResolvedValue({ + typ: "access", sub: "user-123", sid: "signed-session", }); @@ -489,7 +535,7 @@ describe("ensureCookies", () => { verifyCookieJwtMock.mockReturnValue({ sub: "user-123", - token: "access-token", + token: ACCESS_TOKEN, sessionId: "session-123", roles: ["user"], }); @@ -506,7 +552,7 @@ describe("ensureCookies", () => { expect(result.user).toEqual({ sub: "user-123", sessionId: "session-123", - token: "access-token", + token: ACCESS_TOKEN, roles: ["user"], }); }); @@ -516,7 +562,7 @@ describe("ensureCookies", () => { verifyCookieJwtMock.mockReturnValue({ sub: "admin-123", - token: "access-token", + token: ACCESS_TOKEN, sessionId: "session-123", roles: ["admin"], }); @@ -533,7 +579,7 @@ describe("ensureCookies", () => { expect(result.user).toEqual({ sub: "admin-123", sessionId: "session-123", - token: "access-token", + token: ACCESS_TOKEN, roles: ["admin"], }); }); @@ -543,7 +589,7 @@ describe("ensureCookies", () => { verifyCookieJwtMock.mockReturnValue({ sub: "user-123", - token: "access-token", + token: ACCESS_TOKEN, sessionId: "session-123", roles: ["user"], }); @@ -561,7 +607,7 @@ describe("ensureCookies", () => { ); expect(result.type).toBe("ok"); - expect(result.user?.token).toBe("access-token"); + expect(result.user?.token).toBe(ACCESS_TOKEN); } }); @@ -593,7 +639,7 @@ describe("ensureCookies", () => { verifyCookieJwtMock.mockReturnValue({ sub: "user-123", - token: "access-token", + token: ACCESS_TOKEN, sessionId: "session-123", roles: ["user"], }); @@ -610,7 +656,7 @@ describe("ensureCookies", () => { expect(result.user).toEqual({ sub: "user-123", sessionId: "session-123", - token: "access-token", + token: ACCESS_TOKEN, roles: ["user"], }); }); @@ -668,7 +714,7 @@ describe("ensureCookies with a manifest", () => { it("refreshes a missing access cookie for an access route", async () => { const { ensureCookies } = await import("../dist/ensureCookies.js"); - verifySignedAuthResponseMock.mockResolvedValue({ sub: "user-1", sid: "s-1" }); + verifySignedAuthResponseMock.mockResolvedValue({ typ: "access", sub: "user-1", sid: "s-1" }); refreshAccessTokenMock.mockResolvedValue({ sub: "user-1", token: "new-access", diff --git a/packages/core/tests/loginHandler.test.js b/packages/core/tests/loginHandler.test.js index 2a693ff..ec395d5 100644 --- a/packages/core/tests/loginHandler.test.js +++ b/packages/core/tests/loginHandler.test.js @@ -15,7 +15,7 @@ async function createSignedAuthResponse(subject = "user-123") { jwk.kid = "test-key"; jwk.use = "sig"; - const token = await new SignJWT({ sub: subject }) + const token = await new SignJWT({ sub: subject, typ: "ephemeral" }) .setProtectedHeader({ alg: "RS256", kid: "test-key" }) .setIssuer("https://auth.example.com") .setSubject(subject) diff --git a/packages/core/tests/manifestProxy.test.js b/packages/core/tests/manifestProxy.test.js index a47d958..ff97e10 100644 --- a/packages/core/tests/manifestProxy.test.js +++ b/packages/core/tests/manifestProxy.test.js @@ -20,6 +20,9 @@ jest.unstable_mockModule("../dist/deliverAuthMessage.js", () => ({ const { handleManifestRoute } = await import("../dist/manifestProxy.js"); +// Unsigned: the adapter only reads the inner token's type, the auth API verifies it. +const ACCESS_TOKEN = `e30.${Buffer.from(JSON.stringify({ typ: "access" })).toString("base64url")}.sig`; + const OPTIONS = { authServerUrl: "https://auth.test", audience: "https://auth.test", @@ -35,7 +38,7 @@ const OPTIONS = { const SESSION = { message: "Success", sub: "user-1", - token: "access-token", + token: ACCESS_TOKEN, refreshToken: "refresh-token", ttl: 300, refreshTtl: 3600, @@ -62,14 +65,14 @@ function input(route, overrides = {}) { } const ACCESS_COOKIES = { - cookiePayload: { sub: "user-1", token: "access-token" }, + cookiePayload: { sub: "user-1", token: ACCESS_TOKEN }, cookies: { "seamless-access": "signed" }, }; beforeEach(() => { authFetchMock.mockReset(); verifySignedAuthResponseMock.mockReset(); - verifySignedAuthResponseMock.mockResolvedValue({ sub: "user-1", sid: "s-1" }); + verifySignedAuthResponseMock.mockResolvedValue({ typ: "access", sub: "user-1", sid: "s-1" }); applyExternalDeliveryMock.mockReset(); deliverAuthMessageMock.mockReset(); deliverableNoticeKindsMock.mockReset(); @@ -92,7 +95,7 @@ describe("handleManifestRoute", () => { "https://auth.test/admin/users/a%2Fb?limit=5", expect.objectContaining({ method: "GET", - authorization: "Bearer access-token", + authorization: `Bearer ${ACCESS_TOKEN}`, serviceAuthorization: "Bearer proxy-token", }), ); @@ -193,7 +196,7 @@ describe("handleManifestRoute", () => { expect(result.setCookies[0].value).toMatchObject({ sub: "user-1", sessionId: "s-1", - token: "access-token", + token: ACCESS_TOKEN, }); }); @@ -247,6 +250,7 @@ describe("handleManifestRoute", () => { }); it("stores an ephemeral token under the route's cookie", async () => { + verifySignedAuthResponseMock.mockResolvedValue({ typ: "ephemeral", sub: "user-1" }); authFetchMock.mockResolvedValue( upstream(200, { message: "ok", sub: "user-1", token: "reg-token", ttl: 300 }), ); @@ -274,6 +278,7 @@ describe("handleManifestRoute", () => { }); it("passes only picked fields to the browser", async () => { + verifySignedAuthResponseMock.mockResolvedValue({ typ: "ephemeral", sub: "user-1" }); authFetchMock.mockResolvedValue( upstream(200, { message: "Success", diff --git a/packages/core/tests/oauthHandlers.test.js b/packages/core/tests/oauthHandlers.test.js index 0466d6d..94863fe 100644 --- a/packages/core/tests/oauthHandlers.test.js +++ b/packages/core/tests/oauthHandlers.test.js @@ -81,6 +81,7 @@ describe("oauthHandlers", () => { }), ); verifySignedAuthResponseMock.mockResolvedValue({ + typ: "access", sub: "user-123", sid: "session-123", }); diff --git a/packages/core/tests/pathParam.test.js b/packages/core/tests/pathParam.test.js new file mode 100644 index 0000000..c442473 --- /dev/null +++ b/packages/core/tests/pathParam.test.js @@ -0,0 +1,53 @@ +import { jest } from "@jest/globals"; + +const authFetchMock = jest.fn(); + +jest.unstable_mockModule("../dist/authFetch.js", () => ({ + authFetch: authFetchMock, +})); + +const { + encodePathParam, + InvalidPathParamError, + revokeSessionHandler, + updateUserHandler, + verifyMagicLinkHandler, +} = await import("../dist/index.js"); + +describe("encodePathParam", () => { + it.each(["", ".", ".."])("refuses %j", (value) => { + expect(() => encodePathParam(value, "id")).toThrow(InvalidPathParamError); + + try { + encodePathParam(value, "id"); + } catch (error) { + expect(error.status).toBe(400); + } + }); + + it.each([ + ["abc", "abc"], + ["a..b", "a..b"], + ["...", "..."], + ["a/b", "a%2Fb"], + ["x?y=1", "x%3Fy%3D1"], + ])("encodes %j as one segment", (value, encoded) => { + expect(encodePathParam(value)).toBe(encoded); + }); +}); + +describe("core handlers refuse dot-segment params", () => { + beforeEach(() => authFetchMock.mockReset()); + + it.each([ + ["revokeSessionHandler", async () => revokeSessionHandler("..", { authServerUrl: "https://a" })], + ["updateUserHandler", async () => updateUserHandler(".", { authServerUrl: "https://a" })], + [ + "verifyMagicLinkHandler", + () => verifyMagicLinkHandler({ token: ".." }, { authServerUrl: "https://a" }), + ], + ])("%s", async (_name, run) => { + await expect(run()).rejects.toBeInstanceOf(InvalidPathParamError); + expect(authFetchMock).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/core/tests/sessionBodyMaterial.test.js b/packages/core/tests/sessionBodyMaterial.test.js index 103ca93..7ddc453 100644 --- a/packages/core/tests/sessionBodyMaterial.test.js +++ b/packages/core/tests/sessionBodyMaterial.test.js @@ -76,6 +76,7 @@ describe("session responses do not repeat the credentials in the body", () => { global.fetch = jest.fn(); verifySignedAuthResponseMock.mockReset(); verifySignedAuthResponseMock.mockResolvedValue({ + typ: "access", sub: "user-123", sid: "session-123", }); diff --git a/packages/core/tests/upstreamSession.test.js b/packages/core/tests/upstreamSession.test.js index 204d752..8fbfc25 100644 --- a/packages/core/tests/upstreamSession.test.js +++ b/packages/core/tests/upstreamSession.test.js @@ -35,6 +35,7 @@ beforeEach(() => verifySignedAuthResponseMock.mockReset()); describe("verifyUpstreamSession", () => { it("returns the session id from the sid claim", async () => { verifySignedAuthResponseMock.mockResolvedValue({ + typ: "access", sub: "user-1", sid: "s-1", }); @@ -45,13 +46,13 @@ describe("verifyUpstreamSession", () => { }); it("omits the session id when the claim is absent or not a string", async () => { - verifySignedAuthResponseMock.mockResolvedValue({ sub: "user-1" }); + verifySignedAuthResponseMock.mockResolvedValue({ typ: "access", sub: "user-1" }); expect( (await verifyUpstreamSession(SESSION, "https://auth.test", "aud")) .sessionId, ).toBeUndefined(); - verifySignedAuthResponseMock.mockResolvedValue({ sub: "user-1", sid: 7 }); + verifySignedAuthResponseMock.mockResolvedValue({ typ: "access", sub: "user-1", sid: 7 }); expect( (await verifyUpstreamSession(SESSION, "https://auth.test", "aud")) .sessionId, @@ -68,8 +69,36 @@ describe("verifyUpstreamSession", () => { ).rejects.toThrow("Invalid signed response from Auth Server"); }); + it("refuses an ephemeral token where an access token is expected", async () => { + verifySignedAuthResponseMock.mockResolvedValue({ typ: "ephemeral", sub: "user-1" }); + + await expect( + verifyUpstreamSession(SESSION, "https://auth.test", "aud"), + ).rejects.toThrow("did not carry an access token"); + }); + + it("refuses a token with no type", async () => { + verifySignedAuthResponseMock.mockResolvedValue({ sub: "user-1" }); + + await expect( + verifyUpstreamSession(SESSION, "https://auth.test", "aud"), + ).rejects.toThrow("did not carry an access token"); + }); + + it("checks for an ephemeral token when asked to", async () => { + verifySignedAuthResponseMock.mockResolvedValue({ typ: "ephemeral", sub: "user-1" }); + await expect( + verifyUpstreamSession(SESSION, "https://auth.test", "aud", undefined, "ephemeral"), + ).resolves.toEqual({ sessionId: undefined }); + + verifySignedAuthResponseMock.mockResolvedValue({ typ: "access", sub: "user-1" }); + await expect( + verifyUpstreamSession(SESSION, "https://auth.test", "aud", undefined, "ephemeral"), + ).rejects.toThrow("did not carry an ephemeral token"); + }); + it("throws when the token describes a different subject", async () => { - verifySignedAuthResponseMock.mockResolvedValue({ sub: "someone-else" }); + verifySignedAuthResponseMock.mockResolvedValue({ typ: "access", sub: "someone-else" }); await expect( verifyUpstreamSession(SESSION, "https://auth.test", "aud"), @@ -77,7 +106,7 @@ describe("verifyUpstreamSession", () => { }); it("verifies against the configured server and audience", async () => { - verifySignedAuthResponseMock.mockResolvedValue({ sub: "user-1" }); + verifySignedAuthResponseMock.mockResolvedValue({ typ: "access", sub: "user-1" }); await verifyUpstreamSession(SESSION, "https://auth.test", "the-audience"); @@ -90,7 +119,7 @@ describe("verifyUpstreamSession", () => { }); it("passes a configured auth server issuer through to verification", async () => { - verifySignedAuthResponseMock.mockResolvedValue({ sub: "user-1" }); + verifySignedAuthResponseMock.mockResolvedValue({ typ: "access", sub: "user-1" }); await verifyUpstreamSession( SESSION, @@ -111,6 +140,7 @@ describe("verifyUpstreamSession", () => { describe("issueSessionCookies", () => { it("builds the access and refresh cookies", async () => { verifySignedAuthResponseMock.mockResolvedValue({ + typ: "access", sub: "user-1", sid: "s-1", }); @@ -140,7 +170,7 @@ describe("issueSessionCookies", () => { }); it("omits the session id key entirely when there is no sid", async () => { - verifySignedAuthResponseMock.mockResolvedValue({ sub: "user-1" }); + verifySignedAuthResponseMock.mockResolvedValue({ typ: "access", sub: "user-1" }); const [access] = await issueSessionCookies(SESSION, OPTIONS); @@ -150,7 +180,7 @@ describe("issueSessionCookies", () => { // The refresh path writes `organizationId: null` on every reissue, so a // session that omitted it disagreed with itself after one refresh. it("always carries an organization id, null when there is none", async () => { - verifySignedAuthResponseMock.mockResolvedValue({ sub: "user-1" }); + verifySignedAuthResponseMock.mockResolvedValue({ typ: "access", sub: "user-1" }); const [access] = await issueSessionCookies( { ...SESSION, organizationId: undefined }, @@ -161,7 +191,7 @@ describe("issueSessionCookies", () => { }); it("issues only the access cookie when no refresh cookie is named", async () => { - verifySignedAuthResponseMock.mockResolvedValue({ sub: "user-1" }); + verifySignedAuthResponseMock.mockResolvedValue({ typ: "access", sub: "user-1" }); const cookies = await issueSessionCookies(SESSION, { ...OPTIONS, @@ -173,7 +203,7 @@ describe("issueSessionCookies", () => { }); it("verifies against the configured auth server issuer", async () => { - verifySignedAuthResponseMock.mockResolvedValue({ sub: "user-1" }); + verifySignedAuthResponseMock.mockResolvedValue({ typ: "access", sub: "user-1" }); await issueSessionCookies(SESSION, { ...OPTIONS, diff --git a/packages/core/tests/validateSecrets.test.js b/packages/core/tests/validateSecrets.test.js index af94ec7..bbd5a1e 100644 --- a/packages/core/tests/validateSecrets.test.js +++ b/packages/core/tests/validateSecrets.test.js @@ -145,3 +145,55 @@ describe("core entry points enforce secret strength", () => { ).rejects.toThrow(/serviceSecret must be at least/); }); }); + +describe("placeholder secrets", () => { + const PLACEHOLDERS = [ + "dev-only-cookie-signing-key-replace-me", + "GRAB_FROM_SEAMLESS_AUTH_PORTAL_DEV", + "my-own-long-secret-value-REPLACE-ME-please", + ]; + + it.each(PLACEHOLDERS)("assertSecretStrength refuses %s", (value) => { + expect(() => assertSecretStrength("cookieSecret", value)).toThrow( + /cookieSecret is a published placeholder value/, + ); + }); + + it.each(PLACEHOLDERS)("assertSecretStrength accepts %s when allowed", (value) => { + expect(() => + assertSecretStrength("cookieSecret", value, { allowPlaceholder: true }), + ).not.toThrow(); + }); + + it.each([ + ["cookieSecret", { cookieSecret: PLACEHOLDERS[0], serviceSecret: STRONG_SERVICE_SECRET }], + ["serviceSecret", { cookieSecret: STRONG_COOKIE_SECRET, serviceSecret: PLACEHOLDERS[1] }], + ])("assertSecrets refuses a placeholder %s", (name, secrets) => { + expect(() => assertSecrets(secrets)).toThrow( + new RegExp(`${name} is a published placeholder value`), + ); + expect(() => assertSecrets({ ...secrets, cookieSecure: true })).toThrow( + /published placeholder/, + ); + }); + + it("assertSecrets accepts a placeholder in insecure local mode", () => { + expect(() => + assertSecrets({ + cookieSecret: PLACEHOLDERS[0], + serviceSecret: PLACEHOLDERS[1], + cookieSecure: false, + }), + ).not.toThrow(); + }); + + it("still refuses a short secret in insecure local mode", () => { + expect(() => + assertSecrets({ + cookieSecret: "replace-me", + serviceSecret: STRONG_SERVICE_SECRET, + cookieSecure: false, + }), + ).toThrow(/at least 32 characters/); + }); +}); diff --git a/packages/express/README.md b/packages/express/README.md index e378391..2117db0 100644 --- a/packages/express/README.md +++ b/packages/express/README.md @@ -250,7 +250,12 @@ otherwise `createSeamlessAuthServer` (and `createEnsureCookiesMiddleware`) throw is mounted. A short secret can be brute forced offline, which would let an attacker forge cookie sessions and service tokens. -The same check applies to `requireAuth({ cookieSecret })`, which throws when the guard is +A known placeholder is refused the same way: `dev-only-cookie-signing-key-replace-me`, +`GRAB_FROM_SEAMLESS_AUTH_PORTAL_DEV`, or any value containing `replace-me`. Anyone who has read the +example can forge sessions with one. The router accepts a placeholder only with +`cookieSecure: false`, the local HTTP development mode. + +The length check also applies to `requireAuth({ cookieSecret })`, which throws when the guard is constructed, and to `getSeamlessUser`, which validates through `@seamless-auth/core`. Generate secrets with a CSPRNG, for example `openssl rand -base64 48`, and supply them through the @@ -344,7 +349,8 @@ cross-site state-changing requests with `403 { "error": send and page JavaScript cannot forge. A non-safe request (anything other than `GET`, `HEAD`, or `OPTIONS`) is rejected when that header is `cross-site`. Same-origin single-page-app calls send `same-origin` or `same-site` and pass - with no allowlist. + with no allowlist; with `allowedOrigins` set, a `same-site` call must also + come from a listed `Origin`. - Safe methods are never gated, so `GET /magic-link/verify/:token` and the CORS preflight (`OPTIONS`) are unaffected. `POST /magic-link/verify/:token/confirm` is a state change and is guarded like any other, so serve the page that calls @@ -352,12 +358,16 @@ cross-site state-changing requests with `403 { "error": - A caller that sends neither `Origin` nor `Sec-Fetch-Site`, such as a server-to-server request, passes. -`allowedOrigins` is an opt-in fallback for older browsers that do not send -`Sec-Fetch-Site`. It is consulted only when that header is absent: the request -`Origin` must appear in the list, otherwise the request is rejected. When -`Sec-Fetch-Site` is absent and `allowedOrigins` is unset, the request passes, so -setting it is never required. Set it if you support browsers old enough to lack -`Sec-Fetch-Site` and want the `Origin` check for them: +`allowedOrigins` is an opt-in `Origin` allowlist. When it is set, the request +`Origin` must appear in the list for a state-changing request that is +`same-site`, and for one from an older browser that sends no `Sec-Fetch-Site`; +otherwise the request is rejected. `same-site` covers every subdomain of your +registrable domain, so without the list a page on a sibling subdomain can send +a request that carries a `SameSite=None` cookie. A `same-origin` request always +passes. When `allowedOrigins` is unset the guard behaves as before (a +`same-site` request passes, as does one with no `Sec-Fetch-Site`), so setting it +is never required. Set it when you do not trust every subdomain of your site, or +support browsers old enough to lack `Sec-Fetch-Site`: ```ts app.use( @@ -628,7 +638,9 @@ With `authServerIssuer` set, pass the same value as `audience`: the auth API set `aud` to its `ISSUER`. `authServerUrl` and `audience` must be given together. Leave both out and the guard accepts -cookies only, which is what every earlier version did. +cookies only, which is what every earlier version did. With them configured, the guard also +verifies the access token inside the cookie against the auth API's JWKS and takes the session's +roles from it, so a cookie signed with `cookieSecret` alone cannot carry claims of its own. **`req.user` shape (`SeamlessAuthUser`)** diff --git a/packages/express/src/consoleProxy.ts b/packages/express/src/consoleProxy.ts index 89b2c49..5c30434 100644 --- a/packages/express/src/consoleProxy.ts +++ b/packages/express/src/consoleProxy.ts @@ -10,8 +10,18 @@ const FORWARDED_RESPONSE_HEADERS = [ "cache-control", "etag", "last-modified", + "content-security-policy", + "x-frame-options", + "x-content-type-options", ]; +// The console runs with an admin session on the app's origin, so it must not be +// framed or sniffed even when the API leaves these off. +const FALLBACK_RESPONSE_HEADERS: Record = { + "x-frame-options": "DENY", + "x-content-type-options": "nosniff", +}; + function normalizeBasePath(basePath: string): string { const withLeadingSlash = basePath.startsWith("/") ? basePath : `/${basePath}`; return withLeadingSlash.replace(/\/+$/, "") || "/"; @@ -125,6 +135,12 @@ export function createSeamlessConsoleProxy( } } + for (const [header, value] of Object.entries(FALLBACK_RESPONSE_HEADERS)) { + if (!res.hasHeader(header)) { + res.setHeader(header, value); + } + } + res.status(response.status); if (req.method === "HEAD" || !response.body) { diff --git a/packages/express/src/createServer.ts b/packages/express/src/createServer.ts index bd32573..ce855d5 100644 --- a/packages/express/src/createServer.ts +++ b/packages/express/src/createServer.ts @@ -34,6 +34,7 @@ import { AuthFetchOptions, checkProxyIdentity, createAdapterManifestSource, + encodePathParam, handleManifestRoute, matchManifestRoute, proxyRequest, @@ -422,7 +423,7 @@ export function createSeamlessAuthServer( "/organizations/:organizationId", proxyWithIdentity( (req) => - `organizations/${encodeURIComponent(routeParam(req, "organizationId"))}`, + `organizations/${encodePathParam(routeParam(req, "organizationId"), "organizationId")}`, "access", "GET", ), @@ -431,7 +432,7 @@ export function createSeamlessAuthServer( "/organizations/:organizationId", proxyWithIdentity( (req) => - `organizations/${encodeURIComponent(routeParam(req, "organizationId"))}`, + `organizations/${encodePathParam(routeParam(req, "organizationId"), "organizationId")}`, "access", "PATCH", ), @@ -443,7 +444,7 @@ export function createSeamlessAuthServer( "/organizations/:organizationId/members", proxyWithIdentity( (req) => - `organizations/${encodeURIComponent(routeParam(req, "organizationId"))}/members`, + `organizations/${encodePathParam(routeParam(req, "organizationId"), "organizationId")}/members`, "access", "GET", ), @@ -452,7 +453,7 @@ export function createSeamlessAuthServer( "/organizations/:organizationId/members", proxyWithIdentity( (req) => - `organizations/${encodeURIComponent(routeParam(req, "organizationId"))}/members`, + `organizations/${encodePathParam(routeParam(req, "organizationId"), "organizationId")}/members`, "access", ), ); @@ -460,7 +461,7 @@ export function createSeamlessAuthServer( "/organizations/:organizationId/members/:userId", proxyWithIdentity( (req) => - `organizations/${encodeURIComponent(routeParam(req, "organizationId"))}/members/${encodeURIComponent(routeParam(req, "userId"))}`, + `organizations/${encodePathParam(routeParam(req, "organizationId"), "organizationId")}/members/${encodePathParam(routeParam(req, "userId"), "userId")}`, "access", "PATCH", ), @@ -469,7 +470,7 @@ export function createSeamlessAuthServer( "/organizations/:organizationId/members/:userId", proxyWithIdentity( (req) => - `organizations/${encodeURIComponent(routeParam(req, "organizationId"))}/members/${encodeURIComponent(routeParam(req, "userId"))}`, + `organizations/${encodePathParam(routeParam(req, "organizationId"), "organizationId")}/members/${encodePathParam(routeParam(req, "userId"), "userId")}`, "access", "DELETE", ), @@ -544,7 +545,7 @@ export function createSeamlessAuthServer( "/system-config/oauth-providers/:id", proxyWithIdentity( (req) => - `system-config/oauth-providers/${encodeURIComponent(routeParam(req, "id"))}`, + `system-config/oauth-providers/${encodePathParam(routeParam(req, "id"), "id")}`, "access", "PATCH", ), @@ -553,7 +554,7 @@ export function createSeamlessAuthServer( "/system-config/oauth-providers/:id", proxyWithIdentity( (req) => - `system-config/oauth-providers/${encodeURIComponent(routeParam(req, "id"))}`, + `system-config/oauth-providers/${encodePathParam(routeParam(req, "id"), "id")}`, "access", "DELETE", ), @@ -630,7 +631,7 @@ export function createSeamlessAuthServer( "/admin/organizations/:organizationId", proxyWithIdentity( (req) => - `admin/organizations/${encodeURIComponent(routeParam(req, "organizationId"))}`, + `admin/organizations/${encodePathParam(routeParam(req, "organizationId"), "organizationId")}`, "access", "GET", ), @@ -639,7 +640,7 @@ export function createSeamlessAuthServer( "/admin/organizations/:organizationId", proxyWithIdentity( (req) => - `admin/organizations/${encodeURIComponent(routeParam(req, "organizationId"))}`, + `admin/organizations/${encodePathParam(routeParam(req, "organizationId"), "organizationId")}`, "access", "PATCH", ), @@ -648,7 +649,7 @@ export function createSeamlessAuthServer( "/admin/organizations/:organizationId", proxyWithIdentity( (req) => - `admin/organizations/${encodeURIComponent(routeParam(req, "organizationId"))}`, + `admin/organizations/${encodePathParam(routeParam(req, "organizationId"), "organizationId")}`, "access", "DELETE", ), @@ -657,7 +658,7 @@ export function createSeamlessAuthServer( "/admin/organizations/:organizationId/members", proxyWithIdentity( (req) => - `admin/organizations/${encodeURIComponent(routeParam(req, "organizationId"))}/members`, + `admin/organizations/${encodePathParam(routeParam(req, "organizationId"), "organizationId")}/members`, "access", "GET", ), @@ -666,7 +667,7 @@ export function createSeamlessAuthServer( "/admin/organizations/:organizationId/members", proxyWithIdentity( (req) => - `admin/organizations/${encodeURIComponent(routeParam(req, "organizationId"))}/members`, + `admin/organizations/${encodePathParam(routeParam(req, "organizationId"), "organizationId")}/members`, "access", ), ); @@ -674,7 +675,7 @@ export function createSeamlessAuthServer( "/admin/organizations/:organizationId/members/:userId", proxyWithIdentity( (req) => - `admin/organizations/${encodeURIComponent(routeParam(req, "organizationId"))}/members/${encodeURIComponent(routeParam(req, "userId"))}`, + `admin/organizations/${encodePathParam(routeParam(req, "organizationId"), "organizationId")}/members/${encodePathParam(routeParam(req, "userId"), "userId")}`, "access", "PATCH", ), @@ -683,7 +684,7 @@ export function createSeamlessAuthServer( "/admin/organizations/:organizationId/members/:userId", proxyWithIdentity( (req) => - `admin/organizations/${encodeURIComponent(routeParam(req, "organizationId"))}/members/${encodeURIComponent(routeParam(req, "userId"))}`, + `admin/organizations/${encodePathParam(routeParam(req, "organizationId"), "organizationId")}/members/${encodePathParam(routeParam(req, "userId"), "userId")}`, "access", "DELETE", ), @@ -692,7 +693,7 @@ export function createSeamlessAuthServer( "/admin/organizations/:organizationId/oauth-providers/:providerId/retirement", proxyWithIdentity( (req) => - `admin/organizations/${encodeURIComponent(routeParam(req, "organizationId"))}/oauth-providers/${encodeURIComponent(routeParam(req, "providerId"))}/retirement`, + `admin/organizations/${encodePathParam(routeParam(req, "organizationId"), "organizationId")}/oauth-providers/${encodePathParam(routeParam(req, "providerId"), "providerId")}/retirement`, "access", "PUT", ), @@ -701,7 +702,7 @@ export function createSeamlessAuthServer( "/admin/organizations/:organizationId/oauth-providers/:providerId/retirement", proxyWithIdentity( (req) => - `admin/organizations/${encodeURIComponent(routeParam(req, "organizationId"))}/oauth-providers/${encodeURIComponent(routeParam(req, "providerId"))}/retirement`, + `admin/organizations/${encodePathParam(routeParam(req, "organizationId"), "organizationId")}/oauth-providers/${encodePathParam(routeParam(req, "providerId"), "providerId")}/retirement`, "access", "DELETE", ), diff --git a/packages/express/src/middleware/requireAuth.ts b/packages/express/src/middleware/requireAuth.ts index eababdf..cb7a919 100644 --- a/packages/express/src/middleware/requireAuth.ts +++ b/packages/express/src/middleware/requireAuth.ts @@ -8,7 +8,8 @@ export interface RequireAuthOptions { * Together with `audience`, lets the guard accept the auth API's own access * token in `Authorization: Bearer`, which is how a native client with no * cookie jar authenticates. Leave both out and the guard accepts cookies - * only, as it always has. + * only, as it always has. With both set, the access token inside a cookie + * is verified against the auth API's JWKS too. */ authServerUrl?: string; /** @@ -76,7 +77,10 @@ export function requireAuth(opts: RequireAuthOptions) { } = opts; // Eagerly, so a weak secret fails at setup rather than on the first request. - assertSecretStrength("requireAuth: cookieSecret", cookieSecret); + // A guard has no local-mode setting, so only the adapter refuses a placeholder. + assertSecretStrength("requireAuth: cookieSecret", cookieSecret, { + allowPlaceholder: true, + }); if ((authServerUrl === undefined) !== (audience === undefined)) { throw new Error( diff --git a/packages/express/tests/adminRoutes.test.js b/packages/express/tests/adminRoutes.test.js index 543235c..c5fbf51 100644 --- a/packages/express/tests/adminRoutes.test.js +++ b/packages/express/tests/adminRoutes.test.js @@ -5,6 +5,9 @@ import request from "supertest"; const { default: createSeamlessAuthServer } = await import("../dist/index.js"); +// Unsigned: the adapter only reads the inner token's type, the auth API verifies it. +const ACCESS_TOKEN = `e30.${Buffer.from(JSON.stringify({ typ: "access" })).toString("base64url")}.sig`; + function createJsonResponse(status, body) { return { ok: status >= 200 && status < 300, @@ -19,7 +22,7 @@ function createAccessCookie(subject = "admin-123") { sub: subject, roles: ["admin"], sessionId: "session-123", - token: "access-token", + token: ACCESS_TOKEN, }, "cookie-secret-cookie-secret-cookie-secret", { @@ -80,8 +83,8 @@ describe("admin routes", () => { method: "DELETE", body: JSON.stringify(body), headers: expect.objectContaining({ - Authorization: "Bearer access-token", - "x-seamless-service-token": expect.not.stringContaining("access-token"), + Authorization: `Bearer ${ACCESS_TOKEN}`, + "x-seamless-service-token": expect.not.stringContaining(ACCESS_TOKEN), }), }), ); diff --git a/packages/express/tests/cookieRequirementRoutes.test.js b/packages/express/tests/cookieRequirementRoutes.test.js index 29c17eb..c375895 100644 --- a/packages/express/tests/cookieRequirementRoutes.test.js +++ b/packages/express/tests/cookieRequirementRoutes.test.js @@ -5,6 +5,9 @@ import request from "supertest"; const { default: createSeamlessAuthServer } = await import("../dist/index.js"); +// Unsigned: the adapter only reads the inner token's type, the auth API verifies it. +const ACCESS_TOKEN = `e30.${Buffer.from(JSON.stringify({ typ: "access" })).toString("base64url")}.sig`; + function createJsonResponse(status, body) { return { ok: status >= 200 && status < 300, @@ -19,7 +22,7 @@ function createAccessCookie(subject = "user-123") { sub: subject, roles: ["admin"], sessionId: "session-123", - token: "access-token", + token: ACCESS_TOKEN, }, "cookie-secret-cookie-secret-cookie-secret", { @@ -79,7 +82,7 @@ describe("cookie requirement routes", () => { method: "POST", body: JSON.stringify(body), headers: expect.objectContaining({ - Authorization: "Bearer access-token", + Authorization: `Bearer ${ACCESS_TOKEN}`, }), }), ); @@ -101,7 +104,7 @@ describe("cookie requirement routes", () => { expect.objectContaining({ method: "POST", headers: expect.objectContaining({ - Authorization: "Bearer access-token", + Authorization: `Bearer ${ACCESS_TOKEN}`, }), }), ); @@ -122,7 +125,7 @@ describe("cookie requirement routes", () => { "https://auth.example.com/users/credentials", expect.objectContaining({ headers: expect.objectContaining({ - Authorization: "Bearer access-token", + Authorization: `Bearer ${ACCESS_TOKEN}`, }), }), ); @@ -147,7 +150,7 @@ describe("cookie requirement routes", () => { expect.stringContaining("https://auth.example.com/sessions"), expect.objectContaining({ headers: expect.objectContaining({ - Authorization: "Bearer access-token", + Authorization: `Bearer ${ACCESS_TOKEN}`, }), }), ); @@ -165,7 +168,7 @@ describe("cookie requirement routes", () => { expect.stringContaining("https://auth.example.com/admin/credential-count"), expect.objectContaining({ headers: expect.objectContaining({ - Authorization: "Bearer access-token", + Authorization: `Bearer ${ACCESS_TOKEN}`, }), }), ); @@ -229,7 +232,7 @@ describe("cookie requirement routes", () => { expect.stringContaining("https://auth.example.com/admin/sessions"), expect.objectContaining({ headers: expect.objectContaining({ - Authorization: "Bearer access-token", + Authorization: `Bearer ${ACCESS_TOKEN}`, }), }), ); diff --git a/packages/express/tests/cookieSecurity.test.js b/packages/express/tests/cookieSecurity.test.js index 95f99a7..7e303ea 100644 --- a/packages/express/tests/cookieSecurity.test.js +++ b/packages/express/tests/cookieSecurity.test.js @@ -7,6 +7,9 @@ import request from "supertest"; const { default: createSeamlessAuthServer, createEnsureCookiesMiddleware } = await import("../dist/index.js"); +// Unsigned: the adapter only reads the inner token's type, the auth API verifies it. +const ACCESS_TOKEN = `e30.${Buffer.from(JSON.stringify({ typ: "access" })).toString("base64url")}.sig`; + function createJsonResponse(status, body) { return { ok: status >= 200 && status < 300, @@ -55,7 +58,7 @@ async function setCookieHeaders(app, path = "/auth/users/me") { function createAccessCookie(subject = "user-123") { const token = jwt.sign( - { sub: subject, roles: ["user"], sessionId: "session-123", token: "access-token" }, + { sub: subject, roles: ["user"], sessionId: "session-123", token: ACCESS_TOKEN }, "cookie-secret-cookie-secret-cookie-secret", { algorithm: "HS256", expiresIn: "300s" }, ); diff --git a/packages/express/tests/failureWireFormat.test.js b/packages/express/tests/failureWireFormat.test.js index abcc53d..c21c418 100644 --- a/packages/express/tests/failureWireFormat.test.js +++ b/packages/express/tests/failureWireFormat.test.js @@ -11,6 +11,9 @@ import request from "supertest"; const { default: createSeamlessAuthServer } = await import("../dist/index.js"); +// Unsigned: the adapter only reads the inner token's type, the auth API verifies it. +const ACCESS_TOKEN = `e30.${Buffer.from(JSON.stringify({ typ: "access" })).toString("base64url")}.sig`; + const COOKIE_SECRET = "cookie-secret-cookie-secret-cookie-secret"; function createJsonResponse(status, body) { @@ -27,7 +30,7 @@ function createAccessCookie() { sub: "admin-123", roles: ["admin"], sessionId: "session-123", - token: "access-token", + token: ACCESS_TOKEN, }, COOKIE_SECRET, { algorithm: "HS256", expiresIn: "300s" }, diff --git a/packages/express/tests/getSeamlessUser.test.js b/packages/express/tests/getSeamlessUser.test.js index 0106691..b143a45 100644 --- a/packages/express/tests/getSeamlessUser.test.js +++ b/packages/express/tests/getSeamlessUser.test.js @@ -5,7 +5,8 @@ const { getSeamlessUser } = await import("../dist/index.js"); const SERVICE_SECRET = "service-secret-service-secret-service-secret"; const COOKIE_SECRET = "cookie-secret-cookie-secret-cookie-secret"; -const USER_ACCESS_TOKEN = "user-access-token"; +// Unsigned: the adapter only reads the inner token's type, the auth API verifies it. +const USER_ACCESS_TOKEN = `e30.${Buffer.from(JSON.stringify({ typ: "access" })).toString("base64url")}.sig`; const USER = { id: "user-1", email: "user@example.com", diff --git a/packages/express/tests/internalMetricsRoutes.test.js b/packages/express/tests/internalMetricsRoutes.test.js index 0e8b25c..3ea75ec 100644 --- a/packages/express/tests/internalMetricsRoutes.test.js +++ b/packages/express/tests/internalMetricsRoutes.test.js @@ -5,6 +5,9 @@ import request from "supertest"; const { default: createSeamlessAuthServer } = await import("../dist/index.js"); +// Unsigned: the adapter only reads the inner token's type, the auth API verifies it. +const ACCESS_TOKEN = `e30.${Buffer.from(JSON.stringify({ typ: "access" })).toString("base64url")}.sig`; + function createJsonResponse(status, body) { return { ok: status >= 200 && status < 300, @@ -19,7 +22,7 @@ function createAccessCookie(subject = "admin-123") { sub: subject, roles: ["admin"], sessionId: "session-123", - token: "access-token", + token: ACCESS_TOKEN, }, "cookie-secret-cookie-secret-cookie-secret", { @@ -80,8 +83,8 @@ describe("internal metrics routes", () => { expect.objectContaining({ method: "GET", headers: expect.objectContaining({ - Authorization: "Bearer access-token", - "x-seamless-service-token": expect.not.stringContaining("access-token"), + Authorization: `Bearer ${ACCESS_TOKEN}`, + "x-seamless-service-token": expect.not.stringContaining(ACCESS_TOKEN), }), }), ); diff --git a/packages/express/tests/logoutRoutes.test.js b/packages/express/tests/logoutRoutes.test.js index 9ef8ca4..ff93abb 100644 --- a/packages/express/tests/logoutRoutes.test.js +++ b/packages/express/tests/logoutRoutes.test.js @@ -5,6 +5,9 @@ import request from "supertest"; const { default: createSeamlessAuthServer } = await import("../dist/index.js"); +// Unsigned: the adapter only reads the inner token's type, the auth API verifies it. +const ACCESS_TOKEN = `e30.${Buffer.from(JSON.stringify({ typ: "access" })).toString("base64url")}.sig`; + function createResponse(status = 200) { return { ok: status >= 200 && status < 300, @@ -18,7 +21,7 @@ function createAccessCookie(subject = "user-123") { sub: subject, roles: ["user"], sessionId: "session-123", - token: "access-token", + token: ACCESS_TOKEN, }, "cookie-secret-cookie-secret-cookie-secret", { @@ -80,8 +83,8 @@ describe("logout routes", () => { expect.objectContaining({ method: "DELETE", headers: expect.objectContaining({ - Authorization: "Bearer access-token", - "x-seamless-service-token": expect.not.stringContaining("access-token"), + Authorization: `Bearer ${ACCESS_TOKEN}`, + "x-seamless-service-token": expect.not.stringContaining(ACCESS_TOKEN), }), }), ); diff --git a/packages/express/tests/manifestRoutes.test.js b/packages/express/tests/manifestRoutes.test.js index a968047..2c9d832 100644 --- a/packages/express/tests/manifestRoutes.test.js +++ b/packages/express/tests/manifestRoutes.test.js @@ -6,6 +6,9 @@ import request from "supertest"; const { default: createSeamlessAuthServer } = await import("../dist/index.js"); +// Unsigned: the adapter only reads the inner token's type, the auth API verifies it. +const ACCESS_TOKEN = `e30.${Buffer.from(JSON.stringify({ typ: "access" })).toString("base64url")}.sig`; + const AUTH = "https://auth.example.com"; const COOKIE_SECRET = "cookie-secret-cookie-secret-cookie-secret"; @@ -19,7 +22,7 @@ beforeAll(async () => { }); function signAccessToken(sub = "user-1") { - return new SignJWT({ sub, sid: "session-1" }) + return new SignJWT({ sub, sid: "session-1", typ: "access" }) .setProtectedHeader({ alg: "RS256", kid: "k1" }) .setIssuer(AUTH) .setAudience(AUTH) @@ -156,7 +159,7 @@ describe("routes proxied from the adapter manifest", () => { }); const app = createApp(); - const accessCookie = cookie("seamless-access", { sub: "user-1", token: "access-token" }); + const accessCookie = cookie("seamless-access", { sub: "user-1", token: ACCESS_TOKEN }); for (let i = 0; i < 2; i++) { const res = await request(app) @@ -169,7 +172,7 @@ describe("routes proxied from the adapter manifest", () => { expect(upstreamCalls("/.well-known/seamless-adapter.json")).toHaveLength(1); expect(upstreamCalls("/brand-new/a%20b")[0][1].headers.Authorization).toBe( - "Bearer access-token", + `Bearer ${ACCESS_TOKEN}`, ); }); @@ -181,7 +184,7 @@ describe("routes proxied from the adapter manifest", () => { const res = await request(createApp({ fetchManifest: false })) .get("/auth/users/me") - .set("Cookie", cookie("seamless-access", { sub: "user-1", token: "access-token" })); + .set("Cookie", cookie("seamless-access", { sub: "user-1", token: ACCESS_TOKEN })); expect(res.status).toBe(200); expect(res.body).toEqual({ user: { id: "user-1" }, credentials: [] }); diff --git a/packages/express/tests/minorCorrectness.test.js b/packages/express/tests/minorCorrectness.test.js index dc66522..64b9bd0 100644 --- a/packages/express/tests/minorCorrectness.test.js +++ b/packages/express/tests/minorCorrectness.test.js @@ -3,6 +3,9 @@ import express from "express"; import jwt from "jsonwebtoken"; import request from "supertest"; +// Unsigned: the adapter only reads the inner token's type, the auth API verifies it. +const ACCESS_TOKEN = `e30.${Buffer.from(JSON.stringify({ typ: "access" })).toString("base64url")}.sig`; + const { default: createSeamlessAuthServer, getSeamlessUser } = await import( "../dist/index.js" ); @@ -24,7 +27,7 @@ function createAccessCookie(subject = "admin-123") { sub: subject, roles: ["admin"], sessionId: "session-123", - token: "access-token", + token: ACCESS_TOKEN, }, "cookie-secret-cookie-secret-cookie-secret", { diff --git a/packages/express/tests/oauthProvidersProxy.test.js b/packages/express/tests/oauthProvidersProxy.test.js index 39abb83..c97b6a7 100644 --- a/packages/express/tests/oauthProvidersProxy.test.js +++ b/packages/express/tests/oauthProvidersProxy.test.js @@ -5,6 +5,9 @@ import request from "supertest"; const { default: createSeamlessAuthServer } = await import("../dist/index.js"); +// Unsigned: the adapter only reads the inner token's type, the auth API verifies it. +const ACCESS_TOKEN = `e30.${Buffer.from(JSON.stringify({ typ: "access" })).toString("base64url")}.sig`; + function createJsonResponse(status, body) { return { ok: status >= 200 && status < 300, @@ -15,7 +18,7 @@ function createJsonResponse(status, body) { function createAccessCookie(subject = "admin-123") { const token = jwt.sign( - { sub: subject, roles: ["admin"], token: "access-token" }, + { sub: subject, roles: ["admin"], token: ACCESS_TOKEN }, "cookie-secret-cookie-secret-cookie-secret", { algorithm: "HS256", @@ -72,7 +75,7 @@ describe("oauth provider admin proxy routes", () => { expect.objectContaining({ method: "GET", headers: expect.objectContaining({ - Authorization: "Bearer access-token", + Authorization: `Bearer ${ACCESS_TOKEN}`, }), }), ); diff --git a/packages/express/tests/organizationRoutes.test.js b/packages/express/tests/organizationRoutes.test.js index f3b9bfe..497f75b 100644 --- a/packages/express/tests/organizationRoutes.test.js +++ b/packages/express/tests/organizationRoutes.test.js @@ -5,6 +5,9 @@ import request from "supertest"; const { default: createSeamlessAuthServer } = await import("../dist/index.js"); +// Unsigned: the adapter only reads the inner token's type, the auth API verifies it. +const ACCESS_TOKEN = `e30.${Buffer.from(JSON.stringify({ typ: "access" })).toString("base64url")}.sig`; + function createJsonResponse(status, body) { return { ok: status >= 200 && status < 300, @@ -19,7 +22,7 @@ function createAccessCookie(subject = "user-123") { sub: subject, roles: ["admin"], sessionId: "session-123", - token: "access-token", + token: ACCESS_TOKEN, }, "cookie-secret-cookie-secret-cookie-secret", { @@ -82,8 +85,8 @@ describe("organization proxy routes", () => { expect.objectContaining({ method: "GET", headers: expect.objectContaining({ - Authorization: "Bearer access-token", - "x-seamless-service-token": expect.not.stringContaining("access-token"), + Authorization: `Bearer ${ACCESS_TOKEN}`, + "x-seamless-service-token": expect.not.stringContaining(ACCESS_TOKEN), }), }), ); @@ -126,7 +129,7 @@ describe("organization proxy routes", () => { expect.objectContaining({ method: "DELETE", headers: expect.objectContaining({ - Authorization: "Bearer access-token", + Authorization: `Bearer ${ACCESS_TOKEN}`, }), }), ); @@ -196,7 +199,7 @@ describe("organization proxy routes", () => { expect.objectContaining({ method, headers: expect.objectContaining({ - Authorization: "Bearer access-token", + Authorization: `Bearer ${ACCESS_TOKEN}`, }), }), ); diff --git a/packages/express/tests/originGuard.test.js b/packages/express/tests/originGuard.test.js index 254eb89..4b727d7 100644 --- a/packages/express/tests/originGuard.test.js +++ b/packages/express/tests/originGuard.test.js @@ -177,4 +177,57 @@ describe("origin guard (#104)", () => { expect(res.body).not.toEqual(BLOCKED); }); }); + + // A sibling subdomain is same-site, so with SameSite=None cookies the header + // alone cannot tell it from the app (#230). + describe("Sec-Fetch-Site present with allowedOrigins", () => { + const allowlisted = () => + createApp({ allowedOrigins: ["https://app.example.com"] }); + + it("rejects a same-site request from an Origin not on the list", async () => { + const res = await request(allowlisted()) + .post("/auth/magic-link") + .set("Sec-Fetch-Site", "same-site") + .set("Origin", "https://evil.example.com"); + + expect(res.status).toBe(403); + expect(res.body).toEqual(BLOCKED); + }); + + it("rejects a same-site request with no Origin", async () => { + const res = await request(allowlisted()) + .post("/auth/magic-link") + .set("Sec-Fetch-Site", "same-site"); + + expect(res.status).toBe(403); + expect(res.body).toEqual(BLOCKED); + }); + + it("accepts a same-site request from a listed Origin", async () => { + const res = await request(allowlisted()) + .post("/auth/magic-link") + .set("Sec-Fetch-Site", "same-site") + .set("Origin", "https://APP.example.com"); + + expect(res.status).not.toBe(403); + }); + + it("accepts a same-origin request whatever the list says", async () => { + const res = await request(allowlisted()) + .post("/auth/magic-link") + .set("Sec-Fetch-Site", "same-origin") + .set("Origin", "https://auth-adapter.example.com"); + + expect(res.status).not.toBe(403); + }); + + it("still rejects cross-site from a listed Origin", async () => { + const res = await request(allowlisted()) + .post("/auth/magic-link") + .set("Sec-Fetch-Site", "cross-site") + .set("Origin", "https://app.example.com"); + + expect(res.status).toBe(403); + }); + }); }); diff --git a/packages/express/tests/proxyQueryForwarding.test.js b/packages/express/tests/proxyQueryForwarding.test.js index 6c49572..c19b58f 100644 --- a/packages/express/tests/proxyQueryForwarding.test.js +++ b/packages/express/tests/proxyQueryForwarding.test.js @@ -9,6 +9,9 @@ import request from "supertest"; const { default: createSeamlessAuthServer } = await import("../dist/index.js"); +// Unsigned: the adapter only reads the inner token's type, the auth API verifies it. +const ACCESS_TOKEN = `e30.${Buffer.from(JSON.stringify({ typ: "access" })).toString("base64url")}.sig`; + const COOKIE_SECRET = "cookie-secret-cookie-secret-cookie-secret"; function createJsonResponse(status, body) { @@ -21,7 +24,7 @@ function createAccessCookie() { sub: "admin-123", roles: ["admin"], sessionId: "session-123", - token: "access-token", + token: ACCESS_TOKEN, }, COOKIE_SECRET, { algorithm: "HS256", expiresIn: "300s" }, diff --git a/packages/express/tests/proxyServiceToken.test.js b/packages/express/tests/proxyServiceToken.test.js index f752faf..6b545bd 100644 --- a/packages/express/tests/proxyServiceToken.test.js +++ b/packages/express/tests/proxyServiceToken.test.js @@ -7,7 +7,8 @@ const { default: createSeamlessAuthServer } = await import("../dist/index.js"); const SERVICE_SECRET = "service-secret-service-secret-service-secret"; const COOKIE_SECRET = "cookie-secret-cookie-secret-cookie-secret"; -const USER_ACCESS_TOKEN = "user-access-token"; +// Unsigned: the adapter only reads the inner token's type, the auth API verifies it. +const USER_ACCESS_TOKEN = `e30.${Buffer.from(JSON.stringify({ typ: "access" })).toString("base64url")}.sig`; function createJsonResponse(status, body) { return { diff --git a/packages/express/tests/queryForwarding.test.js b/packages/express/tests/queryForwarding.test.js index 15c0228..473e97a 100644 --- a/packages/express/tests/queryForwarding.test.js +++ b/packages/express/tests/queryForwarding.test.js @@ -5,6 +5,9 @@ import request from "supertest"; const { default: createSeamlessAuthServer } = await import("../dist/index.js"); +// Unsigned: the adapter only reads the inner token's type, the auth API verifies it. +const ACCESS_TOKEN = `e30.${Buffer.from(JSON.stringify({ typ: "access" })).toString("base64url")}.sig`; + const COOKIE_SECRET = "cookie-secret-cookie-secret-cookie-secret"; function accessCookie() { @@ -13,7 +16,7 @@ function accessCookie() { sub: "user-123", roles: ["admin"], sessionId: "s-1", - token: "access-token", + token: ACCESS_TOKEN, }, COOKIE_SECRET, { algorithm: "HS256", expiresIn: "300s" }, diff --git a/packages/express/tests/requireAuth.bearer.test.js b/packages/express/tests/requireAuth.bearer.test.js index 846ac89..dcb666b 100644 --- a/packages/express/tests/requireAuth.bearer.test.js +++ b/packages/express/tests/requireAuth.bearer.test.js @@ -127,8 +127,9 @@ describe("requireAuth with bearer tokens (express)", () => { it("still honours the cookie, and prefers it over a bearer header", async () => { const server = nextServer(); - mockAuthServer(server); - const cookie = jwt.sign({ sub: "cookie-user", token: INNER }, COOKIE_SECRET, { + const meCalls = mockAuthServer(server); + const inner = await accessToken(server, { sub: "cookie-user" }); + const cookie = jwt.sign({ sub: "cookie-user", token: inner }, COOKIE_SECRET, { expiresIn: "1h", }); @@ -141,7 +142,68 @@ describe("requireAuth with bearer tokens (express)", () => { expect(res.status).toBe(200); expect(res.body.user.id).toBe("cookie-user"); - expect(global.fetch).not.toHaveBeenCalled(); + expect(res.body.user.token).toBe(inner); + expect(meCalls).toHaveLength(0); + }); + + // The cookie secret alone signs a cookie, so with JWKS available the inner + // token is held to the auth API's key set too (#231). + describe("verifying the cookie's inner access token", () => { + function cookieFor(payload) { + return `seamless-access=${jwt.sign(payload, COOKIE_SECRET, { expiresIn: "1h" })}`; + } + + it("refuses a cookie whose inner token the auth API did not sign", async () => { + const server = nextServer(); + mockAuthServer(server); + + const res = await request( + buildApp({ cookieSecret: COOKIE_SECRET, authServerUrl: server, audience: server }), + ) + .get("/protected") + .set("Cookie", [cookieFor({ sub: "user-1", roles: ["admin"], token: INNER })]); + + expect(res.status).toBe(401); + expect(res.body).toEqual({ error: "Invalid or expired session" }); + }); + + it("refuses a cookie naming a different subject than its inner token", async () => { + const server = nextServer(); + mockAuthServer(server); + const inner = await accessToken(server, { sub: "someone-else" }); + + const res = await request( + buildApp({ cookieSecret: COOKIE_SECRET, authServerUrl: server, audience: server }), + ) + .get("/protected") + .set("Cookie", [cookieFor({ sub: "user-1", token: inner })]); + + expect(res.status).toBe(401); + }); + + it("takes the roles from the verified token, not the cookie", async () => { + const server = nextServer(); + mockAuthServer(server); + const inner = await accessToken(server, { sub: "user-1", roles: ["athlete"] }); + + const res = await request( + buildApp({ cookieSecret: COOKIE_SECRET, authServerUrl: server, audience: server }), + ) + .get("/protected") + .set("Cookie", [cookieFor({ sub: "user-1", roles: ["admin"], token: inner })]); + + expect(res.status).toBe(200); + expect(res.body.user.roles).toEqual(["athlete"]); + }); + + it("only decodes the inner token when the guard has no JWKS to check against", async () => { + const res = await request(buildApp({ cookieSecret: COOKIE_SECRET })) + .get("/protected") + .set("Cookie", [cookieFor({ sub: "user-1", roles: ["admin"], token: INNER })]); + + expect(res.status).toBe(200); + expect(res.body.user.roles).toEqual(["admin"]); + }); }); it("mentions both credentials in the warning when neither is sent", async () => { diff --git a/packages/express/tests/secretStrength.test.js b/packages/express/tests/secretStrength.test.js index e787f54..6778d7b 100644 --- a/packages/express/tests/secretStrength.test.js +++ b/packages/express/tests/secretStrength.test.js @@ -37,6 +37,30 @@ function middlewareOptions(overrides = {}) { } describe("secret strength validation", () => { + it.each([ + ["cookieSecret", "dev-only-cookie-signing-key-replace-me"], + ["serviceSecret", "GRAB_FROM_SEAMLESS_AUTH_PORTAL_DEV"], + ])("refuses a placeholder %s at startup", (name, value) => { + expect(() => createSeamlessAuthServer(serverOptions({ [name]: value }))).toThrow( + new RegExp(`${name} is a published placeholder value`), + ); + expect(() => + createEnsureCookiesMiddleware(middlewareOptions({ [name]: value })), + ).toThrow(/published placeholder/); + }); + + it("accepts a placeholder with cookieSecure: false, for local HTTP development", () => { + const local = { + cookieSecure: false, + cookieSecret: "dev-only-cookie-signing-key-replace-me", + serviceSecret: "GRAB_FROM_SEAMLESS_AUTH_PORTAL_DEV", + }; + + expect(() => createSeamlessAuthServer(serverOptions(local))).not.toThrow(); + expect(() => createEnsureCookiesMiddleware(middlewareOptions(local))).not.toThrow(); + expect(() => requireAuth({ cookieSecret: local.cookieSecret })).not.toThrow(); + }); + it("accepts secrets at or above the minimum length", () => { expect(() => createSeamlessAuthServer(serverOptions())).not.toThrow(); expect(() => diff --git a/packages/express/tests/stepUpProxy.test.js b/packages/express/tests/stepUpProxy.test.js index bb430fd..a170951 100644 --- a/packages/express/tests/stepUpProxy.test.js +++ b/packages/express/tests/stepUpProxy.test.js @@ -5,6 +5,9 @@ import request from "supertest"; const { default: createSeamlessAuthServer } = await import("../dist/index.js"); +// Unsigned: the adapter only reads the inner token's type, the auth API verifies it. +const ACCESS_TOKEN = `e30.${Buffer.from(JSON.stringify({ typ: "access" })).toString("base64url")}.sig`; + function createJsonResponse(status, body) { return { ok: status >= 200 && status < 300, @@ -15,7 +18,7 @@ function createJsonResponse(status, body) { function createAccessCookie(subject = "user-123") { const token = jwt.sign( - { sub: subject, roles: ["user"], token: "access-token" }, + { sub: subject, roles: ["user"], token: ACCESS_TOKEN }, "cookie-secret-cookie-secret-cookie-secret", { algorithm: "HS256", @@ -97,8 +100,8 @@ describe("step-up proxy routes", () => { expect.objectContaining({ method: "GET", headers: expect.objectContaining({ - Authorization: "Bearer access-token", - "x-seamless-service-token": expect.not.stringContaining("access-token"), + Authorization: `Bearer ${ACCESS_TOKEN}`, + "x-seamless-service-token": expect.not.stringContaining(ACCESS_TOKEN), }), }), ); @@ -139,8 +142,8 @@ describe("step-up proxy routes", () => { method: "POST", body: JSON.stringify(body), headers: expect.objectContaining({ - Authorization: "Bearer access-token", - "x-seamless-service-token": expect.not.stringContaining("access-token"), + Authorization: `Bearer ${ACCESS_TOKEN}`, + "x-seamless-service-token": expect.not.stringContaining(ACCESS_TOKEN), }), }), ); @@ -198,7 +201,7 @@ describe("step-up proxy routes", () => { expect.objectContaining({ method: "GET", headers: expect.objectContaining({ - Authorization: "Bearer access-token", + Authorization: `Bearer ${ACCESS_TOKEN}`, }), }), ); diff --git a/packages/express/tests/totpProxy.test.js b/packages/express/tests/totpProxy.test.js index f227bb4..5cd3c59 100644 --- a/packages/express/tests/totpProxy.test.js +++ b/packages/express/tests/totpProxy.test.js @@ -5,6 +5,9 @@ import request from "supertest"; const { default: createSeamlessAuthServer } = await import("../dist/index.js"); +// Unsigned: the adapter only reads the inner token's type, the auth API verifies it. +const ACCESS_TOKEN = `e30.${Buffer.from(JSON.stringify({ typ: "access" })).toString("base64url")}.sig`; + function createJsonResponse(status, body) { return { ok: status >= 200 && status < 300, @@ -15,7 +18,7 @@ function createJsonResponse(status, body) { function createAccessCookie(subject = "user-123") { const token = jwt.sign( - { sub: subject, roles: ["user"], token: "access-token" }, + { sub: subject, roles: ["user"], token: ACCESS_TOKEN }, "cookie-secret-cookie-secret-cookie-secret", { algorithm: "HS256", @@ -80,7 +83,7 @@ describe("TOTP proxy routes", () => { expect.objectContaining({ method: "GET", headers: expect.objectContaining({ - Authorization: "Bearer access-token", + Authorization: `Bearer ${ACCESS_TOKEN}`, }), }), ); diff --git a/packages/express/tests/usersRoutes.test.js b/packages/express/tests/usersRoutes.test.js index 8c84c50..eec2aee 100644 --- a/packages/express/tests/usersRoutes.test.js +++ b/packages/express/tests/usersRoutes.test.js @@ -5,6 +5,9 @@ import request from "supertest"; const { default: createSeamlessAuthServer } = await import("../dist/index.js"); +// Unsigned: the adapter only reads the inner token's type, the auth API verifies it. +const ACCESS_TOKEN = `e30.${Buffer.from(JSON.stringify({ typ: "access" })).toString("base64url")}.sig`; + function createJsonResponse(status, body) { return { ok: status >= 200 && status < 300, @@ -19,7 +22,7 @@ function createAccessCookie(subject = "user-123") { sub: subject, roles: ["admin"], sessionId: "session-123", - token: "access-token", + token: ACCESS_TOKEN, }, "cookie-secret-cookie-secret-cookie-secret", { @@ -123,7 +126,7 @@ describe("users proxy routes", () => { expect.objectContaining({ method: "DELETE", headers: expect.objectContaining({ - Authorization: "Bearer access-token", + Authorization: `Bearer ${ACCESS_TOKEN}`, }), }), ); diff --git a/packages/fastify/README.md b/packages/fastify/README.md index 21149a4..2a01836 100644 --- a/packages/fastify/README.md +++ b/packages/fastify/README.md @@ -46,7 +46,10 @@ hooks scoped to those routes, so the rest of your application is untouched. `serviceSecret` must match the auth API's. The auth server reads it from `API_SERVICE_TOKEN` and the app from `SEAMLESS_SERVICE_TOKEN`, with the same value. Both are checked at registration, so a -weak secret fails at startup rather than on the first request. +weak secret fails at startup rather than on the first request. A known +placeholder (`dev-only-cookie-signing-key-replace-me`, +`GRAB_FROM_SEAMLESS_AUTH_PORTAL_DEV`, or anything containing `replace-me`) is +refused too, unless `cookieSecure` is `false` for local HTTP development. ## Guarding your own routes @@ -190,7 +193,7 @@ into the dashboard work: the upstream answers them with the SPA shell. | `cookieDomain` | none | Domain attribute for the auth cookies | | `cookieSecure` | `true` | Set `false` only for local HTTP development | | `cookieSameSite` | `none` when secure, else `lax` | SameSite policy | -| `allowedOrigins` | none | Origin allowlist for browsers without `Sec-Fetch-Site` | +| `allowedOrigins` | none | Origin allowlist for state-changing requests that are `same-site` or come from browsers without `Sec-Fetch-Site` | | `accessCookieName` | `seamless-access` | Session cookie name | | `registrationCookieName` | `seamless-ephemeral` | Registration cookie name | | `refreshCookieName` | `seamless-refresh` | Refresh cookie name | diff --git a/packages/fastify/src/consoleProxy.ts b/packages/fastify/src/consoleProxy.ts index bc910d2..4b8f543 100644 --- a/packages/fastify/src/consoleProxy.ts +++ b/packages/fastify/src/consoleProxy.ts @@ -10,8 +10,18 @@ const FORWARDED_RESPONSE_HEADERS = [ "cache-control", "etag", "last-modified", + "content-security-policy", + "x-frame-options", + "x-content-type-options", ]; +// The console runs with an admin session on the app's origin, so it must not be +// framed or sniffed even when the API leaves these off. +const FALLBACK_RESPONSE_HEADERS: Record = { + "x-frame-options": "DENY", + "x-content-type-options": "nosniff", +}; + // The console is read-only static hosting, but the route has to claim the other // methods too so they answer 405 rather than falling through to a 404 that says // nothing about why. @@ -173,6 +183,12 @@ export const seamlessConsoleProxy: FastifyPluginAsync< } } + for (const [header, value] of Object.entries(FALLBACK_RESPONSE_HEADERS)) { + if (!reply.hasHeader(header)) { + reply.header(header, value); + } + } + reply.status(response.status); if (req.method === "HEAD" || !response.body) { diff --git a/packages/fastify/src/guards.ts b/packages/fastify/src/guards.ts index 1fc7e4b..f161121 100644 --- a/packages/fastify/src/guards.ts +++ b/packages/fastify/src/guards.ts @@ -14,7 +14,8 @@ export interface RequireAuthOptions { * Together with `audience`, lets the guard accept the auth API's own access * token in `Authorization: Bearer`, which is how a native client with no * cookie jar authenticates. Leave both out and the guard accepts cookies - * only, as it always has. + * only, as it always has. With both set, the access token inside a cookie + * is verified against the auth API's JWKS too. */ authServerUrl?: string; /** @@ -73,7 +74,10 @@ export function requireAuth(opts: RequireAuthOptions) { } = opts; // Eagerly, so a weak secret fails at setup rather than on the first request. - assertSecretStrength("requireAuth: cookieSecret", cookieSecret); + // A guard has no local-mode setting, so only the adapter refuses a placeholder. + assertSecretStrength("requireAuth: cookieSecret", cookieSecret, { + allowPlaceholder: true, + }); if ((authServerUrl === undefined) !== (audience === undefined)) { throw new Error( diff --git a/packages/fastify/src/routes/proxyRoutes.ts b/packages/fastify/src/routes/proxyRoutes.ts index e1ee8f2..4fd7ff9 100644 --- a/packages/fastify/src/routes/proxyRoutes.ts +++ b/packages/fastify/src/routes/proxyRoutes.ts @@ -1,4 +1,4 @@ -import type { ProxyIdentity } from "@seamless-auth/core"; +import { encodePathParam, type ProxyIdentity } from "@seamless-auth/core"; export interface ProxyRouteDefinition { method: "GET" | "POST" | "PUT" | "PATCH" | "DELETE"; @@ -297,10 +297,10 @@ export function resolveUpstreamPath( return template.replace(/:([A-Za-z0-9_]+)/g, (_match, name: string) => { const value = params[name]; - if (typeof value !== "string" || value === "") { + if (typeof value !== "string") { throw new Error(`Missing route parameter "${name}"`); } - return encodeURIComponent(value); + return encodePathParam(value, name); }); } diff --git a/packages/fastify/tests/dotSegmentParams.test.js b/packages/fastify/tests/dotSegmentParams.test.js new file mode 100644 index 0000000..81f19d1 --- /dev/null +++ b/packages/fastify/tests/dotSegmentParams.test.js @@ -0,0 +1,136 @@ +// light-my-request resolves dot segments before routing, as browsers do, so +// these go over a real socket where a raw `%2e%2e` reaches the route params. +import { jest } from "@jest/globals"; +import express from "express"; +import Fastify from "fastify"; +import jwt from "jsonwebtoken"; +import http from "node:http"; + +const { default: seamlessAuth } = await import("../dist/index.js"); +const { default: createSeamlessAuthServer } = + await import("../../express/dist/index.js"); + +// Unsigned: the adapter only reads the inner token's type, the auth API verifies it. +const ACCESS_TOKEN = `e30.${Buffer.from(JSON.stringify({ typ: "access" })).toString("base64url")}.sig`; + +const COOKIE_SECRET = "cookie-secret-cookie-secret-cookie-secret"; + +const OPTIONS = { + fetchManifest: false, + authServerUrl: "https://auth.example.com", + cookieSecret: COOKIE_SECRET, + serviceSecret: "service-secret-service-secret-service-secret", + audience: "https://auth.example.com", + jwksKid: "test-main", +}; + +const accessCookie = () => + `seamless-access=${jwt.sign( + { sub: "user-123", roles: ["admin"], sessionId: "s-1", token: ACCESS_TOKEN }, + COOKIE_SECRET, + { algorithm: "HS256", expiresIn: "300s" }, + )}`; + +// Raw http.request, since supertest also resolves dot segments in the URL. +async function send(handler, method, path) { + const server = http.createServer(handler); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + const body = method === "get" || method === "delete" ? undefined : "{}"; + + try { + return await new Promise((resolve, reject) => { + const req = http.request( + { + host: "127.0.0.1", + port: server.address().port, + method: method.toUpperCase(), + path: `/auth${path}`, + headers: { + cookie: accessCookie(), + ...(body ? { "content-type": "application/json" } : {}), + }, + }, + (res) => { + let text = ""; + res.on("data", (chunk) => (text += chunk)); + res.on("end", () => + resolve({ status: res.statusCode, body: text && JSON.parse(text) }), + ); + }, + ); + req.on("error", reject); + req.end(body); + }); + } finally { + await new Promise((resolve) => server.close(resolve)); + } +} + +describe("dot-segment route params", () => { + const originalFetch = global.fetch; + let fastify; + let fastifyHandler; + + beforeAll(async () => { + fastify = Fastify(); + await fastify.register(seamlessAuth, { prefix: "/auth", ...OPTIONS }); + await fastify.ready(); + fastifyHandler = (req, res) => fastify.routing(req, res); + }); + + afterAll(async () => { + await fastify.close(); + }); + + beforeEach(() => { + global.fetch = jest.fn(async () => ({ + ok: true, + status: 200, + json: async () => ({}), + })); + }); + + afterEach(() => { + global.fetch = originalFetch; + }); + + const expressApp = () => { + const app = express(); + app.use("/auth", createSeamlessAuthServer(OPTIONS)); + return app; + }; + + it.each([ + ["get", "/organizations/%2e%2e"], + ["patch", "/organizations/%2E/members/u1"], + ["delete", "/organizations/org-1/members/%2e%2e"], + ["patch", "/system-config/oauth-providers/%2e%2e"], + ["get", "/admin/organizations/%2e%2e"], + ["delete", "/sessions/%2e%2e"], + ["get", "/admin/users/%2e%2e"], + ["post", "/organizations/%2e%2e/switch"], + ["get", "/magic-link/verify/%2e%2e"], + ["post", "/oauth/%2e%2e/start"], + ])("refuses %s %s without calling upstream", async (method, path) => { + for (const server of [fastifyHandler, expressApp()]) { + const res = await send(server, method, path); + + expect(res.status).toBe(400); + expect(res.body).toEqual({ error: "bad_request" }); + } + + expect(global.fetch).not.toHaveBeenCalled(); + }); + + it("still proxies a param that only contains dots", async () => { + for (const server of [fastifyHandler, expressApp()]) { + const res = await send(server, "get", "/organizations/a..b"); + expect(res.status).toBe(200); + } + + expect(global.fetch.mock.calls.map(([url]) => String(url))).toEqual([ + "https://auth.example.com/organizations/a..b", + "https://auth.example.com/organizations/a..b", + ]); + }); +}); diff --git a/packages/fastify/tests/guards.test.js b/packages/fastify/tests/guards.test.js index 5aeedcb..712bf7a 100644 --- a/packages/fastify/tests/guards.test.js +++ b/packages/fastify/tests/guards.test.js @@ -164,19 +164,53 @@ describe("requireAuth (fastify)", () => { it("prefers the cookie over a bearer header", async () => { const server = nextServer(); - mockAuthServer(server); + const meCalls = mockAuthServer(server); + const inner = await accessToken(server, { sub: "cookie-user" }); const res = await get( await buildApp({ cookieSecret: COOKIE_SECRET, authServerUrl: server, audience: server }), { - cookie: `seamless-access=${signedCookie({ sub: "cookie-user", token: INNER })}`, + cookie: `seamless-access=${signedCookie({ sub: "cookie-user", token: inner })}`, authorization: `Bearer ${await accessToken(server)}`, }, ); expect(res.status).toBe(200); expect(res.body.user.id).toBe("cookie-user"); - expect(global.fetch).not.toHaveBeenCalled(); + expect(meCalls).toHaveLength(0); + }); + + // The cookie secret alone signs a cookie, so with JWKS available the inner + // token is held to the auth API's key set too (#231). + it("refuses a cookie whose inner token the auth API did not sign", async () => { + const server = nextServer(); + mockAuthServer(server); + + const res = await get( + await buildApp({ cookieSecret: COOKIE_SECRET, authServerUrl: server, audience: server }), + { + cookie: `seamless-access=${signedCookie({ sub: "user-1", roles: ["admin"], token: INNER })}`, + }, + ); + + expect(res.status).toBe(401); + expect(res.body).toEqual({ error: "Invalid or expired session" }); + }); + + it("takes a cookie session's roles from its verified inner token", async () => { + const server = nextServer(); + mockAuthServer(server); + const inner = await accessToken(server, { sub: "user-1", roles: ["athlete"] }); + + const res = await get( + await buildApp({ cookieSecret: COOKIE_SECRET, authServerUrl: server, audience: server }), + { + cookie: `seamless-access=${signedCookie({ sub: "user-1", roles: ["admin"], token: inner })}`, + }, + ); + + expect(res.status).toBe(200); + expect(res.body.user.roles).toEqual(["athlete"]); }); it("feeds requireRole from a bearer session", async () => { diff --git a/packages/fastify/tests/parity.test.js b/packages/fastify/tests/parity.test.js index e42bc42..025c680 100644 --- a/packages/fastify/tests/parity.test.js +++ b/packages/fastify/tests/parity.test.js @@ -13,6 +13,9 @@ const { default: seamlessAuth, seamlessConsoleProxy } = const { default: createSeamlessAuthServer, createSeamlessConsoleProxy } = await import("../../express/dist/index.js"); +// Unsigned: the adapter only reads the inner token's type, the auth API verifies it. +const ACCESS_TOKEN = `e30.${Buffer.from(JSON.stringify({ typ: "access" })).toString("base64url")}.sig`; + const COOKIE_SECRET = "cookie-secret-cookie-secret-cookie-secret"; const SERVICE_SECRET = "service-secret-service-secret-service-secret"; @@ -57,7 +60,7 @@ function signed(payload, ttl = "300s") { } const accessCookie = () => - `seamless-access=${signed({ sub: "user-123", roles: ["admin"], sessionId: "s-1", token: "access-token" })}`; + `seamless-access=${signed({ sub: "user-123", roles: ["admin"], sessionId: "s-1", token: ACCESS_TOKEN })}`; const preAuthCookie = () => `seamless-ephemeral=${signed({ sub: "user-123", token: "pre-auth" })}`; @@ -501,6 +504,51 @@ describe("fastify and express adapters agree", () => { expect(fastify).toEqual(expressResult); }); + // Every adapter cookie is signed with the same secret, so a pre-auth token + // copied into the access cookie verifies as a cookie. Its type does not. + it("refuses an access cookie that carries a pre-auth token", async () => { + const ephemeral = `e30.${Buffer.from(JSON.stringify({ typ: "ephemeral" })).toString("base64url")}.sig`; + const scenario = { + method: "get", + path: "/users/me", + cookie: `seamless-access=${signed({ sub: "user-123", roles: ["admin"], token: ephemeral })}`, + }; + const { fastify, express: expressResult } = await bothAdapters( + scenario, + upstream(200, { user: { id: "user-123" } }), + ); + + for (const result of [fastify, expressResult]) { + expect(result.status).toBe(401); + } + expect(global.fetch).not.toHaveBeenCalled(); + expect(fastify).toEqual(expressResult); + }); + + it("answers an upstream redirect as a 502 rather than following it", async () => { + const scenario = { + method: "post", + path: "/users/update", + cookie: accessCookie(), + payload: { email: "a@b.c" }, + }; + const { fastify, express: expressResult } = await bothAdapters( + scenario, + new Response(null, { + status: 302, + headers: { location: "https://elsewhere.example.com/users/update" }, + }), + ); + + for (const result of [fastify, expressResult]) { + expect(result.status).toBe(502); + expect(result.body).toEqual({ error: "upstream_redirect" }); + } + expect(global.fetch).toHaveBeenCalledTimes(1); + expect(global.fetch.mock.calls[0][1].redirect).toBe("manual"); + expect(fastify).toEqual(expressResult); + }); + it("sends no identity upstream for the public system config", async () => { // One explicit user agent for both, since light-my-request sends a default // and supertest sends none, and that difference would show up as a header @@ -721,6 +769,33 @@ describe("fastify and express adapters agree", () => { ); }); + it.each([ + ["same-site", "https://evil.example.com", 403], + ["same-site", undefined, 403], + ["same-site", "https://app.example.com", 200], + ["same-origin", "https://other.example.com", 200], + ["cross-site", "https://app.example.com", 403], + ])( + "with allowedOrigins, Sec-Fetch-Site %s from %s answers %i on both", + async (site, origin, status) => { + const scenario = { + method: "post", + path: "/users/update", + cookie: accessCookie(), + payload: { email: "a@b.c" }, + headers: { "sec-fetch-site": site, ...(origin ? { origin } : {}) }, + options: { allowedOrigins: ["https://app.example.com"] }, + }; + const { fastify, express: expressResult } = await bothAdapters( + scenario, + upstream(200, {}), + ); + + expect(fastify.status).toBe(status); + expect(expressResult.status).toBe(status); + }, + ); + it("blocks a cross-site state change the same way", async () => { global.fetch = jest.fn(async () => upstream(200, {})); @@ -743,6 +818,24 @@ describe("fastify and express adapters agree", () => { }); }); +describe("both adapters refuse a placeholder secret at startup", () => { + it.each([ + ["cookieSecret", "dev-only-cookie-signing-key-replace-me"], + ["serviceSecret", "GRAB_FROM_SEAMLESS_AUTH_PORTAL_DEV"], + ])("%s", async (name, value) => { + await expect(buildFastify({ [name]: value })).rejects.toThrow(/published placeholder/); + expect(() => buildExpress({ [name]: value })).toThrow(/published placeholder/); + }); + + it("accepts one in insecure local mode", async () => { + const local = { cookieSecure: false, cookieSecret: "dev-only-cookie-signing-key-replace-me" }; + + const app = await buildFastify(local); + await app.close(); + expect(() => buildExpress(local)).not.toThrow(); + }); +}); + // Verify forwards the pre-auth credential of the browser that opens the link so // the auth API can recognise the device that asked for it. A missing or bad one // never turns into a 401: the link itself is the credential. @@ -911,6 +1004,14 @@ function consoleUpstream(status, body, headers = {}) { }; } +const SECURITY_HEADERS = ["content-security-policy", "x-frame-options", "x-content-type-options"]; + +function pickHeaders(read) { + return Object.fromEntries( + SECURITY_HEADERS.map((name) => [name, read(name)]).filter(([, v]) => v !== undefined), + ); +} + function fetchedUrls() { return global.fetch.mock.calls.map(([url]) => url.toString()); } @@ -935,6 +1036,7 @@ async function viaFastifyConsole({ method, path, headers }) { body: parseBody(res.body), contentType: res.headers["content-type"], cacheControl: res.headers["cache-control"], + security: pickHeaders((name) => res.headers[name]), }; } finally { await app.close(); @@ -960,6 +1062,7 @@ async function viaExpressConsole({ method, path, headers }) { body: parseBody(res.text), contentType: res.headers["content-type"], cacheControl: res.headers["cache-control"], + security: pickHeaders((name) => res.headers[name]), }; } @@ -1055,6 +1158,39 @@ describe("fastify and express console proxies agree", () => { expect(fastify.urls).toEqual(expressResult.urls); }); + it.each([ + [ + "keeps the API's framing and sniffing headers", + { + "content-security-policy": "default-src 'self'; frame-ancestors 'none'", + "x-frame-options": "SAMEORIGIN", + "x-content-type-options": "nosniff", + }, + { + "content-security-policy": "default-src 'self'; frame-ancestors 'none'", + "x-frame-options": "SAMEORIGIN", + "x-content-type-options": "nosniff", + }, + ], + [ + "refuses framing and sniffing when the API sends neither header", + {}, + { "x-frame-options": "DENY", "x-content-type-options": "nosniff" }, + ], + ])("%s", async (_label, upstreamHeaders, expected) => { + const { fastify, express: expressResult } = await bothConsoleAdapters( + { method: "get", path: "/console/settings" }, + async () => + consoleUpstream(200, "", { + "content-type": "text/html; charset=utf-8", + ...upstreamHeaders, + }), + ); + + expect(fastify.security).toEqual(expected); + expect(expressResult.security).toEqual(expected); + }); + // The two frameworks normalize dot-segments at different points, so the status // they answer with differs. What has to hold on both is that nothing outside // the console subtree is ever requested upstream. @@ -1313,7 +1449,7 @@ describe("fastify and express agree on routes served from the manifest", () => { for (const calls of [result.fastifyCalls, result.expressCalls]) { const [, init] = calls.find(([url]) => String(url).endsWith("/registration/phone")); - expect(init.headers.Authorization).toBe("Bearer access-token"); + expect(init.headers.Authorization).toBe(`Bearer ${ACCESS_TOKEN}`); } }); diff --git a/packages/fastify/tests/queryForwarding.test.js b/packages/fastify/tests/queryForwarding.test.js index b6472fe..ffe6598 100644 --- a/packages/fastify/tests/queryForwarding.test.js +++ b/packages/fastify/tests/queryForwarding.test.js @@ -4,6 +4,9 @@ import jwt from "jsonwebtoken"; const { default: seamlessAuth } = await import("../dist/index.js"); +// Unsigned: the adapter only reads the inner token's type, the auth API verifies it. +const ACCESS_TOKEN = `e30.${Buffer.from(JSON.stringify({ typ: "access" })).toString("base64url")}.sig`; + const COOKIE_SECRET = "cookie-secret-cookie-secret-cookie-secret"; function accessCookie() { @@ -12,7 +15,7 @@ function accessCookie() { sub: "user-123", roles: ["admin"], sessionId: "s-1", - token: "access-token", + token: ACCESS_TOKEN, }, COOKIE_SECRET, { algorithm: "HS256", expiresIn: "300s" }, diff --git a/packages/nextjs/README.md b/packages/nextjs/README.md index b323779..2966257 100644 --- a/packages/nextjs/README.md +++ b/packages/nextjs/README.md @@ -48,6 +48,9 @@ origin: `API_SERVICE_TOKEN` and the app from `SEAMLESS_SERVICE_TOKEN`, with the same value. Both are checked when the handler is created, so a weak secret fails at startup rather than on the first request. +A known placeholder (`dev-only-cookie-signing-key-replace-me`, +`GRAB_FROM_SEAMLESS_AUTH_PORTAL_DEV`, or anything containing `replace-me`) is +refused too, unless `cookieSecure` is `false` for local HTTP development. The adapter signs cookies with `jsonwebtoken`, so the route needs the Node.js runtime. That is the default; do not set `export const runtime = "edge"`. @@ -200,7 +203,7 @@ adapters, plus `basePath`. | `cookieDomain` | none | Domain attribute for the auth cookies | | `cookieSecure` | `true` | Set `false` only for local HTTP development | | `cookieSameSite` | `none` when secure, else `lax` | SameSite policy | -| `allowedOrigins` | none | Origin allowlist for browsers without `Sec-Fetch-Site` | +| `allowedOrigins` | none | Origin allowlist for state-changing requests that are `same-site` or come from browsers without `Sec-Fetch-Site` | | `accessCookieName` | `seamless-access` | Session cookie name | | `registrationCookieName` | `seamless-ephemeral` | Registration cookie name | | `refreshCookieName` | `seamless-refresh` | Refresh cookie name | diff --git a/packages/nextjs/src/consoleProxy.ts b/packages/nextjs/src/consoleProxy.ts index 1e056e4..7ad073b 100644 --- a/packages/nextjs/src/consoleProxy.ts +++ b/packages/nextjs/src/consoleProxy.ts @@ -23,8 +23,18 @@ const FORWARDED_RESPONSE_HEADERS = [ "cache-control", "etag", "last-modified", + "content-security-policy", + "x-frame-options", + "x-content-type-options", ]; +// The console runs with an admin session on the app's origin, so it must not be +// framed or sniffed even when the API leaves these off. +const FALLBACK_RESPONSE_HEADERS: Record = { + "x-frame-options": "DENY", + "x-content-type-options": "nosniff", +}; + const NULL_BODY_STATUSES = new Set([101, 204, 205, 304]); const UPSTREAM_TIMEOUT_MS = 10000; @@ -169,6 +179,12 @@ export function createSeamlessConsoleProxy( } } + for (const [header, value] of Object.entries(FALLBACK_RESPONSE_HEADERS)) { + if (!headers.has(header)) { + headers.set(header, value); + } + } + // A body is read in full rather than streamed through: the upstream fetch // is bound by a timeout, and a stream would outlive it. const body = diff --git a/packages/nextjs/src/handler.ts b/packages/nextjs/src/handler.ts index 3d11b05..b8b6d15 100644 --- a/packages/nextjs/src/handler.ts +++ b/packages/nextjs/src/handler.ts @@ -10,6 +10,7 @@ import { DEV_JWKS_KID, ensureCookies, handleManifestRoute, + InvalidPathParamError, matchManifestRoute, proxyRequest, redactSensitiveText, @@ -164,6 +165,10 @@ export function createSeamlessAuthHandler( return collector.json(error.status, { error: error.message }); } + if (error instanceof InvalidPathParamError) { + return collector.json(400, { error: "bad_request" }); + } + console.error( "[SEAMLESS-AUTH-NEXTJS] - Unhandled route error.", redactSensitiveText( diff --git a/packages/nextjs/src/routes/proxyRoutes.ts b/packages/nextjs/src/routes/proxyRoutes.ts index 5ac44f1..bd8eccc 100644 --- a/packages/nextjs/src/routes/proxyRoutes.ts +++ b/packages/nextjs/src/routes/proxyRoutes.ts @@ -1,4 +1,4 @@ -import type { ProxyIdentity } from "@seamless-auth/core"; +import { encodePathParam, type ProxyIdentity } from "@seamless-auth/core"; export interface ProxyRouteDefinition { method: "GET" | "POST" | "PATCH" | "DELETE"; @@ -283,10 +283,10 @@ export function resolveUpstreamPath( return template.replace(/:([A-Za-z0-9_]+)/g, (_match, name: string) => { const value = params[name]; - if (typeof value !== "string" || value === "") { + if (typeof value !== "string") { throw new Error(`Missing route parameter "${name}"`); } - return encodeURIComponent(value); + return encodePathParam(value, name); }); } diff --git a/packages/nextjs/src/session.ts b/packages/nextjs/src/session.ts index bcbeea7..f704d3f 100644 --- a/packages/nextjs/src/session.ts +++ b/packages/nextjs/src/session.ts @@ -122,7 +122,9 @@ export async function getSeamlessSession( cookies: CookieReader, opts: SeamlessSessionOptions, ): Promise { - assertSecretStrength("cookieSecret", opts.cookieSecret); + assertSecretStrength("cookieSecret", opts.cookieSecret, { + allowPlaceholder: true, + }); const { user } = authenticateCookie({ token: accessCookie(cookies, opts), diff --git a/packages/nextjs/tests/handler.test.js b/packages/nextjs/tests/handler.test.js index 8333dfb..1453376 100644 --- a/packages/nextjs/tests/handler.test.js +++ b/packages/nextjs/tests/handler.test.js @@ -7,6 +7,9 @@ import { exportJWK, generateKeyPair, SignJWT } from "jose"; const { createSeamlessAuthHandler } = await import("../dist/index.js"); +// Unsigned: the adapter only reads the inner token's type, the auth API verifies it. +const ACCESS_TOKEN = `e30.${Buffer.from(JSON.stringify({ typ: "access" })).toString("base64url")}.sig`; + const COOKIE_SECRET = "cookie-secret-cookie-secret-cookie-secret"; const SERVICE_SECRET = "service-secret-service-secret-service-secret"; @@ -45,7 +48,7 @@ const signed = (payload, ttl = "300s") => jwt.sign(payload, COOKIE_SECRET, { algorithm: "HS256", expiresIn: ttl }); const accessCookie = () => - `seamless-access=${signed({ sub: "user-123", token: "access-token" })}`; + `seamless-access=${signed({ sub: "user-123", token: ACCESS_TOKEN })}`; const refreshCookie = () => `seamless-refresh=${signed({ sub: "user-123", refreshToken: "opaque" }, "3600s")}`; diff --git a/packages/nextjs/tests/parity.test.js b/packages/nextjs/tests/parity.test.js index 802bd2e..47c4247 100644 --- a/packages/nextjs/tests/parity.test.js +++ b/packages/nextjs/tests/parity.test.js @@ -15,6 +15,9 @@ const { createSeamlessConsoleProxy: createExpressConsoleProxy, } = await import("../../express/dist/index.js"); +// Unsigned: the adapter only reads the inner token's type, the auth API verifies it. +const ACCESS_TOKEN = `e30.${Buffer.from(JSON.stringify({ typ: "access" })).toString("base64url")}.sig`; + const COOKIE_SECRET = "cookie-secret-cookie-secret-cookie-secret"; const SERVICE_SECRET = "service-secret-service-secret-service-secret"; @@ -59,7 +62,7 @@ function signed(payload, ttl = "300s") { } const accessCookie = () => - `seamless-access=${signed({ sub: "user-123", roles: ["admin"], sessionId: "s-1", token: "access-token" })}`; + `seamless-access=${signed({ sub: "user-123", roles: ["admin"], sessionId: "s-1", token: ACCESS_TOKEN })}`; const preAuthCookie = () => `seamless-ephemeral=${signed({ sub: "user-123", token: "pre-auth" })}`; @@ -467,6 +470,51 @@ describe("next.js and express adapters agree", () => { // stale cookie in the path of the one call a signed-out client has to make. // Asserted with a valid cookie present so a future refactor cannot quietly // start attaching one. + // Every adapter cookie is signed with the same secret, so a pre-auth token + // copied into the access cookie verifies as a cookie. Its type does not. + it("refuses an access cookie that carries a pre-auth token", async () => { + const ephemeral = `e30.${Buffer.from(JSON.stringify({ typ: "ephemeral" })).toString("base64url")}.sig`; + const scenario = { + method: "get", + path: "/users/me", + cookie: `seamless-access=${signed({ sub: "user-123", roles: ["admin"], token: ephemeral })}`, + }; + const { next, express: expressResult } = await bothAdapters( + scenario, + upstream(200, { user: { id: "user-123" } }), + ); + + for (const result of [next, expressResult]) { + expect(result.status).toBe(401); + } + expect(global.fetch).not.toHaveBeenCalled(); + expect(next).toEqual(expressResult); + }); + + it("answers an upstream redirect as a 502 rather than following it", async () => { + const scenario = { + method: "post", + path: "/users/update", + cookie: accessCookie(), + payload: { email: "a@b.c" }, + }; + const { next, express: expressResult } = await bothAdapters( + scenario, + new Response(null, { + status: 302, + headers: { location: "https://elsewhere.example.com/users/update" }, + }), + ); + + for (const result of [next, expressResult]) { + expect(result.status).toBe(502); + expect(result.body).toEqual({ error: "upstream_redirect" }); + } + expect(global.fetch).toHaveBeenCalledTimes(1); + expect(global.fetch.mock.calls[0][1].redirect).toBe("manual"); + expect(next).toEqual(expressResult); + }); + it("sends no identity upstream for the public system config", async () => { // One explicit user agent for both, so a default one side sends and the // other does not cannot show up as a header only one adapter forwards. @@ -690,6 +738,33 @@ describe("next.js and express adapters agree", () => { ); }); + it.each([ + ["same-site", "https://evil.example.com", 403], + ["same-site", undefined, 403], + ["same-site", "https://app.example.com", 200], + ["same-origin", "https://other.example.com", 200], + ["cross-site", "https://app.example.com", 403], + ])( + "with allowedOrigins, Sec-Fetch-Site %s from %s answers %i on both", + async (site, origin, status) => { + const scenario = { + method: "post", + path: "/users/update", + cookie: accessCookie(), + payload: { email: "a@b.c" }, + headers: { "sec-fetch-site": site, ...(origin ? { origin } : {}) }, + options: { allowedOrigins: ["https://app.example.com"] }, + }; + const { next, express: expressResult } = await bothAdapters( + scenario, + upstream(200, {}), + ); + + expect(next.status).toBe(status); + expect(expressResult.status).toBe(status); + }, + ); + it("blocks a cross-site state change the same way", async () => { global.fetch = jest.fn(async () => upstream(200, {})); @@ -754,6 +829,25 @@ describe("both adapters forward a magic link destination", () => { }); }); +describe("both adapters refuse a placeholder secret at startup", () => { + it.each([ + ["cookieSecret", "dev-only-cookie-signing-key-replace-me"], + ["serviceSecret", "GRAB_FROM_SEAMLESS_AUTH_PORTAL_DEV"], + ])("%s", (name, value) => { + expect(() => createSeamlessAuthHandler({ ...OPTIONS, [name]: value })).toThrow( + /published placeholder/, + ); + expect(() => buildExpress({ [name]: value })).toThrow(/published placeholder/); + }); + + it("accepts one in insecure local mode", () => { + const local = { cookieSecure: false, cookieSecret: "dev-only-cookie-signing-key-replace-me" }; + + expect(() => createSeamlessAuthHandler({ ...OPTIONS, ...local })).not.toThrow(); + expect(() => buildExpress(local)).not.toThrow(); + }); +}); + // Verify forwards the pre-auth credential of the browser that opens the link so // the auth API can recognise the device that asked for it. A missing or bad one // never turns into a 401: the link itself is the credential. @@ -911,6 +1005,14 @@ function consoleUpstream(status, body, headers = {}) { return new Response(body, { status, headers }); } +const SECURITY_HEADERS = ["content-security-policy", "x-frame-options", "x-content-type-options"]; + +function pickHeaders(read) { + return Object.fromEntries( + SECURITY_HEADERS.map((name) => [name, read(name)]).filter(([, v]) => v !== undefined), + ); +} + function fetchedUrls() { return global.fetch.mock.calls.map(([url]) => url.toString()); } @@ -931,6 +1033,7 @@ async function viaNextConsole({ method, path }) { body: parseBody(await res.text()), contentType: res.headers.get("content-type") ?? undefined, cacheControl: res.headers.get("cache-control") ?? undefined, + security: pickHeaders((name) => res.headers.get(name) ?? undefined), }; } @@ -948,6 +1051,7 @@ async function viaExpressConsole({ method, path }) { body: parseBody(res.text), contentType: res.headers["content-type"], cacheControl: res.headers["cache-control"], + security: pickHeaders((name) => res.headers[name]), }; } @@ -1040,6 +1144,39 @@ describe("next.js and express console proxies agree", () => { expect(next.urls).toEqual(expressResult.urls); }); + it.each([ + [ + "keeps the API's framing and sniffing headers", + { + "content-security-policy": "default-src 'self'; frame-ancestors 'none'", + "x-frame-options": "SAMEORIGIN", + "x-content-type-options": "nosniff", + }, + { + "content-security-policy": "default-src 'self'; frame-ancestors 'none'", + "x-frame-options": "SAMEORIGIN", + "x-content-type-options": "nosniff", + }, + ], + [ + "refuses framing and sniffing when the API sends neither header", + {}, + { "x-frame-options": "DENY", "x-content-type-options": "nosniff" }, + ], + ])("%s", async (_label, upstreamHeaders, expected) => { + const { next, express: expressResult } = await bothConsoleAdapters( + { method: "get", path: "/console/settings" }, + async () => + consoleUpstream(200, "", { + "content-type": "text/html; charset=utf-8", + ...upstreamHeaders, + }), + ); + + expect(next.security).toEqual(expected); + expect(expressResult.security).toEqual(expected); + }); + // Express and the URL parser normalize dot-segments at different points, so // the status each answers with differs. What has to hold on both is that // nothing outside the console subtree is ever requested upstream.