diff --git a/CLAUDE.md b/CLAUDE.md index 2a7cad3..6012454 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -327,10 +327,12 @@ OAuth integration tests use **automated Playwright browser automation** to compl **OAuth Testing Setup:** - **Main fixtures**: `nc_oauth_client`, `nc_mcp_oauth_client` - Use Playwright automation - **Shared OAuth Client**: All test users authenticate using a single OAuth client - - Stored in `.nextcloud_oauth_shared_test_client.json` - - Matches production MCP server behavior + - **Created fresh for each test session** via Dynamic Client Registration (DCR) + - Matches production MCP server behavior (one client, multiple user tokens) - Each user gets their own unique access token + - **Automatic cleanup**: Client is registered at session start, deleted at session end (RFC 7592) - Implementation: `shared_oauth_client_credentials` fixture in `tests/conftest.py` + - **Note**: Client deletion may fail due to Nextcloud middleware (logged as warning). This doesn't affect tests. - **Available fixtures**: `playwright_oauth_token`, `nc_oauth_client`, `nc_mcp_oauth_client` - **Multi-user fixtures**: `alice_oauth_token`, `bob_oauth_token`, `charlie_oauth_token`, `diana_oauth_token` - **Requirements**: `NEXTCLOUD_HOST`, `NEXTCLOUD_USERNAME`, `NEXTCLOUD_PASSWORD` environment variables @@ -356,7 +358,6 @@ uv run pytest -m oauth -v - `mcp-oauth` (port 8001): Uses OAuth authentication - for OAuth-specific testing - Start OAuth MCP server: `docker-compose up --build -d mcp-oauth` - **Important**: When working on OAuth functionality, always rebuild `mcp-oauth` container, not `mcp` -- OAuth client credentials cached in `.nextcloud_oauth_shared_test_client.json` **CI/CD Notes:** - Playwright tests run in CI/CD environments diff --git a/nextcloud_mcp_server/auth/client_registration.py b/nextcloud_mcp_server/auth/client_registration.py index 3c1f4e9..b540827 100644 --- a/nextcloud_mcp_server/auth/client_registration.py +++ b/nextcloud_mcp_server/auth/client_registration.py @@ -212,6 +212,80 @@ def save_client_to_file(client_info: ClientInfo, storage_path: Path): raise +async def delete_client( + nextcloud_url: str, + client_id: str, + client_secret: str, +) -> bool: + """ + Delete a dynamically registered OAuth client using RFC 7592. + + This implements RFC 7592 Section 2.3 (Client Delete Request). + The client authenticates using client_secret_post method and + requests deletion via DELETE to the client configuration endpoint. + + Args: + nextcloud_url: Base URL of the Nextcloud instance + client_id: Client identifier to delete + client_secret: Client secret for authentication + + Returns: + True if deletion successful, False otherwise + + Note: + Per RFC 7592, the deletion endpoint is: + {nextcloud_url}/apps/oidc/register/{client_id} + + Authentication uses HTTP Basic Auth or client_secret_post: + - HTTP Basic Auth: client_id as username, client_secret as password + - client_secret_post: credentials in request body + """ + deletion_endpoint = f"{nextcloud_url}/apps/oidc/register/{client_id}" + + logger.info(f"Deleting OAuth client: {client_id[:16]}...") + logger.debug(f"Deletion endpoint: {deletion_endpoint}") + + async with httpx.AsyncClient(timeout=30.0) as http_client: + try: + # RFC 7592 requires client authentication + # Use HTTP Basic Auth (client_id as username, client_secret as password) + response = await http_client.delete( + deletion_endpoint, + auth=(client_id, client_secret), + ) + + # RFC 7592: Successful deletion returns 204 No Content + if response.status_code == 204: + logger.info(f"Successfully deleted OAuth client: {client_id[:16]}...") + return True + elif response.status_code == 401: + logger.error( + f"Failed to delete client {client_id[:16]}...: Authentication failed (invalid credentials)" + ) + return False + elif response.status_code == 403: + logger.error( + f"Failed to delete client {client_id[:16]}...: Not authorized (not a DCR client or wrong client)" + ) + return False + else: + logger.error( + f"Failed to delete client {client_id[:16]}...: HTTP {response.status_code}" + ) + logger.debug(f"Response: {response.text}") + return False + + except httpx.HTTPStatusError as e: + logger.error( + f"HTTP error deleting client {client_id[:16]}...: {e.response.status_code}" + ) + logger.debug(f"Response: {e.response.text}") + return False + except Exception as e: + logger.error(f"Unexpected error deleting client {client_id[:16]}...: {e}") + return False + + async def load_or_register_client( nextcloud_url: str, registration_endpoint: str, diff --git a/tests/conftest.py b/tests/conftest.py index f575b65..a245907 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -952,9 +952,13 @@ async def shared_oauth_client_credentials(anyio_backend, oauth_callback_server): server (port 8001). While opaque tokens don't embed scopes, the allowed_scopes configuration ensures tokens have proper scopes when introspected. + The client is automatically deleted from Nextcloud after the test session completes. + Returns: Tuple of (client_id, client_secret, callback_url, token_endpoint, authorization_endpoint) """ + from nextcloud_mcp_server.auth.client_registration import delete_client + nextcloud_host = os.getenv("NEXTCLOUD_HOST") if not nextcloud_host: pytest.skip("Shared OAuth client requires NEXTCLOUD_HOST") @@ -982,13 +986,11 @@ async def shared_oauth_client_credentials(anyio_backend, oauth_callback_server): # Create opaque token client with allowed_scopes (not JWT) # This ensures the token has proper scopes even though they're not embedded - # Cache to file to avoid creating new client on every test run client_id, client_secret = await _create_oauth_client_with_scopes( callback_url=callback_url, client_name="Pytest - Shared Test Client (Opaque)", allowed_scopes=DEFAULT_FULL_SCOPES, token_type="Bearer", # Opaque tokens for port 8001 - cache_file=".nextcloud_oauth_shared_test_client.json", ) logger.info(f"Shared OAuth client ready: {client_id[:16]}...") @@ -996,7 +998,7 @@ async def shared_oauth_client_credentials(anyio_backend, oauth_callback_server): "This opaque token client with full scopes will be reused for all test user authentications" ) - return ( + yield ( client_id, client_secret, callback_url, @@ -1004,6 +1006,27 @@ async def shared_oauth_client_credentials(anyio_backend, oauth_callback_server): authorization_endpoint, ) + # Cleanup: Delete OAuth client from Nextcloud using RFC 7592 + try: + logger.info(f"Cleaning up shared OAuth client: {client_id[:16]}...") + success = await delete_client( + nextcloud_url=nextcloud_host, + client_id=client_id, + client_secret=client_secret, + ) + if success: + logger.info( + f"Successfully deleted shared OAuth client: {client_id[:16]}..." + ) + else: + logger.warning( + f"Failed to delete shared OAuth client: {client_id[:16]}..." + ) + except Exception as e: + logger.warning( + f"Error cleaning up shared OAuth client {client_id[:16]}...: {e}" + ) + @pytest.fixture(scope="session") async def shared_jwt_oauth_client_credentials(anyio_backend, oauth_callback_server): @@ -1014,9 +1037,13 @@ async def shared_jwt_oauth_client_credentials(anyio_backend, oauth_callback_serv is configured with token_type="JWT" to request JWT-formatted access tokens from the OIDC server (instead of opaque tokens). + The client is automatically deleted from Nextcloud after the test session completes. + Returns: Tuple of (client_id, client_secret, callback_url, token_endpoint, authorization_endpoint) """ + from nextcloud_mcp_server.auth.client_registration import delete_client + nextcloud_host = os.getenv("NEXTCLOUD_HOST") if not nextcloud_host: pytest.skip("Shared JWT OAuth client requires NEXTCLOUD_HOST") @@ -1043,13 +1070,11 @@ async def shared_jwt_oauth_client_credentials(anyio_backend, oauth_callback_serv ) # Create JWT client with full scopes (all app read/write scopes) - # Cache to file to avoid creating new client on every test run client_id, client_secret = await _create_oauth_client_with_scopes( callback_url=callback_url, client_name="Pytest - Shared JWT Test Client", allowed_scopes=DEFAULT_FULL_SCOPES, token_type="JWT", # Explicitly set JWT token type - cache_file=".nextcloud_oauth_shared_jwt_test_client.json", ) logger.info(f"Shared JWT OAuth client ready: {client_id[:16]}...") @@ -1057,7 +1082,7 @@ async def shared_jwt_oauth_client_credentials(anyio_backend, oauth_callback_serv "This JWT client with full scopes will be reused for JWT MCP server tests" ) - return ( + yield ( client_id, client_secret, callback_url, @@ -1065,53 +1090,48 @@ async def shared_jwt_oauth_client_credentials(anyio_backend, oauth_callback_serv authorization_endpoint, ) + # Cleanup: Delete OAuth client from Nextcloud using RFC 7592 + try: + logger.info(f"Cleaning up shared JWT OAuth client: {client_id[:16]}...") + success = await delete_client( + nextcloud_url=nextcloud_host, + client_id=client_id, + client_secret=client_secret, + ) + if success: + logger.info( + f"Successfully deleted shared JWT OAuth client: {client_id[:16]}..." + ) + else: + logger.warning( + f"Failed to delete shared JWT OAuth client: {client_id[:16]}..." + ) + except Exception as e: + logger.warning( + f"Error cleaning up shared JWT OAuth client {client_id[:16]}...: {e}" + ) + async def _create_oauth_client_with_scopes( callback_url: str, client_name: str, allowed_scopes: str, token_type: str = "JWT", - cache_file: str | None = None, ) -> tuple[str, str]: """ Helper function to create an OAuth client with specific allowed_scopes using DCR. - Supports optional file-based caching to avoid creating duplicate clients. - Args: callback_url: OAuth callback URL client_name: Name of the OAuth client allowed_scopes: Space-separated list of allowed scopes token_type: Either "JWT" or "Bearer" (default: "JWT") - cache_file: Optional path to cache file (e.g., ".nextcloud_oauth_shared_test_client.json") Returns: Tuple of (client_id, client_secret) """ - import json - from pathlib import Path - from nextcloud_mcp_server.auth.client_registration import register_client - # Try to load from cache if specified - if cache_file: - cache_path = Path(cache_file) - if cache_path.exists(): - try: - with open(cache_path, "r") as f: - cached_data = json.load(f) - - client_id = cached_data.get("client_id") - client_secret = cached_data.get("client_secret") - - if client_id and client_secret: - logger.info( - f"Loaded cached OAuth client from {cache_file}: {client_id[:16]}..." - ) - return client_id, client_secret - except (json.JSONDecodeError, KeyError, OSError) as e: - logger.warning(f"Failed to load cached client from {cache_file}: {e}") - logger.info( f"Creating {token_type} OAuth client '{client_name}' with scopes: {allowed_scopes} using DCR" ) @@ -1149,32 +1169,6 @@ async def _create_oauth_client_with_scopes( f"Created OAuth client via DCR: {client_id[:16]}... with scopes: {allowed_scopes}" ) - # Save to cache if specified - if cache_file: - cache_path = Path(cache_file) - try: - # Create parent directory if needed - cache_path.parent.mkdir(parents=True, exist_ok=True) - - # Save client data - with open(cache_path, "w") as f: - json.dump( - { - "client_id": client_id, - "client_secret": client_secret, - "redirect_uris": [callback_url], - }, - f, - indent=2, - ) - - # Set restrictive permissions - cache_path.chmod(0o600) - - logger.info(f"Cached OAuth client to {cache_file}") - except OSError as e: - logger.warning(f"Failed to cache client to {cache_file}: {e}") - return client_id, client_secret @@ -1183,9 +1177,13 @@ async def read_only_oauth_client_credentials(anyio_backend, oauth_callback_serve """ Fixture for OAuth client with only read scopes. + The client is automatically deleted from Nextcloud after the test session completes. + Returns: Tuple of (client_id, client_secret, callback_url, token_endpoint, authorization_endpoint) """ + from nextcloud_mcp_server.auth.client_registration import delete_client + nextcloud_host = os.getenv("NEXTCLOUD_HOST") if not nextcloud_host: pytest.skip("Read-only OAuth client requires NEXTCLOUD_HOST") @@ -1209,7 +1207,7 @@ async def read_only_oauth_client_credentials(anyio_backend, oauth_callback_serve token_type="JWT", # JWT tokens for scope validation ) - return ( + yield ( client_id, client_secret, callback_url, @@ -1217,15 +1215,40 @@ async def read_only_oauth_client_credentials(anyio_backend, oauth_callback_serve authorization_endpoint, ) + # Cleanup: Delete OAuth client from Nextcloud using RFC 7592 + try: + logger.info(f"Cleaning up read-only OAuth client: {client_id[:16]}...") + success = await delete_client( + nextcloud_url=nextcloud_host, + client_id=client_id, + client_secret=client_secret, + ) + if success: + logger.info( + f"Successfully deleted read-only OAuth client: {client_id[:16]}..." + ) + else: + logger.warning( + f"Failed to delete read-only OAuth client: {client_id[:16]}..." + ) + except Exception as e: + logger.warning( + f"Error cleaning up read-only OAuth client {client_id[:16]}...: {e}" + ) + @pytest.fixture(scope="session") async def write_only_oauth_client_credentials(anyio_backend, oauth_callback_server): """ Fixture for OAuth client with only write scopes. + The client is automatically deleted from Nextcloud after the test session completes. + Returns: Tuple of (client_id, client_secret, callback_url, token_endpoint, authorization_endpoint) """ + from nextcloud_mcp_server.auth.client_registration import delete_client + nextcloud_host = os.getenv("NEXTCLOUD_HOST") if not nextcloud_host: pytest.skip("Write-only OAuth client requires NEXTCLOUD_HOST") @@ -1249,7 +1272,7 @@ async def write_only_oauth_client_credentials(anyio_backend, oauth_callback_serv token_type="JWT", # JWT tokens for scope validation ) - return ( + yield ( client_id, client_secret, callback_url, @@ -1257,15 +1280,40 @@ async def write_only_oauth_client_credentials(anyio_backend, oauth_callback_serv authorization_endpoint, ) + # Cleanup: Delete OAuth client from Nextcloud using RFC 7592 + try: + logger.info(f"Cleaning up write-only OAuth client: {client_id[:16]}...") + success = await delete_client( + nextcloud_url=nextcloud_host, + client_id=client_id, + client_secret=client_secret, + ) + if success: + logger.info( + f"Successfully deleted write-only OAuth client: {client_id[:16]}..." + ) + else: + logger.warning( + f"Failed to delete write-only OAuth client: {client_id[:16]}..." + ) + except Exception as e: + logger.warning( + f"Error cleaning up write-only OAuth client {client_id[:16]}...: {e}" + ) + @pytest.fixture(scope="session") async def full_access_oauth_client_credentials(anyio_backend, oauth_callback_server): """ Fixture for OAuth client with both read and write scopes. + The client is automatically deleted from Nextcloud after the test session completes. + Returns: Tuple of (client_id, client_secret, callback_url, token_endpoint, authorization_endpoint) """ + from nextcloud_mcp_server.auth.client_registration import delete_client + nextcloud_host = os.getenv("NEXTCLOUD_HOST") if not nextcloud_host: pytest.skip("Full-access OAuth client requires NEXTCLOUD_HOST") @@ -1289,7 +1337,7 @@ async def full_access_oauth_client_credentials(anyio_backend, oauth_callback_ser token_type="JWT", # JWT tokens for scope validation ) - return ( + yield ( client_id, client_secret, callback_url, @@ -1297,6 +1345,27 @@ async def full_access_oauth_client_credentials(anyio_backend, oauth_callback_ser authorization_endpoint, ) + # Cleanup: Delete OAuth client from Nextcloud using RFC 7592 + try: + logger.info(f"Cleaning up full-access OAuth client: {client_id[:16]}...") + success = await delete_client( + nextcloud_url=nextcloud_host, + client_id=client_id, + client_secret=client_secret, + ) + if success: + logger.info( + f"Successfully deleted full-access OAuth client: {client_id[:16]}..." + ) + else: + logger.warning( + f"Failed to delete full-access OAuth client: {client_id[:16]}..." + ) + except Exception as e: + logger.warning( + f"Error cleaning up full-access OAuth client {client_id[:16]}...: {e}" + ) + @pytest.fixture(scope="session") async def no_custom_scopes_oauth_client_credentials( @@ -1308,9 +1377,13 @@ async def no_custom_scopes_oauth_client_credentials( Tests the security behavior when a user grants only the default OIDC scopes (openid, profile, email) but declines custom application scopes (notes:read, notes:write, etc.). + The client is automatically deleted from Nextcloud after the test session completes. + Returns: Tuple of (client_id, client_secret, callback_url, token_endpoint, authorization_endpoint) """ + from nextcloud_mcp_server.auth.client_registration import delete_client + nextcloud_host = os.getenv("NEXTCLOUD_HOST") if not nextcloud_host: pytest.skip("No-custom-scopes OAuth client requires NEXTCLOUD_HOST") @@ -1334,7 +1407,7 @@ async def no_custom_scopes_oauth_client_credentials( token_type="JWT", # JWT tokens for scope validation ) - return ( + yield ( client_id, client_secret, callback_url, @@ -1342,6 +1415,29 @@ async def no_custom_scopes_oauth_client_credentials( authorization_endpoint, ) + # Cleanup: Delete OAuth client from Nextcloud using RFC 7592 + try: + logger.info( + f"Cleaning up no-custom-scopes OAuth client: {client_id[:16]}..." + ) + success = await delete_client( + nextcloud_url=nextcloud_host, + client_id=client_id, + client_secret=client_secret, + ) + if success: + logger.info( + f"Successfully deleted no-custom-scopes OAuth client: {client_id[:16]}..." + ) + else: + logger.warning( + f"Failed to delete no-custom-scopes OAuth client: {client_id[:16]}..." + ) + except Exception as e: + logger.warning( + f"Error cleaning up no-custom-scopes OAuth client {client_id[:16]}...: {e}" + ) + @pytest.fixture(scope="session") async def playwright_oauth_token(