fix(login-flow): resolve Login Flow v2 login_url to Nextcloud in external-IdP mode

[cbcoutinho]

What

Fixes a Login Flow v2 bug in external-IdP (Keycloak/OIDC) deployments and adds the missing end-to-end Keycloak Login Flow v2 WebDAV coverage that surfaced it. Started as an attempt to reproduce #980 in the keycloak lane; that investigation found the reproduction isn't feasible on the CI Nextcloud versions (see below) but uncovered two real bugs on the way.

Bug fixed: Login Flow v2 login_url rewritten to the IdP origin

mcp-keycloak rewrote the Login Flow v2 login_url origin to NEXTCLOUD_PUBLIC_ISSUER_URL. That setting is overloaded — it's the OAuth issuer for JWT iss validation and was reused as the browser-reachable Nextcloud URL for the Login Flow redirect. When Nextcloud is its own IdP these coincide; with an external IdP the issuer is the IdP, so the browser-facing login URL was rewritten onto the IdP origin (http://localhost:8888/login/v2/flow/...) — which has no /login/v2 endpoint and 404s. This breaks Login Flow v2 for any external-IdP deployment.

Fix: a dedicated NEXTCLOUD_PUBLIC_URL (browser-reachable Nextcloud base URL), resolved via a new Settings.nextcloud_browser_url property with a backward-compatible fallback chain (nextcloud_public_url → nextcloud_public_issuer_url → nextcloud_host). Login Flow v2 / elicitation rewrites use it; OAuth issuer / JWT validation stays on nextcloud_public_issuer_url. Wired for mcp-keycloak in docker-compose.yml. Existing login-flow deployments that set only NEXTCLOUD_PUBLIC_ISSUER_URL are unchanged.

Also fixed: keycloak OAuth-leg fixture

The OAuth-leg fixture requested talk.* scopes not registered on the Keycloak client → invalid_scope → Keycloak error page → Playwright #username timeout. Replaced the flaky browser auth-code flow with a direct-grant (ROPC) token using realm-supported scopes (the OAuth-leg identity is irrelevant — the Login Flow v2 app password authenticates DAV). The divergent user + provisioned session are now session-scoped (one live user, one browser login) since the app-password store is keyed by the shared Keycloak admin identity.

Tests

tests/server/keycloak/test_keycloak_login_flow_webdav.py — end-to-end Keycloak Login Flow v2 WebDAV round-trip (create/write/read/list/delete). Fills the keycloak-lane coverage gap (previously only DCR/authorize) and guards the NEXTCLOUD_PUBLIC_URL fix — without it the session can't provision. Plus a Settings.nextcloud_browser_url unit test and a NEXTCLOUD_PUBLIC_URL-preference elicitation test. Docs updated in configuration.md and login-flow-v2.md.

Relationship to #980 (why this is not a live reproduction)

The original goal was to reproduce #980's divergent-principal DAV bug via keycloak login-by-email. That doesn't reproduce on NC32/33, verified against a live stack:

• Login-by-email gives loginName != UID, but Nextcloud resolves /remote.php/dav/files/<email>/ to the user's real home (email is a valid path alias), so the WebDAV cycle succeeds with or without fix(client): resolve DAV paths via current-user-principal discovery #980's fix.
• user_oidc (the Keycloak backend) hardcodes loginName == UID (the sha256 hash) in LoginController.php — no config knob changes this.
• fix(client): resolve DAV paths via current-user-principal discovery #980's failure mode needs a backend where the loginName is not a valid files-path alias (LDAP-style, where /files/<login>/ misses /files/<UID>/). That requires LDAP test infra, out of scope here; fix(client): resolve DAV paths via current-user-principal discovery #980's own mocked unit tests cover the fix.

So the keycloak tests are kept as genuine end-to-end coverage (they caught both bugs above), not as a #980 reproduction.

Validation

• uv run ruff check / ruff format --check / ty check — clean
• uv run pytest tests/unit/ — 1927 passed, 1 skipped
• Keycloak integration lane (NC32, local) — 2 passed (WebDAV round-trip GREEN)

---

This PR was generated with the help of AI, and reviewed by a Human

[claude]

Claude finished @cbcoutinho's task in 3m 40s —— View job

---

Code Review

• Gather context (PR body, comments, changed files)
• Read changed files in detail
• Check diff against merge-base (cd74ce9; origin/master...HEAD had no common ancestor in this shallow checkout)
• Review code quality, bugs, security, test coverage
• Post final review

Note: couldn't execute uv run ruff/ty/pytest locally in this sandboxed run (no tool-execution permission for non-git commands), so this is a static read-through plus verification against the codebase's existing conventions. CI (Tests, Pact contract tests) was still in progress at review time — worth confirming green before merge.

Summary

This is a well-scoped, well-documented fix. The core bug (NEXTCLOUD_PUBLIC_ISSUER_URL overloaded as both OAuth issuer and browser-facing Nextcloud URL) is real, and the fix — a dedicated nextcloud_public_url / nextcloud_browser_url property with a backward-compatible fallback chain — is applied consistently everywhere the old nextcloud_public_issuer_url was used for browser redirects (elicitation.py, provision_routes.py ×2, auth_tools.py ×3, userinfo_routes.py), per config.py:1027-1045. Verified no other browser-facing call site of the old field was missed (grep -rn "public_host" and nextcloud_public_issuer_url usages all check out).

Things verified while reading (no issues found)

• Settings.nextcloud_browser_url fallback chain (nextcloud_public_url → nextcloud_public_issuer_url → nextcloud_host) matches the PR description and the new TestNextcloudBrowserUrl unit tests (tests/unit/test_config.py).
• The double rewrite_url_origin call (once inside LoginFlowV2Client.initiate() via public_host, again in provision_routes.py:269 against nextcloud_browser_url) is idempotent — rewriting an already-rewritten URL to the same origin is a no-op — and pre-existing behavior, not introduced by this PR.
• browser fixture dependency on a session-scoped fixture (nc_mcp_keycloak_email_client, divergent_email_user) mirrors the same pattern already used in tests/server/login_flow/conftest.py, so no pytest scope-mismatch risk (pytest-playwright's browser fixture is session-scoped by default).
• nc_client.users.create_user/delete_user signatures match the new divergent_email_user fixture's usage.
• pytest.mark.keycloak is a registered marker (pyproject.toml), and PLC0415/G004 ruff rules aren't violated by the new test files (no f-string logging calls found).
• Settings() construction with no args (used in test_none_when_nothing_set) is an established pattern elsewhere in test_config.py, so it won't hit unrelated __post_init__ validation.
• The two SonarCloud findings raised in the automated review (S5443, S5332) were already addressed in the follow-up commits 59f2aa7/d2eab9d.

Minor observations (non-blocking)

• tests/server/keycloak/conftest.py cross-imports _rewrite_login_flow_url from tests.server.login_flow.conftest (a private, underscore-prefixed helper). Already flagged and consciously deferred by the author in the PR thread as out-of-scope — agree it's reasonable to leave for a future shared-test-support refactor rather than block this PR on it.
• The extensive docstrings in conftest.py/test_keycloak_login_flow_webdav.py explaining the fix(client): resolve DAV paths via current-user-principal discovery #980 non-reproduction are long, but given how easy it'd be for someone to later assume this is a fix(client): resolve DAV paths via current-user-principal discovery #980 regression test, the extra context earns its keep here.

Test coverage

The new test_keycloak_login_flow_webdav.py end-to-end round-trip (create/write/read/list/delete) plus the nextcloud_browser_url unit tests and the NEXTCLOUD_PUBLIC_URL-preference elicitation test give solid regression coverage for the actual bug fixed — without the fix, the Keycloak Login Flow v2 session fixture cannot provision, so these tests would fail/hang rather than silently pass.

No blocking issues found. This looks ready to merge once CI is green.

[cbcoutinho]

Round-1 findings addressed in 8061a38e:

• 🟡 userinfo_routes.py missed the same browser-URL migration: Good catch — fixed. nextcloud_host_for_links (viz-tab app links) now uses settings.nextcloud_browser_url for parity with elicitation.py / provision_routes.py / auth_tools.py. It was indeed a browser-facing link, not issuer validation.
• 🟡 PR description stale: Already addressed before this round — the review ran against the pre-rewrite metadata. The title is now fix(login-flow): resolve Login Flow v2 login_url to Nextcloud in external-IdP mode and the body drops the "RED→GREEN reproduction" framing and explicitly documents that this is not a live fix(client): resolve DAV paths via current-user-principal discovery #980 reproduction on NC32/33 (with the LDAP-backend explanation). This re-review round should see the updated body.

Left as-is (with reasons):

• 🟢 SonarQube "Security Rating D" (hardcoded Keycloak secret/user/pass in conftest.py): Deferred — these are dev-only constants that mirror values already committed in keycloak/realm-export.json and docker-compose.yml (not real credentials), matching the repo's existing convention for the local dev stack. Sonar flags each new literal regardless of duplication; false positive.
• 🟢 Cross-package import of _rewrite_login_flow_url: Deferred — intentional reuse to avoid duplicating the rewrite logic; promoting it to a shared non-underscored test-support module is a broader refactor out of scope for this PR.

ruff / ruff format / ty clean; userinfo + config + elicitation unit tests pass. CI keycloak integration lanes (nc32/nc33) are green on the prior push.

[cbcoutinho]

Round-2 nit addressed in 2a190730: updated the stale module-level comment above ASTROLABE_SETTINGS_PATH in elicitation.py to reference nextcloud_browser_url (the _astrolabe_settings_url docstring just below already did). Thanks for the thorough review — with "No blocking issues found" and the two deferrals agreed, this is merge-ready pending CI.

[sonarqubecloud]

Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud