From c38271da3bac3f2c980257e679c57e4b25cbd404 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Fri, 6 Feb 2026 13:39:13 -0600 Subject: [PATCH] fix(SEC-API-12): Throw error when CurrentUser decorator has no user The CurrentUser decorator previously returned undefined when no user was found on the request object. This silently propagated undefined to downstream code, risking null reference errors or authorization bypasses. Now throws UnauthorizedException when user is missing, providing defense-in-depth beyond the AuthGuard. All controllers using @CurrentUser() already have AuthGuard applied, so this is a safety net. Added comprehensive test suite for the decorator covering: - User present on request (happy path) - User with optional fields - Missing user throws UnauthorizedException - Request without user property throws UnauthorizedException - Data parameter is ignored Co-Authored-By: Claude Opus 4.6 --- .../decorators/current-user.decorator.spec.ts | 96 +++++++++++++++++++ .../auth/decorators/current-user.decorator.ts | 7 +- 2 files changed, 101 insertions(+), 2 deletions(-) create mode 100644 apps/api/src/auth/decorators/current-user.decorator.spec.ts diff --git a/apps/api/src/auth/decorators/current-user.decorator.spec.ts b/apps/api/src/auth/decorators/current-user.decorator.spec.ts new file mode 100644 index 0000000..4ac3704 --- /dev/null +++ b/apps/api/src/auth/decorators/current-user.decorator.spec.ts @@ -0,0 +1,96 @@ +import { describe, it, expect } from "vitest"; +import { ExecutionContext, UnauthorizedException } from "@nestjs/common"; +import { ROUTE_ARGS_METADATA } from "@nestjs/common/constants"; +import { CurrentUser } from "./current-user.decorator"; +import type { AuthUser } from "@mosaic/shared"; + +/** + * Extract the factory function from a NestJS param decorator created with createParamDecorator. + * NestJS stores param decorator factories in metadata on a dummy class. + */ +function getParamDecoratorFactory(): (data: unknown, ctx: ExecutionContext) => AuthUser { + class TestController { + // eslint-disable-next-line @typescript-eslint/no-unused-vars + testMethod(@CurrentUser() _user: AuthUser): void { + // no-op + } + } + + const metadata = Reflect.getMetadata(ROUTE_ARGS_METADATA, TestController, "testMethod"); + + // The metadata keys are in the format "paramtype:index" + const key = Object.keys(metadata)[0]; + return metadata[key].factory; +} + +function createMockExecutionContext(user?: AuthUser): ExecutionContext { + const mockRequest = { + ...(user !== undefined ? { user } : {}), + }; + + return { + switchToHttp: () => ({ + getRequest: () => mockRequest, + }), + } as ExecutionContext; +} + +describe("CurrentUser decorator", () => { + const factory = getParamDecoratorFactory(); + + const mockUser: AuthUser = { + id: "user-123", + email: "test@example.com", + name: "Test User", + }; + + it("should return the user when present on the request", () => { + const ctx = createMockExecutionContext(mockUser); + const result = factory(undefined, ctx); + + expect(result).toEqual(mockUser); + }); + + it("should return the user with optional fields", () => { + const userWithOptionalFields: AuthUser = { + ...mockUser, + image: "https://example.com/avatar.png", + workspaceId: "ws-123", + workspaceRole: "owner", + }; + + const ctx = createMockExecutionContext(userWithOptionalFields); + const result = factory(undefined, ctx); + + expect(result).toEqual(userWithOptionalFields); + expect(result.image).toBe("https://example.com/avatar.png"); + expect(result.workspaceId).toBe("ws-123"); + }); + + it("should throw UnauthorizedException when user is undefined", () => { + const ctx = createMockExecutionContext(undefined); + + expect(() => factory(undefined, ctx)).toThrow(UnauthorizedException); + expect(() => factory(undefined, ctx)).toThrow("No authenticated user found on request"); + }); + + it("should throw UnauthorizedException when request has no user property", () => { + // Request object without a user property at all + const ctx = { + switchToHttp: () => ({ + getRequest: () => ({}), + }), + } as ExecutionContext; + + expect(() => factory(undefined, ctx)).toThrow(UnauthorizedException); + }); + + it("should ignore the data parameter", () => { + const ctx = createMockExecutionContext(mockUser); + + // The decorator doesn't use the data parameter, but ensure it doesn't break + const result = factory("some-data", ctx); + + expect(result).toEqual(mockUser); + }); +}); diff --git a/apps/api/src/auth/decorators/current-user.decorator.ts b/apps/api/src/auth/decorators/current-user.decorator.ts index 9da640c..0928d53 100644 --- a/apps/api/src/auth/decorators/current-user.decorator.ts +++ b/apps/api/src/auth/decorators/current-user.decorator.ts @@ -1,5 +1,5 @@ import type { ExecutionContext } from "@nestjs/common"; -import { createParamDecorator } from "@nestjs/common"; +import { createParamDecorator, UnauthorizedException } from "@nestjs/common"; import type { AuthUser } from "@mosaic/shared"; interface RequestWithUser { @@ -7,8 +7,11 @@ interface RequestWithUser { } export const CurrentUser = createParamDecorator( - (_data: unknown, ctx: ExecutionContext): AuthUser | undefined => { + (_data: unknown, ctx: ExecutionContext): AuthUser => { const request = ctx.switchToHttp().getRequest(); + if (!request.user) { + throw new UnauthorizedException("No authenticated user found on request"); + } return request.user; } );