fix(connect): clear persisted keys on sign-out and removal

packages/unraid-api-plugin-connect/src/__test__/connect.config.service.test.ts

@@ -0,0 +1,161 @@
+import { ConfigService } from '@nestjs/config';
+import { EventEmitter2, EventEmitterModule } from '@nestjs/event-emitter';
+import { Test } from '@nestjs/testing';
+import { mkdtemp, readFile, rm, writeFile } from 'node:fs/promises';
+import { tmpdir } from 'node:os';
+import { join } from 'node:path';
+
+import { afterEach, describe, expect, it, vi } from 'vitest';
+
+import { emptyMyServersConfig } from '../config/connect.config.js';
+import { ConnectConfigService } from '../config/connect.config.service.js';
+import { ConnectSettingsResolver } from '../unraid-connect/connect-settings.resolver.js';
+import { ConnectSettingsService } from '../unraid-connect/connect-settings.service.js';
+
+describe('ConnectConfigService sign-out', () => {
+    const temporaryDirectories: string[] = [];
+
+    afterEach(async () => {
+        await Promise.all(
+            temporaryDirectories
+                .splice(0)
+                .map((directory) => rm(directory, { recursive: true, force: true }))
+        );
+    });
+
+    it('persists the cleared identity and removes the legacy source of the key', async () => {
+        const directory = await mkdtemp(join(tmpdir(), 'connect-sign-out-'));
+        temporaryDirectories.push(directory);
+        const legacyPath = join(directory, 'myservers.cfg');
+        await writeFile(legacyPath, '[remote]\napikey="legacy-key"\n');
+        const configService = new ConfigService({
+            connect: {
+                config: {
+                    ...emptyMyServersConfig(),
+                    apikey: 'current-key',
+                    username: 'signed-in-user',
+                    email: 'signed-in@example.com',
+                    wanaccess: true,
+                    wanport: 33443,
+                },
+            },
+            PATHS_CONFIG_MODULES: directory,
+            PATHS_MY_SERVERS_CONFIG: legacyPath,
+        });
+
+        await new ConnectConfigService(configService).resetUser();
+
+        const persisted = JSON.parse(await readFile(join(directory, 'connect.json'), 'utf8'));
+        expect(persisted.apikey).toBe('');
+        expect(persisted.username).toBe('');
+        expect(persisted.email).toBe('');
+        expect(configService.get('connect.config.email')).toBe('');
+        expect(persisted.wanaccess).toBe(true);
+        expect(persisted.wanport).toBe(33443);
+        await expect(readFile(legacyPath)).rejects.toThrow();
+    });
+
+    it('keeps the legacy config when the cleared identity cannot be persisted', async () => {
+        const directory = await mkdtemp(join(tmpdir(), 'connect-sign-out-'));
+        temporaryDirectories.push(directory);
+        const legacyPath = join(directory, 'myservers.cfg');
+        await writeFile(legacyPath, '[remote]\napikey="legacy-key"\n');
+        const blockedPath = join(directory, 'blocked');
+        await writeFile(blockedPath, 'not a directory');
+        const configService = new ConfigService({
+            connect: { config: { ...emptyMyServersConfig(), apikey: 'current-key' } },
+            PATHS_CONFIG_MODULES: blockedPath,
+            PATHS_MY_SERVERS_CONFIG: legacyPath,
+        });
+
+        await expect(new ConnectConfigService(configService).resetUser()).rejects.toThrow();
+        expect(await readFile(legacyPath, 'utf8')).toContain('legacy-key');
+        expect(configService.get('connect.config.apikey')).toBe('current-key');
+    });
+
+    it('finishes disk cleanup before the sign-out mutation succeeds', async () => {
+        const directory = await mkdtemp(join(tmpdir(), 'connect-sign-out-'));
+        temporaryDirectories.push(directory);
+        const legacyPath = join(directory, 'myservers.cfg');
+        await writeFile(legacyPath, '[remote]\napikey="legacy-key"\n');
+        const configService = new ConfigService({
+            connect: { config: { ...emptyMyServersConfig(), apikey: 'current-key' } },
+            PATHS_CONFIG_MODULES: directory,
+            PATHS_MY_SERVERS_CONFIG: legacyPath,
+        });
+        const module = await Test.createTestingModule({
+            imports: [EventEmitterModule.forRoot()],
+            providers: [
+                { provide: ConfigService, useValue: configService },
+                { provide: ConnectSettingsService, useValue: {} },
+                {
+                    provide: ConnectConfigService,
+                    useFactory: () => new ConnectConfigService(configService),
+                },
+                {
+                    provide: ConnectSettingsResolver,
+                    useFactory: (
+                        settings: ConnectSettingsService,
+                        emitter: EventEmitter2,
+                        connectConfig: ConnectConfigService
+                    ) => new ConnectSettingsResolver(settings, emitter, connectConfig),
+                    inject: [ConnectSettingsService, EventEmitter2, ConnectConfigService],
+                },
+            ],
+        }).compile();
+        await module.init();
+
+        try {
+            const resetUser = vi.spyOn(module.get(ConnectConfigService), 'resetUser');
+            await expect(module.get(ConnectSettingsResolver).connectSignOut()).resolves.toBe(true);
+            expect(resetUser).toHaveBeenCalledTimes(1);
+            const persisted = JSON.parse(await readFile(join(directory, 'connect.json'), 'utf8'));
+            expect(persisted.apikey).toBe('');
+            await expect(readFile(legacyPath)).rejects.toThrow();
+        } finally {
+            await module.close();
+        }
+    });
+
+    it('rejects the sign-out mutation when disk cleanup fails', async () => {
+        const directory = await mkdtemp(join(tmpdir(), 'connect-sign-out-'));
+        temporaryDirectories.push(directory);
+        const legacyPath = join(directory, 'myservers.cfg');
+        await writeFile(legacyPath, '[remote]\napikey="legacy-key"\n');
+        const blockedPath = join(directory, 'blocked');
+        await writeFile(blockedPath, 'not a directory');
+        const configService = new ConfigService({
+            connect: { config: { ...emptyMyServersConfig(), apikey: 'current-key' } },
+            PATHS_CONFIG_MODULES: blockedPath,
+            PATHS_MY_SERVERS_CONFIG: legacyPath,
+        });
+        const module = await Test.createTestingModule({
+            imports: [EventEmitterModule.forRoot()],
+            providers: [
+                { provide: ConfigService, useValue: configService },
+                { provide: ConnectSettingsService, useValue: {} },
+                {
+                    provide: ConnectConfigService,
+                    useFactory: () => new ConnectConfigService(configService),
+                },
+                {
+                    provide: ConnectSettingsResolver,
+                    useFactory: (
+                        settings: ConnectSettingsService,
+                        emitter: EventEmitter2,
+                        connectConfig: ConnectConfigService
+                    ) => new ConnectSettingsResolver(settings, emitter, connectConfig),
+                    inject: [ConnectSettingsService, EventEmitter2, ConnectConfigService],
+                },
+            ],
+        }).compile();
+        await module.init();
+
+        try {
+            await expect(module.get(ConnectSettingsResolver).connectSignOut()).rejects.toThrow();
+            expect(await readFile(legacyPath, 'utf8')).toContain('legacy-key');
+        } finally {
+            await module.close();
+        }
+    });
+});

packages/unraid-api-plugin-connect/src/config/connect.config.service.ts

@@ -1,8 +1,12 @@
 import { Injectable, Logger } from '@nestjs/common';
 import { ConfigService } from '@nestjs/config';
 import { OnEvent } from '@nestjs/event-emitter';
+import { unlink } from 'node:fs/promises';
+
+import { isMatch } from 'lodash-es';
 
 import { EVENTS } from '../helper/nest-tokens.js';
+import { ConnectConfigPersister } from './config.persistence.js';
 import { ConfigType, emptyMyServersConfig, MyServersConfig } from './connect.config.js';
 
 @Injectable()
@@ -40,18 +44,39 @@ export class ConnectConfigService {
      * This is used when the user logs out.
      * It retains the existing config, but resets identity-related fields.
      */
-    resetUser() {
+    async resetUser() {
         // overwrite identity fields, but retain destructured fields
-        const { wanaccess, wanport, upnpEnabled, ...identity } = emptyMyServersConfig();
-        this.configService.set(this.configKey, {
+        const { wanaccess, wanport, upnpEnabled, ...identity } = {
+            ...emptyMyServersConfig(),
+            email: '',
+        };
+        const config = {
             ...this.getConfig(),
             ...identity,
-        });
+        };
+        const persister = new ConnectConfigPersister(this.configService);
+        await persister.persist(config);
+        const savedConfig = await persister.getFileHandler().readConfigFile();
+        if (!isMatch(savedConfig, identity)) {
+            throw new Error('Connect sign-out could not persist cleared identity');
+        }
+
+        const legacyPath = this.configService.get<string>(
+            'PATHS_MY_SERVERS_CONFIG',
+            '/boot/config/plugins/dynamix.my.servers/myservers.cfg'
+        );
+        if (legacyPath) {
+            await unlink(legacyPath).catch((error: NodeJS.ErrnoException) => {
+                if (error.code !== 'ENOENT') throw error;
+            });
+        }
+        this.configService.set(this.configKey, config);
         this.logger.verbose('Reset Connect user identity');
     }
 
     @OnEvent(EVENTS.LOGOUT, { async: true })
-    async onLogout() {
-        this.resetUser();
+    async onLogout({ configResetCompleted = false }: { configResetCompleted?: boolean } = {}) {
+        if (configResetCompleted) return;
+        await this.resetUser();
     }
 }

packages/unraid-api-plugin-connect/src/unraid-connect/connect-settings.resolver.ts

@@ -9,6 +9,7 @@ import { PrefixedID } from '@unraid/shared/prefixed-id-scalar.js';
 import { UsePermissions } from '@unraid/shared/use-permissions.directive.js';
 import { GraphQLJSON } from 'graphql-scalars';
 
+import { ConnectConfigService } from '../config/connect.config.service.js';
 import { EVENTS } from '../helper/nest-tokens.js';
 import { ConnectSettingsService } from './connect-settings.service.js';
 import {
@@ -28,7 +29,8 @@ export class ConnectSettingsResolver {
 
     constructor(
         private readonly connectSettingsService: ConnectSettingsService,
-        private readonly eventEmitter: EventEmitter2
+        private readonly eventEmitter: EventEmitter2,
+        private readonly connectConfigService: ConnectConfigService
     ) {}
 
     @UsePermissions({
@@ -118,7 +120,11 @@ export class ConnectSettingsResolver {
         resource: Resource.CONNECT,
     })
     public async connectSignOut() {
-        this.eventEmitter.emit(EVENTS.LOGOUT, { reason: 'Manual Sign Out Using API' });
+        await this.connectConfigService.resetUser();
+        await this.eventEmitter.emitAsync(EVENTS.LOGOUT, {
+            reason: 'Manual Sign Out Using API',
+            configResetCompleted: true,
+        });
         return true;
     }
 

plugin/plugins/dynamix.unraid.net.plg

@@ -267,6 +267,35 @@ exit 0
       MAINNAME="&name;"
       <![CDATA[
       echo "Removing Plugin"
+
+      # The packaged cleanup script is not invoked by the plugin remove hook.
+      # Stop the API before deleting its Connect configuration so a pending
+      # persistence write cannot recreate the credentials on flash.
+      if [ -f /boot/config/plugins/dynamix.my.servers/configs/connect.json ] || \
+         [ -f /boot/config/plugins/dynamix.my.servers/myservers.cfg ]; then
+        if [ -x /etc/rc.d/rc.unraid-api ]; then
+          /etc/rc.d/rc.unraid-api stop || true
+        elif command -v unraid-api >/dev/null 2>&1; then
+          unraid-api stop || true
+        fi
+
+        # `unraid-api status` reports the PM2 app even when its worker command
+        # does not contain the application script path. An empty successful
+        # status is the stopped state on Unraid 7.
+        if ! api_status=$(unraid-api status 2>&1) || [ -n "$api_status" ]; then
+          echo "Error: Could not verify Unraid API stopped; Connect credentials were not removed"
+          exit 1
+        fi
+
+        rm -f /boot/config/plugins/dynamix.my.servers/configs/connect.json \
+              /boot/config/plugins/dynamix.my.servers/myservers.cfg || exit 1
+        if [ -x /etc/rc.d/rc.nginx ]; then
+          /etc/rc.d/rc.nginx reload || {
+            echo "Error: Could not reload nginx after clearing Connect credentials"
+            exit 1
+          }
+        fi
+      fi
 
       # Check Unraid version
       UNRAID_VERSION=""
@@ -296,6 +325,9 @@ exit 0
         echo "Unraid 7.2+ detected. Using safe removal method."
         if ! /etc/rc.d/rc.unraid-api plugins remove unraid-api-plugin-connect -b; then
           echo "Warning: Failed to remove connect API plugin"
+          if ! /etc/rc.d/rc.unraid-api start; then
+            echo "Warning: Failed to restart Unraid API after Connect plugin removal failure"
+          fi
         fi
 
         # Send notification to user