Round-1 review follow-ups:
- Troubleshooting "Access forbidden": note that existing users must re-authorize
once after switching to a static client (stored sessions were issued to the
now-deleted DCR client).
- Default IdP setup: explain that the `/mcp` resource identifier works because
`_has_mcp_audience` accepts both the bare server URL and the `/mcp` form.
- env.sample.oauth-multi-user: use angle-bracket placeholders
(`<your-client-id>`) to match the template convention and fail loudly if
copied verbatim.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Self-hosting login_flow against Nextcloud's built-in `oidc` app breaks after
~1h when relying on the DCR fallback: the `oidc` app deletes
dynamically-registered clients after `client_expire_time` (default 3600s),
pruning on every /authorize. The MCP server caches the now-deleted client, so
authorize/refresh fail with an "Access forbidden" page permanently — surviving
server restart and connector recreation (issue #907).
- docs/login-flow-v2.md: add "Default IdP setup (Nextcloud oidc app)" with
static-client steps, and a Troubleshooting entry for the #907 symptom/fix;
reframe the OIDC-client env vars as strongly recommended.
- docs/configuration.md: promote NEXTCLOUD_OIDC_CLIENT_ID/_SECRET to strongly
recommended with a DCR-expiry warning; add them to the login_flow example.
- docker-compose.yml: clarify the DCR caveat and point self-hosters to a static
client for login_flow / background sync.
- env.sample.oauth-multi-user: fix the removed `oauth_single_audience` value
(now login_flow) and require a static OIDC client.
- env.sample.oauth-advanced: remove — it configured the removed OAuth
token-exchange mode (no implementation remains; the mode value now errors at
startup). Drop its references in configuration.md / configuration-migration-v2.md.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Once OAUTH_SINGLE_AUDIENCE was renamed to LOGIN_FLOW and the validation
gate ensured the only meaningful configuration was
`MCP_DEPLOYMENT_MODE=login_flow + ENABLE_LOGIN_FLOW=true`, the two
controls became redundant. Setting the mode is sufficient; the
ENABLE_LOGIN_FLOW env var doesn't add information.
This commit makes the deployment mode the single source of truth for
the Login Flow v2 toggle:
- `nextcloud_mcp_server/config.py`: drop the `ENABLE_LOGIN_FLOW`
dynaconf env-var alias. The `enable_login_flow` field stays as an
internal attribute so the 6 runtime call sites (app.py x4,
context.py, auth/scope_authorization.py) keep working unchanged.
Updated field docstring to flag it as derived.
- `nextcloud_mcp_server/config_validators.py`:
- Drop `enable_login_flow` from `MODE_REQUIREMENTS[LOGIN_FLOW].required`.
- Drop the validation gate that required ENABLE_LOGIN_FLOW=true for
LOGIN_FLOW mode (no longer possible to misconfigure — the flag is
derived, not user input).
- Add `_sync_derived_flags()` helper called at every return path of
`detect_auth_mode` to set `settings.enable_login_flow` from the
resolved mode.
- `tests/unit/test_config_validators.py`: drop `enable_login_flow=True`
from happy-path fixtures (no longer needed — detection sets it).
Repurpose `test_login_flow_requires_enable_login_flow_flag` into
`test_login_flow_mode_auto_derives_enable_login_flow_flag` which
asserts the new auto-derivation behaviour for both LOGIN_FLOW and a
non-LOGIN_FLOW mode.
- `docker-compose.yml`: remove `ENABLE_LOGIN_FLOW=true` from the
`mcp-login-flow` and `mcp-keycloak` profiles.
- `env.sample`: remove the ENABLE_LOGIN_FLOW reference; the comment
on `MCP_DEPLOYMENT_MODE` now notes the derived flag.
- `docs/configuration.md`, `docs/authentication.md`,
`docs/login-flow-v2.md`, `docs/auth-flows.md`,
`docs/troubleshooting.md`, `docs/ADR-025-*.md`: replace
ENABLE_LOGIN_FLOW=true examples and references with
MCP_DEPLOYMENT_MODE=login_flow.
BREAKING CHANGE: `ENABLE_LOGIN_FLOW` is no longer read from the
environment. Anyone who relied on `ENABLE_LOGIN_FLOW=true` to activate
Login Flow v2 should set `MCP_DEPLOYMENT_MODE=login_flow` instead (or
rely on it being the default when no other auth env vars are set).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Calls out the apps-to-install matrix (user_oidc required, oidc skip,
astrolabe optional), the OIDC clients to register and what each is for,
the per-app scope advertisement requirement on the IdP side, and the
"OAuth succeeded but Nextcloud returns 401" diagnosis path.
Mined from the cbcoutinho/nextcloud-mcp-server#752 thread.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Address four small items from the latest PR #743 review:
- login-flow-v2.md Compose example: add an inline comment +
follow-up note pointing readers at Docker secrets for
TOKEN_ENCRYPTION_KEY (the snippet is likely to be copy-pasted
into production).
- auth-flows.md: rename the third column in the Astrolabe → MCP
Server diagram from "Nextcloud OIDC" to "OIDC Provider" so the
diagram matches the multi-IdP framing in the surrounding prose.
- login-flow-v2.md OAuth Endpoints section: rewrite the
ambiguous "token issuance still comes from the IdP" line to
make the cryptographic separation explicit — the MCP server
exposes /token, but tokens are signed by the IdP's key and
validated against its JWKS; the MCP server has no signing keys
of its own.
- README.md auth bullet: replace the jargony "OAuth-to-MCP
supported, with app-password conversion to Nextcloud" with the
reviewer's clearer wording: "MCP clients authenticate via
OAuth, the server handles Nextcloud app passwords
transparently".
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The docs claimed scopes are mcp:-prefixed (mcp:notes.read,
mcp:notes.write) and that the notes.* pair "covers all Nextcloud
apps". Both are false. Per @require_scopes decorators across
nextcloud_mcp_server/server/, scopes are unprefixed and per-app:
notes.read/write, talk.read/write, files.read/write,
calendar.read/write, contacts.read/write, deck.read/write,
news.read, tables.read/write, cookbook.read/write,
todo.read/write, collectives.read/write, sharing.write,
semantic.read, plus standard OIDC scopes.
Changes:
- login-flow-v2.md: replace the false 2-row "covers all apps"
scope table with the real per-app reference (links to
scope_authorization.discover_all_scopes() as authoritative
source); strip mcp: prefix from intro paragraph, sequence
diagrams, @require_scopes example, WWW-Authenticate header
example. Also fix sticky-session keying advice per reviewer:
route on user identity (sub claim) rather than the raw bearer
token, since tokens rotate on refresh.
- auth-flows.md: clarify "Astrolabe (hosted UI) → MCP" matrix
column header; strip mcp: from sequence diagram and key
characteristics bullet; correct "issued by MCP server" to
"issued by configured IdP" on the Login Flow v2 token.
- authentication.md: strip mcp: from the high-level diagram and
scope-enforcement prose; cross-link to the scope reference.
- configuration.md: add NEXTCLOUD_OIDC_CLIENT_ID,
NEXTCLOUD_OIDC_CLIENT_SECRET, and OIDC_DISCOVERY_URL to the
Login Flow v2 vars table — these were undocumented in the
table after the round-2 multi-IdP fix.
- running.md: drop deprecated `version: '3.8'` from compose
snippets (Compose v2 ignores it and emits warnings).
- testing-oidc-consent.md: fix sample authorize URL and consent
description to use real scope names instead of mcp:-prefixed
ones (the manual test as written would have failed with
invalid_scope).
- CLAUDE.md: replace dead links to deleted oauth-architecture.md,
oauth-setup.md, and audience-validation-setup.md with
login-flow-v2.md.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previous round narrowed the framing too far in the other direction —
made it sound like Nextcloud OIDC is *the* IdP. The MCP server
actually supports any OIDC-compliant provider (Nextcloud's built-in
OIDC, Keycloak, AWS Cognito, Auth0, etc.) selected via
`OIDC_DISCOVERY_URL`. `NEXTCLOUD_OIDC_CLIENT_ID/SECRET` are generic
OIDC client credentials despite the Nextcloud-flavored naming.
Code references:
- IdP discovery: app.py:607-668 (auto-detects integrated vs external
by comparing discovered issuer to NEXTCLOUD_HOST)
- JWKS: unified_verifier.py:71-73 (dynamically discovered, not
hard-coded to Nextcloud)
- IdP selection knob: OIDC_DISCOVERY_URL (config.py)
Changes:
- login-flow-v2.md: redraw "How It Works" diagram to show the IdP as
a separate component; replace "Nextcloud OIDC" with "configurable
IdP" framing throughout; add OIDC_DISCOVERY_URL to the env-var
reference; clarify NEXTCLOUD_OIDC_CLIENT_ID/SECRET are generic OIDC
creds; rename "OAuth Endpoints" subtitle to point at "the configured
IdP".
- running.md: rewrite the OAuth Mode intro and Quick Start note to
mention IdP configurability and OIDC_DISCOVERY_URL.
- configuration.md: update Best Practices "For Production" multi-user
bullet to reference the IdP selector and generic-creds caveat.
- auth-flows.md: generalize Astrolabe-flow and Login Flow v2
characteristics bullets — IdP and JWKS source are configurable.
- keycloak-multi-client-validation.md: REMOVE the "deprecated"
banner I added in 35c115e. The doc covers active behavior in
external-IdP mode (realm-level token validation by user_oidc),
not retired direct-OAuth-to-Nextcloud architecture. Replaced with
a scope note pointing at when this applies.
oauth-impersonation-findings.md keeps its deprecation banner — that
doc *is* about the rejected service-account / impersonation path
(ADR-002 Tier 2, "Will Not Implement"), so the deprecation framing
remains correct there.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous round of review feedback rested on a misunderstanding —
that the MCP server is "the OAuth issuer" under Login Flow v2 and that
NEXTCLOUD_OIDC_CLIENT_ID/SECRET are external-IdP-only. Code says
otherwise (app.py:619/625/703-717, unified_verifier.py:72):
- The MCP server is an OIDC relying party of Nextcloud OIDC. Tokens are
signed by Nextcloud and validated against Nextcloud's JWKS in all
modes — the server has no private signing keys.
- Static NEXTCLOUD_OIDC_CLIENT_ID/SECRET are the preferred way to
register the MCP server as that relying party; RFC 7591 DCR is a
fallback when both are unset.
- Login Flow v2 layers per-user app-password acquisition on top — it
governs the MCP→Nextcloud data leg, not the relying-party setup.
This commit reverts the inaccuracies introduced by 35c115e and reframes
the original `login-flow-v2.md` to match what the code does:
- login-flow-v2.md: revise "How It Works" to describe the MCP server
as an OIDC RP + OAuth facade (not a standalone issuer); rename
"OAuth Issuer Endpoints" → "OAuth Endpoints" with a note that those
endpoints front Nextcloud OIDC; add NEXTCLOUD_OIDC_CLIENT_ID/SECRET
to the required env vars with DCR documented as fallback.
- running.md: restore the static-creds Docker example (deleted in
35c115e on the wrong reasoning that it was tied to the retired
direct-OAuth-to-Nextcloud flow); rewrite the OAuth Mode section
intro to describe the actual relying-party + facade architecture.
- configuration.md: fix Best Practices "For Production" to mention
static creds as preferred / DCR as fallback; restore the .oauth
Docker volume alongside data so DCR-registered MCP-client state and
the encrypted app-password DB both persist.
- auth-flows.md: drop the note added in 35c115e that wrongly claimed
the MCP server validates Bearer tokens against its own JWKS under
Login Flow v2 — it validates against Nextcloud's JWKS in all modes;
reword the Login Flow v2 "Key characteristics" bullet that called
the MCP server "the OAuth authorization server".
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Round-2 cleanup of PR #743 review comments not covered by d153e96:
- configuration.md: fix broken `#multi-user-oauth-modes` anchor; replace
with the two real anchors (Multi-User BasicAuth, Login Flow v2). Rewrite
the stale "always use OAuth2/OIDC with pre-configured clients" Best
Practices section to reflect the post-pivot mode matrix, and update the
Docker volume example to mount the encrypted app-password store
(`TOKEN_STORAGE_DB`) rather than obsolete `.oauth` client storage.
- semantic-search-architecture.md: rename remaining body references from
the deprecated `VECTOR_SYNC_ENABLED` to `ENABLE_SEMANTIC_SEARCH` so the
doc matches configuration.md / troubleshooting.md.
- running.md: relabel "OAuth Mode (Recommended)" as
"Login Flow v2 / OAuth issuer mode (--oauth)", drop the misleading
"(Legacy)" suffix from BasicAuth, drop the
`NEXTCLOUD_OIDC_CLIENT_ID/SECRET` example (tied to the retired
direct-OAuth-to-Nextcloud flow), and add a note explaining what
`--oauth` actually enables post-pivot.
- keycloak-multi-client-validation.md, oauth-impersonation-findings.md:
add a deprecation banner pointing at ADR-022 / Login Flow v2. Files
retained because ADR-002 and CLAUDE.md still cite them.
- auth-flows.md: clarify under the Astrolabe → MCP diagram that the
Nextcloud-OIDC JWKS path applies to Multi-User BasicAuth; under
Login Flow v2 the MCP server validates tokens against its own JWKS.
- login-flow-v2.md: clarify the sticky-session note — affinity must key
on the OAuth bearer token (or user-bound cookie), not source IP, since
MCP clients may not maintain stable IPs across the provisioning flow.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace the seven OAuth-to-Nextcloud docs (oauth-setup, quickstart-oauth,
oauth-architecture, oauth-upstream-status, oauth-troubleshooting,
jwt-oauth-reference, audience-validation-setup) with a single new
docs/login-flow-v2.md. The deprecated flow required upstream user_oidc
patches that were never merged; Login Flow v2 is the forward-looking
multi-user mode (see ADR-022), and works with stock Nextcloud 16+.
Rewrite docs/authentication.md and docs/auth-flows.md around three modes:
Single-User BasicAuth, Multi-User BasicAuth pass-through, and Login Flow v2.
Update README to add an Astrolabe Cloud (https://astrolabecloud.com)
callout for users who prefer not to self-host, drop the OAuth deployment
mode from the auth table, simplify the Docker block, and trim the
Examples and Security sections.
Sweep configuration.md, installation.md, troubleshooting.md, running.md,
and semantic-search-architecture.md to replace links to the deleted docs
and update deprecated mode names.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
