From 1a40c8174ed9d38baf9a68ca263a30dd3863f01c Mon Sep 17 00:00:00 2001 From: Max Schettler Date: Thu, 28 May 2026 12:47:04 +0200 Subject: [PATCH] Prevent cross-cluster resource access Currently, authentication and resource retrieval is decoupled, allowing API clients to authenticate to one cluster and access resources of a different one. This change introduces checks that resources are accessed in their correct scope. --- simplyblock_web/api/v2/backup.py | 77 ++++++++++++++++++++++---- simplyblock_web/api/v2/device.py | 11 ++-- simplyblock_web/api/v2/migration.py | 27 ++++++--- simplyblock_web/api/v2/pool.py | 7 ++- simplyblock_web/api/v2/snapshot.py | 7 ++- simplyblock_web/api/v2/storage_node.py | 7 ++- simplyblock_web/api/v2/task.py | 4 +- simplyblock_web/api/v2/volume.py | 32 +++++++++-- 8 files changed, 137 insertions(+), 35 deletions(-) diff --git a/simplyblock_web/api/v2/backup.py b/simplyblock_web/api/v2/backup.py index 187bb7e38..e20682567 100644 --- a/simplyblock_web/api/v2/backup.py +++ b/simplyblock_web/api/v2/backup.py @@ -1,10 +1,14 @@ -from typing import List, Optional +from typing import Annotated, List, Optional +from uuid import UUID -from fastapi import APIRouter, HTTPException, Query, Response +from fastapi import APIRouter, Depends, HTTPException, Query, Response from pydantic import BaseModel from simplyblock_core.db_controller import DBController from simplyblock_core.controllers import backup_controller +from simplyblock_core.models.backup import BackupPolicy +from simplyblock_core.models.cluster import Cluster as ClusterModel +from simplyblock_core.models.lvol_model import LVol from .cluster import Cluster from .dtos import BackupDTO, BackupPolicyDTO @@ -97,9 +101,33 @@ def list_sources(cluster: Cluster): return sources -@api.delete('/{lvol_id}', name='clusters:backups:delete', status_code=204, responses={204: {"content": None}}) -def delete_backups(cluster: Cluster, lvol_id: str) -> Response: - success, error = backup_controller.delete_backups(lvol_id) +def _lookup_lvol_in_cluster(volume_id: str, cluster: ClusterModel) -> LVol: + try: + volume = db.get_lvol_by_id(volume_id) + pool = db.get_pool_by_id(volume.pool_uuid) + except KeyError as e: + raise HTTPException(404, str(e)) + if pool.cluster_id != cluster.get_id(): + raise HTTPException(404, f'LVol {volume_id} not found') + return volume + + +@api.delete( + '/{volume_id}', + name='clusters:backups:delete', + status_code=204, + responses={204: {"content": None}}, + deprecated=True, + summary='Deprecated — delete all backups for a volume', + description=( + 'Deprecated. Use ' + '`DELETE /clusters/{cluster_id}/storage-pools/{pool_id}/volumes/{volume_id}/backups` ' + 'instead.' + ), +) +def delete_backups(cluster: Cluster, volume_id: UUID) -> Response: + volume = _lookup_lvol_in_cluster(str(volume_id), cluster) + success, error = backup_controller.delete_backups(volume.get_id()) if error: raise HTTPException(400, error) return Response(status_code=204) @@ -135,9 +163,34 @@ def create_policy(cluster: Cluster, parameters: _PolicyCreateParams) -> Response return Response(status_code=201, headers={'X-Policy-Id': policy_id}) +def _lookup_backup_policy(policy_id: UUID, cluster: Cluster) -> BackupPolicy: + try: + policy = db.get_backup_policy_by_id(str(policy_id)) + except KeyError as e: + raise HTTPException(404, str(e)) + if policy.cluster_id != cluster.get_id(): + raise HTTPException(404, f'BackupPolicy {policy_id} not found') + return policy + + +Policy = Annotated[BackupPolicy, Depends(_lookup_backup_policy)] + + +def _validate_attachment_target(target_type: str, target_id: str, cluster: ClusterModel) -> None: + if target_type == "pool": + try: + pool = db.get_pool_by_id(target_id) + except KeyError as e: + raise HTTPException(404, str(e)) + if pool.cluster_id != cluster.get_id(): + raise HTTPException(404, f'Pool {target_id} not found') + elif target_type == "lvol": + _lookup_lvol_in_cluster(target_id, cluster) + + @policy_api.delete('/{policy_id}', name='clusters:backup-policies:delete', status_code=204, responses={204: {"content": None}}) -def delete_policy(cluster: Cluster, policy_id: str) -> Response: - success, error = backup_controller.remove_policy(policy_id) +def delete_policy(cluster: Cluster, policy: Policy) -> Response: + success, error = backup_controller.remove_policy(policy.uuid) if error: raise HTTPException(400, error) return Response(status_code=204) @@ -149,18 +202,20 @@ class _AttachParams(BaseModel): @policy_api.post('/{policy_id}/attach', name='clusters:backup-policies:attach', status_code=201) -def attach_policy(cluster: Cluster, policy_id: str, parameters: _AttachParams): +def attach_policy(cluster: Cluster, policy: Policy, parameters: _AttachParams): + _validate_attachment_target(parameters.target_type, parameters.target_id, cluster) att_id, error = backup_controller.attach_policy( - policy_id, parameters.target_type, parameters.target_id) + policy.uuid, parameters.target_type, parameters.target_id) if error: raise HTTPException(400, error) return {"attachment_id": att_id} @policy_api.post('/{policy_id}/detach', name='clusters:backup-policies:detach', status_code=204, responses={204: {"content": None}}) -def detach_policy(cluster: Cluster, policy_id: str, parameters: _AttachParams) -> Response: +def detach_policy(cluster: Cluster, policy: Policy, parameters: _AttachParams) -> Response: + _validate_attachment_target(parameters.target_type, parameters.target_id, cluster) success, error = backup_controller.detach_policy( - policy_id, parameters.target_type, parameters.target_id) + policy.uuid, parameters.target_type, parameters.target_id) if error: raise HTTPException(400, error) return Response(status_code=204) diff --git a/simplyblock_web/api/v2/device.py b/simplyblock_web/api/v2/device.py index 4635f4b6d..d4af9d5e2 100644 --- a/simplyblock_web/api/v2/device.py +++ b/simplyblock_web/api/v2/device.py @@ -31,10 +31,13 @@ def list(cluster: Cluster, storage_node: StorageNode) -> List[DeviceDTO]: def _lookup_device(storage_node: StorageNode, device_id: UUID) -> NVMeDevice: - try: - return db.get_storage_device_by_id(str(device_id)) - except KeyError as e: - raise HTTPException(404, str(e)) + device = next( + (d for d in storage_node.nvme_devices if d.get_id() == str(device_id)), + None, + ) + if device is None: + raise HTTPException(404, f'Device {device_id} not found') + return device Device = Annotated[NVMeDevice, Depends(_lookup_device)] diff --git a/simplyblock_web/api/v2/migration.py b/simplyblock_web/api/v2/migration.py index 591abd6f5..800e7c006 100644 --- a/simplyblock_web/api/v2/migration.py +++ b/simplyblock_web/api/v2/migration.py @@ -1,10 +1,10 @@ -from typing import List, TYPE_CHECKING +from typing import Annotated, List +from uuid import UUID -from fastapi import APIRouter, HTTPException +from fastapi import APIRouter, Depends, HTTPException from pydantic import BaseModel -if TYPE_CHECKING: - pass +from simplyblock_core.models.lvol_migration import LVolMigration from .cluster import Cluster from .dtos import MigrationDTO @@ -44,21 +44,30 @@ def start_migration(cluster: Cluster, parameters: _MigrateParams): instance_api = APIRouter(prefix='/{migration_id}') -@instance_api.get('/', name='clusters:migrations:detail') -def get_migration(cluster: Cluster, migration_id: str) -> MigrationDTO: +def _lookup_migration(migration_id: UUID, cluster: Cluster) -> LVolMigration: from simplyblock_core.db_controller import DBController db = DBController() try: - migration = db.get_migration_by_id(migration_id) + migration = db.get_migration_by_id(str(migration_id)) except KeyError as e: raise HTTPException(404, str(e)) + if migration.cluster_id != cluster.get_id(): + raise HTTPException(404, f'Migration {migration_id} not found') + return migration + + +Migration = Annotated[LVolMigration, Depends(_lookup_migration)] + + +@instance_api.get('/', name='clusters:migrations:detail') +def get_migration(cluster: Cluster, migration: Migration) -> MigrationDTO: return MigrationDTO.from_model(migration) @instance_api.post('/cancel', name='clusters:migrations:cancel', status_code=200) -def cancel_migration(cluster: Cluster, migration_id: str): +def cancel_migration(cluster: Cluster, migration: Migration): from simplyblock_core.controllers import migration_controller - ok, error = migration_controller.cancel_migration(migration_id) + ok, error = migration_controller.cancel_migration(migration.get_id()) if not ok: raise HTTPException(400, error) return {"status": "cancelled"} diff --git a/simplyblock_web/api/v2/pool.py b/simplyblock_web/api/v2/pool.py index 97044b997..0dc49aefc 100644 --- a/simplyblock_web/api/v2/pool.py +++ b/simplyblock_web/api/v2/pool.py @@ -65,11 +65,14 @@ def add(request: Request, cluster: Cluster, parameters: StoragePoolParams) -> Re instance_api = APIRouter(prefix='/{pool_id}') -def _lookup_storage_pool(pool_id: UUID) -> PoolModel: +def _lookup_storage_pool(pool_id: UUID, cluster: Cluster) -> PoolModel: try: - return db.get_pool_by_id(str(pool_id)) + pool = db.get_pool_by_id(str(pool_id)) except KeyError as e: raise HTTPException(404, str(e)) + if pool.cluster_id != cluster.get_id(): + raise HTTPException(404, f'Pool {pool_id} not found') + return pool StoragePool = Annotated[PoolModel, Depends(_lookup_storage_pool)] diff --git a/simplyblock_web/api/v2/snapshot.py b/simplyblock_web/api/v2/snapshot.py index 5ca931c7d..062926e22 100644 --- a/simplyblock_web/api/v2/snapshot.py +++ b/simplyblock_web/api/v2/snapshot.py @@ -28,11 +28,14 @@ def list(request: Request, cluster: Cluster, pool: StoragePool) -> List[Snapshot instance_api = APIRouter(prefix='/{snapshot_id}') -def _lookup_snapshot(snapshot_id: UUID) -> SnapshotModel: +def _lookup_snapshot(snapshot_id: UUID, pool: StoragePool) -> SnapshotModel: try: - return db.get_snapshot_by_id(str(snapshot_id)) + snapshot = db.get_snapshot_by_id(str(snapshot_id)) except KeyError as e: raise HTTPException(404, str(e)) + if snapshot.pool_uuid != pool.get_id(): + raise HTTPException(404, f'Snapshot {snapshot_id} not found') + return snapshot Snapshot = Annotated[SnapshotModel, Depends(_lookup_snapshot)] diff --git a/simplyblock_web/api/v2/storage_node.py b/simplyblock_web/api/v2/storage_node.py index 9ac495978..cb3148dab 100644 --- a/simplyblock_web/api/v2/storage_node.py +++ b/simplyblock_web/api/v2/storage_node.py @@ -90,11 +90,14 @@ def add(cluster: Cluster, parameters: StorageNodeParams): instance_api = APIRouter(prefix='/{storage_node_id}') -def _lookup_storage_node(storage_node_id: UUID) -> StorageNodeModel: +def _lookup_storage_node(storage_node_id: UUID, cluster: Cluster) -> StorageNodeModel: try: - return db.get_storage_node_by_id(str(storage_node_id)) + storage_node = db.get_storage_node_by_id(str(storage_node_id)) except KeyError as e: raise HTTPException(404, str(e)) + if storage_node.cluster_id != cluster.get_id(): + raise HTTPException(404, f'StorageNode {storage_node_id} not found') + return storage_node StorageNode = Annotated[StorageNodeModel, Depends(_lookup_storage_node)] diff --git a/simplyblock_web/api/v2/task.py b/simplyblock_web/api/v2/task.py index b6702181c..6d99f42d3 100644 --- a/simplyblock_web/api/v2/task.py +++ b/simplyblock_web/api/v2/task.py @@ -26,10 +26,12 @@ def list(cluster: Cluster) -> List[TaskDTO]: instance_api = APIRouter(prefix='/{task_id}') -def _lookup_task(task_id: UUID) -> JobSchedule: +def _lookup_task(task_id: UUID, cluster: Cluster) -> JobSchedule: task = db.get_task_by_id(str(task_id)) if task is None: raise HTTPException(404, 'Task does not exist') + if task.cluster_id != cluster.get_id(): + raise HTTPException(404, 'Task does not exist') return task diff --git a/simplyblock_web/api/v2/volume.py b/simplyblock_web/api/v2/volume.py index 8c36f6fd8..7454ad95b 100644 --- a/simplyblock_web/api/v2/volume.py +++ b/simplyblock_web/api/v2/volume.py @@ -6,12 +6,12 @@ from simplyblock_core.db_controller import DBController from simplyblock_core import utils as core_utils -from simplyblock_core.controllers import lvol_controller, snapshot_controller +from simplyblock_core.controllers import backup_controller, lvol_controller, snapshot_controller from simplyblock_core.models.lvol_model import LVol from .cluster import Cluster from .pool import StoragePool -from .dtos import VolumeDTO, SnapshotDTO, TaskDTO +from .dtos import BackupDTO, VolumeDTO, SnapshotDTO, TaskDTO from . import util @@ -124,11 +124,14 @@ def add( instance_api = APIRouter(prefix='/{volume_id}') -def _lookup_volume(volume_id: UUID) -> LVol: +def _lookup_volume(volume_id: UUID, pool: StoragePool) -> LVol: try: - return db.get_lvol_by_id(str(volume_id)) + volume = db.get_lvol_by_id(str(volume_id)) except KeyError as e: raise HTTPException(404, str(e)) + if volume.pool_uuid != pool.get_id(): + raise HTTPException(404, f'LVol {volume_id} not found') + return volume Volume = Annotated[LVol, Depends(_lookup_volume)] @@ -352,3 +355,24 @@ def clone( volume_id=clone_id, ) return Response(status_code=201, headers={'Location': str(entity_url)}) + + +@instance_api.get('/backups', name='clusters:storage-pools:volumes:backups:list') +def backups(volume: Volume) -> List[BackupDTO]: + rows = db.get_backups_by_lvol_id(volume.get_id()) + rows = sorted(rows, key=lambda b: (b.created_at, b.uuid), reverse=True) + return [BackupDTO.from_model(b) for b in rows] + + +@instance_api.delete( + '/backups', + name='clusters:storage-pools:volumes:backups:delete', + status_code=204, + responses={204: {"content": None}}, +) +def delete_backups(cluster: Cluster, pool: StoragePool, volume: Volume) -> Response: + success, error = backup_controller.delete_backups(volume.get_id()) + if error: + raise HTTPException(400, error) + return Response(status_code=204) +