From d0a0c052ad1a8c57feaf66384cc3141c153edca6 Mon Sep 17 00:00:00 2001 From: Gyubong Date: Mon, 13 Jul 2026 17:14:31 +0900 Subject: [PATCH 01/13] feat(BA-6859): bind app_config fragments to their RBAC scope on write (repository layer) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Run the app_config_fragment repository on RBACOpsProvider and make every fragment write flow through the RBAC scope machinery, branch-free: - The shared RBAC creator/unbinder specs now model *global-scoped* entities: RBACEntityCreator.scope_ref and RBACScopeEntityUnbinder.scope_ref accept None, meaning the entity lives outside the RBAC scope hierarchy and carries no scope association. - AppConfigScopeType.to_rbac_element_type() maps a fragment scope to its RBAC scope element (public -> None), next to its to_rbac_scope_type sibling. - create binds a user/domain fragment to its scope via association_scopes_entities in the same tx; purge unbinds it through RBACWriteOps.unbind_scope_entities (implemented directly on the ops provider — the standalone execute_rbac_* helpers are transitional), deleting the row and its association atomically. A public fragment takes the same single code path with scope_ref=None. - Registers APP_CONFIG_FRAGMENT as an owner-accessible RBAC resource type. - Repository/db_source create now takes AppConfigFragmentCreatorSpec directly. Data-layer foundation for RBAC-gating fragment writes; service-layer validators + DI registration follow (BA-6860). Co-Authored-By: Claude Opus 4.8 (1M context) --- changes/12801.feature.md | 1 + .../backend/common/data/app_config/types.py | 17 ++- .../backend/common/data/permission/types.py | 1 + .../models/rbac_models/migration/enums.py | 2 + .../db_source/db_source.py | 55 ++++++-- .../app_config_fragment/repositories.py | 5 +- .../app_config_fragment/repository.py | 12 +- .../app_config_fragment/scope_binders.py | 70 ++++++++++ .../repositories/base/rbac/entity_creator.py | 22 +++- .../repositories/base/rbac/scope_unbinder.py | 14 +- .../manager/repositories/ops/rbac/provider.py | 48 +++++++ .../services/app_config_fragment/service.py | 4 +- .../unit/manager/actions/test_action_types.py | 5 +- .../manager/rbac/test_permission_types.py | 3 +- .../app_config_fragment/test_repository.py | 123 ++++++++++++++---- .../app_config_fragment/test_service.py | 3 +- 16 files changed, 321 insertions(+), 64 deletions(-) create mode 100644 changes/12801.feature.md create mode 100644 src/ai/backend/manager/repositories/app_config_fragment/scope_binders.py 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 --- From 265921103e5e26e250c4099c7010f83025983b20 Mon Sep 17 00:00:00 2001 From: Gyubong Date: Tue, 14 Jul 2026 10:18:16 +0900 Subject: [PATCH 02/13] fix(BA-6859): address review comments on app_config fragment RBAC binding - rename db_source field `_ops` -> `_rbac_ops_provider` for consistency - purge: check unbind `deleted_count` and report not-found on a concurrent delete instead of returning a stale success - tighten the news fragment wording Co-Authored-By: Claude Opus 4.8 (1M context) --- changes/12801.feature.md | 2 +- .../db_source/db_source.py | 33 +++++++++++-------- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/changes/12801.feature.md b/changes/12801.feature.md index b012689c75a..09115f0c921 100644 --- a/changes/12801.feature.md +++ b/changes/12801.feature.md @@ -1 +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). +Bind `user`/`domain` AppConfig fragments to their RBAC scope on create and unbind on purge (`public` stays global), so fragment writes can be authorized by RBAC (BEP-1052). 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 b63d0916b89..af84d72d427 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 @@ -72,10 +72,10 @@ class AppConfigFragmentDBSource: """Database source for app config fragment operations.""" - _ops: RBACOpsProvider + _rbac_ops_provider: RBACOpsProvider - def __init__(self, ops_provider: RBACOpsProvider) -> None: - self._ops = ops_provider + def __init__(self, rbac_ops_provider: RBACOpsProvider) -> None: + self._rbac_ops_provider = rbac_ops_provider @app_config_fragment_db_source_resilience.apply() async def create(self, spec: AppConfigFragmentCreatorSpec) -> AppConfigFragmentData: @@ -90,13 +90,13 @@ async def create(self, spec: AppConfigFragmentCreatorSpec) -> AppConfigFragmentD 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: + async with self._rbac_ops_provider.write_ops() as w: 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: - async with self._ops.read_ops() as r: + async with self._rbac_ops_provider.read_ops() as r: result = await r.query(Querier(row_class=AppConfigFragmentRow, pk_value=fragment_id)) if result is None: raise AppConfigFragmentNotFound(f"App config fragment {fragment_id} not found") @@ -107,7 +107,7 @@ async def update(self, updater: Updater[AppConfigFragmentRow]) -> AppConfigFragm # No write-gate here: the FK to the allow-list guarantees a fragment row exists # only while its ``(config_name, scope_type)`` entry does, so an existing # fragment is always writable at its own scope. - async with self._ops.write_ops() as w: + async with self._rbac_ops_provider.write_ops() as w: result = await w.update(updater) if result is None: raise AppConfigFragmentNotFound(f"App config fragment {updater.pk_value} not found") @@ -119,18 +119,23 @@ async def purge(self, purger: Purger[AppConfigFragmentRow]) -> AppConfigFragment # 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: + async with self._rbac_ops_provider.write_ops() as w: 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") data = found.row.to_data() - await w.unbind_scope_entities( + result = await w.unbind_scope_entities( AppConfigFragmentScopeUnbinder( fragment_id=data.id, fragment_scope_type=data.scope_type, fragment_scope_id=data.scope_id, ) ) + # The read and the delete are separate statements under READ COMMITTED, so a + # concurrent purge may remove the row in between. A zero delete count means the + # row was already gone: report not-found rather than a stale success. + if result.deleted_count == 0: + raise AppConfigFragmentNotFound(f"App config fragment {purger.pk_value} not found") return data @app_config_fragment_db_source_resilience.apply() @@ -139,7 +144,7 @@ async def bulk_create( bulk_creator: BulkCreator[AppConfigFragmentRow], ) -> AppConfigFragmentBulkResult: """Create many fragments with per-item partial success.""" - async with self._ops.write_ops() as w: + async with self._rbac_ops_provider.write_ops() as w: result = await w.bulk_create_partial(bulk_creator) return AppConfigFragmentBulkResult( succeeded=[row.to_data() for row in result.successes], @@ -155,7 +160,7 @@ async def bulk_update( updaters: Sequence[Updater[AppConfigFragmentRow]], ) -> AppConfigFragmentBulkResult: """Update many fragments with per-item partial success.""" - async with self._ops.write_ops() as w: + async with self._rbac_ops_provider.write_ops() as w: result = await w.bulk_update_partial(updaters) succeeded = [row.to_data() for row in result.successes] succeeded_ids = {data.id for data in succeeded} @@ -179,7 +184,7 @@ async def bulk_purge( purgers: Sequence[Purger[AppConfigFragmentRow]], ) -> AppConfigFragmentBulkResult: """Purge many fragments with per-item partial success.""" - async with self._ops.write_ops() as w: + async with self._rbac_ops_provider.write_ops() as w: result = await w.bulk_purge_partial(list(purgers)) succeeded = [row.to_data() for row in result.successes] succeeded_ids = {data.id for data in succeeded} @@ -200,7 +205,7 @@ async def bulk_purge( @app_config_fragment_db_source_resilience.apply() async def admin_search(self, querier: BatchQuerier) -> AppConfigFragmentSearchResult: """Superadmin/internal path: query across all fragments with no scope filter.""" - async with self._ops.read_ops() as r: + async with self._rbac_ops_provider.read_ops() as r: result = await r.batch_query_in_global(sa.select(AppConfigFragmentRow), querier) return AppConfigFragmentSearchResult( items=[row.AppConfigFragmentRow.to_data() for row in result.rows], @@ -216,7 +221,7 @@ async def scoped_search( scopes: Sequence[SearchScope], ) -> AppConfigFragmentSearchResult: """Scoped path: query fragments restricted to ``scopes`` (combined with OR).""" - async with self._ops.read_ops() as r: + async with self._rbac_ops_provider.read_ops() as r: result = await r.batch_query_with_scopes( sa.select(AppConfigFragmentRow), querier, scopes ) @@ -264,6 +269,6 @@ async def list_visible_fragments_bulk( AppConfigAllowListRow.scope_type == AppConfigFragmentRow.scope_type, ), ) - async with self._ops.read_ops() as r: + async with self._rbac_ops_provider.read_ops() as r: result = await r.batch_query_in_global(selector, querier) return [row.AppConfigFragmentRow.to_data() for row in result.rows] From 9f249dc86787867ef2beaef7921c3ded86ff554c Mon Sep 17 00:00:00 2001 From: Gyubong Date: Tue, 14 Jul 2026 10:56:58 +0900 Subject: [PATCH 03/13] docs(BA-6859): trim verbose comments in app_config_fragment repository Condense the multi-line prose comments in the db_source and repositories to the project's lead-with-the-conclusion style; no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../db_source/db_source.py | 25 +++++++------------ .../app_config_fragment/repositories.py | 3 +-- 2 files changed, 10 insertions(+), 18 deletions(-) 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 af84d72d427..2c1cb9a09c7 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 @@ -79,12 +79,9 @@ def __init__(self, rbac_ops_provider: RBACOpsProvider) -> None: @app_config_fragment_db_source_resilience.apply() 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. + # The allow-list FK gates existence (no row → ``AppConfigFragmentWriteNotAllowed``), + # and create binds the fragment to its RBAC scope in the same tx. 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, @@ -104,9 +101,8 @@ async def get_by_id(self, fragment_id: AppConfigFragmentID) -> AppConfigFragment @app_config_fragment_db_source_resilience.apply() async def update(self, updater: Updater[AppConfigFragmentRow]) -> AppConfigFragmentData: - # No write-gate here: the FK to the allow-list guarantees a fragment row exists - # only while its ``(config_name, scope_type)`` entry does, so an existing - # fragment is always writable at its own scope. + # No write-gate: the allow-list FK keeps a fragment row alive only while its + # ``(config_name, scope_type)`` entry exists, so an existing fragment is writable. async with self._rbac_ops_provider.write_ops() as w: result = await w.update(updater) if result is None: @@ -115,10 +111,8 @@ async def update(self, updater: Updater[AppConfigFragmentRow]) -> AppConfigFragm @app_config_fragment_db_source_resilience.apply() async def purge(self, purger: Purger[AppConfigFragmentRow]) -> AppConfigFragmentData: - # 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. + # Purge is an RBAC unbind: the row and its scope association are deleted atomically + # (no FK cascade; a ``public`` fragment has none). Fetch first for its scope and data. async with self._rbac_ops_provider.write_ops() as w: found = await w.query(Querier(row_class=AppConfigFragmentRow, pk_value=purger.pk_value)) if found is None: @@ -131,9 +125,8 @@ async def purge(self, purger: Purger[AppConfigFragmentRow]) -> AppConfigFragment fragment_scope_id=data.scope_id, ) ) - # The read and the delete are separate statements under READ COMMITTED, so a - # concurrent purge may remove the row in between. A zero delete count means the - # row was already gone: report not-found rather than a stale success. + # Read and delete are separate statements under READ COMMITTED: a zero count + # means a concurrent purge already removed the row — not-found, not stale success. if result.deleted_count == 0: raise AppConfigFragmentNotFound(f"App config fragment {purger.pk_value} not found") return data 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 037ae015eab..7c8e26c3904 100644 --- a/src/ai/backend/manager/repositories/app_config_fragment/repositories.py +++ b/src/ai/backend/manager/repositories/app_config_fragment/repositories.py @@ -14,8 +14,7 @@ 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. + # Fragment writes bind to an RBAC scope (see db_source), so use the RBAC ops provider. return cls( repository=AppConfigFragmentRepository(RBACOpsProvider(args.db)), ) From a3d34f86d415fe68614bb7d996d6c5e6d1cddfff Mon Sep 17 00:00:00 2001 From: Gyubong Date: Tue, 14 Jul 2026 10:57:10 +0900 Subject: [PATCH 04/13] feat(BA-6859): enforce app_config fragment write authorization via RBAC Wire the RBAC validators into the app_config_fragment write path so each write is authorized at the fragment's scope (BEP-1052): - create: target_element now returns the fragment's user/domain scope so the scope-chain check runs there; a public fragment is global-scoped and superadmin- only, enforced by the new PublicAppConfigFragmentWriteValidator - update/purge: single-entity RBAC validator (scope resolved via the scope binding) - bulk_update/bulk_purge: bulk RBAC validator - register the repository/service/processors in the DI containers Reads stay on the allow-list read tiers, so get/search carry no RBAC validator; bulk_create has no pre-existing targets, so the target-based bulk validator cannot authorize it. Co-Authored-By: Claude Opus 4.8 (1M context) --- changes/12801.feature.md | 2 +- .../manager/repositories/repositories.py | 6 + .../app_config_fragment/actions/create.py | 9 +- .../app_config_fragment/processors.py | 40 ++- .../app_config_fragment/validators.py | 44 +++ src/ai/backend/manager/services/factory.py | 15 + src/ai/backend/manager/services/processors.py | 9 + ...pp_config_fragment_create_authorization.py | 267 ++++++++++++++++++ .../app_config_fragment/test_create_action.py | 57 ++++ 9 files changed, 442 insertions(+), 7 deletions(-) create mode 100644 src/ai/backend/manager/services/app_config_fragment/validators.py create mode 100644 tests/unit/manager/actions/validators/test_app_config_fragment_create_authorization.py create mode 100644 tests/unit/manager/services/app_config_fragment/test_create_action.py diff --git a/changes/12801.feature.md b/changes/12801.feature.md index 09115f0c921..28640602ef5 100644 --- a/changes/12801.feature.md +++ b/changes/12801.feature.md @@ -1 +1 @@ -Bind `user`/`domain` AppConfig fragments to their RBAC scope on create and unbind on purge (`public` stays global), so fragment writes can be authorized by RBAC (BEP-1052). +Authorize AppConfig fragment writes by RBAC (BEP-1052): create/update/purge are gated by the writer's permission at the fragment's `user`/`domain` scope (bound on create, unbound on purge), while `public` fragments are global-scoped and superadmin-only. diff --git a/src/ai/backend/manager/repositories/repositories.py b/src/ai/backend/manager/repositories/repositories.py index fed6ba2df99..f388c0fdbe5 100644 --- a/src/ai/backend/manager/repositories/repositories.py +++ b/src/ai/backend/manager/repositories/repositories.py @@ -8,6 +8,9 @@ from ai.backend.manager.repositories.app_config_definition.repositories import ( AppConfigDefinitionRepositories, ) +from ai.backend.manager.repositories.app_config_fragment.repositories import ( + AppConfigFragmentRepositories, +) from ai.backend.manager.repositories.artifact.repositories import ArtifactRepositories from ai.backend.manager.repositories.artifact_registry.repositories import ( ArtifactRegistryRepositories, @@ -94,6 +97,7 @@ class Repositories: agent: AgentRepositories app_config_allow_list: AppConfigAllowListRepositories app_config_definition: AppConfigDefinitionRepositories + app_config_fragment: AppConfigFragmentRepositories auth: AuthRepositories container_registry: ContainerRegistryRepositories deployment: DeploymentRepositories @@ -148,6 +152,7 @@ def create(cls, args: RepositoryArgs) -> Self: agent_repositories = AgentRepositories.create(args) app_config_allow_list_repositories = AppConfigAllowListRepositories.create(args) app_config_definition_repositories = AppConfigDefinitionRepositories.create(args) + app_config_fragment_repositories = AppConfigFragmentRepositories.create(args) auth_repositories = AuthRepositories.create(args) container_registry_repositories = ContainerRegistryRepositories.create(args) deployment_repositories = DeploymentRepositories.create(args) @@ -203,6 +208,7 @@ def create(cls, args: RepositoryArgs) -> Self: agent=agent_repositories, app_config_allow_list=app_config_allow_list_repositories, app_config_definition=app_config_definition_repositories, + app_config_fragment=app_config_fragment_repositories, auth=auth_repositories, container_registry=container_registry_repositories, deployment=deployment_repositories, diff --git a/src/ai/backend/manager/services/app_config_fragment/actions/create.py b/src/ai/backend/manager/services/app_config_fragment/actions/create.py index 990ce669816..5820abee7ea 100644 --- a/src/ai/backend/manager/services/app_config_fragment/actions/create.py +++ b/src/ai/backend/manager/services/app_config_fragment/actions/create.py @@ -44,7 +44,14 @@ def scope_id(self) -> str: @override def target_element(self) -> RBACElementRef: - return RBACElementRef(RBACElementType.APP_CONFIG_FRAGMENT, "") + # The scope this create acts on, so RBAC checks the writer's permission there: + # a user/domain fragment targets its USER/DOMAIN scope. A public fragment is + # global-scoped (no RBAC scope element) and is superadmin-only — its target has + # no scope to resolve, so a non-superadmin write fails the scope-chain check. + element = self.creator_spec.scope_type.to_rbac_element_type() + if element is None: + return RBACElementRef(RBACElementType.APP_CONFIG_FRAGMENT, "") + return RBACElementRef(element, self.creator_spec.scope_id) @dataclass diff --git a/src/ai/backend/manager/services/app_config_fragment/processors.py b/src/ai/backend/manager/services/app_config_fragment/processors.py index 0d81a523b30..ec58c174568 100644 --- a/src/ai/backend/manager/services/app_config_fragment/processors.py +++ b/src/ai/backend/manager/services/app_config_fragment/processors.py @@ -7,6 +7,8 @@ from ai.backend.manager.actions.processor.scope import ScopeActionProcessor from ai.backend.manager.actions.processor.single_entity import SingleEntityActionProcessor from ai.backend.manager.actions.types import AbstractProcessorPackage, ActionSpec +from ai.backend.manager.actions.validators import ActionValidators +from ai.backend.manager.config.provider import ManagerConfigProvider from ai.backend.manager.services.app_config_fragment.actions.admin_search import ( AdminSearchAppConfigFragmentAction, AdminSearchAppConfigFragmentActionResult, @@ -46,6 +48,9 @@ from ai.backend.manager.services.app_config_fragment.service import ( AppConfigFragmentService, ) +from ai.backend.manager.services.app_config_fragment.validators import ( + PublicAppConfigFragmentWriteValidator, +) class AppConfigFragmentProcessors(AbstractProcessorPackage): @@ -77,16 +82,41 @@ def __init__( self, service: AppConfigFragmentService, action_monitors: list[ActionMonitor], + validators: ActionValidators, + config_provider: ManagerConfigProvider, ) -> None: - self.create = ScopeActionProcessor(service.create, action_monitors) + # Writes are authorized by RBAC (BEP-1052): create acts at the fragment's target + # scope (own user / domain / superadmin-only public), while update / purge act on + # the fragment entity whose scope is resolved through its scope binding. Reads stay + # on the allow-list read tiers, so get / admin_search / scoped_search carry no RBAC + # validator. Bulk create has no pre-existing targets, so the target-based bulk + # validator cannot authorize it — only bulk update / purge are wired. + # + # A public fragment is global-scoped and has no RBAC scope element, so the generic + # scope-chain validator cannot represent it; the public guard runs first to keep + # public writes superadmin-only and defers user / domain scopes to the scope check. + public_write_guard = PublicAppConfigFragmentWriteValidator(config_provider) + self.create = ScopeActionProcessor( + service.create, + action_monitors, + validators=[public_write_guard, validators.rbac.scope], + ) self.get = SingleEntityActionProcessor(service.get, action_monitors) self.admin_search = ScopeActionProcessor(service.admin_search, action_monitors) self.scoped_search = BulkActionProcessor(service.scoped_search, monitors=action_monitors) - self.update = SingleEntityActionProcessor(service.update, action_monitors) - self.purge = SingleEntityActionProcessor(service.purge, action_monitors) + self.update = SingleEntityActionProcessor( + service.update, action_monitors, validators=[validators.rbac.single_entity] + ) + self.purge = SingleEntityActionProcessor( + service.purge, action_monitors, validators=[validators.rbac.single_entity] + ) self.bulk_create = BulkActionProcessor(service.bulk_create, monitors=action_monitors) - self.bulk_update = BulkActionProcessor(service.bulk_update, monitors=action_monitors) - self.bulk_purge = BulkActionProcessor(service.bulk_purge, monitors=action_monitors) + self.bulk_update = BulkActionProcessor( + service.bulk_update, monitors=action_monitors, validators=[validators.rbac.bulk] + ) + self.bulk_purge = BulkActionProcessor( + service.bulk_purge, monitors=action_monitors, validators=[validators.rbac.bulk] + ) @override def supported_actions(self) -> list[ActionSpec]: diff --git a/src/ai/backend/manager/services/app_config_fragment/validators.py b/src/ai/backend/manager/services/app_config_fragment/validators.py new file mode 100644 index 00000000000..9a81de98a7e --- /dev/null +++ b/src/ai/backend/manager/services/app_config_fragment/validators.py @@ -0,0 +1,44 @@ +"""Scope-action validators specific to app config fragment writes (BEP-1052).""" + +from __future__ import annotations + +from typing import override + +from ai.backend.common.contexts.user import current_user +from ai.backend.common.data.permission.types import ScopeType +from ai.backend.common.exception import UnreachableError +from ai.backend.manager.actions.action import BaseActionTriggerMeta +from ai.backend.manager.actions.action.scope import BaseScopeAction +from ai.backend.manager.actions.validator.scope import ScopeActionValidator +from ai.backend.manager.config.provider import ManagerConfigProvider +from ai.backend.manager.errors.permission import NotEnoughPermission + +__all__ = ("PublicAppConfigFragmentWriteValidator",) + + +class PublicAppConfigFragmentWriteValidator(ScopeActionValidator): + """Restrict writes to public (global-scoped) fragments to superadmins. + + A public fragment is global-scoped and has no RBAC scope element, so the generic + scope-chain check cannot represent it. Public writes are therefore superadmin-only + (BEP-1052). This guard runs ahead of ``ScopeActionRBACValidator`` and no-ops for + ``user`` / ``domain`` scopes, leaving those to the scope-chain check. + """ + + _config_provider: ManagerConfigProvider + + def __init__(self, config_provider: ManagerConfigProvider) -> None: + self._config_provider = config_provider + + @override + async def validate(self, action: BaseScopeAction, meta: BaseActionTriggerMeta) -> None: + if not self._config_provider.config.manager.rbac.enforcement_enabled: + return + if action.scope_type() != ScopeType.GLOBAL: + return + user = current_user() + if user is None: + raise UnreachableError("User context is not available") + if user.is_superadmin: + return + raise NotEnoughPermission("Creating a public app config fragment requires superadmin") diff --git a/src/ai/backend/manager/services/factory.py b/src/ai/backend/manager/services/factory.py index 38f52b4dcdc..1c4df7a5347 100644 --- a/src/ai/backend/manager/services/factory.py +++ b/src/ai/backend/manager/services/factory.py @@ -18,6 +18,12 @@ from ai.backend.manager.services.app_config_definition.service import ( AppConfigDefinitionService, ) +from ai.backend.manager.services.app_config_fragment.processors import ( + AppConfigFragmentProcessors, +) +from ai.backend.manager.services.app_config_fragment.service import ( + AppConfigFragmentService, +) from ai.backend.manager.services.artifact.processors import ArtifactProcessors from ai.backend.manager.services.artifact.service import ArtifactService from ai.backend.manager.services.artifact_registry.processors import ArtifactRegistryProcessors @@ -344,6 +350,9 @@ def create_services(args: ServiceArgs) -> Services: app_config_definition=AppConfigDefinitionService( repository=repositories.app_config_definition.repository, ), + app_config_fragment=AppConfigFragmentService( + repository=repositories.app_config_fragment.repository, + ), login_client_type=LoginClientTypeService( repository=repositories.auth.login_client_type, ), @@ -511,6 +520,12 @@ def create_processors( app_config_definition=AppConfigDefinitionProcessors( services.app_config_definition, action_monitors ), + app_config_fragment=AppConfigFragmentProcessors( + services.app_config_fragment, + action_monitors, + validators, + args.service_args.config_provider, + ), login_client_type=LoginClientTypeProcessors(services.login_client_type, action_monitors), login_client_type_admin=LoginClientTypeAdminProcessors( services.login_client_type_admin, action_monitors diff --git a/src/ai/backend/manager/services/processors.py b/src/ai/backend/manager/services/processors.py index 0e046773397..76c54c6bcd8 100644 --- a/src/ai/backend/manager/services/processors.py +++ b/src/ai/backend/manager/services/processors.py @@ -57,6 +57,12 @@ from ai.backend.manager.services.app_config_definition.service import ( AppConfigDefinitionService, ) + from ai.backend.manager.services.app_config_fragment.processors import ( + AppConfigFragmentProcessors, + ) + from ai.backend.manager.services.app_config_fragment.service import ( + AppConfigFragmentService, + ) from ai.backend.manager.services.artifact.processors import ( ArtifactProcessors, ) @@ -377,6 +383,7 @@ class Services: agent: AgentService app_config_allow_list: AppConfigAllowListService app_config_definition: AppConfigDefinitionService + app_config_fragment: AppConfigFragmentService domain: DomainService dotfile: DotfileService error_log: ErrorLogService @@ -444,6 +451,7 @@ class Processors(AbstractProcessorPackage): agent: AgentProcessors app_config_allow_list: AppConfigAllowListProcessors app_config_definition: AppConfigDefinitionProcessors + app_config_fragment: AppConfigFragmentProcessors domain: DomainProcessors dotfile: DotfileProcessors error_log: ErrorLogProcessors @@ -504,6 +512,7 @@ def supported_actions(self) -> list[ActionSpec]: *self.agent.supported_actions(), *self.app_config_allow_list.supported_actions(), *self.app_config_definition.supported_actions(), + *self.app_config_fragment.supported_actions(), *self.domain.supported_actions(), *self.dotfile.supported_actions(), *self.error_log.supported_actions(), diff --git a/tests/unit/manager/actions/validators/test_app_config_fragment_create_authorization.py b/tests/unit/manager/actions/validators/test_app_config_fragment_create_authorization.py new file mode 100644 index 00000000000..01bdbe709bf --- /dev/null +++ b/tests/unit/manager/actions/validators/test_app_config_fragment_create_authorization.py @@ -0,0 +1,267 @@ +"""RBAC authorization tests for creating an app config fragment (BEP-1052). + +Drives the real ``ScopeActionRBACValidator`` (the validator wired to the fragment +``create`` processor) against a real ``PermissionControllerRepository`` and the actual +``CreateAppConfigFragmentAction``. This verifies the end-to-end contract the wiring +promises: + +- a user may create a fragment in their **own** user scope, +- creating in **another** user's scope is denied, +- creating a **public** (global-scoped) fragment is superadmin-only. +""" + +from __future__ import annotations + +import uuid +from collections.abc import AsyncIterator +from datetime import UTC, datetime +from unittest.mock import MagicMock + +import pytest + +from ai.backend.common.contexts.user import with_user +from ai.backend.common.data.app_config.types import AppConfigScopeType +from ai.backend.common.data.permission.types import ( + EntityType, + OperationType, + Permission, + ScopeType, +) +from ai.backend.common.data.user.types import UserData, UserRole +from ai.backend.manager.actions.action.base import BaseActionTriggerMeta +from ai.backend.manager.actions.validators.rbac.scope import ScopeActionRBACValidator +from ai.backend.manager.data.user.types import UserStatus +from ai.backend.manager.errors.permission import NotEnoughPermission + +# ORM cluster: configure_mappers() resolves string relationships against the registry +# when this isolated test registers rows; these keep the referenced mappers live. +from ai.backend.manager.models.agent import AgentRow +from ai.backend.manager.models.domain import DomainRow +from ai.backend.manager.models.image import ImageRow +from ai.backend.manager.models.keypair import KeyPairRow +from ai.backend.manager.models.rbac_models import UserRoleRow +from ai.backend.manager.models.rbac_models.association_scopes_entities import ( + AssociationScopesEntitiesRow, +) +from ai.backend.manager.models.rbac_models.permission.object_permission import ObjectPermissionRow +from ai.backend.manager.models.rbac_models.permission.permission import PermissionRow +from ai.backend.manager.models.rbac_models.role import RoleRow +from ai.backend.manager.models.resource_policy import ( + KeyPairResourcePolicyRow, + UserResourcePolicyRow, +) +from ai.backend.manager.models.scaling_group import ScalingGroupForDomainRow +from ai.backend.manager.models.user import UserRow +from ai.backend.manager.models.utils import ExtendedAsyncSAEngine +from ai.backend.manager.repositories.app_config_fragment.creators import ( + AppConfigFragmentCreatorSpec, +) +from ai.backend.manager.repositories.permission_controller.repository import ( + PermissionControllerRepository, +) +from ai.backend.manager.services.app_config_fragment.actions.create import ( + CreateAppConfigFragmentAction, +) +from ai.backend.manager.services.app_config_fragment.validators import ( + PublicAppConfigFragmentWriteValidator, +) +from ai.backend.testutils.db import with_tables + +_ORM_CLUSTER = ( + AgentRow, + ImageRow, + ScalingGroupForDomainRow, +) + +_DOMAIN = "default" + + +def _create_action(scope_type: AppConfigScopeType, scope_id: str) -> CreateAppConfigFragmentAction: + return CreateAppConfigFragmentAction( + creator_spec=AppConfigFragmentCreatorSpec( + config_name="cfg", + scope_type=scope_type, + scope_id=scope_id, + config={}, + ) + ) + + +def _user_data(user_id: uuid.UUID, *, is_superadmin: bool) -> UserData: + return UserData( + user_id=user_id, + is_authorized=True, + is_admin=is_superadmin, + is_superadmin=is_superadmin, + role=UserRole.SUPERADMIN if is_superadmin else UserRole.USER, + domain_name=_DOMAIN, + ) + + +async def _seed_user_with_role( + db: ExtendedAsyncSAEngine, + *, + user_id: uuid.UUID, + role_id: uuid.UUID, +) -> None: + suffix = user_id.hex[:8] + policy_name = f"policy-{suffix}" + async with db.begin_session() as db_sess: + db_sess.add( + UserResourcePolicyRow( + name=policy_name, + max_vfolder_count=0, + max_quota_scope_size=-1, + max_session_count_per_model_session=0, + max_customized_image_count=0, + ) + ) + db_sess.add( + UserRow( + uuid=user_id, + email=f"user-{suffix}@test.com", + resource_policy=policy_name, + status=UserStatus.ACTIVE, + need_password_change=False, + sudo_session_enabled=False, + ) + ) + await db_sess.flush() + db_sess.add(RoleRow(id=role_id, name=f"role-{suffix}", description="fragment authz test")) + await db_sess.flush() + db_sess.add(UserRoleRow(user_id=user_id, role_id=role_id)) + await db_sess.flush() + + +async def _grant_fragment_create( + db: ExtendedAsyncSAEngine, + *, + role_id: uuid.UUID, + scope_id: str, +) -> None: + async with db.begin_session() as db_sess: + db_sess.add( + PermissionRow( + role_id=role_id, + scope_type=ScopeType.USER, + scope_id=scope_id, + entity_type=EntityType.APP_CONFIG_FRAGMENT, + operation=OperationType.CREATE, + permission=Permission.from_operation(OperationType.CREATE), + ) + ) + await db_sess.flush() + + +@pytest.fixture +def trigger_meta() -> BaseActionTriggerMeta: + return BaseActionTriggerMeta(action_id=uuid.uuid4(), started_at=datetime.now(UTC)) + + +@pytest.fixture +async def db_with_rbac_tables( + database_connection: ExtendedAsyncSAEngine, +) -> AsyncIterator[ExtendedAsyncSAEngine]: + async with with_tables( + database_connection, + [ + DomainRow, + UserResourcePolicyRow, + KeyPairResourcePolicyRow, + RoleRow, + UserRoleRow, + UserRow, + KeyPairRow, + PermissionRow, + ObjectPermissionRow, + AssociationScopesEntitiesRow, + ], + ): + yield database_connection + + +@pytest.fixture +def repository(db_with_rbac_tables: ExtendedAsyncSAEngine) -> PermissionControllerRepository: + return PermissionControllerRepository(db_with_rbac_tables) + + +class _CreateValidatorChain: + """Runs the fragment create processor's validator chain in wired order. + + Mirrors ``AppConfigFragmentProcessors``: the public guard runs first, then the generic + scope-chain RBAC check for user / domain scopes. + """ + + def __init__(self, repository: PermissionControllerRepository) -> None: + # MagicMock config_provider → enforcement_enabled is truthy, so both checks run. + self._guard = PublicAppConfigFragmentWriteValidator(MagicMock()) + self._scope = ScopeActionRBACValidator(repository, MagicMock()) + + async def validate( + self, action: CreateAppConfigFragmentAction, meta: BaseActionTriggerMeta + ) -> None: + await self._guard.validate(action, meta) + await self._scope.validate(action, meta) + + +@pytest.fixture +def validator(repository: PermissionControllerRepository) -> _CreateValidatorChain: + return _CreateValidatorChain(repository) + + +@pytest.fixture +async def owner_user(db_with_rbac_tables: ExtendedAsyncSAEngine) -> UserData: + """A non-superadmin granted APP_CONFIG_FRAGMENT:CREATE on their own user scope.""" + user_id = uuid.uuid4() + role_id = uuid.uuid4() + await _seed_user_with_role(db_with_rbac_tables, user_id=user_id, role_id=role_id) + await _grant_fragment_create(db_with_rbac_tables, role_id=role_id, scope_id=str(user_id)) + return _user_data(user_id, is_superadmin=False) + + +class TestCreateAuthorization: + async def test_own_user_scope_is_allowed( + self, + validator: _CreateValidatorChain, + trigger_meta: BaseActionTriggerMeta, + owner_user: UserData, + ) -> None: + action = _create_action(AppConfigScopeType.USER, str(owner_user.user_id)) + with with_user(owner_user): + await validator.validate(action, trigger_meta) + + async def test_another_user_scope_is_denied( + self, + validator: _CreateValidatorChain, + trigger_meta: BaseActionTriggerMeta, + owner_user: UserData, + ) -> None: + other_user_id = uuid.uuid4() + action = _create_action(AppConfigScopeType.USER, str(other_user_id)) + with with_user(owner_user): + with pytest.raises(NotEnoughPermission): + await validator.validate(action, trigger_meta) + + async def test_public_scope_is_denied_for_non_superadmin( + self, + validator: _CreateValidatorChain, + trigger_meta: BaseActionTriggerMeta, + owner_user: UserData, + ) -> None: + # Public fragments are global-scoped (no RBAC scope element); the empty-id target + # resolves to no scope, so a non-superadmin — even one that owns its user scope — + # is denied. + action = _create_action(AppConfigScopeType.PUBLIC, "") + with with_user(owner_user): + with pytest.raises(NotEnoughPermission): + await validator.validate(action, trigger_meta) + + async def test_public_scope_is_allowed_for_superadmin( + self, + validator: _CreateValidatorChain, + trigger_meta: BaseActionTriggerMeta, + ) -> None: + superadmin = _user_data(uuid.uuid4(), is_superadmin=True) + action = _create_action(AppConfigScopeType.PUBLIC, "") + with with_user(superadmin): + await validator.validate(action, trigger_meta) diff --git a/tests/unit/manager/services/app_config_fragment/test_create_action.py b/tests/unit/manager/services/app_config_fragment/test_create_action.py new file mode 100644 index 00000000000..4a36ee2601a --- /dev/null +++ b/tests/unit/manager/services/app_config_fragment/test_create_action.py @@ -0,0 +1,57 @@ +"""Unit tests for CreateAppConfigFragmentAction's RBAC target resolution. + +``target_element()`` decides the scope the RBAC scope validator authorizes a fragment +create against (BEP-1052): a user/domain fragment targets its USER/DOMAIN scope, while a +public fragment is global-scoped (no RBAC scope element) and thus superadmin-only. +""" + +from __future__ import annotations + +from typing import Any + +from ai.backend.common.data.app_config.types import AppConfigScopeType +from ai.backend.common.data.permission.types import RBACElementType, ScopeType +from ai.backend.manager.data.permission.types import RBACElementRef +from ai.backend.manager.repositories.app_config_fragment.creators import ( + AppConfigFragmentCreatorSpec, +) +from ai.backend.manager.services.app_config_fragment.actions.create import ( + CreateAppConfigFragmentAction, +) + + +def _action(scope_type: AppConfigScopeType, scope_id: str) -> CreateAppConfigFragmentAction: + spec: AppConfigFragmentCreatorSpec = AppConfigFragmentCreatorSpec( + config_name="cfg", + scope_type=scope_type, + scope_id=scope_id, + config={}, + ) + return CreateAppConfigFragmentAction(creator_spec=spec) + + +class TestCreateTargetElement: + def test_user_scope_targets_the_user_scope(self) -> None: + action = _action(AppConfigScopeType.USER, "user-1") + assert action.target_element() == RBACElementRef(RBACElementType.USER, "user-1") + assert action.scope_type() == ScopeType.USER + + def test_domain_scope_targets_the_domain_scope(self) -> None: + action = _action(AppConfigScopeType.DOMAIN, "default") + assert action.target_element() == RBACElementRef(RBACElementType.DOMAIN, "default") + assert action.scope_type() == ScopeType.DOMAIN + + def test_public_scope_has_no_scope_element(self) -> None: + # A public fragment is global-scoped: its target carries the fragment element type + # with an empty id, so the scope-chain check finds no scope and denies any + # non-superadmin writer (public writes are superadmin-only). + action = _action(AppConfigScopeType.PUBLIC, "") + assert action.target_element() == RBACElementRef(RBACElementType.APP_CONFIG_FRAGMENT, "") + assert action.scope_type() == ScopeType.GLOBAL + + def test_target_element_uses_the_spec_scope_id(self) -> None: + # Cross-user create: the target scope is the *supplied* user id, so RBAC checks the + # writer's permission on that other user's scope (which they lack) — not their own. + other: Any = "victim-user" + action = _action(AppConfigScopeType.USER, other) + assert action.target_element() == RBACElementRef(RBACElementType.USER, other) From f311a796c10fa2a6ce8d4f699fd6e632eb797b80 Mon Sep 17 00:00:00 2001 From: Gyubong Date: Tue, 14 Jul 2026 12:57:29 +0900 Subject: [PATCH 05/13] refactor(BA-6859): singular create_scoped + drop DB-layer authz comments - add RBACWriteOps.create_scoped (single scoped insert) and use it in the app_config_fragment db_source instead of bulk_create_scoped with a one-item list - remove the create/update/purge comments that framed authorization/write-gates in the DB layer; authz is enforced by the RBAC validators at the processor layer Co-Authored-By: Claude Opus 4.8 (1M context) --- .../app_config_fragment/db_source/db_source.py | 11 ++--------- .../backend/manager/repositories/ops/rbac/provider.py | 9 +++++++++ 2 files changed, 11 insertions(+), 9 deletions(-) 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 2c1cb9a09c7..f0649b814d0 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 @@ -79,17 +79,14 @@ def __init__(self, rbac_ops_provider: RBACOpsProvider) -> None: @app_config_fragment_db_source_resilience.apply() async def create(self, spec: AppConfigFragmentCreatorSpec) -> AppConfigFragmentData: - # The allow-list FK gates existence (no row → ``AppConfigFragmentWriteNotAllowed``), - # and create binds the fragment to its RBAC scope in the same tx. 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._rbac_ops_provider.write_ops() as w: - created = await w.bulk_create_scoped([rbac_creator]) - return created.rows[0].to_data() + created = await w.create_scoped(rbac_creator) + return created.row.to_data() @app_config_fragment_db_source_resilience.apply() async def get_by_id(self, fragment_id: AppConfigFragmentID) -> AppConfigFragmentData: @@ -101,8 +98,6 @@ async def get_by_id(self, fragment_id: AppConfigFragmentID) -> AppConfigFragment @app_config_fragment_db_source_resilience.apply() async def update(self, updater: Updater[AppConfigFragmentRow]) -> AppConfigFragmentData: - # No write-gate: the allow-list FK keeps a fragment row alive only while its - # ``(config_name, scope_type)`` entry exists, so an existing fragment is writable. async with self._rbac_ops_provider.write_ops() as w: result = await w.update(updater) if result is None: @@ -111,8 +106,6 @@ async def update(self, updater: Updater[AppConfigFragmentRow]) -> AppConfigFragm @app_config_fragment_db_source_resilience.apply() async def purge(self, purger: Purger[AppConfigFragmentRow]) -> AppConfigFragmentData: - # Purge is an RBAC unbind: the row and its scope association are deleted atomically - # (no FK cascade; a ``public`` fragment has none). Fetch first for its scope and data. async with self._rbac_ops_provider.write_ops() as w: found = await w.query(Querier(row_class=AppConfigFragmentRow, pk_value=purger.pk_value)) if found is None: diff --git a/src/ai/backend/manager/repositories/ops/rbac/provider.py b/src/ai/backend/manager/repositories/ops/rbac/provider.py index 7ed7f6ece62..8bf70eaf5d9 100644 --- a/src/ai/backend/manager/repositories/ops/rbac/provider.py +++ b/src/ai/backend/manager/repositories/ops/rbac/provider.py @@ -39,6 +39,8 @@ from ai.backend.manager.repositories.base.rbac.entity_creator import ( RBACBulkEntityCreatorResult, RBACEntityCreator, + RBACEntityCreatorResult, + execute_rbac_entity_creator, execute_rbac_entity_creators, ) from ai.backend.manager.repositories.base.rbac.scope_unbinder import ( @@ -128,6 +130,13 @@ async def _delete_virtual_scopes(self, scopes: Sequence[ScopeRef]) -> None: ) await self._sess.execute(stmt) + async def create_scoped[TRow: Base]( + self, + creator: RBACEntityCreator[TRow], + ) -> RBACEntityCreatorResult[TRow]: + """Insert one row with its RBAC scope association (the creator carries its scope).""" + return await execute_rbac_entity_creator(self._sess, creator) + async def bulk_create_scoped[TRow: Base]( self, creators: Sequence[RBACEntityCreator[TRow]], From 335e96218d4ef1b37517bee44603fd780ffee937 Mon Sep 17 00:00:00 2001 From: Gyubong Date: Tue, 14 Jul 2026 13:05:15 +0900 Subject: [PATCH 06/13] refactor(BA-6859): single-statement scoped purge; inline fragment scope ref MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - add RBACWriteOps.purge_scoped: delete the row and clear its scope association (by entity_id) in one pass, returning None when the row is already gone so a concurrent purge reports not-found — replaces the read-then-unbind flow - app_config_fragment purge uses purge_scoped; create computes its scope ref inline - drop the now-unused scope_binders module (fragment_rbac_scope_ref helper and the AppConfigFragmentScopeUnbinder it fed) Co-Authored-By: Claude Opus 4.8 (1M context) --- .../db_source/db_source.py | 32 ++++----- .../app_config_fragment/scope_binders.py | 70 ------------------- .../manager/repositories/ops/rbac/provider.py | 25 ++++++- 3 files changed, 36 insertions(+), 91 deletions(-) delete mode 100644 src/ai/backend/manager/repositories/app_config_fragment/scope_binders.py 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 f0649b814d0..0af49f15d3e 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 @@ -19,6 +19,7 @@ AppConfigFragmentData, AppConfigFragmentSearchResult, ) +from ai.backend.manager.data.permission.types import RBACElementRef from ai.backend.manager.errors.app_config import ( AppConfigFragmentNotFound, ) @@ -29,10 +30,6 @@ 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, ) @@ -79,10 +76,14 @@ def __init__(self, rbac_ops_provider: RBACOpsProvider) -> None: @app_config_fragment_db_source_resilience.apply() async def create(self, spec: AppConfigFragmentCreatorSpec) -> AppConfigFragmentData: + scope_element = spec.scope_type.to_rbac_element_type() + scope_ref = ( + RBACElementRef(scope_element, spec.scope_id) if scope_element is not None else None + ) rbac_creator = RBACEntityCreator( spec=spec, element_type=RBACElementType.APP_CONFIG_FRAGMENT, - scope_ref=fragment_rbac_scope_ref(spec.scope_type, spec.scope_id), + scope_ref=scope_ref, ) async with self._rbac_ops_provider.write_ops() as w: created = await w.create_scoped(rbac_creator) @@ -106,23 +107,14 @@ async def update(self, updater: Updater[AppConfigFragmentRow]) -> AppConfigFragm @app_config_fragment_db_source_resilience.apply() async def purge(self, purger: Purger[AppConfigFragmentRow]) -> AppConfigFragmentData: + # Purge deletes the fragment row and clears its RBAC scope association atomically + # (no FK cascade; a ``public`` fragment has none). A concurrent purge that already + # removed the row yields no result, so this reports not-found. async with self._rbac_ops_provider.write_ops() as w: - 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") - data = found.row.to_data() - result = await w.unbind_scope_entities( - AppConfigFragmentScopeUnbinder( - fragment_id=data.id, - fragment_scope_type=data.scope_type, - fragment_scope_id=data.scope_id, - ) - ) - # Read and delete are separate statements under READ COMMITTED: a zero count - # means a concurrent purge already removed the row — not-found, not stale success. - if result.deleted_count == 0: + result = await w.purge_scoped(purger, RBACElementType.APP_CONFIG_FRAGMENT) + if result is None: raise AppConfigFragmentNotFound(f"App config fragment {purger.pk_value} not found") - return data + return result.row.to_data() @app_config_fragment_db_source_resilience.apply() async def bulk_create( 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 deleted file mode 100644 index e42d7b6d352..00000000000 --- a/src/ai/backend/manager/repositories/app_config_fragment/scope_binders.py +++ /dev/null @@ -1,70 +0,0 @@ -"""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/ops/rbac/provider.py b/src/ai/backend/manager/repositories/ops/rbac/provider.py index 8bf70eaf5d9..c7f417e9fcd 100644 --- a/src/ai/backend/manager/repositories/ops/rbac/provider.py +++ b/src/ai/backend/manager/repositories/ops/rbac/provider.py @@ -12,7 +12,7 @@ from sqlalchemy.dialects.postgresql import insert as pg_insert from sqlalchemy.ext.asyncio import AsyncSession as SASession -from ai.backend.common.data.permission.types import Permission, RelationType +from ai.backend.common.data.permission.types import Permission, RBACElementType, RelationType from ai.backend.common.entity.types import EntityRef, ScopeRef from ai.backend.common.identifier.user import UserID from ai.backend.common.identifier.virtual_scope import VirtualScopeID @@ -144,6 +144,29 @@ 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 purge_scoped[TRow: Base]( + self, + purger: Purger[TRow], + element_type: RBACElementType, + ) -> PurgerResult[TRow] | None: + """Delete one row via its entity purger and clear its RBAC scope association. + + The counterpart of :meth:`create_scoped` on the delete side: the association table + has no FK to the entity tables, so a scope-bound row's association is removed in the + same transaction. A global-scoped (association-less) row matches nothing. Returns + ``None`` when the row is already gone, so a concurrent purge reports not-found. + """ + result = await self.purge(purger) + if result is None: + return None + await self._sess.execute( + sa.delete(AssociationScopesEntitiesRow).where( + AssociationScopesEntitiesRow.entity_type == element_type.to_entity_type(), + AssociationScopesEntitiesRow.entity_id == str(purger.pk_value), + ) + ) + return result + async def unbind_scope_entities[TRow: Base]( self, unbinder: RBACScopeEntityUnbinder[TRow], From 4fcdeac9fe16640bd28a4a67d788d586cce27b83 Mon Sep 17 00:00:00 2001 From: Gyubong Date: Tue, 14 Jul 2026 13:12:12 +0900 Subject: [PATCH 07/13] refactor(BA-6859): remove now-unused unbind_scope_entities from RBAC ops Fragment purge switched to purge_scoped, and no other caller uses the provider's unbind_scope_entities, so drop the dead method and its scope_unbinder imports. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../db_source/db_source.py | 3 -- .../manager/repositories/ops/rbac/provider.py | 48 ------------------- 2 files changed, 51 deletions(-) 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 0af49f15d3e..25fb9bbf110 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 @@ -107,9 +107,6 @@ async def update(self, updater: Updater[AppConfigFragmentRow]) -> AppConfigFragm @app_config_fragment_db_source_resilience.apply() async def purge(self, purger: Purger[AppConfigFragmentRow]) -> AppConfigFragmentData: - # Purge deletes the fragment row and clears its RBAC scope association atomically - # (no FK cascade; a ``public`` fragment has none). A concurrent purge that already - # removed the row yields no result, so this reports not-found. async with self._rbac_ops_provider.write_ops() as w: result = await w.purge_scoped(purger, RBACElementType.APP_CONFIG_FRAGMENT) if result is None: diff --git a/src/ai/backend/manager/repositories/ops/rbac/provider.py b/src/ai/backend/manager/repositories/ops/rbac/provider.py index c7f417e9fcd..2a09c80030f 100644 --- a/src/ai/backend/manager/repositories/ops/rbac/provider.py +++ b/src/ai/backend/manager/repositories/ops/rbac/provider.py @@ -43,10 +43,6 @@ execute_rbac_entity_creator, 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 @@ -167,50 +163,6 @@ async def purge_scoped[TRow: Base]( ) return result - 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, From 1ff1f7e9af740648829f4e275e889f024783038d Mon Sep 17 00:00:00 2001 From: Gyubong Date: Tue, 14 Jul 2026 13:12:14 +0900 Subject: [PATCH 08/13] test(BA-6859): inline create action and use UserID in fragment authz test Address review comments: drop the _create_action helper (construct the action inline per case) and type user ids as UserID. Co-Authored-By: Claude Opus 4.8 (1M context) --- ...pp_config_fragment_create_authorization.py | 58 ++++++++++++------- 1 file changed, 38 insertions(+), 20 deletions(-) diff --git a/tests/unit/manager/actions/validators/test_app_config_fragment_create_authorization.py b/tests/unit/manager/actions/validators/test_app_config_fragment_create_authorization.py index 01bdbe709bf..7268b318561 100644 --- a/tests/unit/manager/actions/validators/test_app_config_fragment_create_authorization.py +++ b/tests/unit/manager/actions/validators/test_app_config_fragment_create_authorization.py @@ -28,6 +28,7 @@ ScopeType, ) from ai.backend.common.data.user.types import UserData, UserRole +from ai.backend.common.identifier.user import UserID from ai.backend.manager.actions.action.base import BaseActionTriggerMeta from ai.backend.manager.actions.validators.rbac.scope import ScopeActionRBACValidator from ai.backend.manager.data.user.types import UserStatus @@ -76,18 +77,7 @@ _DOMAIN = "default" -def _create_action(scope_type: AppConfigScopeType, scope_id: str) -> CreateAppConfigFragmentAction: - return CreateAppConfigFragmentAction( - creator_spec=AppConfigFragmentCreatorSpec( - config_name="cfg", - scope_type=scope_type, - scope_id=scope_id, - config={}, - ) - ) - - -def _user_data(user_id: uuid.UUID, *, is_superadmin: bool) -> UserData: +def _user_data(user_id: UserID, *, is_superadmin: bool) -> UserData: return UserData( user_id=user_id, is_authorized=True, @@ -101,7 +91,7 @@ def _user_data(user_id: uuid.UUID, *, is_superadmin: bool) -> UserData: async def _seed_user_with_role( db: ExtendedAsyncSAEngine, *, - user_id: uuid.UUID, + user_id: UserID, role_id: uuid.UUID, ) -> None: suffix = user_id.hex[:8] @@ -212,7 +202,7 @@ def validator(repository: PermissionControllerRepository) -> _CreateValidatorCha @pytest.fixture async def owner_user(db_with_rbac_tables: ExtendedAsyncSAEngine) -> UserData: """A non-superadmin granted APP_CONFIG_FRAGMENT:CREATE on their own user scope.""" - user_id = uuid.uuid4() + user_id = UserID(uuid.uuid4()) role_id = uuid.uuid4() await _seed_user_with_role(db_with_rbac_tables, user_id=user_id, role_id=role_id) await _grant_fragment_create(db_with_rbac_tables, role_id=role_id, scope_id=str(user_id)) @@ -226,7 +216,14 @@ async def test_own_user_scope_is_allowed( trigger_meta: BaseActionTriggerMeta, owner_user: UserData, ) -> None: - action = _create_action(AppConfigScopeType.USER, str(owner_user.user_id)) + action = CreateAppConfigFragmentAction( + creator_spec=AppConfigFragmentCreatorSpec( + config_name="cfg", + scope_type=AppConfigScopeType.USER, + scope_id=str(owner_user.user_id), + config={}, + ) + ) with with_user(owner_user): await validator.validate(action, trigger_meta) @@ -236,8 +233,15 @@ async def test_another_user_scope_is_denied( trigger_meta: BaseActionTriggerMeta, owner_user: UserData, ) -> None: - other_user_id = uuid.uuid4() - action = _create_action(AppConfigScopeType.USER, str(other_user_id)) + other_user_id = UserID(uuid.uuid4()) + action = CreateAppConfigFragmentAction( + creator_spec=AppConfigFragmentCreatorSpec( + config_name="cfg", + scope_type=AppConfigScopeType.USER, + scope_id=str(other_user_id), + config={}, + ) + ) with with_user(owner_user): with pytest.raises(NotEnoughPermission): await validator.validate(action, trigger_meta) @@ -251,7 +255,14 @@ async def test_public_scope_is_denied_for_non_superadmin( # Public fragments are global-scoped (no RBAC scope element); the empty-id target # resolves to no scope, so a non-superadmin — even one that owns its user scope — # is denied. - action = _create_action(AppConfigScopeType.PUBLIC, "") + action = CreateAppConfigFragmentAction( + creator_spec=AppConfigFragmentCreatorSpec( + config_name="cfg", + scope_type=AppConfigScopeType.PUBLIC, + scope_id="", + config={}, + ) + ) with with_user(owner_user): with pytest.raises(NotEnoughPermission): await validator.validate(action, trigger_meta) @@ -261,7 +272,14 @@ async def test_public_scope_is_allowed_for_superadmin( validator: _CreateValidatorChain, trigger_meta: BaseActionTriggerMeta, ) -> None: - superadmin = _user_data(uuid.uuid4(), is_superadmin=True) - action = _create_action(AppConfigScopeType.PUBLIC, "") + superadmin = _user_data(UserID(uuid.uuid4()), is_superadmin=True) + action = CreateAppConfigFragmentAction( + creator_spec=AppConfigFragmentCreatorSpec( + config_name="cfg", + scope_type=AppConfigScopeType.PUBLIC, + scope_id="", + config={}, + ) + ) with with_user(superadmin): await validator.validate(action, trigger_meta) From fea374c292755e7e26dd82819b6b56d6597af18f Mon Sep 17 00:00:00 2001 From: Gyubong Date: Tue, 14 Jul 2026 13:22:00 +0900 Subject: [PATCH 09/13] test(BA-6859): drop _CreateValidatorChain, test each validator by responsibility The bundled chain hardcoded the processor's validator order and tested nothing the two validators don't cover on their own. Split into TestScopeAuthorization (ScopeActionRBACValidator: own scope allowed, another's denied) and TestPublicWriteGuard (PublicAppConfigFragmentWriteValidator, no DB needed: non-superadmin denied, superadmin allowed). Co-Authored-By: Claude Opus 4.8 (1M context) --- ...pp_config_fragment_create_authorization.py | 63 ++++++++----------- 1 file changed, 26 insertions(+), 37 deletions(-) diff --git a/tests/unit/manager/actions/validators/test_app_config_fragment_create_authorization.py b/tests/unit/manager/actions/validators/test_app_config_fragment_create_authorization.py index 7268b318561..b0b75265745 100644 --- a/tests/unit/manager/actions/validators/test_app_config_fragment_create_authorization.py +++ b/tests/unit/manager/actions/validators/test_app_config_fragment_create_authorization.py @@ -175,28 +175,16 @@ def repository(db_with_rbac_tables: ExtendedAsyncSAEngine) -> PermissionControll return PermissionControllerRepository(db_with_rbac_tables) -class _CreateValidatorChain: - """Runs the fragment create processor's validator chain in wired order. - - Mirrors ``AppConfigFragmentProcessors``: the public guard runs first, then the generic - scope-chain RBAC check for user / domain scopes. - """ - - def __init__(self, repository: PermissionControllerRepository) -> None: - # MagicMock config_provider → enforcement_enabled is truthy, so both checks run. - self._guard = PublicAppConfigFragmentWriteValidator(MagicMock()) - self._scope = ScopeActionRBACValidator(repository, MagicMock()) - - async def validate( - self, action: CreateAppConfigFragmentAction, meta: BaseActionTriggerMeta - ) -> None: - await self._guard.validate(action, meta) - await self._scope.validate(action, meta) +@pytest.fixture +def scope_validator(repository: PermissionControllerRepository) -> ScopeActionRBACValidator: + # MagicMock config_provider → enforcement_enabled is truthy, so the check runs. + return ScopeActionRBACValidator(repository, MagicMock()) @pytest.fixture -def validator(repository: PermissionControllerRepository) -> _CreateValidatorChain: - return _CreateValidatorChain(repository) +def public_guard() -> PublicAppConfigFragmentWriteValidator: + # MagicMock config_provider → enforcement_enabled is truthy, so the guard runs. + return PublicAppConfigFragmentWriteValidator(MagicMock()) @pytest.fixture @@ -209,10 +197,12 @@ async def owner_user(db_with_rbac_tables: ExtendedAsyncSAEngine) -> UserData: return _user_data(user_id, is_superadmin=False) -class TestCreateAuthorization: +class TestScopeAuthorization: + """user / domain fragment writes are authorized by the scope-chain RBAC check.""" + async def test_own_user_scope_is_allowed( self, - validator: _CreateValidatorChain, + scope_validator: ScopeActionRBACValidator, trigger_meta: BaseActionTriggerMeta, owner_user: UserData, ) -> None: @@ -225,11 +215,11 @@ async def test_own_user_scope_is_allowed( ) ) with with_user(owner_user): - await validator.validate(action, trigger_meta) + await scope_validator.validate(action, trigger_meta) async def test_another_user_scope_is_denied( self, - validator: _CreateValidatorChain, + scope_validator: ScopeActionRBACValidator, trigger_meta: BaseActionTriggerMeta, owner_user: UserData, ) -> None: @@ -244,17 +234,17 @@ async def test_another_user_scope_is_denied( ) with with_user(owner_user): with pytest.raises(NotEnoughPermission): - await validator.validate(action, trigger_meta) + await scope_validator.validate(action, trigger_meta) + + +class TestPublicWriteGuard: + """public fragments are global-scoped, so their writes are superadmin-only.""" - async def test_public_scope_is_denied_for_non_superadmin( + async def test_non_superadmin_is_denied( self, - validator: _CreateValidatorChain, + public_guard: PublicAppConfigFragmentWriteValidator, trigger_meta: BaseActionTriggerMeta, - owner_user: UserData, ) -> None: - # Public fragments are global-scoped (no RBAC scope element); the empty-id target - # resolves to no scope, so a non-superadmin — even one that owns its user scope — - # is denied. action = CreateAppConfigFragmentAction( creator_spec=AppConfigFragmentCreatorSpec( config_name="cfg", @@ -263,16 +253,15 @@ async def test_public_scope_is_denied_for_non_superadmin( config={}, ) ) - with with_user(owner_user): + with with_user(_user_data(UserID(uuid.uuid4()), is_superadmin=False)): with pytest.raises(NotEnoughPermission): - await validator.validate(action, trigger_meta) + await public_guard.validate(action, trigger_meta) - async def test_public_scope_is_allowed_for_superadmin( + async def test_superadmin_is_allowed( self, - validator: _CreateValidatorChain, + public_guard: PublicAppConfigFragmentWriteValidator, trigger_meta: BaseActionTriggerMeta, ) -> None: - superadmin = _user_data(UserID(uuid.uuid4()), is_superadmin=True) action = CreateAppConfigFragmentAction( creator_spec=AppConfigFragmentCreatorSpec( config_name="cfg", @@ -281,5 +270,5 @@ async def test_public_scope_is_allowed_for_superadmin( config={}, ) ) - with with_user(superadmin): - await validator.validate(action, trigger_meta) + with with_user(_user_data(UserID(uuid.uuid4()), is_superadmin=True)): + await public_guard.validate(action, trigger_meta) From 71e39d38127e99287f3c1fb712fab3794223f223 Mon Sep 17 00:00:00 2001 From: Gyubong Date: Tue, 14 Jul 2026 13:41:49 +0900 Subject: [PATCH 10/13] feat(BA-6859): make app_config fragment a full RBAC entity-as-scope Register APP_CONFIG_FRAGMENT as an RBAC scope and finish the write-authorization path on top of it: - ScopeType: add APP_CONFIG_FRAGMENT so to_scope_type() resolves for the fragment in both the scope-chain check and the entity-as-scope purge - ops: add single create_scoped / purge_scoped; purge clears the fragment row, its scope associations, and any permissions granted at its own scope - authorization: public (global-scoped) writes stay superadmin-only through the generic scope validator, so the bespoke PublicAppConfigFragmentWriteValidator is removed and create wires just the scope validator - migration: backfill APP_CONFIG_FRAGMENT permissions onto existing roles - tests: create/public authorization + entity-as-scope purge (with_tables gains the role/permission tables) Co-Authored-By: Claude Opus 4.8 (1M context) --- .../backend/common/data/permission/types.py | 1 + ...app_config_fragment_permissions_to_rbac.py | 88 +++++++++++++++++++ .../db_source/db_source.py | 13 ++- .../app_config_fragment/purgers.py | 26 ++++++ .../manager/repositories/ops/rbac/provider.py | 32 +++---- .../app_config_fragment/processors.py | 21 ++--- .../app_config_fragment/validators.py | 44 ---------- src/ai/backend/manager/services/factory.py | 5 +- ...pp_config_fragment_create_authorization.py | 22 ++--- .../app_config_fragment/test_repository.py | 8 +- 10 files changed, 160 insertions(+), 100 deletions(-) create mode 100644 src/ai/backend/manager/models/alembic/versions/b3d9f7a2c184_add_app_config_fragment_permissions_to_rbac.py create mode 100644 src/ai/backend/manager/repositories/app_config_fragment/purgers.py delete mode 100644 src/ai/backend/manager/services/app_config_fragment/validators.py diff --git a/src/ai/backend/common/data/permission/types.py b/src/ai/backend/common/data/permission/types.py index 7f002a5e654..1ea718bc783 100644 --- a/src/ai/backend/common/data/permission/types.py +++ b/src/ai/backend/common/data/permission/types.py @@ -359,6 +359,7 @@ class ScopeType(enum.StrEnum): ROLE = "role" ROLE_ASSIGNMENT = "role:assignment" NOTIFICATION_CHANNEL = "notification_channel" + APP_CONFIG_FRAGMENT = "app_config_fragment" KEYPAIR = "keypair" KEYPAIR_RESOURCE_POLICY = "keypair_resource_policy" diff --git a/src/ai/backend/manager/models/alembic/versions/b3d9f7a2c184_add_app_config_fragment_permissions_to_rbac.py b/src/ai/backend/manager/models/alembic/versions/b3d9f7a2c184_add_app_config_fragment_permissions_to_rbac.py new file mode 100644 index 00000000000..5dbd0de5d8e --- /dev/null +++ b/src/ai/backend/manager/models/alembic/versions/b3d9f7a2c184_add_app_config_fragment_permissions_to_rbac.py @@ -0,0 +1,88 @@ +"""add_app_config_fragment_permissions_to_rbac + +Backfill ``APP_CONFIG_FRAGMENT`` permissions onto existing RBAC roles so that already +provisioned user / domain / project roles can exercise fragment writes once RBAC +enforcement is enabled (BEP-1052). New roles receive these permissions at creation via the +runtime allow list; this migration covers roles that predate the allow-list change. + +Revision ID: b3d9f7a2c184 +Revises: a3c1d8e5b294 +Create Date: 2026-07-14 00:00:00.000000 + +""" + +import sqlalchemy as sa +from alembic import op + +from ai.backend.manager.models.rbac_models.migration.enums import ( + EntityType, + OperationType, +) + +# revision identifiers, used by Alembic. +revision = "b3d9f7a2c184" +down_revision = "a3c1d8e5b294" +# Part of: NEXT_RELEASE_VERSION +branch_labels = None +depends_on = None + +MEMBER_ROLE_SUFFIX = "member" +APP_CONFIG_FRAGMENT_ENTITY_TYPE = EntityType.APP_CONFIG_FRAGMENT.value + + +def upgrade() -> None: + db_conn = op.get_bind() + _add_app_config_fragment_permissions(db_conn) + + +def downgrade() -> None: + db_conn = op.get_bind() + db_conn.execute( + sa.text("DELETE FROM permissions WHERE entity_type = :entity_type"), + {"entity_type": APP_CONFIG_FRAGMENT_ENTITY_TYPE}, + ) + + +def _add_app_config_fragment_permissions(db_conn: sa.engine.Connection) -> None: + """Backfill APP_CONFIG_FRAGMENT write permissions onto existing write-capable roles. + + A fragment lives only at ``user`` or ``domain`` scope (``public`` is global → + superadmin-only, no role), so only those scopes are touched — a permission at any + other scope (``project`` etc.) could never match a real fragment. Reads go through the + allow list, not RBAC, so only the write path needs backfilling: grant the full + operation set to the write-capable roles, i.e. a user's own ``user``-scope role and + ``domain`` admins. Member roles are excluded (they do not write fragments). + """ + owner_ops = [op_type.value for op_type in OperationType.owner_operations()] + + insert_query = sa.text(""" + WITH role_scopes AS ( + SELECT DISTINCT + p.role_id, + r.name AS role_name, + p.scope_type, + p.scope_id + FROM permissions p + JOIN roles r ON p.role_id = r.id + WHERE p.scope_type IN ('user', 'domain') + ) + INSERT INTO permissions (role_id, scope_type, scope_id, entity_type, operation) + SELECT + rs.role_id, + rs.scope_type, + rs.scope_id, + :entity_type AS entity_type, + unnest(CAST(:owner_ops AS text[])) AS operation + FROM role_scopes rs + WHERE NOT (rs.role_name LIKE :member_pattern) + ON CONFLICT (role_id, scope_type, scope_id, entity_type, operation) DO NOTHING + """) + + db_conn.execute( + insert_query, + { + "owner_ops": owner_ops, + "member_pattern": f"%{MEMBER_ROLE_SUFFIX}", + "entity_type": APP_CONFIG_FRAGMENT_ENTITY_TYPE, + }, + ) 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 25fb9bbf110..4c729b2d529 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 @@ -3,6 +3,7 @@ from __future__ import annotations from collections.abc import Sequence +from typing import cast import sqlalchemy as sa @@ -30,6 +31,9 @@ from ai.backend.manager.repositories.app_config_fragment.creators import ( AppConfigFragmentCreatorSpec, ) +from ai.backend.manager.repositories.app_config_fragment.purgers import ( + AppConfigFragmentPurgerSpec, +) from ai.backend.manager.repositories.app_config_fragment.types import ( AppConfigScopeArguments, ) @@ -42,6 +46,7 @@ Updater, ) from ai.backend.manager.repositories.base.rbac.entity_creator import RBACEntityCreator +from ai.backend.manager.repositories.base.rbac.entity_purger import RBACEntityPurger from ai.backend.manager.repositories.ops.rbac.provider import RBACOpsProvider __all__ = ("AppConfigFragmentDBSource",) @@ -107,8 +112,14 @@ async def update(self, updater: Updater[AppConfigFragmentRow]) -> AppConfigFragm @app_config_fragment_db_source_resilience.apply() async def purge(self, purger: Purger[AppConfigFragmentRow]) -> AppConfigFragmentData: + fragment_id = cast(AppConfigFragmentID, purger.pk_value) + rbac_purger = RBACEntityPurger( + row_class=purger.row_class, + pk_value=purger.pk_value, + spec=AppConfigFragmentPurgerSpec(fragment_id=fragment_id), + ) async with self._rbac_ops_provider.write_ops() as w: - result = await w.purge_scoped(purger, RBACElementType.APP_CONFIG_FRAGMENT) + result = await w.purge_scoped(rbac_purger) if result is None: raise AppConfigFragmentNotFound(f"App config fragment {purger.pk_value} not found") return result.row.to_data() diff --git a/src/ai/backend/manager/repositories/app_config_fragment/purgers.py b/src/ai/backend/manager/repositories/app_config_fragment/purgers.py new file mode 100644 index 00000000000..56bf4ecb80e --- /dev/null +++ b/src/ai/backend/manager/repositories/app_config_fragment/purgers.py @@ -0,0 +1,26 @@ +"""Purger specs for app config fragment repository.""" + +from __future__ import annotations + +from dataclasses import dataclass +from typing import override + +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.repositories.base.rbac.entity_purger import RBACEntityPurgerSpec + + +@dataclass +class AppConfigFragmentPurgerSpec(RBACEntityPurgerSpec): + """RBAC purge info for one fragment: identifies it so its scope association is cleared.""" + + fragment_id: AppConfigFragmentID + + @override + def element_type(self) -> RBACElementType: + return RBACElementType.APP_CONFIG_FRAGMENT + + @override + def entity_ref(self) -> RBACElementRef: + return RBACElementRef(RBACElementType.APP_CONFIG_FRAGMENT, str(self.fragment_id)) diff --git a/src/ai/backend/manager/repositories/ops/rbac/provider.py b/src/ai/backend/manager/repositories/ops/rbac/provider.py index 2a09c80030f..59612b9c828 100644 --- a/src/ai/backend/manager/repositories/ops/rbac/provider.py +++ b/src/ai/backend/manager/repositories/ops/rbac/provider.py @@ -12,7 +12,7 @@ from sqlalchemy.dialects.postgresql import insert as pg_insert from sqlalchemy.ext.asyncio import AsyncSession as SASession -from ai.backend.common.data.permission.types import Permission, RBACElementType, RelationType +from ai.backend.common.data.permission.types import Permission, RelationType from ai.backend.common.entity.types import EntityRef, ScopeRef from ai.backend.common.identifier.user import UserID from ai.backend.common.identifier.virtual_scope import VirtualScopeID @@ -43,6 +43,11 @@ execute_rbac_entity_creator, execute_rbac_entity_creators, ) +from ai.backend.manager.repositories.base.rbac.entity_purger import ( + RBACEntityPurger, + RBACEntityPurgerResult, + execute_rbac_entity_purger, +) from ai.backend.manager.repositories.ops.base.provider import DBOpsProvider, WriteOps from ai.backend.manager.repositories.permission_controller.role_manager import RoleManager @@ -142,26 +147,15 @@ async def bulk_create_scoped[TRow: Base]( async def purge_scoped[TRow: Base]( self, - purger: Purger[TRow], - element_type: RBACElementType, - ) -> PurgerResult[TRow] | None: - """Delete one row via its entity purger and clear its RBAC scope association. + purger: RBACEntityPurger[TRow], + ) -> RBACEntityPurgerResult[TRow] | None: + """Delete one row and its RBAC entries (the counterpart of :meth:`create_scoped`). - The counterpart of :meth:`create_scoped` on the delete side: the association table - has no FK to the entity tables, so a scope-bound row's association is removed in the - same transaction. A global-scoped (association-less) row matches nothing. Returns - ``None`` when the row is already gone, so a concurrent purge reports not-found. + The association table has no FK to the entity tables, so a scope-bound row's + association is cleared in the same transaction. Returns ``None`` when the row is + already gone, so a concurrent purge reports not-found. """ - result = await self.purge(purger) - if result is None: - return None - await self._sess.execute( - sa.delete(AssociationScopesEntitiesRow).where( - AssociationScopesEntitiesRow.entity_type == element_type.to_entity_type(), - AssociationScopesEntitiesRow.entity_id == str(purger.pk_value), - ) - ) - return result + return await execute_rbac_entity_purger(self._sess, purger) async def add_users_to_scope( self, diff --git a/src/ai/backend/manager/services/app_config_fragment/processors.py b/src/ai/backend/manager/services/app_config_fragment/processors.py index ec58c174568..037e883cbd0 100644 --- a/src/ai/backend/manager/services/app_config_fragment/processors.py +++ b/src/ai/backend/manager/services/app_config_fragment/processors.py @@ -8,7 +8,6 @@ from ai.backend.manager.actions.processor.single_entity import SingleEntityActionProcessor from ai.backend.manager.actions.types import AbstractProcessorPackage, ActionSpec from ai.backend.manager.actions.validators import ActionValidators -from ai.backend.manager.config.provider import ManagerConfigProvider from ai.backend.manager.services.app_config_fragment.actions.admin_search import ( AdminSearchAppConfigFragmentAction, AdminSearchAppConfigFragmentActionResult, @@ -48,9 +47,6 @@ from ai.backend.manager.services.app_config_fragment.service import ( AppConfigFragmentService, ) -from ai.backend.manager.services.app_config_fragment.validators import ( - PublicAppConfigFragmentWriteValidator, -) class AppConfigFragmentProcessors(AbstractProcessorPackage): @@ -83,23 +79,16 @@ def __init__( service: AppConfigFragmentService, action_monitors: list[ActionMonitor], validators: ActionValidators, - config_provider: ManagerConfigProvider, ) -> None: # Writes are authorized by RBAC (BEP-1052): create acts at the fragment's target - # scope (own user / domain / superadmin-only public), while update / purge act on - # the fragment entity whose scope is resolved through its scope binding. Reads stay - # on the allow-list read tiers, so get / admin_search / scoped_search carry no RBAC + # scope (own user / domain; a public fragment is global-scoped, so no role grants + # the write and only a superadmin passes), while update / purge act on the fragment + # entity whose scope is resolved through its scope binding. Reads stay on the + # allow-list read tiers, so get / admin_search / scoped_search carry no RBAC # validator. Bulk create has no pre-existing targets, so the target-based bulk # validator cannot authorize it — only bulk update / purge are wired. - # - # A public fragment is global-scoped and has no RBAC scope element, so the generic - # scope-chain validator cannot represent it; the public guard runs first to keep - # public writes superadmin-only and defers user / domain scopes to the scope check. - public_write_guard = PublicAppConfigFragmentWriteValidator(config_provider) self.create = ScopeActionProcessor( - service.create, - action_monitors, - validators=[public_write_guard, validators.rbac.scope], + service.create, action_monitors, validators=[validators.rbac.scope] ) self.get = SingleEntityActionProcessor(service.get, action_monitors) self.admin_search = ScopeActionProcessor(service.admin_search, action_monitors) diff --git a/src/ai/backend/manager/services/app_config_fragment/validators.py b/src/ai/backend/manager/services/app_config_fragment/validators.py deleted file mode 100644 index 9a81de98a7e..00000000000 --- a/src/ai/backend/manager/services/app_config_fragment/validators.py +++ /dev/null @@ -1,44 +0,0 @@ -"""Scope-action validators specific to app config fragment writes (BEP-1052).""" - -from __future__ import annotations - -from typing import override - -from ai.backend.common.contexts.user import current_user -from ai.backend.common.data.permission.types import ScopeType -from ai.backend.common.exception import UnreachableError -from ai.backend.manager.actions.action import BaseActionTriggerMeta -from ai.backend.manager.actions.action.scope import BaseScopeAction -from ai.backend.manager.actions.validator.scope import ScopeActionValidator -from ai.backend.manager.config.provider import ManagerConfigProvider -from ai.backend.manager.errors.permission import NotEnoughPermission - -__all__ = ("PublicAppConfigFragmentWriteValidator",) - - -class PublicAppConfigFragmentWriteValidator(ScopeActionValidator): - """Restrict writes to public (global-scoped) fragments to superadmins. - - A public fragment is global-scoped and has no RBAC scope element, so the generic - scope-chain check cannot represent it. Public writes are therefore superadmin-only - (BEP-1052). This guard runs ahead of ``ScopeActionRBACValidator`` and no-ops for - ``user`` / ``domain`` scopes, leaving those to the scope-chain check. - """ - - _config_provider: ManagerConfigProvider - - def __init__(self, config_provider: ManagerConfigProvider) -> None: - self._config_provider = config_provider - - @override - async def validate(self, action: BaseScopeAction, meta: BaseActionTriggerMeta) -> None: - if not self._config_provider.config.manager.rbac.enforcement_enabled: - return - if action.scope_type() != ScopeType.GLOBAL: - return - user = current_user() - if user is None: - raise UnreachableError("User context is not available") - if user.is_superadmin: - return - raise NotEnoughPermission("Creating a public app config fragment requires superadmin") diff --git a/src/ai/backend/manager/services/factory.py b/src/ai/backend/manager/services/factory.py index 1c4df7a5347..caa16eabdac 100644 --- a/src/ai/backend/manager/services/factory.py +++ b/src/ai/backend/manager/services/factory.py @@ -521,10 +521,7 @@ def create_processors( services.app_config_definition, action_monitors ), app_config_fragment=AppConfigFragmentProcessors( - services.app_config_fragment, - action_monitors, - validators, - args.service_args.config_provider, + services.app_config_fragment, action_monitors, validators ), login_client_type=LoginClientTypeProcessors(services.login_client_type, action_monitors), login_client_type_admin=LoginClientTypeAdminProcessors( diff --git a/tests/unit/manager/actions/validators/test_app_config_fragment_create_authorization.py b/tests/unit/manager/actions/validators/test_app_config_fragment_create_authorization.py index b0b75265745..d638694ddab 100644 --- a/tests/unit/manager/actions/validators/test_app_config_fragment_create_authorization.py +++ b/tests/unit/manager/actions/validators/test_app_config_fragment_create_authorization.py @@ -63,9 +63,6 @@ from ai.backend.manager.services.app_config_fragment.actions.create import ( CreateAppConfigFragmentAction, ) -from ai.backend.manager.services.app_config_fragment.validators import ( - PublicAppConfigFragmentWriteValidator, -) from ai.backend.testutils.db import with_tables _ORM_CLUSTER = ( @@ -181,12 +178,6 @@ def scope_validator(repository: PermissionControllerRepository) -> ScopeActionRB return ScopeActionRBACValidator(repository, MagicMock()) -@pytest.fixture -def public_guard() -> PublicAppConfigFragmentWriteValidator: - # MagicMock config_provider → enforcement_enabled is truthy, so the guard runs. - return PublicAppConfigFragmentWriteValidator(MagicMock()) - - @pytest.fixture async def owner_user(db_with_rbac_tables: ExtendedAsyncSAEngine) -> UserData: """A non-superadmin granted APP_CONFIG_FRAGMENT:CREATE on their own user scope.""" @@ -237,12 +228,13 @@ async def test_another_user_scope_is_denied( await scope_validator.validate(action, trigger_meta) -class TestPublicWriteGuard: - """public fragments are global-scoped, so their writes are superadmin-only.""" +class TestPublicScopeAuthorization: + """A public fragment is global-scoped: no role grants the write, so the scope-chain + check denies everyone and only a superadmin bypasses it.""" async def test_non_superadmin_is_denied( self, - public_guard: PublicAppConfigFragmentWriteValidator, + scope_validator: ScopeActionRBACValidator, trigger_meta: BaseActionTriggerMeta, ) -> None: action = CreateAppConfigFragmentAction( @@ -255,11 +247,11 @@ async def test_non_superadmin_is_denied( ) with with_user(_user_data(UserID(uuid.uuid4()), is_superadmin=False)): with pytest.raises(NotEnoughPermission): - await public_guard.validate(action, trigger_meta) + await scope_validator.validate(action, trigger_meta) async def test_superadmin_is_allowed( self, - public_guard: PublicAppConfigFragmentWriteValidator, + scope_validator: ScopeActionRBACValidator, trigger_meta: BaseActionTriggerMeta, ) -> None: action = CreateAppConfigFragmentAction( @@ -271,4 +263,4 @@ async def test_superadmin_is_allowed( ) ) with with_user(_user_data(UserID(uuid.uuid4()), is_superadmin=True)): - await public_guard.validate(action, trigger_meta) + await scope_validator.validate(action, trigger_meta) 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 cdd2ad74012..41f43fea320 100644 --- a/tests/unit/manager/repositories/app_config_fragment/test_repository.py +++ b/tests/unit/manager/repositories/app_config_fragment/test_repository.py @@ -28,6 +28,8 @@ from ai.backend.manager.models.rbac_models.association_scopes_entities import ( AssociationScopesEntitiesRow, ) +from ai.backend.manager.models.rbac_models.permission.permission import PermissionRow +from ai.backend.manager.models.rbac_models.role import RoleRow from ai.backend.manager.models.utils import ExtendedAsyncSAEngine from ai.backend.manager.repositories.app_config_fragment.creators import ( AppConfigFragmentCreatorSpec, @@ -66,7 +68,9 @@ 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. + # AssociationScopesEntitiesRow holds the RBAC scope↔fragment binding written on create; + # RoleRow + PermissionRow back the entity-as-scope purge, which clears any permissions + # granted at the fragment's own scope when it is purged. async with with_tables( database_connection, [ @@ -74,6 +78,8 @@ async def database( AppConfigAllowListRow, AppConfigFragmentRow, AssociationScopesEntitiesRow, + RoleRow, + PermissionRow, ], ): yield database_connection From 69f9cbf15affdc68208245ecf6b8d83765805f23 Mon Sep 17 00:00:00 2001 From: Gyubong Date: Tue, 14 Jul 2026 13:49:00 +0900 Subject: [PATCH 11/13] feat(BA-6859): RBAC-gate app_config fragment scoped_search Wire the bulk RBAC validator into scoped_search so a caller cannot read another user's fragments: each queried user/domain scope is checked for the caller's read permission, matching the audit_log scoped_search pattern. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../services/app_config_fragment/processors.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/ai/backend/manager/services/app_config_fragment/processors.py b/src/ai/backend/manager/services/app_config_fragment/processors.py index 037e883cbd0..c5d91eecda4 100644 --- a/src/ai/backend/manager/services/app_config_fragment/processors.py +++ b/src/ai/backend/manager/services/app_config_fragment/processors.py @@ -80,19 +80,14 @@ def __init__( action_monitors: list[ActionMonitor], validators: ActionValidators, ) -> None: - # Writes are authorized by RBAC (BEP-1052): create acts at the fragment's target - # scope (own user / domain; a public fragment is global-scoped, so no role grants - # the write and only a superadmin passes), while update / purge act on the fragment - # entity whose scope is resolved through its scope binding. Reads stay on the - # allow-list read tiers, so get / admin_search / scoped_search carry no RBAC - # validator. Bulk create has no pre-existing targets, so the target-based bulk - # validator cannot authorize it — only bulk update / purge are wired. self.create = ScopeActionProcessor( service.create, action_monitors, validators=[validators.rbac.scope] ) self.get = SingleEntityActionProcessor(service.get, action_monitors) self.admin_search = ScopeActionProcessor(service.admin_search, action_monitors) - self.scoped_search = BulkActionProcessor(service.scoped_search, monitors=action_monitors) + self.scoped_search = BulkActionProcessor( + service.scoped_search, monitors=action_monitors, validators=[validators.rbac.bulk] + ) self.update = SingleEntityActionProcessor( service.update, action_monitors, validators=[validators.rbac.single_entity] ) From c4cb88040c60c54d0807ad479dc6381e98714088 Mon Sep 17 00:00:00 2001 From: Gyubong Date: Tue, 14 Jul 2026 14:32:59 +0900 Subject: [PATCH 12/13] feat(BA-6859): RBAC-gate app_config fragment bulk_create per scope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add BulkScopeActionRBACValidator (the bulk analog of ScopeActionRBACValidator: subject is the action's own entity type, targets are scopes) and wire it into bulk_create. bulk_create's action now exposes each spec's scope as a target, so a caller may bulk-create only in scopes it can write (own user / domain; public is superadmin-only). The generic bulk validator cannot authorize this — it would check CREATE on the USER/DOMAIN scope type, which users lack. The validator is added to the RBACValidators bundle as an optional field (like legacy_rbac) so existing test fixtures keep working; the composer always sets it. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../actions/validators/rbac/__init__.py | 4 + .../actions/validators/rbac/bulk_scope.py | 93 +++++++++++++++++++ .../dependencies/processing/composer.py | 4 + .../app_config_fragment/actions/base.py | 10 +- .../actions/bulk_create.py | 51 ++++++++-- .../app_config_fragment/processors.py | 9 +- ...pp_config_fragment_create_authorization.py | 77 +++++++++++++++ 7 files changed, 234 insertions(+), 14 deletions(-) create mode 100644 src/ai/backend/manager/actions/validators/rbac/bulk_scope.py diff --git a/src/ai/backend/manager/actions/validators/rbac/__init__.py b/src/ai/backend/manager/actions/validators/rbac/__init__.py index 8c50284632f..c7b386d10e9 100644 --- a/src/ai/backend/manager/actions/validators/rbac/__init__.py +++ b/src/ai/backend/manager/actions/validators/rbac/__init__.py @@ -1,6 +1,7 @@ from dataclasses import dataclass from ai.backend.manager.actions.validators.rbac.bulk import BulkActionRBACValidator +from ai.backend.manager.actions.validators.rbac.bulk_scope import BulkScopeActionRBACValidator from ai.backend.manager.actions.validators.rbac.legacy import ( LegacyScopeActionRBACValidator, LegacySingleEntityActionRBACValidator, @@ -16,6 +17,9 @@ class RBACValidators: scope: ScopeActionRBACValidator single_entity: SingleEntityActionRBACValidator bulk: BulkActionRBACValidator + # Optional so the many test fixtures that build RBACValidators without it keep working; + # production (the composer) always sets it. + bulk_scope: BulkScopeActionRBACValidator | None = None @dataclass diff --git a/src/ai/backend/manager/actions/validators/rbac/bulk_scope.py b/src/ai/backend/manager/actions/validators/rbac/bulk_scope.py new file mode 100644 index 00000000000..7779cd78bc0 --- /dev/null +++ b/src/ai/backend/manager/actions/validators/rbac/bulk_scope.py @@ -0,0 +1,93 @@ +from typing import Any, override + +from ai.backend.common.contexts.user import current_user +from ai.backend.common.exception import UnreachableError +from ai.backend.manager.actions.action import BaseActionTriggerMeta +from ai.backend.manager.actions.action.bulk import BaseBulkAction +from ai.backend.manager.actions.validator.bulk import ( + BulkActionValidator, + BulkValidationResult, + DeniedEntity, +) +from ai.backend.manager.config.provider import ManagerConfigProvider +from ai.backend.manager.data.permission.role import ( + BulkPermissionCheckInput, + PermissionResolutionKey, +) +from ai.backend.manager.data.permission.types import RBACElementRef +from ai.backend.manager.repositories.permission_controller.repository import ( + PermissionControllerRepository, +) + +_DENY_REASON = "permission_denied" + + +class BulkScopeActionRBACValidator(BulkActionValidator): + """Bulk analog of ``ScopeActionRBACValidator``: authorize a bulk action at each of its + scope targets by the caller's permission on the action's *own* entity type. + + ``BulkActionRBACValidator`` checks each target as the entity itself (subject = + ``ref.element_type``), which fits bulk actions whose targets are entities. Here the + targets are *scopes*, so the checked subject is ``action.entity_type()`` — e.g. "create + APP_CONFIG_FRAGMENT in scope X", not "act on the USER / DOMAIN scope itself". A global + target (no scope grants its resource op) is denied, so only a superadmin passes. + """ + + _repository: PermissionControllerRepository + _config_provider: ManagerConfigProvider + + def __init__( + self, + repository: PermissionControllerRepository, + config_provider: ManagerConfigProvider, + ) -> None: + self._repository = repository + self._config_provider = config_provider + + @classmethod + @override + def name(cls) -> str: + return "rbac_scope" + + @override + async def validate( + self, action: BaseBulkAction[Any], meta: BaseActionTriggerMeta + ) -> BulkValidationResult: + element_refs = [t.to_rbac_element_ref() for t in action.targets()] + if not self._config_provider.config.manager.rbac.enforcement_enabled: + return BulkValidationResult(allowed_entities=element_refs, denied_entities=[]) + + user = current_user() + if user is None: + raise UnreachableError("User context is not available") + if user.is_superadmin: + return BulkValidationResult(allowed_entities=element_refs, denied_entities=[]) + + subject_entity_type = action.entity_type().to_element() + keys = [ + PermissionResolutionKey( + user_id=user.user_id, + element_type=ref.element_type, + entity_id=ref.element_id, + subject_entity_type=subject_entity_type, + ) + for ref in element_refs + ] + permission_map = await self._repository.check_bulk_permission_with_scope_chain( + BulkPermissionCheckInput( + keys=keys, + operation=action.operation_type().to_permission_operation(), + ) + ) + + allowed_entities: list[RBACElementRef] = [] + denied_entities: list[DeniedEntity] = [] + for ref, key in zip(element_refs, keys, strict=True): + if permission_map.get(key, False): + allowed_entities.append(ref) + else: + denied_entities.append(DeniedEntity(entity_ref=ref, deny_reason=_DENY_REASON)) + return BulkValidationResult( + allowed_entities=allowed_entities, + denied_entities=denied_entities, + ) diff --git a/src/ai/backend/manager/dependencies/processing/composer.py b/src/ai/backend/manager/dependencies/processing/composer.py index b4c7c9dee5d..3d4577325b0 100644 --- a/src/ai/backend/manager/dependencies/processing/composer.py +++ b/src/ai/backend/manager/dependencies/processing/composer.py @@ -31,6 +31,7 @@ from ai.backend.manager.actions.validators import ActionValidators from ai.backend.manager.actions.validators.rbac import LegacyRBACValidators, RBACValidators from ai.backend.manager.actions.validators.rbac.bulk import BulkActionRBACValidator +from ai.backend.manager.actions.validators.rbac.bulk_scope import BulkScopeActionRBACValidator from ai.backend.manager.actions.validators.rbac.legacy import ( LegacyScopeActionRBACValidator, LegacySingleEntityActionRBACValidator, @@ -269,6 +270,9 @@ async def compose( permission_controller_repository, config_provider ), bulk=BulkActionRBACValidator(permission_controller_repository, config_provider), + bulk_scope=BulkScopeActionRBACValidator( + permission_controller_repository, config_provider + ), ) legacy_rbac_validators = LegacyRBACValidators( scope=LegacyScopeActionRBACValidator(permission_controller_repository), diff --git a/src/ai/backend/manager/services/app_config_fragment/actions/base.py b/src/ai/backend/manager/services/app_config_fragment/actions/base.py index 667a48ac48f..cb60af87aaf 100644 --- a/src/ai/backend/manager/services/app_config_fragment/actions/base.py +++ b/src/ai/backend/manager/services/app_config_fragment/actions/base.py @@ -53,9 +53,8 @@ class AppConfigFragmentSingleEntityActionResult(BaseSingleEntityActionResult): class AppConfigFragmentBulkTarget(ActionTarget): """One existing fragment touched by a bulk update / purge, exposed for per-entity RBAC. - Bulk create has no targets — the fragments do not exist yet, so its action returns an - empty sequence. The target lets a future ``BulkActionValidator`` iterate the batch; - richer per-item data stays on the action's ``bulk_*`` payload. + Bulk create targets scopes instead (the fragments do not exist yet) — see + ``AppConfigFragmentScopeTarget`` in ``bulk_create``. """ fragment_id: AppConfigFragmentID @@ -68,12 +67,11 @@ def to_rbac_element_ref(self) -> RBACElementRef: class AppConfigFragmentBulkAction(BaseBulkAction[AppConfigFragmentBulkTarget]): - """Base for bulk app config fragment mutations (bulk create / update / purge). + """Base for bulk app config fragment mutations over existing fragments (update / purge). Bulk operations span many fragments (potentially across scopes), so there is no single entity id to report. Each concrete action exposes its per-item targets via ``targets()`` - so a ``BulkActionValidator`` can authorize the batch per entity. No validator is wired - yet — authorization currently lives in the repository's allow-list write-gate. + so the bulk RBAC validator can authorize the batch per fragment. """ @override diff --git a/src/ai/backend/manager/services/app_config_fragment/actions/bulk_create.py b/src/ai/backend/manager/services/app_config_fragment/actions/bulk_create.py index 86f011241f1..b315d15aaa4 100644 --- a/src/ai/backend/manager/services/app_config_fragment/actions/bulk_create.py +++ b/src/ai/backend/manager/services/app_config_fragment/actions/bulk_create.py @@ -4,32 +4,69 @@ from dataclasses import dataclass from typing import override +from ai.backend.common.data.app_config.types import AppConfigScopeType +from ai.backend.common.data.permission.types import EntityType, RBACElementType +from ai.backend.manager.actions.action.bulk import BaseBulkAction +from ai.backend.manager.actions.action.types import ActionTarget from ai.backend.manager.actions.types import ActionOperationType +from ai.backend.manager.data.permission.types import RBACElementRef from ai.backend.manager.repositories.app_config_fragment.creators import ( AppConfigFragmentCreatorSpec, ) from ai.backend.manager.services.app_config_fragment.actions.base import ( - AppConfigFragmentBulkAction, AppConfigFragmentBulkActionResult, - AppConfigFragmentBulkTarget, ) +@dataclass(frozen=True) +class AppConfigFragmentScopeTarget(ActionTarget): + """One scope a bulk create writes into, exposed for per-scope RBAC. + + A ``public`` fragment is global-scoped (no RBAC scope element); its target carries the + fragment element type with an empty id, so the bulk-scope validator treats it as a + global target (superadmin-only). + """ + + scope_type: AppConfigScopeType + scope_id: str + + @override + def to_rbac_element_ref(self) -> RBACElementRef: + element = self.scope_type.to_rbac_element_type() + if element is None: + return RBACElementRef(RBACElementType.APP_CONFIG_FRAGMENT, "") + return RBACElementRef(element, self.scope_id) + + @dataclass -class BulkCreateAppConfigFragmentAction(AppConfigFragmentBulkAction): - """Create many fragments with per-item partial success; the FK to the allow-list gates each write.""" +class BulkCreateAppConfigFragmentAction(BaseBulkAction[AppConfigFragmentScopeTarget]): + """Create many fragments with per-item partial success; each write is authorized at its + scope, and the FK to the allow-list gates each write.""" creator_specs: Sequence[AppConfigFragmentCreatorSpec] + @override + @classmethod + def entity_type(cls) -> EntityType: + return EntityType.APP_CONFIG_FRAGMENT + + @override + def entity_id(self) -> str | None: + return None + @override @classmethod def operation_type(cls) -> ActionOperationType: return ActionOperationType.CREATE @override - def targets(self) -> Sequence[AppConfigFragmentBulkTarget]: - # Fragments do not exist yet, so there are no per-entity targets to validate. - return [] + def targets(self) -> Sequence[AppConfigFragmentScopeTarget]: + # Each new fragment is written at its spec's scope, so RBAC authorizes the batch per + # scope (a public spec resolves to a global target: superadmin-only). + return [ + AppConfigFragmentScopeTarget(scope_type=spec.scope_type, scope_id=spec.scope_id) + for spec in self.creator_specs + ] @dataclass diff --git a/src/ai/backend/manager/services/app_config_fragment/processors.py b/src/ai/backend/manager/services/app_config_fragment/processors.py index c5d91eecda4..181479ef542 100644 --- a/src/ai/backend/manager/services/app_config_fragment/processors.py +++ b/src/ai/backend/manager/services/app_config_fragment/processors.py @@ -94,7 +94,14 @@ def __init__( self.purge = SingleEntityActionProcessor( service.purge, action_monitors, validators=[validators.rbac.single_entity] ) - self.bulk_create = BulkActionProcessor(service.bulk_create, monitors=action_monitors) + # bulk_create writes into scopes (not existing entities), so it uses the bulk-scope + # validator (subject = the fragment entity type) rather than the entity-target one. + bulk_scope = validators.rbac.bulk_scope + self.bulk_create = BulkActionProcessor( + service.bulk_create, + monitors=action_monitors, + validators=[bulk_scope] if bulk_scope is not None else None, + ) self.bulk_update = BulkActionProcessor( service.bulk_update, monitors=action_monitors, validators=[validators.rbac.bulk] ) diff --git a/tests/unit/manager/actions/validators/test_app_config_fragment_create_authorization.py b/tests/unit/manager/actions/validators/test_app_config_fragment_create_authorization.py index d638694ddab..f16c8de8b2f 100644 --- a/tests/unit/manager/actions/validators/test_app_config_fragment_create_authorization.py +++ b/tests/unit/manager/actions/validators/test_app_config_fragment_create_authorization.py @@ -30,6 +30,7 @@ from ai.backend.common.data.user.types import UserData, UserRole from ai.backend.common.identifier.user import UserID from ai.backend.manager.actions.action.base import BaseActionTriggerMeta +from ai.backend.manager.actions.validators.rbac.bulk_scope import BulkScopeActionRBACValidator from ai.backend.manager.actions.validators.rbac.scope import ScopeActionRBACValidator from ai.backend.manager.data.user.types import UserStatus from ai.backend.manager.errors.permission import NotEnoughPermission @@ -60,6 +61,9 @@ from ai.backend.manager.repositories.permission_controller.repository import ( PermissionControllerRepository, ) +from ai.backend.manager.services.app_config_fragment.actions.bulk_create import ( + BulkCreateAppConfigFragmentAction, +) from ai.backend.manager.services.app_config_fragment.actions.create import ( CreateAppConfigFragmentAction, ) @@ -178,6 +182,25 @@ def scope_validator(repository: PermissionControllerRepository) -> ScopeActionRB return ScopeActionRBACValidator(repository, MagicMock()) +@pytest.fixture +def bulk_scope_validator( + repository: PermissionControllerRepository, +) -> BulkScopeActionRBACValidator: + # MagicMock config_provider → enforcement_enabled is truthy, so the check runs. + return BulkScopeActionRBACValidator(repository, MagicMock()) + + +def _bulk_create(*scopes: tuple[AppConfigScopeType, str]) -> BulkCreateAppConfigFragmentAction: + return BulkCreateAppConfigFragmentAction( + creator_specs=[ + AppConfigFragmentCreatorSpec( + config_name="cfg", scope_type=scope_type, scope_id=scope_id, config={} + ) + for scope_type, scope_id in scopes + ] + ) + + @pytest.fixture async def owner_user(db_with_rbac_tables: ExtendedAsyncSAEngine) -> UserData: """A non-superadmin granted APP_CONFIG_FRAGMENT:CREATE on their own user scope.""" @@ -264,3 +287,57 @@ async def test_superadmin_is_allowed( ) with with_user(_user_data(UserID(uuid.uuid4()), is_superadmin=True)): await scope_validator.validate(action, trigger_meta) + + +class TestBulkCreateAuthorization: + """Bulk create authorizes each new fragment at its scope via the bulk-scope validator.""" + + async def test_own_user_scope_is_allowed( + self, + bulk_scope_validator: BulkScopeActionRBACValidator, + trigger_meta: BaseActionTriggerMeta, + owner_user: UserData, + ) -> None: + action = _bulk_create((AppConfigScopeType.USER, str(owner_user.user_id))) + with with_user(owner_user): + result = await bulk_scope_validator.validate(action, trigger_meta) + assert not result.denied_entities + + async def test_another_user_scope_is_denied( + self, + bulk_scope_validator: BulkScopeActionRBACValidator, + trigger_meta: BaseActionTriggerMeta, + owner_user: UserData, + ) -> None: + # One spec in the owner's scope, one in another user's scope: only the latter is denied. + action = _bulk_create( + (AppConfigScopeType.USER, str(owner_user.user_id)), + (AppConfigScopeType.USER, str(UserID(uuid.uuid4()))), + ) + with with_user(owner_user): + result = await bulk_scope_validator.validate(action, trigger_meta) + assert len(result.denied_entities) == 1 + + async def test_public_scope_is_denied_for_non_superadmin( + self, + bulk_scope_validator: BulkScopeActionRBACValidator, + trigger_meta: BaseActionTriggerMeta, + owner_user: UserData, + ) -> None: + action = _bulk_create((AppConfigScopeType.PUBLIC, "")) + with with_user(owner_user): + result = await bulk_scope_validator.validate(action, trigger_meta) + assert len(result.denied_entities) == 1 + + async def test_superadmin_is_allowed( + self, + bulk_scope_validator: BulkScopeActionRBACValidator, + trigger_meta: BaseActionTriggerMeta, + ) -> None: + action = _bulk_create( + (AppConfigScopeType.USER, str(UserID(uuid.uuid4()))), + (AppConfigScopeType.PUBLIC, ""), + ) + with with_user(_user_data(UserID(uuid.uuid4()), is_superadmin=True)): + result = await bulk_scope_validator.validate(action, trigger_meta) + assert not result.denied_entities From a9fc178e6d82de2d67bd6864b640d14b16b18a97 Mon Sep 17 00:00:00 2001 From: Gyubong Date: Tue, 14 Jul 2026 14:34:29 +0900 Subject: [PATCH 13/13] refactor(BA-6859): purge fragment via dedicated purger spec and clean up scope permissions Purge now takes an AppConfigFragmentPurgerSpec and removes the fragment row, its RBAC scope associations, and the permissions granted at its own scope together. Refine the backfill migration docstring to clarify that only permissions rows are backfilled (scope associations are per-fragment and written at creation time). Co-Authored-By: Claude Opus 4.8 (1M context) --- ...dd_app_config_fragment_permissions_to_rbac.py | 13 +++++++++---- .../app_config_fragment/db_source/db_source.py | 16 +++++++++------- .../app_config_fragment/repository.py | 7 +++++-- .../app_config_fragment/actions/purge.py | 12 ++++++------ .../services/app_config_fragment/service.py | 2 +- .../app_config_fragment/test_repository.py | 9 ++++++--- .../services/app_config_fragment/test_service.py | 9 ++++++--- 7 files changed, 42 insertions(+), 26 deletions(-) diff --git a/src/ai/backend/manager/models/alembic/versions/b3d9f7a2c184_add_app_config_fragment_permissions_to_rbac.py b/src/ai/backend/manager/models/alembic/versions/b3d9f7a2c184_add_app_config_fragment_permissions_to_rbac.py index 5dbd0de5d8e..07c64404039 100644 --- a/src/ai/backend/manager/models/alembic/versions/b3d9f7a2c184_add_app_config_fragment_permissions_to_rbac.py +++ b/src/ai/backend/manager/models/alembic/versions/b3d9f7a2c184_add_app_config_fragment_permissions_to_rbac.py @@ -1,9 +1,14 @@ """add_app_config_fragment_permissions_to_rbac -Backfill ``APP_CONFIG_FRAGMENT`` permissions onto existing RBAC roles so that already -provisioned user / domain / project roles can exercise fragment writes once RBAC -enforcement is enabled (BEP-1052). New roles receive these permissions at creation via the -runtime allow list; this migration covers roles that predate the allow-list change. +Backfill ``APP_CONFIG_FRAGMENT`` permissions onto existing write-capable RBAC roles (a +user's own ``user``-scope role and ``domain`` admins) so they can exercise fragment writes +once RBAC enforcement is enabled (BEP-1052). New roles receive these permissions at role +creation; this migration only covers roles that predate the new entity type. + +Only ``permissions`` rows are backfilled. The RBAC scope *associations* +(``association_scopes_entities``, which bind a fragment to its owning scope) are NOT +migrated: they are per-fragment and written at fragment-creation time, and this feature is +still unreleased, so no fragments — and therefore no associations — exist yet. Revision ID: b3d9f7a2c184 Revises: a3c1d8e5b294 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 4c729b2d529..fa5a43d150d 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 @@ -3,7 +3,6 @@ from __future__ import annotations from collections.abc import Sequence -from typing import cast import sqlalchemy as sa @@ -111,17 +110,20 @@ async def update(self, updater: Updater[AppConfigFragmentRow]) -> AppConfigFragm return result.row.to_data() @app_config_fragment_db_source_resilience.apply() - async def purge(self, purger: Purger[AppConfigFragmentRow]) -> AppConfigFragmentData: - fragment_id = cast(AppConfigFragmentID, purger.pk_value) + async def purge(self, purger_spec: AppConfigFragmentPurgerSpec) -> AppConfigFragmentData: + # Purge is an RBAC unbind: the fragment row, its scope associations, and any + # permissions granted at its own scope are deleted together (entity-as-scope). rbac_purger = RBACEntityPurger( - row_class=purger.row_class, - pk_value=purger.pk_value, - spec=AppConfigFragmentPurgerSpec(fragment_id=fragment_id), + row_class=AppConfigFragmentRow, + pk_value=purger_spec.fragment_id, + spec=purger_spec, ) async with self._rbac_ops_provider.write_ops() as w: result = await w.purge_scoped(rbac_purger) if result is None: - raise AppConfigFragmentNotFound(f"App config fragment {purger.pk_value} not found") + raise AppConfigFragmentNotFound( + f"App config fragment {purger_spec.fragment_id} not found" + ) return result.row.to_data() @app_config_fragment_db_source_resilience.apply() 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 be078bb1693..831ce39c496 100644 --- a/src/ai/backend/manager/repositories/app_config_fragment/repository.py +++ b/src/ai/backend/manager/repositories/app_config_fragment/repository.py @@ -21,6 +21,9 @@ from ai.backend.manager.repositories.app_config_fragment.db_source import ( AppConfigFragmentDBSource, ) +from ai.backend.manager.repositories.app_config_fragment.purgers import ( + AppConfigFragmentPurgerSpec, +) from ai.backend.manager.repositories.app_config_fragment.types import ( AppConfigScopeArguments, ) @@ -75,8 +78,8 @@ async def update(self, updater: Updater[AppConfigFragmentRow]) -> AppConfigFragm return await self._db_source.update(updater) @app_config_fragment_repository_resilience.apply() - async def purge(self, purger: Purger[AppConfigFragmentRow]) -> AppConfigFragmentData: - return await self._db_source.purge(purger) + async def purge(self, purger_spec: AppConfigFragmentPurgerSpec) -> AppConfigFragmentData: + return await self._db_source.purge(purger_spec) @app_config_fragment_repository_resilience.apply() async def admin_search(self, querier: BatchQuerier) -> AppConfigFragmentSearchResult: diff --git a/src/ai/backend/manager/services/app_config_fragment/actions/purge.py b/src/ai/backend/manager/services/app_config_fragment/actions/purge.py index f837ab2a9a2..e64a0077f4c 100644 --- a/src/ai/backend/manager/services/app_config_fragment/actions/purge.py +++ b/src/ai/backend/manager/services/app_config_fragment/actions/purge.py @@ -3,12 +3,12 @@ from dataclasses import dataclass from typing import override -from ai.backend.common.data.permission.types import RBACElementType from ai.backend.manager.actions.types import ActionOperationType from ai.backend.manager.data.app_config_fragment.types import AppConfigFragmentData 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 import Purger +from ai.backend.manager.repositories.app_config_fragment.purgers import ( + AppConfigFragmentPurgerSpec, +) from ai.backend.manager.services.app_config_fragment.actions.base import ( AppConfigFragmentSingleEntityAction, AppConfigFragmentSingleEntityActionResult, @@ -25,7 +25,7 @@ class PurgeAppConfigFragmentAction(AppConfigFragmentSingleEntityAction): entry itself cascades to its fragments without going through this action. """ - purger: Purger[AppConfigFragmentRow] + purger_spec: AppConfigFragmentPurgerSpec @override @classmethod @@ -34,11 +34,11 @@ def operation_type(cls) -> ActionOperationType: @override def target_entity_id(self) -> str: - return str(self.purger.pk_value) + return str(self.purger_spec.fragment_id) @override def target_element(self) -> RBACElementRef: - return RBACElementRef(RBACElementType.APP_CONFIG_FRAGMENT, str(self.purger.pk_value)) + return self.purger_spec.entity_ref() @dataclass 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 63dc8f0cb6d..44ec8b3c789 100644 --- a/src/ai/backend/manager/services/app_config_fragment/service.py +++ b/src/ai/backend/manager/services/app_config_fragment/service.py @@ -103,7 +103,7 @@ async def update( async def purge( self, action: PurgeAppConfigFragmentAction ) -> PurgeAppConfigFragmentActionResult: - data = await self._repository.purge(action.purger) + data = await self._repository.purge(action.purger_spec) return PurgeAppConfigFragmentActionResult(fragment=data) async def bulk_create( 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 41f43fea320..1d9697338e6 100644 --- a/tests/unit/manager/repositories/app_config_fragment/test_repository.py +++ b/tests/unit/manager/repositories/app_config_fragment/test_repository.py @@ -34,6 +34,9 @@ from ai.backend.manager.repositories.app_config_fragment.creators import ( AppConfigFragmentCreatorSpec, ) +from ai.backend.manager.repositories.app_config_fragment.purgers import ( + AppConfigFragmentPurgerSpec, +) from ai.backend.manager.repositories.app_config_fragment.repository import ( AppConfigFragmentRepository, ) @@ -285,7 +288,7 @@ async def test_purge_removes_row( domain_scoped_fragment: AppConfigFragmentData, ) -> None: purged = await repository.purge( - Purger(row_class=AppConfigFragmentRow, pk_value=domain_scoped_fragment.id), + AppConfigFragmentPurgerSpec(fragment_id=domain_scoped_fragment.id), ) assert purged.id == domain_scoped_fragment.id with pytest.raises(AppConfigFragmentNotFound): @@ -295,7 +298,7 @@ async def test_purge_missing_raises(self, repository: AppConfigFragmentRepositor missing_id = AppConfigFragmentID(uuid.uuid4()) with pytest.raises(AppConfigFragmentNotFound): await repository.purge( - Purger(row_class=AppConfigFragmentRow, pk_value=missing_id), + AppConfigFragmentPurgerSpec(fragment_id=missing_id), ) @@ -775,5 +778,5 @@ async def test_purge_removes_the_scope_binding( ) -> 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)) + await repository.purge(AppConfigFragmentPurgerSpec(fragment_id=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 726cfbf2158..e1d0feab511 100644 --- a/tests/unit/manager/services/app_config_fragment/test_service.py +++ b/tests/unit/manager/services/app_config_fragment/test_service.py @@ -24,6 +24,9 @@ from ai.backend.manager.repositories.app_config_fragment.creators import ( AppConfigFragmentCreatorSpec, ) +from ai.backend.manager.repositories.app_config_fragment.purgers import ( + AppConfigFragmentPurgerSpec, +) from ai.backend.manager.repositories.app_config_fragment.repository import ( AppConfigFragmentRepository, ) @@ -221,12 +224,12 @@ async def test_purge_delegates_to_repository( ) -> None: fragment = _fragment() mock_repository.purge = AsyncMock(return_value=fragment) - purger = Purger(row_class=AppConfigFragmentRow, pk_value=fragment.id) + purger_spec = AppConfigFragmentPurgerSpec(fragment_id=fragment.id) - result = await service.purge(PurgeAppConfigFragmentAction(purger=purger)) + result = await service.purge(PurgeAppConfigFragmentAction(purger_spec=purger_spec)) assert result.fragment == fragment - mock_repository.purge.assert_called_once_with(purger) + mock_repository.purge.assert_called_once_with(purger_spec) # --- bulk ---