[PRMP-1283] Update Authoriser and Endpoint Logic to Support Placeholder NHS Number Access

lambdas/enums/lambda_error.py

@@ -699,7 +699,7 @@ def create_error_body(self, params: Optional[dict] = None, **kwargs) -> str:
         "message": "Invalid request",
     }
 
-    DocumentReviewUploadForbidden = {"err_code": "UDR_4031", "message": "Forbidden"}
+    DocumentReviewForbidden = {"err_code": "UDR_4031", "message": "Forbidden"}
 
     DocumentReviewPresignedFailure = {
         "err_code": "UDR_5003",

lambdas/services/authoriser_service.py

@@ -2,6 +2,7 @@
 import time
 
 from enums.repository_role import RepositoryRole
+from models.staging_metadata import NHS_NUMBER_PLACEHOLDER
 from services.manage_user_session_access import ManageUserSessionAccess
 from services.token_service import TokenService
 from utils.audit_logging_setup import LoggingService
@@ -77,6 +78,8 @@ def deny_access_policy(self, path, user_role, nhs_number: str = None):
             nhs_number in self.deceased_nhs_numbers if nhs_number else False
         )
 
+        patient_id_is_placeholder = nhs_number == NHS_NUMBER_PLACEHOLDER
+
         is_user_gp_admin = user_role == RepositoryRole.GP_ADMIN.value
         is_user_gp_clinical = user_role == RepositoryRole.GP_CLINICAL.value
         is_user_pcse = user_role == RepositoryRole.PCSE.value
@@ -127,7 +130,7 @@ def deny_access_policy(self, path, user_role, nhs_number: str = None):
                 deny_resource = False
 
             case path if re.match(r"^/DocumentReview/[^/]+/[^/]+$", path):
-                deny_resource = not patient_access_is_allowed
+                deny_resource = False if patient_id_is_placeholder else not patient_access_is_allowed
 
             case "/DocumentReview":
                 deny_resource = False

lambdas/services/get_document_review_service.py

@@ -3,11 +3,13 @@
 from datetime import datetime, timezone
 
 from enums.lambda_error import LambdaError
+from models.staging_metadata import NHS_NUMBER_PLACEHOLDER
 from services.base.s3_service import S3Service
 from services.document_upload_review_service import DocumentUploadReviewService
 from utils.audit_logging_setup import LoggingService
-from utils.exceptions import DynamoServiceException
+from utils.exceptions import DynamoServiceException, OdsErrorException, UserNotAuthorisedException
 from utils.lambda_exceptions import GetDocumentReviewException
+from utils.ods_utils import extract_ods_code_from_request_context
 from utils.utilities import format_cloudfront_url
 
 logger = LoggingService(__name__)
@@ -39,6 +41,9 @@ def get_document_review(
             Dictionary containing the document review details, or None if not found.
         """
         try:
+
+            reviewer_ods_code = extract_ods_code_from_request_context()
+
             logger.info(
                 f"Fetching document review for patient_id: {patient_id}, document_id: {document_id}"
             )
@@ -53,6 +58,10 @@ def get_document_review(
                 logger.info(f"No document review found for document_id: {document_id}")
                 return None
 
+            if reviewer_ods_code != document_review_item.custodian:
+                raise UserNotAuthorisedException(f"{reviewer_ods_code} is not custodian of document.")
+
+
             if document_review_item.nhs_number != patient_id:
                 logger.warning(
                     f"Document {document_id} does not belong to patient {patient_id}"
@@ -88,6 +97,12 @@ def get_document_review(
                 {"Result": "Failed to retrieve document review"},
             )
             raise GetDocumentReviewException(500, LambdaError.DocRefClient)
+        except OdsErrorException as e:
+            logger.error(e)
+            raise GetDocumentReviewException(403, LambdaError.DocumentReviewMissingODS)
+        except UserNotAuthorisedException as e:
+            logger.error(e)
+            raise GetDocumentReviewException(403, LambdaError.DocumentReviewForbidden)
         except Exception as e:
             logger.error(
                 f"Unexpected error retrieving document review: {str(e)}",

lambdas/services/post_document_review_service.py

@@ -49,7 +49,7 @@ def process_event(self, event: DocumentReviewUploadEvent) -> dict:
             ):
                 logger.info("Patient is deceased, upload will not proceed.")
                 raise DocumentReviewLambdaException(
-                    403, LambdaError.DocumentReviewUploadForbidden
+                    403, LambdaError.DocumentReviewForbidden
                 )
 
             document_review_reference = self.create_review_reference_from_event(

lambdas/services/review_document_status_check_service.py

@@ -29,7 +29,7 @@ def get_document_review_status(
 
             logger.info("Checking user is author of review document.")
             if not self.user_is_author(ods_code, review_document_reference):
-                raise DocumentReviewLambdaException(403, LambdaError.DocumentReviewUploadForbidden)
+                raise DocumentReviewLambdaException(403, LambdaError.DocumentReviewForbidden)
 
             return review_document_reference.model_dump_camel_case(
                 mode="json", include={"id", "version", "review_status"}

lambdas/tests/unit/handlers/test_review_document_status_check_handler.py

@@ -164,13 +164,13 @@ def test_lambda_handler_returns_403_document_review_forbidden_raised(
     mock_extract_ods,
 ):
     mock_service.get_document_review_status.side_effect = DocumentReviewLambdaException(
-        403, LambdaError.DocumentReviewUploadForbidden
+        403, LambdaError.DocumentReviewForbidden
     )
 
     body = json.dumps(
         {
-            "message": LambdaError.DocumentReviewUploadForbidden.value["message"],
-            "err_code": LambdaError.DocumentReviewUploadForbidden.value["err_code"],
+            "message": LambdaError.DocumentReviewForbidden.value["message"],
+            "err_code": LambdaError.DocumentReviewForbidden.value["err_code"],
             "interaction_id": MOCK_INTERACTION_ID,
         }
     )

lambdas/tests/unit/services/test_authoriser_service.py

@@ -1,5 +1,6 @@
 import pytest
 from enums.repository_role import RepositoryRole
+from models.staging_metadata import NHS_NUMBER_PLACEHOLDER
 from services.authoriser_service import AuthoriserService
 from tests.unit.conftest import TEST_UUID
 from utils.exceptions import AuthorisationException
@@ -145,6 +146,15 @@ def test_deny_access_policy_for_various_paths_and_roles(
     assert actual == expected
 
 
+def test_deny_access_policy_allows_review_for_placeholder_nhs_number(
+        mock_auth_service: AuthoriserService,
+
+):
+    roles = [RepositoryRole.GP_ADMIN.value, RepositoryRole.GP_CLINICAL.value, RepositoryRole.PCSE.value]
+    for role in roles:
+        actual = mock_auth_service.deny_access_policy(f"/DocumentReview/{TEST_UUID}/1", role, NHS_NUMBER_PLACEHOLDER)
+        assert not actual
+
 @pytest.mark.parametrize(
     "path",
     [

lambdas/tests/unit/services/test_get_document_review_service.py

@@ -1,3 +1,4 @@
+from copy import deepcopy
 from unittest.mock import MagicMock
 
 import pytest
@@ -10,13 +11,14 @@
     DocumentReviewFileDetails,
     DocumentUploadReviewReference,
 )
+from models.staging_metadata import NHS_NUMBER_PLACEHOLDER
 from services.get_document_review_service import GetDocumentReviewService
 from tests.unit.conftest import (
     MOCK_DOCUMENT_REVIEW_BUCKET,
     MOCK_EDGE_REFERENCE_TABLE,
     TEST_NHS_NUMBER,
 )
-from utils.exceptions import DynamoServiceException
+from utils.exceptions import DynamoServiceException, OdsErrorException
 from utils.lambda_exceptions import GetDocumentReviewException
 
 TEST_DOCUMENT_ID = "test-document-id-123"
@@ -30,6 +32,13 @@
 TEST_CLOUDFRONT_URL = "https://mock-cloudfront-url.com"
 
 
+@pytest.fixture
+def mock_extract_ods(mocker):
+    return mocker.patch(
+        "services.get_document_review_service.extract_ods_code_from_request_context"
+    )
+
+
 @pytest.fixture
 def mock_service(set_env, mocker):
     """Fixture to create a GetDocumentReviewService with mocked dependencies."""
@@ -76,8 +85,10 @@ def mock_document_review():
     return review
 
 
-def test_get_document_review_success(mock_service, mock_document_review, mocker):
+def test_get_document_review_success(mock_service, mock_document_review, mocker, mock_extract_ods):
     """Test successful retrieval of a document review with pre-signed URLs."""
+    mock_extract_ods.return_value = TEST_ODS_CODE
+
     mock_service.document_review_service.get_document_review_by_id.return_value = (
         mock_document_review
     )
@@ -133,8 +144,9 @@ def test_get_document_review_success(mock_service, mock_document_review, mocker)
     )
 
 
-def test_get_document_review_not_found(mock_service):
+def test_get_document_review_not_found(mock_service, mock_extract_ods):
     """Test when a document review is not found in DynamoDB."""
+    mock_extract_ods.return_value = TEST_ODS_CODE
     mock_service.document_review_service.get_document_review_by_id.return_value = None
 
     result = mock_service.get_document_review(
@@ -149,8 +161,10 @@ def test_get_document_review_not_found(mock_service):
     )
 
 
-def test_get_document_review_nhs_number_mismatch(mock_service, mock_document_review):
+def test_get_document_review_nhs_number_mismatch(mock_service, mock_document_review, mock_extract_ods):
     """Test when document review exists but the NHS number doesn't match."""
+    mock_extract_ods.return_value = TEST_ODS_CODE
+
     mock_service.document_review_service.get_document_review_by_id.return_value = (
         mock_document_review
     )
@@ -169,8 +183,31 @@ def test_get_document_review_nhs_number_mismatch(mock_service, mock_document_rev
     mock_service.s3_service.create_download_presigned_url.assert_not_called()
 
 
-def test_get_document_review_dynamo_service_exception(mock_service):
+def test_get_document_review_handles_placeholder_nhs_number(mock_service, mock_extract_ods, mock_document_review):
+    mock_extract_ods.return_value = TEST_ODS_CODE
+    unknown_patient_review = deepcopy(mock_document_review)
+    unknown_patient_review.nhs_number = NHS_NUMBER_PLACEHOLDER
+
+    mock_service.document_review_service.get_document_review_by_id.return_value = (
+        unknown_patient_review
+    )
+
+    mock_service.get_document_review(
+        patient_id=NHS_NUMBER_PLACEHOLDER,
+        document_id=TEST_DOCUMENT_ID,
+        document_version=TEST_DOCUMENT_VERSION,
+    )
+    mock_service.document_review_service.get_document_review_by_id.assert_called_once_with(
+        document_id=TEST_DOCUMENT_ID, document_version=TEST_DOCUMENT_VERSION
+    )
+
+    mock_service.s3_service.create_download_presigned_url.assert_called()
+
+
+def test_get_document_review_dynamo_service_exception(mock_service, mock_extract_ods):
     """Test handling of DynamoServiceException."""
+    mock_extract_ods.return_value = TEST_ODS_CODE
+
     mock_service.document_review_service.get_document_review_by_id.side_effect = (
         DynamoServiceException("DynamoDB error")
     )
@@ -186,8 +223,10 @@ def test_get_document_review_dynamo_service_exception(mock_service):
     assert exc_info.value.error == LambdaError.DocRefClient
 
 
-def test_get_document_review_unexpected_exception(mock_service):
+def test_get_document_review_unexpected_exception(mock_service, mock_extract_ods):
     """Test handling of unexpected exceptions."""
+    mock_extract_ods.return_value = TEST_ODS_CODE
+
     mock_service.document_review_service.get_document_review_by_id.side_effect = (
         Exception("Unexpected error")
     )
@@ -203,6 +242,34 @@ def test_get_document_review_unexpected_exception(mock_service):
     assert exc_info.value.error == LambdaError.DocRefClient
 
 
+def test_get_document_review_throws_error_user_not_custodian(mock_service, mock_extract_ods):
+    mock_extract_ods.return_value = "Z67890"
+
+    with pytest.raises(GetDocumentReviewException) as e:
+        mock_service.get_document_review(
+            patient_id=TEST_NHS_NUMBER,
+            document_id=TEST_DOCUMENT_ID,
+            document_version=TEST_DOCUMENT_VERSION,
+        )
+
+    assert e.value.status_code == 403
+    assert e.value.error == LambdaError.DocumentReviewForbidden
+
+
+def test_error_thrown_no_ods_in_request_context(mock_service, mock_extract_ods):
+    mock_extract_ods.side_effect = OdsErrorException()
+
+    with pytest.raises(GetDocumentReviewException) as e:
+        mock_service.get_document_review(
+            patient_id=TEST_NHS_NUMBER,
+            document_id=TEST_DOCUMENT_ID,
+            document_version=TEST_DOCUMENT_VERSION,
+        )
+
+    assert e.value.status_code == 403
+    assert e.value.error == LambdaError.DocumentReviewMissingODS
+
+
 @freeze_time("2023-11-03T12:00:00Z")
 def test_create_cloudfront_presigned_url(mock_service, mock_uuid, mocker):
     """Test creating a CloudFront presigned URL."""