refactor(#411): QA-011 — unify request-with-user types into AuthenticatedRequest
Replace 4 redundant request interfaces (RequestWithSession, AuthRequest, BetterAuthRequest, RequestWithUser) with AuthenticatedRequest and MaybeAuthenticatedRequest in apps/api/src/auth/types/. - AuthenticatedRequest: extends Express Request with non-optional user/session (used in controllers behind AuthGuard) - MaybeAuthenticatedRequest: extends Express Request with optional user/session (used in AuthGuard and CurrentUser decorator before auth is confirmed) - Removed dead-code null checks in getSession (AuthGuard guarantees presence) - Fixed cookies type safety in AuthGuard (cast from any to Record) - Updated test expectations to match new type contract Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -287,41 +287,9 @@ describe("AuthController", () => {
|
||||
expect(result).toEqual(expected);
|
||||
});
|
||||
|
||||
it("should throw HttpException(401) if user not found in request", () => {
|
||||
const mockRequest = {
|
||||
session: {
|
||||
id: "session-123",
|
||||
token: "session-token",
|
||||
expiresAt: new Date(),
|
||||
},
|
||||
};
|
||||
|
||||
expect(() => controller.getSession(mockRequest)).toThrow(HttpException);
|
||||
try {
|
||||
controller.getSession(mockRequest);
|
||||
} catch (err) {
|
||||
expect((err as HttpException).getStatus()).toBe(HttpStatus.UNAUTHORIZED);
|
||||
expect((err as HttpException).getResponse()).toBe("User session not found");
|
||||
}
|
||||
});
|
||||
|
||||
it("should throw HttpException(401) if session not found in request", () => {
|
||||
const mockRequest = {
|
||||
user: {
|
||||
id: "user-123",
|
||||
email: "test@example.com",
|
||||
name: "Test User",
|
||||
},
|
||||
};
|
||||
|
||||
expect(() => controller.getSession(mockRequest)).toThrow(HttpException);
|
||||
try {
|
||||
controller.getSession(mockRequest);
|
||||
} catch (err) {
|
||||
expect((err as HttpException).getStatus()).toBe(HttpStatus.UNAUTHORIZED);
|
||||
expect((err as HttpException).getResponse()).toBe("User session not found");
|
||||
}
|
||||
});
|
||||
// Note: Tests for missing user/session were removed because
|
||||
// AuthenticatedRequest guarantees both are present (enforced by AuthGuard).
|
||||
// NestJS returns 401 before getSession is reached if the guard rejects.
|
||||
});
|
||||
|
||||
describe("getProfile", () => {
|
||||
|
||||
@@ -18,16 +18,7 @@ import { AuthService } from "./auth.service";
|
||||
import { AuthGuard } from "./guards/auth.guard";
|
||||
import { CurrentUser } from "./decorators/current-user.decorator";
|
||||
import { SkipCsrf } from "../common/decorators/skip-csrf.decorator";
|
||||
|
||||
interface RequestWithSession {
|
||||
user?: AuthUser;
|
||||
session?: {
|
||||
id: string;
|
||||
token: string;
|
||||
expiresAt: Date;
|
||||
[key: string]: unknown;
|
||||
};
|
||||
}
|
||||
import type { AuthenticatedRequest } from "./types/better-auth-request.interface";
|
||||
|
||||
@Controller("auth")
|
||||
export class AuthController {
|
||||
@@ -41,12 +32,9 @@ export class AuthController {
|
||||
*/
|
||||
@Get("session")
|
||||
@UseGuards(AuthGuard)
|
||||
getSession(@Request() req: RequestWithSession): AuthSession {
|
||||
if (!req.user || !req.session) {
|
||||
// This should never happen after AuthGuard, but TypeScript needs the check
|
||||
throw new HttpException("User session not found", HttpStatus.UNAUTHORIZED);
|
||||
}
|
||||
|
||||
getSession(@Request() req: AuthenticatedRequest): AuthSession {
|
||||
// AuthGuard guarantees user and session are present — NestJS returns 401
|
||||
// before this method is reached if the guard rejects.
|
||||
return {
|
||||
user: req.user,
|
||||
session: {
|
||||
@@ -140,12 +128,12 @@ export class AuthController {
|
||||
if (!res.headersSent) {
|
||||
throw new HttpException(
|
||||
"Unable to complete authentication. Please try again in a moment.",
|
||||
HttpStatus.INTERNAL_SERVER_ERROR,
|
||||
HttpStatus.INTERNAL_SERVER_ERROR
|
||||
);
|
||||
}
|
||||
|
||||
this.logger.error(
|
||||
`Headers already sent for failed auth request ${req.method} ${req.url} — client may have received partial response`,
|
||||
`Headers already sent for failed auth request ${req.method} ${req.url} — client may have received partial response`
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,14 +1,13 @@
|
||||
import type { ExecutionContext } from "@nestjs/common";
|
||||
import { createParamDecorator, UnauthorizedException } from "@nestjs/common";
|
||||
import type { AuthUser } from "@mosaic/shared";
|
||||
|
||||
interface RequestWithUser {
|
||||
user?: AuthUser;
|
||||
}
|
||||
import type { MaybeAuthenticatedRequest } from "../types/better-auth-request.interface";
|
||||
|
||||
export const CurrentUser = createParamDecorator(
|
||||
(_data: unknown, ctx: ExecutionContext): AuthUser => {
|
||||
const request = ctx.switchToHttp().getRequest<RequestWithUser>();
|
||||
// Use MaybeAuthenticatedRequest because the decorator doesn't know
|
||||
// whether AuthGuard ran — the null check provides defense-in-depth.
|
||||
const request = ctx.switchToHttp().getRequest<MaybeAuthenticatedRequest>();
|
||||
if (!request.user) {
|
||||
throw new UnauthorizedException("No authenticated user found on request");
|
||||
}
|
||||
|
||||
@@ -1,23 +1,14 @@
|
||||
import { Injectable, CanActivate, ExecutionContext, UnauthorizedException } from "@nestjs/common";
|
||||
import { AuthService } from "../auth.service";
|
||||
import type { AuthUser } from "@mosaic/shared";
|
||||
|
||||
/**
|
||||
* Request type with authentication context
|
||||
*/
|
||||
interface AuthRequest {
|
||||
user?: AuthUser;
|
||||
session?: Record<string, unknown>;
|
||||
headers: Record<string, string | string[] | undefined>;
|
||||
cookies?: Record<string, string>;
|
||||
}
|
||||
import type { MaybeAuthenticatedRequest } from "../types/better-auth-request.interface";
|
||||
|
||||
@Injectable()
|
||||
export class AuthGuard implements CanActivate {
|
||||
constructor(private readonly authService: AuthService) {}
|
||||
|
||||
async canActivate(context: ExecutionContext): Promise<boolean> {
|
||||
const request = context.switchToHttp().getRequest<AuthRequest>();
|
||||
const request = context.switchToHttp().getRequest<MaybeAuthenticatedRequest>();
|
||||
|
||||
// Try to get token from either cookie (preferred) or Authorization header
|
||||
const token = this.extractToken(request);
|
||||
@@ -56,7 +47,7 @@ export class AuthGuard implements CanActivate {
|
||||
/**
|
||||
* Extract token from cookie (preferred) or Authorization header
|
||||
*/
|
||||
private extractToken(request: AuthRequest): string | undefined {
|
||||
private extractToken(request: MaybeAuthenticatedRequest): string | undefined {
|
||||
// Try cookie first (BetterAuth default)
|
||||
const cookieToken = this.extractTokenFromCookie(request);
|
||||
if (cookieToken) {
|
||||
@@ -70,19 +61,21 @@ export class AuthGuard implements CanActivate {
|
||||
/**
|
||||
* Extract token from cookie (BetterAuth stores session token in better-auth.session_token cookie)
|
||||
*/
|
||||
private extractTokenFromCookie(request: AuthRequest): string | undefined {
|
||||
if (!request.cookies) {
|
||||
private extractTokenFromCookie(request: MaybeAuthenticatedRequest): string | undefined {
|
||||
// Express types `cookies` as `any`; cast to a known shape for type safety.
|
||||
const cookies = request.cookies as Record<string, string> | undefined;
|
||||
if (!cookies) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
// BetterAuth uses 'better-auth.session_token' as the cookie name by default
|
||||
return request.cookies["better-auth.session_token"];
|
||||
return cookies["better-auth.session_token"];
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract token from Authorization header (Bearer token)
|
||||
*/
|
||||
private extractTokenFromHeader(request: AuthRequest): string | undefined {
|
||||
private extractTokenFromHeader(request: MaybeAuthenticatedRequest): string | undefined {
|
||||
const authHeader = request.headers.authorization;
|
||||
if (typeof authHeader !== "string") {
|
||||
return undefined;
|
||||
|
||||
@@ -1,11 +1,14 @@
|
||||
/**
|
||||
* BetterAuth Request Type
|
||||
* Unified request types for authentication context.
|
||||
*
|
||||
* BetterAuth expects a Request object compatible with the Fetch API standard.
|
||||
* This extends the web standard Request interface with additional properties
|
||||
* that may be present in the Express request object at runtime.
|
||||
* Replaces the previously scattered interfaces:
|
||||
* - RequestWithSession (auth.controller.ts)
|
||||
* - AuthRequest (auth.guard.ts)
|
||||
* - BetterAuthRequest (this file, removed)
|
||||
* - RequestWithUser (current-user.decorator.ts)
|
||||
*/
|
||||
|
||||
import type { Request } from "express";
|
||||
import type { AuthUser } from "@mosaic/shared";
|
||||
|
||||
// Re-export AuthUser for use in other modules
|
||||
@@ -22,19 +25,21 @@ export interface RequestSession {
|
||||
}
|
||||
|
||||
/**
|
||||
* Web standard Request interface extended with Express-specific properties
|
||||
* This matches the Fetch API Request specification that BetterAuth expects.
|
||||
* Request that may or may not have auth data (before guard runs).
|
||||
* Used by AuthGuard and other middleware that processes requests
|
||||
* before authentication is confirmed.
|
||||
*/
|
||||
export interface BetterAuthRequest extends Request {
|
||||
// Express route parameters
|
||||
params?: Record<string, string>;
|
||||
|
||||
// Express query string parameters
|
||||
query?: Record<string, string | string[]>;
|
||||
|
||||
// Session data attached by AuthGuard after successful authentication
|
||||
session?: RequestSession;
|
||||
|
||||
// Authenticated user attached by AuthGuard
|
||||
export interface MaybeAuthenticatedRequest extends Request {
|
||||
user?: AuthUser;
|
||||
session?: Record<string, unknown>;
|
||||
}
|
||||
|
||||
/**
|
||||
* Request with authenticated user attached by AuthGuard.
|
||||
* After AuthGuard runs, user and session are guaranteed present.
|
||||
* Use this type in controllers/decorators that sit behind AuthGuard.
|
||||
*/
|
||||
export interface AuthenticatedRequest extends Request {
|
||||
user: AuthUser;
|
||||
session: RequestSession;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user