EXPECTED_BUCKET_OWNER_ACCOUNT

Author: JamesW1-NHS

lambdas/filenameprocessor/src/constants.py

@@ -12,7 +12,7 @@
 
 SOURCE_BUCKET_NAME = os.getenv("SOURCE_BUCKET_NAME")
 
-# We have used an internal temporary bucket here and an acutal dps bucket will replace this
+# We have used an internal temporary bucket here and an actual dps bucket will replace this
 DPS_DESTINATION_BUCKET_NAME = os.getenv("ACK_BUCKET_NAME")
 EXPECTED_BUCKET_OWNER_ACCOUNT = os.getenv("ACCOUNT_ID")
 AUDIT_TABLE_NAME = os.getenv("AUDIT_TABLE_NAME")

lambdas/filenameprocessor/src/file_name_processor.py

@@ -24,6 +24,7 @@
 from constants import (
     DPS_DESTINATION_BUCKET_NAME,
     ERROR_TYPE_TO_STATUS_CODE_MAP,
+    EXPECTED_BUCKET_OWNER_ACCOUNT,
     EXTENDED_ATTRIBUTES_FILE_PREFIX,
     EXTENDED_ATTRIBUTES_VACC_TYPE,
     SOURCE_BUCKET_NAME,
@@ -277,9 +278,16 @@ def handle_extended_attributes_file(
         )
 
         dest_file_key = f"dps_destination/{file_key}"
-        copy_file_to_external_bucket(bucket_name, file_key, DPS_DESTINATION_BUCKET_NAME, dest_file_key)
+        copy_file_to_external_bucket(
+            bucket_name,
+            file_key,
+            DPS_DESTINATION_BUCKET_NAME,
+            dest_file_key,
+            EXPECTED_BUCKET_OWNER_ACCOUNT,
+            EXPECTED_BUCKET_OWNER_ACCOUNT,
+        )
         is_file_in_bucket(DPS_DESTINATION_BUCKET_NAME, dest_file_key)
-        delete_file(bucket_name, dest_file_key)
+        delete_file(bucket_name, dest_file_key, EXPECTED_BUCKET_OWNER_ACCOUNT)
 
         upsert_audit_table(
             message_id,

lambdas/filenameprocessor/tests/test_lambda_handler.py

@@ -36,10 +36,10 @@
 with patch.dict("os.environ", MOCK_ENVIRONMENT_DICT):
     from common.clients import REGION_NAME
     from constants import (
-        AUDIT_TABLE_NAME, 
-        EXTENDED_ATTRIBUTES_VACC_TYPE, 
-        AuditTableKeys, 
-        FileStatus
+        AUDIT_TABLE_NAME,
+        EXTENDED_ATTRIBUTES_VACC_TYPE,
+        AuditTableKeys,
+        FileStatus,
     )
     from file_name_processor import handle_record, lambda_handler
 
@@ -266,12 +266,14 @@ def test_lambda_handler_extended_attributes_success(self):
             Body=MOCK_EXTENDED_ATTRIBUTES_FILE_CONTENT,
         )
 
+        # TODO: rewrite the bucket patches to use moto
+
         # Patch uuid4 (message id), the identifier extraction, and prevent external copy issues by simulating move
         with (
             patch("file_name_processor.uuid4", return_value=test_cases[0].message_id),
             patch(
                 "file_name_processor.copy_file_to_external_bucket",
-                side_effect=lambda src_bucket, key, dst_bucket, dst_key: (
+                side_effect=lambda src_bucket, key, dst_bucket, dst_key, exp_owner, exp_src_owner: (
                     s3_client.put_object(
                         Bucket=BucketNames.DESTINATION,
                         Key=dst_key,
@@ -281,7 +283,7 @@ def test_lambda_handler_extended_attributes_success(self):
             ),
             patch(
                 "file_name_processor.delete_file",
-                side_effect=lambda src_bucket, key: (
+                side_effect=lambda src_bucket, key, exp_owner: (
                     s3_client.delete_object(
                         Bucket=BucketNames.SOURCE,
                         Key=key,
@@ -335,12 +337,15 @@ def test_lambda_handler_extended_attributes_failure(self):
             Body=MOCK_EXTENDED_ATTRIBUTES_FILE_CONTENT,
         )
 
+        # TODO: rewrite the bucket patches to use moto
+
         # Patch uuid4 (message id), the identifier extraction, and don't move the file
         with (
             patch("file_name_processor.uuid4", return_value=test_cases[0].message_id),
             patch(
                 "file_name_processor.copy_file_to_external_bucket",
-                side_effect=lambda src_bucket, key, dst_bucket, dst_key: (  # effectively do nothing
+                side_effect=lambda src_bucket, key, dst_bucket, dst_key, exp_owner, exp_src_owner: (
+                    # effectively do nothing
                     None,
                 ),
             ),

lambdas/shared/src/common/aws_s3_utils.py

@@ -1,11 +1,8 @@
 """Non-imms Utility Functions"""
 
-import os
 
 from common.clients import get_s3_client, logger
 
-EXPECTED_BUCKET_OWNER_ACCOUNT = os.getenv("ACCOUNT_ID")
-
 
 def move_file(bucket_name: str, source_file_key: str, destination_file_key: str) -> None:
     """Moves a file from one location to another within a single S3 bucket by copying and then deleting the file."""
@@ -20,24 +17,33 @@ def move_file(bucket_name: str, source_file_key: str, destination_file_key: str)
 
 
 def copy_file_to_external_bucket(
-    source_bucket: str, source_key: str, destination_bucket: str, destination_key: str
+    source_bucket: str,
+    source_key: str,
+    destination_bucket: str,
+    destination_key: str,
+    expected_bucket_owner: str,
+    expected_source_bucket_owner: str,
 ) -> None:
     s3_client = get_s3_client()
     s3_client.copy_object(
         CopySource={"Bucket": source_bucket, "Key": source_key},
         Bucket=destination_bucket,
         Key=destination_key,
-        ExpectedBucketOwner=EXPECTED_BUCKET_OWNER_ACCOUNT,
-        ExpectedSourceBucketOwner=EXPECTED_BUCKET_OWNER_ACCOUNT,
+        ExpectedBucketOwner=expected_bucket_owner,
+        ExpectedSourceBucketOwner=expected_source_bucket_owner,
     )
 
 
-def delete_file(source_bucket: str, source_key: str) -> None:
+def delete_file(
+    source_bucket: str,
+    source_key: str,
+    expected_bucket_owner: str,
+) -> None:
     s3_client = get_s3_client()
     s3_client.delete_object(
         Bucket=source_bucket,
         Key=source_key,
-        ExpectedBucketOwner=EXPECTED_BUCKET_OWNER_ACCOUNT,
+        ExpectedBucketOwner=expected_bucket_owner,
     )
 
 

lambdas/shared/tests/test_common/test_s3_utils.py

@@ -95,6 +95,8 @@ def test_move_file_outside_bucket_copies_then_deletes(self):
             source_key=source_key,
             destination_bucket=self.destination_bucket,
             destination_key=destination_key,
+            expected_bucket_owner=aws_s3_utils.EXPECTED_BUCKET_OWNER_ACCOUNT,
+            expected_source_bucket_owner=aws_s3_utils.EXPECTED_BUCKET_OWNER_ACCOUNT,
         )
 
         # Assert destination has the object
@@ -109,6 +111,7 @@ def test_move_file_outside_bucket_copies_then_deletes(self):
         aws_s3_utils.delete_file(
             source_bucket=self.source_bucket,
             source_key=source_key,
+            expected_bucket_owner=aws_s3_utils.EXPECTED_BUCKET_OWNER_ACCOUNT,
         )
 
         # Assert source object was deleted