diff --git a/api/.env.development b/api/.env.development index 511bc0e67..7c42ec26c 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 b346f3a82..5902c8337 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 1e587b405..3d895b2eb 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/dev/keys/b5b4aa3d-8e40-4c92-bc40-d50182071886.json b/api/dev/keys/b5b4aa3d-8e40-4c92-bc40-d50182071886.json deleted file mode 100644 index 0bc2a7891..000000000 --- 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 d734aead6..000000000 --- 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/environment.ts b/api/src/environment.ts index 906c444e6..55033feba 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 7ce3ef9af..f023a3a70 100644 --- a/api/src/unraid-api/auth/auth.module.ts +++ b/api/src/unraid-api/auth/auth.module.ts @@ -11,13 +11,19 @@ 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 { 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'; @Module({ imports: [ PassportModule.register({ - defaultStrategy: [ServerHeaderStrategy.key, UserCookieStrategy.key], + defaultStrategy: [ + ServerHeaderStrategy.key, + LocalSessionStrategy.key, + UserCookieStrategy.key, + ], }), CasbinModule, AuthZModule.register({ @@ -51,10 +57,12 @@ import { getRequest } from '@app/utils.js'; providers: [ AuthService, ApiKeyService, - AdminKeyService, ServerHeaderStrategy, + LocalSessionStrategy, UserCookieStrategy, CookieService, + LocalSessionService, + LocalSessionLifecycleService, { provide: SESSION_COOKIE_CONFIG, useValue: CookieService.defaultOpts(), @@ -65,8 +73,11 @@ import { getRequest } from '@app/utils.js'; ApiKeyService, PassportModule, ServerHeaderStrategy, + LocalSessionStrategy, UserCookieStrategy, CookieService, + LocalSessionService, + LocalSessionLifecycleService, AuthZModule, ], }) diff --git a/api/src/unraid-api/auth/auth.service.spec.ts b/api/src/unraid-api/auth/auth.service.spec.ts index 62178b470..961368b89 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 5ba0ef549..5457a3a04 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 { @@ -12,6 +13,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 +26,7 @@ export class AuthService { constructor( private cookieService: CookieService, private apiKeyService: ApiKeyService, + private localSessionService: LocalSessionService, private authzService: AuthZService ) {} @@ -89,6 +92,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 @@ -254,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')); } /** @@ -321,7 +351,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 +360,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 9c9469d52..367f9c2e3 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/cookie.service.ts b/api/src/unraid-api/auth/cookie.service.ts index e847b7f5e..a474d765c 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'; @@ -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; @@ -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,33 @@ 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 { + 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) { + 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; + } } 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 000000000..33860d416 --- /dev/null +++ b/api/src/unraid-api/auth/local-session-lifecycle.service.ts @@ -0,0 +1,21 @@ +import { Injectable, 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 { + constructor(private readonly localSessionService: LocalSessionService) {} + + async onModuleInit() { + await this.localSessionService.generateLocalSession(); + } +} 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 000000000..f91d8f0ce --- /dev/null +++ b/api/src/unraid-api/auth/local-session.service.ts @@ -0,0 +1,97 @@ +import { Injectable, Logger } from '@nestjs/common'; +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. + */ +@Injectable() +export class LocalSessionService { + private readonly logger = new Logger(LocalSessionService.name); + private sessionToken: string | null = null; + private static readonly SESSION_FILE_PATH = PATHS_LOCAL_SESSION_FILE; + + /** + * Generate a secure local session token and write it to file + */ + async generateLocalSession(): Promise { + // Generate a cryptographically secure random token + this.sessionToken = randomBytes(32).toString('hex'); + + try { + // Ensure directory exists + await mkdir(dirname(LocalSessionService.getSessionFilePath()), { recursive: true }); + + // Write token to file + await writeFile(LocalSessionService.getSessionFilePath(), this.sessionToken, { + encoding: 'utf-8', + mode: 0o600, // Owner read/write only + }); + + // Ensure proper permissions (redundant but explicit) + // 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) { + 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 { + return await readFile(LocalSessionService.getSessionFilePath(), 'utf-8'); + } catch (error) { + this.logger.warn(error, '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 { + // Coerce inputs to strings (or empty string if undefined) + const tokenStr = token || ''; + const currentToken = await this.getLocalSession(); + 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(tokenBuffer, currentTokenBuffer); + } + + 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) + */ + 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 000000000..ef0bb6ef9 --- /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 36f1f1473..8dc1c6cde 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 5e1b644df..0bd584502 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 43794a717..000000000 --- a/api/src/unraid-api/cli/admin-key.service.ts +++ /dev/null @@ -1,44 +0,0 @@ -import { Inject, Injectable, Logger, OnModuleInit } from '@nestjs/common'; - -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. - */ -@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'; - - constructor( - @Inject(API_KEY_SERVICE_TOKEN) - private readonly apiKeyService: ApiKeyService - ) {} - - async onModuleInit() { - try { - await this.getOrCreateLocalAdminKey(); - this.logger.log('Admin API key initialized successfully'); - } catch (error) { - this.logger.error('Failed to initialize admin API key:', error); - } - } - - /** - * Gets or creates a local admin API key for CLI operations. - * Uses the standard API key storage location. - */ - 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 - }); - } -} diff --git a/api/src/unraid-api/cli/api-report.service.ts b/api/src/unraid-api/cli/api-report.service.ts index 6b2ecac41..c2648abad 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 cff41d1b0..7f248390d 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 97fd0ac8e..15164d1c7 100644 --- a/api/src/unraid-api/cli/cli.module.spec.ts +++ b/api/src/unraid-api/cli/cli.module.spec.ts @@ -1,12 +1,10 @@ import { ConfigModule } from '@nestjs/config'; import { Test, TestingModule } from '@nestjs/testing'; -import { 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 { 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,29 +24,23 @@ 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', () => { - const factory = module.get(INTERNAL_CLIENT_SERVICE_TOKEN); + const factory = module.get(INTERNAL_CLIENT_FACTORY_TOKEN); expect(factory).toBeDefined(); 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 @@ -59,16 +51,9 @@ 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(); }); - - 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 31e9566e6..7befdcb0e 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 c03dbaa5c..5dc0784f5 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 c17468d23..000000000 --- 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 b862d64e7..000000000 --- a/api/src/unraid-api/cli/internal-client.service.ts +++ /dev/null @@ -1,97 +0,0 @@ -import { Inject, Injectable, Logger } from '@nestjs/common'; - -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 using the AdminKeyService. - * This ensures the key exists and is available for CLI operations. - */ - private async getLocalApiKey(): Promise { - 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 0440c4831..f12a6df9b 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 ea1e05eea..6ce9977a6 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 bdf483a60..0ca32e14a 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 851b31fdf..d95761d10 100644 --- a/api/src/unraid-api/plugin/global-deps.module.ts +++ b/api/src/unraid-api/plugin/global-deps.module.ts @@ -5,7 +5,9 @@ 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, - INTERNAL_CLIENT_SERVICE_TOKEN, + CANONICAL_INTERNAL_CLIENT_TOKEN, + COOKIE_SERVICE_TOKEN, + INTERNAL_CLIENT_FACTORY_TOKEN, LIFECYCLE_SERVICE_TOKEN, NGINX_SERVICE_TOKEN, UPNP_CLIENT_TOKEN, @@ -14,9 +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, 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'; @@ -27,7 +32,7 @@ import { upnpClient } from '@app/upnp/helpers.js'; providers: [ SocketConfigService, { - provide: INTERNAL_CLIENT_SERVICE_TOKEN, + provide: INTERNAL_CLIENT_FACTORY_TOKEN, useClass: InternalGraphQLClientFactory, }, { @@ -42,10 +47,25 @@ 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, + }, { provide: NGINX_SERVICE_TOKEN, useClass: NginxService, }, + // Canonical internal client service + LocalSessionService, + InternalClientService, + { + provide: CANONICAL_INTERNAL_CLIENT_TOKEN, + useExisting: InternalClientService, + }, PrefixedID, LifecycleService, { @@ -58,8 +78,10 @@ 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, + INTERNAL_CLIENT_FACTORY_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 2f4d403f4..d97745261 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/api/src/unraid-api/shared/internal-client.service.ts b/api/src/unraid-api/shared/internal-client.service.ts new file mode 100644 index 000000000..7096a34b2 --- /dev/null +++ b/api/src/unraid-api/shared/internal-client.service.ts @@ -0,0 +1,94 @@ +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_FACTORY_TOKEN } from '@unraid/shared'; + +import { LocalSessionService } from '@app/unraid-api/auth/local-session.service.js'; + +/** + * 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 implements CanonicalInternalClientService { + private readonly logger = new Logger(InternalClientService.name); + private client: ApolloClient | null = null; + private clientCreationPromise: Promise> | null = null; + + constructor( + @Inject(INTERNAL_CLIENT_FACTORY_TOKEN) + private readonly clientFactory: InternalGraphQLClientFactory, + private readonly localSessionService: LocalSessionService + ) {} + + /** + * 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; + 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(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 (this.clientCreationPromise === creationPromise) { + this.client = client; + } + return client; + } finally { + // Clear the in-flight promise only if it's still ours + if (this.clientCreationPromise === creationPromise) { + this.clientCreationPromise = null; + } + } + } + + 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({ + getLocalSession: () => this.localSessionService.getLocalSession(), + enableSubscriptions, + origin, + }); + + this.logger.debug('Created canonical internal GraphQL client with local session authentication'); + return client; + } + + public clearClient() { + // Stop the Apollo client to terminate any active processes + this.client?.stop(); + this.client = null; + this.clientCreationPromise = 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 bec4e28f5..d01cfa5bc 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 4ee4523a7..9c42bc7bc 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 ) {} @@ -36,20 +39,32 @@ 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) + * @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; + getApiKey?: () => Promise; + getCookieAuth?: () => Promise<{ sessionId: string; csrfToken: string } | null>; + getLocalSession?: () => Promise; 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 && !options.getLocalSession) { + throw new Error( + 'One of getApiKey, getCookieAuth, or getLocalSession function is required for creating a GraphQL client' + ); } - const { getApiKey, enableSubscriptions = false, origin = 'http://localhost' } = options; + const { + getApiKey, + getCookieAuth, + getLocalSession, + enableSubscriptions = false, + origin = 'http://localhost', + } = options; let httpLink: HttpLink; // Get WebSocket URI if subscriptions are enabled @@ -98,15 +113,45 @@ 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 (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(); + if (!cookieAuth) { + throw new Error('No valid session found for cookie authentication'); + } + return { + headers: { + ...headers, + 'x-csrf-token': cookieAuth.csrfToken, + cookie: `${this.sessionCookieConfig.namePrefix}${cookieAuth.sessionId}=${cookieAuth.sessionId}`, + }, + }; + } + + return { headers }; }); const errorLink = onError(({ networkError }) => { @@ -121,8 +166,30 @@ 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 (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) { + 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/authn/connect-api-key.service.ts b/packages/unraid-api-plugin-connect/src/authn/connect-api-key.service.ts deleted file mode 100644 index 834cda322..000000000 --- a/packages/unraid-api-plugin-connect/src/authn/connect-api-key.service.ts +++ /dev/null @@ -1,107 +0,0 @@ -import { Inject, Injectable, Logger } 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 { - 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'; - - constructor( - @Inject(API_KEY_SERVICE_TOKEN) - private readonly apiKeyService: ApiKeyService - ) {} - - // 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 - */ - 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; - } - } - - /** - * Gets or creates a local API key for Connect - */ - public async getOrCreateLocalApiKey(): Promise { - return this.ensureKey({ - name: ConnectApiKeyService.CONNECT_API_KEY_NAME, - description: ConnectApiKeyService.CONNECT_API_KEY_DESCRIPTION, - roles: [Role.CONNECT], - legacyNames: ['Connect'], - }); - } - - 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 cdd7e06ac..000000000 --- a/packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.spec.ts +++ /dev/null @@ -1,275 +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 apiKeyService: any; - - const mockApolloClient = { - query: vi.fn(), - mutate: vi.fn(), - stop: vi.fn(), - }; - - beforeEach(() => { - clientFactory = { - createClient: vi.fn().mockResolvedValue(mockApolloClient), - }; - - apiKeyService = { - getOrCreateLocalApiKey: vi.fn().mockResolvedValue('test-connect-key'), - }; - - service = new InternalClientService( - clientFactory as any, - apiKeyService as any - ); - }); - - afterEach(() => { - vi.clearAllMocks(); - }); - - it('should be defined', () => { - expect(service).toBeDefined(); - }); - - describe('getClient', () => { - it('should create a client with Connect API key and subscriptions', async () => { - const client = await service.getClient(); - - // The API key is now fetched lazily through getApiKey function - expect(clientFactory.createClient).toHaveBeenCalledWith({ - getApiKey: expect.any(Function), - enableSubscriptions: true, - }); - - // 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-connect-key'); - expect(apiKeyService.getOrCreateLocalApiKey).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 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); - }); - }); - - 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/internal-rpc/internal.client.ts b/packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts deleted file mode 100644 index 87200b58b..000000000 --- a/packages/unraid-api-plugin-connect/src/internal-rpc/internal.client.ts +++ /dev/null @@ -1,74 +0,0 @@ -import { Inject, Injectable, Logger } from '@nestjs/common'; -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'; - -/** - * Connect-specific internal GraphQL client. - * - * This uses the shared GraphQL client factory with Connect's API key - * and enables subscriptions for real-time updates. - */ -@Injectable() -export class InternalClientService { - private readonly logger = new Logger(InternalClientService.name); - private client: ApolloClient | null = null; - private clientCreationPromise: Promise> | null = null; - - constructor( - @Inject(INTERNAL_CLIENT_SERVICE_TOKEN) - private readonly clientFactory: InternalGraphQLClientFactory, - private readonly apiKeyService: ConnectApiKeyService - ) {} - - public async getClient(): 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(); - 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; - } - return client; - } finally { - // Clear the in-flight promise only if it's still ours - if (this.clientCreationPromise === creationPromise) { - this.clientCreationPromise = null; - } - } - } - - private async createClient(): Promise> { - // Create a client with a function to get Connect's API key dynamically - const client = await this.clientFactory.createClient({ - getApiKey: () => this.apiKeyService.getOrCreateLocalApiKey(), - enableSubscriptions: true - }); - - this.logger.debug('Created Connect internal GraphQL client with subscriptions enabled'); - return client; - } - - public clearClient() { - // Stop the Apollo client to terminate any active processes - this.client?.stop(); - this.client = null; - this.clientCreationPromise = null; - } -} 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 5282ec618..fefc358bd 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 75bde5acb..267b43826 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 0b5daee9e..149307b0b 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 a25b1236b..be11279ca 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 7804ac524..301f3b222 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 97b79c169..7cea8c218 100644 --- a/packages/unraid-shared/src/tokens.ts +++ b/packages/unraid-shared/src/tokens.ts @@ -2,4 +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'; 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 000000000..c07e37871 --- /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 c557cee1a..c921629d6 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,9 @@ 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>; + getLocalSession?: () => Promise; enableSubscriptions?: boolean; origin?: string; }): Promise>;