Merge pull request #813 from cbcoutinho/feat/acl-aware-vector-index
feat(search): ACL-aware vector filter via Nextcloud Shares
This commit is contained in:
+1
-1
@@ -36,7 +36,7 @@ services:
|
|||||||
# Mount OIDC development directory outside /var/www/html to avoid rsync conflicts
|
# Mount OIDC development directory outside /var/www/html to avoid rsync conflicts
|
||||||
# The post-installation hook will register /opt/apps as an additional app directory
|
# The post-installation hook will register /opt/apps as an additional app directory
|
||||||
#- ./third_party:/opt/apps:ro
|
#- ./third_party:/opt/apps:ro
|
||||||
#- ./third_party/astrolabe:/opt/apps/astrolabe:ro
|
- ./third_party/astrolabe:/opt/apps/astrolabe:ro
|
||||||
#- ./third_party/oidc:/opt/apps/oidc:ro
|
#- ./third_party/oidc:/opt/apps/oidc:ro
|
||||||
environment:
|
environment:
|
||||||
- NEXTCLOUD_TRUSTED_DOMAINS=app
|
- NEXTCLOUD_TRUSTED_DOMAINS=app
|
||||||
|
|||||||
@@ -30,9 +30,11 @@ from ..http import nextcloud_httpx_client
|
|||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
# App password format regex (Nextcloud format: xxxxx-xxxxx-xxxxx-xxxxx-xxxxx)
|
# App password format regex (Nextcloud format: xxxxx-xxxxx-xxxxx-xxxxx-xxxxx)
|
||||||
APP_PASSWORD_PATTERN = re.compile(
|
# Shape guard only — the authoritative check is the BasicAuth validation
|
||||||
r"^[a-zA-Z0-9]{5}-[a-zA-Z0-9]{5}-[a-zA-Z0-9]{5}-[a-zA-Z0-9]{5}-[a-zA-Z0-9]{5}$"
|
# against Nextcloud below. Accepts both the dashed format a user copies from
|
||||||
)
|
# Security settings (xxxxx-xxxxx-xxxxx-xxxxx-xxxxx) and the raw token returned
|
||||||
|
# by the one-click ``core/getapppassword`` flow (a long alphanumeric string).
|
||||||
|
APP_PASSWORD_PATTERN = re.compile(r"^[a-zA-Z0-9-]{20,256}$")
|
||||||
|
|
||||||
# Timeout for Nextcloud API validation requests (seconds)
|
# Timeout for Nextcloud API validation requests (seconds)
|
||||||
NEXTCLOUD_VALIDATION_TIMEOUT = 10.0
|
NEXTCLOUD_VALIDATION_TIMEOUT = 10.0
|
||||||
|
|||||||
@@ -11,6 +11,7 @@ All endpoints require OAuth bearer token authentication via UnifiedTokenVerifier
|
|||||||
|
|
||||||
import base64
|
import base64
|
||||||
import logging
|
import logging
|
||||||
|
from collections.abc import Awaitable, Callable
|
||||||
from typing import Any
|
from typing import Any
|
||||||
|
|
||||||
import pymupdf
|
import pymupdf
|
||||||
@@ -30,10 +31,12 @@ from nextcloud_mcp_server.search import (
|
|||||||
BM25HybridSearchAlgorithm,
|
BM25HybridSearchAlgorithm,
|
||||||
SemanticSearchAlgorithm,
|
SemanticSearchAlgorithm,
|
||||||
)
|
)
|
||||||
|
from nextcloud_mcp_server.search.access_filter import list_accessible_owners
|
||||||
from nextcloud_mcp_server.search.context import (
|
from nextcloud_mcp_server.search.context import (
|
||||||
get_chunk_bbox_and_page_from_qdrant,
|
get_chunk_bbox_and_page_from_qdrant,
|
||||||
get_chunk_with_context,
|
get_chunk_with_context,
|
||||||
)
|
)
|
||||||
|
from nextcloud_mcp_server.search.verification import verify_search_results
|
||||||
from nextcloud_mcp_server.utils.validation import is_valid_nextcloud_doc_id
|
from nextcloud_mcp_server.utils.validation import is_valid_nextcloud_doc_id
|
||||||
from nextcloud_mcp_server.vector.oauth_sync import (
|
from nextcloud_mcp_server.vector.oauth_sync import (
|
||||||
NotProvisionedError,
|
NotProvisionedError,
|
||||||
@@ -43,6 +46,70 @@ from nextcloud_mcp_server.vector.visualization import compute_pca_coordinates
|
|||||||
|
|
||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
_NEXTCLOUD_HOST_NOT_CONFIGURED = "Nextcloud host not configured"
|
||||||
|
|
||||||
|
|
||||||
|
async def _search_with_acl(
|
||||||
|
request: Request,
|
||||||
|
user_id: str,
|
||||||
|
execute: Callable[[list[str] | None], Awaitable[list]],
|
||||||
|
) -> list:
|
||||||
|
"""Resolve the caller's Nextcloud client, run ``execute(accessible_owners)``,
|
||||||
|
and verify-on-read — shared by the /api/v1 search endpoints.
|
||||||
|
|
||||||
|
The OAuth bearer only authenticates Astrolabe → MCP Server; MCP Server →
|
||||||
|
Nextcloud uses the provisioned app password. When the caller never
|
||||||
|
provisioned background sync there is no client to expand shares or verify
|
||||||
|
with, so we fall back to self-only, unverified search (the pre-ACL
|
||||||
|
behaviour) rather than 401 — keeping search working for users who haven't
|
||||||
|
opted into background indexing.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
request: The Starlette request (carries ``app.state.oauth_context``).
|
||||||
|
user_id: The authenticated caller.
|
||||||
|
execute: Coroutine that runs the search for a given owner scope
|
||||||
|
(``None`` ⇒ self-only).
|
||||||
|
|
||||||
|
Returns:
|
||||||
|
The result list (verified for provisioned callers).
|
||||||
|
|
||||||
|
Raises:
|
||||||
|
ValueError: If the Nextcloud host is not configured.
|
||||||
|
"""
|
||||||
|
oauth_ctx = request.app.state.oauth_context
|
||||||
|
nextcloud_host = oauth_ctx.get("config", {}).get("nextcloud_host", "")
|
||||||
|
if not nextcloud_host:
|
||||||
|
raise ValueError(_NEXTCLOUD_HOST_NOT_CONFIGURED)
|
||||||
|
|
||||||
|
try:
|
||||||
|
nc_client = await get_user_client_basic_auth(user_id, nextcloud_host)
|
||||||
|
except NotProvisionedError:
|
||||||
|
logger.debug("User %s not provisioned; self-only unverified search", user_id)
|
||||||
|
results = await execute(None)
|
||||||
|
else:
|
||||||
|
async with nc_client:
|
||||||
|
# Expand to owners who shared content with the caller (same as the
|
||||||
|
# MCP tool path) so shared documents are searchable.
|
||||||
|
accessible_owners = await list_accessible_owners(nc_client.sharing, user_id)
|
||||||
|
results = await execute(accessible_owners)
|
||||||
|
# Verify-on-read (ADR-019): drop documents the caller can no longer
|
||||||
|
# access (e.g. a revoked share). Eviction runs inline — this
|
||||||
|
# Starlette route has no FastMCP lifespan task group.
|
||||||
|
results, _dropped = await verify_search_results(nc_client, results)
|
||||||
|
|
||||||
|
# Safe to log titles now: provisioned callers passed verify-on-read;
|
||||||
|
# non-provisioned ran self-only (unverified titles are never logged — see
|
||||||
|
# the search algorithms).
|
||||||
|
if results:
|
||||||
|
logger.debug(
|
||||||
|
"Top verified results: %s",
|
||||||
|
", ".join(
|
||||||
|
f"{r.doc_type}_{r.id} (score={r.score:.3f}, title='{r.title}')"
|
||||||
|
for r in results[:5]
|
||||||
|
),
|
||||||
|
)
|
||||||
|
return results
|
||||||
|
|
||||||
|
|
||||||
async def unified_search(request: Request) -> JSONResponse:
|
async def unified_search(request: Request) -> JSONResponse:
|
||||||
"""POST /api/v1/search - Search endpoint for Nextcloud Unified Search.
|
"""POST /api/v1/search - Search endpoint for Nextcloud Unified Search.
|
||||||
@@ -164,25 +231,41 @@ async def unified_search(request: Request) -> JSONResponse:
|
|||||||
# Request extra results to handle offset
|
# Request extra results to handle offset
|
||||||
search_limit = limit + offset
|
search_limit = limit + offset
|
||||||
|
|
||||||
# Execute search
|
async def _execute(owners: list[str] | None) -> list:
|
||||||
all_results = []
|
"""Run the search across requested doc_types with the given owner
|
||||||
if doc_types and isinstance(doc_types, list):
|
scope (None ⇒ self-only)."""
|
||||||
for doc_type in doc_types:
|
results: list = []
|
||||||
if doc_type:
|
if doc_types and isinstance(doc_types, list):
|
||||||
results = await search_algo.search(
|
for doc_type in doc_types:
|
||||||
query=query,
|
if doc_type:
|
||||||
user_id=user_id,
|
results.extend(
|
||||||
limit=search_limit,
|
await search_algo.search(
|
||||||
doc_type=doc_type,
|
query=query,
|
||||||
)
|
user_id=user_id,
|
||||||
all_results.extend(results)
|
limit=search_limit,
|
||||||
all_results.sort(key=lambda r: r.score, reverse=True)
|
doc_type=doc_type,
|
||||||
else:
|
accessible_owners=owners,
|
||||||
all_results = await search_algo.search(
|
)
|
||||||
query=query,
|
)
|
||||||
user_id=user_id,
|
# Sort, then cap to a fixed over-fetch budget before the result
|
||||||
limit=search_limit,
|
# reaches verify-on-read. Without this, N doc_types each fetched
|
||||||
)
|
# at search_limit would send N*search_limit candidates into
|
||||||
|
# verification — one Nextcloud round-trip each — scaling the cost
|
||||||
|
# with len(doc_types). 2x leaves headroom for verify-on-read
|
||||||
|
# drops before pagination, matching the nc_semantic_search and
|
||||||
|
# viz_routes pattern.
|
||||||
|
results.sort(key=lambda r: r.score, reverse=True)
|
||||||
|
results = results[: search_limit * 2]
|
||||||
|
else:
|
||||||
|
results = await search_algo.search(
|
||||||
|
query=query,
|
||||||
|
user_id=user_id,
|
||||||
|
limit=search_limit,
|
||||||
|
accessible_owners=owners,
|
||||||
|
)
|
||||||
|
return results
|
||||||
|
|
||||||
|
all_results = await _search_with_acl(request, user_id, _execute)
|
||||||
|
|
||||||
# Sort results by score (no deduplication - show all chunks)
|
# Sort results by score (no deduplication - show all chunks)
|
||||||
sorted_results = sorted(all_results, key=lambda r: r.score, reverse=True)
|
sorted_results = sorted(all_results, key=lambda r: r.score, reverse=True)
|
||||||
@@ -357,29 +440,37 @@ async def vector_search(request: Request) -> JSONResponse:
|
|||||||
score_threshold=score_threshold, fusion=fusion
|
score_threshold=score_threshold, fusion=fusion
|
||||||
)
|
)
|
||||||
|
|
||||||
# Execute search for each doc_type if specified, otherwise search all
|
async def _execute(owners: list[str] | None) -> list:
|
||||||
all_results = []
|
"""Run the search across requested doc_types with the given owner
|
||||||
if doc_types and isinstance(doc_types, list):
|
scope (None ⇒ self-only)."""
|
||||||
# Search each doc_type separately and merge results
|
results: list = []
|
||||||
for doc_type in doc_types:
|
if doc_types and isinstance(doc_types, list):
|
||||||
if doc_type: # Skip empty strings
|
# Search each doc_type separately and merge results
|
||||||
results = await search_algo.search(
|
for doc_type in doc_types:
|
||||||
query=query,
|
if doc_type: # Skip empty strings
|
||||||
user_id=user_id,
|
results.extend(
|
||||||
limit=limit,
|
await search_algo.search(
|
||||||
doc_type=doc_type,
|
query=query,
|
||||||
)
|
user_id=user_id,
|
||||||
all_results.extend(results)
|
limit=limit,
|
||||||
# Sort merged results by score and limit
|
doc_type=doc_type,
|
||||||
all_results.sort(key=lambda r: r.score, reverse=True)
|
accessible_owners=owners,
|
||||||
all_results = all_results[:limit]
|
)
|
||||||
else:
|
)
|
||||||
# Search all document types
|
# Sort merged results by score and limit
|
||||||
all_results = await search_algo.search(
|
results.sort(key=lambda r: r.score, reverse=True)
|
||||||
query=query,
|
results = results[:limit]
|
||||||
user_id=user_id,
|
else:
|
||||||
limit=limit,
|
# Search all document types
|
||||||
)
|
results = await search_algo.search(
|
||||||
|
query=query,
|
||||||
|
user_id=user_id,
|
||||||
|
limit=limit,
|
||||||
|
accessible_owners=owners,
|
||||||
|
)
|
||||||
|
return results
|
||||||
|
|
||||||
|
all_results = await _search_with_acl(request, user_id, _execute)
|
||||||
|
|
||||||
# Format results for PHP client
|
# Format results for PHP client
|
||||||
formatted_results = []
|
formatted_results = []
|
||||||
@@ -554,7 +645,7 @@ async def get_chunk_context(request: Request) -> JSONResponse:
|
|||||||
nextcloud_host = oauth_ctx.get("config", {}).get("nextcloud_host", "")
|
nextcloud_host = oauth_ctx.get("config", {}).get("nextcloud_host", "")
|
||||||
|
|
||||||
if not nextcloud_host:
|
if not nextcloud_host:
|
||||||
raise ValueError("Nextcloud host not configured")
|
raise ValueError(_NEXTCLOUD_HOST_NOT_CONFIGURED)
|
||||||
|
|
||||||
# Use the user's stored app password for Nextcloud calls.
|
# Use the user's stored app password for Nextcloud calls.
|
||||||
# The OAuth bearer is only used to authenticate Astrolabe → MCP Server;
|
# The OAuth bearer is only used to authenticate Astrolabe → MCP Server;
|
||||||
@@ -569,6 +660,10 @@ async def get_chunk_context(request: Request) -> JSONResponse:
|
|||||||
)
|
)
|
||||||
|
|
||||||
async with nc_client:
|
async with nc_client:
|
||||||
|
# Expand to owners who shared content with the caller so the cached
|
||||||
|
# chunk lookup can resolve cross-user SHARED FILES (gated per-file
|
||||||
|
# inside get_chunk_with_context). Same expansion as the search path.
|
||||||
|
accessible_owners = await list_accessible_owners(nc_client.sharing, user_id)
|
||||||
chunk_context = await get_chunk_with_context(
|
chunk_context = await get_chunk_with_context(
|
||||||
nc_client=nc_client,
|
nc_client=nc_client,
|
||||||
user_id=user_id,
|
user_id=user_id,
|
||||||
@@ -579,6 +674,7 @@ async def get_chunk_context(request: Request) -> JSONResponse:
|
|||||||
chunk_index=chunk_index,
|
chunk_index=chunk_index,
|
||||||
total_chunks=total_chunks,
|
total_chunks=total_chunks,
|
||||||
context_chars=context_chars,
|
context_chars=context_chars,
|
||||||
|
accessible_owners=accessible_owners,
|
||||||
)
|
)
|
||||||
|
|
||||||
if chunk_context is None:
|
if chunk_context is None:
|
||||||
@@ -598,12 +694,16 @@ async def get_chunk_context(request: Request) -> JSONResponse:
|
|||||||
page_number = chunk_context.page_number
|
page_number = chunk_context.page_number
|
||||||
|
|
||||||
if doc_type == "file":
|
if doc_type == "file":
|
||||||
|
# Reaching here means the file chunk context resolved, so access was
|
||||||
|
# already confirmed (get_chunk_with_context gates files by id);
|
||||||
|
# the bbox/page lookup uses the same owner scope for cross-user files.
|
||||||
qdrant_bbox, qdrant_page = await get_chunk_bbox_and_page_from_qdrant(
|
qdrant_bbox, qdrant_page = await get_chunk_bbox_and_page_from_qdrant(
|
||||||
user_id=user_id,
|
user_id=user_id,
|
||||||
doc_id=doc_id,
|
doc_id=doc_id,
|
||||||
chunk_index=chunk_index,
|
chunk_index=chunk_index,
|
||||||
chunk_start=start,
|
chunk_start=start,
|
||||||
chunk_end=end,
|
chunk_end=end,
|
||||||
|
accessible_owners=accessible_owners,
|
||||||
)
|
)
|
||||||
if qdrant_bbox is not None:
|
if qdrant_bbox is not None:
|
||||||
chunk_bbox = qdrant_bbox
|
chunk_bbox = qdrant_bbox
|
||||||
@@ -711,7 +811,7 @@ async def get_pdf_preview(request: Request) -> JSONResponse:
|
|||||||
nextcloud_host = oauth_ctx.get("config", {}).get("nextcloud_host", "")
|
nextcloud_host = oauth_ctx.get("config", {}).get("nextcloud_host", "")
|
||||||
|
|
||||||
if not nextcloud_host:
|
if not nextcloud_host:
|
||||||
raise ValueError("Nextcloud host not configured")
|
raise ValueError(_NEXTCLOUD_HOST_NOT_CONFIGURED)
|
||||||
|
|
||||||
# Use the user's stored app password for Nextcloud calls.
|
# Use the user's stored app password for Nextcloud calls.
|
||||||
# The OAuth bearer is only used to authenticate Astrolabe → MCP Server;
|
# The OAuth bearer is only used to authenticate Astrolabe → MCP Server;
|
||||||
|
|||||||
@@ -68,17 +68,27 @@ class LoginFlowV2Client:
|
|||||||
2. Poll for completion to receive the app password
|
2. Poll for completion to receive the app password
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
nextcloud_host: Base URL of the Nextcloud instance
|
nextcloud_host: Base URL of the Nextcloud instance, reachable by this
|
||||||
|
server (may be an internal/Docker hostname, e.g. http://app:80).
|
||||||
verify_ssl: SSL verification setting (True, False, or SSLContext)
|
verify_ssl: SSL verification setting (True, False, or SSLContext)
|
||||||
|
public_host: Externally-reachable Nextcloud base URL for the
|
||||||
|
browser-facing login URL (e.g. https://cloud.example.com). When the
|
||||||
|
server talks to Nextcloud over an internal hostname, Nextcloud
|
||||||
|
builds the login URL with that internal host — unusable in the
|
||||||
|
user's browser. If set, the login URL's origin is rewritten to this
|
||||||
|
public host. When None, the login URL is returned unchanged
|
||||||
|
(correct when nextcloud_host is already the public URL).
|
||||||
"""
|
"""
|
||||||
|
|
||||||
def __init__(
|
def __init__(
|
||||||
self,
|
self,
|
||||||
nextcloud_host: str,
|
nextcloud_host: str,
|
||||||
verify_ssl: bool | ssl.SSLContext = True,
|
verify_ssl: bool | ssl.SSLContext = True,
|
||||||
|
public_host: str | None = None,
|
||||||
):
|
):
|
||||||
self.nextcloud_host = nextcloud_host.rstrip("/")
|
self.nextcloud_host = nextcloud_host.rstrip("/")
|
||||||
self.verify_ssl = verify_ssl
|
self.verify_ssl = verify_ssl
|
||||||
|
self.public_host = public_host.rstrip("/") if public_host else None
|
||||||
|
|
||||||
async def initiate(
|
async def initiate(
|
||||||
self, user_agent: str = "nextcloud-mcp-server"
|
self, user_agent: str = "nextcloud-mcp-server"
|
||||||
@@ -119,8 +129,23 @@ class LoginFlowV2Client:
|
|||||||
# so server-side polling works across Docker networks.
|
# so server-side polling works across Docker networks.
|
||||||
poll_endpoint = self._rewrite_to_nextcloud_host(raw_poll_endpoint)
|
poll_endpoint = self._rewrite_to_nextcloud_host(raw_poll_endpoint)
|
||||||
|
|
||||||
|
# The login URL is opened in the *user's browser*, so it must use
|
||||||
|
# the externally-reachable host. Nextcloud builds it from the
|
||||||
|
# request host (our internal nextcloud_host), so rewrite it to the
|
||||||
|
# public host when one is configured (internal != external).
|
||||||
|
login_url = data["login"]
|
||||||
|
if self.public_host:
|
||||||
|
rewritten = rewrite_url_origin(login_url, self.public_host)
|
||||||
|
if rewritten != login_url:
|
||||||
|
logger.debug(
|
||||||
|
"Rewrote Login Flow v2 login_url to public host: %s → %s",
|
||||||
|
login_url,
|
||||||
|
rewritten,
|
||||||
|
)
|
||||||
|
login_url = rewritten
|
||||||
|
|
||||||
result = LoginFlowInitResponse(
|
result = LoginFlowInitResponse(
|
||||||
login_url=data["login"],
|
login_url=login_url,
|
||||||
poll_endpoint=poll_endpoint,
|
poll_endpoint=poll_endpoint,
|
||||||
poll_token=poll_data["token"],
|
poll_token=poll_data["token"],
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -74,6 +74,7 @@ async def _poll_and_store(provision_id: str) -> None:
|
|||||||
flow_client = LoginFlowV2Client(
|
flow_client = LoginFlowV2Client(
|
||||||
nextcloud_host=nextcloud_host,
|
nextcloud_host=nextcloud_host,
|
||||||
verify_ssl=get_nextcloud_ssl_verify(),
|
verify_ssl=get_nextcloud_ssl_verify(),
|
||||||
|
public_host=settings.nextcloud_public_issuer_url,
|
||||||
)
|
)
|
||||||
|
|
||||||
poll_endpoint = session["poll_endpoint"]
|
poll_endpoint = session["poll_endpoint"]
|
||||||
@@ -205,6 +206,7 @@ async def provision_page(
|
|||||||
flow_client = LoginFlowV2Client(
|
flow_client = LoginFlowV2Client(
|
||||||
nextcloud_host=nextcloud_host,
|
nextcloud_host=nextcloud_host,
|
||||||
verify_ssl=get_nextcloud_ssl_verify(),
|
verify_ssl=get_nextcloud_ssl_verify(),
|
||||||
|
public_host=settings.nextcloud_public_issuer_url,
|
||||||
)
|
)
|
||||||
init_response = await flow_client.initiate()
|
init_response = await flow_client.initiate()
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
|
|||||||
@@ -33,10 +33,12 @@ from nextcloud_mcp_server.search import (
|
|||||||
BM25HybridSearchAlgorithm,
|
BM25HybridSearchAlgorithm,
|
||||||
SemanticSearchAlgorithm,
|
SemanticSearchAlgorithm,
|
||||||
)
|
)
|
||||||
|
from nextcloud_mcp_server.search.access_filter import list_accessible_owners
|
||||||
from nextcloud_mcp_server.search.context import (
|
from nextcloud_mcp_server.search.context import (
|
||||||
get_chunk_bbox_and_page_from_qdrant,
|
get_chunk_bbox_and_page_from_qdrant,
|
||||||
get_chunk_with_context,
|
get_chunk_with_context,
|
||||||
)
|
)
|
||||||
|
from nextcloud_mcp_server.search.verification import verify_search_results
|
||||||
from nextcloud_mcp_server.utils.validation import is_valid_nextcloud_doc_id
|
from nextcloud_mcp_server.utils.validation import is_valid_nextcloud_doc_id
|
||||||
from nextcloud_mcp_server.vector.oauth_sync import (
|
from nextcloud_mcp_server.vector.oauth_sync import (
|
||||||
NotProvisionedError,
|
NotProvisionedError,
|
||||||
@@ -158,7 +160,7 @@ async def vector_visualization_search(request: Request) -> JSONResponse:
|
|||||||
with trace_operation("vector_viz.get_auth_client"):
|
with trace_operation("vector_viz.get_auth_client"):
|
||||||
auth_client_ctx = await _get_authenticated_client_for_userinfo(request)
|
auth_client_ctx = await _get_authenticated_client_for_userinfo(request)
|
||||||
|
|
||||||
async with auth_client_ctx as nc_client: # noqa: F841
|
async with auth_client_ctx as nc_client:
|
||||||
# Create search algorithm (no client needed - verification removed)
|
# Create search algorithm (no client needed - verification removed)
|
||||||
if algorithm == "semantic":
|
if algorithm == "semantic":
|
||||||
search_algo = SemanticSearchAlgorithm(score_threshold=score_threshold)
|
search_algo = SemanticSearchAlgorithm(score_threshold=score_threshold)
|
||||||
@@ -172,6 +174,13 @@ async def vector_visualization_search(request: Request) -> JSONResponse:
|
|||||||
status_code=400,
|
status_code=400,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# Expand the caller to every owner whose content they have
|
||||||
|
# read access to — same logic as the MCP tool path. See
|
||||||
|
# nextcloud_mcp_server.search.access_filter.
|
||||||
|
accessible_owners = await list_accessible_owners(
|
||||||
|
nc_client.sharing, username
|
||||||
|
)
|
||||||
|
|
||||||
# Execute search (supports cross-app when doc_types=None)
|
# Execute search (supports cross-app when doc_types=None)
|
||||||
# Get unverified results with buffer for filtering
|
# Get unverified results with buffer for filtering
|
||||||
search_start = time.perf_counter()
|
search_start = time.perf_counter()
|
||||||
@@ -192,6 +201,7 @@ async def vector_visualization_search(request: Request) -> JSONResponse:
|
|||||||
limit=limit * 2, # Buffer for verification filtering
|
limit=limit * 2, # Buffer for verification filtering
|
||||||
doc_type=None, # Search all types
|
doc_type=None, # Search all types
|
||||||
score_threshold=score_threshold,
|
score_threshold=score_threshold,
|
||||||
|
accessible_owners=accessible_owners,
|
||||||
)
|
)
|
||||||
all_results.extend(unverified_results)
|
all_results.extend(unverified_results)
|
||||||
else:
|
else:
|
||||||
@@ -211,15 +221,45 @@ async def vector_visualization_search(request: Request) -> JSONResponse:
|
|||||||
limit=limit * 2, # Buffer for verification filtering
|
limit=limit * 2, # Buffer for verification filtering
|
||||||
doc_type=doc_type,
|
doc_type=doc_type,
|
||||||
score_threshold=score_threshold,
|
score_threshold=score_threshold,
|
||||||
|
accessible_owners=accessible_owners,
|
||||||
)
|
)
|
||||||
all_results.extend(unverified_results)
|
all_results.extend(unverified_results)
|
||||||
# Sort by score before verification
|
# Sort by score, then cap to the same limit*2 over-fetch budget
|
||||||
|
# as the cross-app branch and the nc_semantic_search tool path
|
||||||
|
# (server/semantic.py). Without this, N doc_types each fetched
|
||||||
|
# at limit*2 would send N*limit*2 candidates into verify-on-read,
|
||||||
|
# multiplying the Nextcloud round-trip cost (and latency) by N.
|
||||||
all_results.sort(key=lambda r: r.score, reverse=True)
|
all_results.sort(key=lambda r: r.score, reverse=True)
|
||||||
|
all_results = all_results[: limit * 2]
|
||||||
|
|
||||||
# No verification needed for visualization - we only need Qdrant metadata
|
# Verify-on-read (ADR-019). Now that accessible_owners is expanded
|
||||||
# (title, excerpt, doc_type) which is already in search results.
|
# via OCS shares, the result set can include OTHER users' shared
|
||||||
# Verification is only needed for sampling (LLM needs full content).
|
# documents — so we must drop any the caller can no longer access
|
||||||
search_results = all_results[:limit]
|
# (e.g. a revoked share whose index entry hasn't reconciled yet),
|
||||||
|
# exactly as the nc_semantic_search tool path does. Skipping this
|
||||||
|
# would let the viz surface stale titles/excerpts from another
|
||||||
|
# user's index after a share is revoked.
|
||||||
|
# Eviction of dropped (e.g. revoked-share) points runs INLINE here
|
||||||
|
# by design: this is a Starlette route with no access to the
|
||||||
|
# FastMCP lifespan-owned ``eviction_task_group`` that the
|
||||||
|
# nc_semantic_search tool path passes for fire-and-forget eviction.
|
||||||
|
# The visualization is an interactive, low-QPS endpoint, so blocking
|
||||||
|
# briefly on the Qdrant delete is acceptable.
|
||||||
|
with trace_operation("vector_viz.verify_on_read"):
|
||||||
|
verified_results, _dropped = await verify_search_results(
|
||||||
|
nc_client, all_results
|
||||||
|
)
|
||||||
|
# Safe to log titles now: these passed verify-on-read (unverified
|
||||||
|
# titles are never logged — see the search algorithms).
|
||||||
|
if verified_results:
|
||||||
|
logger.debug(
|
||||||
|
"Top verified results: %s",
|
||||||
|
", ".join(
|
||||||
|
f"{r.doc_type}_{r.id} (score={r.score:.3f}, title='{r.title}')"
|
||||||
|
for r in verified_results[:5]
|
||||||
|
),
|
||||||
|
)
|
||||||
|
search_results = verified_results[:limit]
|
||||||
search_duration = time.perf_counter() - search_start
|
search_duration = time.perf_counter() - search_start
|
||||||
|
|
||||||
# Store original scores and normalize for visualization
|
# Store original scores and normalize for visualization
|
||||||
@@ -636,6 +676,10 @@ async def chunk_context_endpoint(request: Request) -> JSONResponse:
|
|||||||
)
|
)
|
||||||
|
|
||||||
async with nc_client:
|
async with nc_client:
|
||||||
|
# Expand to owners who shared content with the caller so the cached
|
||||||
|
# chunk lookup can resolve cross-user SHARED FILES (gated per-file
|
||||||
|
# inside get_chunk_with_context). Same expansion as the search path.
|
||||||
|
accessible_owners = await list_accessible_owners(nc_client.sharing, user_id)
|
||||||
chunk_context = await get_chunk_with_context(
|
chunk_context = await get_chunk_with_context(
|
||||||
nc_client=nc_client,
|
nc_client=nc_client,
|
||||||
user_id=user_id,
|
user_id=user_id,
|
||||||
@@ -646,6 +690,7 @@ async def chunk_context_endpoint(request: Request) -> JSONResponse:
|
|||||||
chunk_index=chunk_index,
|
chunk_index=chunk_index,
|
||||||
total_chunks=total_chunks,
|
total_chunks=total_chunks,
|
||||||
context_chars=context_chars,
|
context_chars=context_chars,
|
||||||
|
accessible_owners=accessible_owners,
|
||||||
)
|
)
|
||||||
|
|
||||||
# Check if context expansion succeeded
|
# Check if context expansion succeeded
|
||||||
@@ -674,12 +719,16 @@ async def chunk_context_endpoint(request: Request) -> JSONResponse:
|
|||||||
chunk_bbox = None
|
chunk_bbox = None
|
||||||
page_number = chunk_context.page_number
|
page_number = chunk_context.page_number
|
||||||
if doc_type == "file":
|
if doc_type == "file":
|
||||||
|
# Reaching here means the file chunk context resolved, so access was
|
||||||
|
# already confirmed (get_chunk_with_context gates files by id);
|
||||||
|
# the bbox/page lookup uses the same owner scope for cross-user files.
|
||||||
qdrant_bbox, qdrant_page = await get_chunk_bbox_and_page_from_qdrant(
|
qdrant_bbox, qdrant_page = await get_chunk_bbox_and_page_from_qdrant(
|
||||||
user_id=user_id,
|
user_id=user_id,
|
||||||
doc_id=doc_id,
|
doc_id=doc_id,
|
||||||
chunk_index=chunk_index,
|
chunk_index=chunk_index,
|
||||||
chunk_start=start,
|
chunk_start=start,
|
||||||
chunk_end=end,
|
chunk_end=end,
|
||||||
|
accessible_owners=accessible_owners,
|
||||||
)
|
)
|
||||||
if qdrant_bbox is not None:
|
if qdrant_bbox is not None:
|
||||||
chunk_bbox = qdrant_bbox
|
chunk_bbox = qdrant_bbox
|
||||||
|
|||||||
@@ -1073,6 +1073,49 @@ class WebDAVClient(BaseNextcloudClient):
|
|||||||
limit=limit,
|
limit=limit,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
async def file_accessible_by_id(self, file_id: int) -> bool:
|
||||||
|
"""ACL-aware access check for a file by its global Nextcloud file ID.
|
||||||
|
|
||||||
|
Used by verify-on-read (ADR-019). Searches the authenticated user's
|
||||||
|
whole files tree — which *includes mounted shares* — via WebDAV SEARCH
|
||||||
|
(RFC 5323) filtered on ``oc:fileid``, returning True iff the user can
|
||||||
|
currently access the file.
|
||||||
|
|
||||||
|
This is the only check that resolves shared files correctly:
|
||||||
|
|
||||||
|
- :meth:`get_file_info` resolves a path under the caller's *own* root,
|
||||||
|
so it 404s on a file shared into the caller's account (Nextcloud
|
||||||
|
mounts received shares at the recipient's root by basename, a
|
||||||
|
different path than the owner indexed).
|
||||||
|
- The ``/remote.php/dav/meta/{id}/`` endpoint resolves only the user's
|
||||||
|
*own* storage, so it 404s on shared files too.
|
||||||
|
|
||||||
|
SEARCH-by-fileid handles all cases: owned files, directly-shared files,
|
||||||
|
and files reachable via a shared parent folder (verified empirically).
|
||||||
|
|
||||||
|
Args:
|
||||||
|
file_id: Nextcloud internal (global) file ID.
|
||||||
|
|
||||||
|
Returns:
|
||||||
|
True if the user can access the file, False if it is not present
|
||||||
|
in their tree (not owned and not shared with them).
|
||||||
|
|
||||||
|
Raises:
|
||||||
|
HTTPStatusError: On transport/server errors — callers treat these
|
||||||
|
as transient (keep the result), not as a definitive denial.
|
||||||
|
"""
|
||||||
|
where = (
|
||||||
|
"<d:eq><d:prop><oc:fileid/></d:prop>"
|
||||||
|
f"<d:literal>{int(file_id)}</d:literal></d:eq>"
|
||||||
|
)
|
||||||
|
results = await self.search_files(
|
||||||
|
scope="", # user's whole files tree, incl. mounted shares
|
||||||
|
where_conditions=where,
|
||||||
|
properties=["fileid"],
|
||||||
|
limit=1,
|
||||||
|
)
|
||||||
|
return len(results) > 0
|
||||||
|
|
||||||
async def _get_file_info_by_id(self, file_id: int) -> Dict[str, Any]:
|
async def _get_file_info_by_id(self, file_id: int) -> Dict[str, Any]:
|
||||||
"""Get file information by Nextcloud file ID using WebDAV.
|
"""Get file information by Nextcloud file ID using WebDAV.
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,179 @@
|
|||||||
|
"""ACL-aware ownership filter for semantic / BM25 search.
|
||||||
|
|
||||||
|
The vector store payload carries an ``owner_id`` field — the UID of the user
|
||||||
|
who owns the underlying Nextcloud document. At query time, a user should
|
||||||
|
be able to find every document whose owner has shared it (directly or via
|
||||||
|
group / link) with them, without re-indexing.
|
||||||
|
|
||||||
|
This module turns "who can user X read?" into a Qdrant filter:
|
||||||
|
``owner_id IN accessible_owners`` where ``accessible_owners`` is
|
||||||
|
``{X} ∪ {owners of files / objects shared with X}``.
|
||||||
|
|
||||||
|
A second OR-branch matches the legacy ``user_id`` field so points indexed
|
||||||
|
before this change (which carry only ``user_id``) continue to be findable
|
||||||
|
by their original indexer. New points carry both fields.
|
||||||
|
|
||||||
|
Operator note (existing data): a Qdrant ``owner_id`` field condition matches
|
||||||
|
nothing on points that lack the field, so documents indexed *before* this
|
||||||
|
change never surface to share recipients — only to their original indexer via
|
||||||
|
the legacy ``user_id`` branch. ACL-aware search is therefore effectively a
|
||||||
|
no-op for pre-existing data until each owner's scanner re-indexes it. Trigger a
|
||||||
|
re-index after deploying this feature if it should apply to already-indexed
|
||||||
|
content immediately.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import logging
|
||||||
|
import time
|
||||||
|
from collections import OrderedDict
|
||||||
|
from typing import Any, Protocol
|
||||||
|
|
||||||
|
from qdrant_client.models import Condition, FieldCondition, Filter, MatchAny, MatchValue
|
||||||
|
|
||||||
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
# Short-lived per-user cache for the OCS shares lookup, which otherwise runs on
|
||||||
|
# every search/viz request. Trades up to this many seconds of share-visibility
|
||||||
|
# staleness (a freshly-granted share is searchable a little late) for avoiding
|
||||||
|
# an OCS round-trip per query. Safe: verify-on-read still gates each result
|
||||||
|
# against Nextcloud, so a revoked share is caught there regardless of this cache.
|
||||||
|
_OWNERS_CACHE_TTL_SECONDS = 30.0
|
||||||
|
# Cap the number of cached users so the process-global cache can't grow
|
||||||
|
# unboundedly in a long-running multi-user deployment (one entry per active
|
||||||
|
# user, never evicted otherwise). LRU eviction by insertion/access order via
|
||||||
|
# OrderedDict; the cap is generous relative to any realistic concurrent-user
|
||||||
|
# count, so steady state is effectively all-hit.
|
||||||
|
_OWNERS_CACHE_MAXSIZE = 1024
|
||||||
|
_owners_cache: OrderedDict[str, tuple[float, list[str]]] = OrderedDict()
|
||||||
|
|
||||||
|
|
||||||
|
def clear_accessible_owners_cache() -> None:
|
||||||
|
"""Drop all cached accessible-owners entries (used by tests)."""
|
||||||
|
_owners_cache.clear()
|
||||||
|
|
||||||
|
|
||||||
|
class _SharingClientProtocol(Protocol):
|
||||||
|
"""Subset of SharingClient that this module actually uses."""
|
||||||
|
|
||||||
|
async def list_shares(
|
||||||
|
self, path: str | None = None, shared_with_me: bool = False
|
||||||
|
) -> list[dict[str, Any]]: ...
|
||||||
|
|
||||||
|
|
||||||
|
async def list_accessible_owners(
|
||||||
|
sharing_client: _SharingClientProtocol,
|
||||||
|
user_id: str,
|
||||||
|
) -> list[str]:
|
||||||
|
"""Return every owner UID whose content `user_id` should be able to search.
|
||||||
|
|
||||||
|
The set is ``{user_id} ∪ {uid_owner of each share with shared_with_me=True}``.
|
||||||
|
Duplicates are removed; ordering is not significant (Qdrant ``MatchAny``
|
||||||
|
treats the list as a set).
|
||||||
|
|
||||||
|
Results are cached per user for ``_OWNERS_CACHE_TTL_SECONDS`` to keep the
|
||||||
|
OCS round-trip off the search hot path. Failures are not cached.
|
||||||
|
|
||||||
|
Note: ``list_shares(shared_with_me=True)`` returns whatever the OCS endpoint
|
||||||
|
yields in a single page (SharingClient does not paginate today). A user with
|
||||||
|
more incoming shares than the OCS page size could have some owners omitted;
|
||||||
|
if that becomes real, add pagination to SharingClient.
|
||||||
|
|
||||||
|
Granularity / over-fetch limitation (TODO, finer-grained filtering): this
|
||||||
|
expansion is *owner-level*, not *file-level*. If a prolific content creator
|
||||||
|
shares a single item with the querying user, that owner's whole indexed
|
||||||
|
corpus becomes a Qdrant candidate set for the querier even though only the
|
||||||
|
shared item is accessible. Verify-on-read correctly drops the inaccessible
|
||||||
|
"ghost" candidates, but because there is no second Qdrant pass to replenish,
|
||||||
|
a ``limit=N`` search can return fewer than N results when the over-fetch
|
||||||
|
buffer (2× in nc_semantic_search / viz_routes) is dominated by ghosts. A
|
||||||
|
per-file ownership index would remove this tension and is the natural
|
||||||
|
starting point for future work (intentionally out of scope here).
|
||||||
|
|
||||||
|
Sharing API failures are non-fatal — we degrade to ``[user_id]`` and log
|
||||||
|
so a hiccup in OCS doesn't black-hole the user's own search.
|
||||||
|
"""
|
||||||
|
now = time.monotonic()
|
||||||
|
cached = _owners_cache.get(user_id)
|
||||||
|
if cached is not None and now - cached[0] < _OWNERS_CACHE_TTL_SECONDS:
|
||||||
|
_owners_cache.move_to_end(user_id) # mark as recently used (LRU)
|
||||||
|
return list(cached[1]) # copy so callers can't mutate the cached value
|
||||||
|
|
||||||
|
owners: set[str] = {user_id}
|
||||||
|
try:
|
||||||
|
shares = await sharing_client.list_shares(shared_with_me=True)
|
||||||
|
except Exception as exc: # noqa: BLE001 — degrade gracefully
|
||||||
|
logger.warning(
|
||||||
|
"Sharing API unavailable; falling back to self-only owner filter "
|
||||||
|
"for user %s (%s)",
|
||||||
|
user_id,
|
||||||
|
exc,
|
||||||
|
)
|
||||||
|
return [user_id] # don't cache failures — retry on the next search
|
||||||
|
|
||||||
|
for share in shares:
|
||||||
|
# OCS returns the share owner under `uid_owner` (the file owner,
|
||||||
|
# not the share recipient). Some Nextcloud versions also surface
|
||||||
|
# `owner` as a fallback display field — we tolerate both. The intent is
|
||||||
|
# "absent, not empty": a missing/blank `uid_owner` falls through to
|
||||||
|
# `owner`, and a non-string or empty result skips the (malformed) share.
|
||||||
|
owner = share.get("uid_owner") or share.get("owner") or None
|
||||||
|
if not isinstance(owner, str) or not owner:
|
||||||
|
continue
|
||||||
|
owners.add(owner)
|
||||||
|
|
||||||
|
result = list(owners)
|
||||||
|
_owners_cache[user_id] = (now, result)
|
||||||
|
# Promote to the most-recently-used end. This is a no-op for a brand-new
|
||||||
|
# key (dict insertion already appends) but is needed when re-inserting an
|
||||||
|
# existing key after its TTL expired.
|
||||||
|
_owners_cache.move_to_end(user_id)
|
||||||
|
while len(_owners_cache) > _OWNERS_CACHE_MAXSIZE:
|
||||||
|
_owners_cache.popitem(last=False) # evict the least-recently-used entry
|
||||||
|
logger.debug(
|
||||||
|
"Accessible owners for user %s: %d entries (%d other owner(s))",
|
||||||
|
user_id,
|
||||||
|
len(result),
|
||||||
|
len(result) - 1,
|
||||||
|
)
|
||||||
|
return list(result)
|
||||||
|
|
||||||
|
|
||||||
|
def build_ownership_filter(
|
||||||
|
user_id: str, accessible_owners: list[str] | None = None
|
||||||
|
) -> Filter:
|
||||||
|
"""Build the Qdrant ``Filter`` constraining a search to readable points.
|
||||||
|
|
||||||
|
Matches points whose ``owner_id`` is in ``accessible_owners`` (excluding
|
||||||
|
self) OR whose ``user_id`` equals ``user_id``. The ``user_id`` branch covers
|
||||||
|
*all* of the caller's own content — both new points (where
|
||||||
|
``owner_id == user_id``) and legacy points indexed before ``owner_id``
|
||||||
|
existed — so self is intentionally NOT repeated in the ``owner_id`` branch.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
user_id: Querying user (matched by the ``user_id`` branch, which is the
|
||||||
|
self-only default when ``accessible_owners`` is None).
|
||||||
|
accessible_owners: Pre-computed list of owner UIDs the user has
|
||||||
|
access to. When None, defaults to ``[user_id]`` (no shares
|
||||||
|
expansion — used by callers that genuinely want self-only
|
||||||
|
scope such as eviction sweeps).
|
||||||
|
|
||||||
|
Returns:
|
||||||
|
A Qdrant ``Filter`` ready to be nested under a parent ``must`` clause.
|
||||||
|
"""
|
||||||
|
owners = accessible_owners if accessible_owners is not None else [user_id]
|
||||||
|
# The ``user_id`` branch is always present and already covers self-owned
|
||||||
|
# content (new + legacy). The ``owner_id`` branch is added only for OTHER
|
||||||
|
# owners (share senders) — listing self there too would overlap the
|
||||||
|
# ``user_id`` branch for no benefit. When there are no other owners the
|
||||||
|
# ``owner_id`` branch is omitted entirely, so we never depend on
|
||||||
|
# ``MatchAny(any=[])`` matching nothing (not a documented Qdrant guarantee).
|
||||||
|
other_owners = [owner for owner in owners if owner != user_id]
|
||||||
|
conditions: list[Condition] = [
|
||||||
|
FieldCondition(key="user_id", match=MatchValue(value=user_id)),
|
||||||
|
]
|
||||||
|
if other_owners:
|
||||||
|
conditions.insert(
|
||||||
|
0, FieldCondition(key="owner_id", match=MatchAny(any=other_owners))
|
||||||
|
)
|
||||||
|
return Filter(should=conditions)
|
||||||
@@ -5,9 +5,10 @@ from abc import ABC, abstractmethod
|
|||||||
from dataclasses import dataclass
|
from dataclasses import dataclass
|
||||||
from typing import Any, Protocol, runtime_checkable
|
from typing import Any, Protocol, runtime_checkable
|
||||||
|
|
||||||
from qdrant_client.models import FieldCondition, Filter, MatchValue, ScoredPoint
|
from qdrant_client.models import Filter, ScoredPoint
|
||||||
|
|
||||||
from nextcloud_mcp_server.config import get_settings
|
from nextcloud_mcp_server.config import get_settings
|
||||||
|
from nextcloud_mcp_server.search.access_filter import build_ownership_filter
|
||||||
from nextcloud_mcp_server.vector.placeholder import get_placeholder_filter
|
from nextcloud_mcp_server.vector.placeholder import get_placeholder_filter
|
||||||
from nextcloud_mcp_server.vector.qdrant_client import get_qdrant_client
|
from nextcloud_mcp_server.vector.qdrant_client import get_qdrant_client
|
||||||
|
|
||||||
@@ -75,14 +76,24 @@ class NextcloudClientProtocol(Protocol):
|
|||||||
...
|
...
|
||||||
|
|
||||||
|
|
||||||
async def get_indexed_doc_types(user_id: str) -> set[str]:
|
async def get_indexed_doc_types(
|
||||||
|
user_id: str, accessible_owners: list[str] | None = None
|
||||||
|
) -> set[str]:
|
||||||
"""Query Qdrant to get actually-indexed document types for a user.
|
"""Query Qdrant to get actually-indexed document types for a user.
|
||||||
|
|
||||||
This enables search algorithms to check which document types are available
|
This enables search algorithms to check which document types are available
|
||||||
before attempting to search/verify them, allowing graceful cross-app search.
|
before attempting to search/verify them, allowing graceful cross-app search.
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
user_id: User ID to filter by
|
user_id: User ID to filter by.
|
||||||
|
accessible_owners: Owner UIDs the user may read (self + share senders),
|
||||||
|
as computed by ``access_filter.list_accessible_owners``. When
|
||||||
|
provided, doc-type discovery is ACL-aware and matches the same
|
||||||
|
ownership scope as the actual search (so a share recipient discovers
|
||||||
|
cross-user doc_types). When ``None`` (the default), discovery is
|
||||||
|
**self-only** — a recipient won't see doc_types that exist only in
|
||||||
|
another owner's shared content. Pass the expanded set for cross-user
|
||||||
|
discovery.
|
||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
Set of document type strings (e.g., {"note", "file", "calendar"})
|
Set of document type strings (e.g., {"note", "file", "calendar"})
|
||||||
@@ -106,7 +117,9 @@ async def get_indexed_doc_types(user_id: str) -> set[str]:
|
|||||||
scroll_filter=Filter(
|
scroll_filter=Filter(
|
||||||
must=[
|
must=[
|
||||||
get_placeholder_filter(), # Exclude placeholders from doc_type discovery
|
get_placeholder_filter(), # Exclude placeholders from doc_type discovery
|
||||||
FieldCondition(key="user_id", match=MatchValue(value=user_id)),
|
# ACL-aware ownership scope (owner_id IN owners OR legacy
|
||||||
|
# user_id == user_id), matching the real search filter.
|
||||||
|
build_ownership_filter(user_id, accessible_owners),
|
||||||
]
|
]
|
||||||
),
|
),
|
||||||
limit=1000, # Sample size to discover types
|
limit=1000, # Sample size to discover types
|
||||||
@@ -168,6 +181,9 @@ class SearchResult:
|
|||||||
chunk_index: int = 0
|
chunk_index: int = 0
|
||||||
total_chunks: int = 1
|
total_chunks: int = 1
|
||||||
point_id: str | None = None
|
point_id: str | None = None
|
||||||
|
# Pre-normalization score, set by the visualization route before it rescales
|
||||||
|
# ``score`` to [0, 1] for visual encoding (see auth/viz_routes.py).
|
||||||
|
original_score: float | None = None
|
||||||
|
|
||||||
def __post_init__(self):
|
def __post_init__(self):
|
||||||
"""Validate score is non-negative.
|
"""Validate score is non-negative.
|
||||||
@@ -271,6 +287,8 @@ class SearchAlgorithm(ABC):
|
|||||||
user_id: str,
|
user_id: str,
|
||||||
limit: int = 10,
|
limit: int = 10,
|
||||||
doc_type: str | None = None,
|
doc_type: str | None = None,
|
||||||
|
*,
|
||||||
|
accessible_owners: list[str] | None = None,
|
||||||
**kwargs: Any,
|
**kwargs: Any,
|
||||||
) -> list[SearchResult]:
|
) -> list[SearchResult]:
|
||||||
"""Execute search with the given parameters.
|
"""Execute search with the given parameters.
|
||||||
@@ -280,6 +298,12 @@ class SearchAlgorithm(ABC):
|
|||||||
user_id: User ID for multi-tenant filtering
|
user_id: User ID for multi-tenant filtering
|
||||||
limit: Maximum number of results to return
|
limit: Maximum number of results to return
|
||||||
doc_type: Optional document type filter (note, file, calendar, etc.)
|
doc_type: Optional document type filter (note, file, calendar, etc.)
|
||||||
|
accessible_owners: Owner UIDs the user is allowed to read (self plus
|
||||||
|
the owners of content shared with them), pre-computed from the
|
||||||
|
OCS Sharing API by the caller. Declared explicitly — rather than
|
||||||
|
buried in ``**kwargs`` — so a misspelled keyword is a type error
|
||||||
|
instead of a silent fall back to self-only scope. ``None`` means
|
||||||
|
self-only (``[user_id]``).
|
||||||
**kwargs: Algorithm-specific parameters
|
**kwargs: Algorithm-specific parameters
|
||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
|
|||||||
@@ -10,6 +10,7 @@ from nextcloud_mcp_server.config import get_settings
|
|||||||
from nextcloud_mcp_server.embedding import get_bm25_service, get_embedding_service
|
from nextcloud_mcp_server.embedding import get_bm25_service, get_embedding_service
|
||||||
from nextcloud_mcp_server.observability.metrics import record_qdrant_operation
|
from nextcloud_mcp_server.observability.metrics import record_qdrant_operation
|
||||||
from nextcloud_mcp_server.observability.tracing import trace_operation
|
from nextcloud_mcp_server.observability.tracing import trace_operation
|
||||||
|
from nextcloud_mcp_server.search.access_filter import build_ownership_filter
|
||||||
from nextcloud_mcp_server.search.algorithms import (
|
from nextcloud_mcp_server.search.algorithms import (
|
||||||
SearchAlgorithm,
|
SearchAlgorithm,
|
||||||
SearchResult,
|
SearchResult,
|
||||||
@@ -70,6 +71,8 @@ class BM25HybridSearchAlgorithm(SearchAlgorithm):
|
|||||||
user_id: str,
|
user_id: str,
|
||||||
limit: int = 10,
|
limit: int = 10,
|
||||||
doc_type: str | None = None,
|
doc_type: str | None = None,
|
||||||
|
*,
|
||||||
|
accessible_owners: list[str] | None = None,
|
||||||
**kwargs: Any,
|
**kwargs: Any,
|
||||||
) -> list[SearchResult]:
|
) -> list[SearchResult]:
|
||||||
"""
|
"""
|
||||||
@@ -88,6 +91,9 @@ class BM25HybridSearchAlgorithm(SearchAlgorithm):
|
|||||||
user_id: User ID for filtering
|
user_id: User ID for filtering
|
||||||
limit: Maximum results to return
|
limit: Maximum results to return
|
||||||
doc_type: Optional document type filter
|
doc_type: Optional document type filter
|
||||||
|
accessible_owners: Owner UIDs the user can read (self + share
|
||||||
|
senders), pre-computed by the caller from the OCS Sharing API.
|
||||||
|
Defaults to ``[user_id]`` (self-only) when ``None``.
|
||||||
**kwargs: Additional parameters (score_threshold override)
|
**kwargs: Additional parameters (score_threshold override)
|
||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
@@ -131,10 +137,7 @@ class BM25HybridSearchAlgorithm(SearchAlgorithm):
|
|||||||
# Build Qdrant filter
|
# Build Qdrant filter
|
||||||
filter_conditions = [
|
filter_conditions = [
|
||||||
get_placeholder_filter(), # Always exclude placeholders from user-facing queries
|
get_placeholder_filter(), # Always exclude placeholders from user-facing queries
|
||||||
FieldCondition(
|
build_ownership_filter(user_id, accessible_owners),
|
||||||
key="user_id",
|
|
||||||
match=MatchValue(value=user_id),
|
|
||||||
),
|
|
||||||
]
|
]
|
||||||
|
|
||||||
# Add doc_type filter if specified
|
# Add doc_type filter if specified
|
||||||
@@ -238,12 +241,10 @@ class BM25HybridSearchAlgorithm(SearchAlgorithm):
|
|||||||
if len(results) >= limit:
|
if len(results) >= limit:
|
||||||
break
|
break
|
||||||
|
|
||||||
|
# Log the count only — NOT titles. These results are unverified: with
|
||||||
|
# owner-level share expansion the candidate set can include other users'
|
||||||
|
# documents that verify-on-read will drop, so titles must not be logged
|
||||||
|
# until after verification (the verifying callers log verified titles).
|
||||||
logger.info("Returning %s unverified results after deduplication", len(results))
|
logger.info("Returning %s unverified results after deduplication", len(results))
|
||||||
if results:
|
|
||||||
result_details = [
|
|
||||||
f"{r.doc_type}_{r.id} (score={r.score:.3f}, title='{r.title}')"
|
|
||||||
for r in results[:5] # Show top 5
|
|
||||||
]
|
|
||||||
logger.debug("Top results: %s", ", ".join(result_details))
|
|
||||||
|
|
||||||
return results
|
return results
|
||||||
|
|||||||
@@ -7,10 +7,12 @@ position markers for better visualization and understanding of search results.
|
|||||||
import logging
|
import logging
|
||||||
from dataclasses import dataclass
|
from dataclasses import dataclass
|
||||||
|
|
||||||
|
from httpx import HTTPStatusError
|
||||||
from qdrant_client.models import FieldCondition, Filter, MatchValue
|
from qdrant_client.models import FieldCondition, Filter, MatchValue
|
||||||
|
|
||||||
from nextcloud_mcp_server.client import NextcloudClient
|
from nextcloud_mcp_server.client import NextcloudClient
|
||||||
from nextcloud_mcp_server.config import get_settings
|
from nextcloud_mcp_server.config import get_settings
|
||||||
|
from nextcloud_mcp_server.search.access_filter import build_ownership_filter
|
||||||
from nextcloud_mcp_server.utils.validation import is_valid_nextcloud_doc_id
|
from nextcloud_mcp_server.utils.validation import is_valid_nextcloud_doc_id
|
||||||
from nextcloud_mcp_server.vector.html_processor import html_to_markdown
|
from nextcloud_mcp_server.vector.html_processor import html_to_markdown
|
||||||
from nextcloud_mcp_server.vector.placeholder import get_placeholder_filter
|
from nextcloud_mcp_server.vector.placeholder import get_placeholder_filter
|
||||||
@@ -20,7 +22,12 @@ logger = logging.getLogger(__name__)
|
|||||||
|
|
||||||
|
|
||||||
async def _get_chunk_from_qdrant(
|
async def _get_chunk_from_qdrant(
|
||||||
user_id: str, doc_id: str, doc_type: str, chunk_start: int, chunk_end: int
|
user_id: str,
|
||||||
|
doc_id: str,
|
||||||
|
doc_type: str,
|
||||||
|
chunk_start: int,
|
||||||
|
chunk_end: int,
|
||||||
|
accessible_owners: list[str] | None = None,
|
||||||
) -> str | None:
|
) -> str | None:
|
||||||
"""Retrieve full chunk text from Qdrant payload.
|
"""Retrieve full chunk text from Qdrant payload.
|
||||||
|
|
||||||
@@ -28,11 +35,15 @@ async def _get_chunk_from_qdrant(
|
|||||||
chunk content already stored in Qdrant.
|
chunk content already stored in Qdrant.
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
user_id: User ID who owns the document
|
user_id: Querying user.
|
||||||
doc_id: Document ID
|
doc_id: Document ID
|
||||||
doc_type: Document type (e.g., "note", "file")
|
doc_type: Document type (e.g., "note", "file")
|
||||||
chunk_start: Character offset where chunk starts
|
chunk_start: Character offset where chunk starts
|
||||||
chunk_end: Character offset where chunk ends
|
chunk_end: Character offset where chunk ends
|
||||||
|
accessible_owners: Owner UIDs the caller may read (self + share senders).
|
||||||
|
When None, the lookup is self-only. Callers must only pass an
|
||||||
|
expanded set after confirming the caller can access the document
|
||||||
|
(see ``get_chunk_with_context``) — the filter is owner-level.
|
||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
Full chunk text from Qdrant excerpt field, or None if not found
|
Full chunk text from Qdrant excerpt field, or None if not found
|
||||||
@@ -46,7 +57,7 @@ async def _get_chunk_from_qdrant(
|
|||||||
collection_name=settings.get_collection_name(),
|
collection_name=settings.get_collection_name(),
|
||||||
scroll_filter=Filter(
|
scroll_filter=Filter(
|
||||||
must=[
|
must=[
|
||||||
FieldCondition(key="user_id", match=MatchValue(value=user_id)),
|
build_ownership_filter(user_id, accessible_owners),
|
||||||
FieldCondition(key="doc_id", match=MatchValue(value=doc_id)),
|
FieldCondition(key="doc_id", match=MatchValue(value=doc_id)),
|
||||||
FieldCondition(key="doc_type", match=MatchValue(value=doc_type)),
|
FieldCondition(key="doc_type", match=MatchValue(value=doc_type)),
|
||||||
FieldCondition(
|
FieldCondition(
|
||||||
@@ -93,17 +104,24 @@ async def _get_chunk_from_qdrant(
|
|||||||
|
|
||||||
|
|
||||||
async def _get_chunk_by_index_from_qdrant(
|
async def _get_chunk_by_index_from_qdrant(
|
||||||
user_id: str, doc_id: str, doc_type: str, chunk_index: int
|
user_id: str,
|
||||||
|
doc_id: str,
|
||||||
|
doc_type: str,
|
||||||
|
chunk_index: int,
|
||||||
|
accessible_owners: list[str] | None = None,
|
||||||
) -> str | None:
|
) -> str | None:
|
||||||
"""Retrieve chunk text by chunk_index from Qdrant payload.
|
"""Retrieve chunk text by chunk_index from Qdrant payload.
|
||||||
|
|
||||||
Used to fetch adjacent chunks for context expansion.
|
Used to fetch adjacent chunks for context expansion.
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
user_id: User ID who owns the document
|
user_id: Querying user.
|
||||||
doc_id: Document ID
|
doc_id: Document ID
|
||||||
doc_type: Document type (e.g., "note", "file")
|
doc_type: Document type (e.g., "note", "file")
|
||||||
chunk_index: Zero-based chunk index in document
|
chunk_index: Zero-based chunk index in document
|
||||||
|
accessible_owners: Owner UIDs the caller may read; None ⇒ self-only.
|
||||||
|
Only pass an expanded set after a per-document access check (see
|
||||||
|
``get_chunk_with_context``).
|
||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
Full chunk text from Qdrant excerpt field, or None if not found
|
Full chunk text from Qdrant excerpt field, or None if not found
|
||||||
@@ -117,7 +135,7 @@ async def _get_chunk_by_index_from_qdrant(
|
|||||||
collection_name=settings.get_collection_name(),
|
collection_name=settings.get_collection_name(),
|
||||||
scroll_filter=Filter(
|
scroll_filter=Filter(
|
||||||
must=[
|
must=[
|
||||||
FieldCondition(key="user_id", match=MatchValue(value=user_id)),
|
build_ownership_filter(user_id, accessible_owners),
|
||||||
FieldCondition(key="doc_id", match=MatchValue(value=doc_id)),
|
FieldCondition(key="doc_id", match=MatchValue(value=doc_id)),
|
||||||
FieldCondition(key="doc_type", match=MatchValue(value=doc_type)),
|
FieldCondition(key="doc_type", match=MatchValue(value=doc_type)),
|
||||||
FieldCondition(
|
FieldCondition(
|
||||||
@@ -172,7 +190,13 @@ async def _get_deck_metadata_from_qdrant(
|
|||||||
qdrant_client = await get_qdrant_client()
|
qdrant_client = await get_qdrant_client()
|
||||||
settings = get_settings()
|
settings = get_settings()
|
||||||
|
|
||||||
# Query for any chunk of this card (we just need metadata)
|
# Query for any chunk of this card (we just need metadata).
|
||||||
|
# Intentionally self-only (raw user_id, not build_ownership_filter):
|
||||||
|
# deck cards are a documented cross-user gap — the Deck API is per-user,
|
||||||
|
# so cross-user deck context can't be fetched with the caller's
|
||||||
|
# credentials anyway (see the doc_type=="file"-only gate in
|
||||||
|
# get_chunk_with_context). Every other internal Qdrant lookup here is
|
||||||
|
# ACL-aware; this one is the deliberate exception.
|
||||||
scroll_result = await qdrant_client.scroll(
|
scroll_result = await qdrant_client.scroll(
|
||||||
collection_name=settings.get_collection_name(),
|
collection_name=settings.get_collection_name(),
|
||||||
scroll_filter=Filter(
|
scroll_filter=Filter(
|
||||||
@@ -217,6 +241,7 @@ async def get_chunk_bbox_and_page_from_qdrant(
|
|||||||
chunk_index: int | None,
|
chunk_index: int | None,
|
||||||
chunk_start: int,
|
chunk_start: int,
|
||||||
chunk_end: int,
|
chunk_end: int,
|
||||||
|
accessible_owners: list[str] | None = None,
|
||||||
) -> tuple[list | None, int | None]:
|
) -> tuple[list | None, int | None]:
|
||||||
"""Fetch chunk_bbox and page_number for a chunk from Qdrant payload.
|
"""Fetch chunk_bbox and page_number for a chunk from Qdrant payload.
|
||||||
|
|
||||||
@@ -256,7 +281,7 @@ async def get_chunk_bbox_and_page_from_qdrant(
|
|||||||
must=[
|
must=[
|
||||||
get_placeholder_filter(),
|
get_placeholder_filter(),
|
||||||
FieldCondition(key="doc_id", match=MatchValue(value=doc_id)),
|
FieldCondition(key="doc_id", match=MatchValue(value=doc_id)),
|
||||||
FieldCondition(key="user_id", match=MatchValue(value=user_id)),
|
build_ownership_filter(user_id, accessible_owners),
|
||||||
FieldCondition(
|
FieldCondition(
|
||||||
key="chunk_index", match=MatchValue(value=chunk_index)
|
key="chunk_index", match=MatchValue(value=chunk_index)
|
||||||
),
|
),
|
||||||
@@ -273,7 +298,7 @@ async def get_chunk_bbox_and_page_from_qdrant(
|
|||||||
must=[
|
must=[
|
||||||
get_placeholder_filter(),
|
get_placeholder_filter(),
|
||||||
FieldCondition(key="doc_id", match=MatchValue(value=doc_id)),
|
FieldCondition(key="doc_id", match=MatchValue(value=doc_id)),
|
||||||
FieldCondition(key="user_id", match=MatchValue(value=user_id)),
|
build_ownership_filter(user_id, accessible_owners),
|
||||||
FieldCondition(
|
FieldCondition(
|
||||||
key="chunk_start_offset",
|
key="chunk_start_offset",
|
||||||
match=MatchValue(value=chunk_start),
|
match=MatchValue(value=chunk_start),
|
||||||
@@ -352,6 +377,7 @@ async def get_chunk_with_context(
|
|||||||
chunk_index: int | None = None,
|
chunk_index: int | None = None,
|
||||||
total_chunks: int = 1,
|
total_chunks: int = 1,
|
||||||
context_chars: int = 300,
|
context_chars: int = 300,
|
||||||
|
accessible_owners: list[str] | None = None,
|
||||||
) -> ChunkContext | None:
|
) -> ChunkContext | None:
|
||||||
"""Fetch chunk with surrounding context.
|
"""Fetch chunk with surrounding context.
|
||||||
|
|
||||||
@@ -361,7 +387,7 @@ async def get_chunk_with_context(
|
|||||||
|
|
||||||
Args:
|
Args:
|
||||||
nc_client: Authenticated Nextcloud client
|
nc_client: Authenticated Nextcloud client
|
||||||
user_id: User ID who owns the document
|
user_id: Querying user.
|
||||||
doc_id: Document ID (str — keyword-indexed in Qdrant payload)
|
doc_id: Document ID (str — keyword-indexed in Qdrant payload)
|
||||||
doc_type: Type of document ("note", "file", etc.)
|
doc_type: Type of document ("note", "file", etc.)
|
||||||
chunk_start: Character offset where chunk starts
|
chunk_start: Character offset where chunk starts
|
||||||
@@ -372,6 +398,10 @@ async def get_chunk_with_context(
|
|||||||
field). When None, falls back to the (chunk_start, chunk_end) lookup.
|
field). When None, falls back to the (chunk_start, chunk_end) lookup.
|
||||||
total_chunks: Total number of chunks in document
|
total_chunks: Total number of chunks in document
|
||||||
context_chars: Number of characters to include before/after chunk
|
context_chars: Number of characters to include before/after chunk
|
||||||
|
accessible_owners: Owner UIDs the caller may read (self + share senders).
|
||||||
|
Used to support cross-user context for SHARED FILES only, and only
|
||||||
|
after a per-file access check (see ``lookup_owners`` below). For
|
||||||
|
non-file types the lookup stays self-only.
|
||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
ChunkContext with expanded context and markers, or None if document
|
ChunkContext with expanded context and markers, or None if document
|
||||||
@@ -380,13 +410,53 @@ async def get_chunk_with_context(
|
|||||||
# doc_id is keyword-indexed in Qdrant as str — pass through verbatim
|
# doc_id is keyword-indexed in Qdrant as str — pass through verbatim
|
||||||
# (no int coercion; producers always stringify on write).
|
# (no int coercion; producers always stringify on write).
|
||||||
|
|
||||||
|
# Determine the ownership scope for the Qdrant cached-chunk lookups.
|
||||||
|
#
|
||||||
|
# ``accessible_owners`` is OWNER-level (every owner who shared anything with
|
||||||
|
# the caller), so widening the lookup to it unconditionally would let a
|
||||||
|
# recipient of a single shared file read ANY of that owner's cached chunks
|
||||||
|
# by guessing doc_ids. We therefore honour it only for FILES, and only after
|
||||||
|
# confirming the caller can access THIS file by id (``file_accessible_by_id``
|
||||||
|
# is cross-user-safe: a WebDAV SEARCH over the caller's whole tree incl.
|
||||||
|
# mounted shares). For per-user types (note/deck/news) there is no
|
||||||
|
# share-mounted by-id access via the caller's credentials, so the lookup
|
||||||
|
# stays self-only — cross-user context for those types is a known gap.
|
||||||
|
lookup_owners: list[str] | None = None # None ⇒ self-only
|
||||||
|
if doc_type == "file" and accessible_owners:
|
||||||
|
try:
|
||||||
|
if await nc_client.webdav.file_accessible_by_id(int(doc_id)):
|
||||||
|
lookup_owners = accessible_owners
|
||||||
|
else:
|
||||||
|
# Not owned and not shared with the caller → no access. Return
|
||||||
|
# early rather than falling back to a self-only lookup that
|
||||||
|
# would also miss (and so the result is the same None, but this
|
||||||
|
# is explicit and skips a pointless Qdrant round-trip).
|
||||||
|
logger.debug(
|
||||||
|
"File %s not accessible to %s; no cross-user chunk context",
|
||||||
|
doc_id,
|
||||||
|
user_id,
|
||||||
|
)
|
||||||
|
return None
|
||||||
|
except (ValueError, TypeError):
|
||||||
|
# Non-numeric doc_id: shouldn't happen (endpoints validate), but
|
||||||
|
# degrade to self-only rather than raising.
|
||||||
|
logger.warning("Non-numeric file doc_id %r; using self-only scope", doc_id)
|
||||||
|
except HTTPStatusError as exc:
|
||||||
|
# Transient transport/server error — treat as inconclusive and fall
|
||||||
|
# back to self-only so the caller's own files still resolve.
|
||||||
|
logger.warning(
|
||||||
|
"file_accessible_by_id(%s) failed (%s); using self-only scope",
|
||||||
|
doc_id,
|
||||||
|
exc,
|
||||||
|
)
|
||||||
|
|
||||||
# Try to get chunk from Qdrant (fast path).
|
# Try to get chunk from Qdrant (fast path).
|
||||||
# Prefer chunk_index lookup (always-indexed field) when caller supplied it;
|
# Prefer chunk_index lookup (always-indexed field) when caller supplied it;
|
||||||
# fall back to (chunk_start, chunk_end) lookup otherwise.
|
# fall back to (chunk_start, chunk_end) lookup otherwise.
|
||||||
chunk_text: str | None = None
|
chunk_text: str | None = None
|
||||||
if chunk_index is not None:
|
if chunk_index is not None:
|
||||||
chunk_text = await _get_chunk_by_index_from_qdrant(
|
chunk_text = await _get_chunk_by_index_from_qdrant(
|
||||||
user_id, doc_id, doc_type, chunk_index
|
user_id, doc_id, doc_type, chunk_index, accessible_owners=lookup_owners
|
||||||
)
|
)
|
||||||
# When chunk_index is supplied, the indexed lookup is canonical: both the
|
# When chunk_index is supplied, the indexed lookup is canonical: both the
|
||||||
# index path and the offset path query the same Qdrant collection, so an
|
# index path and the offset path query the same Qdrant collection, so an
|
||||||
@@ -398,7 +468,12 @@ async def get_chunk_with_context(
|
|||||||
skip_offset_lookup = chunk_index is not None
|
skip_offset_lookup = chunk_index is not None
|
||||||
if chunk_text is None and not skip_offset_lookup:
|
if chunk_text is None and not skip_offset_lookup:
|
||||||
chunk_text = await _get_chunk_from_qdrant(
|
chunk_text = await _get_chunk_from_qdrant(
|
||||||
user_id, doc_id, doc_type, chunk_start, chunk_end
|
user_id,
|
||||||
|
doc_id,
|
||||||
|
doc_type,
|
||||||
|
chunk_start,
|
||||||
|
chunk_end,
|
||||||
|
accessible_owners=lookup_owners,
|
||||||
)
|
)
|
||||||
|
|
||||||
if chunk_text:
|
if chunk_text:
|
||||||
@@ -422,7 +497,11 @@ async def get_chunk_with_context(
|
|||||||
# Fetch previous chunk if not first chunk
|
# Fetch previous chunk if not first chunk
|
||||||
if chunk_index > 0:
|
if chunk_index > 0:
|
||||||
before_chunk = await _get_chunk_by_index_from_qdrant(
|
before_chunk = await _get_chunk_by_index_from_qdrant(
|
||||||
user_id, doc_id, doc_type, chunk_index - 1
|
user_id,
|
||||||
|
doc_id,
|
||||||
|
doc_type,
|
||||||
|
chunk_index - 1,
|
||||||
|
accessible_owners=lookup_owners,
|
||||||
)
|
)
|
||||||
if before_chunk:
|
if before_chunk:
|
||||||
# Remove overlap: the last chunk_overlap chars of previous chunk
|
# Remove overlap: the last chunk_overlap chars of previous chunk
|
||||||
@@ -443,7 +522,11 @@ async def get_chunk_with_context(
|
|||||||
# Fetch next chunk if not last chunk
|
# Fetch next chunk if not last chunk
|
||||||
if chunk_index < total_chunks - 1:
|
if chunk_index < total_chunks - 1:
|
||||||
after_chunk = await _get_chunk_by_index_from_qdrant(
|
after_chunk = await _get_chunk_by_index_from_qdrant(
|
||||||
user_id, doc_id, doc_type, chunk_index + 1
|
user_id,
|
||||||
|
doc_id,
|
||||||
|
doc_type,
|
||||||
|
chunk_index + 1,
|
||||||
|
accessible_owners=lookup_owners,
|
||||||
)
|
)
|
||||||
if after_chunk:
|
if after_chunk:
|
||||||
# Remove overlap: the first chunk_overlap chars of next chunk
|
# Remove overlap: the first chunk_overlap chars of next chunk
|
||||||
|
|||||||
@@ -8,6 +8,7 @@ from qdrant_client.models import FieldCondition, Filter, MatchValue
|
|||||||
from nextcloud_mcp_server.config import get_settings
|
from nextcloud_mcp_server.config import get_settings
|
||||||
from nextcloud_mcp_server.embedding import get_embedding_service
|
from nextcloud_mcp_server.embedding import get_embedding_service
|
||||||
from nextcloud_mcp_server.observability.metrics import record_qdrant_operation
|
from nextcloud_mcp_server.observability.metrics import record_qdrant_operation
|
||||||
|
from nextcloud_mcp_server.search.access_filter import build_ownership_filter
|
||||||
from nextcloud_mcp_server.search.algorithms import (
|
from nextcloud_mcp_server.search.algorithms import (
|
||||||
SearchAlgorithm,
|
SearchAlgorithm,
|
||||||
SearchResult,
|
SearchResult,
|
||||||
@@ -48,6 +49,8 @@ class SemanticSearchAlgorithm(SearchAlgorithm):
|
|||||||
user_id: str,
|
user_id: str,
|
||||||
limit: int = 10,
|
limit: int = 10,
|
||||||
doc_type: str | None = None,
|
doc_type: str | None = None,
|
||||||
|
*,
|
||||||
|
accessible_owners: list[str] | None = None,
|
||||||
**kwargs: Any,
|
**kwargs: Any,
|
||||||
) -> list[SearchResult]:
|
) -> list[SearchResult]:
|
||||||
"""Execute semantic search using vector similarity.
|
"""Execute semantic search using vector similarity.
|
||||||
@@ -65,7 +68,11 @@ class SemanticSearchAlgorithm(SearchAlgorithm):
|
|||||||
user_id: User ID for filtering
|
user_id: User ID for filtering
|
||||||
limit: Maximum results to return
|
limit: Maximum results to return
|
||||||
doc_type: Optional document type filter
|
doc_type: Optional document type filter
|
||||||
**kwargs: Additional parameters (score_threshold override)
|
accessible_owners: Owner UIDs the user can read (self + share
|
||||||
|
senders), pre-computed by the caller from the OCS Sharing API.
|
||||||
|
Defaults to ``[user_id]`` (self-only) when ``None``.
|
||||||
|
**kwargs:
|
||||||
|
- score_threshold (float): override the instance default
|
||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
List of unverified SearchResult objects ranked by similarity score
|
List of unverified SearchResult objects ranked by similarity score
|
||||||
@@ -97,10 +104,7 @@ class SemanticSearchAlgorithm(SearchAlgorithm):
|
|||||||
# Build Qdrant filter
|
# Build Qdrant filter
|
||||||
filter_conditions = [
|
filter_conditions = [
|
||||||
get_placeholder_filter(), # Always exclude placeholders from user-facing queries
|
get_placeholder_filter(), # Always exclude placeholders from user-facing queries
|
||||||
FieldCondition(
|
build_ownership_filter(user_id, accessible_owners),
|
||||||
key="user_id",
|
|
||||||
match=MatchValue(value=user_id),
|
|
||||||
),
|
|
||||||
]
|
]
|
||||||
|
|
||||||
# Add doc_type filter if specified
|
# Add doc_type filter if specified
|
||||||
@@ -159,12 +163,10 @@ class SemanticSearchAlgorithm(SearchAlgorithm):
|
|||||||
if len(results) >= limit:
|
if len(results) >= limit:
|
||||||
break
|
break
|
||||||
|
|
||||||
|
# Log the count only — NOT titles. These results are unverified: with
|
||||||
|
# owner-level share expansion the candidate set can include other users'
|
||||||
|
# documents that verify-on-read will drop, so titles must not be logged
|
||||||
|
# until after verification (the verifying callers log verified titles).
|
||||||
logger.info("Returning %s unverified results after deduplication", len(results))
|
logger.info("Returning %s unverified results after deduplication", len(results))
|
||||||
if results:
|
|
||||||
result_details = [
|
|
||||||
f"{r.doc_type}_{r.id} (score={r.score:.3f}, title='{r.title}')"
|
|
||||||
for r in results[:5] # Show top 5
|
|
||||||
]
|
|
||||||
logger.debug("Top results: %s", ", ".join(result_details))
|
|
||||||
|
|
||||||
return results
|
return results
|
||||||
|
|||||||
@@ -132,45 +132,55 @@ async def _verify_files(
|
|||||||
results: list[SearchResult],
|
results: list[SearchResult],
|
||||||
semaphore: anyio.Semaphore,
|
semaphore: anyio.Semaphore,
|
||||||
) -> set[str]:
|
) -> set[str]:
|
||||||
|
"""Return the doc_ids of file results this user may actually access.
|
||||||
|
|
||||||
|
Verifies each file by its *global* Nextcloud file id via an ACL-aware
|
||||||
|
WebDAV SEARCH (``webdav.file_accessible_by_id``), NOT by path. This is the
|
||||||
|
ACL-aware-search fix: a file an owner shared with the querying user mounts
|
||||||
|
at a different path under each tree, so the previous path-based check
|
||||||
|
(``get_file_info``) produced false 404s and dropped legitimate shared-file
|
||||||
|
hits. Definitive 403/404 → inaccessible (dropped + scheduled for eviction
|
||||||
|
by the caller); transient/ambiguous errors → kept (fail-open).
|
||||||
|
"""
|
||||||
# safe: cooperative concurrency, no lock needed (see verify_search_results)
|
# safe: cooperative concurrency, no lock needed (see verify_search_results)
|
||||||
accessible: set[str] = set()
|
accessible: set[str] = set()
|
||||||
|
|
||||||
async def check(result: SearchResult) -> None:
|
async def check(result: SearchResult) -> None:
|
||||||
doc_id = result.id
|
doc_id = result.id
|
||||||
# file_path is propagated from the Qdrant payload by the algorithm
|
# file_path is propagated from the Qdrant payload by the algorithm
|
||||||
# layer (bm25_hybrid.py / semantic.py). No extra Qdrant round-trip.
|
# layer (bm25_hybrid.py / semantic.py); kept here only for log context.
|
||||||
file_path = (result.metadata or {}).get("path")
|
file_path = (result.metadata or {}).get("path")
|
||||||
if not file_path:
|
|
||||||
# Cannot verify without a path; treat as accessible to avoid
|
# Verify by *global* file ID via an ACL-aware WebDAV SEARCH, NOT by
|
||||||
# silently dropping legitimate results when payload is missing
|
# path. For files the vector ``doc_id`` IS the Nextcloud file ID, and
|
||||||
# (legacy data, or a future doc_type that doesn't propagate path).
|
# file_accessible_by_id searches the user's whole tree (incl. mounted
|
||||||
|
# shares), so a file an owner shared with this user verifies as
|
||||||
|
# accessible even though it lives at a different path under the owner's
|
||||||
|
# root. A path-based check (the old behaviour) would 404 on shared
|
||||||
|
# files mounted at the recipient's root by basename and silently drop
|
||||||
|
# legitimate ACL-aware-search results.
|
||||||
|
#
|
||||||
|
# Hoisted cast mirrors _verify_notes: a malformed id keeps the result
|
||||||
|
# (fail open) with a specific log line rather than a generic
|
||||||
|
# "unexpected error" from the catch-all below.
|
||||||
|
try:
|
||||||
|
file_id_int = int(doc_id)
|
||||||
|
except (TypeError, ValueError) as e:
|
||||||
logger.warning(
|
logger.warning(
|
||||||
"No file path in metadata for file_id %s; keeping result "
|
"Non-numeric file id %r (%s): %s; keeping result",
|
||||||
"(verification skipped)",
|
|
||||||
doc_id,
|
doc_id,
|
||||||
|
file_path,
|
||||||
|
e,
|
||||||
)
|
)
|
||||||
accessible.add(doc_id)
|
accessible.add(doc_id)
|
||||||
return
|
return
|
||||||
|
|
||||||
async with semaphore:
|
async with semaphore:
|
||||||
try:
|
try:
|
||||||
info = await client.webdav.get_file_info(file_path)
|
if await client.webdav.file_accessible_by_id(file_id_int):
|
||||||
if info is None:
|
|
||||||
# Contract (see WebDAVClient.get_file_info docstring):
|
|
||||||
# `None` means a malformed PROPFIND response — an
|
|
||||||
# ambiguous state, not a definitive 404. Treat as
|
|
||||||
# transient and KEEP the result rather than evicting.
|
|
||||||
# Real 404s raise HTTPStatusError and land in the
|
|
||||||
# _is_definitive_404_or_403 branch below.
|
|
||||||
logger.warning(
|
|
||||||
"Malformed PROPFIND response verifying file %s (%s); "
|
|
||||||
"keeping result (ambiguous state, not a definitive 404)",
|
|
||||||
doc_id,
|
|
||||||
file_path,
|
|
||||||
)
|
|
||||||
accessible.add(doc_id)
|
accessible.add(doc_id)
|
||||||
return
|
# else: definitively inaccessible (not owned, not shared) —
|
||||||
accessible.add(doc_id)
|
# drop and let the caller schedule eviction.
|
||||||
except HTTPStatusError as e:
|
except HTTPStatusError as e:
|
||||||
if _is_definitive_404_or_403(e):
|
if _is_definitive_404_or_403(e):
|
||||||
return
|
return
|
||||||
@@ -183,6 +193,8 @@ async def _verify_files(
|
|||||||
)
|
)
|
||||||
accessible.add(doc_id)
|
accessible.add(doc_id)
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
|
# Network blip / unexpected WebDAV error — ambiguous, not a
|
||||||
|
# definitive denial. Keep the result; the next query re-verifies.
|
||||||
logger.warning(
|
logger.warning(
|
||||||
"Unexpected error verifying file %s (%s): %s; keeping result",
|
"Unexpected error verifying file %s (%s): %s; keeping result",
|
||||||
doc_id,
|
doc_id,
|
||||||
@@ -584,6 +596,15 @@ async def verify_search_results(
|
|||||||
if evict_on_missing and inaccessible:
|
if evict_on_missing and inaccessible:
|
||||||
|
|
||||||
async def evict(doc_id: str, doc_type: str) -> None:
|
async def evict(doc_id: str, doc_type: str) -> None:
|
||||||
|
# Eviction is scoped to the QUERYING user's own points
|
||||||
|
# (user_id == the searcher). For a cross-user shared document
|
||||||
|
# (owner_id=alice surfaced to bob via accessible_owners), bob
|
||||||
|
# failing verification evicts with user_id=bob — a deliberate
|
||||||
|
# no-op, because alice's points carry user_id=alice and must NOT
|
||||||
|
# be deleted just because bob's share was revoked. Bob's view
|
||||||
|
# self-heals via list_accessible_owners (alice drops out of his
|
||||||
|
# accessible owners once OCS no longer reports the share). See the
|
||||||
|
# legacy-user_id semantics note in build_ownership_filter.
|
||||||
try:
|
try:
|
||||||
await delete_document_points(doc_id, doc_type, user_id)
|
await delete_document_points(doc_id, doc_type, user_id)
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
|
|||||||
@@ -113,6 +113,7 @@ def register_auth_tools(mcp: FastMCP) -> None:
|
|||||||
flow_client = LoginFlowV2Client(
|
flow_client = LoginFlowV2Client(
|
||||||
nextcloud_host=nextcloud_host,
|
nextcloud_host=nextcloud_host,
|
||||||
verify_ssl=get_nextcloud_ssl_verify(),
|
verify_ssl=get_nextcloud_ssl_verify(),
|
||||||
|
public_host=settings.nextcloud_public_issuer_url,
|
||||||
)
|
)
|
||||||
init_response = await flow_client.initiate()
|
init_response = await flow_client.initiate()
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
@@ -258,6 +259,7 @@ def register_auth_tools(mcp: FastMCP) -> None:
|
|||||||
flow_client = LoginFlowV2Client(
|
flow_client = LoginFlowV2Client(
|
||||||
nextcloud_host=nextcloud_host,
|
nextcloud_host=nextcloud_host,
|
||||||
verify_ssl=get_nextcloud_ssl_verify(),
|
verify_ssl=get_nextcloud_ssl_verify(),
|
||||||
|
public_host=settings.nextcloud_public_issuer_url,
|
||||||
)
|
)
|
||||||
poll_result = await flow_client.poll(
|
poll_result = await flow_client.poll(
|
||||||
poll_endpoint=session["poll_endpoint"],
|
poll_endpoint=session["poll_endpoint"],
|
||||||
@@ -431,6 +433,7 @@ def register_auth_tools(mcp: FastMCP) -> None:
|
|||||||
flow_client = LoginFlowV2Client(
|
flow_client = LoginFlowV2Client(
|
||||||
nextcloud_host=nextcloud_host,
|
nextcloud_host=nextcloud_host,
|
||||||
verify_ssl=get_nextcloud_ssl_verify(),
|
verify_ssl=get_nextcloud_ssl_verify(),
|
||||||
|
public_host=settings.nextcloud_public_issuer_url,
|
||||||
)
|
)
|
||||||
init_response = await flow_client.initiate()
|
init_response = await flow_client.initiate()
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
|
|||||||
@@ -17,6 +17,7 @@ from pydantic import BaseModel, Field
|
|||||||
|
|
||||||
from nextcloud_mcp_server.auth import require_scopes
|
from nextcloud_mcp_server.auth import require_scopes
|
||||||
from nextcloud_mcp_server.auth.astrolabe_client import AstrolabeClient
|
from nextcloud_mcp_server.auth.astrolabe_client import AstrolabeClient
|
||||||
|
from nextcloud_mcp_server.auth.scope_authorization import invalidate_scope_cache
|
||||||
from nextcloud_mcp_server.auth.storage import get_shared_storage
|
from nextcloud_mcp_server.auth.storage import get_shared_storage
|
||||||
from nextcloud_mcp_server.auth.token_broker import TokenBrokerService
|
from nextcloud_mcp_server.auth.token_broker import TokenBrokerService
|
||||||
|
|
||||||
@@ -132,6 +133,26 @@ async def _get_provisioning_status(ctx: Context, user_id: str) -> ProvisioningSt
|
|||||||
)
|
)
|
||||||
storage = await get_shared_storage()
|
storage = await get_shared_storage()
|
||||||
|
|
||||||
|
# Login Flow v2 app password stored directly in this server's storage —
|
||||||
|
# written by nc_auth_provision_access and the management app-password API,
|
||||||
|
# and the credential that require_provisioning / get_client actually use.
|
||||||
|
# Checked here so check_provisioning_status and revoke_nextcloud_access stay
|
||||||
|
# consistent with what actually grants tool access (the dual-store drift in
|
||||||
|
# the original code reported "not provisioned" while tools still worked).
|
||||||
|
app_pw = await storage.get_app_password_with_scopes(user_id)
|
||||||
|
if app_pw:
|
||||||
|
logger.debug(
|
||||||
|
" get_provisioning_status: app password (login-flow store) FOUND "
|
||||||
|
"for user_id=%s",
|
||||||
|
user_id,
|
||||||
|
)
|
||||||
|
return ProvisioningStatus(
|
||||||
|
is_provisioned=True,
|
||||||
|
credential_type="app_password",
|
||||||
|
scopes=app_pw.get("scopes"),
|
||||||
|
flow_type="login_flow_v2",
|
||||||
|
)
|
||||||
|
|
||||||
token_data = await storage.get_refresh_token(user_id)
|
token_data = await storage.get_refresh_token(user_id)
|
||||||
|
|
||||||
if not token_data:
|
if not token_data:
|
||||||
@@ -297,9 +318,30 @@ async def _revoke_nextcloud_access(ctx: Context, user_id: str) -> RevocationResu
|
|||||||
message="No Nextcloud access to revoke.",
|
message="No Nextcloud access to revoke.",
|
||||||
)
|
)
|
||||||
|
|
||||||
# Initialize Token Broker to handle revocation
|
|
||||||
storage = await get_shared_storage()
|
storage = await get_shared_storage()
|
||||||
|
|
||||||
|
# App-password credential (Login Flow v2 / management API): there is no
|
||||||
|
# IdP token to revoke — removing it from this server's storage drops the
|
||||||
|
# server's access. Without this, revoke previously only handled refresh
|
||||||
|
# tokens and left the app password in place (tools kept working).
|
||||||
|
if status.credential_type == "app_password":
|
||||||
|
deleted = await storage.delete_app_password(user_id)
|
||||||
|
invalidate_scope_cache(user_id)
|
||||||
|
if deleted:
|
||||||
|
return RevocationResult(
|
||||||
|
success=True,
|
||||||
|
message=(
|
||||||
|
"Successfully revoked Nextcloud access (app password "
|
||||||
|
"removed). You can run provisioning again if needed."
|
||||||
|
),
|
||||||
|
)
|
||||||
|
return RevocationResult(
|
||||||
|
success=True,
|
||||||
|
message="No Nextcloud access to revoke.",
|
||||||
|
)
|
||||||
|
|
||||||
|
# Refresh-token credential: revoke via the Token Broker (IdP revocation).
|
||||||
|
|
||||||
# Get OAuth client credentials from storage
|
# Get OAuth client credentials from storage
|
||||||
client_creds = await storage.get_oauth_client()
|
client_creds = await storage.get_oauth_client()
|
||||||
if not client_creds:
|
if not client_creds:
|
||||||
|
|||||||
@@ -30,6 +30,7 @@ from nextcloud_mcp_server.models.semantic import (
|
|||||||
from nextcloud_mcp_server.observability.metrics import (
|
from nextcloud_mcp_server.observability.metrics import (
|
||||||
instrument_tool,
|
instrument_tool,
|
||||||
)
|
)
|
||||||
|
from nextcloud_mcp_server.search.access_filter import list_accessible_owners
|
||||||
from nextcloud_mcp_server.search.bm25_hybrid import BM25HybridSearchAlgorithm
|
from nextcloud_mcp_server.search.bm25_hybrid import BM25HybridSearchAlgorithm
|
||||||
from nextcloud_mcp_server.search.context import get_chunk_with_context
|
from nextcloud_mcp_server.search.context import get_chunk_with_context
|
||||||
from nextcloud_mcp_server.search.verification import verify_search_results
|
from nextcloud_mcp_server.search.verification import verify_search_results
|
||||||
@@ -121,8 +122,19 @@ def configure_semantic_tools(mcp: FastMCP):
|
|||||||
)
|
)
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# Expand the caller's identity to every owner whose content they
|
||||||
|
# have read access to via Nextcloud shares. Lets a user find files
|
||||||
|
# owners have shared with them without having to re-index those
|
||||||
|
# files under their own user_id.
|
||||||
|
accessible_owners = await list_accessible_owners(client.sharing, username)
|
||||||
|
|
||||||
try:
|
try:
|
||||||
# Create BM25 hybrid search algorithm with specified fusion
|
# The nc_semantic_search tool deliberately uses BM25-hybrid (dense +
|
||||||
|
# sparse with RRF/DBSF fusion) as the single tool-layer algorithm.
|
||||||
|
# SemanticSearchAlgorithm is not dead code — it backs the dense-only
|
||||||
|
# option that the visualization/API surfaces expose explicitly
|
||||||
|
# (auth/viz_routes.py and api/visualization.py). Both algorithms take
|
||||||
|
# accessible_owners, so ACL-aware search works on every surface.
|
||||||
search_algo = BM25HybridSearchAlgorithm(
|
search_algo = BM25HybridSearchAlgorithm(
|
||||||
score_threshold=score_threshold, fusion=fusion
|
score_threshold=score_threshold, fusion=fusion
|
||||||
)
|
)
|
||||||
@@ -153,6 +165,7 @@ def configure_semantic_tools(mcp: FastMCP):
|
|||||||
limit=limit * 2,
|
limit=limit * 2,
|
||||||
doc_type=None, # Signal to search all types
|
doc_type=None, # Signal to search all types
|
||||||
score_threshold=score_threshold,
|
score_threshold=score_threshold,
|
||||||
|
accessible_owners=accessible_owners,
|
||||||
)
|
)
|
||||||
all_results.extend(unverified_results)
|
all_results.extend(unverified_results)
|
||||||
else:
|
else:
|
||||||
@@ -177,6 +190,7 @@ def configure_semantic_tools(mcp: FastMCP):
|
|||||||
limit=limit * 2,
|
limit=limit * 2,
|
||||||
doc_type=dtype,
|
doc_type=dtype,
|
||||||
score_threshold=score_threshold,
|
score_threshold=score_threshold,
|
||||||
|
accessible_owners=accessible_owners,
|
||||||
)
|
)
|
||||||
all_results.extend(unverified_results)
|
all_results.extend(unverified_results)
|
||||||
|
|
||||||
@@ -221,6 +235,17 @@ def configure_semantic_tools(mcp: FastMCP):
|
|||||||
verified_chunk_count,
|
verified_chunk_count,
|
||||||
dropped_count,
|
dropped_count,
|
||||||
)
|
)
|
||||||
|
# Safe to log titles now: these results passed verify-on-read, so the
|
||||||
|
# caller is confirmed to have access (unverified titles were never
|
||||||
|
# logged — see the search algorithms).
|
||||||
|
if verified_results:
|
||||||
|
logger.debug(
|
||||||
|
"Top verified results: %s",
|
||||||
|
", ".join(
|
||||||
|
f"{r.doc_type}_{r.id} (score={r.score:.3f}, title='{r.title}')"
|
||||||
|
for r in verified_results[:5]
|
||||||
|
),
|
||||||
|
)
|
||||||
search_results = verified_results[:limit]
|
search_results = verified_results[:limit]
|
||||||
|
|
||||||
# Convert SearchResult objects to SemanticSearchResult for response.
|
# Convert SearchResult objects to SemanticSearchResult for response.
|
||||||
@@ -314,6 +339,12 @@ def configure_semantic_tools(mcp: FastMCP):
|
|||||||
chunk_index=result.chunk_index,
|
chunk_index=result.chunk_index,
|
||||||
total_chunks=result.total_chunks,
|
total_chunks=result.total_chunks,
|
||||||
context_chars=context_chars,
|
context_chars=context_chars,
|
||||||
|
# Forward the share-expanded owner set so context
|
||||||
|
# expansion works for shared files (the per-file
|
||||||
|
# file_accessible_by_id gate inside still enforces
|
||||||
|
# access). Without this the lookup stays self-only
|
||||||
|
# and silently falls back to the plain excerpt.
|
||||||
|
accessible_owners=accessible_owners,
|
||||||
)
|
)
|
||||||
|
|
||||||
if chunk_context:
|
if chunk_context:
|
||||||
|
|||||||
@@ -45,6 +45,38 @@ class NotProvisionedError(Exception):
|
|||||||
pass
|
pass
|
||||||
|
|
||||||
|
|
||||||
|
# Process-wide app-password storage for the BasicAuth client path.
|
||||||
|
#
|
||||||
|
# get_user_client_basic_auth is on the search hot path (Unified Search and the
|
||||||
|
# /api/v1 viz endpoints call it per request). Creating a fresh
|
||||||
|
# RefreshTokenStorage and running ``initialize()`` — a full Alembic upgrade in
|
||||||
|
# a worker thread — on every call is both wasteful and unsafe: concurrent
|
||||||
|
# upgrades race on Alembic's non-thread-safe module-global EnvironmentContext
|
||||||
|
# proxy, surfacing as ``KeyError: 'script'``. Cache one initialized instance,
|
||||||
|
# guarded by a lock so the one-time migration runs exactly once. The lock is
|
||||||
|
# created lazily inside an async context (anyio primitives must not be built at
|
||||||
|
# import time — trio compatibility), mirroring vector/qdrant_client.py.
|
||||||
|
_basic_auth_storage: "RefreshTokenStorage | None" = None
|
||||||
|
_basic_auth_storage_lock: anyio.Lock | None = None
|
||||||
|
|
||||||
|
|
||||||
|
async def _get_initialized_basic_auth_storage() -> "RefreshTokenStorage":
|
||||||
|
"""Return the process-wide, already-initialized app-password storage."""
|
||||||
|
global _basic_auth_storage, _basic_auth_storage_lock
|
||||||
|
if _basic_auth_storage is not None:
|
||||||
|
return _basic_auth_storage
|
||||||
|
# Safe under cooperative scheduling: no await between the None-check and the
|
||||||
|
# assignment, so two coroutines cannot both create a lock.
|
||||||
|
if _basic_auth_storage_lock is None:
|
||||||
|
_basic_auth_storage_lock = anyio.Lock()
|
||||||
|
async with _basic_auth_storage_lock:
|
||||||
|
if _basic_auth_storage is None:
|
||||||
|
storage = RefreshTokenStorage.from_env()
|
||||||
|
await storage.initialize()
|
||||||
|
_basic_auth_storage = storage
|
||||||
|
return _basic_auth_storage
|
||||||
|
|
||||||
|
|
||||||
@dataclass
|
@dataclass
|
||||||
class UserSyncState:
|
class UserSyncState:
|
||||||
"""State for a single user's scanner task."""
|
"""State for a single user's scanner task."""
|
||||||
@@ -76,10 +108,11 @@ async def get_user_client_basic_auth(
|
|||||||
Raises:
|
Raises:
|
||||||
NotProvisionedError: If user has not provisioned an app password
|
NotProvisionedError: If user has not provisioned an app password
|
||||||
"""
|
"""
|
||||||
# Get or create storage instance
|
# Get or create storage instance. Reuse a process-wide initialized instance
|
||||||
|
# rather than building one (and running an Alembic upgrade) per call — see
|
||||||
|
# _get_initialized_basic_auth_storage for why (hot path + Alembic race).
|
||||||
if storage is None:
|
if storage is None:
|
||||||
storage = RefreshTokenStorage.from_env()
|
storage = await _get_initialized_basic_auth_storage()
|
||||||
await storage.initialize()
|
|
||||||
|
|
||||||
# Retrieve app password from local storage
|
# Retrieve app password from local storage
|
||||||
app_password = await storage.get_app_password(user_id)
|
app_password = await storage.get_app_password(user_id)
|
||||||
|
|||||||
@@ -706,6 +706,16 @@ async def _index_document(
|
|||||||
},
|
},
|
||||||
payload={
|
payload={
|
||||||
"user_id": doc_task.user_id,
|
"user_id": doc_task.user_id,
|
||||||
|
# owner_id is the UID of the file's owner — what
|
||||||
|
# search-time ACL expansion filters on. Today the scanner
|
||||||
|
# always runs as the file's owner (per-user crawl, only
|
||||||
|
# surfaces files the user owns or that fall under their
|
||||||
|
# WebDAV root), so owner_id == user_id is correct for
|
||||||
|
# every doc type indexed here. The fields are kept
|
||||||
|
# separate so a future indexer change that lets a user
|
||||||
|
# crawl shared-with-them content can set owner_id to the
|
||||||
|
# true owner without losing the "who indexed this" trail.
|
||||||
|
"owner_id": doc_task.owner_id or doc_task.user_id,
|
||||||
"doc_id": doc_task.doc_id,
|
"doc_id": doc_task.doc_id,
|
||||||
"doc_type": doc_task.doc_type,
|
"doc_type": doc_task.doc_type,
|
||||||
"is_placeholder": False, # Real indexed document (not placeholder)
|
"is_placeholder": False, # Real indexed document (not placeholder)
|
||||||
|
|||||||
@@ -38,6 +38,15 @@ logger = logging.getLogger(__name__)
|
|||||||
_PAYLOAD_INDEX_FIELDS: dict[str, PayloadSchemaType] = {
|
_PAYLOAD_INDEX_FIELDS: dict[str, PayloadSchemaType] = {
|
||||||
"doc_id": PayloadSchemaType.KEYWORD,
|
"doc_id": PayloadSchemaType.KEYWORD,
|
||||||
"user_id": PayloadSchemaType.KEYWORD,
|
"user_id": PayloadSchemaType.KEYWORD,
|
||||||
|
# owner_id is the ACL-aware filter field: every search applies
|
||||||
|
# MatchAny(key="owner_id", any=accessible_owners) (see
|
||||||
|
# search/access_filter.py). Without a keyword index Qdrant full-scans the
|
||||||
|
# collection to evaluate it — invisible at small scale, but a latency
|
||||||
|
# regression at tens of thousands of points and an HTTP 400 on Qdrant
|
||||||
|
# Cloud strict payload-validation mode. Mirrors the user_id treatment;
|
||||||
|
# _ensure_payload_indexes is idempotent so existing collections migrate
|
||||||
|
# at startup without operator intervention.
|
||||||
|
"owner_id": PayloadSchemaType.KEYWORD,
|
||||||
"doc_type": PayloadSchemaType.KEYWORD,
|
"doc_type": PayloadSchemaType.KEYWORD,
|
||||||
"is_placeholder": PayloadSchemaType.BOOL,
|
"is_placeholder": PayloadSchemaType.BOOL,
|
||||||
"chunk_index": PayloadSchemaType.INTEGER,
|
"chunk_index": PayloadSchemaType.INTEGER,
|
||||||
|
|||||||
@@ -108,6 +108,12 @@ class DocumentTask:
|
|||||||
metadata: dict[str, int | str] | None = (
|
metadata: dict[str, int | str] | None = (
|
||||||
None # Additional metadata (e.g., board_id/stack_id for deck_card)
|
None # Additional metadata (e.g., board_id/stack_id for deck_card)
|
||||||
)
|
)
|
||||||
|
# UID of the true owner of the indexed object, used by the search-time
|
||||||
|
# ACL filter. None today (scanner always runs as the owner, so the
|
||||||
|
# processor falls back to user_id), but settable so a future
|
||||||
|
# shared-with-me crawl can pass through the actual owner without
|
||||||
|
# reshaping the payload contract.
|
||||||
|
owner_id: str | None = None
|
||||||
|
|
||||||
|
|
||||||
# Track documents potentially deleted (grace period before actual deletion)
|
# Track documents potentially deleted (grace period before actual deletion)
|
||||||
|
|||||||
@@ -0,0 +1,212 @@
|
|||||||
|
"""ACL-aware ownership filter — deterministic, in-memory Qdrant.
|
||||||
|
|
||||||
|
Proves the query-time ownership expansion added for ACL-aware search
|
||||||
|
(``search/access_filter.build_ownership_filter`` →
|
||||||
|
``SemanticSearchAlgorithm``): a user finds documents whose owner shared them
|
||||||
|
(``owner_id`` ∈ accessible_owners), does not find documents owned by users who
|
||||||
|
have not shared with them, and legacy points carrying only ``user_id`` stay
|
||||||
|
findable by their original indexer.
|
||||||
|
|
||||||
|
This complements ``tests/unit/search/test_access_filter.py`` (filter
|
||||||
|
construction in isolation) by exercising the filter against a real Qdrant
|
||||||
|
engine through the actual search algorithm — no Nextcloud, no verification
|
||||||
|
layer, no background sync, so it is fast and deterministic. The full
|
||||||
|
real-Nextcloud flow (share + verify-on-read) lives in
|
||||||
|
``test_acl_shared_search.py``.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from unittest.mock import AsyncMock
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
from qdrant_client import AsyncQdrantClient
|
||||||
|
from qdrant_client.models import Distance, PointStruct, VectorParams
|
||||||
|
|
||||||
|
from nextcloud_mcp_server.config import get_settings
|
||||||
|
from nextcloud_mcp_server.embedding import SimpleEmbeddingProvider
|
||||||
|
from nextcloud_mcp_server.search.algorithms import get_indexed_doc_types
|
||||||
|
from nextcloud_mcp_server.search.context import _get_chunk_by_index_from_qdrant
|
||||||
|
from nextcloud_mcp_server.search.semantic import SemanticSearchAlgorithm
|
||||||
|
|
||||||
|
pytestmark = pytest.mark.integration
|
||||||
|
|
||||||
|
# Same text for every point so cosine similarity to the query is ~identical:
|
||||||
|
# the *filter*, not the score, must decide what each user sees.
|
||||||
|
_DOC_TEXT = "Quarterly infrastructure budget planning and resource allocation"
|
||||||
|
|
||||||
|
# (point_id, doc_id, owner_id, user_id) — owner_id=None mimics a legacy point
|
||||||
|
# indexed before the owner_id payload field existed.
|
||||||
|
_ALICE_FILE = (101, "101", "alice", "alice")
|
||||||
|
_CHARLIE_FILE = (102, "102", "charlie", "charlie")
|
||||||
|
_LEGACY_DAVE_FILE = (103, "103", None, "dave")
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
async def seeded_collection(monkeypatch):
|
||||||
|
"""In-memory Qdrant seeded with three file points, wired into the algorithm.
|
||||||
|
|
||||||
|
Yields the ``SimpleEmbeddingProvider`` so the test can build a query vector
|
||||||
|
identical to the one the algorithm will generate.
|
||||||
|
"""
|
||||||
|
provider = SimpleEmbeddingProvider(dimension=384)
|
||||||
|
client = AsyncQdrantClient(":memory:")
|
||||||
|
collection = get_settings().get_collection_name()
|
||||||
|
|
||||||
|
# The production collection uses a named "dense" vector (see
|
||||||
|
# vector/qdrant_client.py); the semantic algorithm queries using="dense".
|
||||||
|
await client.create_collection(
|
||||||
|
collection_name=collection,
|
||||||
|
vectors_config={"dense": VectorParams(size=384, distance=Distance.COSINE)},
|
||||||
|
)
|
||||||
|
|
||||||
|
embedding = await provider.embed(_DOC_TEXT)
|
||||||
|
points = []
|
||||||
|
for point_id, doc_id, owner_id, user_id in (
|
||||||
|
_ALICE_FILE,
|
||||||
|
_CHARLIE_FILE,
|
||||||
|
_LEGACY_DAVE_FILE,
|
||||||
|
):
|
||||||
|
payload = {
|
||||||
|
"doc_id": doc_id,
|
||||||
|
"doc_type": "file",
|
||||||
|
"user_id": user_id,
|
||||||
|
"is_placeholder": False,
|
||||||
|
"file_path": f"docs/{doc_id}.txt",
|
||||||
|
"title": f"file {doc_id}",
|
||||||
|
"excerpt": _DOC_TEXT,
|
||||||
|
"chunk_index": 0,
|
||||||
|
"total_chunks": 1,
|
||||||
|
}
|
||||||
|
# Legacy points carry no owner_id at all.
|
||||||
|
if owner_id is not None:
|
||||||
|
payload["owner_id"] = owner_id
|
||||||
|
points.append(
|
||||||
|
PointStruct(id=point_id, vector={"dense": embedding}, payload=payload)
|
||||||
|
)
|
||||||
|
|
||||||
|
await client.upsert(collection_name=collection, points=points, wait=True)
|
||||||
|
|
||||||
|
# Point the algorithm at the in-memory client + deterministic embeddings.
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"nextcloud_mcp_server.search.semantic.get_qdrant_client",
|
||||||
|
AsyncMock(return_value=client),
|
||||||
|
)
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"nextcloud_mcp_server.search.semantic.get_embedding_service",
|
||||||
|
lambda: provider,
|
||||||
|
)
|
||||||
|
# get_indexed_doc_types reads the client from the algorithms module.
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"nextcloud_mcp_server.search.algorithms.get_qdrant_client",
|
||||||
|
AsyncMock(return_value=client),
|
||||||
|
)
|
||||||
|
# The cached-chunk lookups read the client from the context module.
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"nextcloud_mcp_server.search.context.get_qdrant_client",
|
||||||
|
AsyncMock(return_value=client),
|
||||||
|
)
|
||||||
|
|
||||||
|
yield provider
|
||||||
|
|
||||||
|
await client.close()
|
||||||
|
|
||||||
|
|
||||||
|
def _ids(results):
|
||||||
|
return {r.id for r in results}
|
||||||
|
|
||||||
|
|
||||||
|
async def test_shared_owner_is_visible_unshared_is_not(seeded_collection):
|
||||||
|
"""Bob sees Alice's file (shared → owner in accessible_owners), not Charlie's."""
|
||||||
|
algo = SemanticSearchAlgorithm(score_threshold=0.0)
|
||||||
|
|
||||||
|
results = await algo.search(
|
||||||
|
query=_DOC_TEXT,
|
||||||
|
user_id="bob",
|
||||||
|
limit=10,
|
||||||
|
doc_type="file",
|
||||||
|
accessible_owners=["bob", "alice"],
|
||||||
|
)
|
||||||
|
|
||||||
|
found = _ids(results)
|
||||||
|
assert "101" in found, "Alice's shared file must be discoverable by Bob"
|
||||||
|
assert "102" not in found, "Charlie's unshared file must NOT be visible to Bob"
|
||||||
|
assert "103" not in found, "Legacy file owned by dave must NOT be visible to Bob"
|
||||||
|
|
||||||
|
|
||||||
|
async def test_no_shares_sees_only_own(seeded_collection):
|
||||||
|
"""With no shares, Bob (who owns nothing here) gets nothing."""
|
||||||
|
algo = SemanticSearchAlgorithm(score_threshold=0.0)
|
||||||
|
|
||||||
|
results = await algo.search(
|
||||||
|
query=_DOC_TEXT,
|
||||||
|
user_id="bob",
|
||||||
|
limit=10,
|
||||||
|
doc_type="file",
|
||||||
|
accessible_owners=["bob"],
|
||||||
|
)
|
||||||
|
|
||||||
|
assert _ids(results) == set()
|
||||||
|
|
||||||
|
|
||||||
|
async def test_legacy_user_id_point_still_found_by_indexer(seeded_collection):
|
||||||
|
"""A pre-owner_id point stays findable by its original indexer via the
|
||||||
|
legacy ``user_id`` OR-branch in build_ownership_filter."""
|
||||||
|
algo = SemanticSearchAlgorithm(score_threshold=0.0)
|
||||||
|
|
||||||
|
results = await algo.search(
|
||||||
|
query=_DOC_TEXT,
|
||||||
|
user_id="dave",
|
||||||
|
limit=10,
|
||||||
|
doc_type="file",
|
||||||
|
accessible_owners=["dave"],
|
||||||
|
)
|
||||||
|
|
||||||
|
found = _ids(results)
|
||||||
|
assert "103" in found, "dave must still find his own legacy (user_id-only) file"
|
||||||
|
assert "101" not in found
|
||||||
|
assert "102" not in found
|
||||||
|
|
||||||
|
|
||||||
|
async def test_owner_sees_own_new_style_point(seeded_collection):
|
||||||
|
"""Alice finds her own file via the owner_id branch."""
|
||||||
|
algo = SemanticSearchAlgorithm(score_threshold=0.0)
|
||||||
|
|
||||||
|
results = await algo.search(
|
||||||
|
query=_DOC_TEXT,
|
||||||
|
user_id="alice",
|
||||||
|
limit=10,
|
||||||
|
doc_type="file",
|
||||||
|
accessible_owners=["alice"],
|
||||||
|
)
|
||||||
|
|
||||||
|
found = _ids(results)
|
||||||
|
assert "101" in found
|
||||||
|
assert "102" not in found
|
||||||
|
assert "103" not in found
|
||||||
|
|
||||||
|
|
||||||
|
async def test_get_indexed_doc_types_is_acl_aware(seeded_collection):
|
||||||
|
"""get_indexed_doc_types respects the ownership scope: with the expanded
|
||||||
|
accessible_owners Bob discovers the shared "file" type, but self-only Bob
|
||||||
|
(who owns nothing here) discovers nothing — proving it is no longer
|
||||||
|
ACL-blind."""
|
||||||
|
# ACL-aware: Bob can read Alice's shared file → discovers "file".
|
||||||
|
assert await get_indexed_doc_types("bob", accessible_owners=["bob", "alice"]) == {
|
||||||
|
"file"
|
||||||
|
}
|
||||||
|
# Self-only (default): Bob owns nothing here → discovers nothing.
|
||||||
|
assert await get_indexed_doc_types("bob") == set()
|
||||||
|
|
||||||
|
|
||||||
|
async def test_cached_chunk_lookup_is_acl_aware(seeded_collection):
|
||||||
|
"""The cached-chunk Qdrant lookup honours accessible_owners: Bob retrieves
|
||||||
|
the excerpt of Alice's file point (owner_id=alice, chunk_index=0) when alice
|
||||||
|
is in his accessible owners, but not when scoped self-only. This is the
|
||||||
|
Qdrant-layer half of cross-user file chunk context (the per-file access
|
||||||
|
gate lives in get_chunk_with_context / file_accessible_by_id)."""
|
||||||
|
# Alice's seeded file point (_ALICE_FILE) carries excerpt=_DOC_TEXT at chunk 0.
|
||||||
|
text = await _get_chunk_by_index_from_qdrant(
|
||||||
|
"bob", "101", "file", 0, accessible_owners=["bob", "alice"]
|
||||||
|
)
|
||||||
|
assert text == _DOC_TEXT
|
||||||
|
# Self-only Bob cannot reach Alice's cached chunk.
|
||||||
|
assert await _get_chunk_by_index_from_qdrant("bob", "101", "file", 0) is None
|
||||||
@@ -0,0 +1,271 @@
|
|||||||
|
"""End-to-end ACL-aware semantic search against a real Nextcloud (PR #813).
|
||||||
|
|
||||||
|
This is the card-120 acceptance criterion exercised across the *new* code
|
||||||
|
paths together:
|
||||||
|
|
||||||
|
1. ``list_accessible_owners`` resolves the querying user's real OCS shares into
|
||||||
|
the set of owner UIDs they may search.
|
||||||
|
2. ``SemanticSearchAlgorithm`` applies the expanded ownership filter in Qdrant.
|
||||||
|
3. ``verify_search_results`` re-checks each hit against real Nextcloud
|
||||||
|
(ACL-aware, by global file id).
|
||||||
|
|
||||||
|
Qdrant is in-memory and seeded directly with one point owned by *alice* — this
|
||||||
|
deliberately stands in for the background scanner (whose only relevant change
|
||||||
|
is writing ``owner_id`` into the payload, covered separately). Nextcloud itself
|
||||||
|
is real, so the share lookup (step 1) and the verification (step 3) exercise
|
||||||
|
the live OCS Sharing + WebDAV APIs. The result: bob, with whom alice shared the
|
||||||
|
file, finds it without having indexed anything; diana, with no share, does not.
|
||||||
|
|
||||||
|
The pure-filter matrix lives in ``test_acl_owner_filter.py`` and the
|
||||||
|
verification layer in ``test_verify_on_read.py``; this test is the glue that
|
||||||
|
proves the real share → accessible_owners → filter → verify chain.
|
||||||
|
"""
|
||||||
|
|
||||||
|
import os
|
||||||
|
import uuid
|
||||||
|
from unittest.mock import AsyncMock
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
from httpx import BasicAuth
|
||||||
|
from qdrant_client import AsyncQdrantClient
|
||||||
|
from qdrant_client.models import Distance, PointStruct, VectorParams
|
||||||
|
|
||||||
|
from nextcloud_mcp_server.client import NextcloudClient
|
||||||
|
from nextcloud_mcp_server.config import get_settings
|
||||||
|
from nextcloud_mcp_server.embedding import SimpleEmbeddingProvider
|
||||||
|
from nextcloud_mcp_server.search.access_filter import (
|
||||||
|
clear_accessible_owners_cache,
|
||||||
|
list_accessible_owners,
|
||||||
|
)
|
||||||
|
from nextcloud_mcp_server.search.context import get_chunk_with_context
|
||||||
|
from nextcloud_mcp_server.search.semantic import SemanticSearchAlgorithm
|
||||||
|
from nextcloud_mcp_server.search.verification import verify_search_results
|
||||||
|
|
||||||
|
pytestmark = pytest.mark.integration
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture(autouse=True)
|
||||||
|
def _reset_owners_cache():
|
||||||
|
"""Reset the process-global accessible-owners cache around each test so a
|
||||||
|
real OCS share created in a fixture isn't masked by a stale cached entry."""
|
||||||
|
clear_accessible_owners_cache()
|
||||||
|
yield
|
||||||
|
clear_accessible_owners_cache()
|
||||||
|
|
||||||
|
|
||||||
|
_DOC_TEXT = "Confidential quarterly infrastructure budget and capacity plan"
|
||||||
|
|
||||||
|
|
||||||
|
def _user_client(username: str, password: str) -> NextcloudClient:
|
||||||
|
return NextcloudClient(
|
||||||
|
base_url=os.environ["NEXTCLOUD_HOST"],
|
||||||
|
username=username,
|
||||||
|
auth=BasicAuth(username, password),
|
||||||
|
password=password,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
async def acl_users(test_users_setup):
|
||||||
|
"""alice (owner), bob (recipient), diana (no access) direct clients."""
|
||||||
|
clients = {
|
||||||
|
name: _user_client(name, test_users_setup[name]["password"])
|
||||||
|
for name in ("alice", "bob", "diana")
|
||||||
|
}
|
||||||
|
try:
|
||||||
|
yield clients
|
||||||
|
finally:
|
||||||
|
for c in clients.values():
|
||||||
|
await c._client.aclose()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
async def shared_file(acl_users):
|
||||||
|
"""alice creates a nested file and shares it with bob (not diana).
|
||||||
|
|
||||||
|
Yields (file_id, owner_relative_path); cleans up the directory after.
|
||||||
|
"""
|
||||||
|
alice = acl_users["alice"]
|
||||||
|
suffix = uuid.uuid4().hex[:8]
|
||||||
|
test_dir = f"acl_e2e_{suffix}"
|
||||||
|
nested = f"{test_dir}/reports"
|
||||||
|
path = f"{nested}/budget.txt"
|
||||||
|
|
||||||
|
await alice.webdav.create_directory(test_dir)
|
||||||
|
await alice.webdav.create_directory(nested)
|
||||||
|
await alice.webdav.write_file(path, _DOC_TEXT.encode(), "text/plain")
|
||||||
|
file_id = (await alice.webdav.get_file_info(path))["id"]
|
||||||
|
await alice.sharing.create_share(
|
||||||
|
path=f"/{path}", share_with="bob", share_type=0, permissions=1
|
||||||
|
)
|
||||||
|
try:
|
||||||
|
yield file_id, path
|
||||||
|
finally:
|
||||||
|
await alice.webdav.delete_resource(test_dir)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
async def seeded_semantic(monkeypatch, shared_file):
|
||||||
|
"""In-memory Qdrant carrying alice's file point, wired into the algorithm.
|
||||||
|
|
||||||
|
Stands in for the background scanner: the point carries ``owner_id=alice``
|
||||||
|
exactly as the scanner now writes it.
|
||||||
|
"""
|
||||||
|
file_id, path = shared_file
|
||||||
|
provider = SimpleEmbeddingProvider(dimension=384)
|
||||||
|
client = AsyncQdrantClient(":memory:")
|
||||||
|
collection = get_settings().get_collection_name()
|
||||||
|
await client.create_collection(
|
||||||
|
collection_name=collection,
|
||||||
|
vectors_config={"dense": VectorParams(size=384, distance=Distance.COSINE)},
|
||||||
|
)
|
||||||
|
await client.upsert(
|
||||||
|
collection_name=collection,
|
||||||
|
points=[
|
||||||
|
PointStruct(
|
||||||
|
id=int(file_id),
|
||||||
|
vector={"dense": await provider.embed(_DOC_TEXT)},
|
||||||
|
payload={
|
||||||
|
"doc_id": str(file_id),
|
||||||
|
"doc_type": "file",
|
||||||
|
"owner_id": "alice",
|
||||||
|
"user_id": "alice",
|
||||||
|
"is_placeholder": False,
|
||||||
|
"file_path": path,
|
||||||
|
"title": "budget.txt",
|
||||||
|
"excerpt": _DOC_TEXT,
|
||||||
|
"chunk_index": 0,
|
||||||
|
"total_chunks": 1,
|
||||||
|
},
|
||||||
|
)
|
||||||
|
],
|
||||||
|
wait=True,
|
||||||
|
)
|
||||||
|
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"nextcloud_mcp_server.search.semantic.get_qdrant_client",
|
||||||
|
AsyncMock(return_value=client),
|
||||||
|
)
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"nextcloud_mcp_server.search.semantic.get_embedding_service",
|
||||||
|
lambda: provider,
|
||||||
|
)
|
||||||
|
# The cached-chunk lookups (get_chunk_with_context) read the client from the
|
||||||
|
# context module — point it at the same in-memory Qdrant.
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"nextcloud_mcp_server.search.context.get_qdrant_client",
|
||||||
|
AsyncMock(return_value=client),
|
||||||
|
)
|
||||||
|
yield file_id
|
||||||
|
await client.close()
|
||||||
|
|
||||||
|
|
||||||
|
async def _search_as(user_client, file_id_unused) -> list:
|
||||||
|
"""Run the full new chain (share lookup → filter → verify) as a user."""
|
||||||
|
accessible_owners = await list_accessible_owners(
|
||||||
|
user_client.sharing, user_client.username
|
||||||
|
)
|
||||||
|
algo = SemanticSearchAlgorithm(score_threshold=0.0)
|
||||||
|
unverified = await algo.search(
|
||||||
|
query=_DOC_TEXT,
|
||||||
|
user_id=user_client.username,
|
||||||
|
limit=10,
|
||||||
|
doc_type="file",
|
||||||
|
accessible_owners=accessible_owners,
|
||||||
|
)
|
||||||
|
kept, _dropped = await verify_search_results(user_client, unverified)
|
||||||
|
return kept
|
||||||
|
|
||||||
|
|
||||||
|
async def test_recipient_finds_shared_file_without_indexing(acl_users, seeded_semantic):
|
||||||
|
"""Bob finds alice's shared file end-to-end: real share lookup expands his
|
||||||
|
accessible owners to include alice, the filter surfaces her point, and
|
||||||
|
real verification confirms his ACL access — all without bob indexing."""
|
||||||
|
file_id = seeded_semantic
|
||||||
|
# Sanity: the live OCS lookup really does expand bob to include alice.
|
||||||
|
owners = await list_accessible_owners(acl_users["bob"].sharing, "bob")
|
||||||
|
assert "alice" in owners, "OCS shared-with-me must surface alice as an owner"
|
||||||
|
|
||||||
|
kept = await _search_as(acl_users["bob"], file_id)
|
||||||
|
|
||||||
|
assert [r.id for r in kept] == [str(file_id)], (
|
||||||
|
"bob must find alice's shared file via semantic search"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
async def test_non_recipient_does_not_find_file(acl_users, seeded_semantic):
|
||||||
|
"""Diana, with no share, never sees the file: her accessible-owners set
|
||||||
|
excludes alice, so the ownership filter drops the point before verification."""
|
||||||
|
owners = await list_accessible_owners(acl_users["diana"].sharing, "diana")
|
||||||
|
assert "alice" not in owners
|
||||||
|
|
||||||
|
kept = await _search_as(acl_users["diana"], seeded_semantic)
|
||||||
|
|
||||||
|
assert kept == [], "diana (no share) must not find alice's file"
|
||||||
|
|
||||||
|
|
||||||
|
async def test_file_accessible_by_id_resolves_shares(acl_users, shared_file):
|
||||||
|
"""Lock the verify-on-read contract directly on ``file_accessible_by_id``.
|
||||||
|
|
||||||
|
The WebDAV SEARCH-by-fileid with ``scope=""`` must resolve a file that the
|
||||||
|
caller does NOT own but which is shared with them. This is the exact check
|
||||||
|
verify-on-read depends on for shared, nested files; a Nextcloud change to
|
||||||
|
how ``scope=""`` is interpreted would otherwise silently break ACL-aware
|
||||||
|
verification. The file lives in a subfolder, so a path-based check would
|
||||||
|
404 for the recipient — only the by-id SEARCH gets it right.
|
||||||
|
"""
|
||||||
|
file_id, _path = shared_file
|
||||||
|
fid = int(file_id)
|
||||||
|
|
||||||
|
# Owner and share recipient can both reach it...
|
||||||
|
assert await acl_users["alice"].webdav.file_accessible_by_id(fid) is True
|
||||||
|
assert await acl_users["bob"].webdav.file_accessible_by_id(fid) is True
|
||||||
|
# ...the non-recipient cannot.
|
||||||
|
assert await acl_users["diana"].webdav.file_accessible_by_id(fid) is False
|
||||||
|
|
||||||
|
|
||||||
|
async def test_cross_user_file_chunk_context(acl_users, seeded_semantic):
|
||||||
|
"""End-to-end cross-user FILE chunk context: Bob (a share recipient) gets
|
||||||
|
Alice's cached chunk text, Diana (no share) gets None.
|
||||||
|
|
||||||
|
Exercises the full secure path: the ACL-aware Qdrant cached-chunk lookup
|
||||||
|
(owner_id=alice surfaces for Bob) gated by a real per-file
|
||||||
|
``file_accessible_by_id`` check against live Nextcloud. Diana fails the gate
|
||||||
|
and is denied even though the chunk is cached. Per-user types are covered by
|
||||||
|
the self-only behaviour elsewhere — this is the file path the feature adds.
|
||||||
|
"""
|
||||||
|
file_id = seeded_semantic
|
||||||
|
bob = acl_users["bob"]
|
||||||
|
diana = acl_users["diana"]
|
||||||
|
|
||||||
|
bob_owners = await list_accessible_owners(bob.sharing, "bob")
|
||||||
|
assert "alice" in bob_owners
|
||||||
|
|
||||||
|
ctx = await get_chunk_with_context(
|
||||||
|
nc_client=bob,
|
||||||
|
user_id="bob",
|
||||||
|
doc_id=str(file_id),
|
||||||
|
doc_type="file",
|
||||||
|
chunk_start=0,
|
||||||
|
chunk_end=len(_DOC_TEXT),
|
||||||
|
chunk_index=0,
|
||||||
|
total_chunks=1,
|
||||||
|
accessible_owners=bob_owners,
|
||||||
|
)
|
||||||
|
assert ctx is not None, "Bob (share recipient) must get Alice's cached chunk"
|
||||||
|
assert ctx.chunk_text == _DOC_TEXT
|
||||||
|
|
||||||
|
# Diana has no share → per-file gate denies even though the chunk is cached.
|
||||||
|
diana_owners = await list_accessible_owners(diana.sharing, "diana")
|
||||||
|
denied = await get_chunk_with_context(
|
||||||
|
nc_client=diana,
|
||||||
|
user_id="diana",
|
||||||
|
doc_id=str(file_id),
|
||||||
|
doc_type="file",
|
||||||
|
chunk_start=0,
|
||||||
|
chunk_end=len(_DOC_TEXT),
|
||||||
|
chunk_index=0,
|
||||||
|
total_chunks=1,
|
||||||
|
accessible_owners=diana_owners,
|
||||||
|
)
|
||||||
|
assert denied is None, "Diana (no share) must not get cross-user chunk context"
|
||||||
@@ -133,7 +133,6 @@ async def test_chunk_context_endpoint_uses_app_password(
|
|||||||
try:
|
try:
|
||||||
await login_to_nextcloud(page, username, password)
|
await login_to_nextcloud(page, username, password)
|
||||||
auth_result = await complete_astrolabe_authorization(page, username, password)
|
auth_result = await complete_astrolabe_authorization(page, username, password)
|
||||||
assert auth_result["step1"], "OAuth authorization did not complete"
|
|
||||||
assert auth_result["step2"], "App password provisioning did not complete"
|
assert auth_result["step2"], "App password provisioning did not complete"
|
||||||
|
|
||||||
auth_header = _build_basic_auth_header(username, password)
|
auth_header = _build_basic_auth_header(username, password)
|
||||||
|
|||||||
@@ -1,139 +0,0 @@
|
|||||||
"""Integration test for Astrolabe's "Enable Semantic Search" OAuth flow on
|
|
||||||
the `mcp-login-flow` profile.
|
|
||||||
|
|
||||||
Cross-system interface test. Brings together Astrolabe (Nextcloud PHP app
|
|
||||||
installed at container start by ``app-hooks/post-installation``) with the
|
|
||||||
``mcp-login-flow`` MCP server over OAuth + the management API. Mirrors
|
|
||||||
the production-shaped flow that PR #773's recent
|
|
||||||
`ALLOWED_MGMT_CLIENT` ↔ `astrolabeMcpClientOAuth00000000000` drift was
|
|
||||||
masking — every management API call from Astrolabe (e.g.
|
|
||||||
``/api/v1/users/admin/session``) was returning 401 because the
|
|
||||||
real-deployment client id was not in the test-fixture allowlist, so the
|
|
||||||
Astrolabe settings page never updated to reflect a successful
|
|
||||||
authorization.
|
|
||||||
|
|
||||||
This test is **regression coverage** for that class of drift. If the
|
|
||||||
Astrolabe client id ever falls out of `mcp-login-flow`'s
|
|
||||||
``ALLOWED_MGMT_CLIENT`` again, the post-redirect assertions here will
|
|
||||||
fail because the page state stays on ``oauth-required.php``.
|
|
||||||
|
|
||||||
Requires the login-flow stack to be running:
|
|
||||||
|
|
||||||
MCP_SERVER_URL=http://mcp-login-flow:8004 \\
|
|
||||||
docker compose --profile login-flow up -d app db mcp-login-flow
|
|
||||||
|
|
||||||
The ``app-hooks/before-starting/26-configure-astrolabe-oauth.sh`` hook
|
|
||||||
creates the OAuth client with the production-shaped id
|
|
||||||
``astrolabeMcpClientOAuth00000000000`` automatically when
|
|
||||||
``MCP_SERVER_URL`` is set, so no fixture-level OIDC client creation is
|
|
||||||
needed here.
|
|
||||||
"""
|
|
||||||
|
|
||||||
import logging
|
|
||||||
import os
|
|
||||||
import re
|
|
||||||
|
|
||||||
import pytest
|
|
||||||
from playwright.async_api import Page
|
|
||||||
|
|
||||||
# Reuse helpers from the multi-user-basic Astrolabe test for login + nav.
|
|
||||||
from tests.integration.test_astrolabe_multi_user_background_sync import (
|
|
||||||
login_to_nextcloud,
|
|
||||||
navigate_to_astrolabe_settings,
|
|
||||||
)
|
|
||||||
|
|
||||||
logger = logging.getLogger(__name__)
|
|
||||||
|
|
||||||
pytestmark = [pytest.mark.integration, pytest.mark.login_flow]
|
|
||||||
|
|
||||||
NEXTCLOUD_URL = "http://localhost:8080"
|
|
||||||
ASTROLABE_SETTINGS_URL = f"{NEXTCLOUD_URL}/settings/user/astrolabe"
|
|
||||||
|
|
||||||
|
|
||||||
async def _click_enable_semantic_search(page: Page) -> None:
|
|
||||||
"""Click the "Enable Semantic Search" OAuth link on the
|
|
||||||
``oauth-required.php`` template that login-flow mode renders to a
|
|
||||||
not-yet-authorized user.
|
|
||||||
|
|
||||||
Astrolabe's own e2e helper (``third_party/astrolabe/tests/e2e/helpers/
|
|
||||||
authorize.ts``) targets the same link by accessible name.
|
|
||||||
"""
|
|
||||||
enable_link = page.get_by_role("link", name="Enable Semantic Search")
|
|
||||||
await enable_link.wait_for(state="visible", timeout=10_000)
|
|
||||||
logger.info("Clicking 'Enable Semantic Search' OAuth link")
|
|
||||||
await enable_link.click()
|
|
||||||
|
|
||||||
|
|
||||||
async def _grant_oidc_consent(page: Page) -> None:
|
|
||||||
"""Click "Allow" on the Nextcloud OIDC consent screen, if shown.
|
|
||||||
|
|
||||||
Nextcloud may auto-redirect for already-trusted clients, in which
|
|
||||||
case the consent button never appears — that's not an error.
|
|
||||||
"""
|
|
||||||
allow_button = page.get_by_role("button", name=re.compile(r"^allow$", re.I))
|
|
||||||
try:
|
|
||||||
await allow_button.wait_for(state="visible", timeout=10_000)
|
|
||||||
logger.info("Clicking 'Allow' on OIDC consent")
|
|
||||||
await allow_button.click(force=True)
|
|
||||||
except Exception:
|
|
||||||
logger.info(
|
|
||||||
"OIDC consent screen not visible — assuming auto-grant for "
|
|
||||||
"already-trusted client"
|
|
||||||
)
|
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.timeout(180)
|
|
||||||
async def test_enable_semantic_search_completes_oauth_for_login_flow(browser):
|
|
||||||
"""Click the "Enable Semantic Search" link, grant consent, and assert
|
|
||||||
the post-redirect page reflects a completed authorization.
|
|
||||||
|
|
||||||
The success criterion is intentionally negative: after the OAuth
|
|
||||||
flow, the original "Enable Semantic Search" link must be gone. If
|
|
||||||
Astrolabe's management API call is rejected by the MCP server (HTTP
|
|
||||||
401, the original bug), the page falls back to the same
|
|
||||||
``oauth-required.php`` template and the link reappears — making this
|
|
||||||
test the canary for the drift class.
|
|
||||||
"""
|
|
||||||
admin_password = os.getenv("NEXTCLOUD_PASSWORD")
|
|
||||||
if admin_password is None:
|
|
||||||
raise RuntimeError("NEXTCLOUD_PASSWORD must be set")
|
|
||||||
page = await browser.new_page()
|
|
||||||
try:
|
|
||||||
await login_to_nextcloud(page, "admin", admin_password)
|
|
||||||
await navigate_to_astrolabe_settings(page)
|
|
||||||
|
|
||||||
# Sanity-check we're on the not-yet-authorized template.
|
|
||||||
enable_link = page.get_by_role("link", name="Enable Semantic Search")
|
|
||||||
if await enable_link.count() == 0:
|
|
||||||
pytest.skip(
|
|
||||||
"Astrolabe is already authorized for admin (oauth-required.php "
|
|
||||||
"not rendered). Reset by clearing the user's OAuth tokens "
|
|
||||||
"before re-running this test."
|
|
||||||
)
|
|
||||||
|
|
||||||
await _click_enable_semantic_search(page)
|
|
||||||
await _grant_oidc_consent(page)
|
|
||||||
|
|
||||||
# OAuth callback returns to /apps/astrolabe/oauth/callback then the
|
|
||||||
# controller redirects to /settings/user/astrolabe.
|
|
||||||
await page.wait_for_url(re.compile(r"/settings/user/astrolabe"), timeout=30_000)
|
|
||||||
await page.wait_for_load_state("networkidle", timeout=15_000)
|
|
||||||
|
|
||||||
# Regression assertion for the ALLOWED_MGMT_CLIENT drift bug:
|
|
||||||
# the page must have moved past oauth-required.php. If
|
|
||||||
# Astrolabe's management API call to /api/v1/users/{id}/session
|
|
||||||
# is rejected (401), the session lookup falls back to "no token",
|
|
||||||
# and the same oauth-required.php template re-renders with the
|
|
||||||
# link still present.
|
|
||||||
post_auth_count = await page.get_by_role(
|
|
||||||
"link", name="Enable Semantic Search"
|
|
||||||
).count()
|
|
||||||
assert post_auth_count == 0, (
|
|
||||||
"'Enable Semantic Search' link still visible after completing "
|
|
||||||
"OAuth flow — Astrolabe could not read the user's session from "
|
|
||||||
"the MCP server. Most likely cause: "
|
|
||||||
"`astrolabeMcpClientOAuth00000000000` missing from "
|
|
||||||
"`ALLOWED_MGMT_CLIENT` on `mcp-login-flow`."
|
|
||||||
)
|
|
||||||
finally:
|
|
||||||
await page.close()
|
|
||||||
@@ -7,19 +7,19 @@ lives in a separate repository (https://github.com/cbcoutinho/astrolabe).
|
|||||||
|
|
||||||
This test verifies that multiple users can independently:
|
This test verifies that multiple users can independently:
|
||||||
1. Log in to Nextcloud
|
1. Log in to Nextcloud
|
||||||
2. Generate an app password in Security settings
|
2. Click the one-click "Enable background indexing" opt-in in Astrolabe settings
|
||||||
3. Enter the app password in Astrolabe personal settings
|
3. Have a dedicated app password minted from their session and handed to the
|
||||||
4. Enable background sync for the mcp-multi-user-basic service
|
MCP server (core/getapppassword — no Security-settings step, no copy-paste)
|
||||||
5. Verify app password is stored in the database
|
4. Verify the app password is stored in the database
|
||||||
|
|
||||||
Tests the complete app password provisioning flow:
|
Tests the one-click background-indexing provisioning flow:
|
||||||
user login → Security settings → app password generation → Astrolabe settings →
|
user login → Astrolabe settings → Enable background indexing → session app
|
||||||
app password entry → background sync activation → database verification.
|
password minted + forwarded to MCP → background sync active → DB verification.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
import logging
|
import logging
|
||||||
import re
|
|
||||||
import subprocess
|
import subprocess
|
||||||
|
import tempfile
|
||||||
|
|
||||||
import anyio
|
import anyio
|
||||||
import pytest
|
import pytest
|
||||||
@@ -91,621 +91,64 @@ async def navigate_to_astrolabe_settings(page: Page):
|
|||||||
logger.info("✓ Successfully loaded Astrolabe settings page")
|
logger.info("✓ Successfully loaded Astrolabe settings page")
|
||||||
|
|
||||||
|
|
||||||
async def authorize_search_access(page: Page, username: str) -> bool:
|
async def enable_background_sync(page: Page, username: str) -> bool:
|
||||||
"""Complete Step 1: OAuth Authorization for Astrolabe.
|
"""Provision background indexing via the one-click opt-in button.
|
||||||
|
|
||||||
Handles the OAuth flow:
|
The refactored settings page mints a dedicated app password from the
|
||||||
1. Check if already authorized (Step 1 shows "Complete")
|
current Nextcloud session (core/getapppassword) and hands it to the MCP
|
||||||
2. Click "Authorize" link
|
server — there is no app-password generation in Security settings and no
|
||||||
3. Handle Nextcloud OIDC consent screen
|
copy-paste. Idempotent: if already enabled (the revoke form is shown
|
||||||
4. Wait for redirect back to Astrolabe settings
|
instead of the enable button), returns True without acting.
|
||||||
5. Verify "Complete" badge appears on Step 1
|
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
page: Playwright page instance (must be on Astrolabe settings page)
|
page: Playwright page instance (must be logged in)
|
||||||
username: Username for logging
|
|
||||||
|
|
||||||
Returns:
|
|
||||||
True if authorization completed successfully
|
|
||||||
"""
|
|
||||||
nextcloud_url = "http://localhost:8080"
|
|
||||||
|
|
||||||
logger.info("Authorizing search access (Step 1) for %s...", username)
|
|
||||||
|
|
||||||
# Check if already on Astrolabe settings page, if not navigate there
|
|
||||||
if "/settings/user/astrolabe" not in page.url:
|
|
||||||
await navigate_to_astrolabe_settings(page)
|
|
||||||
|
|
||||||
# Wait for page to fully render
|
|
||||||
await anyio.sleep(1)
|
|
||||||
|
|
||||||
# Check if already authorized (either "Active" badge or Step 1 "Complete" badge)
|
|
||||||
try:
|
|
||||||
# Check for "Active" badge (fully configured state)
|
|
||||||
active_badge = page.get_by_text("Active", exact=True)
|
|
||||||
if await active_badge.count() > 0 and await active_badge.is_visible():
|
|
||||||
logger.info("✓ Already fully authorized for %s (Active badge)", username)
|
|
||||||
return True
|
|
||||||
except Exception:
|
|
||||||
pass
|
|
||||||
|
|
||||||
try:
|
|
||||||
step1_section = page.locator('h4:has-text("Step 1")')
|
|
||||||
if await step1_section.count() > 0:
|
|
||||||
# Look for "Complete" text in the Step 1 section's parent
|
|
||||||
step1_parent = step1_section.locator("..")
|
|
||||||
complete_badge = step1_parent.get_by_text("Complete", exact=True)
|
|
||||||
if await complete_badge.count() > 0 and await complete_badge.is_visible():
|
|
||||||
logger.info("✓ Step 1 already complete for %s", username)
|
|
||||||
return True
|
|
||||||
except Exception:
|
|
||||||
pass
|
|
||||||
|
|
||||||
# Find and click the "Authorize" button
|
|
||||||
authorize_button = page.locator('a.button.primary:has-text("Authorize")')
|
|
||||||
|
|
||||||
try:
|
|
||||||
await authorize_button.wait_for(timeout=5000, state="visible")
|
|
||||||
logger.info("Found Authorize button for %s", username)
|
|
||||||
except Exception:
|
|
||||||
# Take screenshot for debugging
|
|
||||||
screenshot_path = f"/tmp/astrolabe_no_authorize_button_{username}.png"
|
|
||||||
await page.screenshot(path=screenshot_path)
|
|
||||||
logger.error(
|
|
||||||
"Could not find Authorize button for %s. Screenshot: %s",
|
|
||||||
username,
|
|
||||||
screenshot_path,
|
|
||||||
)
|
|
||||||
raise ValueError(f"Authorize button not found for {username}")
|
|
||||||
|
|
||||||
# Click the Authorize button - this will redirect to OAuth provider
|
|
||||||
# Use force=True to bypass stability check which can timeout due to CSS transitions
|
|
||||||
await authorize_button.click(force=True)
|
|
||||||
logger.info("Clicked Authorize button for %s", username)
|
|
||||||
|
|
||||||
# Wait for OAuth redirect to complete
|
|
||||||
await page.wait_for_load_state("networkidle", timeout=30000)
|
|
||||||
logger.info("After networkidle, current URL: %s", page.url)
|
|
||||||
|
|
||||||
# Take screenshot to see current state
|
|
||||||
await page.screenshot(path=f"/tmp/astrolabe_after_authorize_{username}.png")
|
|
||||||
logger.info("Screenshot saved: /tmp/astrolabe_after_authorize_%s.png", username)
|
|
||||||
|
|
||||||
# Handle OIDC consent screen if present
|
|
||||||
consent_handled = await _handle_oauth_consent_screen(page, username)
|
|
||||||
if consent_handled:
|
|
||||||
logger.info("✓ OAuth consent granted for %s", username)
|
|
||||||
else:
|
|
||||||
logger.info(
|
|
||||||
"No consent screen required for %s (may be previously authorized)", username
|
|
||||||
)
|
|
||||||
|
|
||||||
# Wait for redirect back to Astrolabe settings
|
|
||||||
# The OAuth callback will redirect back to /settings/user/astrolabe
|
|
||||||
try:
|
|
||||||
await page.wait_for_url(
|
|
||||||
f"**{nextcloud_url}/settings/user/astrolabe**", timeout=30000
|
|
||||||
)
|
|
||||||
logger.info("Redirected back to Astrolabe settings for %s", username)
|
|
||||||
except Exception:
|
|
||||||
# Check if we're already on settings page
|
|
||||||
if "/settings/user/astrolabe" not in page.url:
|
|
||||||
logger.warning(
|
|
||||||
"Not redirected to Astrolabe settings, current URL: %s", page.url
|
|
||||||
)
|
|
||||||
# Navigate manually
|
|
||||||
await page.goto(
|
|
||||||
f"{nextcloud_url}/settings/user/astrolabe", wait_until="networkidle"
|
|
||||||
)
|
|
||||||
|
|
||||||
# Wait for page to reload and render
|
|
||||||
await anyio.sleep(2)
|
|
||||||
|
|
||||||
# Verify authorization completed - check for various success indicators
|
|
||||||
# When fully configured, shows "Active" badge; when only Step 1 done, shows "Complete"
|
|
||||||
try:
|
|
||||||
# First check if "Active" badge is shown (fully configured state)
|
|
||||||
active_badge = page.get_by_text("Active", exact=True)
|
|
||||||
if await active_badge.count() > 0 and await active_badge.is_visible():
|
|
||||||
logger.info(
|
|
||||||
"✓ OAuth authorization complete for %s (Active badge)", username
|
|
||||||
)
|
|
||||||
return True
|
|
||||||
except Exception:
|
|
||||||
pass
|
|
||||||
|
|
||||||
try:
|
|
||||||
# Check for Step 1 "Complete" badge (partial configuration)
|
|
||||||
step1_section = page.locator('h4:has-text("Step 1")')
|
|
||||||
if await step1_section.count() > 0:
|
|
||||||
step1_parent = step1_section.locator("..")
|
|
||||||
complete_badge = step1_parent.get_by_text("Complete", exact=True)
|
|
||||||
await complete_badge.wait_for(timeout=5000, state="visible")
|
|
||||||
logger.info("✓ Step 1 OAuth authorization complete for %s", username)
|
|
||||||
return True
|
|
||||||
except Exception:
|
|
||||||
pass
|
|
||||||
|
|
||||||
# Neither badge found - authorization failed
|
|
||||||
screenshot_path = f"/tmp/astrolabe_step1_not_complete_{username}.png"
|
|
||||||
await page.screenshot(path=screenshot_path)
|
|
||||||
logger.error(
|
|
||||||
"Authorization badge not visible for %s. Screenshot: %s",
|
|
||||||
username,
|
|
||||||
screenshot_path,
|
|
||||||
)
|
|
||||||
raise ValueError(f"OAuth authorization did not complete for {username}")
|
|
||||||
|
|
||||||
|
|
||||||
async def _handle_oauth_consent_screen(page: Page, username: str) -> bool:
|
|
||||||
"""Handle the OIDC consent screen during OAuth flow.
|
|
||||||
|
|
||||||
Reuses the proven pattern from tests/conftest.py.
|
|
||||||
|
|
||||||
Args:
|
|
||||||
page: Playwright page instance
|
|
||||||
username: Username for logging
|
|
||||||
|
|
||||||
Returns:
|
|
||||||
True if consent was handled, False if no consent screen was found
|
|
||||||
"""
|
|
||||||
try:
|
|
||||||
logger.info("Checking for consent screen at URL: %s", page.url)
|
|
||||||
|
|
||||||
# Check if consent screen is present - try multiple selectors
|
|
||||||
# The consent screen may be #oidc-consent or use a different format
|
|
||||||
consent_div = await page.query_selector("#oidc-consent")
|
|
||||||
|
|
||||||
if consent_div:
|
|
||||||
logger.info("Consent screen detected via #oidc-consent for %s", username)
|
|
||||||
# Get consent screen data attributes for logging
|
|
||||||
client_name = await consent_div.get_attribute("data-client-name")
|
|
||||||
scopes_attr = await consent_div.get_attribute("data-scopes")
|
|
||||||
logger.info(" Client: %s", client_name)
|
|
||||||
logger.info(" Requested scopes: %s", scopes_attr)
|
|
||||||
else:
|
|
||||||
# Check for Allow button directly (different consent screen format)
|
|
||||||
allow_button = page.locator('button:has-text("Allow")')
|
|
||||||
if await allow_button.count() > 0:
|
|
||||||
logger.info("Consent screen detected via Allow button for %s", username)
|
|
||||||
else:
|
|
||||||
logger.info("No consent screen found for %s at %s", username, page.url)
|
|
||||||
await page.screenshot(path=f"/tmp/no_consent_screen_{username}.png")
|
|
||||||
logger.info("Screenshot: /tmp/no_consent_screen_%s.png", username)
|
|
||||||
return False
|
|
||||||
|
|
||||||
# Wait for Vue.js to render the Allow button
|
|
||||||
try:
|
|
||||||
await page.wait_for_selector('button:has-text("Allow")', timeout=10000)
|
|
||||||
logger.info(" Allow button rendered by Vue.js")
|
|
||||||
except Exception as e:
|
|
||||||
screenshot_path = f"/tmp/consent_no_allow_button_{username}.png"
|
|
||||||
await page.screenshot(path=screenshot_path)
|
|
||||||
logger.error(" Timeout waiting for Allow button: %s", e)
|
|
||||||
raise
|
|
||||||
|
|
||||||
# Check all scope checkboxes
|
|
||||||
scope_checkboxes = await page.query_selector_all('input[type="checkbox"]')
|
|
||||||
if scope_checkboxes:
|
|
||||||
logger.info(" Found %s scope checkboxes", len(scope_checkboxes))
|
|
||||||
for i, checkbox in enumerate(scope_checkboxes):
|
|
||||||
is_checked = await checkbox.is_checked()
|
|
||||||
is_disabled = await checkbox.is_disabled()
|
|
||||||
if not is_checked and not is_disabled:
|
|
||||||
await checkbox.check()
|
|
||||||
logger.info(" ✓ Checked scope checkbox %s", i + 1)
|
|
||||||
|
|
||||||
# Click the Allow button using JavaScript (handles viewport issues)
|
|
||||||
allow_button_locator = page.locator('button:has-text("Allow")')
|
|
||||||
|
|
||||||
# Debug: take screenshot before clicking Allow
|
|
||||||
await page.screenshot(path=f"/tmp/consent_before_allow_{username}.png")
|
|
||||||
logger.info(
|
|
||||||
" Screenshot before Allow: /tmp/consent_before_allow_%s.png", username
|
|
||||||
)
|
|
||||||
|
|
||||||
button_count = await allow_button_locator.count()
|
|
||||||
logger.info(" Found %s Allow button(s)", button_count)
|
|
||||||
|
|
||||||
if button_count > 0:
|
|
||||||
current_url = page.url
|
|
||||||
logger.info(" Current URL: %s", current_url)
|
|
||||||
logger.info(" Clicking Allow button for %s...", username)
|
|
||||||
|
|
||||||
# Use JavaScript click to handle consent buttons (proven pattern from conftest.py)
|
|
||||||
# This is more reliable than Playwright's click for Vue.js rendered buttons
|
|
||||||
await page.evaluate(
|
|
||||||
"""
|
|
||||||
const buttons = document.querySelectorAll('button');
|
|
||||||
for (const btn of buttons) {
|
|
||||||
if (btn.textContent.trim() === 'Allow') {
|
|
||||||
btn.click();
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
"""
|
|
||||||
)
|
|
||||||
|
|
||||||
# Wait for URL to change (Vue.js uses window.location.href after fetch)
|
|
||||||
# networkidle doesn't detect fetch-based redirects
|
|
||||||
try:
|
|
||||||
await page.wait_for_url(
|
|
||||||
lambda url: url != current_url,
|
|
||||||
timeout=30000,
|
|
||||||
)
|
|
||||||
logger.info(" URL changed to: %s", page.url)
|
|
||||||
except Exception as wait_error:
|
|
||||||
# If URL didn't change, check console for errors
|
|
||||||
logger.warning(" URL didn't change after click: %s", wait_error)
|
|
||||||
await page.screenshot(path=f"/tmp/consent_after_allow_{username}.png")
|
|
||||||
|
|
||||||
# Try alternative: manually POST consent and navigate
|
|
||||||
logger.info(" Trying manual consent submission...")
|
|
||||||
try:
|
|
||||||
redirect_url = await page.evaluate(
|
|
||||||
"""
|
|
||||||
async () => {
|
|
||||||
const selectedScopes = Array.from(document.querySelectorAll('input[type="checkbox"]:checked'))
|
|
||||||
.map(cb => cb.value).join(' ');
|
|
||||||
|
|
||||||
const response = await fetch('/index.php/apps/oidc/consent/grant', {
|
|
||||||
method: 'POST',
|
|
||||||
headers: {
|
|
||||||
'Content-Type': 'application/x-www-form-urlencoded',
|
|
||||||
'requesttoken': OC.requestToken,
|
|
||||||
},
|
|
||||||
body: 'scopes=' + encodeURIComponent(selectedScopes),
|
|
||||||
redirect: 'follow',
|
|
||||||
});
|
|
||||||
|
|
||||||
return response.url || '/index.php/apps/oidc/authorize';
|
|
||||||
}
|
|
||||||
"""
|
|
||||||
)
|
|
||||||
logger.info(" Manual consent returned URL: %s", redirect_url)
|
|
||||||
await page.goto(redirect_url, wait_until="networkidle")
|
|
||||||
except Exception as manual_error:
|
|
||||||
logger.error(" Manual consent also failed: %s", manual_error)
|
|
||||||
raise
|
|
||||||
|
|
||||||
await page.screenshot(path=f"/tmp/consent_after_allow_{username}.png")
|
|
||||||
logger.info(" Consent granted for %s", username)
|
|
||||||
return True
|
|
||||||
else:
|
|
||||||
logger.error(" Allow button not found for %s", username)
|
|
||||||
return False
|
|
||||||
|
|
||||||
except Exception as e:
|
|
||||||
logger.error("Error handling consent screen for %s: %s", username, e)
|
|
||||||
raise
|
|
||||||
|
|
||||||
|
|
||||||
async def generate_app_password(
|
|
||||||
page: Page, username: str, app_name: str = "Astrolabe Background Sync"
|
|
||||||
) -> str:
|
|
||||||
"""Generate an app password in Nextcloud Security settings.
|
|
||||||
|
|
||||||
Args:
|
|
||||||
page: Playwright page instance (must be authenticated)
|
|
||||||
username: Username (for logging)
|
username: Username (for logging)
|
||||||
app_name: Name for the app password
|
|
||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
The generated app password string
|
True once background indexing is enabled.
|
||||||
"""
|
"""
|
||||||
logger.info("Generating app password for %s...", username)
|
logger.info("Enabling background indexing for %s...", username)
|
||||||
|
|
||||||
nextcloud_url = "http://localhost:8080"
|
|
||||||
|
|
||||||
# Navigate to Security settings
|
|
||||||
await page.goto(f"{nextcloud_url}/settings/user/security", wait_until="networkidle")
|
|
||||||
logger.info("Navigated to Security settings")
|
|
||||||
|
|
||||||
# Fill the app password input field (selector confirmed via Playwright MCP)
|
|
||||||
app_password_input = page.locator('input[placeholder="App name"]')
|
|
||||||
await app_password_input.fill(app_name)
|
|
||||||
logger.info("Entered app name: %s", app_name)
|
|
||||||
|
|
||||||
# Wait for Vue.js to react and enable the button (needs 1 second, not 0.5)
|
|
||||||
await anyio.sleep(1.0)
|
|
||||||
logger.info("Waited for Vue.js to process input and enable button")
|
|
||||||
|
|
||||||
# Click the create button - use force=True to bypass stability check (CSS transitions)
|
|
||||||
create_button = page.locator(
|
|
||||||
'button[type="submit"]:has-text("Create new app password")'
|
|
||||||
)
|
|
||||||
try:
|
|
||||||
await create_button.click(force=True, timeout=10000)
|
|
||||||
except Exception:
|
|
||||||
# Fallback: JavaScript click
|
|
||||||
logger.info("Using JavaScript click for create button...")
|
|
||||||
await page.evaluate(
|
|
||||||
"""
|
|
||||||
const btn = document.querySelector('button[type="submit"]');
|
|
||||||
if (btn) btn.click();
|
|
||||||
"""
|
|
||||||
)
|
|
||||||
logger.info("Clicked create app password button")
|
|
||||||
|
|
||||||
# Wait for app password to be generated and displayed in the dialog
|
|
||||||
await anyio.sleep(3) # Give it more time to generate and display
|
|
||||||
|
|
||||||
# Debug screenshot after clicking create
|
|
||||||
await page.screenshot(path=f"/tmp/app_password_after_create_{username}.png")
|
|
||||||
logger.info(
|
|
||||||
"Screenshot after create: /tmp/app_password_after_create_%s.png", username
|
|
||||||
)
|
|
||||||
|
|
||||||
# Find the Login input field which should have the username value
|
|
||||||
# Then find the Password input field which is in the same form
|
|
||||||
app_password = None
|
|
||||||
try:
|
|
||||||
# Wait for heading "New app password" to appear
|
|
||||||
await page.wait_for_selector('text="New app password"', timeout=10000)
|
|
||||||
logger.info("App password dialog appeared with heading")
|
|
||||||
|
|
||||||
# Get all visible input elements
|
|
||||||
all_inputs = await page.locator('input[type="text"]').all()
|
|
||||||
logger.info("Found %s text input elements", len(all_inputs))
|
|
||||||
|
|
||||||
# Check each input to find the one with the app password
|
|
||||||
for idx, input_elem in enumerate(all_inputs):
|
|
||||||
try:
|
|
||||||
value = await input_elem.input_value()
|
|
||||||
if value and "-" in value and len(value) > 20:
|
|
||||||
app_password = value.strip()
|
|
||||||
logger.info(
|
|
||||||
"Found app password in input %s: '%s' (length: %s)",
|
|
||||||
idx,
|
|
||||||
app_password,
|
|
||||||
len(app_password),
|
|
||||||
)
|
|
||||||
break
|
|
||||||
except Exception as e:
|
|
||||||
logger.debug("Could not get value from input %s: %s", idx, e)
|
|
||||||
continue
|
|
||||||
|
|
||||||
except Exception as e:
|
|
||||||
logger.error("Failed to find app password dialog or extract password: %s", e)
|
|
||||||
|
|
||||||
if not app_password:
|
|
||||||
# Take screenshot for debugging
|
|
||||||
screenshot_path = f"/tmp/app_password_generation_{username}.png"
|
|
||||||
await page.screenshot(path=screenshot_path)
|
|
||||||
raise ValueError(
|
|
||||||
f"Could not find generated app password. Screenshot: {screenshot_path}"
|
|
||||||
)
|
|
||||||
|
|
||||||
# Validate password format before returning
|
|
||||||
|
|
||||||
if not re.match(
|
|
||||||
r"^[a-zA-Z0-9]{5}-[a-zA-Z0-9]{5}-[a-zA-Z0-9]{5}-[a-zA-Z0-9]{5}-[a-zA-Z0-9]{5}$",
|
|
||||||
app_password,
|
|
||||||
):
|
|
||||||
logger.error(
|
|
||||||
"Extracted password does not match expected format: '%s'", app_password
|
|
||||||
)
|
|
||||||
logger.error("Password repr: %s", repr(app_password))
|
|
||||||
screenshot_path = f"/tmp/app_password_invalid_format_{username}.png"
|
|
||||||
await page.screenshot(path=screenshot_path)
|
|
||||||
raise ValueError(
|
|
||||||
f"App password format validation failed. Screenshot: {screenshot_path}"
|
|
||||||
)
|
|
||||||
|
|
||||||
logger.info(
|
|
||||||
"✓ Generated app password for %s: %s... (validated)",
|
|
||||||
username,
|
|
||||||
app_password[:10],
|
|
||||||
)
|
|
||||||
|
|
||||||
# Close dialog with Escape key (bypasses CSS layout issues with h2 intercepting clicks)
|
|
||||||
logger.info("Closing app password dialog with Escape key...")
|
|
||||||
await page.keyboard.press("Escape")
|
|
||||||
await anyio.sleep(0.5) # Wait for dialog close animation
|
|
||||||
logger.info("Closed app password dialog")
|
|
||||||
|
|
||||||
return app_password
|
|
||||||
|
|
||||||
|
|
||||||
async def enable_background_sync_via_app_password(
|
|
||||||
page: Page, username: str, app_password: str
|
|
||||||
):
|
|
||||||
"""Enable background sync by entering app password in Astrolabe settings.
|
|
||||||
|
|
||||||
Args:
|
|
||||||
page: Playwright page instance
|
|
||||||
username: Username (for logging)
|
|
||||||
app_password: App password to enter
|
|
||||||
|
|
||||||
Returns:
|
|
||||||
True if background sync was enabled successfully
|
|
||||||
"""
|
|
||||||
logger.info("Enabling background sync via app password for %s...", username)
|
|
||||||
|
|
||||||
nextcloud_url = "http://localhost:8080"
|
|
||||||
|
|
||||||
# Set up network request and console listeners BEFORE navigation
|
|
||||||
network_requests = []
|
|
||||||
network_responses = []
|
|
||||||
console_messages = []
|
|
||||||
|
|
||||||
def log_request(req):
|
|
||||||
network_requests.append(f"{req.method} {req.url}")
|
|
||||||
|
|
||||||
def log_response(resp):
|
|
||||||
response_info = f"{resp.status} {resp.url}"
|
|
||||||
network_responses.append(response_info)
|
|
||||||
logger.info("Response: %s", response_info)
|
|
||||||
|
|
||||||
def log_console(msg):
|
|
||||||
console_messages.append(f"[{msg.type}] {msg.text}")
|
|
||||||
|
|
||||||
page.on("request", log_request)
|
|
||||||
page.on("response", log_response)
|
|
||||||
page.on("console", log_console)
|
|
||||||
|
|
||||||
# Navigate to Astrolabe settings
|
|
||||||
await page.goto(
|
await page.goto(
|
||||||
f"{nextcloud_url}/settings/user/astrolabe", wait_until="networkidle"
|
"http://localhost:8080/settings/user/astrolabe", wait_until="networkidle"
|
||||||
)
|
)
|
||||||
|
|
||||||
# Wait for page to load
|
|
||||||
await anyio.sleep(1)
|
await anyio.sleep(1)
|
||||||
|
|
||||||
# Check if already complete (look for Step 2 "Complete" badge or overall "Active" state)
|
if await page.locator("#mcp-revoke-background-button").count() > 0:
|
||||||
try:
|
logger.info("✓ Background indexing already enabled for %s", username)
|
||||||
# First check for overall "Active" badge (both steps complete)
|
return True
|
||||||
active_text = page.get_by_text("Active", exact=True)
|
|
||||||
if await active_text.is_visible(timeout=2000):
|
|
||||||
logger.info("✓ Background sync already active for %s", username)
|
|
||||||
return True
|
|
||||||
except Exception:
|
|
||||||
pass
|
|
||||||
|
|
||||||
try:
|
enable_button = page.locator("#mcp-enable-background-button")
|
||||||
# Check for Step 2 "Complete" badge (app password already set)
|
await enable_button.wait_for(timeout=5000, state="visible")
|
||||||
step2_section = page.locator('h4:has-text("Step 2")')
|
await enable_button.click()
|
||||||
if await step2_section.count() > 0:
|
logger.info("Clicked 'Enable background indexing' for %s", username)
|
||||||
step2_parent = step2_section.locator("..")
|
|
||||||
complete_badge = step2_parent.get_by_text("Complete", exact=True)
|
|
||||||
if await complete_badge.count() > 0 and await complete_badge.is_visible():
|
|
||||||
logger.info("✓ Step 2 (app password) already complete for %s", username)
|
|
||||||
return True
|
|
||||||
except Exception:
|
|
||||||
pass
|
|
||||||
|
|
||||||
# Find the app password input field using the placeholder text
|
|
||||||
# Based on manual testing: textbox with placeholder "xxxxx-xxxxx-xxxxx-xxxxx-xxxxx"
|
|
||||||
app_password_input = page.get_by_placeholder("xxxxx-xxxxx-xxxxx-xxxxx-xxxxx")
|
|
||||||
|
|
||||||
|
# On success the page JS reloads to the enabled state (revoke form shown).
|
||||||
try:
|
try:
|
||||||
await app_password_input.wait_for(timeout=5000, state="visible")
|
await page.locator("#mcp-revoke-background-button").wait_for(
|
||||||
logger.info("Found app password input field")
|
timeout=15000, state="visible"
|
||||||
|
)
|
||||||
|
logger.info("✓ Background indexing enabled for %s", username)
|
||||||
|
return True
|
||||||
except Exception:
|
except Exception:
|
||||||
# Take screenshot for debugging
|
screenshot_path = (
|
||||||
screenshot_path = f"/tmp/astrolabe_no_password_field_{username}.png"
|
f"{tempfile.gettempdir()}/astrolabe_enable_failed_{username}.png"
|
||||||
|
)
|
||||||
await page.screenshot(path=screenshot_path)
|
await page.screenshot(path=screenshot_path)
|
||||||
raise ValueError(
|
raise ValueError(
|
||||||
f"Could not find app password input field for {username}. Screenshot: {screenshot_path}"
|
f"Background indexing did not enable for {username}. "
|
||||||
|
f"Screenshot: {screenshot_path}"
|
||||||
)
|
)
|
||||||
|
|
||||||
# Enter the app password
|
|
||||||
await app_password_input.fill(app_password)
|
|
||||||
logger.info("Entered app password for %s", username)
|
|
||||||
|
|
||||||
# Wait a moment for any validation to complete
|
|
||||||
await anyio.sleep(0.5)
|
|
||||||
|
|
||||||
# Take screenshot before clicking Save to check for warnings
|
|
||||||
screenshot_path = f"/tmp/before_save_{username}.png"
|
|
||||||
await page.screenshot(path=screenshot_path)
|
|
||||||
logger.info("Screenshot taken before Save: %s", screenshot_path)
|
|
||||||
|
|
||||||
# Find and click the Save button
|
|
||||||
save_button = page.get_by_role("button", name="Save")
|
|
||||||
|
|
||||||
# Check if Save button is enabled
|
|
||||||
is_disabled = await save_button.is_disabled()
|
|
||||||
logger.info("Save button disabled state: %s", is_disabled)
|
|
||||||
|
|
||||||
await save_button.click()
|
|
||||||
logger.info("Clicked Save button")
|
|
||||||
|
|
||||||
# Give the request time to complete before checking logs
|
|
||||||
await anyio.sleep(0.5)
|
|
||||||
|
|
||||||
# Log network requests after clicking Save
|
|
||||||
logger.info("Network requests after Save for %s:", username)
|
|
||||||
for req in network_requests[-10:]: # Last 10 requests
|
|
||||||
logger.info(" %s", req)
|
|
||||||
|
|
||||||
# Log network responses after clicking Save
|
|
||||||
logger.info("Network responses after Save for %s:", username)
|
|
||||||
for resp in network_responses[-10:]: # Last 10 responses
|
|
||||||
logger.info(" %s", resp)
|
|
||||||
|
|
||||||
# Check specifically for the credentials POST response
|
|
||||||
credentials_responses = [
|
|
||||||
r for r in network_responses if "background-sync/credentials" in r
|
|
||||||
]
|
|
||||||
if credentials_responses:
|
|
||||||
logger.info("Credentials endpoint response: %s", credentials_responses[-1])
|
|
||||||
if "200" not in credentials_responses[-1]:
|
|
||||||
logger.error(
|
|
||||||
"Credentials POST did not return 200 OK: %s", credentials_responses[-1]
|
|
||||||
)
|
|
||||||
else:
|
|
||||||
logger.warning("No response found for credentials endpoint!")
|
|
||||||
|
|
||||||
# Wait for the page to reload after successful save
|
|
||||||
# The JavaScript in personalSettings.js does: setTimeout(() => window.location.reload(), 1000)
|
|
||||||
await page.wait_for_load_state("networkidle", timeout=15000)
|
|
||||||
await anyio.sleep(2)
|
|
||||||
|
|
||||||
# Log any console messages
|
|
||||||
if console_messages:
|
|
||||||
logger.info("Console messages for %s:", username)
|
|
||||||
for msg in console_messages:
|
|
||||||
logger.info(" %s", msg)
|
|
||||||
|
|
||||||
# Check for error notifications (toast messages)
|
|
||||||
try:
|
|
||||||
error_toast = page.locator(".toastify.toast-error, .toast-error")
|
|
||||||
if await error_toast.count() > 0:
|
|
||||||
error_text = await error_toast.first.text_content()
|
|
||||||
logger.error("Error notification for %s: %s", username, error_text)
|
|
||||||
except Exception:
|
|
||||||
pass
|
|
||||||
|
|
||||||
# Verify Step 2 "Complete" badge or overall "Active" badge appears after reload
|
|
||||||
try:
|
|
||||||
# First try to find "Active" badge (both steps complete)
|
|
||||||
active_text = page.get_by_text("Active", exact=True)
|
|
||||||
if await active_text.count() > 0:
|
|
||||||
await active_text.wait_for(timeout=5000, state="visible")
|
|
||||||
logger.info(
|
|
||||||
"✓ Background sync enabled for %s - Active badge visible", username
|
|
||||||
)
|
|
||||||
return True
|
|
||||||
except Exception:
|
|
||||||
pass
|
|
||||||
|
|
||||||
try:
|
|
||||||
# Check for Step 2 "Complete" badge
|
|
||||||
step2_section = page.locator('h4:has-text("Step 2")')
|
|
||||||
if await step2_section.count() > 0:
|
|
||||||
step2_parent = step2_section.locator("..")
|
|
||||||
complete_badge = step2_parent.get_by_text("Complete", exact=True)
|
|
||||||
await complete_badge.wait_for(timeout=5000, state="visible")
|
|
||||||
logger.info(
|
|
||||||
"✓ Step 2 (app password) enabled for %s - Complete badge visible",
|
|
||||||
username,
|
|
||||||
)
|
|
||||||
return True
|
|
||||||
except Exception:
|
|
||||||
pass
|
|
||||||
|
|
||||||
# If neither badge found, raise error
|
|
||||||
screenshot_path = f"/tmp/astrolabe_after_password_{username}.png"
|
|
||||||
await page.screenshot(path=screenshot_path)
|
|
||||||
logger.error(
|
|
||||||
"Neither Active nor Complete badge appeared for %s. Screenshot: %s",
|
|
||||||
username,
|
|
||||||
screenshot_path,
|
|
||||||
)
|
|
||||||
raise ValueError(f"Background sync setup did not complete for {username}")
|
|
||||||
|
|
||||||
|
|
||||||
async def complete_astrolabe_authorization(
|
async def complete_astrolabe_authorization(
|
||||||
page: Page, username: str, password: str
|
page: Page, username: str, password: str
|
||||||
) -> dict:
|
) -> dict:
|
||||||
"""Complete full Astrolabe two-step authorization.
|
"""Provision background indexing for a user (one-click app-password opt-in).
|
||||||
|
|
||||||
Performs the complete authorization flow:
|
The auth refactor dropped the per-user OAuth step entirely — search now
|
||||||
1. Navigate to Astrolabe settings
|
uses a session-minted JWT, so the only remaining "authorization" is the
|
||||||
2. OAuth authorization (Step 1) if needed
|
one-click background-indexing opt-in (a dedicated app password minted from
|
||||||
3. Generate app password in Security settings
|
the session and handed to the MCP server).
|
||||||
4. App password entry (Step 2) if needed
|
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
page: Playwright page instance (must be logged in)
|
page: Playwright page instance (must be logged in)
|
||||||
@@ -713,64 +156,15 @@ async def complete_astrolabe_authorization(
|
|||||||
password: Nextcloud password (for reference, not used directly)
|
password: Nextcloud password (for reference, not used directly)
|
||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
Dict with {"step1": bool, "step2": bool, "app_password": str | None}
|
Dict with {"step1": True (no-op, kept for caller compat),
|
||||||
|
"step2": bool, "app_password": None}
|
||||||
"""
|
"""
|
||||||
logger.info("Starting full Astrolabe authorization for %s...", username)
|
logger.info("Provisioning Astrolabe background indexing for %s...", username)
|
||||||
|
|
||||||
result = {"step1": False, "step2": False, "app_password": None}
|
# step1 is retained as always-True for backward compat with callers — there
|
||||||
|
# is no longer an OAuth authorize step to perform.
|
||||||
# Navigate to Astrolabe settings
|
result = {"step1": True, "step2": False, "app_password": None}
|
||||||
await navigate_to_astrolabe_settings(page)
|
result["step2"] = await enable_background_sync(page, username)
|
||||||
|
|
||||||
# Step 1: OAuth authorization
|
|
||||||
try:
|
|
||||||
result["step1"] = await authorize_search_access(page, username)
|
|
||||||
logger.info("✓ Step 1 complete for %s", username)
|
|
||||||
except Exception as e:
|
|
||||||
logger.error("Step 1 failed for %s: %s", username, e)
|
|
||||||
raise
|
|
||||||
|
|
||||||
# Navigate back to settings if needed (OAuth might have redirected elsewhere)
|
|
||||||
if "/settings/user/astrolabe" not in page.url:
|
|
||||||
await navigate_to_astrolabe_settings(page)
|
|
||||||
|
|
||||||
# Check if Step 2 is already complete
|
|
||||||
try:
|
|
||||||
step2_section = page.locator('h4:has-text("Step 2")')
|
|
||||||
if await step2_section.count() > 0:
|
|
||||||
step2_parent = step2_section.locator("..")
|
|
||||||
complete_badge = step2_parent.get_by_text("Complete", exact=True)
|
|
||||||
if await complete_badge.count() > 0 and await complete_badge.is_visible():
|
|
||||||
logger.info("✓ Step 2 already complete for %s", username)
|
|
||||||
result["step2"] = True
|
|
||||||
return result
|
|
||||||
except Exception:
|
|
||||||
pass
|
|
||||||
|
|
||||||
# Also check for overall "Active" badge
|
|
||||||
try:
|
|
||||||
active_text = page.get_by_text("Active", exact=True)
|
|
||||||
if await active_text.count() > 0 and await active_text.is_visible():
|
|
||||||
logger.info("✓ Authorization already fully active for %s", username)
|
|
||||||
result["step2"] = True
|
|
||||||
return result
|
|
||||||
except Exception:
|
|
||||||
pass
|
|
||||||
|
|
||||||
# Step 2: Generate app password and enter it
|
|
||||||
app_password = await generate_app_password(page, username)
|
|
||||||
result["app_password"] = app_password
|
|
||||||
|
|
||||||
try:
|
|
||||||
result["step2"] = await enable_background_sync_via_app_password(
|
|
||||||
page, username, app_password
|
|
||||||
)
|
|
||||||
logger.info("✓ Step 2 complete for %s", username)
|
|
||||||
except Exception as e:
|
|
||||||
logger.error("Step 2 failed for %s: %s", username, e)
|
|
||||||
raise
|
|
||||||
|
|
||||||
logger.info("✓ Full Astrolabe authorization complete for %s", username)
|
|
||||||
return result
|
return result
|
||||||
|
|
||||||
|
|
||||||
@@ -993,15 +387,11 @@ async def test_multi_user_astrolabe_background_sync_enablement(
|
|||||||
# Step 1: Login to Nextcloud
|
# Step 1: Login to Nextcloud
|
||||||
await login_to_nextcloud(page, username, password)
|
await login_to_nextcloud(page, username, password)
|
||||||
|
|
||||||
# Step 2: Generate app password in Security settings
|
# Step 2: One-click "Enable background indexing" (mints a dedicated
|
||||||
app_password = await generate_app_password(page, username)
|
# app password from the session and hands it to the MCP server).
|
||||||
|
sync_enabled = await enable_background_sync(page, username)
|
||||||
|
|
||||||
# Step 3: Enable background sync by entering app password in Astrolabe
|
# Step 3: Verify app password was stored in database
|
||||||
sync_enabled = await enable_background_sync_via_app_password(
|
|
||||||
page, username, app_password
|
|
||||||
)
|
|
||||||
|
|
||||||
# Step 4: Verify app password was stored in database
|
|
||||||
app_password_stored = await verify_app_password_created(username)
|
app_password_stored = await verify_app_password_created(username)
|
||||||
|
|
||||||
# Give it time to complete
|
# Give it time to complete
|
||||||
@@ -1009,7 +399,6 @@ async def test_multi_user_astrolabe_background_sync_enablement(
|
|||||||
|
|
||||||
results[username] = {
|
results[username] = {
|
||||||
"settings_accessed": True,
|
"settings_accessed": True,
|
||||||
"app_password_generated": bool(app_password),
|
|
||||||
"sync_enabled": sync_enabled,
|
"sync_enabled": sync_enabled,
|
||||||
"app_password_stored": app_password_stored,
|
"app_password_stored": app_password_stored,
|
||||||
"background_sync_active": sync_enabled and app_password_stored,
|
"background_sync_active": sync_enabled and app_password_stored,
|
||||||
@@ -1017,7 +406,6 @@ async def test_multi_user_astrolabe_background_sync_enablement(
|
|||||||
|
|
||||||
logger.info("\\n%s results:", username)
|
logger.info("\\n%s results:", username)
|
||||||
logger.info(" Settings accessed: ✓")
|
logger.info(" Settings accessed: ✓")
|
||||||
logger.info(" App password generated: %s", "✓" if app_password else "✗")
|
|
||||||
logger.info(" Sync enabled: %s", "✓" if sync_enabled else "✗")
|
logger.info(" Sync enabled: %s", "✓" if sync_enabled else "✗")
|
||||||
logger.info(
|
logger.info(
|
||||||
" App password stored: %s", "✓" if app_password_stored else "✗"
|
" App password stored: %s", "✓" if app_password_stored else "✗"
|
||||||
@@ -1061,9 +449,6 @@ async def test_multi_user_astrolabe_background_sync_enablement(
|
|||||||
assert result["settings_accessed"], (
|
assert result["settings_accessed"], (
|
||||||
f"{username} could not access Astrolabe settings"
|
f"{username} could not access Astrolabe settings"
|
||||||
)
|
)
|
||||||
assert result["app_password_generated"], (
|
|
||||||
f"{username} app password was not generated"
|
|
||||||
)
|
|
||||||
assert result["sync_enabled"], (
|
assert result["sync_enabled"], (
|
||||||
f"{username} background sync enablement did not complete successfully"
|
f"{username} background sync enablement did not complete successfully"
|
||||||
)
|
)
|
||||||
@@ -1081,7 +466,7 @@ async def test_multi_user_astrolabe_background_sync_enablement(
|
|||||||
|
|
||||||
|
|
||||||
async def revoke_background_sync_access(page: Page, username: str) -> bool:
|
async def revoke_background_sync_access(page: Page, username: str) -> bool:
|
||||||
"""Revoke background sync access by clicking the Revoke Access button.
|
"""Revoke background sync access by clicking the "Disable background indexing" button.
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
page: Playwright page instance (must be authenticated)
|
page: Playwright page instance (must be authenticated)
|
||||||
@@ -1122,37 +507,37 @@ async def revoke_background_sync_access(page: Page, username: str) -> bool:
|
|||||||
# Wait for page to load
|
# Wait for page to load
|
||||||
await anyio.sleep(1)
|
await anyio.sleep(1)
|
||||||
|
|
||||||
# Check if "Active" badge is visible (indicating background sync is enabled)
|
# The revoke form (#mcp-revoke-background-form) is only rendered while
|
||||||
|
# background indexing is enabled.
|
||||||
|
revoke_button = page.locator("#mcp-revoke-background-button")
|
||||||
try:
|
try:
|
||||||
active_text = page.get_by_text("Active", exact=True)
|
if await revoke_button.count() == 0:
|
||||||
if not await active_text.is_visible(timeout=2000):
|
|
||||||
logger.warning(
|
logger.warning(
|
||||||
"Background sync not active for %s, nothing to revoke", username
|
"Background indexing not enabled for %s, nothing to revoke", username
|
||||||
)
|
)
|
||||||
return False
|
return False
|
||||||
except Exception:
|
except Exception:
|
||||||
logger.warning("Could not find Active badge for %s", username)
|
logger.warning("Could not find revoke button for %s", username)
|
||||||
return False
|
return False
|
||||||
|
|
||||||
# Find the "Revoke Access" button
|
|
||||||
revoke_button = page.get_by_role("button", name="Revoke Access")
|
|
||||||
|
|
||||||
try:
|
try:
|
||||||
await revoke_button.wait_for(timeout=5000, state="visible")
|
await revoke_button.wait_for(timeout=5000, state="visible")
|
||||||
logger.info("Found Revoke Access button")
|
logger.info("Found 'Disable background indexing' button")
|
||||||
except Exception:
|
except Exception:
|
||||||
screenshot_path = f"/tmp/astrolabe_no_revoke_button_{username}.png"
|
screenshot_path = (
|
||||||
|
f"{tempfile.gettempdir()}/astrolabe_no_revoke_button_{username}.png"
|
||||||
|
)
|
||||||
await page.screenshot(path=screenshot_path)
|
await page.screenshot(path=screenshot_path)
|
||||||
raise ValueError(
|
raise ValueError(
|
||||||
f"Could not find Revoke Access button for {username}. Screenshot: {screenshot_path}"
|
f"Could not find revoke button for {username}. Screenshot: {screenshot_path}"
|
||||||
)
|
)
|
||||||
|
|
||||||
# Set up dialog handler for confirmation dialog
|
# Set up dialog handler for confirmation dialog
|
||||||
page.once("dialog", lambda dialog: dialog.accept())
|
page.once("dialog", lambda dialog: dialog.accept())
|
||||||
|
|
||||||
# Click the Revoke Access button
|
# Click the "Disable background indexing" button
|
||||||
await revoke_button.click()
|
await revoke_button.click()
|
||||||
logger.info("Clicked Revoke Access button")
|
logger.info("Clicked the revoke button")
|
||||||
|
|
||||||
# Wait for the request to complete and page to reload
|
# Wait for the request to complete and page to reload
|
||||||
await page.wait_for_load_state("networkidle", timeout=15000)
|
await page.wait_for_load_state("networkidle", timeout=15000)
|
||||||
@@ -1178,7 +563,9 @@ async def revoke_background_sync_access(page: Page, username: str) -> bool:
|
|||||||
else:
|
else:
|
||||||
logger.warning("No response found for credentials/revoke endpoint!")
|
logger.warning("No response found for credentials/revoke endpoint!")
|
||||||
# Take screenshot for debugging
|
# Take screenshot for debugging
|
||||||
screenshot_path = f"/tmp/astrolabe_revoke_no_response_{username}.png"
|
screenshot_path = (
|
||||||
|
f"{tempfile.gettempdir()}/astrolabe_revoke_no_response_{username}.png"
|
||||||
|
)
|
||||||
await page.screenshot(path=screenshot_path)
|
await page.screenshot(path=screenshot_path)
|
||||||
return False
|
return False
|
||||||
|
|
||||||
@@ -1198,12 +585,14 @@ async def revoke_background_sync_access(page: Page, username: str) -> bool:
|
|||||||
except Exception:
|
except Exception:
|
||||||
pass
|
pass
|
||||||
|
|
||||||
# Verify "Active" badge is no longer visible
|
# After revoke + reload the settings page returns to the un-provisioned
|
||||||
|
# state: the revoke button is gone and the app-password input is shown again.
|
||||||
try:
|
try:
|
||||||
active_text = page.get_by_text("Active", exact=True)
|
if await page.locator("#mcp-revoke-background-button").is_visible(timeout=2000):
|
||||||
if await active_text.is_visible(timeout=2000):
|
logger.error("Revoke button still visible for %s after revoke!", username)
|
||||||
logger.error("Active badge still visible for %s after revoke!", username)
|
screenshot_path = (
|
||||||
screenshot_path = f"/tmp/astrolabe_revoke_still_active_{username}.png"
|
f"{tempfile.gettempdir()}/astrolabe_revoke_still_enabled_{username}.png"
|
||||||
|
)
|
||||||
await page.screenshot(path=screenshot_path)
|
await page.screenshot(path=screenshot_path)
|
||||||
return False
|
return False
|
||||||
except Exception:
|
except Exception:
|
||||||
@@ -1280,11 +669,11 @@ async def test_revoke_background_sync_access(
|
|||||||
test_users_setup,
|
test_users_setup,
|
||||||
configure_astrolabe_for_mcp_server,
|
configure_astrolabe_for_mcp_server,
|
||||||
):
|
):
|
||||||
"""Test that users can revoke background sync access via the Revoke Access button.
|
"""Test that users can revoke background sync access via the "Disable background indexing" button.
|
||||||
|
|
||||||
This test verifies:
|
This test verifies:
|
||||||
1. User enables background sync via app password
|
1. User enables background sync via app password
|
||||||
2. User clicks "Revoke Access" button
|
2. User clicks "Disable background indexing" button
|
||||||
3. Confirmation dialog is handled
|
3. Confirmation dialog is handled
|
||||||
4. POST request is sent to /api/v1/background-sync/credentials/revoke
|
4. POST request is sent to /api/v1/background-sync/credentials/revoke
|
||||||
5. "Active" badge disappears from settings page
|
5. "Active" badge disappears from settings page
|
||||||
@@ -1319,14 +708,9 @@ async def test_revoke_background_sync_access(
|
|||||||
# Step 1: Login to Nextcloud
|
# Step 1: Login to Nextcloud
|
||||||
await login_to_nextcloud(page, username, password)
|
await login_to_nextcloud(page, username, password)
|
||||||
|
|
||||||
# Step 2: Complete full authorization (OAuth Step 1 + App Password Step 2)
|
# Provision background indexing (app-password opt-in; no OAuth step).
|
||||||
auth_result = await complete_astrolabe_authorization(page, username, password)
|
auth_result = await complete_astrolabe_authorization(page, username, password)
|
||||||
assert auth_result["step1"], (
|
assert auth_result["step2"], f"App password provisioning failed for {username}"
|
||||||
f"OAuth authorization (Step 1) failed for {username}"
|
|
||||||
)
|
|
||||||
assert auth_result["step2"], (
|
|
||||||
f"App password setup (Step 2) failed for {username}"
|
|
||||||
)
|
|
||||||
|
|
||||||
# Step 3: Verify background sync is enabled
|
# Step 3: Verify background sync is enabled
|
||||||
assert await verify_app_password_created(username), (
|
assert await verify_app_password_created(username), (
|
||||||
|
|||||||
@@ -108,7 +108,7 @@ async def navigate_to_astrolabe_main(page: Page):
|
|||||||
@pytest.mark.multi_user_basic
|
@pytest.mark.multi_user_basic
|
||||||
@pytest.mark.timeout(
|
@pytest.mark.timeout(
|
||||||
300
|
300
|
||||||
) # 5 minutes - this test involves OAuth, app password, and vector sync
|
) # 5 minutes - this test involves app-password provisioning + vector sync
|
||||||
async def test_astrolabe_plotly_visualization_with_basic_auth(
|
async def test_astrolabe_plotly_visualization_with_basic_auth(
|
||||||
browser,
|
browser,
|
||||||
test_users_setup,
|
test_users_setup,
|
||||||
@@ -143,7 +143,7 @@ async def test_astrolabe_plotly_visualization_with_basic_auth(
|
|||||||
page = await context.new_page()
|
page = await context.new_page()
|
||||||
|
|
||||||
try:
|
try:
|
||||||
# Phase 2: Complete full Astrolabe authorization (OAuth + app password)
|
# Phase 2: Provision background indexing (app-password opt-in; no OAuth)
|
||||||
await login_to_nextcloud(page, username, password)
|
await login_to_nextcloud(page, username, password)
|
||||||
auth_result = await complete_astrolabe_authorization(page, username, password)
|
auth_result = await complete_astrolabe_authorization(page, username, password)
|
||||||
logger.info("Authorization result: %s", auth_result)
|
logger.info("Authorization result: %s", auth_result)
|
||||||
|
|||||||
@@ -0,0 +1,109 @@
|
|||||||
|
"""Astrolabe session-derived JWT search path (card 120 auth refactor).
|
||||||
|
|
||||||
|
Cross-system interface test. The astrolabe app was refactored to mint a
|
||||||
|
short-lived JWT for the current Nextcloud session user on demand (via the
|
||||||
|
`oidc` app's ``TokenGenerationRequestEvent``), replacing the old OAuth
|
||||||
|
authorize + offline_access + stored-refresh-token flow. This test proves the
|
||||||
|
new path end-to-end:
|
||||||
|
|
||||||
|
logged-in NC user → GET /apps/astrolabe/api/search → astrolabe mints a JWT
|
||||||
|
(McpTokenMinter) → calls the MCP server with ``Authorization: Bearer`` →
|
||||||
|
MCP validates the JWT (unified_verifier, aud=astrolabe_client_id) → results.
|
||||||
|
|
||||||
|
The headline behavioural change is that a user needs **no provisioning** to
|
||||||
|
search: there is no authorize redirect and ``has_background_access`` stays
|
||||||
|
False (app-password provisioning is now only for *background indexing*, a
|
||||||
|
separate opt-in covered by the background-sync tests).
|
||||||
|
|
||||||
|
Astrolabe is installed + configured (astrolabe_client_id, mcp_server_url) by
|
||||||
|
the container app-hooks; the test skips if that wiring is absent. Driven over
|
||||||
|
HTTP with BasicAuth (which establishes a Nextcloud session for the request) —
|
||||||
|
no browser needed.
|
||||||
|
"""
|
||||||
|
|
||||||
|
import os
|
||||||
|
|
||||||
|
import httpx
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
pytestmark = [pytest.mark.integration, pytest.mark.login_flow]
|
||||||
|
|
||||||
|
NEXTCLOUD_URL = "http://localhost:8080"
|
||||||
|
ASTROLABE_API = f"{NEXTCLOUD_URL}/apps/astrolabe/api"
|
||||||
|
_HEADERS = {"OCS-APIRequest": "true"}
|
||||||
|
|
||||||
|
|
||||||
|
async def _astrolabe_configured(client: httpx.AsyncClient, auth) -> bool:
|
||||||
|
"""Readiness probe: astrolabe must be able to reach its MCP server."""
|
||||||
|
try:
|
||||||
|
resp = await client.get(
|
||||||
|
f"{ASTROLABE_API}/vector-status", auth=auth, headers=_HEADERS
|
||||||
|
)
|
||||||
|
except httpx.HTTPError:
|
||||||
|
return False
|
||||||
|
if resp.status_code != 200:
|
||||||
|
return False
|
||||||
|
return bool(resp.json().get("success"))
|
||||||
|
|
||||||
|
|
||||||
|
async def test_session_user_searches_without_provisioning(test_users_setup):
|
||||||
|
"""A non-admin session user searches with no OAuth/provisioning step.
|
||||||
|
|
||||||
|
success=True proves astrolabe minted a JWT from the session and the MCP
|
||||||
|
server accepted it. Results may be empty (nothing indexed) — the auth
|
||||||
|
chain, not recall, is under test here.
|
||||||
|
"""
|
||||||
|
auth = httpx.BasicAuth("bob", test_users_setup["bob"]["password"])
|
||||||
|
async with httpx.AsyncClient(timeout=30) as client:
|
||||||
|
if not await _astrolabe_configured(client, auth):
|
||||||
|
pytest.skip("Astrolabe not wired to an MCP server in this stack")
|
||||||
|
|
||||||
|
# No provisioning: search must work purely from the session JWT.
|
||||||
|
status = await client.get(
|
||||||
|
f"{ASTROLABE_API}/v1/background-sync/status", auth=auth, headers=_HEADERS
|
||||||
|
)
|
||||||
|
assert status.json()["has_background_access"] is False, (
|
||||||
|
"precondition: bob has not opted into background indexing"
|
||||||
|
)
|
||||||
|
|
||||||
|
resp = await client.get(
|
||||||
|
f"{ASTROLABE_API}/search",
|
||||||
|
params={"query": "quarterly planning", "limit": 3},
|
||||||
|
auth=auth,
|
||||||
|
headers=_HEADERS,
|
||||||
|
)
|
||||||
|
|
||||||
|
assert resp.status_code == 200, resp.text
|
||||||
|
body = resp.json()
|
||||||
|
assert body["success"] is True, (
|
||||||
|
f"session-JWT search must succeed without provisioning; got {body}"
|
||||||
|
)
|
||||||
|
assert "results" in body and "algorithm_used" in body
|
||||||
|
|
||||||
|
|
||||||
|
async def test_admin_session_search_succeeds():
|
||||||
|
"""The same JWT-mint path works for the admin session user."""
|
||||||
|
admin_pw = os.environ["NEXTCLOUD_PASSWORD"]
|
||||||
|
auth = httpx.BasicAuth(os.environ["NEXTCLOUD_USERNAME"], admin_pw)
|
||||||
|
async with httpx.AsyncClient(timeout=30) as client:
|
||||||
|
if not await _astrolabe_configured(client, auth):
|
||||||
|
pytest.skip("Astrolabe not wired to an MCP server in this stack")
|
||||||
|
resp = await client.get(
|
||||||
|
f"{ASTROLABE_API}/search",
|
||||||
|
params={"query": "infrastructure", "limit": 3},
|
||||||
|
auth=auth,
|
||||||
|
headers=_HEADERS,
|
||||||
|
)
|
||||||
|
assert resp.status_code == 200, resp.text
|
||||||
|
assert resp.json()["success"] is True
|
||||||
|
|
||||||
|
|
||||||
|
async def test_search_requires_authentication():
|
||||||
|
"""Unauthenticated search is rejected (no anonymous JWT minting)."""
|
||||||
|
async with httpx.AsyncClient(follow_redirects=False, timeout=30) as client:
|
||||||
|
resp = await client.get(
|
||||||
|
f"{ASTROLABE_API}/search",
|
||||||
|
params={"query": "x"},
|
||||||
|
headers=_HEADERS,
|
||||||
|
)
|
||||||
|
assert resp.status_code in (401, 302, 303, 307, 308), resp.status_code
|
||||||
@@ -1,96 +1,63 @@
|
|||||||
"""Integration tests for Astrolabe personal settings page buttons.
|
"""Integration tests for Astrolabe personal-settings background-sync endpoints.
|
||||||
|
|
||||||
Cross-system interface test: Tests the MCP server's integration with the
|
Cross-system interface test. The astrolabe app (installed by
|
||||||
Astrolabe Nextcloud app, which is installed from the Nextcloud app store via
|
app-hooks/post-installation/20-install-astrolabe-app.sh; source in
|
||||||
app-hooks/post-installation/20-install-astrolabe-app.sh. Astrolabe source
|
./third_party/astrolabe) was refactored to session-minted JWTs — the old
|
||||||
lives in a separate repository (https://github.com/cbcoutinho/astrolabe).
|
per-user OAuth flow and its ``/apps/astrolabe/oauth/disconnect`` route are
|
||||||
|
gone. Background indexing is now an app-password opt-in with a single revoke
|
||||||
|
endpoint.
|
||||||
|
|
||||||
Tests the button functionality on /settings/user/astrolabe:
|
These tests assert the *current* HTTP surface of the settings page:
|
||||||
1. Disable Indexing button (POST to /apps/astrolabe/api/revoke)
|
- the revoke endpoint exists and is auth-gated
|
||||||
2. Disconnect button (POST to /apps/astrolabe/oauth/disconnect)
|
(POST /apps/astrolabe/api/v1/background-sync/credentials/revoke)
|
||||||
|
- the obsolete OAuth disconnect route is gone (404)
|
||||||
These tests verify that:
|
- the personal settings page route resolves
|
||||||
- The endpoints respond correctly to POST requests
|
|
||||||
- CSRF token validation works
|
|
||||||
- User actions are properly handled
|
|
||||||
- Appropriate redirects occur
|
|
||||||
"""
|
"""
|
||||||
|
|
||||||
import httpx
|
import httpx
|
||||||
import pytest
|
import pytest
|
||||||
|
|
||||||
|
pytestmark = pytest.mark.integration
|
||||||
|
|
||||||
@pytest.mark.integration
|
NEXTCLOUD_URL = "http://localhost:8080"
|
||||||
async def test_disable_indexing_button_endpoint_exists():
|
ASTROLABE = f"{NEXTCLOUD_URL}/apps/astrolabe"
|
||||||
"""Test that the Disable Indexing endpoint is accessible."""
|
|
||||||
async with httpx.AsyncClient() as client:
|
|
||||||
# Try without authentication - should return 401 or redirect
|
|
||||||
response = await client.post(
|
|
||||||
"http://localhost:8080/apps/astrolabe/api/revoke",
|
|
||||||
follow_redirects=False,
|
|
||||||
)
|
|
||||||
|
|
||||||
# Should get 401 Unauthorized or 30x redirect
|
# Auth failures (no session) surface as 401 or a login redirect.
|
||||||
assert response.status_code in [401, 301, 302, 303, 307, 308], (
|
_UNAUTH = {401, 302, 303, 307, 308}
|
||||||
f"Expected 401 or redirect without auth, got {response.status_code}"
|
|
||||||
|
|
||||||
|
async def test_revoke_endpoint_requires_auth():
|
||||||
|
"""The background-sync revoke endpoint exists and rejects anonymous calls."""
|
||||||
|
async with httpx.AsyncClient(follow_redirects=False) as client:
|
||||||
|
resp = await client.post(
|
||||||
|
f"{ASTROLABE}/api/v1/background-sync/credentials/revoke",
|
||||||
|
headers={"OCS-APIRequest": "true"},
|
||||||
)
|
)
|
||||||
|
# Must NOT be 404 — the route must exist — and must be auth-gated.
|
||||||
|
assert resp.status_code != 404, "revoke route missing"
|
||||||
|
assert resp.status_code in _UNAUTH, (
|
||||||
|
f"expected auth rejection, got {resp.status_code}"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.integration
|
async def test_obsolete_oauth_disconnect_route_removed():
|
||||||
async def test_disconnect_button_endpoint_exists():
|
"""The pre-refactor OAuth disconnect route must no longer exist.
|
||||||
"""Test that the Disconnect endpoint is accessible."""
|
|
||||||
async with httpx.AsyncClient() as client:
|
|
||||||
# Try without authentication - should return 401 or redirect
|
|
||||||
response = await client.post(
|
|
||||||
"http://localhost:8080/apps/astrolabe/oauth/disconnect",
|
|
||||||
follow_redirects=False,
|
|
||||||
)
|
|
||||||
|
|
||||||
# Should get 401 Unauthorized or 30x redirect
|
Regression guard for the auth refactor: ``/apps/astrolabe/oauth/disconnect``
|
||||||
assert response.status_code in [401, 301, 302, 303, 307, 308], (
|
(and the rest of the OAuth authorize/callback/disconnect surface) was
|
||||||
f"Expected 401 or redirect without auth, got {response.status_code}"
|
removed in favour of session-minted JWTs.
|
||||||
)
|
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.integration
|
|
||||||
async def test_settings_page_renders_buttons():
|
|
||||||
"""Test that the settings page template includes button forms.
|
|
||||||
|
|
||||||
This test verifies that the PHP template renders the form elements.
|
|
||||||
It doesn't require authentication since we're just checking the route exists.
|
|
||||||
"""
|
"""
|
||||||
async with httpx.AsyncClient(follow_redirects=False) as client:
|
async with httpx.AsyncClient(follow_redirects=False) as client:
|
||||||
# Try to access settings page
|
resp = await client.post(f"{ASTROLABE}/oauth/disconnect")
|
||||||
response = await client.get("http://localhost:8080/settings/user/astrolabe")
|
assert resp.status_code == 404, (
|
||||||
|
f"obsolete oauth/disconnect route still resolves ({resp.status_code})"
|
||||||
# Should get 401/redirect if not authenticated (expected)
|
)
|
||||||
# or 200 if user session exists from browser testing
|
|
||||||
assert response.status_code in [200, 401, 302, 303, 307, 308], (
|
|
||||||
f"Unexpected status code: {response.status_code}"
|
|
||||||
)
|
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.integration
|
async def test_settings_page_route_resolves():
|
||||||
@pytest.mark.skip(
|
"""The personal settings page route exists (auth-gated when no session)."""
|
||||||
reason="Requires manual authentication - test with Playwright instead"
|
async with httpx.AsyncClient(follow_redirects=False) as client:
|
||||||
)
|
resp = await client.get(f"{NEXTCLOUD_URL}/settings/user/astrolabe")
|
||||||
async def test_disconnect_button_functionality():
|
assert resp.status_code in ({200} | _UNAUTH), (
|
||||||
"""Test that clicking Disconnect button clears user OAuth tokens.
|
f"unexpected status for settings page: {resp.status_code}"
|
||||||
|
)
|
||||||
NOTE: This test is skipped because programmatic login to Nextcloud is complex.
|
|
||||||
Use Playwright-based tests or manual testing instead.
|
|
||||||
"""
|
|
||||||
pass
|
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.integration
|
|
||||||
@pytest.mark.skip(
|
|
||||||
reason="Requires manual authentication - test with Playwright instead"
|
|
||||||
)
|
|
||||||
async def test_disable_indexing_button_functionality():
|
|
||||||
"""Test that clicking Disable Indexing button revokes background access.
|
|
||||||
|
|
||||||
NOTE: This test is skipped because programmatic login to Nextcloud is complex.
|
|
||||||
Use Playwright-based tests or manual testing instead.
|
|
||||||
"""
|
|
||||||
pass
|
|
||||||
|
|||||||
@@ -1,702 +0,0 @@
|
|||||||
"""Integration tests for Astrolabe token refresh flow.
|
|
||||||
|
|
||||||
Cross-system interface test: Tests the MCP server's integration with the
|
|
||||||
Astrolabe Nextcloud app, which is installed from the Nextcloud app store via
|
|
||||||
app-hooks/post-installation/20-install-astrolabe-app.sh. Astrolabe source
|
|
||||||
lives in a separate repository (https://github.com/cbcoutinho/astrolabe).
|
|
||||||
|
|
||||||
Tests the token refresh mechanism between Astrolabe (Nextcloud app)
|
|
||||||
and the MCP server backend in a multi-user basic auth deployment.
|
|
||||||
|
|
||||||
This test verifies:
|
|
||||||
1. User provisions access via Astrolabe personal settings
|
|
||||||
2. Token is stored encrypted in Nextcloud database
|
|
||||||
3. Token expires (simulated via database manipulation)
|
|
||||||
4. MCP server requests new token via refresh
|
|
||||||
5. Astrolabe refreshes token with IdP
|
|
||||||
6. New token is stored and used successfully
|
|
||||||
|
|
||||||
Note: The mcp-multi-user-basic deployment uses "hybrid mode" which requires
|
|
||||||
BOTH OAuth authorization AND app password for full configuration. These tests
|
|
||||||
focus on the app password/credential storage aspects and verify database state
|
|
||||||
directly rather than relying on UI elements that require both steps.
|
|
||||||
"""
|
|
||||||
|
|
||||||
import logging
|
|
||||||
import re
|
|
||||||
import subprocess
|
|
||||||
|
|
||||||
import anyio
|
|
||||||
import pytest
|
|
||||||
from playwright.async_api import Page
|
|
||||||
|
|
||||||
pytestmark = [pytest.mark.integration, pytest.mark.multi_user_basic]
|
|
||||||
|
|
||||||
logger = logging.getLogger(__name__)
|
|
||||||
|
|
||||||
|
|
||||||
async def login_to_nextcloud(page: Page, username: str, password: str):
|
|
||||||
"""Helper function to login to Nextcloud via Playwright.
|
|
||||||
|
|
||||||
Args:
|
|
||||||
page: Playwright page instance
|
|
||||||
username: Nextcloud username
|
|
||||||
password: Nextcloud password
|
|
||||||
"""
|
|
||||||
nextcloud_url = "http://localhost:8080"
|
|
||||||
|
|
||||||
logger.info("Logging in to Nextcloud as %s...", username)
|
|
||||||
await page.goto(f"{nextcloud_url}/login", wait_until="networkidle")
|
|
||||||
|
|
||||||
# Fill in login form
|
|
||||||
await page.wait_for_selector('input[name="user"]', timeout=10000)
|
|
||||||
await page.fill('input[name="user"]', username)
|
|
||||||
await page.fill('input[name="password"]', password)
|
|
||||||
|
|
||||||
# Submit form
|
|
||||||
await page.click('button[type="submit"]')
|
|
||||||
await page.wait_for_load_state("networkidle", timeout=30000)
|
|
||||||
|
|
||||||
# Verify logged in (should redirect away from login page)
|
|
||||||
current_url = page.url
|
|
||||||
assert "/login" not in current_url, (
|
|
||||||
f"Login failed for {username}, still on login page"
|
|
||||||
)
|
|
||||||
logger.info("✓ Successfully logged in as %s", username)
|
|
||||||
|
|
||||||
|
|
||||||
async def generate_app_password(
|
|
||||||
page: Page, username: str, app_name: str = "Astrolabe Test"
|
|
||||||
) -> str:
|
|
||||||
"""Generate an app password in Nextcloud Security settings.
|
|
||||||
|
|
||||||
Args:
|
|
||||||
page: Playwright page instance (must be authenticated)
|
|
||||||
username: Username (for logging)
|
|
||||||
app_name: Name for the app password
|
|
||||||
|
|
||||||
Returns:
|
|
||||||
The generated app password string
|
|
||||||
"""
|
|
||||||
logger.info("Generating app password for %s...", username)
|
|
||||||
|
|
||||||
nextcloud_url = "http://localhost:8080"
|
|
||||||
|
|
||||||
# Navigate to Security settings
|
|
||||||
await page.goto(f"{nextcloud_url}/settings/user/security", wait_until="networkidle")
|
|
||||||
logger.info("Navigated to Security settings")
|
|
||||||
|
|
||||||
# Fill the app password input field
|
|
||||||
app_password_input = page.locator('input[placeholder="App name"]')
|
|
||||||
await app_password_input.fill(app_name)
|
|
||||||
logger.info("Entered app name: %s", app_name)
|
|
||||||
|
|
||||||
# Wait for Vue.js to react and enable the button
|
|
||||||
await anyio.sleep(1.0)
|
|
||||||
|
|
||||||
# Click the create button
|
|
||||||
create_button = page.locator(
|
|
||||||
'button[type="submit"]:has-text("Create new app password")'
|
|
||||||
)
|
|
||||||
await create_button.click()
|
|
||||||
logger.info("Clicked create app password button")
|
|
||||||
|
|
||||||
# Wait for app password to be generated
|
|
||||||
await anyio.sleep(3)
|
|
||||||
|
|
||||||
# Find the generated app password
|
|
||||||
app_password = None
|
|
||||||
try:
|
|
||||||
await page.wait_for_selector('text="New app password"', timeout=10000)
|
|
||||||
logger.info("App password dialog appeared")
|
|
||||||
|
|
||||||
all_inputs = await page.locator('input[type="text"]').all()
|
|
||||||
for idx, input_elem in enumerate(all_inputs):
|
|
||||||
try:
|
|
||||||
value = await input_elem.input_value()
|
|
||||||
if value and "-" in value and len(value) > 20:
|
|
||||||
app_password = value.strip()
|
|
||||||
logger.info("Found app password in input %s", idx)
|
|
||||||
break
|
|
||||||
except Exception:
|
|
||||||
continue
|
|
||||||
except Exception as e:
|
|
||||||
logger.error("Failed to find app password dialog: %s", e)
|
|
||||||
|
|
||||||
if not app_password:
|
|
||||||
screenshot_path = f"/tmp/app_password_generation_{username}.png"
|
|
||||||
await page.screenshot(path=screenshot_path)
|
|
||||||
raise ValueError(
|
|
||||||
f"Could not find generated app password. Screenshot: {screenshot_path}"
|
|
||||||
)
|
|
||||||
|
|
||||||
# Validate password format
|
|
||||||
if not re.match(
|
|
||||||
r"^[a-zA-Z0-9]{5}-[a-zA-Z0-9]{5}-[a-zA-Z0-9]{5}-[a-zA-Z0-9]{5}-[a-zA-Z0-9]{5}$",
|
|
||||||
app_password,
|
|
||||||
):
|
|
||||||
raise ValueError(f"App password format validation failed: {app_password}")
|
|
||||||
|
|
||||||
logger.info("✓ Generated app password for %s", username)
|
|
||||||
|
|
||||||
# Close the dialog
|
|
||||||
close_button = page.get_by_role("button", name="Close")
|
|
||||||
await close_button.click()
|
|
||||||
await anyio.sleep(0.5)
|
|
||||||
|
|
||||||
return app_password
|
|
||||||
|
|
||||||
|
|
||||||
async def save_app_password_in_astrolabe(
|
|
||||||
page: Page, username: str, app_password: str
|
|
||||||
) -> bool:
|
|
||||||
"""Save app password in Astrolabe settings (Step 2 of hybrid mode).
|
|
||||||
|
|
||||||
This function only saves the app password - it does NOT verify the "Active"
|
|
||||||
badge since that requires both OAuth and app password in hybrid mode.
|
|
||||||
|
|
||||||
Args:
|
|
||||||
page: Playwright page instance
|
|
||||||
username: Username (for logging)
|
|
||||||
app_password: App password to enter
|
|
||||||
|
|
||||||
Returns:
|
|
||||||
True if the password was saved successfully (based on network response)
|
|
||||||
"""
|
|
||||||
logger.info("Saving app password in Astrolabe for %s...", username)
|
|
||||||
|
|
||||||
nextcloud_url = "http://localhost:8080"
|
|
||||||
|
|
||||||
# Track network responses
|
|
||||||
credentials_response_status = None
|
|
||||||
|
|
||||||
def capture_response(resp):
|
|
||||||
nonlocal credentials_response_status
|
|
||||||
if "background-sync/credentials" in resp.url or "storeAppPassword" in resp.url:
|
|
||||||
credentials_response_status = resp.status
|
|
||||||
logger.info("Credentials endpoint response: %s %s", resp.status, resp.url)
|
|
||||||
|
|
||||||
page.on("response", capture_response)
|
|
||||||
|
|
||||||
# Navigate to Astrolabe settings
|
|
||||||
await page.goto(
|
|
||||||
f"{nextcloud_url}/settings/user/astrolabe", wait_until="networkidle"
|
|
||||||
)
|
|
||||||
await anyio.sleep(1)
|
|
||||||
|
|
||||||
# Check if Step 2 already shows "Complete"
|
|
||||||
try:
|
|
||||||
complete_badge = page.locator('text="Complete"').first
|
|
||||||
if await complete_badge.is_visible(timeout=2000):
|
|
||||||
logger.info("✓ App password already configured for %s", username)
|
|
||||||
return True
|
|
||||||
except Exception:
|
|
||||||
pass
|
|
||||||
|
|
||||||
# Find the app password input field
|
|
||||||
app_password_input = page.get_by_placeholder("xxxxx-xxxxx-xxxxx-xxxxx-xxxxx")
|
|
||||||
|
|
||||||
try:
|
|
||||||
await app_password_input.wait_for(timeout=5000, state="visible")
|
|
||||||
logger.info("Found app password input field")
|
|
||||||
except Exception:
|
|
||||||
screenshot_path = f"/tmp/astrolabe_no_password_field_{username}.png"
|
|
||||||
await page.screenshot(path=screenshot_path)
|
|
||||||
raise ValueError(
|
|
||||||
f"Could not find app password input field. Screenshot: {screenshot_path}"
|
|
||||||
)
|
|
||||||
|
|
||||||
# Enter the app password
|
|
||||||
await app_password_input.fill(app_password)
|
|
||||||
logger.info("Entered app password for %s", username)
|
|
||||||
|
|
||||||
await anyio.sleep(0.5)
|
|
||||||
|
|
||||||
# Click Save button
|
|
||||||
save_button = page.get_by_role("button", name="Save")
|
|
||||||
await save_button.click()
|
|
||||||
logger.info("Clicked Save button")
|
|
||||||
|
|
||||||
# Wait for the request to complete and page to reload
|
|
||||||
await page.wait_for_load_state("networkidle", timeout=15000)
|
|
||||||
await anyio.sleep(2)
|
|
||||||
|
|
||||||
# Verify the save was successful by checking network response
|
|
||||||
if credentials_response_status == 200:
|
|
||||||
logger.info("✓ App password saved successfully for %s", username)
|
|
||||||
return True
|
|
||||||
else:
|
|
||||||
logger.error(
|
|
||||||
"App password save failed for %s, status: %s",
|
|
||||||
username,
|
|
||||||
credentials_response_status,
|
|
||||||
)
|
|
||||||
screenshot_path = f"/tmp/astrolabe_save_failed_{username}.png"
|
|
||||||
await page.screenshot(path=screenshot_path)
|
|
||||||
return False
|
|
||||||
|
|
||||||
|
|
||||||
def get_background_sync_credentials(username: str) -> dict | None:
|
|
||||||
"""Get background sync credentials for a user from the database.
|
|
||||||
|
|
||||||
Args:
|
|
||||||
username: Nextcloud username
|
|
||||||
|
|
||||||
Returns:
|
|
||||||
Dict with credential details, or None if not found
|
|
||||||
"""
|
|
||||||
query = f"""
|
|
||||||
SELECT configkey, configvalue
|
|
||||||
FROM oc_preferences
|
|
||||||
WHERE userid = '{username}'
|
|
||||||
AND appid = 'astrolabe'
|
|
||||||
AND configkey IN ('background_sync_password', 'background_sync_type', 'background_sync_provisioned_at')
|
|
||||||
ORDER BY configkey;
|
|
||||||
"""
|
|
||||||
|
|
||||||
try:
|
|
||||||
result = subprocess.run(
|
|
||||||
[
|
|
||||||
"docker",
|
|
||||||
"compose",
|
|
||||||
"exec",
|
|
||||||
"-T",
|
|
||||||
"db",
|
|
||||||
"mariadb",
|
|
||||||
"-u",
|
|
||||||
"root",
|
|
||||||
"-ppassword",
|
|
||||||
"nextcloud",
|
|
||||||
"-e",
|
|
||||||
query,
|
|
||||||
],
|
|
||||||
capture_output=True,
|
|
||||||
text=True,
|
|
||||||
timeout=10,
|
|
||||||
)
|
|
||||||
|
|
||||||
output = result.stdout
|
|
||||||
if "background_sync_type" in output:
|
|
||||||
return {
|
|
||||||
"has_password": "background_sync_password" in output,
|
|
||||||
"has_type": "background_sync_type" in output,
|
|
||||||
"has_timestamp": "background_sync_provisioned_at" in output,
|
|
||||||
"is_app_password": "app_password" in output,
|
|
||||||
}
|
|
||||||
return None
|
|
||||||
|
|
||||||
except Exception as e:
|
|
||||||
logger.error("Error getting credentials for %s: %s", username, e)
|
|
||||||
return None
|
|
||||||
|
|
||||||
|
|
||||||
def delete_user_credentials(username: str) -> bool:
|
|
||||||
"""Delete all stored credentials for a user (for cleanup).
|
|
||||||
|
|
||||||
Args:
|
|
||||||
username: Nextcloud username
|
|
||||||
|
|
||||||
Returns:
|
|
||||||
True if successful
|
|
||||||
"""
|
|
||||||
query = f"""
|
|
||||||
DELETE FROM oc_preferences
|
|
||||||
WHERE userid = '{username}'
|
|
||||||
AND appid = 'astrolabe'
|
|
||||||
AND configkey IN ('oauth_tokens', 'background_sync_password', 'background_sync_type', 'background_sync_provisioned_at');
|
|
||||||
"""
|
|
||||||
|
|
||||||
try:
|
|
||||||
result = subprocess.run(
|
|
||||||
[
|
|
||||||
"docker",
|
|
||||||
"compose",
|
|
||||||
"exec",
|
|
||||||
"-T",
|
|
||||||
"db",
|
|
||||||
"mariadb",
|
|
||||||
"-u",
|
|
||||||
"root",
|
|
||||||
"-ppassword",
|
|
||||||
"nextcloud",
|
|
||||||
"-e",
|
|
||||||
query,
|
|
||||||
],
|
|
||||||
capture_output=True,
|
|
||||||
text=True,
|
|
||||||
timeout=10,
|
|
||||||
)
|
|
||||||
|
|
||||||
logger.info("Deleted credentials for %s", username)
|
|
||||||
return result.returncode == 0
|
|
||||||
|
|
||||||
except Exception as e:
|
|
||||||
logger.error("Error deleting credentials for %s: %s", username, e)
|
|
||||||
return False
|
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.integration
|
|
||||||
@pytest.mark.multi_user_basic
|
|
||||||
async def test_app_password_storage_and_cleanup(
|
|
||||||
browser,
|
|
||||||
nc_client,
|
|
||||||
test_users_setup,
|
|
||||||
configure_astrolabe_for_mcp_server,
|
|
||||||
):
|
|
||||||
"""Test that app passwords are stored and cleaned up correctly.
|
|
||||||
|
|
||||||
This test verifies:
|
|
||||||
1. User can save app password in Astrolabe settings
|
|
||||||
2. Password is stored encrypted in the database
|
|
||||||
3. Credentials can be revoked and are deleted from database
|
|
||||||
|
|
||||||
Note: In hybrid mode (mcp-multi-user-basic), this only tests Step 2
|
|
||||||
(app password storage). The "Active" badge requires both OAuth and
|
|
||||||
app password, which is tested separately.
|
|
||||||
"""
|
|
||||||
# Configure Astrolabe for mcp-multi-user-basic
|
|
||||||
logger.info("Configuring Astrolabe for mcp-multi-user-basic server...")
|
|
||||||
await configure_astrolabe_for_mcp_server(
|
|
||||||
mcp_server_internal_url="http://mcp-multi-user-basic:8000",
|
|
||||||
mcp_server_public_url="http://localhost:8003",
|
|
||||||
)
|
|
||||||
|
|
||||||
username = "alice"
|
|
||||||
user_config = test_users_setup[username]
|
|
||||||
password = user_config["password"]
|
|
||||||
|
|
||||||
# Cleanup any existing credentials
|
|
||||||
delete_user_credentials(username)
|
|
||||||
|
|
||||||
context = await browser.new_context(ignore_https_errors=True)
|
|
||||||
page = await context.new_page()
|
|
||||||
|
|
||||||
try:
|
|
||||||
# Step 1: Login
|
|
||||||
await login_to_nextcloud(page, username, password)
|
|
||||||
|
|
||||||
# Step 2: Verify no credentials exist initially
|
|
||||||
initial_creds = get_background_sync_credentials(username)
|
|
||||||
assert initial_creds is None, f"Expected no credentials, found: {initial_creds}"
|
|
||||||
logger.info("✓ Verified no initial credentials")
|
|
||||||
|
|
||||||
# Step 3: Generate app password
|
|
||||||
app_password = await generate_app_password(page, username)
|
|
||||||
assert app_password, "Failed to generate app password"
|
|
||||||
|
|
||||||
# Step 4: Save app password in Astrolabe
|
|
||||||
save_success = await save_app_password_in_astrolabe(
|
|
||||||
page, username, app_password
|
|
||||||
)
|
|
||||||
assert save_success, "Failed to save app password"
|
|
||||||
|
|
||||||
# Step 5: Verify credentials are stored in database
|
|
||||||
stored_creds = get_background_sync_credentials(username)
|
|
||||||
assert stored_creds is not None, "Expected credentials to be stored"
|
|
||||||
assert stored_creds["has_password"], "Expected password to be stored"
|
|
||||||
assert stored_creds["has_type"], "Expected type to be stored"
|
|
||||||
assert stored_creds["is_app_password"], "Expected type to be 'app_password'"
|
|
||||||
logger.info("✓ Verified credentials stored in database")
|
|
||||||
|
|
||||||
# Step 6: Verify password is encrypted (not plaintext)
|
|
||||||
query = f"""
|
|
||||||
SELECT configvalue
|
|
||||||
FROM oc_preferences
|
|
||||||
WHERE userid = '{username}'
|
|
||||||
AND appid = 'astrolabe'
|
|
||||||
AND configkey = 'background_sync_password';
|
|
||||||
"""
|
|
||||||
|
|
||||||
result = subprocess.run(
|
|
||||||
[
|
|
||||||
"docker",
|
|
||||||
"compose",
|
|
||||||
"exec",
|
|
||||||
"-T",
|
|
||||||
"db",
|
|
||||||
"mariadb",
|
|
||||||
"-u",
|
|
||||||
"root",
|
|
||||||
"-ppassword",
|
|
||||||
"nextcloud",
|
|
||||||
"-N",
|
|
||||||
"-e",
|
|
||||||
query,
|
|
||||||
],
|
|
||||||
capture_output=True,
|
|
||||||
text=True,
|
|
||||||
timeout=10,
|
|
||||||
)
|
|
||||||
|
|
||||||
encrypted_value = result.stdout.strip()
|
|
||||||
assert app_password not in encrypted_value, "Password appears in plaintext!"
|
|
||||||
assert len(encrypted_value) > len(app_password), (
|
|
||||||
"Encrypted value should be longer"
|
|
||||||
)
|
|
||||||
logger.info("✓ Verified password is encrypted")
|
|
||||||
|
|
||||||
finally:
|
|
||||||
await context.close()
|
|
||||||
# Cleanup
|
|
||||||
delete_user_credentials(username)
|
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.integration
|
|
||||||
@pytest.mark.multi_user_basic
|
|
||||||
async def test_credential_isolation_between_users(
|
|
||||||
browser,
|
|
||||||
nc_client,
|
|
||||||
test_users_setup,
|
|
||||||
configure_astrolabe_for_mcp_server,
|
|
||||||
):
|
|
||||||
"""Test that credentials are properly isolated between users.
|
|
||||||
|
|
||||||
This test verifies:
|
|
||||||
1. Multiple users can provision credentials independently
|
|
||||||
2. Each user's encrypted credentials are unique
|
|
||||||
3. Deleting one user's credentials doesn't affect others
|
|
||||||
"""
|
|
||||||
await configure_astrolabe_for_mcp_server(
|
|
||||||
mcp_server_internal_url="http://mcp-multi-user-basic:8000",
|
|
||||||
mcp_server_public_url="http://localhost:8003",
|
|
||||||
)
|
|
||||||
|
|
||||||
test_users = ["alice", "bob"]
|
|
||||||
user_passwords = {}
|
|
||||||
|
|
||||||
# Cleanup all users first
|
|
||||||
for username in test_users:
|
|
||||||
delete_user_credentials(username)
|
|
||||||
|
|
||||||
# Provision each user
|
|
||||||
for username in test_users:
|
|
||||||
user_config = test_users_setup[username]
|
|
||||||
password = user_config["password"]
|
|
||||||
|
|
||||||
context = await browser.new_context(ignore_https_errors=True)
|
|
||||||
page = await context.new_page()
|
|
||||||
|
|
||||||
try:
|
|
||||||
await login_to_nextcloud(page, username, password)
|
|
||||||
app_password = await generate_app_password(
|
|
||||||
page, username, f"Test {username}"
|
|
||||||
)
|
|
||||||
save_success = await save_app_password_in_astrolabe(
|
|
||||||
page, username, app_password
|
|
||||||
)
|
|
||||||
|
|
||||||
assert save_success, f"Failed to save app password for {username}"
|
|
||||||
user_passwords[username] = app_password
|
|
||||||
|
|
||||||
# Verify stored
|
|
||||||
creds = get_background_sync_credentials(username)
|
|
||||||
assert creds is not None, f"Credentials not stored for {username}"
|
|
||||||
logger.info("✓ Credentials provisioned for %s", username)
|
|
||||||
|
|
||||||
finally:
|
|
||||||
await context.close()
|
|
||||||
|
|
||||||
# Verify isolation - get encrypted values
|
|
||||||
encrypted_values = {}
|
|
||||||
for username in test_users:
|
|
||||||
query = f"""
|
|
||||||
SELECT configvalue
|
|
||||||
FROM oc_preferences
|
|
||||||
WHERE userid = '{username}'
|
|
||||||
AND appid = 'astrolabe'
|
|
||||||
AND configkey = 'background_sync_password';
|
|
||||||
"""
|
|
||||||
|
|
||||||
result = subprocess.run(
|
|
||||||
[
|
|
||||||
"docker",
|
|
||||||
"compose",
|
|
||||||
"exec",
|
|
||||||
"-T",
|
|
||||||
"db",
|
|
||||||
"mariadb",
|
|
||||||
"-u",
|
|
||||||
"root",
|
|
||||||
"-ppassword",
|
|
||||||
"nextcloud",
|
|
||||||
"-N",
|
|
||||||
"-e",
|
|
||||||
query,
|
|
||||||
],
|
|
||||||
capture_output=True,
|
|
||||||
text=True,
|
|
||||||
timeout=10,
|
|
||||||
)
|
|
||||||
encrypted_values[username] = result.stdout.strip()
|
|
||||||
|
|
||||||
# Different users should have different encrypted values
|
|
||||||
assert encrypted_values["alice"] != encrypted_values["bob"], (
|
|
||||||
"Different users should have different encrypted values"
|
|
||||||
)
|
|
||||||
logger.info("✓ Verified credentials are unique per user")
|
|
||||||
|
|
||||||
# Delete alice's credentials and verify bob's are unaffected
|
|
||||||
delete_user_credentials("alice")
|
|
||||||
|
|
||||||
alice_creds = get_background_sync_credentials("alice")
|
|
||||||
bob_creds = get_background_sync_credentials("bob")
|
|
||||||
|
|
||||||
assert alice_creds is None, "Alice's credentials should be deleted"
|
|
||||||
assert bob_creds is not None, "Bob's credentials should still exist"
|
|
||||||
logger.info("✓ Verified credential deletion is isolated")
|
|
||||||
|
|
||||||
# Cleanup
|
|
||||||
for username in test_users:
|
|
||||||
delete_user_credentials(username)
|
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.integration
|
|
||||||
@pytest.mark.multi_user_basic
|
|
||||||
async def test_credential_revoke_and_reprovision(
|
|
||||||
browser,
|
|
||||||
nc_client,
|
|
||||||
test_users_setup,
|
|
||||||
configure_astrolabe_for_mcp_server,
|
|
||||||
):
|
|
||||||
"""Test that credentials can be revoked and reprovisioned.
|
|
||||||
|
|
||||||
This test verifies:
|
|
||||||
1. User provisions credentials
|
|
||||||
2. User revokes credentials (deletes from database)
|
|
||||||
3. User provisions again with new app password
|
|
||||||
4. New credentials are stored correctly
|
|
||||||
|
|
||||||
Note: The UI prevents overwriting credentials directly - users must
|
|
||||||
revoke first before provisioning new credentials.
|
|
||||||
"""
|
|
||||||
await configure_astrolabe_for_mcp_server(
|
|
||||||
mcp_server_internal_url="http://mcp-multi-user-basic:8000",
|
|
||||||
mcp_server_public_url="http://localhost:8003",
|
|
||||||
)
|
|
||||||
|
|
||||||
username = "alice"
|
|
||||||
user_config = test_users_setup[username]
|
|
||||||
password = user_config["password"]
|
|
||||||
|
|
||||||
delete_user_credentials(username)
|
|
||||||
|
|
||||||
context = await browser.new_context(ignore_https_errors=True)
|
|
||||||
page = await context.new_page()
|
|
||||||
|
|
||||||
try:
|
|
||||||
await login_to_nextcloud(page, username, password)
|
|
||||||
|
|
||||||
# First provisioning
|
|
||||||
app_password_1 = await generate_app_password(page, username, "First Password")
|
|
||||||
await save_app_password_in_astrolabe(page, username, app_password_1)
|
|
||||||
|
|
||||||
# Get first encrypted value
|
|
||||||
query = f"""
|
|
||||||
SELECT configvalue
|
|
||||||
FROM oc_preferences
|
|
||||||
WHERE userid = '{username}'
|
|
||||||
AND appid = 'astrolabe'
|
|
||||||
AND configkey = 'background_sync_password';
|
|
||||||
"""
|
|
||||||
|
|
||||||
result1 = subprocess.run(
|
|
||||||
[
|
|
||||||
"docker",
|
|
||||||
"compose",
|
|
||||||
"exec",
|
|
||||||
"-T",
|
|
||||||
"db",
|
|
||||||
"mariadb",
|
|
||||||
"-u",
|
|
||||||
"root",
|
|
||||||
"-ppassword",
|
|
||||||
"nextcloud",
|
|
||||||
"-N",
|
|
||||||
"-e",
|
|
||||||
query,
|
|
||||||
],
|
|
||||||
capture_output=True,
|
|
||||||
text=True,
|
|
||||||
timeout=10,
|
|
||||||
)
|
|
||||||
first_encrypted = result1.stdout.strip()
|
|
||||||
assert first_encrypted, "First credential should be stored"
|
|
||||||
logger.info("✓ First credential stored")
|
|
||||||
|
|
||||||
# Revoke credentials (simulating user clicking "Revoke Access")
|
|
||||||
delete_user_credentials(username)
|
|
||||||
logger.info("✓ Credentials revoked")
|
|
||||||
|
|
||||||
# Verify credentials are gone
|
|
||||||
creds_after_revoke = get_background_sync_credentials(username)
|
|
||||||
assert creds_after_revoke is None, "Credentials should be deleted after revoke"
|
|
||||||
|
|
||||||
# Second provisioning with different password
|
|
||||||
app_password_2 = await generate_app_password(page, username, "Second Password")
|
|
||||||
await save_app_password_in_astrolabe(page, username, app_password_2)
|
|
||||||
|
|
||||||
result2 = subprocess.run(
|
|
||||||
[
|
|
||||||
"docker",
|
|
||||||
"compose",
|
|
||||||
"exec",
|
|
||||||
"-T",
|
|
||||||
"db",
|
|
||||||
"mariadb",
|
|
||||||
"-u",
|
|
||||||
"root",
|
|
||||||
"-ppassword",
|
|
||||||
"nextcloud",
|
|
||||||
"-N",
|
|
||||||
"-e",
|
|
||||||
query,
|
|
||||||
],
|
|
||||||
capture_output=True,
|
|
||||||
text=True,
|
|
||||||
timeout=10,
|
|
||||||
)
|
|
||||||
second_encrypted = result2.stdout.strip()
|
|
||||||
assert second_encrypted, "Second credential should be stored"
|
|
||||||
logger.info("✓ Second credential stored")
|
|
||||||
|
|
||||||
# Verify the encrypted values are different (different passwords)
|
|
||||||
assert first_encrypted != second_encrypted, (
|
|
||||||
"Different passwords should produce different encrypted values"
|
|
||||||
)
|
|
||||||
|
|
||||||
# Verify only one row exists
|
|
||||||
count_query = f"""
|
|
||||||
SELECT COUNT(*)
|
|
||||||
FROM oc_preferences
|
|
||||||
WHERE userid = '{username}'
|
|
||||||
AND appid = 'astrolabe'
|
|
||||||
AND configkey = 'background_sync_password';
|
|
||||||
"""
|
|
||||||
|
|
||||||
count_result = subprocess.run(
|
|
||||||
[
|
|
||||||
"docker",
|
|
||||||
"compose",
|
|
||||||
"exec",
|
|
||||||
"-T",
|
|
||||||
"db",
|
|
||||||
"mariadb",
|
|
||||||
"-u",
|
|
||||||
"root",
|
|
||||||
"-ppassword",
|
|
||||||
"nextcloud",
|
|
||||||
"-N",
|
|
||||||
"-e",
|
|
||||||
count_query,
|
|
||||||
],
|
|
||||||
capture_output=True,
|
|
||||||
text=True,
|
|
||||||
timeout=10,
|
|
||||||
)
|
|
||||||
count = int(count_result.stdout.strip())
|
|
||||||
assert count == 1, f"Expected 1 credential row, found {count}"
|
|
||||||
logger.info("✓ Verified clean reprovision after revoke")
|
|
||||||
|
|
||||||
finally:
|
|
||||||
await context.close()
|
|
||||||
delete_user_credentials(username)
|
|
||||||
@@ -23,10 +23,11 @@ behaviour separately.
|
|||||||
"""
|
"""
|
||||||
|
|
||||||
import logging
|
import logging
|
||||||
|
import os
|
||||||
import uuid
|
import uuid
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
from httpx import HTTPStatusError
|
from httpx import BasicAuth, HTTPStatusError
|
||||||
|
|
||||||
from nextcloud_mcp_server.client import NextcloudClient
|
from nextcloud_mcp_server.client import NextcloudClient
|
||||||
from nextcloud_mcp_server.search import verification
|
from nextcloud_mcp_server.search import verification
|
||||||
@@ -48,6 +49,28 @@ def _result_for_note(note_id: int) -> SearchResult:
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _result_for_file(file_id: int, path: str) -> SearchResult:
|
||||||
|
# Mirrors what the algorithm layer propagates: doc_id IS the global file id,
|
||||||
|
# ``path`` is carried in metadata (owner-relative) for log context only.
|
||||||
|
return SearchResult(
|
||||||
|
id=file_id,
|
||||||
|
doc_type="file",
|
||||||
|
title=path.split("/")[-1],
|
||||||
|
excerpt="...",
|
||||||
|
score=0.9,
|
||||||
|
metadata={"path": path},
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _user_client(username: str, password: str) -> NextcloudClient:
|
||||||
|
return NextcloudClient(
|
||||||
|
base_url=os.environ["NEXTCLOUD_HOST"],
|
||||||
|
username=username,
|
||||||
|
auth=BasicAuth(username, password),
|
||||||
|
password=password,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
async def test_verify_keeps_accessible_note(
|
async def test_verify_keeps_accessible_note(
|
||||||
nc_client: NextcloudClient, temporary_note: dict, mocker
|
nc_client: NextcloudClient, temporary_note: dict, mocker
|
||||||
):
|
):
|
||||||
@@ -170,3 +193,95 @@ async def test_verify_dedupes_chunks_of_same_document(
|
|||||||
assert dropped_count == 0
|
assert dropped_count == 0
|
||||||
# ...but verification only fetched the note ONCE
|
# ...but verification only fetched the note ONCE
|
||||||
assert spy_get_note.await_count == 1
|
assert spy_get_note.await_count == 1
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# File verifier — cross-user shared access (ACL-aware search, PR #813)
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
#
|
||||||
|
# These exercise the verifier fix that makes ACL-aware search actually work
|
||||||
|
# end-to-end: a file an owner shared with another user must survive
|
||||||
|
# verify-on-read for the *recipient*, even when it lives in a subfolder of the
|
||||||
|
# owner's tree (Nextcloud mounts received shares at the recipient's root by
|
||||||
|
# basename, so the owner-relative path does NOT resolve under the recipient's
|
||||||
|
# root). The fix verifies by global file id, which is ACL-aware.
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
async def alice_bob_clients(test_users_setup):
|
||||||
|
"""Direct NextcloudClients for alice (owner) and bob (recipient)."""
|
||||||
|
alice = _user_client("alice", test_users_setup["alice"]["password"])
|
||||||
|
bob = _user_client("bob", test_users_setup["bob"]["password"])
|
||||||
|
try:
|
||||||
|
yield alice, bob
|
||||||
|
finally:
|
||||||
|
await alice._client.aclose()
|
||||||
|
await bob._client.aclose()
|
||||||
|
|
||||||
|
|
||||||
|
async def test_verify_keeps_nested_file_shared_with_recipient(
|
||||||
|
alice_bob_clients, mocker
|
||||||
|
):
|
||||||
|
"""The PR #813 acceptance check at the verifier layer.
|
||||||
|
|
||||||
|
Alice owns a file in a *subfolder* and shares it with Bob. Verifying the
|
||||||
|
result as Bob must KEEP it — proving the id-based check sees the share.
|
||||||
|
A path-based check (the old behaviour) would 404 here and wrongly drop it.
|
||||||
|
"""
|
||||||
|
spy_evict = mocker.AsyncMock()
|
||||||
|
mocker.patch.object(verification, "delete_document_points", spy_evict)
|
||||||
|
|
||||||
|
alice, bob = alice_bob_clients
|
||||||
|
suffix = uuid.uuid4().hex[:8]
|
||||||
|
test_dir = f"acl_verify_{suffix}"
|
||||||
|
nested_dir = f"{test_dir}/reports"
|
||||||
|
shared_path = f"{nested_dir}/shared.txt"
|
||||||
|
|
||||||
|
await alice.webdav.create_directory(test_dir)
|
||||||
|
await alice.webdav.create_directory(nested_dir)
|
||||||
|
await alice.webdav.write_file(shared_path, b"alice's shared report", "text/plain")
|
||||||
|
file_id = (await alice.webdav.get_file_info(shared_path))["id"]
|
||||||
|
|
||||||
|
await alice.sharing.create_share(
|
||||||
|
path=f"/{shared_path}", share_with="bob", share_type=0, permissions=1
|
||||||
|
)
|
||||||
|
|
||||||
|
try:
|
||||||
|
kept, dropped_count = await verify_search_results(
|
||||||
|
bob, [_result_for_file(file_id, shared_path)]
|
||||||
|
)
|
||||||
|
|
||||||
|
assert [r.id for r in kept] == [file_id], (
|
||||||
|
"a nested file shared with bob must pass verification for bob"
|
||||||
|
)
|
||||||
|
assert dropped_count == 0
|
||||||
|
spy_evict.assert_not_awaited()
|
||||||
|
finally:
|
||||||
|
await alice.webdav.delete_resource(test_dir)
|
||||||
|
|
||||||
|
|
||||||
|
async def test_verify_drops_unshared_file_for_other_user(alice_bob_clients, mocker):
|
||||||
|
"""Negative control: a file Alice did NOT share is inaccessible to Bob and
|
||||||
|
must be dropped + scheduled for eviction under his identity."""
|
||||||
|
spy_evict = mocker.AsyncMock()
|
||||||
|
mocker.patch.object(verification, "delete_document_points", spy_evict)
|
||||||
|
|
||||||
|
alice, bob = alice_bob_clients
|
||||||
|
suffix = uuid.uuid4().hex[:8]
|
||||||
|
test_dir = f"acl_verify_priv_{suffix}"
|
||||||
|
private_path = f"{test_dir}/private.txt"
|
||||||
|
|
||||||
|
await alice.webdav.create_directory(test_dir)
|
||||||
|
await alice.webdav.write_file(private_path, b"alice's private note", "text/plain")
|
||||||
|
file_id = (await alice.webdav.get_file_info(private_path))["id"]
|
||||||
|
|
||||||
|
try:
|
||||||
|
kept, dropped_count = await verify_search_results(
|
||||||
|
bob, [_result_for_file(file_id, private_path)]
|
||||||
|
)
|
||||||
|
|
||||||
|
assert kept == [], "an unshared file must not pass verification for bob"
|
||||||
|
assert dropped_count == 1
|
||||||
|
spy_evict.assert_awaited_once_with(file_id, "file", bob.username)
|
||||||
|
finally:
|
||||||
|
await alice.webdav.delete_resource(test_dir)
|
||||||
|
|||||||
@@ -0,0 +1,188 @@
|
|||||||
|
"""Tests for nextcloud_mcp_server.search.access_filter."""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
from unittest.mock import AsyncMock
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
from nextcloud_mcp_server.search import access_filter
|
||||||
|
from nextcloud_mcp_server.search.access_filter import (
|
||||||
|
build_ownership_filter,
|
||||||
|
clear_accessible_owners_cache,
|
||||||
|
list_accessible_owners,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture(autouse=True)
|
||||||
|
def _reset_owners_cache():
|
||||||
|
"""The accessible-owners cache is process-global; reset it around each test
|
||||||
|
so the shared "alice" user_id can't leak cached results between tests."""
|
||||||
|
clear_accessible_owners_cache()
|
||||||
|
yield
|
||||||
|
clear_accessible_owners_cache()
|
||||||
|
|
||||||
|
|
||||||
|
class TestListAccessibleOwners:
|
||||||
|
@pytest.mark.unit
|
||||||
|
async def test_includes_self_even_with_no_shares(self) -> None:
|
||||||
|
sharing = AsyncMock()
|
||||||
|
sharing.list_shares.return_value = []
|
||||||
|
|
||||||
|
owners = await list_accessible_owners(sharing, "alice")
|
||||||
|
assert owners == ["alice"]
|
||||||
|
|
||||||
|
@pytest.mark.unit
|
||||||
|
async def test_collects_uid_owner_from_shares(self) -> None:
|
||||||
|
sharing = AsyncMock()
|
||||||
|
sharing.list_shares.return_value = [
|
||||||
|
{"uid_owner": "bob", "share_with": "alice"},
|
||||||
|
{"uid_owner": "carol", "share_with": "alice"},
|
||||||
|
]
|
||||||
|
|
||||||
|
owners = await list_accessible_owners(sharing, "alice")
|
||||||
|
assert set(owners) == {"alice", "bob", "carol"}
|
||||||
|
|
||||||
|
@pytest.mark.unit
|
||||||
|
async def test_deduplicates_repeated_owners(self) -> None:
|
||||||
|
sharing = AsyncMock()
|
||||||
|
sharing.list_shares.return_value = [
|
||||||
|
{"uid_owner": "bob"},
|
||||||
|
{"uid_owner": "bob"}, # same owner shares many files
|
||||||
|
{"uid_owner": "bob"},
|
||||||
|
]
|
||||||
|
|
||||||
|
owners = await list_accessible_owners(sharing, "alice")
|
||||||
|
assert sorted(owners) == ["alice", "bob"]
|
||||||
|
|
||||||
|
@pytest.mark.unit
|
||||||
|
async def test_falls_back_to_owner_field_when_uid_owner_missing(self) -> None:
|
||||||
|
# Some Nextcloud versions surface `owner` instead of `uid_owner`
|
||||||
|
# on the shared-with-me response.
|
||||||
|
sharing = AsyncMock()
|
||||||
|
sharing.list_shares.return_value = [{"owner": "bob"}]
|
||||||
|
|
||||||
|
owners = await list_accessible_owners(sharing, "alice")
|
||||||
|
assert sorted(owners) == ["alice", "bob"]
|
||||||
|
|
||||||
|
@pytest.mark.unit
|
||||||
|
async def test_ignores_share_with_no_owner_field(self) -> None:
|
||||||
|
sharing = AsyncMock()
|
||||||
|
sharing.list_shares.return_value = [
|
||||||
|
{"id": 42}, # malformed share entry
|
||||||
|
{"uid_owner": "bob"},
|
||||||
|
{"uid_owner": 12345}, # non-string owner — skip
|
||||||
|
]
|
||||||
|
|
||||||
|
owners = await list_accessible_owners(sharing, "alice")
|
||||||
|
assert sorted(owners) == ["alice", "bob"]
|
||||||
|
|
||||||
|
@pytest.mark.unit
|
||||||
|
async def test_degrades_to_self_on_sharing_api_failure(self) -> None:
|
||||||
|
sharing = AsyncMock()
|
||||||
|
sharing.list_shares.side_effect = RuntimeError("OCS down")
|
||||||
|
|
||||||
|
owners = await list_accessible_owners(sharing, "alice")
|
||||||
|
# Fail-open to "self only" rather than blowing up search.
|
||||||
|
assert owners == ["alice"]
|
||||||
|
|
||||||
|
@pytest.mark.unit
|
||||||
|
async def test_calls_shared_with_me(self) -> None:
|
||||||
|
sharing = AsyncMock()
|
||||||
|
sharing.list_shares.return_value = []
|
||||||
|
|
||||||
|
await list_accessible_owners(sharing, "alice")
|
||||||
|
sharing.list_shares.assert_awaited_once_with(shared_with_me=True)
|
||||||
|
|
||||||
|
|
||||||
|
class TestOwnersCacheBehavior:
|
||||||
|
@pytest.mark.unit
|
||||||
|
async def test_second_call_within_ttl_uses_cache(self) -> None:
|
||||||
|
sharing = AsyncMock()
|
||||||
|
sharing.list_shares.return_value = [{"uid_owner": "bob"}]
|
||||||
|
|
||||||
|
first = await list_accessible_owners(sharing, "alice")
|
||||||
|
second = await list_accessible_owners(sharing, "alice")
|
||||||
|
|
||||||
|
assert sorted(first) == ["alice", "bob"]
|
||||||
|
assert second == first
|
||||||
|
# Only one OCS round-trip — the second call was served from cache.
|
||||||
|
sharing.list_shares.assert_awaited_once()
|
||||||
|
|
||||||
|
@pytest.mark.unit
|
||||||
|
async def test_expired_entry_triggers_fresh_ocs_call(self) -> None:
|
||||||
|
sharing = AsyncMock()
|
||||||
|
sharing.list_shares.return_value = [{"uid_owner": "bob"}]
|
||||||
|
|
||||||
|
await list_accessible_owners(sharing, "alice")
|
||||||
|
# Age the cached entry past the TTL without sleeping/patching the clock.
|
||||||
|
ts, value = access_filter._owners_cache["alice"]
|
||||||
|
access_filter._owners_cache["alice"] = (
|
||||||
|
ts - access_filter._OWNERS_CACHE_TTL_SECONDS - 1.0,
|
||||||
|
value,
|
||||||
|
)
|
||||||
|
await list_accessible_owners(sharing, "alice")
|
||||||
|
|
||||||
|
assert sharing.list_shares.await_count == 2
|
||||||
|
|
||||||
|
@pytest.mark.unit
|
||||||
|
async def test_failure_is_not_cached(self) -> None:
|
||||||
|
sharing = AsyncMock()
|
||||||
|
sharing.list_shares.side_effect = RuntimeError("OCS down")
|
||||||
|
|
||||||
|
await list_accessible_owners(sharing, "alice") # degrades to self-only
|
||||||
|
# A later success must not be masked by a cached failure.
|
||||||
|
sharing.list_shares.side_effect = None
|
||||||
|
sharing.list_shares.return_value = [{"uid_owner": "bob"}]
|
||||||
|
|
||||||
|
owners = await list_accessible_owners(sharing, "alice")
|
||||||
|
assert sorted(owners) == ["alice", "bob"]
|
||||||
|
|
||||||
|
@pytest.mark.unit
|
||||||
|
async def test_cache_is_bounded_lru(self, monkeypatch) -> None:
|
||||||
|
monkeypatch.setattr(access_filter, "_OWNERS_CACHE_MAXSIZE", 2)
|
||||||
|
sharing = AsyncMock()
|
||||||
|
sharing.list_shares.return_value = []
|
||||||
|
|
||||||
|
await list_accessible_owners(sharing, "u1")
|
||||||
|
await list_accessible_owners(sharing, "u2")
|
||||||
|
await list_accessible_owners(sharing, "u3") # evicts u1 (least recent)
|
||||||
|
|
||||||
|
assert set(access_filter._owners_cache.keys()) == {"u2", "u3"}
|
||||||
|
assert len(access_filter._owners_cache) == 2
|
||||||
|
|
||||||
|
|
||||||
|
class TestBuildOwnershipFilter:
|
||||||
|
def test_defaults_to_self_only_when_owners_omitted(self) -> None:
|
||||||
|
flt = build_ownership_filter("alice")
|
||||||
|
|
||||||
|
# Self-only: just the user_id branch. Self is NOT duplicated into an
|
||||||
|
# owner_id branch (the user_id branch already covers self-owned content).
|
||||||
|
assert flt.should is not None
|
||||||
|
assert len(flt.should) == 1
|
||||||
|
(user_branch,) = flt.should
|
||||||
|
assert user_branch.key == "user_id"
|
||||||
|
assert user_branch.match.value == "alice"
|
||||||
|
|
||||||
|
def test_expands_owner_branch_with_accessible_owners(self) -> None:
|
||||||
|
flt = build_ownership_filter("alice", ["alice", "bob", "carol"])
|
||||||
|
|
||||||
|
owner_branch, user_branch = flt.should
|
||||||
|
# Owner branch holds only the OTHER owners — self ("alice") is excluded
|
||||||
|
# because the user_id branch already matches self-owned content.
|
||||||
|
assert set(owner_branch.match.any) == {"bob", "carol"}
|
||||||
|
assert user_branch.key == "user_id"
|
||||||
|
assert user_branch.match.value == "alice"
|
||||||
|
|
||||||
|
def test_explicit_empty_list_omits_owner_branch_keeps_legacy(self) -> None:
|
||||||
|
# Edge case: caller passed an explicit empty list. The owner_id branch
|
||||||
|
# is omitted entirely (rather than relying on MatchAny(any=[]) matching
|
||||||
|
# nothing); the legacy user_id branch remains as the safety net so the
|
||||||
|
# user still finds their own content from before the migration.
|
||||||
|
flt = build_ownership_filter("alice", [])
|
||||||
|
|
||||||
|
assert flt.should is not None
|
||||||
|
assert len(flt.should) == 1
|
||||||
|
(user_branch,) = flt.should
|
||||||
|
assert user_branch.key == "user_id"
|
||||||
|
assert user_branch.match.value == "alice"
|
||||||
@@ -438,10 +438,16 @@ async def test_verify_news_items_malformed_api_response_keeps_all(mocker):
|
|||||||
|
|
||||||
|
|
||||||
@pytest.mark.unit
|
@pytest.mark.unit
|
||||||
async def test_verify_files_uses_path_from_metadata(mocker):
|
async def test_verify_files_accessible_by_global_id_is_kept(mocker):
|
||||||
"""File verifier reads path from SearchResult.metadata, no Qdrant round-trip."""
|
"""File verifier resolves the file by its global ID (the doc_id), ACL-aware.
|
||||||
|
|
||||||
|
This is what lets a recipient verify a file an owner shared with them:
|
||||||
|
file_accessible_by_id searches the user's whole tree (incl. mounted
|
||||||
|
shares) by global file id, not a path under the caller's own root (which
|
||||||
|
would 404 on shared files mounted at a different path).
|
||||||
|
"""
|
||||||
webdav_client = SimpleNamespace(
|
webdav_client = SimpleNamespace(
|
||||||
get_file_info=mocker.AsyncMock(return_value={"id": 100})
|
file_accessible_by_id=mocker.AsyncMock(return_value=True)
|
||||||
)
|
)
|
||||||
client = SimpleNamespace(webdav=webdav_client, username="alice")
|
client = SimpleNamespace(webdav=webdav_client, username="alice")
|
||||||
|
|
||||||
@@ -452,14 +458,15 @@ async def test_verify_files_uses_path_from_metadata(mocker):
|
|||||||
)
|
)
|
||||||
|
|
||||||
assert result == {"100"}
|
assert result == {"100"}
|
||||||
webdav_client.get_file_info.assert_awaited_once_with("Documents/foo.txt")
|
webdav_client.file_accessible_by_id.assert_awaited_once_with(100)
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.unit
|
@pytest.mark.unit
|
||||||
async def test_verify_files_404_drops(mocker):
|
async def test_verify_files_inaccessible_id_drops(mocker):
|
||||||
"""get_file_info raising HTTPStatusError(404) is a definitive drop."""
|
"""file_accessible_by_id returning False (file not in the user's tree) is a
|
||||||
|
definitive drop — the file is neither owned by nor shared with the user."""
|
||||||
webdav_client = SimpleNamespace(
|
webdav_client = SimpleNamespace(
|
||||||
get_file_info=mocker.AsyncMock(side_effect=_http_error(404))
|
file_accessible_by_id=mocker.AsyncMock(return_value=False)
|
||||||
)
|
)
|
||||||
client = SimpleNamespace(webdav=webdav_client, username="alice")
|
client = SimpleNamespace(webdav=webdav_client, username="alice")
|
||||||
|
|
||||||
@@ -473,68 +480,49 @@ async def test_verify_files_404_drops(mocker):
|
|||||||
|
|
||||||
|
|
||||||
@pytest.mark.unit
|
@pytest.mark.unit
|
||||||
async def test_verify_files_malformed_propfind_keeps_result(mocker):
|
async def test_verify_files_403_404_drops(mocker):
|
||||||
"""get_file_info returning None means malformed PROPFIND — keep the result.
|
"""A 403/404 raised by the SEARCH call is treated as a definitive drop,
|
||||||
|
consistent with the shared _is_definitive_404_or_403 policy used by every
|
||||||
|
verifier. (Normal inaccessibility surfaces as an empty result set, not a
|
||||||
|
status code, and is covered by test_verify_files_inaccessible_id_drops.)"""
|
||||||
|
for status in (403, 404):
|
||||||
|
webdav_client = SimpleNamespace(
|
||||||
|
file_accessible_by_id=mocker.AsyncMock(side_effect=_http_error(status))
|
||||||
|
)
|
||||||
|
client = SimpleNamespace(webdav=webdav_client, username="alice")
|
||||||
|
|
||||||
Per the contract change in webdav.py: ``None`` is now reserved for the
|
result = await _verify_files(
|
||||||
ambiguous "malformed XML" case. Real 404s raise HTTPStatusError. The
|
client,
|
||||||
file verifier must NOT evict on the ambiguous case (we cannot tell
|
[_make_result(124, doc_type="file", metadata={"path": "x.txt"})],
|
||||||
whether the file exists), only log a warning and keep the result.
|
_sem(),
|
||||||
"""
|
)
|
||||||
webdav_client = SimpleNamespace(get_file_info=mocker.AsyncMock(return_value=None))
|
|
||||||
client = SimpleNamespace(webdav=webdav_client, username="alice")
|
|
||||||
|
|
||||||
result = await _verify_files(
|
assert result == set(), f"{status} on the SEARCH call must drop"
|
||||||
client,
|
|
||||||
[_make_result(123, doc_type="file", metadata={"path": "brittle.txt"})],
|
|
||||||
_sem(),
|
|
||||||
)
|
|
||||||
|
|
||||||
assert result == {"123"}, "ambiguous None must keep result, not evict"
|
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.unit
|
@pytest.mark.unit
|
||||||
async def test_verify_files_403_drops(mocker):
|
async def test_verify_files_non_numeric_id_keeps_unverified(mocker):
|
||||||
"""get_file_info raising HTTPStatusError(403) is a definitive drop."""
|
"""Without a numeric file id we cannot verify — fail open, don't drop."""
|
||||||
webdav_client = SimpleNamespace(
|
webdav_client = SimpleNamespace(
|
||||||
get_file_info=mocker.AsyncMock(side_effect=_http_error(403))
|
file_accessible_by_id=mocker.AsyncMock(
|
||||||
|
side_effect=AssertionError("must not be called")
|
||||||
|
)
|
||||||
)
|
)
|
||||||
client = SimpleNamespace(webdav=webdav_client, username="alice")
|
client = SimpleNamespace(webdav=webdav_client, username="alice")
|
||||||
|
|
||||||
result = await _verify_files(
|
result = await _verify_files(
|
||||||
client,
|
client,
|
||||||
[_make_result(124, doc_type="file", metadata={"path": "forbidden.txt"})],
|
[_make_result("not-a-file-id", doc_type="file", metadata={"path": "x.txt"})],
|
||||||
_sem(),
|
_sem(),
|
||||||
)
|
)
|
||||||
|
assert result == {"not-a-file-id"}
|
||||||
assert result == set()
|
webdav_client.file_accessible_by_id.assert_not_awaited()
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.unit
|
|
||||||
async def test_verify_files_missing_path_metadata_keeps_unverified(mocker):
|
|
||||||
"""Without a path in metadata we cannot verify — fail open, don't drop."""
|
|
||||||
webdav_client = SimpleNamespace(
|
|
||||||
get_file_info=mocker.AsyncMock(side_effect=AssertionError("must not be called"))
|
|
||||||
)
|
|
||||||
client = SimpleNamespace(webdav=webdav_client, username="alice")
|
|
||||||
|
|
||||||
# No metadata at all
|
|
||||||
result = await _verify_files(client, [_make_result(555, doc_type="file")], _sem())
|
|
||||||
assert result == {"555"}
|
|
||||||
webdav_client.get_file_info.assert_not_awaited()
|
|
||||||
|
|
||||||
# Metadata present but no "path" key
|
|
||||||
result = await _verify_files(
|
|
||||||
client, [_make_result(556, doc_type="file", metadata={})], _sem()
|
|
||||||
)
|
|
||||||
assert result == {"556"}
|
|
||||||
webdav_client.get_file_info.assert_not_awaited()
|
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.unit
|
@pytest.mark.unit
|
||||||
async def test_verify_files_transient_5xx_keeps(mocker):
|
async def test_verify_files_transient_5xx_keeps(mocker):
|
||||||
webdav_client = SimpleNamespace(
|
webdav_client = SimpleNamespace(
|
||||||
get_file_info=mocker.AsyncMock(side_effect=_http_error(503))
|
file_accessible_by_id=mocker.AsyncMock(side_effect=_http_error(503))
|
||||||
)
|
)
|
||||||
client = SimpleNamespace(webdav=webdav_client, username="alice")
|
client = SimpleNamespace(webdav=webdav_client, username="alice")
|
||||||
|
|
||||||
@@ -549,9 +537,9 @@ async def test_verify_files_transient_5xx_keeps(mocker):
|
|||||||
|
|
||||||
@pytest.mark.unit
|
@pytest.mark.unit
|
||||||
async def test_verify_files_429_keeps_as_transient(mocker):
|
async def test_verify_files_429_keeps_as_transient(mocker):
|
||||||
"""HTTP 429 from get_file_info must NOT silently drop file results."""
|
"""HTTP 429 from the SEARCH call must NOT silently drop file results."""
|
||||||
webdav_client = SimpleNamespace(
|
webdav_client = SimpleNamespace(
|
||||||
get_file_info=mocker.AsyncMock(side_effect=_http_error(429))
|
file_accessible_by_id=mocker.AsyncMock(side_effect=_http_error(429))
|
||||||
)
|
)
|
||||||
client = SimpleNamespace(webdav=webdav_client, username="alice")
|
client = SimpleNamespace(webdav=webdav_client, username="alice")
|
||||||
|
|
||||||
@@ -566,14 +554,14 @@ async def test_verify_files_429_keeps_as_transient(mocker):
|
|||||||
|
|
||||||
@pytest.mark.unit
|
@pytest.mark.unit
|
||||||
async def test_verify_files_unexpected_exception_keeps(mocker):
|
async def test_verify_files_unexpected_exception_keeps(mocker):
|
||||||
"""A non-HTTP exception from get_file_info must not drop the result.
|
"""A non-HTTP exception from file_accessible_by_id must not drop the result.
|
||||||
|
|
||||||
The catch-all ``except Exception`` branch in the file verifier exists
|
The catch-all ``except Exception`` branch in the file verifier exists
|
||||||
so a bug in the WebDAV client (or an httpx ConnectError on a flaky
|
so a bug in the WebDAV client (or an httpx ConnectError on a flaky
|
||||||
network) cannot silently shrink result pages.
|
network) cannot silently shrink result pages.
|
||||||
"""
|
"""
|
||||||
webdav_client = SimpleNamespace(
|
webdav_client = SimpleNamespace(
|
||||||
get_file_info=mocker.AsyncMock(side_effect=RuntimeError("dav blew up"))
|
file_accessible_by_id=mocker.AsyncMock(side_effect=RuntimeError("dav blew up"))
|
||||||
)
|
)
|
||||||
client = SimpleNamespace(webdav=webdav_client, username="alice")
|
client = SimpleNamespace(webdav=webdav_client, username="alice")
|
||||||
|
|
||||||
@@ -886,6 +874,37 @@ async def test_verify_search_results_drops_inaccessible_and_evicts(mocker):
|
|||||||
spy_evict.assert_awaited_once_with("99", "note", "alice")
|
spy_evict.assert_awaited_once_with("99", "note", "alice")
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.unit
|
||||||
|
async def test_verify_evicts_cross_user_file_under_querying_user_id(mocker):
|
||||||
|
"""A shared file the recipient can no longer access is evicted under the
|
||||||
|
QUERYING user's id, never the owner's.
|
||||||
|
|
||||||
|
This guards the cross-user eviction no-op: a point owned by alice
|
||||||
|
(user_id=alice) surfaced to bob via accessible_owners and then found
|
||||||
|
inaccessible must be evicted with user_id=bob — which deletes nothing of
|
||||||
|
alice's (her points carry user_id=alice). So a recipient's revoked access
|
||||||
|
can never delete the owner's index entries; bob's view self-heals via
|
||||||
|
list_accessible_owners instead. A future change that evicted under the
|
||||||
|
owner's id would corrupt the owner's index, and this test would catch it.
|
||||||
|
"""
|
||||||
|
spy_evict = mocker.AsyncMock()
|
||||||
|
mocker.patch.object(verification, "delete_document_points", spy_evict)
|
||||||
|
|
||||||
|
webdav_client = SimpleNamespace(
|
||||||
|
file_accessible_by_id=mocker.AsyncMock(return_value=False)
|
||||||
|
)
|
||||||
|
client = SimpleNamespace(webdav=webdav_client, username="bob")
|
||||||
|
|
||||||
|
kept, dropped_count = await verify_search_results(
|
||||||
|
client,
|
||||||
|
[_make_result(777, doc_type="file", metadata={"path": "shared.txt"})],
|
||||||
|
)
|
||||||
|
|
||||||
|
assert kept == []
|
||||||
|
assert dropped_count == 1
|
||||||
|
spy_evict.assert_awaited_once_with("777", "file", "bob")
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.unit
|
@pytest.mark.unit
|
||||||
async def test_verify_search_results_fire_and_forget_eviction(mocker):
|
async def test_verify_search_results_fire_and_forget_eviction(mocker):
|
||||||
"""When eviction_task_group is provided, eviction does not block the response.
|
"""When eviction_task_group is provided, eviction does not block the response.
|
||||||
|
|||||||
@@ -68,7 +68,11 @@ class TestIndexedPath:
|
|||||||
# One scroll call, and the filter must include chunk_index (not offsets)
|
# One scroll call, and the filter must include chunk_index (not offsets)
|
||||||
qdrant_client.scroll.assert_awaited_once()
|
qdrant_client.scroll.assert_awaited_once()
|
||||||
scroll_kwargs = qdrant_client.scroll.await_args.kwargs
|
scroll_kwargs = qdrant_client.scroll.await_args.kwargs
|
||||||
filter_keys = [c.key for c in scroll_kwargs["scroll_filter"].must]
|
# Skip nested Filters (the ACL ownership sub-filter) — only field
|
||||||
|
# conditions carry a `.key`.
|
||||||
|
filter_keys = [
|
||||||
|
c.key for c in scroll_kwargs["scroll_filter"].must if hasattr(c, "key")
|
||||||
|
]
|
||||||
assert "chunk_index" in filter_keys
|
assert "chunk_index" in filter_keys
|
||||||
assert "chunk_start_offset" not in filter_keys
|
assert "chunk_start_offset" not in filter_keys
|
||||||
assert "chunk_end_offset" not in filter_keys
|
assert "chunk_end_offset" not in filter_keys
|
||||||
@@ -92,7 +96,11 @@ class TestOffsetFallbackPath:
|
|||||||
|
|
||||||
assert result == (bbox, 2)
|
assert result == (bbox, 2)
|
||||||
scroll_kwargs = qdrant_client.scroll.await_args.kwargs
|
scroll_kwargs = qdrant_client.scroll.await_args.kwargs
|
||||||
filter_keys = [c.key for c in scroll_kwargs["scroll_filter"].must]
|
# Skip nested Filters (the ACL ownership sub-filter) — only field
|
||||||
|
# conditions carry a `.key`.
|
||||||
|
filter_keys = [
|
||||||
|
c.key for c in scroll_kwargs["scroll_filter"].must if hasattr(c, "key")
|
||||||
|
]
|
||||||
assert "chunk_start_offset" in filter_keys
|
assert "chunk_start_offset" in filter_keys
|
||||||
assert "chunk_end_offset" in filter_keys
|
assert "chunk_end_offset" in filter_keys
|
||||||
assert "chunk_index" not in filter_keys
|
assert "chunk_index" not in filter_keys
|
||||||
|
|||||||
@@ -151,6 +151,45 @@ async def test_poll_expired(flow_client):
|
|||||||
assert result.app_password is None
|
assert result.app_password is None
|
||||||
|
|
||||||
|
|
||||||
|
async def test_initiate_rewrites_login_url_to_public_host():
|
||||||
|
"""When server↔Nextcloud uses an internal host (e.g. the ``app`` Docker
|
||||||
|
service), the browser-facing login URL must be rewritten to the configured
|
||||||
|
public host; the poll endpoint stays on the internal host for server-side
|
||||||
|
polling. Mock URLs use https to match this file's convention (the rewrite
|
||||||
|
is scheme-agnostic, so this exercises the same origin-replacement logic)."""
|
||||||
|
client = LoginFlowV2Client(
|
||||||
|
nextcloud_host="https://nc-internal.test", # server↔Nextcloud origin
|
||||||
|
verify_ssl=False,
|
||||||
|
public_host="https://cloud.example.com", # browser-reachable origin
|
||||||
|
)
|
||||||
|
mock_response = _mock_response(
|
||||||
|
200,
|
||||||
|
{
|
||||||
|
# Nextcloud builds these from the request (internal) host.
|
||||||
|
"login": "https://nc-internal.test/login/v2/flow/tok123",
|
||||||
|
"poll": {
|
||||||
|
"endpoint": "https://nc-internal.test/login/v2/poll",
|
||||||
|
"token": "tok", # value irrelevant here; this test asserts the URLs
|
||||||
|
},
|
||||||
|
},
|
||||||
|
)
|
||||||
|
mock_client = AsyncMock()
|
||||||
|
mock_client.post.return_value = mock_response
|
||||||
|
mock_client.__aenter__ = AsyncMock(return_value=mock_client)
|
||||||
|
mock_client.__aexit__ = AsyncMock(return_value=False)
|
||||||
|
|
||||||
|
with patch(
|
||||||
|
"nextcloud_mcp_server.auth.login_flow.nextcloud_httpx_client",
|
||||||
|
return_value=mock_client,
|
||||||
|
):
|
||||||
|
result = await client.initiate()
|
||||||
|
|
||||||
|
# Browser-facing URL uses the public host...
|
||||||
|
assert result.login_url == "https://cloud.example.com/login/v2/flow/tok123"
|
||||||
|
# ...while the poll endpoint stays on the internal host (server polls it).
|
||||||
|
assert result.poll_endpoint == "https://nc-internal.test/login/v2/poll"
|
||||||
|
|
||||||
|
|
||||||
async def test_initiate_with_custom_user_agent(flow_client):
|
async def test_initiate_with_custom_user_agent(flow_client):
|
||||||
"""Test that custom user agent is passed in the request."""
|
"""Test that custom user agent is passed in the request."""
|
||||||
mock_response = _mock_response(
|
mock_response = _mock_response(
|
||||||
|
|||||||
@@ -180,6 +180,25 @@ async def test_provision_app_password_invalid_format():
|
|||||||
assert "Invalid app password format" in response.json()["error"]
|
assert "Invalid app password format" in response.json()["error"]
|
||||||
|
|
||||||
|
|
||||||
|
def test_app_password_pattern_accepts_dashed_and_raw_tokens():
|
||||||
|
"""The format guard accepts both the dashed Security-settings format and
|
||||||
|
the raw token from the one-click ``core/getapppassword`` flow, and still
|
||||||
|
rejects short / illegal-character input."""
|
||||||
|
from nextcloud_mcp_server.api.passwords import APP_PASSWORD_PATTERN
|
||||||
|
|
||||||
|
# Dashed format a user copies from Security settings.
|
||||||
|
assert APP_PASSWORD_PATTERN.match("abcde-ABCDE-12345-fghij-67890")
|
||||||
|
# Raw 72-char token returned by core/getapppassword (one-click opt-in).
|
||||||
|
assert APP_PASSWORD_PATTERN.match(
|
||||||
|
"kZmgLDQnqQHUAxhRq4d2VssBfjsI0PaHbL4JySWtwJkzVgAf34c0sZshEjZjuj1PLbwrf83q"
|
||||||
|
)
|
||||||
|
# Still rejects obviously-bad input.
|
||||||
|
assert not APP_PASSWORD_PATTERN.match("short")
|
||||||
|
assert not APP_PASSWORD_PATTERN.match("invalid-password") # < 20 chars
|
||||||
|
assert not APP_PASSWORD_PATTERN.match("has spaces not allowed in this token")
|
||||||
|
assert not APP_PASSWORD_PATTERN.match("contains/slash/" + "a" * 20)
|
||||||
|
|
||||||
|
|
||||||
async def test_provision_app_password_success(temp_storage, mocker):
|
async def test_provision_app_password_success(temp_storage, mocker):
|
||||||
"""Test successful app password provisioning."""
|
"""Test successful app password provisioning."""
|
||||||
# Mock settings (imported locally in the function)
|
# Mock settings (imported locally in the function)
|
||||||
|
|||||||
@@ -0,0 +1,93 @@
|
|||||||
|
"""Unit tests for app-password-store awareness in the provisioning tools.
|
||||||
|
|
||||||
|
Login Flow v2 (nc_auth_provision_access) and the management app-password API
|
||||||
|
write the credential to this server's ``app_passwords`` store — the same store
|
||||||
|
``require_provisioning``/``get_client`` use to grant tool access. The OAuth
|
||||||
|
provisioning tools (check_provisioning_status / revoke_nextcloud_access) must
|
||||||
|
read and clear that store too, otherwise they report "not provisioned" while
|
||||||
|
tools still work, and "nothing to revoke" while the credential persists.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from types import SimpleNamespace
|
||||||
|
from unittest.mock import AsyncMock, MagicMock
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
from nextcloud_mcp_server.server import oauth_tools
|
||||||
|
from nextcloud_mcp_server.server.oauth_tools import (
|
||||||
|
_get_provisioning_status,
|
||||||
|
_revoke_nextcloud_access,
|
||||||
|
)
|
||||||
|
|
||||||
|
pytestmark = pytest.mark.unit
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
def _no_astrolabe_settings(mocker):
|
||||||
|
"""Disable the astrolabe-status branch so the app_passwords store is hit."""
|
||||||
|
mocker.patch.object(
|
||||||
|
oauth_tools,
|
||||||
|
"get_settings",
|
||||||
|
return_value=SimpleNamespace(oidc_client_id=None, oidc_client_secret=None),
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
async def test_status_reports_provisioned_for_app_password_store(
|
||||||
|
mocker, _no_astrolabe_settings
|
||||||
|
):
|
||||||
|
"""A Login Flow v2 app password in storage => is_provisioned with the
|
||||||
|
app_password credential type (was previously reported as not provisioned)."""
|
||||||
|
storage = MagicMock()
|
||||||
|
# Only truthiness + "scopes" are read by _get_provisioning_status; omit the
|
||||||
|
# app_password value entirely (avoids a false-positive hard-coded-credential
|
||||||
|
# finding and keeps the mock to what the code under test actually uses).
|
||||||
|
storage.get_app_password_with_scopes = AsyncMock(
|
||||||
|
return_value={"scopes": ["notes.read"]}
|
||||||
|
)
|
||||||
|
storage.get_refresh_token = AsyncMock(return_value=None)
|
||||||
|
mocker.patch.object(
|
||||||
|
oauth_tools, "get_shared_storage", AsyncMock(return_value=storage)
|
||||||
|
)
|
||||||
|
|
||||||
|
status = await _get_provisioning_status(MagicMock(), "tester")
|
||||||
|
|
||||||
|
assert status.is_provisioned is True
|
||||||
|
assert status.credential_type == "app_password"
|
||||||
|
assert status.flow_type == "login_flow_v2"
|
||||||
|
assert status.scopes == ["notes.read"]
|
||||||
|
storage.get_refresh_token.assert_not_awaited() # app password short-circuits
|
||||||
|
|
||||||
|
|
||||||
|
async def test_revoke_deletes_app_password(mocker, _no_astrolabe_settings):
|
||||||
|
"""Revoke must delete the app password from storage (not just refresh tokens)."""
|
||||||
|
storage = MagicMock()
|
||||||
|
storage.get_app_password_with_scopes = AsyncMock(return_value={"scopes": None})
|
||||||
|
storage.get_refresh_token = AsyncMock(return_value=None)
|
||||||
|
storage.delete_app_password = AsyncMock(return_value=True)
|
||||||
|
mocker.patch.object(
|
||||||
|
oauth_tools, "get_shared_storage", AsyncMock(return_value=storage)
|
||||||
|
)
|
||||||
|
mocker.patch.object(oauth_tools, "invalidate_scope_cache")
|
||||||
|
|
||||||
|
result = await _revoke_nextcloud_access(MagicMock(), "tester")
|
||||||
|
|
||||||
|
assert result.success is True
|
||||||
|
storage.delete_app_password.assert_awaited_once_with("tester")
|
||||||
|
oauth_tools.invalidate_scope_cache.assert_called_once_with("tester")
|
||||||
|
|
||||||
|
|
||||||
|
async def test_revoke_noop_when_nothing_provisioned(mocker, _no_astrolabe_settings):
|
||||||
|
"""No credential of any kind => graceful no-op, no deletion attempted."""
|
||||||
|
storage = MagicMock()
|
||||||
|
storage.get_app_password_with_scopes = AsyncMock(return_value=None)
|
||||||
|
storage.get_refresh_token = AsyncMock(return_value=None)
|
||||||
|
storage.delete_app_password = AsyncMock()
|
||||||
|
mocker.patch.object(
|
||||||
|
oauth_tools, "get_shared_storage", AsyncMock(return_value=storage)
|
||||||
|
)
|
||||||
|
|
||||||
|
result = await _revoke_nextcloud_access(MagicMock(), "tester")
|
||||||
|
|
||||||
|
assert result.success is True
|
||||||
|
assert "No Nextcloud access to revoke" in result.message
|
||||||
|
storage.delete_app_password.assert_not_awaited()
|
||||||
Vendored
+1
-1
Submodule third_party/astrolabe updated: 1d2dc12280...c2dec99906
Reference in New Issue
Block a user