Merge pull request #542 from dzekojohn4/fix/issue-511-admin-role-header-bypass

fix(auth): remove X-Roles admin bypass and gate admin endpoints behind AdminGuard

Author: Xhristin3

api/.env.example

@@ -90,11 +90,6 @@ SWAGGER_ENABLED=false
 # Leave empty for single-replica local development.
 REDIS_URL=
 
-# Dev-only convenience: allow roles to be supplied via request headers so auth
-# flows can be exercised locally without minting tokens. Production deployments
-# MUST set this to 0 to disable the fallback.
-ALLOW_HEADER_ROLES=1
-
 # ──────────────────────────────────────────────────────────────────────────────
 # OpenTelemetry distributed tracing (issue #346)
 #

api/src/audit/admin-audit.controller.ts api/src/admin/admin-audit.controller.tsapi/src/audit/admin-audit.controller.ts renamed to api/src/admin/admin-audit.controller.ts

@@ -1,15 +1,24 @@
 import { Controller, Get, Query, UseGuards } from "@nestjs/common"
-import { AuditService } from "./audit.service"
+
+import { AuditService } from "../audit/audit.service"
+import { AdminGuard } from "../common/auth/admin.guard"
+import { Roles } from "../common/auth/roles.guard"
 import { PaginationQueryDto } from "../common/dto/pagination.dto"
-import { Roles, RolesGuard } from "../common/auth/roles.guard"
 
+/**
+ * Admin-only audit log reader.
+ *
+ * Guarded by the same {@link AdminGuard} composition as `AdminController`
+ * (issue #511): authentication runs first, then the `admin` role is
+ * enforced from the JWT's `isAdmin` claim. A bare `X-Roles: admin`
+ * header is never honored.
+ */
 @Controller("admin/audit-logs")
-@UseGuards(RolesGuard)
+@UseGuards(AdminGuard)
 @Roles("admin")
 export class AdminAuditController {
   constructor(private readonly auditService: AuditService) {}
 
-  //get function
   @Get()
   async findAll(@Query() query: PaginationQueryDto) {
     const page = query.page ?? 1

api/src/admin/admin-guards.integration.spec.ts

@@ -0,0 +1,210 @@
+/**
+ * Integration tests for the admin guard composition (issue #511).
+ *
+ * Proves the security contract end-to-end through real HTTP:
+ * - No bearer token → 401 on both admin endpoints, even when the
+ *   `X-Roles: admin` header is present (the header path is gone).
+ * - Authenticated non-admin → 403.
+ * - Authenticated admin → 200.
+ *
+ * The harness follows api/src/auth/auth-rate-limit.integration.spec.ts:
+ * a real Nest application with the real guard chain and mocked
+ * services, exercised via supertest.
+ */
+import { CACHE_MANAGER } from "@nestjs/cache-manager"
+import { INestApplication, UnauthorizedException } from "@nestjs/common"
+import { Test, TestingModule } from "@nestjs/testing"
+import request from "supertest"
+
+import { AdminAuditController } from "./admin-audit.controller"
+import { AdminStatsService } from "./admin-stats.service"
+import { AdminController } from "./admin.controller"
+import { AuditService } from "../audit/audit.service"
+import { AdminGuard } from "../common/auth/admin.guard"
+import { RolesGuard } from "../common/auth/roles.guard"
+import { AuthGuard } from "../common/guards/auth.guard"
+import { JwtExtractorService } from "../common/guards/jwt-extractor.service"
+
+describe("Admin endpoints — guard composition (Integration)", () => {
+  let app: INestApplication
+
+  const mockJwtExtractor = {
+    authenticate: jest.fn(),
+    extractBearerToken: jest.fn(),
+  }
+  const mockAuditService = { findAll: jest.fn() }
+  const mockStatsService = { compute: jest.fn() }
+  const mockCache = { get: jest.fn(), set: jest.fn() }
+
+  beforeAll(async () => {
+    const moduleFixture: TestingModule = await Test.createTestingModule({
+      controllers: [AdminController, AdminAuditController],
+      providers: [
+        { provide: JwtExtractorService, useValue: mockJwtExtractor },
+        { provide: AuditService, useValue: mockAuditService },
+        { provide: AdminStatsService, useValue: mockStatsService },
+        { provide: CACHE_MANAGER, useValue: mockCache },
+        AuthGuard,
+        RolesGuard,
+        AdminGuard,
+      ],
+    }).compile()
+
+    app = moduleFixture.createNestApplication()
+    await app.init()
+  })
+
+  afterAll(async () => {
+    await app.close()
+  })
+
+  beforeEach(() => {
+    jest.clearAllMocks()
+    mockCache.get.mockResolvedValue(null)
+    mockStatsService.compute.mockResolvedValue({
+      totalUsers: 3,
+      totalStreams: 5,
+      activeStreams: 2,
+      eventsLast24h: 10,
+      generatedAt: "2026-08-24T00:00:00.000Z",
+    })
+    mockAuditService.findAll.mockResolvedValue({
+      data: [{ id: 1, action: "AUTH_LOGIN_SUCCESS" }],
+      total: 1,
+      page: 1,
+      limit: 20,
+    })
+  })
+
+  describe("GET /admin/audit-logs", () => {
+    it("returns 401 without a bearer token", async () => {
+      mockJwtExtractor.authenticate.mockRejectedValue(
+        new UnauthorizedException(
+          "Authorization header must contain a Bearer token",
+        ),
+      )
+
+      const res = await request(app.getHttpServer()).get("/admin/audit-logs")
+
+      expect(res.status).toBe(401)
+      expect(mockAuditService.findAll).not.toHaveBeenCalled()
+    })
+
+    it("returns 401 for X-Roles: admin with no bearer token — the header grants nothing", async () => {
+      mockJwtExtractor.authenticate.mockRejectedValue(
+        new UnauthorizedException(
+          "Authorization header must contain a Bearer token",
+        ),
+      )
+
+      const res = await request(app.getHttpServer())
+        .get("/admin/audit-logs")
+        .set("X-Roles", "admin")
+
+      expect(res.status).toBe(401)
+      expect(mockAuditService.findAll).not.toHaveBeenCalled()
+    })
+
+    it("returns 403 for an authenticated non-admin user", async () => {
+      mockJwtExtractor.authenticate.mockResolvedValue({
+        userId: 1,
+        isAdmin: false,
+      })
+
+      const res = await request(app.getHttpServer())
+        .get("/admin/audit-logs")
+        .set("Authorization", "Bearer non-admin-token")
+
+      expect(res.status).toBe(403)
+      expect(mockAuditService.findAll).not.toHaveBeenCalled()
+    })
+
+    it("returns 403 for a non-admin user even when they send X-Roles: admin", async () => {
+      mockJwtExtractor.authenticate.mockResolvedValue({
+        userId: 1,
+        isAdmin: false,
+      })
+
+      const res = await request(app.getHttpServer())
+        .get("/admin/audit-logs")
+        .set("Authorization", "Bearer non-admin-token")
+        .set("X-Roles", "admin")
+
+      expect(res.status).toBe(403)
+      expect(mockAuditService.findAll).not.toHaveBeenCalled()
+    })
+
+    it("returns 200 for an authenticated admin user", async () => {
+      mockJwtExtractor.authenticate.mockResolvedValue({
+        userId: 1,
+        isAdmin: true,
+      })
+
+      const res = await request(app.getHttpServer())
+        .get("/admin/audit-logs")
+        .set("Authorization", "Bearer admin-token")
+
+      expect(res.status).toBe(200)
+      expect(mockAuditService.findAll).toHaveBeenCalledWith(1, 20)
+      expect(res.body.data).toHaveLength(1)
+    })
+  })
+
+  describe("GET /admin/stats", () => {
+    it("returns 401 without a bearer token", async () => {
+      mockJwtExtractor.authenticate.mockRejectedValue(
+        new UnauthorizedException(
+          "Authorization header must contain a Bearer token",
+        ),
+      )
+
+      const res = await request(app.getHttpServer()).get("/admin/stats")
+
+      expect(res.status).toBe(401)
+      expect(mockStatsService.compute).not.toHaveBeenCalled()
+    })
+
+    it("returns 401 for X-Roles: admin with no bearer token", async () => {
+      mockJwtExtractor.authenticate.mockRejectedValue(
+        new UnauthorizedException(
+          "Authorization header must contain a Bearer token",
+        ),
+      )
+
+      const res = await request(app.getHttpServer())
+        .get("/admin/stats")
+        .set("X-Roles", "admin")
+
+      expect(res.status).toBe(401)
+      expect(mockStatsService.compute).not.toHaveBeenCalled()
+    })
+
+    it("returns 403 for an authenticated non-admin user", async () => {
+      mockJwtExtractor.authenticate.mockResolvedValue({
+        userId: 1,
+        isAdmin: false,
+      })
+
+      const res = await request(app.getHttpServer())
+        .get("/admin/stats")
+        .set("Authorization", "Bearer non-admin-token")
+
+      expect(res.status).toBe(403)
+      expect(mockStatsService.compute).not.toHaveBeenCalled()
+    })
+
+    it("returns 200 with a snapshot for an authenticated admin user", async () => {
+      mockJwtExtractor.authenticate.mockResolvedValue({
+        userId: 1,
+        isAdmin: true,
+      })
+
+      const res = await request(app.getHttpServer())
+        .get("/admin/stats")
+        .set("Authorization", "Bearer admin-token")
+
+      expect(res.status).toBe(200)
+      expect(res.body.totalUsers).toBe(3)
+    })
+  })
+})

api/src/admin/admin.controller.ts

@@ -15,15 +15,16 @@ import {
   ApiUnauthorizedResponse,
 } from "@nestjs/swagger"
 import { Cache } from "cache-manager"
-import { AuthGuard } from "../common/guards/auth.guard"
-import { Roles, RolesGuard } from "../common/auth/roles.guard"
+
 import { AdminStats, AdminStatsService } from "./admin-stats.service"
+import { AdminGuard } from "../common/auth/admin.guard"
+import { Roles } from "../common/auth/roles.guard"
 
 const STATS_CACHE_TTL_MS = 60_000
 
 @ApiTags("admin")
 @Controller("admin")
-@UseGuards(AuthGuard, RolesGuard)
+@UseGuards(AdminGuard)
 @Roles("admin")
 export class AdminController {
   constructor(

api/src/admin/admin.module.ts

@@ -1,18 +1,23 @@
 import { CacheModule } from "@nestjs/cache-manager"
 import { Module } from "@nestjs/common"
+
+import { AdminAuditController } from "./admin-audit.controller"
+import { AdminStatsService } from "./admin-stats.service"
+import { AdminController } from "./admin.controller"
+import { AuditModule } from "../audit/audit.module"
 import { AuthModule } from "../auth/auth.module"
-import { adminCacheConfig } from "../config/cache.config"
+import { AdminGuard } from "../common/auth/admin.guard"
 import { RolesGuard } from "../common/auth/roles.guard"
 import { AuthGuard } from "../common/guards/auth.guard"
-import { AdminStatsService } from "./admin-stats.service"
-import { AdminController } from "./admin.controller"
+import { adminCacheConfig } from "../config/cache.config"
 
 @Module({
   imports: [
     CacheModule.register(adminCacheConfig()),
     AuthModule,
+    AuditModule,
   ],
-  controllers: [AdminController],
-  providers: [AdminStatsService, RolesGuard, AuthGuard],
+  controllers: [AdminController, AdminAuditController],
+  providers: [AdminStatsService, AdminGuard, RolesGuard, AuthGuard],
 })
-export class AdminModule {}
+export class AdminModule {}

api/src/audit/audit.module.ts

@@ -1,11 +1,10 @@
 import { Module } from "@nestjs/common"
 import { APP_INTERCEPTOR } from "@nestjs/core"
-import { AuditService } from "./audit.service"
+
 import { AuditInterceptor } from "./audit.interceptor"
-import { AdminAuditController } from "./admin-audit.controller"
+import { AuditService } from "./audit.service"
 
 @Module({
-  controllers: [AdminAuditController],
   providers: [
     AuditService,
     { provide: APP_INTERCEPTOR, useClass: AuditInterceptor },