feat(tenant): add tenant_domains allowlist and /v1/domains management API
Why: - Domain values are denormalized into every Qdrant point payload. Without validation, an unregistered or typo'd domain (e.g. "fier" for "fire") silently creates a new partition that retrieval never queries — the file ends up invisible rather than rejected. Tenants also need independently sized domain sets (one may run 14 insurance lines, another 6), which rules out an enum. Changes: - tenant_domains table (migration 41335d162de8) + repository, unique on (tenant_id, domain). - src/application/domains/: ensure_domain_allowed() is the strict-allowlist check now run inside upload_source_file()'s first transaction, before any MinIO object, job row, or Qdrant point is written. - /v1/domains (list/create/patch/disable/enable) gated on its own domains:read/domains:write scopes, deliberately separate from files:write so an upload key cannot create partitions. domain itself is immutable (denormalized into every point payload); only display_name is editable. Disable blocks new uploads without touching already-indexed points. Impact: - BREAKING: POST /v1/files now rejects any domain without an active tenant_domains row (400, unknown_domain). A domain must be created via POST /v1/domains before the first upload to it.
This commit is contained in:
193
tests/integration/postgres/test_domains_api.py
Normal file
193
tests/integration/postgres/test_domains_api.py
Normal file
@@ -0,0 +1,193 @@
|
||||
"""`/v1/domains` over HTTP, against real Postgres (ADR-0008, ADR-0009).
|
||||
|
||||
The property worth testing at this layer is the scope boundary: an upload key
|
||||
must not be able to create domains, or the allowlist stops preventing anything.
|
||||
"""
|
||||
|
||||
from collections.abc import AsyncIterator
|
||||
|
||||
import pytest
|
||||
import pytest_asyncio
|
||||
from asgi_lifespan import LifespanManager
|
||||
from httpx import ASGITransport, AsyncClient
|
||||
from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker
|
||||
|
||||
from src.bootstrap.dependencies import get_sessionmaker
|
||||
from src.config import Settings
|
||||
from src.main import create_app
|
||||
from tests.support.factories import create_api_key, create_tenant, create_tenant_domain
|
||||
|
||||
pytestmark = [
|
||||
pytest.mark.integration,
|
||||
pytest.mark.postgres,
|
||||
pytest.mark.asyncio(loop_scope="session"),
|
||||
]
|
||||
|
||||
|
||||
@pytest_asyncio.fixture(loop_scope="session")
|
||||
async def api_client(
|
||||
settings: Settings, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> AsyncIterator[AsyncClient]:
|
||||
"""The real app, with only the database swapped for the test's session
|
||||
factory -- so routing, auth, scopes, and the error envelope are exercised.
|
||||
"""
|
||||
app = create_app(settings)
|
||||
app.dependency_overrides[get_sessionmaker] = lambda: db_sessionmaker
|
||||
async with (
|
||||
LifespanManager(app) as manager,
|
||||
AsyncClient(transport=ASGITransport(app=manager.app), base_url="http://test") as client,
|
||||
):
|
||||
yield client
|
||||
|
||||
|
||||
def _auth(token: str) -> dict[str, str]:
|
||||
return {"Authorization": f"Bearer {token}"}
|
||||
|
||||
|
||||
async def test_create_domain_returns_201_and_lists_it(
|
||||
api_client: AsyncClient, db_session: AsyncSession
|
||||
) -> None:
|
||||
tenant = await create_tenant(db_session)
|
||||
_, token = await create_api_key(
|
||||
db_session, tenant=tenant, scopes=["domains:read", "domains:write"]
|
||||
)
|
||||
await db_session.commit()
|
||||
|
||||
created = await api_client.post(
|
||||
"/v1/domains",
|
||||
json={"domain": "fire", "display_name": "Fire insurance"},
|
||||
headers=_auth(token),
|
||||
)
|
||||
listed = await api_client.get("/v1/domains", headers=_auth(token))
|
||||
|
||||
assert created.status_code == 201
|
||||
assert created.json()["domain"] == "fire"
|
||||
assert [item["domain"] for item in listed.json()["domains"]] == ["fire"]
|
||||
|
||||
|
||||
async def test_create_domain_requires_the_domains_write_scope(
|
||||
api_client: AsyncClient, db_session: AsyncSession
|
||||
) -> None:
|
||||
"""An upload key creating domains would defeat the allowlist entirely."""
|
||||
tenant = await create_tenant(db_session)
|
||||
_, token = await create_api_key(db_session, tenant=tenant, scopes=["files:write"])
|
||||
await db_session.commit()
|
||||
|
||||
response = await api_client.post(
|
||||
"/v1/domains",
|
||||
json={"domain": "fire", "display_name": "Fire"},
|
||||
headers=_auth(token),
|
||||
)
|
||||
|
||||
assert response.status_code == 403
|
||||
assert response.json()["error"]["code"] == "missing_scope"
|
||||
|
||||
|
||||
async def test_list_domains_never_shows_another_tenants_domains(
|
||||
api_client: AsyncClient, db_session: AsyncSession
|
||||
) -> None:
|
||||
owner = await create_tenant(db_session)
|
||||
other = await create_tenant(db_session)
|
||||
await create_tenant_domain(db_session, tenant=owner, domain="fire")
|
||||
_, other_token = await create_api_key(db_session, tenant=other, scopes=["domains:read"])
|
||||
await db_session.commit()
|
||||
|
||||
response = await api_client.get("/v1/domains", headers=_auth(other_token))
|
||||
|
||||
assert response.status_code == 200
|
||||
assert response.json()["domains"] == []
|
||||
|
||||
|
||||
async def test_create_domain_rejects_a_duplicate_with_409(
|
||||
api_client: AsyncClient, db_session: AsyncSession
|
||||
) -> None:
|
||||
tenant = await create_tenant(db_session)
|
||||
await create_tenant_domain(db_session, tenant=tenant, domain="fire")
|
||||
_, token = await create_api_key(db_session, tenant=tenant, scopes=["domains:write"])
|
||||
await db_session.commit()
|
||||
|
||||
response = await api_client.post(
|
||||
"/v1/domains",
|
||||
json={"domain": "fire", "display_name": "Fire"},
|
||||
headers=_auth(token),
|
||||
)
|
||||
|
||||
assert response.status_code == 409
|
||||
assert response.json()["error"]["code"] == "conflict"
|
||||
|
||||
|
||||
async def test_create_domain_rejects_a_malformed_key(
|
||||
api_client: AsyncClient, db_session: AsyncSession
|
||||
) -> None:
|
||||
tenant = await create_tenant(db_session)
|
||||
_, token = await create_api_key(db_session, tenant=tenant, scopes=["domains:write"])
|
||||
await db_session.commit()
|
||||
|
||||
response = await api_client.post(
|
||||
"/v1/domains",
|
||||
json={"domain": "Fire Insurance!", "display_name": "Fire"},
|
||||
headers=_auth(token),
|
||||
)
|
||||
|
||||
assert response.status_code == 422
|
||||
|
||||
|
||||
async def test_patch_domain_cannot_rename_the_key(
|
||||
api_client: AsyncClient, db_session: AsyncSession
|
||||
) -> None:
|
||||
"""`domain` is not part of the update schema -- it is denormalized into
|
||||
every point payload, so renaming it is a migration, not an edit.
|
||||
"""
|
||||
tenant = await create_tenant(db_session)
|
||||
await create_tenant_domain(db_session, tenant=tenant, domain="fire")
|
||||
_, token = await create_api_key(
|
||||
db_session, tenant=tenant, scopes=["domains:read", "domains:write"]
|
||||
)
|
||||
await db_session.commit()
|
||||
|
||||
response = await api_client.patch(
|
||||
"/v1/domains/fire",
|
||||
json={"display_name": "Fire & perils", "domain": "renamed"},
|
||||
headers=_auth(token),
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
assert response.json()["domain"] == "fire"
|
||||
assert response.json()["display_name"] == "Fire & perils"
|
||||
|
||||
|
||||
async def test_delete_domain_disables_it_without_removing_it(
|
||||
api_client: AsyncClient, db_session: AsyncSession
|
||||
) -> None:
|
||||
tenant = await create_tenant(db_session)
|
||||
await create_tenant_domain(db_session, tenant=tenant, domain="fire")
|
||||
_, token = await create_api_key(
|
||||
db_session, tenant=tenant, scopes=["domains:read", "domains:write"]
|
||||
)
|
||||
await db_session.commit()
|
||||
|
||||
deleted = await api_client.delete("/v1/domains/fire", headers=_auth(token))
|
||||
default_list = await api_client.get("/v1/domains", headers=_auth(token))
|
||||
full_list = await api_client.get(
|
||||
"/v1/domains", params={"include_disabled": True}, headers=_auth(token)
|
||||
)
|
||||
|
||||
assert deleted.status_code == 200
|
||||
assert deleted.json()["status"] == "disabled"
|
||||
assert default_list.json()["domains"] == []
|
||||
assert [item["domain"] for item in full_list.json()["domains"]] == ["fire"]
|
||||
|
||||
|
||||
async def test_patch_unknown_domain_returns_400(
|
||||
api_client: AsyncClient, db_session: AsyncSession
|
||||
) -> None:
|
||||
tenant = await create_tenant(db_session)
|
||||
_, token = await create_api_key(db_session, tenant=tenant, scopes=["domains:write"])
|
||||
await db_session.commit()
|
||||
|
||||
response = await api_client.patch(
|
||||
"/v1/domains/absent", json={"display_name": "x"}, headers=_auth(token)
|
||||
)
|
||||
|
||||
assert response.status_code == 400
|
||||
assert response.json()["error"]["code"] == "unknown_domain"
|
||||
184
tests/integration/postgres/test_domains_service.py
Normal file
184
tests/integration/postgres/test_domains_service.py
Normal file
@@ -0,0 +1,184 @@
|
||||
"""Tenant-domain management and the upload-time allowlist (ADR-0009)."""
|
||||
|
||||
import pytest
|
||||
from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker
|
||||
|
||||
from src.application.domains import (
|
||||
DomainAlreadyExistsError,
|
||||
UnknownDomainError,
|
||||
create_domain,
|
||||
ensure_domain_allowed,
|
||||
list_domains,
|
||||
set_domain_status,
|
||||
update_domain,
|
||||
)
|
||||
from tests.support.factories import create_tenant, create_tenant_domain
|
||||
|
||||
pytestmark = [
|
||||
pytest.mark.integration,
|
||||
pytest.mark.postgres,
|
||||
pytest.mark.asyncio(loop_scope="session"),
|
||||
]
|
||||
|
||||
|
||||
async def test_ensure_domain_allowed_passes_for_a_registered_active_domain(
|
||||
db_session: AsyncSession,
|
||||
) -> None:
|
||||
tenant = await create_tenant(db_session)
|
||||
await create_tenant_domain(db_session, tenant=tenant, domain="fire")
|
||||
|
||||
await ensure_domain_allowed(db_session, tenant_id=tenant.id, domain="fire")
|
||||
|
||||
|
||||
async def test_ensure_domain_allowed_rejects_an_unregistered_domain(
|
||||
db_session: AsyncSession,
|
||||
) -> None:
|
||||
"""The typo case: `fier` must not silently become a new Qdrant partition."""
|
||||
tenant = await create_tenant(db_session)
|
||||
await create_tenant_domain(db_session, tenant=tenant, domain="fire")
|
||||
|
||||
with pytest.raises(UnknownDomainError, match="fier"):
|
||||
await ensure_domain_allowed(db_session, tenant_id=tenant.id, domain="fier")
|
||||
|
||||
|
||||
async def test_ensure_domain_allowed_rejects_a_disabled_domain(
|
||||
db_session: AsyncSession,
|
||||
) -> None:
|
||||
tenant = await create_tenant(db_session)
|
||||
await create_tenant_domain(db_session, tenant=tenant, domain="fire", status="disabled")
|
||||
|
||||
with pytest.raises(UnknownDomainError, match="disabled"):
|
||||
await ensure_domain_allowed(db_session, tenant_id=tenant.id, domain="fire")
|
||||
|
||||
|
||||
async def test_ensure_domain_allowed_rejects_another_tenants_domain(
|
||||
db_session: AsyncSession,
|
||||
) -> None:
|
||||
"""Domain lists are per-tenant; one tenant's `fire` is not another's."""
|
||||
owner = await create_tenant(db_session)
|
||||
other = await create_tenant(db_session)
|
||||
await create_tenant_domain(db_session, tenant=owner, domain="fire")
|
||||
|
||||
with pytest.raises(UnknownDomainError):
|
||||
await ensure_domain_allowed(db_session, tenant_id=other.id, domain="fire")
|
||||
|
||||
|
||||
async def test_tenants_hold_independent_domain_sets_of_different_sizes(
|
||||
db_session: AsyncSession, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> None:
|
||||
big = await create_tenant(db_session)
|
||||
small = await create_tenant(db_session)
|
||||
for index in range(14):
|
||||
await create_tenant_domain(db_session, tenant=big, domain=f"line-{index:02d}")
|
||||
for index in range(6):
|
||||
await create_tenant_domain(db_session, tenant=small, domain=f"line-{index:02d}")
|
||||
await db_session.commit()
|
||||
|
||||
assert len(await list_domains(db_sessionmaker, tenant_id=big.id)) == 14
|
||||
assert len(await list_domains(db_sessionmaker, tenant_id=small.id)) == 6
|
||||
|
||||
|
||||
async def test_create_domain_then_upload_is_allowed(
|
||||
db_session: AsyncSession, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> None:
|
||||
tenant = await create_tenant(db_session)
|
||||
await db_session.commit()
|
||||
|
||||
created = await create_domain(
|
||||
db_sessionmaker, tenant_id=tenant.id, domain="car", display_name="Car insurance"
|
||||
)
|
||||
|
||||
assert created.domain == "car"
|
||||
assert created.status == "active"
|
||||
async with db_sessionmaker() as session:
|
||||
await ensure_domain_allowed(session, tenant_id=tenant.id, domain="car")
|
||||
|
||||
|
||||
async def test_create_domain_rejects_a_duplicate_key_for_the_same_tenant(
|
||||
db_session: AsyncSession, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> None:
|
||||
tenant = await create_tenant(db_session)
|
||||
await create_tenant_domain(db_session, tenant=tenant, domain="fire")
|
||||
await db_session.commit()
|
||||
|
||||
with pytest.raises(DomainAlreadyExistsError):
|
||||
await create_domain(
|
||||
db_sessionmaker, tenant_id=tenant.id, domain="fire", display_name="Fire again"
|
||||
)
|
||||
|
||||
|
||||
async def test_create_domain_allows_the_same_key_for_different_tenants(
|
||||
db_session: AsyncSession, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> None:
|
||||
first = await create_tenant(db_session)
|
||||
second = await create_tenant(db_session)
|
||||
await db_session.commit()
|
||||
|
||||
await create_domain(db_sessionmaker, tenant_id=first.id, domain="fire", display_name="Fire")
|
||||
await create_domain(db_sessionmaker, tenant_id=second.id, domain="fire", display_name="Fire")
|
||||
|
||||
assert len(await list_domains(db_sessionmaker, tenant_id=first.id)) == 1
|
||||
assert len(await list_domains(db_sessionmaker, tenant_id=second.id)) == 1
|
||||
|
||||
|
||||
async def test_update_domain_changes_only_the_display_name(
|
||||
db_session: AsyncSession, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> None:
|
||||
tenant = await create_tenant(db_session)
|
||||
await create_tenant_domain(db_session, tenant=tenant, domain="fire")
|
||||
await db_session.commit()
|
||||
|
||||
updated = await update_domain(
|
||||
db_sessionmaker, tenant_id=tenant.id, domain="fire", display_name="Fire & perils"
|
||||
)
|
||||
|
||||
assert updated.display_name == "Fire & perils"
|
||||
# The key is immutable: it is denormalized into every point payload.
|
||||
assert updated.domain == "fire"
|
||||
|
||||
|
||||
async def test_disabling_a_domain_blocks_new_uploads_without_deleting_it(
|
||||
db_session: AsyncSession, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> None:
|
||||
tenant = await create_tenant(db_session)
|
||||
await create_tenant_domain(db_session, tenant=tenant, domain="fire")
|
||||
await db_session.commit()
|
||||
|
||||
disabled = await set_domain_status(
|
||||
db_sessionmaker, tenant_id=tenant.id, domain="fire", status="disabled"
|
||||
)
|
||||
|
||||
assert disabled.status == "disabled"
|
||||
async with db_sessionmaker() as session:
|
||||
with pytest.raises(UnknownDomainError):
|
||||
await ensure_domain_allowed(session, tenant_id=tenant.id, domain="fire")
|
||||
# Still there, just hidden from the default listing.
|
||||
assert await list_domains(db_sessionmaker, tenant_id=tenant.id) == []
|
||||
assert len(await list_domains(db_sessionmaker, tenant_id=tenant.id, include_disabled=True)) == 1
|
||||
|
||||
|
||||
async def test_re_enabling_a_domain_restores_uploads(
|
||||
db_session: AsyncSession, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> None:
|
||||
tenant = await create_tenant(db_session)
|
||||
await create_tenant_domain(db_session, tenant=tenant, domain="fire", status="disabled")
|
||||
await db_session.commit()
|
||||
|
||||
await set_domain_status(db_sessionmaker, tenant_id=tenant.id, domain="fire", status="active")
|
||||
|
||||
async with db_sessionmaker() as session:
|
||||
await ensure_domain_allowed(session, tenant_id=tenant.id, domain="fire")
|
||||
|
||||
|
||||
async def test_update_domain_rejects_another_tenants_domain(
|
||||
db_session: AsyncSession, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> None:
|
||||
owner = await create_tenant(db_session)
|
||||
other = await create_tenant(db_session)
|
||||
await create_tenant_domain(db_session, tenant=owner, domain="fire")
|
||||
await db_session.commit()
|
||||
|
||||
with pytest.raises(UnknownDomainError):
|
||||
await update_domain(
|
||||
db_sessionmaker, tenant_id=other.id, domain="fire", display_name="hijacked"
|
||||
)
|
||||
@@ -10,6 +10,7 @@ pytestmark = [
|
||||
|
||||
EXPECTED_TABLES = {
|
||||
"tenants",
|
||||
"tenant_domains",
|
||||
"api_keys",
|
||||
"source_files",
|
||||
"ingestion_jobs",
|
||||
@@ -25,3 +26,17 @@ async def test_migrations_create_schema_from_empty_database(postgres_engine: Asy
|
||||
)
|
||||
|
||||
assert EXPECTED_TABLES.issubset(set(table_names))
|
||||
|
||||
|
||||
async def test_tenant_domains_enforces_one_row_per_tenant_and_key(
|
||||
postgres_engine: AsyncEngine,
|
||||
) -> None:
|
||||
"""The unique constraint is what stops the same domain being registered
|
||||
twice for a tenant while still letting two tenants share a key.
|
||||
"""
|
||||
async with postgres_engine.connect() as connection:
|
||||
constraints = await connection.run_sync(
|
||||
lambda sync_conn: inspect(sync_conn).get_unique_constraints("tenant_domains")
|
||||
)
|
||||
|
||||
assert any(constraint["column_names"] == ["tenant_id", "domain"] for constraint in constraints)
|
||||
|
||||
@@ -11,6 +11,7 @@ from sqlalchemy import select
|
||||
from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker
|
||||
|
||||
from src.application.auth.context import AuthContext
|
||||
from src.application.domains import UnknownDomainError
|
||||
from src.application.files.models import UploadResult
|
||||
from src.application.files.upload import upload_source_file
|
||||
from src.application.ingestion.errors import (
|
||||
@@ -28,7 +29,7 @@ from tests.fakes import (
|
||||
FakePointStorage,
|
||||
FakeSparseEmbedder,
|
||||
)
|
||||
from tests.support.factories import create_api_key, create_tenant
|
||||
from tests.support.factories import create_api_key, create_tenant, create_tenant_domain
|
||||
|
||||
pytestmark = [
|
||||
pytest.mark.integration,
|
||||
@@ -70,9 +71,12 @@ async def _upload(
|
||||
)
|
||||
|
||||
|
||||
async def _auth_for(db_session: AsyncSession) -> AuthContext:
|
||||
async def _auth_for(db_session: AsyncSession, *, domain: str = "general") -> AuthContext:
|
||||
tenant = await create_tenant(db_session)
|
||||
api_key, _ = await create_api_key(db_session, tenant=tenant)
|
||||
# Uploads reject an unregistered domain (ADR-0009), so register the one the
|
||||
# helper below uploads to.
|
||||
await create_tenant_domain(db_session, tenant=tenant, domain=domain)
|
||||
await db_session.commit()
|
||||
return AuthContext(
|
||||
tenant_id=tenant.id,
|
||||
@@ -289,7 +293,7 @@ async def test_upload_source_file_indexes_points_and_records_real_counters(
|
||||
async def test_upload_source_file_indexes_points_under_the_authenticated_tenant(
|
||||
db_session: AsyncSession, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> None:
|
||||
auth = await _auth_for(db_session)
|
||||
auth = await _auth_for(db_session, domain="fire")
|
||||
point_storage = FakePointStorage()
|
||||
|
||||
result = await _upload(
|
||||
@@ -395,3 +399,53 @@ async def test_upload_source_file_retry_after_index_failure_produces_no_duplicat
|
||||
assert len(tenant_jobs) == 2
|
||||
assert {job.status for job in tenant_jobs} == {"failed", "succeeded"}
|
||||
|
||||
|
||||
async def test_upload_source_file_rejects_an_unregistered_domain_before_any_write(
|
||||
db_session: AsyncSession, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> None:
|
||||
"""A typo'd domain must fail loudly, not create a new Qdrant partition
|
||||
whose contents retrieval never queries (ADR-0009).
|
||||
"""
|
||||
auth = await _auth_for(db_session, domain="fire")
|
||||
storage = FakeObjectStorage()
|
||||
point_storage = FakePointStorage()
|
||||
|
||||
with pytest.raises(UnknownDomainError, match="fier"):
|
||||
await _upload(
|
||||
sessionmaker=db_sessionmaker,
|
||||
storage=storage,
|
||||
auth=auth,
|
||||
point_storage=point_storage,
|
||||
domain="fier",
|
||||
)
|
||||
|
||||
# Nothing was written anywhere: no object, no points, and no job row.
|
||||
assert storage.objects == {}
|
||||
assert point_storage.points == {}
|
||||
async with db_sessionmaker() as verify_session:
|
||||
jobs = (await verify_session.execute(select(IngestionJob))).scalars().all()
|
||||
assert [job for job in jobs if job.tenant_id == auth.tenant_id] == []
|
||||
|
||||
|
||||
async def test_upload_source_file_rejects_a_disabled_domain(
|
||||
db_session: AsyncSession, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> None:
|
||||
tenant = await create_tenant(db_session)
|
||||
api_key, _ = await create_api_key(db_session, tenant=tenant)
|
||||
await create_tenant_domain(db_session, tenant=tenant, domain="fire", status="disabled")
|
||||
await db_session.commit()
|
||||
auth = AuthContext(
|
||||
tenant_id=tenant.id,
|
||||
tenant_slug=tenant.slug,
|
||||
api_key_id=api_key.id,
|
||||
scopes=frozenset({"files:write"}),
|
||||
actor_type="backend",
|
||||
)
|
||||
|
||||
with pytest.raises(UnknownDomainError, match="disabled"):
|
||||
await _upload(
|
||||
sessionmaker=db_sessionmaker,
|
||||
storage=FakeObjectStorage(),
|
||||
auth=auth,
|
||||
domain="fire",
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user