Implements AccountRepository against the control-plane schema plus the phase-1 identity migration, and wires it in so production can select it. The adapter is not a thin SQL wrapper, because the control plane does not store the two things the application needs on the users row. Role and organization come from user_roles. There is no role column and no organization column on users; a role is assigned through user_roles with a scope, and app.validate_user_role_scope() enforces that the role's scope matches the assignment. Resolving one role per user is therefore a join, and a user holding more than one active assignment is refused rather than resolved to an arbitrary row. The organization on that assignment becomes the scope of every administrative decision, so guessing would hand out the wrong tenant's authority. Reconciling this with project-scoped membership stays an open review question, recorded as such. Email became a required field on account creation. users.email is NOT NULL UNIQUE, so accepting a missing address would let account creation succeed against the development repositories and then fail on the authoritative store. It is now required in the service, the API schema, and the bootstrap command, so it fails identically everywhere. Audit identifiers now belong to the store. audit_events.id is a bigint identity, so the adapter reports that value; the application correlation UUID is preserved in the detail and never displaces the row key. AuditEvent.id is a string for that reason, which removes a type the two stores disagreed about. Three further fail-closed behaviours: a users.status of invited or locked is refused rather than coerced onto active; a row with mfa_required is refused, because this phase cannot satisfy a second factor and authenticating it anyway would grant access to someone who was supposed to present one; and a missing organization blocks access rather than granting it. save_user deliberately does not write username or email, since both are identity keys with no reviewed change path and no audit event behind them. Every operation runs in one transaction on one pooled connection and publishes the caller with SET LOCAL app.current_user_id, so a returned connection cannot carry one request's identity into the next. That is inert until the reviewed RLS policies exist; the explicit WHERE clauses enforce scope today. A statement timeout bounds how long a pooled connection can be held. Production fails closed at two independent gates: the settings layer refuses without a DSN, and the repository boundary refuses to build a PostgreSQL store without one. Verified by running both, including that an unreachable host refuses to start rather than degrading. The migration gains an index on audit_events (target_type, target_id), which the administrator audit filter needs and nothing covered, and the review questions now include the three things that must be checked before provisioning: the roles rows and their scopes, any row with mfa_required set, and any user holding more than one active assignment. Verification, and its limit. 236 tests pass with 3 skipped. The adapter's row mapping, role resolution, conflict mapping, and configuration are unit tested and run everywhere. Its SQL, transactions, and pooled reuse are NOT executed: this environment has no database credentials, because sudo to the postgres user needs a password, there is no umbra database role, and there is no .pgpass. The integration suite is written and selected by UMBRATRANS_TEST_DSN, and its fixture refuses any DSN whose database name contains umbracontrol. A green run here says nothing about whether the SQL is correct.
515 lines
17 KiB
Python
515 lines
17 KiB
Python
"""Tests for the PostgreSQL adapter.
|
|
|
|
Two layers, because the two have very different verification status.
|
|
|
|
**Mapping and configuration tests run everywhere.** The row mappers, the
|
|
role-assignment rules, and the unique-violation mapping are pure functions, so
|
|
they are asserted directly against synthetic rows. These are the places where a
|
|
silent misreading of the control-plane schema would produce a wrong tenant scope
|
|
rather than an obvious crash, which is why they are covered in detail.
|
|
|
|
**Integration tests require a real database and skip without one.** They are
|
|
selected by ``UMBRATRANS_TEST_DSN`` and must point at a *scratch* database that
|
|
the phase-1 migration has been applied to, never at ``umbracontrol``. They are
|
|
not expected to pass in an environment with no database access, and a green run
|
|
here says nothing about them either way.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import os
|
|
import uuid
|
|
from datetime import datetime, timedelta, timezone
|
|
from typing import Any
|
|
|
|
import pytest
|
|
|
|
from umbratrans_web.errors import ConflictError, RepositoryError
|
|
from umbratrans_web.models import AccountStatus, OrganizationStatus, RoleName, User
|
|
from umbratrans_web.postgres_repository import (
|
|
RLS_USER_SETTING,
|
|
STATEMENT_TIMEOUT_MS,
|
|
PostgresAccountRepository,
|
|
_assignment_shape,
|
|
_audit_from_row,
|
|
_organization_from_row,
|
|
_session_from_row,
|
|
_unique_conflict,
|
|
_user_from_row,
|
|
)
|
|
from umbratrans_web.repository import AccountRepository
|
|
|
|
TEST_DSN_ENV = "UMBRATRANS_TEST_DSN"
|
|
|
|
|
|
def _row(**overrides: Any) -> dict[str, Any]:
|
|
"""One ``users`` row as psycopg would return it with ``dict_row``."""
|
|
|
|
now = datetime(2026, 1, 1, tzinfo=timezone.utc)
|
|
base: dict[str, Any] = {
|
|
"id": uuid.uuid4(),
|
|
"username": "Umbraadmin",
|
|
"display_name": "Umbra Administrator",
|
|
"email": "umbraadmin@example.test",
|
|
"status": "active",
|
|
"created_at": now,
|
|
"updated_at": now,
|
|
}
|
|
base.update(overrides)
|
|
return base
|
|
|
|
|
|
# ----------------------------------------------------------------------
|
|
# Protocol conformance
|
|
# ----------------------------------------------------------------------
|
|
|
|
|
|
def test_the_adapter_satisfies_the_repository_protocol() -> None:
|
|
for name in AccountRepository.__protocol_attrs__: # type: ignore[attr-defined]
|
|
assert hasattr(PostgresAccountRepository, name), name
|
|
|
|
|
|
def test_the_adapter_imports_without_opening_a_connection() -> None:
|
|
"""Importing the module must not touch the network.
|
|
|
|
``psycopg_pool`` is imported at module scope, so a missing database is a
|
|
runtime failure at construction, never an import-time surprise.
|
|
"""
|
|
|
|
assert PostgresAccountRepository.__doc__ is not None
|
|
assert RLS_USER_SETTING == "app.current_user_id"
|
|
assert STATEMENT_TIMEOUT_MS > 0
|
|
|
|
|
|
# ----------------------------------------------------------------------
|
|
# Role and organization resolution
|
|
# ----------------------------------------------------------------------
|
|
|
|
|
|
def test_a_platform_admin_is_assigned_at_platform_scope() -> None:
|
|
"""A platform assignment must carry no organization.
|
|
|
|
This is what the ``user_roles`` check constraint requires, and getting it
|
|
wrong is a database error on insert rather than a silent bug.
|
|
"""
|
|
|
|
user = User(
|
|
id=uuid.uuid4(),
|
|
username="Umbraadmin",
|
|
username_key="umbraadmin",
|
|
display_name="Umbra Administrator",
|
|
role=RoleName.PLATFORM_ADMIN,
|
|
status=AccountStatus.ACTIVE,
|
|
)
|
|
assert _assignment_shape(user) == ("platform", None)
|
|
|
|
|
|
def test_an_organization_scoped_role_requires_an_organization() -> None:
|
|
"""An organization assignment without one is refused before the database.
|
|
|
|
The same combination is rejected by the ``user_roles`` check constraint;
|
|
refusing here names the real problem instead of surfacing a constraint
|
|
violation.
|
|
"""
|
|
|
|
user = User(
|
|
id=uuid.uuid4(),
|
|
username="Orphan.Member",
|
|
username_key="orphan.member",
|
|
display_name="Orphan",
|
|
role=RoleName.MEMBER,
|
|
status=AccountStatus.ACTIVE,
|
|
)
|
|
with pytest.raises(RepositoryError) as error:
|
|
_assignment_shape(user)
|
|
assert error.value.code == "organization_required"
|
|
|
|
|
|
def test_a_member_carries_its_organization() -> None:
|
|
organization_id = uuid.uuid4()
|
|
user = User(
|
|
id=uuid.uuid4(),
|
|
username="Member.One",
|
|
username_key="member.one",
|
|
display_name="Member One",
|
|
role=RoleName.MEMBER,
|
|
status=AccountStatus.ACTIVE,
|
|
organization_id=organization_id,
|
|
)
|
|
assert _assignment_shape(user) == ("organization", organization_id)
|
|
|
|
|
|
def test_an_organization_admin_is_also_organization_scoped() -> None:
|
|
organization_id = uuid.uuid4()
|
|
user = User(
|
|
id=uuid.uuid4(),
|
|
username="Org.Admin",
|
|
username_key="org.admin",
|
|
display_name="Org Admin",
|
|
role=RoleName.ORGANIZATION_ADMIN,
|
|
status=AccountStatus.ACTIVE,
|
|
organization_id=organization_id,
|
|
)
|
|
assert _assignment_shape(user) == ("organization", organization_id)
|
|
|
|
|
|
# ----------------------------------------------------------------------
|
|
# User row mapping
|
|
# ----------------------------------------------------------------------
|
|
|
|
|
|
def test_a_user_row_maps_with_a_resolved_assignment() -> None:
|
|
row = _row()
|
|
organization_id = uuid.uuid4()
|
|
user = _user_from_row(row, role=RoleName.MEMBER, organization_id=organization_id)
|
|
|
|
assert user.id == row["id"]
|
|
assert user.username == "Umbraadmin"
|
|
assert user.username_key == "umbraadmin"
|
|
assert user.role is RoleName.MEMBER
|
|
assert user.status is AccountStatus.ACTIVE
|
|
assert user.organization_id == organization_id
|
|
assert user.email == "umbraadmin@example.test"
|
|
|
|
|
|
def test_a_user_row_falls_back_to_the_username_when_the_name_is_null() -> None:
|
|
"""users.display_name is nullable in the control plane."""
|
|
|
|
user = _user_from_row(_row(display_name=None))
|
|
assert user.display_name == "Umbraadmin"
|
|
|
|
|
|
@pytest.mark.parametrize("status", ["active", "suspended", "deactivated"])
|
|
def test_modelled_account_statuses_are_accepted(status: str) -> None:
|
|
assert _user_from_row(_row(status=status)).status is AccountStatus(status)
|
|
|
|
|
|
def test_an_account_requiring_mfa_is_refused() -> None:
|
|
"""The control plane can require a second factor this phase cannot do.
|
|
|
|
Authenticating such an account anyway would grant access to someone who was
|
|
supposed to present a second factor, so the read fails closed.
|
|
"""
|
|
|
|
with pytest.raises(RepositoryError) as error:
|
|
_user_from_row(_row(mfa_required=True))
|
|
assert error.value.code == "mfa_required_unsupported"
|
|
|
|
|
|
def test_a_profile_row_without_the_mfa_column_is_accepted() -> None:
|
|
"""Tolerates a row shape that predates the column being selected."""
|
|
|
|
assert _user_from_row(_row()).status is AccountStatus.ACTIVE
|
|
|
|
|
|
@pytest.mark.parametrize("status", ["invited", "locked", "paused", "ACTIVE"])
|
|
def test_an_unmodelled_status_is_refused_rather_than_coerced(status: str) -> None:
|
|
"""``invited`` and ``locked`` exist in the control plane but not here.
|
|
|
|
Mapping one of them onto ``active`` would grant access to an account that
|
|
the control plane considers not yet usable, so the read fails closed.
|
|
"""
|
|
|
|
with pytest.raises(RepositoryError) as error:
|
|
_user_from_row(_row(status=status))
|
|
assert error.value.code == "unmapped_account_status"
|
|
|
|
|
|
# ----------------------------------------------------------------------
|
|
# Organization row mapping
|
|
# ----------------------------------------------------------------------
|
|
|
|
|
|
def test_an_organization_row_maps() -> None:
|
|
now = datetime(2026, 1, 1, tzinfo=timezone.utc)
|
|
organization = _organization_from_row(
|
|
{
|
|
"id": uuid.uuid4(),
|
|
"name": "Northstar Media",
|
|
"slug": "northstar-media",
|
|
"status": "active",
|
|
"created_at": now,
|
|
"updated_at": now,
|
|
}
|
|
)
|
|
assert organization.name == "Northstar Media"
|
|
assert organization.slug == "northstar-media"
|
|
assert organization.status is OrganizationStatus.ACTIVE
|
|
|
|
|
|
def test_an_unmodelled_organization_status_is_refused() -> None:
|
|
now = datetime(2026, 1, 1, tzinfo=timezone.utc)
|
|
with pytest.raises(ValueError):
|
|
_organization_from_row(
|
|
{
|
|
"id": uuid.uuid4(),
|
|
"name": "Odd",
|
|
"slug": "odd",
|
|
"status": "deleted",
|
|
"created_at": now,
|
|
"updated_at": now,
|
|
}
|
|
)
|
|
|
|
|
|
# ----------------------------------------------------------------------
|
|
# Session row mapping
|
|
# ----------------------------------------------------------------------
|
|
|
|
|
|
def test_a_session_row_maps_expires_at_to_the_absolute_deadline() -> None:
|
|
"""``sessions.expires_at`` predates this application.
|
|
|
|
The adapter keeps the two equal on write, so anything else reading
|
|
``expires_at`` sees the same deadline.
|
|
"""
|
|
|
|
created = datetime(2026, 1, 1, tzinfo=timezone.utc)
|
|
session = _session_from_row(
|
|
{
|
|
"id": uuid.uuid4(),
|
|
"user_id": uuid.uuid4(),
|
|
"token_hash": "a" * 64,
|
|
"created_at": created,
|
|
"last_seen_at": created + timedelta(minutes=1),
|
|
"idle_deadline_at": created + timedelta(minutes=30),
|
|
"expires_at": created + timedelta(hours=12),
|
|
"revoked_at": None,
|
|
"mfa_verified_at": None,
|
|
}
|
|
)
|
|
assert session.absolute_deadline_at == created + timedelta(hours=12)
|
|
assert session.idle_deadline_at == created + timedelta(minutes=30)
|
|
assert session.is_revoked() is False
|
|
# A hash must not surface through repr.
|
|
assert "a" * 64 not in repr(session)
|
|
|
|
|
|
# ----------------------------------------------------------------------
|
|
# Audit row mapping
|
|
# ----------------------------------------------------------------------
|
|
|
|
|
|
def test_an_audit_row_reports_the_store_identifier() -> None:
|
|
"""``audit_events.id`` is a bigint identity owned by the database.
|
|
|
|
The application mints a UUID for correlation, which is preserved in the
|
|
detail, but the API must report the identifier the store will actually find.
|
|
"""
|
|
|
|
event = _audit_from_row(
|
|
{
|
|
"id": 4211,
|
|
"actor_user_id": uuid.uuid4(),
|
|
"organization_id": None,
|
|
"action": "account.created",
|
|
"target_type": "user",
|
|
"target_id": str(uuid.uuid4()),
|
|
"request_id": "req-1",
|
|
"metadata": {"role": "member", "application_event_id": str(uuid.uuid4())},
|
|
"created_at": datetime(2026, 1, 1, tzinfo=timezone.utc),
|
|
},
|
|
original=None,
|
|
)
|
|
assert event.id == "4211"
|
|
assert event.action == "account.created"
|
|
assert event.target_user_id is not None
|
|
# The correlation UUID is retained for the caller but is not the row key.
|
|
assert "application_event_id" not in event.detail
|
|
assert event.detail["role"] == "member"
|
|
|
|
|
|
def test_a_non_user_target_maps_to_no_user() -> None:
|
|
event = _audit_from_row(
|
|
{
|
|
"id": 7,
|
|
"actor_user_id": None,
|
|
"organization_id": uuid.uuid4(),
|
|
"action": "organization.created",
|
|
"target_type": "organization",
|
|
"target_id": str(uuid.uuid4()),
|
|
"request_id": None,
|
|
"metadata": {},
|
|
"created_at": datetime(2026, 1, 1, tzinfo=timezone.utc),
|
|
},
|
|
original=None,
|
|
)
|
|
assert event.target_user_id is None
|
|
|
|
|
|
# ----------------------------------------------------------------------
|
|
# Conflict mapping
|
|
# ----------------------------------------------------------------------
|
|
|
|
|
|
class _FakeDiagnostic:
|
|
def __init__(self, constraint_name: str) -> None:
|
|
self.constraint_name = constraint_name
|
|
|
|
|
|
class _FakeUniqueViolation(Exception):
|
|
def __init__(self, constraint_name: str) -> None:
|
|
super().__init__("duplicate key value violates unique constraint")
|
|
self.diag = _FakeDiagnostic(constraint_name)
|
|
|
|
|
|
def test_a_username_violation_maps_to_username_taken() -> None:
|
|
error = _unique_conflict(_FakeUniqueViolation("users_username_key"), "Umbraadmin")
|
|
assert isinstance(error, ConflictError)
|
|
assert error.code == "username_taken"
|
|
|
|
|
|
def test_an_email_violation_maps_to_email_taken() -> None:
|
|
error = _unique_conflict(_FakeUniqueViolation("users_email_key"), "Umbraadmin")
|
|
assert error.code == "email_taken"
|
|
|
|
|
|
def test_an_unknown_constraint_falls_back_to_the_username_rule() -> None:
|
|
error = _unique_conflict(_FakeUniqueViolation("something_else"), "Umbraadmin")
|
|
assert error.code == "username_taken"
|
|
|
|
|
|
# ----------------------------------------------------------------------
|
|
# Adapter source hygiene
|
|
# ----------------------------------------------------------------------
|
|
|
|
|
|
def test_the_adapter_never_interpolates_a_value_into_sql() -> None:
|
|
"""Every value reaches PostgreSQL as a bound parameter.
|
|
|
|
The statements are assembled from fixed fragments only; a value is never
|
|
formatted into the string. This asserts the absence of the common mistake
|
|
rather than trying to prove the presence of the correct pattern.
|
|
"""
|
|
|
|
import inspect
|
|
|
|
source = inspect.getsource(PostgresAccountRepository)
|
|
for pattern in (
|
|
f"{{{{{'user'}.username}}}}",
|
|
f"{{{{{'user'}.email}}}}",
|
|
f"{{{{{'event'}.action}}}}",
|
|
f"{{{{{'organization'}.slug}}}}",
|
|
):
|
|
assert pattern not in source, pattern
|
|
|
|
|
|
def test_the_adapter_issues_no_destructive_statement() -> None:
|
|
"""Insert and update are the adapter's job; nothing that destroys data is.
|
|
|
|
``INSERT INTO`` is expected here, unlike in the migration file. What must
|
|
never appear is anything that drops, truncates, deletes, or alters schema:
|
|
those belong in a reviewed migration, not in request handling.
|
|
"""
|
|
|
|
import inspect
|
|
|
|
source = inspect.getsource(PostgresAccountRepository)
|
|
for statement in (
|
|
"DROP ",
|
|
"TRUNCATE",
|
|
"DELETE FROM",
|
|
"ALTER TABLE",
|
|
"CREATE TABLE",
|
|
"GRANT ",
|
|
"REVOKE ",
|
|
):
|
|
assert statement not in source, statement
|
|
|
|
|
|
def test_the_adapter_never_selects_a_hash_into_a_user_row() -> None:
|
|
"""A ``users`` read must not drag a credential hash along with the profile.
|
|
|
|
Checked against the actual column list of every statement that reads
|
|
``public.users``, rather than a blanket scan of the module, because the
|
|
credential reads legitimately do select ``password_hash``.
|
|
"""
|
|
|
|
import inspect
|
|
import re
|
|
|
|
source = inspect.getsource(PostgresAccountRepository)
|
|
statements = re.findall(r"SELECT\s+(.*?)\s+FROM\s+public\.users", source, re.DOTALL)
|
|
assert statements, "expected at least one statement reading public.users"
|
|
for columns in statements:
|
|
for protected in ("password_hash", "token_hash"):
|
|
assert protected not in columns, (protected, columns)
|
|
|
|
|
|
# ----------------------------------------------------------------------
|
|
# Integration: requires a real scratch database
|
|
# ----------------------------------------------------------------------
|
|
|
|
|
|
def _integration_dsn() -> str | None:
|
|
return os.environ.get(TEST_DSN_ENV) or None
|
|
|
|
|
|
integration = pytest.mark.skipif(
|
|
_integration_dsn() is None,
|
|
reason=(
|
|
f"{TEST_DSN_ENV} is unset. Point it at a scratch database with "
|
|
"migrations/0001_phase1_identity.sql applied. Never at umbracontrol."
|
|
),
|
|
)
|
|
|
|
|
|
@pytest.fixture
|
|
def postgres() -> Any:
|
|
dsn = _integration_dsn()
|
|
if dsn is None: # pragma: no cover - the skip above already guards this
|
|
pytest.skip("no test DSN")
|
|
if "umbracontrol" in dsn: # pragma: no cover - a safety net
|
|
pytest.skip("refusing to run the adapter suite against umbracontrol")
|
|
repository = PostgresAccountRepository.from_dsn(
|
|
dsn, min_size=1, max_size=4, application_name="umbratrans-adapter-test"
|
|
)
|
|
try:
|
|
yield repository
|
|
finally:
|
|
repository.close()
|
|
|
|
|
|
@integration
|
|
def test_a_user_round_trips_through_postgres(postgres: PostgresAccountRepository) -> None:
|
|
from umbratrans_web.security import utc_now
|
|
|
|
user = User(
|
|
id=uuid.uuid4(),
|
|
username=f"Round.Trip.{uuid.uuid4().hex[:8]}",
|
|
username_key="",
|
|
display_name="Round Trip",
|
|
role=RoleName.MEMBER,
|
|
status=AccountStatus.ACTIVE,
|
|
organization_id=None,
|
|
email=f"round-{uuid.uuid4().hex[:8]}@example.test",
|
|
created_at=utc_now(),
|
|
updated_at=utc_now(),
|
|
)
|
|
# organization_id is required for a member assignment, so this test asserts
|
|
# the refusal rather than working around it.
|
|
with pytest.raises(RepositoryError) as error:
|
|
postgres.create_user(user)
|
|
assert error.value.code == "organization_required"
|
|
|
|
|
|
@integration
|
|
def test_the_health_probe_reports_reachability(postgres: PostgresAccountRepository) -> None:
|
|
assert postgres.check() is True
|
|
|
|
|
|
@integration
|
|
def test_the_database_stores_no_plaintext_password(postgres: PostgresAccountRepository) -> None:
|
|
"""A stored credential holds an Argon2id hash and nothing else."""
|
|
|
|
from umbratrans_web.security import utc_now
|
|
|
|
rows = []
|
|
with postgres._pool.connection() as connection: # noqa: SLF001 - test introspection
|
|
rows = connection.execute(
|
|
"SELECT password_hash FROM public.user_credentials WHERE password_hash IS NOT NULL LIMIT 5"
|
|
).fetchall()
|
|
for row in rows:
|
|
assert row["password_hash"].startswith("$argon2id$")
|
|
assert utc_now() is not None
|