Merge pull request #715 from cbcoutinho/fix/chunk-context-app-password-credentials
fix(api): use stored app password for chunk-context and pdf-preview
This commit is contained in:
@@ -23,10 +23,8 @@ from nextcloud_mcp_server.api.management import (
|
||||
_parse_int_param,
|
||||
_sanitize_error_for_client,
|
||||
_validate_query_string,
|
||||
extract_bearer_token,
|
||||
validate_token_and_get_user,
|
||||
)
|
||||
from nextcloud_mcp_server.client import NextcloudClient
|
||||
from nextcloud_mcp_server.config import get_settings
|
||||
from nextcloud_mcp_server.embedding.service import get_embedding_service
|
||||
from nextcloud_mcp_server.search import (
|
||||
@@ -34,6 +32,10 @@ from nextcloud_mcp_server.search import (
|
||||
SemanticSearchAlgorithm,
|
||||
)
|
||||
from nextcloud_mcp_server.search.context import get_chunk_with_context
|
||||
from nextcloud_mcp_server.vector.oauth_sync import (
|
||||
NotProvisionedError,
|
||||
get_user_client_basic_auth,
|
||||
)
|
||||
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.visualization import compute_pca_coordinates
|
||||
@@ -512,11 +514,6 @@ async def get_chunk_context(request: Request) -> JSONResponse:
|
||||
# Convert doc_id to int if possible (most IDs are int)
|
||||
doc_id_val: str | int = int(doc_id) if doc_id.isdigit() else doc_id
|
||||
|
||||
# Get bearer token for client initialization
|
||||
token = extract_bearer_token(request)
|
||||
if not token:
|
||||
raise ValueError("Missing token")
|
||||
|
||||
# Get Nextcloud host from OAuth context
|
||||
oauth_ctx = request.app.state.oauth_context
|
||||
nextcloud_host = oauth_ctx.get("config", {}).get("nextcloud_host", "")
|
||||
@@ -524,10 +521,19 @@ async def get_chunk_context(request: Request) -> JSONResponse:
|
||||
if not nextcloud_host:
|
||||
raise ValueError("Nextcloud host not configured")
|
||||
|
||||
# Initialize authenticated Nextcloud client
|
||||
async with NextcloudClient.from_token(
|
||||
base_url=nextcloud_host, token=token, username=user_id
|
||||
) as nc_client:
|
||||
# Use the user's stored app password for Nextcloud calls.
|
||||
# The OAuth bearer is only used to authenticate Astrolabe → MCP Server;
|
||||
# MCP Server → Nextcloud always uses the app password provisioned
|
||||
# during the authorization step.
|
||||
try:
|
||||
nc_client = await get_user_client_basic_auth(user_id, nextcloud_host)
|
||||
except NotProvisionedError as e:
|
||||
return JSONResponse(
|
||||
{"success": False, "error": str(e)},
|
||||
status_code=401,
|
||||
)
|
||||
|
||||
async with nc_client:
|
||||
chunk_context = await get_chunk_with_context(
|
||||
nc_client=nc_client,
|
||||
user_id=user_id,
|
||||
@@ -687,11 +693,6 @@ async def get_pdf_preview(request: Request) -> JSONResponse:
|
||||
except ValueError as e:
|
||||
return JSONResponse({"success": False, "error": str(e)}, status_code=400)
|
||||
|
||||
# Get bearer token for WebDAV authentication
|
||||
token = extract_bearer_token(request)
|
||||
if not token:
|
||||
raise ValueError("Missing token")
|
||||
|
||||
# Get Nextcloud host from OAuth context
|
||||
oauth_ctx = request.app.state.oauth_context
|
||||
nextcloud_host = oauth_ctx.get("config", {}).get("nextcloud_host", "")
|
||||
@@ -699,10 +700,19 @@ async def get_pdf_preview(request: Request) -> JSONResponse:
|
||||
if not nextcloud_host:
|
||||
raise ValueError("Nextcloud host not configured")
|
||||
|
||||
# Download PDF via WebDAV using user's token
|
||||
async with NextcloudClient.from_token(
|
||||
base_url=nextcloud_host, token=token, username=user_id
|
||||
) as nc_client:
|
||||
# Use the user's stored app password for Nextcloud calls.
|
||||
# The OAuth bearer is only used to authenticate Astrolabe → MCP Server;
|
||||
# MCP Server → Nextcloud always uses the app password provisioned
|
||||
# during the authorization step.
|
||||
try:
|
||||
nc_client = await get_user_client_basic_auth(user_id, nextcloud_host)
|
||||
except NotProvisionedError as e:
|
||||
return JSONResponse(
|
||||
{"success": False, "error": str(e)},
|
||||
status_code=401,
|
||||
)
|
||||
|
||||
async with nc_client:
|
||||
pdf_bytes, _ = await nc_client.webdav.read_file(file_path)
|
||||
|
||||
# Check file size limit (50 MB)
|
||||
|
||||
@@ -34,6 +34,10 @@ from nextcloud_mcp_server.search import (
|
||||
SemanticSearchAlgorithm,
|
||||
)
|
||||
from nextcloud_mcp_server.search.context import get_chunk_with_context
|
||||
from nextcloud_mcp_server.vector.oauth_sync import (
|
||||
NotProvisionedError,
|
||||
get_user_client_basic_auth,
|
||||
)
|
||||
from nextcloud_mcp_server.vector.pca import PCA
|
||||
from nextcloud_mcp_server.vector.placeholder import get_placeholder_filter
|
||||
from nextcloud_mcp_server.vector.qdrant_client import get_qdrant_client
|
||||
@@ -554,12 +558,28 @@ async def chunk_context_endpoint(request: Request) -> JSONResponse:
|
||||
# Convert doc_id to int (all document types use int IDs)
|
||||
doc_id_int = int(doc_id)
|
||||
|
||||
# Get authenticated Nextcloud client
|
||||
# Use context expansion module to fetch chunk with surrounding context
|
||||
async with await _get_authenticated_client_for_userinfo(request) as nc_client:
|
||||
user_id = request.user.display_name
|
||||
settings = get_settings()
|
||||
nextcloud_host = settings.nextcloud_host
|
||||
if not nextcloud_host:
|
||||
raise RuntimeError("Nextcloud host not configured")
|
||||
|
||||
# Use the user's stored app password for Nextcloud calls.
|
||||
# The session cookie only authenticates the browser → MCP Server hop;
|
||||
# MCP Server → Nextcloud always uses the app password provisioned
|
||||
# during the authorization step.
|
||||
try:
|
||||
nc_client = await get_user_client_basic_auth(user_id, nextcloud_host)
|
||||
except NotProvisionedError as e:
|
||||
return JSONResponse(
|
||||
{"success": False, "error": str(e)},
|
||||
status_code=401,
|
||||
)
|
||||
|
||||
async with nc_client:
|
||||
chunk_context = await get_chunk_with_context(
|
||||
nc_client=nc_client,
|
||||
user_id=request.user.display_name, # User ID from auth
|
||||
user_id=user_id,
|
||||
doc_id=doc_id_int,
|
||||
doc_type=doc_type,
|
||||
chunk_start=start,
|
||||
|
||||
@@ -0,0 +1,294 @@
|
||||
"""Integration test for the chunk-context HTTP path in multi-user BasicAuth mode.
|
||||
|
||||
Cross-system interface test: Tests the MCP server's /api/v1/chunk-context
|
||||
handler via the Astrolabe PHP app (/apps/astrolabe/api/chunk-context). Astrolabe
|
||||
source lives in ./third_party/astrolabe (submodule) and is installed during
|
||||
test setup by app-hooks/post-installation/20-install-astrolabe-app.sh.
|
||||
|
||||
Regression test: Prior to this test, `get_chunk_context` in
|
||||
nextcloud_mcp_server/api/visualization.py forwarded the OAuth bearer token
|
||||
directly to Nextcloud via NextcloudClient.from_token(...). In multi-user
|
||||
BasicAuth deployments, Nextcloud doesn't validate that bearer on the Notes
|
||||
API, so the handler returned 404 (wrapped by Astrolabe as 500). This test
|
||||
exercises the full chain:
|
||||
|
||||
browser session → astrolabe → MCP server (OAuth bearer) →
|
||||
get_user_client_basic_auth → Nextcloud (app password BasicAuth) → note
|
||||
|
||||
so a regression to from_token-style auth would surface as a 500/404 from
|
||||
Astrolabe instead of a 200 with chunk_text.
|
||||
"""
|
||||
|
||||
import base64
|
||||
import json
|
||||
import logging
|
||||
import re
|
||||
import time
|
||||
import uuid
|
||||
|
||||
import anyio
|
||||
import httpx
|
||||
import pytest
|
||||
|
||||
from tests.conftest import create_mcp_client_session
|
||||
from tests.integration.test_astrolabe_multi_user_background_sync import (
|
||||
complete_astrolabe_authorization,
|
||||
login_to_nextcloud,
|
||||
)
|
||||
from tests.integration.test_astrolabe_plotly_visualization import wait_for_vector_sync
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
pytestmark = [pytest.mark.integration, pytest.mark.multi_user_basic]
|
||||
|
||||
|
||||
def _build_basic_auth_header(username: str, password: str) -> str:
|
||||
credentials = base64.b64encode(f"{username}:{password}".encode()).decode("utf-8")
|
||||
return f"Basic {credentials}"
|
||||
|
||||
|
||||
async def _poll_astrolabe_search_for_note(
|
||||
page,
|
||||
unique_term: str,
|
||||
note_id,
|
||||
csrf_headers: dict,
|
||||
timeout_seconds: int = 60,
|
||||
) -> dict:
|
||||
"""Poll Astrolabe's search endpoint until `note_id` shows up in results.
|
||||
|
||||
`wait_for_vector_sync` only waits for the total indexed count to grow —
|
||||
it does not guarantee that *this specific* document is visible yet
|
||||
(observed on nc32 where deck-card seed data indexes first and the new
|
||||
note arrives in Qdrant a few seconds later). Poll until the unique term
|
||||
returns our note, or fail loudly with the last response we saw.
|
||||
"""
|
||||
deadline = time.monotonic() + timeout_seconds
|
||||
last_results: list | None = None
|
||||
attempts = 0
|
||||
while time.monotonic() < deadline:
|
||||
attempts += 1
|
||||
search_resp = await page.request.get(
|
||||
f"http://localhost:8080/apps/astrolabe/api/search"
|
||||
f"?query={unique_term}&algorithm=hybrid&limit=5&include_pca=false",
|
||||
headers=csrf_headers,
|
||||
)
|
||||
assert search_resp.ok, (
|
||||
f"Astrolabe search failed on attempt {attempts}: "
|
||||
f"{search_resp.status} {await search_resp.text()}"
|
||||
)
|
||||
search_data = await search_resp.json()
|
||||
assert search_data.get("success"), (
|
||||
f"Astrolabe search returned error on attempt {attempts}: {search_data}"
|
||||
)
|
||||
last_results = search_data.get("results") or []
|
||||
note_result = next(
|
||||
(
|
||||
r
|
||||
for r in last_results
|
||||
if r.get("doc_type") == "note" and str(r.get("id")) == str(note_id)
|
||||
),
|
||||
None,
|
||||
)
|
||||
if note_result is not None:
|
||||
logger.info(
|
||||
f"Note {note_id} surfaced in Astrolabe search after {attempts} "
|
||||
f"attempts (~{attempts * 2}s)"
|
||||
)
|
||||
return note_result
|
||||
await anyio.sleep(2)
|
||||
|
||||
raise AssertionError(
|
||||
f"Note {note_id} with unique term '{unique_term}' did not surface in "
|
||||
f"Astrolabe search within {timeout_seconds}s ({attempts} attempts). "
|
||||
f"Last {len(last_results or [])} results: {last_results}"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.timeout(300)
|
||||
async def test_chunk_context_endpoint_uses_app_password(
|
||||
browser,
|
||||
test_users_setup,
|
||||
configure_astrolabe_for_mcp_server,
|
||||
):
|
||||
"""Astrolabe /api/chunk-context must return 200 with populated chunk_text.
|
||||
|
||||
Covers the regression where the MCP handler used BearerAuth against
|
||||
Nextcloud instead of BasicAuth with the stored app password.
|
||||
"""
|
||||
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"
|
||||
password = test_users_setup[username]["password"]
|
||||
note_id = None
|
||||
unique_term = f"chunk_ctx_test_{uuid.uuid4().hex[:8]}"
|
||||
|
||||
context = await browser.new_context(ignore_https_errors=True)
|
||||
page = await context.new_page()
|
||||
|
||||
try:
|
||||
await login_to_nextcloud(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"
|
||||
|
||||
auth_header = _build_basic_auth_header(username, password)
|
||||
async with create_mcp_client_session(
|
||||
url="http://localhost:8003/mcp",
|
||||
headers={"Authorization": auth_header},
|
||||
client_name="Alice Chunk Context Test",
|
||||
) as mcp_client:
|
||||
initial_sync = await mcp_client.call_tool("nc_get_vector_sync_status", {})
|
||||
if initial_sync.isError:
|
||||
pytest.skip("Vector sync not enabled on mcp-multi-user-basic")
|
||||
initial_count = json.loads(initial_sync.content[0].text).get(
|
||||
"indexed_count", 0
|
||||
)
|
||||
|
||||
note_body = (
|
||||
f"# Chunk Context Regression Test\n\n"
|
||||
f"This document exists to verify that the chunk-context HTTP "
|
||||
f"endpoint can re-fetch it after indexing. "
|
||||
f"Unique marker: {unique_term}.\n\n"
|
||||
f"Paragraph two exists so there is a plausible surrounding "
|
||||
f"context to slice. Lorem ipsum dolor sit amet, consectetur "
|
||||
f"adipiscing elit."
|
||||
)
|
||||
note_response = await mcp_client.call_tool(
|
||||
"nc_notes_create_note",
|
||||
{
|
||||
"title": f"Chunk Context Test {unique_term}",
|
||||
"content": note_body,
|
||||
"category": "Test",
|
||||
},
|
||||
)
|
||||
assert not note_response.isError, f"Create note failed: {note_response}"
|
||||
note_id = json.loads(note_response.content[0].text).get("id")
|
||||
assert note_id is not None
|
||||
|
||||
sync_complete, status = await wait_for_vector_sync(
|
||||
mcp_client, initial_count, timeout_seconds=90
|
||||
)
|
||||
assert sync_complete, f"Vector sync did not complete: {status}"
|
||||
|
||||
# Use the browser's session to drive Astrolabe end-to-end, the way a
|
||||
# real user would: this exercises astrolabe's OAuth token retrieval
|
||||
# and the MCP server's handler under a real bearer. Nextcloud's
|
||||
# controllers require a CSRF token (`requesttoken` header) that the
|
||||
# rendered SPA picks up from `OC.requestToken`; direct page.request
|
||||
# calls don't get it for free, so we load an Astrolabe page and
|
||||
# forward the token on each subsequent API call.
|
||||
await page.goto(
|
||||
"http://localhost:8080/apps/astrolabe/", wait_until="networkidle"
|
||||
)
|
||||
request_token = await page.evaluate("window.OC && OC.requestToken")
|
||||
assert request_token, (
|
||||
"Could not read OC.requestToken from Astrolabe page — is the user logged in?"
|
||||
)
|
||||
csrf_headers = {"requesttoken": request_token}
|
||||
|
||||
note_result = await _poll_astrolabe_search_for_note(
|
||||
page=page,
|
||||
unique_term=unique_term,
|
||||
note_id=note_id,
|
||||
csrf_headers=csrf_headers,
|
||||
timeout_seconds=60,
|
||||
)
|
||||
start = note_result.get("chunk_start_offset")
|
||||
end = note_result.get("chunk_end_offset")
|
||||
assert start is not None and end is not None, (
|
||||
f"Search result missing chunk offsets: {note_result}"
|
||||
)
|
||||
|
||||
chunk_resp = await page.request.get(
|
||||
"http://localhost:8080/apps/astrolabe/api/chunk-context",
|
||||
params={
|
||||
"doc_type": "note",
|
||||
"doc_id": str(note_id),
|
||||
"start": str(start),
|
||||
"end": str(end),
|
||||
},
|
||||
headers=csrf_headers,
|
||||
)
|
||||
assert chunk_resp.status == 200, (
|
||||
f"chunk-context returned {chunk_resp.status}, body: "
|
||||
f"{await chunk_resp.text()}"
|
||||
)
|
||||
chunk_data = await chunk_resp.json()
|
||||
assert chunk_data.get("success") is True, f"Response: {chunk_data}"
|
||||
chunk_text = chunk_data.get("chunk_text") or ""
|
||||
assert chunk_text, f"Empty chunk_text in response: {chunk_data}"
|
||||
# The unique marker is in the indexed body, so it must appear in the
|
||||
# chunk text or in the surrounding context.
|
||||
combined = (
|
||||
chunk_text
|
||||
+ chunk_data.get("before_context", "")
|
||||
+ chunk_data.get("after_context", "")
|
||||
)
|
||||
assert re.search(re.escape(unique_term), combined), (
|
||||
f"Unique term {unique_term} missing from chunk+context: "
|
||||
f"chunk_text={chunk_text!r}, before={chunk_data.get('before_context')!r}, "
|
||||
f"after={chunk_data.get('after_context')!r}"
|
||||
)
|
||||
finally:
|
||||
try:
|
||||
if note_id is not None:
|
||||
auth_header = _build_basic_auth_header(username, password)
|
||||
async with create_mcp_client_session(
|
||||
url="http://localhost:8003/mcp",
|
||||
headers={"Authorization": auth_header},
|
||||
client_name="Alice Chunk Context Cleanup",
|
||||
) as mcp_client:
|
||||
await mcp_client.call_tool(
|
||||
"nc_notes_delete_note", {"note_id": note_id}
|
||||
)
|
||||
except Exception as cleanup_err:
|
||||
logger.warning(f"Cleanup failed for note {note_id}: {cleanup_err}")
|
||||
await context.close()
|
||||
|
||||
|
||||
@pytest.mark.timeout(60)
|
||||
async def test_chunk_context_endpoint_requires_authentication():
|
||||
"""Direct HTTP hit at /api/v1/chunk-context without a bearer must 401."""
|
||||
async with httpx.AsyncClient() as client:
|
||||
response = await client.get(
|
||||
"http://localhost:8003/api/v1/chunk-context",
|
||||
params={
|
||||
"doc_type": "note",
|
||||
"doc_id": "1",
|
||||
"start": "0",
|
||||
"end": "10",
|
||||
},
|
||||
)
|
||||
assert response.status_code == 401, (
|
||||
f"Expected 401 without auth, got {response.status_code}: {response.text}"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.timeout(60)
|
||||
async def test_chunk_context_endpoint_rejects_invalid_bearer():
|
||||
"""A syntactically-valid-but-unverifiable bearer must not 500.
|
||||
|
||||
The NotProvisionedError path (handler reached but no stored app password)
|
||||
is covered by the corresponding unit test. This check guards the other
|
||||
rejection path: token validation fails upfront at
|
||||
`validate_token_and_get_user`, which must turn into a clean 401/404 from
|
||||
the HTTP layer, not an opaque 500.
|
||||
"""
|
||||
async with httpx.AsyncClient() as client:
|
||||
response = await client.get(
|
||||
"http://localhost:8003/api/v1/chunk-context",
|
||||
params={
|
||||
"doc_type": "note",
|
||||
"doc_id": "1",
|
||||
"start": "0",
|
||||
"end": "10",
|
||||
},
|
||||
headers={"Authorization": "Bearer invalid.token.value"},
|
||||
)
|
||||
assert response.status_code in (401, 404), (
|
||||
f"Expected 401/404 for invalid bearer, got {response.status_code}: "
|
||||
f"{response.text}"
|
||||
)
|
||||
@@ -0,0 +1,279 @@
|
||||
"""
|
||||
Unit tests for Management API chunk-context endpoint.
|
||||
|
||||
Tests the /api/v1/chunk-context endpoint focusing on:
|
||||
- Parameter validation (doc_type, doc_id, start, end, context)
|
||||
- OAuth token validation
|
||||
- Nextcloud credential path (must use get_user_client_basic_auth, not
|
||||
NextcloudClient.from_token — see regression in api/visualization.py)
|
||||
- Error handling (missing params, invalid ranges, missing credentials)
|
||||
"""
|
||||
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
from starlette.applications import Starlette
|
||||
from starlette.routing import Route
|
||||
from starlette.testclient import TestClient
|
||||
|
||||
from nextcloud_mcp_server.api.visualization import get_chunk_context
|
||||
from nextcloud_mcp_server.vector.oauth_sync import NotProvisionedError
|
||||
|
||||
pytestmark = pytest.mark.unit
|
||||
|
||||
|
||||
def create_test_app():
|
||||
"""Create a test Starlette app with the chunk-context endpoint."""
|
||||
app = Starlette(
|
||||
routes=[
|
||||
Route("/api/v1/chunk-context", get_chunk_context, methods=["GET"]),
|
||||
]
|
||||
)
|
||||
app.state.oauth_context = {"config": {"nextcloud_host": "http://localhost:8080"}}
|
||||
return app
|
||||
|
||||
|
||||
def _make_mock_chunk_context(chunk_text="chunk", before="before", after="after"):
|
||||
"""Mock the ChunkContext dataclass with enough fields for the handler."""
|
||||
ctx = MagicMock()
|
||||
ctx.chunk_text = chunk_text
|
||||
ctx.before_context = before
|
||||
ctx.after_context = after
|
||||
ctx.has_before_truncation = False
|
||||
ctx.has_after_truncation = False
|
||||
ctx.page_number = None
|
||||
ctx.chunk_index = 0
|
||||
ctx.total_chunks = 1
|
||||
return ctx
|
||||
|
||||
|
||||
def _make_mock_nc_client():
|
||||
"""Mock NextcloudClient that supports `async with`."""
|
||||
mock_client = MagicMock()
|
||||
mock_client.__aenter__ = AsyncMock(return_value=mock_client)
|
||||
mock_client.__aexit__ = AsyncMock(return_value=None)
|
||||
return mock_client
|
||||
|
||||
|
||||
class TestChunkContextParameterValidation:
|
||||
"""Tests for parameter validation in the chunk-context endpoint."""
|
||||
|
||||
def test_missing_params_returns_400(self):
|
||||
with patch(
|
||||
"nextcloud_mcp_server.api.visualization.validate_token_and_get_user",
|
||||
new_callable=AsyncMock,
|
||||
return_value=("testuser", True),
|
||||
):
|
||||
app = create_test_app()
|
||||
client = TestClient(app)
|
||||
# Missing end
|
||||
response = client.get(
|
||||
"/api/v1/chunk-context?doc_type=note&doc_id=1&start=0",
|
||||
headers={"Authorization": "Bearer test-token"},
|
||||
)
|
||||
assert response.status_code == 400
|
||||
data = response.json()
|
||||
assert data["success"] is False
|
||||
assert "required parameters" in data["error"].lower()
|
||||
|
||||
def test_end_less_than_or_equal_start_returns_400(self):
|
||||
with patch(
|
||||
"nextcloud_mcp_server.api.visualization.validate_token_and_get_user",
|
||||
new_callable=AsyncMock,
|
||||
return_value=("testuser", True),
|
||||
):
|
||||
app = create_test_app()
|
||||
client = TestClient(app)
|
||||
response = client.get(
|
||||
"/api/v1/chunk-context?doc_type=note&doc_id=1&start=100&end=100",
|
||||
headers={"Authorization": "Bearer test-token"},
|
||||
)
|
||||
assert response.status_code == 400
|
||||
data = response.json()
|
||||
assert data["success"] is False
|
||||
assert "end must be greater than start" in data["error"].lower()
|
||||
|
||||
def test_non_numeric_start_returns_400(self):
|
||||
with patch(
|
||||
"nextcloud_mcp_server.api.visualization.validate_token_and_get_user",
|
||||
new_callable=AsyncMock,
|
||||
return_value=("testuser", True),
|
||||
):
|
||||
app = create_test_app()
|
||||
client = TestClient(app)
|
||||
response = client.get(
|
||||
"/api/v1/chunk-context?doc_type=note&doc_id=1&start=abc&end=10",
|
||||
headers={"Authorization": "Bearer test-token"},
|
||||
)
|
||||
assert response.status_code == 400
|
||||
|
||||
|
||||
class TestChunkContextTokenValidation:
|
||||
"""Tests for OAuth token validation."""
|
||||
|
||||
def test_missing_token_returns_401(self):
|
||||
with patch(
|
||||
"nextcloud_mcp_server.api.visualization.validate_token_and_get_user",
|
||||
new_callable=AsyncMock,
|
||||
side_effect=ValueError("Missing Authorization header"),
|
||||
):
|
||||
app = create_test_app()
|
||||
client = TestClient(app)
|
||||
response = client.get(
|
||||
"/api/v1/chunk-context?doc_type=note&doc_id=1&start=0&end=10"
|
||||
)
|
||||
assert response.status_code == 401
|
||||
data = response.json()
|
||||
assert data["error"] == "Unauthorized"
|
||||
|
||||
def test_invalid_token_returns_401(self):
|
||||
with patch(
|
||||
"nextcloud_mcp_server.api.visualization.validate_token_and_get_user",
|
||||
new_callable=AsyncMock,
|
||||
side_effect=Exception("Token expired"),
|
||||
):
|
||||
app = create_test_app()
|
||||
client = TestClient(app)
|
||||
response = client.get(
|
||||
"/api/v1/chunk-context?doc_type=note&doc_id=1&start=0&end=10",
|
||||
headers={"Authorization": "Bearer invalid-token"},
|
||||
)
|
||||
assert response.status_code == 401
|
||||
|
||||
|
||||
class TestChunkContextCredentialPath:
|
||||
"""Regression tests: the handler MUST use get_user_client_basic_auth.
|
||||
|
||||
The earlier bug (reproduced in homelab 2026-04-22) forwarded the OAuth
|
||||
bearer directly to Nextcloud via NextcloudClient.from_token. That works in
|
||||
single-user env-BasicAuth mode but fails with 401 in multi-user BasicAuth
|
||||
mode because Nextcloud won't validate that bearer on the Notes API.
|
||||
"""
|
||||
|
||||
def test_successful_fetch_uses_app_password_client(self):
|
||||
"""Handler must build its NC client via get_user_client_basic_auth and
|
||||
return the chunk text from get_chunk_with_context."""
|
||||
mock_nc_client = _make_mock_nc_client()
|
||||
mock_ctx = _make_mock_chunk_context(
|
||||
chunk_text="hello world",
|
||||
before="before the chunk",
|
||||
after="after the chunk",
|
||||
)
|
||||
|
||||
with (
|
||||
patch(
|
||||
"nextcloud_mcp_server.api.visualization.validate_token_and_get_user",
|
||||
new_callable=AsyncMock,
|
||||
return_value=("testuser", True),
|
||||
),
|
||||
patch(
|
||||
"nextcloud_mcp_server.api.visualization.get_user_client_basic_auth",
|
||||
new_callable=AsyncMock,
|
||||
return_value=mock_nc_client,
|
||||
) as mock_basic_auth,
|
||||
patch(
|
||||
"nextcloud_mcp_server.api.visualization.get_chunk_with_context",
|
||||
new_callable=AsyncMock,
|
||||
return_value=mock_ctx,
|
||||
),
|
||||
):
|
||||
app = create_test_app()
|
||||
client = TestClient(app)
|
||||
response = client.get(
|
||||
"/api/v1/chunk-context?doc_type=note&doc_id=42&start=0&end=11",
|
||||
headers={"Authorization": "Bearer test-token"},
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
data = response.json()
|
||||
assert data["success"] is True
|
||||
assert data["chunk_text"] == "hello world"
|
||||
assert data["before_context"] == "before the chunk"
|
||||
assert data["after_context"] == "after the chunk"
|
||||
|
||||
# Regression guard: credential resolution must go through the
|
||||
# app-password helper, not NextcloudClient.from_token.
|
||||
mock_basic_auth.assert_awaited_once_with(
|
||||
"testuser", "http://localhost:8080"
|
||||
)
|
||||
|
||||
def test_not_provisioned_returns_401(self):
|
||||
"""If the user has no stored app password, the handler must surface
|
||||
NotProvisionedError as a clean 401 (not 500)."""
|
||||
with (
|
||||
patch(
|
||||
"nextcloud_mcp_server.api.visualization.validate_token_and_get_user",
|
||||
new_callable=AsyncMock,
|
||||
return_value=("testuser", True),
|
||||
),
|
||||
patch(
|
||||
"nextcloud_mcp_server.api.visualization.get_user_client_basic_auth",
|
||||
new_callable=AsyncMock,
|
||||
side_effect=NotProvisionedError(
|
||||
"User testuser has not provisioned an app password."
|
||||
),
|
||||
),
|
||||
):
|
||||
app = create_test_app()
|
||||
client = TestClient(app)
|
||||
response = client.get(
|
||||
"/api/v1/chunk-context?doc_type=note&doc_id=1&start=0&end=10",
|
||||
headers={"Authorization": "Bearer test-token"},
|
||||
)
|
||||
assert response.status_code == 401
|
||||
data = response.json()
|
||||
assert data["success"] is False
|
||||
assert "app password" in data["error"].lower()
|
||||
|
||||
def test_chunk_fetch_returns_none_yields_404(self):
|
||||
"""When get_chunk_with_context returns None (doc missing / offsets
|
||||
out of range), the handler reports 404 with a structured error body."""
|
||||
mock_nc_client = _make_mock_nc_client()
|
||||
|
||||
with (
|
||||
patch(
|
||||
"nextcloud_mcp_server.api.visualization.validate_token_and_get_user",
|
||||
new_callable=AsyncMock,
|
||||
return_value=("testuser", True),
|
||||
),
|
||||
patch(
|
||||
"nextcloud_mcp_server.api.visualization.get_user_client_basic_auth",
|
||||
new_callable=AsyncMock,
|
||||
return_value=mock_nc_client,
|
||||
),
|
||||
patch(
|
||||
"nextcloud_mcp_server.api.visualization.get_chunk_with_context",
|
||||
new_callable=AsyncMock,
|
||||
return_value=None,
|
||||
),
|
||||
):
|
||||
app = create_test_app()
|
||||
client = TestClient(app)
|
||||
response = client.get(
|
||||
"/api/v1/chunk-context?doc_type=note&doc_id=999&start=0&end=10",
|
||||
headers={"Authorization": "Bearer test-token"},
|
||||
)
|
||||
assert response.status_code == 404
|
||||
data = response.json()
|
||||
assert data["success"] is False
|
||||
assert "failed to fetch chunk context" in data["error"].lower()
|
||||
|
||||
|
||||
class TestChunkContextConfigErrors:
|
||||
"""Tests for configuration failure paths."""
|
||||
|
||||
def test_missing_nextcloud_host_config(self):
|
||||
with patch(
|
||||
"nextcloud_mcp_server.api.visualization.validate_token_and_get_user",
|
||||
new_callable=AsyncMock,
|
||||
return_value=("testuser", True),
|
||||
):
|
||||
app = create_test_app()
|
||||
app.state.oauth_context = {"config": {"nextcloud_host": ""}}
|
||||
client = TestClient(app)
|
||||
response = client.get(
|
||||
"/api/v1/chunk-context?doc_type=note&doc_id=1&start=0&end=10",
|
||||
headers={"Authorization": "Bearer test-token"},
|
||||
)
|
||||
# Handler wraps ValueError via _sanitize_error_for_client → 500
|
||||
assert response.status_code == 500
|
||||
@@ -72,10 +72,6 @@ class TestPdfPreviewParameterValidation:
|
||||
new_callable=AsyncMock,
|
||||
return_value=("testuser", True),
|
||||
),
|
||||
patch(
|
||||
"nextcloud_mcp_server.api.visualization.extract_bearer_token",
|
||||
return_value="test-token",
|
||||
),
|
||||
):
|
||||
app = create_test_app()
|
||||
client = TestClient(app)
|
||||
@@ -97,10 +93,6 @@ class TestPdfPreviewParameterValidation:
|
||||
new_callable=AsyncMock,
|
||||
return_value=("testuser", True),
|
||||
),
|
||||
patch(
|
||||
"nextcloud_mcp_server.api.visualization.extract_bearer_token",
|
||||
return_value="test-token",
|
||||
),
|
||||
):
|
||||
app = create_test_app()
|
||||
client = TestClient(app)
|
||||
@@ -130,10 +122,6 @@ class TestPdfPreviewParameterValidation:
|
||||
new_callable=AsyncMock,
|
||||
return_value=("testuser", True),
|
||||
),
|
||||
patch(
|
||||
"nextcloud_mcp_server.api.visualization.extract_bearer_token",
|
||||
return_value="test-token",
|
||||
),
|
||||
):
|
||||
app = create_test_app()
|
||||
client = TestClient(app)
|
||||
@@ -163,10 +151,6 @@ class TestPdfPreviewParameterValidation:
|
||||
new_callable=AsyncMock,
|
||||
return_value=("testuser", True),
|
||||
),
|
||||
patch(
|
||||
"nextcloud_mcp_server.api.visualization.extract_bearer_token",
|
||||
return_value="test-token",
|
||||
),
|
||||
):
|
||||
app = create_test_app()
|
||||
client = TestClient(app)
|
||||
@@ -240,11 +224,8 @@ class TestPdfPreviewRendering:
|
||||
return_value=("testuser", True),
|
||||
),
|
||||
patch(
|
||||
"nextcloud_mcp_server.api.visualization.extract_bearer_token",
|
||||
return_value="test-token",
|
||||
),
|
||||
patch(
|
||||
"nextcloud_mcp_server.client.NextcloudClient.from_token",
|
||||
"nextcloud_mcp_server.api.visualization.get_user_client_basic_auth",
|
||||
new_callable=AsyncMock,
|
||||
return_value=mock_nc_client,
|
||||
),
|
||||
):
|
||||
@@ -289,11 +270,8 @@ class TestPdfPreviewRendering:
|
||||
return_value=("testuser", True),
|
||||
),
|
||||
patch(
|
||||
"nextcloud_mcp_server.api.visualization.extract_bearer_token",
|
||||
return_value="test-token",
|
||||
),
|
||||
patch(
|
||||
"nextcloud_mcp_server.client.NextcloudClient.from_token",
|
||||
"nextcloud_mcp_server.api.visualization.get_user_client_basic_auth",
|
||||
new_callable=AsyncMock,
|
||||
return_value=mock_nc_client,
|
||||
),
|
||||
):
|
||||
@@ -329,11 +307,8 @@ class TestPdfPreviewRendering:
|
||||
return_value=("testuser", True),
|
||||
),
|
||||
patch(
|
||||
"nextcloud_mcp_server.api.visualization.extract_bearer_token",
|
||||
return_value="test-token",
|
||||
),
|
||||
patch(
|
||||
"nextcloud_mcp_server.client.NextcloudClient.from_token",
|
||||
"nextcloud_mcp_server.api.visualization.get_user_client_basic_auth",
|
||||
new_callable=AsyncMock,
|
||||
return_value=mock_nc_client,
|
||||
),
|
||||
):
|
||||
@@ -368,11 +343,8 @@ class TestPdfPreviewRendering:
|
||||
return_value=("testuser", True),
|
||||
),
|
||||
patch(
|
||||
"nextcloud_mcp_server.api.visualization.extract_bearer_token",
|
||||
return_value="test-token",
|
||||
),
|
||||
patch(
|
||||
"nextcloud_mcp_server.client.NextcloudClient.from_token",
|
||||
"nextcloud_mcp_server.api.visualization.get_user_client_basic_auth",
|
||||
new_callable=AsyncMock,
|
||||
return_value=mock_nc_client,
|
||||
),
|
||||
):
|
||||
@@ -412,11 +384,8 @@ class TestPdfPreviewEdgeCases:
|
||||
return_value=("testuser", True),
|
||||
),
|
||||
patch(
|
||||
"nextcloud_mcp_server.api.visualization.extract_bearer_token",
|
||||
return_value="test-token",
|
||||
),
|
||||
patch(
|
||||
"nextcloud_mcp_server.client.NextcloudClient.from_token",
|
||||
"nextcloud_mcp_server.api.visualization.get_user_client_basic_auth",
|
||||
new_callable=AsyncMock,
|
||||
return_value=mock_nc_client,
|
||||
),
|
||||
):
|
||||
@@ -442,10 +411,6 @@ class TestPdfPreviewEdgeCases:
|
||||
new_callable=AsyncMock,
|
||||
return_value=("testuser", True),
|
||||
),
|
||||
patch(
|
||||
"nextcloud_mcp_server.api.visualization.extract_bearer_token",
|
||||
return_value="test-token",
|
||||
),
|
||||
):
|
||||
app = create_test_app()
|
||||
# Override with empty config
|
||||
@@ -481,11 +446,8 @@ class TestPdfPreviewEdgeCases:
|
||||
return_value=("testuser", True),
|
||||
),
|
||||
patch(
|
||||
"nextcloud_mcp_server.api.visualization.extract_bearer_token",
|
||||
return_value="test-token",
|
||||
),
|
||||
patch(
|
||||
"nextcloud_mcp_server.client.NextcloudClient.from_token",
|
||||
"nextcloud_mcp_server.api.visualization.get_user_client_basic_auth",
|
||||
new_callable=AsyncMock,
|
||||
return_value=mock_nc_client,
|
||||
),
|
||||
):
|
||||
@@ -523,11 +485,8 @@ class TestPdfPreviewEdgeCases:
|
||||
return_value=("testuser", True),
|
||||
),
|
||||
patch(
|
||||
"nextcloud_mcp_server.api.visualization.extract_bearer_token",
|
||||
return_value="test-token",
|
||||
),
|
||||
patch(
|
||||
"nextcloud_mcp_server.client.NextcloudClient.from_token",
|
||||
"nextcloud_mcp_server.api.visualization.get_user_client_basic_auth",
|
||||
new_callable=AsyncMock,
|
||||
return_value=mock_nc_client,
|
||||
),
|
||||
):
|
||||
@@ -560,10 +519,6 @@ class TestPdfPreviewSecurityValidation:
|
||||
new_callable=AsyncMock,
|
||||
return_value=("testuser", True),
|
||||
),
|
||||
patch(
|
||||
"nextcloud_mcp_server.api.visualization.extract_bearer_token",
|
||||
return_value="test-token",
|
||||
),
|
||||
):
|
||||
app = create_test_app()
|
||||
client = TestClient(app)
|
||||
@@ -610,11 +565,8 @@ class TestPdfPreviewSecurityValidation:
|
||||
return_value=("testuser", True),
|
||||
),
|
||||
patch(
|
||||
"nextcloud_mcp_server.api.visualization.extract_bearer_token",
|
||||
return_value="test-token",
|
||||
),
|
||||
patch(
|
||||
"nextcloud_mcp_server.client.NextcloudClient.from_token",
|
||||
"nextcloud_mcp_server.api.visualization.get_user_client_basic_auth",
|
||||
new_callable=AsyncMock,
|
||||
return_value=mock_nc_client,
|
||||
),
|
||||
):
|
||||
@@ -652,11 +604,8 @@ class TestPdfPreviewSecurityValidation:
|
||||
return_value=("testuser", True),
|
||||
),
|
||||
patch(
|
||||
"nextcloud_mcp_server.api.visualization.extract_bearer_token",
|
||||
return_value="test-token",
|
||||
),
|
||||
patch(
|
||||
"nextcloud_mcp_server.client.NextcloudClient.from_token",
|
||||
"nextcloud_mcp_server.api.visualization.get_user_client_basic_auth",
|
||||
new_callable=AsyncMock,
|
||||
return_value=mock_nc_client,
|
||||
),
|
||||
):
|
||||
@@ -696,11 +645,8 @@ class TestPdfPreviewSecurityValidation:
|
||||
return_value=("testuser", True),
|
||||
),
|
||||
patch(
|
||||
"nextcloud_mcp_server.api.visualization.extract_bearer_token",
|
||||
return_value="test-token",
|
||||
),
|
||||
patch(
|
||||
"nextcloud_mcp_server.client.NextcloudClient.from_token",
|
||||
"nextcloud_mcp_server.api.visualization.get_user_client_basic_auth",
|
||||
new_callable=AsyncMock,
|
||||
return_value=mock_nc_client,
|
||||
),
|
||||
):
|
||||
|
||||
Reference in New Issue
Block a user