diff --git a/api/src/unraid-api/app/app.module.ts b/api/src/unraid-api/app/app.module.ts index 12a87914ea..93eb262930 100644 --- a/api/src/unraid-api/app/app.module.ts +++ b/api/src/unraid-api/app/app.module.ts @@ -2,7 +2,6 @@ import { CacheModule } from '@nestjs/cache-manager'; import { Module } from '@nestjs/common'; import { APP_GUARD } from '@nestjs/core'; import { ScheduleModule } from '@nestjs/schedule'; -import { ThrottlerModule } from '@nestjs/throttler'; import { LoggerModule } from 'nestjs-pino'; @@ -51,12 +50,6 @@ import { UnraidFileModifierModule } from '@app/unraid-api/unraid-file-modifier/u CacheModule.register({ isGlobal: true }), GraphModule, RestModule, - ThrottlerModule.forRoot([ - { - ttl: 10000, // 10 seconds - limit: 100, // 100 requests per 10 seconds - }, - ]), UnraidFileModifierModule, ], controllers: [], diff --git a/api/src/unraid-api/auth/fastify-throttler.guard.ts b/api/src/unraid-api/auth/fastify-throttler.guard.ts index b8d135438f..48de4509cf 100644 --- a/api/src/unraid-api/auth/fastify-throttler.guard.ts +++ b/api/src/unraid-api/auth/fastify-throttler.guard.ts @@ -1,17 +1,45 @@ -import { ExecutionContext, Injectable } from '@nestjs/common'; +import { ExecutionContext, Inject, Injectable, Optional } from '@nestjs/common'; +import { Reflector } from '@nestjs/core'; import { GqlExecutionContext } from '@nestjs/graphql'; -import { ThrottlerGuard } from '@nestjs/throttler'; +import { + getOptionsToken, + InjectThrottlerStorage, + ThrottlerGuard, + ThrottlerModuleOptions, + ThrottlerStorage, + ThrottlerStorageService, +} from '@nestjs/throttler'; import { type FastifyRequest } from 'fastify'; @Injectable() export class FastifyThrottlerGuard extends ThrottlerGuard { + constructor( + @Optional() @Inject(getOptionsToken()) options: ThrottlerModuleOptions | undefined, + @Optional() @InjectThrottlerStorage() storageService: ThrottlerStorage | undefined, + reflector: Reflector + ) { + super( + options ?? [{ ttl: 10000, limit: 100 }], + storageService ?? new ThrottlerStorageService(), + reflector + ); + } + protected async getTracker(req: Record): Promise { const request = req as unknown as FastifyRequest; return request.ip ?? request.ips?.[0] ?? request.headers?.['x-forwarded-for'] ?? '0.0.0.0'; } getRequestResponse(context: ExecutionContext) { + if (context.getType() === 'http') { + const httpContext = context.switchToHttp(); + return { + req: httpContext.getRequest(), + res: httpContext.getResponse(), + }; + } + const gqlContext = GqlExecutionContext.create(context); const ctx = gqlContext.getContext(); diff --git a/api/src/unraid-api/graph/resolvers/sso/session/oidc-state.service.ts b/api/src/unraid-api/graph/resolvers/sso/session/oidc-state.service.ts index cc9ca332ad..e60c6b6e37 100644 --- a/api/src/unraid-api/graph/resolvers/sso/session/oidc-state.service.ts +++ b/api/src/unraid-api/graph/resolvers/sso/session/oidc-state.service.ts @@ -11,6 +11,12 @@ interface StateData { codeVerifier?: string; } +interface ActiveState { + expiresAt: number; + providerId: string; + scopeKey: string; +} + @Injectable() export class OidcStateService { private static instanceCount = 0; @@ -19,6 +25,14 @@ export class OidcStateService { private readonly hmacSecret: string; private readonly STATE_TTL_MS = 600000; // 10 minutes in milliseconds (cache-manager v7+ expects milliseconds, not seconds) private readonly STATE_CACHE_PREFIX = 'oidc_state:'; + private readonly MAX_ACTIVE_STATES = 256; + private readonly MAX_ACTIVE_STATES_PER_PROVIDER = 64; + private readonly MAX_ACTIVE_STATES_PER_SCOPE = 32; + private readonly MAX_CLIENT_STATE_LENGTH = 256; + private readonly MAX_REDIRECT_URI_LENGTH = 2048; + private readonly activeStates = new Map(); + private readonly activeStatesByProvider = new Map(); + private readonly activeStatesByScope = new Map(); constructor(@Inject(CACHE_MANAGER) private cacheManager: Cache) { // Track instance creation @@ -37,9 +51,45 @@ export class OidcStateService { redirectUri?: string, codeVerifier?: string ): Promise { + if ( + typeof providerId !== 'string' || + !providerId || + providerId.length > 128 || + typeof clientState !== 'string' || + !clientState || + clientState.length > this.MAX_CLIENT_STATE_LENGTH || + (redirectUri !== undefined && + (typeof redirectUri !== 'string' || + redirectUri.length > this.MAX_REDIRECT_URI_LENGTH)) || + (codeVerifier !== undefined && typeof codeVerifier !== 'string') + ) { + throw new Error('Invalid OIDC authorization parameters'); + } + + this.pruneExpiredStates(); + const nonce = crypto.randomBytes(16).toString('hex'); const timestamp = Date.now(); + const providerStateCount = this.activeStatesByProvider.get(providerId) ?? 0; + const scopeKey = this.getStateScopeKey(providerId, redirectUri); + const scopeStateCount = this.activeStatesByScope.get(scopeKey) ?? 0; + if ( + this.activeStates.size >= this.MAX_ACTIVE_STATES || + providerStateCount >= this.MAX_ACTIVE_STATES_PER_PROVIDER || + scopeStateCount >= this.MAX_ACTIVE_STATES_PER_SCOPE + ) { + throw new Error('Too many pending OIDC authorization requests'); + } + + this.activeStates.set(nonce, { + expiresAt: timestamp + this.STATE_TTL_MS, + providerId, + scopeKey, + }); + this.activeStatesByProvider.set(providerId, providerStateCount + 1); + this.activeStatesByScope.set(scopeKey, scopeStateCount + 1); + // Store state data in cache const stateData: StateData = { nonce, @@ -53,14 +103,19 @@ export class OidcStateService { // Store in cache with TTL (in milliseconds for cache-manager v7) const cacheKey = `${this.STATE_CACHE_PREFIX}${nonce}`; this.logger.debug(`Storing state with key: ${cacheKey}, TTL: ${this.STATE_TTL_MS}ms`); - await this.cacheManager.set(cacheKey, stateData, this.STATE_TTL_MS); + try { + await this.cacheManager.set(cacheKey, stateData, this.STATE_TTL_MS); + + // Verify it was stored + const verifyStored = await this.cacheManager.get(cacheKey); + if (!verifyStored) { + throw new Error('Unable to persist OIDC authorization state'); + } - // Verify it was stored - const verifyStored = await this.cacheManager.get(cacheKey); - if (verifyStored) { this.logger.debug(`State successfully stored and verified for key: ${cacheKey}`); - } else { - this.logger.error(`CRITICAL: State was NOT stored in cache for key: ${cacheKey}`); + } catch (error) { + this.releaseState(nonce); + throw error; } // Create signed state: nonce.timestamp.signature @@ -88,7 +143,22 @@ export class OidcStateService { codeVerifier?: string; error?: string; }> { + let parsedNonce: string | undefined; + try { + if ( + typeof state !== 'string' || + !state || + state.length > 4096 || + typeof expectedProviderId !== 'string' || + !expectedProviderId + ) { + return { + isValid: false, + error: 'Invalid state format', + }; + } + // Extract provider ID and signed state const parts = state.split(':'); if (parts.length < 2) { @@ -122,8 +192,16 @@ export class OidcStateService { } const [nonce, timestampStr, signature] = stateParts; + parsedNonce = nonce; const timestamp = parseInt(timestampStr, 10); + if (!Number.isSafeInteger(timestamp)) { + return { + isValid: false, + error: 'Invalid state format', + }; + } + // Verify signature const dataToSign = `${nonce}.${timestampStr}`; const expectedSignature = crypto @@ -142,7 +220,8 @@ export class OidcStateService { // Check timestamp expiration const now = Date.now(); const age = now - timestamp; - if (age > this.STATE_TTL_MS) { + if (age < 0 || age > this.STATE_TTL_MS) { + this.releaseState(nonce); this.logger.warn(`State validation failed: token expired (age: ${age}ms)`); return { isValid: false, @@ -161,6 +240,7 @@ export class OidcStateService { const cachedState = await this.cacheManager.get(cacheKey); if (!cachedState) { + this.releaseState(nonce); this.logger.warn( `State validation failed: nonce ${nonce} not found in cache (possible replay attack)` ); @@ -174,6 +254,7 @@ export class OidcStateService { // Verify the cached provider ID matches if (cachedState.providerId !== expectedProviderId) { + this.releaseState(nonce); this.logger.warn(`State validation failed: cached provider mismatch`); return { isValid: false, @@ -183,6 +264,7 @@ export class OidcStateService { // Remove from cache to prevent reuse await this.cacheManager.del(cacheKey); + this.releaseState(nonce); this.logger.debug(`State validation successful for provider ${expectedProviderId}`); return { @@ -192,6 +274,9 @@ export class OidcStateService { codeVerifier: cachedState.codeVerifier, }; } catch (error) { + if (parsedNonce) { + this.releaseState(parsedNonce); + } this.logger.error( `State validation error: ${error instanceof Error ? error.message : 'Unknown error'}` ); @@ -202,6 +287,40 @@ export class OidcStateService { } } + private pruneExpiredStates(now = Date.now()): void { + for (const [nonce, state] of this.activeStates) { + if (state.expiresAt <= now) { + this.releaseState(nonce); + } + } + } + + private getStateScopeKey(providerId: string, redirectUri?: string): string { + return JSON.stringify([providerId, redirectUri ?? '']); + } + + private releaseState(nonce: string): void { + const state = this.activeStates.get(nonce); + if (!state) { + return; + } + + this.activeStates.delete(nonce); + const providerStateCount = this.activeStatesByProvider.get(state.providerId) ?? 0; + if (providerStateCount <= 1) { + this.activeStatesByProvider.delete(state.providerId); + } else { + this.activeStatesByProvider.set(state.providerId, providerStateCount - 1); + } + + const scopeStateCount = this.activeStatesByScope.get(state.scopeKey) ?? 0; + if (scopeStateCount <= 1) { + this.activeStatesByScope.delete(state.scopeKey); + } else { + this.activeStatesByScope.set(state.scopeKey, scopeStateCount - 1); + } + } + extractProviderFromLegacyState(state: string): { providerId: string; originalState: string } { // Backward compatibility: handle old format states const parts = state.split(':'); diff --git a/api/src/unraid-api/graph/resolvers/sso/utils/oidc-request-handler.util.ts b/api/src/unraid-api/graph/resolvers/sso/utils/oidc-request-handler.util.ts index d96d6a2034..fd105a2224 100644 --- a/api/src/unraid-api/graph/resolvers/sso/utils/oidc-request-handler.util.ts +++ b/api/src/unraid-api/graph/resolvers/sso/utils/oidc-request-handler.util.ts @@ -22,6 +22,10 @@ export interface OidcCallbackResult extends OidcFlowResult { * between authorize and callback endpoints */ export class OidcRequestHandler { + private static readonly MAX_PROVIDER_ID_LENGTH = 128; + private static readonly MAX_STATE_LENGTH = 256; + private static readonly MAX_REDIRECT_URI_LENGTH = 2048; + /** * Extract request information from Fastify request headers */ @@ -104,19 +108,26 @@ export class OidcRequestHandler { * Validate required parameters for authorization flow */ static validateAuthorizeParams( - providerId: string | undefined, - state: string | undefined, - redirectUri: string | undefined + providerId: string | string[] | undefined, + state: string | string[] | undefined, + redirectUri: string | string[] | undefined ): { providerId: string; state: string; redirectUri: string } { - if (!providerId) { + if (typeof providerId !== 'string' || !providerId) { throw new Error('Provider ID is required'); } - if (!state) { + if (typeof state !== 'string' || !state) { throw new Error('State parameter is required'); } - if (!redirectUri) { + if (typeof redirectUri !== 'string' || !redirectUri) { throw new Error('Redirect URI is required'); } + if ( + providerId.length > OidcRequestHandler.MAX_PROVIDER_ID_LENGTH || + state.length > OidcRequestHandler.MAX_STATE_LENGTH || + redirectUri.length > OidcRequestHandler.MAX_REDIRECT_URI_LENGTH + ) { + throw new Error('Invalid OIDC authorization parameters'); + } return { providerId, state, redirectUri }; } @@ -125,10 +136,10 @@ export class OidcRequestHandler { * Validate required parameters for callback flow */ static validateCallbackParams( - code: string | undefined, - state: string | undefined + code: string | string[] | undefined, + state: string | string[] | undefined ): { code: string; state: string } { - if (!code || !state) { + if (typeof code !== 'string' || !code || typeof state !== 'string' || !state) { throw new Error('Missing required parameters'); } diff --git a/api/src/unraid-api/rest/rest.controller.ts b/api/src/unraid-api/rest/rest.controller.ts index 5a662bb6b8..a22e6b2d7a 100644 --- a/api/src/unraid-api/rest/rest.controller.ts +++ b/api/src/unraid-api/rest/rest.controller.ts @@ -1,4 +1,15 @@ -import { Controller, Get, Logger, Param, Query, Req, Res, UnauthorizedException } from '@nestjs/common'; +import { + Controller, + Get, + Logger, + Param, + Query, + Req, + Res, + UnauthorizedException, + UseGuards, +} from '@nestjs/common'; +import { Throttle } from '@nestjs/throttler'; import { AuthAction, Resource } from '@unraid/shared/graphql.model.js'; import { UsePermissions } from '@unraid/shared/use-permissions.directive.js'; @@ -6,6 +17,7 @@ import escapeHtml from 'escape-html'; import type { CustomizationType } from '@app/unraid-api/rest/rest.service.js'; import type { FastifyReply, FastifyRequest } from '@app/unraid-api/types/fastify.js'; +import { FastifyThrottlerGuard } from '@app/unraid-api/auth/fastify-throttler.guard.js'; import { Public } from '@app/unraid-api/auth/public.decorator.js'; import { OidcConfigPersistence } from '@app/unraid-api/graph/resolvers/sso/core/oidc-config.service.js'; import { OidcService } from '@app/unraid-api/graph/resolvers/sso/core/oidc.service.js'; @@ -58,6 +70,8 @@ export class RestController { : ['/graphql/api/auth/oidc/authorize/:providerId'] ) @Public() + @UseGuards(FastifyThrottlerGuard) + @Throttle({ default: { limit: 10, ttl: 60_000 } }) async oidcAuthorize( @Param('providerId') providerId: string, @Query('state') state: string, @@ -136,6 +150,8 @@ export class RestController { : ['/graphql/api/auth/oidc/callback'] ) @Public() + @UseGuards(FastifyThrottlerGuard) + @Throttle({ default: { limit: 10, ttl: 60_000 } }) async oidcCallback( @Query('code') code: string, @Query('state') state: string, diff --git a/api/src/unraid-api/rest/rest.module.ts b/api/src/unraid-api/rest/rest.module.ts index bc5187caa7..d458c5cf9f 100644 --- a/api/src/unraid-api/rest/rest.module.ts +++ b/api/src/unraid-api/rest/rest.module.ts @@ -1,13 +1,24 @@ import { Module } from '@nestjs/common'; +import { ThrottlerModule } from '@nestjs/throttler'; +import { FastifyThrottlerGuard } from '@app/unraid-api/auth/fastify-throttler.guard.js'; import { RCloneModule } from '@app/unraid-api/graph/resolvers/rclone/rclone.module.js'; import { SsoModule } from '@app/unraid-api/graph/resolvers/sso/sso.module.js'; import { RestController } from '@app/unraid-api/rest/rest.controller.js'; import { RestService } from '@app/unraid-api/rest/rest.service.js'; @Module({ - imports: [RCloneModule, SsoModule], + imports: [ + RCloneModule, + SsoModule, + ThrottlerModule.forRoot([ + { + ttl: 10000, + limit: 100, + }, + ]), + ], controllers: [RestController], - providers: [RestService], + providers: [RestService, FastifyThrottlerGuard], }) export class RestModule {}