Ensure sb_metadata_db is given

Author: natthan-pigoux

diracx-routers/src/diracx/routers/jobs/access_policies.py

@@ -113,6 +113,7 @@ async def policy(
         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:
 
@@ -132,25 +133,20 @@ async def policy(
                 raise NotImplementedError(
                     "required_prefix is None. This shouldn't happen"
                 )
-            assert sandbox_metadata_db, "sandbox_metadata_db is a mandatory parameter"
             for pfn in pfns:
                 if not pfn.startswith(required_prefix):
                     raise HTTPException(
                         status_code=status.HTTP_403_FORBIDDEN,
                         detail=f"Invalid PFN. PFN must start with {required_prefix}",
                     )
-                if action in [ActionType.READ, ActionType.QUERY]:
-                    # We are reading or querying
-                    # 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
+                # 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",
                     )
-                    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)]

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):
 
     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,6 +294,7 @@ 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],
         )
 
@@ -287,6 +303,7 @@ 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],
         required_prefix=SANDBOX_PREFIX,
     )
@@ -297,6 +314,7 @@ async def test_sandbox_access_policy_read():
             SANDBOX_POLICY_NAME,
             normal_user,
             action=ActionType.READ,
+            sandbox_metadata_db=sandbox_metadata_db,
             pfns=[OTHER_USER_SANDBOX_PFN],
             required_prefix=SANDBOX_PREFIX,
         )