[PRMP-1407] Get specific version of a document reference

lambdas/enums/feature_flags.py

@@ -12,3 +12,4 @@ class FeatureFlags(StrEnum):
     UPLOAD_DOCUMENT_ITERATION_2_ENABLED = "uploadDocumentIteration2Enabled"
     UPLOAD_DOCUMENT_ITERATION_3_ENABLED = "uploadDocumentIteration3Enabled"
     BULK_UPLOAD_SEND_TO_REVIEW_ENABLED = "bulkUploadSendToReviewEnabled"
+    VERSION_HISTORY_ENABLED = "versionHistoryEnabled"

lambdas/handlers/get_document_reference_handler.py

@@ -30,31 +30,51 @@
         "APPCONFIG_CONFIGURATION",
         "EDGE_REFERENCE_TABLE",
         "CLOUDFRONT_URL",
-    ]
+    ],
 )
 @override_error_check
 def lambda_handler(event: dict[str, any], context):
     request_context.app_interaction = LoggingAppInteraction.VIEW_LG_RECORD.value
 
     feature_flag_service = FeatureFlagService()
     feature_flag_service.validate_feature_flag(
-        FeatureFlags.UPLOAD_DOCUMENT_ITERATION_3_ENABLED
+        FeatureFlags.UPLOAD_DOCUMENT_ITERATION_3_ENABLED,
+    )
+
+    version_history_flag = FeatureFlags.VERSION_HISTORY_ENABLED.value
+    version_history_flag_object = feature_flag_service.get_feature_flags_by_flag(
+        version_history_flag,
     )
 
     logger.info("Starting document fetch by ID process")
 
     try:
         document_id = event["pathParameters"]["id"]
         nhs_number = event["queryStringParameters"]["patientId"]
+        version = event["pathParameters"].get("version", None)
+        if (
+            not version_history_flag_object[version_history_flag]
+            and version is not None
+        ):
+            logger.error(
+                "Version history feature flag is disabled, but version was provided in request",
+            )
+            raise GetDocumentRefException(
+                400,
+                LambdaError.FeatureFlagDisabled,
+            )
     except KeyError:
         raise GetDocumentRefException(
-            400, LambdaError.DocumentReferenceMissingParameters
+            400,
+            LambdaError.DocumentReferenceMissingParameters,
         )
 
     service = GetDocumentReferenceService()
 
-    document_info = service.get_document_url_by_id(document_id, nhs_number)
+    document_info = service.get_document_url_by_id(document_id, nhs_number, version)
 
     return ApiGatewayResponse(
-        status_code=200, body=json.dumps(document_info), methods="GET"
+        status_code=200,
+        body=json.dumps(document_info),
+        methods="GET",
     ).create_api_gateway_response()

lambdas/models/auth_policy.py

@@ -29,7 +29,7 @@ class AuthPolicy(object):
     aws_account_id = ""
     principal_id = ""
     version = "2012-10-17"
-    path_regex = r"^[/.a-zA-Z0-9-*]+$"
+    path_regex = r"^[/.a-zA-Z0-9-*_]+$"
     allow_methods = []
     deny_methods = []
 
@@ -48,15 +48,15 @@ def _add_method(self, effect, verb, resource, conditions):
         statement can be null."""
         if verb != "*" and not hasattr(HttpVerb, verb):
             raise NameError(
-                "Invalid HTTP verb " + verb + ". Allowed verbs in HttpVerb class"
+                "Invalid HTTP verb " + verb + ". Allowed verbs in HttpVerb class",
             )
         resource_pattern = re.compile(self.path_regex)
         if not resource_pattern.match(resource):
             raise NameError(
                 "Invalid resource path: "
                 + resource
                 + ". Path should match "
-                + self.path_regex
+                + self.path_regex,
             )
 
         if resource[:1] == "/":
@@ -79,11 +79,11 @@ def _add_method(self, effect, verb, resource, conditions):
 
         if effect.lower() == "allow":
             self.allow_methods.append(
-                {"resourceArn": resource_arn, "conditions": conditions}
+                {"resourceArn": resource_arn, "conditions": conditions},
             )
         elif effect.lower() == "deny":
             self.deny_methods.append(
-                {"resourceArn": resource_arn, "conditions": conditions}
+                {"resourceArn": resource_arn, "conditions": conditions},
             )
 
     def _get_empty_statement(self, effect):
@@ -155,10 +155,10 @@ def build(self):
         }
 
         policy["policyDocument"]["Statement"].extend(
-            self._get_statement_for_effect("Allow", self.allow_methods)
+            self._get_statement_for_effect("Allow", self.allow_methods),
         )
         policy["policyDocument"]["Statement"].extend(
-            self._get_statement_for_effect("Deny", self.deny_methods)
+            self._get_statement_for_effect("Deny", self.deny_methods),
         )
 
         return policy

lambdas/services/authoriser_service.py

@@ -95,8 +95,11 @@ def deny_access_policy(self, path, http_verb, user_role, nhs_number: str = None)
         is_user_pcse = user_role == RepositoryRole.PCSE.value
 
         doc_ref_pattern = (
-            r"^/DocumentReference(/([0-9a-fA-F]{8}-[0-9a-fA-F]{4}-"
-            r"[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}))?(\?.*)?$"
+            r"^/DocumentReference"
+            r"(/([0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}))?"
+            r"(/_history)?"
+            r"(/[0-9]+)?"
+            r"(\?.*)?$"
         )
 
         match path:

lambdas/services/get_document_reference_service.py

@@ -25,11 +25,21 @@ def __init__(self):
         self.cloudfront_table_name = os.environ.get("EDGE_REFERENCE_TABLE")
         self.cloudfront_url = os.environ.get("CLOUDFRONT_URL")
 
-    def get_document_url_by_id(self, document_id: str, nhs_number: str):
-        document_reference = self.get_document_reference(document_id, nhs_number)
+    def get_document_url_by_id(
+        self,
+        document_id: str,
+        nhs_number: str,
+        version: str = None,
+    ) -> dict:
+        document_reference = self.get_document_reference(
+            document_id,
+            nhs_number,
+            version,
+        )
 
         presigned_s3_url = self.create_document_presigned_url(
-            document_reference.s3_bucket_name, document_reference.s3_file_key
+            document_reference.s3_bucket_name,
+            document_reference.s3_file_key,
         )
 
         return {"url": presigned_s3_url, "contentType": document_reference.content_type}
@@ -56,11 +66,17 @@ def create_document_presigned_url(self, bucket_name, file_location):
         return format_cloudfront_url(presigned_id, self.cloudfront_url)
 
     def get_document_reference(
-        self, document_id: str, nhs_number: str
+        self,
+        document_id: str,
+        nhs_number: str,
+        version: str = None,
     ) -> DocumentReference:
         filter_builder = DynamoQueryFilterBuilder()
-        filter_builder.add_condition("DocStatus", AttributeOperator.EQUAL, "final")
         filter_builder.add_condition("NhsNumber", AttributeOperator.EQUAL, nhs_number)
+        if version is None:
+            filter_builder.add_condition("DocStatus", AttributeOperator.EQUAL, "final")
+        else:
+            filter_builder.add_condition("Version", AttributeOperator.EQUAL, version)
 
         table_filter = filter_builder.build()
 
@@ -75,5 +91,4 @@ def get_document_reference(
         if len(documents) > 0:
             logger.info("Document found for given id")
             return documents[0]
-        else:
-            raise GetDocumentRefException(404, LambdaError.DocumentReferenceNotFound)
+        raise GetDocumentRefException(404, LambdaError.DocumentReferenceNotFound)

lambdas/tests/unit/handlers/test_authoriser_handler.py

@@ -1,4 +1,5 @@
 import pytest
+
 from handlers.authoriser_handler import lambda_handler
 from tests.unit.conftest import SSM_PARAM_JWT_TOKEN_PUBLIC_KEY, TEST_NHS_NUMBER
 from utils.exceptions import AuthorisationException
@@ -239,13 +240,26 @@ def test_deny_dr_for_deceased_patients(set_env, context, mocker, endpoint_path):
     assert response["policyDocument"] == expected_deny_policy
 
 
-def test_deny_dr_for_non_gp_admins_or_clinicians(set_env, context, mocker):
+@pytest.mark.parametrize(
+    "http_method, path",
+    [
+        ("POST", "/DocumentReference"),
+        ("PUT", "/DocumentReference/59d2e82d-9e6a-44e5-b7ed-befc5518f376"),
+    ],
+)
+def test_deny_dr_for_non_gp_admins_or_clinicians(
+    set_env,
+    context,
+    mocker,
+    http_method,
+    path,
+):
     expected_deny_policy = {
         "Statement": [
             {
                 "Action": "execute-api:Invoke",
                 "Effect": "Deny",
-                "Resource": [f"{MOCK_METHOD_ARN_PREFIX}/POST/DocumentReference"],
+                "Resource": [f"{MOCK_METHOD_ARN_PREFIX}/{http_method}{path}"],
             },
         ],
         "Version": "2012-10-17",
@@ -255,7 +269,7 @@ def test_deny_dr_for_non_gp_admins_or_clinicians(set_env, context, mocker):
     test_pcse_event = {
         "headers": {"authorization": auth_pcse_token},
         "queryStringParameters": {"patientId": TEST_NHS_NUMBER},
-        "methodArn": f"{MOCK_METHOD_ARN_PREFIX}/POST/DocumentReference",
+        "methodArn": f"{MOCK_METHOD_ARN_PREFIX}/{http_method}{path}",
     }
     mock_auth_service = mocker.patch(
         "services.authoriser_service.AuthoriserService.auth_request",
@@ -265,8 +279,8 @@ def test_deny_dr_for_non_gp_admins_or_clinicians(set_env, context, mocker):
     pcse_response = lambda_handler(test_pcse_event, context=context)
 
     mock_auth_service.assert_called_with(
-        "/DocumentReference",
-        "POST",
+        path,
+        http_method,
         SSM_PARAM_JWT_TOKEN_PUBLIC_KEY,
         auth_pcse_token,
         TEST_NHS_NUMBER,