diff --git a/docker-compose.yml b/docker-compose.yml index 3a1c6c0e..6e8a1f8c 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -288,6 +288,12 @@ services: - ENABLE_SEMANTIC_SEARCH=true - VECTOR_SYNC_SCAN_INTERVAL=60 - VECTOR_SYNC_PROCESSOR_WORKERS=1 + + # Management API allowlist (ADR-018) — matches mcp-multi-user-basic so + # the same configure_astrolabe_for_mcp_server fixture (which creates the + # static `nextcloudMcpServerUIPublicClient` OIDC client) works for tests + # against this profile too. + - ALLOWED_MGMT_CLIENT=nextcloudMcpServerUIPublicClient volumes: - login-flow-data:/app/data - login-flow-oauth-storage:/app/.oauth diff --git a/nextcloud_mcp_server/api/webhooks.py b/nextcloud_mcp_server/api/webhooks.py index 3f9a3a28..c281d3d1 100644 --- a/nextcloud_mcp_server/api/webhooks.py +++ b/nextcloud_mcp_server/api/webhooks.py @@ -47,7 +47,9 @@ async def get_installed_apps(request: Request) -> JSONResponse: ) try: - # Get Bearer token from request + # Get Bearer token from request — forwarded to Nextcloud so the + # capabilities response includes per-user / per-app entries (anonymous + # capabilities omits notes, tables, forms etc.). token = extract_bearer_token(request) if not token: raise ValueError("Missing Authorization header") @@ -59,21 +61,20 @@ async def get_installed_apps(request: Request) -> JSONResponse: if not nextcloud_host: raise ValueError("Nextcloud host not configured") - # Create authenticated HTTP client + # Use OCS v2 capabilities. The legacy /ocs/v1.php/cloud/apps endpoint is + # admin-only AND @PasswordConfirmationRequired — neither is satisfiable + # via an OAuth bearer token, so it always 401s. Capabilities has no such + # gates and returns a map keyed by app id for every enabled app that + # implements OCSCapabilities, which is sufficient to populate the + # webhook presets UI. async with nextcloud_httpx_client( base_url=nextcloud_host, headers={"Authorization": f"Bearer {token}"}, timeout=30.0, ) as client: - # Get installed apps using OCS API - # Notes, Calendar, Deck, Tables, etc. are apps that support webhooks - # We check which ones are installed and enabled - ocs_url = "/ocs/v1.php/cloud/apps" - params = {"filter": "enabled"} - response = await client.get( - ocs_url, - params=params, + "/ocs/v2.php/cloud/capabilities", + params={"format": "json"}, headers={"OCS-APIRequest": "true", "Accept": "application/json"}, ) @@ -81,7 +82,8 @@ async def get_installed_apps(request: Request) -> JSONResponse: raise ValueError(f"OCS API returned status {response.status_code}") data = response.json() - apps = data.get("ocs", {}).get("data", {}).get("apps", []) + capabilities = data.get("ocs", {}).get("data", {}).get("capabilities", {}) + apps = sorted(capabilities.keys()) return JSONResponse({"apps": apps}) diff --git a/tests/server/login_flow/conftest.py b/tests/server/login_flow/conftest.py index ac4a81d2..f56d6aa7 100644 --- a/tests/server/login_flow/conftest.py +++ b/tests/server/login_flow/conftest.py @@ -933,3 +933,200 @@ async def diana_login_flow_mcp_client( password=test_users_setup["diana"]["password"], ): yield session + + +# Static OIDC client used by the management API integration tests. +# Matches the value `mcp-login-flow` and `mcp-multi-user-basic` allowlist +# (`ALLOWED_MGMT_CLIENT=nextcloudMcpServerUIPublicClient`) so tokens issued +# to it pass the management API allowlist check. +STATIC_MGMT_CLIENT_ID = "nextcloudMcpServerUIPublicClient" + + +@pytest.fixture(scope="session") +async def login_flow_static_client_credentials(anyio_backend, oauth_callback_server): + """Pre-create the static OIDC client `nextcloudMcpServerUIPublicClient` + via `occ oidc:create` with the test's OAuth callback URL. + + The static client_id is allowlisted on `mcp-login-flow` (and + `mcp-multi-user-basic`) via `ALLOWED_MGMT_CLIENT`, so tokens it issues + pass the management API allowlist check. Uses a confidential JWT-token + client to match production Astrolabe configuration. + + Yields: (client_id, client_secret, callback_url, token_endpoint, authorization_endpoint) + """ + import json + import subprocess + + nextcloud_host = os.getenv("NEXTCLOUD_HOST") + if not nextcloud_host: + pytest.skip("Static client tests require NEXTCLOUD_HOST") + + auth_states, callback_url = oauth_callback_server + client_id = STATIC_MGMT_CLIENT_ID + + # Idempotent: remove if a previous session left one behind + subprocess.run( + [ + "docker", + "compose", + "exec", + "-T", + "app", + "php", + "/var/www/html/occ", + "oidc:remove", + client_id, + ], + check=False, + capture_output=True, + ) + + logger.info(f"Creating static OIDC client {client_id} with callback {callback_url}") + result = subprocess.run( + [ + "docker", + "compose", + "exec", + "-T", + "app", + "php", + "/var/www/html/occ", + "oidc:create", + "Login Flow Static Client (test)", + callback_url, + "--client_id", + client_id, + "--type", + "confidential", + "--flow", + "code", + "--token_type", + "jwt", + "--resource_url", + LOGIN_FLOW_MCP_BASE_URL, + "--allowed_scopes", + DEFAULT_FULL_SCOPES, + ], + check=True, + capture_output=True, + text=True, + ) + try: + client_output = json.loads(result.stdout.strip()) + except json.JSONDecodeError as e: + raise RuntimeError( + f"occ oidc:create returned non-JSON output: {result.stdout[:200]!r}" + ) from e + client_secret = client_output.get("client_secret") + if not client_secret: + raise ValueError("occ oidc:create did not return client_secret in JSON output") + + async with httpx.AsyncClient(timeout=30.0) as http_client: + discovery_response = await http_client.get( + f"{nextcloud_host}/.well-known/openid-configuration" + ) + discovery_response.raise_for_status() + oidc_config = discovery_response.json() + + yield ( + client_id, + client_secret, + callback_url, + oidc_config["token_endpoint"], + oidc_config["authorization_endpoint"], + ) + + subprocess.run( + [ + "docker", + "compose", + "exec", + "-T", + "app", + "php", + "/var/www/html/occ", + "oidc:remove", + client_id, + ], + check=False, + capture_output=True, + ) + + +@pytest.fixture(scope="session") +async def login_flow_static_client_token( + anyio_backend, + browser, + login_flow_static_client_credentials, + oauth_callback_server, +) -> str: + """Drive the OAuth auth-code flow using the static OIDC client and + return the raw access_token string. + + Mirrors `login_flow_oauth_token` but feeds it static credentials instead + of a DCR-generated client. Required for hitting management API + endpoints which gate on `ALLOWED_MGMT_CLIENT`. + """ + nextcloud_host = os.getenv("NEXTCLOUD_HOST") + username = os.getenv("NEXTCLOUD_USERNAME") + password = os.getenv("NEXTCLOUD_PASSWORD") + if not all([nextcloud_host, username, password]): + pytest.skip( + "Static client OAuth requires NEXTCLOUD_HOST, NEXTCLOUD_USERNAME, NEXTCLOUD_PASSWORD" + ) + + auth_states, _ = oauth_callback_server + client_id, client_secret, callback_url, token_endpoint, authorization_endpoint = ( + login_flow_static_client_credentials + ) + + state = secrets.token_urlsafe(32) + scopes_encoded = quote(DEFAULT_FULL_SCOPES, safe="") + auth_url = ( + f"{authorization_endpoint}?" + f"response_type=code&" + f"client_id={client_id}&" + f"redirect_uri={quote(callback_url, safe='')}&" + f"state={state}&" + f"scope={scopes_encoded}" + ) + + context = await browser.new_context(ignore_https_errors=True) + page = await context.new_page() + try: + await page.goto(auth_url, wait_until="networkidle", timeout=60000) + if "/login" in page.url or "/index.php/login" in page.url: + await page.wait_for_selector('input[name="user"]', timeout=10000) + await page.fill('input[name="user"]', username) + await page.fill('input[name="password"]', password) + await page.click('button[type="submit"]') + await page.wait_for_load_state("networkidle", timeout=60000) + try: + await _handle_oauth_consent_screen(page, username) + except Exception: + pass + + start = time.time() + while state not in auth_states: + if time.time() - start > 30: + raise TimeoutError("Timeout waiting for OAuth callback") + await anyio.sleep(0.5) + auth_code = auth_states[state] + finally: + await context.close() + + async with httpx.AsyncClient(timeout=30.0) as http_client: + token_response = await http_client.post( + token_endpoint, + data={ + "grant_type": "authorization_code", + "code": auth_code, + "redirect_uri": callback_url, + "client_id": client_id, + "client_secret": client_secret, + }, + ) + token_response.raise_for_status() + token_data = token_response.json() + + return token_data["access_token"] diff --git a/tests/server/login_flow/test_management_api.py b/tests/server/login_flow/test_management_api.py new file mode 100644 index 00000000..366e4990 --- /dev/null +++ b/tests/server/login_flow/test_management_api.py @@ -0,0 +1,52 @@ +"""Integration tests for the management API on the login-flow MCP server. + +These tests drive a real OAuth flow against Nextcloud's `oidc` app using the +static `nextcloudMcpServerUIPublicClient` client (which is allowlisted on the +`mcp-login-flow` container via `ALLOWED_MGMT_CLIENT`), then hit the +management API endpoints with the resulting bearer token. + +Regression coverage for the bug where /api/v1/apps proxied to OCS v1 +/cloud/apps and always 401'd. The handler now uses /ocs/v2.php/cloud/capabilities, +which is reachable for OAuth bearer tokens. +""" + +import httpx +import pytest + +LOGIN_FLOW_API_BASE_URL = "http://localhost:8004" + +pytestmark = [pytest.mark.integration, pytest.mark.oauth] + + +async def test_get_installed_apps_returns_capability_keys( + login_flow_static_client_token: str, +): + """GET /api/v1/apps returns 200 with a list of enabled-app capability keys.""" + async with httpx.AsyncClient(timeout=30.0) as client: + response = await client.get( + f"{LOGIN_FLOW_API_BASE_URL}/api/v1/apps", + headers={"Authorization": f"Bearer {login_flow_static_client_token}"}, + ) + + assert response.status_code == 200, ( + f"/api/v1/apps returned {response.status_code}: {response.text}" + ) + + data = response.json() + assert "apps" in data + assert isinstance(data["apps"], list) + + # Anonymous capabilities always exposes core; authenticated also exposes + # files. Both should be present whether or not the oidc app's + # BearerAuthMiddleware ran for this OCS route. + apps = data["apps"] + assert "core" in apps, f"expected 'core' in apps, got {apps}" + assert "files" in apps, f"expected 'files' in apps, got {apps}" + + +async def test_get_installed_apps_requires_bearer_token(): + """No Authorization header → 401 (handler's token validator rejects it).""" + async with httpx.AsyncClient(timeout=30.0) as client: + response = await client.get(f"{LOGIN_FLOW_API_BASE_URL}/api/v1/apps") + + assert response.status_code == 401 diff --git a/tests/unit/test_management_apps_endpoint.py b/tests/unit/test_management_apps_endpoint.py new file mode 100644 index 00000000..e2e82098 --- /dev/null +++ b/tests/unit/test_management_apps_endpoint.py @@ -0,0 +1,165 @@ +""" +Unit tests for the Management API /api/v1/apps endpoint. + +These tests cover the regression where /api/v1/apps proxied to +/ocs/v1.php/cloud/apps — an admin-only and @PasswordConfirmationRequired +endpoint that always 401s for OAuth bearer tokens. The handler now uses +/ocs/v2.php/cloud/capabilities, which accepts the bearer and returns an +authenticated capability map keyed by app id. +""" + +from unittest.mock import AsyncMock, MagicMock + +import pytest +from starlette.applications import Starlette +from starlette.routing import Route +from starlette.testclient import TestClient + +from nextcloud_mcp_server.api.webhooks import get_installed_apps + +pytestmark = pytest.mark.unit + + +def _build_test_app() -> Starlette: + app = Starlette(routes=[Route("/api/v1/apps", get_installed_apps, methods=["GET"])]) + app.state.oauth_context = {"config": {"nextcloud_host": "http://nc.test"}} + return app + + +def _patch_token_validation(mocker, user_id: str = "admin") -> None: + mocker.patch( + "nextcloud_mcp_server.api.webhooks.validate_token_and_get_user", + new=AsyncMock(return_value=(user_id, {"sub": user_id})), + ) + + +def _patch_outbound_client(mocker, response: MagicMock) -> AsyncMock: + """Patch the outbound httpx client; return the mocked .get() AsyncMock.""" + mock_get = AsyncMock(return_value=response) + mock_client = AsyncMock() + mock_client.get = mock_get + mock_client.__aenter__ = AsyncMock(return_value=mock_client) + # Must return False (or None) so async-with does NOT suppress exceptions + # raised inside the block — default AsyncMock() resolves to a truthy + # MagicMock and would silently swallow ValueError. + mock_client.__aexit__ = AsyncMock(return_value=False) + + mock_factory = MagicMock(return_value=mock_client) + mocker.patch( + "nextcloud_mcp_server.api.webhooks.nextcloud_httpx_client", mock_factory + ) + # Return both so tests can assert on factory kwargs and call args + mock_factory.attach_mock(mock_get, "get") + return mock_factory + + +def _capabilities_response(capabilities: dict[str, dict]) -> MagicMock: + response = MagicMock() + response.status_code = 200 + response.json.return_value = {"ocs": {"data": {"capabilities": capabilities}}} + return response + + +async def test_returns_sorted_capability_keys(mocker): + """Happy path: handler hits OCS v2 capabilities and returns sorted app keys.""" + _patch_token_validation(mocker) + response = _capabilities_response( + { + "notes": {"api_version": "1.4"}, + "files": {"bigfilechunking": True}, + "core": {"webdav-root": "remote.php/webdav"}, + "tables": {"foo": "bar"}, + } + ) + factory = _patch_outbound_client(mocker, response) + + app = _build_test_app() + client = TestClient(app) + http_response = client.get( + "/api/v1/apps", headers={"Authorization": "Bearer test-token"} + ) + + assert http_response.status_code == 200 + assert http_response.json() == {"apps": ["core", "files", "notes", "tables"]} + + # Regression check: outbound URL is OCS v2 capabilities, NOT v1 cloud/apps. + # /ocs/v1.php/cloud/apps was admin-only + @PasswordConfirmationRequired + # which always 401s for OAuth bearer tokens — that was the bug. + factory.get.assert_awaited_once() + called_path = factory.get.call_args.args[0] + assert called_path == "/ocs/v2.php/cloud/capabilities" + assert "/cloud/apps" not in called_path + + # Bearer token forwarded so authenticated capabilities are returned + factory_call_kwargs = factory.call_args.kwargs + assert factory_call_kwargs["headers"] == {"Authorization": "Bearer test-token"} + + +async def test_empty_capabilities_returns_empty_list(mocker): + """Empty capabilities map → empty apps list, not an error.""" + _patch_token_validation(mocker) + response = _capabilities_response({}) + _patch_outbound_client(mocker, response) + + app = _build_test_app() + client = TestClient(app) + http_response = client.get( + "/api/v1/apps", headers={"Authorization": "Bearer test-token"} + ) + + assert http_response.status_code == 200 + assert http_response.json() == {"apps": []} + + +async def test_ocs_error_returns_500_with_sanitized_message(mocker): + """Non-200 from Nextcloud OCS surfaces as a generic 500 to the caller.""" + _patch_token_validation(mocker) + error_response = MagicMock() + error_response.status_code = 503 + _patch_outbound_client(mocker, error_response) + + app = _build_test_app() + client = TestClient(app) + http_response = client.get( + "/api/v1/apps", headers={"Authorization": "Bearer test-token"} + ) + + assert http_response.status_code == 500 + body = http_response.json() + assert body["error"] == "Internal error" + # _sanitize_error_for_client returns a generic message; specific status + # code must NOT leak to the client + assert "503" not in body["message"] + + +async def test_missing_nextcloud_host_returns_500(mocker): + """Misconfigured oauth_context (no nextcloud_host) surfaces as 500.""" + _patch_token_validation(mocker) + response = _capabilities_response({}) + _patch_outbound_client(mocker, response) + + app = Starlette(routes=[Route("/api/v1/apps", get_installed_apps, methods=["GET"])]) + app.state.oauth_context = {"config": {}} # no nextcloud_host + + client = TestClient(app) + http_response = client.get( + "/api/v1/apps", headers={"Authorization": "Bearer test-token"} + ) + + assert http_response.status_code == 500 + + +async def test_missing_authorization_returns_500(mocker): + """When token validation passes but Authorization header is absent, the + handler can't construct the outbound bearer header — returns 500 with a + sanitized message.""" + _patch_token_validation(mocker) + response = _capabilities_response({}) + _patch_outbound_client(mocker, response) + + app = _build_test_app() + client = TestClient(app) + # No Authorization header + http_response = client.get("/api/v1/apps") + + assert http_response.status_code == 500