- _ensure_keyword_payload_indexes: distinguish 400 (schema conflict, warning)
from other status codes (5xx/network, error) so a transient outage doesn't
silently leave the collection unindexed.
- build_search_result_from_point: use .get("doc_id") + return None on missing
instead of KeyError-crashing the search; reverse metadata merge order so
payload-derived chunk_index/total_chunks win over caller-supplied extras.
- docs/configuration.md: restore the OpenAI/Mistral/Bedrock/Simple provider
sections + reference-table rows that were dropped in the rebase. Reword
the "Startup migrations" bullet to describe what the code actually does
(no sampling — full scroll, zero writes when clean). Add operator note
about the SemanticSearchResult.id TypeError path.
- tests: pytest.approx for float equality (Sonar python:S1244); coverage
for non-400 → ERROR, payload={doc_id: None}, and missing doc_id key.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Production was logging two cascading classes of Qdrant errors against the
welcomed-malamute deployment:
1. HTTP 400 — "Bad request: Index required but not found for \"doc_id\" of
one of the following types: [keyword]". The collection was created via
create_collection() with no payload indexes, so any FieldCondition
filter on doc_id failed at the Qdrant layer (placeholder writes/reads,
eviction, search context lookups).
2. Compounding the missing index, producers wrote a mix of int and str
doc_ids: webhook_parser stringified node_id, scanner stringified note
IDs, news IDs, and deck card IDs — but the file scanner passed the
numeric file_id through unchanged. A keyword index would not have
covered both kinds even if it had existed.
This change:
- Normalizes doc_id to str at every producer site (scanner.py:459,
DocumentTask.doc_id, indexed_*_ids reads from Qdrant).
- Tightens str|int annotations to str across placeholder.py,
eviction.py, search/verification.py, search/context.py,
SearchResult.id, and the auth/api visualization endpoints.
- Defensive str() coercion on doc_id reads in semantic.py /
bm25_hybrid.py / vector/visualization.py for the transition window
before the backfill runs.
- Adds an idempotent startup migration in get_qdrant_client():
- _ensure_keyword_payload_indexes creates KEYWORD indexes for
doc_id, user_id, and doc_type (tolerates "already exists" 400s).
- _backfill_doc_id_to_string scrolls the collection once and rewrites
int doc_ids to str. Skipped after a quick sample shows no legacy
int payloads.
- Public API preserved: SemanticSearchResult.id stays int via explicit
int(r.id) narrowing in server/semantic.py — surfaces a TypeError with
actionable context if a future doc_type ships non-numeric ids.
- Documents the startup migration in docs/configuration.md.
Tests: 11 new unit tests in tests/unit/vector/test_qdrant_client.py
covering happy path / already-exists / unrelated-400 for the index
helpers, and sample-skip / mixed-batch rewrite / payload=None edge cases
for the backfill. 889 unit tests pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- test_registry.py: stub `mistralai.client.Mistral` in
`test_registry_mistral_wins_over_ollama`, mirroring the sibling
picker test, so the test doesn't depend on the SDK accepting
arbitrary keys.
- openai.py: convert remaining f-string `logger.info(...)` calls to
lazy `%s` formatting, aligning with the pattern in mistral.py and
the repo's logging convention.
- test_mistral.py: add four tests covering the defensive RuntimeError
guards in `embed()` and `_embed_batch_request()` — empty
response.data, single null embedding, batch null embedding, and
count-mismatch.
- docs/configuration.md: add `AWS_ACCESS_KEY_ID` and
`AWS_SECRET_ACCESS_KEY` rows to the env-var reference table; they
were already mentioned in prose but missing from the table.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- _retry.py: replace `assert last_error is not None` with explicit
`if last_error is None: raise RuntimeError(...)` so the original
rate-limit error is preserved under `python -O`.
- openai.py: drop the `_retry_factory` alias chain; rename the bound
decorator to `_retry_429` to match the pattern in mistral.py.
- mistral.py: comment the imports so future reviewers understand why
`from mistralai.client import …` is the canonical path on 2.x (no
top-level `__init__.py`; no `mistralai.models` subpackage either).
- docs/configuration.md: add `OPENAI_GENERATION_MODEL` and
`OLLAMA_GENERATION_MODEL` rows to the env-var reference table.
- test_mistral.py: add direct unit test for the `_is_rate_limit`
predicate (429 → True, 500 → False, missing-attr → False).
- test_registry.py: stub `mistralai.client.Mistral` in the registry
picker test, mirroring the Ollama sibling, so the test doesn't
depend on the SDK accepting arbitrary keys.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a hosted Mistral embedding option (mistral-embed, 1024-dim) alongside
the existing Bedrock / OpenAI / Ollama / Simple providers. Implementation
mirrors OpenAIProvider: lazy dimension detection with a known-models lookup,
chunked batch requests, defensive index sort, and a 429-aware retry decorator.
In the same change, ProviderRegistry switches from os.getenv to the
dynaconf-backed Settings dataclass so all five providers share a single
configuration path. config.py gains the previously-uncovered Bedrock keys,
the new Mistral keys, the missing OPENAI_GENERATION_MODEL /
OLLAMA_GENERATION_MODEL, and SIMPLE_EMBEDDING_DIMENSION.
Auto-detection priority: Bedrock → OpenAI → Mistral → Ollama → Simple.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Hide sensitive files/folders from the WebDAV MCP tool surface by
tagging them with a configured Nextcloud system tag. Defence-in-depth
control for users who connect LLMs to accounts holding contracts,
medical records, credentials, etc.
A new EXCLUDED_TAGS env var (comma-separated tag names, empty by
default) gates an exclusion layer that runs at the start of every
WebDAV tool call: tag names are resolved to tag IDs, those IDs are
expanded to the set of tagged paths, then listings/searches are
filtered and read/write/delete/move/copy operations on excluded paths
raise ToolError. Tagged folders exclude their descendants via prefix
match. Empty EXCLUDED_TAGS disables the feature entirely.
The threat model is preventing accidental data exfiltration via the
LLM tool surface — not hiding files from a determined operator. The
docs explicitly recommend creating exclusion tags with
user_assignable=false so the credentials the MCP server uses cannot
remove the tag.
Implementation:
- config.py: add `excluded_tags` to _DEFAULTS, Settings, and the
_field_map alongside other comma-separated env vars.
- client/webdav.py: get_files_by_tag now requests <d:resourcetype/>
and surfaces is_directory so tagged directories can recursively
exclude descendants.
- server/tag_exclusion.py (new): get_excluded_tag_names,
get_excluded_file_paths, is_path_excluded.
- server/webdav.py: exclusion guards in all 11 WebDAV tools;
read/write/create/delete/move/copy raise ToolError, list/search
tools silently filter excluded entries. Existing f-string log
calls converted to lazy %-style.
- tests: 17 new unit tests covering path-matching edge cases
(shared-prefix non-match, descendants of excluded dirs), tag-name
parsing, and get_excluded_file_paths with mocked WebDAV; 1 new
client test asserting <d:resourcetype/> -> is_directory parsing.
- docs/configuration.md: new "Tag-Based File Exclusion" section with
per-tool effect table, security guidance, and per-call cost note.
- README.md: feature mention under Key Features.
Closes#710.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three findings from the latest review on #758 (1 medium, 2 low):
Medium:
- browser_oauth_routes.oauth_logout: move delete_browser_session into a
finally block so an error from delete_refresh_token can no longer leave
an orphan browser_sessions row. The orphan was not exploitable
(SessionAuthBackend rejects sessions without a live refresh token), but
it lingered until the hourly cleanup cron — a correctness gap. New
regression test pins the fix.
Low:
- oauth_callback_nextcloud: drop redundant ``or None`` from
``expected_nonce=nonce``. ``nonce`` is already ``str | None`` and
``secrets.token_urlsafe`` never produces an empty string, so the
coercion was a no-op that could mislead future readers into thinking
empty-string was a valid skip-the-check path.
- storage.RefreshTokenStorage.initialize: fail fast at startup when
SQLite < 3.35, since ``DELETE ... RETURNING`` (used in
``delete_browser_session``) needs that minimum. Ubuntu 20.04 ships
3.31 and would otherwise hit OperationalError on every logout.
Prerequisite also documented in docs/installation.md.
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>
- Rename `verified_count` → `verified_chunk_count` to make the count
granularity explicit at the field name (chunks vs unique docs).
- News verifier now fails open *per-item* on non-numeric stored doc_ids
(matches notes/files/deck shape); a single bad id no longer rescues
definitively-missing siblings from eviction.
- Update note-verifier integration test to use string doc_ids end-to-end
to match production storage (scanner.py:241 stringifies note ids).
- Add regression test for the closed-task-group race guard in
`verify_search_results` so the RuntimeError swallow is locked in.
- Convert remaining f-string logger calls in `server/semantic.py` to
lazy %-style formatting (per repo convention).
- Document `evict_on_missing` as a developer/test flag (no env var) and
flag the `get_file_info` 404→raise contract change in its docstring.
- Add a TODO(ADR-019) breadcrumb for the hardcoded 2× over-fetch so
future tuning has a clear hook.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes out the remaining nits flagged in the round-6 review.
Critical:
- _verify_files contract comment now enumerates all None-return cases
(404 + malformed PROPFIND XML) and documents the false-eviction
trade-off; self-healing via re-indexing recovers
- int(r.id) cast at the SemanticSearchResult boundary now raises a
TypeError with explicit doc_type/value context instead of bubbling
up as an opaque "Search failed: ..." McpError
Design observations:
- nc_semantic_search_answer docstring documents the per-note
round-trip cost from the post-verification race guard
- News verification latency hint added to configuration.md
- SemanticSearchResponse exposes verified_count + dropped_count so
short result pages on high-ghost-density indexes are
distinguishable from genuine scarcity. verify_search_results now
returns (kept, dropped_count); production caller and tests updated
Minor:
- Comment clarifies the .get() fallback in verify_search_results is
defensive only (run_verifier always populates the entry)
- Eviction task-group guard narrowed from except Exception to
except RuntimeError (the only documented failure mode of
TaskGroup.start_soon on a closed group)
- Indexer logs a warning when a deck_card task is missing
board_id/stack_id, surfacing data-quality issues at index time
rather than at verification time
- New unit test covers the news verifier's non-numeric-id fail-open
path (one bad doc_id keeps the entire batch)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Tightens verifier consistency, closes test gaps, hardens the fire-and-forget
eviction snapshot, and routes the new concurrency knob through Settings.
- Pre-flight ``int()`` guard in ``_verify_notes`` mirrors ``_verify_deck_cards``,
so a non-numeric note id produces a type-specific log line instead of
falling through to the generic "unexpected error" branch.
- Adds explicit 403 tests for the file and news verifiers (symmetry with the
existing notes/deck 403 tests) plus a ``non_numeric_id_keeps`` test.
- ``AppContext`` and ``OAuthAppContext`` no longer snapshot
``_vector_sync_state.eviction_task_group`` at lifespan-yield time. Both
expose it as a ``@property`` that reads the singleton dynamically, removing
the order-sensitive race where a future startup-ordering change could
silently degrade fire-and-forget eviction to inline forever.
- Adds ``verification_concurrency`` (env var ``VERIFICATION_CONCURRENCY``,
default 20) to ``Settings`` with a dynaconf validator; ``verify_search_results``
resolves the cap lazily from settings when the caller doesn't override it.
- Enriches the news verifier TODO to call out that ``batch_size=-1`` is
intentional — a numeric ceiling would silently break correctness because
any item beyond the cap would be missing from ``present_ids`` and dropped.
- Updates ``Optional[TaskGroup]`` to ``TaskGroup | None`` per project style.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Guard eviction_task_group.start_soon against shutdown race so a
RuntimeError on a closed group never surfaces as a search error.
- Correct ADR-019 news_item row: there is no per-item REST endpoint;
verification batches via get_items(batch_size=-1) and intersects.
- Modernize models/semantic.py typing to PEP 604 / lowercase generics
per CLAUDE.md.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- _verify_deck_cards: hoist int(board_id|stack_id|doc_id) out of the generic
except Exception into an explicit try/except (TypeError, ValueError) before
the network call, mirroring _verify_news_items. Malformed payloads now log
a specific warning instead of "unexpected error".
- _verify_news_items: add TODO(perf) above the get_items(batch_size=-1) call
to mark the known fetch-all cost as a future profiling target.
- SemanticSearchResult.id: revert from int|str back to int. The internal
SearchResult.id stays int|str for forward-compat; the MCP response model
narrows at the boundary. server/semantic.py casts r.id to int when
constructing the response so future string-id types fail loudly here
instead of silently widening the public API.
- nc_semantic_search: replace the terse "extra for access filtering" comment
with an ADR-019 NOTE block explaining the 2x over-fetch trade-off and the
ghost-density under-delivery case (self-heals via lazy eviction).
- tests/integration/test_verify_on_read.py: extend the module docstring to
call out that only the note verifier is exercised against real Nextcloud,
while file/deck_card/news_item are unit-only — documenting the suite split
for future contributors.
- ADR-019: rewrite "Module shape", "Verifier registry", example verifier,
and "Deduplication" sections to match the shipped BatchVerifier interface
(was per-id Verifier in the original draft). Add a "Why batch?" paragraph
explaining the design choice. Update implementation checklist — every
item is now [x] with corrected verifier names (plural) and the eviction
module path (vector/eviction.py).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Implements fire-and-forget eviction (ADR-019 §"Lazy eviction"): the
search response no longer waits on Qdrant deletes, instead spawning
evict() on a long-lived lifespan-owned task group. Falls back to inline
eviction in modes without vector sync and in unit tests.
Also: harden _verify_news_items against non-numeric ids (fail open
instead of crashing the verifier); document the get_file_info None-on-404
contract; add INDEXED_DOC_TYPES single source of truth in vector/scanner.py
referenced by the CI-guard test; write a Verify-on-Read Latency Budget
section in docs/configuration.md covering the unbounded news.get_items
fetch. Closes the two remaining ADR-019 implementation checklist items.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Cap all_results to limit*2 after sort in the per-doc_types branch of
nc_semantic_search to bound over-verification (was unbounded N-types).
- Switch BatchVerifier from (client, doc_ids, user_id) to (client, results,
semaphore). Verifiers now read file paths and deck board/stack ids from
SearchResult.metadata instead of doing fresh Qdrant scrolls — eliminates
one duplicate round-trip per file/deck-card verification.
- Bound per-id verification concurrency with a shared anyio.Semaphore
(default 20, matching server/semantic.py context-expansion convention).
Prevents httpx pool exhaustion / rate limiting on large search pages.
- Propagate stack_id from Qdrant payload to SearchResult.metadata in both
bm25_hybrid.py and semantic.py (board_id was already propagated).
- Drop now-unused _resolve_file_path / _resolve_deck_metadata helpers.
- Drop redundant int(d) in requested predicate from _verify_news_items.
- Rewrite eviction comment to be honest about inline (not background)
execution and the resulting latency coupling.
- ADR-019 status: Proposed -> Accepted.
- Add news property to NextcloudClientProtocol.
- Widen SearchResult.id and SemanticSearchResult.id to int | str to match
BatchVerifier signature and document support for future string-id types.
- Flip openWorldHint to True on nc_semantic_search_answer (it calls into
Nextcloud via nc_semantic_search).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The vector index lags Nextcloud (5-min webhook cron + scanner interval),
producing ghost records for deleted/unshared documents until the next
reconciliation. Verify each unique document against Nextcloud at query
time, drop inaccessible results, and lazily evict the corresponding
Qdrant points.
Per-doc_type batch verifiers: notes/files/deck cards run concurrently
per id; news items use a single fetch + intersect to avoid the per-item
fetch-all amplification. Transient errors fail open (keep result, log
warning) — only definitive 4xx drops. Multiple chunks of the same doc
collapse to one verification call.
Wired into nc_semantic_search before the limit trim and before context
expansion. nc_semantic_search_answer's per-note re-fetch retained as a
sub-second race guard since verification now happens upstream.
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>
Fix issues raised by reviewer on PR #743:
- troubleshooting.md: renumber "Getting Help" steps (4→3, 5→4) after
earlier consolidation left a gap
- installation.md: drop stale "OIDC app" prerequisite; admin access is
now optional under Login Flow v2 (works on stock Nextcloud 16+)
- semantic-search-architecture.md: rename VECTOR_SYNC_ENABLED to
ENABLE_SEMANTIC_SEARCH in the Status callout (renamed in v0.58.0)
- configuration.md: remove Quick Start references to deprecated
oauth-multi-user / oauth-advanced templates and point to
login-flow-v2.md; update "OAuth, Multi-User BasicAuth" label to
"Login Flow v2, Multi-User BasicAuth"
- auth-flows.md: fix background-sync diagram so Encrypt+persist step
no longer crosses into the Nextcloud column
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>
- Use dynaconf (get_settings()) instead of os.getenv for OIDC_RESOURCE_SERVER_ID
- Re-add Settings field, _field_map entry, and settings.toml default
- Add trailing-slash guard (.rstrip("/")) to prevent double-slash in scopes
- Add double-prefixing guard: skip scopes already carrying the prefix
- Add @pytest.mark.unit to test module
- Add test for already-prefixed scopes
- Document OIDC_RESOURCE_SERVER_ID in docs/configuration.md
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The helm chart has been migrated to a dedicated repository at
https://github.com/cbcoutinho/helm-charts. This removes the chart
source, release workflow, bump script, and updates all documentation
to point to the new repository.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Incorporate reviewer feedback across three review rounds:
- Remove post_hooks from Phase 1 constructor; defer to Phase 4
- Fix Validator syntax: use condition=lambda instead of ne= kwarg
- Add MCP_DEPLOYMENT_MODE validator to catch typos at startup
- Add CRITICAL to LOG_LEVEL validator enum
- Make OTEL_TRACES_SAMPLER_ARG validation conditional on ratio samplers
- Add all missing provider env vars to settings.toml (Bedrock, Anthropic, Ollama, Simple)
- Add provider secrets to .secrets.toml.example
- Fix DynaconfDict import to stable public API path
- Strengthen ignore_unknown_envvars risk: CI lint check mandatory before Phase 2
- Document ValidationError vs ValueError breaking change in Phase 3
- Acknowledge environments=True legacy risk with mitigation
- Address root_path pip-install concern (intentional: pip uses env vars)
- Add enable_token_exchange to adapter example; note exhaustive field mapping
- Clarify Provider Registry is Phase 6 with explanation of os.getenv coexistence
- Improve test isolation fixture with teardown reload + _dynaconf visibility note
- Add Docker Compose volume mount host-file existence note
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Many identity providers (AWS Cognito, Okta, Azure AD) reject or mishandle
colons in OAuth scope names. This migrates all custom scopes from
`resource:action` to `resource.action` format (e.g., `notes:read` →
`notes.read`), which is universally accepted and aligns with industry
conventions (Microsoft, Google).
Includes Alembic migration 004 for stored scope strings and ADR-024
documenting the rationale and RFC references.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Address all 9 review points from PR #680:
- Fix post_hooks code examples to use correct return-dict signature
- Expand test isolation section with fixture factory, DynaconfDict, and
reload patterns
- Document ignore_unknown_envvars silent failure mode in Negative
Consequences and add env var audit to Phase 1 checklist
- Fix NEXTCLOUD_HOST validator to be unconditional (required in all modes)
- Document environments=True edge cases (unset mode, ENV_FOR_DYNACONF
shadowing)
- Add upper bound to dynaconf version pin (>=3.2.13,<4.0)
- Tighten Pydantic Settings comparison to acknowledge 2.x TOML support
- Make .gitignore additions explicit in Phase 1 checklist
- Clarify that shell-level .env loading still works with load_dotenv=False
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fix incorrect hook syntax (@hookable.post → Dynaconf(post_hooks=[...])),
broken Qdrant mutual exclusivity validator, missing root_path for settings
file resolution, and empty string defaults that bypass validators. Add test
isolation section, mark Phase 4 as optional/future with risk note, and
correct Pydantic comparison (already a project dependency).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Propose migrating from manual os.getenv() calls to dynaconf for
file-based configuration. Key decisions: envvar_prefix=False for
backward compatibility, MCP_DEPLOYMENT_MODE as environment switcher,
TOML settings files with secret separation, and incremental migration
via adapter pattern.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Smithery is no longer a supported deployment mode. Remove all Smithery-specific
code paths, middleware, configuration, and tests. This simplifies the codebase
by eliminating DeploymentMode enum, SmitheryConfigMiddleware, session config
context variables, and the smithery_main entrypoint.
Files deleted: Dockerfile.smithery, smithery.yaml, smithery_main.py
ADR-016 retained with deprecated status for historical reference.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
MCP clients like Claude Code were unable to use tools because tokens
obtained directly from Nextcloud had the wrong audience claim. The MCP
server now acts as its own OAuth Authorization Server, proxying auth
to Nextcloud with its own client_id so tokens have the correct audience.
New endpoints: /.well-known/oauth-authorization-server, /oauth/token,
/oauth/register. Modified /oauth/authorize from pass-through to
intermediary pattern. PRM now points authorization_servers to the MCP
server instead of Nextcloud.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Changes based on review:
1. Add Nextcloud platform limitation section documenting OAuth/scope
support by endpoint type (WebDAV supports OAuth, others don't)
2. Update MCP elicitation to show capability negotiation and graceful
fallback - URL in error message when elicitation not supported
3. Simplify Smithery section - recommend self-hosted for privacy,
don't detail platform changes
4. Expand re-auth section with scope merging behavior, scenarios table,
and explicit design choice for tool-based re-auth over auto-elicitation
5. Make rate limiting configurable with environment variables and
admin guidance by deployment size
6. Clarify OAuth alternative - keep simple now, revisit if Nextcloud
adds scoped OAuth support
7. Expand verification steps with required tests, add recommended
Nextcloud configuration, add required README security notice
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Proposes consolidating five deployment modes into two:
- Single-User: App password in env vars (trusted environment)
- Multi-User: Login Flow v2 for per-user app password acquisition
Key changes:
- Use Nextcloud Login Flow v2 (NC 16+) for delegated authentication
- Application-level scope enforcement (app passwords have no native scopes)
- MCP elicitation for seamless authorization prompting
- Astrolabe front-end integration for scope management UI
- Clear security posture documentation for administrators
This removes the need for upstream Nextcloud OAuth patches and simplifies
deployment while maintaining security through defense-in-depth.
Related: #521
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add NEXTCLOUD_VERIFY_SSL and NEXTCLOUD_CA_BUNDLE env vars to configure
TLS certificate verification for all outbound Nextcloud connections.
Centralizes SSL config via a new HTTP client factory (http.py) used by
all 27 Nextcloud-bound call sites, including API clients, OIDC endpoints,
OAuth flows, and health checks.
Closes#560
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove astrolabe-specific docs and sections that belong in the
astrolabe repo. Update remaining references to point to the
astrolabe repo where appropriate.
- Fix .gitmodules SSH → HTTPS URL for astrolabe submodule
- Remove bump-version.yml stale "astrolabe" scope comment
- Delete blog-introducing-astrolabe.md (moved to astrolabe repo)
- Remove "Astrolabe Background Token Refresh" section from auth-flows.md
- Replace "Astrolabe User Setup" section in authentication.md with link
- Remove "Astrolabe Internal URL" section from configuration.md
- Remove "Webhook Presets (via Astrolabe UI)" from webhook guide
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Create unified documentation covering authentication flows across all five
deployment modes. Documents three communication patterns (MCP Client → MCP
Server → Nextcloud, background sync, Astrolabe → MCP Server) with ASCII
sequence diagrams and implementation references.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add unit tests for /api/v1/status endpoint focusing on OIDC config:
- Test hybrid mode (multi_user_basic + enable_offline_access) returns OIDC
- Test pure multi_user_basic mode without offline_access omits OIDC
- Test OAuth mode returns OIDC config
- Test single-user BasicAuth mode omits OIDC config
- Test partial OIDC config (only discovery_url or only issuer)
Also updates docs/authentication.md with Astrolabe hybrid mode setup:
- Two-step credential setup (OAuth + app password)
- Technical details for each credential type
- Request direction table explaining why two credentials needed
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Replace direct os.getenv() calls with get_settings().vector_sync_enabled
to ensure consistent behavior with both VECTOR_SYNC_ENABLED (deprecated)
and ENABLE_SEMANTIC_SEARCH environment variables.
Also add webhook management documentation guide.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Updated docs/running.md to use Docker container examples instead of
direct Python commands. This aligns with the CLI change to require
explicit 'run' subcommand while maintaining backward compatibility
for Docker users (ENTRYPOINT includes 'run').
Key changes:
- Quick Start: Use Docker commands instead of uv run
- Running Locally → Running with Docker: All examples use Docker
- Development Mode: Added CLI subcommands documentation (run/db)
- Database Migrations: Documented Alembic integration for developers
- Server Options: Docker port mapping instead of --host/--port flags
- Process Management: Simplified to Docker Compose only (removed systemd)
- Performance Tuning: Production Docker Compose with resource limits
- Troubleshooting: Docker logs and debug commands
Updated Dockerfile ENTRYPOINT:
- Changed from: ["/app/.venv/bin/nextcloud-mcp-server", "--host", "0.0.0.0"]
- Changed to: ["/app/.venv/bin/nextcloud-mcp-server", "run", "--host", "0.0.0.0"]
No breaking changes for Docker/Helm users - container interface unchanged.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Implements Alembic for managing token storage database schema versions.
Migrations run automatically on startup with full backward compatibility.
**Changes:**
- Add Alembic dependency (1.14.0+) and SQLAlchemy (auto-installed)
- Create migration infrastructure in alembic/ directory
- Add initial migration (001) capturing current schema
- Modify RefreshTokenStorage.initialize() to run migrations via anyio
- Add CLI commands: db upgrade, current, history, downgrade, migrate
- Add comprehensive migration documentation
**Backward Compatibility:**
- Pre-Alembic databases automatically stamped with revision 001
- No schema changes for existing databases
- Automatic upgrade on first startup after update
**Migration Strategy:**
Three scenarios handled:
1. New database → Run migrations from scratch
2. Pre-Alembic database → Stamp with 001 (no changes)
3. Alembic-managed → Upgrade to latest
**Architecture:**
- Uses anyio.to_thread.run_sync() for structured concurrency
- Alembic env.py runs with anyio.run() in worker thread
- SQLite-friendly migration patterns documented
- No ThreadPoolExecutor needed (anyio handles it)
**CLI Usage:**
```bash
nextcloud-mcp-server db upgrade # Upgrade to latest
nextcloud-mcp-server db current # Show version
nextcloud-mcp-server db history # View changelog
nextcloud-mcp-server db downgrade # Rollback (with confirmation)
nextcloud-mcp-server db migrate "description" # Create migration
```
**Testing:**
- All 13 webhook storage tests pass
- New/pre-Alembic database scenarios validated
- anyio integration tested
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
