feat: add real elicitation integration test with python-sdk MCP client
This commit adds proper integration testing of the login elicitation flow (ADR-006) using python-sdk's MCP client with actual elicitation callback support, and fixes user_id extraction to support both JWT and opaque tokens. ## Changes ### 1. Enhanced create_mcp_client_session helper (tests/conftest.py) - Added `elicitation_callback` parameter to function signature - Pass callback to ClientSession constructor - Added necessary imports: RequestContext, ElicitRequestParams, ElicitResult, ErrorData from mcp package - Allows fixtures to provide custom elicitation handlers ### 2. New fixture: nc_mcp_oauth_client_with_elicitation (tests/conftest.py) - Creates MCP client with Playwright-based elicitation callback - Callback implementation: - Extracts OAuth URL from elicitation message using regex - Uses Playwright browser to complete OAuth flow automatically - Handles Nextcloud login form (username/password) - Handles consent screen if present - Waits for OAuth callback completion - Returns ElicitResult(action="accept") on success - Function-scoped to allow independent test state - Tracks elicitation invocations via session.elicitation_triggered ### 3. Fixed user_id extraction for opaque tokens (oauth_tools.py) - Created extract_user_id_from_token() helper to handle both JWT and opaque tokens by calling userinfo endpoint when needed - Fixed check_logged_in to use helper instead of broken ctx.authorization - Fixed revoke_nextcloud_access to use helper instead of ctx.context.get() - Both tools now properly extract user_id from access tokens ### 4. Enhanced integration tests (test_elicitation_integration.py) - Updated tests to revoke refresh tokens via MCP tool - All 4 tests now pass: - test_check_logged_in_with_real_elicitation_callback: Complete flow - test_elicitation_callback_url_extraction: URL extraction validation - test_elicitation_stores_refresh_token: Token persistence verification - test_second_check_logged_in_does_not_elicit: No redundant elicitations ### 5. Added diagnostic logging (oauth_routes.py) - Track user_id extraction from ID tokens during OAuth callbacks - Log refresh token storage with user_id and flow_type ## Test Results ✅ 4/4 tests pass The test suite successfully validates: - Elicitation callback is triggered when no refresh token exists - Playwright automation completes OAuth flow - Refresh token is stored after OAuth with correct user_id - Tool returns "yes" after successful login - Already-logged-in users don't get redundant elicitations ## Why This Matters Previous tests (test_login_elicitation.py) only validated response formats and acknowledged they couldn't test real elicitation protocol. This test exercises the REAL MCP elicitation protocol end-to-end: 1. MCP server calls ctx.elicit() 2. python-sdk ClientSession invokes custom callback 3. Callback completes OAuth via Playwright 4. Client returns acceptance to server 5. Tool proceeds with authenticated state This proves the python-sdk MCP client can handle elicitation in production environments with both JWT and opaque tokens. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Chris Coutinho
@@ -493,14 +493,22 @@ async def oauth_callback_nextcloud(request: Request):
|
||||
id_token = token_data.get("id_token")
|
||||
|
||||
# Decode ID token to get user info
|
||||
logger.info("=" * 60)
|
||||
logger.info("oauth_callback_nextcloud: Extracting user_id from ID token")
|
||||
logger.info("=" * 60)
|
||||
try:
|
||||
userinfo = jwt.decode(id_token, options={"verify_signature": False})
|
||||
user_id = userinfo.get("sub")
|
||||
username = userinfo.get("preferred_username") or userinfo.get("email")
|
||||
logger.info(" ✓ ID token decode SUCCESSFUL")
|
||||
logger.info(f" Extracted user_id: {user_id}")
|
||||
logger.info(f" Username: {username}")
|
||||
logger.info(f" ID token payload keys: {list(userinfo.keys())}")
|
||||
logger.info(f"Flow 2: User {username} provisioned resource access")
|
||||
except Exception as e:
|
||||
logger.warning(f"Failed to decode ID token: {e}")
|
||||
logger.error(f" ✗ ID token decode FAILED: {type(e).__name__}: {e}")
|
||||
user_id = "unknown"
|
||||
logger.error(f" Using fallback user_id: {user_id}")
|
||||
|
||||
# Store master refresh token for Flow 2
|
||||
if refresh_token:
|
||||
@@ -509,6 +517,13 @@ async def oauth_callback_nextcloud(request: Request):
|
||||
token_data.get("scope", "").split() if token_data.get("scope") else None
|
||||
)
|
||||
|
||||
logger.info("Storing refresh token:")
|
||||
logger.info(f" user_id: {user_id}")
|
||||
logger.info(" flow_type: flow2")
|
||||
logger.info(" token_audience: nextcloud")
|
||||
logger.info(f" provisioning_client_id: {state[:16]}...")
|
||||
logger.info(f" scopes: {granted_scopes}")
|
||||
|
||||
await storage.store_refresh_token(
|
||||
user_id=user_id,
|
||||
refresh_token=refresh_token,
|
||||
@@ -518,7 +533,8 @@ async def oauth_callback_nextcloud(request: Request):
|
||||
scopes=granted_scopes,
|
||||
expires_at=None, # Refresh tokens typically don't expire
|
||||
)
|
||||
logger.info(f"Stored Flow 2 master refresh token for user {user_id}")
|
||||
logger.info(f"✓ Stored Flow 2 master refresh token for user {user_id}")
|
||||
logger.info("=" * 60)
|
||||
|
||||
# Return success HTML page
|
||||
success_html = """
|
||||
|
||||
@@ -11,16 +11,88 @@ import secrets
|
||||
from typing import Optional
|
||||
from urllib.parse import urlencode
|
||||
|
||||
import httpx
|
||||
from mcp.server.auth.middleware.auth_context import get_access_token
|
||||
from mcp.server.auth.provider import AccessToken
|
||||
from mcp.server.fastmcp import Context
|
||||
from pydantic import BaseModel, Field
|
||||
|
||||
from nextcloud_mcp_server.auth import require_scopes
|
||||
from nextcloud_mcp_server.auth.refresh_token_storage import RefreshTokenStorage
|
||||
from nextcloud_mcp_server.auth.token_broker import TokenBrokerService
|
||||
from nextcloud_mcp_server.auth.userinfo_routes import _query_idp_userinfo
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
async def extract_user_id_from_token(ctx: Context) -> str:
|
||||
"""Extract user_id from the MCP access token (Flow 1).
|
||||
|
||||
Handles both JWT and opaque tokens:
|
||||
- JWT: Decode and extract 'sub' claim
|
||||
- Opaque: Call userinfo endpoint to get 'sub'
|
||||
|
||||
Args:
|
||||
ctx: MCP context with access token
|
||||
|
||||
Returns:
|
||||
user_id extracted from token, or "default_user" as fallback
|
||||
"""
|
||||
# Use MCP SDK's get_access_token() which uses contextvars
|
||||
access_token: AccessToken | None = get_access_token()
|
||||
|
||||
if not access_token or not access_token.token:
|
||||
logger.warning(" ✗ No access token found via get_access_token()")
|
||||
return "default_user"
|
||||
|
||||
token = access_token.token
|
||||
is_jwt = "." in token and token.count(".") >= 2
|
||||
logger.info(f" Token type: {'JWT' if is_jwt else 'Opaque'}")
|
||||
|
||||
# Try JWT decode first
|
||||
if is_jwt:
|
||||
try:
|
||||
import jwt
|
||||
|
||||
payload = jwt.decode(token, options={"verify_signature": False})
|
||||
user_id = payload.get("sub", "unknown")
|
||||
logger.info(f" ✓ JWT decode successful: user_id={user_id}")
|
||||
return user_id
|
||||
except Exception as e:
|
||||
logger.error(f" ✗ JWT decode failed: {type(e).__name__}: {e}")
|
||||
|
||||
# Opaque token - call userinfo endpoint
|
||||
logger.info(" Opaque token detected, calling userinfo endpoint...")
|
||||
try:
|
||||
# Get userinfo endpoint from OIDC discovery
|
||||
oidc_discovery_uri = os.getenv(
|
||||
"OIDC_DISCOVERY_URI",
|
||||
"http://localhost:8080/.well-known/openid-configuration",
|
||||
)
|
||||
async with httpx.AsyncClient() as http_client:
|
||||
discovery_response = await http_client.get(oidc_discovery_uri)
|
||||
discovery_response.raise_for_status()
|
||||
discovery = discovery_response.json()
|
||||
userinfo_endpoint = discovery.get("userinfo_endpoint")
|
||||
|
||||
if userinfo_endpoint:
|
||||
userinfo = await _query_idp_userinfo(token, userinfo_endpoint)
|
||||
if userinfo:
|
||||
user_id = userinfo.get("sub", "unknown")
|
||||
logger.info(f" ✓ Userinfo query successful: user_id={user_id}")
|
||||
return user_id
|
||||
else:
|
||||
logger.error(" ✗ Userinfo query failed")
|
||||
else:
|
||||
logger.error(" ✗ No userinfo_endpoint available")
|
||||
except Exception as e:
|
||||
logger.error(f" ✗ Userinfo query failed: {type(e).__name__}: {e}")
|
||||
|
||||
# Fallback
|
||||
logger.warning(" Using fallback user_id: default_user")
|
||||
return "default_user"
|
||||
|
||||
|
||||
class ProvisioningStatus(BaseModel):
|
||||
"""Status of Nextcloud provisioning for a user."""
|
||||
|
||||
@@ -80,14 +152,28 @@ async def get_provisioning_status(ctx: Context, user_id: str) -> ProvisioningSta
|
||||
Returns:
|
||||
ProvisioningStatus with current provisioning state
|
||||
"""
|
||||
logger.info(
|
||||
f" get_provisioning_status: Looking up refresh token for user_id={user_id}"
|
||||
)
|
||||
storage = RefreshTokenStorage.from_env()
|
||||
await storage.initialize()
|
||||
|
||||
token_data = await storage.get_refresh_token(user_id)
|
||||
|
||||
if not token_data:
|
||||
logger.info(
|
||||
f" get_provisioning_status: ✗ No refresh token found for user_id={user_id}"
|
||||
)
|
||||
return ProvisioningStatus(is_provisioned=False)
|
||||
|
||||
logger.info(
|
||||
f" get_provisioning_status: ✓ Refresh token FOUND for user_id={user_id}"
|
||||
)
|
||||
logger.info(f" flow_type: {token_data.get('flow_type')}")
|
||||
logger.info(
|
||||
f" provisioning_client_id: {token_data.get('provisioning_client_id', 'N/A')}"
|
||||
)
|
||||
|
||||
# Convert timestamp to ISO format if present
|
||||
provisioned_at_str = None
|
||||
if token_data.get("provisioned_at"):
|
||||
@@ -313,13 +399,11 @@ async def revoke_nextcloud_access(
|
||||
RevocationResult with status
|
||||
"""
|
||||
try:
|
||||
# Get user ID from context if not provided
|
||||
# Get user ID from token if not provided
|
||||
if not user_id:
|
||||
user_id = (
|
||||
ctx.context.get("user_id", "default_user") # type: ignore
|
||||
if hasattr(ctx, "context")
|
||||
else "default_user"
|
||||
)
|
||||
logger.info("Extracting user_id from access token for revoke...")
|
||||
user_id = await extract_user_id_from_token(ctx)
|
||||
logger.info(f" Revoke using user_id: {user_id}")
|
||||
|
||||
# Check current status
|
||||
status = await get_provisioning_status(ctx, user_id)
|
||||
@@ -419,28 +503,29 @@ async def check_logged_in(ctx: Context, user_id: Optional[str] = None) -> str:
|
||||
"""
|
||||
try:
|
||||
# Extract user ID from the MCP access token (Flow 1 token)
|
||||
if not user_id:
|
||||
# Get the authorization token from context
|
||||
if hasattr(ctx, "authorization") and ctx.authorization:
|
||||
token = ctx.authorization.token # type: ignore
|
||||
# Decode token to get user info
|
||||
try:
|
||||
import jwt
|
||||
logger.info("=" * 60)
|
||||
logger.info("check_logged_in: Starting user_id extraction")
|
||||
logger.info("=" * 60)
|
||||
|
||||
payload = jwt.decode(token, options={"verify_signature": False})
|
||||
user_id = payload.get("sub", "unknown")
|
||||
logger.info(f"Extracted user_id from Flow 1 token: {user_id}")
|
||||
except Exception as e:
|
||||
logger.warning(f"Failed to decode token: {e}")
|
||||
user_id = "default_user"
|
||||
else:
|
||||
user_id = "default_user"
|
||||
if not user_id:
|
||||
user_id = await extract_user_id_from_token(ctx)
|
||||
logger.info(f" Final user_id for check_logged_in: {user_id}")
|
||||
else:
|
||||
logger.info(f" user_id provided as argument: {user_id}")
|
||||
|
||||
# Check if already logged in
|
||||
logger.info(f"Checking provisioning status for user_id: {user_id}")
|
||||
status = await get_provisioning_status(ctx, user_id)
|
||||
logger.info(f" Provisioning status: is_provisioned={status.is_provisioned}")
|
||||
|
||||
if status.is_provisioned:
|
||||
logger.info(f"✓ User {user_id} is already logged in - returning 'yes'")
|
||||
logger.info("=" * 60)
|
||||
return "yes"
|
||||
|
||||
logger.info(f"✗ User {user_id} is NOT logged in - triggering elicitation")
|
||||
logger.info("=" * 60)
|
||||
|
||||
# Not logged in - generate OAuth URL for Flow 2
|
||||
enable_offline_access = (
|
||||
os.getenv("ENABLE_OFFLINE_ACCESS", "false").lower() == "true"
|
||||
|
||||
Reference in New Issue
Block a user