fix: remove unused api key calls (#1628)

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

## Summary by CodeRabbit

- New Features
  - None.
- Bug Fixes
- Improved reliability of API key updates and clearer errors when keys
are missing or data is invalid.
- More robust initialization to prevent intermittent failures during API
key operations.
- Refactor
- Simplified API key management by unifying lookup flows and adopting
consistent async handling.
- Tests
- Expanded coverage for error scenarios and strengthened test setup for
initialization and file-read issues.
- Chores
  - Minor formatting cleanups.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

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

@@ -226,12 +226,12 @@ describe('ApiKeyService', () => {
         });
     });
 
-    describe('findByIdWithSecret', () => {
-        it('should return API key with secret when found', async () => {
+    describe('findById', () => {
+        it('should return API key when found', async () => {
             vi.spyOn(apiKeyService, 'loadAllFromDisk').mockResolvedValue([mockApiKey]);
             await apiKeyService.onModuleInit();
 
-            const result = await apiKeyService.findByIdWithSecret(mockApiKey.id);
+            const result = await apiKeyService.findById(mockApiKey.id);
 
             expect(result).toEqual(mockApiKey);
         });
@@ -240,7 +240,7 @@ describe('ApiKeyService', () => {
             vi.spyOn(apiKeyService, 'loadAllFromDisk').mockResolvedValue([]);
             await apiKeyService.onModuleInit();
 
-            const result = await apiKeyService.findByIdWithSecret('non-existent-id');
+            const result = await apiKeyService.findById('non-existent-id');
 
             expect(result).toBeNull();
         });
@@ -353,7 +353,7 @@ describe('ApiKeyService', () => {
     describe('update', () => {
         let updateMockApiKey: ApiKey;
 
-        beforeEach(() => {
+        beforeEach(async () => {
             // Create a fresh copy of the mock data for update tests
             updateMockApiKey = {
                 id: 'test-api-id',
@@ -370,9 +370,11 @@ describe('ApiKeyService', () => {
                 createdAt: new Date().toISOString(),
             };
 
-            vi.spyOn(apiKeyService, 'loadAllFromDisk').mockResolvedValue([updateMockApiKey]);
+            // Initialize the memoryApiKeys with the test data
+            // The loadAllFromDisk mock will be called by onModuleInit
+            vi.spyOn(apiKeyService, 'loadAllFromDisk').mockResolvedValue([{ ...updateMockApiKey }]);
             vi.spyOn(apiKeyService, 'saveApiKey').mockResolvedValue();
-            apiKeyService.onModuleInit();
+            await apiKeyService.onModuleInit();
         });
 
         it('should update name and description', async () => {
@@ -384,7 +386,6 @@ describe('ApiKeyService', () => {
                 name: updatedName,
                 description: updatedDescription,
             });
-
             expect(result.name).toBe(updatedName);
             expect(result.description).toBe(updatedDescription);
             expect(result.roles).toEqual(updateMockApiKey.roles);

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

@@ -226,10 +226,6 @@ export class ApiKeyService implements OnModuleInit {
         }
     }
 
-    public findByIdWithSecret(id: string): ApiKey | null {
-        return this.findByField('id', id);
-    }
-
     public findByField(field: keyof ApiKey, value: string): ApiKey | null {
         if (!value) return null;
 
@@ -330,7 +326,7 @@ export class ApiKeyService implements OnModuleInit {
         roles?: Role[];
         permissions?: Permission[] | AddPermissionInput[];
     }): Promise<ApiKey> {
-        const apiKey = this.findByIdWithSecret(id);
+        const apiKey = await this.findById(id);
         if (!apiKey) {
             throw new GraphQLError('API key not found');
         }

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

@@ -228,10 +228,6 @@ describe('AuthService', () => {
             };
 
             vi.spyOn(apiKeyService, 'findById').mockResolvedValue(mockApiKeyWithoutRole);
-            vi.spyOn(apiKeyService, 'findByIdWithSecret').mockResolvedValue({
-                ...mockApiKey,
-                roles: [Role.ADMIN],
-            });
             vi.spyOn(apiKeyService, 'saveApiKey').mockResolvedValue();
             vi.spyOn(authzService, 'addRoleForUser').mockResolvedValue(true);
 
@@ -239,9 +235,8 @@ describe('AuthService', () => {
 
             expect(result).toBe(true);
             expect(apiKeyService.findById).toHaveBeenCalledWith(apiKeyId);
-            expect(apiKeyService.findByIdWithSecret).toHaveBeenCalledWith(apiKeyId);
             expect(apiKeyService.saveApiKey).toHaveBeenCalledWith({
-                ...mockApiKey,
+                ...mockApiKeyWithoutRole,
                 roles: [Role.ADMIN, role],
             });
             expect(authzService.addRoleForUser).toHaveBeenCalledWith(apiKeyId, role);
@@ -259,13 +254,8 @@ describe('AuthService', () => {
     describe('removeRoleFromApiKey', () => {
         it('should remove role from API key', async () => {
             const apiKey = { ...mockApiKey, roles: [Role.ADMIN, Role.GUEST] };
-            const apiKeyWithSecret = {
-                ...mockApiKey,
-                roles: [Role.ADMIN, Role.GUEST],
-            };
 
             vi.spyOn(apiKeyService, 'findById').mockResolvedValue(apiKey);
-            vi.spyOn(apiKeyService, 'findByIdWithSecret').mockResolvedValue(apiKeyWithSecret);
             vi.spyOn(apiKeyService, 'saveApiKey').mockResolvedValue();
             vi.spyOn(authzService, 'deleteRoleForUser').mockResolvedValue(true);
 
@@ -273,9 +263,8 @@ describe('AuthService', () => {
 
             expect(result).toBe(true);
             expect(apiKeyService.findById).toHaveBeenCalledWith(apiKey.id);
-            expect(apiKeyService.findByIdWithSecret).toHaveBeenCalledWith(apiKey.id);
             expect(apiKeyService.saveApiKey).toHaveBeenCalledWith({
-                ...apiKeyWithSecret,
+                ...apiKey,
                 roles: [Role.GUEST],
             });
             expect(authzService.deleteRoleForUser).toHaveBeenCalledWith(apiKey.id, Role.ADMIN);

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

@@ -201,15 +201,12 @@ export class AuthService {
         }
 
         try {
+            if (!apiKey.roles) {
+                apiKey.roles = [];
+            }
             if (!apiKey.roles.includes(role)) {
-                const apiKeyWithSecret = await this.apiKeyService.findByIdWithSecret(apiKeyId);
-
-                if (!apiKeyWithSecret) {
-                    throw new UnauthorizedException('API key not found with secret');
-                }
-
-                apiKeyWithSecret.roles.push(role);
-                await this.apiKeyService.saveApiKey(apiKeyWithSecret);
+                apiKey.roles.push(role);
+                await this.apiKeyService.saveApiKey(apiKey);
                 await this.authzService.addRoleForUser(apiKeyId, role);
             }
 
@@ -231,14 +228,11 @@ export class AuthService {
         }
 
         try {
-            const apiKeyWithSecret = await this.apiKeyService.findByIdWithSecret(apiKeyId);
-
-            if (!apiKeyWithSecret) {
-                throw new UnauthorizedException('API key not found with secret');
+            if (!apiKey.roles) {
+                apiKey.roles = [];
             }
-
-            apiKeyWithSecret.roles = apiKeyWithSecret.roles.filter((r) => r !== role);
-            await this.apiKeyService.saveApiKey(apiKeyWithSecret);
+            apiKey.roles = apiKey.roles.filter((r) => r !== role);
+            await this.apiKeyService.saveApiKey(apiKey);
             await this.authzService.deleteRoleForUser(apiKeyId, role);
 
             return true;

packages/unraid-shared/src/services/api-key.ts

@@ -12,11 +12,6 @@ export interface ApiKeyService {
      */
     findById(id: string): Promise<ApiKey | null>;
 
-    /**
-     * Find an API key by its ID
-     * Note: This returns ApiKey without the secret for security
-     */
-    findByIdWithSecret(id: string): ApiKey | null;
 
     /**
      * Find an API key by a specific field