feat: move api key fetching to use api key service (#1439)

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **Refactor**
- Simplified and streamlined the management of the local Connect API
key, renaming it to "ConnectInternal" and removing legacy keys.
- Updated internal logic to directly retrieve or create the Connect API
key without storing or emitting it in configuration or events.
- Replaced custom WebSocket handling with default implementation and
improved asynchronous API key retrieval for client connections.
- Enhanced asynchronous handling for subscription and query execution to
ensure proper client initialization.

- **Chores**
- Removed obsolete methods and test suites related to previous Connect
API key management logic.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Eli Bosley
2025-06-26 16:29:49 -04:00
committed by GitHub
parent af6e56de60
commit 86bea56272
7 changed files with 31 additions and 239 deletions

View File

@@ -199,91 +199,6 @@ describe('ApiKeyService', () => {
});
});
describe('createLocalApiKeyForConnectIfNecessary', () => {
beforeEach(() => {
// Mock config getter
vi.mocked(getters.config).mockReturnValue({
status: FileLoadStatus.LOADED,
remote: {
apikey: 'remote-api-key',
localApiKey: null,
},
} as any);
// Mock store dispatch
vi.mocked(store.dispatch).mockResolvedValue({} as any);
});
it('should not create key if config is not loaded', async () => {
vi.mocked(getters.config).mockReturnValue({
status: FileLoadStatus.UNLOADED,
} as any);
await apiKeyService['createLocalApiKeyForConnectIfNecessary']();
expect(mockLogger.error).toHaveBeenCalledWith(
'Config file not loaded, cannot create local API key'
);
expect(store.dispatch).not.toHaveBeenCalled();
});
it('should not create key if remote apikey is not set', async () => {
vi.mocked(getters.config).mockReturnValue({
status: FileLoadStatus.LOADED,
remote: {
apikey: null,
localApiKey: null,
},
} as any);
await apiKeyService['createLocalApiKeyForConnectIfNecessary']();
expect(store.dispatch).not.toHaveBeenCalled();
});
it('should dispatch to update config if Connect key already exists', async () => {
vi.spyOn(apiKeyService, 'findByField').mockReturnValue(mockApiKeyWithSecret);
await apiKeyService['createLocalApiKeyForConnectIfNecessary']();
expect(store.dispatch).toHaveBeenCalled();
});
it('should create new Connect key and update config', async () => {
vi.spyOn(apiKeyService, 'findByField').mockReturnValue(null);
vi.spyOn(apiKeyService, 'create').mockResolvedValue(mockApiKeyWithSecret);
await apiKeyService['createLocalApiKeyForConnectIfNecessary']();
expect(apiKeyService.create).toHaveBeenCalledWith({
name: 'Connect',
description: 'API key for Connect user',
roles: [Role.CONNECT],
overwrite: true,
});
expect(store.dispatch).toHaveBeenCalledWith(
updateUserConfig({
remote: {
localApiKey: mockApiKeyWithSecret.key,
},
})
);
});
it('should log an error if key creation fails', async () => {
vi.spyOn(apiKeyService, 'findByField').mockReturnValue(null);
vi.spyOn(apiKeyService, 'createLocalConnectApiKey').mockResolvedValue(null);
await expect(apiKeyService['createLocalApiKeyForConnectIfNecessary']()).resolves.toBe(
undefined
);
expect(mockLogger.error).toHaveBeenCalledWith(
'Failed to create local API key - no key returned'
);
expect(store.dispatch).not.toHaveBeenCalled();
});
});
describe('findAll', () => {
it('should return all API keys', async () => {
vi.spyOn(apiKeyService, 'loadAllFromDisk').mockResolvedValue([

View File

@@ -39,7 +39,6 @@ export class ApiKeyService implements OnModuleInit {
async onModuleInit() {
this.memoryApiKeys = await this.loadAllFromDisk();
if (environment.IS_MAIN_PROCESS) {
await this.createLocalApiKeyForConnectIfNecessary();
this.setupWatch();
}
}
@@ -160,42 +159,6 @@ export class ApiKeyService implements OnModuleInit {
return apiKey as ApiKeyWithSecret;
}
private async createLocalApiKeyForConnectIfNecessary(): Promise<void> {
if (!environment.IS_MAIN_PROCESS) {
return;
}
const { remote, status } = getters.config();
if (status !== FileLoadStatus.LOADED) {
this.logger.error('Config file not loaded, cannot create local API key');
return;
}
if (!remote.apikey) {
return;
}
// If the remote API Key is set and the local key is either not set or not found on disk, create a key
if (!remote.localApiKey || !this.findByKey(remote.localApiKey)) {
const existingKey = this.findByField('name', 'Connect');
if (existingKey) {
this.logger.debug('Found existing Connect key, not set in config, setting');
store.dispatch(setLocalApiKey(existingKey.key));
} else {
this.logger.debug('Creating a new key for Connect');
// Create local API key
const localApiKey = await this.createLocalConnectApiKey();
if (localApiKey?.key) {
store.dispatch(setLocalApiKey(localApiKey.key));
} else {
this.logger.error('Failed to create local API key - no key returned');
}
}
}
}
async loadAllFromDisk(): Promise<ApiKeyWithSecret[]> {
const files = await readdir(this.basePath).catch((error) => {
this.logger.error(`Failed to read API key directory: ${error}`);
@@ -293,20 +256,6 @@ export class ApiKeyService implements OnModuleInit {
Errors: ${JSON.stringify(error.constraints, null, 2)}`);
}
public async createLocalConnectApiKey(): Promise<ApiKeyWithSecret | null> {
try {
return await this.create({
name: 'Connect',
description: 'API key for Connect user',
roles: [Role.CONNECT],
overwrite: true,
});
} catch (err) {
this.logger.error(`Failed to create local API key for Connect user: ${err}`);
return null;
}
}
public async saveApiKey(apiKey: ApiKeyWithSecret): Promise<void> {
try {
const validatedApiKey = await validateObject(ApiKeyWithSecret, apiKey);

View File

@@ -4,6 +4,7 @@ import { MothershipHandler } from '../event-handler/mothership.handler.js';
import { TimeoutCheckerJob } from '../job/timeout-checker.job.js';
import { CloudResolver } from '../resolver/cloud.resolver.js';
import { CloudService } from '../service/cloud.service.js';
import { ConnectApiKeyService } from '../service/connect-api-key.service.js';
import { MothershipConnectionService } from '../service/connection.service.js';
import { MothershipGraphqlClientService } from '../service/graphql.client.js';
import { InternalClientService } from '../service/internal.client.js';
@@ -13,6 +14,7 @@ import { RemoteAccessModule } from './remote-access.module.js';
@Module({
imports: [RemoteAccessModule],
providers: [
ConnectApiKeyService,
MothershipConnectionService,
MothershipGraphqlClientService,
InternalClientService,

View File

@@ -1,25 +1,20 @@
import { Inject, Injectable, Logger } from '@nestjs/common';
import { ConfigService } from '@nestjs/config';
import { ApiKey, ApiKeyWithSecret, Permission, Resource, Role } from '@unraid/shared/graphql.model.js';
import { ApiKey, ApiKeyWithSecret, Permission, Role } from '@unraid/shared/graphql.model.js';
import { ApiKeyService } from '@unraid/shared/services/api-key.js';
import { API_KEY_SERVICE_TOKEN } from '@unraid/shared/tokens.js';
import { AuthActionVerb } from 'nest-authz';
import { ConnectConfigService } from './connect-config.service.js';
@Injectable()
export class ConnectApiKeyService implements ApiKeyService {
private readonly logger = new Logger(ConnectApiKeyService.name);
private static readonly validRoles: Set<Role> = new Set(Object.values(Role));
private static readonly CONNECT_API_KEY_NAME = 'Connect';
private static readonly CONNECT_API_KEY_DESCRIPTION = 'Internal API Key Used By Unraid Connect to access your server resources for the connect.myunraid.net dashboard';
private 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,
private readonly configService: ConfigService,
private readonly connectConfig: ConnectConfigService
) {}
async findById(id: string): Promise<ApiKey | null> {
@@ -75,12 +70,10 @@ export class ConnectApiKeyService implements ApiKeyService {
try {
return await this.create({
name: ConnectApiKeyService.CONNECT_API_KEY_NAME,
description: 'API key for Connect user',
description: ConnectApiKeyService.CONNECT_API_KEY_DESCRIPTION,
roles: [Role.CONNECT],
overwrite: true,
});
// Delete all other API keys with the role CONNECT
} catch (err) {
this.logger.error(`Failed to create local API key for Connect user: ${err}`);
return null;
@@ -91,57 +84,25 @@ export class ConnectApiKeyService implements ApiKeyService {
* Gets or creates a local API key for Connect
*/
public async getOrCreateLocalApiKey(): Promise<string> {
const targetDescription = ConnectApiKeyService.CONNECT_API_KEY_DESCRIPTION;
// 1. Get all API keys first
const allKeys = await this.findAll();
// 2. Check in-memory config and verify key exists
const { localApiKey: localApiKeyFromConfig } = this.connectConfig.getConfig();
if (localApiKeyFromConfig && localApiKeyFromConfig !== '') {
const keyExists = allKeys.some(key => {
const keyWithSecret = this.findByIdWithSecret(key.id);
return keyWithSecret?.key === localApiKeyFromConfig;
});
if (keyExists) {
return localApiKeyFromConfig;
}
const legacyConnectKeys = allKeys.filter((key) => key.name === 'Connect');
if (legacyConnectKeys.length > 0) {
await this.deleteApiKeys(legacyConnectKeys.map((key) => key.id));
this.logger.log(`Deleted legacy Connect API keys`);
}
// 3. Filter by name "Connect"
const connectKeys = allKeys.filter(key => key.name === ConnectApiKeyService.CONNECT_API_KEY_NAME);
// 4. Find keys with correct description vs incorrect description
const correctKeys = connectKeys.filter(key => key.description === targetDescription);
const incorrectKeys = connectKeys.filter(key => key.description !== targetDescription);
// 5. Delete keys with incorrect description
if (incorrectKeys.length > 0) {
const idsToDelete = incorrectKeys.map(key => key.id);
await this.deleteApiKeys(idsToDelete);
this.logger.log(`Deleted ${incorrectKeys.length} Connect API keys with incorrect descriptions`);
const connectKey = this.findByField('name', ConnectApiKeyService.CONNECT_API_KEY_NAME);
if (connectKey) {
return connectKey.key;
}
// 6. If we have a correct key, return it
if (correctKeys.length > 0) {
const correctKeyWithSecret = this.findByIdWithSecret(correctKeys[0].id);
if (correctKeyWithSecret) {
return correctKeyWithSecret.key;
}
}
// 7. Create a new key with the correct description
const localApiKey = await this.create({
name: ConnectApiKeyService.CONNECT_API_KEY_NAME,
description: targetDescription,
roles: [Role.CONNECT],
overwrite: true,
});
const localApiKey = await this.createLocalConnectApiKey();
if (!localApiKey?.key) {
throw new Error('Failed to create local API key');
}
return localApiKey.key;
}
}

View File

@@ -147,21 +147,6 @@ export class ConnectSettingsService {
return restartRequired;
}
private async getOrCreateLocalApiKey() {
const { localApiKey: localApiKeyFromConfig } =
this.configService.getOrThrow<MyServersConfig>('connect.config');
if (localApiKeyFromConfig === '') {
const localApiKey = await this.apiKeyService.createLocalConnectApiKey();
if (!localApiKey?.key) {
throw new GraphQLError('Failed to create local API key', {
extensions: { code: 'INTERNAL_SERVER_ERROR' },
});
}
return localApiKey.key;
}
return localApiKeyFromConfig;
}
async signIn(input: ConnectSignInInput) {
const status = this.configService.get('store.emhttp.status');
if (status === 'LOADED') {
@@ -180,7 +165,8 @@ export class ConnectSettingsService {
}
try {
const localApiKey = await this.getOrCreateLocalApiKey();
// Make sure we have a local API key for Connect
await this.apiKeyService.getOrCreateLocalApiKey();
// Update config with user info
this.configService.set(
@@ -190,7 +176,6 @@ export class ConnectSettingsService {
this.configService.set('connect.config.username', userInfo.preferred_username);
this.configService.set('connect.config.email', userInfo.email);
this.configService.set('connect.config.apikey', input.apiKey);
this.configService.set('connect.config.localApiKey', localApiKey);
// Emit login event
this.eventEmitter.emit(EVENTS.LOGIN, {
@@ -198,7 +183,6 @@ export class ConnectSettingsService {
avatar: typeof userInfo.avatar === 'string' ? userInfo.avatar : '',
email: userInfo.email,
apikey: input.apiKey,
localApiKey,
});
return true;

View File

@@ -8,16 +8,14 @@ import { HttpLink } from '@apollo/client/link/http/index.js';
import { GraphQLWsLink } from '@apollo/client/link/subscriptions/index.js';
import { getMainDefinition } from '@apollo/client/utilities/index.js';
import { createClient } from 'graphql-ws';
import { WebSocket } from 'ws';
import { MyServersConfig } from '../model/connect-config.model.js';
import { MothershipConnectionService } from './connection.service.js';
import { ConnectApiKeyService } from './connect-api-key.service.js';
@Injectable()
export class InternalClientService {
constructor(
private readonly configService: ConfigService,
private readonly connectionService: MothershipConnectionService
private readonly apiKeyService: ConnectApiKeyService
) {}
private PROD_NGINX_PORT = 80;
@@ -57,20 +55,6 @@ export class InternalClientService {
return `${protocol}://127.0.0.1/graphql`;
}
/**
* Create a WebSocket class with Mothership headers
*/
private getWebsocketWithMothershipHeaders() {
const getHeaders = () => this.connectionService.getMothershipWebsocketHeaders();
return class WebsocketWithMothershipHeaders extends WebSocket {
constructor(address: string | URL, protocols?: string | string[]) {
super(address, protocols, {
headers: getHeaders(),
});
}
};
}
private createApiClient({ apiKey }: { apiKey: string }) {
const httpUri = this.getApiAddress('http');
const wsUri = this.getApiAddress('ws');
@@ -88,11 +72,8 @@ export class InternalClientService {
const wsLink = new GraphQLWsLink(
createClient({
webSocketImpl: this.getWebsocketWithMothershipHeaders(),
url: wsUri,
connectionParams: () => {
return { 'x-api-key': apiKey };
},
connectionParams: () => ({ 'x-api-key': apiKey }),
})
);
@@ -127,12 +108,12 @@ export class InternalClientService {
});
}
public getClient() {
public async getClient() {
if (this.client) {
return this.client;
}
const config = this.configService.getOrThrow<MyServersConfig>('connect.config');
this.client = this.createApiClient({ apiKey: config.localApiKey });
const localApiKey = await this.apiKeyService.getOrCreateLocalApiKey();
this.client = this.createApiClient({ apiKey: localApiKey });
return this.client;
}

View File

@@ -85,12 +85,12 @@ export class MothershipSubscriptionHandler {
}
}
public addSubscription({ sha256, body }: SubscriptionProxy) {
public async addSubscription({ sha256, body }: SubscriptionProxy) {
if (this.subscriptions.has(sha256)) {
throw new Error(`Subscription already exists for SHA256: ${sha256}`);
}
const parsedBody = parseGraphQLQuery(body);
const client = this.internalClientService.getClient();
const client = await this.internalClientService.getClient();
const observable = client.subscribe({
query: parsedBody.query,
variables: parsedBody.variables,
@@ -123,7 +123,7 @@ export class MothershipSubscriptionHandler {
}
async executeQuery(sha256: string, body: string) {
const internalClient = this.internalClientService.getClient();
const internalClient = await this.internalClientService.getClient();
const parsedBody = parseGraphQLQuery(body);
const queryInput = {
query: parsedBody.query,