diff --git a/docker-compose.yml b/docker-compose.yml index e810d087a..ef6a107e1 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -264,6 +264,10 @@ services: - NEXTCLOUD_MCP_SERVER_URL=http://localhost:8002 - NEXTCLOUD_RESOURCE_URI=nextcloud # ADR-005: Keycloak uses client IDs as audiences, not URLs - NEXTCLOUD_PUBLIC_ISSUER_URL=http://localhost:8888/realms/nextcloud-mcp + # External-IdP mode: the OAuth issuer (above) is Keycloak, but Login Flow + # v2 must send the browser to *Nextcloud*. NEXTCLOUD_HOST is the internal + # Docker hostname, so give the browser-reachable Nextcloud URL explicitly. + - NEXTCLOUD_PUBLIC_URL=http://localhost:8080 # Refresh token storage (ADR-002 Tier 1 & 2). Source from .env. - ENABLE_BACKGROUND_OPERATIONS=true diff --git a/docs/configuration.md b/docs/configuration.md index 46c48de81..cd8eac140 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -116,7 +116,8 @@ NEXTCLOUD_PUBLIC_ISSUER_URL=https://your.nextcloud.instance.com | `TOKEN_ENCRYPTION_KEY` | ✅ Yes | Fernet key for app-password encryption — generate with `python -c "from cryptography.fernet import Fernet; print(Fernet.generate_key().decode())"` | | `TOKEN_STORAGE_DB` | ✅ Yes | Path to SQLite DB for stored app passwords (use a persistent volume) | | `NEXTCLOUD_MCP_SERVER_URL` | ✅ Yes | Public URL of the MCP server (used as the audience claim and for browser redirects) | -| `NEXTCLOUD_PUBLIC_ISSUER_URL` | ✅ Yes | Public URL of Nextcloud (for browser redirects during Login Flow v2) | +| `NEXTCLOUD_PUBLIC_ISSUER_URL` | ✅ Yes | Public URL used as the OAuth issuer for JWT validation **and** (by default) the browser-reachable Nextcloud URL for Login Flow v2 redirects. When Nextcloud is its own IdP these coincide. | +| `NEXTCLOUD_PUBLIC_URL` | Optional (required for external IdPs) | Browser-reachable public URL of **Nextcloud** for Login Flow v2 login pages and elicitation links. Only needed when the OAuth issuer is a *separate* IdP (e.g. Keycloak/Cognito): there `NEXTCLOUD_PUBLIC_ISSUER_URL` points at the IdP, so set this to Nextcloud's own URL or the Login Flow v2 login page is built on the IdP origin and 404s. Falls back to `NEXTCLOUD_PUBLIC_ISSUER_URL` then `NEXTCLOUD_HOST` when unset. | | `NEXTCLOUD_OIDC_CLIENT_ID` | ✅ Strongly recommended | OIDC client ID for the MCP server's relying-party registration with the IdP (Nextcloud's built-in OIDC by default; Keycloak / Cognito / etc. via `OIDC_DISCOVERY_URL`). If unset and the IdP advertises a `registration_endpoint`, the server falls back to RFC 7591 Dynamic Client Registration (DCR) — **but with Nextcloud's built-in `oidc` app this fallback breaks after ~1 hour** (see warning below). Create a static client and set this instead. | | `NEXTCLOUD_OIDC_CLIENT_SECRET` | ✅ Strongly recommended | OIDC client secret paired with `NEXTCLOUD_OIDC_CLIENT_ID`. | | `OIDC_DISCOVERY_URL` | Optional | Override the IdP discovery URL. Defaults to `${NEXTCLOUD_HOST}/.well-known/openid-configuration` (Nextcloud's built-in OIDC). Set to a Keycloak realm or AWS Cognito user-pool discovery URL to use an external IdP. | diff --git a/docs/login-flow-v2.md b/docs/login-flow-v2.md index fa7042b96..481b5a1bd 100644 --- a/docs/login-flow-v2.md +++ b/docs/login-flow-v2.md @@ -391,7 +391,11 @@ The MCP server cannot reach Nextcloud at `NEXTCLOUD_HOST`. Verify network connec ### "Login URL points to localhost in browser" -`NEXTCLOUD_PUBLIC_ISSUER_URL` is missing or wrong. Set it to the public URL of Nextcloud as the user's browser sees it. The server rewrites the login URL's origin from the internal `NEXTCLOUD_HOST` to `NEXTCLOUD_PUBLIC_ISSUER_URL` before redirecting the browser. +`NEXTCLOUD_PUBLIC_ISSUER_URL` is missing or wrong. Set it to the public URL of Nextcloud as the user's browser sees it. The server rewrites the login URL's origin from the internal `NEXTCLOUD_HOST` to the browser-reachable Nextcloud URL before redirecting the browser. + +### Login page 404s / lands on the IdP (external IdP, e.g. Keycloak) + +With an **external** IdP, `NEXTCLOUD_PUBLIC_ISSUER_URL` points at the IdP (it doubles as the OAuth issuer for JWT validation), so the Login Flow v2 login URL gets rewritten onto the IdP's origin — which has no `/login/v2` endpoint and 404s. Set **`NEXTCLOUD_PUBLIC_URL`** to Nextcloud's own browser-reachable URL; it takes precedence over `NEXTCLOUD_PUBLIC_ISSUER_URL` for the login-page and elicitation-link rewrites while leaving JWT issuer validation on the IdP. Single-IdP (Nextcloud-is-the-IdP) deployments don't need it — the issuer URL already resolves to Nextcloud. ### Stored app password rejected by Nextcloud (401) diff --git a/nextcloud_mcp_server/auth/elicitation.py b/nextcloud_mcp_server/auth/elicitation.py index 57b59d4c5..03c2590ab 100644 --- a/nextcloud_mcp_server/auth/elicitation.py +++ b/nextcloud_mcp_server/auth/elicitation.py @@ -15,8 +15,8 @@ logger = logging.getLogger(__name__) # Path of the Astrolabe Nextcloud app's settings UI. The full URL is -# reconstructed at elicitation time from settings.nextcloud_public_issuer_url -# / settings.nextcloud_host so the user gets a browser-reachable link without +# reconstructed at elicitation time from settings.nextcloud_browser_url (the +# browser-reachable Nextcloud base URL) so the user gets a working link without # needing a separate config knob. If the Astrolabe app is not installed this # path will 404, and the user falls back to the nc_auth_provision_access tool # path mentioned in the same message. @@ -44,17 +44,16 @@ class ProvisioningRequiredConfirmation(BaseModel): def _astrolabe_settings_url() -> str | None: """Construct the Astrolabe settings page URL from settings. - Prefers ``nextcloud_public_issuer_url`` (the browser-reachable public URL) - over ``nextcloud_host`` (which may be an internal hostname in Docker - deployments). Returns None if neither is set (or set to the empty - string), or if the configured base URL is missing an http:// or - https:// scheme — in the latter case the caller renders the tool-only - fallback message instead of a broken link. + Uses ``nextcloud_browser_url`` (the browser-reachable Nextcloud base URL: + ``nextcloud_public_url`` → ``nextcloud_public_issuer_url`` → ``nextcloud_host``) + so the link points at Nextcloud even in external-IdP deployments where the + OAuth issuer URL is the IdP, not Nextcloud. Returns None if none is set (or + set to the empty string), or if the configured base URL is missing an + http:// or https:// scheme — in the latter case the caller renders the + tool-only fallback message instead of a broken link. """ settings = get_settings() - base = ( - settings.nextcloud_public_issuer_url or settings.nextcloud_host or "" - ).strip() + base = (settings.nextcloud_browser_url or "").strip() if not base: return None if not base.startswith(("http://", "https://")): @@ -182,8 +181,7 @@ async def present_provisioning_required(ctx: Context) -> str: has to translate. The Astrolabe settings URL is reconstructed from - ``settings.nextcloud_public_issuer_url`` / - ``settings.nextcloud_host``; if Astrolabe is not installed the link + ``settings.nextcloud_browser_url``; if Astrolabe is not installed the link 404s and the user falls back to the tool path suggested in the same message. diff --git a/nextcloud_mcp_server/auth/provision_routes.py b/nextcloud_mcp_server/auth/provision_routes.py index 7fa2e92f8..1e1d9d6e5 100644 --- a/nextcloud_mcp_server/auth/provision_routes.py +++ b/nextcloud_mcp_server/auth/provision_routes.py @@ -74,7 +74,7 @@ async def _poll_and_store(provision_id: str) -> None: flow_client = LoginFlowV2Client( nextcloud_host=nextcloud_host, verify_ssl=get_nextcloud_ssl_verify(), - public_host=settings.nextcloud_public_issuer_url, + public_host=settings.nextcloud_browser_url, ) poll_endpoint = session["poll_endpoint"] @@ -214,7 +214,7 @@ async def provision_page( flow_client = LoginFlowV2Client( nextcloud_host=nextcloud_host, verify_ssl=get_nextcloud_ssl_verify(), - public_host=settings.nextcloud_public_issuer_url, + public_host=settings.nextcloud_browser_url, ) init_response = await flow_client.initiate() except Exception as e: @@ -262,12 +262,14 @@ async def provision_page( # The login_url may use the internal Docker hostname (http://app/...). # Replace with the public Nextcloud URL for the browser. # Note: poll_endpoint is rewritten to NEXTCLOUD_HOST (server-side, in - # LoginFlowV2Client) while login_url is rewritten to the public issuer - # URL here because the browser needs a publicly-reachable address. + # LoginFlowV2Client) while login_url is rewritten to the public *Nextcloud* + # URL here because the browser needs a publicly-reachable address. This must + # use nextcloud_browser_url (not the OAuth issuer URL): in external-IdP mode + # the issuer is the IdP, which has no Login Flow v2 endpoint. login_url = init_response.login_url - public_issuer = settings.nextcloud_public_issuer_url or "" - if public_issuer and nextcloud_host: - login_url = rewrite_url_origin(login_url, public_issuer.rstrip("/")) + public_browser_url = settings.nextcloud_browser_url or "" + if public_browser_url and nextcloud_host: + login_url = rewrite_url_origin(login_url, public_browser_url.rstrip("/")) return RedirectResponse(login_url) diff --git a/nextcloud_mcp_server/auth/userinfo_routes.py b/nextcloud_mcp_server/auth/userinfo_routes.py index 276ede124..706a622ae 100644 --- a/nextcloud_mcp_server/auth/userinfo_routes.py +++ b/nextcloud_mcp_server/auth/userinfo_routes.py @@ -482,13 +482,12 @@ async def user_info_html(request: Request) -> HTMLResponse: str(request.url_for("oauth_logout")) if oauth_ctx else "/oauth/logout" ) - # Get Nextcloud host for generating links to apps (used by viz tab) - # Use public issuer URL if available (for browser-accessible links), - # otherwise fall back to NEXTCLOUD_HOST from settings + # Get Nextcloud host for generating browser-accessible links to apps (viz + # tab). Use nextcloud_browser_url so the links point at Nextcloud even in + # external-IdP mode, where nextcloud_public_issuer_url is the IdP, not + # Nextcloud (falls back to public_issuer_url → nextcloud_host). settings = get_settings() - nextcloud_host_for_links = ( - settings.nextcloud_public_issuer_url or settings.nextcloud_host - ) + nextcloud_host_for_links = settings.nextcloud_browser_url # Build host info HTML (BasicAuth only) host_info_html = "" diff --git a/nextcloud_mcp_server/config.py b/nextcloud_mcp_server/config.py index 7d75e87d7..c1b6b7070 100644 --- a/nextcloud_mcp_server/config.py +++ b/nextcloud_mcp_server/config.py @@ -35,6 +35,7 @@ "nextcloud_mcp_server_url": None, "nextcloud_resource_uri": None, "nextcloud_public_issuer_url": None, + "nextcloud_public_url": None, "cookie_secure": None, # OAuth/OIDC "oidc_discovery_url": None, @@ -734,8 +735,25 @@ class Settings: # Browser-reachable public URL for OAuth/Login-Flow-v2 redirects when # NEXTCLOUD_HOST is an internal Docker hostname. Falls back to # nextcloud_host when unset. + # + # NOTE: this doubles as the OAuth *issuer* URL used for JWT ``iss`` + # validation. In external-IdP mode (e.g. Keycloak) the issuer is the IdP, + # NOT Nextcloud — so this value points at the IdP, not the browser-reachable + # Nextcloud host. Use ``nextcloud_public_url`` / ``nextcloud_browser_url`` + # for anything that must resolve to Nextcloud itself (Login Flow v2 login + # URLs, elicitation links). nextcloud_public_issuer_url: str | None = None + # Browser-reachable public URL of the *Nextcloud* instance, used to rewrite + # Login Flow v2 login URLs and elicitation links when NEXTCLOUD_HOST is an + # internal Docker hostname. Distinct from ``nextcloud_public_issuer_url`` + # because, in external-IdP (Keycloak/OIDC) deployments, the OAuth issuer is + # the IdP while Login Flow v2 must still point the browser at Nextcloud. + # Falls back to ``nextcloud_public_issuer_url`` then ``nextcloud_host`` (see + # ``nextcloud_browser_url``) so single-IdP (login-flow) deployments that set + # only NEXTCLOUD_PUBLIC_ISSUER_URL keep working unchanged. + nextcloud_public_url: str | None = None + # Browser cookie Secure flag. None = auto-detect from nextcloud_host # scheme (https → True, else False). Set COOKIE_SECURE=true/false to # override. @@ -1006,6 +1024,26 @@ class Settings: # control plane to pull. See nextcloud_mcp_server/usage/store.py. usage_metering_enabled: bool = False + @property + def nextcloud_browser_url(self) -> str | None: + """Browser-reachable base URL of the Nextcloud instance. + + Resolves the URL the *user's browser* must use to reach Nextcloud for + Login Flow v2 login pages and elicitation links. Prefers the dedicated + ``nextcloud_public_url``; falls back to ``nextcloud_public_issuer_url`` + (correct in single-IdP / login-flow deployments where the OAuth issuer + IS Nextcloud) and finally the internal ``nextcloud_host``. + + In external-IdP mode (e.g. Keycloak) set ``NEXTCLOUD_PUBLIC_URL`` so this + does not fall back to the IdP issuer URL, which would send the browser to + the IdP instead of Nextcloud. + """ + return ( + self.nextcloud_public_url + or self.nextcloud_public_issuer_url + or self.nextcloud_host + ) + def __post_init__(self): """Validate configuration and set defaults.""" @@ -1588,6 +1626,7 @@ def get_settings() -> Settings: "nextcloud_password": "NEXTCLOUD_PASSWORD", "nextcloud_app_password": "NEXTCLOUD_APP_PASSWORD", "nextcloud_public_issuer_url": "NEXTCLOUD_PUBLIC_ISSUER_URL", + "nextcloud_public_url": "NEXTCLOUD_PUBLIC_URL", "cookie_secure": "COOKIE_SECURE", # Nextcloud SSL/TLS settings "nextcloud_verify_ssl": "NEXTCLOUD_VERIFY_SSL", diff --git a/nextcloud_mcp_server/server/auth_tools.py b/nextcloud_mcp_server/server/auth_tools.py index c9ce59b41..514a450d9 100644 --- a/nextcloud_mcp_server/server/auth_tools.py +++ b/nextcloud_mcp_server/server/auth_tools.py @@ -113,7 +113,7 @@ async def nc_auth_provision_access( flow_client = LoginFlowV2Client( nextcloud_host=nextcloud_host, verify_ssl=get_nextcloud_ssl_verify(), - public_host=settings.nextcloud_public_issuer_url, + public_host=settings.nextcloud_browser_url, ) init_response = await flow_client.initiate() except Exception as e: @@ -259,7 +259,7 @@ async def nc_auth_check_status( flow_client = LoginFlowV2Client( nextcloud_host=nextcloud_host, verify_ssl=get_nextcloud_ssl_verify(), - public_host=settings.nextcloud_public_issuer_url, + public_host=settings.nextcloud_browser_url, ) poll_result = await flow_client.poll( poll_endpoint=session["poll_endpoint"], @@ -441,7 +441,7 @@ async def nc_auth_update_scopes( flow_client = LoginFlowV2Client( nextcloud_host=nextcloud_host, verify_ssl=get_nextcloud_ssl_verify(), - public_host=settings.nextcloud_public_issuer_url, + public_host=settings.nextcloud_browser_url, ) init_response = await flow_client.initiate() except Exception as e: diff --git a/tests/server/keycloak/conftest.py b/tests/server/keycloak/conftest.py new file mode 100644 index 000000000..bd37716d1 --- /dev/null +++ b/tests/server/keycloak/conftest.py @@ -0,0 +1,339 @@ +"""Fixtures for Keycloak-service Login Flow v2 integration tests (port 8002). + +The ``mcp-keycloak`` service uses Keycloak as the external OAuth IdP but reaches +Nextcloud via Login Flow v2 **app passwords** (``MCP_DEPLOYMENT_MODE=login_flow``), +exactly like the ``mcp-login-flow`` service — the only difference is the OAuth +IdP (Keycloak vs Nextcloud's built-in ``oidc`` app). The keycloak lane previously +only had DCR/authorize tests; it never provisioned a Login Flow v2 app password or +issued a real Nextcloud API call. These fixtures fill that gap end-to-end: + +* **OAuth leg** — a Keycloak direct-grant (ROPC) obtains an access token that the + ``mcp-keycloak`` session accepts. This exercises the keycloak service without + driving Keycloak's browser login form (its identity is irrelevant here — the + Login Flow v2 app password is what authenticates DAV requests). +* **Login Flow v2 leg** — the browser completes Nextcloud Login Flow v2 by logging + in as a *local* Nextcloud user using its **email address**. Nextcloud keys the + resulting app password on the *loginName* (the email), which differs from the + user's canonical UID (loginName != UID). + +Getting these fixtures to provision at all is the regression guard for +``NEXTCLOUD_PUBLIC_URL``: in external-IdP mode the OAuth issuer URL is Keycloak, +so without a dedicated browser-reachable Nextcloud URL the Login Flow v2 +``login_url`` is rewritten to Keycloak's origin and 404s. + +Relation to PR #980: the login-by-email leg produces the same ``loginName != UID`` +identity shape as #980's client fix, but it does NOT reproduce #980's wrong-path +failure on the CI Nextcloud versions — Nextcloud resolves +``/remote.php/dav/files//`` to the user's real home (email is a valid path +alias), so the WebDAV round-trip succeeds with or without the principal-discovery +fix. Neither can a plain Keycloak/``user_oidc`` login: ``user_oidc``'s +``LoginController`` hardcodes ``loginName == UID`` (the sha256 hash), so its DAV +paths are already correct. #980's failure mode needs a backend (e.g. LDAP) where +the loginName is not a valid files-path alias; it is covered by #980's own mocked +unit tests. +""" + +import json +import logging +import uuid +from typing import Any, AsyncGenerator + +import anyio +import httpx +import pytest +from mcp import ClientSession +from mcp.types import ElicitRequestParams, ElicitResult + +from nextcloud_mcp_server.client import NextcloudClient +from tests.conftest import ( + create_mcp_client_session, +) +from tests.server.login_flow.conftest import _rewrite_login_flow_url + +logger = logging.getLogger(__name__) + +KEYCLOAK_MCP_URL = "http://localhost:8002/mcp" +KEYCLOAK_MCP_BASE_URL = "http://localhost:8002" +KEYCLOAK_BASE_URL = "http://localhost:8888" +KEYCLOAK_REALM = "nextcloud-mcp" + +# Static confidential client from keycloak/realm-export.json. It permits the +# test callback (redirectUris include http://localhost:*) and carries audience +# mappers for both `nextcloud-mcp-server` (MCP validation) and `nextcloud` +# (user_oidc validation). +KEYCLOAK_CLIENT_ID = "nextcloud-mcp-server" +# Dev-only value mirrored from keycloak/realm-export.json + docker-compose.yml, +# not a real secret. NOSONAR suppresses the hardcoded-credentials hotspot. +KEYCLOAK_CLIENT_SECRET = "mcp-secret-change-in-production" # NOSONAR(S2068) + +# Keycloak user used only for the OAuth leg (session identity key). It does not +# have to match the Nextcloud data user — the app password minted by the Login +# Flow leg is what authenticates DAV requests. Direct Access Grants (ROPC) are +# enabled for the `nextcloud-mcp-server` client in realm-export.json. +KEYCLOAK_OAUTH_USER = "admin" +KEYCLOAK_OAUTH_PASSWORD = "admin" # NOSONAR(S2068) - dev-only Keycloak bootstrap creds + +# Scopes registered on the Keycloak `nextcloud-mcp-server` client +# (realm-export.json optionalClientScopes). This deliberately EXCLUDES +# ``talk.read``/``talk.write``: those are part of ``DEFAULT_FULL_SCOPES`` but are +# NOT registered on the Keycloak client, so requesting them makes Keycloak reject +# the whole token request with ``invalid_scope``. ``files.read``/``files.write`` +# are what the WebDAV reproduction test actually needs. +KEYCLOAK_SUPPORTED_SCOPES = ( + "openid profile email " + "notes.read notes.write " + "calendar.read calendar.write " + "todo.read todo.write " + "contacts.read contacts.write " + "cookbook.read cookbook.write " + "deck.read deck.write " + "tables.read tables.write " + "files.read files.write " + "sharing.read sharing.write" +) + + +@pytest.fixture(scope="session") +async def divergent_email_user( + anyio_backend, nc_client: NextcloudClient +) -> AsyncGenerator[dict[str, str], Any]: + """Create a local Nextcloud user whose loginName (email) differs from its UID. + + Yields a dict with ``uid``, ``email``, ``password`` and ``display_name``. + The user is deleted on teardown. Nextcloud login-by-email is enabled by + default, so logging in with the email during Login Flow v2 produces an app + password whose stored loginName is the email — not the UID. + + Session-scoped: the ``mcp-keycloak`` app-password store is keyed by the + Keycloak OAuth identity (a single shared ``admin``), so all tests share one + provisioned app password. The divergent user must therefore stay alive for + the whole session — a per-test user would be deleted while its app password + is still cached server-side, turning the WebDAV reproduction into a spurious + 401-on-deleted-user instead of the #980 wrong-path failure. + """ + suffix = uuid.uuid4().hex[:8] + uid = f"divprincipal_{suffix}" + user = { + "uid": uid, + "email": f"{uid}@example.com", + "password": "DivergentPrincipalPass123!", # NOSONAR(S2068) - ephemeral test user + "display_name": f"Divergent Principal {suffix}", + } + + logger.info("Creating divergent-principal user uid=%s email=%s", uid, user["email"]) + await nc_client.users.create_user( + userid=uid, + password=user["password"], + display_name=user["display_name"], + email=user["email"], + ) + + try: + yield user + finally: + try: + await nc_client.users.delete_user(uid) + logger.info("Deleted divergent-principal user %s", uid) + except Exception as e: # noqa: BLE001 - best-effort cleanup + logger.warning("Failed to delete divergent-principal user %s: %s", uid, e) + + +@pytest.fixture(scope="session") +async def keycloak_service_oauth_token(anyio_backend) -> str: + """Obtain a Keycloak access token accepted by the ``mcp-keycloak`` session. + + Uses the OAuth 2.0 Resource Owner Password Credentials (direct access) + grant against the static ``nextcloud-mcp-server`` client. The OAuth leg's + identity is irrelevant to the reproduction — the Login Flow v2 app password + is what authenticates DAV requests — so there is no need to drive Keycloak's + browser login form here. Direct grant is faster and avoids the flakiness of + the auth-code + Playwright flow (whose native ``#username`` login page also + breaks when the request carries scopes the client does not know about). + """ + async with httpx.AsyncClient(timeout=30.0) as http: + discovery = await http.get( + f"{KEYCLOAK_BASE_URL}/realms/{KEYCLOAK_REALM}" + "/.well-known/openid-configuration" + ) + try: + discovery.raise_for_status() + except httpx.HTTPStatusError as e: + pytest.skip(f"Keycloak realm not available: {e}") + token_endpoint = discovery.json()["token_endpoint"] + + token_resp = await http.post( + token_endpoint, + data={ + "grant_type": "password", + "client_id": KEYCLOAK_CLIENT_ID, + "client_secret": KEYCLOAK_CLIENT_SECRET, + "username": KEYCLOAK_OAUTH_USER, + "password": KEYCLOAK_OAUTH_PASSWORD, + "scope": KEYCLOAK_SUPPORTED_SCOPES, + }, + ) + token_resp.raise_for_status() + access_token = token_resp.json()["access_token"] + + logger.info("Obtained Keycloak OAuth token (direct grant) for mcp-keycloak session") + return access_token + + +async def _complete_login_flow_v2_with_email( + browser, login_url: str, email: str, password: str +) -> None: + """Complete Nextcloud Login Flow v2 logging in as a local user via EMAIL. + + Identical to the login_flow helper, but fills the Nextcloud login form's + user field with the *email* address so the resulting app password's stored + loginName is the email (not the UID) — the ``loginName != UID`` identity + shape exercised by this lane. + """ + login_url = _rewrite_login_flow_url(login_url) + + context = await browser.new_context(ignore_https_errors=True) + page = await context.new_page() + try: + logger.info("Opening Login Flow v2 URL: %s...", login_url[:80]) + await page.goto(login_url, wait_until="networkidle", timeout=60000) + + # Step 1: "Connect to your account" -> click "Log in" (exact match; the + # connect page also renders "Alternative log in using app password"). + login_btn = page.get_by_role("button", name="Log in", exact=True) + try: + await login_btn.wait_for(timeout=10000) + await login_btn.click() + await page.wait_for_load_state("networkidle", timeout=30000) + except Exception: + logger.info("No 'Log in' button - may already be on login/grant page") + + # Step 2: native login form -> fill EMAIL as the user identifier. + user_field = page.locator('input[name="user"]') + if await user_field.count() > 0: + logger.info("Login form detected, logging in via email %s", email) + await user_field.fill(email) + await page.locator('input[name="password"]').fill(password) + await page.get_by_role("button", name="Log in", exact=True).click() + await page.wait_for_load_state("networkidle", timeout=60000) + else: + logger.info("No login form - already logged in via session") + + # Step 3: "Account access" grant page -> "Grant access". + grant_btn = page.get_by_role("button", name="Grant access") + try: + await grant_btn.wait_for(timeout=15000) + await grant_btn.click() + except Exception as e: + logger.warning("No Grant access button: %s", e) + # Debug artifact uploaded by CI on failure (test.yml collects + # /tmp/*.png). NOSONAR suppresses the world-writable-dir hotspot. + shot_path = "/tmp/keycloak_login_flow_no_grant.png" # NOSONAR(S5443) + await page.screenshot(path=shot_path) + + # Step 4: password confirmation dialog. + confirm_password = page.get_by_role("dialog").get_by_role( + "textbox", name="Password" + ) + try: + await confirm_password.wait_for(timeout=10000) + await confirm_password.fill(password) + confirm_btn = page.get_by_role("dialog").get_by_role( + "button", name="Confirm" + ) + await confirm_btn.wait_for(timeout=5000) + await confirm_btn.click() + except Exception: + logger.info( + "No password confirmation dialog (may have been auto-confirmed)" + ) + + # Step 5: "Account connected" success page. + try: + await page.get_by_text("Account connected").wait_for(timeout=15000) + logger.info("Login Flow v2 completed: Account connected!") + except Exception: + await page.wait_for_load_state("networkidle", timeout=10000) + logger.info("Login Flow v2 done. Final URL: %s", page.url) + finally: + await context.close() + + +@pytest.fixture(scope="session") +async def nc_mcp_keycloak_email_client( + anyio_backend, + keycloak_service_oauth_token: str, + browser, + divergent_email_user: dict[str, str], +) -> AsyncGenerator[ClientSession, Any]: + """Provisioned ``mcp-keycloak`` session whose app password loginName is an email. + + Session-scoped so a single Login Flow v2 provisioning (one browser login) + serves every test in the keycloak lane. This is both faster and correct: + the app password is keyed by the shared Keycloak ``admin`` identity, so + re-provisioning per test would just hit ``already_provisioned`` and reuse the + same stored password anyway (see ``divergent_email_user``). + + 1. Connects to mcp-keycloak (8002) with a Keycloak OAuth token. + 2. Calls ``nc_auth_provision_access`` to start Login Flow v2. + 3. Completes the browser login as the local ``divergent_email_user`` **via + its email**, minting an app password whose loginName is the email. + 4. Polls ``nc_auth_check_status`` until provisioned, then yields the session. + """ + email = divergent_email_user["email"] + password = divergent_email_user["password"] + login_url_holder: dict[str, str] = {} + + async def elicitation_callback( + context: Any, params: ElicitRequestParams + ) -> ElicitResult: + for line in params.message.split("\n"): + stripped = line.strip() + if stripped.startswith("http") and "/login/v2/" in stripped: + login_url_holder["url"] = stripped + break + if "url" in login_url_holder: + await _complete_login_flow_v2_with_email( + browser, login_url_holder["url"], email, password + ) + return ElicitResult(action="accept", content={"acknowledged": True}) + + async with create_mcp_client_session( + url=KEYCLOAK_MCP_URL, + token=keycloak_service_oauth_token, + client_name="Keycloak MCP (email login)", + elicitation_callback=elicitation_callback, + ) as session: + provision_result = await session.call_tool( + "nc_auth_provision_access", {"scopes": None} + ) + provision_data = json.loads(provision_result.content[0].text) + logger.info("Provision status: %s", provision_data.get("status")) + + if provision_data.get("status") == "login_required": + login_url = provision_data.get("login_url") + if login_url and "url" not in login_url_holder: + await _complete_login_flow_v2_with_email( + browser, login_url, email, password + ) + + for attempt in range(15): + status_result = await session.call_tool("nc_auth_check_status", {}) + status_data = json.loads(status_result.content[0].text) + status = status_data.get("status") + logger.info("Status %s/15: %s", attempt + 1, status) + if status == "provisioned": + logger.info( + "Provisioned. Stored loginName=%s (expected email=%s)", + status_data.get("username"), + email, + ) + break + if status in ("not_initiated", "error"): + raise RuntimeError( + f"Login Flow v2 failed: {status_data.get('message')}" + ) + await anyio.sleep(2) + else: + raise TimeoutError("Login Flow v2 did not complete after 15 attempts") + + yield session diff --git a/tests/server/keycloak/test_keycloak_login_flow_webdav.py b/tests/server/keycloak/test_keycloak_login_flow_webdav.py new file mode 100644 index 000000000..5fe9a7c21 --- /dev/null +++ b/tests/server/keycloak/test_keycloak_login_flow_webdav.py @@ -0,0 +1,116 @@ +"""End-to-end WebDAV coverage for the ``mcp-keycloak`` service (port 8002). + +The keycloak lane previously only covered DCR / authorize — it never provisioned +a Login Flow v2 app password or exercised a real Nextcloud API call. These tests +close that gap: they drive a full Keycloak-fronted Login Flow v2 provisioning and +then run a WebDAV round-trip through the resulting app password. + +``mcp-keycloak`` uses Keycloak as the external OAuth IdP but reaches Nextcloud via +Login Flow v2 app passwords (``MCP_DEPLOYMENT_MODE=login_flow``), exactly like +``mcp-login-flow``; only the OAuth IdP differs. Getting these tests to pass +requires the Login Flow v2 ``login_url`` to point the browser at *Nextcloud* — so +they are also the regression guard for ``NEXTCLOUD_PUBLIC_URL`` (in external-IdP +mode the OAuth issuer URL is Keycloak, not Nextcloud; without the dedicated +public-URL setting the login page 404s on Keycloak). + +The fixtures log into Nextcloud Login Flow v2 as a *local* user via its **email**, +so the app password's stored ``loginName`` is the email while the canonical UID is +``divprincipal_`` (loginName != UID). This exercises the same +identity-divergence shape as PR #980's client fix. Note: it does not reproduce +#980's wrong-path failure on the CI Nextcloud versions — Nextcloud resolves +``/remote.php/dav/files//`` to the user's real home, so the round-trip +succeeds regardless of the client-side principal-discovery fix. #980's failure +mode needs a backend (e.g. LDAP) where the loginName is not a valid files-path +alias; it is covered by #980's own mocked unit tests. +""" + +import json +import logging + +import pytest +from mcp import ClientSession + +logger = logging.getLogger(__name__) + +pytestmark = [pytest.mark.integration, pytest.mark.keycloak] + + +async def test_login_flow_stores_email_login_name( + nc_mcp_keycloak_email_client: ClientSession, + divergent_email_user: dict[str, str], +): + """Login Flow v2 via email stores the email as the app password loginName. + + Confirms the Keycloak-fronted Login Flow v2 provisioning completed and that + logging in by email yields a stored loginName that differs from the canonical + UID (loginName != UID) — the identity shape PR #980 targets. + """ + status_result = await nc_mcp_keycloak_email_client.call_tool( + "nc_auth_check_status", {} + ) + status_data = json.loads(status_result.content[0].text) + + assert status_data.get("status") == "provisioned" + login_name = status_data.get("username") + assert login_name == divergent_email_user["email"], ( + f"Expected loginName to be the email {divergent_email_user['email']!r}, " + f"got {login_name!r}" + ) + assert login_name != divergent_email_user["uid"], ( + "Expected loginName (email) to differ from the canonical UID." + ) + + +async def test_webdav_round_trip_via_keycloak_login_flow( + nc_mcp_keycloak_email_client: ClientSession, + divergent_email_user: dict[str, str], +): + """A full WebDAV cycle works end-to-end through the keycloak service. + + Exercises create/write/read/list/delete against Nextcloud using the app + password minted by Keycloak-fronted Login Flow v2. This is the keycloak-lane + counterpart of the existing ``tests/server/login_flow`` WebDAV coverage, and + it doubles as the regression guard for ``NEXTCLOUD_PUBLIC_URL`` — if the + Login Flow v2 ``login_url`` were rewritten to the Keycloak origin again, the + session fixture could not provision and this test would never run. + """ + suffix = divergent_email_user["uid"].split("_")[-1] + dir_path = f"/KeycloakLoginFlowTest_{suffix}" + file_path = f"{dir_path}/keycloak_login_flow.txt" + content = f"webdav round-trip via keycloak service {suffix}" + + mkdir_result = await nc_mcp_keycloak_email_client.call_tool( + "nc_webdav_create_directory", {"path": dir_path} + ) + assert mkdir_result.isError is False, ( + "create_directory failed — Keycloak Login Flow v2 WebDAV path is broken" + ) + + try: + write_result = await nc_mcp_keycloak_email_client.call_tool( + "nc_webdav_write_file", + {"path": file_path, "content": content}, + ) + assert write_result.isError is False + + read_result = await nc_mcp_keycloak_email_client.call_tool( + "nc_webdav_read_file", {"path": file_path} + ) + assert read_result.isError is False + read_data = json.loads(read_result.content[0].text) + assert content in read_data.get("content", "") + + list_result = await nc_mcp_keycloak_email_client.call_tool( + "nc_webdav_list_directory", {"path": dir_path} + ) + assert list_result.isError is False + list_data = json.loads(list_result.content[0].text) + names = [f.get("name", "") for f in list_data.get("files", [])] + assert "keycloak_login_flow.txt" in names + finally: + await nc_mcp_keycloak_email_client.call_tool( + "nc_webdav_delete_resource", {"path": file_path} + ) + await nc_mcp_keycloak_email_client.call_tool( + "nc_webdav_delete_resource", {"path": dir_path} + ) diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index 1741ba9f8..ebf14dee2 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -630,3 +630,34 @@ def test_valid_log_format_json(self): _reload_config() settings = get_settings() assert settings.log_format == "json" + + +class TestNextcloudBrowserUrl: + """Test the ``nextcloud_browser_url`` resolver property (Login Flow v2 rewrite).""" + + def test_prefers_public_url(self): + """nextcloud_public_url wins — the external-IdP (Keycloak) case.""" + settings = Settings( + nextcloud_public_url="https://nc.example.com", + nextcloud_public_issuer_url="https://keycloak.example.com/realms/x", + nextcloud_host="https://app.internal", + ) + assert settings.nextcloud_browser_url == "https://nc.example.com" + + def test_falls_back_to_public_issuer(self): + """Without public_url, the OAuth issuer URL is used (single-IdP case).""" + settings = Settings( + nextcloud_public_issuer_url="https://nc.example.com", + nextcloud_host="https://app.internal", + ) + assert settings.nextcloud_browser_url == "https://nc.example.com" + + def test_falls_back_to_host(self): + """With neither public URL set, the internal host is used.""" + settings = Settings(nextcloud_host="https://app.internal") + assert settings.nextcloud_browser_url == "https://app.internal" + + def test_none_when_nothing_set(self): + """Returns None when no Nextcloud URL is configured at all.""" + settings = Settings() + assert settings.nextcloud_browser_url is None diff --git a/tests/unit/test_elicitation.py b/tests/unit/test_elicitation.py index 4183ebb8c..c7f3ca775 100644 --- a/tests/unit/test_elicitation.py +++ b/tests/unit/test_elicitation.py @@ -15,13 +15,35 @@ def _fake_settings( - public_issuer_url: str | None = None, host: str | None = None + public_issuer_url: str | None = None, + host: str | None = None, + public_url: str | None = None, ) -> SimpleNamespace: - """Build a Settings-shaped object exposing only the fields elicitation reads.""" + """Build a Settings-shaped object exposing only the fields elicitation reads. + + ``nextcloud_browser_url`` mirrors the real ``Settings`` property's fallback + chain (public_url → public_issuer_url → host). + """ return SimpleNamespace( + nextcloud_public_url=public_url, nextcloud_public_issuer_url=public_issuer_url, nextcloud_host=host, + nextcloud_browser_url=public_url or public_issuer_url or host, + ) + + +def test_astrolabe_settings_url_prefers_public_url(): + """nextcloud_public_url wins over the OAuth issuer URL (external-IdP mode).""" + fake = _fake_settings( + public_url="https://nc.example.com", + public_issuer_url="https://keycloak.example.com/realms/x", + host="https://internal.example", ) + with patch("nextcloud_mcp_server.auth.elicitation.get_settings", return_value=fake): + assert ( + _astrolabe_settings_url() + == f"https://nc.example.com{ASTROLABE_SETTINGS_PATH}" + ) def test_astrolabe_settings_url_prefers_public_issuer():