Login rejects an unknown username faster than a wrong password, revealing which usernames exist #1734

Activity

1. joshunrau

   added

   BugType: existing behavior is wrongType: existing behavior is wrong

   Priority: MediumReal value; schedule normallyReal value; schedule normally

   Difficulty: LowIsolated change, about 2 hours or lessIsolated change, about 2 hours or less

   Area: APIapps/api (NestJS backend, Prisma models, CASL permissions)apps/api (NestJS backend, Prisma models, CASL permissions)

   SecurityAuthentication, authorization, data exposure or a vulnerable dependencyAuthentication, authorization, data exposure or a vulnerable dependency

   on Oct 7, 2026
2. joshunrau commented on Oct 26, 2026

   

   joshunrau

   on Oct 26, 2026

   CollaboratorAuthor

   More actions

   Still reproduces on main (d86740e). AuthService.login is unchanged since this was filed, and no open PR touches auth.service.ts or users.service.ts. No other unauthenticated route looks up a user: the only @RouteAccess('public') handlers are POST /v1/auth/login and GET/POST /v1/setup, and GET /v1/users/check-username/:username needs read User.

   The dummy hash can be a constant

   comparePassword costs the same for any well-formed stored hash. The iteration count comes from the CryptoService instance (libnest src/modules/crypto/crypto.module.ts:16: 100,000, or 1 under DANGEROUSLY_DISABLE_PBKDF2_ITERATION). The stored value is only hash$salt (crypto.service.ts:20 and :34). So a module constant can replace the cached hashPassword result, with no first-use state:

   const UNKNOWN_USER_PASSWORD_HASH = `${'0'.repeat(128)}$${'0'.repeat(32)}`;

   The $ is required. Without it salt is undefined, Node's pbkdf2 throws ERR_INVALID_ARG_TYPE, and an unknown username gets a 500 instead of a 401. That is a plainer oracle than the timing. libnest 10.0.0, which #1660 moves to, ships the same CryptoService.

   Verify on one path

   Instead of a second comparePassword call in the catch, have the NotFoundException branch set user = null and verify once:

   const isCorrectPassword = await this.cryptoService.comparePassword(
     credentials.password,
     user?.hashedPassword ?? UNKNOWN_USER_PASSWORD_HASH
   );
   if (!user || !isCorrectPassword) {
     throw new UnauthorizedException('Invalid Credentials');
   }

   Then no path reaches the 401 without the derivation.

   A smaller gap remains

   findByUsername runs include: { groups: true } (users.service.ts:159), so a real username also loads its group documents before the password is checked, and a miss loads none. The dummy hash does not remove that difference, and it grows with the group documents (#1534). Closing it means verifying against a lookup without the include and loading the groups only once the password is proven. The fix for #1534 already changes what findByUsername loads for the token, so do it there. Land this issue first, since both edit AuthService.login.

   Tests

   • apps/api/src/auth/__tests__/auth.service.test.ts: add the test the issue names beside the unknown-username test at line 113, asserting that comparePassword runs exactly once for the miss.
   • Same file: it('should answer an unknown username with 401 under a real CryptoService, so a malformed dummy hash cannot turn the miss into a 500'). Build that test's AuthService with new CryptoService({ pbkdf2Params: { iterations: 1 }, secretKey: 'test' }). The rest of the file mocks CryptoService, so nothing else there catches a dummy without the $.
   • e2e: testing/src/specs/auth.spec.ts:99-109 already sends an unknown username through the real API and expects the Unauthorized heading. A 500 fails it, because login.tsx accepts only 200, 401 and 403. Add an API-level test beside it, using request.post as authorization.spec.ts does, asserting that an unknown username and a wrong password for an api.createUser() account both return 401 with the same body.

   _{Triage note written with Claude Code. Code references point at main @ d86740e.}