Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions api/src/unraid-api/app/app.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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: [],
Expand Down
32 changes: 30 additions & 2 deletions api/src/unraid-api/auth/fastify-throttler.guard.ts
Original file line number Diff line number Diff line change
@@ -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<string, any>): Promise<string> {
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();

Expand Down
133 changes: 126 additions & 7 deletions api/src/unraid-api/graph/resolvers/sso/session/oidc-state.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ interface StateData {
codeVerifier?: string;
}

interface ActiveState {
expiresAt: number;
providerId: string;
scopeKey: string;
}

@Injectable()
export class OidcStateService {
private static instanceCount = 0;
Expand All @@ -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<string, ActiveState>();
private readonly activeStatesByProvider = new Map<string, number>();
private readonly activeStatesByScope = new Map<string, number>();

constructor(@Inject(CACHE_MANAGER) private cacheManager: Cache) {
// Track instance creation
Expand All @@ -37,9 +51,45 @@ export class OidcStateService {
redirectUri?: string,
codeVerifier?: string
): Promise<string> {
if (
typeof providerId !== 'string' ||
!providerId ||
providerId.length > 128 ||
typeof clientState !== 'string' ||
!clientState ||
clientState.length > this.MAX_CLIENT_STATE_LENGTH ||
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
(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,
Expand All @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -161,6 +240,7 @@ export class OidcStateService {
const cachedState = await this.cacheManager.get<StateData>(cacheKey);

if (!cachedState) {
this.releaseState(nonce);
this.logger.warn(
`State validation failed: nonce ${nonce} not found in cache (possible replay attack)`
);
Expand All @@ -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,
Expand All @@ -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 {
Expand All @@ -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'}`
);
Expand All @@ -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(':');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -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 ||
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
redirectUri.length > OidcRequestHandler.MAX_REDIRECT_URI_LENGTH
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
) {
throw new Error('Invalid OIDC authorization parameters');
}

return { providerId, state, redirectUri };
}
Expand All @@ -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');
}

Expand Down
18 changes: 17 additions & 1 deletion api/src/unraid-api/rest/rest.controller.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,23 @@
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';
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';
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
Loading
Loading