diff --git a/diracx-db/src/diracx/db/sql/sandbox_metadata/db.py b/diracx-db/src/diracx/db/sql/sandbox_metadata/db.py index 6e106eef2..0d955be06 100644 --- a/diracx-db/src/diracx/db/sql/sandbox_metadata/db.py +++ b/diracx-db/src/diracx/db/sql/sandbox_metadata/db.py @@ -30,6 +30,14 @@ async def get_owner_id(self, user: UserInfo) -> int | None: ) return (await self.conn.execute(stmt)).scalar_one_or_none() + async def get_sandbox_owner_id(self, pfn: str) -> int | None: + """Get the id of the owner of a sandbox.""" + stmt = select(SBOwners.OwnerID).where( + SBOwners.OwnerID == SandBoxes.OwnerId, + SandBoxes.SEPFN == pfn, + ) + return (await self.conn.execute(stmt)).scalar_one_or_none() + async def insert_owner(self, user: UserInfo) -> int: stmt = insert(SBOwners).values( Owner=user.preferred_username, diff --git a/diracx-db/tests/jobs/test_sandbox_metadata.py b/diracx-db/tests/jobs/test_sandbox_metadata.py index 45ff3d9a6..c8937a896 100644 --- a/diracx-db/tests/jobs/test_sandbox_metadata.py +++ b/diracx-db/tests/jobs/test_sandbox_metadata.py @@ -186,3 +186,25 @@ async def test_assign_and_unsassign_sandbox_to_jobs( res = await sandbox_metadata_db.conn.execute(stmt) res_sb_id = [row.SBId for row in res] assert sb_id_1 not in res_sb_id + + +async def test_get_sandbox_owner_id(sandbox_metadata_db: SandboxMetadataDB): + user_info = UserInfo( + sub="vo:sub", preferred_username="user1", dirac_group="group1", vo="vo" + ) + pfn = secrets.token_hex() + sandbox_se = "SandboxSE" + # Insert the sandbox + async with sandbox_metadata_db: + owner_id = await sandbox_metadata_db.insert_owner(user_info) + await sandbox_metadata_db.insert_sandbox(owner_id, sandbox_se, pfn, 100) + + async with sandbox_metadata_db: + sb_owner_id = await sandbox_metadata_db.get_sandbox_owner_id(pfn) + + assert owner_id == 1 + assert sb_owner_id == owner_id + + async with sandbox_metadata_db: + sb_owner_id = await sandbox_metadata_db.get_sandbox_owner_id("not_found") + assert sb_owner_id is None diff --git a/diracx-routers/src/diracx/routers/jobs/access_policies.py b/diracx-routers/src/diracx/routers/jobs/access_policies.py index 3fe2330b0..461fab2ca 100644 --- a/diracx-routers/src/diracx/routers/jobs/access_policies.py +++ b/diracx-routers/src/diracx/routers/jobs/access_policies.py @@ -7,7 +7,7 @@ from fastapi import Depends, HTTPException, status from diracx.core.properties import JOB_ADMINISTRATOR, NORMAL_USER -from diracx.db.sql import JobDB +from diracx.db.sql import JobDB, SandboxMetadataDB from diracx.routers.access_policies import BaseAccessPolicy from diracx.routers.utils.users import AuthorizedUserInfo @@ -108,10 +108,12 @@ async def policy( /, *, action: ActionType | None = None, + sandbox_metadata_db: SandboxMetadataDB | None = None, pfns: list[str] | None = None, required_prefix: str | None = None, ): assert action, "action is a mandatory parameter" + assert sandbox_metadata_db, "sandbox_metadata_db is a mandatory parameter" if action == ActionType.CREATE: @@ -137,6 +139,14 @@ async def policy( status_code=status.HTTP_403_FORBIDDEN, detail=f"Invalid PFN. PFN must start with {required_prefix}", ) + # Checking if the user owns the sandbox + owner_id = await sandbox_metadata_db.get_owner_id(user_info) + sandbox_owner_id = await sandbox_metadata_db.get_sandbox_owner_id(pfn) + if not owner_id or owner_id != sandbox_owner_id: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail=f"{user_info.preferred_username} is not the owner of the sandbox", + ) CheckSandboxPolicyCallable = Annotated[Callable, Depends(SandboxAccessPolicy.check)] diff --git a/diracx-routers/src/diracx/routers/jobs/sandboxes.py b/diracx-routers/src/diracx/routers/jobs/sandboxes.py index 09b2a6d93..7f063c5e5 100644 --- a/diracx-routers/src/diracx/routers/jobs/sandboxes.py +++ b/diracx-routers/src/diracx/routers/jobs/sandboxes.py @@ -55,7 +55,9 @@ async def initiate_sandbox_upload( If the sandbox does not exist in the database then the "url" and "fields" should be used to upload the sandbox to the storage backend. """ - await check_permissions(action=ActionType.CREATE) + await check_permissions( + action=ActionType.CREATE, sandbox_metadata_db=sandbox_metadata_db + ) try: sandbox_upload_response = await initiate_sandbox_upload_bl( @@ -73,6 +75,7 @@ async def initiate_sandbox_upload( async def get_sandbox_file( pfn: Annotated[str, Query(max_length=256, pattern=SANDBOX_PFN_REGEX)], settings: SandboxStoreSettings, + sandbox_metadata_db: SandboxMetadataDB, user_info: Annotated[AuthorizedUserInfo, Depends(verify_dirac_access_token)], check_permissions: CheckSandboxPolicyCallable, ) -> SandboxDownloadResponse: @@ -92,6 +95,7 @@ async def get_sandbox_file( ) await check_permissions( action=ActionType.READ, + sandbox_metadata_db=sandbox_metadata_db, pfns=[short_pfn], required_prefix=required_prefix, ) diff --git a/diracx-routers/tests/jobs/test_wms_access_policy.py b/diracx-routers/tests/jobs/test_wms_access_policy.py index 9852d7887..857642046 100644 --- a/diracx-routers/tests/jobs/test_wms_access_policy.py +++ b/diracx-routers/tests/jobs/test_wms_access_policy.py @@ -23,13 +23,23 @@ } -class FakeDB: +class FakeJobDB: async def summary(self, *args): ... +class FakeSBMetadataDB: + async def get_owner_id(self, *args): ... + async def get_sandbox_owner_id(self, *args): ... + + @pytest.fixture def job_db(): - yield FakeDB() + yield FakeJobDB() + + +@pytest.fixture +def sandbox_metadata_db(): + yield FakeSBMetadataDB() WMS_POLICY_NAME = "WMSAccessPolicy_AlthoughItDoesNotMatter" @@ -220,7 +230,7 @@ async def summary_other_vo(*args): ) -async def test_sandbox_access_policy_create(): +async def test_sandbox_access_policy_create(sandbox_metadata_db): admin_user = AuthorizedUserInfo(properties=[JOB_ADMINISTRATOR], **base_payload) normal_user = AuthorizedUserInfo(properties=[NORMAL_USER], **base_payload) @@ -230,6 +240,7 @@ async def test_sandbox_access_policy_create(): await SandboxAccessPolicy.policy( SANDBOX_POLICY_NAME, normal_user, + sandbox_metadata_db=sandbox_metadata_db, ) # An admin cannot create any resource @@ -238,6 +249,7 @@ async def test_sandbox_access_policy_create(): SANDBOX_POLICY_NAME, admin_user, action=ActionType.CREATE, + sandbox_metadata_db=sandbox_metadata_db, pfns=[USER_SANDBOX_PFN], ) @@ -246,13 +258,14 @@ async def test_sandbox_access_policy_create(): SANDBOX_POLICY_NAME, normal_user, action=ActionType.CREATE, + sandbox_metadata_db=sandbox_metadata_db, pfns=[USER_SANDBOX_PFN], ) ############## -async def test_sandbox_access_policy_read(): +async def test_sandbox_access_policy_read(sandbox_metadata_db, monkeypatch): admin_user = AuthorizedUserInfo(properties=[JOB_ADMINISTRATOR], **base_payload) normal_user = AuthorizedUserInfo(properties=[NORMAL_USER], **base_payload) @@ -261,6 +274,7 @@ async def test_sandbox_access_policy_read(): SANDBOX_POLICY_NAME, admin_user, action=ActionType.READ, + sandbox_metadata_db=sandbox_metadata_db, pfns=[USER_SANDBOX_PFN], required_prefix=SANDBOX_PREFIX, ) @@ -269,6 +283,7 @@ async def test_sandbox_access_policy_read(): SANDBOX_POLICY_NAME, admin_user, action=ActionType.READ, + sandbox_metadata_db=sandbox_metadata_db, pfns=[OTHER_USER_SANDBOX_PFN], required_prefix=SANDBOX_PREFIX, ) @@ -279,24 +294,43 @@ async def test_sandbox_access_policy_read(): SANDBOX_POLICY_NAME, normal_user, action=ActionType.READ, + sandbox_metadata_db=sandbox_metadata_db, pfns=[USER_SANDBOX_PFN], ) # User can act on his own sandbox + async def get_owner_id(*args): + return 1 + + async def get_sandbox_owner_id(*args): + return 1 + + monkeypatch.setattr(sandbox_metadata_db, "get_owner_id", get_owner_id) + monkeypatch.setattr( + sandbox_metadata_db, "get_sandbox_owner_id", get_sandbox_owner_id + ) + await SandboxAccessPolicy.policy( SANDBOX_POLICY_NAME, normal_user, action=ActionType.READ, + sandbox_metadata_db=sandbox_metadata_db, pfns=[USER_SANDBOX_PFN], required_prefix=SANDBOX_PREFIX, ) # User cannot act on others + async def get_owner_id(*args): + return 2 + + monkeypatch.setattr(sandbox_metadata_db, "get_owner_id", get_owner_id) + with pytest.raises(HTTPException): await SandboxAccessPolicy.policy( SANDBOX_POLICY_NAME, normal_user, action=ActionType.READ, + sandbox_metadata_db=sandbox_metadata_db, pfns=[OTHER_USER_SANDBOX_PFN], required_prefix=SANDBOX_PREFIX, )