mirror of
https://github.com/unraid/api.git
synced 2025-12-31 13:39:52 -06:00
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:
@@ -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([
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user