diff --git a/CLAUDE.md b/CLAUDE.md index 8be7445..1d4ddde 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -421,7 +421,7 @@ curl http://localhost:8888/realms/nextcloud-mcp/.well-known/openid-configuration # 3. Verify user_oidc provider is configured docker compose exec app php occ user_oidc:provider keycloak -# 4. Generate encryption key for refresh token storage (optional, for ADR-002 Tier 1) +# 4. Generate encryption key for refresh token storage (optional, for offline access) python -c "from cryptography.fernet import Fernet; print(Fernet.generate_key().decode())" # Set in environment: export TOKEN_ENCRYPTION_KEY='' @@ -507,13 +507,15 @@ docker compose exec app curl http://keycloak:8080/realms/nextcloud-mcp/.well-kno ``` **ADR-002 Offline Access Testing:** -The Keycloak integration enables testing ADR-002 Tier 1 (offline access with refresh tokens): +The Keycloak integration enables testing ADR-002's primary authentication pattern (offline access with refresh tokens): 1. **Refresh token storage**: Tokens stored encrypted in SQLite (`/app/data/tokens.db`) 2. **Token refresh**: Access tokens refreshed automatically when expired 3. **Background workers**: Can access APIs using stored refresh tokens 4. **No admin credentials**: All operations use user's OAuth tokens +**Note**: Service account tokens (client_credentials grant) were considered but rejected as they create Nextcloud user accounts and violate OAuth "act on-behalf-of" principles. See ADR-002 "Will Not Implement" section. + See `docs/ADR-002-vector-sync-authentication.md` for architectural details. **Audience Validation:** diff --git a/docs/ADR-002-vector-sync-authentication.md b/docs/ADR-002-vector-sync-authentication.md index 42b5b2f..c2ca4f1 100644 --- a/docs/ADR-002-vector-sync-authentication.md +++ b/docs/ADR-002-vector-sync-authentication.md @@ -1,7 +1,9 @@ # ADR-002: Vector Database Background Sync Authentication ## Status -Accepted - Tier 2 (Token Exchange) Implemented +Accepted - Tier 2 (Token Exchange with Delegation) Implemented + +**Important**: Service account tokens (old Tier 1) have been rejected as they violate OAuth "act on-behalf-of" principles by creating Nextcloud user accounts for the MCP server. ## Context @@ -43,26 +45,28 @@ We will implement a **tiered OAuth authentication strategy** for background oper **Note**: This ADR applies only to **OAuth mode**. In BasicAuth mode (single-user deployments), credentials are already available via environment variables, and background operations work without additional configuration. -### Tier 1: Service Account Token (client_credentials) ✅ **IMPLEMENTED** +### OAuth "Act On-Behalf-Of" Principle -**Most Compatible Option** - Works with all OIDC providers supporting `client_credentials` +**Core Requirement**: The MCP server must NEVER create its own user identity in Nextcloud when operating in OAuth mode. -- MCP server obtains service account token via `client_credentials` grant -- Background worker uses service account token directly -- No user-specific delegation or impersonation -- **Implementation**: `KeycloakOAuthClient.get_service_account_token()` (keycloak_oauth.py:341-395) -- **Testing**: Manual test in `tests/manual/test_token_exchange.py` -- **TODO**: Automated integration tests needed for both Keycloak and Nextcloud OIDC app +**Valid Patterns**: +- ✅ **Foreground operations**: Use user's access token from MCP request (currently implemented) +- ✅ **Background operations**: Token exchange to impersonate/delegate as user (requires provider support) +- ❌ **Service account**: Creates independent identity in Nextcloud (violates OAuth principles) -**Trade-offs**: -- ✅ Works with nearly all OIDC providers -- ✅ Simple implementation and configuration -- ✅ No additional provider features required -- ❌ Service account needs broad permissions across users -- ❌ Less granular audit trail (all actions attributed to service account) -- ❌ No per-user permission enforcement +**Why This Matters**: +1. **Audit Trail**: All operations must be attributable to the actual user, not a service account +2. **Stateless Server**: MCP server should not have persistent identity/state in Nextcloud +3. **Security Model**: Avoid creating "admin by another name" with broad cross-user permissions +4. **OAuth Design**: OAuth tokens represent user authorization, not server authorization -### Tier 2: Token Exchange with Impersonation (RFC 8693) ⚠️ **NOT IMPLEMENTED** +**If Token Exchange Not Available**: +- Background operations simply cannot happen in OAuth mode +- This is correct behavior - not a limitation to work around +- Don't create service accounts as "workaround" - this defeats OAuth's purpose +- Use BasicAuth mode if background operations are critical to your deployment + +### Tier 1: Token Exchange with Impersonation (RFC 8693) ⚠️ **NOT IMPLEMENTED** **Better Security** - Requires provider support for user impersonation @@ -79,7 +83,7 @@ We will implement a **tiered OAuth authentication strategy** for background oper **Status**: ⚠️ Not implemented - Keycloak Standard V2 doesn't support impersonation **Reference**: See `docs/oauth-impersonation-findings.md` for investigation details -### Tier 3: Token Exchange with Delegation (RFC 8693) ✅ **IMPLEMENTED** +### Tier 2: Token Exchange with Delegation (RFC 8693) ✅ **IMPLEMENTED** **Best Security** - Requires provider support for delegation with `act` claim @@ -100,13 +104,26 @@ We will implement a **tiered OAuth authentication strategy** for background oper ### ❌ Will Not Implement -**1. Offline Access with Refresh Tokens** +**1. Service Account with Independent Identity (client_credentials)** +- **Status**: Previously proposed as Tier 1, now rejected +- **Why Invalid**: Creates Nextcloud user account for MCP server (e.g., `service-account-nextcloud-mcp-server`) +- **Problems**: + - **Violates OAuth "act on-behalf-of" principle**: Actions attributed to service account instead of real user + - **Breaks audit trail**: Can't determine which user initiated the action + - **Creates stateful server identity**: MCP server has persistent identity/data in Nextcloud + - **Security risk**: Service account becomes "admin by another name" with broad cross-user permissions + - **User provisioning side effect**: Nextcloud's `user_oidc` app auto-provisions service account as real user +- **Code Status**: Implementation exists (`KeycloakOAuthClient.get_service_account_token()`) but marked with warnings +- **Alternative**: If service account pattern truly needed, use BasicAuth mode instead of OAuth mode +- **Reference**: See commit c12df98 for detailed analysis of why this approach was rejected + +**2. Offline Access with Refresh Tokens** - **MCP Protocol Architecture**: FastMCP SDK manages OAuth where MCP Client handles refresh tokens - **Security Model**: Refresh tokens must never be shared between client and server (OAuth best practice) - **Technical Impossibility**: MCP Server has no access to refresh tokens from the OAuth callback - **Alternative**: Token exchange provides similar benefits without violating OAuth security model -**2. Admin Credentials Fallback** +**3. Admin Credentials Fallback** - **Out of Scope**: This ADR focuses on OAuth mode only - **Not Appropriate**: Admin credentials bypass OAuth security model - **BasicAuth Mode**: For single-user deployments needing background operations, use BasicAuth mode instead @@ -122,50 +139,11 @@ We will implement a **tiered OAuth authentication strategy** for background oper ## Implementation Details -### 1. Service Account Token (Tier 1 - Primary) ✅ IMPLEMENTED - -#### 1.1 Service Account Token Acquisition -```python -async def get_service_token() -> str: - """Get token for MCP server's service account""" - - async with httpx.AsyncClient() as client: - response = await client.post( - token_endpoint, - data={ - "grant_type": "client_credentials", - "scope": "notes:read files:read calendar:read" - }, - auth=(client_id, client_secret) - ) - response.raise_for_status() - return response.json()["access_token"] -``` - -**Implementation**: `KeycloakOAuthClient.get_service_account_token()` (keycloak_oauth.py:341-395) - -**Usage**: -```python -# Background worker uses service account token directly -service_token_data = await oauth_client.get_service_account_token( - scopes=["notes:read", "files:read", "calendar:read"] -) - -client = NextcloudClient.from_token( - base_url=nextcloud_host, - token=service_token_data["access_token"], - username="service-account" -) - -# All operations are performed as the service account -notes = await client.notes.list_notes() -``` - -### 2. Token Exchange with Impersonation (Tier 2) ⚠️ NOT IMPLEMENTED +### 1. Token Exchange with Impersonation (Tier 1) ⚠️ NOT IMPLEMENTED This tier is documented for completeness but is not currently implemented due to lack of provider support. -#### 2.1 Impersonation Flow (Conceptual) +#### 1.1 Impersonation Flow (Conceptual) ```python async def exchange_for_impersonated_user_token( @@ -201,9 +179,9 @@ async def exchange_for_impersonated_user_token( **See**: `docs/oauth-impersonation-findings.md` for detailed investigation -### 3. Token Exchange with Delegation (Tier 3) ✅ IMPLEMENTED +### 2. Token Exchange with Delegation (Tier 2) ✅ IMPLEMENTED -#### 3.1 Capability Detection +#### 2.1 Capability Detection ```python async def check_token_exchange_support(discovery_url: str) -> bool: """Check if OIDC provider supports RFC 8693 token exchange""" @@ -217,7 +195,7 @@ async def check_token_exchange_support(discovery_url: str) -> bool: return "urn:ietf:params:oauth:grant-type:token-exchange" in grant_types ``` -#### 3.2 Delegation Token Exchange +#### 2.2 Delegation Token Exchange ```python async def exchange_for_user_token( service_token: str, diff --git a/docs/audience-validation-setup.md b/docs/audience-validation-setup.md index 2786ec9..4d40125 100644 --- a/docs/audience-validation-setup.md +++ b/docs/audience-validation-setup.md @@ -389,9 +389,11 @@ Security: Refresh tokens stored encrypted, rotated on use ## Authentication Strategies for Background Jobs -### Current Approach: Offline Access with Refresh Tokens (Tier 1) +> **Note on Service Account Tokens**: Service account tokens (`client_credentials` grant) were evaluated but **rejected** as they create Nextcloud user accounts (e.g., `service-account-{client_id}`) which violates OAuth "act on-behalf-of" principles. See ADR-002 "Will Not Implement" section for details. -The MCP server currently uses **offline_access** scope to enable background operations: +### Current Approach: Offline Access with Refresh Tokens + +The MCP server uses **offline_access** scope to enable background operations: **How it works:** 1. User grants `offline_access` scope during OAuth consent @@ -412,7 +414,7 @@ The MCP server currently uses **offline_access** scope to enable background oper - ⚠️ Weak audit trail - API requests appear to come from user directly - ⚠️ No visibility that MCP Server is the actual actor -### Future Enhancement: Token Exchange with Delegation (Tier 2) +### Token Exchange with Delegation (ADR-002 Tier 2 - Implemented) **RFC 8693 Delegation** would provide better audit trail and security: diff --git a/docs/oauth-impersonation-findings.md b/docs/oauth-impersonation-findings.md index aae9927..0421c14 100644 --- a/docs/oauth-impersonation-findings.md +++ b/docs/oauth-impersonation-findings.md @@ -5,6 +5,23 @@ **Status**: Implementation Complete - Token Exchange Working **Conclusion**: Keycloak Standard Token Exchange (RFC 8693) working for internal-to-internal token exchange. User impersonation requires Legacy V1. +--- + +## ⚠️ IMPORTANT UPDATE (2025-11-02) + +**This document contains outdated information regarding service account tokens.** + +After implementation and testing, we discovered that service account tokens (`client_credentials` grant) **violate OAuth "act on-behalf-of" principles** by creating Nextcloud user accounts (e.g., `service-account-nextcloud-mcp-server`). This approach has been **REJECTED** and moved to ADR-002's "Will Not Implement" section. + +**Key Changes:** +- ❌ **Service account tokens (client_credentials) are INVALID** - Creates user accounts, breaks audit trail +- ✅ **Token exchange (RFC 8693) is the correct approach** - Implemented and working (ADR-002 Tier 2) +- ✅ **Offline access with refresh tokens** - Still valid for background operations (ADR-002 primary approach) + +**For current architecture, see**: `docs/ADR-002-vector-sync-authentication.md` + +--- + ## Summary We investigated options for implementing user impersonation to enable background operations without requiring admin credentials (ADR-002 Tier 2). Here are the findings: diff --git a/nextcloud_mcp_server/auth/keycloak_oauth.py b/nextcloud_mcp_server/auth/keycloak_oauth.py index c3abe99..38ffd34 100644 --- a/nextcloud_mcp_server/auth/keycloak_oauth.py +++ b/nextcloud_mcp_server/auth/keycloak_oauth.py @@ -342,9 +342,27 @@ class KeycloakOAuthClient: """ Get a service account token using client_credentials grant. - This requires the client to have serviceAccountsEnabled=true in Keycloak. - The service account token can be used for server-initiated operations - or as the subject_token for token exchange. + ⚠️ **WARNING: DO NOT USE FOR DIRECT API ACCESS IN OAUTH MODE** ⚠️ + + This method creates a service account user in Nextcloud which VIOLATES + OAuth "act on-behalf-of" principles. Using this token directly for API + access will: + - Create a Nextcloud user: `service-account-{client_id}` + - Attribute all actions to service account instead of real user + - Break audit trail and user attribution + - Create stateful server identity in Nextcloud + - Violate OAuth security model + + **Valid Use Case**: ONLY as subject_token for RFC 8693 token exchange + (ADR-002 Tier 2) where it's immediately exchanged for a user token. + + **Invalid Use Case**: Direct API access with this token (ADR-002 rejected + this as "Tier 1" - see docs/ADR-002-vector-sync-authentication.md). + + **Alternative**: Use token exchange (impersonation/delegation) for + background operations, or use BasicAuth mode if truly need service account. + + This requires the client to have serviceAccountsEnabled=true in provider. Args: scopes: Optional list of scopes to request (default: openid profile email) @@ -359,9 +377,9 @@ class KeycloakOAuthClient: Raises: httpx.HTTPError: If token request fails - Note: - This is used for ADR-002 Tier 2 (Token Exchange). The service account - token is exchanged for user-scoped tokens via RFC 8693. + See Also: + - ADR-002 "Will Not Implement" section for detailed critique + - exchange_token_for_user() for proper token exchange usage """ if not self.token_endpoint: await self.discover() diff --git a/tests/manual/test_token_exchange.py b/tests/manual/test_token_exchange.py index 139d142..3c4b585 100644 --- a/tests/manual/test_token_exchange.py +++ b/tests/manual/test_token_exchange.py @@ -83,6 +83,16 @@ async def main(): logger.info("") # Step 3: Get service account token + # ⚠️ WARNING: Service account tokens MUST NOT be used directly with Nextcloud APIs! + # Using this token directly violates OAuth "act on-behalf-of" principles: + # - Creates Nextcloud user: service-account-{client_id} + # - Breaks audit trail (actions not attributable to real user) + # - Creates stateful server identity in Nextcloud + # + # VALID USE: ONLY as subject_token for RFC 8693 token exchange (Step 4 below) + # INVALID USE: Direct API access (see ADR-002 "Will Not Implement" section) + # + # If you need background operations without token exchange support, use BasicAuth mode. logger.info("Step 3: Requesting service account token (client_credentials)...") try: service_token_response = await oauth_client.get_service_account_token(