From d2e34b323af28949466c608a28242fc02ddd254f Mon Sep 17 00:00:00 2001 From: Albrecht Degering Date: Mon, 20 Jul 2026 20:03:11 +0200 Subject: [PATCH] refactor(idm): consume identity and organization contracts --- src/govoplan_idm/backend/api/v1/routes.py | 166 +++++++++++++--------- src/govoplan_idm/backend/directory.py | 60 ++++++-- src/govoplan_idm/backend/manifest.py | 19 ++- tests/test_directory.py | 138 ++++++++++++++++++ 4 files changed, 298 insertions(+), 85 deletions(-) create mode 100644 tests/test_directory.py diff --git a/src/govoplan_idm/backend/api/v1/routes.py b/src/govoplan_idm/backend/api/v1/routes.py index 134b20e..e52daa2 100644 --- a/src/govoplan_idm/backend/api/v1/routes.py +++ b/src/govoplan_idm/backend/api/v1/routes.py @@ -5,7 +5,6 @@ from typing import Any, TypeVar from fastapi import APIRouter, Depends, HTTPException, Query, status from fastapi.encoders import jsonable_encoder -from sqlalchemy import func, or_ from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import Session @@ -18,11 +17,21 @@ from govoplan_core.core.configuration_control import ( ensure_configuration_change_allowed, record_configuration_change_applied, ) +from govoplan_core.core.identity import ( + CAPABILITY_IDENTITY_DIRECTORY, + CAPABILITY_IDENTITY_SEARCH, + IdentityDirectory, + IdentityRef, + IdentitySearchProvider, +) +from govoplan_core.core.organizations import ( + CAPABILITY_ORGANIZATION_DIRECTORY, + OrganizationDirectory, + OrganizationFunctionRef, +) from govoplan_core.core.runtime import get_registry from govoplan_core.db.session import get_session -from govoplan_identity.backend.db.models import Identity, IdentityAccountLink from govoplan_idm.backend.db.models import IdmOrganizationFunctionAssignment, IdmTenantSettings -from govoplan_organizations.backend.db.models import OrganizationFunction from .schemas import ( IdmSettingsItem, @@ -126,31 +135,76 @@ def _default_settings(tenant_id: str) -> IdmSettingsItem: ) -def _ensure_identity(session: Session, identity_id: str) -> None: - identity = session.get(Identity, identity_id) - if identity is None: +def _identity_directory() -> IdentityDirectory: + registry = get_registry() + if registry is None or not registry.has_capability(CAPABILITY_IDENTITY_DIRECTORY): + raise HTTPException( + status_code=status.HTTP_503_SERVICE_UNAVAILABLE, + detail="Identity directory is unavailable", + ) + capability = registry.require_capability(CAPABILITY_IDENTITY_DIRECTORY) + if not isinstance(capability, IdentityDirectory): + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail=f"Invalid capability: {CAPABILITY_IDENTITY_DIRECTORY}", + ) + return capability + + +def _identity_search() -> IdentitySearchProvider: + registry = get_registry() + if registry is None or not registry.has_capability(CAPABILITY_IDENTITY_SEARCH): + raise HTTPException( + status_code=status.HTTP_503_SERVICE_UNAVAILABLE, + detail="Identity search is unavailable", + ) + capability = registry.require_capability(CAPABILITY_IDENTITY_SEARCH) + if not isinstance(capability, IdentitySearchProvider): + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail=f"Invalid capability: {CAPABILITY_IDENTITY_SEARCH}", + ) + return capability + + +def _organization_directory() -> OrganizationDirectory: + registry = get_registry() + if registry is None or not registry.has_capability(CAPABILITY_ORGANIZATION_DIRECTORY): + raise HTTPException( + status_code=status.HTTP_503_SERVICE_UNAVAILABLE, + detail="Organization directory is unavailable", + ) + capability = registry.require_capability(CAPABILITY_ORGANIZATION_DIRECTORY) + if not isinstance(capability, OrganizationDirectory): + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail=f"Invalid capability: {CAPABILITY_ORGANIZATION_DIRECTORY}", + ) + return capability + + +def _organization_function(function_id: str, tenant_id: str) -> OrganizationFunctionRef: + function = _organization_directory().get_function(function_id) + if function is None or function.tenant_id != tenant_id: + raise _not_found("Organization function") + return function + + +def _ensure_identity(identity_id: str) -> None: + if _identity_directory().get_identity(identity_id) is None: raise _not_found("Identity") -def _ensure_account_link(session: Session, identity_id: str, account_id: str | None) -> None: +def _ensure_account_link(identity_id: str, account_id: str | None) -> None: if account_id is None: return - exists = ( - session.query(IdentityAccountLink) - .filter(IdentityAccountLink.identity_id == identity_id, IdentityAccountLink.account_id == account_id) - .first() - ) - if exists is None: + links = _identity_directory().accounts_for_identity(identity_id) + if not any(link.account_id == account_id for link in links): raise _invalid("Account is not linked to the selected identity.") -def _account_linked_to_identity(session: Session, identity_id: str, account_id: str) -> bool: - return ( - session.query(IdentityAccountLink) - .filter(IdentityAccountLink.identity_id == identity_id, IdentityAccountLink.account_id == account_id) - .first() - is not None - ) +def _account_linked_to_identity(identity_id: str, account_id: str) -> bool: + return any(link.account_id == account_id for link in _identity_directory().accounts_for_identity(identity_id)) def _ensure_assignment_workflow( @@ -160,13 +214,13 @@ def _ensure_assignment_workflow( tenant_id: str, ) -> None: _ensure_assignment_shape(item) - function = _get_tenant_row(session, OrganizationFunction, item.function_id, tenant_id, "Organization function") + function = _organization_function(item.function_id, tenant_id) base = _assignment_source_assignment(session, item, tenant_id=tenant_id) _ensure_assignment_source_rules( item, function=function, base=base, - account_linked_to_identity=lambda identity_id, account_id: _account_linked_to_identity(session, identity_id, account_id), + account_linked_to_identity=_account_linked_to_identity, ) @@ -198,7 +252,7 @@ def _assignment_source_assignment( def _ensure_assignment_source_rules( item: IdmOrganizationFunctionAssignment, *, - function: OrganizationFunction, + function: OrganizationFunctionRef, base: IdmOrganizationFunctionAssignment | None, account_linked_to_identity: Callable[[str, str], bool], ) -> None: @@ -214,7 +268,7 @@ def _ensure_assignment_source_rules( def _ensure_delegated_assignment( item: IdmOrganizationFunctionAssignment, *, - function: OrganizationFunction, + function: OrganizationFunctionRef, base: IdmOrganizationFunctionAssignment | None, ) -> None: if base is None: @@ -230,7 +284,7 @@ def _ensure_delegated_assignment( def _ensure_acting_for_assignment( item: IdmOrganizationFunctionAssignment, *, - function: OrganizationFunction, + function: OrganizationFunctionRef, base: IdmOrganizationFunctionAssignment | None, account_linked_to_identity: Callable[[str, str], bool], ) -> None: @@ -434,9 +488,9 @@ def create_organization_function_assignment( ) -> OrganizationFunctionAssignmentItem: tenant_id = _tenant_id(principal) approval, target, _value = _ensure_assignment_change_allowed(session, principal, tenant_id=tenant_id, operation="created", payload=payload) - _ensure_identity(session, payload.identity_id) - _ensure_account_link(session, payload.identity_id, payload.account_id) - function = _get_tenant_row(session, OrganizationFunction, payload.function_id, tenant_id, "Organization function") + _ensure_identity(payload.identity_id) + _ensure_account_link(payload.identity_id, payload.account_id) + function = _organization_function(payload.function_id, tenant_id) item = IdmOrganizationFunctionAssignment( tenant_id=tenant_id, identity_id=payload.identity_id, @@ -486,7 +540,7 @@ def update_organization_function_assignment( if "function_id" in payload.model_fields_set: if payload.function_id is None: raise _invalid("Function is required.") - function = _get_tenant_row(session, OrganizationFunction, payload.function_id, tenant_id, "Organization function") + function = _organization_function(payload.function_id, tenant_id) item.function_id = function.id item.organization_unit_id = function.organization_unit_id identity_id = payload.identity_id if "identity_id" in payload.model_fields_set else item.identity_id @@ -494,9 +548,9 @@ def update_organization_function_assignment( if "identity_id" in payload.model_fields_set: if payload.identity_id is None: raise _invalid("Identity is required.") - _ensure_identity(session, payload.identity_id) + _ensure_identity(payload.identity_id) item.identity_id = payload.identity_id - _ensure_account_link(session, identity_id, account_id) + _ensure_account_link(identity_id, account_id) if "source" in payload.model_fields_set and payload.source is None: raise _invalid("Assignment source is required.") if "applies_to_subunits" in payload.model_fields_set and payload.applies_to_subunits is None: @@ -543,53 +597,25 @@ def list_organization_identity_candidates( session: Session = Depends(get_session), principal: ApiPrincipal = Depends(require_any_scope(*ORGANIZATION_IDENTITY_READ_SCOPES)), ) -> OrganizationIdentityCandidateList: - del principal - identity_query = session.query(Identity) - if not include_inactive: - identity_query = identity_query.filter(Identity.is_active.is_(True)) - if query: - pattern = f"%{query.strip().casefold()}%" - matching_account_links = session.query(IdentityAccountLink.identity_id).filter( - func.lower(IdentityAccountLink.account_id).like(pattern) - ) - identity_query = identity_query.filter( - or_( - func.lower(Identity.id).like(pattern), - func.lower(Identity.display_name).like(pattern), - func.lower(Identity.external_subject).like(pattern), - Identity.id.in_(matching_account_links), - ) - ) - - identities = identity_query.order_by(Identity.display_name.asc(), Identity.id.asc()).limit(limit).all() - identity_ids = [identity.id for identity in identities] - links_by_identity: dict[str, list[IdentityAccountLink]] = {identity_id: [] for identity_id in identity_ids} - if identity_ids: - links = ( - session.query(IdentityAccountLink) - .filter(IdentityAccountLink.identity_id.in_(identity_ids)) - .order_by(IdentityAccountLink.is_primary.desc(), IdentityAccountLink.account_id.asc()) - .all() - ) - for link in links: - links_by_identity.setdefault(link.identity_id, []).append(link) + del session, principal + identities = _identity_search().search_identities( + query, + include_inactive=include_inactive, + limit=limit, + ) return OrganizationIdentityCandidateList( - identities=[ - _identity_candidate(identity, links_by_identity.get(identity.id, [])) - for identity in identities - ] + identities=[_identity_candidate(identity) for identity in identities] ) -def _identity_candidate(identity: Identity, links: list[IdentityAccountLink]) -> OrganizationIdentityCandidate: - primary_link = next((link for link in links if link.is_primary), None) +def _identity_candidate(identity: IdentityRef) -> OrganizationIdentityCandidate: return OrganizationIdentityCandidate( id=identity.id, display_name=identity.display_name, external_subject=identity.external_subject, source=identity.source, - primary_account_id=primary_link.account_id if primary_link is not None else None, - account_ids=[link.account_id for link in links], - status="active" if identity.is_active else "inactive", + primary_account_id=identity.primary_account_id, + account_ids=list(identity.account_ids), + status=identity.status, ) diff --git a/src/govoplan_idm/backend/directory.py b/src/govoplan_idm/backend/directory.py index 65e0ef2..5b8146f 100644 --- a/src/govoplan_idm/backend/directory.py +++ b/src/govoplan_idm/backend/directory.py @@ -2,12 +2,12 @@ from __future__ import annotations from sqlalchemy import or_ +from govoplan_core.core.identity import IdentityDirectory from govoplan_core.core.idm import IdmDirectory, OrganizationFunctionAssignmentRef +from govoplan_core.core.organizations import OrganizationDirectory from govoplan_core.db.session import get_database from govoplan_core.security.time import utc_now -from govoplan_identity.backend.db.models import IdentityAccountLink from govoplan_idm.backend.db.models import IdmOrganizationFunctionAssignment -from govoplan_organizations.backend.db.models import OrganizationFunction def _status(active: bool) -> str: @@ -33,6 +33,10 @@ def _assignment_ref(item: IdmOrganizationFunctionAssignment) -> OrganizationFunc class SqlIdmDirectory(IdmDirectory): + def __init__(self, *, identities: IdentityDirectory, organizations: OrganizationDirectory) -> None: + self._identities = identities + self._organizations = organizations + def get_organization_function_assignment(self, assignment_id: str) -> OrganizationFunctionAssignmentRef | None: with get_database().session() as session: item = session.get(IdmOrganizationFunctionAssignment, assignment_id) @@ -52,13 +56,7 @@ class SqlIdmDirectory(IdmDirectory): *, tenant_id: str | None = None, ) -> tuple[OrganizationFunctionAssignmentRef, ...]: - with get_database().session() as session: - identity_ids = [ - row[0] - for row in session.query(IdentityAccountLink.identity_id) - .filter(IdentityAccountLink.account_id == account_id) - .all() - ] + identity_ids = [identity.id for identity in self._identities.identities_for_accounts((account_id,))] if not identity_ids: return () return self._assignments_for_identity_ids(identity_ids=tuple(identity_ids), tenant_id=tenant_id, account_id=account_id) @@ -76,11 +74,9 @@ class SqlIdmDirectory(IdmDirectory): with get_database().session() as session: query = ( session.query(IdmOrganizationFunctionAssignment) - .join(OrganizationFunction, OrganizationFunction.id == IdmOrganizationFunctionAssignment.function_id) .filter( IdmOrganizationFunctionAssignment.identity_id.in_(identity_ids), IdmOrganizationFunctionAssignment.is_active.is_(True), - OrganizationFunction.is_active.is_(True), or_(IdmOrganizationFunctionAssignment.valid_from.is_(None), IdmOrganizationFunctionAssignment.valid_from <= now), or_(IdmOrganizationFunctionAssignment.valid_until.is_(None), IdmOrganizationFunctionAssignment.valid_until > now), ) @@ -90,4 +86,44 @@ class SqlIdmDirectory(IdmDirectory): query = query.filter(or_(IdmOrganizationFunctionAssignment.account_id.is_(None), IdmOrganizationFunctionAssignment.account_id == account_id)) if tenant_id is not None: query = query.filter(IdmOrganizationFunctionAssignment.tenant_id == tenant_id) - return tuple(_assignment_ref(item) for item in query.all()) + items = query.all() + source_ids = { + item.delegated_from_assignment_id + for item in items + if item.source in {"delegated", "acting_for"} + and item.delegated_from_assignment_id is not None + } + effective_sources = ( + session.query(IdmOrganizationFunctionAssignment) + .filter( + IdmOrganizationFunctionAssignment.id.in_(source_ids), + IdmOrganizationFunctionAssignment.is_active.is_(True), + or_( + IdmOrganizationFunctionAssignment.valid_from.is_(None), + IdmOrganizationFunctionAssignment.valid_from <= now, + ), + or_( + IdmOrganizationFunctionAssignment.valid_until.is_(None), + IdmOrganizationFunctionAssignment.valid_until > now, + ), + ) + .all() + if source_ids + else () + ) + effective_sources_by_id = {item.id: item for item in effective_sources} + active_items: list[IdmOrganizationFunctionAssignment] = [] + for item in items: + if item.source in {"delegated", "acting_for"}: + source = effective_sources_by_id.get(item.delegated_from_assignment_id or "") + if ( + source is None + or source.tenant_id != item.tenant_id + or source.function_id != item.function_id + ): + continue + function = self._organizations.get_function(item.function_id) + if function is None or function.status != "active" or function.tenant_id != item.tenant_id: + continue + active_items.append(item) + return tuple(_assignment_ref(item) for item in active_items) diff --git a/src/govoplan_idm/backend/manifest.py b/src/govoplan_idm/backend/manifest.py index de39d49..64c5371 100644 --- a/src/govoplan_idm/backend/manifest.py +++ b/src/govoplan_idm/backend/manifest.py @@ -3,7 +3,9 @@ from __future__ import annotations from pathlib import Path from govoplan_core.core.access import CAPABILITY_AUTH_PERMISSION_EVALUATOR, CAPABILITY_AUTH_PRINCIPAL_RESOLVER +from govoplan_core.core.identity import CAPABILITY_IDENTITY_DIRECTORY, CAPABILITY_IDENTITY_SEARCH, IdentityDirectory from govoplan_core.core.idm import CAPABILITY_IDM_DIRECTORY +from govoplan_core.core.organizations import CAPABILITY_ORGANIZATION_DIRECTORY, OrganizationDirectory from govoplan_core.core.module_guards import persistent_table_uninstall_guard from govoplan_core.core.modules import ( DocumentationTopic, @@ -93,10 +95,15 @@ def _route_factory(context: ModuleContext): def _idm_directory(context: ModuleContext) -> object: - del context from govoplan_idm.backend.directory import SqlIdmDirectory - return SqlIdmDirectory() + identities = context.registry.require_capability(CAPABILITY_IDENTITY_DIRECTORY) + organizations = context.registry.require_capability(CAPABILITY_ORGANIZATION_DIRECTORY) + if not isinstance(identities, IdentityDirectory): + raise RuntimeError(f"Invalid capability: {CAPABILITY_IDENTITY_DIRECTORY}") + if not isinstance(organizations, OrganizationDirectory): + raise RuntimeError(f"Invalid capability: {CAPABILITY_ORGANIZATION_DIRECTORY}") + return SqlIdmDirectory(identities=identities, organizations=organizations) manifest = ModuleManifest( @@ -105,7 +112,13 @@ manifest = ModuleManifest( version="0.1.8", dependencies=("identity", "organizations"), optional_dependencies=("access", "audit"), - required_capabilities=(CAPABILITY_AUTH_PRINCIPAL_RESOLVER, CAPABILITY_AUTH_PERMISSION_EVALUATOR), + required_capabilities=( + CAPABILITY_AUTH_PRINCIPAL_RESOLVER, + CAPABILITY_AUTH_PERMISSION_EVALUATOR, + CAPABILITY_IDENTITY_DIRECTORY, + CAPABILITY_IDENTITY_SEARCH, + CAPABILITY_ORGANIZATION_DIRECTORY, + ), permissions=PERMISSIONS, role_templates=ROLE_TEMPLATES, route_factory=_route_factory, diff --git a/tests/test_directory.py b/tests/test_directory.py new file mode 100644 index 0000000..f3927dd --- /dev/null +++ b/tests/test_directory.py @@ -0,0 +1,138 @@ +from __future__ import annotations + +import unittest +from datetime import datetime, timedelta, timezone + +from govoplan_core.core.identity import IdentityAccountLinkRef, IdentityRef +from govoplan_core.core.organizations import OrganizationFunctionRef +from govoplan_core.db.base import Base +from govoplan_core.db.session import configure_database, reset_database +from govoplan_identity.backend.db import models as identity_models # noqa: F401 - resolve assignment foreign keys +from govoplan_idm.backend.db.models import IdmOrganizationFunctionAssignment +from govoplan_idm.backend.directory import SqlIdmDirectory +from govoplan_organizations.backend.db import models as organization_models # noqa: F401 - resolve assignment foreign keys + + +class StubIdentityDirectory: + def get_identity(self, identity_id: str) -> IdentityRef | None: + return None + + def identity_for_account(self, account_id: str) -> IdentityRef | None: + return None + + def identities_for_accounts(self, account_ids: tuple[str, ...]) -> tuple[IdentityRef, ...]: + return () + + def accounts_for_identity(self, identity_id: str) -> tuple[IdentityAccountLinkRef, ...]: + return () + + +class StubOrganizationDirectory: + def get_function(self, function_id: str) -> OrganizationFunctionRef | None: + return OrganizationFunctionRef( + id=function_id, + tenant_id="tenant-1", + organization_unit_id="unit-1", + slug=function_id, + name=function_id, + status="active", + ) + + def get_organization_unit(self, organization_unit_id: str): + return None + + def organization_units_for_tenant(self, tenant_id: str): + return () + + def functions_for_organization_unit(self, organization_unit_id: str, *, include_subunits: bool = False): + return () + + +class IdmDirectoryDelegationTests(unittest.TestCase): + def setUp(self) -> None: + self.database = configure_database("sqlite:///:memory:") + Base.metadata.create_all( + self.database.engine, + tables=[IdmOrganizationFunctionAssignment.__table__], + ) + self.directory = SqlIdmDirectory( + identities=StubIdentityDirectory(), # type: ignore[arg-type] + organizations=StubOrganizationDirectory(), # type: ignore[arg-type] + ) + + def tearDown(self) -> None: + reset_database(dispose=True) + + def _source_and_child( + self, + *, + case: str, + child_source: str = "delegated", + source_tenant_id: str = "tenant-1", + source_function_id: str = "function-1", + source_is_active: bool = True, + source_valid_from: datetime | None = None, + source_valid_until: datetime | None = None, + ) -> tuple[IdmOrganizationFunctionAssignment, IdmOrganizationFunctionAssignment]: + source = IdmOrganizationFunctionAssignment( + id=f"source-{case}", + tenant_id=source_tenant_id, + identity_id=f"source-identity-{case}", + function_id=source_function_id, + organization_unit_id="unit-1", + source="direct", + is_active=source_is_active, + valid_from=source_valid_from, + valid_until=source_valid_until, + settings={}, + ) + child = IdmOrganizationFunctionAssignment( + id=f"child-{case}", + tenant_id="tenant-1", + identity_id=f"target-identity-{case}", + function_id="function-1", + organization_unit_id="unit-1", + source=child_source, + delegated_from_assignment_id=source.id, + acting_for_account_id="represented-account" if child_source == "acting_for" else None, + is_active=True, + settings={}, + ) + return source, child + + def test_effective_delegations_require_a_current_matching_source(self) -> None: + now = datetime.now(timezone.utc) + cases = ( + self._source_and_child(case="valid"), + self._source_and_child(case="acting", child_source="acting_for"), + self._source_and_child(case="revoked", source_is_active=False), + self._source_and_child(case="expired", source_valid_until=now - timedelta(days=1)), + self._source_and_child(case="future", source_valid_from=now + timedelta(days=1)), + self._source_and_child(case="wrong-tenant", source_tenant_id="tenant-2"), + self._source_and_child(case="wrong-function", source_function_id="function-2"), + ) + with self.database.session() as session: + for source, child in cases: + session.add_all((source, child)) + session.commit() + + expected = { + "valid": ("child-valid",), + "acting": ("child-acting",), + "revoked": (), + "expired": (), + "future": (), + "wrong-tenant": (), + "wrong-function": (), + } + for case, expected_ids in expected.items(): + with self.subTest(case=case): + assignments = self.directory.organization_function_assignments_for_identity( + f"target-identity-{case}", + tenant_id="tenant-1", + ) + self.assertEqual(expected_ids, tuple(item.id for item in assignments)) + + +if __name__ == "__main__": + unittest.main()