diff --git a/api/src/unraid-api/config/api-config.module.ts b/api/src/unraid-api/config/api-config.module.ts index 8c167a14b..d3c04a717 100644 --- a/api/src/unraid-api/config/api-config.module.ts +++ b/api/src/unraid-api/config/api-config.module.ts @@ -4,7 +4,7 @@ import { ConfigService, registerAs } from '@nestjs/config'; import type { ApiConfig } from '@unraid/shared/services/api-config.js'; import { csvStringToArray } from '@unraid/shared/util/data.js'; import { fileExists } from '@unraid/shared/util/file.js'; -import { debounceTime } from 'rxjs/operators'; +import { bufferTime, debounceTime } from 'rxjs/operators'; import { API_VERSION } from '@app/environment.js'; import { ApiStateConfig } from '@app/unraid-api/config/factory/api-state.model.js'; @@ -56,7 +56,7 @@ export const loadApiConfig = async () => { export const apiConfig = registerAs('api', loadApiConfig); @Injectable() -class ApiConfigPersistence { +export class ApiConfigPersistence { private configModel: ApiStateConfig; private logger = new Logger(ApiConfigPersistence.name); get filePath() { @@ -85,10 +85,10 @@ class ApiConfigPersistence { this.migrateFromMyServersConfig(); } await this.persistenceHelper.persistIfChanged(this.filePath, this.config); - this.configService.changes$.pipe(debounceTime(25)).subscribe({ - next: async ({ newValue, oldValue, path }) => { - if (path.startsWith('api')) { - this.logger.verbose(`Config changed: ${path} from ${oldValue} to ${newValue}`); + this.configService.changes$.pipe(bufferTime(25)).subscribe({ + next: async (changes) => { + if (changes.some((change) => change.path.startsWith('api'))) { + this.logger.verbose(`API Config changed ${JSON.stringify(changes)}`); await this.persistenceHelper.persistIfChanged(this.filePath, this.config); } }, @@ -98,15 +98,26 @@ class ApiConfigPersistence { }); } - private migrateFromMyServersConfig() { - const { local, api, remote } = this.configService.get('store.config', {}); - const sandbox = local?.sandbox; - const extraOrigins = csvStringToArray(api?.extraOrigins ?? '').filter( - (origin) => origin.startsWith('http://') || origin.startsWith('https://') - ); - const ssoSubIds = csvStringToArray(remote?.ssoSubIds ?? ''); + convertLegacyConfig( + config?: Partial<{ + local: { sandbox?: string }; + api: { extraOrigins?: string }; + remote: { ssoSubIds?: string }; + }> + ) { + return { + sandbox: config?.local?.sandbox === 'yes', + extraOrigins: csvStringToArray(config?.api?.extraOrigins ?? '').filter( + (origin) => origin.startsWith('http://') || origin.startsWith('https://') + ), + ssoSubIds: csvStringToArray(config?.remote?.ssoSubIds ?? ''), + }; + } - this.configService.set('api.sandbox', sandbox === 'yes'); + migrateFromMyServersConfig() { + const legacyConfig = this.configService.get('store.config', {}); + const { sandbox, extraOrigins, ssoSubIds } = this.convertLegacyConfig(legacyConfig); + this.configService.set('api.sandbox', sandbox); this.configService.set('api.extraOrigins', extraOrigins); this.configService.set('api.ssoSubIds', ssoSubIds); } diff --git a/api/src/unraid-api/config/api-config.test.ts b/api/src/unraid-api/config/api-config.test.ts new file mode 100644 index 000000000..58f9603c2 --- /dev/null +++ b/api/src/unraid-api/config/api-config.test.ts @@ -0,0 +1,137 @@ +import { ConfigService } from '@nestjs/config'; + +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { ApiConfigPersistence } from '@app/unraid-api/config/api-config.module.js'; +import { ConfigPersistenceHelper } from '@app/unraid-api/config/persistence.helper.js'; + +describe('ApiConfigPersistence', () => { + let service: ApiConfigPersistence; + let configService: ConfigService; + let persistenceHelper: ConfigPersistenceHelper; + + beforeEach(() => { + configService = { + get: vi.fn(), + set: vi.fn(), + } as any; + + persistenceHelper = {} as ConfigPersistenceHelper; + service = new ApiConfigPersistence(configService, persistenceHelper); + }); + + describe('convertLegacyConfig', () => { + it('should migrate sandbox from string "yes" to boolean true', () => { + const legacyConfig = { + local: { sandbox: 'yes' }, + api: { extraOrigins: '' }, + remote: { ssoSubIds: '' }, + }; + + const result = service.convertLegacyConfig(legacyConfig); + + expect(result.sandbox).toBe(true); + }); + + it('should migrate sandbox from string "no" to boolean false', () => { + const legacyConfig = { + local: { sandbox: 'no' }, + api: { extraOrigins: '' }, + remote: { ssoSubIds: '' }, + }; + + const result = service.convertLegacyConfig(legacyConfig); + + expect(result.sandbox).toBe(false); + }); + + it('should migrate extraOrigins from comma-separated string to array', () => { + const legacyConfig = { + local: { sandbox: 'no' }, + api: { extraOrigins: 'https://example.com,https://test.com' }, + remote: { ssoSubIds: '' }, + }; + + const result = service.convertLegacyConfig(legacyConfig); + + expect(result.extraOrigins).toEqual(['https://example.com', 'https://test.com']); + }); + + it('should filter out non-HTTP origins from extraOrigins', () => { + const legacyConfig = { + local: { sandbox: 'no' }, + api: { + extraOrigins: 'https://example.com,invalid-origin,http://test.com,ftp://bad.com', + }, + remote: { ssoSubIds: '' }, + }; + + const result = service.convertLegacyConfig(legacyConfig); + + expect(result.extraOrigins).toEqual(['https://example.com', 'http://test.com']); + }); + + it('should handle empty extraOrigins string', () => { + const legacyConfig = { + local: { sandbox: 'no' }, + api: { extraOrigins: '' }, + remote: { ssoSubIds: '' }, + }; + + const result = service.convertLegacyConfig(legacyConfig); + + expect(result.extraOrigins).toEqual([]); + }); + + it('should migrate ssoSubIds from comma-separated string to array', () => { + const legacyConfig = { + local: { sandbox: 'no' }, + api: { extraOrigins: '' }, + remote: { ssoSubIds: 'user1,user2,user3' }, + }; + + const result = service.convertLegacyConfig(legacyConfig); + + expect(result.ssoSubIds).toEqual(['user1', 'user2', 'user3']); + }); + + it('should handle empty ssoSubIds string', () => { + const legacyConfig = { + local: { sandbox: 'no' }, + api: { extraOrigins: '' }, + remote: { ssoSubIds: '' }, + }; + + const result = service.convertLegacyConfig(legacyConfig); + + expect(result.ssoSubIds).toEqual([]); + }); + + it('should handle undefined config sections', () => { + const legacyConfig = {}; + + const result = service.convertLegacyConfig(legacyConfig); + + expect(result.sandbox).toBe(false); + expect(result.extraOrigins).toEqual([]); + expect(result.ssoSubIds).toEqual([]); + }); + + it('should handle complete migration with all fields', () => { + const legacyConfig = { + local: { sandbox: 'yes' }, + api: { extraOrigins: 'https://app1.example.com,https://app2.example.com' }, + remote: { ssoSubIds: 'sub1,sub2,sub3' }, + }; + + const result = service.convertLegacyConfig(legacyConfig); + + expect(result.sandbox).toBe(true); + expect(result.extraOrigins).toEqual([ + 'https://app1.example.com', + 'https://app2.example.com', + ]); + expect(result.ssoSubIds).toEqual(['sub1', 'sub2', 'sub3']); + }); + }); +}); diff --git a/packages/unraid-api-plugin-connect/src/model/connect-config.model.ts b/packages/unraid-api-plugin-connect/src/model/connect-config.model.ts index 4083a9072..fdbb33bf9 100644 --- a/packages/unraid-api-plugin-connect/src/model/connect-config.model.ts +++ b/packages/unraid-api-plugin-connect/src/model/connect-config.model.ts @@ -92,14 +92,6 @@ export class MyServersConfig { @IsEnum(DynamicRemoteAccessType) dynamicRemoteAccessType!: DynamicRemoteAccessType; - @Field(() => [String]) - @IsArray() - @Matches(/^[a-zA-Z0-9-]+$/, { - each: true, - message: 'Each SSO ID must be alphanumeric with dashes', - }) - ssoSubIds!: string[]; - // Connection Status // @Field(() => MinigraphStatus) // @IsEnum(MinigraphStatus) @@ -223,7 +215,6 @@ export const emptyMyServersConfig = (): MyServersConfig => ({ idtoken: '', refreshtoken: '', dynamicRemoteAccessType: DynamicRemoteAccessType.DISABLED, - ssoSubIds: [], }); export const configFeature = registerAs('connect', () => ({ diff --git a/packages/unraid-api-plugin-connect/src/service/config.persistence.ts b/packages/unraid-api-plugin-connect/src/service/config.persistence.ts index f5ede7c27..51a4dcd26 100644 --- a/packages/unraid-api-plugin-connect/src/service/config.persistence.ts +++ b/packages/unraid-api-plugin-connect/src/service/config.persistence.ts @@ -4,7 +4,6 @@ import { existsSync, readFileSync } from 'fs'; import { writeFile } from 'fs/promises'; import path from 'path'; -import { csvStringToArray } from '@unraid/shared/util/data.js'; import { plainToInstance } from 'class-transformer'; import { validateOrReject } from 'class-validator'; import { parse as parseIni } from 'ini'; @@ -137,25 +136,30 @@ export class ConnectConfigPersister implements OnModuleInit, OnModuleDestroy { * @throws {Error} - If the legacy config file does not exist. * @throws {Error} - If the legacy config file is not parse-able. */ - private async migrateLegacyConfig() { - const legacyConfig = await this.parseLegacyConfig(); - this.configService.set('connect.config', legacyConfig); + private async migrateLegacyConfig(filePath?: string) { + const myServersCfgFile = await this.readLegacyConfig(filePath); + const legacyConfig = this.parseLegacyConfig(myServersCfgFile); + const newConfig = await this.convertLegacyConfig(legacyConfig); + this.configService.set('connect.config', newConfig); } /** - * Parse the legacy config file and return a new config object. + * Transform the legacy config object to the new config format. * @param filePath - The path to the legacy config file. * @returns A new config object. * @throws {Error} - If the legacy config file does not exist. * @throws {Error} - If the legacy config file is not parse-able. */ - private async parseLegacyConfig(filePath?: string): Promise { - const config = await this.getLegacyConfig(filePath); + public async convertLegacyConfig(config:LegacyConfig): Promise { return this.validate({ ...config.api, ...config.local, ...config.remote, - extraOrigins: csvStringToArray(config.api.extraOrigins), + // Convert string yes/no to boolean + wanaccess: config.remote.wanaccess === 'yes', + upnpEnabled: config.remote.upnpEnabled === 'yes', + // Convert string port to number + wanport: config.remote.wanport ? parseInt(config.remote.wanport, 10) : 0, }); } @@ -166,7 +170,7 @@ export class ConnectConfigPersister implements OnModuleInit, OnModuleDestroy { * @throws {Error} - If the legacy config file does not exist. * @throws {Error} - If the legacy config file is not parse-able. */ - private async getLegacyConfig(filePath?: string) { + private async readLegacyConfig(filePath?: string) { filePath ??= this.configService.get( 'PATHS_MY_SERVERS_CONFIG', '/boot/config/plugins/dynamix.my.servers/myservers.cfg' @@ -177,6 +181,10 @@ export class ConnectConfigPersister implements OnModuleInit, OnModuleDestroy { if (!existsSync(filePath)) { throw new Error(`Legacy config file does not exist: ${filePath}`); } - return parseIni(readFileSync(filePath, 'utf8')) as LegacyConfig; + return readFileSync(filePath, 'utf8'); + } + + public parseLegacyConfig(iniFileContent: string): LegacyConfig { + return parseIni(iniFileContent) as LegacyConfig; } } diff --git a/packages/unraid-api-plugin-connect/src/service/connect-config.service.ts b/packages/unraid-api-plugin-connect/src/service/connect-config.service.ts index 8df421309..8285098c0 100644 --- a/packages/unraid-api-plugin-connect/src/service/connect-config.service.ts +++ b/packages/unraid-api-plugin-connect/src/service/connect-config.service.ts @@ -42,7 +42,7 @@ export class ConnectConfigService { */ resetUser() { // overwrite identity fields, but retain destructured fields - const { wanaccess, wanport, upnpEnabled, ssoSubIds, ...identity } = emptyMyServersConfig(); + const { wanaccess, wanport, upnpEnabled, ...identity } = emptyMyServersConfig(); this.configService.set(this.configKey, { ...this.getConfig(), ...identity, diff --git a/packages/unraid-api-plugin-connect/src/test/config.persistence.test.ts b/packages/unraid-api-plugin-connect/src/test/config.persistence.test.ts new file mode 100644 index 000000000..3fcba266c --- /dev/null +++ b/packages/unraid-api-plugin-connect/src/test/config.persistence.test.ts @@ -0,0 +1,333 @@ +import { ConfigService } from '@nestjs/config'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { ConnectConfigPersister } from '../service/config.persistence.js'; +import { ConfigType } from '../model/connect-config.model.js'; + +describe('ConnectConfigPersister', () => { + let service: ConnectConfigPersister; + let configService: ConfigService; + + beforeEach(() => { + configService = { + getOrThrow: vi.fn(), + get: vi.fn(), + set: vi.fn(), + changes$: { + pipe: vi.fn(() => ({ + subscribe: vi.fn(), + })), + }, + } as any; + + service = new ConnectConfigPersister(configService); + }); + + describe('parseLegacyConfig', () => { + it('should parse INI format legacy config correctly', () => { + const iniContent = ` +[api] +version="4.8.0+9485809" +extraOrigins="https://example1.com,https://example2.com" +[local] +sandbox="no" +[remote] +wanaccess="yes" +wanport="3333" +upnpEnabled="no" +apikey="unraid_test_key" +localApiKey="test_local_key" +email="test@example.com" +username="testuser" +avatar="" +regWizTime="" +accesstoken="" +idtoken="" +refreshtoken="" +dynamicRemoteAccessType="DISABLED" +ssoSubIds="user1,user2" + `.trim(); + + const result = service.parseLegacyConfig(iniContent); + + expect(result.api.version).toBe('4.8.0+9485809'); + expect(result.api.extraOrigins).toBe('https://example1.com,https://example2.com'); + expect(result.local.sandbox).toBe('no'); + expect(result.remote.wanaccess).toBe('yes'); + expect(result.remote.wanport).toBe('3333'); + expect(result.remote.upnpEnabled).toBe('no'); + expect(result.remote.ssoSubIds).toBe('user1,user2'); + }); + }); + + describe('convertLegacyConfig', () => { + it('should migrate wanaccess from string "yes" to boolean true', async () => { + const legacyConfig = { + api: { version: '4.8.0+9485809', extraOrigins: '' }, + local: { sandbox: 'no' }, + remote: { + wanaccess: 'yes', + wanport: '3333', + upnpEnabled: 'no', + apikey: 'unraid_test_key', + localApiKey: 'test_local_key', + email: 'test@example.com', + username: 'testuser', + avatar: '', + regWizTime: '', + accesstoken: '', + idtoken: '', + refreshtoken: '', + dynamicRemoteAccessType: 'DISABLED', + ssoSubIds: '' + } + } as any; + + const result = await service.convertLegacyConfig(legacyConfig); + + expect(result.wanaccess).toBe(true); + }); + + it('should migrate wanaccess from string "no" to boolean false', async () => { + const legacyConfig = { + api: { version: '4.8.0+9485809', extraOrigins: '' }, + local: { sandbox: 'no' }, + remote: { + wanaccess: 'no', + wanport: '3333', + upnpEnabled: 'no', + apikey: 'unraid_test_key', + localApiKey: 'test_local_key', + email: 'test@example.com', + username: 'testuser', + avatar: '', + regWizTime: '', + accesstoken: '', + idtoken: '', + refreshtoken: '', + dynamicRemoteAccessType: 'DISABLED', + ssoSubIds: '' + } + } as any; + + const result = await service.convertLegacyConfig(legacyConfig); + + expect(result.wanaccess).toBe(false); + }); + + it('should migrate wanport from string to number', async () => { + const legacyConfig = { + api: { version: '4.8.0+9485809', extraOrigins: '' }, + local: { sandbox: 'no' }, + remote: { + wanaccess: 'yes', + wanport: '8080', + upnpEnabled: 'no', + apikey: 'unraid_test_key', + localApiKey: 'test_local_key', + email: 'test@example.com', + username: 'testuser', + avatar: '', + regWizTime: '', + accesstoken: '', + idtoken: '', + refreshtoken: '', + dynamicRemoteAccessType: 'DISABLED', + ssoSubIds: '' + } + } as any; + + const result = await service.convertLegacyConfig(legacyConfig); + + expect(result.wanport).toBe(8080); + expect(typeof result.wanport).toBe('number'); + }); + + it('should migrate upnpEnabled from string "yes" to boolean true', async () => { + const legacyConfig = { + api: { version: '4.8.0+9485809', extraOrigins: '' }, + local: { sandbox: 'no' }, + remote: { + wanaccess: 'yes', + wanport: '3333', + upnpEnabled: 'yes', + apikey: 'unraid_test_key', + localApiKey: 'test_local_key', + email: 'test@example.com', + username: 'testuser', + avatar: '', + regWizTime: '', + accesstoken: '', + idtoken: '', + refreshtoken: '', + dynamicRemoteAccessType: 'DISABLED', + ssoSubIds: '' + } + } as any; + + const result = await service.convertLegacyConfig(legacyConfig); + + expect(result.upnpEnabled).toBe(true); + }); + + it('should migrate upnpEnabled from string "no" to boolean false', async () => { + const legacyConfig = { + api: { version: '4.8.0+9485809', extraOrigins: '' }, + local: { sandbox: 'no' }, + remote: { + wanaccess: 'yes', + wanport: '3333', + upnpEnabled: 'no', + apikey: 'unraid_test_key', + localApiKey: 'test_local_key', + email: 'test@example.com', + username: 'testuser', + avatar: '', + regWizTime: '', + accesstoken: '', + idtoken: '', + refreshtoken: '', + dynamicRemoteAccessType: 'DISABLED', + ssoSubIds: '' + } + } as any; + + const result = await service.convertLegacyConfig(legacyConfig); + + expect(result.upnpEnabled).toBe(false); + }); + + it('should migrate signed in user information correctly', async () => { + const legacyConfig = { + api: { version: '4.8.0+9485809', extraOrigins: '' }, + local: { sandbox: 'no' }, + remote: { + wanaccess: 'yes', + wanport: '3333', + upnpEnabled: 'no', + apikey: 'unraid_sfHboeSNzTzx24816QBssqi0A3nIT0f4Xg4c9Ht49WQfQKLMojU81Sb3f', + localApiKey: '101d204832d24fc7e5d387f6fce47067ba230f8aa0ac3bcc6c12a415aa27dbd9', + email: 'pujitm2009@gmail.com', + username: 'pujitm2009@gmail.com', + avatar: '', + regWizTime: '', + accesstoken: '', + idtoken: '', + refreshtoken: '', + dynamicRemoteAccessType: 'DISABLED', + ssoSubIds: '' + } + } as any; + + const result = await service.convertLegacyConfig(legacyConfig); + + expect(result.apikey).toBe('unraid_sfHboeSNzTzx24816QBssqi0A3nIT0f4Xg4c9Ht49WQfQKLMojU81Sb3f'); + expect(result.localApiKey).toBe('101d204832d24fc7e5d387f6fce47067ba230f8aa0ac3bcc6c12a415aa27dbd9'); + expect(result.email).toBe('pujitm2009@gmail.com'); + expect(result.username).toBe('pujitm2009@gmail.com'); + expect(result.avatar).toBe(''); + }); + + + + it('should merge all sections (api, local, remote) into single config object', async () => { + const legacyConfig = { + api: { version: '4.8.0+9485809', extraOrigins: 'https://example.com' }, + local: { sandbox: 'yes' }, + remote: { + wanaccess: 'yes', + wanport: '8080', + upnpEnabled: 'yes', + apikey: 'test_api_key', + localApiKey: 'test_local_key', + email: 'user@test.com', + username: 'testuser', + avatar: 'https://avatar.url', + regWizTime: '2023-01-01T00:00:00Z', + accesstoken: 'access_token_value', + idtoken: 'id_token_value', + refreshtoken: 'refresh_token_value', + dynamicRemoteAccessType: 'UPNP', + ssoSubIds: 'sub1,sub2' + } + } as any; + + const result = await service.convertLegacyConfig(legacyConfig); + + expect(result.wanaccess).toBe(true); + expect(result.wanport).toBe(8080); + expect(result.upnpEnabled).toBe(true); + expect(result.apikey).toBe('test_api_key'); + expect(result.localApiKey).toBe('test_local_key'); + expect(result.email).toBe('user@test.com'); + expect(result.username).toBe('testuser'); + expect(result.avatar).toBe('https://avatar.url'); + expect(result.regWizTime).toBe('2023-01-01T00:00:00Z'); + expect(result.accesstoken).toBe('access_token_value'); + expect(result.idtoken).toBe('id_token_value'); + expect(result.refreshtoken).toBe('refresh_token_value'); + expect(result.dynamicRemoteAccessType).toBe('UPNP'); + }); + + it('should validate the migrated config and reject invalid email', async () => { + const legacyConfig = { + api: { version: '4.8.0+9485809', extraOrigins: '' }, + local: { sandbox: 'no' }, + remote: { + wanaccess: 'yes', + wanport: '3333', + upnpEnabled: 'no', + apikey: 'unraid_test_key', + localApiKey: 'test_local_key', + email: 'invalid-email', + username: 'testuser', + avatar: '', + regWizTime: '', + accesstoken: '', + idtoken: '', + refreshtoken: '', + dynamicRemoteAccessType: 'DISABLED', + ssoSubIds: '' + } + } as any; + + await expect(service.convertLegacyConfig(legacyConfig)).rejects.toThrow(); + }); + + it('should handle integration of parsing and conversion together', async () => { + const iniContent = ` +[api] +version="4.8.0+9485809" +extraOrigins="https://example.com" +[local] +sandbox="yes" +[remote] +wanaccess="yes" +wanport="8080" +upnpEnabled="yes" +apikey="test_api_key" +localApiKey="test_local_key" +email="user@test.com" +username="testuser" +avatar="https://avatar.url" +regWizTime="2023-01-01T00:00:00Z" +accesstoken="access_token_value" +idtoken="id_token_value" +refreshtoken="refresh_token_value" +dynamicRemoteAccessType="UPNP" +ssoSubIds="sub1,sub2" + `.trim(); + + // Parse the INI content + const legacyConfig = service.parseLegacyConfig(iniContent); + + // Convert to new format + const result = await service.convertLegacyConfig(legacyConfig); + + // Verify the end-to-end conversion (extraOrigins and ssoSubIds are now handled by API config) + expect(result.wanaccess).toBe(true); + expect(result.wanport).toBe(8080); + expect(result.upnpEnabled).toBe(true); + }); + }); +}); \ No newline at end of file