From b78505d48b3d0f9a41b7fcd8948264f01f8e08a4 Mon Sep 17 00:00:00 2001 From: Shweta <216860557+shweta-nhs@users.noreply.github.com> Date: Mon, 11 Aug 2025 11:22:29 +0100 Subject: [PATCH 1/2] ELI-404: Fix Error message returned for authorisation failure --- .../common/api_error_response.py | 6 +++--- .../common/request_validator.py | 6 ++---- .../lambda/test_app_running_as_lambda.py | 13 ++++++------- tests/unit/common/test_request_validator.py | 4 +++- 4 files changed, 14 insertions(+), 15 deletions(-) diff --git a/src/eligibility_signposting_api/common/api_error_response.py b/src/eligibility_signposting_api/common/api_error_response.py index cbb07e8ca..44ed590c8 100644 --- a/src/eligibility_signposting_api/common/api_error_response.py +++ b/src/eligibility_signposting_api/common/api_error_response.py @@ -26,7 +26,7 @@ class FHIRIssueCode(str, Enum): class FHIRSpineErrorCode(str, Enum): - INVALID_NHS_NUMBER = "INVALID_NHS_NUMBER" + ACCESS_DENIED = "ACCESS_DENIED" INVALID_PARAMETER = "INVALID_PARAMETER" BAD_REQUEST = "BAD_REQUEST" INTERNAL_SERVER_ERROR = "INTERNAL_SERVER_ERROR" @@ -144,8 +144,8 @@ def log_and_generate_response( fhir_issue_code=FHIRIssueCode.FORBIDDEN, fhir_issue_severity=FHIRIssueSeverity.ERROR, fhir_coding_system="https://fhir.nhs.uk/STU3/ValueSet/Spine-ErrorOrWarningCode-1", - fhir_error_code=FHIRSpineErrorCode.INVALID_NHS_NUMBER, - fhir_display_message="The provided NHS number does not match the record.", + fhir_error_code=FHIRSpineErrorCode.ACCESS_DENIED, + fhir_display_message="Access has been denied to process this request.", ) diff --git a/src/eligibility_signposting_api/common/request_validator.py b/src/eligibility_signposting_api/common/request_validator.py index 8cf5fbe40..572c748f1 100644 --- a/src/eligibility_signposting_api/common/request_validator.py +++ b/src/eligibility_signposting_api/common/request_validator.py @@ -67,10 +67,8 @@ def wrapper(event: LambdaEvent, context: LambdaContext) -> dict[str, Any] | None ) if not validate_nhs_number(path_nhs_no, header_nhs_no): - message = f"NHS Number {path_nhs_no or ''} does not match the header NHS Number {header_nhs_no or ''}" - return NHS_NUMBER_MISMATCH_ERROR.log_and_generate_response( - log_message=message, diagnostics=message, location_param="id" - ) + message = "You are not authorised to request information for the supplied NHS Number" + return NHS_NUMBER_MISMATCH_ERROR.log_and_generate_response(log_message=message, diagnostics=message) query_params = event.get("queryStringParameters") if query_params: diff --git a/tests/integration/lambda/test_app_running_as_lambda.py b/tests/integration/lambda/test_app_running_as_lambda.py index b4cb61744..86c64992e 100644 --- a/tests/integration/lambda/test_app_running_as_lambda.py +++ b/tests/integration/lambda/test_app_running_as_lambda.py @@ -303,14 +303,13 @@ def test_given_nhs_number_in_path_does_not_match_with_nhs_number_in_headers_resu has_entries( severity="error", code="forbidden", - diagnostics=f"NHS Number {persisted_person} does " - f"not match the header NHS Number 123{persisted_person!s}", + diagnostics="You are not authorised to request information for the supplied NHS Number", details={ "coding": [ { "system": "https://fhir.nhs.uk/STU3/ValueSet/Spine-ErrorOrWarningCode-1", - "code": "INVALID_NHS_NUMBER", - "display": "The provided NHS number does not match the record.", + "code": "ACCESS_DENIED", + "display": "Access has been denied to process this request.", } ] }, @@ -350,13 +349,13 @@ def test_given_nhs_number_not_present_in_headers_results_in_error_response( has_entries( severity="error", code="forbidden", - diagnostics=f"NHS Number {persisted_person} does not match the header NHS Number ", + diagnostics="You are not authorised to request information for the supplied NHS Number", details={ "coding": [ { "system": "https://fhir.nhs.uk/STU3/ValueSet/Spine-ErrorOrWarningCode-1", - "code": "INVALID_NHS_NUMBER", - "display": "The provided NHS number does not match the record.", + "code": "ACCESS_DENIED", + "display": "Access has been denied to process this request.", } ] }, diff --git a/tests/unit/common/test_request_validator.py b/tests/unit/common/test_request_validator.py index 99a09d40e..0ec6726d5 100644 --- a/tests/unit/common/test_request_validator.py +++ b/tests/unit/common/test_request_validator.py @@ -79,7 +79,9 @@ def test_validate_request_params_nhs_mismatch(self, caplog): response_body = json.loads(response["body"]) issue = response_body["issue"][0] assert issue["code"] == "forbidden" - assert issue["diagnostics"] == ("NHS Number 0987654321 does not match the header NHS Number 1234567890") + assert issue["details"]["coding"][0]["code"] == "ACCESS_DENIED" + assert issue["details"]["coding"][0]["display"] == "Access has been denied to process this request." + assert issue["diagnostics"] == "You are not authorised to request information for the supplied NHS Number" def test_validate_request_params_nhs_missing_in_path(self, caplog): mock_handler = MagicMock() From 3b2946c83e46b9c8dc373c33983850a28a07b873 Mon Sep 17 00:00:00 2001 From: Shweta <216860557+shweta-nhs@users.noreply.github.com> Date: Mon, 11 Aug 2025 13:51:48 +0100 Subject: [PATCH 2/2] ELI-404: Fix sonar --- .../common/api_error_response.py | 12 ++---------- .../common/request_validator.py | 2 +- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/src/eligibility_signposting_api/common/api_error_response.py b/src/eligibility_signposting_api/common/api_error_response.py index 44ed590c8..afdbfe962 100644 --- a/src/eligibility_signposting_api/common/api_error_response.py +++ b/src/eligibility_signposting_api/common/api_error_response.py @@ -34,19 +34,18 @@ class FHIRSpineErrorCode(str, Enum): class APIErrorResponse: - def __init__( # noqa: PLR0913 + def __init__( self, status_code: HTTPStatus, fhir_issue_code: FHIRIssueCode, fhir_issue_severity: FHIRIssueSeverity, - fhir_coding_system: str, fhir_error_code: str, fhir_display_message: str, ) -> None: self.status_code = status_code self.fhir_issue_code = fhir_issue_code self.fhir_issue_severity = fhir_issue_severity - self.fhir_coding_system = fhir_coding_system + self.fhir_coding_system = "https://fhir.nhs.uk/STU3/ValueSet/Spine-ErrorOrWarningCode-1" self.fhir_error_code = fhir_error_code self.fhir_display_message = fhir_display_message @@ -96,7 +95,6 @@ def log_and_generate_response( status_code=HTTPStatus.UNPROCESSABLE_ENTITY, fhir_issue_code=FHIRIssueCode.VALUE, fhir_issue_severity=FHIRIssueSeverity.ERROR, - fhir_coding_system="https://fhir.nhs.uk/STU3/ValueSet/Spine-ErrorOrWarningCode-1", fhir_error_code=FHIRSpineErrorCode.INVALID_PARAMETER, fhir_display_message="The supplied value was not recognised by the API.", ) @@ -105,7 +103,6 @@ def log_and_generate_response( status_code=HTTPStatus.UNPROCESSABLE_ENTITY, fhir_issue_code=FHIRIssueCode.VALUE, fhir_issue_severity=FHIRIssueSeverity.ERROR, - fhir_coding_system="https://fhir.nhs.uk/STU3/ValueSet/Spine-ErrorOrWarningCode-1", fhir_error_code=FHIRSpineErrorCode.INVALID_PARAMETER, fhir_display_message="The supplied category was not recognised by the API.", ) @@ -114,7 +111,6 @@ def log_and_generate_response( status_code=HTTPStatus.BAD_REQUEST, fhir_issue_code=FHIRIssueCode.VALUE, fhir_issue_severity=FHIRIssueSeverity.ERROR, - fhir_coding_system="https://fhir.nhs.uk/STU3/ValueSet/Spine-ErrorOrWarningCode-1", fhir_error_code=FHIRSpineErrorCode.INVALID_PARAMETER, fhir_display_message="The given conditions were not in the expected format.", ) @@ -123,7 +119,6 @@ def log_and_generate_response( status_code=HTTPStatus.NOT_FOUND, fhir_issue_code=FHIRIssueCode.PROCESSING, fhir_issue_severity=FHIRIssueSeverity.ERROR, - fhir_coding_system="https://fhir.nhs.uk/STU3/ValueSet/Spine-ErrorOrWarningCode-1", fhir_error_code=FHIRSpineErrorCode.REFERENCE_NOT_FOUND, fhir_display_message="The given NHS number was not found in our datasets. " "This could be because the number is incorrect or " @@ -134,7 +129,6 @@ def log_and_generate_response( status_code=HTTPStatus.INTERNAL_SERVER_ERROR, fhir_issue_code=FHIRIssueCode.PROCESSING, fhir_issue_severity=FHIRIssueSeverity.ERROR, - fhir_coding_system="https://fhir.nhs.uk/STU3/ValueSet/Spine-ErrorOrWarningCode-1", fhir_error_code=FHIRSpineErrorCode.INTERNAL_SERVER_ERROR, fhir_display_message="An unexpected internal server error occurred.", ) @@ -143,7 +137,6 @@ def log_and_generate_response( status_code=HTTPStatus.FORBIDDEN, fhir_issue_code=FHIRIssueCode.FORBIDDEN, fhir_issue_severity=FHIRIssueSeverity.ERROR, - fhir_coding_system="https://fhir.nhs.uk/STU3/ValueSet/Spine-ErrorOrWarningCode-1", fhir_error_code=FHIRSpineErrorCode.ACCESS_DENIED, fhir_display_message="Access has been denied to process this request.", ) @@ -153,7 +146,6 @@ def log_and_generate_response( status_code=HTTPStatus.BAD_REQUEST, fhir_issue_code=FHIRIssueCode.INVALID, fhir_issue_severity=FHIRIssueSeverity.ERROR, - fhir_coding_system="https://fhir.nhs.uk/STU3/ValueSet/Spine-ErrorOrWarningCode-1", fhir_error_code=FHIRSpineErrorCode.BAD_REQUEST, fhir_display_message="Bad Request", ) diff --git a/src/eligibility_signposting_api/common/request_validator.py b/src/eligibility_signposting_api/common/request_validator.py index 572c748f1..9375fd729 100644 --- a/src/eligibility_signposting_api/common/request_validator.py +++ b/src/eligibility_signposting_api/common/request_validator.py @@ -17,7 +17,7 @@ logger = logging.getLogger(__name__) -condition_pattern = re.compile(r"^\s*[a-zA-Z0-9]+\s*$", re.IGNORECASE) +condition_pattern = re.compile(r"^\s*[a-z0-9]+\s*$", re.IGNORECASE) category_pattern = re.compile(r"^\s*(VACCINATIONS|SCREENING|ALL)\s*$", re.IGNORECASE) include_actions_pattern = re.compile(r"^\s*([YN])\s*$", re.IGNORECASE)