provision_app_password validated the supplied app password by calling the
OCS cloud/user endpoint with BasicAuth as the *path user_id* (the UID).
Nextcloud keys app-password BasicAuth on the loginName, which differs from
the UID for OIDC-provisioned accounts whose UID is their display name
(UID "Chris Coutinho", loginName "chris@coutinho.io"). Authenticating as
the UID is rejected with HTTP 401 ("App password validation failed"), so
provisioning never completes.
Parse the request body up front and authenticate the OCS validation as the
body's `username` (the Nextcloud loginName), falling back to the path
user_id for legacy callers where UID == loginName. The OCS-returned account
id is still checked against the path user_id (the UID), and the password is
still stored keyed by UID with the loginName alongside.
Note this is not an encoding issue: BasicAuth places the user-id literally
in the header (RFC 7617, no URL-encoding); %20/+/literal-space forms of the
UID all fail — only the loginName authenticates.
Adds a regression test asserting the OCS BasicAuth uses the loginName while
storage is keyed by the UID, plus a backward-compat assertion that callers
without a loginName fall back to the UID.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
🟡 Performance: unified_search's _execute sorted but did not cap the merged
multi-doc_type pool, so N doc_types each fetched at search_limit sent
N*search_limit candidates into verify-on-read (one Nextcloud round-trip each).
Cap to search_limit*2 after the sort, matching vector_search, nc_semantic_search
and the viz_routes pattern — bounding verification cost to O(2*search_limit)
regardless of how many doc_types are requested.
🟡 Consistency: _get_deck_metadata_from_qdrant is the one internal Qdrant lookup
that uses a raw user_id filter instead of build_ownership_filter. This is not a
bug — deck cards are a documented cross-user gap (the Deck API is per-user, so
cross-user context can't be fetched with the caller's credentials) — but the
inconsistency was unexplained. Added a comment documenting the deliberate
self-only scope.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
get_user_client_basic_auth built a fresh RefreshTokenStorage and ran
storage.initialize() — a full Alembic `upgrade` in a worker thread — on every
call. Once the /api/v1 search endpoints (unified_search, vector_search) and the
chunk-context endpoint were wired to use it, concurrent requests ran concurrent
Alembic upgrades, which race on Alembic's non-thread-safe module-global
EnvironmentContext proxy and intermittently raise `KeyError: 'script'` →
HTTP 500 (seen on multi-user-basic/nc31; nc32 got lucky).
Cache one process-wide, already-initialized storage instance behind a lazily
created anyio.Lock so the one-time migration runs exactly once and never races.
Callers passing an explicit `storage` are unaffected. Also removes a redundant
per-request migration from the search hot path.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
🟡 Important: nc_semantic_search's include_context branch did not forward
accessible_owners to get_chunk_with_context, so context expansion for shared
files stayed self-only, found nothing in Qdrant, and silently fell back to the
plain excerpt. Forward accessible_owners (the per-file file_accessible_by_id
gate still enforces access).
🟡 Performance: auth/viz_routes.py's multi-doc_type branch sorted but did not
cap the candidate pool before verify-on-read, so N doc_types × limit*2 went
into verification (N× the Nextcloud round-trips). Cap to limit*2 after the
sort, matching server/semantic.py and the cross-app branch.
Also clear the SonarCloud gate (new_duplicated_lines_density 5.1% > 3%) the
ACL wiring introduced: extract the duplicated /api/v1 client-resolution +
owner-expansion + verify-on-read block from unified_search/vector_search into a
shared _search_with_acl helper, define a constant for the repeated
"Nextcloud host not configured" literal (S1192), and reword the access_filter
move_to_end comment so it isn't misread as commented-out code (S125) while
adding the other-owner count to its debug log (review nits).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1. Don't log unverified result titles: both search algorithms logged top-5
titles at DEBUG before verify-on-read; with owner-level share expansion the
unverified set can contain other users' docs. Algorithms now log a count
only; the verifying callers (server/semantic, viz_routes, api/visualization)
log verified titles after verify-on-read.
2. Cross-user FILE chunk context: get_chunk_with_context + the Qdrant chunk
helpers now take accessible_owners and use build_ownership_filter. For files
the expanded scope is honoured only after a per-file file_accessible_by_id
check (accessible_owners is owner-level, so the gate prevents a one-file
share recipient from reading any of the owner's cached chunks). note/deck/
news stay self-only (per-user APIs) — a documented gap. Both chunk endpoints
pass accessible_owners.
3. Algorithm usage: SemanticSearchAlgorithm is not dead (it backs the dense-only
option on the viz/API surfaces); added a clarifying comment in server/
semantic.py. Additionally wired accessible_owners + verify-on-read into the
/api/v1 search routes (unified_search, vector_search) so the astrolabe
surface is ACL-aware too — degrading gracefully to self-only/unverified for
non-provisioned callers instead of 401.
4. Overlapping conditions: build_ownership_filter no longer lists self in the
owner_id MatchAny branch (self is already covered by the user_id branch);
the owner_id branch carries only the OTHER owners.
Tests: build_ownership_filter dedup + chunk-bbox filter-shape updates; new
ACL-aware get_indexed_doc_types, cached-chunk lookup, and end-to-end cross-user
file chunk-context (recipient gets the chunk, non-recipient denied) tests.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
PR #89 (session-derived JWT auth) merged to astrolabe main and released as
v0.16.1; track the released tag instead of the feature-branch commit.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- get_indexed_doc_types: add optional accessible_owners param and reuse
build_ownership_filter so cross-user doc-type discovery matches the real
search scope (was self-only / ACL-blind); docstring documents the self-only
default. Covered by test_get_indexed_doc_types_is_acl_aware.
- access_filter: build_ownership_filter now omits the owner_id branch entirely
for an empty owner set instead of relying on undocumented MatchAny(any=[])
semantics; updated the empty-list unit test accordingly.
- access_filter: make the uid_owner/owner share-owner extraction explicit
("absent, not empty") to avoid skipping on a falsy-but-present field.
- access_filter: add an operator note that pre-owner_id points need a re-index
to surface to share recipients (ACL search is a no-op for legacy data).
- verification/webdav: lock the file_accessible_by_id(scope="") contract with a
targeted multi-user test (owner + recipient True, non-recipient False).
- viz_routes: comment that verify-on-read eviction runs inline by design (no
lifespan task group available on the Starlette route).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fixes the multi-user-basic regression introduced by the previous bump: the
astrolabe app's hard https-refusal blocked sending the app password to the
in-cluster http MCP endpoint (http://mcp-multi-user-basic:8000), so background
indexing never ran. Now warns instead of blocking; also clears the SonarCloud
S5332 hotspot on the dropped test's http literal.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- vector/qdrant_client.py: add owner_id to _PAYLOAD_INDEX_FIELDS (BLOCKING).
Every search applies MatchAny(key="owner_id", ...); without a keyword index
Qdrant full-scans the collection and may 400 on Qdrant Cloud strict mode.
_ensure_payload_indexes is idempotent so existing collections migrate at
startup.
- search/access_filter.py: bound the process-global _owners_cache with an LRU
cap (was one unbounded entry per active user, never evicted); document the
owner-level over-fetch limitation (a prolific sharer floods the recall
buffer with ghost candidates that verify-on-read drops, with no second
Qdrant pass) as a TODO toward per-file filtering.
- search/algorithms.py + semantic.py + bm25_hybrid.py: promote
accessible_owners from **kwargs to an explicit keyword-only parameter on the
SearchAlgorithm ABC and both implementations, so a misspelled keyword is a
type error rather than a silent fall back to self-only scope.
- search/verification.py: document that _verify_files now verifies by global
file id (WebDAV SEARCH), not by path.
- tests/unit/search/test_access_filter.py: add cache-hit, TTL-expiry,
failure-not-cached, and LRU-bound tests.
Bumps the astrolabe submodule with the matching #89 review fixes.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Tracks astrolabe feat/session-derived-jwt-auth tip (86616d9). Test-only
change (rename a unit-test constant to clear SonarCloud S2068); no functional
or runtime difference from the previously-verified submodule commit.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Bumps the astrolabe submodule to validate the one-click background-sync app
password internally (IProvider::getToken) instead of via an HTTP loopback to
overwrite.cli.url, which is unreachable from inside the app container and broke
the multi-user-basic integration legs (enable_background_sync never succeeded).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- viz_routes: run verify_search_results before returning results. After the
accessible_owners expansion the viz can surface OTHER users' shared docs, so
it must drop ones the caller can no longer access (revoked share) — same as
the nc_semantic_search tool path. (Blocking review item.)
- access_filter: cache list_accessible_owners per user for 30s to keep the OCS
shares round-trip off the search hot path (failures aren't cached); document
the single-page OCS limitation; add a clear_accessible_owners_cache() test
helper. Comment the empty-accessible_owners MatchAny([]) edge case.
- verification: comment why cross-user eviction is a deliberate no-op (eviction
is scoped to the querying user's id, so a recipient's revoked access never
deletes the owner's points; the recipient self-heals via accessible_owners).
- algorithms: declare SearchResult.original_score (set by the viz route) so the
now-precisely-typed result list type-checks.
- tests: cross-user eviction-no-op safety test; autouse owners-cache reset in
the access_filter + shared-search tests; replace async-no-await qdrant fakes
with AsyncMock (clears SonarCloud S7503).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Aligns PR #813's submodule pointer with astrolabe PR #89 tip (05176f9).
No MCP-server runtime change.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Aligns PR #813's submodule pointer with astrolabe PR #89 tip (091c68b),
which makes `composer run psalm` clean. No MCP-server runtime change.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The remaining SonarCloud S5443 (publicly-writable directory) findings were the
hard-coded /tmp screenshot paths in revoke_background_sync_access, which became
"new code" once the surrounding function was edited. Replace every /tmp literal
in the file with tempfile.gettempdir() (which S5443 accepts), eliminating the
findings consistently rather than per-line.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
SonarCloud Automatic Analysis does not honour # NOSONAR, so the S6418
(hard-coded token) and S5443 (publicly writable /tmp) findings in the new
tests persisted. Fix them by construction instead:
- test_login_flow: use a trivial poll-token value ("tok") in the rewrite test
(it asserts the URLs, not the token) so it no longer looks like a secret.
- test_astrolabe bg-sync: build the debug screenshot path from
tempfile.gettempdir() rather than a hard-coded /tmp literal.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The new-code quality gate flagged test-only mock fixtures as security issues
(new_security_rating E):
- S2068 "hard-coded password" ×2: drop the unused "app_password" value from the
get_app_password_with_scopes mocks (the code under test only reads truthiness
+ "scopes").
- S6418 "hard-coded token": NOSONAR on the Login Flow v2 poll-token test fixture.
- S5443 "publicly writable directory": NOSONAR on the /tmp debug screenshot path
(matches this file's existing convention).
No behaviour change; all are test fixtures, not real credentials.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
No runtime change — keeps the PR #813 submodule pointer aligned with the
astrolabe PR #89 tip (587caa6).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
SonarCloud flagged the http:// mock URLs in the new login_url-rewrite test as
clear-text-protocol hotspots, failing the new-code quality gate (they were new
+ unreviewed). They're harmless test fixtures; switch to https mock origins to
match this file's existing convention. The login_url rewrite is scheme-agnostic
so the test still exercises the same internal->public origin replacement.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The OAuth provisioning tools (check_provisioning_status, revoke_nextcloud_
access) only consulted the refresh-token store + Astrolabe status, ignoring the
app_passwords store that Login Flow v2 (nc_auth_provision_access) and the
management API write to — the same store require_provisioning / get_client use
to grant tool access. Result: status reported "not provisioned" while tools
worked, and revoke said "nothing to revoke" while the credential persisted.
- _get_provisioning_status: also check storage.get_app_password_with_scopes,
reporting is_provisioned with credential_type=app_password,
flow_type=login_flow_v2.
- _revoke_nextcloud_access: when the credential is an app password, delete it
from storage + invalidate the scope cache (no IdP token to revoke);
refresh-token revocation via the Token Broker is unchanged.
- tests/unit/test_oauth_tools_app_password_provisioning.py: cover status +
revoke for the app-password path.
- bump astrolabe submodule (deprovision MCP on disable); fix a stale assertion
in the migrated bg-sync test (one-click flow has no separate app-password
generation step).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Astrolabe was refactored to mint session-derived JWTs (TokenGenerationRequest
Event) and a one-click background-indexing opt-in, dropping the OAuth
authorize/callback/refresh surface. Bump the submodule and bring the test
suite in line:
- New test_astrolabe_session_jwt_search.py: a logged-in user searches via the
minted JWT with no provisioning (replaces the obsolete login_flow_provisioning
OAuth-authorize test; token_refresh test deleted — refresh flow is gone).
- settings_buttons: assert the new revoke endpoint + that oauth/disconnect is
gone (404).
- multi_user_background_sync / plotly / chunk_context: drop the OAuth authorize
step; provision via the one-click "Enable background indexing" button
(#mcp-enable-background-button -> #mcp-revoke-background-button) instead of
generating + pasting an app password.
- docker-compose.yml: mount the astrolabe submodule into the app container.
- third_party/astrolabe: bump to the one-click opt-in commit.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two fixes surfaced while testing Login Flow v2 provisioning behind a split
internal/external host (Docker: server↔Nextcloud over http://app, browser
over http://localhost:8080):
1. login_url pointed at the internal host. Nextcloud builds the login URL
from the request host, so the browser-facing URL came back as
http://app/login/v2/flow/... — unreachable from the user's browser. The
poll endpoint was already rewritten to the internal host (correct, the
server polls it); now LoginFlowV2Client also rewrites the login_url origin
to settings.nextcloud_public_issuer_url when set (passed at all 5
construction sites). When unset, behaviour is unchanged.
2. The app-password format guard rejected raw session tokens. core/
getapppassword returns a long alphanumeric token, not the dashed 25-char
Security-settings format, so the dashed-only regex 400'd the one-click
opt-in handoff. Relax APP_PASSWORD_PATTERN to `^[a-zA-Z0-9-]{20,256}$`;
the authoritative validation is still the BasicAuth check against Nextcloud.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The ACL-aware vector filter (PR #813) expands a user's search to documents
whose owner shared them, but verify-on-read still re-checked each file by
PATH under the *searching* user's WebDAV root. Nextcloud mounts received
shares at the recipient's root by basename, so a nested shared file (e.g.
owner's /docs/report.pdf) 404s for the recipient and was silently dropped —
defeating the filter for everything but root-level files.
Verify files by their global Nextcloud file id instead (the file doc_id IS
that id): WebDAVClient.get_file_info_by_id was insufficient (the dav/meta
endpoint only resolves the user's own storage, not shares), so add
WebDAVClient.file_accessible_by_id which runs a WebDAV SEARCH over the user's
whole tree (incl. mounted shares) filtered on oc:fileid. Empirically this
resolves owned, directly-shared, and folder-shared files; an empty result is
a definitive drop, transport errors are kept as transient.
- search/verification.py: _verify_files now checks file_accessible_by_id.
- client/webdav.py: add file_accessible_by_id (SEARCH by fileid).
- tests/integration/test_acl_owner_filter.py: filter matrix vs real Qdrant.
- tests/integration/test_acl_shared_search.py: real-Nextcloud share -> search.
- tests/integration/test_verify_on_read.py: nested shared file kept for the
recipient; unshared file dropped.
- tests/unit/search/test_verification.py: id-based verifier semantics.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The vector index has always been strictly per-user: every Qdrant payload
carries a `user_id` and the search filter is `user_id == querying_user`.
A file Alice indexed cannot be discovered by Bob even if she has shared
it with him — Bob would have to re-index it under his own user_id to
make it searchable, which means duplicate index entries for every share
recipient.
Switch to ownership-with-ACL-expansion:
- New `nextcloud_mcp_server.search.access_filter` module:
- `list_accessible_owners(sharing_client, user_id)` calls the OCS
Sharing API (`shared_with_me=true`) and returns
`{user_id} ∪ {uid_owner of each share}`. Fails open to `[user_id]`
so a misbehaving Sharing API doesn't black-hole search.
- `build_ownership_filter(user_id, accessible_owners)` returns a
Qdrant `Filter` whose `should` branch matches either the new
`owner_id IN accessible_owners` field or the legacy `user_id` field.
The legacy branch keeps points indexed before this change reachable
without a migration backfill.
- Indexer payload (`vector/processor.py`) now writes `owner_id` alongside
`user_id`. `DocumentTask` gains an optional `owner_id` field; today the
scanner always runs as the owner so the processor falls back to
`user_id`, but the field is plumbed so a future shared-with-me crawler
can set the true owner without reshaping the payload contract.
- `SemanticSearchAlgorithm.search` and `BM25HybridSearchAlgorithm.search`
accept `accessible_owners` via kwargs and use the new ownership filter.
Default behaviour with no kwarg is unchanged (self-only).
- Both user-facing callers — the MCP tool path (`server/semantic.py`) and
the visualization Starlette route (`auth/viz_routes.py`) — compute
`accessible_owners` from the authenticated Nextcloud client before
invoking the search algorithm. Eviction, scanner deletion, placeholder,
and chunk-context paths intentionally keep the legacy `user_id`
semantics (those are "operations on a specific user's records", not
cross-user reads).
- 10 new unit tests in `tests/unit/search/test_access_filter.py` cover
self-only default, owner expansion, dedup, fallback fields, OCS
failure, and the legacy `should`-branch shape.
Pairs with cbcoutinho/astrolabe#89 — together they let an Astrolabe user
find content owners have shared with them without going through any
re-authorization flow or re-indexing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The startup sweep was reading settings.qdrant_collection (the raw config
value, default "nextcloud_content") instead of settings.get_collection_name(),
which is what every other vector-sync operation uses. When QDRANT_COLLECTION
is not overridden, get_collection_name() auto-generates a
{deployment-id}-{model-name} name; the sweep was targeting a non-existent
collection and silently returning (0, 0).
Also adds the AsyncQdrantClient type annotation that was missing on
sweep_orphan_placeholders, and renames its parameter from collection_name
to collection to make it clear the value must be the resolved name.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When the per-tenant nextcloud-mcp-server Pod OOMKills mid-batch, the
in-memory anyio processor queue is lost but the placeholder Qdrant
points (is_placeholder=true, status=pending) survive. The next Pod's
scanner re-runs, sees the existing placeholders, applies the
5 × VECTOR_SYNC_SCAN_INTERVAL staleness gate (~5h with the deployed
1h scan interval), and skips them. Result: 0 documents indexed for
the duration of the gate after every restart.
Stamps a process-level instance_id (UUID per Pod-process) onto every
placeholder write. A new sweep_orphan_placeholders helper, called
once from starlette_lifespan after the Qdrant client is initialised
and before the scanner / user-manager spawns, scrolls the collection
and deletes any placeholder whose instance_id doesn't match the
current Pod's (including placeholders with no instance_id field —
back-compat for pre-fix Pod versions). The scanner's next cycle
naturally re-creates fresh placeholders and queues work normally;
no DocumentTask reconstruction needed.
Sweep is one-shot at startup, not periodic — the existing staleness
gate still covers same-Pod recovery, and the cross-Pod-restart gap
was the only failure mode. Failure is non-fatal (logged via
vector_sync.orphan_sweep_failed) so a transient Qdrant hiccup at
boot doesn't prevent the scanner from running.
Both lifespan branches (single-user BasicAuth, OAuth / multi-user
BasicAuth) call the sweep via a module-local helper. A new
VECTOR_SYNC_ORPHAN_SWEEP_ENABLED setting (default True) provides
an escape hatch.
Closes Deck #101.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
