feat!: require explicit user on every tenant-data endpoint
Remove the implicit jpmschweitzer default tenant (DEFAULT_USER) from src/core/multi_tenancy.py and every endpoint and request model that inherited it (~40 endpoints across /query, /wiki, /vector, /graph, /ingest, /volatile, /documents, /stats, /rag). - Add validate_required_user() + RequiredUser pydantic type in multi_tenancy and a shared require_user FastAPI dependency (RequiredUserQuery) that rejects missing, empty, and whitespace-only users with 422, following the /maintenance/* pattern. - Wiki page create / smart-create / dossier request models now require user (no fallback in wiki_service). - /maintenance/cleanup/test-data derives the tenant from the page path instead of using the production tenant collection. - Wiki.js change listener skips changes when no tenant user can be derived from the notification email instead of defaulting to the production tenant. - Consolidation service internal helpers no longer default to the production tenant. - Tool catalog marks user as required with honest descriptions. - OpenAPI descriptions updated honestly; CHANGELOG notes that callers (tatlock, Scheduler ingest tasks) must now send explicit user. - Offline tests: 422 coverage for query/body endpoints, required-user validator tests; updated legacy tests that assumed a default tenant. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -7,8 +7,8 @@ from src.core.multi_tenancy import (
|
||||
get_wikijs_namespace,
|
||||
get_neo4j_user_label,
|
||||
validate_user_id,
|
||||
validate_required_user,
|
||||
is_path_in_user_namespace,
|
||||
DEFAULT_USER
|
||||
)
|
||||
|
||||
|
||||
@@ -46,8 +46,8 @@ class TestQdrantCollectionName:
|
||||
def test_email_user(self):
|
||||
assert get_qdrant_collection_name("john@example.com") == "library_desk_john_at_example_com"
|
||||
|
||||
def test_default_user(self):
|
||||
assert get_qdrant_collection_name(DEFAULT_USER) == f"library_desk_{DEFAULT_USER}"
|
||||
def test_test_tenant(self):
|
||||
assert get_qdrant_collection_name("llm_tester") == "library_desk_llm_tester"
|
||||
|
||||
|
||||
class TestWikijsNamespace:
|
||||
@@ -99,6 +99,41 @@ class TestValidateUserId:
|
||||
assert validate_user_id("___") is False
|
||||
|
||||
|
||||
class TestValidateRequiredUser:
|
||||
"""Test the required-user validator (no default tenant)."""
|
||||
|
||||
def test_no_default_user_constant(self):
|
||||
"""The DEFAULT_USER escape hatch must not exist anymore."""
|
||||
import src.core.multi_tenancy as mt
|
||||
assert not hasattr(mt, "DEFAULT_USER")
|
||||
|
||||
def test_valid_user_returned(self):
|
||||
assert validate_required_user("llm_tester") == "llm_tester"
|
||||
|
||||
def test_valid_user_stripped(self):
|
||||
assert validate_required_user(" llm_tester ") == "llm_tester"
|
||||
|
||||
def test_empty_rejected(self):
|
||||
with pytest.raises(ValueError):
|
||||
validate_required_user("")
|
||||
|
||||
def test_whitespace_rejected(self):
|
||||
with pytest.raises(ValueError):
|
||||
validate_required_user(" ")
|
||||
|
||||
def test_none_rejected(self):
|
||||
with pytest.raises(ValueError):
|
||||
validate_required_user(None)
|
||||
|
||||
def test_no_alphanumeric_rejected(self):
|
||||
with pytest.raises(ValueError):
|
||||
validate_required_user("___")
|
||||
|
||||
def test_too_long_rejected(self):
|
||||
with pytest.raises(ValueError):
|
||||
validate_required_user("a" * 101)
|
||||
|
||||
|
||||
class TestPathInNamespace:
|
||||
"""Test path namespace checking."""
|
||||
|
||||
|
||||
@@ -37,12 +37,19 @@ class TestRAGSearchModels:
|
||||
"""Tests for RAG search Pydantic models."""
|
||||
|
||||
def test_search_request_defaults(self):
|
||||
"""Test RAGSearchRequest with default values."""
|
||||
request = RAGSearchRequest(query="test query")
|
||||
"""Test RAGSearchRequest defaults (user is required, no default tenant)."""
|
||||
request = RAGSearchRequest(query="test query", user="llm_tester")
|
||||
|
||||
assert request.query == "test query"
|
||||
assert request.search_type == SearchType.WEB
|
||||
assert request.limit == 10
|
||||
assert request.user == "llm_tester"
|
||||
|
||||
def test_search_request_requires_user(self):
|
||||
"""A request without an explicit user must be rejected."""
|
||||
import pytest
|
||||
with pytest.raises(ValueError):
|
||||
RAGSearchRequest(query="test query")
|
||||
|
||||
def test_search_request_custom_values(self):
|
||||
"""Test RAGSearchRequest with custom values."""
|
||||
|
||||
@@ -0,0 +1,117 @@
|
||||
"""
|
||||
Offline unit tests: every tenant-data endpoint must REQUIRE an explicit user.
|
||||
|
||||
A request without a user (query param or body field) must be rejected with
|
||||
422 before any service is touched. Empty/whitespace users are also rejected.
|
||||
|
||||
No external services are contacted: validation failures short-circuit the
|
||||
request before the endpoint body executes.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from src.main import app
|
||||
from src.core.dependencies import verify_api_key
|
||||
|
||||
|
||||
@pytest.fixture(scope="module")
|
||||
def client():
|
||||
"""TestClient with API-key auth stubbed out (no lifespan startup)."""
|
||||
app.dependency_overrides[verify_api_key] = lambda: "test-key"
|
||||
try:
|
||||
# No context manager: startup/lifespan events are NOT triggered,
|
||||
# so no connections to external services are attempted.
|
||||
yield TestClient(app)
|
||||
finally:
|
||||
app.dependency_overrides.pop(verify_api_key, None)
|
||||
|
||||
|
||||
QUERY_PARAM_ENDPOINTS = [
|
||||
("GET", "/stats"),
|
||||
("POST", "/query/semantic?query=test"),
|
||||
("POST", "/query/graph?query=MATCH%20(n)%20RETURN%20n"),
|
||||
("GET", "/wiki/pages"),
|
||||
("GET", "/wiki/pages/1"),
|
||||
("PUT", "/wiki/pages/1"),
|
||||
("DELETE", "/wiki/pages/1"),
|
||||
("GET", "/wiki/search?q=test"),
|
||||
("GET", "/wiki/dossiers"),
|
||||
("POST", "/vector/update-from-page/1"),
|
||||
("DELETE", "/vector/pages/1"),
|
||||
("GET", "/graph/nodes"),
|
||||
("POST", "/graph/update-from-page/1"),
|
||||
("POST", "/graph/generate-entity-pages"),
|
||||
("POST", "/ingest/all"),
|
||||
("GET", "/volatile/stats"),
|
||||
("GET", "/volatile/search?q=test"),
|
||||
("POST", "/volatile/store?namespace=weather&key=test"),
|
||||
("GET", "/volatile/weather/rotterdam"),
|
||||
("DELETE", "/volatile/weather/rotterdam"),
|
||||
("POST", "/documents/webhook-simple?doc_url=http://x/documents/1/"),
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestUserQueryParamRequired:
|
||||
"""Endpoints with a user query parameter must 422 without it."""
|
||||
|
||||
@pytest.mark.parametrize("method,path", QUERY_PARAM_ENDPOINTS)
|
||||
def test_missing_user_is_422(self, client, method, path):
|
||||
response = client.request(method, path, json={})
|
||||
assert response.status_code == 422, (
|
||||
f"{method} {path} returned {response.status_code}, expected 422"
|
||||
)
|
||||
|
||||
@pytest.mark.parametrize("blank", ["", " ", "%20%20"])
|
||||
def test_blank_user_is_422(self, client, blank):
|
||||
response = client.get(f"/wiki/pages?user={blank}")
|
||||
assert response.status_code == 422
|
||||
|
||||
def test_hybrid_query_missing_user_is_422(self, client):
|
||||
response = client.post("/query/hybrid", json={"query": "test"})
|
||||
assert response.status_code == 422
|
||||
|
||||
def test_hybrid_query_whitespace_user_is_422(self, client):
|
||||
response = client.post("/query/hybrid?user=%20", json={"query": "test"})
|
||||
assert response.status_code == 422
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestUserBodyFieldRequired:
|
||||
"""Request models with a user field must reject missing/blank values."""
|
||||
|
||||
def test_ingest_page_missing_user_is_422(self, client):
|
||||
response = client.post("/ingest/page", json={"page_id": 1})
|
||||
assert response.status_code == 422
|
||||
|
||||
def test_ingest_page_blank_user_is_422(self, client):
|
||||
response = client.post("/ingest/page", json={"page_id": 1, "user": " "})
|
||||
assert response.status_code == 422
|
||||
|
||||
def test_ingest_batch_missing_user_is_422(self, client):
|
||||
response = client.post("/ingest/batch", json={"page_ids": [1]})
|
||||
assert response.status_code == 422
|
||||
|
||||
def test_wiki_create_page_missing_user_is_422(self, client):
|
||||
response = client.post(
|
||||
"/wiki/pages",
|
||||
json={"title": "T", "path": "/t", "content": "c"},
|
||||
)
|
||||
assert response.status_code == 422
|
||||
|
||||
def test_wiki_smart_create_missing_user_is_422(self, client):
|
||||
response = client.post("/wiki/pages/smart-create", json={"topic": "T"})
|
||||
assert response.status_code == 422
|
||||
|
||||
def test_vector_search_missing_user_is_422(self, client):
|
||||
response = client.post("/vector/search", json={"query": "test"})
|
||||
assert response.status_code == 422
|
||||
|
||||
def test_graph_query_missing_user_is_422(self, client):
|
||||
response = client.post("/graph/query", json={"query": "MATCH (n) RETURN n"})
|
||||
assert response.status_code == 422
|
||||
|
||||
def test_rag_search_missing_user_is_422(self, client):
|
||||
response = client.post("/rag/search", json={"query": "test"})
|
||||
assert response.status_code == 422
|
||||
+20
-10
@@ -26,15 +26,20 @@ class TestWikiSmartCreateRequest:
|
||||
"""Tests for WikiSmartCreateRequest model validation."""
|
||||
|
||||
def test_minimal_request(self):
|
||||
"""Test request with only required field."""
|
||||
request = WikiSmartCreateRequest(topic="Docker containers")
|
||||
"""Test request with only required fields (topic AND user)."""
|
||||
request = WikiSmartCreateRequest(topic="Docker containers", user="llm_tester")
|
||||
assert request.topic == "Docker containers"
|
||||
assert request.path is None
|
||||
assert request.tags == []
|
||||
assert request.user is None
|
||||
assert request.user == "llm_tester"
|
||||
assert request.include_web_research is True
|
||||
assert request.include_wiki_search is True
|
||||
|
||||
def test_user_is_required(self):
|
||||
"""A request without an explicit user must be rejected."""
|
||||
with pytest.raises(ValueError):
|
||||
WikiSmartCreateRequest(topic="Docker containers")
|
||||
|
||||
def test_full_request(self):
|
||||
"""Test request with all fields."""
|
||||
request = WikiSmartCreateRequest(
|
||||
@@ -68,7 +73,8 @@ class TestWikiSmartCreateRequest:
|
||||
"""Test that path without leading slash gets one added."""
|
||||
request = WikiSmartCreateRequest(
|
||||
topic="Test",
|
||||
path="technology/test"
|
||||
path="technology/test",
|
||||
user="llm_tester"
|
||||
)
|
||||
assert request.path == "/technology/test"
|
||||
|
||||
@@ -76,7 +82,8 @@ class TestWikiSmartCreateRequest:
|
||||
"""Test that trailing slash is removed."""
|
||||
request = WikiSmartCreateRequest(
|
||||
topic="Test",
|
||||
path="/technology/test/"
|
||||
path="/technology/test/",
|
||||
user="llm_tester"
|
||||
)
|
||||
assert request.path == "/technology/test"
|
||||
|
||||
@@ -84,7 +91,8 @@ class TestWikiSmartCreateRequest:
|
||||
"""Test that duplicate tags are removed."""
|
||||
request = WikiSmartCreateRequest(
|
||||
topic="Test",
|
||||
tags=["devops", "devops", "containers", "devops"]
|
||||
tags=["devops", "devops", "containers", "devops"],
|
||||
user="llm_tester"
|
||||
)
|
||||
assert len(request.tags) == 2
|
||||
assert "devops" in request.tags
|
||||
@@ -94,7 +102,8 @@ class TestWikiSmartCreateRequest:
|
||||
"""Test that tag whitespace is cleaned."""
|
||||
request = WikiSmartCreateRequest(
|
||||
topic="Test",
|
||||
tags=[" devops ", "containers", " ", ""]
|
||||
tags=[" devops ", "containers", " ", ""],
|
||||
user="llm_tester"
|
||||
)
|
||||
assert "devops" in request.tags
|
||||
assert "containers" in request.tags
|
||||
@@ -536,7 +545,8 @@ class TestSmartCreateEndpoint:
|
||||
# For now, we test the model validation
|
||||
request = WikiSmartCreateRequest(
|
||||
topic="Test Topic",
|
||||
tags=["test"]
|
||||
tags=["test"],
|
||||
user="llm_tester"
|
||||
)
|
||||
assert request.topic == "Test Topic"
|
||||
|
||||
@@ -546,8 +556,8 @@ class TestSmartCreateEndpoint:
|
||||
WikiSmartCreateRequest(topic="")
|
||||
|
||||
def test_request_accepts_minimal_input(self):
|
||||
"""Test that only topic is required."""
|
||||
request = WikiSmartCreateRequest(topic="Minimal test")
|
||||
"""Test that topic and user are the only required fields."""
|
||||
request = WikiSmartCreateRequest(topic="Minimal test", user="llm_tester")
|
||||
assert request.topic == "Minimal test"
|
||||
assert request.include_web_research is True # default
|
||||
assert request.include_wiki_search is True # default
|
||||
|
||||
@@ -231,9 +231,8 @@ class TestWikiChangeListener:
|
||||
'UPDATE:123:invaliduser'
|
||||
)
|
||||
|
||||
# Should use default user
|
||||
call_args = mock_process.call_args[1]
|
||||
assert call_args['user'] == 'jpmschweitzer'
|
||||
# No default tenant: change must be skipped entirely
|
||||
mock_process.assert_not_awaited()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_process_page_delete_calls_cleanup(self, listener):
|
||||
|
||||
Reference in New Issue
Block a user