[WIP] Feat/p1 security hardening

backend/src/auth/auth.module.ts

@@ -6,6 +6,7 @@ import { AuthentikProxyStrategy } from './strategies/authentik-proxy.strategy';
 import { JwtStrategy } from './strategies/jwt.strategy';
 import { ApiKeyStrategy } from './strategies/api-key.strategy';
 import { ServiceKeyStrategy } from './strategies/service-key.strategy';
+import { ApiKeySecurityService } from './services/api-key-security.service';
 import { TypeOrmModule } from '@nestjs/typeorm';
 import { UserApiKey } from './entities/user-api-key.entity';
 import { ServiceApiKey } from './entities/service-api-key.entity';
@@ -23,6 +24,7 @@ import { BillingModule } from '../billing/billing.module';
   controllers: [AuthController],
   providers: [
     AuthService,
+    ApiKeySecurityService,
     AuthentikProxyStrategy,
     JwtStrategy,
     ApiKeyStrategy,

backend/src/auth/auth.service.spec.ts

@@ -11,6 +11,7 @@ import { Domain } from '../domains/entities/domain.entity';
 import { TlsCrt } from '../certs/tls/entities/tls-crt.entity';
 import { BillingService } from '../billing/billing.service';
 import { EmailService } from '../notifications/email.service';
+import { ApiKeySecurityService } from './services/api-key-security.service';
 
 const HMAC_SECRET = 'test-hmac-secret';
 
@@ -38,6 +39,11 @@ describe('AuthService', () => {
     count: jest.Mock;
   };
   let mockUserRepo: { findOne: jest.Mock; create: jest.Mock; save: jest.Mock };
+  let mockEmailService: {
+    sendWelcome: jest.Mock;
+    sendApiKeyExpiredUse: jest.Mock;
+  };
+  let mockApiKeySecurity: { shouldNotifyExpiredKeyUse: jest.Mock };
 
   beforeEach(async () => {
     mockUserApiKeyRepo = {
@@ -53,6 +59,13 @@ describe('AuthService', () => {
       create: jest.fn(),
       save: jest.fn(),
     };
+    mockEmailService = {
+      sendWelcome: jest.fn().mockResolvedValue(undefined),
+      sendApiKeyExpiredUse: jest.fn().mockResolvedValue(undefined),
+    };
+    mockApiKeySecurity = {
+      shouldNotifyExpiredKeyUse: jest.fn().mockResolvedValue(true),
+    };
 
     const module: TestingModule = await Test.createTestingModule({
       providers: [
@@ -76,9 +89,11 @@ describe('AuthService', () => {
         { provide: getRepositoryToken(TlsCrt), useValue: { count: jest.fn() } },
         {
           provide: EmailService,
-          useValue: {
-            sendWelcome: jest.fn().mockResolvedValue(undefined),
-          },
+          useValue: mockEmailService,
+        },
+        {
+          provide: ApiKeySecurityService,
+          useValue: mockApiKeySecurity,
         },
         {
           provide: BillingService,
@@ -396,6 +411,61 @@ describe('AuthService', () => {
 
       expect(await service.validateApiKey('kk_nonexistent')).toBeNull();
     });
+
+    it('returns null for an expired key and notifies the owner', async () => {
+      const record = {
+        id: 'key-1',
+        name: 'ci-deploy',
+        hash: 'h',
+        expiresAt: new Date(Date.now() - 1000),
+        user: { id: 'user-1', username: 'luke', email: 'luke@example.com' },
+      };
+      mockUserApiKeyRepo.findOne.mockResolvedValue(record);
+
+      expect(
+        await service.validateApiKey('kk_expired', { ip: '10.0.0.1' }),
+      ).toBeNull();
+      expect(mockApiKeySecurity.shouldNotifyExpiredKeyUse).toHaveBeenCalledWith(
+        'key-1',
+      );
+      expect(mockEmailService.sendApiKeyExpiredUse).toHaveBeenCalledWith({
+        userId: 'user-1',
+        username: 'luke',
+        email: 'luke@example.com',
+        keyId: 'key-1',
+        keyName: 'ci-deploy',
+        ip: '10.0.0.1',
+      });
+    });
+
+    it('rate-limits expired-key notifications via the security service', async () => {
+      mockApiKeySecurity.shouldNotifyExpiredKeyUse.mockResolvedValue(false);
+      mockUserApiKeyRepo.findOne.mockResolvedValue({
+        id: 'key-1',
+        name: 'ci-deploy',
+        hash: 'h',
+        expiresAt: new Date(Date.now() - 1000),
+        user: { id: 'user-1', username: 'luke', email: 'luke@example.com' },
+      });
+
+      expect(await service.validateApiKey('kk_expired')).toBeNull();
+      expect(mockEmailService.sendApiKeyExpiredUse).not.toHaveBeenCalled();
+    });
+
+    it('still returns null when the expired-key notification fails', async () => {
+      mockApiKeySecurity.shouldNotifyExpiredKeyUse.mockRejectedValue(
+        new Error('redis down'),
+      );
+      mockUserApiKeyRepo.findOne.mockResolvedValue({
+        id: 'key-1',
+        name: 'ci-deploy',
+        hash: 'h',
+        expiresAt: new Date(Date.now() - 1000),
+        user: { id: 'user-1', username: 'luke', email: 'luke@example.com' },
+      });
+
+      expect(await service.validateApiKey('kk_expired')).toBeNull();
+    });
   });
 
   // ---------------------------------------------------------------------------

backend/src/auth/auth.service.ts

@@ -19,6 +19,7 @@ import { UpdateProfileDto } from './dto/update-profile.dto';
 import { BillingService } from '../billing/billing.service';
 import { PLAN_LIMITS } from '../billing/constants/plan-limits';
 import { EmailService } from '../notifications/email.service';
+import { ApiKeySecurityService } from './services/api-key-security.service';
 import type {
   ApiKey,
   AuthCallbackResponse,
@@ -45,6 +46,7 @@ export class AuthService implements OnModuleInit {
     private readonly tlsCrtRepo: Repository<TlsCrt>,
     private readonly billingService: BillingService,
     private readonly emailService: EmailService,
+    private readonly apiKeySecurity: ApiKeySecurityService,
   ) {}
 
   async onModuleInit() {
@@ -321,18 +323,55 @@ export class AuthService implements OnModuleInit {
    * Validates an API key by hashing and looking up in the database.
    *
    * Returns the UserApiKey record with user relation if valid, null otherwise.
+   * Use of a correct-but-expired key triggers an owner notification (the only
+   * failure mode attributable to a specific key), rate-limited per key.
    */
-  async validateApiKey(rawKey: string) {
+  async validateApiKey(rawKey: string, meta?: { ip?: string }) {
     const hash = this.hashKey(rawKey);
     const record = await this.userApiKeyRepo.findOne({
       where: { hash },
       relations: ['user'],
     });
     if (!record) return null;
-    if (record.expiresAt && record.expiresAt < new Date()) return null;
+    if (record.expiresAt && record.expiresAt < new Date()) {
+      await this.notifyExpiredKeyUse(record, meta?.ip);
+      return null;
+    }
     return record;
   }
 
+  /**
+   * Notifies a key owner that their expired key was presented for auth —
+   * a signal the key is still deployed somewhere (or leaked). Never throws.
+   */
+  private async notifyExpiredKeyUse(
+    record: UserApiKey,
+    ip?: string,
+  ): Promise<void> {
+    try {
+      if (!record.user) return;
+      if (!(await this.apiKeySecurity.shouldNotifyExpiredKeyUse(record.id))) {
+        return;
+      }
+      this.logger.warn(
+        `Expired API key ${record.id} ("${record.name}") presented for user ${record.user.id}${ip ? ` from ${ip}` : ''}`,
+      );
+      await this.emailService.sendApiKeyExpiredUse({
+        userId: record.user.id,
+        username: record.user.username,
+        email: record.user.email,
+        keyId: record.id,
+        keyName: record.name,
+        ip,
+      });
+    } catch (err) {
+      this.logger.error(
+        'Failed to send expired-key-use notification',
+        err instanceof Error ? err.stack : err,
+      );
+    }
+  }
+
   // --- Profile Management ---
 
   async getFullProfile(userId: string): Promise<UserProfile> {

backend/src/auth/services/api-key-security.service.spec.ts

@@ -0,0 +1,102 @@
+import { ConfigService } from '@nestjs/config';
+import { ApiKeySecurityService } from './api-key-security.service';
+
+describe('ApiKeySecurityService', () => {
+  let service: ApiKeySecurityService;
+  let mockRedis: Record<string, jest.Mock>;
+
+  const configService = {
+    get: jest.fn((key: string, def?: string) => def),
+  } as unknown as ConfigService;
+
+  beforeEach(() => {
+    service = new ApiKeySecurityService(configService);
+    mockRedis = {
+      exists: jest.fn().mockResolvedValue(0),
+      incr: jest.fn().mockResolvedValue(1),
+      expire: jest.fn().mockResolvedValue(1),
+      set: jest.fn().mockResolvedValue('OK'),
+      disconnect: jest.fn(),
+    };
+    // Replace the lazy-connecting real client with a mock.
+    (service as any).redis.disconnect();
+    (service as any).redis = mockRedis;
+  });
+
+  describe('isLockedOut', () => {
+    it('returns false when no lock exists', async () => {
+      expect(await service.isLockedOut('1.2.3.4')).toBe(false);
+      expect(mockRedis.exists).toHaveBeenCalledWith('lock:1.2.3.4');
+    });
+
+    it('returns true when a lock exists', async () => {
+      mockRedis.exists.mockResolvedValue(1);
+      expect(await service.isLockedOut('1.2.3.4')).toBe(true);
+    });
+
+    it('returns false for empty IP without touching Redis', async () => {
+      expect(await service.isLockedOut('')).toBe(false);
+      expect(mockRedis.exists).not.toHaveBeenCalled();
+    });
+
+    it('fails open on Redis errors', async () => {
+      mockRedis.exists.mockRejectedValue(new Error('down'));
+      expect(await service.isLockedOut('1.2.3.4')).toBe(false);
+    });
+  });
+
+  describe('recordFailure', () => {
+    it('increments the counter and sets the window on first failure', async () => {
+      await service.recordFailure('1.2.3.4');
+      expect(mockRedis.incr).toHaveBeenCalledWith('fail:1.2.3.4');
+      expect(mockRedis.expire).toHaveBeenCalledWith('fail:1.2.3.4', 900);
+      expect(mockRedis.set).not.toHaveBeenCalled();
+    });
+
+    it('does not re-arm the window on subsequent failures', async () => {
+      mockRedis.incr.mockResolvedValue(5);
+      await service.recordFailure('1.2.3.4');
+      expect(mockRedis.expire).not.toHaveBeenCalled();
+    });
+
+    it('activates a lockout at the threshold', async () => {
+      mockRedis.incr.mockResolvedValue(10);
+      await service.recordFailure('1.2.3.4');
+      expect(mockRedis.set).toHaveBeenCalledWith(
+        'lock:1.2.3.4',
+        '1',
+        'EX',
+        900,
+        'NX',
+      );
+    });
+
+    it('swallows Redis errors (fail open)', async () => {
+      mockRedis.incr.mockRejectedValue(new Error('down'));
+      await expect(service.recordFailure('1.2.3.4')).resolves.toBeUndefined();
+    });
+  });
+
+  describe('shouldNotifyExpiredKeyUse', () => {
+    it('returns true on first use within the dedup window', async () => {
+      expect(await service.shouldNotifyExpiredKeyUse('key-1')).toBe(true);
+      expect(mockRedis.set).toHaveBeenCalledWith(
+        'expired-notice:key-1',
+        '1',
+        'EX',
+        86_400,
+        'NX',
+      );
+    });
+
+    it('returns false when already notified', async () => {
+      mockRedis.set.mockResolvedValue(null);
+      expect(await service.shouldNotifyExpiredKeyUse('key-1')).toBe(false);
+    });
+
+    it('fails closed (no notification) on Redis errors', async () => {
+      mockRedis.set.mockRejectedValue(new Error('down'));
+      expect(await service.shouldNotifyExpiredKeyUse('key-1')).toBe(false);
+    });
+  });
+});