fix(login): stop reading a 401 from /login as "no such account"

.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.

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 () => {

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(

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 () => {

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.`,
+  );
 }

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<typeof registerAndVerifyEmail>[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<string>();
+    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);
+  });
+});