From 027fc0b2d6b7b573060777cfc4cf5f488f8b54f7 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Mon, 3 Nov 2025 16:36:47 +0100 Subject: [PATCH] docs: Add critical token exchange pattern documentation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Documents the architectural flaw in current implementation where session tokens and background tokens are not properly separated. Key issues identified: - Session tokens should be exchanged on-demand (RFC 8693) - Background tokens should use separate refresh token grant - Current implementation reuses refresh tokens incorrectly - No separation between foreground and background operations This is a P0 blocker that must be fixed before production use. πŸ€– Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- docs/CRITICAL-TOKEN-EXCHANGE-PATTERN.md | 290 ++++++++++++++++++++++++ 1 file changed, 290 insertions(+) create mode 100644 docs/CRITICAL-TOKEN-EXCHANGE-PATTERN.md diff --git a/docs/CRITICAL-TOKEN-EXCHANGE-PATTERN.md b/docs/CRITICAL-TOKEN-EXCHANGE-PATTERN.md new file mode 100644 index 0000000..d74a43c --- /dev/null +++ b/docs/CRITICAL-TOKEN-EXCHANGE-PATTERN.md @@ -0,0 +1,290 @@ +# CRITICAL: Token Exchange Pattern for ADR-004 + +## Problem Statement + +The current implementation of ADR-004 Progressive Consent does **NOT** correctly implement the token exchange pattern. This is a **critical architectural flaw** that must be corrected. + +## Current (Incorrect) Implementation + +### What Happens Now: +1. Client gets Flow 1 token (`aud: "mcp-server"`) +2. Client calls MCP tool +3. Server validates Flow 1 token +4. **WRONG**: Server uses stored refresh token to get Nextcloud token +5. **WRONG**: Same refresh token used for all sessions and background jobs + +### Problems: +- ❌ No separation between session tokens and background tokens +- ❌ Refresh tokens are reused across different contexts +- ❌ Session tokens could have different scope requirements than background tokens +- ❌ No on-demand delegation during tool calls +- ❌ Violates principle of least privilege + +## Correct Implementation Required + +### Token Exchange Pattern + +**MCP Session (Foreground Operations)**: + +``` +β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β” Flow 1 Token β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β” +β”‚ MCP Client β”‚ ───(aud: mcp-server)──> β”‚ MCP Server β”‚ +β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜ β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜ + β”‚ + Tool Call β”‚ + "search_notes()" β”‚ + β–Ό + β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β” + β”‚ Token Exchange β”‚ + β”‚ 1. Validate Flow 1 β”‚ + β”‚ 2. Check permission β”‚ + β”‚ 3. Request delegatedβ”‚ + β”‚ Nextcloud token β”‚ + β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜ + β”‚ + β”‚ Exchange Request + β–Ό + β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β” + β”‚ IdP Token Endpoint β”‚ + β”‚ (Token Exchange) β”‚ + β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜ + β”‚ + β”‚ Delegated Token + β”‚ (aud: nextcloud) + β”‚ (limited scopes) + β”‚ (short-lived) + β–Ό + β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β” + β”‚ Nextcloud API Call β”‚ + β”‚ GET /notes β”‚ + β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜ +``` + +**Key Properties of Session Tokens:** +- βœ… Generated **on-demand** during tool execution +- βœ… **Ephemeral** - used only for current operation +- βœ… **NOT stored** - discarded after use +- βœ… **Limited scopes** - only what tool needs (e.g., `notes:read` for search) +- βœ… **Short-lived** - expires quickly (e.g., 5 minutes) + +**Background Jobs (Offline Operations)**: + +``` +β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β” Scheduled Job β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β” +β”‚ Background β”‚ ──────────────────────> β”‚ Worker β”‚ +β”‚ Scheduler β”‚ β”‚ Process β”‚ +β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜ β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜ + β”‚ + β”‚ Use stored + β”‚ refresh token + β–Ό + β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β” + β”‚ Refresh Token Store β”‚ + β”‚ (Flow 2 provisioned)β”‚ + β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜ + β”‚ + β”‚ Refresh Token + β–Ό + β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β” + β”‚ IdP Token Endpoint β”‚ + β”‚ (Refresh Grant) β”‚ + β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜ + β”‚ + β”‚ Background Token + β”‚ (aud: nextcloud) + β”‚ (different scopes) + β”‚ (longer-lived) + β–Ό + β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β” + β”‚ Nextcloud API β”‚ + β”‚ (Background Sync) β”‚ + β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜ +``` + +**Key Properties of Background Tokens:** +- βœ… Obtained from **stored refresh token** (Flow 2) +- βœ… **Different scopes** than session tokens (e.g., `notes:sync`, `files:sync`) +- βœ… **Longer-lived** for background operations +- βœ… **Never used for MCP sessions** +- βœ… **Only for offline/background jobs** + +## Implementation Requirements + +### 1. Token Exchange Endpoint + +Implement RFC 8693 Token Exchange: + +```python +# nextcloud_mcp_server/auth/token_exchange.py + +async def exchange_token_for_delegation( + flow1_token: str, + requested_scopes: list[str], + requested_audience: str = "nextcloud" +) -> tuple[str, int]: + """ + Exchange Flow 1 MCP token for delegated Nextcloud token. + + This implements RFC 8693 Token Exchange for on-behalf-of delegation. + + Args: + flow1_token: The MCP session token (aud: "mcp-server") + requested_scopes: Scopes needed for this operation + requested_audience: Target audience (usually "nextcloud") + + Returns: + Tuple of (delegated_token, expires_in) + """ + # 1. Validate Flow 1 token + # 2. Check user has provisioned Nextcloud access (Flow 2) + # 3. Request token exchange from IdP + # 4. Return ephemeral delegated token +``` + +### 2. Context-Aware Token Broker + +Update Token Broker to distinguish contexts: + +```python +class TokenBrokerService: + async def get_session_token( + self, + flow1_token: str, + required_scopes: list[str] + ) -> str: + """Get ephemeral token for MCP session (on-demand).""" + # Exchange Flow 1 token for delegated token + # DO NOT use stored refresh token + # Return short-lived token + + async def get_background_token( + self, + user_id: str, + required_scopes: list[str] + ) -> str: + """Get token for background job (uses refresh token).""" + # Use stored refresh token from Flow 2 + # Different scope requirements + # Longer-lived token +``` + +### 3. Update MCP Tool Pattern + +Tools should request token exchange: + +```python +@mcp.tool() +@require_scopes("notes:read") +@require_provisioning +async def nc_notes_search_notes(query: str, ctx: Context) -> SearchNotesResponse: + """Search notes by title or content.""" + + # Extract Flow 1 token from context + flow1_token = ctx.authorization.token + + # Get Token Broker + broker = get_token_broker() + + # CRITICAL: Exchange for delegated token + nextcloud_token = await broker.get_session_token( + flow1_token=flow1_token, + required_scopes=["notes:read"] # Minimal scopes for this operation + ) + + # Create Nextcloud client with delegated token + client = await create_nextcloud_client( + host=NEXTCLOUD_HOST, + token=nextcloud_token # Ephemeral delegated token + ) + + # Execute operation + results = await client.notes_search_notes(query=query) + + # Token automatically expires - NOT stored + return SearchNotesResponse(results=results) +``` + +### 4. Background Job Pattern + +```python +# Background worker +async def sync_notes_job(user_id: str): + """Background job to sync notes.""" + + broker = get_token_broker() + + # CRITICAL: Use background token pattern + background_token = await broker.get_background_token( + user_id=user_id, + required_scopes=["notes:sync", "files:sync"] # Background-specific scopes + ) + + # Create client with background token + client = await create_nextcloud_client( + host=NEXTCLOUD_HOST, + token=background_token + ) + + # Perform background sync + await client.notes.sync_all() +``` + +## Security Benefits + +### Proper Token Exchange: +1. βœ… **Least Privilege**: Each operation gets only needed scopes +2. βœ… **Time-Limited**: Session tokens expire quickly +3. βœ… **Audit Trail**: Each exchange can be logged +4. βœ… **Token Isolation**: Session β‰  Background tokens +5. βœ… **Revocation**: Can revoke background access without affecting active sessions + +### Current Incorrect Pattern: +1. ❌ **Over-Privileged**: Refresh token has all scopes +2. ❌ **Long-Lived**: Same token reused indefinitely +3. ❌ **No Separation**: Sessions and background jobs use same credential +4. ❌ **Revocation Issues**: Revoking affects everything + +## Implementation Steps + +### Phase 1: Token Exchange (High Priority) +1. Implement RFC 8693 token exchange endpoint +2. Update Token Broker with `get_session_token()` vs `get_background_token()` +3. Modify tool pattern to use token exchange + +### Phase 2: Scope Separation (High Priority) +1. Define session scopes vs background scopes +2. Update provisioning flow to request appropriate scopes +3. Validate scopes in token exchange + +### Phase 3: Background Jobs (Medium Priority) +1. Implement background worker pattern +2. Create scheduled jobs (note sync, etc.) +3. Use background token pattern + +### Phase 4: Testing (High Priority) +1. Test token exchange flow end-to-end +2. Verify session tokens are ephemeral +3. Verify background tokens are separate +4. Load test token exchange performance + +## References + +- **RFC 8693**: OAuth 2.0 Token Exchange +- **RFC 9068**: JSON Web Token (JWT) Profile for OAuth 2.0 Access Tokens +- **ADR-004**: Progressive Consent OAuth Flows +- **OAuth 2.0 Delegation**: On-Behalf-Of vs Impersonation patterns + +## Status + +**Current Status**: ❌ CRITICAL ISSUE - Token exchange not implemented +**Target Status**: βœ… Proper token exchange with session/background separation +**Priority**: **P0 - Blocker for production use** + +## Next Actions + +1. [ ] Implement `token_exchange.py` module with RFC 8693 support +2. [ ] Update `TokenBrokerService` with session vs background methods +3. [ ] Refactor MCP tools to use token exchange pattern +4. [ ] Add integration tests for token exchange +5. [ ] Document background job patterns +6. [ ] Update ADR-004 with implementation details