From 5d75bb74f49919de8ab952c527ddd773ba9bfc3f Mon Sep 17 00:00:00 2001 From: David Wass Date: Mon, 10 Feb 2025 10:34:21 +0000 Subject: [PATCH 1/2] [ERSSUP-68202]-[JW]-[Return 403 - Forbidden for missing ASID in application]-[DMW] --- ...Message.OperationOutcomeErrorResponse.xml} | 4 +- ...sage.SetOperationOutcomeIssueCodeLogin.xml | 4 + ...ignMessage.SetOperationOutcomeIssueIal.xml | 4 + ...Message.SetOperationOutcomeMissingAsid.xml | 14 ++ .../policies/RaiseFault.MissingAsid.xml | 10 ++ proxies/live/apiproxy/targets/ers-target.xml | 27 +++- tests/conftest.py | 71 +++++++++ tests/integration/test_headers.py | 144 ++++++++++++++++++ 8 files changed, 273 insertions(+), 5 deletions(-) rename proxies/live/apiproxy/policies/{AssignMessage.AuthenticationOperationOutcomeErrorResponse.xml => AssignMessage.OperationOutcomeErrorResponse.xml} (87%) create mode 100644 proxies/live/apiproxy/policies/AssignMessage.SetOperationOutcomeMissingAsid.xml create mode 100644 proxies/live/apiproxy/policies/RaiseFault.MissingAsid.xml diff --git a/proxies/live/apiproxy/policies/AssignMessage.AuthenticationOperationOutcomeErrorResponse.xml b/proxies/live/apiproxy/policies/AssignMessage.OperationOutcomeErrorResponse.xml similarity index 87% rename from proxies/live/apiproxy/policies/AssignMessage.AuthenticationOperationOutcomeErrorResponse.xml rename to proxies/live/apiproxy/policies/AssignMessage.OperationOutcomeErrorResponse.xml index 0fd000d00..ca205b0fc 100644 --- a/proxies/live/apiproxy/policies/AssignMessage.AuthenticationOperationOutcomeErrorResponse.xml +++ b/proxies/live/apiproxy/policies/AssignMessage.OperationOutcomeErrorResponse.xml @@ -1,6 +1,6 @@ - + - 401 + {status_code} Unauthorized { "resourceType": "OperationOutcome", "meta": { "lastUpdated": "%current_timestamp#", "profile" : [ "%op_outcome_fhir_profile#" ] }, "issue": [ { "severity": "error", "code": "%op_outcome_issue_code#", "details": { "coding": [ { "system": "%op_outcome_issue_details_coding_system#", "code": "%op_outcome_issue_details_coding_code#" } ] }, "diagnostics": "%faultstring#" } ] } diff --git a/proxies/live/apiproxy/policies/AssignMessage.SetOperationOutcomeIssueCodeLogin.xml b/proxies/live/apiproxy/policies/AssignMessage.SetOperationOutcomeIssueCodeLogin.xml index f1700fc3e..288ca58e5 100644 --- a/proxies/live/apiproxy/policies/AssignMessage.SetOperationOutcomeIssueCodeLogin.xml +++ b/proxies/live/apiproxy/policies/AssignMessage.SetOperationOutcomeIssueCodeLogin.xml @@ -1,4 +1,8 @@ + + status_code + 401 + op_outcome_issue_code login diff --git a/proxies/live/apiproxy/policies/AssignMessage.SetOperationOutcomeIssueIal.xml b/proxies/live/apiproxy/policies/AssignMessage.SetOperationOutcomeIssueIal.xml index c3901b446..4bed1bf31 100644 --- a/proxies/live/apiproxy/policies/AssignMessage.SetOperationOutcomeIssueIal.xml +++ b/proxies/live/apiproxy/policies/AssignMessage.SetOperationOutcomeIssueIal.xml @@ -1,4 +1,8 @@ + + status_code + 401 + op_outcome_issue_code forbidden diff --git a/proxies/live/apiproxy/policies/AssignMessage.SetOperationOutcomeMissingAsid.xml b/proxies/live/apiproxy/policies/AssignMessage.SetOperationOutcomeMissingAsid.xml new file mode 100644 index 000000000..8e1082bd8 --- /dev/null +++ b/proxies/live/apiproxy/policies/AssignMessage.SetOperationOutcomeMissingAsid.xml @@ -0,0 +1,14 @@ + + + op_outcome_issue_code + forbidden + + + faultstring + ASID is not configured in the application + + + status_code + 403 + + diff --git a/proxies/live/apiproxy/policies/RaiseFault.MissingAsid.xml b/proxies/live/apiproxy/policies/RaiseFault.MissingAsid.xml new file mode 100644 index 000000000..72eb5a081 --- /dev/null +++ b/proxies/live/apiproxy/policies/RaiseFault.MissingAsid.xml @@ -0,0 +1,10 @@ + + + + + 403 + Forbidden + + + true + diff --git a/proxies/live/apiproxy/targets/ers-target.xml b/proxies/live/apiproxy/targets/ers-target.xml index b3620750a..5ea639c61 100644 --- a/proxies/live/apiproxy/targets/ers-target.xml +++ b/proxies/live/apiproxy/targets/ers-target.xml @@ -15,7 +15,7 @@ AssignMessage.SetOperationOutcomeIssueCodeLogin - AssignMessage.AuthenticationOperationOutcomeErrorResponse + AssignMessage.OperationOutcomeErrorResponse (oauthV2.OauthV2.VerifyAccessToken.failed = true) and (isFhirR4Path = true) @@ -43,7 +43,7 @@ aalError != true - AssignMessage.AuthenticationOperationOutcomeErrorResponse + AssignMessage.OperationOutcomeErrorResponse aalError = true (oauthV2.OauthV2.VerifyAccessToken.failed = true) and (isFhirR4Path = false) @@ -85,10 +85,27 @@ AssignMessage.SetOperationOutcomeIssueIal - AssignMessage.AuthenticationOperationOutcomeErrorResponse + AssignMessage.OperationOutcomeErrorResponse (raisefault.RaiseFault.401InsufficientIal.failed = true) + + + (isFhirR4Path = true) + AssignMessage.SetOperationOutcomeVariablesR4 + + + (isFhirR4Path = false) + AssignMessage.SetOperationOutcomeVariablesPreR4 + + + AssignMessage.SetOperationOutcomeMissingAsid + + + AssignMessage.OperationOutcomeErrorResponse + + (raisefault.RaiseFault.MissingAsid.failed = true) + @@ -101,6 +118,10 @@ OauthV2.VerifyAccessToken + + RaiseFault.MissingAsid + (app.asid == null) Or (app.asid == "") + AssignMessage.PopulateAsidFromApp diff --git a/tests/conftest.py b/tests/conftest.py index a1c18c4a0..97de92084 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -167,6 +167,77 @@ def _update_function(append_scopes: Collection[str]): return _update_function +@pytest.fixture +def delete_user_restricted_app_attr( + user_restricted_app, client: ApigeeClient +) -> Callable[[Collection[str]], Generator[Dict[str, str], None, None]]: + @contextmanager + def _update_function(attr): + app_api = DeveloperAppsAPI(client=client) + app = app_api.get_app_by_name( + email="apm-testing-internal-dev@nhs.net", + app_name=user_restricted_app["name"], + ) + + warnings.warn(f"Existing app = {app}") + + existing_attributes = app_api.get_app_attributes( + email="apm-testing-internal-dev@nhs.net", app_name=app["name"] + ) + + yield app_api.delete_app_attribute_by_name( + email="apm-testing-internal-dev@nhs.net", + app_name=app["name"], + attribute_name=attr, + ) + + # reset the product once the context manager has been closed. + + app_api.post_app_attributes( + email="apm-testing-internal-dev@nhs.net", + app_name=app["name"], + body=existing_attributes, + ) + + return _update_function + + +@pytest.fixture +def update_user_restricted_app_attr( + user_restricted_app, client: ApigeeClient +) -> Callable[[Collection[str]], Generator[Dict[str, str], None, None]]: + @contextmanager + def _update_function(attr, value): + app_api = DeveloperAppsAPI(client=client) + app = app_api.get_app_by_name( + email="apm-testing-internal-dev@nhs.net", + app_name=user_restricted_app["name"], + ) + + warnings.warn(f"Existing app = {app}") + + existing_attributes = app_api.get_app_attributes( + email="apm-testing-internal-dev@nhs.net", app_name=app["name"] + ) + + yield app_api.post_app_attribute_by_name( + email="apm-testing-internal-dev@nhs.net", + app_name=app["name"], + attribute_name=attr, + body={"value": value}, + ) + + # reset the product once the context manager has been closed. + + app_api.post_app_attributes( + email="apm-testing-internal-dev@nhs.net", + app_name=app["name"], + body=existing_attributes, + ) + + return _update_function + + @pytest.fixture def make_product(client, environment, service_name): async def _make_product(product_scopes): diff --git a/tests/integration/test_headers.py b/tests/integration/test_headers.py index 2c2e15cf7..849a6d9b4 100644 --- a/tests/integration/test_headers.py +++ b/tests/integration/test_headers.py @@ -517,3 +517,147 @@ async def test_access_code_not_supported( for renamed_header in RenamedHeader: assert renamed_header.renamed not in client_response_headers + + @pytest.mark.asyncio + @pytest.mark.parametrize( + "user, endpoint_url, is_fhir_4", + [ + (Actor.RC, "/FHIR/R4/", True), + (Actor.AAL2_USER, "/FHIR/R4/", True), + (Actor.RC, "/FHIR/STU3/", False), + (Actor.AAL2_USER, "/FHIR/STU3/", False), + ], + ) + async def test_headers_on_echo_target_no_asid( + self, + authenticate_user, + endpoint_url, + is_fhir_4, + service_url, + delete_user_restricted_app_attr, + user: Actor, + ): + with delete_user_restricted_app_attr("asid"): + + access_code = await authenticate_user(user) + + client_request_headers = { + _HEADER_ECHO: "", # enable echo target + _HEADER_AUTHORIZATION: "Bearer " + access_code, + _HEADER_REQUEST_ID: "DUMMY-VALUE", + RenamedHeader.REFERRAL_ID.original: _EXPECTED_REFERRAL_ID, + RenamedHeader.CORRELATION_ID.original: _EXPECTED_CORRELATION_ID, + RenamedHeader.BUSINESS_FUNCTION.original: user.business_function, + RenamedHeader.ODS_CODE.original: user.org_code, + RenamedHeader.FILENAME.original: _EXPECTED_FILENAME, + RenamedHeader.COMM_RULE_ORG.original: _EXPECTED_COMM_RULE_ORG, + RenamedHeader.OBO_USER_ID.original: _EXPECTED_OBO_USER_ID, + } + + # Make the API call + response = requests.get( + f"{service_url}{endpoint_url}", headers=client_request_headers + ) + # Verify the status + assert ( + response.status_code == 403 + ), "Expected a 403 when accessing the api but got " + str( + response.status_code + ) + + response_data = response.json() + assert response_data["resourceType"] == "OperationOutcome" + assert response_data["meta"]["lastUpdated"] is not None + assert response_data["meta"]["profile"][0] == ( + "https://www.hl7.org/fhir/R4/operationoutcome.html" + if is_fhir_4 + else "https://fhir.nhs.uk/STU3/StructureDefinition/eRS-OperationOutcome-1" + ) + assert len(response_data["issue"]) == 1 + issue = response_data["issue"][0] + assert issue["severity"] == "error" + assert issue["code"] == "forbidden" + assert issue["diagnostics"] == "ASID is not configured in the application" + assert len(issue["details"]["coding"]) == 1 + issue_details = issue["details"]["coding"][0] + assert ( + issue_details["system"] + == "https://fhir.nhs.uk/CodeSystem/NHSD-API-ErrorOrWarningCode" + if is_fhir_4 + else "https://fhir.nhs.uk/STU3/CodeSystem/eRS-APIErrorCode-1" + ) + assert ( + issue_details["code"] == "ACCESS_DENIED" if is_fhir_4 else "NO_ACCESS" + ) + + @pytest.mark.asyncio + @pytest.mark.parametrize( + "user, endpoint_url, is_fhir_4", + [ + (Actor.RC, "/FHIR/R4/", True), + (Actor.AAL2_USER, "/FHIR/R4/", True), + (Actor.RC, "/FHIR/STU3/", False), + (Actor.AAL2_USER, "/FHIR/STU3/", False), + ], + ) + async def test_headers_on_echo_target_empty_asid( + self, + authenticate_user, + endpoint_url, + is_fhir_4, + service_url, + update_user_restricted_app_attr, + user: Actor, + ): + with update_user_restricted_app_attr("asid", ""): + + access_code = await authenticate_user(user) + + client_request_headers = { + _HEADER_ECHO: "", # enable echo target + _HEADER_AUTHORIZATION: "Bearer " + access_code, + _HEADER_REQUEST_ID: "DUMMY-VALUE", + RenamedHeader.REFERRAL_ID.original: _EXPECTED_REFERRAL_ID, + RenamedHeader.CORRELATION_ID.original: _EXPECTED_CORRELATION_ID, + RenamedHeader.BUSINESS_FUNCTION.original: user.business_function, + RenamedHeader.ODS_CODE.original: user.org_code, + RenamedHeader.FILENAME.original: _EXPECTED_FILENAME, + RenamedHeader.COMM_RULE_ORG.original: _EXPECTED_COMM_RULE_ORG, + RenamedHeader.OBO_USER_ID.original: _EXPECTED_OBO_USER_ID, + } + + # Make the API call + response = requests.get( + f"{service_url}{endpoint_url}", headers=client_request_headers + ) + # Verify the status + assert ( + response.status_code == 403 + ), "Expected a 403 when accessing the api but got " + str( + response.status_code + ) + + response_data = response.json() + assert response_data["resourceType"] == "OperationOutcome" + assert response_data["meta"]["lastUpdated"] is not None + assert response_data["meta"]["profile"][0] == ( + "https://www.hl7.org/fhir/R4/operationoutcome.html" + if is_fhir_4 + else "https://fhir.nhs.uk/STU3/StructureDefinition/eRS-OperationOutcome-1" + ) + assert len(response_data["issue"]) == 1 + issue = response_data["issue"][0] + assert issue["severity"] == "error" + assert issue["code"] == "forbidden" + assert issue["diagnostics"] == "ASID is not configured in the application" + assert len(issue["details"]["coding"]) == 1 + issue_details = issue["details"]["coding"][0] + assert ( + issue_details["system"] + == "https://fhir.nhs.uk/CodeSystem/NHSD-API-ErrorOrWarningCode" + if is_fhir_4 + else "https://fhir.nhs.uk/STU3/CodeSystem/eRS-APIErrorCode-1" + ) + assert ( + issue_details["code"] == "ACCESS_DENIED" if is_fhir_4 else "NO_ACCESS" + ) From f10c3a29518499ceb51c09a71c9c843fd9bd5c51 Mon Sep 17 00:00:00 2001 From: David Wass Date: Tue, 18 Mar 2025 15:35:40 +0000 Subject: [PATCH 2/2] [ERSSUP-83710]-[AO]-[Update 401 and 403 error descriptions]-[DMW] --- sandbox/src/mocks/stu3/STU3-Unauthorised.json | 23 +++++++++++++++++++ .../r4/schemas/responses/Forbidden.yaml | 3 ++- .../r4/schemas/responses/Unauthorized.yaml | 4 ++-- .../stu3/schemas/responses/Forbidden.yaml | 7 +++--- .../stu3/schemas/responses/Unauthorized.yaml | 21 ++++++++++++++++- 5 files changed, 51 insertions(+), 7 deletions(-) create mode 100644 sandbox/src/mocks/stu3/STU3-Unauthorised.json diff --git a/sandbox/src/mocks/stu3/STU3-Unauthorised.json b/sandbox/src/mocks/stu3/STU3-Unauthorised.json new file mode 100644 index 000000000..5d7290596 --- /dev/null +++ b/sandbox/src/mocks/stu3/STU3-Unauthorised.json @@ -0,0 +1,23 @@ +{ + "meta": { + "profile": [ + "https://fhir.nhs.uk/STU3/StructureDefinition/eRS-OperationOutcome-1" + ] + }, + "resourceType": "OperationOutcome", + "issue": [ + { + "severity": "error", + "code": "login", + "details": { + "coding": [ + { + "system": "https://fhir.nhs.uk/STU3/CodeSystem/eRS-APIErrorCode-1", + "code": "NO_ACCESS" + } + ] + }, + "diagnostics": "Example diagnostics message." + } + ] +} diff --git a/specification/components/r4/schemas/responses/Forbidden.yaml b/specification/components/r4/schemas/responses/Forbidden.yaml index ce2375e64..2c4263859 100644 --- a/specification/components/r4/schemas/responses/Forbidden.yaml +++ b/specification/components/r4/schemas/responses/Forbidden.yaml @@ -5,6 +5,7 @@ description: | | issue.details.coding.code | issue.code | Coding System | Description | | ------------------------- | ---------- | ------------------------------------------------------------------ | ---------------------------------------------------------------------------------- | | REC_FORBIDDEN | forbidden | [BaRS Error Code](https://fhir.nhs.uk/CodeSystem/http-error-codes) | A call attempts to access or operate upon a resource without proper authorisation. | + | ACCESS_DENIED | forbidden | [APIM Error Code](https://fhir.nhs.uk/CodeSystem/NHSD-API-ErrorOrWarningCode) | The request could not be authenticated due to insufficient credentials being provided. | headers: X-Correlation-ID: @@ -18,4 +19,4 @@ content: schema: $ref: '../NHSDigital-OperationOutcome.yaml' example: - $ref: '../../examples/NHSDigital-OperationOutcome-403.json' \ No newline at end of file + $ref: '../../examples/NHSDigital-OperationOutcome-403.json' diff --git a/specification/components/r4/schemas/responses/Unauthorized.yaml b/specification/components/r4/schemas/responses/Unauthorized.yaml index cd1fb9b54..7f2a5ffc9 100644 --- a/specification/components/r4/schemas/responses/Unauthorized.yaml +++ b/specification/components/r4/schemas/responses/Unauthorized.yaml @@ -4,7 +4,7 @@ description: | | issue.details.coding.code | issue.code | Coding System | Description | | ------------------------- | ---------------- | ----------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | - | ACCESS_DENIED | login | [APIM Error Code](https://fhir.nhs.uk/CodeSystem/NHSD-API-ErrorOrWarningCode) | The request could not be authenticated due to either no credentials being provided or the provided credentials no longer being valid. Callers receiving this code should reauthenticate. | + | ACCESS_DENIED |
  • login
  • forbidden
| [APIM Error Code](https://fhir.nhs.uk/CodeSystem/NHSD-API-ErrorOrWarningCode) | The request could not be authenticated due to either no credentials being provided or the provided credentials no longer being valid. Callers receiving this code should reauthenticate. | headers: X-Correlation-ID: $ref: '../headers/response/CorrelationID.yaml' @@ -15,4 +15,4 @@ content: schema: $ref: '../NHSDigital-OperationOutcome.yaml' example: - $ref: '../../examples/NHSDigital-OperationOutcome-401.json' \ No newline at end of file + $ref: '../../examples/NHSDigital-OperationOutcome-401.json' diff --git a/specification/components/stu3/schemas/responses/Forbidden.yaml b/specification/components/stu3/schemas/responses/Forbidden.yaml index e16a9157a..7ab90f68c 100644 --- a/specification/components/stu3/schemas/responses/Forbidden.yaml +++ b/specification/components/stu3/schemas/responses/Forbidden.yaml @@ -2,9 +2,10 @@ description: | Where status code 403 (Forbidden) is returned then an eRS-OperationOutcome-1 will be included in the body, as detailed below. Check diagnostics property for specific information regarding the error. - | Error code | Description | - | ---------------------------------- | --------------------------------------------------------------------------------------------------------------------------- | - | FORBIDDEN | Access Forbidden. | + | issue.details.coding.code | issue.code | Coding System | Description | + | ------------------------- | ---------- | ------------------------------------------------------------------ | ---------------------------------------------------------------------------------- | + | FORBIDDEN | forbidden | [eRS Error Code](https://fhir.nhs.uk/CodeSystem/ers-error-codes) | A call attempts to access or operate upon a resource without proper authorisation. | + | NO_ACCESS | forbidden | [eRS Error Code](https://fhir.nhs.uk/CodeSystem/ers-error-codes) | The request could not be authenticated due to insufficient credentials being provided. | headers: X-Correlation-ID: $ref: '../headers/response/CorrelationID.yaml' diff --git a/specification/components/stu3/schemas/responses/Unauthorized.yaml b/specification/components/stu3/schemas/responses/Unauthorized.yaml index 9d440d867..1dc2279e5 100644 --- a/specification/components/stu3/schemas/responses/Unauthorized.yaml +++ b/specification/components/stu3/schemas/responses/Unauthorized.yaml @@ -1 +1,20 @@ -description: Unauthorized \ No newline at end of file +description: | + Where status code 401 (Unauthorised) is returned then an eRS-OperationOutcome-1 will be included in the body, as detailed below. + Check diagnostics property for specific information regarding the error. + + | issue.details.coding.code | issue.code | Coding System | Description | + | ------------------------- | ---------- | ------------------------------------------------------------------ | ---------------------------------------------------------------------------------- | + | NO_ACCESS |
  • login
  • forbidden
| [eRS Error Code](https://fhir.nhs.uk/CodeSystem/ers-error-codes) | The request could not be authenticated due to either no credentials being provided or the provided credentials no longer being valid. Callers receiving this code should reauthenticate. | +headers: + X-Correlation-ID: + $ref: '../headers/response/CorrelationID.yaml' + X-Request-ID: + $ref: '../headers/response/RequestID.yaml' + Content-Type: + $ref: '../headers/response/ContentTypeFhirJson.yaml' +content: + application/fhir+json: + schema: + $ref: '../STU3-OperationOutcome.yaml' + example: + $ref: '../../examples/STU3-Unauthorised.json'