Fix/auth jti suspended apikey lookup

SECURITY_FIXES_AUTH.md

@@ -0,0 +1,43 @@
+# Auth security fixes
+
+Branch: `fix/auth-jti-suspended-apikey-lookup`
+
+## 1. Missing `jti` in signed JWTs (bug)
+`AuthService.signToken()` now includes a unique `jti` (`crypto.randomUUID()`)
+in every issued token. `JwtStrategy.validate()` already fell back to
+`payload.jti ?? payload.sub` for the blacklist/session cache key — without a
+`jti`, every token for a given merchant shared the same key, so revoking one
+session would have blacklisted all of that merchant's sessions/devices at
+once. With a per-token `jti`, blacklist/session-cache keys are now per-token.
+
+## 2. Suspended merchants could still log in
+`AuthService.login()` verified the merchant exists and the password matches,
+but never checked `merchant.status`. A suspended merchant (e.g. flagged for
+fraud/AML) with correct credentials could still obtain a valid access token.
+`login()` now throws `UnauthorizedException('Account suspended')` when
+`status === MerchantStatus.SUSPENDED`.
+
+## 3. Suspended status wasn't re-checked for existing tokens
+Even after fixing (2), a merchant suspended *after* issuing a token could
+keep using that token for its full lifetime. `JwtStrategy.validate()` now
+re-checks `merchant.status` in its DB-fallback path and rejects suspended
+accounts.
+
+## 4. Linear bcrypt scan on every API-key request (perf / DoS)
+`AuthService.findMerchantByApiKey()` loaded every merchant with a non-null
+`apiKeyHash` and ran `bcrypt.compare` in a loop until it found a match. This
+is invoked on every request authenticated via `X-API-Key`
+(`JwtAuthGuard.canActivate()`), making latency scale linearly with merchant
+count and giving any client an easy CPU-exhaustion lever.
+
+Added an indexed `apiKeyLookupHash` column (SHA-256 hex digest of the raw
+key) on `Merchant`. `MerchantsService.generateApiKey()` now populates it
+alongside the existing bcrypt hash. `findMerchantByApiKey()` looks up the
+single candidate row by `apiKeyLookupHash` (O(1), indexed, unique) and only
+then runs one `bcrypt.compare` against that candidate.
+
+Migration: `src/database/migrations/1772300000002-AddMerchantApiKeyLookupHash.ts`.
+
+**Note:** merchants with an API key issued before this migration will need
+to regenerate it — the raw key needed to compute the new lookup hash can't
+be recovered from the existing bcrypt hash.

src/auth/auth.service.ts

@@ -1,8 +1,9 @@
 import { Injectable, UnauthorizedException, ConflictException } from '@nestjs/common';
 import { JwtService } from '@nestjs/jwt';
 import { InjectRepository } from '@nestjs/typeorm';
-import { Repository, Not, IsNull } from 'typeorm';
+import { Repository } from 'typeorm';
 import * as bcrypt from 'bcrypt';
+import { randomUUID, createHash } from 'crypto';
 import { Merchant, MerchantStatus } from '../merchants/entities/merchant.entity';
 import { RegisterDto } from './dto/register.dto';
 import { LoginDto } from './dto/login.dto';
@@ -46,6 +47,10 @@ export class AuthService {
     const valid = await bcrypt.compare(dto.password, merchant.passwordHash);
     if (!valid) throw new UnauthorizedException('Invalid credentials');
 
+    if (merchant.status === MerchantStatus.SUSPENDED) {
+      throw new UnauthorizedException('Account suspended');
+    }
+
     const token = this.signToken(merchant.id, merchant.email, merchant.role);
     return { accessToken: token, merchant };
   }
@@ -60,18 +65,16 @@ export class AuthService {
   }
 
   async findMerchantByApiKey(rawKey: string): Promise<Merchant | null> {
-    const merchants = await this.merchantsRepo.find({
-      where: { apiKeyHash: Not(IsNull()) },
+    const lookupHash = createHash('sha256').update(rawKey).digest('hex');
+    const merchant = await this.merchantsRepo.findOne({
+      where: { apiKeyLookupHash: lookupHash },
     });
-    for (const m of merchants) {
-      if (m.apiKeyHash && (await bcrypt.compare(rawKey, m.apiKeyHash))) {
-        return m;
-      }
-    }
-    return null;
+
+    if (!merchant?.apiKeyHash) return null;
+    return (await bcrypt.compare(rawKey, merchant.apiKeyHash)) ? merchant : null;
   }
 
   private signToken(sub: string, email: string, role?: string): string {
-    return this.jwtService.sign({ sub, email, role });
+    return this.jwtService.sign({ sub, email, role, jti: randomUUID() });
   }
 }

src/auth/strategies/jwt.strategy.ts

@@ -4,7 +4,7 @@ import { ExtractJwt, Strategy } from 'passport-jwt';
 import { ConfigService } from '@nestjs/config';
 import { InjectRepository } from '@nestjs/typeorm';
 import { Repository } from 'typeorm';
-import { Merchant } from '../../merchants/entities/merchant.entity';
+import { Merchant, MerchantStatus } from '../../merchants/entities/merchant.entity';
 import { CacheService } from '../../cache/cache.service';
 import { AuthService } from '../auth.service';
 
@@ -44,6 +44,9 @@ export class JwtStrategy extends PassportStrategy(Strategy) {
     // DB fallback
     const merchant = await this.merchantsRepo.findOne({ where: { id: payload.sub } });
     if (!merchant) throw new UnauthorizedException('Merchant not found');
+    if (merchant.status === MerchantStatus.SUSPENDED) {
+      throw new UnauthorizedException('Account suspended');
+    }
 
     const result = { merchantId: merchant.id, email: merchant.email, role: merchant.role };
 

src/database/migrations/1772300000002-AddMerchantApiKeyLookupHash.ts

@@ -0,0 +1,25 @@
+import { MigrationInterface, QueryRunner } from 'typeorm';
+
+export class AddMerchantApiKeyLookupHash1772300000002 implements MigrationInterface {
+  name = 'AddMerchantApiKeyLookupHash1772300000002';
+
+  public async up(queryRunner: QueryRunner): Promise<void> {
+    await queryRunner.query(
+      `ALTER TABLE "merchants" ADD COLUMN "api_key_lookup_hash" character varying`,
+    );
+    await queryRunner.query(
+      `ALTER TABLE "merchants" ADD CONSTRAINT "UQ_merchants_api_key_lookup_hash" UNIQUE ("api_key_lookup_hash")`,
+    );
+    await queryRunner.query(
+      `CREATE INDEX "IDX_merchants_api_key_lookup_hash" ON "merchants" ("api_key_lookup_hash")`,
+    );
+  }
+
+  public async down(queryRunner: QueryRunner): Promise<void> {
+    await queryRunner.query(`DROP INDEX "IDX_merchants_api_key_lookup_hash"`);
+    await queryRunner.query(
+      `ALTER TABLE "merchants" DROP CONSTRAINT "UQ_merchants_api_key_lookup_hash"`,
+    );
+    await queryRunner.query(`ALTER TABLE "merchants" DROP COLUMN "api_key_lookup_hash"`);
+  }
+}

src/merchants/entities/merchant.entity.ts

@@ -2,6 +2,7 @@ import {
   Entity,
   PrimaryGeneratedColumn,
   Column,
+  Index,
   CreateDateColumn,
   UpdateDateColumn,
   DeleteDateColumn,

src/merchants/merchants.service.ts

@@ -107,6 +107,7 @@ export class MerchantsService {
 
     merchant.apiKey = rawKey.substring(0, 12) + '...';
     merchant.apiKeyHash = hash;
+    merchant.apiKeyLookupHash = crypto.createHash('sha256').update(rawKey).digest('hex');
     merchant.apiKeyScopes = scopes?.length ? scopes : API_KEY_SCOPES;
     await this.merchantsRepo.save(merchant);