fix: api key creation cli

api/dev/configs/api.json

@@ -1,5 +1,5 @@
 {
-  "version": "4.15.1",
+  "version": "4.17.0",
   "extraOrigins": [],
   "sandbox": true,
   "ssoSubIds": [],

api/src/unraid-api/auth/api-key.service.spec.ts

@@ -681,4 +681,104 @@ describe('ApiKeyService', () => {
             ]);
         });
     });
+
+    describe('convertRolesStringArrayToRoles', () => {
+        beforeEach(async () => {
+            vi.mocked(getters.paths).mockReturnValue({
+                'auth-keys': mockBasePath,
+            } as ReturnType<typeof getters.paths>);
+
+            // Create a fresh mock logger for each test
+            mockLogger = {
+                log: vi.fn(),
+                error: vi.fn(),
+                warn: vi.fn(),
+                debug: vi.fn(),
+                verbose: vi.fn(),
+            };
+
+            apiKeyService = new ApiKeyService();
+            // Replace the logger with our mock
+            (apiKeyService as any).logger = mockLogger;
+        });
+
+        it('should convert uppercase role strings to Role enum values', () => {
+            const roles = ['ADMIN', 'CONNECT', 'VIEWER'];
+            const result = apiKeyService.convertRolesStringArrayToRoles(roles);
+
+            expect(result).toEqual([Role.ADMIN, Role.CONNECT, Role.VIEWER]);
+        });
+
+        it('should convert lowercase role strings to Role enum values', () => {
+            const roles = ['admin', 'connect', 'guest'];
+            const result = apiKeyService.convertRolesStringArrayToRoles(roles);
+
+            expect(result).toEqual([Role.ADMIN, Role.CONNECT, Role.GUEST]);
+        });
+
+        it('should convert mixed case role strings to Role enum values', () => {
+            const roles = ['Admin', 'CoNnEcT', 'ViEwEr'];
+            const result = apiKeyService.convertRolesStringArrayToRoles(roles);
+
+            expect(result).toEqual([Role.ADMIN, Role.CONNECT, Role.VIEWER]);
+        });
+
+        it('should handle roles with whitespace', () => {
+            const roles = [' ADMIN ', '  CONNECT  ', 'VIEWER  '];
+            const result = apiKeyService.convertRolesStringArrayToRoles(roles);
+
+            expect(result).toEqual([Role.ADMIN, Role.CONNECT, Role.VIEWER]);
+        });
+
+        it('should filter out invalid roles and warn', () => {
+            const roles = ['ADMIN', 'INVALID_ROLE', 'VIEWER', 'ANOTHER_INVALID'];
+            const result = apiKeyService.convertRolesStringArrayToRoles(roles);
+
+            expect(result).toEqual([Role.ADMIN, Role.VIEWER]);
+            expect(mockLogger.warn).toHaveBeenCalledWith(
+                'Ignoring invalid roles: INVALID_ROLE, ANOTHER_INVALID'
+            );
+        });
+
+        it('should return empty array when all roles are invalid', () => {
+            const roles = ['INVALID1', 'INVALID2', 'INVALID3'];
+            const result = apiKeyService.convertRolesStringArrayToRoles(roles);
+
+            expect(result).toEqual([]);
+            expect(mockLogger.warn).toHaveBeenCalledWith(
+                'Ignoring invalid roles: INVALID1, INVALID2, INVALID3'
+            );
+        });
+
+        it('should return empty array for empty input', () => {
+            const result = apiKeyService.convertRolesStringArrayToRoles([]);
+
+            expect(result).toEqual([]);
+            expect(mockLogger.warn).not.toHaveBeenCalled();
+        });
+
+        it('should handle all valid Role enum values', () => {
+            const roles = Object.values(Role);
+            const result = apiKeyService.convertRolesStringArrayToRoles(roles);
+
+            expect(result).toEqual(Object.values(Role));
+            expect(mockLogger.warn).not.toHaveBeenCalled();
+        });
+
+        it('should deduplicate roles', () => {
+            const roles = ['ADMIN', 'admin', 'ADMIN', 'VIEWER', 'viewer'];
+            const result = apiKeyService.convertRolesStringArrayToRoles(roles);
+
+            // Note: Current implementation doesn't deduplicate, but this test documents the behavior
+            expect(result).toEqual([Role.ADMIN, Role.ADMIN, Role.ADMIN, Role.VIEWER, Role.VIEWER]);
+        });
+
+        it('should handle mixed valid and invalid roles correctly', () => {
+            const roles = ['ADMIN', 'invalid', 'CONNECT', 'bad_role', 'GUEST', 'VIEWER'];
+            const result = apiKeyService.convertRolesStringArrayToRoles(roles);
+
+            expect(result).toEqual([Role.ADMIN, Role.CONNECT, Role.GUEST, Role.VIEWER]);
+            expect(mockLogger.warn).toHaveBeenCalledWith('Ignoring invalid roles: invalid, bad_role');
+        });
+    });
 });

api/src/unraid-api/auth/api-key.service.ts

@@ -110,9 +110,25 @@ export class ApiKeyService implements OnModuleInit {
     }
 
     public convertRolesStringArrayToRoles(roles: string[]): Role[] {
-        return roles
-            .map((roleStr) => Role[roleStr.trim().toUpperCase() as keyof typeof Role])
-            .filter(Boolean);
+        const validRoles: Role[] = [];
+        const invalidRoles: string[] = [];
+
+        for (const roleStr of roles) {
+            const upperRole = roleStr.trim().toUpperCase();
+            const role = Role[upperRole as keyof typeof Role];
+
+            if (role && ApiKeyService.validRoles.has(role)) {
+                validRoles.push(role);
+            } else {
+                invalidRoles.push(roleStr);
+            }
+        }
+
+        if (invalidRoles.length > 0) {
+            this.logger.warn(`Ignoring invalid roles: ${invalidRoles.join(', ')}`);
+        }
+
+        return validRoles;
     }
 
     async create({

api/src/unraid-api/cli/__test__/api-key.command.test.ts

@@ -0,0 +1,192 @@
+import { Test, TestingModule } from '@nestjs/testing';
+
+import { InquirerService } from 'nest-commander';
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+
+import { ApiKeyService } from '@app/unraid-api/auth/api-key.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';
+import { LogService } from '@app/unraid-api/cli/log.service.js';
+
+describe('ApiKeyCommand', () => {
+    let command: ApiKeyCommand;
+    let apiKeyService: ApiKeyService;
+    let logService: LogService;
+    let inquirerService: InquirerService;
+    let questionSet: AddApiKeyQuestionSet;
+
+    beforeEach(async () => {
+        const module: TestingModule = await Test.createTestingModule({
+            providers: [
+                ApiKeyCommand,
+                AddApiKeyQuestionSet,
+                {
+                    provide: ApiKeyService,
+                    useValue: {
+                        findByField: vi.fn(),
+                        create: vi.fn(),
+                        findAll: vi.fn(),
+                        deleteApiKeys: vi.fn(),
+                        convertRolesStringArrayToRoles: vi.fn((roles) => roles),
+                        convertPermissionsStringArrayToPermissions: vi.fn((perms) => perms),
+                        getAllValidPermissions: vi.fn(() => []),
+                    },
+                },
+                {
+                    provide: LogService,
+                    useValue: {
+                        log: vi.fn(),
+                        error: vi.fn(),
+                    },
+                },
+                {
+                    provide: InquirerService,
+                    useValue: {
+                        prompt: vi.fn(),
+                    },
+                },
+            ],
+        }).compile();
+
+        command = module.get<ApiKeyCommand>(ApiKeyCommand);
+        apiKeyService = module.get<ApiKeyService>(ApiKeyService);
+        logService = module.get<LogService>(LogService);
+        inquirerService = module.get<InquirerService>(InquirerService);
+        questionSet = module.get<AddApiKeyQuestionSet>(AddApiKeyQuestionSet);
+    });
+
+    describe('AddApiKeyQuestionSet', () => {
+        describe('shouldAskOverwrite', () => {
+            it('should return true when an API key with the given name exists', () => {
+                vi.mocked(apiKeyService.findByField).mockReturnValue({
+                    key: 'existing-key',
+                    name: 'test-key',
+                    description: 'Test key',
+                    roles: [],
+                    permissions: [],
+                } as any);
+
+                const result = questionSet.shouldAskOverwrite({ name: 'test-key' });
+
+                expect(result).toBe(true);
+                expect(apiKeyService.findByField).toHaveBeenCalledWith('name', 'test-key');
+            });
+
+            it('should return false when no API key with the given name exists', () => {
+                vi.mocked(apiKeyService.findByField).mockReturnValue(null);
+
+                const result = questionSet.shouldAskOverwrite({ name: 'non-existent-key' });
+
+                expect(result).toBe(false);
+                expect(apiKeyService.findByField).toHaveBeenCalledWith('name', 'non-existent-key');
+            });
+        });
+    });
+
+    describe('run', () => {
+        it('should find and return existing key when not creating', async () => {
+            const mockKey = { key: 'test-api-key-123', name: 'test-key' };
+            vi.mocked(apiKeyService.findByField).mockReturnValue(mockKey as any);
+
+            await command.run([], { name: 'test-key', create: false });
+
+            expect(apiKeyService.findByField).toHaveBeenCalledWith('name', 'test-key');
+            expect(logService.log).toHaveBeenCalledWith('test-api-key-123');
+        });
+
+        it('should create new key when key does not exist and create flag is set', async () => {
+            vi.mocked(apiKeyService.findByField).mockReturnValue(null);
+            vi.mocked(apiKeyService.create).mockResolvedValue({ key: 'new-api-key-456' } as any);
+
+            await command.run([], {
+                name: 'new-key',
+                create: true,
+                roles: ['ADMIN'] as any,
+                description: 'Test description',
+            });
+
+            expect(apiKeyService.create).toHaveBeenCalledWith({
+                name: 'new-key',
+                description: 'Test description',
+                roles: ['ADMIN'],
+                permissions: undefined,
+                overwrite: false,
+            });
+            expect(logService.log).toHaveBeenCalledWith('new-api-key-456');
+        });
+
+        it('should error when key exists and overwrite is not set in non-interactive mode', async () => {
+            const mockKey = { key: 'existing-key', name: 'test-key' };
+            vi.mocked(apiKeyService.findByField)
+                .mockReturnValueOnce(null) // First call in line 131
+                .mockReturnValueOnce(mockKey as any); // Second call in non-interactive check
+            const exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => {
+                throw new Error('process.exit');
+            });
+
+            await expect(
+                command.run([], {
+                    name: 'test-key',
+                    create: true,
+                    roles: ['ADMIN'] as any,
+                })
+            ).rejects.toThrow();
+
+            expect(logService.error).toHaveBeenCalledWith(
+                "API key with name 'test-key' already exists. Use --overwrite to replace it."
+            );
+            expect(exitSpy).toHaveBeenCalledWith(1);
+            exitSpy.mockRestore();
+        });
+
+        it('should create key with overwrite when key exists and overwrite is set', async () => {
+            const mockKey = { key: 'existing-key', name: 'test-key' };
+            vi.mocked(apiKeyService.findByField)
+                .mockReturnValueOnce(null) // First call in line 131
+                .mockReturnValueOnce(mockKey as any); // Second call in non-interactive check
+            vi.mocked(apiKeyService.create).mockResolvedValue({ key: 'overwritten-key' } as any);
+
+            await command.run([], {
+                name: 'test-key',
+                create: true,
+                roles: ['ADMIN'] as any,
+                overwrite: true,
+            });
+
+            expect(apiKeyService.create).toHaveBeenCalledWith({
+                name: 'test-key',
+                description: 'CLI generated key: test-key',
+                roles: ['ADMIN'],
+                permissions: undefined,
+                overwrite: true,
+            });
+            expect(logService.log).toHaveBeenCalledWith('overwritten-key');
+        });
+
+        it('should prompt for missing fields when creating without sufficient info', async () => {
+            vi.mocked(apiKeyService.findByField).mockReturnValue(null);
+            vi.mocked(inquirerService.prompt).mockResolvedValue({
+                name: 'prompted-key',
+                roles: ['USER'],
+                permissions: [],
+                description: 'Prompted description',
+                overwrite: false,
+            } as any);
+            vi.mocked(apiKeyService.create).mockResolvedValue({ key: 'prompted-api-key' } as any);
+
+            await command.run([], { name: '', create: true });
+
+            expect(inquirerService.prompt).toHaveBeenCalledWith('add-api-key', {
+                name: '',
+                create: true,
+            });
+            expect(apiKeyService.create).toHaveBeenCalledWith({
+                name: 'prompted-key',
+                description: 'Prompted description',
+                roles: ['USER'],
+                permissions: [],
+                overwrite: false,
+            });
+        });
+    });
+});

api/src/unraid-api/cli/apikey/add-api-key.questions.ts

@@ -39,6 +39,12 @@ export class AddApiKeyQuestionSet {
         return this.apiKeyService.convertRolesStringArrayToRoles(val);
     }
 
+    @WhenFor({ name: 'roles' })
+    shouldAskRoles(options: { roles?: Role[]; permissions?: Permission[] }): boolean {
+        // Ask for roles if they weren't provided or are empty
+        return !options.roles || options.roles.length === 0;
+    }
+
     @ChoicesFor({ name: 'roles' })
     async getRoles() {
         return Object.values(Role);
@@ -53,6 +59,12 @@ export class AddApiKeyQuestionSet {
         return this.apiKeyService.convertPermissionsStringArrayToPermissions(val);
     }
 
+    @WhenFor({ name: 'permissions' })
+    shouldAskPermissions(options: { roles?: Role[]; permissions?: Permission[] }): boolean {
+        // Ask for permissions if they weren't provided or are empty
+        return !options.permissions || options.permissions.length === 0;
+    }
+
     @ChoicesFor({ name: 'permissions' })
     async getPermissions() {
         return this.apiKeyService
@@ -72,6 +84,6 @@ export class AddApiKeyQuestionSet {
 
     @WhenFor({ name: 'overwrite' })
     shouldAskOverwrite(options: { name: string }): boolean {
-        return Boolean(this.apiKeyService.findByKey(options.name));
+        return Boolean(this.apiKeyService.findByField('name', options.name));
     }
 }