Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changes/10059.feature.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Apply RBAC validators to Image service actions for proper authorization checks
21 changes: 21 additions & 0 deletions src/ai/backend/manager/services/image/actions/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@

from ai.backend.common.data.permission.types import EntityType
from ai.backend.manager.actions.action import BaseAction, BaseBatchAction
from ai.backend.manager.actions.action.single_entity import (
BaseSingleEntityAction,
BaseSingleEntityActionResult,
)
from ai.backend.manager.actions.action.types import FieldData


@dataclass
Expand All @@ -19,3 +24,19 @@ class ImageBatchAction(BaseBatchAction):
@classmethod
def entity_type(cls) -> EntityType:
return EntityType.IMAGE


@dataclass
class ImageSingleEntityAction(BaseSingleEntityAction):
@override
@classmethod
def entity_type(cls) -> EntityType:
return EntityType.IMAGE

@override
def field_data(self) -> FieldData | None:
return None


class ImageSingleEntityActionResult(BaseSingleEntityActionResult):
Comment thread
fregataa marked this conversation as resolved.
pass
26 changes: 18 additions & 8 deletions src/ai/backend/manager/services/image/actions/forget_image.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,17 @@
from dataclasses import dataclass
from typing import override

from ai.backend.common.data.permission.types import RBACElementType
from ai.backend.common.types import ImageID
from ai.backend.manager.actions.action import BaseActionResult
from ai.backend.manager.actions.types import ActionOperationType
from ai.backend.manager.data.image.types import ImageData
from ai.backend.manager.services.image.actions.base import ImageAction
from ai.backend.manager.data.permission.types import RBACElementRef
from ai.backend.manager.services.image.actions.base import (
ImageAction,
ImageSingleEntityAction,
ImageSingleEntityActionResult,
)


@dataclass
Expand Down Expand Up @@ -37,23 +43,27 @@ def entity_id(self) -> str | None:


@dataclass
class ForgetImageByIdAction(ImageAction):
class ForgetImageByIdAction(ImageSingleEntityAction):
image_id: ImageID

@override
def entity_id(self) -> str | None:
return str(self.image_id)

@override
@classmethod
def operation_type(cls) -> ActionOperationType:
return ActionOperationType.DELETE

@override
def target_entity_id(self) -> str:
return str(self.image_id)

@override
def target_element(self) -> RBACElementRef:
return RBACElementRef(RBACElementType.IMAGE, str(self.image_id))


@dataclass
class ForgetImageByIdActionResult(BaseActionResult):
class ForgetImageByIdActionResult(ImageSingleEntityActionResult):
image: ImageData

@override
def entity_id(self) -> str | None:
def target_entity_id(self) -> str:
return str(self.image.id)
26 changes: 18 additions & 8 deletions src/ai/backend/manager/services/image/actions/purge_images.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,17 @@
from dataclasses import dataclass
from typing import override

from ai.backend.common.data.permission.types import RBACElementType
from ai.backend.common.types import AgentId, ImageID
from ai.backend.manager.actions.action import BaseActionResult
from ai.backend.manager.actions.types import ActionOperationType
from ai.backend.manager.data.image.types import ImageData
from ai.backend.manager.services.image.actions.base import ImageAction
from ai.backend.manager.data.permission.types import RBACElementRef
from ai.backend.manager.services.image.actions.base import (
ImageAction,
ImageSingleEntityAction,
ImageSingleEntityActionResult,
)
from ai.backend.manager.services.image.types import ImageRefData


Expand Down Expand Up @@ -78,23 +84,27 @@ def entity_id(self) -> str | None:


@dataclass
class PurgeImageByIdAction(ImageAction):
class PurgeImageByIdAction(ImageSingleEntityAction):
image_id: ImageID

@override
def entity_id(self) -> str | None:
return str(self.image_id)

@override
@classmethod
def operation_type(cls) -> ActionOperationType:
return ActionOperationType.PURGE

@override
def target_entity_id(self) -> str:
return str(self.image_id)

@override
def target_element(self) -> RBACElementRef:
return RBACElementRef(RBACElementType.IMAGE, str(self.image_id))


@dataclass
class PurgeImageByIdActionResult(BaseActionResult):
class PurgeImageByIdActionResult(ImageSingleEntityActionResult):
image: ImageData

@override
def entity_id(self) -> str | None:
def target_entity_id(self) -> str:
return str(self.image.id)
25 changes: 19 additions & 6 deletions src/ai/backend/manager/services/image/processors.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from ai.backend.manager.actions.monitors.monitor import ActionMonitor
from ai.backend.manager.actions.processor import ActionProcessor
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.services.image.actions.alias_image import (
Expand Down Expand Up @@ -92,8 +93,10 @@

class ImageProcessors(AbstractProcessorPackage):
forget_image: ActionProcessor[ForgetImageAction, ForgetImageActionResult]
forget_image_by_id: ActionProcessor[ForgetImageByIdAction, ForgetImageByIdActionResult]
purge_image_by_id: ActionProcessor[PurgeImageByIdAction, PurgeImageByIdActionResult]
forget_image_by_id: SingleEntityActionProcessor[
ForgetImageByIdAction, ForgetImageByIdActionResult
]
purge_image_by_id: SingleEntityActionProcessor[PurgeImageByIdAction, PurgeImageByIdActionResult]
alias_image: ActionProcessor[AliasImageAction, AliasImageActionResult]
alias_image_by_id: ActionProcessor[AliasImageByIdAction, AliasImageByIdActionResult]
Comment thread
fregataa marked this conversation as resolved.
dealias_image: ActionProcessor[DealiasImageAction, DealiasImageActionResult]
Expand Down Expand Up @@ -138,10 +141,10 @@ def __init__(
action_monitors: list[ActionMonitor],
validators: ActionValidators,
) -> None:
# Actions without RBAC validation (internal/system or special entity types)
self.get_image_installed_agents = ActionProcessor(
service.get_image_installed_agents, action_monitors
)
self.get_all_images = ActionProcessor(service.get_all_images, action_monitors)
self.get_images_by_canonicals = ActionProcessor(
service.get_images_by_canonicals, action_monitors
)
Expand All @@ -150,8 +153,19 @@ def __init__(
)
self.get_image_by_id = ActionProcessor(service.get_image_by_id, action_monitors)
self.forget_image = ActionProcessor(service.forget_image, action_monitors)
self.forget_image_by_id = ActionProcessor(service.forget_image_by_id, action_monitors)
self.purge_image_by_id = ActionProcessor(service.purge_image_by_id, action_monitors)

self.get_all_images = ActionProcessor(service.get_all_images, action_monitors)
self.search_images = ActionProcessor(service.search_images, action_monitors)

# Single entity actions with RBAC validation
self.forget_image_by_id = SingleEntityActionProcessor(
service.forget_image_by_id, action_monitors, validators=[validators.rbac.single_entity]
)
self.purge_image_by_id = SingleEntityActionProcessor(
service.purge_image_by_id, action_monitors, validators=[validators.rbac.single_entity]
)
Comment thread
fregataa marked this conversation as resolved.
# Superadmin-only mutations — access is enforced by check_admin_only at the API layer,
# so per-entity RBAC validation is not required here.
self.alias_image = ActionProcessor(service.alias_image, action_monitors)
self.alias_image_by_id = ActionProcessor(service.alias_image_by_id, action_monitors)
self.dealias_image = ActionProcessor(service.dealias_image, action_monitors)
Expand All @@ -173,7 +187,6 @@ def __init__(
self.set_image_resource_limit_by_id = ActionProcessor(
service.set_image_resource_limit_by_id, action_monitors
)
self.search_images = ActionProcessor(service.search_images, action_monitors)
self.search_aliases = ActionProcessor(service.search_aliases, action_monitors)
self.load_image_last_used = ActionProcessor(service.load_image_last_used, action_monitors)

Expand Down
16 changes: 13 additions & 3 deletions tests/component/image/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,19 @@

import uuid
from collections.abc import AsyncIterator, Callable
from unittest.mock import MagicMock
from unittest.mock import AsyncMock, MagicMock

import pytest
import sqlalchemy as sa
from sqlalchemy.ext.asyncio.engine import AsyncEngine as SAEngine

from ai.backend.common.container_registry import ContainerRegistryType
from ai.backend.manager.actions.validators import ActionValidators
from ai.backend.manager.actions.validators.rbac import RBACValidators
from ai.backend.manager.actions.validators.rbac.scope import ScopeActionRBACValidator
from ai.backend.manager.actions.validators.rbac.single_entity import (
SingleEntityActionRBACValidator,
)
from ai.backend.manager.api.rest.admin.handler import AdminHandler
from ai.backend.manager.api.rest.admin.registry import register_admin_routes
from ai.backend.manager.api.rest.image.handler import ImageHandler
Expand Down Expand Up @@ -37,9 +42,14 @@ def image_processors(
) -> ImageProcessors:
repo = ImageRepository(database_engine, valkey_clients.image, config_provider)
service = ImageService(agent_registry, repo, config_provider)
return ImageProcessors(
service=service, action_monitors=[], validators=MagicMock(spec=ActionValidators)
mock_scope = MagicMock(spec=ScopeActionRBACValidator)
mock_scope.validate = AsyncMock()
mock_single_entity = MagicMock(spec=SingleEntityActionRBACValidator)
mock_single_entity.validate = AsyncMock()
validators = ActionValidators(
rbac=RBACValidators(scope=mock_scope, single_entity=mock_single_entity),
)
return ImageProcessors(service=service, action_monitors=[], validators=validators)


@pytest.fixture()
Expand Down
14 changes: 13 additions & 1 deletion tests/unit/manager/services/image/test_image_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@
from ai.backend.common.exception import UnknownImageReference
from ai.backend.common.types import AgentId, ImageCanonical, ImageID, SlotName
from ai.backend.manager.actions.validators import ActionValidators
from ai.backend.manager.actions.validators.rbac import RBACValidators
from ai.backend.manager.actions.validators.rbac.scope import ScopeActionRBACValidator
from ai.backend.manager.actions.validators.rbac.single_entity import (
SingleEntityActionRBACValidator,
)
from ai.backend.manager.data.container_registry.types import ContainerRegistryData
from ai.backend.manager.data.image.types import (
ImageAliasData,
Expand Down Expand Up @@ -119,7 +124,14 @@ def image_service(
@pytest.fixture
def processors(self, image_service: ImageService) -> ImageProcessors:
"""Create ImageProcessors with mock ImageService."""
return ImageProcessors(image_service, [], MagicMock(spec=ActionValidators))
mock_scope = MagicMock(spec=ScopeActionRBACValidator)
mock_scope.validate = AsyncMock()
mock_single_entity = MagicMock(spec=SingleEntityActionRBACValidator)
mock_single_entity.validate = AsyncMock()
validators = ActionValidators(
rbac=RBACValidators(scope=mock_scope, single_entity=mock_single_entity),
)
return ImageProcessors(image_service, [], validators)

@pytest.fixture
def container_registry_id(self) -> uuid.UUID:
Expand Down
Loading