Release govoplan-access v0.1.25: harden authentication and repair identity mappings
Module Package Release / publish-packages (push) Successful in 15s
Module Package Release / publish-packages (push) Successful in 15s
This commit is contained in:
@@ -0,0 +1,120 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import unittest
|
||||
from datetime import timedelta
|
||||
from unittest.mock import patch
|
||||
|
||||
from fastapi import HTTPException
|
||||
from sqlalchemy import create_engine
|
||||
from sqlalchemy.orm import sessionmaker
|
||||
from starlette.requests import Request
|
||||
|
||||
from govoplan_access.backend.auth.dependencies import _resolve_legacy_principal_context
|
||||
from govoplan_access.backend.auth.principal_cache import principal_summary_cache
|
||||
from govoplan_access.backend.auth.tokens import hash_secret
|
||||
from govoplan_access.backend.db.base import AccessBase
|
||||
from govoplan_access.backend.db.models import Account, AuthSession, Role, ServiceAccount, User, UserRoleAssignment
|
||||
from govoplan_access.backend.security.api_keys import create_api_key
|
||||
from govoplan_core.core.change_sequence import ChangeSequenceEntry, ChangeSequenceRetentionFloor
|
||||
from govoplan_core.db.base import Base
|
||||
from govoplan_core.security.time import utc_now
|
||||
from govoplan_core.settings import settings
|
||||
from govoplan_core.tenancy.scope import Tenant, create_scope_tables, scope_registry
|
||||
|
||||
|
||||
class AuthCacheSecurityTests(unittest.TestCase):
|
||||
def setUp(self) -> None:
|
||||
principal_summary_cache.clear()
|
||||
self.engine = create_engine("sqlite:///:memory:")
|
||||
create_scope_tables(self.engine)
|
||||
AccessBase.metadata.create_all(self.engine)
|
||||
self.revision_tables = [ChangeSequenceEntry.__table__, ChangeSequenceRetentionFloor.__table__]
|
||||
Base.metadata.create_all(self.engine, tables=self.revision_tables)
|
||||
self.session = sessionmaker(bind=self.engine)()
|
||||
self.cache_setting = patch.object(settings, "auth_principal_cache_enabled", True)
|
||||
self.cache_setting.start()
|
||||
self.tenant = Tenant(id="cache-tenant", slug="cache-tenant", name="Cache tenant")
|
||||
self.session.add(self.tenant)
|
||||
self.session.commit()
|
||||
|
||||
def tearDown(self) -> None:
|
||||
self.cache_setting.stop()
|
||||
principal_summary_cache.clear()
|
||||
self.session.close()
|
||||
AccessBase.metadata.drop_all(self.engine)
|
||||
scope_registry.metadata.drop_all(self.engine)
|
||||
Base.metadata.drop_all(self.engine, tables=self.revision_tables)
|
||||
self.engine.dispose()
|
||||
|
||||
def identity(self, *, service: bool = False) -> tuple[Account, User]:
|
||||
account = Account(id="cache-account", email="cache@example.test", normalized_email="cache@example.test", auth_provider="service_account" if service else "local")
|
||||
user = User(id="cache-user", tenant_id=self.tenant.id, account_id=account.id, email=account.email, auth_provider=account.auth_provider)
|
||||
role = Role(id="cache-role", tenant_id=self.tenant.id, slug="reader", name="Reader", permissions=["files:file:read"])
|
||||
assignment = UserRoleAssignment(tenant_id=self.tenant.id, user_id=user.id, role_id=role.id)
|
||||
self.session.add_all([account, user, role, assignment])
|
||||
self.session.commit()
|
||||
return account, user
|
||||
|
||||
def resolve(self, token: str, *, cookie: bool = False, csrf: str | None = None):
|
||||
headers = []
|
||||
if cookie:
|
||||
cookies = f"{settings.auth_session_cookie_name}={token}"
|
||||
if csrf is not None:
|
||||
cookies += f"; {settings.auth_csrf_cookie_name}={csrf}"
|
||||
headers.append((b"x-csrf-token", csrf.encode()))
|
||||
headers.append((b"cookie", cookies.encode()))
|
||||
request = Request({"type": "http", "method": "POST", "path": "/protected", "headers": headers})
|
||||
return _resolve_legacy_principal_context(request, self.session, authorization=None if cookie else f"Bearer {token}", x_api_key=None)
|
||||
|
||||
def test_warmed_api_key_is_never_accepted_as_a_session_cookie(self) -> None:
|
||||
_, user = self.identity()
|
||||
key = create_api_key(self.session, user=user, name="Test", scopes=["files:file:read"])
|
||||
self.session.commit()
|
||||
with self.assertRaises(HTTPException) as cold:
|
||||
self.resolve(key.secret, cookie=True)
|
||||
self.assertEqual(401, cold.exception.status_code)
|
||||
self.assertEqual("api_key", self.resolve(key.secret).principal.auth_method)
|
||||
with self.assertRaises(HTTPException) as warm:
|
||||
self.resolve(key.secret, cookie=True)
|
||||
self.assertEqual(401, warm.exception.status_code)
|
||||
self.assertEqual("api_key", self.resolve(key.secret).principal.auth_method)
|
||||
|
||||
def test_service_account_keeps_current_ceiling_and_provenance_with_cache_enabled(self) -> None:
|
||||
account, user = self.identity(service=True)
|
||||
item = ServiceAccount(id="cache-service", tenant_id=self.tenant.id, account_id=account.id, membership_id=user.id, name="Cache worker", normalized_name="cache worker", scope_ceiling=["dataflow:pipeline:run"])
|
||||
# A credential issued before a ceiling reduction can retain wider stored
|
||||
# scopes. Ordinary membership roles must not override the current ceiling.
|
||||
key = create_api_key(self.session, user=user, name="Worker", scopes=["dataflow:pipeline:run", "files:file:read"])
|
||||
self.session.add(item)
|
||||
self.session.commit()
|
||||
for _ in range(2):
|
||||
context = self.resolve(key.secret)
|
||||
self.assertEqual(frozenset({"dataflow:pipeline:run"}), context.principal.scopes)
|
||||
self.assertEqual("service_account", context.principal.auth_method)
|
||||
self.assertEqual(item.id, context.principal.service_account_id)
|
||||
self.assertFalse(context.principal.role_ids)
|
||||
item.scope_ceiling = []
|
||||
self.session.commit()
|
||||
self.assertEqual(frozenset(), self.resolve(key.secret).principal.scopes)
|
||||
item.is_active = False
|
||||
self.session.commit()
|
||||
with self.assertRaises(HTTPException) as inactive:
|
||||
self.resolve(key.secret)
|
||||
self.assertEqual(401, inactive.exception.status_code)
|
||||
|
||||
def test_warmed_session_cookie_still_requires_matching_csrf(self) -> None:
|
||||
account, user = self.identity()
|
||||
token, csrf = "ms_cache-session", "cache-csrf"
|
||||
auth_session = AuthSession(id="cache-session", tenant_id=self.tenant.id, user_id=user.id, account_id=account.id, token_hash=hash_secret(token), csrf_token_hash=hash_secret(csrf), expires_at=utc_now() + timedelta(hours=1))
|
||||
self.session.add(auth_session)
|
||||
self.session.commit()
|
||||
self.resolve(token)
|
||||
for supplied in (None, "incorrect"):
|
||||
with self.subTest(csrf=supplied), self.assertRaises(HTTPException) as denied:
|
||||
self.resolve(token, cookie=True, csrf=supplied)
|
||||
self.assertEqual(403, denied.exception.status_code)
|
||||
self.assertEqual("session", self.resolve(token, cookie=True, csrf=csrf).principal.auth_method)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -0,0 +1,164 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import tempfile
|
||||
import unittest
|
||||
from datetime import datetime, timezone
|
||||
from pathlib import Path
|
||||
|
||||
from alembic import command
|
||||
from fastapi import FastAPI
|
||||
from fastapi.testclient import TestClient
|
||||
from sqlalchemy import create_engine, event, inspect, text
|
||||
from sqlalchemy.exc import IntegrityError
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
from govoplan_access.backend.api.v1.routes import router
|
||||
from govoplan_access.backend.auth.dependencies import get_api_principal
|
||||
from govoplan_access.backend.db.models import ExternalFunctionRoleAssignment, Role
|
||||
from govoplan_core.auth import ApiPrincipal
|
||||
from govoplan_core.core.access import PrincipalRef
|
||||
from govoplan_core.db.migrations import alembic_config
|
||||
from govoplan_core.db.session import get_session
|
||||
from govoplan_core.tenancy.scope import Tenant
|
||||
|
||||
|
||||
TABLE_NAME = "access_external_function_role_assignments"
|
||||
REPAIR_REVISION = "d8f1b4e7a0c3"
|
||||
|
||||
|
||||
class ExternalFunctionMappingMigrationTests(unittest.TestCase):
|
||||
def test_release_missing_table_repair(self) -> None:
|
||||
self._verify_upgrade("release", missing=True)
|
||||
|
||||
def test_release_existing_mappings_preserved(self) -> None:
|
||||
self._verify_upgrade("release", missing=False)
|
||||
|
||||
def test_dev_missing_table_repair(self) -> None:
|
||||
self._verify_upgrade("dev", missing=True)
|
||||
|
||||
def test_dev_existing_mappings_preserved(self) -> None:
|
||||
self._verify_upgrade("dev", missing=False)
|
||||
|
||||
def _verify_upgrade(self, track: str, *, missing: bool) -> None:
|
||||
previous = "c7e0a3d6f9b2" if track == "release" else "b6d9f2a5c8e1"
|
||||
with tempfile.TemporaryDirectory(prefix="govoplan-function-mapping-upgrade-") as directory:
|
||||
url = f"sqlite:///{Path(directory) / 'upgrade.db'}"
|
||||
config = alembic_config(database_url=url, enabled_modules=("access",), migration_track=track)
|
||||
command.upgrade(config, "4f2a9c8e7b6d")
|
||||
command.upgrade(config, previous)
|
||||
engine = create_engine(url, connect_args={"check_same_thread": False})
|
||||
|
||||
@event.listens_for(engine, "connect")
|
||||
def enforce_foreign_keys(connection, _record) -> None:
|
||||
connection.execute("PRAGMA foreign_keys=ON")
|
||||
|
||||
try:
|
||||
with Session(engine) as session:
|
||||
session.add_all([
|
||||
Tenant(id="tenant-1", slug="tenant-1", name="Existing tenant"),
|
||||
Tenant(id="tenant-2", slug="tenant-2", name="Other tenant"),
|
||||
])
|
||||
session.flush()
|
||||
session.add_all([
|
||||
Role(id="role-1", tenant_id="tenant-1", slug="role-1", name="Existing role", permissions=["access:function:read"]),
|
||||
Role(id="role-2", tenant_id="tenant-2", slug="role-2", name="Other role", permissions=["access:role:read"]),
|
||||
])
|
||||
session.commit()
|
||||
if missing:
|
||||
# Reproduce only in this isolated database: a recorded baseline
|
||||
# with the exact missing table observed in the live 500 response.
|
||||
with engine.begin() as connection:
|
||||
connection.execute(text("DROP TABLE access_external_function_role_assignments"))
|
||||
else:
|
||||
self._insert_mapping(engine, "mapping-1", "tenant-1", "role-1")
|
||||
self._insert_mapping(engine, "mapping-2", "tenant-2", "role-2")
|
||||
with engine.connect() as connection:
|
||||
tables_before = set(inspect(connection).get_table_names())
|
||||
parents_before = self._parent_rows(connection)
|
||||
mappings_before = [] if missing else self._mapping_rows(connection)
|
||||
|
||||
app = FastAPI()
|
||||
app.include_router(router, prefix="/api/v1")
|
||||
principal = ApiPrincipal(
|
||||
principal=PrincipalRef(account_id="reader", membership_id="reader-1", tenant_id="tenant-1", scopes=frozenset({"access:function:read"})),
|
||||
account=None,
|
||||
user=None,
|
||||
)
|
||||
|
||||
def test_session():
|
||||
with Session(engine) as session:
|
||||
yield session
|
||||
|
||||
app.dependency_overrides[get_session] = test_session
|
||||
app.dependency_overrides[get_api_principal] = lambda: principal
|
||||
with TestClient(app, raise_server_exceptions=False) as client:
|
||||
path = "/api/v1/admin/external-function-role-mappings"
|
||||
if missing:
|
||||
self.assertEqual(client.get(f"{path}/delta").status_code, 500)
|
||||
command.upgrade(config, REPAIR_REVISION)
|
||||
command.upgrade(config, REPAIR_REVISION)
|
||||
for suffix in ("", "/delta"):
|
||||
response = client.get(f"{path}{suffix}")
|
||||
self.assertEqual(response.status_code, 200, response.text)
|
||||
self.assertEqual([item["id"] for item in response.json()["mappings"]], [] if missing else ["mapping-1"])
|
||||
self.assertEqual(response.json()["total"], 0 if missing else 1)
|
||||
self.assertEqual(client.get(f"{path}{suffix}?tenant_id=tenant-2").status_code, 409)
|
||||
principal.principal = PrincipalRef(account_id="reader", membership_id="reader-1", tenant_id="tenant-1", scopes=frozenset())
|
||||
self.assertEqual(client.get(f"{path}/delta").status_code, 403)
|
||||
|
||||
with engine.connect() as connection:
|
||||
inspector = inspect(connection)
|
||||
self.assertEqual(set(inspector.get_table_names()), tables_before | {TABLE_NAME})
|
||||
self.assertEqual(self._parent_rows(connection), parents_before)
|
||||
self.assertEqual(self._mapping_rows(connection), mappings_before)
|
||||
columns = inspector.get_columns(TABLE_NAME)
|
||||
self.assertEqual({item["name"] for item in columns}, {"id", "tenant_id", "source_module", "function_id", "role_id", "settings", "created_at", "updated_at"})
|
||||
self.assertTrue(all(not item["nullable"] for item in columns))
|
||||
self.assertEqual(inspector.get_pk_constraint(TABLE_NAME)["constrained_columns"], ["id"])
|
||||
self.assertIn(["tenant_id", "source_module", "function_id", "role_id"], [item["column_names"] for item in inspector.get_unique_constraints(TABLE_NAME)])
|
||||
self.assertEqual({tuple(item["column_names"]) for item in inspector.get_indexes(TABLE_NAME)}, {("tenant_id",), ("role_id",), ("function_id",), ("source_module",)})
|
||||
self.assertEqual({(tuple(item["constrained_columns"]), item["referred_table"], item["options"]["ondelete"]) for item in inspector.get_foreign_keys(TABLE_NAME)}, {(("role_id",), "access_roles", "CASCADE"), (("tenant_id",), "core_scopes", "CASCADE")})
|
||||
|
||||
self._insert_mapping(engine, "mapping-after-repair", "tenant-1", "role-1", function_id="new-function")
|
||||
with self.assertRaises(IntegrityError):
|
||||
self._insert_mapping(engine, "duplicate", "tenant-1", "role-1", function_id="new-function")
|
||||
with self.assertRaises(IntegrityError):
|
||||
self._insert_mapping(engine, "bad-role", "tenant-1", "missing-role")
|
||||
with self.assertRaises(IntegrityError):
|
||||
self._insert_mapping(engine, "bad-tenant", "missing-tenant", "role-1")
|
||||
with engine.connect() as connection:
|
||||
all_mappings = self._mapping_rows(connection)
|
||||
command.downgrade(config, previous)
|
||||
command.upgrade(config, REPAIR_REVISION)
|
||||
with engine.connect() as connection:
|
||||
self.assertEqual(self._mapping_rows(connection), all_mappings)
|
||||
self.assertEqual(self._parent_rows(connection), parents_before)
|
||||
finally:
|
||||
engine.dispose()
|
||||
|
||||
@staticmethod
|
||||
def _insert_mapping(engine, mapping_id: str, tenant_id: str, role_id: str, *, function_id: str = "function-1") -> None:
|
||||
now = datetime.now(timezone.utc)
|
||||
with Session(engine) as session:
|
||||
session.add(ExternalFunctionRoleAssignment(
|
||||
id=mapping_id, tenant_id=tenant_id, role_id=role_id,
|
||||
source_module="organizations", function_id=function_id,
|
||||
settings={"meaning": "Existing mapping", "nested": {"retained": True}},
|
||||
created_at=now, updated_at=now,
|
||||
))
|
||||
session.commit()
|
||||
|
||||
@staticmethod
|
||||
def _mapping_rows(connection):
|
||||
return [dict(row) for row in connection.execute(text("SELECT * FROM access_external_function_role_assignments ORDER BY id")).mappings()]
|
||||
|
||||
@staticmethod
|
||||
def _parent_rows(connection):
|
||||
return {
|
||||
"roles": [dict(row) for row in connection.execute(text("SELECT * FROM access_roles ORDER BY id")).mappings()],
|
||||
"tenants": [dict(row) for row in connection.execute(text("SELECT * FROM core_scopes ORDER BY id")).mappings()],
|
||||
}
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -6,6 +6,12 @@ from govoplan_access.backend.manifest import manifest
|
||||
|
||||
|
||||
class InterfaceDocumentationContractTests(unittest.TestCase):
|
||||
def test_password_change_flag_is_documented_as_unenforced(self) -> None:
|
||||
topic = next(item for item in manifest.documentation if item.id == "access.reference.authentication-fields")
|
||||
self.assertIn("currently advisory metadata", topic.body)
|
||||
self.assertIn("server-side enforcement are not implemented", topic.body)
|
||||
self.assertIn("serverseitige Durchsetzung sind noch nicht umgesetzt", topic.translations["de"]["body"])
|
||||
|
||||
def test_all_static_topics_have_complete_german_content(self) -> None:
|
||||
for topic in manifest.documentation:
|
||||
german = (topic.translations or {}).get("de", {})
|
||||
|
||||
@@ -30,6 +30,34 @@ class PermissionCatalogContractTests(unittest.TestCase):
|
||||
self.assertIn("files:file:read", scopes)
|
||||
self.assertIn("files:read", scopes)
|
||||
|
||||
def test_api_key_intersection_excludes_canonical_and_legacy_system_scopes(self) -> None:
|
||||
for scope in ("access:system_credential:write", "access:tenant:create", "system:tenants:create"):
|
||||
with self.subTest(scope=scope):
|
||||
self.assertEqual([], access_catalog.intersect_api_key_scopes([scope], [scope]))
|
||||
|
||||
def test_api_key_module_wildcards_expand_only_to_concrete_tenant_scopes(self) -> None:
|
||||
scopes = access_catalog.intersect_api_key_scopes(["access:*"], ["access:*"])
|
||||
self.assertIn("access:membership:read", scopes)
|
||||
self.assertNotIn("access:*", scopes)
|
||||
self.assertFalse(access_catalog.scopes_grant(scopes, "access:system_credential:write"))
|
||||
catalog = access_catalog.permission_map()
|
||||
self.assertTrue(all(catalog[scope].level == "tenant" for scope in scopes if scope in catalog))
|
||||
|
||||
def test_api_key_intersection_preserves_unknown_concrete_module_grants(self) -> None:
|
||||
self.assertEqual(
|
||||
["optional-module:record:read"],
|
||||
access_catalog.intersect_api_key_scopes(["optional-module:record:read"], ["optional-module:record:read"]),
|
||||
)
|
||||
|
||||
def test_api_key_intersection_preserves_concrete_tenant_compatibility_aliases(self) -> None:
|
||||
scopes = access_catalog.intersect_api_key_scopes(["files:read"], ["files:file:read"])
|
||||
self.assertIn("files:read", scopes)
|
||||
self.assertIn("files:file:read", scopes)
|
||||
self.assertTrue(access_catalog.scopes_grant(scopes, "files:file:read"))
|
||||
|
||||
def test_api_key_intersection_does_not_retain_unknown_wildcards(self) -> None:
|
||||
self.assertEqual([], access_catalog.intersect_api_key_scopes(["optional-module:*"], ["optional-module:*"]))
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
@@ -3,7 +3,7 @@ from __future__ import annotations
|
||||
import unittest
|
||||
from datetime import datetime, timedelta, timezone
|
||||
|
||||
from sqlalchemy import create_engine
|
||||
from sqlalchemy import create_engine, event
|
||||
from sqlalchemy.orm import sessionmaker
|
||||
|
||||
from govoplan_access.backend.db.base import AccessBase
|
||||
@@ -182,6 +182,36 @@ class SessionManagementTests(unittest.TestCase):
|
||||
self.assertIsNone(hidden)
|
||||
self.assertFalse(changed)
|
||||
|
||||
def test_listing_applies_activity_filter_and_limit_before_loading_history(self) -> None:
|
||||
statements: list[str] = []
|
||||
|
||||
def capture_query(connection, cursor, statement, parameters, context, executemany):
|
||||
if statement.lstrip().upper().startswith("SELECT") and "access_auth_sessions" in statement:
|
||||
statements.append(statement)
|
||||
|
||||
event.listen(self.engine, "before_cursor_execute", capture_query)
|
||||
try:
|
||||
for include_inactive in (False, True):
|
||||
with self.subTest(include_inactive=include_inactive):
|
||||
statements.clear()
|
||||
summaries = list_account_sessions(
|
||||
self.session,
|
||||
account_id="account-1",
|
||||
current_session_id="session-current",
|
||||
include_inactive=include_inactive,
|
||||
limit=1,
|
||||
now=self.now,
|
||||
)
|
||||
self.assertEqual(1, len(summaries))
|
||||
self.assertEqual(1, len(statements))
|
||||
self.assertIn("LIMIT", statements[0])
|
||||
if not include_inactive:
|
||||
self.assertEqual("active", summaries[0].status)
|
||||
self.assertIn("revoked_at IS NULL", statements[0])
|
||||
self.assertIn("expires_at >", statements[0])
|
||||
finally:
|
||||
event.remove(self.engine, "before_cursor_execute", capture_query)
|
||||
|
||||
def test_current_session_is_protected_and_revoke_others_skips_expired(self) -> None:
|
||||
with self.assertRaisesRegex(ValueError, "current session"):
|
||||
revoke_account_session(
|
||||
|
||||
Reference in New Issue
Block a user