From 3a800c588dfdb181b408140be7b732e81a8b9dc6 Mon Sep 17 00:00:00 2001 From: SimonFair <39065407+SimonFair@users.noreply.github.com> Date: Tue, 29 Sep 2026 12:06:59 +0100 Subject: [PATCH 1/5] fix(auth): limit repeated authorization requests --- .../auth/fastify-throttler.guard.ts | 8 ++ .../sso/session/oidc-state.service.ts | 108 ++++++++++++++++-- .../sso/utils/oidc-request-handler.util.ts | 11 ++ api/src/unraid-api/rest/rest.controller.ts | 16 ++- api/src/unraid-api/rest/rest.module.ts | 3 +- 5 files changed, 137 insertions(+), 9 deletions(-) diff --git a/api/src/unraid-api/auth/fastify-throttler.guard.ts b/api/src/unraid-api/auth/fastify-throttler.guard.ts index b8d135438f..9e0f66e0fc 100644 --- a/api/src/unraid-api/auth/fastify-throttler.guard.ts +++ b/api/src/unraid-api/auth/fastify-throttler.guard.ts @@ -12,6 +12,14 @@ export class FastifyThrottlerGuard extends ThrottlerGuard { } 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..6ad899a8ed 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,11 @@ interface StateData { codeVerifier?: string; } +interface ActiveState { + expiresAt: number; + bucket: string; +} + @Injectable() export class OidcStateService { private static instanceCount = 0; @@ -19,6 +24,12 @@ 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_BUCKET = 32; + private readonly MAX_CLIENT_STATE_LENGTH = 256; + private readonly MAX_REDIRECT_URI_LENGTH = 2048; + private readonly activeStates = new Map(); + private readonly activeStateCounts = new Map(); constructor(@Inject(CACHE_MANAGER) private cacheManager: Cache) { // Track instance creation @@ -37,8 +48,35 @@ export class OidcStateService { redirectUri?: string, codeVerifier?: string ): Promise { + if ( + !providerId || + providerId.length > 128 || + !clientState || + clientState.length > this.MAX_CLIENT_STATE_LENGTH || + (redirectUri !== undefined && redirectUri.length > this.MAX_REDIRECT_URI_LENGTH) + ) { + throw new Error('Invalid OIDC authorization parameters'); + } + + this.pruneExpiredStates(); + const nonce = crypto.randomBytes(16).toString('hex'); const timestamp = Date.now(); + const bucket = this.getStateBucket(providerId, redirectUri); + const bucketCount = this.activeStateCounts.get(bucket) ?? 0; + + if ( + this.activeStates.size >= this.MAX_ACTIVE_STATES || + bucketCount >= this.MAX_ACTIVE_STATES_PER_BUCKET + ) { + throw new Error('Too many pending OIDC authorization requests'); + } + + this.activeStates.set(nonce, { + expiresAt: timestamp + this.STATE_TTL_MS, + bucket, + }); + this.activeStateCounts.set(bucket, bucketCount + 1); // Store state data in cache const stateData: StateData = { @@ -53,14 +91,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 +131,16 @@ export class OidcStateService { codeVerifier?: string; error?: string; }> { + let parsedNonce: string | undefined; + try { + if (!state || state.length > 4096) { + return { + isValid: false, + error: 'Invalid state format', + }; + } + // Extract provider ID and signed state const parts = state.split(':'); if (parts.length < 2) { @@ -122,8 +174,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 +202,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 +222,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 +236,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 +246,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 +256,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 +269,33 @@ export class OidcStateService { } } + private getStateBucket(providerId: string, redirectUri?: string): string { + return `${providerId}:${redirectUri ?? ''}`; + } + + private pruneExpiredStates(now = Date.now()): void { + for (const [nonce, state] of this.activeStates) { + if (state.expiresAt <= now) { + this.releaseState(nonce); + } + } + } + + private releaseState(nonce: string): void { + const state = this.activeStates.get(nonce); + if (!state) { + return; + } + + this.activeStates.delete(nonce); + const nextCount = (this.activeStateCounts.get(state.bucket) ?? 1) - 1; + if (nextCount > 0) { + this.activeStateCounts.set(state.bucket, nextCount); + } else { + this.activeStateCounts.delete(state.bucket); + } + } + 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..2318a48207 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 */ @@ -117,6 +121,13 @@ export class OidcRequestHandler { if (!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 }; } diff --git a/api/src/unraid-api/rest/rest.controller.ts b/api/src/unraid-api/rest/rest.controller.ts index 5a662bb6b8..8657bc4a11 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, diff --git a/api/src/unraid-api/rest/rest.module.ts b/api/src/unraid-api/rest/rest.module.ts index bc5187caa7..3232dfedfc 100644 --- a/api/src/unraid-api/rest/rest.module.ts +++ b/api/src/unraid-api/rest/rest.module.ts @@ -1,5 +1,6 @@ import { Module } from '@nestjs/common'; +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'; @@ -8,6 +9,6 @@ import { RestService } from '@app/unraid-api/rest/rest.service.js'; @Module({ imports: [RCloneModule, SsoModule], controllers: [RestController], - providers: [RestService], + providers: [RestService, FastifyThrottlerGuard], }) export class RestModule {} From 894b294cdedc6fee3f8b2718b4366558d47b03e1 Mon Sep 17 00:00:00 2001 From: SimonFair <39065407+SimonFair@users.noreply.github.com> Date: Tue, 29 Sep 2026 12:45:18 +0100 Subject: [PATCH 2/5] fix(auth): validate authorization inputs --- api/src/unraid-api/app/app.module.ts | 7 ------ .../auth/fastify-throttler.guard.ts | 24 +++++++++++++++++-- .../sso/session/oidc-state.service.ts | 15 ++++++++++-- .../sso/utils/oidc-request-handler.util.ts | 18 +++++++------- api/src/unraid-api/rest/rest.module.ts | 12 +++++++++- 5 files changed, 55 insertions(+), 21 deletions(-) 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 9e0f66e0fc..48de4509cf 100644 --- a/api/src/unraid-api/auth/fastify-throttler.guard.ts +++ b/api/src/unraid-api/auth/fastify-throttler.guard.ts @@ -1,11 +1,31 @@ -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'; 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 6ad899a8ed..db54ab13cd 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 @@ -49,11 +49,16 @@ export class OidcStateService { codeVerifier?: string ): Promise { if ( + typeof providerId !== 'string' || !providerId || providerId.length > 128 || + typeof clientState !== 'string' || !clientState || clientState.length > this.MAX_CLIENT_STATE_LENGTH || - (redirectUri !== undefined && redirectUri.length > this.MAX_REDIRECT_URI_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'); } @@ -134,7 +139,13 @@ export class OidcStateService { let parsedNonce: string | undefined; try { - if (!state || state.length > 4096) { + if ( + typeof state !== 'string' || + !state || + state.length > 4096 || + typeof expectedProviderId !== 'string' || + !expectedProviderId + ) { return { isValid: false, error: 'Invalid state format', 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 2318a48207..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 @@ -108,17 +108,17 @@ 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 ( @@ -136,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.module.ts b/api/src/unraid-api/rest/rest.module.ts index 3232dfedfc..d458c5cf9f 100644 --- a/api/src/unraid-api/rest/rest.module.ts +++ b/api/src/unraid-api/rest/rest.module.ts @@ -1,4 +1,5 @@ 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'; @@ -7,7 +8,16 @@ 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, FastifyThrottlerGuard], }) From 4513255c69d60dd114846d8692bd83bc67bc005f Mon Sep 17 00:00:00 2001 From: SimonFair <39065407+SimonFair@users.noreply.github.com> Date: Tue, 29 Sep 2026 14:34:22 +0100 Subject: [PATCH 3/5] fix(auth): bound authorization flow OS-982 --- .../sso/session/oidc-state.service.ts | 22 +------------------ api/src/unraid-api/rest/rest.controller.ts | 2 ++ 2 files changed, 3 insertions(+), 21 deletions(-) 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 db54ab13cd..213c711cce 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 @@ -13,7 +13,6 @@ interface StateData { interface ActiveState { expiresAt: number; - bucket: string; } @Injectable() @@ -25,11 +24,9 @@ export class OidcStateService { 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_BUCKET = 32; private readonly MAX_CLIENT_STATE_LENGTH = 256; private readonly MAX_REDIRECT_URI_LENGTH = 2048; private readonly activeStates = new Map(); - private readonly activeStateCounts = new Map(); constructor(@Inject(CACHE_MANAGER) private cacheManager: Cache) { // Track instance creation @@ -67,21 +64,14 @@ export class OidcStateService { const nonce = crypto.randomBytes(16).toString('hex'); const timestamp = Date.now(); - const bucket = this.getStateBucket(providerId, redirectUri); - const bucketCount = this.activeStateCounts.get(bucket) ?? 0; - if ( - this.activeStates.size >= this.MAX_ACTIVE_STATES || - bucketCount >= this.MAX_ACTIVE_STATES_PER_BUCKET - ) { + if (this.activeStates.size >= this.MAX_ACTIVE_STATES) { throw new Error('Too many pending OIDC authorization requests'); } this.activeStates.set(nonce, { expiresAt: timestamp + this.STATE_TTL_MS, - bucket, }); - this.activeStateCounts.set(bucket, bucketCount + 1); // Store state data in cache const stateData: StateData = { @@ -280,10 +270,6 @@ export class OidcStateService { } } - private getStateBucket(providerId: string, redirectUri?: string): string { - return `${providerId}:${redirectUri ?? ''}`; - } - private pruneExpiredStates(now = Date.now()): void { for (const [nonce, state] of this.activeStates) { if (state.expiresAt <= now) { @@ -299,12 +285,6 @@ export class OidcStateService { } this.activeStates.delete(nonce); - const nextCount = (this.activeStateCounts.get(state.bucket) ?? 1) - 1; - if (nextCount > 0) { - this.activeStateCounts.set(state.bucket, nextCount); - } else { - this.activeStateCounts.delete(state.bucket); - } } extractProviderFromLegacyState(state: string): { providerId: string; originalState: string } { diff --git a/api/src/unraid-api/rest/rest.controller.ts b/api/src/unraid-api/rest/rest.controller.ts index 8657bc4a11..a22e6b2d7a 100644 --- a/api/src/unraid-api/rest/rest.controller.ts +++ b/api/src/unraid-api/rest/rest.controller.ts @@ -150,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, From 081a9802dd350de277d14f7b744df1d1bb58fe28 Mon Sep 17 00:00:00 2001 From: SimonFair <39065407+SimonFair@users.noreply.github.com> Date: Tue, 29 Sep 2026 16:05:23 +0100 Subject: [PATCH 4/5] fix(auth): isolate pending oidc state capacity --- .../resolvers/sso/session/oidc-state.service.ts | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) 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 213c711cce..2f2bf5ae56 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 @@ -13,6 +13,7 @@ interface StateData { interface ActiveState { expiresAt: number; + providerId: string; } @Injectable() @@ -24,9 +25,11 @@ export class OidcStateService { 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_CLIENT_STATE_LENGTH = 256; private readonly MAX_REDIRECT_URI_LENGTH = 2048; private readonly activeStates = new Map(); + private readonly activeStatesByProvider = new Map(); constructor(@Inject(CACHE_MANAGER) private cacheManager: Cache) { // Track instance creation @@ -65,13 +68,19 @@ export class OidcStateService { const nonce = crypto.randomBytes(16).toString('hex'); const timestamp = Date.now(); - if (this.activeStates.size >= this.MAX_ACTIVE_STATES) { + const providerStateCount = this.activeStatesByProvider.get(providerId) ?? 0; + if ( + this.activeStates.size >= this.MAX_ACTIVE_STATES || + providerStateCount >= this.MAX_ACTIVE_STATES_PER_PROVIDER + ) { throw new Error('Too many pending OIDC authorization requests'); } this.activeStates.set(nonce, { expiresAt: timestamp + this.STATE_TTL_MS, + providerId, }); + this.activeStatesByProvider.set(providerId, providerStateCount + 1); // Store state data in cache const stateData: StateData = { @@ -285,6 +294,12 @@ export class OidcStateService { } 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); + } } extractProviderFromLegacyState(state: string): { providerId: string; originalState: string } { From 232bb690193e65d9ad35319fdb67f9333b8c6d62 Mon Sep 17 00:00:00 2001 From: SimonFair <39065407+SimonFair@users.noreply.github.com> Date: Wed, 30 Sep 2026 11:11:25 +0100 Subject: [PATCH 5/5] fix: scope pending state capacity --- .../sso/session/oidc-state.service.ts | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) 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 2f2bf5ae56..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 @@ -14,6 +14,7 @@ interface StateData { interface ActiveState { expiresAt: number; providerId: string; + scopeKey: string; } @Injectable() @@ -26,10 +27,12 @@ export class OidcStateService { 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 @@ -69,9 +72,12 @@ export class OidcStateService { 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 + providerStateCount >= this.MAX_ACTIVE_STATES_PER_PROVIDER || + scopeStateCount >= this.MAX_ACTIVE_STATES_PER_SCOPE ) { throw new Error('Too many pending OIDC authorization requests'); } @@ -79,8 +85,10 @@ export class OidcStateService { 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 = { @@ -287,6 +295,10 @@ export class OidcStateService { } } + 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) { @@ -300,6 +312,13 @@ export class OidcStateService { } 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 } {