From 14708e20504333b862000bc2f1a9cfca30933200 Mon Sep 17 00:00:00 2001 From: Eli Bosley Date: Wed, 20 Aug 2025 19:21:57 -0400 Subject: [PATCH 01/16] feat(api): implement ephemeral and persistent API key management - Introduced persistent and ephemeral key storage in ApiKeyService for better key management. - Updated onModuleInit to load persistent keys and clean up legacy internal keys. - Enhanced findAll and findByField methods to prioritize ephemeral keys. - Added methods for registering ephemeral keys and modules, improving CLI integration. - Updated AdminKeyService to generate and manage ephemeral keys for CLI operations. - Adjusted ConnectApiKeyService to utilize module tokens instead of static keys for better security. --- api/src/unraid-api/auth/api-key.service.ts | 191 +++++++++++++++--- api/src/unraid-api/cli/admin-key.service.ts | 69 +++++-- .../unraid-api/cli/internal-client.service.ts | 18 +- .../src/authn/connect-api-key.service.ts | 58 +++--- 4 files changed, 272 insertions(+), 64 deletions(-) diff --git a/api/src/unraid-api/auth/api-key.service.ts b/api/src/unraid-api/auth/api-key.service.ts index 1108485b2a..5b9a99c2d9 100644 --- a/api/src/unraid-api/auth/api-key.service.ts +++ b/api/src/unraid-api/auth/api-key.service.ts @@ -26,6 +26,8 @@ export class ApiKeyService implements OnModuleInit { private readonly logger = new Logger(ApiKeyService.name); protected readonly basePath: string; protected memoryApiKeys: Array = []; + private persistentKeys = new Map(); + private ephemeralKeys = new Map(); private static readonly validRoles: Set = new Set(Object.values(Role)); constructor() { @@ -34,22 +36,60 @@ export class ApiKeyService implements OnModuleInit { } async onModuleInit() { - this.memoryApiKeys = await this.loadAllFromDisk(); - if (environment.IS_MAIN_PROCESS) { - this.setupWatch(); + // Load persistent keys once at startup + const diskKeys = await this.loadAllFromDisk(); + for (const key of diskKeys) { + this.persistentKeys.set(key.id, key); } + + // Clean up legacy internal keys + await this.cleanupLegacyInternalKeys(); + + // Update memoryApiKeys for backwards compatibility + this.updateMemoryApiKeys(); + + // NO file watching - manage in memory + this.logger.log(`Loaded ${this.persistentKeys.size} persistent API keys`); } public async findAll(): Promise { - return this.memoryApiKeys; + // Only return persistent keys, not ephemeral ones + return Array.from(this.persistentKeys.values()).map((key) => + this.convertApiKeyWithSecretToApiKey(key) + ); } - private setupWatch() { - watch(this.basePath, { ignoreInitial: false }).on('all', async (path) => { - this.logger.debug(`API key changed: ${path}`); - this.memoryApiKeys = []; - this.memoryApiKeys = await this.loadAllFromDisk(); - }); + private updateMemoryApiKeys() { + // Combine persistent and ephemeral keys for backwards compatibility + this.memoryApiKeys = [ + ...Array.from(this.persistentKeys.values()), + ...Array.from(this.ephemeralKeys.values()), + ]; + } + + private async cleanupLegacyInternalKeys() { + const legacyNames = ['CliInternal', 'ConnectInternal', 'CLI', 'Connect', 'CliAdmin', 'Internal']; + const keysToDelete: string[] = []; + + for (const [id, key] of this.persistentKeys) { + if (legacyNames.includes(key.name)) { + keysToDelete.push(id); + this.logger.log(`Removing legacy internal key: ${key.name}`); + } + } + + if (keysToDelete.length > 0) { + // Remove from disk + for (const id of keysToDelete) { + try { + await unlink(join(this.basePath, `${id}.json`)); + } catch (error) { + // File might not exist, that's ok + } + this.persistentKeys.delete(id); + } + this.logger.log(`Cleaned up ${keysToDelete.length} legacy internal keys`); + } } private sanitizeName(name: string): string { @@ -144,6 +184,10 @@ export class ApiKeyService implements OnModuleInit { await this.saveApiKey(apiKey as ApiKey); + // Update persistent keys in memory + this.persistentKeys.set(apiKey.id as string, apiKey as ApiKey); + this.updateMemoryApiKeys(); + return apiKey as ApiKey; } @@ -233,17 +277,97 @@ export class ApiKeyService implements OnModuleInit { public findByField(field: keyof ApiKey, value: string): ApiKey | null { if (!value) return null; - return this.memoryApiKeys.find((k) => k[field] === value) ?? null; + // Check ephemeral keys first + for (const keyData of this.ephemeralKeys.values()) { + if (keyData[field] === value) { + return keyData; + } + } + + // Then check persistent keys + for (const keyData of this.persistentKeys.values()) { + if (keyData[field] === value) { + return keyData; + } + } + + return null; } - findByKey(key: string): ApiKey | null { - return this.findByField('key', key); + findByKey(key: string): ApiKeyWithSecret | null { + if (!key) return null; + + // Check ephemeral keys first (faster, in-memory) + for (const keyData of this.ephemeralKeys.values()) { + if (keyData.key === key) { + return keyData; + } + } + + // Then check persistent keys + for (const keyData of this.persistentKeys.values()) { + if (keyData.key === key) { + return keyData; + } + } + + return null; } private generateApiKey(): string { return crypto.randomBytes(32).toString('hex'); } + /** + * Register an in-process module with an ephemeral token. + * Used by Connect and other modules running in the same process. + */ + public async registerModule(moduleId: string, roles: Role[]): Promise { + // Generate 256-bit cryptographically secure token + const token = crypto.randomBytes(32).toString('base64url'); + + const keyData: ApiKeyWithSecret = { + id: `module-${moduleId}`, + key: token, + name: `Module-${moduleId}`, + description: `Ephemeral token for ${moduleId} module`, + roles, + permissions: [], + createdAt: new Date().toISOString(), + }; + + this.ephemeralKeys.set(keyData.id, keyData); + this.updateMemoryApiKeys(); + this.logger.debug(`Registered module ${moduleId} with ephemeral token`); + + return token; + } + + /** + * Register an ephemeral key for external processes like CLI. + * The key is provided by the caller and registered in memory. + */ + public async registerEphemeralKey(config: { + key: string; + name: string; + roles: Role[]; + type: string; + }): Promise { + const keyData: ApiKeyWithSecret = { + id: `ephemeral-${config.type}`, + key: config.key, + name: config.name, + description: `Ephemeral key for ${config.type}`, + roles: config.roles, + permissions: [], + createdAt: new Date().toISOString(), + }; + + this.ephemeralKeys.set(keyData.id, keyData); + this.updateMemoryApiKeys(); + this.logger.debug(`Registered ephemeral key for ${config.type}`); + } + private logApiKeyValidationError(file: string, error: ValidationError): void { this.logger.error(`Invalid API key structure in file ${file}. Errors: ${JSON.stringify(error.constraints, null, 2)}`); @@ -304,17 +428,30 @@ export class ApiKeyService implements OnModuleInit { throw new Error(`API keys not found: ${missingKeys.join(', ')}`); } - // Delete all files in parallel - const { errors, data: deletedIds } = await batchProcess(ids, async (id) => { - await unlink(join(this.basePath, `${id}.json`)); - return id; - }); + // Separate persistent and ephemeral keys + const persistentIds = ids.filter((id) => this.persistentKeys.has(id)); + const ephemeralIds = ids.filter((id) => this.ephemeralKeys.has(id)); - const deletedSet = new Set(deletedIds); - this.memoryApiKeys = this.memoryApiKeys.filter((key) => !deletedSet.has(key.id)); - if (errors.length > 0) { - throw errors; + // Delete persistent keys from disk + if (persistentIds.length > 0) { + const { errors } = await batchProcess(persistentIds, async (id) => { + await unlink(join(this.basePath, `${id}.json`)); + this.persistentKeys.delete(id); + return id; + }); + + if (errors.length > 0) { + throw errors; + } } + + // Remove ephemeral keys from memory + for (const id of ephemeralIds) { + this.ephemeralKeys.delete(id); + } + + // Update memory cache + this.updateMemoryApiKeys(); } async update({ @@ -351,7 +488,15 @@ export class ApiKeyService implements OnModuleInit { // Handle both empty array (to clear permissions) and populated array apiKey.permissions = permissions; } - await this.saveApiKey(apiKey); + + // Only save to disk if it's a persistent key + if (this.persistentKeys.has(id)) { + await this.saveApiKey(apiKey); + this.persistentKeys.set(id, apiKey); + } + + // Update memory cache + this.updateMemoryApiKeys(); return apiKey; } diff --git a/api/src/unraid-api/cli/admin-key.service.ts b/api/src/unraid-api/cli/admin-key.service.ts index 43794a7178..bcd8e76ebf 100644 --- a/api/src/unraid-api/cli/admin-key.service.ts +++ b/api/src/unraid-api/cli/admin-key.service.ts @@ -1,19 +1,21 @@ import { Inject, Injectable, Logger, OnModuleInit } from '@nestjs/common'; +import crypto from 'crypto'; +import { mkdir, writeFile } from 'fs/promises'; +import { join } from 'path'; import type { ApiKeyService } from '@unraid/shared/services/api-key.js'; import { Role } from '@unraid/shared/graphql.model.js'; import { API_KEY_SERVICE_TOKEN } from '@unraid/shared/tokens.js'; /** - * Service that creates and manages the admin API key used by CLI commands. - * Uses the standard API key storage location via helper methods in ApiKeyService. + * Service that creates and manages the ephemeral admin key used by CLI commands. + * Generates a temporary key stored in /var/run that expires on restart. */ @Injectable() export class AdminKeyService implements OnModuleInit { private readonly logger = new Logger(AdminKeyService.name); - private static readonly ADMIN_KEY_NAME = 'CliInternal'; - private static readonly ADMIN_KEY_DESCRIPTION = - 'Internal admin API key used by CLI commands for system operations'; + private ephemeralKey: string | null = null; + private static readonly RUNTIME_KEY_PATH = '/var/run/unraid-api/cli.key'; constructor( @Inject(API_KEY_SERVICE_TOKEN) @@ -22,23 +24,58 @@ export class AdminKeyService implements OnModuleInit { async onModuleInit() { try { - await this.getOrCreateLocalAdminKey(); - this.logger.log('Admin API key initialized successfully'); + await this.generateEphemeralCliKey(); + this.logger.log('CLI ephemeral key initialized'); } catch (error) { - this.logger.error('Failed to initialize admin API key:', error); + this.logger.error('Failed to initialize CLI ephemeral key:', error); } } /** - * Gets or creates a local admin API key for CLI operations. - * Uses the standard API key storage location. + * Generates an ephemeral key for CLI operations. + * Writes the key to /var/run for the CLI process to read. */ - public async getOrCreateLocalAdminKey(): Promise { - return this.apiKeyService.ensureKey({ - name: AdminKeyService.ADMIN_KEY_NAME, - description: AdminKeyService.ADMIN_KEY_DESCRIPTION, - roles: [Role.ADMIN], // Full admin privileges for CLI operations - legacyNames: ['CLI', 'Internal', 'CliAdmin'], // Clean up old keys + private async generateEphemeralCliKey(): Promise { + // Generate cryptographically secure key + this.ephemeralKey = crypto.randomBytes(32).toString('hex'); + + // Register in ApiKeyService memory store + await this.apiKeyService.registerEphemeralKey({ + key: this.ephemeralKey, + name: 'CLI-Runtime', + roles: [Role.ADMIN], + type: 'cli', }); + + // Write to /var/run for CLI process + const runDir = '/var/run/unraid-api'; + try { + await mkdir(runDir, { recursive: true }); + await writeFile( + AdminKeyService.RUNTIME_KEY_PATH, + this.ephemeralKey, + { mode: 0o600 } // Root only + ); + this.logger.debug('CLI ephemeral key written to runtime directory'); + } catch (error) { + // If we can't write to /var/run, it's okay - CLI will fall back to getting it from service + this.logger.warn(`Could not write CLI key to ${AdminKeyService.RUNTIME_KEY_PATH}: ${error}`); + } + } + + /** + * Gets the ephemeral admin key for CLI operations. + * If not yet generated, creates it on-demand. + */ + public async getOrCreateLocalAdminKey(): Promise { + if (!this.ephemeralKey) { + await this.generateEphemeralCliKey(); + } + + if (!this.ephemeralKey) { + throw new Error('Failed to generate CLI ephemeral key'); + } + + return this.ephemeralKey; } } diff --git a/api/src/unraid-api/cli/internal-client.service.ts b/api/src/unraid-api/cli/internal-client.service.ts index b862d64e74..4199aab9f7 100644 --- a/api/src/unraid-api/cli/internal-client.service.ts +++ b/api/src/unraid-api/cli/internal-client.service.ts @@ -1,4 +1,5 @@ import { Inject, Injectable, Logger } from '@nestjs/common'; +import { readFile } from 'fs/promises'; import type { InternalGraphQLClientFactory } from '@unraid/shared'; import { ApolloClient, NormalizedCacheObject } from '@apollo/client/core/index.js'; @@ -25,10 +26,23 @@ export class CliInternalClientService { ) {} /** - * Get the admin API key using the AdminKeyService. - * This ensures the key exists and is available for CLI operations. + * Get the admin API key for CLI operations. + * First tries to read from /var/run/unraid-api/cli.key, then falls back to AdminKeyService. */ private async getLocalApiKey(): Promise { + // Try to read ephemeral key from /var/run first + try { + const ephemeralKey = await readFile('/var/run/unraid-api/cli.key', 'utf-8'); + if (ephemeralKey) { + this.logger.debug('Using ephemeral CLI key from runtime directory'); + return ephemeralKey.trim(); + } + } catch (error) { + // File doesn't exist or not readable, fall back to service + this.logger.debug('Ephemeral key file not found, falling back to AdminKeyService'); + } + + // Fall back to AdminKeyService try { return await this.adminKeyService.getOrCreateLocalAdminKey(); } catch (error) { diff --git a/packages/unraid-api-plugin-connect/src/authn/connect-api-key.service.ts b/packages/unraid-api-plugin-connect/src/authn/connect-api-key.service.ts index 834cda322b..4f1e72db3f 100644 --- a/packages/unraid-api-plugin-connect/src/authn/connect-api-key.service.ts +++ b/packages/unraid-api-plugin-connect/src/authn/connect-api-key.service.ts @@ -1,20 +1,28 @@ -import { Inject, Injectable, Logger } from '@nestjs/common'; +import { Inject, Injectable, Logger, OnModuleInit } from '@nestjs/common'; import { ApiKey, AuthAction, Permission, Resource, Role } from '@unraid/shared/graphql.model.js'; import { ApiKeyService, CreatePermissionsInput } from '@unraid/shared/services/api-key.js'; import { API_KEY_SERVICE_TOKEN } from '@unraid/shared/tokens.js'; @Injectable() -export class ConnectApiKeyService implements ApiKeyService { +export class ConnectApiKeyService implements ApiKeyService, OnModuleInit { private readonly logger = new Logger(ConnectApiKeyService.name); - private static readonly CONNECT_API_KEY_NAME = 'ConnectInternal'; - private static readonly CONNECT_API_KEY_DESCRIPTION = - 'Internal API Key Used By Unraid Connect to access your server resources for the connect.myunraid.net dashboard'; + private moduleToken: string | null = null; constructor( @Inject(API_KEY_SERVICE_TOKEN) private readonly apiKeyService: ApiKeyService ) {} + + async onModuleInit() { + try { + // Register module and get secure token + this.moduleToken = await this.apiKeyService.registerModule('connect', [Role.CONNECT]); + this.logger.log('Connect module registered with ephemeral token'); + } catch (error) { + this.logger.error('Failed to register Connect module:', error); + } + } // Delegate all standard ApiKeyService methods to the injected service async findById(id: string): Promise { @@ -65,31 +73,35 @@ export class ConnectApiKeyService implements ApiKeyService { /** * Creates a local API key specifically for Connect + * @deprecated Use module tokens instead */ public async createLocalConnectApiKey(): Promise { - try { - return await this.create({ - name: ConnectApiKeyService.CONNECT_API_KEY_NAME, - description: ConnectApiKeyService.CONNECT_API_KEY_DESCRIPTION, - roles: [Role.CONNECT], - overwrite: true, - }); - } catch (err) { - this.logger.error(`Failed to create local API key for Connect user: ${err}`); - return null; - } + // Return a mock key for backwards compatibility + return { + id: 'module-connect', + key: this.moduleToken || '', + name: 'Module-Connect', + description: 'Ephemeral token for Connect module', + roles: [Role.CONNECT], + permissions: [], + createdAt: new Date().toISOString() + }; } /** - * Gets or creates a local API key for Connect + * Gets the ephemeral module token for Connect. + * If not yet generated, creates it on-demand. */ public async getOrCreateLocalApiKey(): Promise { - return this.ensureKey({ - name: ConnectApiKeyService.CONNECT_API_KEY_NAME, - description: ConnectApiKeyService.CONNECT_API_KEY_DESCRIPTION, - roles: [Role.CONNECT], - legacyNames: ['Connect'], - }); + if (!this.moduleToken) { + this.moduleToken = await this.apiKeyService.registerModule('connect', [Role.CONNECT]); + } + + if (!this.moduleToken) { + throw new Error('Failed to generate Connect module token'); + } + + return this.moduleToken; } async ensureKey(config: { From a8ddd1cd399150aa31bccfb5ddeaaa4e907a0b4e Mon Sep 17 00:00:00 2001 From: Pujit Mehrotra Date: Tue, 26 Aug 2025 17:06:42 -0400 Subject: [PATCH 02/16] use csrf token + session for auth in internal client factory --- api/src/unraid-api/auth/cookie.service.ts | 23 +++++- .../unraid-api/plugin/global-deps.module.ts | 7 ++ .../shared/internal-graphql-client.factory.ts | 71 +++++++++++++++---- .../src/internal-rpc/internal.client.spec.ts | 60 +++++++++++++--- .../src/internal-rpc/internal.client.ts | 29 +++++--- packages/unraid-shared/src/tokens.ts | 1 + .../types/internal-graphql-client.factory.ts | 3 +- 7 files changed, 157 insertions(+), 37 deletions(-) diff --git a/api/src/unraid-api/auth/cookie.service.ts b/api/src/unraid-api/auth/cookie.service.ts index e847b7f5e8..2e3748c200 100644 --- a/api/src/unraid-api/auth/cookie.service.ts +++ b/api/src/unraid-api/auth/cookie.service.ts @@ -1,5 +1,5 @@ import { Inject, Injectable, Logger } from '@nestjs/common'; -import { readFile } from 'fs/promises'; +import { readdir, readFile } from 'fs/promises'; import { join } from 'path'; import { fileExists } from '@app/core/utils/files/file-exists.js'; @@ -68,13 +68,17 @@ export class CookieService { } try { const sessionData = await readFile(sessionFile, 'ascii'); - return sessionData.includes('unraid_login') && sessionData.includes('unraid_user'); + return this.isSessionValid(sessionData); } catch (e) { this.logger.error(e, 'Error reading session file'); return false; } } + private isSessionValid(sessionData: string): boolean { + return sessionData.includes('unraid_login') && sessionData.includes('unraid_user'); + } + /** * Given a session id, returns the full path to the session file on disk. * @@ -91,4 +95,19 @@ export class CookieService { const sanitizedSessionId = sessionId.replace(/[^a-zA-Z0-9]/g, ''); return join(this.opts.sessionDir, `sess_${sanitizedSessionId}`); } + + /** + * Returns the active session id, if any. + * @returns the active session id, if any, or null if no active session is found. + */ + async getActiveSession(): Promise { + const sessionFiles = await readdir(this.opts.sessionDir); + for (const sessionFile of sessionFiles) { + const sessionData = await readFile(join(this.opts.sessionDir, sessionFile), 'ascii'); + if (this.isSessionValid(sessionData)) { + return sessionFile.replace('sess_', ''); + } + } + return null; + } } diff --git a/api/src/unraid-api/plugin/global-deps.module.ts b/api/src/unraid-api/plugin/global-deps.module.ts index 851b31fdfc..37f623070e 100644 --- a/api/src/unraid-api/plugin/global-deps.module.ts +++ b/api/src/unraid-api/plugin/global-deps.module.ts @@ -5,6 +5,7 @@ import { PrefixedID } from '@unraid/shared/prefixed-id-scalar.js'; import { GRAPHQL_PUBSUB_TOKEN } from '@unraid/shared/pubsub/graphql.pubsub.js'; import { API_KEY_SERVICE_TOKEN, + COOKIE_SERVICE_TOKEN, INTERNAL_CLIENT_SERVICE_TOKEN, LIFECYCLE_SERVICE_TOKEN, NGINX_SERVICE_TOKEN, @@ -14,6 +15,7 @@ import { import { pubsub } from '@app/core/pubsub.js'; import { LifecycleService } from '@app/unraid-api/app/lifecycle.service.js'; import { ApiKeyService } from '@app/unraid-api/auth/api-key.service.js'; +import { CookieService } from '@app/unraid-api/auth/cookie.service.js'; import { ApiKeyModule } from '@app/unraid-api/graph/resolvers/api-key/api-key.module.js'; import { NginxModule } from '@app/unraid-api/nginx/nginx.module.js'; import { NginxService } from '@app/unraid-api/nginx/nginx.service.js'; @@ -42,6 +44,10 @@ import { upnpClient } from '@app/upnp/helpers.js'; provide: API_KEY_SERVICE_TOKEN, useClass: ApiKeyService, }, + { + provide: COOKIE_SERVICE_TOKEN, + useClass: CookieService, + }, { provide: NGINX_SERVICE_TOKEN, useClass: NginxService, @@ -58,6 +64,7 @@ import { upnpClient } from '@app/upnp/helpers.js'; UPNP_CLIENT_TOKEN, GRAPHQL_PUBSUB_TOKEN, API_KEY_SERVICE_TOKEN, + COOKIE_SERVICE_TOKEN, NGINX_SERVICE_TOKEN, INTERNAL_CLIENT_SERVICE_TOKEN, PrefixedID, diff --git a/api/src/unraid-api/shared/internal-graphql-client.factory.ts b/api/src/unraid-api/shared/internal-graphql-client.factory.ts index 4ee4523a74..9672aad696 100644 --- a/api/src/unraid-api/shared/internal-graphql-client.factory.ts +++ b/api/src/unraid-api/shared/internal-graphql-client.factory.ts @@ -36,20 +36,29 @@ export class InternalGraphQLClientFactory implements IInternalGraphQLClientFacto * Create a GraphQL client with the provided configuration. * * @param options Configuration options - * @param options.getApiKey Function to get the current API key + * @param options.getApiKey Function to get the current API key (optional if using cookie auth) + * @param options.getCookieAuth Function to get session and CSRF token for cookie auth (optional if using API key) * @param options.enableSubscriptions Optional flag to enable WebSocket subscriptions * @param options.origin Optional origin header (defaults to 'http://localhost') */ public async createClient(options: { - getApiKey: () => Promise; + getApiKey?: () => Promise; + getCookieAuth?: () => Promise<{ sessionId: string; csrfToken: string } | null>; enableSubscriptions?: boolean; origin?: string; }): Promise> { - if (!options.getApiKey) { - throw new Error('getApiKey function is required for creating a GraphQL client'); + if (!options.getApiKey && !options.getCookieAuth) { + throw new Error( + 'Either getApiKey or getCookieAuth function is required for creating a GraphQL client' + ); } - const { getApiKey, enableSubscriptions = false, origin = 'http://localhost' } = options; + const { + getApiKey, + getCookieAuth, + enableSubscriptions = false, + origin = 'http://localhost', + } = options; let httpLink: HttpLink; // Get WebSocket URI if subscriptions are enabled @@ -98,15 +107,33 @@ export class InternalGraphQLClientFactory implements IInternalGraphQLClientFacto }); } - // Create auth link that dynamically fetches the API key for each request + // Create auth link that dynamically fetches authentication info for each request const authLink = setContext(async (_, { headers }) => { - const apiKey = await getApiKey(); - return { - headers: { - ...headers, - 'x-api-key': apiKey, - }, - }; + if (getApiKey) { + // Use API key authentication + const apiKey = await getApiKey(); + return { + headers: { + ...headers, + 'x-api-key': apiKey, + }, + }; + } else if (getCookieAuth) { + // Use cookie-based authentication + const cookieAuth = await getCookieAuth(); + if (!cookieAuth) { + throw new Error('No valid session found for cookie authentication'); + } + return { + headers: { + ...headers, + 'x-csrf-token': cookieAuth.csrfToken, + cookie: `unraid_${cookieAuth.sessionId}=${cookieAuth.sessionId}`, + }, + }; + } + + return { headers }; }); const errorLink = onError(({ networkError }) => { @@ -121,8 +148,22 @@ export class InternalGraphQLClientFactory implements IInternalGraphQLClientFacto createClient({ url: wsUri, connectionParams: async () => { - const apiKey = await getApiKey(); - return { 'x-api-key': apiKey }; + if (getApiKey) { + const apiKey = await getApiKey(); + return { 'x-api-key': apiKey }; + } else if (getCookieAuth) { + const cookieAuth = await getCookieAuth(); + if (!cookieAuth) { + throw new Error( + 'No valid session found for WebSocket cookie authentication' + ); + } + return { + 'x-csrf-token': cookieAuth.csrfToken, + cookie: `unraid_${cookieAuth.sessionId}=${cookieAuth.sessionId}`, + }; + } + return {}; }, webSocketImpl: WebSocket, }) diff --git a/packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.spec.ts b/packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.spec.ts index cdd7e06ac1..dffa93fdfc 100644 --- a/packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.spec.ts +++ b/packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.spec.ts @@ -5,7 +5,8 @@ import { InternalClientService } from './internal.client.js'; describe('InternalClientService', () => { let service: InternalClientService; let clientFactory: any; - let apiKeyService: any; + let cookieService: any; + let configService: any; const mockApolloClient = { query: vi.fn(), @@ -18,13 +19,23 @@ describe('InternalClientService', () => { createClient: vi.fn().mockResolvedValue(mockApolloClient), }; - apiKeyService = { - getOrCreateLocalApiKey: vi.fn().mockResolvedValue('test-connect-key'), + cookieService = { + getActiveSession: vi.fn().mockResolvedValue('test-session-id'), + }; + + configService = { + get: vi.fn().mockImplementation((key: string) => { + if (key === 'store.emhttp.var.csrfToken') { + return 'test-csrf-token'; + } + return undefined; + }), }; service = new InternalClientService( clientFactory as any, - apiKeyService as any + cookieService as any, + configService as any ); }); @@ -37,20 +48,24 @@ describe('InternalClientService', () => { }); describe('getClient', () => { - it('should create a client with Connect API key and subscriptions', async () => { + it('should create a client with cookie authentication and subscriptions', async () => { const client = await service.getClient(); - // The API key is now fetched lazily through getApiKey function + // The cookie auth is now fetched lazily through getCookieAuth function expect(clientFactory.createClient).toHaveBeenCalledWith({ - getApiKey: expect.any(Function), + getCookieAuth: expect.any(Function), enableSubscriptions: true, }); - // Verify the getApiKey function works correctly when called + // Verify the getCookieAuth function works correctly when called const callArgs = vi.mocked(clientFactory.createClient).mock.calls[0][0]; - const apiKey = await callArgs.getApiKey(); - expect(apiKey).toBe('test-connect-key'); - expect(apiKeyService.getOrCreateLocalApiKey).toHaveBeenCalled(); + const cookieAuth = await callArgs.getCookieAuth(); + expect(cookieAuth).toEqual({ + sessionId: 'test-session-id', + csrfToken: 'test-csrf-token' + }); + expect(cookieService.getActiveSession).toHaveBeenCalled(); + expect(configService.get).toHaveBeenCalledWith('store.emhttp.var.csrfToken'); expect(client).toBe(mockApolloClient); }); @@ -105,6 +120,29 @@ describe('InternalClientService', () => { expect(client).toBe(mockApolloClient); expect(clientFactory.createClient).toHaveBeenCalledTimes(2); }); + + it('should handle missing session', async () => { + vi.mocked(cookieService.getActiveSession).mockResolvedValue(null); + + const client = await service.getClient(); + + // Verify getCookieAuth function returns null when no session + const callArgs = vi.mocked(clientFactory.createClient).mock.calls[0][0]; + const cookieAuth = await callArgs.getCookieAuth(); + expect(cookieAuth).toBeNull(); + expect(client).toBe(mockApolloClient); + }); + + it('should handle missing CSRF token', async () => { + vi.mocked(configService.get).mockReturnValue(undefined); + + const client = await service.getClient(); + + // Verify getCookieAuth function throws when CSRF token is missing + const callArgs = vi.mocked(clientFactory.createClient).mock.calls[0][0]; + await expect(callArgs.getCookieAuth()).rejects.toThrow('CSRF token not found in configuration'); + expect(client).toBe(mockApolloClient); + }); }); describe('clearClient', () => { diff --git a/packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts b/packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts index 87200b58b7..b9203e4d5f 100644 --- a/packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts +++ b/packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts @@ -1,13 +1,12 @@ import { Inject, Injectable, Logger } from '@nestjs/common'; +import { ConfigService } from '@nestjs/config'; import { ApolloClient, NormalizedCacheObject } from '@apollo/client/core/index.js'; -import { INTERNAL_CLIENT_SERVICE_TOKEN, type InternalGraphQLClientFactory } from '@unraid/shared'; - -import { ConnectApiKeyService } from '../authn/connect-api-key.service.js'; +import { COOKIE_SERVICE_TOKEN, INTERNAL_CLIENT_SERVICE_TOKEN, type InternalGraphQLClientFactory } from '@unraid/shared'; /** * Connect-specific internal GraphQL client. * - * This uses the shared GraphQL client factory with Connect's API key + * This uses the shared GraphQL client factory with cookie-based authentication * and enables subscriptions for real-time updates. */ @Injectable() @@ -19,7 +18,9 @@ export class InternalClientService { constructor( @Inject(INTERNAL_CLIENT_SERVICE_TOKEN) private readonly clientFactory: InternalGraphQLClientFactory, - private readonly apiKeyService: ConnectApiKeyService + @Inject(COOKIE_SERVICE_TOKEN) + private readonly cookieService: any, + private readonly configService: ConfigService ) {} public async getClient(): Promise> { @@ -55,13 +56,25 @@ export class InternalClientService { } private async createClient(): Promise> { - // Create a client with a function to get Connect's API key dynamically + // Create a client with cookie-based authentication const client = await this.clientFactory.createClient({ - getApiKey: () => this.apiKeyService.getOrCreateLocalApiKey(), + getCookieAuth: async () => { + const sessionId = await this.cookieService.getActiveSession(); + if (!sessionId) { + return null; + } + + const csrfToken = this.configService.get('store.emhttp.var.csrfToken'); + if (!csrfToken) { + throw new Error('CSRF token not found in configuration'); + } + + return { sessionId, csrfToken }; + }, enableSubscriptions: true }); - this.logger.debug('Created Connect internal GraphQL client with subscriptions enabled'); + this.logger.debug('Created Connect internal GraphQL client with cookie auth and subscriptions enabled'); return client; } diff --git a/packages/unraid-shared/src/tokens.ts b/packages/unraid-shared/src/tokens.ts index 97b79c1698..b33a1119b7 100644 --- a/packages/unraid-shared/src/tokens.ts +++ b/packages/unraid-shared/src/tokens.ts @@ -3,3 +3,4 @@ export const API_KEY_SERVICE_TOKEN = 'ApiKeyService'; export const LIFECYCLE_SERVICE_TOKEN = 'LifecycleService'; export const NGINX_SERVICE_TOKEN = 'NginxService'; export const INTERNAL_CLIENT_SERVICE_TOKEN = 'InternalClientService'; +export const COOKIE_SERVICE_TOKEN = 'CookieService'; diff --git a/packages/unraid-shared/src/types/internal-graphql-client.factory.ts b/packages/unraid-shared/src/types/internal-graphql-client.factory.ts index c557cee1ad..52a7afbb4e 100644 --- a/packages/unraid-shared/src/types/internal-graphql-client.factory.ts +++ b/packages/unraid-shared/src/types/internal-graphql-client.factory.ts @@ -6,7 +6,8 @@ import type { ApolloClient, NormalizedCacheObject } from '@apollo/client/core/in */ export interface InternalGraphQLClientFactory { createClient(options: { - getApiKey: () => Promise; + getApiKey?: () => Promise; + getCookieAuth?: () => Promise<{ sessionId: string; csrfToken: string } | null>; enableSubscriptions?: boolean; origin?: string; }): Promise>; From e128fbd8a238f236531d8cb3acb1bdbc9e9f0bdb Mon Sep 17 00:00:00 2001 From: Pujit Mehrotra Date: Tue, 26 Aug 2025 18:07:52 -0400 Subject: [PATCH 03/16] feat: canonical internal client --- .../b5b4aa3d-8e40-4c92-bc40-d50182071886.json | 11 - .../fc91da7b-0284-46f4-9018-55aa9759fba9.json | 11 - api/src/unraid-api/auth/auth.module.ts | 14 +- api/src/unraid-api/auth/auth.service.spec.ts | 7 +- api/src/unraid-api/auth/auth.service.ts | 45 ++- .../unraid-api/auth/authentication.guard.ts | 3 +- .../unraid-api/auth/local-session.service.ts | 98 ++++++ .../unraid-api/auth/local-session.strategy.ts | 46 +++ .../cli/__test__/api-report.service.test.ts | 5 +- .../__test__/developer-tools.service.test.ts | 9 +- api/src/unraid-api/cli/admin-key.service.ts | 81 ----- api/src/unraid-api/cli/api-report.service.ts | 13 +- api/src/unraid-api/cli/cli-services.module.ts | 14 +- api/src/unraid-api/cli/cli.module.spec.ts | 33 +- api/src/unraid-api/cli/cli.module.ts | 4 - .../cli/developer/developer-tools.service.ts | 11 +- .../cli/internal-client.service.spec.ts | 203 ------------ .../unraid-api/cli/internal-client.service.ts | 111 ------- .../cli/sso/validate-token.command.ts | 10 +- .../api-key/api-key.mutation.spec.ts | 3 +- .../api-key/api-key.resolver.spec.ts | 3 +- .../unraid-api/plugin/global-deps.module.ts | 17 +- .../__test__/rest-module.integration.test.ts | 4 +- .../shared/internal-client.service.ts | 68 ++-- .../shared/internal-graphql-client.factory.ts | 31 +- .../src/authn/connect-api-key.service.ts | 119 ------- .../src/internal-rpc/internal.client.spec.ts | 313 ------------------ .../mothership-subscription.handler.ts | 7 +- .../src/mothership-proxy/mothership.module.ts | 5 +- .../connect-settings.service.ts | 6 +- .../src/unraid-connect/connect.module.ts | 4 +- packages/unraid-shared/src/index.ts | 1 + packages/unraid-shared/src/tokens.ts | 1 + .../canonical-internal-client.interface.ts | 14 + .../types/internal-graphql-client.factory.ts | 1 + 35 files changed, 357 insertions(+), 969 deletions(-) delete mode 100644 api/dev/keys/b5b4aa3d-8e40-4c92-bc40-d50182071886.json delete mode 100644 api/dev/keys/fc91da7b-0284-46f4-9018-55aa9759fba9.json create mode 100644 api/src/unraid-api/auth/local-session.service.ts create mode 100644 api/src/unraid-api/auth/local-session.strategy.ts delete mode 100644 api/src/unraid-api/cli/admin-key.service.ts delete mode 100644 api/src/unraid-api/cli/internal-client.service.spec.ts delete mode 100644 api/src/unraid-api/cli/internal-client.service.ts rename packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts => api/src/unraid-api/shared/internal-client.service.ts (53%) delete mode 100644 packages/unraid-api-plugin-connect/src/authn/connect-api-key.service.ts delete mode 100644 packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.spec.ts create mode 100644 packages/unraid-shared/src/types/canonical-internal-client.interface.ts diff --git a/api/dev/keys/b5b4aa3d-8e40-4c92-bc40-d50182071886.json b/api/dev/keys/b5b4aa3d-8e40-4c92-bc40-d50182071886.json deleted file mode 100644 index 0bc2a78914..0000000000 --- a/api/dev/keys/b5b4aa3d-8e40-4c92-bc40-d50182071886.json +++ /dev/null @@ -1,11 +0,0 @@ -{ - "createdAt": "2025-01-27T16:22:56.501Z", - "description": "API key for Connect user", - "id": "b5b4aa3d-8e40-4c92-bc40-d50182071886", - "key": "_______________________LOCAL_API_KEY_HERE_________________________", - "name": "Connect", - "permissions": [], - "roles": [ - "CONNECT" - ] -} \ No newline at end of file diff --git a/api/dev/keys/fc91da7b-0284-46f4-9018-55aa9759fba9.json b/api/dev/keys/fc91da7b-0284-46f4-9018-55aa9759fba9.json deleted file mode 100644 index d734aead6f..0000000000 --- a/api/dev/keys/fc91da7b-0284-46f4-9018-55aa9759fba9.json +++ /dev/null @@ -1,11 +0,0 @@ -{ - "createdAt": "2025-07-23T17:34:06.301Z", - "description": "Internal admin API key used by CLI commands for system operations", - "id": "fc91da7b-0284-46f4-9018-55aa9759fba9", - "key": "_______SUPER_SECRET_KEY_______", - "name": "CliInternal", - "permissions": [], - "roles": [ - "ADMIN" - ] -} \ No newline at end of file diff --git a/api/src/unraid-api/auth/auth.module.ts b/api/src/unraid-api/auth/auth.module.ts index 7ce3ef9af6..29121c2ab5 100644 --- a/api/src/unraid-api/auth/auth.module.ts +++ b/api/src/unraid-api/auth/auth.module.ts @@ -11,13 +11,18 @@ import { BASE_POLICY, CASBIN_MODEL } from '@app/unraid-api/auth/casbin/index.js' import { CookieService, SESSION_COOKIE_CONFIG } from '@app/unraid-api/auth/cookie.service.js'; import { UserCookieStrategy } from '@app/unraid-api/auth/cookie.strategy.js'; import { ServerHeaderStrategy } from '@app/unraid-api/auth/header.strategy.js'; -import { AdminKeyService } from '@app/unraid-api/cli/admin-key.service.js'; +import { LocalSessionService } from '@app/unraid-api/auth/local-session.service.js'; +import { LocalSessionStrategy } from '@app/unraid-api/auth/local-session.strategy.js'; import { getRequest } from '@app/utils.js'; @Module({ imports: [ PassportModule.register({ - defaultStrategy: [ServerHeaderStrategy.key, UserCookieStrategy.key], + defaultStrategy: [ + ServerHeaderStrategy.key, + LocalSessionStrategy.key, + UserCookieStrategy.key, + ], }), CasbinModule, AuthZModule.register({ @@ -51,10 +56,11 @@ import { getRequest } from '@app/utils.js'; providers: [ AuthService, ApiKeyService, - AdminKeyService, ServerHeaderStrategy, + LocalSessionStrategy, UserCookieStrategy, CookieService, + LocalSessionService, { provide: SESSION_COOKIE_CONFIG, useValue: CookieService.defaultOpts(), @@ -65,8 +71,10 @@ import { getRequest } from '@app/utils.js'; ApiKeyService, PassportModule, ServerHeaderStrategy, + LocalSessionStrategy, UserCookieStrategy, CookieService, + LocalSessionService, AuthZModule, ], }) diff --git a/api/src/unraid-api/auth/auth.service.spec.ts b/api/src/unraid-api/auth/auth.service.spec.ts index 62178b4703..961368b89d 100644 --- a/api/src/unraid-api/auth/auth.service.spec.ts +++ b/api/src/unraid-api/auth/auth.service.spec.ts @@ -8,6 +8,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; 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 { LocalSessionService } from '@app/unraid-api/auth/local-session.service.js'; import { ApiKey } from '@app/unraid-api/graph/resolvers/api-key/api-key.model.js'; import { UserAccount } from '@app/unraid-api/graph/user/user.model.js'; import { FastifyRequest } from '@app/unraid-api/types/fastify.js'; @@ -17,6 +18,7 @@ describe('AuthService', () => { let apiKeyService: ApiKeyService; let authzService: AuthZService; let cookieService: CookieService; + let localSessionService: LocalSessionService; const mockApiKey: ApiKey = { id: 'test-api-id', @@ -55,7 +57,10 @@ describe('AuthService', () => { apiKeyService = new ApiKeyService(); authzService = new AuthZService(enforcer); cookieService = new CookieService(); - authService = new AuthService(cookieService, apiKeyService, authzService); + localSessionService = { + validateLocalSession: vi.fn(), + } as any; + authService = new AuthService(cookieService, apiKeyService, localSessionService, authzService); }); afterEach(() => { diff --git a/api/src/unraid-api/auth/auth.service.ts b/api/src/unraid-api/auth/auth.service.ts index 5ba0ef5495..1b7afb27b2 100644 --- a/api/src/unraid-api/auth/auth.service.ts +++ b/api/src/unraid-api/auth/auth.service.ts @@ -12,6 +12,7 @@ import { AuthZService } from 'nest-authz'; 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 { LocalSessionService } from '@app/unraid-api/auth/local-session.service.js'; import { Permission } from '@app/unraid-api/graph/resolvers/api-key/api-key.model.js'; import { UserAccount } from '@app/unraid-api/graph/user/user.model.js'; import { FastifyRequest } from '@app/unraid-api/types/fastify.js'; @@ -24,6 +25,7 @@ export class AuthService { constructor( private cookieService: CookieService, private apiKeyService: ApiKeyService, + private localSessionService: LocalSessionService, private authzService: AuthZService ) {} @@ -89,6 +91,30 @@ export class AuthService { } } + async validateLocalSession(localSessionToken: string): Promise { + try { + const isValid = await this.localSessionService.validateLocalSession(localSessionToken); + + if (!isValid) { + throw new UnauthorizedException('Invalid local session token'); + } + + // Local session has admin privileges + const user = await this.getLocalSessionUser(); + + // Sync the user's roles before checking them + await this.syncUserRoles(user.id, user.roles); + + // Now get the updated roles + const existingRoles = await this.authzService.getRolesForUser(user.id); + this.logger.debug(`Local session user ${user.id} has roles: ${existingRoles}`); + + return user; + } catch (error: unknown) { + handleAuthError(this.logger, 'Failed to validate local session', error); + } + } + public async syncApiKeyRoles(apiKeyId: string, roles: string[]): Promise { try { // Get existing roles and convert to Set @@ -321,7 +347,7 @@ export class AuthService { * @returns a service account that represents the user session (i.e. a webgui user). */ async getSessionUser(): Promise { - this.logger.debug('getSessionUser called!'); + this.logger.verbose('getSessionUser called!'); return { id: '-1', description: 'Session receives administrator permissions', @@ -330,4 +356,21 @@ export class AuthService { permissions: [], }; } + + /** + * Returns a user object representing a local session. + * Note: Does NOT perform validation. + * + * @returns a service account that represents the local session user (i.e. CLI/system operations). + */ + async getLocalSessionUser(): Promise { + this.logger.verbose('getLocalSessionUser called!'); + return { + id: '-2', + description: 'Local session receives administrator permissions for CLI/system operations', + name: 'local-admin', + roles: [Role.ADMIN], + permissions: [], + }; + } } diff --git a/api/src/unraid-api/auth/authentication.guard.ts b/api/src/unraid-api/auth/authentication.guard.ts index 9c9469d52f..367f9c2e35 100644 --- a/api/src/unraid-api/auth/authentication.guard.ts +++ b/api/src/unraid-api/auth/authentication.guard.ts @@ -13,6 +13,7 @@ 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'; +import { LocalSessionStrategy } from '@app/unraid-api/auth/local-session.strategy.js'; import { IS_PUBLIC_ENDPOINT_KEY } from '@app/unraid-api/auth/public.decorator.js'; /** @@ -37,7 +38,7 @@ type GraphQLContext = @Injectable() export class AuthenticationGuard - extends AuthGuard([ServerHeaderStrategy.key, UserCookieStrategy.key]) + extends AuthGuard([ServerHeaderStrategy.key, LocalSessionStrategy.key, UserCookieStrategy.key]) implements CanActivate { protected logger = new Logger(AuthenticationGuard.name); diff --git a/api/src/unraid-api/auth/local-session.service.ts b/api/src/unraid-api/auth/local-session.service.ts new file mode 100644 index 0000000000..901b89f742 --- /dev/null +++ b/api/src/unraid-api/auth/local-session.service.ts @@ -0,0 +1,98 @@ +import { Injectable, Logger, OnModuleInit } from '@nestjs/common'; +import { randomBytes } from 'crypto'; +import { chmod, mkdir, readFile, writeFile } from 'fs/promises'; +import { dirname } from 'path'; + +/** + * Service that manages a local session file for internal CLI/system authentication. + * Creates a secure token on startup that can be used for local system operations. + */ +@Injectable() +export class LocalSessionService implements OnModuleInit { + private readonly logger = new Logger(LocalSessionService.name); + private sessionToken: string | null = null; + private static readonly SESSION_FILE_PATH = '/var/run/unraid-api/local-session'; + + async onModuleInit() { + try { + await this.generateLocalSession(); + this.logger.log('Local session initialized'); + } catch (error) { + this.logger.error('Failed to initialize local session:', error); + } + } + + /** + * Generate a secure local session token and write it to file + */ + private async generateLocalSession(): Promise { + // Generate a cryptographically secure random token + this.sessionToken = randomBytes(32).toString('hex'); + + try { + // Ensure directory exists + await mkdir(dirname(LocalSessionService.SESSION_FILE_PATH), { recursive: true }); + + // Write token to file + await writeFile(LocalSessionService.SESSION_FILE_PATH, this.sessionToken, { + encoding: 'utf-8', + mode: 0o600, // Owner read/write only + }); + + // Ensure proper permissions (redundant but explicit) + await chmod(LocalSessionService.SESSION_FILE_PATH, 0o600); + + this.logger.debug(`Local session written to ${LocalSessionService.SESSION_FILE_PATH}`); + } catch (error) { + this.logger.error(`Failed to write local session: ${error}`); + throw error; + } + } + + /** + * Read and return the current local session token from file + */ + public async getLocalSession(): Promise { + try { + const token = await readFile(LocalSessionService.SESSION_FILE_PATH, 'utf-8'); + return token.trim(); + } catch (error) { + this.logger.debug('Local session file not found or not readable'); + return null; + } + } + + /** + * Validate if a given token matches the current local session + */ + public async validateLocalSession(token: string): Promise { + if (!token) return false; + + const currentToken = await this.getLocalSession(); + if (!currentToken) return false; + + // Use constant-time comparison to prevent timing attacks + return this.constantTimeEqual(token, currentToken); + } + + /** + * Constant-time string comparison to prevent timing attacks + */ + private constantTimeEqual(a: string, b: string): boolean { + if (a.length !== b.length) return false; + + let result = 0; + for (let i = 0; i < a.length; i++) { + result |= a.charCodeAt(i) ^ b.charCodeAt(i); + } + + return result === 0; + } + + /** + * Get the file path for the local session (useful for external readers) + */ + public static getSessionFilePath(): string { + return LocalSessionService.SESSION_FILE_PATH; + } +} diff --git a/api/src/unraid-api/auth/local-session.strategy.ts b/api/src/unraid-api/auth/local-session.strategy.ts new file mode 100644 index 0000000000..ef0bb6ef91 --- /dev/null +++ b/api/src/unraid-api/auth/local-session.strategy.ts @@ -0,0 +1,46 @@ +import { Injectable, Logger } from '@nestjs/common'; +import { PassportStrategy } from '@nestjs/passport'; + +import { Strategy } from 'passport-custom'; + +import { AuthService } from '@app/unraid-api/auth/auth.service.js'; +import { UserAccount } from '@app/unraid-api/graph/user/user.model.js'; +import { FastifyRequest } from '@app/unraid-api/types/fastify.js'; + +/** + * Passport strategy for local session authentication. + * Validates the x-local-session header for internal CLI/system operations. + */ +@Injectable() +export class LocalSessionStrategy extends PassportStrategy(Strategy, 'local-session') { + static readonly key = 'local-session'; + private readonly logger = new Logger(LocalSessionStrategy.name); + + constructor(private readonly authService: AuthService) { + super(); + } + + async validate(request: FastifyRequest): Promise { + try { + const localSessionToken = request.headers['x-local-session'] as string; + + if (!localSessionToken) { + this.logger.verbose('No local session token found in request headers'); + return null; + } + + this.logger.verbose('Attempting to validate local session token'); + const user = await this.authService.validateLocalSession(localSessionToken); + + if (user) { + this.logger.verbose(`Local session authenticated user: ${user.name}`); + return user; + } + + return null; + } catch (error) { + this.logger.verbose(error, `Local session validation failed`); + return null; + } + } +} diff --git a/api/src/unraid-api/cli/__test__/api-report.service.test.ts b/api/src/unraid-api/cli/__test__/api-report.service.test.ts index 36f1f14737..8dc1c6cde5 100644 --- a/api/src/unraid-api/cli/__test__/api-report.service.test.ts +++ b/api/src/unraid-api/cli/__test__/api-report.service.test.ts @@ -1,9 +1,10 @@ import { Test } from '@nestjs/testing'; +import type { CanonicalInternalClientService } from '@unraid/shared'; +import { CANONICAL_INTERNAL_CLIENT_TOKEN } from '@unraid/shared'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { ApiReportService } from '@app/unraid-api/cli/api-report.service.js'; -import { CliInternalClientService } from '@app/unraid-api/cli/internal-client.service.js'; import { LogService } from '@app/unraid-api/cli/log.service.js'; import { CONNECT_STATUS_QUERY, @@ -40,7 +41,7 @@ describe('ApiReportService', () => { providers: [ ApiReportService, { provide: LogService, useValue: mockLogService }, - { provide: CliInternalClientService, useValue: mockInternalClientService }, + { provide: CANONICAL_INTERNAL_CLIENT_TOKEN, useValue: mockInternalClientService }, ], }).compile(); diff --git a/api/src/unraid-api/cli/__test__/developer-tools.service.test.ts b/api/src/unraid-api/cli/__test__/developer-tools.service.test.ts index 5e1b644df8..0bd584502a 100644 --- a/api/src/unraid-api/cli/__test__/developer-tools.service.test.ts +++ b/api/src/unraid-api/cli/__test__/developer-tools.service.test.ts @@ -1,10 +1,11 @@ import { Test, TestingModule } from '@nestjs/testing'; import { access, readFile, unlink, writeFile } from 'fs/promises'; +import type { CanonicalInternalClientService } from '@unraid/shared'; +import { CANONICAL_INTERNAL_CLIENT_TOKEN } from '@unraid/shared'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { DeveloperToolsService } from '@app/unraid-api/cli/developer/developer-tools.service.js'; -import { CliInternalClientService } from '@app/unraid-api/cli/internal-client.service.js'; import { LogService } from '@app/unraid-api/cli/log.service.js'; import { RestartCommand } from '@app/unraid-api/cli/restart.command.js'; @@ -15,7 +16,7 @@ describe('DeveloperToolsService', () => { let service: DeveloperToolsService; let logService: LogService; let restartCommand: RestartCommand; - let internalClient: CliInternalClientService; + let internalClient: CanonicalInternalClientService; const mockClient = { mutate: vi.fn(), @@ -42,7 +43,7 @@ describe('DeveloperToolsService', () => { }, }, { - provide: CliInternalClientService, + provide: CANONICAL_INTERNAL_CLIENT_TOKEN, useValue: { getClient: vi.fn().mockResolvedValue(mockClient), }, @@ -53,7 +54,7 @@ describe('DeveloperToolsService', () => { service = module.get(DeveloperToolsService); logService = module.get(LogService); restartCommand = module.get(RestartCommand); - internalClient = module.get(CliInternalClientService); + internalClient = module.get(CANONICAL_INTERNAL_CLIENT_TOKEN); }); describe('setSandboxMode', () => { diff --git a/api/src/unraid-api/cli/admin-key.service.ts b/api/src/unraid-api/cli/admin-key.service.ts deleted file mode 100644 index bcd8e76ebf..0000000000 --- a/api/src/unraid-api/cli/admin-key.service.ts +++ /dev/null @@ -1,81 +0,0 @@ -import { Inject, Injectable, Logger, OnModuleInit } from '@nestjs/common'; -import crypto from 'crypto'; -import { mkdir, writeFile } from 'fs/promises'; -import { join } from 'path'; - -import type { ApiKeyService } from '@unraid/shared/services/api-key.js'; -import { Role } from '@unraid/shared/graphql.model.js'; -import { API_KEY_SERVICE_TOKEN } from '@unraid/shared/tokens.js'; - -/** - * Service that creates and manages the ephemeral admin key used by CLI commands. - * Generates a temporary key stored in /var/run that expires on restart. - */ -@Injectable() -export class AdminKeyService implements OnModuleInit { - private readonly logger = new Logger(AdminKeyService.name); - private ephemeralKey: string | null = null; - private static readonly RUNTIME_KEY_PATH = '/var/run/unraid-api/cli.key'; - - constructor( - @Inject(API_KEY_SERVICE_TOKEN) - private readonly apiKeyService: ApiKeyService - ) {} - - async onModuleInit() { - try { - await this.generateEphemeralCliKey(); - this.logger.log('CLI ephemeral key initialized'); - } catch (error) { - this.logger.error('Failed to initialize CLI ephemeral key:', error); - } - } - - /** - * Generates an ephemeral key for CLI operations. - * Writes the key to /var/run for the CLI process to read. - */ - private async generateEphemeralCliKey(): Promise { - // Generate cryptographically secure key - this.ephemeralKey = crypto.randomBytes(32).toString('hex'); - - // Register in ApiKeyService memory store - await this.apiKeyService.registerEphemeralKey({ - key: this.ephemeralKey, - name: 'CLI-Runtime', - roles: [Role.ADMIN], - type: 'cli', - }); - - // Write to /var/run for CLI process - const runDir = '/var/run/unraid-api'; - try { - await mkdir(runDir, { recursive: true }); - await writeFile( - AdminKeyService.RUNTIME_KEY_PATH, - this.ephemeralKey, - { mode: 0o600 } // Root only - ); - this.logger.debug('CLI ephemeral key written to runtime directory'); - } catch (error) { - // If we can't write to /var/run, it's okay - CLI will fall back to getting it from service - this.logger.warn(`Could not write CLI key to ${AdminKeyService.RUNTIME_KEY_PATH}: ${error}`); - } - } - - /** - * Gets the ephemeral admin key for CLI operations. - * If not yet generated, creates it on-demand. - */ - public async getOrCreateLocalAdminKey(): Promise { - if (!this.ephemeralKey) { - await this.generateEphemeralCliKey(); - } - - if (!this.ephemeralKey) { - throw new Error('Failed to generate CLI ephemeral key'); - } - - return this.ephemeralKey; - } -} diff --git a/api/src/unraid-api/cli/api-report.service.ts b/api/src/unraid-api/cli/api-report.service.ts index 6b2ecac41d..c2648abad3 100644 --- a/api/src/unraid-api/cli/api-report.service.ts +++ b/api/src/unraid-api/cli/api-report.service.ts @@ -1,7 +1,9 @@ -import { Injectable } from '@nestjs/common'; +import { Inject, Injectable } from '@nestjs/common'; + +import type { CanonicalInternalClientService } from '@unraid/shared'; +import { CANONICAL_INTERNAL_CLIENT_TOKEN } from '@unraid/shared'; import type { ConnectStatusQuery, SystemReportQuery } from '@app/unraid-api/cli/generated/graphql.js'; -import { CliInternalClientService } from '@app/unraid-api/cli/internal-client.service.js'; import { LogService } from '@app/unraid-api/cli/log.service.js'; import { CONNECT_STATUS_QUERY, @@ -60,7 +62,8 @@ export interface ApiReportData { @Injectable() export class ApiReportService { constructor( - private readonly internalClient: CliInternalClientService, + @Inject(CANONICAL_INTERNAL_CLIENT_TOKEN) + private readonly internalClient: CanonicalInternalClientService, private readonly logger: LogService ) {} @@ -135,7 +138,7 @@ export class ApiReportService { }); } - const client = await this.internalClient.getClient(); + const client = await this.internalClient.getClient({ enableSubscriptions: false }); // Query system data let systemResult: { data: SystemReportQuery } | null = null; @@ -190,7 +193,7 @@ export class ApiReportService { return this.createApiReportData({ apiRunning, - systemData: systemResult.data, + systemData: systemResult?.data, connectData, servicesData, }); diff --git a/api/src/unraid-api/cli/cli-services.module.ts b/api/src/unraid-api/cli/cli-services.module.ts index cff41d1b07..7f248390d0 100644 --- a/api/src/unraid-api/cli/cli-services.module.ts +++ b/api/src/unraid-api/cli/cli-services.module.ts @@ -2,9 +2,7 @@ import { Module } from '@nestjs/common'; import { DependencyService } from '@app/unraid-api/app/dependency.service.js'; import { ApiKeyService } from '@app/unraid-api/auth/api-key.service.js'; -import { AdminKeyService } from '@app/unraid-api/cli/admin-key.service.js'; import { ApiReportService } from '@app/unraid-api/cli/api-report.service.js'; -import { CliInternalClientService } from '@app/unraid-api/cli/internal-client.service.js'; import { LogService } from '@app/unraid-api/cli/log.service.js'; import { PM2Service } from '@app/unraid-api/cli/pm2.service.js'; import { ApiConfigModule } from '@app/unraid-api/config/api-config.module.js'; @@ -23,15 +21,7 @@ import { UnraidFileModifierModule } from '@app/unraid-api/unraid-file-modifier/u PluginCliModule.register(), UnraidFileModifierModule, ], - providers: [ - LogService, - PM2Service, - ApiKeyService, - DependencyService, - AdminKeyService, - ApiReportService, - CliInternalClientService, - ], - exports: [ApiReportService, LogService, ApiKeyService, CliInternalClientService], + providers: [LogService, PM2Service, ApiKeyService, DependencyService, ApiReportService], + exports: [ApiReportService, LogService, ApiKeyService], }) export class CliServicesModule {} diff --git a/api/src/unraid-api/cli/cli.module.spec.ts b/api/src/unraid-api/cli/cli.module.spec.ts index 97fd0ac8e1..b18385146e 100644 --- a/api/src/unraid-api/cli/cli.module.spec.ts +++ b/api/src/unraid-api/cli/cli.module.spec.ts @@ -1,12 +1,14 @@ import { ConfigModule } from '@nestjs/config'; import { Test, TestingModule } from '@nestjs/testing'; -import { INTERNAL_CLIENT_SERVICE_TOKEN } from '@unraid/shared'; +import { + CANONICAL_INTERNAL_CLIENT_TOKEN, + CANONICAL_INTERNAL_CLIENT_TOKEN, + INTERNAL_CLIENT_SERVICE_TOKEN, +} from '@unraid/shared'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; -import { AdminKeyService } from '@app/unraid-api/cli/admin-key.service.js'; import { CliServicesModule } from '@app/unraid-api/cli/cli-services.module.js'; -import { CliInternalClientService } from '@app/unraid-api/cli/internal-client.service.js'; import { InternalGraphQLClientFactory } from '@app/unraid-api/shared/internal-graphql-client.factory.js'; describe('CliServicesModule', () => { @@ -26,16 +28,10 @@ describe('CliServicesModule', () => { expect(module).toBeDefined(); }); - it('should provide CliInternalClientService', () => { - const service = module.get(CliInternalClientService); + it('should provide CanonicalInternalClient', () => { + const service = module.get(CANONICAL_INTERNAL_CLIENT_TOKEN); expect(service).toBeDefined(); - expect(service).toBeInstanceOf(CliInternalClientService); - }); - - it('should provide AdminKeyService', () => { - const service = module.get(AdminKeyService); - expect(service).toBeDefined(); - expect(service).toBeInstanceOf(AdminKeyService); + expect(service.getClient).toBeInstanceOf(Function); }); it('should provide InternalGraphQLClientFactory via token', () => { @@ -44,11 +40,11 @@ describe('CliServicesModule', () => { expect(factory).toBeInstanceOf(InternalGraphQLClientFactory); }); - describe('CliInternalClientService dependencies', () => { + describe('CanonicalInternalClient dependencies', () => { it('should have all required dependencies available', () => { - // This test ensures that CliInternalClientService can be instantiated + // This test ensures that CanonicalInternalClient can be instantiated // with all its dependencies properly resolved - const service = module.get(CliInternalClientService); + const service = module.get(CANONICAL_INTERNAL_CLIENT_TOKEN); expect(service).toBeDefined(); // Verify the service has its dependencies injected @@ -63,12 +59,5 @@ describe('CliServicesModule', () => { expect(factory).toBeDefined(); expect(factory.createClient).toBeDefined(); }); - - it('should resolve AdminKeyService dependency', () => { - // Explicitly test that AdminKeyService is available in the module context - const adminKeyService = module.get(AdminKeyService); - expect(adminKeyService).toBeDefined(); - expect(adminKeyService.getOrCreateLocalAdminKey).toBeDefined(); - }); }); }); diff --git a/api/src/unraid-api/cli/cli.module.ts b/api/src/unraid-api/cli/cli.module.ts index 31e9566e68..7befdcb0e4 100644 --- a/api/src/unraid-api/cli/cli.module.ts +++ b/api/src/unraid-api/cli/cli.module.ts @@ -3,7 +3,6 @@ import { ConfigModule } from '@nestjs/config'; import { DependencyService } from '@app/unraid-api/app/dependency.service.js'; import { ApiKeyService } from '@app/unraid-api/auth/api-key.service.js'; -import { AdminKeyService } from '@app/unraid-api/cli/admin-key.service.js'; import { ApiReportService } from '@app/unraid-api/cli/api-report.service.js'; import { AddApiKeyQuestionSet } from '@app/unraid-api/cli/apikey/add-api-key.questions.js'; import { ApiKeyCommand } from '@app/unraid-api/cli/apikey/api-key.command.js'; @@ -12,7 +11,6 @@ import { ConfigCommand } from '@app/unraid-api/cli/config.command.js'; import { DeveloperToolsService } from '@app/unraid-api/cli/developer/developer-tools.service.js'; import { DeveloperCommand } from '@app/unraid-api/cli/developer/developer.command.js'; import { DeveloperQuestions } from '@app/unraid-api/cli/developer/developer.questions.js'; -import { CliInternalClientService } from '@app/unraid-api/cli/internal-client.service.js'; import { LogService } from '@app/unraid-api/cli/log.service.js'; import { LogsCommand } from '@app/unraid-api/cli/logs.command.js'; import { @@ -69,9 +67,7 @@ const DEFAULT_PROVIDERS = [ PM2Service, ApiKeyService, DependencyService, - AdminKeyService, ApiReportService, - CliInternalClientService, ] as const; @Module({ diff --git a/api/src/unraid-api/cli/developer/developer-tools.service.ts b/api/src/unraid-api/cli/developer/developer-tools.service.ts index c03dbaa5c7..5dc0784f56 100644 --- a/api/src/unraid-api/cli/developer/developer-tools.service.ts +++ b/api/src/unraid-api/cli/developer/developer-tools.service.ts @@ -1,8 +1,10 @@ -import { Injectable, Logger } from '@nestjs/common'; +import { Inject, Injectable, Logger } from '@nestjs/common'; import { access, readFile, unlink, writeFile } from 'fs/promises'; import * as path from 'path'; -import { CliInternalClientService } from '@app/unraid-api/cli/internal-client.service.js'; +import type { CanonicalInternalClientService } from '@unraid/shared'; +import { CANONICAL_INTERNAL_CLIENT_TOKEN } from '@unraid/shared'; + import { LogService } from '@app/unraid-api/cli/log.service.js'; import { UPDATE_SANDBOX_MUTATION } from '@app/unraid-api/cli/queries/developer.mutation.js'; import { RestartCommand } from '@app/unraid-api/cli/restart.command.js'; @@ -52,12 +54,13 @@ unraid-dev-modal-test { constructor( private readonly logger: LogService, private readonly restartCommand: RestartCommand, - private readonly internalClient: CliInternalClientService + @Inject(CANONICAL_INTERNAL_CLIENT_TOKEN) + private readonly internalClient: CanonicalInternalClientService ) {} async setSandboxMode(enable: boolean): Promise { try { - const client = await this.internalClient.getClient(); + const client = await this.internalClient.getClient({ enableSubscriptions: false }); const result = await client.mutate({ mutation: UPDATE_SANDBOX_MUTATION, diff --git a/api/src/unraid-api/cli/internal-client.service.spec.ts b/api/src/unraid-api/cli/internal-client.service.spec.ts deleted file mode 100644 index c17468d23e..0000000000 --- a/api/src/unraid-api/cli/internal-client.service.spec.ts +++ /dev/null @@ -1,203 +0,0 @@ -import { ConfigModule, ConfigService } from '@nestjs/config'; -import { Test, TestingModule } from '@nestjs/testing'; - -import type { InternalGraphQLClientFactory } from '@unraid/shared'; -import { ApolloClient } from '@apollo/client/core/index.js'; -import { INTERNAL_CLIENT_SERVICE_TOKEN } from '@unraid/shared'; -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; - -import { AdminKeyService } from '@app/unraid-api/cli/admin-key.service.js'; -import { CliInternalClientService } from '@app/unraid-api/cli/internal-client.service.js'; - -describe('CliInternalClientService', () => { - let service: CliInternalClientService; - let clientFactory: InternalGraphQLClientFactory; - let adminKeyService: AdminKeyService; - let module: TestingModule; - - const mockApolloClient = { - query: vi.fn(), - mutate: vi.fn(), - stop: vi.fn(), - }; - - beforeEach(async () => { - module = await Test.createTestingModule({ - imports: [ConfigModule.forRoot()], - providers: [ - CliInternalClientService, - { - provide: INTERNAL_CLIENT_SERVICE_TOKEN, - useValue: { - createClient: vi.fn().mockResolvedValue(mockApolloClient), - }, - }, - { - provide: AdminKeyService, - useValue: { - getOrCreateLocalAdminKey: vi.fn().mockResolvedValue('test-admin-key'), - }, - }, - ], - }).compile(); - - service = module.get(CliInternalClientService); - clientFactory = module.get(INTERNAL_CLIENT_SERVICE_TOKEN); - adminKeyService = module.get(AdminKeyService); - }); - - afterEach(async () => { - await module?.close(); - }); - - it('should be defined', () => { - expect(service).toBeDefined(); - }); - - describe('dependency injection', () => { - it('should have InternalGraphQLClientFactory injected', () => { - expect(clientFactory).toBeDefined(); - expect(clientFactory.createClient).toBeDefined(); - }); - - it('should have AdminKeyService injected', () => { - expect(adminKeyService).toBeDefined(); - expect(adminKeyService.getOrCreateLocalAdminKey).toBeDefined(); - }); - }); - - describe('getClient', () => { - it('should create a client with getApiKey function', async () => { - const client = await service.getClient(); - - // The API key is now fetched lazily, not immediately - expect(clientFactory.createClient).toHaveBeenCalledWith({ - getApiKey: expect.any(Function), - enableSubscriptions: false, - }); - - // Verify the getApiKey function works correctly when called - const callArgs = vi.mocked(clientFactory.createClient).mock.calls[0][0]; - const apiKey = await callArgs.getApiKey(); - expect(apiKey).toBe('test-admin-key'); - expect(adminKeyService.getOrCreateLocalAdminKey).toHaveBeenCalled(); - - expect(client).toBe(mockApolloClient); - }); - - it('should return cached client on subsequent calls', async () => { - const client1 = await service.getClient(); - const client2 = await service.getClient(); - - expect(client1).toBe(client2); - expect(clientFactory.createClient).toHaveBeenCalledTimes(1); - }); - - it('should handle errors when getting admin key', async () => { - const error = new Error('Failed to get admin key'); - vi.mocked(adminKeyService.getOrCreateLocalAdminKey).mockRejectedValueOnce(error); - - // The client creation will succeed, but the API key error happens later - const client = await service.getClient(); - expect(client).toBe(mockApolloClient); - - // Now test that the getApiKey function throws the expected error - const callArgs = vi.mocked(clientFactory.createClient).mock.calls[0][0]; - await expect(callArgs.getApiKey()).rejects.toThrow(); - }); - }); - - describe('clearClient', () => { - it('should stop and clear the client', async () => { - // First create a client - await service.getClient(); - - // Clear the client - service.clearClient(); - - expect(mockApolloClient.stop).toHaveBeenCalled(); - }); - - it('should handle clearing when no client exists', () => { - // Should not throw when clearing a non-existent client - expect(() => service.clearClient()).not.toThrow(); - }); - - it('should create a new client after clearing', async () => { - // Create initial client - await service.getClient(); - - // Clear it - service.clearClient(); - - // Create new client - await service.getClient(); - - // Should have created client twice - expect(clientFactory.createClient).toHaveBeenCalledTimes(2); - }); - }); - - describe('race condition protection', () => { - it('should prevent stale client resurrection when clearClient() is called during creation', async () => { - let resolveClientCreation!: (client: any) => void; - - // Mock createClient to return a controllable promise - const clientCreationPromise = new Promise((resolve) => { - resolveClientCreation = resolve; - }); - vi.mocked(clientFactory.createClient).mockReturnValueOnce(clientCreationPromise); - - // Start client creation (but don't await yet) - const getClientPromise = service.getClient(); - - // Clear the client while creation is in progress - service.clearClient(); - - // Now complete the client creation - resolveClientCreation(mockApolloClient); - - // Wait for getClient to complete - const client = await getClientPromise; - - // The client should be returned from getClient - expect(client).toBe(mockApolloClient); - - // But subsequent getClient calls should create a new client - // because the race condition protection prevented assignment - await service.getClient(); - - // Should have created a second client, proving the first wasn't assigned - expect(clientFactory.createClient).toHaveBeenCalledTimes(2); - }); - - it('should handle concurrent getClient calls during race condition', async () => { - let resolveClientCreation!: (client: any) => void; - - // Mock createClient to return a controllable promise - const clientCreationPromise = new Promise((resolve) => { - resolveClientCreation = resolve; - }); - vi.mocked(clientFactory.createClient).mockReturnValueOnce(clientCreationPromise); - - // Start multiple concurrent client creation calls - const getClientPromise1 = service.getClient(); - const getClientPromise2 = service.getClient(); // Should wait for first one - - // Clear the client while creation is in progress - service.clearClient(); - - // Complete the client creation - resolveClientCreation(mockApolloClient); - - // Both calls should resolve with the same client - const [client1, client2] = await Promise.all([getClientPromise1, getClientPromise2]); - expect(client1).toBe(mockApolloClient); - expect(client2).toBe(mockApolloClient); - - // But the client should not be cached due to race condition protection - await service.getClient(); - expect(clientFactory.createClient).toHaveBeenCalledTimes(2); - }); - }); -}); diff --git a/api/src/unraid-api/cli/internal-client.service.ts b/api/src/unraid-api/cli/internal-client.service.ts deleted file mode 100644 index 4199aab9f7..0000000000 --- a/api/src/unraid-api/cli/internal-client.service.ts +++ /dev/null @@ -1,111 +0,0 @@ -import { Inject, Injectable, Logger } from '@nestjs/common'; -import { readFile } from 'fs/promises'; - -import type { InternalGraphQLClientFactory } from '@unraid/shared'; -import { ApolloClient, NormalizedCacheObject } from '@apollo/client/core/index.js'; -import { INTERNAL_CLIENT_SERVICE_TOKEN } from '@unraid/shared'; - -import { AdminKeyService } from '@app/unraid-api/cli/admin-key.service.js'; - -/** - * Internal GraphQL client for CLI commands. - * - * This service creates an Apollo client that queries the local API server - * with admin privileges for CLI operations. - */ -@Injectable() -export class CliInternalClientService { - private readonly logger = new Logger(CliInternalClientService.name); - private client: ApolloClient | null = null; - private creatingClient: Promise> | null = null; - - constructor( - @Inject(INTERNAL_CLIENT_SERVICE_TOKEN) - private readonly clientFactory: InternalGraphQLClientFactory, - private readonly adminKeyService: AdminKeyService - ) {} - - /** - * Get the admin API key for CLI operations. - * First tries to read from /var/run/unraid-api/cli.key, then falls back to AdminKeyService. - */ - private async getLocalApiKey(): Promise { - // Try to read ephemeral key from /var/run first - try { - const ephemeralKey = await readFile('/var/run/unraid-api/cli.key', 'utf-8'); - if (ephemeralKey) { - this.logger.debug('Using ephemeral CLI key from runtime directory'); - return ephemeralKey.trim(); - } - } catch (error) { - // File doesn't exist or not readable, fall back to service - this.logger.debug('Ephemeral key file not found, falling back to AdminKeyService'); - } - - // Fall back to AdminKeyService - try { - return await this.adminKeyService.getOrCreateLocalAdminKey(); - } catch (error) { - this.logger.error('Failed to get admin API key:', error); - throw new Error( - 'Unable to get admin API key for internal client. Ensure the API server is running.' - ); - } - } - - /** - * Get the default CLI client with admin API key. - * This is for CLI commands that need admin access. - */ - public async getClient(): Promise> { - // If client already exists, return it - if (this.client) { - return this.client; - } - - // If another call is already creating the client, wait for it - if (this.creatingClient) { - return await this.creatingClient; - } - - // Start creating the client with race condition protection - let creationPromise!: Promise>; - // eslint-disable-next-line prefer-const - creationPromise = (async () => { - try { - const client = await this.clientFactory.createClient({ - getApiKey: () => this.getLocalApiKey(), - enableSubscriptions: false, // CLI doesn't need subscriptions - }); - - // awaiting *before* checking this.creatingClient is important! - // by yielding to the event loop, it ensures - // `this.creatingClient = creationPromise;` is executed before the next check. - - // This prevents race conditions where the client is assigned to the wrong instance. - // Only assign client if this creation is still current - if (this.creatingClient === creationPromise) { - this.client = client; - this.logger.debug('Created CLI internal GraphQL client with admin privileges'); - } - - return client; - } finally { - // Only clear if this creation is still current - if (this.creatingClient === creationPromise) { - this.creatingClient = null; - } - } - })(); - - this.creatingClient = creationPromise; - return await creationPromise; - } - - public clearClient() { - // Stop the Apollo client to terminate any active processes - this.client?.stop(); - this.client = null; - this.creatingClient = null; - } -} diff --git a/api/src/unraid-api/cli/sso/validate-token.command.ts b/api/src/unraid-api/cli/sso/validate-token.command.ts index 0440c48313..f12a6df9bf 100644 --- a/api/src/unraid-api/cli/sso/validate-token.command.ts +++ b/api/src/unraid-api/cli/sso/validate-token.command.ts @@ -1,6 +1,9 @@ +import { Inject } from '@nestjs/common'; + +import type { CanonicalInternalClientService } from '@unraid/shared'; +import { CANONICAL_INTERNAL_CLIENT_TOKEN } from '@unraid/shared'; import { CommandRunner, SubCommand } from 'nest-commander'; -import { CliInternalClientService } from '@app/unraid-api/cli/internal-client.service.js'; import { LogService } from '@app/unraid-api/cli/log.service.js'; import { VALIDATE_OIDC_SESSION_QUERY } from '@app/unraid-api/cli/queries/validate-oidc-session.query.js'; @@ -13,7 +16,8 @@ import { VALIDATE_OIDC_SESSION_QUERY } from '@app/unraid-api/cli/queries/validat export class ValidateTokenCommand extends CommandRunner { constructor( private readonly logger: LogService, - private readonly internalClient: CliInternalClientService + @Inject(CANONICAL_INTERNAL_CLIENT_TOKEN) + private readonly internalClient: CanonicalInternalClientService ) { super(); } @@ -45,7 +49,7 @@ export class ValidateTokenCommand extends CommandRunner { private async validateOidcToken(token: string): Promise { try { - const client = await this.internalClient.getClient(); + const client = await this.internalClient.getClient({ enableSubscriptions: false }); const { data, errors } = await client.query({ query: VALIDATE_OIDC_SESSION_QUERY, variables: { token }, diff --git a/api/src/unraid-api/graph/resolvers/api-key/api-key.mutation.spec.ts b/api/src/unraid-api/graph/resolvers/api-key/api-key.mutation.spec.ts index ea1e05eea9..6ce9977a67 100644 --- a/api/src/unraid-api/graph/resolvers/api-key/api-key.mutation.spec.ts +++ b/api/src/unraid-api/graph/resolvers/api-key/api-key.mutation.spec.ts @@ -38,7 +38,8 @@ describe('ApiKeyMutationsResolver', () => { apiKeyService = new ApiKeyService(); authzService = new AuthZService(enforcer); cookieService = new CookieService(); - authService = new AuthService(cookieService, apiKeyService, authzService); + const localSessionService = { validateLocalSession: vi.fn() } as any; + authService = new AuthService(cookieService, apiKeyService, localSessionService, authzService); resolver = new ApiKeyMutationsResolver(authService, apiKeyService); }); diff --git a/api/src/unraid-api/graph/resolvers/api-key/api-key.resolver.spec.ts b/api/src/unraid-api/graph/resolvers/api-key/api-key.resolver.spec.ts index bdf483a609..0ca32e14a0 100644 --- a/api/src/unraid-api/graph/resolvers/api-key/api-key.resolver.spec.ts +++ b/api/src/unraid-api/graph/resolvers/api-key/api-key.resolver.spec.ts @@ -34,7 +34,8 @@ describe('ApiKeyResolver', () => { apiKeyService = new ApiKeyService(); authzService = new AuthZService(enforcer); cookieService = new CookieService(); - authService = new AuthService(cookieService, apiKeyService, authzService); + const localSessionService = { validateLocalSession: vi.fn() } as any; + authService = new AuthService(cookieService, apiKeyService, localSessionService, authzService); resolver = new ApiKeyResolver(apiKeyService); }); diff --git a/api/src/unraid-api/plugin/global-deps.module.ts b/api/src/unraid-api/plugin/global-deps.module.ts index 37f623070e..8107f0e03d 100644 --- a/api/src/unraid-api/plugin/global-deps.module.ts +++ b/api/src/unraid-api/plugin/global-deps.module.ts @@ -5,6 +5,7 @@ import { PrefixedID } from '@unraid/shared/prefixed-id-scalar.js'; import { GRAPHQL_PUBSUB_TOKEN } from '@unraid/shared/pubsub/graphql.pubsub.js'; import { API_KEY_SERVICE_TOKEN, + CANONICAL_INTERNAL_CLIENT_TOKEN, COOKIE_SERVICE_TOKEN, INTERNAL_CLIENT_SERVICE_TOKEN, LIFECYCLE_SERVICE_TOKEN, @@ -15,10 +16,12 @@ import { import { pubsub } from '@app/core/pubsub.js'; import { LifecycleService } from '@app/unraid-api/app/lifecycle.service.js'; import { ApiKeyService } from '@app/unraid-api/auth/api-key.service.js'; -import { CookieService } from '@app/unraid-api/auth/cookie.service.js'; +import { CookieService, SESSION_COOKIE_CONFIG } from '@app/unraid-api/auth/cookie.service.js'; +import { LocalSessionService } from '@app/unraid-api/auth/local-session.service.js'; import { ApiKeyModule } from '@app/unraid-api/graph/resolvers/api-key/api-key.module.js'; import { NginxModule } from '@app/unraid-api/nginx/nginx.module.js'; import { NginxService } from '@app/unraid-api/nginx/nginx.service.js'; +import { InternalClientService } from '@app/unraid-api/shared/internal-client.service.js'; import { InternalGraphQLClientFactory } from '@app/unraid-api/shared/internal-graphql-client.factory.js'; import { upnpClient } from '@app/upnp/helpers.js'; @@ -44,6 +47,10 @@ import { upnpClient } from '@app/upnp/helpers.js'; provide: API_KEY_SERVICE_TOKEN, useClass: ApiKeyService, }, + { + provide: SESSION_COOKIE_CONFIG, + useValue: CookieService.defaultOpts(), + }, { provide: COOKIE_SERVICE_TOKEN, useClass: CookieService, @@ -52,6 +59,13 @@ import { upnpClient } from '@app/upnp/helpers.js'; provide: NGINX_SERVICE_TOKEN, useClass: NginxService, }, + // Canonical internal client service + LocalSessionService, + InternalClientService, + { + provide: CANONICAL_INTERNAL_CLIENT_TOKEN, + useExisting: InternalClientService, + }, PrefixedID, LifecycleService, { @@ -67,6 +81,7 @@ import { upnpClient } from '@app/upnp/helpers.js'; COOKIE_SERVICE_TOKEN, NGINX_SERVICE_TOKEN, INTERNAL_CLIENT_SERVICE_TOKEN, + CANONICAL_INTERNAL_CLIENT_TOKEN, PrefixedID, LIFECYCLE_SERVICE_TOKEN, LifecycleService, diff --git a/api/src/unraid-api/rest/__test__/rest-module.integration.test.ts b/api/src/unraid-api/rest/__test__/rest-module.integration.test.ts index 2f4d403f43..d977452614 100644 --- a/api/src/unraid-api/rest/__test__/rest-module.integration.test.ts +++ b/api/src/unraid-api/rest/__test__/rest-module.integration.test.ts @@ -1,9 +1,9 @@ import { Test } from '@nestjs/testing'; +import { CANONICAL_INTERNAL_CLIENT_TOKEN } from '@unraid/shared'; import { describe, expect, it, vi } from 'vitest'; import { ApiReportService } from '@app/unraid-api/cli/api-report.service.js'; -import { CliInternalClientService } from '@app/unraid-api/cli/internal-client.service.js'; import { LogService } from '@app/unraid-api/cli/log.service.js'; import { RestModule } from '@app/unraid-api/rest/rest.module.js'; import { RestService } from '@app/unraid-api/rest/rest.service.js'; @@ -63,7 +63,7 @@ describe('RestModule Integration', () => { imports: [RestModule], }) // Override services that have complex dependencies for testing - .overrideProvider(CliInternalClientService) + .overrideProvider(CANONICAL_INTERNAL_CLIENT_TOKEN) .useValue({ getClient: vi.fn() }) .overrideProvider(LogService) .useValue({ error: vi.fn(), debug: vi.fn() }) diff --git a/packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts b/api/src/unraid-api/shared/internal-client.service.ts similarity index 53% rename from packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts rename to api/src/unraid-api/shared/internal-client.service.ts index b9203e4d5f..35b2f8e64d 100644 --- a/packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts +++ b/api/src/unraid-api/shared/internal-client.service.ts @@ -1,16 +1,19 @@ import { Inject, Injectable, Logger } from '@nestjs/common'; -import { ConfigService } from '@nestjs/config'; + +import type { CanonicalInternalClientService, InternalGraphQLClientFactory } from '@unraid/shared'; import { ApolloClient, NormalizedCacheObject } from '@apollo/client/core/index.js'; -import { COOKIE_SERVICE_TOKEN, INTERNAL_CLIENT_SERVICE_TOKEN, type InternalGraphQLClientFactory } from '@unraid/shared'; +import { INTERNAL_CLIENT_SERVICE_TOKEN } from '@unraid/shared'; + +import { LocalSessionService } from '@app/unraid-api/auth/local-session.service.js'; /** - * Connect-specific internal GraphQL client. - * - * This uses the shared GraphQL client factory with cookie-based authentication - * and enables subscriptions for real-time updates. + * Canonical internal GraphQL client service. + * + * This service provides a GraphQL client for internal use with local session authentication. + * It replaces the need for separate internal client implementations in different packages. */ @Injectable() -export class InternalClientService { +export class InternalClientService implements CanonicalInternalClientService { private readonly logger = new Logger(InternalClientService.name); private client: ApolloClient | null = null; private clientCreationPromise: Promise> | null = null; @@ -18,31 +21,34 @@ export class InternalClientService { constructor( @Inject(INTERNAL_CLIENT_SERVICE_TOKEN) private readonly clientFactory: InternalGraphQLClientFactory, - @Inject(COOKIE_SERVICE_TOKEN) - private readonly cookieService: any, - private readonly configService: ConfigService + private readonly localSessionService: LocalSessionService ) {} - public async getClient(): Promise> { + /** + * Get GraphQL client with local session authentication. + */ + public async getClient(options?: { + enableSubscriptions?: boolean; + origin?: string; + }): Promise> { // If client already exists, return it if (this.client) { return this.client; } - + // If client creation is in progress, wait for it if (this.clientCreationPromise) { return this.clientCreationPromise; } - + // Start client creation and store the promise - const creationPromise = this.createClient(); + const creationPromise = this.createClient(options); this.clientCreationPromise = creationPromise; - + try { // Wait for client creation to complete const client = await creationPromise; // Only set the client if this is still the current creation promise - // (if clearClient was called, clientCreationPromise would be null) if (this.clientCreationPromise === creationPromise) { this.client = client; } @@ -55,26 +61,20 @@ export class InternalClientService { } } - private async createClient(): Promise> { - // Create a client with cookie-based authentication + private async createClient(options?: { + enableSubscriptions?: boolean; + origin?: string; + }): Promise> { + const { enableSubscriptions = true, origin } = options || {}; + + // Create client with local session authentication const client = await this.clientFactory.createClient({ - getCookieAuth: async () => { - const sessionId = await this.cookieService.getActiveSession(); - if (!sessionId) { - return null; - } - - const csrfToken = this.configService.get('store.emhttp.var.csrfToken'); - if (!csrfToken) { - throw new Error('CSRF token not found in configuration'); - } - - return { sessionId, csrfToken }; - }, - enableSubscriptions: true + getLocalSession: () => this.localSessionService.getLocalSession(), + enableSubscriptions, + origin, }); - - this.logger.debug('Created Connect internal GraphQL client with cookie auth and subscriptions enabled'); + + this.logger.debug('Created canonical internal GraphQL client with local session authentication'); return client; } diff --git a/api/src/unraid-api/shared/internal-graphql-client.factory.ts b/api/src/unraid-api/shared/internal-graphql-client.factory.ts index 9672aad696..0e390c04a2 100644 --- a/api/src/unraid-api/shared/internal-graphql-client.factory.ts +++ b/api/src/unraid-api/shared/internal-graphql-client.factory.ts @@ -36,26 +36,29 @@ export class InternalGraphQLClientFactory implements IInternalGraphQLClientFacto * Create a GraphQL client with the provided configuration. * * @param options Configuration options - * @param options.getApiKey Function to get the current API key (optional if using cookie auth) - * @param options.getCookieAuth Function to get session and CSRF token for cookie auth (optional if using API key) + * @param options.getApiKey Function to get the current API key (optional) + * @param options.getCookieAuth Function to get session and CSRF token for cookie auth (optional) + * @param options.getLocalSession Function to get local session token (optional) * @param options.enableSubscriptions Optional flag to enable WebSocket subscriptions * @param options.origin Optional origin header (defaults to 'http://localhost') */ public async createClient(options: { getApiKey?: () => Promise; getCookieAuth?: () => Promise<{ sessionId: string; csrfToken: string } | null>; + getLocalSession?: () => Promise; enableSubscriptions?: boolean; origin?: string; }): Promise> { - if (!options.getApiKey && !options.getCookieAuth) { + if (!options.getApiKey && !options.getCookieAuth && !options.getLocalSession) { throw new Error( - 'Either getApiKey or getCookieAuth function is required for creating a GraphQL client' + 'One of getApiKey, getCookieAuth, or getLocalSession function is required for creating a GraphQL client' ); } const { getApiKey, getCookieAuth, + getLocalSession, enableSubscriptions = false, origin = 'http://localhost', } = options; @@ -118,6 +121,18 @@ export class InternalGraphQLClientFactory implements IInternalGraphQLClientFacto 'x-api-key': apiKey, }, }; + } else if (getLocalSession) { + // Use local session authentication + const localSession = await getLocalSession(); + if (!localSession) { + throw new Error('No valid local session found'); + } + return { + headers: { + ...headers, + 'x-local-session': localSession, + }, + }; } else if (getCookieAuth) { // Use cookie-based authentication const cookieAuth = await getCookieAuth(); @@ -151,6 +166,14 @@ export class InternalGraphQLClientFactory implements IInternalGraphQLClientFacto if (getApiKey) { const apiKey = await getApiKey(); return { 'x-api-key': apiKey }; + } else if (getLocalSession) { + const localSession = await getLocalSession(); + if (!localSession) { + throw new Error( + 'No valid local session found for WebSocket authentication' + ); + } + return { 'x-local-session': localSession }; } else if (getCookieAuth) { const cookieAuth = await getCookieAuth(); if (!cookieAuth) { diff --git a/packages/unraid-api-plugin-connect/src/authn/connect-api-key.service.ts b/packages/unraid-api-plugin-connect/src/authn/connect-api-key.service.ts deleted file mode 100644 index 4f1e72db3f..0000000000 --- a/packages/unraid-api-plugin-connect/src/authn/connect-api-key.service.ts +++ /dev/null @@ -1,119 +0,0 @@ -import { Inject, Injectable, Logger, OnModuleInit } from '@nestjs/common'; - -import { ApiKey, AuthAction, Permission, Resource, Role } from '@unraid/shared/graphql.model.js'; -import { ApiKeyService, CreatePermissionsInput } from '@unraid/shared/services/api-key.js'; -import { API_KEY_SERVICE_TOKEN } from '@unraid/shared/tokens.js'; - -@Injectable() -export class ConnectApiKeyService implements ApiKeyService, OnModuleInit { - private readonly logger = new Logger(ConnectApiKeyService.name); - private moduleToken: string | null = null; - - constructor( - @Inject(API_KEY_SERVICE_TOKEN) - private readonly apiKeyService: ApiKeyService - ) {} - - async onModuleInit() { - try { - // Register module and get secure token - this.moduleToken = await this.apiKeyService.registerModule('connect', [Role.CONNECT]); - this.logger.log('Connect module registered with ephemeral token'); - } catch (error) { - this.logger.error('Failed to register Connect module:', error); - } - } - - // Delegate all standard ApiKeyService methods to the injected service - async findById(id: string): Promise { - return this.apiKeyService.findById(id); - } - - findByIdWithSecret(id: string): ApiKey | null { - return this.apiKeyService.findByIdWithSecret(id); - } - - findByField(field: keyof ApiKey, value: string): ApiKey | null { - return this.apiKeyService.findByField(field, value); - } - - findByKey(key: string): ApiKey | null { - return this.apiKeyService.findByKey(key); - } - - async create(input: { - name: string; - description?: string; - roles?: Role[]; - permissions?: CreatePermissionsInput; - overwrite?: boolean; - }): Promise { - return this.apiKeyService.create(input); - } - - getAllValidPermissions(): Permission[] { - return this.apiKeyService.getAllValidPermissions(); - } - - convertPermissionsStringArrayToPermissions(permissions: string[]): Permission[] { - return this.apiKeyService.convertPermissionsStringArrayToPermissions(permissions); - } - - convertRolesStringArrayToRoles(roles: string[]): Role[] { - return this.apiKeyService.convertRolesStringArrayToRoles(roles); - } - - async deleteApiKeys(ids: string[]): Promise { - return this.apiKeyService.deleteApiKeys(ids); - } - - async findAll(): Promise { - return this.apiKeyService.findAll(); - } - - /** - * Creates a local API key specifically for Connect - * @deprecated Use module tokens instead - */ - public async createLocalConnectApiKey(): Promise { - // Return a mock key for backwards compatibility - return { - id: 'module-connect', - key: this.moduleToken || '', - name: 'Module-Connect', - description: 'Ephemeral token for Connect module', - roles: [Role.CONNECT], - permissions: [], - createdAt: new Date().toISOString() - }; - } - - /** - * Gets the ephemeral module token for Connect. - * If not yet generated, creates it on-demand. - */ - public async getOrCreateLocalApiKey(): Promise { - if (!this.moduleToken) { - this.moduleToken = await this.apiKeyService.registerModule('connect', [Role.CONNECT]); - } - - if (!this.moduleToken) { - throw new Error('Failed to generate Connect module token'); - } - - return this.moduleToken; - } - - async ensureKey(config: { - name: string; - description: string; - roles: Role[]; - legacyNames?: string[]; - }): Promise { - return this.apiKeyService.ensureKey(config); - } - - async getOrCreateLocalKey(name: string, description: string, roles: Role[]): Promise { - return this.apiKeyService.getOrCreateLocalKey(name, description, roles); - } -} diff --git a/packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.spec.ts b/packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.spec.ts deleted file mode 100644 index dffa93fdfc..0000000000 --- a/packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.spec.ts +++ /dev/null @@ -1,313 +0,0 @@ -import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest'; - -import { InternalClientService } from './internal.client.js'; - -describe('InternalClientService', () => { - let service: InternalClientService; - let clientFactory: any; - let cookieService: any; - let configService: any; - - const mockApolloClient = { - query: vi.fn(), - mutate: vi.fn(), - stop: vi.fn(), - }; - - beforeEach(() => { - clientFactory = { - createClient: vi.fn().mockResolvedValue(mockApolloClient), - }; - - cookieService = { - getActiveSession: vi.fn().mockResolvedValue('test-session-id'), - }; - - configService = { - get: vi.fn().mockImplementation((key: string) => { - if (key === 'store.emhttp.var.csrfToken') { - return 'test-csrf-token'; - } - return undefined; - }), - }; - - service = new InternalClientService( - clientFactory as any, - cookieService as any, - configService as any - ); - }); - - afterEach(() => { - vi.clearAllMocks(); - }); - - it('should be defined', () => { - expect(service).toBeDefined(); - }); - - describe('getClient', () => { - it('should create a client with cookie authentication and subscriptions', async () => { - const client = await service.getClient(); - - // The cookie auth is now fetched lazily through getCookieAuth function - expect(clientFactory.createClient).toHaveBeenCalledWith({ - getCookieAuth: expect.any(Function), - enableSubscriptions: true, - }); - - // Verify the getCookieAuth function works correctly when called - const callArgs = vi.mocked(clientFactory.createClient).mock.calls[0][0]; - const cookieAuth = await callArgs.getCookieAuth(); - expect(cookieAuth).toEqual({ - sessionId: 'test-session-id', - csrfToken: 'test-csrf-token' - }); - expect(cookieService.getActiveSession).toHaveBeenCalled(); - expect(configService.get).toHaveBeenCalledWith('store.emhttp.var.csrfToken'); - - expect(client).toBe(mockApolloClient); - }); - - it('should return cached client on subsequent calls', async () => { - const client1 = await service.getClient(); - const client2 = await service.getClient(); - - expect(client1).toBe(client2); - expect(clientFactory.createClient).toHaveBeenCalledTimes(1); - }); - - it('should handle concurrent calls correctly', async () => { - // Create a delayed mock to simulate async client creation - let resolveClientCreation: (value: any) => void; - const clientCreationPromise = new Promise((resolve) => { - resolveClientCreation = resolve; - }); - - vi.mocked(clientFactory.createClient).mockReturnValueOnce(clientCreationPromise); - - // Start multiple concurrent calls - const promise1 = service.getClient(); - const promise2 = service.getClient(); - const promise3 = service.getClient(); - - // Resolve the client creation - resolveClientCreation!(mockApolloClient); - - // Wait for all promises to resolve - const [client1, client2, client3] = await Promise.all([promise1, promise2, promise3]); - - // All should return the same client - expect(client1).toBe(mockApolloClient); - expect(client2).toBe(mockApolloClient); - expect(client3).toBe(mockApolloClient); - - // createClient should only have been called once - expect(clientFactory.createClient).toHaveBeenCalledTimes(1); - }); - - it('should handle errors during client creation', async () => { - const error = new Error('Failed to create client'); - vi.mocked(clientFactory.createClient).mockRejectedValueOnce(error); - - await expect(service.getClient()).rejects.toThrow(); - - // The in-flight promise should be cleared after error - // A subsequent call should attempt creation again - vi.mocked(clientFactory.createClient).mockResolvedValueOnce(mockApolloClient); - const client = await service.getClient(); - expect(client).toBe(mockApolloClient); - expect(clientFactory.createClient).toHaveBeenCalledTimes(2); - }); - - it('should handle missing session', async () => { - vi.mocked(cookieService.getActiveSession).mockResolvedValue(null); - - const client = await service.getClient(); - - // Verify getCookieAuth function returns null when no session - const callArgs = vi.mocked(clientFactory.createClient).mock.calls[0][0]; - const cookieAuth = await callArgs.getCookieAuth(); - expect(cookieAuth).toBeNull(); - expect(client).toBe(mockApolloClient); - }); - - it('should handle missing CSRF token', async () => { - vi.mocked(configService.get).mockReturnValue(undefined); - - const client = await service.getClient(); - - // Verify getCookieAuth function throws when CSRF token is missing - const callArgs = vi.mocked(clientFactory.createClient).mock.calls[0][0]; - await expect(callArgs.getCookieAuth()).rejects.toThrow('CSRF token not found in configuration'); - expect(client).toBe(mockApolloClient); - }); - }); - - describe('clearClient', () => { - it('should stop and clear the client', async () => { - // First create a client - await service.getClient(); - - // Clear the client - service.clearClient(); - - expect(mockApolloClient.stop).toHaveBeenCalled(); - }); - - it('should handle clearing when no client exists', () => { - // Should not throw when clearing a non-existent client - expect(() => service.clearClient()).not.toThrow(); - }); - - it('should create a new client after clearing', async () => { - // Create initial client - await service.getClient(); - - // Clear it - service.clearClient(); - - // Reset mock to return a new client - const newMockClient = { - query: vi.fn(), - mutate: vi.fn(), - stop: vi.fn(), - }; - vi.mocked(clientFactory.createClient).mockResolvedValueOnce(newMockClient); - - // Create new client - const newClient = await service.getClient(); - - // Should have created client twice total - expect(clientFactory.createClient).toHaveBeenCalledTimes(2); - expect(newClient).toBe(newMockClient); - }); - - it('should clear in-flight promise when clearing client', async () => { - // Create a delayed mock to simulate async client creation - let resolveClientCreation: (value: any) => void; - const clientCreationPromise = new Promise((resolve) => { - resolveClientCreation = resolve; - }); - - vi.mocked(clientFactory.createClient).mockReturnValueOnce(clientCreationPromise); - - // Start client creation - const promise1 = service.getClient(); - - // Clear client while creation is in progress - service.clearClient(); - - // Resolve the original creation - resolveClientCreation!(mockApolloClient); - await promise1; - - // Reset mock for new client - const newMockClient = { - query: vi.fn(), - mutate: vi.fn(), - stop: vi.fn(), - }; - vi.mocked(clientFactory.createClient).mockResolvedValueOnce(newMockClient); - - // Try to get client again - should create a new one - const client = await service.getClient(); - expect(client).toBe(newMockClient); - expect(clientFactory.createClient).toHaveBeenCalledTimes(2); - }); - - it('should handle clearClient during creation followed by new getClient call', async () => { - // Create two delayed mocks to simulate async client creation - let resolveFirstCreation: (value: any) => void; - let resolveSecondCreation: (value: any) => void; - - const firstCreationPromise = new Promise((resolve) => { - resolveFirstCreation = resolve; - }); - const secondCreationPromise = new Promise((resolve) => { - resolveSecondCreation = resolve; - }); - - const firstMockClient = { - query: vi.fn(), - mutate: vi.fn(), - stop: vi.fn(), - }; - const secondMockClient = { - query: vi.fn(), - mutate: vi.fn(), - stop: vi.fn(), - }; - - vi.mocked(clientFactory.createClient) - .mockReturnValueOnce(firstCreationPromise) - .mockReturnValueOnce(secondCreationPromise); - - // Thread A: Start first client creation - const promiseA = service.getClient(); - - // Thread B: Clear client while first creation is in progress - service.clearClient(); - - // Thread C: Start second client creation - const promiseC = service.getClient(); - - // Resolve first creation (should not set client) - resolveFirstCreation!(firstMockClient); - const clientA = await promiseA; - - // Resolve second creation (should set client) - resolveSecondCreation!(secondMockClient); - const clientC = await promiseC; - - // Both should return their respective clients - expect(clientA).toBe(firstMockClient); - expect(clientC).toBe(secondMockClient); - - // But only the second client should be cached - const cachedClient = await service.getClient(); - expect(cachedClient).toBe(secondMockClient); - - // Should have created exactly 2 clients - expect(clientFactory.createClient).toHaveBeenCalledTimes(2); - }); - - it('should handle rapid clear and get cycles correctly', async () => { - // Test rapid clear/get cycles - const clients: any[] = []; - for (let i = 0; i < 3; i++) { - const mockClient = { - query: vi.fn(), - mutate: vi.fn(), - stop: vi.fn(), - }; - clients.push(mockClient); - vi.mocked(clientFactory.createClient).mockResolvedValueOnce(mockClient); - } - - // Cycle 1: Create and immediately clear - const promise1 = service.getClient(); - service.clearClient(); - const client1 = await promise1; - expect(client1).toBe(clients[0]); - - // Cycle 2: Create and immediately clear - const promise2 = service.getClient(); - service.clearClient(); - const client2 = await promise2; - expect(client2).toBe(clients[1]); - - // Cycle 3: Create and let it complete - const client3 = await service.getClient(); - expect(client3).toBe(clients[2]); - - // Verify the third client is cached - const cachedClient = await service.getClient(); - expect(cachedClient).toBe(clients[2]); - - // Should have created exactly 3 clients - expect(clientFactory.createClient).toHaveBeenCalledTimes(3); - }); - }); -}); \ No newline at end of file diff --git a/packages/unraid-api-plugin-connect/src/mothership-proxy/mothership-subscription.handler.ts b/packages/unraid-api-plugin-connect/src/mothership-proxy/mothership-subscription.handler.ts index 5282ec618f..fefc358bdc 100644 --- a/packages/unraid-api-plugin-connect/src/mothership-proxy/mothership-subscription.handler.ts +++ b/packages/unraid-api-plugin-connect/src/mothership-proxy/mothership-subscription.handler.ts @@ -1,7 +1,8 @@ -import { Injectable, Logger } from '@nestjs/common'; +import { Inject, Injectable, Logger } from '@nestjs/common'; import { isDefined } from 'class-validator'; import { type Subscription } from 'zen-observable-ts'; +import { CANONICAL_INTERNAL_CLIENT_TOKEN, type CanonicalInternalClientService } from '@unraid/shared'; import { EVENTS_SUBSCRIPTION, RemoteGraphQL_Fragment } from '../graphql/event.js'; import { @@ -12,7 +13,6 @@ import { import { useFragment } from '../graphql/generated/client/index.js'; import { SEND_REMOTE_QUERY_RESPONSE } from '../graphql/remote-response.js'; import { parseGraphQLQuery } from '../helper/parse-graphql.js'; -import { InternalClientService } from '../internal-rpc/internal.client.js'; import { MothershipConnectionService } from './connection.service.js'; import { MothershipGraphqlClientService } from './graphql.client.js'; @@ -29,7 +29,8 @@ type ActiveSubscription = { @Injectable() export class MothershipSubscriptionHandler { constructor( - private readonly internalClientService: InternalClientService, + @Inject(CANONICAL_INTERNAL_CLIENT_TOKEN) + private readonly internalClientService: CanonicalInternalClientService, private readonly mothershipClient: MothershipGraphqlClientService, private readonly connectionService: MothershipConnectionService ) {} diff --git a/packages/unraid-api-plugin-connect/src/mothership-proxy/mothership.module.ts b/packages/unraid-api-plugin-connect/src/mothership-proxy/mothership.module.ts index 75bde5acb9..267b438262 100644 --- a/packages/unraid-api-plugin-connect/src/mothership-proxy/mothership.module.ts +++ b/packages/unraid-api-plugin-connect/src/mothership-proxy/mothership.module.ts @@ -1,11 +1,10 @@ import { Module } from '@nestjs/common'; -import { ConnectApiKeyService } from '../authn/connect-api-key.service.js'; + import { CloudResolver } from '../connection-status/cloud.resolver.js'; import { CloudService } from '../connection-status/cloud.service.js'; import { ConnectStatusWriterService } from '../connection-status/connect-status-writer.service.js'; import { TimeoutCheckerJob } from '../connection-status/timeout-checker.job.js'; -import { InternalClientService } from '../internal-rpc/internal.client.js'; import { RemoteAccessModule } from '../remote-access/remote-access.module.js'; import { MothershipConnectionService } from './connection.service.js'; import { MothershipGraphqlClientService } from './graphql.client.js'; @@ -17,10 +16,8 @@ import { MothershipHandler } from './mothership.events.js'; imports: [RemoteAccessModule], providers: [ ConnectStatusWriterService, - ConnectApiKeyService, MothershipConnectionService, MothershipGraphqlClientService, - InternalClientService, MothershipHandler, MothershipSubscriptionHandler, TimeoutCheckerJob, diff --git a/packages/unraid-api-plugin-connect/src/unraid-connect/connect-settings.service.ts b/packages/unraid-api-plugin-connect/src/unraid-connect/connect-settings.service.ts index 0b5daee9eb..149307b0be 100644 --- a/packages/unraid-api-plugin-connect/src/unraid-connect/connect-settings.service.ts +++ b/packages/unraid-api-plugin-connect/src/unraid-connect/connect-settings.service.ts @@ -21,7 +21,7 @@ import type { RemoteAccess, SetupRemoteAccessInput, } from './connect.model.js'; -import { ConnectApiKeyService } from '../authn/connect-api-key.service.js'; + import { ConfigType, MyServersConfig } from '../config/connect.config.js'; import { EVENTS } from '../helper/nest-tokens.js'; import { NetworkService } from '../network/network.service.js'; @@ -39,7 +39,6 @@ export class ConnectSettingsService { constructor( private readonly configService: ConfigService, private readonly remoteAccess: DynamicRemoteAccessService, - private readonly apiKeyService: ConnectApiKeyService, private readonly eventEmitter: EventEmitter2, private readonly userSettings: UserSettingsService, private readonly networkService: NetworkService @@ -165,9 +164,6 @@ export class ConnectSettingsService { } try { - // Make sure we have a local API key for Connect - await this.apiKeyService.getOrCreateLocalApiKey(); - // Update config with user info this.configService.set( 'connect.config.avatar', diff --git a/packages/unraid-api-plugin-connect/src/unraid-connect/connect.module.ts b/packages/unraid-api-plugin-connect/src/unraid-connect/connect.module.ts index a25b1236bc..be11279ca1 100644 --- a/packages/unraid-api-plugin-connect/src/unraid-connect/connect.module.ts +++ b/packages/unraid-api-plugin-connect/src/unraid-connect/connect.module.ts @@ -3,7 +3,7 @@ import { ConfigModule } from '@nestjs/config'; import { UserSettingsModule } from '@unraid/shared/services/user-settings.js'; -import { ConnectApiKeyService } from '../authn/connect-api-key.service.js'; + import { ConnectLoginHandler } from '../authn/connect-login.events.js'; import { ConnectConfigService } from '../config/connect.config.service.js'; import { RemoteAccessModule } from '../remote-access/remote-access.module.js'; @@ -16,7 +16,6 @@ import { ConnectResolver } from './connect.resolver.js'; providers: [ ConnectSettingsService, ConnectLoginHandler, - ConnectApiKeyService, ConnectSettingsResolver, ConnectResolver, ConnectConfigService, @@ -24,7 +23,6 @@ import { ConnectResolver } from './connect.resolver.js'; exports: [ ConnectSettingsService, ConnectLoginHandler, - ConnectApiKeyService, ConnectSettingsResolver, ConnectResolver, ConnectConfigService, diff --git a/packages/unraid-shared/src/index.ts b/packages/unraid-shared/src/index.ts index 7804ac5248..301f3b2222 100644 --- a/packages/unraid-shared/src/index.ts +++ b/packages/unraid-shared/src/index.ts @@ -5,3 +5,4 @@ export * from './tokens.js'; export * from './use-permissions.directive.js'; export * from './util/permissions.js'; export type { InternalGraphQLClientFactory } from './types/internal-graphql-client.factory.js'; +export type { CanonicalInternalClientService } from './types/canonical-internal-client.interface.js'; diff --git a/packages/unraid-shared/src/tokens.ts b/packages/unraid-shared/src/tokens.ts index b33a1119b7..ce5a350d5c 100644 --- a/packages/unraid-shared/src/tokens.ts +++ b/packages/unraid-shared/src/tokens.ts @@ -4,3 +4,4 @@ export const LIFECYCLE_SERVICE_TOKEN = 'LifecycleService'; export const NGINX_SERVICE_TOKEN = 'NginxService'; export const INTERNAL_CLIENT_SERVICE_TOKEN = 'InternalClientService'; export const COOKIE_SERVICE_TOKEN = 'CookieService'; +export const CANONICAL_INTERNAL_CLIENT_TOKEN = 'CanonicalInternalClient'; diff --git a/packages/unraid-shared/src/types/canonical-internal-client.interface.ts b/packages/unraid-shared/src/types/canonical-internal-client.interface.ts new file mode 100644 index 0000000000..c07e378718 --- /dev/null +++ b/packages/unraid-shared/src/types/canonical-internal-client.interface.ts @@ -0,0 +1,14 @@ +import type { ApolloClient, NormalizedCacheObject } from '@apollo/client/core/index.js'; + +export interface CanonicalInternalClientService { + /** + * Get GraphQL client with cookie authentication. + * This is the canonical internal client for the application. + */ + getClient(options?: { enableSubscriptions?: boolean; origin?: string }): Promise>; + + /** + * Clear the current client and force recreation on next use. + */ + clearClient(): void; +} diff --git a/packages/unraid-shared/src/types/internal-graphql-client.factory.ts b/packages/unraid-shared/src/types/internal-graphql-client.factory.ts index 52a7afbb4e..c921629d68 100644 --- a/packages/unraid-shared/src/types/internal-graphql-client.factory.ts +++ b/packages/unraid-shared/src/types/internal-graphql-client.factory.ts @@ -8,6 +8,7 @@ export interface InternalGraphQLClientFactory { createClient(options: { getApiKey?: () => Promise; getCookieAuth?: () => Promise<{ sessionId: string; csrfToken: string } | null>; + getLocalSession?: () => Promise; enableSubscriptions?: boolean; origin?: string; }): Promise>; From 2e2be24de357cc361bacb7ac2da76cb0a45a3882 Mon Sep 17 00:00:00 2001 From: Pujit Mehrotra Date: Tue, 26 Aug 2025 18:14:52 -0400 Subject: [PATCH 04/16] revert changes to api key service --- api/src/unraid-api/auth/api-key.service.ts | 192 +++------------------ api/src/unraid-api/cli/cli.module.spec.ts | 6 +- 2 files changed, 27 insertions(+), 171 deletions(-) diff --git a/api/src/unraid-api/auth/api-key.service.ts b/api/src/unraid-api/auth/api-key.service.ts index 5b9a99c2d9..4c0ca87fc9 100644 --- a/api/src/unraid-api/auth/api-key.service.ts +++ b/api/src/unraid-api/auth/api-key.service.ts @@ -26,8 +26,6 @@ export class ApiKeyService implements OnModuleInit { private readonly logger = new Logger(ApiKeyService.name); protected readonly basePath: string; protected memoryApiKeys: Array = []; - private persistentKeys = new Map(); - private ephemeralKeys = new Map(); private static readonly validRoles: Set = new Set(Object.values(Role)); constructor() { @@ -36,60 +34,27 @@ export class ApiKeyService implements OnModuleInit { } async onModuleInit() { - // Load persistent keys once at startup - const diskKeys = await this.loadAllFromDisk(); - for (const key of diskKeys) { - this.persistentKeys.set(key.id, key); + this.memoryApiKeys = await this.loadAllFromDisk(); + if (environment.IS_MAIN_PROCESS) { + this.setupWatch(); } - - // Clean up legacy internal keys - await this.cleanupLegacyInternalKeys(); - - // Update memoryApiKeys for backwards compatibility - this.updateMemoryApiKeys(); - - // NO file watching - manage in memory - this.logger.log(`Loaded ${this.persistentKeys.size} persistent API keys`); } public async findAll(): Promise { - // Only return persistent keys, not ephemeral ones - return Array.from(this.persistentKeys.values()).map((key) => - this.convertApiKeyWithSecretToApiKey(key) + return Promise.all( + this.memoryApiKeys.map(async (key) => { + const keyWithoutSecret = this.convertApiKeyWithSecretToApiKey(key); + return keyWithoutSecret; + }) ); } - private updateMemoryApiKeys() { - // Combine persistent and ephemeral keys for backwards compatibility - this.memoryApiKeys = [ - ...Array.from(this.persistentKeys.values()), - ...Array.from(this.ephemeralKeys.values()), - ]; - } - - private async cleanupLegacyInternalKeys() { - const legacyNames = ['CliInternal', 'ConnectInternal', 'CLI', 'Connect', 'CliAdmin', 'Internal']; - const keysToDelete: string[] = []; - - for (const [id, key] of this.persistentKeys) { - if (legacyNames.includes(key.name)) { - keysToDelete.push(id); - this.logger.log(`Removing legacy internal key: ${key.name}`); - } - } - - if (keysToDelete.length > 0) { - // Remove from disk - for (const id of keysToDelete) { - try { - await unlink(join(this.basePath, `${id}.json`)); - } catch (error) { - // File might not exist, that's ok - } - this.persistentKeys.delete(id); - } - this.logger.log(`Cleaned up ${keysToDelete.length} legacy internal keys`); - } + private setupWatch() { + watch(this.basePath, { ignoreInitial: false }).on('all', async (path) => { + this.logger.debug(`API key changed: ${path}`); + this.memoryApiKeys = []; + this.memoryApiKeys = await this.loadAllFromDisk(); + }); } private sanitizeName(name: string): string { @@ -184,10 +149,6 @@ export class ApiKeyService implements OnModuleInit { await this.saveApiKey(apiKey as ApiKey); - // Update persistent keys in memory - this.persistentKeys.set(apiKey.id as string, apiKey as ApiKey); - this.updateMemoryApiKeys(); - return apiKey as ApiKey; } @@ -277,97 +238,17 @@ export class ApiKeyService implements OnModuleInit { public findByField(field: keyof ApiKey, value: string): ApiKey | null { if (!value) return null; - // Check ephemeral keys first - for (const keyData of this.ephemeralKeys.values()) { - if (keyData[field] === value) { - return keyData; - } - } - - // Then check persistent keys - for (const keyData of this.persistentKeys.values()) { - if (keyData[field] === value) { - return keyData; - } - } - - return null; + return this.memoryApiKeys.find((k) => k[field] === value) ?? null; } findByKey(key: string): ApiKeyWithSecret | null { - if (!key) return null; - - // Check ephemeral keys first (faster, in-memory) - for (const keyData of this.ephemeralKeys.values()) { - if (keyData.key === key) { - return keyData; - } - } - - // Then check persistent keys - for (const keyData of this.persistentKeys.values()) { - if (keyData.key === key) { - return keyData; - } - } - - return null; + return this.findByField('key', key); } private generateApiKey(): string { return crypto.randomBytes(32).toString('hex'); } - /** - * Register an in-process module with an ephemeral token. - * Used by Connect and other modules running in the same process. - */ - public async registerModule(moduleId: string, roles: Role[]): Promise { - // Generate 256-bit cryptographically secure token - const token = crypto.randomBytes(32).toString('base64url'); - - const keyData: ApiKeyWithSecret = { - id: `module-${moduleId}`, - key: token, - name: `Module-${moduleId}`, - description: `Ephemeral token for ${moduleId} module`, - roles, - permissions: [], - createdAt: new Date().toISOString(), - }; - - this.ephemeralKeys.set(keyData.id, keyData); - this.updateMemoryApiKeys(); - this.logger.debug(`Registered module ${moduleId} with ephemeral token`); - - return token; - } - - /** - * Register an ephemeral key for external processes like CLI. - * The key is provided by the caller and registered in memory. - */ - public async registerEphemeralKey(config: { - key: string; - name: string; - roles: Role[]; - type: string; - }): Promise { - const keyData: ApiKeyWithSecret = { - id: `ephemeral-${config.type}`, - key: config.key, - name: config.name, - description: `Ephemeral key for ${config.type}`, - roles: config.roles, - permissions: [], - createdAt: new Date().toISOString(), - }; - - this.ephemeralKeys.set(keyData.id, keyData); - this.updateMemoryApiKeys(); - this.logger.debug(`Registered ephemeral key for ${config.type}`); - } - private logApiKeyValidationError(file: string, error: ValidationError): void { this.logger.error(`Invalid API key structure in file ${file}. Errors: ${JSON.stringify(error.constraints, null, 2)}`); @@ -428,30 +309,17 @@ export class ApiKeyService implements OnModuleInit { throw new Error(`API keys not found: ${missingKeys.join(', ')}`); } - // Separate persistent and ephemeral keys - const persistentIds = ids.filter((id) => this.persistentKeys.has(id)); - const ephemeralIds = ids.filter((id) => this.ephemeralKeys.has(id)); - - // Delete persistent keys from disk - if (persistentIds.length > 0) { - const { errors } = await batchProcess(persistentIds, async (id) => { - await unlink(join(this.basePath, `${id}.json`)); - this.persistentKeys.delete(id); - return id; - }); - - if (errors.length > 0) { - throw errors; - } - } + // Delete all files in parallel + const { errors, data: deletedIds } = await batchProcess(ids, async (id) => { + await unlink(join(this.basePath, `${id}.json`)); + return id; + }); - // Remove ephemeral keys from memory - for (const id of ephemeralIds) { - this.ephemeralKeys.delete(id); + const deletedSet = new Set(deletedIds); + this.memoryApiKeys = this.memoryApiKeys.filter((key) => !deletedSet.has(key.id)); + if (errors.length > 0) { + throw errors; } - - // Update memory cache - this.updateMemoryApiKeys(); } async update({ @@ -488,15 +356,7 @@ export class ApiKeyService implements OnModuleInit { // Handle both empty array (to clear permissions) and populated array apiKey.permissions = permissions; } - - // Only save to disk if it's a persistent key - if (this.persistentKeys.has(id)) { - await this.saveApiKey(apiKey); - this.persistentKeys.set(id, apiKey); - } - - // Update memory cache - this.updateMemoryApiKeys(); + await this.saveApiKey(apiKey); return apiKey; } diff --git a/api/src/unraid-api/cli/cli.module.spec.ts b/api/src/unraid-api/cli/cli.module.spec.ts index b18385146e..43332b177e 100644 --- a/api/src/unraid-api/cli/cli.module.spec.ts +++ b/api/src/unraid-api/cli/cli.module.spec.ts @@ -1,11 +1,7 @@ import { ConfigModule } from '@nestjs/config'; import { Test, TestingModule } from '@nestjs/testing'; -import { - CANONICAL_INTERNAL_CLIENT_TOKEN, - CANONICAL_INTERNAL_CLIENT_TOKEN, - INTERNAL_CLIENT_SERVICE_TOKEN, -} from '@unraid/shared'; +import { CANONICAL_INTERNAL_CLIENT_TOKEN, INTERNAL_CLIENT_SERVICE_TOKEN } from '@unraid/shared'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import { CliServicesModule } from '@app/unraid-api/cli/cli-services.module.js'; From 2eed31a3b9081a6b7ecc5e2681501301d69ba96c Mon Sep 17 00:00:00 2001 From: Pujit Mehrotra Date: Wed, 27 Aug 2025 09:39:23 -0400 Subject: [PATCH 05/16] rename INTERNAL_CLIENT_SERVICE_TOKEN to INTERNAL_CLIENT_FACTORY_TOKEN --- api/src/unraid-api/cli/cli.module.spec.ts | 6 +++--- api/src/unraid-api/plugin/global-deps.module.ts | 6 +++--- api/src/unraid-api/shared/internal-client.service.ts | 4 ++-- packages/unraid-shared/src/tokens.ts | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/api/src/unraid-api/cli/cli.module.spec.ts b/api/src/unraid-api/cli/cli.module.spec.ts index 43332b177e..15164d1c7f 100644 --- a/api/src/unraid-api/cli/cli.module.spec.ts +++ b/api/src/unraid-api/cli/cli.module.spec.ts @@ -1,7 +1,7 @@ import { ConfigModule } from '@nestjs/config'; import { Test, TestingModule } from '@nestjs/testing'; -import { CANONICAL_INTERNAL_CLIENT_TOKEN, INTERNAL_CLIENT_SERVICE_TOKEN } from '@unraid/shared'; +import { CANONICAL_INTERNAL_CLIENT_TOKEN, INTERNAL_CLIENT_FACTORY_TOKEN } from '@unraid/shared'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import { CliServicesModule } from '@app/unraid-api/cli/cli-services.module.js'; @@ -31,7 +31,7 @@ describe('CliServicesModule', () => { }); it('should provide InternalGraphQLClientFactory via token', () => { - const factory = module.get(INTERNAL_CLIENT_SERVICE_TOKEN); + const factory = module.get(INTERNAL_CLIENT_FACTORY_TOKEN); expect(factory).toBeDefined(); expect(factory).toBeInstanceOf(InternalGraphQLClientFactory); }); @@ -51,7 +51,7 @@ describe('CliServicesModule', () => { it('should resolve InternalGraphQLClientFactory dependency via token', () => { // Explicitly test that the factory is available in the module context via token - const factory = module.get(INTERNAL_CLIENT_SERVICE_TOKEN); + const factory = module.get(INTERNAL_CLIENT_FACTORY_TOKEN); expect(factory).toBeDefined(); expect(factory.createClient).toBeDefined(); }); diff --git a/api/src/unraid-api/plugin/global-deps.module.ts b/api/src/unraid-api/plugin/global-deps.module.ts index 8107f0e03d..d95761d100 100644 --- a/api/src/unraid-api/plugin/global-deps.module.ts +++ b/api/src/unraid-api/plugin/global-deps.module.ts @@ -7,7 +7,7 @@ import { API_KEY_SERVICE_TOKEN, CANONICAL_INTERNAL_CLIENT_TOKEN, COOKIE_SERVICE_TOKEN, - INTERNAL_CLIENT_SERVICE_TOKEN, + INTERNAL_CLIENT_FACTORY_TOKEN, LIFECYCLE_SERVICE_TOKEN, NGINX_SERVICE_TOKEN, UPNP_CLIENT_TOKEN, @@ -32,7 +32,7 @@ import { upnpClient } from '@app/upnp/helpers.js'; providers: [ SocketConfigService, { - provide: INTERNAL_CLIENT_SERVICE_TOKEN, + provide: INTERNAL_CLIENT_FACTORY_TOKEN, useClass: InternalGraphQLClientFactory, }, { @@ -80,7 +80,7 @@ import { upnpClient } from '@app/upnp/helpers.js'; API_KEY_SERVICE_TOKEN, COOKIE_SERVICE_TOKEN, NGINX_SERVICE_TOKEN, - INTERNAL_CLIENT_SERVICE_TOKEN, + INTERNAL_CLIENT_FACTORY_TOKEN, CANONICAL_INTERNAL_CLIENT_TOKEN, PrefixedID, LIFECYCLE_SERVICE_TOKEN, diff --git a/api/src/unraid-api/shared/internal-client.service.ts b/api/src/unraid-api/shared/internal-client.service.ts index 35b2f8e64d..841ba7026f 100644 --- a/api/src/unraid-api/shared/internal-client.service.ts +++ b/api/src/unraid-api/shared/internal-client.service.ts @@ -2,7 +2,7 @@ import { Inject, Injectable, Logger } from '@nestjs/common'; import type { CanonicalInternalClientService, InternalGraphQLClientFactory } from '@unraid/shared'; import { ApolloClient, NormalizedCacheObject } from '@apollo/client/core/index.js'; -import { INTERNAL_CLIENT_SERVICE_TOKEN } from '@unraid/shared'; +import { INTERNAL_CLIENT_FACTORY_TOKEN } from '@unraid/shared'; import { LocalSessionService } from '@app/unraid-api/auth/local-session.service.js'; @@ -19,7 +19,7 @@ export class InternalClientService implements CanonicalInternalClientService { private clientCreationPromise: Promise> | null = null; constructor( - @Inject(INTERNAL_CLIENT_SERVICE_TOKEN) + @Inject(INTERNAL_CLIENT_FACTORY_TOKEN) private readonly clientFactory: InternalGraphQLClientFactory, private readonly localSessionService: LocalSessionService ) {} diff --git a/packages/unraid-shared/src/tokens.ts b/packages/unraid-shared/src/tokens.ts index ce5a350d5c..7cea8c218d 100644 --- a/packages/unraid-shared/src/tokens.ts +++ b/packages/unraid-shared/src/tokens.ts @@ -2,6 +2,6 @@ export const UPNP_CLIENT_TOKEN = 'UPNP_CLIENT'; export const API_KEY_SERVICE_TOKEN = 'ApiKeyService'; export const LIFECYCLE_SERVICE_TOKEN = 'LifecycleService'; export const NGINX_SERVICE_TOKEN = 'NginxService'; -export const INTERNAL_CLIENT_SERVICE_TOKEN = 'InternalClientService'; +export const INTERNAL_CLIENT_FACTORY_TOKEN = 'InternalClientService'; export const COOKIE_SERVICE_TOKEN = 'CookieService'; export const CANONICAL_INTERNAL_CLIENT_TOKEN = 'CanonicalInternalClient'; From 0239887deaec54a3a32ae60cb542867911f5251e Mon Sep 17 00:00:00 2001 From: Pujit Mehrotra Date: Wed, 27 Aug 2025 09:44:11 -0400 Subject: [PATCH 06/16] harden getActiveSession in cookie service --- api/src/unraid-api/auth/cookie.service.ts | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/api/src/unraid-api/auth/cookie.service.ts b/api/src/unraid-api/auth/cookie.service.ts index 2e3748c200..2c2e4c232f 100644 --- a/api/src/unraid-api/auth/cookie.service.ts +++ b/api/src/unraid-api/auth/cookie.service.ts @@ -101,11 +101,25 @@ export class CookieService { * @returns the active session id, if any, or null if no active session is found. */ async getActiveSession(): Promise { - const sessionFiles = await readdir(this.opts.sessionDir); + let sessionFiles: string[] = []; + try { + sessionFiles = await readdir(this.opts.sessionDir); + } catch (e) { + this.logger.warn(e, 'Error reading session directory'); + return null; + } for (const sessionFile of sessionFiles) { - const sessionData = await readFile(join(this.opts.sessionDir, sessionFile), 'ascii'); - if (this.isSessionValid(sessionData)) { - return sessionFile.replace('sess_', ''); + if (!sessionFile.startsWith('sess_')) { + continue; + } + try { + const sessionData = await readFile(join(this.opts.sessionDir, sessionFile), 'ascii'); + if (this.isSessionValid(sessionData)) { + return sessionFile.replace('sess_', ''); + } + } catch { + // Ignore unreadable files and continue scanning + continue; } } return null; From 3d37f6a4fd742b628f227bae9e3138d38a1bed7f Mon Sep 17 00:00:00 2001 From: Pujit Mehrotra Date: Wed, 27 Aug 2025 09:47:41 -0400 Subject: [PATCH 07/16] add cleanup to local session service --- .../unraid-api/auth/local-session.service.ts | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/api/src/unraid-api/auth/local-session.service.ts b/api/src/unraid-api/auth/local-session.service.ts index 901b89f742..0314d55111 100644 --- a/api/src/unraid-api/auth/local-session.service.ts +++ b/api/src/unraid-api/auth/local-session.service.ts @@ -1,6 +1,6 @@ -import { Injectable, Logger, OnModuleInit } from '@nestjs/common'; +import { Injectable, Logger, OnModuleDestroy, OnModuleInit } from '@nestjs/common'; import { randomBytes } from 'crypto'; -import { chmod, mkdir, readFile, writeFile } from 'fs/promises'; +import { chmod, mkdir, readFile, unlink, writeFile } from 'fs/promises'; import { dirname } from 'path'; /** @@ -8,7 +8,7 @@ import { dirname } from 'path'; * Creates a secure token on startup that can be used for local system operations. */ @Injectable() -export class LocalSessionService implements OnModuleInit { +export class LocalSessionService implements OnModuleInit, OnModuleDestroy { private readonly logger = new Logger(LocalSessionService.name); private sessionToken: string | null = null; private static readonly SESSION_FILE_PATH = '/var/run/unraid-api/local-session'; @@ -16,12 +16,22 @@ export class LocalSessionService implements OnModuleInit { async onModuleInit() { try { await this.generateLocalSession(); - this.logger.log('Local session initialized'); + this.logger.verbose('Local session initialized'); } catch (error) { this.logger.error('Failed to initialize local session:', error); } } + async onModuleDestroy() { + if (!this.sessionToken) return; + try { + await unlink(LocalSessionService.SESSION_FILE_PATH); + this.logger.verbose('Local session file deleted'); + } catch (error) { + this.logger.warn(error, 'Error deleting local session file'); + } + } + /** * Generate a secure local session token and write it to file */ From 51b3e95f266e939d20bed66740ca0694f47edb2d Mon Sep 17 00:00:00 2001 From: Pujit Mehrotra Date: Wed, 27 Aug 2025 09:54:03 -0400 Subject: [PATCH 08/16] use crypto timingSafeEqual for csrf & session validation --- api/src/unraid-api/auth/auth.service.ts | 6 +++++- .../unraid-api/auth/local-session.service.ts | 18 ++---------------- 2 files changed, 7 insertions(+), 17 deletions(-) diff --git a/api/src/unraid-api/auth/auth.service.ts b/api/src/unraid-api/auth/auth.service.ts index 1b7afb27b2..5457a3a045 100644 --- a/api/src/unraid-api/auth/auth.service.ts +++ b/api/src/unraid-api/auth/auth.service.ts @@ -1,4 +1,5 @@ import { Injectable, Logger, UnauthorizedException } from '@nestjs/common'; +import { timingSafeEqual } from 'node:crypto'; import { AuthAction, Resource, Role } from '@unraid/shared/graphql.model.js'; import { @@ -280,7 +281,10 @@ export class AuthService { } public validateCsrfToken(token?: string): boolean { - return Boolean(token) && token === getters.emhttp().var.csrfToken; + if (!token) return false; + const csrfToken = getters.emhttp().var.csrfToken; + if (!csrfToken) return false; + return timingSafeEqual(Buffer.from(token, 'utf-8'), Buffer.from(csrfToken, 'utf-8')); } /** diff --git a/api/src/unraid-api/auth/local-session.service.ts b/api/src/unraid-api/auth/local-session.service.ts index 0314d55111..ebcf9bdd25 100644 --- a/api/src/unraid-api/auth/local-session.service.ts +++ b/api/src/unraid-api/auth/local-session.service.ts @@ -1,5 +1,5 @@ import { Injectable, Logger, OnModuleDestroy, OnModuleInit } from '@nestjs/common'; -import { randomBytes } from 'crypto'; +import { randomBytes, timingSafeEqual } from 'crypto'; import { chmod, mkdir, readFile, unlink, writeFile } from 'fs/promises'; import { dirname } from 'path'; @@ -82,21 +82,7 @@ export class LocalSessionService implements OnModuleInit, OnModuleDestroy { if (!currentToken) return false; // Use constant-time comparison to prevent timing attacks - return this.constantTimeEqual(token, currentToken); - } - - /** - * Constant-time string comparison to prevent timing attacks - */ - private constantTimeEqual(a: string, b: string): boolean { - if (a.length !== b.length) return false; - - let result = 0; - for (let i = 0; i < a.length; i++) { - result |= a.charCodeAt(i) ^ b.charCodeAt(i); - } - - return result === 0; + return timingSafeEqual(Buffer.from(token, 'utf-8'), Buffer.from(currentToken, 'utf-8')); } /** From 660120b9e560d465db9d4fc9ea9e449e8567cad5 Mon Sep 17 00:00:00 2001 From: Pujit Mehrotra Date: Wed, 27 Aug 2025 10:01:14 -0400 Subject: [PATCH 09/16] document internal client service behavior --- api/src/unraid-api/shared/internal-client.service.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/api/src/unraid-api/shared/internal-client.service.ts b/api/src/unraid-api/shared/internal-client.service.ts index 841ba7026f..7096a34b27 100644 --- a/api/src/unraid-api/shared/internal-client.service.ts +++ b/api/src/unraid-api/shared/internal-client.service.ts @@ -26,6 +26,13 @@ export class InternalClientService implements CanonicalInternalClientService { /** * Get GraphQL client with local session authentication. + * If no client exists, one will be created with the given options. + * Otherwise, the options are ignored, and the existing client is returned. + * + * @param options - Options for creating the client + * @param options.enableSubscriptions - Whether to enable WebSocket subscriptions + * @param options.origin - The origin of the client + * @returns The GraphQL client */ public async getClient(options?: { enableSubscriptions?: boolean; From 7f74d6822996670518c8d248337e487478fc5871 Mon Sep 17 00:00:00 2001 From: Pujit Mehrotra Date: Wed, 27 Aug 2025 10:29:24 -0400 Subject: [PATCH 10/16] use dependency injection for cookie prefix in internal client factory --- api/src/unraid-api/auth/cookie.service.ts | 2 +- api/src/unraid-api/auth/local-session.service.ts | 5 ++--- .../shared/internal-graphql-client.factory.spec.ts | 10 ++++++++++ .../shared/internal-graphql-client.factory.ts | 11 +++++++---- 4 files changed, 20 insertions(+), 8 deletions(-) diff --git a/api/src/unraid-api/auth/cookie.service.ts b/api/src/unraid-api/auth/cookie.service.ts index 2c2e4c232f..a474d765ce 100644 --- a/api/src/unraid-api/auth/cookie.service.ts +++ b/api/src/unraid-api/auth/cookie.service.ts @@ -9,7 +9,7 @@ import { batchProcess } from '@app/utils.js'; /** token for dependency injection of a session cookie options object */ export const SESSION_COOKIE_CONFIG = 'SESSION_COOKIE_CONFIG'; -type SessionCookieConfig = { +export type SessionCookieConfig = { namePrefix: string; sessionDir: string; secure: boolean; diff --git a/api/src/unraid-api/auth/local-session.service.ts b/api/src/unraid-api/auth/local-session.service.ts index ebcf9bdd25..70e762f144 100644 --- a/api/src/unraid-api/auth/local-session.service.ts +++ b/api/src/unraid-api/auth/local-session.service.ts @@ -64,10 +64,9 @@ export class LocalSessionService implements OnModuleInit, OnModuleDestroy { */ public async getLocalSession(): Promise { try { - const token = await readFile(LocalSessionService.SESSION_FILE_PATH, 'utf-8'); - return token.trim(); + return await readFile(LocalSessionService.SESSION_FILE_PATH, 'utf-8'); } catch (error) { - this.logger.debug('Local session file not found or not readable'); + this.logger.warn(error, 'Local session file not found or not readable'); return null; } } diff --git a/api/src/unraid-api/shared/internal-graphql-client.factory.spec.ts b/api/src/unraid-api/shared/internal-graphql-client.factory.spec.ts index bec4e28f58..d01cfa5bca 100644 --- a/api/src/unraid-api/shared/internal-graphql-client.factory.spec.ts +++ b/api/src/unraid-api/shared/internal-graphql-client.factory.spec.ts @@ -5,6 +5,7 @@ import { ApolloClient } from '@apollo/client/core/index.js'; import { SocketConfigService } from '@unraid/shared'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { SESSION_COOKIE_CONFIG } from '@app/unraid-api/auth/cookie.service.js'; import { InternalGraphQLClientFactory } from '@app/unraid-api/shared/internal-graphql-client.factory.js'; // Mock the graphql-ws module @@ -49,6 +50,15 @@ describe('InternalGraphQLClientFactory', () => { getWebSocketUri: vi.fn(), }, }, + { + provide: SESSION_COOKIE_CONFIG, + useValue: { + namePrefix: 'unraid_', + sessionDir: '/dev/sessions', + secure: true, + httpOnly: true, + }, + }, ], }).compile(); diff --git a/api/src/unraid-api/shared/internal-graphql-client.factory.ts b/api/src/unraid-api/shared/internal-graphql-client.factory.ts index 0e390c04a2..9c42bc7bc4 100644 --- a/api/src/unraid-api/shared/internal-graphql-client.factory.ts +++ b/api/src/unraid-api/shared/internal-graphql-client.factory.ts @@ -1,5 +1,4 @@ -import { Injectable, Logger } from '@nestjs/common'; -import { ConfigService } from '@nestjs/config'; +import { Inject, Injectable, Logger } from '@nestjs/common'; import type { InternalGraphQLClientFactory as IInternalGraphQLClientFactory } from '@unraid/shared'; import { ApolloClient, InMemoryCache, NormalizedCacheObject } from '@apollo/client/core/index.js'; @@ -14,6 +13,9 @@ import { createClient } from 'graphql-ws'; import { Agent, fetch as undiciFetch } from 'undici'; import WebSocket from 'ws'; +import type { SessionCookieConfig } from '@app/unraid-api/auth/cookie.service.js'; +import { SESSION_COOKIE_CONFIG } from '@app/unraid-api/auth/cookie.service.js'; + /** * Factory service for creating internal GraphQL clients. * @@ -28,7 +30,8 @@ export class InternalGraphQLClientFactory implements IInternalGraphQLClientFacto private readonly logger = new Logger(InternalGraphQLClientFactory.name); constructor( - private readonly configService: ConfigService, + @Inject(SESSION_COOKIE_CONFIG) + private readonly sessionCookieConfig: SessionCookieConfig, private readonly socketConfig: SocketConfigService ) {} @@ -143,7 +146,7 @@ export class InternalGraphQLClientFactory implements IInternalGraphQLClientFacto headers: { ...headers, 'x-csrf-token': cookieAuth.csrfToken, - cookie: `unraid_${cookieAuth.sessionId}=${cookieAuth.sessionId}`, + cookie: `${this.sessionCookieConfig.namePrefix}${cookieAuth.sessionId}=${cookieAuth.sessionId}`, }, }; } From d5eddb2c88400d1ac3bbe9c7a08a6e93d00228c9 Mon Sep 17 00:00:00 2001 From: Pujit Mehrotra Date: Wed, 27 Aug 2025 10:51:15 -0400 Subject: [PATCH 11/16] rm cleanup logic in local session service --- .../unraid-api/auth/local-session.service.ts | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/api/src/unraid-api/auth/local-session.service.ts b/api/src/unraid-api/auth/local-session.service.ts index 70e762f144..916818668e 100644 --- a/api/src/unraid-api/auth/local-session.service.ts +++ b/api/src/unraid-api/auth/local-session.service.ts @@ -1,6 +1,6 @@ -import { Injectable, Logger, OnModuleDestroy, OnModuleInit } from '@nestjs/common'; +import { Injectable, Logger, OnModuleInit } from '@nestjs/common'; import { randomBytes, timingSafeEqual } from 'crypto'; -import { chmod, mkdir, readFile, unlink, writeFile } from 'fs/promises'; +import { chmod, mkdir, readFile, writeFile } from 'fs/promises'; import { dirname } from 'path'; /** @@ -8,11 +8,14 @@ import { dirname } from 'path'; * Creates a secure token on startup that can be used for local system operations. */ @Injectable() -export class LocalSessionService implements OnModuleInit, OnModuleDestroy { +export class LocalSessionService implements OnModuleInit { private readonly logger = new Logger(LocalSessionService.name); private sessionToken: string | null = null; private static readonly SESSION_FILE_PATH = '/var/run/unraid-api/local-session'; + // NOTE: do NOT cleanup the session file upon eg. module/application shutdown. + // That would invalidate the session after each cli invocation, which is incorrect. + // Instead, rely on the startup logic to invalidate/overwrite any obsolete session. async onModuleInit() { try { await this.generateLocalSession(); @@ -22,16 +25,6 @@ export class LocalSessionService implements OnModuleInit, OnModuleDestroy { } } - async onModuleDestroy() { - if (!this.sessionToken) return; - try { - await unlink(LocalSessionService.SESSION_FILE_PATH); - this.logger.verbose('Local session file deleted'); - } catch (error) { - this.logger.warn(error, 'Error deleting local session file'); - } - } - /** * Generate a secure local session token and write it to file */ From 8b6fb6698990dc556b9e41420327534a56c0db3e Mon Sep 17 00:00:00 2001 From: Pujit Mehrotra Date: Wed, 27 Aug 2025 11:31:06 -0400 Subject: [PATCH 12/16] use separate LocalSessionLifecycleService to manage local session validation --- api/src/unraid-api/auth/auth.module.ts | 2 + .../auth/local-session-lifecycle.service.ts | 25 ++++++++++++ .../unraid-api/auth/local-session.service.ts | 38 +++++++++---------- 3 files changed, 44 insertions(+), 21 deletions(-) create mode 100644 api/src/unraid-api/auth/local-session-lifecycle.service.ts diff --git a/api/src/unraid-api/auth/auth.module.ts b/api/src/unraid-api/auth/auth.module.ts index 29121c2ab5..e27b0cc2ea 100644 --- a/api/src/unraid-api/auth/auth.module.ts +++ b/api/src/unraid-api/auth/auth.module.ts @@ -11,6 +11,7 @@ import { BASE_POLICY, CASBIN_MODEL } from '@app/unraid-api/auth/casbin/index.js' import { CookieService, SESSION_COOKIE_CONFIG } from '@app/unraid-api/auth/cookie.service.js'; import { UserCookieStrategy } from '@app/unraid-api/auth/cookie.strategy.js'; import { ServerHeaderStrategy } from '@app/unraid-api/auth/header.strategy.js'; +import { LocalSessionLifecycleService } from '@app/unraid-api/auth/local-session-lifecycle.service.js'; import { LocalSessionService } from '@app/unraid-api/auth/local-session.service.js'; import { LocalSessionStrategy } from '@app/unraid-api/auth/local-session.strategy.js'; import { getRequest } from '@app/utils.js'; @@ -75,6 +76,7 @@ import { getRequest } from '@app/utils.js'; UserCookieStrategy, CookieService, LocalSessionService, + LocalSessionLifecycleService, AuthZModule, ], }) diff --git a/api/src/unraid-api/auth/local-session-lifecycle.service.ts b/api/src/unraid-api/auth/local-session-lifecycle.service.ts new file mode 100644 index 0000000000..0a6264b35f --- /dev/null +++ b/api/src/unraid-api/auth/local-session-lifecycle.service.ts @@ -0,0 +1,25 @@ +import { Injectable, OnModuleDestroy, OnModuleInit } from '@nestjs/common'; + +import { LocalSessionService } from '@app/unraid-api/auth/local-session.service.js'; + +/** + * Service for managing the lifecycle of the local session. + * + * Used for tying the local session's lifecycle to the API's life, rather + * than the LocalSessionService's lifecycle, since it may also be used by + * other applications, like the CLI. + * + * This service is only used in the API, and not in the CLI. + */ +@Injectable() +export class LocalSessionLifecycleService implements OnModuleInit, OnModuleDestroy { + constructor(private readonly localSessionService: LocalSessionService) {} + + async onModuleInit() { + await this.localSessionService.generateLocalSession(); + } + + async onModuleDestroy() { + await this.localSessionService.deleteLocalSession(); + } +} diff --git a/api/src/unraid-api/auth/local-session.service.ts b/api/src/unraid-api/auth/local-session.service.ts index 916818668e..6471d9ff89 100644 --- a/api/src/unraid-api/auth/local-session.service.ts +++ b/api/src/unraid-api/auth/local-session.service.ts @@ -1,6 +1,6 @@ -import { Injectable, Logger, OnModuleInit } from '@nestjs/common'; +import { Injectable, Logger } from '@nestjs/common'; import { randomBytes, timingSafeEqual } from 'crypto'; -import { chmod, mkdir, readFile, writeFile } from 'fs/promises'; +import { chmod, mkdir, readFile, unlink, writeFile } from 'fs/promises'; import { dirname } from 'path'; /** @@ -8,44 +8,32 @@ import { dirname } from 'path'; * Creates a secure token on startup that can be used for local system operations. */ @Injectable() -export class LocalSessionService implements OnModuleInit { +export class LocalSessionService { private readonly logger = new Logger(LocalSessionService.name); private sessionToken: string | null = null; private static readonly SESSION_FILE_PATH = '/var/run/unraid-api/local-session'; - // NOTE: do NOT cleanup the session file upon eg. module/application shutdown. - // That would invalidate the session after each cli invocation, which is incorrect. - // Instead, rely on the startup logic to invalidate/overwrite any obsolete session. - async onModuleInit() { - try { - await this.generateLocalSession(); - this.logger.verbose('Local session initialized'); - } catch (error) { - this.logger.error('Failed to initialize local session:', error); - } - } - /** * Generate a secure local session token and write it to file */ - private async generateLocalSession(): Promise { + async generateLocalSession(): Promise { // Generate a cryptographically secure random token this.sessionToken = randomBytes(32).toString('hex'); try { // Ensure directory exists - await mkdir(dirname(LocalSessionService.SESSION_FILE_PATH), { recursive: true }); + await mkdir(dirname(LocalSessionService.getSessionFilePath()), { recursive: true }); // Write token to file - await writeFile(LocalSessionService.SESSION_FILE_PATH, this.sessionToken, { + await writeFile(LocalSessionService.getSessionFilePath(), this.sessionToken, { encoding: 'utf-8', mode: 0o600, // Owner read/write only }); // Ensure proper permissions (redundant but explicit) - await chmod(LocalSessionService.SESSION_FILE_PATH, 0o600); + await chmod(LocalSessionService.getSessionFilePath(), 0o600); - this.logger.debug(`Local session written to ${LocalSessionService.SESSION_FILE_PATH}`); + this.logger.debug(`Local session written to ${LocalSessionService.getSessionFilePath()}`); } catch (error) { this.logger.error(`Failed to write local session: ${error}`); throw error; @@ -57,7 +45,7 @@ export class LocalSessionService implements OnModuleInit { */ public async getLocalSession(): Promise { try { - return await readFile(LocalSessionService.SESSION_FILE_PATH, 'utf-8'); + return await readFile(LocalSessionService.getSessionFilePath(), 'utf-8'); } catch (error) { this.logger.warn(error, 'Local session file not found or not readable'); return null; @@ -77,6 +65,14 @@ export class LocalSessionService implements OnModuleInit { return timingSafeEqual(Buffer.from(token, 'utf-8'), Buffer.from(currentToken, 'utf-8')); } + public async deleteLocalSession(): Promise { + try { + await unlink(LocalSessionService.getSessionFilePath()); + } catch (error) { + this.logger.error(error, 'Error deleting local session file'); + } + } + /** * Get the file path for the local session (useful for external readers) */ From 54933d8f7460209a12f5fb579bf0e6341e6d96d7 Mon Sep 17 00:00:00 2001 From: Pujit Mehrotra Date: Wed, 27 Aug 2025 12:01:50 -0400 Subject: [PATCH 13/16] handle chmod failure due to race condition --- api/.env.development | 1 + api/.env.test | 1 + api/.gitignore | 2 ++ api/src/environment.ts | 3 +++ api/src/unraid-api/auth/auth.module.ts | 1 + api/src/unraid-api/auth/local-session.service.ts | 9 +++++++-- 6 files changed, 15 insertions(+), 2 deletions(-) diff --git a/api/.env.development b/api/.env.development index 511bc0e670..7c42ec26ce 100644 --- a/api/.env.development +++ b/api/.env.development @@ -18,6 +18,7 @@ PATHS_LOG_BASE=./dev/log # Where we store logs PATHS_LOGS_FILE=./dev/log/graphql-api.log PATHS_CONNECT_STATUS_FILE_PATH=./dev/connectStatus.json # Connect plugin status file PATHS_OIDC_JSON=./dev/configs/oidc.local.json +PATHS_LOCAL_SESSION_FILE=./dev/local-session ENVIRONMENT="development" NODE_ENV="development" PORT="3001" diff --git a/api/.env.test b/api/.env.test index b346f3a824..5902c8337e 100644 --- a/api/.env.test +++ b/api/.env.test @@ -14,5 +14,6 @@ PATHS_CONFIG_MODULES=./dev/configs PATHS_ACTIVATION_BASE=./dev/activation PATHS_PASSWD=./dev/passwd PATHS_LOGS_FILE=./dev/log/graphql-api.log +PATHS_LOCAL_SESSION_FILE=./dev/local-session PORT=5000 NODE_ENV="test" diff --git a/api/.gitignore b/api/.gitignore index 1e587b4054..3d895b2eb4 100644 --- a/api/.gitignore +++ b/api/.gitignore @@ -88,6 +88,8 @@ dev/connectStatus.json dev/configs/* # local status - doesn't need to be tracked dev/connectStatus.json +# mock local session file +dev/local-session # local OIDC config for testing - contains secrets dev/configs/oidc.local.json diff --git a/api/src/environment.ts b/api/src/environment.ts index 906c444e60..55033febad 100644 --- a/api/src/environment.ts +++ b/api/src/environment.ts @@ -108,3 +108,6 @@ export const PATHS_LOGS_FILE = process.env.PATHS_LOGS_FILE ?? '/var/log/graphql- export const PATHS_CONFIG_MODULES = process.env.PATHS_CONFIG_MODULES ?? '/boot/config/plugins/dynamix.my.servers/configs'; + +export const PATHS_LOCAL_SESSION_FILE = + process.env.PATHS_LOCAL_SESSION_FILE ?? '/var/run/unraid-api/local-session'; diff --git a/api/src/unraid-api/auth/auth.module.ts b/api/src/unraid-api/auth/auth.module.ts index e27b0cc2ea..f023a3a705 100644 --- a/api/src/unraid-api/auth/auth.module.ts +++ b/api/src/unraid-api/auth/auth.module.ts @@ -62,6 +62,7 @@ import { getRequest } from '@app/utils.js'; UserCookieStrategy, CookieService, LocalSessionService, + LocalSessionLifecycleService, { provide: SESSION_COOKIE_CONFIG, useValue: CookieService.defaultOpts(), diff --git a/api/src/unraid-api/auth/local-session.service.ts b/api/src/unraid-api/auth/local-session.service.ts index 6471d9ff89..b2ef881fc0 100644 --- a/api/src/unraid-api/auth/local-session.service.ts +++ b/api/src/unraid-api/auth/local-session.service.ts @@ -3,6 +3,8 @@ import { randomBytes, timingSafeEqual } from 'crypto'; import { chmod, mkdir, readFile, unlink, writeFile } from 'fs/promises'; import { dirname } from 'path'; +import { PATHS_LOCAL_SESSION_FILE } from '@app/environment.js'; + /** * Service that manages a local session file for internal CLI/system authentication. * Creates a secure token on startup that can be used for local system operations. @@ -11,7 +13,7 @@ import { dirname } from 'path'; export class LocalSessionService { private readonly logger = new Logger(LocalSessionService.name); private sessionToken: string | null = null; - private static readonly SESSION_FILE_PATH = '/var/run/unraid-api/local-session'; + private static readonly SESSION_FILE_PATH = PATHS_LOCAL_SESSION_FILE; /** * Generate a secure local session token and write it to file @@ -31,7 +33,10 @@ export class LocalSessionService { }); // Ensure proper permissions (redundant but explicit) - await chmod(LocalSessionService.getSessionFilePath(), 0o600); + // Check if file exists first to handle race conditions in test environments + await chmod(LocalSessionService.getSessionFilePath(), 0o600).catch((error) => { + this.logger.warn(error, 'Failed to set permissions on local session file'); + }); this.logger.debug(`Local session written to ${LocalSessionService.getSessionFilePath()}`); } catch (error) { From c6a2873eef8bcd8c5c082e59ee2a4f3317734e69 Mon Sep 17 00:00:00 2001 From: Pujit Mehrotra Date: Wed, 27 Aug 2025 13:03:32 -0400 Subject: [PATCH 14/16] rm onModuleDestroy hook --- .../unraid-api/auth/local-session-lifecycle.service.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/api/src/unraid-api/auth/local-session-lifecycle.service.ts b/api/src/unraid-api/auth/local-session-lifecycle.service.ts index 0a6264b35f..33860d416a 100644 --- a/api/src/unraid-api/auth/local-session-lifecycle.service.ts +++ b/api/src/unraid-api/auth/local-session-lifecycle.service.ts @@ -1,4 +1,4 @@ -import { Injectable, OnModuleDestroy, OnModuleInit } from '@nestjs/common'; +import { Injectable, OnModuleInit } from '@nestjs/common'; import { LocalSessionService } from '@app/unraid-api/auth/local-session.service.js'; @@ -12,14 +12,10 @@ import { LocalSessionService } from '@app/unraid-api/auth/local-session.service. * This service is only used in the API, and not in the CLI. */ @Injectable() -export class LocalSessionLifecycleService implements OnModuleInit, OnModuleDestroy { +export class LocalSessionLifecycleService implements OnModuleInit { constructor(private readonly localSessionService: LocalSessionService) {} async onModuleInit() { await this.localSessionService.generateLocalSession(); } - - async onModuleDestroy() { - await this.localSessionService.deleteLocalSession(); - } } From 6ea17192c649e40d7699ed6477d0da820535a775 Mon Sep 17 00:00:00 2001 From: Pujit Mehrotra Date: Wed, 27 Aug 2025 14:52:21 -0400 Subject: [PATCH 15/16] restore api-key.service to main --- api/src/unraid-api/auth/api-key.service.ts | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/api/src/unraid-api/auth/api-key.service.ts b/api/src/unraid-api/auth/api-key.service.ts index 4c0ca87fc9..1108485b2a 100644 --- a/api/src/unraid-api/auth/api-key.service.ts +++ b/api/src/unraid-api/auth/api-key.service.ts @@ -41,12 +41,7 @@ export class ApiKeyService implements OnModuleInit { } public async findAll(): Promise { - return Promise.all( - this.memoryApiKeys.map(async (key) => { - const keyWithoutSecret = this.convertApiKeyWithSecretToApiKey(key); - return keyWithoutSecret; - }) - ); + return this.memoryApiKeys; } private setupWatch() { @@ -241,7 +236,7 @@ export class ApiKeyService implements OnModuleInit { return this.memoryApiKeys.find((k) => k[field] === value) ?? null; } - findByKey(key: string): ApiKeyWithSecret | null { + findByKey(key: string): ApiKey | null { return this.findByField('key', key); } From b3e48bdf863b250c25a99b1df09f5c679f7f559d Mon Sep 17 00:00:00 2001 From: Eli Bosley Date: Wed, 27 Aug 2025 14:49:46 -0400 Subject: [PATCH 16/16] refactor LocalSessionService: improve token validation by coercing inputs and using constant-time comparison --- .../unraid-api/auth/local-session.service.ts | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/api/src/unraid-api/auth/local-session.service.ts b/api/src/unraid-api/auth/local-session.service.ts index b2ef881fc0..f91d8f0ce4 100644 --- a/api/src/unraid-api/auth/local-session.service.ts +++ b/api/src/unraid-api/auth/local-session.service.ts @@ -61,13 +61,23 @@ export class LocalSessionService { * Validate if a given token matches the current local session */ public async validateLocalSession(token: string): Promise { - if (!token) return false; - + // Coerce inputs to strings (or empty string if undefined) + const tokenStr = token || ''; const currentToken = await this.getLocalSession(); - if (!currentToken) return false; + const currentTokenStr = currentToken || ''; + + // Early return if either is empty + if (!tokenStr || !currentTokenStr) return false; + + // Create buffers + const tokenBuffer = Buffer.from(tokenStr, 'utf-8'); + const currentTokenBuffer = Buffer.from(currentTokenStr, 'utf-8'); + + // Check length equality first to prevent timingSafeEqual from throwing + if (tokenBuffer.length !== currentTokenBuffer.length) return false; // Use constant-time comparison to prevent timing attacks - return timingSafeEqual(Buffer.from(token, 'utf-8'), Buffer.from(currentToken, 'utf-8')); + return timingSafeEqual(tokenBuffer, currentTokenBuffer); } public async deleteLocalSession(): Promise {