sec(auth): unscoped API key fails closed instead of inheriting owner permissions

frontend/src/__tests__/apikeys.test.ts

@@ -21,6 +21,7 @@ jest.mock('../api', () => ({
   createApiKey: jest.fn(),
   revokeApiKey: jest.fn(),
   deleteApiKey: jest.fn(),
+  getUserPermissions: jest.fn(),
   // Real implementation (not mocked): createApiKey encodes the password
   // field with this before building the request, so tests need the real
   // encoding to assert on the resulting request shape.
@@ -48,9 +49,16 @@ describe('API Keys Module', () => {
         <form id="create-apikey-form">
           <input type="text" id="apikey-name" value="">
           <input type="password" id="apikey-password" value="">
+          <div id="apikey-permissions-list">
+            <label class="apikey-permission-option">
+              <input type="checkbox" class="apikey-permission-checkbox" id="apikey-perm-0"
+                     data-action="view" data-resource="recommendations" checked>
+              view:recommendations
+            </label>
+          </div>
           <input type="date" id="apikey-expires-at" value="">
           <div id="create-apikey-error" class="hidden"></div>
-          <button type="submit">Create</button>
+          <button type="submit" id="create-apikey-submit-btn">Create</button>
         </form>
         <button id="close-create-apikey-modal-btn"></button>
       </div>
@@ -72,6 +80,13 @@ describe('API Keys Module', () => {
       total_requests_lifetime: 0,
       top_keys: [],
     });
+    // Default the owner's effective-permissions fetch (issue #61's
+    // frontend follow-up) so showCreateKeyModal tests that don't care
+    // about the picker's own behavior aren't forced to stub it too.
+    (api.getUserPermissions as jest.Mock).mockResolvedValue({
+      permissions: [{ action: 'view', resource: 'recommendations' }],
+      is_admin: false,
+    });
   });
 
   describe('loadApiKeys', () => {
@@ -404,6 +419,108 @@ describe('API Keys Module', () => {
       // Should not throw
       expect(() => showCreateKeyModal()).not.toThrow();
     });
+
+    // Issue #61 frontend follow-up: the create-key modal now fetches the
+    // caller's own effective permissions and renders one checkbox per
+    // entry, since the backend rejects an empty permissions array.
+    describe('permission picker (issue #61)', () => {
+      test('fetches and renders the owner\'s effective permissions as unchecked checkboxes', async () => {
+        (api.getUserPermissions as jest.Mock).mockResolvedValue({
+          permissions: [
+            { action: 'view', resource: 'recommendations' },
+            { action: 'create', resource: 'plans' },
+          ],
+          is_admin: false,
+        });
+
+        await showCreateKeyModal();
+
+        expect(api.getUserPermissions).toHaveBeenCalled();
+        const container = document.getElementById('apikey-permissions-list');
+        const checkboxes = container?.querySelectorAll<HTMLInputElement>('.apikey-permission-checkbox');
+        expect(checkboxes?.length).toBe(2);
+        checkboxes?.forEach(cb => expect(cb.checked).toBe(false));
+        expect(container?.textContent).toContain('view:recommendations');
+        expect(container?.textContent).toContain('create:plans');
+      });
+
+      test('labels the admin wildcard entry in plain language', async () => {
+        (api.getUserPermissions as jest.Mock).mockResolvedValue({
+          permissions: [{ action: 'admin', resource: '*' }],
+          is_admin: true,
+        });
+
+        await showCreateKeyModal();
+
+        const container = document.getElementById('apikey-permissions-list');
+        expect(container?.textContent).toContain('Admin (all actions, all resources)');
+      });
+
+      test('escapes hostile action/resource values instead of injecting markup', async () => {
+        const hostile = '"><img src=x onerror=alert(1)>';
+        (api.getUserPermissions as jest.Mock).mockResolvedValue({
+          permissions: [{ action: hostile, resource: 'plans' }],
+          is_admin: false,
+        });
+
+        await showCreateKeyModal();
+
+        const container = document.getElementById('apikey-permissions-list');
+        // No element was injected -- the quote-breakout attempt stayed
+        // inside the data-action attribute's value rather than closing it
+        // and opening a real <img> tag.
+        expect(container?.querySelector('img')).toBeNull();
+        // The checkbox's dataset round-trips the exact hostile string,
+        // proving escapeHtml neutralized it as data rather than mangling
+        // or truncating it.
+        const checkbox = container?.querySelector<HTMLInputElement>('.apikey-permission-checkbox');
+        expect(checkbox?.dataset['action']).toBe(hostile);
+        // The visible label text is HTML-escaped, not raw markup.
+        expect(container?.textContent).toContain(`${hostile}:plans`);
+      });
+
+      test('shows a message and no checkboxes when the owner holds no permissions', async () => {
+        (api.getUserPermissions as jest.Mock).mockResolvedValue({ permissions: [], is_admin: false });
+
+        await showCreateKeyModal();
+
+        const container = document.getElementById('apikey-permissions-list');
+        expect(container?.querySelectorAll('.apikey-permission-checkbox').length).toBe(0);
+        expect(container?.textContent).toContain('No permissions available');
+      });
+
+      test('shows an inline error when the permissions fetch fails', async () => {
+        const consoleError = jest.spyOn(console, 'error').mockImplementation(() => {});
+        (api.getUserPermissions as jest.Mock).mockRejectedValue(new Error('boom'));
+
+        await showCreateKeyModal();
+
+        const container = document.getElementById('apikey-permissions-list');
+        expect(container?.querySelector('.error')?.textContent).toContain('Failed to load your permissions');
+        consoleError.mockRestore();
+      });
+
+      test('Create button starts disabled and enables only once a permission is checked', async () => {
+        (api.getUserPermissions as jest.Mock).mockResolvedValue({
+          permissions: [{ action: 'view', resource: 'recommendations' }],
+          is_admin: false,
+        });
+
+        await showCreateKeyModal();
+
+        const submitBtn = document.getElementById('create-apikey-submit-btn') as HTMLButtonElement;
+        expect(submitBtn.disabled).toBe(true);
+
+        const checkbox = document.querySelector('.apikey-permission-checkbox') as HTMLInputElement;
+        checkbox.checked = true;
+        checkbox.dispatchEvent(new Event('change'));
+        expect(submitBtn.disabled).toBe(false);
+
+        checkbox.checked = false;
+        checkbox.dispatchEvent(new Event('change'));
+        expect(submitBtn.disabled).toBe(true);
+      });
+    });
   });
 
   describe('closeCreateKeyModal', () => {
@@ -426,8 +543,12 @@ describe('API Keys Module', () => {
 
   describe('createApiKey', () => {
     const expiresAt = new Date('2025-12-31T00:00:00Z');
+    // permissions is required (issue #61): the backend rejects an empty
+    // array with 400, so every real caller (handleCreateApiKey) always has
+    // a non-empty selection by the time it calls this wrapper.
+    const permissions = [{ action: 'view', resource: 'recommendations' }];
 
-    test('creates API key with name, password and expiration', async () => {
+    test('creates API key with name, password, expiration and permissions', async () => {
       const mockResponse = {
         api_key: 'full-api-key-value',
         key_id: 'key-1',
@@ -436,17 +557,18 @@ describe('API Keys Module', () => {
 
       (api.createApiKey as jest.Mock).mockResolvedValue(mockResponse);
 
-      const result = await createApiKey('Test Key', 'my-password', expiresAt);
+      const result = await createApiKey('Test Key', 'my-password', expiresAt, permissions);
 
       expect(api.createApiKey).toHaveBeenCalledWith({
         name: 'Test Key',
         password: Buffer.from('my-password', 'binary').toString('base64'),
-        expires_at: expiresAt.toISOString()
+        expires_at: expiresAt.toISOString(),
+        permissions,
       });
       expect(result.api_key).toBe('full-api-key-value');
     });
 
-    test('creates API key with permissions', async () => {
+    test('sends every permission passed in', async () => {
       const mockResponse = {
         api_key: 'full-api-key-value',
         key_id: 'key-1',
@@ -455,22 +577,25 @@ describe('API Keys Module', () => {
 
       (api.createApiKey as jest.Mock).mockResolvedValue(mockResponse);
 
-      const permissions = [{ action: 'read', resource: '*' }];
-      await createApiKey('Test Key', 'my-password', expiresAt, permissions);
+      const multiplePermissions = [
+        { action: 'view', resource: 'recommendations' },
+        { action: 'create', resource: 'plans' },
+      ];
+      await createApiKey('Test Key', 'my-password', expiresAt, multiplePermissions);
 
       expect(api.createApiKey).toHaveBeenCalledWith({
         name: 'Test Key',
         password: Buffer.from('my-password', 'binary').toString('base64'),
         expires_at: expiresAt.toISOString(),
-        permissions: permissions
+        permissions: multiplePermissions
       });
     });
 
     test('throws error on API failure', async () => {
       const consoleError = jest.spyOn(console, 'error').mockImplementation(() => {});
       (api.createApiKey as jest.Mock).mockRejectedValue(new Error('Create failed'));
 
-      await expect(createApiKey('Test Key', 'my-password', expiresAt)).rejects.toThrow('Create failed');
+      await expect(createApiKey('Test Key', 'my-password', expiresAt, permissions)).rejects.toThrow('Create failed');
       expect(consoleError).toHaveBeenCalled();
       consoleError.mockRestore();
     });
@@ -592,6 +717,81 @@ describe('API Keys Module', () => {
       const errorEl = document.getElementById('create-apikey-error');
       expect(errorEl?.textContent).toContain('Failed to create API key');
     });
+
+    // Issue #61: the backend now 400s on an empty/missing permissions
+    // array, so the request body sent from the form must always carry the
+    // checked selection.
+    test('sends the checked permissions in the request body', async () => {
+      const futureDate = new Date(Date.now() + 30 * 24 * 60 * 60 * 1000);
+      (api.createApiKey as jest.Mock).mockResolvedValue({
+        api_key: 'test-key',
+        key_id: 'key-1',
+        key: { id: 'key-1', name: 'Test', key_prefix: 'abc' }
+      });
+      (api.getApiKeys as jest.Mock).mockResolvedValue({ api_keys: [] });
+
+      (document.getElementById('apikey-name') as HTMLInputElement).value = 'Test Key';
+      (document.getElementById('apikey-password') as HTMLInputElement).value = 'my-password';
+      (document.getElementById('apikey-expires-at') as HTMLInputElement).value = futureDate.toISOString().split('T')[0] || "";
+      // The default fixture ships one pre-checked view:recommendations box.
+
+      const event = { preventDefault: jest.fn() } as unknown as Event;
+      await handleCreateApiKey(event);
+
+      expect(api.createApiKey).toHaveBeenCalledWith(
+        expect.objectContaining({
+          name: 'Test Key',
+          permissions: [{ action: 'view', resource: 'recommendations' }],
+        })
+      );
+      const [request] = (api.createApiKey as jest.Mock).mock.calls[0] as [{ permissions: unknown[] }];
+      expect(request.permissions.length).toBeGreaterThan(0);
+    });
+
+    test('rejects submission with an error, and never calls the API, when no permission is checked', async () => {
+      const futureDate = new Date(Date.now() + 30 * 24 * 60 * 60 * 1000);
+      (document.getElementById('apikey-name') as HTMLInputElement).value = 'Test Key';
+      (document.getElementById('apikey-password') as HTMLInputElement).value = 'my-password';
+      (document.getElementById('apikey-expires-at') as HTMLInputElement).value = futureDate.toISOString().split('T')[0] || "";
+      const checkbox = document.querySelector('.apikey-permission-checkbox') as HTMLInputElement;
+      checkbox.checked = false;
+
+      const event = { preventDefault: jest.fn() } as unknown as Event;
+      await handleCreateApiKey(event);
+
+      const errorEl = document.getElementById('create-apikey-error');
+      expect(errorEl?.textContent).toBe('Select at least one permission');
+      expect(api.createApiKey).not.toHaveBeenCalled();
+    });
+
+    test('sends every checked permission when more than one is selected', async () => {
+      const futureDate = new Date(Date.now() + 30 * 24 * 60 * 60 * 1000);
+      document.getElementById('apikey-permissions-list')!.innerHTML = `
+        <label><input type="checkbox" class="apikey-permission-checkbox" data-action="view" data-resource="recommendations" checked></label>
+        <label><input type="checkbox" class="apikey-permission-checkbox" data-action="create" data-resource="plans" checked></label>
+      `;
+      (api.createApiKey as jest.Mock).mockResolvedValue({
+        api_key: 'test-key',
+        key_id: 'key-1',
+        key: { id: 'key-1', name: 'Test', key_prefix: 'abc' }
+      });
+      (api.getApiKeys as jest.Mock).mockResolvedValue({ api_keys: [] });
+      (document.getElementById('apikey-name') as HTMLInputElement).value = 'Test Key';
+      (document.getElementById('apikey-password') as HTMLInputElement).value = 'my-password';
+      (document.getElementById('apikey-expires-at') as HTMLInputElement).value = futureDate.toISOString().split('T')[0] || "";
+
+      const event = { preventDefault: jest.fn() } as unknown as Event;
+      await handleCreateApiKey(event);
+
+      expect(api.createApiKey).toHaveBeenCalledWith(
+        expect.objectContaining({
+          permissions: [
+            { action: 'view', resource: 'recommendations' },
+            { action: 'create', resource: 'plans' },
+          ],
+        })
+      );
+    });
   });
 
   describe('showKeyCreatedModal', () => {