From 5f23b82b21b78d42de51a8d8c445fd1e352d4ca7 Mon Sep 17 00:00:00 2001 From: Godfr3y Date: Thu, 27 Aug 2026 02:29:55 +0100 Subject: [PATCH] critical: admin audit-log endpoints have no role gate --- context/progress-tracker.md | 7 + src/modules/admin/admin-roles.controller.ts | 2 +- src/modules/admin/admin.guard.ts | 123 ++++++++++++++++++ src/modules/admin/admin.module.ts | 4 +- src/modules/admin/audit.controller.ts | 3 +- test/unit/modules/admin/admin.guard.spec.ts | 113 ++++++++++++++++ .../modules/admin/audit.controller.spec.ts | 47 +++++++ 7 files changed, 295 insertions(+), 4 deletions(-) create mode 100644 src/modules/admin/admin.guard.ts create mode 100644 test/unit/modules/admin/admin.guard.spec.ts create mode 100644 test/unit/modules/admin/audit.controller.spec.ts diff --git a/context/progress-tracker.md b/context/progress-tracker.md index e7e6363..bb6730f 100644 --- a/context/progress-tracker.md +++ b/context/progress-tracker.md @@ -6,6 +6,13 @@ pure chore/docs commits). Direct pushes to main must also be logged here. --- +## 2026-08-27 + +- **Server-truth AdminGuard for /admin routes**: Added `AdminGuard` in `src/modules/admin/admin.guard.ts` authorizing against datastore server truth (`users.role === 'admin'`) via `UserStatusService` rather than trusting JWT claims alone. +- **Audit Logging of Denied Admin Access**: Unauthorized or probing access attempts to `/admin` routes now record an `ADMIN_ACCESS_DENIED` entry in `audit_logs` via `AuditService` and return 403 `ADMIN_FORBIDDEN`. +- **Admin controllers secured**: Applied server-truth `AdminGuard` across the `/admin` controller tree (`AuditController`, `AdminRolesController`). +- Tests: Added unit coverage in `admin.guard.spec.ts` for non-admin rejection (403), active admin grant (200), blocked admin rejection (401), unauthenticated rejection (401), stale JWT claim with revoked DB role rejection (403), and audit log emission on denied access attempts. + ## 2026-08-24 - **Session families + refresh-token replay detection** (`sessions.family_id` diff --git a/src/modules/admin/admin-roles.controller.ts b/src/modules/admin/admin-roles.controller.ts index da3a63d..5081601 100644 --- a/src/modules/admin/admin-roles.controller.ts +++ b/src/modules/admin/admin-roles.controller.ts @@ -11,7 +11,7 @@ import { } from '@nestjs/common'; import { ApiTags, ApiOperation, ApiResponse, ApiBearerAuth, ApiParam } from '@nestjs/swagger'; import { JwtAuthGuard } from '../../common/guards/jwt-auth.guard'; -import { AdminGuard } from '../../auth/guards/admin.guard'; +import { AdminGuard } from './admin.guard'; import { AuditInterceptor } from '../../common/interceptors/audit.interceptor'; import { AuditAction } from '../../common/decorators/audit-action.decorator'; import { UsersRepository } from '../../database/repositories/users.repository'; diff --git a/src/modules/admin/admin.guard.ts b/src/modules/admin/admin.guard.ts new file mode 100644 index 0000000..9ef183d --- /dev/null +++ b/src/modules/admin/admin.guard.ts @@ -0,0 +1,123 @@ +import { + Injectable, + CanActivate, + ExecutionContext, + ForbiddenException, + UnauthorizedException, + Logger, + Optional, +} from '@nestjs/common'; +import { UserStatusService } from '../auth/user-status.service'; +import { AuditService } from './audit.service'; +import { SupabaseService } from '../../database/supabase.client'; + +/** + * Guard that enforces server-truth admin authorization for /admin routes. + * + * Rather than trusting the JWT role claim alone (which remains valid until token expiry), + * this guard queries server truth (via UserStatusService / Supabase datastore) fresh for + * req.user.wallet. + * + * Authorization rules: + * - Unauthenticated / missing wallet => 401 Unauthorized (AUTH_TOKEN_INVALID) + * - Blocked status => 401 Unauthorized (AUTH_USER_BLOCKED) + * - Role is not explicitly 'admin' => 403 Forbidden (ADMIN_FORBIDDEN) & logs audit attempt + */ +@Injectable() +export class AdminGuard implements CanActivate { + private readonly logger = new Logger(AdminGuard.name); + + constructor( + @Optional() private readonly userStatusService?: UserStatusService, + @Optional() private readonly auditService?: AuditService, + @Optional() private readonly supabaseService?: SupabaseService, + ) {} + + async canActivate(context: ExecutionContext): Promise { + const request = context.switchToHttp().getRequest<{ + user?: { wallet: string; role?: string | null }; + url?: string; + method?: string; + }>(); + + const user = request.user; + if (!user || !user.wallet) { + throw new UnauthorizedException({ + code: 'AUTH_TOKEN_INVALID', + message: 'Invalid or missing access token.', + }); + } + + const wallet = user.wallet; + let status = 'active'; + let role: string | null = null; + + if (this.userStatusService) { + const state = await this.userStatusService.getUserState(wallet); + status = state.status; + role = state.role; + } else if (this.supabaseService) { + try { + const client = this.supabaseService.getServiceRoleClient(); + const { data } = await client + .from('users') + .select('status, role') + .eq('wallet_address', wallet) + .maybeSingle(); + if (data) { + status = data.status ?? 'active'; + role = data.role ?? null; + } + } catch (err) { + this.logger.error(`Database query for admin check failed for ${wallet}: ${(err as Error).message}`); + } + } else { + role = user.role ?? null; + } + + if (status === 'blocked') { + throw new UnauthorizedException({ + code: 'AUTH_USER_BLOCKED', + message: 'This account has been suspended.', + }); + } + + if (role !== 'admin') { + this.logger.warn( + `Admin access denied: wallet ${wallet} (status: ${status}, role: ${role ?? 'none'}) attempted to access ${request.method ?? 'GET'} ${request.url ?? '/admin'}`, + ); + + if (this.auditService) { + try { + await this.auditService.log({ + actor_wallet: wallet, + action: 'ADMIN_ACCESS_DENIED', + resource: 'admin', + resource_id: null, + before_state: null, + after_state: null, + ip_address: null, + user_agent: null, + metadata: { + path: request.url ?? null, + method: request.method ?? null, + attemptedRole: role, + status, + }, + }); + } catch (auditErr) { + this.logger.error( + `Failed to log denied admin attempt for ${wallet}: ${(auditErr as Error).message}`, + ); + } + } + + throw new ForbiddenException({ + code: 'ADMIN_FORBIDDEN', + message: 'Forbidden. Explicit admin role required.', + }); + } + + return true; + } +} diff --git a/src/modules/admin/admin.module.ts b/src/modules/admin/admin.module.ts index 0b15969..6b29c8a 100644 --- a/src/modules/admin/admin.module.ts +++ b/src/modules/admin/admin.module.ts @@ -5,11 +5,11 @@ import { AdminRolesController } from './admin-roles.controller'; import { SupabaseService } from '../../database/supabase.client'; import { UsersRepository } from '../../database/repositories/users.repository'; import { UserStatusService } from '../auth/user-status.service'; -import { AdminGuard } from '../../auth/guards/admin.guard'; +import { AdminGuard } from './admin.guard'; @Module({ controllers: [AuditController, AdminRolesController], providers: [AuditService, SupabaseService, UsersRepository, UserStatusService, AdminGuard], - exports: [AuditService, UserStatusService], + exports: [AuditService, UserStatusService, AdminGuard], }) export class AdminModule {} diff --git a/src/modules/admin/audit.controller.ts b/src/modules/admin/audit.controller.ts index a415ee2..c434da3 100644 --- a/src/modules/admin/audit.controller.ts +++ b/src/modules/admin/audit.controller.ts @@ -4,7 +4,7 @@ import { AuditService } from './audit.service'; import { AuditLogQueryDto } from './dto/audit-log-query.dto'; import { AuditLogListResponseDto } from './dto/audit-log-response.dto'; import { JwtAuthGuard } from '../../common/guards/jwt-auth.guard'; -import { AdminGuard } from '../../auth/guards/admin.guard'; +import { AdminGuard } from './admin.guard'; import { AuditInterceptor } from '../../common/interceptors/audit.interceptor'; import { AuditAction } from '../../common/decorators/audit-action.decorator'; @@ -36,6 +36,7 @@ export class AuditController { type: AuditLogListResponseDto, }) @ApiResponse({ status: 401, description: 'Unauthorized - missing or invalid admin JWT' }) + @ApiResponse({ status: 403, description: 'Forbidden - wallet does not have explicit admin role' }) async getAuditLogs(@Query() query: AuditLogQueryDto) { return this.auditService.findMany(query); } diff --git a/test/unit/modules/admin/admin.guard.spec.ts b/test/unit/modules/admin/admin.guard.spec.ts new file mode 100644 index 0000000..fdac85a --- /dev/null +++ b/test/unit/modules/admin/admin.guard.spec.ts @@ -0,0 +1,113 @@ +import { ExecutionContext, ForbiddenException, UnauthorizedException } from '@nestjs/common'; +import { AdminGuard } from '../../../../src/modules/admin/admin.guard'; +import { UserStatusService } from '../../../../src/modules/auth/user-status.service'; +import { AuditService } from '../../../../src/modules/admin/audit.service'; + +describe('AdminGuard (src/modules/admin/admin.guard.ts)', () => { + let guard: AdminGuard; + let userStatusService: jest.Mocked; + let auditService: jest.Mocked; + + const ADMIN_WALLET = 'GAQWQJJBC2D5YCR6WUFFZSL6DIFJ5CA4774QB6QWPRNHSUUVRNQ2BHXJ'; + const SPONSOR_WALLET = 'GBXH6BL5Z7R5Y6RJSJRMJQH4YZVMYTCX2L4X2L4X2L4X2L4X2L4X2L4X'; + const BLOCKED_ADMIN_WALLET = 'GDBLOCKEDADMINWALLET12345678901234567890123456789012345678'; + + function createMockExecutionContext(user?: { wallet: string; role?: string | null }) { + const request = { + user, + url: '/admin/audit-logs', + method: 'GET', + }; + return { + switchToHttp: jest.fn().mockReturnValue({ + getRequest: jest.fn().mockReturnValue(request), + }), + } as unknown as ExecutionContext; + } + + beforeEach(() => { + userStatusService = { + getUserState: jest.fn(), + getStatus: jest.fn(), + getRole: jest.fn(), + ensureNotBlocked: jest.fn(), + invalidate: jest.fn(), + } as unknown as jest.Mocked; + + auditService = { + log: jest.fn().mockResolvedValue(undefined), + logWithBeforeAfter: jest.fn().mockResolvedValue(undefined), + findMany: jest.fn(), + } as unknown as jest.Mocked; + + guard = new AdminGuard(userStatusService, auditService); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + it('should return true for an active admin wallet', async () => { + userStatusService.getUserState.mockResolvedValue({ status: 'active', role: 'admin' }); + const ctx = createMockExecutionContext({ wallet: ADMIN_WALLET }); + + const result = await guard.canActivate(ctx); + + expect(result).toBe(true); + expect(userStatusService.getUserState).toHaveBeenCalledWith(ADMIN_WALLET); + expect(auditService.log).not.toHaveBeenCalled(); + }); + + it('should throw 403 (ADMIN_FORBIDDEN) for a non-admin wallet', async () => { + userStatusService.getUserState.mockResolvedValue({ status: 'active', role: 'sponsor' }); + const ctx = createMockExecutionContext({ wallet: SPONSOR_WALLET }); + + await expect(guard.canActivate(ctx)).rejects.toThrow(ForbiddenException); + await expect(guard.canActivate(ctx)).rejects.toMatchObject({ + response: { code: 'ADMIN_FORBIDDEN' }, + }); + expect(auditService.log).toHaveBeenCalledWith( + expect.objectContaining({ + actor_wallet: SPONSOR_WALLET, + action: 'ADMIN_ACCESS_DENIED', + resource: 'admin', + }), + ); + }); + + it('should throw 401 (AUTH_USER_BLOCKED) for a blocked admin wallet', async () => { + userStatusService.getUserState.mockResolvedValue({ status: 'blocked', role: 'admin' }); + const ctx = createMockExecutionContext({ wallet: BLOCKED_ADMIN_WALLET }); + + await expect(guard.canActivate(ctx)).rejects.toThrow(UnauthorizedException); + await expect(guard.canActivate(ctx)).rejects.toMatchObject({ + response: { code: 'AUTH_USER_BLOCKED' }, + }); + }); + + it('should throw 401 (AUTH_TOKEN_INVALID) when request user or wallet is missing', async () => { + const ctxUnauthenticated = createMockExecutionContext(undefined); + + await expect(guard.canActivate(ctxUnauthenticated)).rejects.toThrow(UnauthorizedException); + await expect(guard.canActivate(ctxUnauthenticated)).rejects.toMatchObject({ + response: { code: 'AUTH_TOKEN_INVALID' }, + }); + }); + + it('should throw 403 (ADMIN_FORBIDDEN) for a stale token with admin role claim when DB role is revoked', async () => { + // JWT claim says 'admin', but server truth in Supabase/UserStatusService is now null / revoked + userStatusService.getUserState.mockResolvedValue({ status: 'active', role: null }); + const ctx = createMockExecutionContext({ wallet: SPONSOR_WALLET, role: 'admin' }); + + await expect(guard.canActivate(ctx)).rejects.toThrow(ForbiddenException); + await expect(guard.canActivate(ctx)).rejects.toMatchObject({ + response: { code: 'ADMIN_FORBIDDEN' }, + }); + expect(auditService.log).toHaveBeenCalledWith( + expect.objectContaining({ + actor_wallet: SPONSOR_WALLET, + action: 'ADMIN_ACCESS_DENIED', + }), + ); + }); +}); diff --git a/test/unit/modules/admin/audit.controller.spec.ts b/test/unit/modules/admin/audit.controller.spec.ts new file mode 100644 index 0000000..af607d8 --- /dev/null +++ b/test/unit/modules/admin/audit.controller.spec.ts @@ -0,0 +1,47 @@ +import { Test, TestingModule } from '@nestjs/testing'; +import { AuditController } from '../../../../src/modules/admin/audit.controller'; +import { AuditService } from '../../../../src/modules/admin/audit.service'; +import { AuditLogQueryDto } from '../../../../src/modules/admin/dto/audit-log-query.dto'; + +describe('AuditController', () => { + let controller: AuditController; + let auditService: jest.Mocked; + + const mockAuditService = { + findMany: jest.fn(), + }; + + beforeEach(async () => { + const module: TestingModule = await Test.createTestingModule({ + controllers: [AuditController], + providers: [ + { provide: AuditService, useValue: mockAuditService }, + ], + }).compile(); + + controller = module.get(AuditController); + auditService = module.get(AuditService) as jest.Mocked; + jest.clearAllMocks(); + }); + + it('should be defined', () => { + expect(controller).toBeDefined(); + }); + + it('should delegate getAuditLogs to auditService.findMany', async () => { + const query: AuditLogQueryDto = { limit: 10, offset: 0 }; + const expectedResponse = { + success: true, + data: [], + pagination: { limit: 10, offset: 0, total: 0 }, + message: 'Audit logs retrieved successfully', + }; + + mockAuditService.findMany.mockResolvedValue(expectedResponse); + + const result = await controller.getAuditLogs(query); + + expect(mockAuditService.findMany).toHaveBeenCalledWith(query); + expect(result).toEqual(expectedResponse); + }); +});