From 95b73019abcca0726eb3ec041ada14b4426397d6 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Mon, 3 Nov 2025 20:31:39 +0100 Subject: [PATCH] fix: make ENABLE_PROGRESSIVE_CONSENT consistently opt-in (default false) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes inconsistent default values for ENABLE_PROGRESSIVE_CONSENT across the codebase. Previously had contradictory defaults (true in 4 files, false in 5). Also removes the confusing REQUIRE_PROVISIONING variable. Changes: - app.py (2 locations): Changed default from "true" to "false" - oauth_routes.py (2 locations): Changed default from "true" to "false" - provisioning_decorator.py: Replaced REQUIRE_PROVISIONING with ENABLE_PROGRESSIVE_CONSENT - Updated docstrings to clarify Progressive Consent is opt-in - CLAUDE.md: Added comprehensive Progressive Consent documentation Progressive Consent Mode (opt-in): - Enable with ENABLE_PROGRESSIVE_CONSENT=true - Dual OAuth flows: Flow 1 (client auth) + Flow 2 (resource provisioning) - Flow 2 requires separate login outside MCP session - Provides separation between session tokens and background job tokens Default (Hybrid Flow): - Single OAuth flow with server interception - Backward compatible with existing deployments - No separate provisioning step required Testing: - All 5 smoke tests passing (including OAuth) - All 36 unit tests passing 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- CLAUDE.md | 27 +++++++++++++++++++ nextcloud_mcp_server/app.py | 6 ++--- nextcloud_mcp_server/auth/oauth_routes.py | 26 ++++++++++-------- .../auth/provisioning_decorator.py | 14 +++++----- 4 files changed, 52 insertions(+), 21 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 3716d15..4203c31 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -165,6 +165,33 @@ docker compose exec db mariadb -u root -ppassword nextcloud -e \ 3. MCP tools use context pattern: `get_client(ctx)` → `NextcloudClient` 4. All operations are async using httpx +### Progressive Consent Mode (ADR-004) + +**Status**: Opt-in feature (disabled by default) + +**Enable**: Set `ENABLE_PROGRESSIVE_CONSENT=true` + +**Default**: Hybrid Flow (backward compatible, single OAuth flow) + +**What is Progressive Consent?** +- Dual OAuth flow architecture that separates client authentication (Flow 1) from resource provisioning (Flow 2) +- Flow 1: MCP client authenticates directly to IdP (aud: "mcp-server") +- Flow 2: User explicitly provisions Nextcloud access via separate login (not during MCP session) +- Provides clear separation between session tokens and background job tokens + +**When to use:** +- Background jobs requiring offline access +- Enhanced security with separate authorization contexts +- Explicit user control over resource access + +**When NOT to use:** +- Simple single-user deployments (use BasicAuth) +- Standard OAuth without background jobs (use default Hybrid Flow) + +**Key difference from Hybrid Flow:** +- Hybrid Flow: Server intercepts OAuth callback, stores refresh token automatically +- Progressive Consent: User explicitly authorizes via `provision_nextcloud_access` tool + ## MCP Response Patterns (CRITICAL) **Never return raw `List[Dict]` from MCP tools** - FastMCP mangles them into dicts with numeric string keys. diff --git a/nextcloud_mcp_server/app.py b/nextcloud_mcp_server/app.py index 4fa06ff..462bbab 100644 --- a/nextcloud_mcp_server/app.py +++ b/nextcloud_mcp_server/app.py @@ -564,9 +564,9 @@ async def setup_oauth_config(): jwt_validation_issuer = issuer client_issuer = issuer - # Check if Progressive Consent mode is enabled + # Check if Progressive Consent mode is enabled (opt-in, defaults to false) enable_progressive = ( - os.getenv("ENABLE_PROGRESSIVE_CONSENT", "true").lower() == "true" + os.getenv("ENABLE_PROGRESSIVE_CONSENT", "false").lower() == "true" ) # Create token verifier @@ -814,7 +814,7 @@ def get_app(transport: str = "sse", enabled_apps: list[str] | None = None): # Register OAuth provisioning tools if in OAuth mode with Progressive Consent if oauth_enabled: enable_progressive = ( - os.getenv("ENABLE_PROGRESSIVE_CONSENT", "true").lower() == "true" + os.getenv("ENABLE_PROGRESSIVE_CONSENT", "false").lower() == "true" ) if enable_progressive: logger.info("Registering OAuth provisioning tools for Progressive Consent") diff --git a/nextcloud_mcp_server/auth/oauth_routes.py b/nextcloud_mcp_server/auth/oauth_routes.py index 9c40209..b2223c1 100644 --- a/nextcloud_mcp_server/auth/oauth_routes.py +++ b/nextcloud_mcp_server/auth/oauth_routes.py @@ -2,12 +2,13 @@ OAuth 2.0 Login Routes for ADR-004 Progressive Consent Architecture Implements OAuth endpoints that support both: -1. Hybrid Flow (backward compatible) - Single OAuth flow with server interception -2. Progressive Consent (ADR-004) - Dual OAuth flows with explicit provisioning +1. Hybrid Flow (default, backward compatible) - Single OAuth flow with server interception +2. Progressive Consent (opt-in via ENABLE_PROGRESSIVE_CONSENT=true) - Dual OAuth flows with explicit provisioning -Progressive Consent Mode (when ENABLE_PROGRESSIVE_CONSENT=true): +Progressive Consent Mode (opt-in, requires separate login): +- Enable with ENABLE_PROGRESSIVE_CONSENT=true - Flow 1: Client Authentication - MCP client authenticates directly to IdP -- Flow 2: Resource Provisioning - MCP server gets delegated Nextcloud access +- Flow 2: Resource Provisioning - MCP server gets delegated Nextcloud access (separate login, not during MCP session) Hybrid Flow Mode (default, backward compatible): 1. MCP client initiates OAuth at /oauth/authorize @@ -39,9 +40,9 @@ async def oauth_authorize(request: Request) -> RedirectResponse | JSONResponse: """ OAuth authorization endpoint with PKCE support. - Supports both Hybrid Flow (default) and Progressive Consent Flow 1. + Supports both Hybrid Flow (default) and Progressive Consent Flow 1 (opt-in). - In Progressive Consent mode (ENABLE_PROGRESSIVE_CONSENT=true): + In Progressive Consent mode (opt-in, ENABLE_PROGRESSIVE_CONSENT=true): - Flow 1: Client authenticates directly to IdP with its own client_id - Server validates client_id is in ALLOWED_MCP_CLIENTS list - Issues tokens with aud: "mcp-server" for MCP authentication only @@ -61,9 +62,9 @@ async def oauth_authorize(request: Request) -> RedirectResponse | JSONResponse: Returns: 302 redirect to IdP authorization endpoint """ - # Check if Progressive Consent is enabled (default: true for ADR-004) + # Check if Progressive Consent is enabled (opt-in, defaults to false) enable_progressive = ( - os.getenv("ENABLE_PROGRESSIVE_CONSENT", "true").lower() == "true" + os.getenv("ENABLE_PROGRESSIVE_CONSENT", "false").lower() == "true" ) # Extract parameters @@ -635,7 +636,10 @@ async def oauth_authorize_nextcloud( OAuth authorization endpoint for Flow 2: Resource Provisioning. This endpoint is used by the provision_nextcloud_access MCP tool - to initiate delegated resource access to Nextcloud. + to initiate delegated resource access to Nextcloud. Requires a separate + login flow outside of the MCP session. + + Only available when Progressive Consent is enabled (opt-in). Query parameters: state: Session state for tracking @@ -643,9 +647,9 @@ async def oauth_authorize_nextcloud( Returns: 302 redirect to IdP authorization endpoint """ - # Check if Progressive Consent is enabled (default: true for ADR-004) + # Check if Progressive Consent is enabled (opt-in, defaults to false) enable_progressive = ( - os.getenv("ENABLE_PROGRESSIVE_CONSENT", "true").lower() == "true" + os.getenv("ENABLE_PROGRESSIVE_CONSENT", "false").lower() == "true" ) if not enable_progressive: return JSONResponse( diff --git a/nextcloud_mcp_server/auth/provisioning_decorator.py b/nextcloud_mcp_server/auth/provisioning_decorator.py index d9d18d1..b531b13 100644 --- a/nextcloud_mcp_server/auth/provisioning_decorator.py +++ b/nextcloud_mcp_server/auth/provisioning_decorator.py @@ -63,20 +63,20 @@ def require_provisioning(func: Callable) -> Callable: logger.debug("BasicAuth mode detected - skipping provisioning check") return await func(*args, **kwargs) - # Check if provisioning is required (opt-in, defaults to false) - # Provisioning is only needed when using Progressive Consent with Flow 2 + # Check if Progressive Consent is enabled (opt-in, defaults to false) + # Provisioning checks only apply when using Progressive Consent Flow 2 import os - require_provisioning = ( - os.getenv("REQUIRE_PROVISIONING", "false").lower() == "true" + enable_progressive = ( + os.getenv("ENABLE_PROGRESSIVE_CONSENT", "false").lower() == "true" ) - if not require_provisioning: + if not enable_progressive: logger.debug( - "Provisioning not required (REQUIRE_PROVISIONING=false) - skipping check" + "Progressive Consent disabled (ENABLE_PROGRESSIVE_CONSENT=false) - skipping provisioning check" ) return await func(*args, **kwargs) - # OAuth mode with provisioning required - check provisioning status + # Progressive Consent mode - check if user has completed Flow 2 provisioning # Get user_id from authorization token user_id = None if hasattr(ctx, "authorization") and ctx.authorization: