diff --git a/docs/ADR-005-token-audience-validation.md b/docs/ADR-005-token-audience-validation.md index 931df55..29bde78 100644 --- a/docs/ADR-005-token-audience-validation.md +++ b/docs/ADR-005-token-audience-validation.md @@ -65,14 +65,15 @@ Based on analysis of the existing code and python-sdk constraints, we will: ### Mode 1: Multi-Audience Token Validation (Default) -Accept tokens that include **both** the MCP server and Nextcloud resource URIs in their audience claims. This is the default mode when `ENABLE_TOKEN_EXCHANGE` is false or not set. +Use multi-audience tokens directly. Per RFC 7519 Section 4.1.3, the MCP server validates only its own presence in the audience claim. Nextcloud independently validates its own audience when receiving API calls. This is the default mode when `ENABLE_TOKEN_EXCHANGE` is false or not set. **Requirements**: -- Token must have `aud` claim containing valid audiences for: +- Token must have `aud` claim containing: - **MCP server**: Client ID OR MCP server URL (e.g., `http://localhost:8000`) +- For Nextcloud API access to work, token should also include: - **Nextcloud**: Nextcloud resource URI (e.g., `http://localhost:8080`) - Single token works for both MCP authentication and Nextcloud API access -- IdP must support multi-audience tokens +- IdP must support multi-audience tokens for full functionality **Resource URI Configuration**: - Nextcloud OIDC app: Set via `default_resource_identifier` (default: `http://localhost:8080`) @@ -94,33 +95,39 @@ NEXTCLOUD_RESOURCE_URI=http://localhost:8080 # Nextcloud resource identifier OIDC_CLIENT_ID=nextcloud-mcp-server ``` -**Token validation logic**: +**Token validation logic (RFC 7519 compliant)**: ```python async def validate_token_audiences(token: dict, settings: Settings) -> bool: - """Validate token has required audiences for both MCP and Nextcloud.""" + """ + Validate token has MCP audience per RFC 7519. + + Resource servers validate only their own presence in the audience claim. + Nextcloud will independently validate its own audience when receiving API calls. + This is NOT token passthrough (we validate the token). This IS token reuse + which is allowed by RFC 8707 for multi-audience tokens between trusted services. + """ audiences = token.get("aud", []) if isinstance(audiences, str): audiences = [audiences] audiences_set = set(audiences) - # MCP must have at least one: client_id OR server_url + # MCP validates ONLY its own audience (client_id OR server_url OR server_url/mcp) mcp_valid = ( settings.oidc_client_id in audiences_set or - settings.nextcloud_mcp_server_url in audiences_set + settings.nextcloud_mcp_server_url in audiences_set or + f"{settings.nextcloud_mcp_server_url}/mcp" in audiences_set ) - # Nextcloud must have its resource URI - nextcloud_valid = settings.nextcloud_resource_uri in audiences_set - - if not (mcp_valid and nextcloud_valid): + if not mcp_valid: logger.error( - f"Token rejected: Invalid audiences. " + f"Token rejected: Missing MCP audience. " f"Got {audiences}, need MCP ({settings.oidc_client_id} or " - f"{settings.nextcloud_mcp_server_url}) AND Nextcloud ({settings.nextcloud_resource_uri})" + f"{settings.nextcloud_mcp_server_url})" ) return False + # Note: We do NOT validate Nextcloud's audience - that's Nextcloud's responsibility return True ``` @@ -894,10 +901,12 @@ verifier = UnifiedTokenVerifier(settings) ### Positive 1. **Security Compliance**: Eliminates token passthrough vulnerability -2. **Clear Architecture**: Explicit validation modes with resource URI semantics -3. **Performance**: Negligible impact in LLM context (1-2% of request time) -4. **Flexibility**: Supports both simple (multi-audience) and strict (exchange) modes -5. **Audit Trail**: Proper audience separation enables accurate logging +2. **OAuth Spec Compliance**: Follows RFC 7519 Section 4.1.3 - resource servers validate only their own audience +3. **Clear Architecture**: Explicit validation modes with resource URI semantics +4. **Performance**: Negligible impact in LLM context (1-2% of request time) +5. **Flexibility**: Supports both simple (multi-audience) and strict (exchange) modes +6. **Audit Trail**: Proper audience separation enables accurate logging +7. **Simpler Logic**: Each resource server independently validates its own audience, reducing complexity ### Negative diff --git a/docs/ADR-006-progressive-consent-elicitation.md b/docs/ADR-006-progressive-consent-elicitation.md new file mode 100644 index 0000000..d39a473 --- /dev/null +++ b/docs/ADR-006-progressive-consent-elicitation.md @@ -0,0 +1,651 @@ +# ADR-006: Progressive Consent via URL Elicitation (SEP-1036) + +**Status**: Proposed +**Date**: 2025-01-05 +**Related**: [SEP-1036](https://github.com/modelcontextprotocol/specification/pull/887), ADR-004 +**Depends On**: ADR-005 (token validation) + +## Context + +The current progressive consent implementation (ADR-004) requires users to manually visit OAuth URLs returned by MCP tools. This creates a poor user experience: + +1. User calls `provision_nextcloud_access` tool +2. Tool returns a URL as text in the response +3. User must manually copy URL and open in browser +4. No indication when provisioning is complete +5. User must retry the original operation manually + +### SEP-1036: URL Mode Elicitation + +The MCP specification now supports **URL mode elicitation** ([SEP-1036](https://github.com/modelcontextprotocol/specification/pull/887)), which enables servers to: + +- Request out-of-band user interactions via secure URLs +- Handle sensitive operations like OAuth flows without exposing credentials to the client +- Provide progress tracking for async operations +- Return errors that automatically trigger elicitation flows + +**Key benefits for progressive consent**: +- **Automatic URL Opening**: Client opens URL in browser automatically (with user consent) +- **Progress Tracking**: Server can notify client when provisioning is complete +- **Error-Triggered Flows**: Server can return `ElicitationRequired` error to trigger provisioning +- **Better UX**: User doesn't manually copy/paste URLs + +### Current Implementation Limitations + +The current progressive consent flow in `nextcloud_mcp_server/server/oauth_tools.py`: + +```python +@mcp.tool(name="provision_nextcloud_access") +async def tool_provision_access(ctx: Context) -> ProvisioningResult: + """Returns OAuth URL as text - user must manually open it.""" + return ProvisioningResult( + success=True, + authorization_url=auth_url, # User must copy this + message="Please visit the authorization URL..." + ) +``` + +**Problems**: +1. Manual URL handling (copy/paste) +2. No progress tracking +3. No automatic retry after provisioning +4. Tool call required just to get URL +5. No client integration (URL just displayed as text) + +## Decision + +We will **migrate progressive consent from manual tools to URL mode elicitation**, leveraging SEP-1036 for better user experience and OAuth security. + +### New Architecture: Elicitation-Driven Consent + +Instead of explicit tools, use **automatic elicitation** triggered by authorization errors: + +``` +User → Calls Nextcloud Tool → Server Checks Provisioning + ↓ Not Provisioned + Error: ElicitationRequired + ↓ + Client Shows Consent UI + ↓ User Accepts + Client Opens OAuth URL + ↓ + User Completes OAuth + ↓ + Server Sends Progress Update + ↓ + Original Tool Call Auto-Retries +``` + +### Mode 1: Elicitation-Required Error (Primary) + +When a tool requires provisioning, return an **ElicitationRequired error** (-32000): + +```python +# In any Nextcloud tool decorated with @require_provisioning +@mcp.tool() +@require_provisioning # New decorator +async def nc_notes_list_notes(ctx: Context): + """List notes - auto-triggers provisioning if needed.""" + # If not provisioned, decorator returns ElicitationRequired error + # If provisioned, continues normally + client = await get_client(ctx) + return await client.notes.list_notes() +``` + +**Error response structure**: +```json +{ + "jsonrpc": "2.0", + "id": 1, + "error": { + "code": -32000, + "message": "Nextcloud access provisioning required", + "data": { + "elicitations": [ + { + "mode": "url", + "elicitationId": "550e8400-e29b-41d4-a716-446655440000", + "url": "https://mcp.example.com/oauth/provision?id=550e8400...", + "message": "Grant the MCP server access to your Nextcloud account to continue." + } + ] + } + } +} +``` + +**Client behavior**: +1. Receives error with elicitation +2. Shows consent UI: "App wants to access Nextcloud. Open authorization page?" +3. On user acceptance, opens URL in browser +4. Optionally tracks progress via `elicitation/track` +5. Auto-retries original tool call when complete + +### Mode 2: Explicit Elicitation Request (Fallback) + +For clients that don't support error-triggered elicitation, provide explicit tool: + +```python +@mcp.tool(name="request_nextcloud_access") +async def request_access(ctx: Context) -> ElicitationResponse: + """Explicitly request provisioning via elicitation.""" + # Send elicitation/create request + return await create_elicitation( + mode="url", + url=generate_oauth_url(), + message="Grant access to Nextcloud", + elicitation_id=generate_id() + ) +``` + +**Note**: This is a fallback for compatibility. Primary flow uses error-triggered elicitation. + +## Implementation + +### 1. New Decorator: `@require_provisioning` + +Replace explicit provisioning checks with a decorator that returns `ElicitationRequired`: + +```python +# nextcloud_mcp_server/auth/provisioning_decorator.py + +def require_provisioning(func): + """ + Decorator that ensures user has provisioned Nextcloud access. + + If not provisioned, returns ElicitationRequired error with OAuth URL. + Otherwise, proceeds with normal tool execution. + """ + @functools.wraps(func) + async def wrapper(ctx: Context, *args, **kwargs): + # Extract user ID from token + user_id = get_user_id_from_context(ctx) + + # Check if provisioned + storage = RefreshTokenStorage.from_env() + await storage.initialize() + + if not await storage.has_refresh_token(user_id): + # Not provisioned - return ElicitationRequired error + elicitation_id = str(uuid.uuid4()) + oauth_url = await generate_oauth_url_for_provisioning( + user_id=user_id, + elicitation_id=elicitation_id, + ctx=ctx + ) + + # Store elicitation for tracking + await storage.store_elicitation( + elicitation_id=elicitation_id, + user_id=user_id, + status="pending", + created_at=datetime.now(timezone.utc) + ) + + raise McpError( + code=ErrorCode.ELICITATION_REQUIRED, # -32000 + message="Nextcloud access provisioning required", + data={ + "elicitations": [ + { + "mode": "url", + "elicitationId": elicitation_id, + "url": oauth_url, + "message": ( + "Grant the MCP server access to your Nextcloud " + "account to continue. This is a one-time setup." + ) + } + ] + } + ) + + # Already provisioned - proceed normally + return await func(ctx, *args, **kwargs) + + return wrapper +``` + +### 2. Elicitation Tracking Endpoint + +Implement `elicitation/track` to provide progress updates: + +```python +# nextcloud_mcp_server/server/elicitation.py + +@mcp.request_handler("elicitation/track") +async def track_elicitation( + elicitation_id: str, + _meta: dict = None +) -> dict: + """ + Track progress of an elicitation request. + + Returns when elicitation is complete or times out. + """ + progress_token = _meta.get("progressToken") if _meta else None + + storage = RefreshTokenStorage.from_env() + await storage.initialize() + + # Poll for completion (with timeout) + timeout = 300 # 5 minutes + start_time = datetime.now(timezone.utc) + + while (datetime.now(timezone.utc) - start_time).seconds < timeout: + elicitation = await storage.get_elicitation(elicitation_id) + + if not elicitation: + raise McpError( + code=-32602, # Invalid params + message=f"Unknown elicitation ID: {elicitation_id}" + ) + + # Send progress notification if token provided + if progress_token and elicitation["status"] == "pending": + await send_progress_notification( + progress_token=progress_token, + progress=50, + message="Waiting for OAuth authorization..." + ) + + # Check if complete + if elicitation["status"] == "complete": + return {"status": "complete"} + + # Check if failed + if elicitation["status"] == "failed": + return { + "status": "failed", + "error": elicitation.get("error_message") + } + + # Wait before polling again + await asyncio.sleep(2) + + # Timeout + raise McpError( + code=-32000, + message="Elicitation timed out - user did not complete authorization" + ) +``` + +### 3. OAuth Callback Updates + +Update the OAuth callback to mark elicitations as complete: + +```python +# nextcloud_mcp_server/auth/oauth_routes.py + +async def oauth_callback(request: Request) -> Response: + """Handle OAuth callback and mark elicitation complete.""" + code = request.query_params.get("code") + state = request.query_params.get("state") + + # Validate and exchange code for tokens + tokens = await exchange_authorization_code(code) + + # Store refresh token + await storage.store_refresh_token( + user_id=user_id, + refresh_token=tokens["refresh_token"] + ) + + # Mark elicitation as complete + elicitation_id = request.query_params.get("elicitation_id") + if elicitation_id: + await storage.update_elicitation( + elicitation_id=elicitation_id, + status="complete", + completed_at=datetime.now(timezone.utc) + ) + + return Response( + content="

Authorization Complete!

" + "

You can close this window and return to the application.

", + media_type="text/html" + ) +``` + +### 4. Update All Nextcloud Tools + +Add `@require_provisioning` decorator to all Nextcloud tools: + +```python +# nextcloud_mcp_server/server/notes.py + +@mcp.tool() +@require_scopes("notes:read") +@require_provisioning # NEW: Auto-triggers provisioning +async def nc_notes_list_notes( + ctx: Context, + category: Optional[str] = None +) -> NotesListResponse: + """List all notes - automatically handles provisioning.""" + client = await get_client(ctx) + # Tool logic proceeds only if provisioned + notes = await client.notes.list_notes(category=category) + return NotesListResponse(results=notes) +``` + +### 5. Capability Declaration + +Declare URL elicitation support during initialization: + +```python +# nextcloud_mcp_server/app.py + +capabilities = { + "elicitation": { + "url": {} # Declare URL mode support + # Note: We don't support "form" mode (in-band data collection) + }, + # ... other capabilities +} +``` + +### 6. Environment Variables + +**New variables**: +```bash +# ELICITATION_CALLBACK_URL: Base URL for OAuth callbacks with elicitation tracking +# Default: NEXTCLOUD_MCP_SERVER_URL + /oauth/callback +ELICITATION_CALLBACK_URL=http://localhost:8000/oauth/callback + +# ELICITATION_TIMEOUT_SECONDS: How long to wait for user to complete OAuth +# Default: 300 (5 minutes) +ELICITATION_TIMEOUT_SECONDS=300 +``` + +**Removed variables** (no longer needed): +```bash +# ENABLE_PROGRESSIVE_CONSENT - removed, now always enabled in OAuth mode +# MCP_SERVER_CLIENT_ID - merged into OIDC_CLIENT_ID +``` + +## User Experience Comparison + +### Before (ADR-004 Manual Tools) + +``` +User: "List my notes" +Assistant: *calls nc_notes_list_notes* +Server: Error - not provisioned +Assistant: "You need to provision access first. Let me do that." +Assistant: *calls provision_nextcloud_access* +Server: {authorization_url: "https://..."} +Assistant: "Please visit this URL: https://..." +User: *copies URL, opens browser, completes OAuth* +User: "OK, I'm done" +Assistant: *calls nc_notes_list_notes again* +Server: Success! [notes...] +``` + +**Issues**: 4 interactions, manual URL handling, no automation + +### After (ADR-006 Elicitation) + +``` +User: "List my notes" +Assistant: *calls nc_notes_list_notes* +Server: ElicitationRequired error +Client: Shows dialog: "Grant access to Nextcloud? [Yes] [No]" +User: *clicks Yes* +Client: Opens OAuth URL in browser automatically +User: *completes OAuth* +Server: Sends progress notification "Complete!" +Client: Auto-retries nc_notes_list_notes +Server: Success! [notes...] +Assistant: "Here are your notes: ..." +``` + +**Benefits**: 1 interaction, automatic URL opening, seamless retry + +## Migration Path + +### Phase 1: Add Elicitation Support (v0.26.0) + +- Implement `@require_provisioning` decorator +- Add `elicitation/track` endpoint +- Keep existing tools (`provision_nextcloud_access`) for compatibility +- Update OAuth callback to track elicitations +- Add capability declaration + +**Breaking changes**: None (additive) + +### Phase 2: Update Documentation (v0.27.0) + +- Document elicitation-based flow as primary +- Mark manual tools as deprecated +- Update examples and guides + +**Breaking changes**: None (documentation only) + +### Phase 3: Remove Manual Tools (v0.28.0) + +- Remove `provision_nextcloud_access` tool +- Remove `check_provisioning_status` tool (status in error message) +- Remove `revoke_nextcloud_access` (or keep for explicit revocation?) + +**Breaking changes**: Yes (removed tools) + +### Phase 4: Optimize (v0.29.0+) + +- Add elicitation result caching +- Implement retry strategies +- Add metrics and monitoring + +## Testing + +### Test Cases + +1. **First-Time User Flow** + ```python + @pytest.mark.oauth + async def test_elicitation_first_time_user(nc_mcp_oauth_client): + """Test that first tool call triggers elicitation.""" + # User has no provisioning + with pytest.raises(McpError) as exc: + await nc_mcp_oauth_client.call_tool("nc_notes_list_notes") + + # Should get ElicitationRequired error + assert exc.value.code == -32000 + assert "elicitations" in exc.value.data + assert exc.value.data["elicitations"][0]["mode"] == "url" + + # Verify URL is valid OAuth URL + url = exc.value.data["elicitations"][0]["url"] + assert "oauth" in url + assert "elicitationId" in url + ``` + +2. **Progress Tracking** + ```python + @pytest.mark.oauth + async def test_elicitation_progress_tracking(nc_mcp_oauth_client): + """Test progress tracking during OAuth flow.""" + # Trigger elicitation + elicitation_id = trigger_elicitation() + + # Start tracking + track_task = asyncio.create_task( + nc_mcp_oauth_client.track_elicitation( + elicitation_id=elicitation_id, + progress_token="test-token" + ) + ) + + # Simulate OAuth completion + await asyncio.sleep(1) + await complete_oauth_flow(elicitation_id) + + # Track should complete + result = await track_task + assert result["status"] == "complete" + ``` + +3. **Auto-Retry After Provisioning** + ```python + @pytest.mark.oauth + async def test_auto_retry_after_provisioning(nc_mcp_oauth_client): + """Test that client auto-retries after elicitation.""" + # Mock client that auto-retries on ElicitationRequired + client = AutoRetryMcpClient(nc_mcp_oauth_client) + + # First call triggers elicitation, client handles it, retries + result = await client.call_tool_with_elicitation("nc_notes_list_notes") + + # Should succeed after provisioning + assert result.success + assert "notes" in result.data + ``` + +4. **Timeout Handling** + ```python + @pytest.mark.oauth + async def test_elicitation_timeout(nc_mcp_oauth_client): + """Test timeout if user doesn't complete OAuth.""" + elicitation_id = trigger_elicitation() + + # Track with short timeout + with pytest.raises(McpError, match="timed out"): + await nc_mcp_oauth_client.track_elicitation( + elicitation_id=elicitation_id, + timeout=5 # 5 seconds + ) + ``` + +## Security Considerations + +### Out-of-Band OAuth Flow + +**Benefit**: OAuth credentials never pass through MCP client +- User enters credentials directly on IdP page +- MCP server receives only authorization code +- Client never sees passwords or refresh tokens + +**Threat mitigation**: +- **Credential theft**: Client can't intercept credentials (out-of-band) +- **Token exposure**: Client never receives Nextcloud refresh tokens +- **CSRF**: State parameter validates OAuth callback +- **URL tampering**: Elicitation ID ties OAuth flow to user session + +### Elicitation ID as Security Token + +The `elicitationId` serves as a capability token: +- Cryptographically random (UUID v4) +- Single-use (invalidated after completion) +- Time-limited (expires after timeout) +- User-scoped (tied to user session) + +**Validation**: +```python +async def validate_elicitation_id(elicitation_id: str, user_id: str) -> bool: + """Validate that elicitation belongs to user and is still valid.""" + elicitation = await storage.get_elicitation(elicitation_id) + + if not elicitation: + return False + + # Check ownership + if elicitation["user_id"] != user_id: + logger.warning(f"Elicitation ID mismatch: {elicitation_id}") + return False + + # Check expiry + if elicitation["expires_at"] < datetime.now(timezone.utc): + return False + + # Check not already used + if elicitation["status"] != "pending": + return False + + return True +``` + +### Progress Tracking Security + +**Risk**: Progress token reuse across users + +**Mitigation**: +- Progress tokens tied to elicitation ID +- Elicitation ID tied to user session +- Server validates ownership before sending updates + +## Consequences + +### Positive + +1. **Better UX**: Automatic URL opening, no manual copy/paste +2. **Seamless Flow**: Auto-retry after provisioning +3. **Progress Feedback**: User knows when OAuth is complete +4. **Spec Compliance**: Implements SEP-1036 correctly +5. **Secure by Design**: Out-of-band OAuth prevents credential exposure +6. **Simpler API**: No explicit provisioning tools needed + +### Negative + +1. **Client Dependency**: Requires client support for URL elicitation +2. **Complexity**: More moving parts (elicitation tracking, callbacks) +3. **Polling**: Progress tracking uses polling (not ideal) +4. **Breaking Change**: Removes manual provisioning tools (in v0.28.0) + +### Neutral + +1. **Storage Requirements**: Need to store elicitation state +2. **Timeout Management**: Must handle long-running OAuth flows +3. **Fallback Support**: Still need compatibility for older clients + +## Alternatives Considered + +### 1. Keep Manual Tools Only (Rejected) + +**Pros**: Simple, no client changes needed +**Cons**: Poor UX, doesn't leverage SEP-1036 + +**Rejection reason**: SEP-1036 provides better UX and security + +### 2. Form Mode Elicitation (Rejected) + +**Pros**: No browser redirect needed +**Cons**: Would expose OAuth credentials to client (security violation) + +**Rejection reason**: Form mode only for non-sensitive data per SEP-1036 + +### 3. Hybrid: Both Tools and Elicitation (Considered) + +**Pros**: Maximum compatibility, gradual migration +**Cons**: API duplication, maintenance burden, confusing for users + +**Decision**: Support during migration (v0.26-0.27), remove in v0.28 + +### 4. WebSocket for Progress (Rejected) + +**Pros**: Real-time updates instead of polling +**Cons**: MCP spec uses polling pattern, adds complexity + +**Rejection reason**: Follow spec pattern (polling via elicitation/track) + +## References + +- [SEP-1036: URL Mode Elicitation](https://github.com/modelcontextprotocol/specification/pull/887) +- [MCP Elicitation Specification](https://modelcontextprotocol.io/specification/draft/client/elicitation) +- [ADR-004: Federated Authentication Architecture](./ADR-004-mcp-application-oauth.md) +- [ADR-005: Token Audience Validation](./ADR-005-token-audience-validation.md) +- [RFC 8252: OAuth 2.0 for Native Apps](https://datatracker.ietf.org/doc/html/rfc8252) + +## Implementation Checklist + +- [ ] Implement `@require_provisioning` decorator with ElicitationRequired error +- [ ] Add `elicitation/track` request handler +- [ ] Update OAuth callback to mark elicitations complete +- [ ] Add elicitation storage (ID, user, status, timestamps) +- [ ] Update all Nextcloud tools with `@require_provisioning` +- [ ] Add URL elicitation capability declaration +- [ ] Write integration tests for elicitation flow +- [ ] Write tests for progress tracking +- [ ] Update documentation with elicitation examples +- [ ] Add migration guide for manual tools → elicitation +- [ ] Keep manual tools with deprecation warnings (v0.26-0.27) +- [ ] Remove manual tools (v0.28.0) +- [ ] Update CHANGELOG.md with migration timeline diff --git a/nextcloud_mcp_server/auth/context_helper.py b/nextcloud_mcp_server/auth/context_helper.py index 7922237..6c8049e 100644 --- a/nextcloud_mcp_server/auth/context_helper.py +++ b/nextcloud_mcp_server/auth/context_helper.py @@ -24,9 +24,9 @@ def get_client_from_context(ctx: Context, base_url: str) -> NextcloudClient: """ Create NextcloudClient for multi-audience mode (no exchange needed). - ADR-005 Mode 1: Token already contains both MCP and Nextcloud audiences. - The UnifiedTokenVerifier validated both audiences are present, so we can - use the token directly without exchange. + ADR-005 Mode 1: Use multi-audience tokens directly. + The UnifiedTokenVerifier validated MCP audience per RFC 7519. + Nextcloud will independently validate its own audience. Args: ctx: MCP request context containing session info @@ -65,8 +65,8 @@ def get_client_from_context(ctx: Context, base_url: str) -> NextcloudClient: f"(no exchange needed)" ) - # Token was already validated to have both audiences - # Can use directly without exchange + # Token was validated to have MCP audience + # Nextcloud will validate its own audience independently return NextcloudClient.from_token( base_url=base_url, token=access_token.token, username=username ) diff --git a/nextcloud_mcp_server/auth/unified_verifier.py b/nextcloud_mcp_server/auth/unified_verifier.py index 0cb4b05..e3412c6 100644 --- a/nextcloud_mcp_server/auth/unified_verifier.py +++ b/nextcloud_mcp_server/auth/unified_verifier.py @@ -4,13 +4,15 @@ Unified Token Verifier for ADR-005 Token Audience Validation. This module replaces both NextcloudTokenVerifier and ProgressiveConsentTokenVerifier with a single implementation that supports two compliant OAuth modes: -1. Multi-audience mode (default): Tokens must contain BOTH MCP and Nextcloud audiences +1. Multi-audience mode (default): Validates MCP audience per RFC 7519 (resource servers + validate only their own audience). Nextcloud independently validates its own audience. 2. Token exchange mode (opt-in): Tokens have MCP audience only, exchanged for Nextcloud tokens Key Design Principles: -- Token verification happens HERE (validates audiences) +- Token verification happens HERE (validates MCP audience per OAuth spec) - Token exchange happens in context_helper.py (when creating NextcloudClient) - No token passthrough allowed (complies with MCP Security Specification) +- Token reuse IS allowed for multi-audience tokens (RFC 8707) """ import hashlib @@ -39,8 +41,9 @@ class UnifiedTokenVerifier(TokenVerifier): 3. Caches successful validations to avoid repeated API calls Mode Selection (via ENABLE_TOKEN_EXCHANGE setting): - - False/omit (default): Multi-audience mode - requires BOTH MCP and Nextcloud audiences - - True: Exchange mode - requires MCP audience only (exchange happens later) + - False/omit (default): Multi-audience mode - validates MCP audience only (per RFC 7519). + Nextcloud independently validates its own audience when receiving API calls. + - True: Exchange mode - requires MCP audience only, then exchanges for Nextcloud token """ def __init__(self, settings: Settings): @@ -90,7 +93,7 @@ class UnifiedTokenVerifier(TokenVerifier): CRITICAL: This method only validates tokens - it does NOT perform exchange. Token exchange happens later in context_helper.py when creating NextcloudClient. - Multi-audience mode: Validates token has BOTH MCP and Nextcloud audiences + Multi-audience mode: Validates token has MCP audience (per RFC 7519) Exchange mode: Validates token has MCP audience ONLY (exchange happens later) Args: @@ -115,14 +118,17 @@ class UnifiedTokenVerifier(TokenVerifier): async def _verify_multi_audience_token(self, token: str) -> AccessToken | None: """ - Validate token has both MCP and Nextcloud audiences (Mode 1). + Validate token has MCP audience (Mode 1). Token can be used directly without exchange. + Per RFC 7519, we only validate our own (MCP) audience. Nextcloud will + independently validate its own audience when it receives the token. + Args: token: Bearer token to verify Returns: - AccessToken if valid with both audiences, None otherwise + AccessToken if valid with MCP audience, None otherwise """ try: # Attempt JWT verification first @@ -138,19 +144,18 @@ class UnifiedTokenVerifier(TokenVerifier): if not payload: return None - # Validate both audiences are present + # Validate MCP audience is present if not self._validate_multi_audience(payload): audiences = payload.get("aud", []) logger.error( - f"Token rejected: Missing required audiences. " - f"Got {audiences}, need both MCP ({self.settings.oidc_client_id} or " - f"{self.settings.nextcloud_mcp_server_url}) AND Nextcloud " - f"({self.settings.nextcloud_resource_uri})" + f"Token rejected: Missing MCP audience. " + f"Got {audiences}, need MCP ({self.settings.oidc_client_id} or " + f"{self.settings.nextcloud_mcp_server_url})" ) return None logger.info( - "Multi-audience validation passed - token has both MCP and Nextcloud audiences" + "MCP audience validation passed - token authorized for MCP server" ) return self._create_access_token(token, payload) @@ -204,13 +209,20 @@ class UnifiedTokenVerifier(TokenVerifier): def _validate_multi_audience(self, payload: dict[str, Any]) -> bool: """ - Check if token has both MCP and Nextcloud audiences. + Check if token has MCP audience. + + Per RFC 7519 Section 4.1.3, resource servers should only validate their own + presence in the audience claim. We don't validate Nextcloud's audience - that's + Nextcloud's responsibility when it receives the token. + + This is NOT token passthrough (we validate the token). This IS token reuse + which is allowed by RFC 8707 for multi-audience tokens between trusted services. Args: payload: Decoded token payload Returns: - True if both audiences present, False otherwise + True if MCP audience present, False otherwise """ audiences = payload.get("aud", []) if isinstance(audiences, str): @@ -227,13 +239,7 @@ class UnifiedTokenVerifier(TokenVerifier): ) ) - # Nextcloud must have its resource URI - nextcloud_valid = bool( - self.settings.nextcloud_resource_uri - and self.settings.nextcloud_resource_uri in audiences_set - ) - - return bool(mcp_valid and nextcloud_valid) + return bool(mcp_valid) def _has_mcp_audience(self, payload: dict[str, Any]) -> bool: """ diff --git a/nextcloud_mcp_server/context.py b/nextcloud_mcp_server/context.py index f3e86d6..505274b 100644 --- a/nextcloud_mcp_server/context.py +++ b/nextcloud_mcp_server/context.py @@ -66,7 +66,8 @@ async def get_client(ctx: Context) -> NextcloudClient: ) else: # Mode 1: Multi-audience token - use directly - # Token was validated to have BOTH audiences in UnifiedTokenVerifier + # Token was validated to have MCP audience in UnifiedTokenVerifier + # Nextcloud will independently validate its own audience when receiving API calls return get_client_from_context(ctx, lifespan_ctx.nextcloud_host) # Unknown context type diff --git a/tests/unit/test_unified_verifier.py b/tests/unit/test_unified_verifier.py index cb1c639..08d2ec8 100644 --- a/tests/unit/test_unified_verifier.py +++ b/tests/unit/test_unified_verifier.py @@ -94,7 +94,7 @@ class TestAudienceValidation: assert verifier._validate_multi_audience(payload) is False def test_validate_multi_audience_missing_nextcloud(self, base_settings): - """Test multi-audience validation fails without Nextcloud audience.""" + """Test multi-audience validation succeeds with only MCP audience (RFC 7519 compliant).""" verifier = UnifiedTokenVerifier(base_settings) payload = { "aud": ["test-client-id"], # Only MCP @@ -102,10 +102,11 @@ class TestAudienceValidation: "exp": int(time.time() + 3600), } - assert verifier._validate_multi_audience(payload) is False + # Per RFC 7519, we only validate MCP audience. Nextcloud validates its own. + assert verifier._validate_multi_audience(payload) is True def test_validate_multi_audience_string_audience(self, base_settings): - """Test multi-audience validation with string audience (should still work).""" + """Test multi-audience validation with string audience works (RFC 7519 compliant).""" verifier = UnifiedTokenVerifier(base_settings) payload = { "aud": "test-client-id", # Single audience as string @@ -113,8 +114,8 @@ class TestAudienceValidation: "exp": int(time.time() + 3600), } - # Should fail - needs both audiences - assert verifier._validate_multi_audience(payload) is False + # Should pass - we only validate MCP audience per RFC 7519 + assert verifier._validate_multi_audience(payload) is True def test_has_mcp_audience_with_client_id(self, exchange_settings): """Test MCP audience validation with client ID.""" @@ -266,14 +267,16 @@ class TestMultiAudienceVerification: async def test_verify_multi_audience_fails_without_both_audiences( self, base_settings ): - """Test multi-audience verification fails without both audiences.""" + """Test multi-audience verification succeeds with only MCP audience (RFC 7519 compliant).""" verifier = UnifiedTokenVerifier(base_settings) - # Mock introspection response with only one audience + # Mock introspection response with only MCP audience introspection_response = { "active": True, "sub": "testuser", - "aud": ["test-client-id"], # Missing Nextcloud audience + "aud": [ + "test-client-id" + ], # Only MCP audience (Nextcloud validates its own) "scope": "openid profile", "exp": int(time.time() + 3600), } @@ -284,7 +287,9 @@ class TestMultiAudienceVerification: opaque_token = "opaque-token-12345" result = await verifier._verify_multi_audience_token(opaque_token) - assert result is None + # Should succeed with only MCP audience per RFC 7519 + assert result is not None + assert result.resource == "testuser" class TestExchangeModeVerification: