From 1d4dbcf6b11d1bfc97a2a70fc408b047e2d8a47b Mon Sep 17 00:00:00 2001 From: JonnyKay Date: Mon, 24 Aug 2026 10:23:58 +0000 Subject: [PATCH] fix(api): remove header role bypass and gate admin endpoints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The audit log at GET /admin/audit-logs was readable by any caller sending an X-Roles: admin header: RolesGuard fell back to the header when req.user was absent, and AdminAuditController ran RolesGuard with no upstream AuthGuard. Meanwhile the legitimate path was broken — AuthGuard hardcoded req.user.roles = [] and the users table had no admin flag, so /admin/stats 403'd for everyone. Add users.is_admin (default false, migration + schema.sql), carry it as an isAdmin claim on access tokens, derive req.user.roles from the claim in AuthGuard, and delete the header fallback from RolesGuard so a missing identity is a 401. Both admin controllers now share a single AdminGuard (AuthGuard + RolesGuard) composition. Remove the dead ALLOW_HEADER_ROLES config from docker-compose.yml, api/.env.example, and the SDK type-generation script. --- api/.env.example | 5 - .../admin-audit.controller.ts | 17 +- .../admin/admin-guards.integration.spec.ts | 210 ++++++++++++++++++ api/src/admin/admin.controller.ts | 7 +- api/src/admin/admin.module.ts | 17 +- api/src/audit/audit.module.ts | 5 +- api/src/auth/auth.service.spec.ts | 49 ++-- api/src/auth/auth.service.ts | 28 ++- api/src/auth/users.repository.ts | 15 +- api/src/common/auth/admin.guard.spec.ts | 66 ++++++ api/src/common/auth/admin.guard.ts | 34 +++ api/src/common/auth/roles.guard.spec.ts | 95 ++++++++ api/src/common/auth/roles.guard.ts | 41 ++-- api/src/common/guards/auth.guard.spec.ts | 31 ++- api/src/common/guards/auth.guard.ts | 11 +- .../guards/jwt-extractor.service.spec.ts | 92 ++++++++ .../common/guards/jwt-extractor.service.ts | 19 +- api/src/contract-provider.spec.ts | 4 +- api/src/database.integration.spec.ts | 3 + api/src/openapi-security.spec.ts | 9 + api/src/users/users.service.ts | 6 +- .../2026082401_add_users_is_admin.down.sql | 3 + .../2026082401_add_users_is_admin.up.sql | 4 + database/migrations/README.md | 18 ++ database/schema.sql | 3 + docker-compose.yml | 1 - xstreamroll-sdk/scripts/generate-types.sh | 1 - 27 files changed, 704 insertions(+), 90 deletions(-) rename api/src/{audit => admin}/admin-audit.controller.ts (50%) create mode 100644 api/src/admin/admin-guards.integration.spec.ts create mode 100644 api/src/common/auth/admin.guard.spec.ts create mode 100644 api/src/common/auth/admin.guard.ts create mode 100644 api/src/common/auth/roles.guard.spec.ts create mode 100644 api/src/common/guards/jwt-extractor.service.spec.ts create mode 100644 database/migrations/2026082401_add_users_is_admin.down.sql create mode 100644 database/migrations/2026082401_add_users_is_admin.up.sql diff --git a/api/.env.example b/api/.env.example index f4b3445..a1bb2bd 100644 --- a/api/.env.example +++ b/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) # diff --git a/api/src/audit/admin-audit.controller.ts b/api/src/admin/admin-audit.controller.ts similarity index 50% rename from api/src/audit/admin-audit.controller.ts rename to api/src/admin/admin-audit.controller.ts index 4877567..1106bfd 100644 --- a/api/src/audit/admin-audit.controller.ts +++ b/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 diff --git a/api/src/admin/admin-guards.integration.spec.ts b/api/src/admin/admin-guards.integration.spec.ts new file mode 100644 index 0000000..d43d4f2 --- /dev/null +++ b/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) + }) + }) +}) diff --git a/api/src/admin/admin.controller.ts b/api/src/admin/admin.controller.ts index 3b1a098..7df2a76 100644 --- a/api/src/admin/admin.controller.ts +++ b/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( diff --git a/api/src/admin/admin.module.ts b/api/src/admin/admin.module.ts index 9232673..2f3f124 100644 --- a/api/src/admin/admin.module.ts +++ b/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 {} \ No newline at end of file +export class AdminModule {} diff --git a/api/src/audit/audit.module.ts b/api/src/audit/audit.module.ts index cfd5c4a..0d5d841 100644 --- a/api/src/audit/audit.module.ts +++ b/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 }, diff --git a/api/src/auth/auth.service.spec.ts b/api/src/auth/auth.service.spec.ts index 521f22a..d49ca4b 100644 --- a/api/src/auth/auth.service.spec.ts +++ b/api/src/auth/auth.service.spec.ts @@ -1,9 +1,10 @@ import { ConflictException, UnauthorizedException } from "@nestjs/common" import { JwtService } from "@nestjs/jwt" import * as bcrypt from "bcrypt" + import { AuthService } from "./auth.service" -import { User, UsersRepository } from "./users.repository" import { TokenDenylistService } from "./token-denylist.service" +import { User, UsersRepository } from "./users.repository" jest.mock("bcrypt", () => ({ hash: jest.fn(), @@ -87,6 +88,7 @@ function dummyUser(overrides: Partial = {}): User { password_hash: "$2b$10$abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ12", created_at: new Date("2026-01-01T00:00:00Z"), + is_admin: false, ...overrides, } } @@ -142,7 +144,7 @@ describe("AuthService", () => { const result = await service.register(dto, { ip: "127.0.0.1", headers: { "user-agent": "test" }, - } as any) + } as any) // eslint-disable-line @typescript-eslint/no-explicit-any -- partial request stub for test expect(users.findByEmail).toHaveBeenCalledWith(dto.email) expect(users.findByUsername).toHaveBeenCalledWith(dto.username) @@ -156,6 +158,7 @@ describe("AuthService", () => { email: dto.email, username: dto.username, passwordChangedAt: expect.any(Number), + isAdmin: false, }) expect(refreshJwt.sign).toHaveBeenCalledWith({ sub: 1, @@ -182,7 +185,7 @@ describe("AuthService", () => { service.register(dto, { ip: "127.0.0.1", headers: { "user-agent": "test" }, - } as any), + } as any), // eslint-disable-line @typescript-eslint/no-explicit-any -- partial request stub for test ).rejects.toThrow(ConflictException) expect(users.create).not.toHaveBeenCalled() }) @@ -198,7 +201,7 @@ describe("AuthService", () => { service.register(dto, { ip: "127.0.0.1", headers: { "user-agent": "test" }, - } as any), + } as any), // eslint-disable-line @typescript-eslint/no-explicit-any -- partial request stub for test ).rejects.toThrow(ConflictException) expect(users.create).not.toHaveBeenCalled() }) @@ -215,7 +218,7 @@ describe("AuthService", () => { await service.register(dto, { ip: "127.0.0.1", headers: { "user-agent": "test" }, - } as any) + } as any) // eslint-disable-line @typescript-eslint/no-explicit-any -- partial request stub for test expect(bcrypt.hash).toHaveBeenCalledWith(dto.password, 12) const [storedUsername, storedEmail, storedHash] = @@ -235,8 +238,8 @@ describe("AuthService", () => { email: "dup@x.com", password: "someOtherPassword", }, - { ip: "127.0.0.1", headers: { "user-agent": "test" } } as any, - ), // eslint-disable-line @typescript-eslint/no-explicit-any + { ip: "127.0.0.1", headers: { "user-agent": "test" } } as any, // eslint-disable-line @typescript-eslint/no-explicit-any -- partial request stub for test + ), ).rejects.toThrow(ConflictException) }) }) @@ -289,7 +292,7 @@ describe("AuthService", () => { const result = await service.login(dto, { ip: "127.0.0.1", headers: { "user-agent": "test" }, - } as any) + } as any) // eslint-disable-line @typescript-eslint/no-explicit-any -- partial request stub for test expect(users.findByEmail).toHaveBeenCalledWith(dto.email) expect(bcrypt.compare).toHaveBeenCalledWith( @@ -301,6 +304,7 @@ describe("AuthService", () => { email: user.email, username: user.username, passwordChangedAt: expect.any(Number), + isAdmin: false, }) expect(refreshJwt.sign).toHaveBeenCalledWith({ sub: user.id, @@ -327,7 +331,7 @@ describe("AuthService", () => { service.login(dto, { ip: "127.0.0.1", headers: { "user-agent": "test" }, - } as any), + } as any), // eslint-disable-line @typescript-eslint/no-explicit-any -- partial request stub for test ).rejects.toThrow(UnauthorizedException) expect(accessJwt.sign).not.toHaveBeenCalled() }) @@ -342,7 +346,7 @@ describe("AuthService", () => { service.login({ email: dto.email, password: "wrongPassword" }, { ip: "127.0.0.1", headers: { "user-agent": "test" }, - } as any), + } as any), // eslint-disable-line @typescript-eslint/no-explicit-any -- partial request stub for test ).rejects.toThrow(UnauthorizedException) expect(accessJwt.sign).not.toHaveBeenCalled() @@ -356,7 +360,7 @@ describe("AuthService", () => { .login({ email: "no@user.com", password: "any" }, { ip: "127.0.0.1", headers: { "user-agent": "test" }, - } as any) + } as any) // eslint-disable-line @typescript-eslint/no-explicit-any -- partial request stub for test .catch((e) => e) expect(e1).toBeInstanceOf(UnauthorizedException) @@ -368,7 +372,7 @@ describe("AuthService", () => { .login({ email: dto.email, password: "bad" }, { ip: "127.0.0.1", headers: { "user-agent": "test" }, - } as any) + } as any) // eslint-disable-line @typescript-eslint/no-explicit-any -- partial request stub for test .catch((e) => e) expect(e2).toBeInstanceOf(UnauthorizedException) @@ -386,7 +390,7 @@ describe("AuthService", () => { await service.login(dto, { ip: "127.0.0.1", headers: { "user-agent": "test" }, - } as any) + } as any) // eslint-disable-line @typescript-eslint/no-explicit-any -- partial request stub for test expect(bcrypt.compare).toHaveBeenCalledWith( dto.password, @@ -397,6 +401,7 @@ describe("AuthService", () => { email: user.email, username: user.username, passwordChangedAt: expect.any(Number), + isAdmin: false, }) expect(refreshJwt.sign).toHaveBeenCalledWith({ sub: user.id, @@ -406,6 +411,24 @@ describe("AuthService", () => { jti: expect.any(String), }) }) + + it("carries isAdmin: true in the access token when the user is flagged admin", async () => { + const user = dummyUser({ email: dto.email, is_admin: true }) + users.findByEmail.mockResolvedValue(user) + ;(bcrypt.compare as jest.Mock).mockResolvedValue(true) + accessJwt.sign.mockReturnValue("jwt.token.here") + refreshJwt.sign.mockReturnValue("refresh.token.here") + + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- partial request stub for test + await service.login(dto, { + ip: "127.0.0.1", + headers: { "user-agent": "test" }, + } as any) // eslint-disable-line @typescript-eslint/no-explicit-any -- partial request stub for test + + expect(accessJwt.sign).toHaveBeenCalledWith( + expect.objectContaining({ isAdmin: true }), + ) + }) }) // -- logout ------------------------------------------------------------ diff --git a/api/src/auth/auth.service.ts b/api/src/auth/auth.service.ts index 1a16420..0d93d41 100644 --- a/api/src/auth/auth.service.ts +++ b/api/src/auth/auth.service.ts @@ -1,3 +1,5 @@ +import { randomUUID } from "node:crypto" + import { ConflictException, Injectable, @@ -5,19 +7,21 @@ import { UnauthorizedException, } from "@nestjs/common" import { JwtService } from "@nestjs/jwt" -import type { User as SharedUser } from "@xstreamroll/types" import * as bcrypt from "bcrypt" -import { randomUUID } from "node:crypto" -import type { Request } from "express" -import { RegisterDto } from "./dto/register.dto" -import { LoginDto } from "./dto/login.dto" + + import { ForgotPasswordDto } from "./dto/forgot-password.dto" +import { LoginDto } from "./dto/login.dto" +import { RegisterDto } from "./dto/register.dto" import { ResetPasswordDto } from "./dto/reset-password.dto" +import { PasswordResetService } from "./password-reset.service" import { TokenDenylistService } from "./token-denylist.service" import { User, UsersRepository } from "./users.repository" -import { PasswordResetService } from "./password-reset.service" -import { AuditService } from "../audit/audit.service" import { AuditAction } from "../audit/audit-action.enum" +import { AuditService } from "../audit/audit.service" + +import type { User as SharedUser } from "@xstreamroll/types" +import type { Request } from "express" /** Rounds for bcrypt key derivation (auto-salt). */ const BCRYPT_ROUNDS = 12 @@ -271,7 +275,14 @@ export class AuthService { return match[1] } - /** Create a short-lived JWT access token for the given user. */ + /** + * Create a short-lived JWT access token for the given user. + * + * The `isAdmin` claim is read from the users row at issuance time, so + * a promotion/demotion takes effect on the user's next login — the + * same freshness model as `passwordChangedAt`. Tokens minted before + * this claim existed simply lack it and are treated as non-admin. + */ private signAccessToken(user: User): string { return this.accessJwt.sign({ sub: user.id, @@ -279,6 +290,7 @@ export class AuthService { username: user.username, passwordChangedAt: user.password_changed_at?.getTime() ?? user.created_at.getTime(), + isAdmin: user.is_admin === true, }) } diff --git a/api/src/auth/users.repository.ts b/api/src/auth/users.repository.ts index 6764e6f..531ae4b 100644 --- a/api/src/auth/users.repository.ts +++ b/api/src/auth/users.repository.ts @@ -1,5 +1,6 @@ import { Inject, Injectable } from "@nestjs/common" import { Pool } from "pg" + import { PG_POOL } from "../database/database.module" export interface User { @@ -9,6 +10,8 @@ export interface User { password_hash: string created_at: Date password_changed_at?: Date + /** Issue #511: single admin bit; read at access-token issuance. */ + is_admin: boolean } /** @@ -23,7 +26,7 @@ export class UsersRepository { async findByEmail(email: string): Promise { const { rows } = await this.pool.query( - "SELECT id, username, email, password_hash, created_at, password_changed_at FROM users WHERE email = $1", + "SELECT id, username, email, password_hash, created_at, password_changed_at, is_admin FROM users WHERE email = $1", [email], ) return rows[0] ?? null @@ -31,7 +34,7 @@ export class UsersRepository { async findByUsername(username: string): Promise { const { rows } = await this.pool.query( - "SELECT id, username, email, password_hash, created_at, password_changed_at FROM users WHERE username = $1", + "SELECT id, username, email, password_hash, created_at, password_changed_at, is_admin FROM users WHERE username = $1", [username], ) return rows[0] ?? null @@ -39,7 +42,7 @@ export class UsersRepository { async findById(id: number): Promise { const { rows } = await this.pool.query( - "SELECT id, username, email, password_hash, created_at, password_changed_at FROM users WHERE id = $1", + "SELECT id, username, email, password_hash, created_at, password_changed_at, is_admin FROM users WHERE id = $1", [id], ) return rows[0] ?? null @@ -53,7 +56,7 @@ export class UsersRepository { const { rows } = await this.pool.query( `INSERT INTO users (username, email, password_hash) VALUES ($1, $2, $3) - RETURNING id, username, email, password_hash, created_at, password_changed_at`, + RETURNING id, username, email, password_hash, created_at, password_changed_at, is_admin`, [username, email, passwordHash], ) return rows[0] @@ -85,7 +88,7 @@ export class UsersRepository { `UPDATE users SET ${sets.join(", ")} WHERE id = $${idx} - RETURNING id, username, email, password_hash, created_at, password_changed_at`, + RETURNING id, username, email, password_hash, created_at, password_changed_at, is_admin`, values, ) return rows[0] @@ -101,7 +104,7 @@ export class UsersRepository { SET password_hash = $1, password_changed_at = $2 WHERE id = $3 - RETURNING id, username, email, password_hash, created_at, password_changed_at`, + RETURNING id, username, email, password_hash, created_at, password_changed_at, is_admin`, [passwordHash, passwordChangedAt, id], ) return rows[0] diff --git a/api/src/common/auth/admin.guard.spec.ts b/api/src/common/auth/admin.guard.spec.ts new file mode 100644 index 0000000..3b305c3 --- /dev/null +++ b/api/src/common/auth/admin.guard.spec.ts @@ -0,0 +1,66 @@ +import { + ForbiddenException, + UnauthorizedException, +} from "@nestjs/common" + +import { AdminGuard } from "./admin.guard" +import { RolesGuard } from "./roles.guard" +import { AuthGuard } from "../guards/auth.guard" + +interface MockCanActivate { + canActivate: jest.Mock | boolean> +} + +function makeGuard( + authGuard: MockCanActivate, + rolesGuard: MockCanActivate, +): AdminGuard { + return new AdminGuard( + authGuard as unknown as AuthGuard, + rolesGuard as unknown as RolesGuard, + ) +} + +describe("AdminGuard", () => { + let authGuard: MockCanActivate + let rolesGuard: MockCanActivate + let guard: AdminGuard + + beforeEach(() => { + authGuard = { canActivate: jest.fn() } + rolesGuard = { canActivate: jest.fn() } + guard = makeGuard(authGuard, rolesGuard) + jest.clearAllMocks() + }) + + it("passes when authentication and the role check both succeed", async () => { + authGuard.canActivate.mockResolvedValue(true) + rolesGuard.canActivate.mockReturnValue(true) + + await expect(guard.canActivate({} as never)).resolves.toBe(true) + expect(authGuard.canActivate).toHaveBeenCalledTimes(1) + expect(rolesGuard.canActivate).toHaveBeenCalledTimes(1) + }) + + it("propagates UnauthorizedException from AuthGuard and skips the role check", async () => { + authGuard.canActivate.mockRejectedValue( + new UnauthorizedException("invalid or expired access token"), + ) + + await expect(guard.canActivate({} as never)).rejects.toThrow( + UnauthorizedException, + ) + expect(rolesGuard.canActivate).not.toHaveBeenCalled() + }) + + it("propagates ForbiddenException from RolesGuard for a non-admin identity", async () => { + authGuard.canActivate.mockResolvedValue(true) + rolesGuard.canActivate.mockImplementation(() => { + throw new ForbiddenException("requires one of role(s): admin") + }) + + await expect(guard.canActivate({} as never)).rejects.toThrow( + ForbiddenException, + ) + }) +}) diff --git a/api/src/common/auth/admin.guard.ts b/api/src/common/auth/admin.guard.ts new file mode 100644 index 0000000..362bcf0 --- /dev/null +++ b/api/src/common/auth/admin.guard.ts @@ -0,0 +1,34 @@ +import { CanActivate, ExecutionContext, Injectable } from "@nestjs/common" + +import { RolesGuard } from "./roles.guard" +import { AuthGuard } from "../guards/auth.guard" + +/** + * Shared guard composition for the admin surface: authenticate first, + * then enforce the `admin` role. + * + * Both `AdminController` (`/admin/stats`) and `AdminAuditController` + * (`/admin/audit-logs`) are gated by this single guard so the auth and + * role layers can never drift apart again (issue #511 — the audit + * controller previously ran `RolesGuard` with no upstream `AuthGuard`, + * which let a bare `X-Roles: admin` header read the audit log). + * + * @UseGuards(AdminGuard) + * @Roles("admin") + * @Controller("admin") + */ +@Injectable() +export class AdminGuard implements CanActivate { + constructor( + private readonly authGuard: AuthGuard, + private readonly rolesGuard: RolesGuard, + ) {} + + async canActivate(context: ExecutionContext): Promise { + // AuthGuard throws UnauthorizedException on any authentication + // failure and populates req.user.roles from the token's isAdmin + // claim; RolesGuard then denies (403) non-admin identities. + await this.authGuard.canActivate(context) + return this.rolesGuard.canActivate(context) + } +} diff --git a/api/src/common/auth/roles.guard.spec.ts b/api/src/common/auth/roles.guard.spec.ts new file mode 100644 index 0000000..0330bc3 --- /dev/null +++ b/api/src/common/auth/roles.guard.spec.ts @@ -0,0 +1,95 @@ +import { ForbiddenException, UnauthorizedException } from "@nestjs/common" +import { Reflector } from "@nestjs/core" + +import { RolesGuard } from "./roles.guard" + +function makeGuard(requiredRoles: string[] | undefined): RolesGuard { + const reflector = { + getAllAndOverride: jest.fn().mockReturnValue(requiredRoles), + } as unknown as Reflector + return new RolesGuard(reflector) +} + +function contextWith(req: { + user?: { sub: number; roles?: string[] } + header?: jest.Mock +}): unknown { + const context = { + switchToHttp: () => ({ getRequest: () => req }), + getHandler: () => ({}), + getClass: () => ({}), + } + return context +} + +describe("RolesGuard", () => { + it("is a no-op when no @Roles metadata is declared", () => { + const guard = makeGuard(undefined) + const context = contextWith({}) as never + + expect(guard.canActivate(context)).toBe(true) + }) + + it("rejects with 401 when no authenticated user is present", () => { + const guard = makeGuard(["admin"]) + const context = contextWith({ header: jest.fn().mockReturnValue(undefined) }) + + expect(() => guard.canActivate(context as never)).toThrow( + UnauthorizedException, + ) + }) + + it("rejects with 401 even when the X-Roles header claims admin — the header fallback is gone", () => { + const guard = makeGuard(["admin"]) + const context = contextWith({ + header: jest.fn().mockReturnValue("admin"), + }) + + expect(() => guard.canActivate(context as never)).toThrow( + UnauthorizedException, + ) + }) + + it("rejects with 403 when the authenticated user lacks the required role", () => { + const guard = makeGuard(["admin"]) + const context = contextWith({ + user: { sub: 1, roles: [] }, + header: jest.fn().mockReturnValue(undefined), + }) + + expect(() => guard.canActivate(context as never)).toThrow( + ForbiddenException, + ) + }) + + it("ignores the X-Roles header for an authenticated user without the role", () => { + const guard = makeGuard(["admin"]) + const context = contextWith({ + user: { sub: 1, roles: [] }, + header: jest.fn().mockReturnValue("admin"), + }) + + expect(() => guard.canActivate(context as never)).toThrow( + ForbiddenException, + ) + }) + + it("allows an authenticated user with the required role", () => { + const guard = makeGuard(["admin"]) + const context = contextWith({ + user: { sub: 1, roles: ["admin"] }, + header: jest.fn().mockReturnValue(undefined), + }) + + expect(guard.canActivate(context as never)).toBe(true) + }) + + it("allows any authenticated user when the handler requires a role they hold", () => { + const guard = makeGuard(["moderator", "admin"]) + const context = contextWith({ + user: { sub: 1, roles: ["moderator"] }, + }) + + expect(guard.canActivate(context as never)).toBe(true) + }) +}) diff --git a/api/src/common/auth/roles.guard.ts b/api/src/common/auth/roles.guard.ts index 6f0f2c2..403ea44 100644 --- a/api/src/common/auth/roles.guard.ts +++ b/api/src/common/auth/roles.guard.ts @@ -7,6 +7,7 @@ import { UnauthorizedException, } from "@nestjs/common" import { Reflector } from "@nestjs/core" + import type { Request } from "express" export const ROLES_METADATA_KEY = "auth:roles" @@ -16,7 +17,7 @@ export const ROLES_METADATA_KEY = "auth:roles" * to invoke the annotated endpoint. * * @Roles("admin") - * @UseGuards(RolesGuard) + * @UseGuards(AdminGuard) * @Get("stats") * stats() { ... } */ @@ -29,17 +30,15 @@ interface AuthenticatedRequest extends Request { /** * Role-based access control. * - * The guard expects an upstream auth layer (JWT strategy, session, etc.) - * to populate `req.user.roles`. Until that lands the guard supports a - * dev-only fallback: if the request is missing `req.user` it inspects - * the `X-Roles` header (comma-separated list) so endpoints can still be - * exercised locally. Production deployments MUST set - * \`ALLOW_HEADER_ROLES=0\` to disable this fallback. + * Roles are read exclusively from `req.user.roles`, which an upstream + * auth guard ({@link AuthGuard}) populates from the verified JWT's + * `isAdmin` claim. A request without `req.user` is rejected with 401 — + * there is no header-based fallback, so a bare `X-Roles: admin` header + * can never grant access. Compose with {@link AdminGuard} (or an + * `AuthGuard` + this guard pair) on any handler that declares `@Roles`. */ @Injectable() export class RolesGuard implements CanActivate { - private readonly headerFallbackEnabled = process.env.ALLOW_HEADER_ROLES !== "0" - constructor(private readonly reflector: Reflector) {} canActivate(context: ExecutionContext): boolean { @@ -53,12 +52,16 @@ export class RolesGuard implements CanActivate { if (!required || required.length === 0) return true const req = context.switchToHttp().getRequest() - const roles = this.extractRoles(req) - if (!roles) { + + // Authentication must have run first (AuthGuard). Without an + // authenticated user there is no identity to authorize — treat this + // as 401, not 403, so unauthenticated callers can't probe role + // requirements. + if (!req.user) { throw new UnauthorizedException("authentication required") } - const granted = required.some((r) => roles.includes(r)) + const granted = required.some((r) => req.user?.roles?.includes(r)) if (!granted) { throw new ForbiddenException( `requires one of role(s): ${required.join(", ")}`, @@ -66,18 +69,4 @@ export class RolesGuard implements CanActivate { } return true } - - private extractRoles(req: AuthenticatedRequest): string[] | null { - if (req.user && Array.isArray(req.user.roles)) { - return req.user.roles - } - if (!this.headerFallbackEnabled) return null - - const header = req.header("x-roles") - if (!header) return null - return header - .split(",") - .map((r) => r.trim().toLowerCase()) - .filter(Boolean) - } } diff --git a/api/src/common/guards/auth.guard.spec.ts b/api/src/common/guards/auth.guard.spec.ts index accb793..56732ab 100644 --- a/api/src/common/guards/auth.guard.spec.ts +++ b/api/src/common/guards/auth.guard.spec.ts @@ -1,9 +1,10 @@ import { UnauthorizedException } from "@nestjs/common" + import { AuthGuard } from "./auth.guard" import { JwtExtractorService } from "./jwt-extractor.service" interface MockJwtExtractor { - authenticate: jest.Mock> + authenticate: jest.Mock> extractBearerToken: jest.Mock } @@ -12,7 +13,11 @@ function makeGuard(extractor: MockJwtExtractor): AuthGuard { } function contextWithToken(token: string) { - const req: { header: jest.Mock; auth?: { userId: number } } = { + const req: { + header: jest.Mock + auth?: { userId: number } + user?: { sub: number; roles: string[] } + } = { header: jest.fn().mockReturnValue(`Bearer ${token}`), } const context = { @@ -27,20 +32,34 @@ describe("AuthGuard", () => { let guard: AuthGuard beforeEach(() => { - extractor = { authenticate: jest.fn(), extractBearerToken: jest.fn() } + extractor = { + authenticate: jest.fn(), + extractBearerToken: jest.fn(), + } guard = makeGuard(extractor) jest.clearAllMocks() }) it("allows a verified token whose jti is not revoked", async () => { const { req, context } = contextWithToken("tok") - extractor.authenticate.mockResolvedValue(1) + extractor.authenticate.mockResolvedValue({ userId: 1, isAdmin: false }) const result = await guard.canActivate(context) expect(result).toBe(true) expect(extractor.authenticate).toHaveBeenCalledWith("Bearer tok") expect(req.auth).toEqual({ userId: 1 }) + expect(req.user).toEqual({ sub: 1, roles: [] }) + }) + + it("maps the isAdmin claim to the admin role on req.user", async () => { + const { req, context } = contextWithToken("tok") + extractor.authenticate.mockResolvedValue({ userId: 1, isAdmin: true }) + + await guard.canActivate(context) + + expect(req.auth).toEqual({ userId: 1 }) + expect(req.user).toEqual({ sub: 1, roles: ["admin"] }) }) it("rejects a token whose jti is on the denylist", async () => { @@ -56,7 +75,7 @@ describe("AuthGuard", () => { it("skips the denylist lookup for tokens issued before the jti claim", async () => { const { req, context } = contextWithToken("tok") - extractor.authenticate.mockResolvedValue(7) + extractor.authenticate.mockResolvedValue({ userId: 7, isAdmin: false }) const result = await guard.canActivate(context) @@ -114,7 +133,7 @@ describe("AuthGuard", () => { it("allows a token minted at or after the password change", async () => { const { req, context } = contextWithToken("tok") - extractor.authenticate.mockResolvedValue(1) + extractor.authenticate.mockResolvedValue({ userId: 1, isAdmin: false }) const result = await guard.canActivate(context) diff --git a/api/src/common/guards/auth.guard.ts b/api/src/common/guards/auth.guard.ts index d2d0d41..e7238b4 100644 --- a/api/src/common/guards/auth.guard.ts +++ b/api/src/common/guards/auth.guard.ts @@ -1,7 +1,9 @@ import { CanActivate, ExecutionContext, Injectable } from "@nestjs/common" -import type { Request } from "express" + import { JwtExtractorService } from "./jwt-extractor.service" +import type { Request } from "express" + /** * Auth guard that validates a JWT access token from the Authorization header * and rejects revoked tokens. @@ -15,7 +17,7 @@ export class AuthGuard implements CanActivate { async canActivate(context: ExecutionContext): Promise { const req = context.switchToHttp().getRequest() - const userId = await this.jwtExtractor.authenticate( + const { userId, isAdmin } = await this.jwtExtractor.authenticate( req.header("authorization"), ) @@ -24,7 +26,10 @@ export class AuthGuard implements CanActivate { user?: { sub: number; roles: string[] } } authenticatedReq.auth = { userId } - authenticatedReq.user = { sub: userId, roles: [] } + // Issue #511: roles are derived exclusively from the token's isAdmin + // claim — never from request headers. A token without the claim yields + // an empty role set, so RolesGuard denies admin-gated routes. + authenticatedReq.user = { sub: userId, roles: isAdmin ? ["admin"] : [] } return true } } diff --git a/api/src/common/guards/jwt-extractor.service.spec.ts b/api/src/common/guards/jwt-extractor.service.spec.ts new file mode 100644 index 0000000..affa4ed --- /dev/null +++ b/api/src/common/guards/jwt-extractor.service.spec.ts @@ -0,0 +1,92 @@ +import { UnauthorizedException } from "@nestjs/common" +import { JwtService } from "@nestjs/jwt" + +import { JwtExtractorService } from "./jwt-extractor.service" +import { TokenDenylistService } from "../../auth/token-denylist.service" +import { UsersRepository } from "../../auth/users.repository" + +describe("JwtExtractorService", () => { + const mockJwtService = { verifyAsync: jest.fn() } + const mockDenylist = { isRevoked: jest.fn() } + const mockUsersRepository = { findById: jest.fn() } + let service: JwtExtractorService + + beforeEach(() => { + jest.clearAllMocks() + mockJwtService.verifyAsync.mockReset() + mockDenylist.isRevoked.mockResolvedValue(false) + mockUsersRepository.findById.mockReset() + service = new JwtExtractorService( + mockJwtService as unknown as JwtService, + mockDenylist as unknown as TokenDenylistService, + mockUsersRepository as unknown as UsersRepository, + ) + }) + + it("returns the userId and isAdmin=true when the token carries the claim", async () => { + mockJwtService.verifyAsync.mockResolvedValue({ sub: 5, isAdmin: true }) + + await expect(service.authenticate("Bearer tok")).resolves.toEqual({ + userId: 5, + isAdmin: true, + }) + }) + + it("returns isAdmin=false when the token carries isAdmin=false", async () => { + mockJwtService.verifyAsync.mockResolvedValue({ sub: 5, isAdmin: false }) + + await expect(service.authenticate("Bearer tok")).resolves.toEqual({ + userId: 5, + isAdmin: false, + }) + }) + + it("treats a legacy token without the isAdmin claim as non-admin", async () => { + // Tokens minted before issue #511 land here — they must never grant + // admin access, so the default has to be false. + mockJwtService.verifyAsync.mockResolvedValue({ sub: 5 }) + + await expect(service.authenticate("Bearer tok")).resolves.toEqual({ + userId: 5, + isAdmin: false, + }) + }) + + it("rejects a revoked token", async () => { + mockJwtService.verifyAsync.mockResolvedValue({ sub: 5, jti: "abc" }) + mockDenylist.isRevoked.mockResolvedValue(true) + + await expect(service.authenticate("Bearer tok")).rejects.toThrow( + UnauthorizedException, + ) + }) + + it("rejects a payload with a non-integer subject", async () => { + mockJwtService.verifyAsync.mockResolvedValue({ sub: "not-a-number" }) + + await expect(service.authenticate("Bearer tok")).rejects.toThrow( + UnauthorizedException, + ) + }) + + it("rejects a request with no Bearer token", async () => { + await expect(service.authenticate(undefined)).rejects.toThrow( + UnauthorizedException, + ) + }) + + it("rejects a token minted before the user's last password change", async () => { + mockJwtService.verifyAsync.mockResolvedValue({ + sub: 5, + passwordChangedAt: 1000, + }) + mockUsersRepository.findById.mockResolvedValue({ + id: 5, + password_changed_at: new Date(2000), + }) + + await expect(service.authenticate("Bearer tok")).rejects.toThrow( + UnauthorizedException, + ) + }) +}) diff --git a/api/src/common/guards/jwt-extractor.service.ts b/api/src/common/guards/jwt-extractor.service.ts index de84d2c..949c5d3 100644 --- a/api/src/common/guards/jwt-extractor.service.ts +++ b/api/src/common/guards/jwt-extractor.service.ts @@ -1,5 +1,6 @@ import { Injectable, UnauthorizedException } from "@nestjs/common" import { JwtService } from "@nestjs/jwt" + import { TokenDenylistService } from "../../auth/token-denylist.service" import { UsersRepository } from "../../auth/users.repository" @@ -26,10 +27,16 @@ export class JwtExtractorService { * 4. Validate the `sub` claim resolves to a positive integer. * 5. Reject tokens minted before the user's last password change. * - * @returns The authenticated user's id. + * @returns The authenticated user's id and admin flag. The admin flag is + * read from the token's `isAdmin` claim and defaults to `false` + * for tokens minted before the claim existed, so legacy tokens + * can never grant admin access. * @throws UnauthorizedException at any step when credentials are invalid. */ - async authenticate(header: string | undefined): Promise { + async authenticate(header: string | undefined): Promise<{ + userId: number + isAdmin: boolean + }> { const token = this.extractBearerToken(header ?? "") const payload = await this.verifyToken(token) @@ -65,7 +72,13 @@ export class JwtExtractorService { } } - return userId + return { + userId, + // Default to false: tokens issued before the isAdmin claim existed + // (or tokens for users since demoted without re-login) must never + // carry admin privileges. + isAdmin: (payload as { isAdmin?: unknown }).isAdmin === true, + } } /** diff --git a/api/src/contract-provider.spec.ts b/api/src/contract-provider.spec.ts index 29693dc..33c2b7b 100644 --- a/api/src/contract-provider.spec.ts +++ b/api/src/contract-provider.spec.ts @@ -25,17 +25,18 @@ import { type Contract, } from "@xstreamroll/contract-tests" import request from "supertest" + import { AuditService } from "./audit/audit.service" import { AuthController } from "./auth/auth.controller" import { AuthService } from "./auth/auth.service" import { PasswordResetService } from "./auth/password-reset.service" import { TokenDenylistService } from "./auth/token-denylist.service" import { User, UsersRepository } from "./auth/users.repository" -import createJwtConfig, { createRefreshJwtConfig } from "./config/jwt.config" import { AuthGuard } from "./common/guards/auth.guard" import { JwtExtractorService } from "./common/guards/jwt-extractor.service" import { StreamOwnershipGuard } from "./common/guards/stream-ownership.guard" import { StreamOwnershipService } from "./common/guards/stream-ownership.service" +import createJwtConfig, { createRefreshJwtConfig } from "./config/jwt.config" import { StreamsRepository } from "./streams/repository/streams.repository" import { StreamsController } from "./streams/streams.controller" import { StreamsService } from "./streams/streams.service" @@ -73,6 +74,7 @@ class InMemoryUsersRepository { email, password_hash: passwordHash, created_at: new Date(), + is_admin: false, } this.byId.set(user.id, user) return user diff --git a/api/src/database.integration.spec.ts b/api/src/database.integration.spec.ts index c07b82a..02725b3 100644 --- a/api/src/database.integration.spec.ts +++ b/api/src/database.integration.spec.ts @@ -1,4 +1,5 @@ import { Pool } from "pg" + import { resetDb, createTestApp, @@ -37,6 +38,8 @@ describe("Database Integration Tests", () => { expect(columns).toContain("email") expect(columns).toContain("password_hash") expect(columns).toContain("created_at") + // Issue #511: single admin bit, defaulting to non-admin. + expect(columns).toContain("is_admin") }) it("has the streams table with foreign key to users", async () => { diff --git a/api/src/openapi-security.spec.ts b/api/src/openapi-security.spec.ts index f86f79f..b2bbc1f 100644 --- a/api/src/openapi-security.spec.ts +++ b/api/src/openapi-security.spec.ts @@ -5,6 +5,9 @@ import { Test } from "@nestjs/testing" import { AdminStatsService } from "./admin/admin-stats.service" import { AdminController } from "./admin/admin.controller" +import { AdminGuard } from "./common/auth/admin.guard" +import { RolesGuard } from "./common/auth/roles.guard" +import { AuthGuard } from "./common/guards/auth.guard" import { StreamsController } from "./streams/streams.controller" import { StreamsService } from "./streams/streams.service" @@ -44,6 +47,12 @@ async function buildOpenApiDoc(): Promise<{ paths: Record }> { { provide: StreamsService, useValue: {} }, { provide: AdminStatsService, useValue: {} }, { provide: CACHE_MANAGER, useValue: { get: jest.fn(), set: jest.fn() } }, + // AdminController is gated by AdminGuard (AuthGuard + RolesGuard); + // supply inert doubles so the Swagger doc can be built without + // booting the real auth pipeline. + { provide: AuthGuard, useValue: { canActivate: () => true } }, + { provide: RolesGuard, useValue: { canActivate: () => true } }, + { provide: AdminGuard, useValue: { canActivate: () => true } }, ], }).compile() diff --git a/api/src/users/users.service.ts b/api/src/users/users.service.ts index 6d52027..4362321 100644 --- a/api/src/users/users.service.ts +++ b/api/src/users/users.service.ts @@ -5,10 +5,11 @@ import { } from "@nestjs/common" import { JwtService } from "@nestjs/jwt" import * as bcrypt from "bcrypt" + +import { AuditService } from "../audit/audit.service" import { SafeUser, toSafeUser } from "../auth/auth.service" import { TokenDenylistService } from "../auth/token-denylist.service" import { User, UsersRepository } from "../auth/users.repository" -import { AuditService } from "../audit/audit.service" import { ChangePasswordDto } from "./dto/change-password.dto" import { UpdateProfileDto } from "./dto/update-profile.dto" @@ -116,6 +117,9 @@ export class UsersService { username: user.username, passwordChangedAt: user.password_changed_at?.getTime() ?? user.created_at.getTime(), + // Issue #511: reissued tokens (email/password change) must keep the + // admin claim so a re-login does not silently drop admin access. + isAdmin: user.is_admin === true, }) } diff --git a/database/migrations/2026082401_add_users_is_admin.down.sql b/database/migrations/2026082401_add_users_is_admin.down.sql new file mode 100644 index 0000000..4c46101 --- /dev/null +++ b/database/migrations/2026082401_add_users_is_admin.down.sql @@ -0,0 +1,3 @@ +-- Issue #511: rollback of the admin flag. Drops the column and any +-- admin grants made since the migration was applied. +ALTER TABLE users DROP COLUMN IF EXISTS is_admin; diff --git a/database/migrations/2026082401_add_users_is_admin.up.sql b/database/migrations/2026082401_add_users_is_admin.up.sql new file mode 100644 index 0000000..3dedf2e --- /dev/null +++ b/database/migrations/2026082401_add_users_is_admin.up.sql @@ -0,0 +1,4 @@ +-- Issue #511: single admin bit for the admin surface. +-- All existing users default to non-admin; the first admin is promoted +-- with a documented UPDATE (see database/migrations/README.md). +ALTER TABLE users ADD COLUMN IF NOT EXISTS is_admin BOOLEAN NOT NULL DEFAULT false; diff --git a/database/migrations/README.md b/database/migrations/README.md index 71864f2..a46e71b 100644 --- a/database/migrations/README.md +++ b/database/migrations/README.md @@ -86,6 +86,24 @@ psql -d "$DATABASE_URL" -f database/migrations/2026051501_add_stream_tags.down.s | `2026072501_add_composite_stream_events_index.up.sql` | `idx_stream_events_stream_id_created_at_desc` — composite index for the `WHERE stream_id = ? ORDER BY created_at DESC` query pattern | | `2026072801_alter_timestamp_to_timestamptz.up.sql` | Converts all `TIMESTAMP` columns to `TIMESTAMPTZ` across every table; rewrites `DEFAULT CURRENT_TIMESTAMP` → `DEFAULT NOW()` | | `2026080501_add_stream_visibility.up.sql` | `streams.visibility` (`public` \| `private`, default `private`), CHECK constraint, supporting index | +| `2026082401_add_users_is_admin.up.sql` | `users.is_admin` (`BOOLEAN NOT NULL DEFAULT false`) — single admin bit for the admin surface (issue #511) | + +## Promoting the first admin (issue #511) + +The admin flag lives on the `users` row, not in any config file, so the +only way to become an admin is a deliberate database write: + +```bash +psql "$DATABASE_URL" -c "UPDATE users SET is_admin = true WHERE email = 'you@example.com';" +``` + +The flag is read at access-token issuance: the user must **log in again** +after the `UPDATE` so the new token carries the `isAdmin` claim. Tokens +minted before the promotion do not grant admin access and expire within +15 minutes (`JWT_ACCESS_TOKEN_EXPIRES_IN`). + +Role management endpoints (assign/revoke admin over HTTP) are out of +scope for issue #511; the SQL above is the bootstrap path until they land. > **Note on `2026061001` / `2026061002`:** both migrations add the same > `users.password_hash` column. `2026061001_add_password_hash` is the diff --git a/database/schema.sql b/database/schema.sql index c157f95..ce62dae 100644 --- a/database/schema.sql +++ b/database/schema.sql @@ -6,6 +6,9 @@ CREATE TABLE IF NOT EXISTS users ( username VARCHAR(255) UNIQUE NOT NULL, email VARCHAR(255) UNIQUE NOT NULL, password_hash VARCHAR(255) NOT NULL, + -- Issue #511: single admin bit. Defaults to false; promote the first + -- admin with: UPDATE users SET is_admin = true WHERE email = '...'; + is_admin BOOLEAN NOT NULL DEFAULT false, created_at TIMESTAMPTZ DEFAULT NOW() ); diff --git a/docker-compose.yml b/docker-compose.yml index d36c4db..067eed0 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -57,7 +57,6 @@ services: THROTTLE_TTL: 60000 THROTTLE_LIMIT: 100 DB_STATEMENT_TIMEOUT_MS: 5000 - ALLOW_HEADER_ROLES: 1 depends_on: postgres: condition: service_healthy diff --git a/xstreamroll-sdk/scripts/generate-types.sh b/xstreamroll-sdk/scripts/generate-types.sh index 7d04473..42d35c8 100755 --- a/xstreamroll-sdk/scripts/generate-types.sh +++ b/xstreamroll-sdk/scripts/generate-types.sh @@ -30,7 +30,6 @@ if [ -z "$SPEC_JSON" ]; then process.env.STREAM_API_KEY = process.env.STREAM_API_KEY || 'dev-key'; process.env.JWT_SECRET = process.env.JWT_SECRET || 'dev-secret'; process.env.NODE_ENV = 'development'; - process.env.ALLOW_HEADER_ROLES = '1'; const { Test } = require('@nestjs/testing'); const { SwaggerModule, DocumentBuilder } = require('@nestjs/swagger');