diff --git a/changes/12801.feature.md b/changes/12801.feature.md new file mode 100644 index 00000000000..b012689c75a --- /dev/null +++ b/changes/12801.feature.md @@ -0,0 +1 @@ +Bind AppConfig fragments to their RBAC scope on write (repository layer): a `user` / `domain` fragment is associated to its RBAC scope on create and unbound on purge (`public` maps to GLOBAL and carries none), so fragment write authorization can be enforced by RBAC. The allow list becomes a read allowlist (registration + `rank`) while writes move to RBAC (BEP-1052). diff --git a/src/ai/backend/common/data/app_config/types.py b/src/ai/backend/common/data/app_config/types.py index a473c714f00..0846d93caf1 100644 --- a/src/ai/backend/common/data/app_config/types.py +++ b/src/ai/backend/common/data/app_config/types.py @@ -4,7 +4,7 @@ import enum -from ai.backend.common.data.permission.types import ScopeType +from ai.backend.common.data.permission.types import RBACElementType, ScopeType __all__ = ("AppConfigScopeType",) @@ -34,6 +34,21 @@ def to_rbac_scope_type(self) -> ScopeType: case AppConfigScopeType.USER: return ScopeType.USER + def to_rbac_element_type(self) -> RBACElementType | None: + """The RBAC scope element a fragment at this scope belongs to. + + ``public`` maps to the global scope, which has no RBAC scope element — a public + fragment is *global-scoped* (no scope association; superadmin-only writes), so it + returns ``None``. + """ + match self: + case AppConfigScopeType.PUBLIC: + return None + case AppConfigScopeType.DOMAIN: + return RBACElementType.DOMAIN + case AppConfigScopeType.USER: + return RBACElementType.USER + def to_rbac_scope_id(self, scope_id: str) -> str: """The RBAC scope id for a write at this fragment scope. diff --git a/src/ai/backend/common/data/permission/types.py b/src/ai/backend/common/data/permission/types.py index b052fbb57fa..7f002a5e654 100644 --- a/src/ai/backend/common/data/permission/types.py +++ b/src/ai/backend/common/data/permission/types.py @@ -270,6 +270,7 @@ def _resource_types(cls) -> set[EntityType]: cls.ARTIFACT, cls.ARTIFACT_REGISTRY, cls.APP_CONFIG, + cls.APP_CONFIG_FRAGMENT, cls.NOTIFICATION_CHANNEL, cls.NOTIFICATION_RULE, cls.MODEL_DEPLOYMENT, diff --git a/src/ai/backend/manager/models/rbac_models/migration/enums.py b/src/ai/backend/manager/models/rbac_models/migration/enums.py index 9dadc475272..048d0a9c089 100644 --- a/src/ai/backend/manager/models/rbac_models/migration/enums.py +++ b/src/ai/backend/manager/models/rbac_models/migration/enums.py @@ -115,6 +115,7 @@ class EntityType(enum.StrEnum): ARTIFACT = "artifact" ARTIFACT_REGISTRY = "artifact_registry" APP_CONFIG = "app_config" + APP_CONFIG_FRAGMENT = "app_config_fragment" NOTIFICATION_CHANNEL = "notification_channel" NOTIFICATION_RULE = "notification_rule" MODEL_DEPLOYMENT = "model_deployment" @@ -142,6 +143,7 @@ def _resource_types(cls) -> set[EntityType]: cls.ARTIFACT, cls.ARTIFACT_REGISTRY, cls.APP_CONFIG, + cls.APP_CONFIG_FRAGMENT, cls.NOTIFICATION_CHANNEL, cls.NOTIFICATION_RULE, cls.MODEL_DEPLOYMENT, diff --git a/src/ai/backend/manager/repositories/app_config_fragment/db_source/db_source.py b/src/ai/backend/manager/repositories/app_config_fragment/db_source/db_source.py index 45513ab2872..b63d0916b89 100644 --- a/src/ai/backend/manager/repositories/app_config_fragment/db_source/db_source.py +++ b/src/ai/backend/manager/repositories/app_config_fragment/db_source/db_source.py @@ -6,6 +6,7 @@ import sqlalchemy as sa +from ai.backend.common.data.permission.types import RBACElementType from ai.backend.common.exception import BackendAIError from ai.backend.common.identifier.app_config_fragment import AppConfigFragmentID from ai.backend.common.metrics.metric import DomainType, LayerType @@ -25,19 +26,26 @@ from ai.backend.manager.models.app_config_fragment.conditions import AppConfigFragmentConditions from ai.backend.manager.models.app_config_fragment.row import AppConfigFragmentRow from ai.backend.manager.models.scopes import SearchScope +from ai.backend.manager.repositories.app_config_fragment.creators import ( + AppConfigFragmentCreatorSpec, +) +from ai.backend.manager.repositories.app_config_fragment.scope_binders import ( + AppConfigFragmentScopeUnbinder, + fragment_rbac_scope_ref, +) from ai.backend.manager.repositories.app_config_fragment.types import ( AppConfigScopeArguments, ) from ai.backend.manager.repositories.base import ( BatchQuerier, BulkCreator, - Creator, NoPagination, Purger, Querier, Updater, ) -from ai.backend.manager.repositories.ops import DBOpsProvider +from ai.backend.manager.repositories.base.rbac.entity_creator import RBACEntityCreator +from ai.backend.manager.repositories.ops.rbac.provider import RBACOpsProvider __all__ = ("AppConfigFragmentDBSource",) @@ -64,19 +72,27 @@ class AppConfigFragmentDBSource: """Database source for app config fragment operations.""" - _ops: DBOpsProvider + _ops: RBACOpsProvider - def __init__(self, ops_provider: DBOpsProvider) -> None: + def __init__(self, ops_provider: RBACOpsProvider) -> None: self._ops = ops_provider @app_config_fragment_db_source_resilience.apply() - async def create(self, creator: Creator[AppConfigFragmentRow]) -> AppConfigFragmentData: - # The FK to the allow-list is the gate: inserting a fragment with no - # allow-list row for its ``(config_name, scope_type)`` raises - # ``AppConfigFragmentWriteNotAllowed`` (see the spec's integrity checks). + async def create(self, spec: AppConfigFragmentCreatorSpec) -> AppConfigFragmentData: + # The FK to the allow-list gates *existence* (a missing allow-list row for the + # ``(config_name, scope_type)`` raises ``AppConfigFragmentWriteNotAllowed``). Create + # also binds the fragment to its owning RBAC scope via + # ``association_scopes_entities`` (same tx) so a later update/purge can resolve its + # scope for the RBAC write check; a ``public`` fragment is global-scoped + # (``scope_ref`` is ``None``) and carries no association. + rbac_creator = RBACEntityCreator( + spec=spec, + element_type=RBACElementType.APP_CONFIG_FRAGMENT, + scope_ref=fragment_rbac_scope_ref(spec.scope_type, spec.scope_id), + ) async with self._ops.write_ops() as w: - created = await w.create(creator) - return created.row.to_data() + created = await w.bulk_create_scoped([rbac_creator]) + return created.rows[0].to_data() @app_config_fragment_db_source_resilience.apply() async def get_by_id(self, fragment_id: AppConfigFragmentID) -> AppConfigFragmentData: @@ -99,12 +115,23 @@ async def update(self, updater: Updater[AppConfigFragmentRow]) -> AppConfigFragm @app_config_fragment_db_source_resilience.apply() async def purge(self, purger: Purger[AppConfigFragmentRow]) -> AppConfigFragmentData: - # No write-gate here — see ``update``. + # A fragment is a config bound at a scope, so purging is an RBAC *unbind*: the row + # and its scope association are deleted atomically (the association table has no FK + # cascade). A public fragment is global-scoped and simply has no association. The + # row is fetched first to resolve its scope and to return its data. async with self._ops.write_ops() as w: - result = await w.purge(purger) - if result is None: + found = await w.query(Querier(row_class=AppConfigFragmentRow, pk_value=purger.pk_value)) + if found is None: raise AppConfigFragmentNotFound(f"App config fragment {purger.pk_value} not found") - return result.row.to_data() + data = found.row.to_data() + await w.unbind_scope_entities( + AppConfigFragmentScopeUnbinder( + fragment_id=data.id, + fragment_scope_type=data.scope_type, + fragment_scope_id=data.scope_id, + ) + ) + return data @app_config_fragment_db_source_resilience.apply() async def bulk_create( diff --git a/src/ai/backend/manager/repositories/app_config_fragment/repositories.py b/src/ai/backend/manager/repositories/app_config_fragment/repositories.py index 93dca9a4a7a..037ae015eab 100644 --- a/src/ai/backend/manager/repositories/app_config_fragment/repositories.py +++ b/src/ai/backend/manager/repositories/app_config_fragment/repositories.py @@ -4,6 +4,7 @@ from ai.backend.manager.repositories.app_config_fragment.repository import ( AppConfigFragmentRepository, ) +from ai.backend.manager.repositories.ops.rbac.provider import RBACOpsProvider from ai.backend.manager.repositories.types import RepositoryArgs @@ -13,6 +14,8 @@ class AppConfigFragmentRepositories: @classmethod def create(cls, args: RepositoryArgs) -> Self: + # Fragment writes bind the fragment to its RBAC scope (see the db_source), so the + # repository runs on the RBAC-scoped ops provider rather than the plain one. return cls( - repository=AppConfigFragmentRepository(args.ops_provider), + repository=AppConfigFragmentRepository(RBACOpsProvider(args.db)), ) diff --git a/src/ai/backend/manager/repositories/app_config_fragment/repository.py b/src/ai/backend/manager/repositories/app_config_fragment/repository.py index 43334e915c5..be078bb1693 100644 --- a/src/ai/backend/manager/repositories/app_config_fragment/repository.py +++ b/src/ai/backend/manager/repositories/app_config_fragment/repository.py @@ -15,6 +15,9 @@ ) from ai.backend.manager.models.app_config_fragment.row import AppConfigFragmentRow from ai.backend.manager.models.scopes import SearchScope +from ai.backend.manager.repositories.app_config_fragment.creators import ( + AppConfigFragmentCreatorSpec, +) from ai.backend.manager.repositories.app_config_fragment.db_source import ( AppConfigFragmentDBSource, ) @@ -24,11 +27,10 @@ from ai.backend.manager.repositories.base import ( BatchQuerier, BulkCreator, - Creator, Purger, Updater, ) -from ai.backend.manager.repositories.ops import DBOpsProvider +from ai.backend.manager.repositories.ops.rbac.provider import RBACOpsProvider __all__ = ("AppConfigFragmentRepository",) @@ -57,12 +59,12 @@ class AppConfigFragmentRepository: _db_source: AppConfigFragmentDBSource - def __init__(self, ops_provider: DBOpsProvider) -> None: + def __init__(self, ops_provider: RBACOpsProvider) -> None: self._db_source = AppConfigFragmentDBSource(ops_provider) @app_config_fragment_repository_resilience.apply() - async def create(self, creator: Creator[AppConfigFragmentRow]) -> AppConfigFragmentData: - return await self._db_source.create(creator) + async def create(self, spec: AppConfigFragmentCreatorSpec) -> AppConfigFragmentData: + return await self._db_source.create(spec) @app_config_fragment_repository_resilience.apply() async def get_by_id(self, fragment_id: AppConfigFragmentID) -> AppConfigFragmentData: diff --git a/src/ai/backend/manager/repositories/app_config_fragment/scope_binders.py b/src/ai/backend/manager/repositories/app_config_fragment/scope_binders.py new file mode 100644 index 00000000000..e42d7b6d352 --- /dev/null +++ b/src/ai/backend/manager/repositories/app_config_fragment/scope_binders.py @@ -0,0 +1,70 @@ +"""RBAC scope binding helpers for app config fragments.""" + +from __future__ import annotations + +from collections.abc import Sequence +from dataclasses import dataclass +from typing import override + +import sqlalchemy as sa + +from ai.backend.common.data.app_config.types import AppConfigScopeType +from ai.backend.common.data.permission.types import RBACElementType +from ai.backend.common.identifier.app_config_fragment import AppConfigFragmentID +from ai.backend.manager.data.permission.types import RBACElementRef +from ai.backend.manager.models.app_config_fragment.row import AppConfigFragmentRow +from ai.backend.manager.repositories.base.purger import BatchPurgerSpec +from ai.backend.manager.repositories.base.rbac.scope_unbinder import RBACScopeEntityUnbinder + + +def fragment_rbac_scope_ref(scope_type: AppConfigScopeType, scope_id: str) -> RBACElementRef | None: + """The RBAC scope a fragment belongs to; ``None`` for global-scoped (public) fragments.""" + element = scope_type.to_rbac_element_type() + if element is None: + return None + return RBACElementRef(element, scope_id) + + +@dataclass +class AppConfigFragmentByIdPurgerSpec(BatchPurgerSpec[AppConfigFragmentRow]): + """Select a single fragment row (by id) for deletion.""" + + fragment_id: AppConfigFragmentID + + @override + def build_subquery(self) -> sa.sql.Select[tuple[AppConfigFragmentRow]]: + return sa.select(AppConfigFragmentRow).where(AppConfigFragmentRow.id == self.fragment_id) + + +@dataclass +class AppConfigFragmentScopeUnbinder(RBACScopeEntityUnbinder[AppConfigFragmentRow]): + """Unbind (purge) one fragment from its owning scope. + + A fragment is a config bound at a scope, so purging it is an unbind: the fragment row + and its RBAC scope association are deleted atomically. A ``public`` fragment is + global-scoped (``scope_ref`` is ``None``), so only its row is deleted — it never had + an association. + """ + + fragment_id: AppConfigFragmentID + fragment_scope_type: AppConfigScopeType + fragment_scope_id: str + + @override + def build_purger_spec(self) -> BatchPurgerSpec[AppConfigFragmentRow]: + return AppConfigFragmentByIdPurgerSpec(fragment_id=self.fragment_id) + + @property + @override + def entity_type(self) -> RBACElementType: + return RBACElementType.APP_CONFIG_FRAGMENT + + @property + @override + def scope_ref(self) -> RBACElementRef | None: + return fragment_rbac_scope_ref(self.fragment_scope_type, self.fragment_scope_id) + + @property + @override + def entity_ids(self) -> Sequence[str] | None: + return [str(self.fragment_id)] diff --git a/src/ai/backend/manager/repositories/base/rbac/entity_creator.py b/src/ai/backend/manager/repositories/base/rbac/entity_creator.py index 064a264ccdf..2217bbc2fd1 100644 --- a/src/ai/backend/manager/repositories/base/rbac/entity_creator.py +++ b/src/ai/backend/manager/repositories/base/rbac/entity_creator.py @@ -37,19 +37,21 @@ class RBACEntityCreator[TRow: Base]: """Creator for a single entity with scope associations for RBAC. Creates an entity row and associates it with one or more permission scopes. - The primary scope is required; additional scopes are optional. Attributes: spec: CreatorSpec implementation defining the row to create. element_type: The RBAC element type for this entity. scope_ref: Primary scope reference (scope_type + scope_id) for this entity. + ``None`` marks a *global-scoped* entity — one living outside the RBAC scope + hierarchy (e.g. a public app-config fragment) — which carries no scope + association (only a superadmin reaches it, bypassing scope resolution). additional_scope_refs: Additional scope references for multi-scope entities. relation_type: The relation type for the scope-entity association. Defaults to AUTO. """ spec: CreatorSpec[TRow] element_type: RBACElementType - scope_ref: RBACElementRef + scope_ref: RBACElementRef | None additional_scope_refs: Sequence[RBACElementRef] = field(default_factory=list) relation_type: RelationType = RelationType.AUTO @@ -106,7 +108,10 @@ async def execute_rbac_entity_creator[TRow: Base]( instance_state = inspect(row) pk_value = instance_state.identity[0] entity_type = creator.element_type.to_entity_type() - all_scope_refs = [creator.scope_ref, *creator.additional_scope_refs] + # A None primary scope marks a global-scoped entity: no association row. + all_scope_refs = [ + ref for ref in (creator.scope_ref, *creator.additional_scope_refs) if ref is not None + ] associations = [ AssociationScopesEntitiesRow( scope_type=scope_ref.element_type.to_scope_type(), @@ -117,7 +122,8 @@ async def execute_rbac_entity_creator[TRow: Base]( ) for scope_ref in all_scope_refs ] - await bulk_insert_on_conflict_do_nothing(db_sess, associations) + if associations: + await bulk_insert_on_conflict_do_nothing(db_sess, associations) return RBACEntityCreatorResult(row=row) @@ -255,11 +261,14 @@ async def execute_rbac_entity_creators[TRow: Base]( match_integrity_error(parsed, checks) # 3. Collect all associations from each creator's scope refs + # (a None primary scope marks a global-scoped entity: no association row) associations: list[AssociationScopesEntitiesRow] = [] for creator, row in zip(creators, rows, strict=True): pk_value = inspect(row).identity[0] entity_type = creator.element_type.to_entity_type() - all_scope_refs = [creator.scope_ref, *creator.additional_scope_refs] + all_scope_refs = [ + ref for ref in (creator.scope_ref, *creator.additional_scope_refs) if ref is not None + ] for scope_ref in all_scope_refs: associations.append( AssociationScopesEntitiesRow( @@ -270,6 +279,7 @@ async def execute_rbac_entity_creators[TRow: Base]( relation_type=creator.relation_type, ), ) - await bulk_insert_on_conflict_do_nothing(db_sess, associations) + if associations: + await bulk_insert_on_conflict_do_nothing(db_sess, associations) return RBACBulkEntityCreatorResult(rows=rows) diff --git a/src/ai/backend/manager/repositories/base/rbac/scope_unbinder.py b/src/ai/backend/manager/repositories/base/rbac/scope_unbinder.py index 1b2bb7f13ba..6e4298ab60a 100644 --- a/src/ai/backend/manager/repositories/base/rbac/scope_unbinder.py +++ b/src/ai/backend/manager/repositories/base/rbac/scope_unbinder.py @@ -53,8 +53,12 @@ def entity_type(self) -> RBACElementType: @property @abstractmethod - def scope_ref(self) -> RBACElementRef: - """RBAC element ref for the scope to unbind entities from.""" + def scope_ref(self) -> RBACElementRef | None: + """RBAC element ref for the scope to unbind entities from. + + ``None`` marks global-scoped entities (outside the RBAC scope hierarchy): + only the business rows are deleted — there is no scope association to remove. + """ raise NotImplementedError @property @@ -110,6 +114,12 @@ async def execute_rbac_scope_entity_unbinder[TRow: Base]( db_sess, BatchPurger(spec=unbinder.build_purger_spec()) ) scope_ref = unbinder.scope_ref + if scope_ref is None: + # Global-scoped entities carry no scope association — nothing more to delete. + return RBACUnbinderResult( + deleted_count=purge_result.deleted_count, + association_rows=[], + ) where_clauses = [ AssociationScopesEntitiesRow.entity_type == unbinder.entity_type.to_entity_type(), AssociationScopesEntitiesRow.scope_type == scope_ref.element_type.to_scope_type(), diff --git a/src/ai/backend/manager/repositories/ops/rbac/provider.py b/src/ai/backend/manager/repositories/ops/rbac/provider.py index 33527208906..7ed7f6ece62 100644 --- a/src/ai/backend/manager/repositories/ops/rbac/provider.py +++ b/src/ai/backend/manager/repositories/ops/rbac/provider.py @@ -41,6 +41,10 @@ RBACEntityCreator, execute_rbac_entity_creators, ) +from ai.backend.manager.repositories.base.rbac.scope_unbinder import ( + RBACScopeEntityUnbinder, + RBACUnbinderResult, +) from ai.backend.manager.repositories.ops.base.provider import DBOpsProvider, WriteOps from ai.backend.manager.repositories.permission_controller.role_manager import RoleManager @@ -131,6 +135,50 @@ async def bulk_create_scoped[TRow: Base]( """Insert rows with their RBAC scope associations (each creator carries its scope).""" return await execute_rbac_entity_creators(self._sess, creators) + async def unbind_scope_entities[TRow: Base]( + self, + unbinder: RBACScopeEntityUnbinder[TRow], + ) -> RBACUnbinderResult: + """Delete the business rows and their RBAC scope associations atomically. + + The counterpart of :meth:`bulk_create_scoped` on the delete side: the association + table has no FK to the entity tables, so removing a scope-bound row must clear its + association in the same transaction. Implemented here directly — the standalone + ``execute_rbac_*`` helpers are transitional and their logic is being folded into + the ops providers. + """ + purge_result = await self.batch_purge(BatchPurger(spec=unbinder.build_purger_spec())) + scope_ref = unbinder.scope_ref + if scope_ref is None: + # Global-scoped entities carry no scope association — nothing more to delete. + return RBACUnbinderResult( + deleted_count=purge_result.deleted_count, + association_rows=[], + ) + where_clauses = [ + AssociationScopesEntitiesRow.entity_type == unbinder.entity_type.to_entity_type(), + AssociationScopesEntitiesRow.scope_type == scope_ref.element_type.to_scope_type(), + AssociationScopesEntitiesRow.scope_id == scope_ref.element_id, + ] + entity_ids = unbinder.entity_ids + if entity_ids is not None: + if not entity_ids: + return RBACUnbinderResult( + deleted_count=purge_result.deleted_count, + association_rows=[], + ) + where_clauses.append(AssociationScopesEntitiesRow.entity_id.in_(entity_ids)) + assoc_stmt = ( + sa.delete(AssociationScopesEntitiesRow) + .where(*where_clauses) + .returning(AssociationScopesEntitiesRow) + ) + association_rows = list((await self._sess.scalars(assoc_stmt)).all()) + return RBACUnbinderResult( + deleted_count=purge_result.deleted_count, + association_rows=association_rows, + ) + async def add_users_to_scope( self, scope_id: ScopeId, diff --git a/src/ai/backend/manager/services/app_config_fragment/service.py b/src/ai/backend/manager/services/app_config_fragment/service.py index 0c0ce5afbbf..63dc8f0cb6d 100644 --- a/src/ai/backend/manager/services/app_config_fragment/service.py +++ b/src/ai/backend/manager/services/app_config_fragment/service.py @@ -3,7 +3,7 @@ from ai.backend.manager.repositories.app_config_fragment.repository import ( AppConfigFragmentRepository, ) -from ai.backend.manager.repositories.base import BulkCreator, Creator +from ai.backend.manager.repositories.base import BulkCreator from ai.backend.manager.services.app_config_fragment.actions.admin_search import ( AdminSearchAppConfigFragmentAction, AdminSearchAppConfigFragmentActionResult, @@ -62,7 +62,7 @@ def __init__(self, repository: AppConfigFragmentRepository) -> None: async def create( self, action: CreateAppConfigFragmentAction ) -> CreateAppConfigFragmentActionResult: - data = await self._repository.create(Creator(spec=action.creator_spec)) + data = await self._repository.create(action.creator_spec) return CreateAppConfigFragmentActionResult(fragment=data) async def get(self, action: GetAppConfigFragmentAction) -> GetAppConfigFragmentActionResult: diff --git a/tests/unit/manager/actions/test_action_types.py b/tests/unit/manager/actions/test_action_types.py index 518b1f2bdec..8fd78b50686 100644 --- a/tests/unit/manager/actions/test_action_types.py +++ b/tests/unit/manager/actions/test_action_types.py @@ -58,7 +58,7 @@ def test_scope_types_returns_original_three(self) -> None: scope_types = EntityType._scope_types() assert scope_types == {EntityType.USER, EntityType.PROJECT, EntityType.DOMAIN} - def test_resource_types_returns_original_nine(self) -> None: + def test_resource_types_returns_expected_set(self) -> None: resource_types = EntityType._resource_types() expected = { EntityType.VFOLDER, @@ -67,13 +67,14 @@ def test_resource_types_returns_original_nine(self) -> None: EntityType.ARTIFACT, EntityType.ARTIFACT_REGISTRY, EntityType.APP_CONFIG, + EntityType.APP_CONFIG_FRAGMENT, EntityType.NOTIFICATION_CHANNEL, EntityType.NOTIFICATION_RULE, EntityType.MODEL_DEPLOYMENT, EntityType.MODEL_CARD, } assert resource_types == expected - assert len(resource_types) == 10 + assert len(resource_types) == 11 def test_scope_and_resource_types_no_overlap(self) -> None: scope_types = EntityType._scope_types() diff --git a/tests/unit/manager/rbac/test_permission_types.py b/tests/unit/manager/rbac/test_permission_types.py index dac073f34aa..b3601cf94bf 100644 --- a/tests/unit/manager/rbac/test_permission_types.py +++ b/tests/unit/manager/rbac/test_permission_types.py @@ -129,7 +129,7 @@ def test_rbac_entity_types_are_categorized(self) -> None: # Verify scope types are exactly 3 assert scope_types == {EntityType.USER, EntityType.PROJECT, EntityType.DOMAIN} - # Verify resource types are exactly 10 + # Verify resource types are exactly 11 assert resource_types == { EntityType.VFOLDER, EntityType.IMAGE, @@ -137,6 +137,7 @@ def test_rbac_entity_types_are_categorized(self) -> None: EntityType.ARTIFACT, EntityType.ARTIFACT_REGISTRY, EntityType.APP_CONFIG, + EntityType.APP_CONFIG_FRAGMENT, EntityType.NOTIFICATION_CHANNEL, EntityType.NOTIFICATION_RULE, EntityType.MODEL_DEPLOYMENT, diff --git a/tests/unit/manager/repositories/app_config_fragment/test_repository.py b/tests/unit/manager/repositories/app_config_fragment/test_repository.py index cf1a624040f..cdd2ad74012 100644 --- a/tests/unit/manager/repositories/app_config_fragment/test_repository.py +++ b/tests/unit/manager/repositories/app_config_fragment/test_repository.py @@ -6,8 +6,10 @@ from collections.abc import AsyncGenerator import pytest +import sqlalchemy as sa from ai.backend.common.data.app_config.types import AppConfigScopeType +from ai.backend.common.data.permission.types import EntityType, ScopeType from ai.backend.common.identifier.app_config_fragment import AppConfigFragmentID from ai.backend.common.identifier.domain import DomainID from ai.backend.common.identifier.user import UserID @@ -23,6 +25,9 @@ from ai.backend.manager.models.app_config_definition.row import AppConfigDefinitionRow from ai.backend.manager.models.app_config_fragment.conditions import AppConfigFragmentConditions from ai.backend.manager.models.app_config_fragment.row import AppConfigFragmentRow +from ai.backend.manager.models.rbac_models.association_scopes_entities import ( + AssociationScopesEntitiesRow, +) from ai.backend.manager.models.utils import ExtendedAsyncSAEngine from ai.backend.manager.repositories.app_config_fragment.creators import ( AppConfigFragmentCreatorSpec, @@ -41,12 +46,11 @@ from ai.backend.manager.repositories.base import ( BatchQuerier, BulkCreator, - Creator, OffsetPagination, Purger, Updater, ) -from ai.backend.manager.repositories.ops import DBOpsProvider +from ai.backend.manager.repositories.ops.rbac.provider import RBACOpsProvider from ai.backend.manager.types import OptionalState from ai.backend.testutils.db import with_tables @@ -62,16 +66,22 @@ async def database( database_connection: ExtendedAsyncSAEngine, ) -> AsyncGenerator[ExtendedAsyncSAEngine, None]: # FK order: app_config_definitions (parent) before the allow-list and fragments (children). + # AssociationScopesEntitiesRow holds the RBAC scope↔fragment binding written on create. async with with_tables( database_connection, - [AppConfigDefinitionRow, AppConfigAllowListRow, AppConfigFragmentRow], + [ + AppConfigDefinitionRow, + AppConfigAllowListRow, + AppConfigFragmentRow, + AssociationScopesEntitiesRow, + ], ): yield database_connection @pytest.fixture def repository(database: ExtendedAsyncSAEngine) -> AppConfigFragmentRepository: - return AppConfigFragmentRepository(DBOpsProvider(database)) + return AppConfigFragmentRepository(RBACOpsProvider(database)) def _allow_list_row(config_name: str, scope_type: AppConfigScopeType) -> AppConfigAllowListRow: @@ -190,14 +200,12 @@ async def test_create_then_get_by_id( self, repository: AppConfigFragmentRepository, theme_registered: None ) -> None: created = await repository.create( - Creator( - spec=AppConfigFragmentCreatorSpec( - config_name="theme", - scope_type=AppConfigScopeType.PUBLIC, - scope_id="public", - config={"theme": "dark"}, - ) - ), + AppConfigFragmentCreatorSpec( + config_name="theme", + scope_type=AppConfigScopeType.PUBLIC, + scope_id="public", + config={"theme": "dark"}, + ) ) fetched = await repository.get_by_id(created.id) assert fetched.id == created.id @@ -214,14 +222,12 @@ async def test_create_rejected_when_not_allow_listed( ) -> None: with pytest.raises(AppConfigFragmentWriteNotAllowed): await repository.create( - Creator( - spec=AppConfigFragmentCreatorSpec( - config_name="theme", - scope_type=AppConfigScopeType.PUBLIC, - scope_id="public", - config={"theme": "dark"}, - ) - ), + AppConfigFragmentCreatorSpec( + config_name="theme", + scope_type=AppConfigScopeType.PUBLIC, + scope_id="public", + config={"theme": "dark"}, + ) ) async def test_unique_constraint_violation( @@ -231,14 +237,12 @@ async def test_unique_constraint_violation( ) -> None: with pytest.raises(UniqueConstraintViolationError): await repository.create( - Creator( - spec=AppConfigFragmentCreatorSpec( - config_name=domain_scoped_fragment.config_name, - scope_type=domain_scoped_fragment.scope_type, - scope_id=domain_scoped_fragment.scope_id, - config={"k": "v"}, - ) - ), + AppConfigFragmentCreatorSpec( + config_name=domain_scoped_fragment.config_name, + scope_type=domain_scoped_fragment.scope_type, + scope_id=domain_scoped_fragment.scope_id, + config={"k": "v"}, + ) ) @@ -704,3 +708,66 @@ async def test_bulk_empty_names_returns_empty( AppConfigScopeArguments(domain_id=DomainID(_DOMAIN_UUID), user_id=UserID(_USER_UUID)), ) assert applicable == [] + + +class TestRBACScopeAssociation: + """Create binds a ``user`` / ``domain`` fragment to its RBAC scope so the RBAC validator can + resolve ownership on a later update/purge; ``public`` (GLOBAL, no RBAC scope) gets none.""" + + @staticmethod + async def _scope_bindings( + database: ExtendedAsyncSAEngine, entity_id: str + ) -> list[AssociationScopesEntitiesRow]: + async with database.begin_readonly_session() as db_sess: + rows = await db_sess.scalars( + sa.select(AssociationScopesEntitiesRow).where( + AssociationScopesEntitiesRow.entity_type == EntityType.APP_CONFIG_FRAGMENT, + AssociationScopesEntitiesRow.entity_id == entity_id, + ) + ) + return list(rows) + + @staticmethod + async def _create( + repository: AppConfigFragmentRepository, scope_type: AppConfigScopeType, scope_id: str + ) -> AppConfigFragmentData: + return await repository.create( + AppConfigFragmentCreatorSpec( + config_name="theme", + scope_type=scope_type, + scope_id=scope_id, + config={"k": "v"}, + ) + ) + + async def test_user_scope_create_binds_to_user_scope( + self, + repository: AppConfigFragmentRepository, + database: ExtendedAsyncSAEngine, + theme_registered: None, + ) -> None: + created = await self._create(repository, AppConfigScopeType.USER, _USER_ID) + bindings = await self._scope_bindings(database, str(created.id)) + assert len(bindings) == 1 + assert bindings[0].scope_type is ScopeType.USER + assert bindings[0].scope_id == _USER_ID + + async def test_public_scope_create_binds_nothing( + self, + repository: AppConfigFragmentRepository, + database: ExtendedAsyncSAEngine, + theme_registered: None, + ) -> None: + created = await self._create(repository, AppConfigScopeType.PUBLIC, "public") + assert await self._scope_bindings(database, str(created.id)) == [] + + async def test_purge_removes_the_scope_binding( + self, + repository: AppConfigFragmentRepository, + database: ExtendedAsyncSAEngine, + theme_registered: None, + ) -> None: + created = await self._create(repository, AppConfigScopeType.USER, _USER_ID) + assert len(await self._scope_bindings(database, str(created.id))) == 1 + await repository.purge(Purger(row_class=AppConfigFragmentRow, pk_value=created.id)) + assert await self._scope_bindings(database, str(created.id)) == [] diff --git a/tests/unit/manager/services/app_config_fragment/test_service.py b/tests/unit/manager/services/app_config_fragment/test_service.py index e594eb6da19..726cfbf2158 100644 --- a/tests/unit/manager/services/app_config_fragment/test_service.py +++ b/tests/unit/manager/services/app_config_fragment/test_service.py @@ -33,7 +33,6 @@ from ai.backend.manager.repositories.base import ( BatchQuerier, BulkCreator, - Creator, OffsetPagination, Purger, Updater, @@ -120,7 +119,7 @@ async def test_create_delegates_to_repository( result = await service.create(CreateAppConfigFragmentAction(creator_spec=spec)) assert result.fragment == fragment - mock_repository.create.assert_called_once_with(Creator(spec=spec)) + mock_repository.create.assert_called_once_with(spec) # --- get / search ---