chore: remove cors and implement helmet (#1219)

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **New Features**
- Enhanced API security by incorporating advanced HTTP header
protection.
- Streamlined CORS configuration to allow broader client compatibility
with defined methods and headers.
- Improved cookie handling in API requests for more robust and reliable
processing.

- **Bug Fixes**
- Resolved potential issues related to cookie validation during
cross-origin requests.

- **Type Safety Improvements**
- Enhanced type definitions for cookie handling methods to ensure
clarity and prevent errors related to undefined values.
- Introduced a mock request object for improved testing of cookie
validation scenarios.
- Updated request handling to enforce the presence of cookies and
headers in API requests.
- Updated import paths for core Fastify types to reflect new
organizational structure.
  
- **Security Changes**
- Removed authentication and rate limiting features from the API key
resolver methods.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: mdatelle <mike@datelle.net>
This commit is contained in:
Michael Datelle
2025-03-11 12:56:50 -04:00
committed by GitHub
parent c8d20eb01c
commit 3ea3953f4c
15 changed files with 163 additions and 48 deletions

View File

@@ -52,6 +52,7 @@
"@apollo/server": "^4.11.2",
"@as-integrations/fastify": "^2.1.1",
"@fastify/cookie": "^9.4.0",
"@fastify/helmet": "^13.0.1",
"@graphql-codegen/client-preset": "^4.5.0",
"@graphql-tools/load-files": "^7.0.0",
"@graphql-tools/merge": "^9.0.8",
@@ -174,8 +175,8 @@
"cz-conventional-changelog": "3.3.0",
"eslint": "^9.20.1",
"eslint-plugin-import": "^2.31.0",
"eslint-plugin-no-relative-import-paths": "^1.6.1",
"eslint-plugin-n": "^17.0.0",
"eslint-plugin-no-relative-import-paths": "^1.6.1",
"eslint-plugin-prettier": "^5.2.3",
"graphql-codegen-typescript-validation-schema": "^0.17.0",
"jiti": "^2.4.0",

View File

@@ -1,9 +0,0 @@
import type {
FastifyInstance as BaseFastifyInstance,
FastifyReply as BaseFastifyReply,
FastifyRequest as BaseFastifyRequest,
} from 'fastify';
export type FastifyInstance = BaseFastifyInstance;
export type FastifyRequest = BaseFastifyRequest;
export type FastifyReply = BaseFastifyReply;

View File

@@ -5,8 +5,8 @@ import { GraphQLError } from 'graphql';
import { getAllowedOrigins } from '@app/common/allowed-origins.js';
import { apiLogger } from '@app/core/log.js';
import { BYPASS_CORS_CHECKS } from '@app/environment.js';
import { FastifyRequest } from '@app/types/fastify.js';
import { type CookieService } from '@app/unraid-api/auth/cookie.service.js';
import { FastifyRequest } from '@app/unraid-api/types/fastify.js';
/**
* Returns whether the origin is allowed to access the API.
@@ -69,7 +69,7 @@ export const configureFastifyCors =
*/
(req: FastifyRequest, callback: (error: Error | null, options: CorsOptions) => void) => {
const { cookies } = req;
if (typeof cookies === 'object') {
if (cookies && typeof cookies === 'object') {
service.hasValidAuthCookie(cookies).then((isValid) => {
if (isValid) {
callback(null, { credentials: true, origin: true });

View File

@@ -8,7 +8,7 @@ import type { IncomingMessage } from 'http';
import type { Observable } from 'rxjs';
import { parse as parseCookies } from 'cookie';
import type { FastifyRequest } from '@app/types/fastify.js';
import type { FastifyRequest } from '@app/unraid-api/types/fastify.js';
import { apiLogger } from '@app/core/log.js';
import { UserCookieStrategy } from '@app/unraid-api/auth/cookie.strategy.js';
import { ServerHeaderStrategy } from '@app/unraid-api/auth/header.strategy.js';

View File

@@ -9,6 +9,7 @@ import { Resource, Role } from '@app/graphql/generated/api/types.js';
import { ApiKeyService } from '@app/unraid-api/auth/api-key.service.js';
import { AuthService } from '@app/unraid-api/auth/auth.service.js';
import { CookieService } from '@app/unraid-api/auth/cookie.service.js';
import { FastifyRequest } from '@app/unraid-api/types/fastify.js';
describe('AuthService', () => {
let authService: AuthService;
@@ -48,6 +49,15 @@ describe('AuthService', () => {
roles: [Role.GUEST, Role.CONNECT],
};
// Mock FastifyRequest object for tests
const createMockRequest = (overrides = {}) =>
({
headers: { 'x-csrf-token': undefined },
query: { csrf_token: undefined },
cookies: {},
...overrides,
}) as FastifyRequest;
beforeEach(async () => {
const enforcer = await newEnforcer();
@@ -66,36 +76,77 @@ describe('AuthService', () => {
vi.spyOn(cookieService, 'hasValidAuthCookie').mockResolvedValue(true);
vi.spyOn(authService, 'getSessionUser').mockResolvedValue(mockUser);
vi.spyOn(authzService, 'getRolesForUser').mockResolvedValue([Role.ADMIN]);
vi.spyOn(authService, 'validateCsrfToken').mockReturnValue(true);
const result = await authService.validateCookiesCasbin({});
const mockRequest = createMockRequest({
headers: { 'x-csrf-token': 'valid-token' },
});
const result = await authService.validateCookiesWithCsrfToken(mockRequest);
expect(result).toEqual(mockUser);
});
it('should throw UnauthorizedException when auth cookie is invalid', async () => {
vi.spyOn(cookieService, 'hasValidAuthCookie').mockResolvedValue(false);
vi.spyOn(authService, 'validateCsrfToken').mockReturnValue(true);
await expect(authService.validateCookiesCasbin({})).rejects.toThrow(UnauthorizedException);
const mockRequest = createMockRequest({
headers: { 'x-csrf-token': 'valid-token' },
});
await expect(authService.validateCookiesWithCsrfToken(mockRequest)).rejects.toThrow(
UnauthorizedException
);
});
it('should throw UnauthorizedException when session user is missing', async () => {
vi.spyOn(cookieService, 'hasValidAuthCookie').mockResolvedValue(true);
vi.spyOn(authService, 'getSessionUser').mockResolvedValue(null as unknown as UserAccount);
vi.spyOn(authService, 'validateCsrfToken').mockReturnValue(true);
await expect(authService.validateCookiesCasbin({})).rejects.toThrow(UnauthorizedException);
const mockRequest = createMockRequest();
await expect(authService.validateCookiesWithCsrfToken(mockRequest)).rejects.toThrow(
UnauthorizedException
);
});
it('should add guest role when user has no roles', async () => {
vi.spyOn(cookieService, 'hasValidAuthCookie').mockResolvedValue(true);
vi.spyOn(authService, 'getSessionUser').mockResolvedValue(mockUser);
vi.spyOn(authzService, 'getRolesForUser').mockResolvedValue([]);
vi.spyOn(authService, 'validateCsrfToken').mockReturnValue(true);
const addRoleSpy = vi.spyOn(authzService, 'addRoleForUser');
const result = await authService.validateCookiesCasbin({});
const mockRequest = createMockRequest();
const result = await authService.validateCookiesWithCsrfToken(mockRequest);
expect(result).toEqual(mockUser);
expect(addRoleSpy).toHaveBeenCalledWith(mockUser.id, 'guest');
});
it('should throw UnauthorizedException when CSRF token is invalid', async () => {
vi.spyOn(authService, 'validateCsrfToken').mockReturnValue(false);
const mockRequest = createMockRequest({
headers: { 'x-csrf-token': 'invalid-token' },
});
await expect(authService.validateCookiesWithCsrfToken(mockRequest)).rejects.toThrow(
new UnauthorizedException('Invalid CSRF token')
);
});
it('should accept CSRF token from query parameter', async () => {
vi.spyOn(cookieService, 'hasValidAuthCookie').mockResolvedValue(true);
vi.spyOn(authService, 'getSessionUser').mockResolvedValue(mockUser);
vi.spyOn(authzService, 'getRolesForUser').mockResolvedValue([Role.ADMIN]);
vi.spyOn(authService, 'validateCsrfToken').mockReturnValue(true);
const mockRequest = createMockRequest({
query: { csrf_token: 'valid-token' },
});
const result = await authService.validateCookiesWithCsrfToken(mockRequest);
expect(result).toEqual(mockUser);
});
});
describe('syncApiKeyRoles', () => {

View File

@@ -7,6 +7,7 @@ import { Role } from '@app/graphql/generated/api/types.js';
import { getters } from '@app/store/index.js';
import { ApiKeyService } from '@app/unraid-api/auth/api-key.service.js';
import { CookieService } from '@app/unraid-api/auth/cookie.service.js';
import { FastifyRequest } from '@app/unraid-api/types/fastify.js';
import { batchProcess, handleAuthError } from '@app/utils.js';
@Injectable()
@@ -48,9 +49,13 @@ export class AuthService {
}
}
async validateCookiesCasbin(cookies: object): Promise<UserAccount> {
async validateCookiesWithCsrfToken(request: FastifyRequest): Promise<UserAccount> {
try {
if (!(await this.cookieService.hasValidAuthCookie(cookies))) {
if (!this.validateCsrfToken(request.headers['x-csrf-token'] || request.query.csrf_token)) {
throw new UnauthorizedException('Invalid CSRF token');
}
if (!(await this.cookieService.hasValidAuthCookie(request.cookies))) {
throw new UnauthorizedException('No user session found');
}

View File

@@ -43,9 +43,9 @@ export class CookieService {
* @param opts optional overrides for the session directory & prefix of the session cookie to look for
* @returns true if any of the cookies are a valid unraid session cookie, false otherwise
*/
async hasValidAuthCookie(cookies: object): Promise<boolean> {
async hasValidAuthCookie(cookies: Record<string, string | undefined>): Promise<boolean> {
const { data } = await batchProcess(Object.entries(cookies), ([cookieName, cookieValue]) =>
this.isValidAuthCookie(String(cookieName), String(cookieValue))
this.isValidAuthCookie(String(cookieName), String(cookieValue ?? ''))
);
return data.some((valid) => valid);
}

View File

@@ -1,27 +1,22 @@
import { Injectable, Logger } from '@nestjs/common';
import { Injectable } from '@nestjs/common';
import { PassportStrategy } from '@nestjs/passport';
import { Strategy } from 'passport-custom';
import type { CustomRequest } from '@app/unraid-api/types/request.js';
import { AuthService } from '@app/unraid-api/auth/auth.service.js';
import { FastifyRequest } from '@app/unraid-api/types/fastify.js';
const strategyName = 'user-cookie';
@Injectable()
export class UserCookieStrategy extends PassportStrategy(Strategy, strategyName) {
static key = strategyName;
private readonly logger = new Logger(UserCookieStrategy.name);
constructor(private authService: AuthService) {
super();
}
public validate = async (req: CustomRequest): Promise<any> => {
return (
this.authService.validateCsrfToken(
req.headers['x-csrf-token'] || (req.query as { csrf_token?: string })?.csrf_token
) && this.authService.validateCookiesCasbin(req.cookies)
);
public validate = async (request: FastifyRequest): Promise<any> => {
return this.authService.validateCookiesWithCsrfToken(request);
};
}

View File

@@ -1,6 +1,4 @@
import { UseGuards } from '@nestjs/common';
import { Args, Mutation, Query, Resolver } from '@nestjs/graphql';
import { Throttle } from '@nestjs/throttler';
import { AuthActionVerb, AuthPossession, UsePermissions } from 'nest-authz';
@@ -13,12 +11,9 @@ import type {
} from '@app/graphql/generated/api/types.js';
import { Resource, Role } from '@app/graphql/generated/api/types.js';
import { ApiKeyService } from '@app/unraid-api/auth/api-key.service.js';
import { GraphqlAuthGuard } from '@app/unraid-api/auth/auth.guard.js';
import { AuthService } from '@app/unraid-api/auth/auth.service.js';
@Resolver('ApiKey')
@UseGuards(GraphqlAuthGuard)
@Throttle({ default: { limit: 100, ttl: 60000 } }) // 100 requests per minute
export class ApiKeyResolver {
constructor(
private authService: AuthService,

View File

@@ -1,15 +1,14 @@
import type { NestFastifyApplication } from '@nestjs/platform-fastify';
import { NestFactory } from '@nestjs/core';
import { FastifyAdapter } from '@nestjs/platform-fastify';
import { FastifyAdapter } from '@nestjs/platform-fastify/adapters';
import fastifyCookie from '@fastify/cookie';
import fastifyHelmet from '@fastify/helmet';
import { LoggerErrorInterceptor, Logger as PinoLogger } from 'nestjs-pino';
import { apiLogger } from '@app/core/log.js';
import { LOG_LEVEL, PORT } from '@app/environment.js';
import { AppModule } from '@app/unraid-api/app/app.module.js';
import { configureFastifyCors } from '@app/unraid-api/app/cors.js';
import { CookieService } from '@app/unraid-api/auth/cookie.service.js';
import { GraphQLExceptionsFilter } from '@app/unraid-api/exceptions/graphql-exceptions.filter.js';
import { HttpExceptionFilter } from '@app/unraid-api/exceptions/http-exceptions.filter.js';
@@ -23,10 +22,38 @@ export async function bootstrapNestServer(): Promise<NestFastifyApplication> {
const server = app.getHttpAdapter().getInstance();
await app.register(fastifyCookie); // parse cookies before cors
await server.register(fastifyCookie);
const cookieService = app.get(CookieService);
app.enableCors(configureFastifyCors(cookieService));
// Minimal Helmet configuration to avoid blocking plugin functionality
await server.register(fastifyHelmet, {
// Disable restrictive policies
contentSecurityPolicy: false,
crossOriginEmbedderPolicy: false,
crossOriginOpenerPolicy: false,
crossOriginResourcePolicy: false,
// Basic security headers that don't restrict functionality
xssFilter: true,
hidePoweredBy: true,
// Additional safe headers
noSniff: true, // Prevents MIME type sniffing
ieNoOpen: true, // Prevents IE from executing downloads in site context
permittedCrossDomainPolicies: true, // Restricts Adobe Flash and PDF access
referrerPolicy: { policy: 'no-referrer-when-downgrade' }, // Safe referrer policy
frameguard: false, // Turn off for plugin compatibility
// HSTS disabled to avoid issues with running on local networks
hsts: false,
});
// Allows all origins but still checks authentication
app.enableCors({
origin: true, // Allows all origins
credentials: true,
methods: ['GET', 'PUT', 'POST', 'DELETE', 'OPTIONS'],
allowedHeaders: ['Content-Type', 'Authorization', 'X-Requested-With'],
});
// Setup Nestjs Pino Logger
app.useLogger(app.get(PinoLogger));

View File

@@ -2,7 +2,7 @@ import { Controller, Get, Logger, Param, Res } from '@nestjs/common';
import { AuthActionVerb, AuthPossession, UsePermissions } from 'nest-authz';
import type { FastifyReply } from '@app/types/fastify.js';
import type { FastifyReply } from '@app/unraid-api/types/fastify.js';
import { Resource } from '@app/graphql/generated/api/types.js';
import { Public } from '@app/unraid-api/auth/public.decorator.js';
import { RestService } from '@app/unraid-api/rest/rest.service.js';

View File

@@ -0,0 +1,33 @@
import type {
FastifyInstance as BaseFastifyInstance,
FastifyReply as BaseFastifyReply,
FastifyRequest as BaseFastifyRequest,
} from 'fastify';
// Common headers
export interface CommonHeaders {
'x-api-key'?: string;
'x-csrf-token'?: string;
'x-unraid-api-version'?: string;
'x-flash-guid'?: string;
}
// Common query parameters
export interface CommonQuery {
csrf_token?: string;
}
// Base types
type Headers = BaseFastifyRequest['headers'] & Partial<CommonHeaders>;
type Query = BaseFastifyRequest['query'] & Partial<CommonQuery>;
type Cookies = BaseFastifyRequest['cookies'];
export type FastifyRequest = BaseFastifyRequest<{
Headers: Headers;
Querystring: Query;
}> & {
cookies?: Cookies;
};
export type FastifyInstance = BaseFastifyInstance;
export type FastifyReply = BaseFastifyReply;

View File

@@ -1,5 +0,0 @@
import type { FastifyRequest } from '@app/types/fastify.js';
export interface CustomRequest extends FastifyRequest {
headers: FastifyRequest['headers'] & { 'x-csrf-token'?: string };
}

View File

@@ -6,7 +6,7 @@ import { dirname } from 'node:path';
import strftime from 'strftime';
import { UserAccount } from '@app/graphql/generated/api/types.js';
import { FastifyRequest } from '@app/types/fastify.js';
import { FastifyRequest } from '@app/unraid-api/types/fastify.js';
export function notNull<T>(value: T): value is NonNullable<T> {
return value !== null;

22
pnpm-lock.yaml generated
View File

@@ -26,6 +26,9 @@ importers:
'@fastify/cookie':
specifier: ^9.4.0
version: 9.4.0
'@fastify/helmet':
specifier: ^13.0.1
version: 13.0.1
'@graphql-codegen/client-preset':
specifier: ^4.5.0
version: 4.6.3(graphql@16.10.0)
@@ -1831,6 +1834,9 @@ packages:
'@fastify/formbody@7.4.0':
resolution: {integrity: sha512-H3C6h1GN56/SMrZS8N2vCT2cZr7mIHzBHzOBa5OPpjfB/D6FzP9mMpE02ZzrFX0ANeh0BAJdoXKOF2e7IbV+Og==}
'@fastify/helmet@13.0.1':
resolution: {integrity: sha512-i+ifqazG3d0HwHL3zuZdg6B/WPc9Ee6kVfGpwGho4nxm0UaK1htss0zq+1rVhOoAorZlCgTZ3/i4S58hUGkkoA==}
'@fastify/merge-json-schemas@0.1.1':
resolution: {integrity: sha512-fERDVz7topgNjtXsJTTW1JKLy0rhuLRcquYqNR9rF7OcVpCa2OVW49ZPDIhaRRCaUuvVxI+N416xUoF76HNSXA==}
@@ -6218,6 +6224,9 @@ packages:
fastify-plugin@4.5.1:
resolution: {integrity: sha512-stRHYGeuqpEZTL1Ef0Ovr2ltazUT9g844X5z/zEBFLG8RYlpDiOCIG+ATvYEp+/zmc7sN29mcIMp8gvYplYPIQ==}
fastify-plugin@5.0.1:
resolution: {integrity: sha512-HCxs+YnRaWzCl+cWRYFnHmeRFyR5GVnJTAaCJQiYzQSDwK9MgJdyAsuL3nh0EWRCYMgQ5MeziymvmAhUHYHDUQ==}
fastify@4.28.1:
resolution: {integrity: sha512-kFWUtpNr4i7t5vY2EJPCN2KgMVpuqfU4NjnJNCgiNB900oiDeYqaNDRcAfeBbOF5hGixixxcKnOU4KN9z6QncQ==}
@@ -6790,6 +6799,10 @@ packages:
header-case@2.0.4:
resolution: {integrity: sha512-H/vuk5TEEVZwrR0lp2zed9OCo1uAILMlx0JEMgC26rzyJJ3N1v6XkwHHXJQdR2doSjcGPM6OKPYoJgf0plJ11Q==}
helmet@8.0.0:
resolution: {integrity: sha512-VyusHLEIIO5mjQPUI1wpOAEu+wl6Q0998jzTxqUYGE45xCIcAxy3MsbEK/yyJUJ3ADeMoB6MornPH6GMWAf+Pw==}
engines: {node: '>=18.0.0'}
help-me@5.0.0:
resolution: {integrity: sha512-7xgomUX6ADmcYzFik0HzAxh/73YlKR9bmFzf51CZwR+b6YtzU2m0u49hQCqV6SvlqIqsaxovfwdvbnsw3b/zpg==}
@@ -12442,6 +12455,11 @@ snapshots:
fast-querystring: 1.1.2
fastify-plugin: 4.5.1
'@fastify/helmet@13.0.1':
dependencies:
fastify-plugin: 5.0.1
helmet: 8.0.0
'@fastify/merge-json-schemas@0.1.1':
dependencies:
fast-deep-equal: 3.1.3
@@ -18096,6 +18114,8 @@ snapshots:
fastify-plugin@4.5.1: {}
fastify-plugin@5.0.1: {}
fastify@4.28.1:
dependencies:
'@fastify/ajv-compiler': 3.6.0
@@ -18760,6 +18780,8 @@ snapshots:
capital-case: 1.0.4
tslib: 2.8.1
helmet@8.0.0: {}
help-me@5.0.0: {}
hex-to-rgba@2.0.1: {}