From 2acea63876a09015c6aa7a84662e94250e8a9d70 Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Thu, 3 Sep 2026 22:34:43 -0400 Subject: [PATCH] fix(login): stop reading a 401 from /login as "no such account" POST /login no longer answers 401. An identifier with no usable account, which used to cover unknown, unverified, and no permitted method, now gets 200 and a decoy pre-auth token so the response cannot be used to test whether an account exists. Two branches in completeLogin read that 401 and are unreachable now: the "not verified yet" message and "No account was found for X". Both go, because there is no longer an answer for them to read. The messages around them were claiming something the CLI can no longer support. "A code was sent to X" becomes "If an account exists for X, a code is on its way", and "This account cannot use email otp login" is phrased as what the instance offered, since that list comes back for an unknown identifier too. An unknown identifier therefore runs the ordinary flow and fails at the code step. That is intended and not something the CLI can shortcut, so the final error names the identifier and points at registering rather than saying only that a code could not be verified. 423 is reported on its own terms with how long to wait, instead of falling through to a bare status. It is also the one answer left that does imply an account exists. Adds a conformance spec pinning the guarantee against a running instance. The loginMethods assertion is deliberately not equality between one account and one decoy: that list is filtered by what an account can do and a decoy's capabilities are derived per identifier, so any two can differ. What must hold is that a real account's list is one a decoy can also produce, and the first draft of the spec asserted equality and failed against a live instance for that reason. --- .changeset/login-no-longer-enumerable.md | 41 ++++++ src/commands/login.test.ts | 6 +- src/core/interactiveLogin.ts | 7 +- src/core/loginFlow.test.ts | 73 +++++++++- src/core/loginFlow.ts | 39 ++++-- verify/harness/api/loginEnumeration.spec.ts | 139 ++++++++++++++++++++ 6 files changed, 284 insertions(+), 21 deletions(-) create mode 100644 .changeset/login-no-longer-enumerable.md create mode 100644 verify/harness/api/loginEnumeration.spec.ts diff --git a/.changeset/login-no-longer-enumerable.md b/.changeset/login-no-longer-enumerable.md new file mode 100644 index 0000000..27eefd3 --- /dev/null +++ b/.changeset/login-no-longer-enumerable.md @@ -0,0 +1,41 @@ +--- +'seamless-cli': patch +--- + +Stop reading a `401` from `/login` as "no such account", and stop claiming an account +exists when the instance will not say. + +`POST /login` no longer answers `401`. An identifier with no usable account, which used +to cover unknown, unverified, and no-permitted-method, now gets `200` and a decoy pre-auth +token so the response cannot be used to test whether an account exists. Two branches in +`completeLogin` read that `401` and are now unreachable: the "not verified yet" message +and "No account was found for X". Both are removed, because there is no longer an answer +for them to read. + +The messages that surrounded them were making a claim the CLI can no longer support. "A +code was sent to X" is now "If an account exists for X, a code is on its way", and "This +account cannot use email otp login" is now phrased as what the instance offered, since +that method list comes back for an unknown identifier too. + +An unknown identifier therefore runs the ordinary flow and fails at the code step. That is +the intended behaviour and not something the CLI can shortcut, so the final error now says +so: it names the identifier and points at registering, instead of "Could not verify a +code" with no explanation of the likeliest reason. + +`423` is now reported on its own terms, with how long to wait when the instance says. +Previously it fell through to "Login request failed (423)". It is also the one answer left +that does imply an account exists, which is a deliberate and documented tradeoff on the +API side. + +Adds `verify/harness/api/loginEnumeration.spec.ts` to the conformance matrix, pinning the +guarantee against a running instance: an unknown identifier gets the same status and the +same fields as a registered one, the same identifier keeps the same subject and the same +method list across attempts, the OTP send reports success and sends nothing, the verify +fails the way a wrong code fails, and a credential id that cannot exist is refused +identically for both. + +The `loginMethods` assertion is the one worth reading. It is deliberately not equality +between one account and one decoy: that list is filtered by what an account can do, and a +decoy's capabilities are derived per identifier, so any two can legitimately differ. What +must hold is that a real account's list is one a decoy can also produce, which is what +makes a narrow list stop being proof of existence. The spec asserts that over a sample. diff --git a/src/commands/login.test.ts b/src/commands/login.test.ts index 1c67a4c..d06098c 100644 --- a/src/commands/login.test.ts +++ b/src/commands/login.test.ts @@ -292,7 +292,11 @@ describe("runLogin: success", () => { const out = logs(); expect(out.some((l) => l.includes("2 attempts left"))).toBe(true); expect(out.some((l) => l.includes("1 attempt left"))).toBe(true); - expect(out.some((l) => l.includes("A code was sent to dev@example.com."))).toBe(true); + expect( + out.some((l) => + l.includes("If an account exists for dev@example.com, a code is on its way."), + ), + ).toBe(true); }); it("validates the code prompt input as letters for an email login", async () => { diff --git a/src/core/interactiveLogin.ts b/src/core/interactiveLogin.ts index a5a39f3..da946f2 100644 --- a/src/core/interactiveLogin.ts +++ b/src/core/interactiveLogin.ts @@ -78,7 +78,12 @@ export async function promptLogin( notify: (event) => { switch (event.type) { case "code_sent": - console.log(kleur.dim(`A code was sent to ${resolved}.`)); + // The instance answers the same way whether or not the identifier has an + // account, so claiming a code was sent would be stating something this cannot + // know. + console.log( + kleur.dim(`If an account exists for ${resolved}, a code is on its way.`), + ); break; case "code_resent": console.log( diff --git a/src/core/loginFlow.test.ts b/src/core/loginFlow.test.ts index 29cd5ee..ca52db2 100644 --- a/src/core/loginFlow.test.ts +++ b/src/core/loginFlow.test.ts @@ -214,9 +214,11 @@ describe("completeLogin", () => { expect(authHeader(verify)).toBe("Bearer e2"); }); - it("rejects an unverified account with a clear message", async () => { + it("reports a locked account with how long to wait", async () => { mockRouter({ - "/login": [() => json({ error: "Login failed. Need to verify." }, 401)], + "/login": [ + () => json({ error: "account_locked", retryAfterSeconds: 900 }, 423), + ], }); await expect( @@ -225,7 +227,21 @@ describe("completeLogin", () => { identifier: "dev@example.com", getCode: async () => "123456", }), - ).rejects.toThrow(/not verified/i); + ).rejects.toThrow(/Too many failed attempts.*15 minute/s); + }); + + it("reports a locked account without a retry hint when none is given", async () => { + mockRouter({ + "/login": [() => json({ error: "account_locked" }, 423)], + }); + + await expect( + completeLogin({ + instanceUrl: INSTANCE, + identifier: "dev@example.com", + getCode: async () => "123456", + }), + ).rejects.toThrow(/Too many failed attempts for dev@example.com\.$/); }); it("rejects when email OTP is not an available login method", async () => { @@ -241,7 +257,9 @@ describe("completeLogin", () => { identifier: "dev@example.com", getCode: async () => "123456", }), - ).rejects.toThrow(/cannot use email otp/i); + // Phrased as what is on offer rather than what "this account" can do: the method + // list comes back for an unknown identifier too. + ).rejects.toThrow(/email otp login is not available for/i); }); it("returns null when the user cancels the code prompt", async () => { @@ -330,9 +348,50 @@ describe("completeLogin", () => { ).rejects.toThrow(/not a valid email or phone number/); }); - it("rejects an unknown account with a 401 that isn't a verify message", async () => { + // An unknown identifier is answered exactly like a real one: 200, a decoy pre-auth + // token, a method list, and an OTP send that reports success without sending. The CLI + // cannot tell the difference and must not pretend to, so the only place this can fail + // is the code step. + it("runs an unknown identifier through the ordinary flow and fails at the code", async () => { + const attempts: number[] = []; + + mockRouter({ + "/login": [ + () => + json({ + message: "Success", + sub: "4f7158fa-ca90-4c22-a1d1-eba3f0c1a2b3", + token: "decoy", + identifierType: "email", + loginMethods: ["email_otp"], + ttl: 900, + }), + ], + "/otp/generate-login-email-otp": [() => json({ message: "success", token: "decoy" })], + "/otp/verify-login-email-otp": [ + () => json({ error: "Not allowed" }, 401), + () => json({ error: "Not allowed" }, 401), + () => json({ error: "Not allowed" }, 401), + ], + }); + + await expect( + completeLogin({ + instanceUrl: INSTANCE, + identifier: "nobody@example.com", + getCode: async ({ attempt }) => { + attempts.push(attempt); + return "ABCDEF"; + }, + }), + ).rejects.toThrow(/If that address or number has no account yet, register it first/); + + expect(attempts).toEqual([1, 2, 3]); + }); + + it("rejects a forbidden login without claiming the account is missing", async () => { mockRouter({ - "/login": [() => json({ error: "No such account" }, 401)], + "/login": [() => json({ error: "Not allowed" }, 403)], }); await expect( @@ -341,7 +400,7 @@ describe("completeLogin", () => { identifier: "dev@example.com", getCode: async () => "123456", }), - ).rejects.toThrow(/No account was found/); + ).rejects.toThrow(/Login is not permitted for dev@example.com/); }); it("maps other login failures to a generic status error", async () => { diff --git a/src/core/loginFlow.ts b/src/core/loginFlow.ts index 6caa6dc..ebd15fe 100644 --- a/src/core/loginFlow.ts +++ b/src/core/loginFlow.ts @@ -107,22 +107,31 @@ async function startLogin( ); } + // `/login` no longer answers 401. An identifier with no usable account, which used to + // mean unknown, unverified, or with no permitted method, now gets a 200 and a decoy + // pre-auth token so the response cannot be used to test whether an account exists. The + // branches that read 401 as "no such user" and "not verified yet" were removed with it: + // there is no longer an answer for them to read. Such a login fails at the code step + // instead, which is what `completeLogin` reports. if (!res.ok) { - const message = apiMessage(res.data) ?? ""; - if (res.status === 401 && /verify/i.test(message)) { - throw new LoginError( - `The account for ${identifier} is not verified yet. Finish registration, then log in.`, - ); - } if (res.status === 400) { throw new LoginError( `"${identifier}" is not a valid email or phone number.`, ); } - if (res.status === 401 || res.status === 403) { - throw new LoginError( - `No account was found for ${identifier}, or login is not permitted.`, - ); + if (res.status === 423) { + // The one remaining answer that does imply an account, and the one worth naming: + // it needs prior failed attempts against this identifier, and the developer can + // act on it by waiting. + const retryAfter = res.data?.retryAfterSeconds; + const wait = + typeof retryAfter === "number" && retryAfter > 0 + ? ` Try again in about ${Math.ceil(retryAfter / 60)} minute(s).` + : ""; + throw new LoginError(`Too many failed attempts for ${identifier}.${wait}`); + } + if (res.status === 403) { + throw new LoginError(`Login is not permitted for ${identifier}.`); } throw new LoginError(`Login request failed (${res.status}).`); } @@ -233,8 +242,10 @@ export async function completeLogin( const channel = started.channel; const required = channel === "email" ? "email_otp" : "phone_otp"; if (started.loginMethods.length > 0 && !started.loginMethods.includes(required)) { + // Deliberately not "this account cannot": the method list comes back for an unknown + // identifier too, so saying so would report an account that may not exist. throw new LoginError( - `This account cannot use ${required.replace("_", " ")} login. Available methods: ${started.loginMethods.join(", ")}.`, + `${required.replace("_", " ")} login is not available for ${opts.identifier}. Offered: ${started.loginMethods.join(", ")}.`, ); } @@ -294,5 +305,9 @@ export async function completeLogin( notify({ type: "incorrect", attemptsLeft: maxAttempts - attempt }); } - throw new LoginError("Could not verify a code. Run seamless login to try again."); + // The instance does not say whether the identifier has an account, so neither can this. + // A wrong code and an identifier nobody has registered both land here. + throw new LoginError( + `Could not verify a code for ${opts.identifier}. If that address or number has no account yet, register it first. Otherwise run seamless login to try again.`, + ); } diff --git a/verify/harness/api/loginEnumeration.spec.ts b/verify/harness/api/loginEnumeration.spec.ts new file mode 100644 index 0000000..0759691 --- /dev/null +++ b/verify/harness/api/loginEnumeration.spec.ts @@ -0,0 +1,139 @@ +import { expect, test } from '../lib/fixtures'; +import { registerAndVerifyEmail } from '../lib/flows'; + +// `POST /login` answers the same way for an identifier with an account and one without, +// and the endpoints that accept the resulting pre-auth token do too. The API has its own +// unit coverage for this; what only this harness can check is that the guarantee survives +// a real instance, with real signing keys and the real login policy in place. +// +// The specific failures worth catching are the ones where a decoy responder reproduces +// the success path and forgets a refusal, since that is the shape every regression here +// has taken so far. + +const UNKNOWN = () => `nobody-${Date.now()}-${Math.random().toString(36).slice(2)}@example.com`; + +async function startLogin(ctx: Parameters[0], identifier: string) { + const res = await ctx.post('/login', { data: { identifier } }); + return { status: res.status(), body: await res.json() }; +} + +test.describe('login enumeration (api)', () => { + test('an unknown identifier is answered in the same shape as a real one', async ({ + actor, + }) => { + await registerAndVerifyEmail(actor.ctx, actor.email); + + const real = await startLogin(actor.ctx, actor.email); + const unknown = await startLogin(actor.ctx, UNKNOWN()); + + expect(unknown.status).toBe(200); + expect(unknown.status).toBe(real.status); + // `sub` and `token` differ between them exactly as they differ between two real + // accounts, so the comparison is over everything else. + expect(Object.keys(unknown.body).sort()).toEqual(Object.keys(real.body).sort()); + expect(unknown.body.identifierType).toBe(real.body.identifierType); + expect(unknown.body.ttl).toBe(real.body.ttl); + expect(typeof unknown.body.token).toBe('string'); + // Deliberately not `loginMethods`. That list is filtered by what an account can do, + // and a decoy's capabilities are derived per identifier, so any one decoy and any + // one account can legitimately differ. What has to hold is the next test. + }); + + test('a real account\'s method list is one a decoy can also produce', async ({ actor }) => { + // The guarantee is not that two given answers match, it is that a given answer does + // not identify an account. A decoy that always claimed everything would make any + // narrower list proof of existence, so the decoy's passkey and phone are derived per + // identifier and a narrow list has to be reachable without an account behind it. + await registerAndVerifyEmail(actor.ctx, actor.email); + const real = await startLogin(actor.ctx, actor.email); + const target = JSON.stringify(real.body.loginMethods); + + const seen = new Set(); + for (let i = 0; i < 40; i += 1) { + const { body } = await startLogin(actor.ctx, UNKNOWN()); + seen.add(JSON.stringify(body.loginMethods)); + } + + // Each decoy draws two independent bits, so this account's exact list comes up about + // a quarter of the time; over 40 identifiers, missing it entirely is a 1-in-100,000 + // event rather than a flake worth retrying. + expect(seen.has(target), `no decoy offered ${target}; saw ${[...seen].join(' | ')}`).toBe( + true, + ); + expect(seen.size).toBeGreaterThan(1); + }); + + test('the same unknown identifier keeps the same subject', async ({ actor }) => { + const identifier = UNKNOWN(); + + const first = await startLogin(actor.ctx, identifier); + const second = await startLogin(actor.ctx, identifier); + const other = await startLogin(actor.ctx, UNKNOWN()); + + // A real identifier resolves to the same row every time and to a different one from + // anyone else's. A subject that rerolled, or that collided, would be the oracle again + // one request later. + expect(second.body.sub).toBe(first.body.sub); + expect(other.body.sub).not.toBe(first.body.sub); + // The offered methods have to be stable for the same reason: a list that changed + // between attempts would separate a decoy from an account on its own. + expect(second.body.loginMethods).toEqual(first.body.loginMethods); + }); + + test('an unknown identifier gets an OTP send that reports success and sends nothing', async ({ + actor, + }) => { + const { body } = await startLogin(actor.ctx, UNKNOWN()); + + const res = await actor.ctx.get('/otp/generate-login-email-otp', { + headers: { Authorization: `Bearer ${body.token}` }, + }); + + expect(res.status()).toBe(200); + expect((await res.json()).message).toBe('success'); + }); + + test('a decoy OTP verify fails the way a wrong code fails', async ({ actor }) => { + const { body } = await startLogin(actor.ctx, UNKNOWN()); + + const res = await actor.ctx.post('/otp/verify-login-email-otp', { + headers: { Authorization: `Bearer ${body.token}` }, + data: { verificationToken: 'ZZZZZZ' }, + }); + + expect(res.status()).toBe(401); + }); + + test('a credential id that cannot exist is refused for real and unknown alike', async ({ + actor, + }) => { + // The sharpest oracle this surface had. `/webauthn/login/start` filters the account's + // credentials by the requested id and refuses when none survive, so an id no + // credential can hold is refused by every real account. A decoy that answered with a + // challenge anyway was identifiable in two requests, whatever the policy. + await registerAndVerifyEmail(actor.ctx, actor.email); + + const real = await startLogin(actor.ctx, actor.email); + const unknown = await startLogin(actor.ctx, UNKNOWN()); + + const ask = (token: string) => + actor.ctx.post('/webauthn/login/start', { + headers: { Authorization: `Bearer ${token}` }, + data: { credentialId: 'not-a-real-credential-id' }, + }); + + const realRes = await ask(real.body.token); + const unknownRes = await ask(unknown.body.token); + + expect(unknownRes.status()).toBe(realRes.status()); + expect(await unknownRes.text()).toBe(await realRes.text()); + }); + + test('a malformed identifier is still rejected', async ({ actor }) => { + // Not an enumeration signal: it does not depend on whether an account exists, and + // answering 200 here would leave a caller with no way to learn it typed nonsense. + const { status } = await startLogin(actor.ctx, 'not-an-identifier'); + + expect(status).toBe(400); + }); +});