feat: enhance OAuth scopes and UI for agent-driven approval process (#544)

* feat: enhance OAuth scopes and UI for agent-driven approval process

* refactor: update OAuth scopes to enforce explicit user consent for write and approval actions

app/api/mcp-oauth/authorize/__tests__/route.test.ts

@@ -103,10 +103,10 @@ describe('GET /api/mcp-oauth/authorize — CSP', () => {
 
   it('renders both read and write rows when client passes only the legacy `mcp` scope marker', async () => {
     // Claude's connector sends scope=mcp today. The consent UI must render
-    // every scope group so the user can opt into :write grants — only the
-    // :read rows are pre-checked (Art. 25(2) data-protection-by-default).
-    // Regression: commit 04c097c2 hid all write rows by clamping the
-    // ceiling to DEFAULT_OAUTH_SCOPES.
+    // every scope group so the user can opt into write/approval rows if they
+    // want — but each write/approve row MUST start unchecked. Affirmative
+    // opt-in is the access-control gate (GDPR Art. 25(2), ISO 27001:2022
+    // A.5.18 / A.8.2, SOC 2 CC6.3, ASVS V10.2.2 / V2.3.1).
     const request = new Request(
       buildAuthorizeUrl({
         response_type: 'code',
@@ -120,24 +120,28 @@ describe('GET /api/mcp-oauth/authorize — CSP', () => {
     expect(response.status).toBe(200)
     const html = await response.text()
 
-    // Write scopes must render as checkboxes (un-pre-checked).
+    // Every scope row is rendered so the user can opt into / out of each one.
     expect(html).toMatch(/value="transactions:write"/)
     expect(html).toMatch(/value="bookkeeping:write"/)
     expect(html).toMatch(/value="invoices:write"/)
     expect(html).toMatch(/value="pending_operations:approve"/)
 
-    // The :write checkbox must NOT be pre-checked when the client passed
-    // no explicit scope request — the user has to opt in deliberately.
-    const writeRow = html.match(
-      /<input[^>]*value="transactions:write"[^>]*>/
-    )?.[0]
+    // Write and approval scopes MUST render unchecked. Users have to make an
+    // affirmative, deliberate selection for each destructive permission.
+    const writeRow = html.match(/<input[^>]*value="transactions:write"[^>]*>/)?.[0]
     expect(writeRow).toBeDefined()
     expect(writeRow!).not.toContain('checked')
 
-    // The :read counterpart must still be pre-checked (safe default).
-    const readRow = html.match(
-      /<input[^>]*value="transactions:read"[^>]*>/
-    )?.[0]
+    const approveRow = html.match(/<input[^>]*value="pending_operations:approve"[^>]*>/)?.[0]
+    expect(approveRow).toBeDefined()
+    expect(approveRow!).not.toContain('checked')
+
+    const bookkeepingRow = html.match(/<input[^>]*value="bookkeeping:write"[^>]*>/)?.[0]
+    expect(bookkeepingRow).toBeDefined()
+    expect(bookkeepingRow!).not.toContain('checked')
+
+    // The :read counterpart is pre-checked (safe default).
+    const readRow = html.match(/<input[^>]*value="transactions:read"[^>]*>/)?.[0]
     expect(readRow).toBeDefined()
     expect(readRow!).toContain('checked')
   })

app/api/mcp-oauth/authorize/route.ts

@@ -267,9 +267,20 @@ export async function GET(request: Request) {
       border: 1px solid var(--border);
       border-radius: 12px;
       padding: 2.5rem;
-      max-width: 560px;
+      max-width: 960px;
       width: 100%;
     }
+    .scope-groups {
+      display: grid;
+      grid-template-columns: repeat(2, minmax(0, 1fr));
+      column-gap: 1.5rem;
+      row-gap: 0;
+      align-items: start;
+    }
+    @media (max-width: 720px) {
+      .card { padding: 1.5rem; }
+      .scope-groups { grid-template-columns: 1fr; column-gap: 0; }
+    }
     .eyebrow {
       display: inline-flex;
       align-items: center;
@@ -369,10 +380,8 @@ export async function GET(request: Request) {
       outline-offset: 2px;
     }
     .scope-group {
-      padding: 0.375rem 0;
-      border-bottom: 1px solid var(--border);
+      padding: 0.375rem 0 0.75rem;
     }
-    .scope-group:last-of-type { border-bottom: none; padding-bottom: 0; }
     .scope-group-title {
       font-size: 0.6875rem;
       font-weight: 500;
@@ -530,7 +539,7 @@ export async function GET(request: Request) {
         </div>
       </div>
 
-      ${scopeCheckboxesHtml}
+      <div class="scope-groups">${scopeCheckboxesHtml}</div>
 
       <div class="warn">
         <svg class="warn-icon" viewBox="0 0 16 16" fill="none" stroke="currentColor" stroke-width="1.5" aria-hidden="true">

app/api/mcp-oauth/token/__tests__/route.test.ts

@@ -296,14 +296,25 @@ describe('POST /api/mcp-oauth/token', () => {
       )
       expect(res.status).toBe(200)
       const body = await res.json()
-      // DEFAULT_OAUTH_SCOPES is read-only by design (GDPR Art. 25(2) —
-      // destructive scopes must be requested explicitly).
+      // DEFAULT_OAUTH_SCOPES is read-only by design. Write and approval scopes
+      // must be requested explicitly by the client AND ticked by the user on
+      // the consent screen — GDPR Art. 25(2), ISO 27001:2022 A.5.18 / A.8.2,
+      // SOC 2 CC6.3, ASVS V8.1.1 / V10.2.1.
       const granted = body.scope.split(' ')
       expect(granted).toContain('transactions:read')
+      expect(granted).toContain('invoices:read')
+      expect(granted).toContain('suppliers:read')
       expect(granted).toContain('reports:read')
-      expect(granted).not.toContain('bookkeeping:write')
-      expect(granted).not.toContain('pending_operations:approve')
+      // No silent write or approval grants:
       expect(granted).not.toContain('transactions:write')
+      expect(granted).not.toContain('invoices:write')
+      expect(granted).not.toContain('suppliers:write')
+      expect(granted).not.toContain('customers:write')
+      expect(granted).not.toContain('documents:write')
+      expect(granted).not.toContain('pending_operations:approve')
+      expect(granted).not.toContain('bookkeeping:write')
+      expect(granted).not.toContain('payroll:write')
+      expect(granted).not.toContain('webhooks:manage')
     })
 
     it('honours scopes from the auth code when present', async () => {
@@ -403,7 +414,7 @@ describe('POST /api/mcp-oauth/token', () => {
       )
     })
 
-    it('falls back to DEFAULT_OAUTH_SCOPES for legacy keys with null scopes', async () => {
+    it('falls back to read-only DEFAULT_OAUTH_SCOPES for legacy keys with null scopes', async () => {
       const { supabase, enqueueMany } = createQueuedMockSupabase()
       mocks.supabaseFactory.mockReturnValue(supabase)
       enqueueMany([
@@ -421,8 +432,12 @@ describe('POST /api/mcp-oauth/token', () => {
       const body = await res.json()
       const granted = body.scope.split(' ')
       expect(granted).toContain('transactions:read')
-      expect(granted).not.toContain('bookkeeping:write')
+      // No silent grant of write or approval scopes (GDPR Art. 25(2),
+      // SoD per findStageApproveConflict — see lib/auth/api-keys.ts).
+      expect(granted).not.toContain('transactions:write')
       expect(granted).not.toContain('pending_operations:approve')
+      expect(granted).not.toContain('bookkeeping:write')
+      expect(granted).not.toContain('payroll:write')
     })
   })
 })