diff --git a/.changeset/passkey-enrollment-access-identity.md b/.changeset/passkey-enrollment-access-identity.md new file mode 100644 index 0000000..e8f428e --- /dev/null +++ b/.changeset/passkey-enrollment-access-identity.md @@ -0,0 +1,39 @@ +--- +'@seamless-auth/core': minor +'@seamless-auth/express': minor +'@seamless-auth/fastify': minor +--- + +Passkey enrollment now forwards the access session. This is a coordinated contract +change with `seamless-auth-api`, which refuses a pre-auth token on these routes as of the +matching release. + +`/webAuthn/register/start` and `/webAuthn/register/finish` read the registration cookie +and sent the ephemeral token upstream. The auth API mints one of those for an account +that already exists, from an email address alone, so anyone who knew an address could +enroll a credential against the account and sign in as its owner. Both routes now read +the access cookie and forward the access token. `/webAuthn/login/start` and +`/webAuthn/login/finish` are unchanged and still take the pre-auth cookie, because +authenticating is what they are for. + +No shipped flow loses a step. Registration proves an address with an email OTP, and +verifying it issues a session, so the client holds an access cookie by the time it offers +a passkey. An application that offered enrollment before verifying an address has to move +that step after it. + +Upgrade this and the auth API together. There is no safe order between them: an older +adapter sends the token the new API refuses, and this release sends one an older API +refuses, so enrollment answers `401` until both sides land. + +`finishRegisterHandler` no longer issues session cookies. Enrolling a passkey is not a +sign-in, and the caller now arrives holding a session, so minting a second one left the +first live and unrevoked while counting against the API's concurrent session limit, which +can evict the user's other devices. The route still answers `204`. + +`FinishRegisterOptions` drops `audience`, `cookieDomain`, `accessCookieName` and +`refreshCookieName`, and `FinishRegisterResult` drops `setCookies`. Only the two adapters +in this workspace passed them, and both are updated. Code calling +`finishRegisterHandler` directly should pass `{ authServerUrl }` alone. + +This does not change the pre-auth routes' silent refresh, which still mints an access +token into a cookie those routes cannot use. That is tracked separately. diff --git a/packages/core/src/ensureCookies.ts b/packages/core/src/ensureCookies.ts index 56f227e..245ebdc 100644 --- a/packages/core/src/ensureCookies.ts +++ b/packages/core/src/ensureCookies.ts @@ -60,12 +60,18 @@ const COOKIE_REQUIREMENTS: Record< > = { "/webAuthn/login/finish": { name: "preAuthCookieName", required: true }, "/webAuthn/login/start": { name: "preAuthCookieName", required: true }, + // Enrollment takes the access cookie, not the registration one. The auth + // API refuses an ephemeral token here: it mints one for an account that + // already exists from an email address alone, so accepting it let anyone + // who knew an address enroll a credential and take the account over. + // Signup reaches this point holding a session already, because verifying + // the email OTP issues one. "/webAuthn/register/start": { - name: "registrationCookieName", + name: "accessCookieName", required: true, }, "/webAuthn/register/finish": { - name: "registrationCookieName", + name: "accessCookieName", required: true, }, "/otp/verify-email-otp": { diff --git a/packages/core/src/handlers/finishRegister.ts b/packages/core/src/handlers/finishRegister.ts index 83c653b..a3c8178 100644 --- a/packages/core/src/handlers/finishRegister.ts +++ b/packages/core/src/handlers/finishRegister.ts @@ -1,8 +1,6 @@ import { authFetch } from "../authFetch.js"; -import { issueSessionCookies } from "../upstreamSession.js"; import { readPassthroughFailure } from "../upstreamError.js"; import type { ResultFailure } from "../result.js"; -import type { CookiePayload } from "../ensureCookies.js"; export interface FinishRegisterInput { authorization?: string; @@ -14,20 +12,10 @@ export interface FinishRegisterInput { export interface FinishRegisterOptions { authServerUrl: string; - audience: string; - cookieDomain?: string; - accessCookieName: string; - refreshCookieName: string; } export interface FinishRegisterResult extends ResultFailure { status: number; - setCookies?: { - name: string; - value: CookiePayload; - ttl: number; - domain?: string; - }[]; } export async function finishRegisterHandler( @@ -52,14 +40,10 @@ export async function finishRegisterHandler( }; } - return { - status: 204, - setCookies: await issueSessionCookies(data, { - authServerUrl: opts.authServerUrl, - audience: opts.audience, - accessCookieName: opts.accessCookieName, - refreshCookieName: opts.refreshCookieName, - cookieDomain: opts.cookieDomain, - }), - }; + // Enrolling a passkey is not a sign-in. This route takes the access cookie, + // so the caller already holds a session, and issuing a second one left the + // first live and unrevoked while counting against the API's concurrent + // session limit, which can evict the user's other devices. The auth API + // stopped returning tokens here for the same reason. + return { status: 204 }; } diff --git a/packages/express/src/createServer.ts b/packages/express/src/createServer.ts index 349d1d5..a8c85e1 100644 --- a/packages/express/src/createServer.ts +++ b/packages/express/src/createServer.ts @@ -307,7 +307,7 @@ export function createSeamlessAuthServer( r.get( "/webAuthn/register/start", - proxyWithIdentity("webAuthn/register/start", "preAuth", "GET"), + proxyWithIdentity("webAuthn/register/start", "access", "GET"), ); r.post("/webAuthn/register/finish", (req, res) => finishRegister(req, res, resolvedOpts), diff --git a/packages/express/src/handlers/finishRegister.ts b/packages/express/src/handlers/finishRegister.ts index 9212304..fb9e751 100644 --- a/packages/express/src/handlers/finishRegister.ts +++ b/packages/express/src/handlers/finishRegister.ts @@ -22,13 +22,7 @@ export async function finishRegister( serviceAuthorization: buildProxyServiceAuthorization(opts), forwardedClientIp: buildForwardedClientIp(req, opts.resolveClientIp), }, - { - authServerUrl: opts.authServerUrl, - audience: opts.audience, - cookieDomain: opts.cookieDomain, - accessCookieName: opts.accessCookieName!, - refreshCookieName: opts.refreshCookieName!, - }, + { authServerUrl: opts.authServerUrl }, ); respond(res, { ...result, body: { message: "success" } }, opts); diff --git a/packages/express/tests/stepUpProxy.test.js b/packages/express/tests/stepUpProxy.test.js index 4d7a00a..2c6a5ac 100644 --- a/packages/express/tests/stepUpProxy.test.js +++ b/packages/express/tests/stepUpProxy.test.js @@ -189,14 +189,39 @@ describe("step-up proxy routes", () => { const res = await request(createApp()) .get("/auth/webAuthn/register/start") .query({ requirePrf: "true" }) - .set("Cookie", createRegistrationCookie()); + .set("Cookie", createAccessCookie()); expect(res.status).toBe(200); expect(global.fetch).toHaveBeenCalledWith( "https://auth.example.com/webAuthn/register/start?requirePrf=true", expect.objectContaining({ method: "GET", + headers: expect.objectContaining({ + Authorization: "Bearer access-token", + }), }), ); }); + + // The auth API refuses an ephemeral token at enrollment: it mints one for + // an account that already exists from an email address alone, so forwarding + // it would offer anyone who knew an address a credential on that account. + it("refuses passkey registration start on a registration cookie", async () => { + const res = await request(createApp()) + .get("/auth/webAuthn/register/start") + .set("Cookie", createRegistrationCookie()); + + expect(res.status).toBe(401); + expect(global.fetch).not.toHaveBeenCalled(); + }); + + it("refuses passkey registration finish on a registration cookie", async () => { + const res = await request(createApp()) + .post("/auth/webAuthn/register/finish") + .set("Cookie", createRegistrationCookie()) + .send({ attestationResponse: {} }); + + expect(res.status).toBe(401); + expect(global.fetch).not.toHaveBeenCalled(); + }); }); diff --git a/packages/fastify/src/routes/authRoutes.ts b/packages/fastify/src/routes/authRoutes.ts index 13086f5..2cf9927 100644 --- a/packages/fastify/src/routes/authRoutes.ts +++ b/packages/fastify/src/routes/authRoutes.ts @@ -120,7 +120,7 @@ export function registerAuthRoutes( serviceAuthorization: buildProxyServiceAuthorization(opts), forwardedClientIp: buildForwardedClientIp(req, opts.resolveClientIp), }, - { authServerUrl: opts.authServerUrl, ...sessionCookies }, + { authServerUrl: opts.authServerUrl }, ); respond(reply, { ...result, body: { message: "success" } }, opts); diff --git a/packages/fastify/src/routes/proxyRoutes.ts b/packages/fastify/src/routes/proxyRoutes.ts index fd5048b..d3e278d 100644 --- a/packages/fastify/src/routes/proxyRoutes.ts +++ b/packages/fastify/src/routes/proxyRoutes.ts @@ -30,7 +30,7 @@ export const PROXY_ROUTES: ProxyRouteDefinition[] = [ method: "GET", path: "/webAuthn/register/start", upstream: "webAuthn/register/start", - identity: "preAuth", + identity: "access", }, { diff --git a/packages/fastify/tests/parity.test.js b/packages/fastify/tests/parity.test.js index 8714e15..2b835f1 100644 --- a/packages/fastify/tests/parity.test.js +++ b/packages/fastify/tests/parity.test.js @@ -252,6 +252,28 @@ describe("fastify and express adapters agree", () => { { method: "get", path: "/system-config/public" }, upstream(503, { error: "upstream_unavailable" }), ], + [ + "passkey enrollment start on an access session", + { + method: "get", + path: "/webAuthn/register/start", + cookie: accessCookie(), + }, + upstream(200, { challenge: "challenge" }), + ], + // Enrollment moved off the pre-auth cookie because the auth API mints + // one for an account that already exists from an email address alone. + // Both adapters have to refuse it, or the one that does not hands the + // account over. + [ + "passkey enrollment start refuses a pre-auth session", + { + method: "get", + path: "/webAuthn/register/start", + cookie: preAuthCookie(), + }, + upstream(200, { challenge: "challenge" }), + ], ])("%s", async (_label, scenario, upstreamResponse) => { const { fastify, express: expressResult } = await bothAdapters( scenario, @@ -268,12 +290,27 @@ describe("fastify and express adapters agree", () => { // itself is the contract a consumer reads to tell "sign in again" from "that // request was not understood", so it is pinned here by value. it.each([ - ["access-gated", { method: "get", path: "/organizations" }], [ - "pre-auth gated", + "an access-gated route with no session", + { method: "get", path: "/organizations" }, + ], + [ + "a pre-auth gated route with no session", { method: "post", path: "/webAuthn/login/start", payload: {} }, ], - ])("answers 401 on a %s route with no session, and asks upstream nothing", async ( + [ + "an enrollment route with no session", + { method: "get", path: "/webAuthn/register/start" }, + ], + [ + "an enrollment route holding only a pre-auth session", + { + method: "get", + path: "/webAuthn/register/start", + cookie: preAuthCookie(), + }, + ], + ])("answers 401 on %s, and asks upstream nothing", async ( _label, scenario, ) => {