From f9bf926f949f2c726c7792ac1df991adb0a2fa60 Mon Sep 17 00:00:00 2001 From: Ben Taylor <59649826+Vox-Ben@users.noreply.github.com> Date: Fri, 20 Feb 2026 11:39:12 +0000 Subject: [PATCH 01/18] Add pyjwt, clinical_jwt, make ruff happy --- gateway-api/poetry.lock | 4 ++-- gateway-api/src/fhir/bundle.py | 5 +++-- gateway-api/src/fhir/parameters.py | 5 +++-- gateway-api/src/fhir/patient.py | 9 +++++---- .../gateway_api/get_structured_record/test_request.py | 2 +- gateway-api/src/gateway_api/provider/test_client.py | 4 ++++ gateway-api/stubs/stubs/provider/stub.py | 5 +++-- gateway-api/tests/acceptance/conftest.py | 6 +++++- gateway-api/tests/conftest.py | 6 ++++-- gateway-api/tests/schema/test_openapi_schema.py | 2 +- ruff.toml | 2 ++ 11 files changed, 33 insertions(+), 17 deletions(-) diff --git a/gateway-api/poetry.lock b/gateway-api/poetry.lock index 288db038..8cb0276a 100644 --- a/gateway-api/poetry.lock +++ b/gateway-api/poetry.lock @@ -1,4 +1,4 @@ -# This file is automatically @generated by Poetry 2.2.1 and should not be changed by hand. +# This file is automatically @generated by Poetry 2.3.2 and should not be changed by hand. [[package]] name = "anyio" @@ -753,7 +753,7 @@ fqdn = {version = "*", optional = true, markers = "extra == \"format\""} idna = {version = "*", optional = true, markers = "extra == \"format\""} isoduration = {version = "*", optional = true, markers = "extra == \"format\""} jsonpointer = {version = ">1.13", optional = true, markers = "extra == \"format\""} -jsonschema-specifications = ">=2023.03.6" +jsonschema-specifications = ">=2023.3.6" referencing = ">=0.28.4" rfc3339-validator = {version = "*", optional = true, markers = "extra == \"format\""} rfc3987 = {version = "*", optional = true, markers = "extra == \"format\""} diff --git a/gateway-api/src/fhir/bundle.py b/gateway-api/src/fhir/bundle.py index 5fbc9a3b..b041d483 100644 --- a/gateway-api/src/fhir/bundle.py +++ b/gateway-api/src/fhir/bundle.py @@ -1,8 +1,9 @@ """FHIR Bundle resource.""" -from typing import TypedDict +from typing import TYPE_CHECKING, TypedDict -from fhir.patient import Patient +if TYPE_CHECKING: + from fhir.patient import Patient class BundleEntry(TypedDict): diff --git a/gateway-api/src/fhir/parameters.py b/gateway-api/src/fhir/parameters.py index 30b7cce8..abac7ebb 100644 --- a/gateway-api/src/fhir/parameters.py +++ b/gateway-api/src/fhir/parameters.py @@ -1,8 +1,9 @@ """FHIR Parameters resource.""" -from typing import TypedDict +from typing import TYPE_CHECKING, TypedDict -from fhir.identifier import Identifier +if TYPE_CHECKING: + from fhir.identifier import Identifier class Parameter(TypedDict): diff --git a/gateway-api/src/fhir/patient.py b/gateway-api/src/fhir/patient.py index 453a6f2a..0386e46c 100644 --- a/gateway-api/src/fhir/patient.py +++ b/gateway-api/src/fhir/patient.py @@ -1,10 +1,11 @@ """FHIR Patient resource.""" -from typing import NotRequired, TypedDict +from typing import TYPE_CHECKING, TypedDict, NotRequired -from fhir.general_practitioner import GeneralPractitioner -from fhir.human_name import HumanName -from fhir.identifier import Identifier +if TYPE_CHECKING: + from fhir.general_practitioner import GeneralPractitioner + from fhir.human_name import HumanName + from fhir.identifier import Identifier class Patient(TypedDict): diff --git a/gateway-api/src/gateway_api/get_structured_record/test_request.py b/gateway-api/src/gateway_api/get_structured_record/test_request.py index 34498655..c6f6fcf2 100644 --- a/gateway-api/src/gateway_api/get_structured_record/test_request.py +++ b/gateway-api/src/gateway_api/get_structured_record/test_request.py @@ -2,7 +2,6 @@ from typing import TYPE_CHECKING, cast import pytest -from fhir.parameters import Parameters from flask import Request from gateway_api.common.common import FlaskResponse @@ -12,6 +11,7 @@ if TYPE_CHECKING: from fhir.bundle import Bundle + from fhir.parameters import Parameters @pytest.fixture diff --git a/gateway-api/src/gateway_api/provider/test_client.py b/gateway-api/src/gateway_api/provider/test_client.py index 07d4e587..9e3fe307 100644 --- a/gateway-api/src/gateway_api/provider/test_client.py +++ b/gateway-api/src/gateway_api/provider/test_client.py @@ -19,6 +19,10 @@ from gateway_api.common.error import ProviderRequestFailedError from gateway_api.provider import GpProviderClient, client +if TYPE_CHECKING: + from requests import Response + from requests.structures import CaseInsensitiveDict + @pytest.fixture def stub() -> GpProviderStub: diff --git a/gateway-api/stubs/stubs/provider/stub.py b/gateway-api/stubs/stubs/provider/stub.py index a1f8d58d..9938b8c7 100644 --- a/gateway-api/stubs/stubs/provider/stub.py +++ b/gateway-api/stubs/stubs/provider/stub.py @@ -24,11 +24,12 @@ import json from typing import Any -from requests import Response - from stubs.base_stub import StubBase from stubs.data.bundles import Bundles +if TYPE_CHECKING: + from requests import Response + class GpProviderStub(StubBase): """ diff --git a/gateway-api/tests/acceptance/conftest.py b/gateway-api/tests/acceptance/conftest.py index 6fd4da3c..caeae1cd 100644 --- a/gateway-api/tests/acceptance/conftest.py +++ b/gateway-api/tests/acceptance/conftest.py @@ -1,5 +1,9 @@ +from typing import TYPE_CHECKING + import pytest -import requests + +if TYPE_CHECKING: + import requests class ResponseContext: diff --git a/gateway-api/tests/conftest.py b/gateway-api/tests/conftest.py index 00783a77..0c80696f 100644 --- a/gateway-api/tests/conftest.py +++ b/gateway-api/tests/conftest.py @@ -2,12 +2,14 @@ import os from datetime import timedelta -from typing import cast +from typing import TYPE_CHECKING, cast import pytest import requests from dotenv import find_dotenv, load_dotenv -from fhir.parameters import Parameters + +if TYPE_CHECKING: + from fhir.parameters import Parameters # Load environment variables from .env file in the workspace root # find_dotenv searches upward from current directory for .env file diff --git a/gateway-api/tests/schema/test_openapi_schema.py b/gateway-api/tests/schema/test_openapi_schema.py index 5e83b004..199da70a 100644 --- a/gateway-api/tests/schema/test_openapi_schema.py +++ b/gateway-api/tests/schema/test_openapi_schema.py @@ -8,7 +8,7 @@ import schemathesis import yaml -from schemathesis.generation.case import Case +from schemathesis.generation.case import Case # NOQA TC002 (Is needed) from schemathesis.openapi import from_dict # Load the OpenAPI schema from the local file diff --git a/ruff.toml b/ruff.toml index db28865d..09a8e9cd 100644 --- a/ruff.toml +++ b/ruff.toml @@ -1,3 +1,5 @@ +target-version = "py314" + [lint] select = [ # Standard configuration taken from https://docs.astral.sh/ruff/linter/#rule-selection. From be503462fee521dd5c0f8e892f81ef1e9545d841 Mon Sep 17 00:00:00 2001 From: Ben Taylor <59649826+Vox-Ben@users.noreply.github.com> Date: Thu, 26 Feb 2026 17:12:23 +0000 Subject: [PATCH 02/18] Review comments --- gateway-api/tests/acceptance/steps/happy_path.py | 9 ++++++--- gateway-api/tests/schema/test_openapi_schema.py | 5 ++++- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/gateway-api/tests/acceptance/steps/happy_path.py b/gateway-api/tests/acceptance/steps/happy_path.py index 88cd84af..a7d4d649 100644 --- a/gateway-api/tests/acceptance/steps/happy_path.py +++ b/gateway-api/tests/acceptance/steps/happy_path.py @@ -2,14 +2,17 @@ import json from datetime import timedelta +from typing import TYPE_CHECKING import requests -from fhir.parameters import Parameters from pytest_bdd import given, parsers, then, when from stubs.data.bundles import Bundles -from tests.acceptance.conftest import ResponseContext -from tests.conftest import Client +if TYPE_CHECKING: + from fhir.parameters import Parameters + + from tests.acceptance.conftest import ResponseContext + from tests.conftest import Client @given("the API is running") diff --git a/gateway-api/tests/schema/test_openapi_schema.py b/gateway-api/tests/schema/test_openapi_schema.py index 199da70a..bec928d2 100644 --- a/gateway-api/tests/schema/test_openapi_schema.py +++ b/gateway-api/tests/schema/test_openapi_schema.py @@ -5,12 +5,15 @@ """ from pathlib import Path +from typing import TYPE_CHECKING import schemathesis import yaml -from schemathesis.generation.case import Case # NOQA TC002 (Is needed) from schemathesis.openapi import from_dict +if TYPE_CHECKING: + from schemathesis.generation.case import Case + # Load the OpenAPI schema from the local file schema_path = Path(__file__).parent.parent.parent / "openapi.yaml" with open(schema_path) as f: From 78ef3f4ca0dca1e2d8b5c10d9110bce34cea888a Mon Sep 17 00:00:00 2001 From: Ben Taylor <59649826+Vox-Ben@users.noreply.github.com> Date: Mon, 2 Mar 2026 21:43:50 +0000 Subject: [PATCH 03/18] Make ruff happy, refactor controller JWT test --- gateway-api/src/fhir/bundle.py | 5 ++--- gateway-api/src/fhir/parameters.py | 5 ++--- gateway-api/src/fhir/patient.py | 9 ++++----- .../gateway_api/get_structured_record/test_request.py | 2 +- gateway-api/stubs/stubs/provider/stub.py | 5 ++--- gateway-api/tests/acceptance/conftest.py | 6 +----- gateway-api/tests/acceptance/steps/happy_path.py | 9 +++------ gateway-api/tests/conftest.py | 6 ++---- gateway-api/tests/schema/test_openapi_schema.py | 5 +---- ruff.toml | 2 -- 10 files changed, 18 insertions(+), 36 deletions(-) diff --git a/gateway-api/src/fhir/bundle.py b/gateway-api/src/fhir/bundle.py index b041d483..5fbc9a3b 100644 --- a/gateway-api/src/fhir/bundle.py +++ b/gateway-api/src/fhir/bundle.py @@ -1,9 +1,8 @@ """FHIR Bundle resource.""" -from typing import TYPE_CHECKING, TypedDict +from typing import TypedDict -if TYPE_CHECKING: - from fhir.patient import Patient +from fhir.patient import Patient class BundleEntry(TypedDict): diff --git a/gateway-api/src/fhir/parameters.py b/gateway-api/src/fhir/parameters.py index abac7ebb..30b7cce8 100644 --- a/gateway-api/src/fhir/parameters.py +++ b/gateway-api/src/fhir/parameters.py @@ -1,9 +1,8 @@ """FHIR Parameters resource.""" -from typing import TYPE_CHECKING, TypedDict +from typing import TypedDict -if TYPE_CHECKING: - from fhir.identifier import Identifier +from fhir.identifier import Identifier class Parameter(TypedDict): diff --git a/gateway-api/src/fhir/patient.py b/gateway-api/src/fhir/patient.py index 0386e46c..453a6f2a 100644 --- a/gateway-api/src/fhir/patient.py +++ b/gateway-api/src/fhir/patient.py @@ -1,11 +1,10 @@ """FHIR Patient resource.""" -from typing import TYPE_CHECKING, TypedDict, NotRequired +from typing import NotRequired, TypedDict -if TYPE_CHECKING: - from fhir.general_practitioner import GeneralPractitioner - from fhir.human_name import HumanName - from fhir.identifier import Identifier +from fhir.general_practitioner import GeneralPractitioner +from fhir.human_name import HumanName +from fhir.identifier import Identifier class Patient(TypedDict): diff --git a/gateway-api/src/gateway_api/get_structured_record/test_request.py b/gateway-api/src/gateway_api/get_structured_record/test_request.py index c6f6fcf2..34498655 100644 --- a/gateway-api/src/gateway_api/get_structured_record/test_request.py +++ b/gateway-api/src/gateway_api/get_structured_record/test_request.py @@ -2,6 +2,7 @@ from typing import TYPE_CHECKING, cast import pytest +from fhir.parameters import Parameters from flask import Request from gateway_api.common.common import FlaskResponse @@ -11,7 +12,6 @@ if TYPE_CHECKING: from fhir.bundle import Bundle - from fhir.parameters import Parameters @pytest.fixture diff --git a/gateway-api/stubs/stubs/provider/stub.py b/gateway-api/stubs/stubs/provider/stub.py index 9938b8c7..a1f8d58d 100644 --- a/gateway-api/stubs/stubs/provider/stub.py +++ b/gateway-api/stubs/stubs/provider/stub.py @@ -24,12 +24,11 @@ import json from typing import Any +from requests import Response + from stubs.base_stub import StubBase from stubs.data.bundles import Bundles -if TYPE_CHECKING: - from requests import Response - class GpProviderStub(StubBase): """ diff --git a/gateway-api/tests/acceptance/conftest.py b/gateway-api/tests/acceptance/conftest.py index caeae1cd..6fd4da3c 100644 --- a/gateway-api/tests/acceptance/conftest.py +++ b/gateway-api/tests/acceptance/conftest.py @@ -1,9 +1,5 @@ -from typing import TYPE_CHECKING - import pytest - -if TYPE_CHECKING: - import requests +import requests class ResponseContext: diff --git a/gateway-api/tests/acceptance/steps/happy_path.py b/gateway-api/tests/acceptance/steps/happy_path.py index a7d4d649..88cd84af 100644 --- a/gateway-api/tests/acceptance/steps/happy_path.py +++ b/gateway-api/tests/acceptance/steps/happy_path.py @@ -2,17 +2,14 @@ import json from datetime import timedelta -from typing import TYPE_CHECKING import requests +from fhir.parameters import Parameters from pytest_bdd import given, parsers, then, when from stubs.data.bundles import Bundles -if TYPE_CHECKING: - from fhir.parameters import Parameters - - from tests.acceptance.conftest import ResponseContext - from tests.conftest import Client +from tests.acceptance.conftest import ResponseContext +from tests.conftest import Client @given("the API is running") diff --git a/gateway-api/tests/conftest.py b/gateway-api/tests/conftest.py index 0c80696f..00783a77 100644 --- a/gateway-api/tests/conftest.py +++ b/gateway-api/tests/conftest.py @@ -2,14 +2,12 @@ import os from datetime import timedelta -from typing import TYPE_CHECKING, cast +from typing import cast import pytest import requests from dotenv import find_dotenv, load_dotenv - -if TYPE_CHECKING: - from fhir.parameters import Parameters +from fhir.parameters import Parameters # Load environment variables from .env file in the workspace root # find_dotenv searches upward from current directory for .env file diff --git a/gateway-api/tests/schema/test_openapi_schema.py b/gateway-api/tests/schema/test_openapi_schema.py index bec928d2..5e83b004 100644 --- a/gateway-api/tests/schema/test_openapi_schema.py +++ b/gateway-api/tests/schema/test_openapi_schema.py @@ -5,15 +5,12 @@ """ from pathlib import Path -from typing import TYPE_CHECKING import schemathesis import yaml +from schemathesis.generation.case import Case from schemathesis.openapi import from_dict -if TYPE_CHECKING: - from schemathesis.generation.case import Case - # Load the OpenAPI schema from the local file schema_path = Path(__file__).parent.parent.parent / "openapi.yaml" with open(schema_path) as f: diff --git a/ruff.toml b/ruff.toml index 09a8e9cd..db28865d 100644 --- a/ruff.toml +++ b/ruff.toml @@ -1,5 +1,3 @@ -target-version = "py314" - [lint] select = [ # Standard configuration taken from https://docs.astral.sh/ruff/linter/#rule-selection. From b2e9906289d0fb24855d7e7e22c74ee19baa9db7 Mon Sep 17 00:00:00 2001 From: Ben Taylor <59649826+Vox-Ben@users.noreply.github.com> Date: Wed, 4 Mar 2026 10:35:22 +0000 Subject: [PATCH 04/18] Start of orange box connection --- gateway-api/src/gateway_api/common/common.py | 6 ++++++ gateway-api/src/gateway_api/controller.py | 15 +++++++++++---- gateway-api/src/gateway_api/provider/client.py | 7 ++++++- 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/gateway-api/src/gateway_api/common/common.py b/gateway-api/src/gateway_api/common/common.py index 3891b8f3..f9452f1a 100644 --- a/gateway-api/src/gateway_api/common/common.py +++ b/gateway-api/src/gateway_api/common/common.py @@ -4,6 +4,7 @@ import re from dataclasses import dataclass +from http import HTTPStatus # This project uses JSON request/response bodies as strings in the controller layer. # The alias is used to make intent clearer in function signatures. @@ -61,3 +62,8 @@ def validate_nhs_number(value: str | int) -> bool: return False # invalid NHS number return check == provided_check_digit + + +def get_http_text(status_code: int) -> str: + status = HTTPStatus(status_code) + return status.phrase diff --git a/gateway-api/src/gateway_api/controller.py b/gateway-api/src/gateway_api/controller.py index 78568d0c..576578d2 100644 --- a/gateway-api/src/gateway_api/controller.py +++ b/gateway-api/src/gateway_api/controller.py @@ -95,11 +95,18 @@ def get_jwt_for_provider(self, provider_endpoint: str, consumer_ods: str) -> JWT # https://webarchive.nationalarchives.gov.uk/ukgwa/20250307092533/https://developer.nhs.uk/apis/gpconnect/integration_cross_organisation_audit_and_provenance.html#requesting_practitioner-claim # TODO: Get requesting device details from consumer, somehow? + # requesting_device = Device( + # system="https://consumersupplier.com/Id/device-identifier", + # value="CONS-APP-4", + # model="Consumer product name", + # version="5.3.0", + # ) + requesting_device = Device( - system="https://consumersupplier.com/Id/device-identifier", - value="CONS-APP-4", - model="Consumer product name", - version="5.3.0", + system="https://orange.testlab.nhs.uk/gpconnect-demonstrator/Id/local-system-instance-id", + value="gpcdemonstrator-1-orange", + model="GP Connect Demonstrator", + version="1.5.0", ) # TODO: Get practitioner details from consumer, somehow? diff --git a/gateway-api/src/gateway_api/provider/client.py b/gateway-api/src/gateway_api/provider/client.py index 4b6c7e53..467a38d9 100644 --- a/gateway-api/src/gateway_api/provider/client.py +++ b/gateway-api/src/gateway_api/provider/client.py @@ -28,6 +28,7 @@ from requests import HTTPError, Response from gateway_api.clinical_jwt import JWT +from gateway_api.common.common import get_http_text from gateway_api.common.error import ProviderRequestFailedError from gateway_api.get_structured_record import ACCESS_RECORD_STRUCTURED_INTERACTION_ID @@ -114,6 +115,10 @@ def access_structured_record( try: response.raise_for_status() except HTTPError as err: - raise ProviderRequestFailedError(error_reason=err.response.reason) from err + errstr = "GPProvider FHIR API request failed:\n" + errstr += f"{response.status_code}: " + errstr += f"{get_http_text(response.status_code)}: {response.reason}\n" + errstr += response.text + raise ProviderRequestFailedError(error_reason=errstr) from err return response From e0e8212cfb27e360cf38c87ab3e729897cdf1526 Mon Sep 17 00:00:00 2001 From: Ben Taylor <59649826+Vox-Ben@users.noreply.github.com> Date: Thu, 5 Mar 2026 22:24:06 +0000 Subject: [PATCH 05/18] Orange box working with debug script --- .../src/gateway_api/clinical_jwt/__init__.py | 3 +- .../src/gateway_api/clinical_jwt/device.py | 35 ++++++++------ .../src/gateway_api/clinical_jwt/jwt.py | 6 +-- .../gateway_api/clinical_jwt/organization.py | 36 ++++++++++++++ .../src/gateway_api/clinical_jwt/test_jwt.py | 48 +++++++++---------- gateway-api/src/gateway_api/controller.py | 14 ++++-- .../src/gateway_api/provider/client.py | 29 +++++++---- .../src/gateway_api/provider/test_client.py | 6 +-- .../src/gateway_api/test_controller.py | 2 +- gateway-api/stubs/stubs/sds/stub.py | 21 ++++++++ 10 files changed, 139 insertions(+), 61 deletions(-) create mode 100644 gateway-api/src/gateway_api/clinical_jwt/organization.py diff --git a/gateway-api/src/gateway_api/clinical_jwt/__init__.py b/gateway-api/src/gateway_api/clinical_jwt/__init__.py index 5b6effaf..63d06bf2 100644 --- a/gateway-api/src/gateway_api/clinical_jwt/__init__.py +++ b/gateway-api/src/gateway_api/clinical_jwt/__init__.py @@ -1,5 +1,6 @@ from .device import Device from .jwt import JWT +from .organization import Organization from .practitioner import Practitioner -__all__ = ["JWT", "Device", "Practitioner"] +__all__ = ["JWT", "Device", "Organization", "Practitioner"] diff --git a/gateway-api/src/gateway_api/clinical_jwt/device.py b/gateway-api/src/gateway_api/clinical_jwt/device.py index d0d28864..2c733103 100644 --- a/gateway-api/src/gateway_api/clinical_jwt/device.py +++ b/gateway-api/src/gateway_api/clinical_jwt/device.py @@ -1,4 +1,5 @@ from dataclasses import dataclass +from typing import Any @dataclass(frozen=True, kw_only=True) @@ -8,22 +9,26 @@ class Device: model: str version: str + def to_dict(self) -> dict[str, Any]: + """ + Return the Device as a dictionary suitable for JWT payload. + """ + return { + "resourceType": "Device", + "identifier": [{"system": self.system, "value": self.value}], + "model": self.model, + "version": self.version, + } + @property - def json(self) -> str: - outstr = f""" - {{ - "resourceType": "Device", - "identifier": [ - {{ - "system": "{self.system}", - "value": "{self.value}" - }} - ], - "model": "{self.model}", - "version": "{self.version}" - }} + def json(self) -> dict[str, Any]: """ - return outstr.strip() + Return the Device as a dictionary suitable for JWT payload. + Provided for backwards compatibility. + """ + return self.to_dict() def __str__(self) -> str: - return self.json + import json + + return json.dumps(self.to_dict(), indent=2) diff --git a/gateway-api/src/gateway_api/clinical_jwt/jwt.py b/gateway-api/src/gateway_api/clinical_jwt/jwt.py index c8ec78cb..f965d4df 100644 --- a/gateway-api/src/gateway_api/clinical_jwt/jwt.py +++ b/gateway-api/src/gateway_api/clinical_jwt/jwt.py @@ -11,9 +11,9 @@ class JWT: issuer: str subject: str audience: str - requesting_device: str - requesting_organization: str - requesting_practitioner: str + requesting_device: dict[str, Any] + requesting_organization: dict[str, Any] + requesting_practitioner: dict[str, Any] # Time fields issued_at: int = field(default_factory=lambda: int(time())) diff --git a/gateway-api/src/gateway_api/clinical_jwt/organization.py b/gateway-api/src/gateway_api/clinical_jwt/organization.py new file mode 100644 index 00000000..18f69354 --- /dev/null +++ b/gateway-api/src/gateway_api/clinical_jwt/organization.py @@ -0,0 +1,36 @@ +from dataclasses import dataclass +from typing import Any + + +@dataclass(frozen=True, kw_only=True) +class Organization: + ods_code: str + name: str + + def to_dict(self) -> dict[str, Any]: + """ + Return the Organization as a dictionary suitable for JWT payload. + """ + return { + "resourceType": "Organization", + "identifier": [ + { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": self.ods_code, + } + ], + "name": self.name, + } + + @property + def json(self) -> dict[str, Any]: + """ + Return the Organization as a dictionary suitable for JWT payload. + Provided for backwards compatibility. + """ + return self.to_dict() + + def __str__(self) -> str: + import json + + return json.dumps(self.to_dict(), indent=2) diff --git a/gateway-api/src/gateway_api/clinical_jwt/test_jwt.py b/gateway-api/src/gateway_api/clinical_jwt/test_jwt.py index 418fb039..a8f0f038 100644 --- a/gateway-api/src/gateway_api/clinical_jwt/test_jwt.py +++ b/gateway-api/src/gateway_api/clinical_jwt/test_jwt.py @@ -18,17 +18,17 @@ def test_jwt_creation_with_required_fields() -> None: issuer="https://example.com", subject="user-123", audience="https://provider.example.com", - requesting_device='{"device": "info"}', - requesting_organization="ORG-123", - requesting_practitioner='{"practitioner": "info"}', + requesting_device={"device": "info"}, + requesting_organization={"org": "info"}, + requesting_practitioner={"practitioner": "info"}, ) assert token.issuer == "https://example.com" assert token.subject == "user-123" assert token.audience == "https://provider.example.com" - assert token.requesting_device == '{"device": "info"}' - assert token.requesting_organization == "ORG-123" - assert token.requesting_practitioner == '{"practitioner": "info"}' + assert token.requesting_device == {"device": "info"} + assert token.requesting_organization == {"org": "info"} + assert token.requesting_practitioner == {"practitioner": "info"} assert token.algorithm is None assert token.type == "JWT" assert token.reason_for_request == "directcare" @@ -46,9 +46,9 @@ def test_jwt_default_issued_at_and_expiration(mock_time: Mock) -> None: issuer="https://example.com", subject="user-123", audience="https://provider.example.com", - requesting_device='{"device": "info"}', - requesting_organization="ORG-123", - requesting_practitioner='{"practitioner": "info"}', + requesting_device={"device": "info"}, + requesting_organization={"org": "info"}, + requesting_practitioner={"practitioner": "info"}, ) assert token.issued_at == 1000 @@ -63,9 +63,9 @@ def test_jwt_issue_time_property() -> None: issuer="https://example.com", subject="user-123", audience="https://provider.example.com", - requesting_device='{"device": "info"}', - requesting_organization="ORG-123", - requesting_practitioner='{"practitioner": "info"}', + requesting_device={"device": "info"}, + requesting_organization={"org": "info"}, + requesting_practitioner={"practitioner": "info"}, issued_at=1609459200, # 2021-01-01 00:00:00 UTC ) @@ -80,9 +80,9 @@ def test_jwt_exp_time_property() -> None: issuer="https://example.com", subject="user-123", audience="https://provider.example.com", - requesting_device='{"device": "info"}', - requesting_organization="ORG-123", - requesting_practitioner='{"practitioner": "info"}', + requesting_device={"device": "info"}, + requesting_organization={"org": "info"}, + requesting_practitioner={"practitioner": "info"}, expiration=1609459500, # 2021-01-01 00:05:00 UTC ) @@ -97,9 +97,9 @@ def test_jwt_payload_contains_all_required_fields() -> None: issuer="https://example.com", subject="user-123", audience="https://provider.example.com", - requesting_device='{"device": "info"}', - requesting_organization="ORG-123", - requesting_practitioner='{"practitioner": "info"}', + requesting_device={"device": "info"}, + requesting_organization={"org": "info"}, + requesting_practitioner={"practitioner": "info"}, issued_at=1000, expiration=1300, ) @@ -130,9 +130,9 @@ def test_jwt_encode_returns_string() -> None: issuer="https://example.com", subject="user-123", audience="https://provider.example.com", - requesting_device='{"device": "info"}', - requesting_organization="ORG-123", - requesting_practitioner='{"practitioner": "info"}', + requesting_device={"device": "info"}, + requesting_organization={"org": "info"}, + requesting_practitioner={"practitioner": "info"}, issued_at=1000, expiration=1300, ) @@ -159,9 +159,9 @@ def test_jwt_decode_reconstructs_token() -> None: issuer="https://example.com", subject="user-123", audience="https://provider.example.com", - requesting_device='{"device": "info"}', - requesting_organization="ORG-123", - requesting_practitioner='{"practitioner": "info"}', + requesting_device={"device": "info"}, + requesting_organization={"org": "info"}, + requesting_practitioner={"practitioner": "info"}, issued_at=1000, expiration=1300, ) diff --git a/gateway-api/src/gateway_api/controller.py b/gateway-api/src/gateway_api/controller.py index 576578d2..631e2e02 100644 --- a/gateway-api/src/gateway_api/controller.py +++ b/gateway-api/src/gateway_api/controller.py @@ -2,7 +2,7 @@ Controller layer for orchestrating calls to external services """ -from gateway_api.clinical_jwt import JWT, Device, Practitioner +from gateway_api.clinical_jwt import JWT, Device, Organization, Practitioner from gateway_api.common.common import FlaskResponse from gateway_api.common.error import ( NoAsidFoundError, @@ -121,18 +121,22 @@ def get_jwt_for_provider(self, provider_endpoint: str, consumer_ods: str) -> JWT prefix="Mr", ) + # TODO: Get consumer organization details properly + requesting_organization = Organization( + ods_code=consumer_ods, name="Consumer organisation name" + ) + # TODO: Get consumer URL for issuer. Use CDG API URL for now. issuer = "https://clinical-data-gateway-api.sandbox.nhs.uk" audience = provider_endpoint - requesting_organization = consumer_ods token = JWT( issuer=issuer, subject=requesting_practitioner.id, audience=audience, - requesting_device=requesting_device.json, - requesting_organization=requesting_organization, - requesting_practitioner=requesting_practitioner.json, + requesting_device=requesting_device.to_dict(), + requesting_organization=requesting_organization.to_dict(), + requesting_practitioner=requesting_practitioner.to_dict(), ) return token diff --git a/gateway-api/src/gateway_api/provider/client.py b/gateway-api/src/gateway_api/provider/client.py index 467a38d9..360301c1 100644 --- a/gateway-api/src/gateway_api/provider/client.py +++ b/gateway-api/src/gateway_api/provider/client.py @@ -44,9 +44,8 @@ provider_stub = GpProviderStub() post = provider_stub.post # type: ignore -ARS_FHIR_BASE = "FHIR/STU3" -FHIR_RESOURCE = "patient" -ARS_FHIR_OPERATION = "$gpc.getstructuredrecord" +# Default endpoint path for access record structured interaction (standard GP Connect) +ARS_ENDPOINT_PATH = "FHIR/STU3/patient/$gpc.getstructuredrecord" TIMEOUT: int | None = None # None used for quicker dev, adjust as needed @@ -58,10 +57,12 @@ class GpProviderClient: including fetching structured patient records. Attributes: - provider_endpoint (str): The FHIR API endpoint for the provider. + provider_endpoint (str): The base URL for the provider (from SDS). provider_asid (str): The ASID for the provider. consumer_asid (str): The ASID for the consumer. token (JWT): JWT object for authentication with the provider API. + endpoint_path (str): The endpoint path for the operation + (default: "FHIR/STU3/patient/$gpc.getstructuredrecord"). Methods: access_structured_record(trace_id: str, body: str) -> Response: @@ -69,12 +70,18 @@ class GpProviderClient: """ def __init__( - self, provider_endpoint: str, provider_asid: str, consumer_asid: str, token: JWT + self, + provider_endpoint: str, + provider_asid: str, + consumer_asid: str, + token: JWT, + endpoint_path: str = ARS_ENDPOINT_PATH, ) -> None: self.provider_endpoint = provider_endpoint self.provider_asid = provider_asid self.consumer_asid = consumer_asid self.token = token + self.endpoint_path = endpoint_path def _build_headers(self, trace_id: str) -> dict[str, str]: """ @@ -82,8 +89,8 @@ def _build_headers(self, trace_id: str) -> dict[str, str]: """ # TODO: Post-steel-thread, probably check whether JWT is valid/not expired return { - "Content-Type": "application/fhir+json", - "Accept": "application/fhir+json", + "Content-Type": "application/fhir+json;charset=utf-8", + "Accept": "application/fhir+json;charset=utf-8", "Ssp-InteractionID": ACCESS_RECORD_STRUCTURED_INTERACTION_ID, "Ssp-To": self.provider_asid, "Ssp-From": self.consumer_asid, @@ -102,8 +109,7 @@ def access_structured_record( headers = self._build_headers(trace_id) - endpoint_path = "/".join([ARS_FHIR_BASE, FHIR_RESOURCE, ARS_FHIR_OPERATION]) - url = urljoin(self.provider_endpoint, endpoint_path) + url = urljoin(self.provider_endpoint, self.endpoint_path) response = post( url, @@ -119,6 +125,11 @@ def access_structured_record( errstr += f"{response.status_code}: " errstr += f"{get_http_text(response.status_code)}: {response.reason}\n" errstr += response.text + errstr += "\nHeaders were:\n" + for header, value in headers.items(): + errstr += f"{header}: {value}\n" + errstr += "\nBody payload was:\n" + errstr += body raise ProviderRequestFailedError(error_reason=errstr) from err return response diff --git a/gateway-api/src/gateway_api/provider/test_client.py b/gateway-api/src/gateway_api/provider/test_client.py index 9e3fe307..f1a59165 100644 --- a/gateway-api/src/gateway_api/provider/test_client.py +++ b/gateway-api/src/gateway_api/provider/test_client.py @@ -69,9 +69,9 @@ def dummy_jwt() -> JWT: issuer="https://example.com", subject="user-123", audience="https://provider.example.com", - requesting_device='{"device": "info"}', - requesting_organization="ORG-123", - requesting_practitioner='{"practitioner": "info"}', + requesting_device={"device": "info"}, + requesting_organization={"org": "info"}, + requesting_practitioner={"practitioner": "info"}, ) diff --git a/gateway-api/src/gateway_api/test_controller.py b/gateway-api/src/gateway_api/test_controller.py index 257a7d95..a55abbc2 100644 --- a/gateway-api/src/gateway_api/test_controller.py +++ b/gateway-api/src/gateway_api/test_controller.py @@ -349,4 +349,4 @@ def test_controller_creates_jwt_token_with_correct_claims( assert jwt_token.audience == provider_endpoint # Verify the requesting organization matches the consumer ODS - assert jwt_token.requesting_organization == consumer_ods + assert jwt_token.requesting_organization["identifier"][0]["value"] == consumer_ods diff --git a/gateway-api/stubs/stubs/sds/stub.py b/gateway-api/stubs/stubs/sds/stub.py index afbd6e3b..b95a5206 100644 --- a/gateway-api/stubs/stubs/sds/stub.py +++ b/gateway-api/stubs/stubs/sds/stub.py @@ -403,6 +403,20 @@ def _seed_default_devices(self) -> None: "asid": "ASIDforGPWithoutEndpoint", "display": "GP with no provider endpoint - testing error handling", }, + { + "org_ods": "S44444", + "party_key": "S44444-0000809", + "device_id": "B2B2E921-92CA-4A88-A550-2DBB36F703AF", + "asid": "200000000359", + "display": "Dummy ODS/ASID for Orange Box", + }, + { + "org_ods": "S55555", + "party_key": "S55555-0000809", + "device_id": "B3B3E921-92CA-4A88-A550-2DBB36F703AF", + "asid": "918999198738", + "display": "Dummy ODS/ASID for Orange Box", + }, ] # Iterate through test data and create devices @@ -445,6 +459,13 @@ def _seed_default_endpoints(self) -> None: "asid": "asid_A12345", "address": "https://a12345.example.com/fhir", }, + { + "org_ods": "S55555", + "party_key": "S55555-0000809", + "endpoint_id": "E3E3E921-92CA-4A88-A550-2DBB36F703AF", + "asid": "918999198738", + "address": "https://orange.testlab.nhs.uk/B82617/STU3/1/gpconnect/structured/fhir/", + }, ] # Iterate through test data and create endpoints From 8d9e6d9e526228cb17308652d3e7678fff795add Mon Sep 17 00:00:00 2001 From: Ben Taylor <59649826+Vox-Ben@users.noreply.github.com> Date: Fri, 6 Mar 2026 16:21:30 +0000 Subject: [PATCH 06/18] Orange box connection working --- .../src/gateway_api/provider/client.py | 2 +- .../orange_box_trigger_9690937278.json | 34 +++++++++++++++++++ .../stubs/stubs/data/patients/patients.py | 1 + gateway-api/stubs/stubs/pds/stub.py | 1 + gateway-api/stubs/stubs/sds/stub.py | 2 +- 5 files changed, 38 insertions(+), 2 deletions(-) create mode 100644 gateway-api/stubs/stubs/data/patients/orange_box_trigger_9690937278.json diff --git a/gateway-api/src/gateway_api/provider/client.py b/gateway-api/src/gateway_api/provider/client.py index 360301c1..fca1c465 100644 --- a/gateway-api/src/gateway_api/provider/client.py +++ b/gateway-api/src/gateway_api/provider/client.py @@ -45,7 +45,7 @@ post = provider_stub.post # type: ignore # Default endpoint path for access record structured interaction (standard GP Connect) -ARS_ENDPOINT_PATH = "FHIR/STU3/patient/$gpc.getstructuredrecord" +ARS_ENDPOINT_PATH = "Patient/$gpc.getstructuredrecord" TIMEOUT: int | None = None # None used for quicker dev, adjust as needed diff --git a/gateway-api/stubs/stubs/data/patients/orange_box_trigger_9690937278.json b/gateway-api/stubs/stubs/data/patients/orange_box_trigger_9690937278.json new file mode 100644 index 00000000..1660e792 --- /dev/null +++ b/gateway-api/stubs/stubs/data/patients/orange_box_trigger_9690937278.json @@ -0,0 +1,34 @@ +{ + "resourceType": "Patient", + "id": "9690937278", + "meta": { + "versionId": "1", + "lastUpdated": "2020-01-01T00:00:00Z" + }, + "identifier": [ + { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "9690937278" + } + ], + "name": [ + { + "use": "official", + "family": "Samual", + "given": ["Lucien"], + "period": {"start": "1900-01-01", "end": "9999-12-31"} + } + ], + "gender": "male", + "birthDate": "1938-12-11", + "generalPractitioner": [ + { + "id": "1", + "type": "Organization", + "identifier": { + "value": "S55555", + "period": {"start": "2020-01-01", "end": "9999-12-31"} + } + } + ] +} diff --git a/gateway-api/stubs/stubs/data/patients/patients.py b/gateway-api/stubs/stubs/data/patients/patients.py index e21ce2d1..1b6a5a0d 100644 --- a/gateway-api/stubs/stubs/data/patients/patients.py +++ b/gateway-api/stubs/stubs/data/patients/patients.py @@ -26,3 +26,4 @@ def load_patient(filename: str) -> dict[str, Any]: "blank_endpoint_sds_result_9000000013.json" ) ALICE_JONES_9999999999 = load_patient("alice_jones_9999999999.json") + ORANGE_BOX_TRIGGER_9690937278 = load_patient("orange_box_trigger_9690937278.json") diff --git a/gateway-api/stubs/stubs/pds/stub.py b/gateway-api/stubs/stubs/pds/stub.py index ed88b446..c7c29014 100644 --- a/gateway-api/stubs/stubs/pds/stub.py +++ b/gateway-api/stubs/stubs/pds/stub.py @@ -52,6 +52,7 @@ def __init__(self, strict_headers: bool = True) -> None: ("9000000011", Patients.BLANK_ASID_SDS_RESULT_9000000011), ("9000000012", Patients.INDUCE_PROVIDER_ERROR_9000000012), ("9000000013", Patients.BLANK_ENDPOINT_SDS_RESULT_9000000013), + ("9690937278", Patients.ORANGE_BOX_TRIGGER_9690937278), ] for nhs_number, patient in test_patients: self.upsert_patient( diff --git a/gateway-api/stubs/stubs/sds/stub.py b/gateway-api/stubs/stubs/sds/stub.py index b95a5206..fdd04f5c 100644 --- a/gateway-api/stubs/stubs/sds/stub.py +++ b/gateway-api/stubs/stubs/sds/stub.py @@ -415,7 +415,7 @@ def _seed_default_devices(self) -> None: "party_key": "S55555-0000809", "device_id": "B3B3E921-92CA-4A88-A550-2DBB36F703AF", "asid": "918999198738", - "display": "Dummy ODS/ASID for Orange Box", + "display": "ODS/ASID triggering Orange Box", }, ] From 3debed40b40e941bcb1942212fb82450bfbfb870 Mon Sep 17 00:00:00 2001 From: Ben Taylor <59649826+Vox-Ben@users.noreply.github.com> Date: Fri, 6 Mar 2026 17:36:57 +0000 Subject: [PATCH 07/18] Pass $STUB_PROVIDER --- Makefile | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 40b5f7fe..cf612034 100644 --- a/Makefile +++ b/Makefile @@ -56,9 +56,17 @@ publish: # Publish the project artefact @Pipeline deploy: clean build # Deploy the project artefact to the target environment @Pipeline @if [[ -n "$${IN_BUILD_CONTAINER}" ]]; then \ echo "Starting using local docker network ..." ; \ - $(docker) run --platform linux/amd64 --name gateway-api -p 5000:8080 --network gateway-local -d ${IMAGE_NAME} ; \ + if [[ -n "$${STUB_PROVIDER}" ]]; then \ + $(docker) run --platform linux/amd64 --name gateway-api -p 5000:8080 --network gateway-local -e STUB_PROVIDER=$${STUB_PROVIDER} -d ${IMAGE_NAME} ; \ + else \ + $(docker) run --platform linux/amd64 --name gateway-api -p 5000:8080 --network gateway-local -d ${IMAGE_NAME} ; \ + fi ; \ else \ - $(docker) run --platform linux/amd64 --name gateway-api -p 5000:8080 -d ${IMAGE_NAME} ; \ + if [[ -n "$${STUB_PROVIDER}" ]]; then \ + $(docker) run --platform linux/amd64 --name gateway-api -p 5000:8080 -e STUB_PROVIDER=$${STUB_PROVIDER} -d ${IMAGE_NAME} ; \ + else \ + $(docker) run --platform linux/amd64 --name gateway-api -p 5000:8080 -d ${IMAGE_NAME} ; \ + fi ; \ fi clean:: stop # Clean-up project resources (main) @Operations From 2a8431112163d0679ec780297f9d2032ddfd1486 Mon Sep 17 00:00:00 2001 From: Ben Taylor <59649826+Vox-Ben@users.noreply.github.com> Date: Fri, 6 Mar 2026 17:49:44 +0000 Subject: [PATCH 08/18] Pass STUB env vars --- Makefile | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/Makefile b/Makefile index cf612034..e5ad5732 100644 --- a/Makefile +++ b/Makefile @@ -54,19 +54,21 @@ publish: # Publish the project artefact @Pipeline # TODO: Implement the artefact publishing step deploy: clean build # Deploy the project artefact to the target environment @Pipeline - @if [[ -n "$${IN_BUILD_CONTAINER}" ]]; then \ + @PROVIDER_STRING="" ; \ + if [[ -n "$${STUB_PROVIDER}" ]]; then \ + PROVIDER_STRING="$${PROVIDER_STRING} -e STUB_PROVIDER=$${STUB_PROVIDER}" ; \ + fi ; \ + if [[ -n "$${STUB_PDS}" ]]; then \ + PROVIDER_STRING="$${PROVIDER_STRING} -e STUB_PDS=$${STUB_PDS}" ; \ + fi ; \ + if [[ -n "$${STUB_SDS}" ]]; then \ + PROVIDER_STRING="$${PROVIDER_STRING} -e STUB_SDS=$${STUB_SDS}" ; \ + fi ; \ + if [[ -n "$${IN_BUILD_CONTAINER}" ]]; then \ echo "Starting using local docker network ..." ; \ - if [[ -n "$${STUB_PROVIDER}" ]]; then \ - $(docker) run --platform linux/amd64 --name gateway-api -p 5000:8080 --network gateway-local -e STUB_PROVIDER=$${STUB_PROVIDER} -d ${IMAGE_NAME} ; \ - else \ - $(docker) run --platform linux/amd64 --name gateway-api -p 5000:8080 --network gateway-local -d ${IMAGE_NAME} ; \ - fi ; \ + $(docker) run --platform linux/amd64 --name gateway-api -p 5000:8080 --network gateway-local $${PROVIDER_STRING} -d ${IMAGE_NAME} ; \ else \ - if [[ -n "$${STUB_PROVIDER}" ]]; then \ - $(docker) run --platform linux/amd64 --name gateway-api -p 5000:8080 -e STUB_PROVIDER=$${STUB_PROVIDER} -d ${IMAGE_NAME} ; \ - else \ - $(docker) run --platform linux/amd64 --name gateway-api -p 5000:8080 -d ${IMAGE_NAME} ; \ - fi ; \ + $(docker) run --platform linux/amd64 --name gateway-api -p 5000:8080 $${PROVIDER_STRING} -d ${IMAGE_NAME} ; \ fi clean:: stop # Clean-up project resources (main) @Operations From 22a7499cc34f35e4250a88ccbf540c00b3adb1ba Mon Sep 17 00:00:00 2001 From: Ben Taylor <59649826+Vox-Ben@users.noreply.github.com> Date: Fri, 6 Mar 2026 17:54:18 +0000 Subject: [PATCH 09/18] Create call script --- gateway-api/scripts/call_gateway.py | 71 +++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 gateway-api/scripts/call_gateway.py diff --git a/gateway-api/scripts/call_gateway.py b/gateway-api/scripts/call_gateway.py new file mode 100644 index 00000000..1f9ca25e --- /dev/null +++ b/gateway-api/scripts/call_gateway.py @@ -0,0 +1,71 @@ +#!/usr/bin/env python3 + +import argparse +import json +import os +import sys +from uuid import uuid4 + +import requests + + +def main() -> None: + # Parse command-line arguments + parser = argparse.ArgumentParser( + description="POST request to GPC getstructuredrecord endpoint" + ) + parser.add_argument( + "nhs_number", help="NHS number to search for (e.g., 9690937278)" + ) + args = parser.parse_args() + + # Check if BASE_URL is set + base_url = os.environ.get("BASE_URL") + if not base_url: + print("Error: BASE_URL environment variable is not set") + sys.exit(1) + + # Endpoint URL + url = f"{base_url}/patient/$gpc.getstructuredrecord" + + # Request headers + headers = { + "Content-Type": "application/fhir+json", + "Accept": "application/fhir+json", + "Ssp-TraceID": str(uuid4()), + "Ods-From": "S44444", + } + + # Request body + payload = { + "resourceType": "Parameters", + "parameter": [ + { + "name": "patientNHSNumber", + "valueIdentifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": args.nhs_number, + }, + } + ], + } + + # Make the POST request + try: + response = requests.post(url, headers=headers, json=payload, timeout=10) + response.raise_for_status() + + print(f"Status Code: {response.status_code}") + print(f"Response:\n{json.dumps(response.json(), indent=2)}") + + except requests.exceptions.RequestException as e: + errtext = f"Error: {e}\n" + if e.response is not None: + errtext += f"Status Code: {e.response.status_code}\n" + errtext += f"Response Body: {e.response.text}\n" + print(errtext) + sys.exit(1) + + +if __name__ == "__main__": + main() From d6db976b735647cc6e97080ed9f955d22d8a3cf8 Mon Sep 17 00:00:00 2001 From: Ben Taylor <59649826+Vox-Ben@users.noreply.github.com> Date: Mon, 9 Mar 2026 21:17:17 +0000 Subject: [PATCH 10/18] Fix algorithm string none --- gateway-api/src/gateway_api/clinical_jwt/jwt.py | 2 +- gateway-api/src/gateway_api/clinical_jwt/test_jwt.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/gateway-api/src/gateway_api/clinical_jwt/jwt.py b/gateway-api/src/gateway_api/clinical_jwt/jwt.py index f965d4df..b0f3eb3e 100644 --- a/gateway-api/src/gateway_api/clinical_jwt/jwt.py +++ b/gateway-api/src/gateway_api/clinical_jwt/jwt.py @@ -20,7 +20,7 @@ class JWT: expiration: int = field(default_factory=lambda: int(time()) + 300) # These are here for future proofing but are not expected ever to be changed - algorithm: str | None = None + algorithm: str = "none" type: str = "JWT" reason_for_request: str = "directcare" requested_scope: str = "patient/*.read" diff --git a/gateway-api/src/gateway_api/clinical_jwt/test_jwt.py b/gateway-api/src/gateway_api/clinical_jwt/test_jwt.py index a8f0f038..4e9ce721 100644 --- a/gateway-api/src/gateway_api/clinical_jwt/test_jwt.py +++ b/gateway-api/src/gateway_api/clinical_jwt/test_jwt.py @@ -29,7 +29,7 @@ def test_jwt_creation_with_required_fields() -> None: assert token.requesting_device == {"device": "info"} assert token.requesting_organization == {"org": "info"} assert token.requesting_practitioner == {"practitioner": "info"} - assert token.algorithm is None + assert token.algorithm == "none" assert token.type == "JWT" assert token.reason_for_request == "directcare" assert token.requested_scope == "patient/*.read" From dfd80cd670ff3347b4e6c21fc7cc870d5b045f4a Mon Sep 17 00:00:00 2001 From: Ben Taylor <59649826+Vox-Ben@users.noreply.github.com> Date: Tue, 10 Mar 2026 17:13:42 +0000 Subject: [PATCH 11/18] Tidy up error handling --- .../src/gateway_api/provider/client.py | 25 +++++++++++-------- .../src/gateway_api/provider/test_client.py | 12 +++------ 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/gateway-api/src/gateway_api/provider/client.py b/gateway-api/src/gateway_api/provider/client.py index fca1c465..ec602143 100644 --- a/gateway-api/src/gateway_api/provider/client.py +++ b/gateway-api/src/gateway_api/provider/client.py @@ -28,7 +28,6 @@ from requests import HTTPError, Response from gateway_api.clinical_jwt import JWT -from gateway_api.common.common import get_http_text from gateway_api.common.error import ProviderRequestFailedError from gateway_api.get_structured_record import ACCESS_RECORD_STRUCTURED_INTERACTION_ID @@ -121,15 +120,19 @@ def access_structured_record( try: response.raise_for_status() except HTTPError as err: - errstr = "GPProvider FHIR API request failed:\n" - errstr += f"{response.status_code}: " - errstr += f"{get_http_text(response.status_code)}: {response.reason}\n" - errstr += response.text - errstr += "\nHeaders were:\n" - for header, value in headers.items(): - errstr += f"{header}: {value}\n" - errstr += "\nBody payload was:\n" - errstr += body - raise ProviderRequestFailedError(error_reason=errstr) from err + # TODO: Consider what error information we want to return here. + # if os.environ.get("CDG_DEBUG", "false").lower() == "true": + # errstr = "GPProvider FHIR API request failed:\n" + # errstr += f"{response.status_code}: " + # errstr += f"{get_http_text(response.status_code)}:{response.reason}\n" + # errstr += response.text + # errstr += "\nHeaders were:\n" + # for header, value in headers.items(): + # errstr += f"{header}: {value}\n" + # errstr += "\nBody payload was:\n" + # errstr += body + # else: + # errstr = str(err.response.reason) + raise ProviderRequestFailedError(error_reason=err.response.reason) from err return response diff --git a/gateway-api/src/gateway_api/provider/test_client.py b/gateway-api/src/gateway_api/provider/test_client.py index f1a59165..c45d2181 100644 --- a/gateway-api/src/gateway_api/provider/test_client.py +++ b/gateway-api/src/gateway_api/provider/test_client.py @@ -105,10 +105,7 @@ def test_valid_gpprovider_access_structured_record_makes_request_correct_url_pos captured_url = mock_request_post.get("url", provider_endpoint) - assert ( - captured_url - == provider_endpoint + "/FHIR/STU3/patient/$gpc.getstructuredrecord" - ) + assert captured_url == provider_endpoint + "/Patient/$gpc.getstructuredrecord" assert result.status_code == 200 @@ -137,8 +134,8 @@ def test_valid_gpprovider_access_structured_record_with_correct_headers_post_200 token=dummy_jwt, ) expected_headers = { - "Content-Type": "application/fhir+json", - "Accept": "application/fhir+json", + "Content-Type": "application/fhir+json;charset=utf-8", + "Accept": "application/fhir+json;charset=utf-8", "Ssp-TraceID": str(trace_id), "Ssp-From": consumer_asid, "Ssp-To": provider_asid, @@ -251,8 +248,7 @@ def test_access_structured_record_raises_external_service_error( ) with pytest.raises( - ProviderRequestFailedError, - match="Provider request failed: Bad Request", + ProviderRequestFailedError, match=r"Provider request failed: Bad Request" ): client.access_structured_record( trace_id, json.dumps(valid_simple_request_payload) From e142d0aab061da81b863c6f01fa557a39697a0f1 Mon Sep 17 00:00:00 2001 From: Ben Taylor <59649826+Vox-Ben@users.noreply.github.com> Date: Wed, 11 Mar 2026 11:08:02 +0000 Subject: [PATCH 12/18] Add request validation to provider stub --- .../src/gateway_api/clinical_jwt/__init__.py | 3 +- .../clinical_jwt/test_validator.py | 199 +++++++++++++++ .../src/gateway_api/clinical_jwt/validator.py | 240 ++++++++++++++++++ gateway-api/src/gateway_api/common/error.py | 6 + gateway-api/src/gateway_api/conftest.py | 41 +++ .../src/gateway_api/provider/test_client.py | 63 +++-- gateway-api/stubs/stubs/provider/stub.py | 175 ++++++++++++- 7 files changed, 690 insertions(+), 37 deletions(-) create mode 100644 gateway-api/src/gateway_api/clinical_jwt/test_validator.py create mode 100644 gateway-api/src/gateway_api/clinical_jwt/validator.py diff --git a/gateway-api/src/gateway_api/clinical_jwt/__init__.py b/gateway-api/src/gateway_api/clinical_jwt/__init__.py index 63d06bf2..38d0dafb 100644 --- a/gateway-api/src/gateway_api/clinical_jwt/__init__.py +++ b/gateway-api/src/gateway_api/clinical_jwt/__init__.py @@ -2,5 +2,6 @@ from .jwt import JWT from .organization import Organization from .practitioner import Practitioner +from .validator import JWTValidator -__all__ = ["JWT", "Device", "Organization", "Practitioner"] +__all__ = ["JWT", "Device", "Organization", "Practitioner", "JWTValidator"] diff --git a/gateway-api/src/gateway_api/clinical_jwt/test_validator.py b/gateway-api/src/gateway_api/clinical_jwt/test_validator.py new file mode 100644 index 00000000..914c307d --- /dev/null +++ b/gateway-api/src/gateway_api/clinical_jwt/test_validator.py @@ -0,0 +1,199 @@ +"""Unit tests for JWT validator.""" + +import pytest + +from gateway_api.clinical_jwt import JWT +from gateway_api.clinical_jwt.validator import JWTValidator +from gateway_api.common.error import JWTValidationError + + +class TestValidateRequiredFields: + """Tests for validate_required_fields method.""" + + def test_valid_jwt_passes(self, valid_jwt: JWT) -> None: + """Test that a valid JWT passes required fields validation.""" + try: + JWTValidator.validate_required_fields(valid_jwt) # Should not raise + except JWTValidationError as err: + pytest.fail(f"JWT validation failed: {err}") + + +class TestValidateTimestamps: + """Tests for validate_timestamps method.""" + + def test_valid_timestamps_pass(self, valid_jwt: JWT) -> None: + """Test that valid timestamps pass validation.""" + JWTValidator.validate_timestamps(valid_jwt) # Should not raise + + def test_non_integer_issued_at_raises_error(self, valid_jwt: JWT) -> None: + """Test that non-integer issued_at raises validation error.""" + jwt = JWT( + issuer=valid_jwt.issuer, + subject=valid_jwt.subject, + audience=valid_jwt.audience, + requesting_device=valid_jwt.requesting_device, + requesting_organization=valid_jwt.requesting_organization, + requesting_practitioner=valid_jwt.requesting_practitioner, + issued_at="not an int", # type: ignore + expiration=valid_jwt.expiration, + ) + with pytest.raises(JWTValidationError) as exc_info: + JWTValidator.validate_timestamps(jwt) + assert "issued_at must be a unix timestamp" in str(exc_info.value) + + def test_non_integer_expiration_raises_error(self, valid_jwt: JWT) -> None: + """Test that non-integer expiration raises validation error.""" + jwt = JWT( + issuer=valid_jwt.issuer, + subject=valid_jwt.subject, + audience=valid_jwt.audience, + requesting_device=valid_jwt.requesting_device, + requesting_organization=valid_jwt.requesting_organization, + requesting_practitioner=valid_jwt.requesting_practitioner, + issued_at=valid_jwt.issued_at, + expiration="not an int", # type: ignore + ) + with pytest.raises(JWTValidationError) as exc_info: + JWTValidator.validate_timestamps(jwt) + assert "expiration must be a unix timestamp" in str(exc_info.value) + + def test_expiration_not_300_seconds_after_issued_at_raises_error( + self, valid_jwt: JWT + ) -> None: + """Test that expiration not exactly 300 seconds after issued_at raises error.""" + jwt = JWT( + issuer=valid_jwt.issuer, + subject=valid_jwt.subject, + audience=valid_jwt.audience, + requesting_device=valid_jwt.requesting_device, + requesting_organization=valid_jwt.requesting_organization, + requesting_practitioner=valid_jwt.requesting_practitioner, + issued_at=1000000, + expiration=1000400, # 400 seconds, not 300 + ) + with pytest.raises(JWTValidationError) as exc_info: + JWTValidator.validate_timestamps(jwt) + assert "exactly 5 minutes (300 seconds)" in str(exc_info.value) + + +class TestValidateDevice: + """Tests for validate_device method.""" + + def test_valid_device_passes(self, valid_jwt: JWT) -> None: + """Test that a valid device passes validation.""" + JWTValidator.validate_device(valid_jwt.requesting_device) # Should not raise + + def test_missing_resource_type_raises_error(self, valid_jwt: JWT) -> None: + """Test that missing resourceType raises validation error.""" + device = valid_jwt.requesting_device.copy() + device["resourceType"] = "WrongType" + with pytest.raises(JWTValidationError) as exc_info: + JWTValidator.validate_device(device) + assert "resourceType must be 'Device'" in str(exc_info.value) + + def test_missing_identifier_raises_error(self, valid_jwt: JWT) -> None: + """Test that missing identifier raises validation error.""" + device = valid_jwt.requesting_device.copy() + del device["identifier"] + with pytest.raises(JWTValidationError) as exc_info: + JWTValidator.validate_device(device) + assert "identifier must be a non-empty list" in str(exc_info.value) + + def test_missing_model_raises_error(self, valid_jwt: JWT) -> None: + """Test that missing model raises validation error.""" + device = valid_jwt.requesting_device.copy() + del device["model"] + with pytest.raises(JWTValidationError) as exc_info: + JWTValidator.validate_device(device) + assert "model is required" in str(exc_info.value) + + +class TestValidateOrganization: + """Tests for validate_organization method.""" + + def test_valid_organization_passes(self, valid_jwt: JWT) -> None: + """Test that a valid organization passes validation.""" + JWTValidator.validate_organization(valid_jwt.requesting_organization) + + def test_missing_resource_type_raises_error(self, valid_jwt: JWT) -> None: + """Test that missing resourceType raises validation error.""" + org = valid_jwt.requesting_organization.copy() + org["resourceType"] = "WrongType" + with pytest.raises(JWTValidationError) as exc_info: + JWTValidator.validate_organization(org) + assert "resourceType must be 'Organization'" in str(exc_info.value) + + def test_missing_name_raises_error(self, valid_jwt: JWT) -> None: + """Test that missing name raises validation error.""" + org = valid_jwt.requesting_organization.copy() + del org["name"] + with pytest.raises(JWTValidationError) as exc_info: + JWTValidator.validate_organization(org) + assert "name is required" in str(exc_info.value) + + +class TestValidatePractitioner: + """Tests for validate_practitioner method.""" + + def test_valid_practitioner_passes(self, valid_jwt: JWT) -> None: + """Test that a valid practitioner passes validation.""" + JWTValidator.validate_practitioner(valid_jwt.requesting_practitioner) + + def test_missing_resource_type_raises_error(self, valid_jwt: JWT) -> None: + """Test that missing resourceType raises validation error.""" + pract = valid_jwt.requesting_practitioner.copy() + pract["resourceType"] = "WrongType" + with pytest.raises(JWTValidationError) as exc_info: + JWTValidator.validate_practitioner(pract) + assert "resourceType must be 'Practitioner'" in str(exc_info.value) + + def test_missing_id_raises_error(self, valid_jwt: JWT) -> None: + """Test that missing id raises validation error.""" + pract = valid_jwt.requesting_practitioner.copy() + del pract["id"] + with pytest.raises(JWTValidationError) as exc_info: + JWTValidator.validate_practitioner(pract) + assert "id is required" in str(exc_info.value) + + def test_insufficient_identifiers_raises_error(self, valid_jwt: JWT) -> None: + """Test that less than 3 identifiers raises validation error.""" + pract = valid_jwt.requesting_practitioner.copy() + pract["identifier"] = [{"system": "sys", "value": "val"}] # Only 1 + with pytest.raises(JWTValidationError) as exc_info: + JWTValidator.validate_practitioner(pract) + assert "at least 3 items" in str(exc_info.value) + + def test_missing_family_name_raises_error(self, valid_jwt: JWT) -> None: + """Test that missing family name raises validation error.""" + pract = valid_jwt.requesting_practitioner.copy() + pract["name"] = [{"given": ["Test"]}] # Missing family + with pytest.raises(JWTValidationError) as exc_info: + JWTValidator.validate_practitioner(pract) + assert "family is required" in str(exc_info.value) + + +class TestValidate: + """Tests for the main validate method.""" + + def test_valid_jwt_passes_all_validation(self, valid_jwt: JWT) -> None: + """Test that a completely valid JWT passes all validation.""" + JWTValidator.validate(valid_jwt) # Should not raise + + def test_invalid_jwt_reports_all_errors(self, valid_jwt: JWT) -> None: + """Test that validation reports all errors, not just the first one.""" + jwt = JWT( + issuer="", # Invalid - missing required field + subject=valid_jwt.subject, + audience=valid_jwt.audience, + requesting_device={"resourceType": "Wrong"}, # Invalid - wrong resourceType + requesting_organization=valid_jwt.requesting_organization, + requesting_practitioner=valid_jwt.requesting_practitioner, + issued_at=valid_jwt.issued_at, + expiration=valid_jwt.expiration, + ) + with pytest.raises(JWTValidationError) as exc_info: + JWTValidator.validate(jwt) + error_message = str(exc_info.value) + # Should contain both the missing issuer error and the device error + assert "issuer" in error_message + assert "Device" in error_message or "device" in error_message diff --git a/gateway-api/src/gateway_api/clinical_jwt/validator.py b/gateway-api/src/gateway_api/clinical_jwt/validator.py new file mode 100644 index 00000000..d819541b --- /dev/null +++ b/gateway-api/src/gateway_api/clinical_jwt/validator.py @@ -0,0 +1,240 @@ +"""JWT validation for GPConnect FHIR API requests.""" + +from typing import Any + +from gateway_api.clinical_jwt import JWT +from gateway_api.common.error import JWTValidationError + + +class JWTValidator: + """Validator for JWT claims structure and contents.""" + + @staticmethod + def validate_required_fields(jwt_obj: JWT) -> None: + """ + Validate that all required JWT fields are present and non-empty. + + Raises: + JWTValidationError: If any required field is missing or empty. + """ + missing_fields = [] + if not jwt_obj.issuer: + missing_fields.append("issuer") + if not jwt_obj.subject: + missing_fields.append("subject") + if not jwt_obj.audience: + missing_fields.append("audience") + if not jwt_obj.requesting_device: + missing_fields.append("requesting_device") + if not jwt_obj.requesting_organization: + missing_fields.append("requesting_organization") + if not jwt_obj.requesting_practitioner: + missing_fields.append("requesting_practitioner") + if jwt_obj.issued_at is None: + missing_fields.append("issued_at") + if jwt_obj.expiration is None: + missing_fields.append("expiration") + + if missing_fields: + field_list = ", ".join(missing_fields) + raise JWTValidationError( + error_details=f"JWT missing required fields: {field_list}" + ) + + @staticmethod + def validate_timestamps(jwt_obj: JWT) -> None: + """ + Validate JWT timestamp fields are integers and expiration is 5 minutes after + issued_at. + + Raises: + JWTValidationError: If timestamps are invalid. + """ + if not isinstance(jwt_obj.issued_at, int): + raise JWTValidationError( + error_details="JWT issued_at must be a unix timestamp (integer)" + ) + + if not isinstance(jwt_obj.expiration, int): + raise JWTValidationError( + error_details="JWT expiration must be a unix timestamp (integer)" + ) + + expected_expiration = jwt_obj.issued_at + 300 + if jwt_obj.expiration != expected_expiration: + raise JWTValidationError( + error_details=( + f"JWT expiration must be exactly 5 minutes (300 seconds) " + f"after issued_at. " + f"Expected {expected_expiration}, got {jwt_obj.expiration}" + ) + ) + + @staticmethod + def validate_device(device: dict[str, Any]) -> None: + """ + Validate JWT requesting_device structure. + + Raises: + JWTValidationError: If device structure is invalid. + """ + device_errors = [] + if device.get("resourceType") != "Device": + device_errors.append("resourceType must be 'Device'") + if not device.get("identifier") or not isinstance( + device.get("identifier"), list + ): + device_errors.append("Device identifier must be a non-empty list") + else: + identifier = device["identifier"][0] + if not identifier.get("system"): + device_errors.append("identifier[0].system is required") + if not identifier.get("value"): + device_errors.append("identifier[0].value is required") + if not device.get("model"): + device_errors.append("model is required") + if not device.get("version"): + device_errors.append("version is required") + + if device_errors: + raise JWTValidationError( + error_details=f"Invalid requesting_device: {', '.join(device_errors)}" + ) + + @staticmethod + def validate_organization(org: dict[str, Any]) -> None: + """ + Validate JWT requesting_organization structure. + + Raises: + JWTValidationError: If organization structure is invalid. + """ + org_errors = [] + if org.get("resourceType") != "Organization": + org_errors.append("resourceType must be 'Organization'") + if not org.get("identifier") or not isinstance(org.get("identifier"), list): + org_errors.append("Organization identifier must be a non-empty list") + else: + identifier = org["identifier"][0] + if not identifier.get("system"): + org_errors.append("identifier[0].system is required") + if not identifier.get("value"): + org_errors.append("identifier[0].value is required") + if not org.get("name"): + org_errors.append("name is required") + + if org_errors: + raise JWTValidationError( + error_details=( + f"Invalid requesting_organization: {', '.join(org_errors)}" + ) + ) + + @staticmethod + def _validate_practitioner_identifiers( + identifiers: list[dict[str, Any]], + ) -> list[str]: + """Validate practitioner identifier list structure and contents.""" + errors = [] + if len(identifiers) < 3: + errors.append("Practitioner identifier must contain at least 3 items") + return errors + + for i, identifier in enumerate(identifiers[:3]): + if not identifier.get("system"): + errors.append(f"identifier[{i}].system is required") + if not identifier.get("value"): + errors.append(f"identifier[{i}].value is required") + return errors + + @staticmethod + def _validate_practitioner_name(names: list[dict[str, Any]]) -> list[str]: + """Validate practitioner name list structure and contents.""" + errors = [] + if len(names) == 0: + errors.append("name list cannot be empty") + return errors + + name = names[0] + if not name.get("family"): + errors.append("name[0].family is required") + return errors + + @staticmethod + def validate_practitioner(pract: dict[str, Any]) -> None: + """ + Validate JWT requesting_practitioner structure. + + Raises: + JWTValidationError: If practitioner structure is invalid. + """ + pract_errors = [] + + if pract.get("resourceType") != "Practitioner": + pract_errors.append("resourceType must be 'Practitioner'") + + if not pract.get("id"): + pract_errors.append("id is required") + + # Validate identifiers + identifiers = pract.get("identifier") + if not identifiers or not isinstance(identifiers, list): + pract_errors.append("Practitioner identifier must be a non-empty list") + else: + pract_errors.extend( + JWTValidator._validate_practitioner_identifiers(identifiers) + ) + + # Validate name + names = pract.get("name") + if not names or not isinstance(names, list): + pract_errors.append("name must be a non-empty list") + else: + pract_errors.extend(JWTValidator._validate_practitioner_name(names)) + + if pract_errors: + raise JWTValidationError( + error_details=( + f"Invalid requesting_practitioner: {', '.join(pract_errors)}" + ) + ) + + @staticmethod + def validate(jwt_obj: JWT) -> None: + """ + Validate JWT claims structure and contents. + + Collects all validation errors before raising to provide complete feedback. + + Raises: + JWTValidationError: If validation fails, with all errors listed. + """ + errors = [] + + try: + JWTValidator.validate_required_fields(jwt_obj) + except JWTValidationError as e: + errors.append(str(e)) + + try: + JWTValidator.validate_timestamps(jwt_obj) + except JWTValidationError as e: + errors.append(str(e)) + + try: + JWTValidator.validate_device(jwt_obj.requesting_device) + except JWTValidationError as e: + errors.append(str(e)) + + try: + JWTValidator.validate_organization(jwt_obj.requesting_organization) + except JWTValidationError as e: + errors.append(str(e)) + + try: + JWTValidator.validate_practitioner(jwt_obj.requesting_practitioner) + except JWTValidationError as e: + errors.append(str(e)) + + if errors: + raise JWTValidationError(error_details="; ".join(errors)) diff --git a/gateway-api/src/gateway_api/common/error.py b/gateway-api/src/gateway_api/common/error.py index 3915502d..79f2a561 100644 --- a/gateway-api/src/gateway_api/common/error.py +++ b/gateway-api/src/gateway_api/common/error.py @@ -117,6 +117,12 @@ class ProviderRequestFailedError(AbstractCDGError): error_code = ErrorCode.EXCEPTION +class JWTValidationError(AbstractCDGError): + _message = "{error_details}" + status_code = BAD_REQUEST + error_code = ErrorCode.INVALID + + class UnexpectedError(AbstractCDGError): _message = "Internal Server Error: {traceback}" status_code = INTERNAL_SERVER_ERROR diff --git a/gateway-api/src/gateway_api/conftest.py b/gateway-api/src/gateway_api/conftest.py index 8ef4c2d9..0757ea11 100644 --- a/gateway-api/src/gateway_api/conftest.py +++ b/gateway-api/src/gateway_api/conftest.py @@ -12,6 +12,8 @@ from requests.structures import CaseInsensitiveDict from werkzeug.test import EnvironBuilder +from gateway_api.clinical_jwt import JWT + @dataclass class FakeResponse: @@ -153,3 +155,42 @@ def happy_path_pds_response_body() -> Patient: @pytest.fixture def auth_token() -> str: return "AUTH_TOKEN123" + + +@pytest.fixture +def valid_jwt() -> JWT: + """Create a valid JWT for testing.""" + return JWT( + issuer="https://example.com", + subject="user-123", + audience="https://provider.example.com", + requesting_device={ + "resourceType": "Device", + "identifier": [{"system": "https://example.com/device", "value": "dev123"}], + "model": "TestModel", + "version": "1.0", + }, + requesting_organization={ + "resourceType": "Organization", + "identifier": [ + { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": "T1234", + } + ], + "name": "Test Organization", + }, + requesting_practitioner={ + "resourceType": "Practitioner", + "id": "prac123", + "identifier": [ + {"system": "https://fhir.nhs.uk/Id/sds-user-id", "value": "user123"}, + { + "system": "https://fhir.nhs.uk/Id/sds-role-profile-id", + "value": "role123", + }, + {"system": "https://example.com/userid", "value": "userid123"}, + ], + "name": [{"family": "TestPractitioner", "given": ["Test"]}], + }, + ) diff --git a/gateway-api/src/gateway_api/provider/test_client.py b/gateway-api/src/gateway_api/provider/test_client.py index c45d2181..cff97b45 100644 --- a/gateway-api/src/gateway_api/provider/test_client.py +++ b/gateway-api/src/gateway_api/provider/test_client.py @@ -56,29 +56,19 @@ def _fake_post( # Provide dummy or captured arguments as required by the stub signature return stub.access_record_structured( - trace_id=headers.get("Ssp-TraceID", "dummy-trace-id"), body=data + trace_id=headers.get("Ssp-TraceID", "dummy-trace-id"), + body=data, + headers=dict(headers), ) monkeypatch.setattr(client, "post", _fake_post) return capture -@pytest.fixture -def dummy_jwt() -> JWT: - return JWT( - issuer="https://example.com", - subject="user-123", - audience="https://provider.example.com", - requesting_device={"device": "info"}, - requesting_organization={"org": "info"}, - requesting_practitioner={"practitioner": "info"}, - ) - - def test_valid_gpprovider_access_structured_record_makes_request_correct_url_post_200( mock_request_post: dict[str, Any], valid_simple_request_payload: Parameters, - dummy_jwt: JWT, + valid_jwt: JWT, ) -> None: """ Test that the `access_structured_record` method constructs the correct URL @@ -96,7 +86,7 @@ def test_valid_gpprovider_access_structured_record_makes_request_correct_url_pos provider_endpoint=provider_endpoint, provider_asid=provider_asid, consumer_asid=consumer_asid, - token=dummy_jwt, + token=valid_jwt, ) result = client.access_structured_record( @@ -112,15 +102,11 @@ def test_valid_gpprovider_access_structured_record_makes_request_correct_url_pos def test_valid_gpprovider_access_structured_record_with_correct_headers_post_200( mock_request_post: dict[str, Any], valid_simple_request_payload: Parameters, - dummy_jwt: JWT, + valid_jwt: JWT, ) -> None: """ Test that the `access_structured_record` method includes the correct headers in the GPProvider FHIR API request and receives a 200 OK response. - - This test verifies that the headers include: - - Content-Type and Accept headers for FHIR+JSON. - - Ssp-TraceID, Ssp-From, Ssp-To, and Ssp-InteractionID for GPConnect. """ provider_asid = "200000001154" consumer_asid = "200000001152" @@ -131,7 +117,7 @@ def test_valid_gpprovider_access_structured_record_with_correct_headers_post_200 provider_endpoint=provider_endpoint, provider_asid=provider_asid, consumer_asid=consumer_asid, - token=dummy_jwt, + token=valid_jwt, ) expected_headers = { "Content-Type": "application/fhir+json;charset=utf-8", @@ -142,7 +128,7 @@ def test_valid_gpprovider_access_structured_record_with_correct_headers_post_200 "Ssp-InteractionID": ( "urn:nhs:names:services:gpconnect:fhir:operation:gpc.getstructuredrecord-1" ), - "Authorization": f"Bearer {dummy_jwt}", + "Authorization": f"Bearer {valid_jwt}", } result = client.access_structured_record( @@ -158,7 +144,7 @@ def test_valid_gpprovider_access_structured_record_with_correct_headers_post_200 def test_valid_gpprovider_access_structured_record_with_correct_body_200( mock_request_post: dict[str, Any], valid_simple_request_payload: Parameters, - dummy_jwt: JWT, + valid_jwt: JWT, ) -> None: """ Test that the `access_structured_record` method includes the correct body @@ -178,7 +164,7 @@ def test_valid_gpprovider_access_structured_record_with_correct_body_200( provider_endpoint=provider_endpoint, provider_asid=provider_asid, consumer_asid=consumer_asid, - token=dummy_jwt, + token=valid_jwt, ) result = client.access_structured_record(trace_id, request_body) @@ -193,7 +179,7 @@ def test_valid_gpprovider_access_structured_record_returns_stub_response_200( mock_request_post: dict[str, Any], # NOQA ARG001 (Mock not called directly) stub: GpProviderStub, valid_simple_request_payload: Parameters, - dummy_jwt: JWT, + valid_jwt: JWT, ) -> None: """ Test that the `access_structured_record` method returns the same response @@ -211,11 +197,24 @@ def test_valid_gpprovider_access_structured_record_returns_stub_response_200( provider_endpoint=provider_endpoint, provider_asid=provider_asid, consumer_asid=consumer_asid, - token=dummy_jwt, + token=valid_jwt, ) + # Build headers for stub call + stub_headers = { + "Content-Type": "application/fhir+json;charset=utf-8", + "Accept": "application/fhir+json;charset=utf-8", + "Ssp-TraceID": trace_id, + "Ssp-From": consumer_asid, + "Ssp-To": provider_asid, + "Ssp-InteractionID": ( + "urn:nhs:names:services:gpconnect:fhir:operation:gpc.getstructuredrecord-1" + ), + "Authorization": f"Bearer {valid_jwt}", + } + expected_response = stub.access_record_structured( - trace_id, json.dumps(valid_simple_request_payload) + trace_id, json.dumps(valid_simple_request_payload), stub_headers ) result = client.access_structured_record( @@ -229,7 +228,7 @@ def test_valid_gpprovider_access_structured_record_returns_stub_response_200( def test_access_structured_record_raises_external_service_error( mock_request_post: dict[str, Any], # NOQA ARG001 (Mock not called directly) valid_simple_request_payload: Parameters, - dummy_jwt: JWT, + valid_jwt: JWT, ) -> None: """ Test that the `access_structured_record` method raises an `SdsRequestFailed` @@ -244,7 +243,7 @@ def test_access_structured_record_raises_external_service_error( provider_endpoint=provider_endpoint, provider_asid=provider_asid, consumer_asid=consumer_asid, - token=dummy_jwt, + token=valid_jwt, ) with pytest.raises( @@ -258,7 +257,7 @@ def test_access_structured_record_raises_external_service_error( def test_gpprovider_client_includes_authorization_header_with_bearer_token( mock_request_post: dict[str, Any], valid_simple_request_payload: Parameters, - dummy_jwt: JWT, + valid_jwt: JWT, ) -> None: """ Test that the GpProviderClient includes an Authorization header with the @@ -273,7 +272,7 @@ def test_gpprovider_client_includes_authorization_header_with_bearer_token( provider_endpoint=provider_endpoint, provider_asid=provider_asid, consumer_asid=consumer_asid, - token=dummy_jwt, + token=valid_jwt, ) result = client.access_structured_record( @@ -283,5 +282,5 @@ def test_gpprovider_client_includes_authorization_header_with_bearer_token( captured_headers = mock_request_post["headers"] assert "Authorization" in captured_headers - assert captured_headers["Authorization"] == f"Bearer {dummy_jwt}" + assert captured_headers["Authorization"] == f"Bearer {valid_jwt}" assert result.status_code == 200 diff --git a/gateway-api/stubs/stubs/provider/stub.py b/gateway-api/stubs/stubs/provider/stub.py index a1f8d58d..145b9d4e 100644 --- a/gateway-api/stubs/stubs/provider/stub.py +++ b/gateway-api/stubs/stubs/provider/stub.py @@ -24,6 +24,9 @@ import json from typing import Any +from gateway_api.clinical_jwt import JWT, JWTValidator +from gateway_api.common.error import JWTValidationError +from gateway_api.get_structured_record import ACCESS_RECORD_STRUCTURED_INTERACTION_ID from requests import Response from stubs.base_stub import StubBase @@ -41,10 +44,144 @@ class GpProviderStub(StubBase): # https://simplifier.net/guide/gp-connect-access-record-structured/Home/Examples/Allergy-examples?version=1.6.2 """ + def _validate_headers(self, headers: dict[str, Any]) -> Response | None: + """ + Validate required headers for GPConnect FHIR API request. + + Returns: + Response: Error response if validation fails, None if valid. + """ + required_headers = { + "Ssp-TraceID", + "Ssp-From", + "Ssp-To", + "Ssp-InteractionID", + "Content-Type", + "Authorization", + } + + # Check for missing headers + missing_headers = [] + for header in required_headers: + if header not in headers or not headers[header]: + missing_headers.append(header) + + if missing_headers: + error_msg = ", ".join(f"{h} is required" for h in missing_headers) + return self._create_response( + status_code=400, + json_data={ + "resourceType": "OperationOutcome", + "issue": [ + { + "severity": "error", + "code": "invalid", + "diagnostics": error_msg, + } + ], + }, + ) + + # Validate Content-Type + content_type = headers.get("Content-Type", "") + if "application/fhir+json" not in content_type: + return self._create_response( + status_code=400, + json_data={ + "resourceType": "OperationOutcome", + "issue": [ + { + "severity": "error", + "code": "invalid", + "diagnostics": "Content-Type must be application/fhir+json", + } + ], + }, + ) + + # Validate Ssp-InteractionID + interaction_id = headers.get("Ssp-InteractionID", "") + if interaction_id != ACCESS_RECORD_STRUCTURED_INTERACTION_ID: + return self._create_response( + status_code=400, + json_data={ + "resourceType": "OperationOutcome", + "issue": [ + { + "severity": "error", + "code": "invalid", + "diagnostics": ( + f"Invalid Ssp-InteractionID: expected " + f"{ACCESS_RECORD_STRUCTURED_INTERACTION_ID}" + ), + } + ], + }, + ) + + # Validate Authorization header format and JWT + auth_header = headers.get("Authorization", "") + if not auth_header.startswith("Bearer "): + return self._create_response( + status_code=400, + json_data={ + "resourceType": "OperationOutcome", + "issue": [ + { + "severity": "error", + "code": "invalid", + "diagnostics": ( + "Authorization header must start with 'Bearer '", + ), + } + ], + }, + ) + + # Extract and validate JWT + token = auth_header[7:] # Remove "Bearer " prefix + try: + jwt_obj = JWT.decode(token) + except Exception as e: + return self._create_response( + status_code=400, + json_data={ + "resourceType": "OperationOutcome", + "issue": [ + { + "severity": "error", + "code": "invalid", + "diagnostics": f"Invalid JWT: {e!s}", + } + ], + }, + ) + + # Validate JWT structure and contents + try: + JWTValidator.validate(jwt_obj) + except JWTValidationError as e: + return self._create_response( + status_code=400, + json_data={ + "resourceType": "OperationOutcome", + "issue": [ + { + "severity": "error", + "code": "invalid", + "diagnostics": str(e), + } + ], + }, + ) + + return None + def access_record_structured( self, trace_id: str, - body: str, # NOQA ARG002 # NOSONAR S1172: unused parameter maintains method signature in stub + body: str, + headers: dict[str, Any], ) -> Response: """ Simulate accessRecordStructured operation of GPConnect FHIR API. @@ -52,6 +189,35 @@ def access_record_structured( returns: Response: The stub patient bundle wrapped in a Response object. """ + # Validate that all required parameters are provided + missing_params: list[str] = [] + if trace_id is None: + missing_params.append("trace_id") + if body is None: + missing_params.append("body") + if headers is None: + missing_params.append("headers") + + if missing_params: + error_msg = ", ".join(f"{param} is required" for param in missing_params) + return self._create_response( + status_code=400, + json_data={ + "resourceType": "OperationOutcome", + "issue": [ + { + "severity": "error", + "code": "invalid", + "diagnostics": error_msg, + } + ], + }, + ) + + # Validate headers + validation_error = self._validate_headers(headers) + if validation_error is not None: + return validation_error if trace_id == "invalid for test": return self._create_response( @@ -70,7 +236,7 @@ def access_record_structured( try: nhs_number = json.loads(body)["parameter"][0]["valueIdentifier"]["value"] - except json.JSONDecodeError, KeyError, IndexError: + except (json.JSONDecodeError, KeyError, IndexError): return self._create_response( status_code=400, json_data={ @@ -121,8 +287,9 @@ def post( :param timeout: Request timeout in seconds. :return: A :class:`requests.Response` instance. """ - trace_id = kwargs.get("headers", {}).get("Ssp-TraceID", "no-trace-id") - return self.access_record_structured(trace_id, data) + headers = kwargs.get("headers", {}) + trace_id = headers.get("Ssp-TraceID", "no-trace-id") + return self.access_record_structured(trace_id, data, headers) def get( self, From 4d05553d118376a58f3a029417a9824dce0c8158 Mon Sep 17 00:00:00 2001 From: Ben Taylor <59649826+Vox-Ben@users.noreply.github.com> Date: Thu, 12 Mar 2026 12:48:51 +0000 Subject: [PATCH 13/18] Fix rebase mess and tidy up --- Makefile | 3 + gateway-api/poetry.lock | 2 +- .../src/gateway_api/clinical_jwt/device.py | 13 ----- .../gateway_api/clinical_jwt/practitioner.py | 56 ++++++++----------- .../gateway_api/clinical_jwt/test_device.py | 19 +------ .../clinical_jwt/test_practitioner.py | 46 ++------------- gateway-api/src/gateway_api/controller.py | 4 +- .../src/gateway_api/provider/client.py | 27 ++++----- .../src/gateway_api/provider/test_client.py | 4 -- 9 files changed, 49 insertions(+), 125 deletions(-) diff --git a/Makefile b/Makefile index e5ad5732..2082c30d 100644 --- a/Makefile +++ b/Makefile @@ -64,6 +64,9 @@ deploy: clean build # Deploy the project artefact to the target environment @Pip if [[ -n "$${STUB_SDS}" ]]; then \ PROVIDER_STRING="$${PROVIDER_STRING} -e STUB_SDS=$${STUB_SDS}" ; \ fi ; \ + if [[ -n "$${CDG_DEBUG}" ]]; then \ + PROVIDER_STRING="$${PROVIDER_STRING} -e CDG_DEBUG=$${CDG_DEBUG}" ; \ + fi ; \ if [[ -n "$${IN_BUILD_CONTAINER}" ]]; then \ echo "Starting using local docker network ..." ; \ $(docker) run --platform linux/amd64 --name gateway-api -p 5000:8080 --network gateway-local $${PROVIDER_STRING} -d ${IMAGE_NAME} ; \ diff --git a/gateway-api/poetry.lock b/gateway-api/poetry.lock index 8cb0276a..bcb8d5e2 100644 --- a/gateway-api/poetry.lock +++ b/gateway-api/poetry.lock @@ -2443,4 +2443,4 @@ propcache = ">=0.2.1" [metadata] lock-version = "2.1" python-versions = ">=3.14,<4.0.0" -content-hash = "9469083897a863dc2b0353e51ef7ba41394a8eef6fcf4731ee25bf2cae7addd0" +content-hash = "e93abc9f24ced8a556606149bce2a1b36adfc1cb7e5ebd0d654ad8f6bb1bce0b" diff --git a/gateway-api/src/gateway_api/clinical_jwt/device.py b/gateway-api/src/gateway_api/clinical_jwt/device.py index 2c733103..3d829887 100644 --- a/gateway-api/src/gateway_api/clinical_jwt/device.py +++ b/gateway-api/src/gateway_api/clinical_jwt/device.py @@ -19,16 +19,3 @@ def to_dict(self) -> dict[str, Any]: "model": self.model, "version": self.version, } - - @property - def json(self) -> dict[str, Any]: - """ - Return the Device as a dictionary suitable for JWT payload. - Provided for backwards compatibility. - """ - return self.to_dict() - - def __str__(self) -> str: - import json - - return json.dumps(self.to_dict(), indent=2) diff --git a/gateway-api/src/gateway_api/clinical_jwt/practitioner.py b/gateway-api/src/gateway_api/clinical_jwt/practitioner.py index 17d155e9..0475fa5e 100644 --- a/gateway-api/src/gateway_api/clinical_jwt/practitioner.py +++ b/gateway-api/src/gateway_api/clinical_jwt/practitioner.py @@ -1,7 +1,8 @@ from dataclasses import dataclass +from typing import Any -@dataclass(kw_only=True) +@dataclass(frozen=True, kw_only=True) class Practitioner: id: str sds_userid: str @@ -12,38 +13,29 @@ class Practitioner: given_name: str | None = None prefix: str | None = None - def __post_init__(self) -> None: - given = "" if self.given_name is None else f',"given":["{self.given_name}"]' - prefix = "" if self.prefix is None else f',"prefix":["{self.prefix}"]' - self._name_str = f'[{{"family": "{self.family_name}"{given}{prefix}}}]' + def _build_name(self) -> list[dict[str, Any]]: + """Build the name array with proper structure for JWT.""" + name_dict: dict[str, Any] = {"family": self.family_name} + if self.given_name is not None: + name_dict["given"] = [self.given_name] + if self.prefix is not None: + name_dict["prefix"] = [self.prefix] + return [name_dict] - @property - def json(self) -> str: + def to_dict(self) -> dict[str, Any]: + """ + Return the Practitioner as a dictionary suitable for JWT payload. + """ user_id_system = "https://fhir.nhs.uk/Id/sds-user-id" role_id_system = "https://fhir.nhs.uk/Id/sds-role-profile-id" - outstr = f""" - {{ - "resourceType": "Practitioner", - "id": "{self.id}", - "identifier": [ - {{ - "system": "{user_id_system}", - "value": "{self.sds_userid}" - }}, - {{ - "system": "{role_id_system}", - "value": "{self.role_profile_id}" - }}, - {{ - "system": "{self.userid_url}", - "value": "{self.userid_value}" - }} - ], - "name": {self._name_str} - }} - """ - return outstr.strip() - - def __str__(self) -> str: - return self.json + return { + "resourceType": "Practitioner", + "id": self.id, + "identifier": [ + {"system": user_id_system, "value": self.sds_userid}, + {"system": role_id_system, "value": self.role_profile_id}, + {"system": self.userid_url, "value": self.userid_value}, + ], + "name": self._build_name(), + } diff --git a/gateway-api/src/gateway_api/clinical_jwt/test_device.py b/gateway-api/src/gateway_api/clinical_jwt/test_device.py index d3f46138..c936f553 100644 --- a/gateway-api/src/gateway_api/clinical_jwt/test_device.py +++ b/gateway-api/src/gateway_api/clinical_jwt/test_device.py @@ -2,8 +2,6 @@ Unit tests for :mod:`gateway_api.clinical_jwt.device`. """ -from json import loads - from gateway_api.clinical_jwt import Device @@ -35,8 +33,7 @@ def test_device_json_property_returns_valid_json_structure() -> None: version="5.3.0", ) - json_output = input_device.json - jdict = loads(json_output) + jdict = input_device.to_dict() output_device = Device( system=jdict["identifier"][0]["system"], @@ -46,17 +43,3 @@ def test_device_json_property_returns_valid_json_structure() -> None: ) assert input_device == output_device - - -def test_device_str_returns_json() -> None: - """ - Test that __str__ returns the same value as the json property. - """ - device = Device( - system="https://test.com/device", - value="TEST-001", - model="Test Model", - version="1.0.0", - ) - - assert str(device) == device.json diff --git a/gateway-api/src/gateway_api/clinical_jwt/test_practitioner.py b/gateway-api/src/gateway_api/clinical_jwt/test_practitioner.py index 78ea5065..5d158e59 100644 --- a/gateway-api/src/gateway_api/clinical_jwt/test_practitioner.py +++ b/gateway-api/src/gateway_api/clinical_jwt/test_practitioner.py @@ -2,8 +2,6 @@ Unit tests for :mod:`gateway_api.clinical_jwt.practitioner`. """ -from json import loads - from gateway_api.clinical_jwt import Practitioner @@ -48,8 +46,8 @@ def test_practitioner_json_property_returns_valid_structure() -> None: prefix="Mr", ) - json_output = input_practitioner.json - jdict = loads(json_output) + jdict = input_practitioner.to_dict() + output_practitioner = Practitioner( id=jdict["id"], sds_userid=jdict["identifier"][0]["value"], @@ -57,44 +55,8 @@ def test_practitioner_json_property_returns_valid_structure() -> None: userid_url=jdict["identifier"][2]["system"], userid_value=jdict["identifier"][2]["value"], family_name=jdict["name"][0]["family"], - given_name=jdict["name"][0].get("given", [None])[0], - prefix=jdict["name"][0].get("prefix", [None])[0], + given_name=jdict["name"][0].get("given", None)[0], + prefix=jdict["name"][0].get("prefix", None)[0], ) assert input_practitioner == output_practitioner - - -def test_practitioner_str_returns_json() -> None: - """ - Test that __str__ returns the same value as the json property. - """ - practitioner = Practitioner( - id="10026", - sds_userid="888999000111", - role_profile_id="111222333444", - userid_url="https://test.com/user", - userid_value="test-guid-7", - family_name="Taylor", - ) - - assert str(practitioner) == practitioner.json - - -def test_practitioner_identifier_systems() -> None: - """ - Test that the correct identifier systems are used in the JSON output. - """ - practitioner = Practitioner( - id="10027", - sds_userid="999000111222", - role_profile_id="222333444555", - userid_url="https://test.com/user", - userid_value="test-guid-8", - family_name="Anderson", - ) - - json_output = practitioner.json - - # Verify the correct system URLs are used - assert "https://fhir.nhs.uk/Id/sds-user-id" in json_output - assert "https://fhir.nhs.uk/Id/sds-role-profile-id" in json_output diff --git a/gateway-api/src/gateway_api/controller.py b/gateway-api/src/gateway_api/controller.py index 631e2e02..7249791c 100644 --- a/gateway-api/src/gateway_api/controller.py +++ b/gateway-api/src/gateway_api/controller.py @@ -114,14 +114,14 @@ def get_jwt_for_provider(self, provider_endpoint: str, consumer_ods: str) -> JWT id="10019", sds_userid="111222333444", role_profile_id="444555666777", - userid_url="https://consumersupplier.com/Id/user-guid", + userid_url="https://orange.testlab.nhs.uk/gpconnect-demonstrator/Id/local-user-id", userid_value="98ed4f78-814d-4266-8d5b-cde742f3093c", family_name="Doe", given_name="John", prefix="Mr", ) - # TODO: Get consumer organization details properly + # TODO: Where do we get the consumer org name from? SDS? requesting_organization = Organization( ods_code=consumer_ods, name="Consumer organisation name" ) diff --git a/gateway-api/src/gateway_api/provider/client.py b/gateway-api/src/gateway_api/provider/client.py index ec602143..305eb399 100644 --- a/gateway-api/src/gateway_api/provider/client.py +++ b/gateway-api/src/gateway_api/provider/client.py @@ -28,6 +28,7 @@ from requests import HTTPError, Response from gateway_api.clinical_jwt import JWT +from gateway_api.common.common import get_http_text from gateway_api.common.error import ProviderRequestFailedError from gateway_api.get_structured_record import ACCESS_RECORD_STRUCTURED_INTERACTION_ID @@ -121,18 +122,18 @@ def access_structured_record( response.raise_for_status() except HTTPError as err: # TODO: Consider what error information we want to return here. - # if os.environ.get("CDG_DEBUG", "false").lower() == "true": - # errstr = "GPProvider FHIR API request failed:\n" - # errstr += f"{response.status_code}: " - # errstr += f"{get_http_text(response.status_code)}:{response.reason}\n" - # errstr += response.text - # errstr += "\nHeaders were:\n" - # for header, value in headers.items(): - # errstr += f"{header}: {value}\n" - # errstr += "\nBody payload was:\n" - # errstr += body - # else: - # errstr = str(err.response.reason) - raise ProviderRequestFailedError(error_reason=err.response.reason) from err + if os.environ.get("CDG_DEBUG", "false").lower() == "true": + errstr = "GPProvider FHIR API request failed:\n" + errstr += f"{response.status_code}: " + errstr += f"{get_http_text(response.status_code)}:{response.reason}\n" + errstr += response.text + errstr += "\nHeaders were:\n" + for header, value in headers.items(): + errstr += f"{header}: {value}\n" + errstr += "\nBody payload was:\n" + errstr += body + else: + errstr = str(err.response.reason) + raise ProviderRequestFailedError(error_reason=errstr) from err return response diff --git a/gateway-api/src/gateway_api/provider/test_client.py b/gateway-api/src/gateway_api/provider/test_client.py index cff97b45..56b891fb 100644 --- a/gateway-api/src/gateway_api/provider/test_client.py +++ b/gateway-api/src/gateway_api/provider/test_client.py @@ -19,10 +19,6 @@ from gateway_api.common.error import ProviderRequestFailedError from gateway_api.provider import GpProviderClient, client -if TYPE_CHECKING: - from requests import Response - from requests.structures import CaseInsensitiveDict - @pytest.fixture def stub() -> GpProviderStub: From 335eb79a9d92145a0d44940b712381ffa44c063f Mon Sep 17 00:00:00 2001 From: Ben Taylor <59649826+Vox-Ben@users.noreply.github.com> Date: Thu, 12 Mar 2026 12:57:55 +0000 Subject: [PATCH 14/18] Update TODO comment --- gateway-api/src/gateway_api/controller.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gateway-api/src/gateway_api/controller.py b/gateway-api/src/gateway_api/controller.py index 7249791c..a4fd89d9 100644 --- a/gateway-api/src/gateway_api/controller.py +++ b/gateway-api/src/gateway_api/controller.py @@ -121,7 +121,7 @@ def get_jwt_for_provider(self, provider_endpoint: str, consumer_ods: str) -> JWT prefix="Mr", ) - # TODO: Where do we get the consumer org name from? SDS? + # TODO: Where do we get the consumer org name from? SDS only returns ODS/ASID requesting_organization = Organization( ods_code=consumer_ods, name="Consumer organisation name" ) From 36e7f7f5e48b5969a34ef484df0096df5feed995 Mon Sep 17 00:00:00 2001 From: Ben Taylor <59649826+Vox-Ben@users.noreply.github.com> Date: Thu, 12 Mar 2026 15:25:18 +0000 Subject: [PATCH 15/18] Improve test coverage --- gateway-api/scripts/test_call_gateway.py | 201 ++++++++++++++++++ .../gateway_api/clinical_jwt/organization.py | 13 -- .../clinical_jwt/test_validator.py | 153 +++++++------ .../src/gateway_api/clinical_jwt/validator.py | 3 - .../src/gateway_api/common/test_common.py | 10 + .../src/gateway_api/provider/test_client.py | 38 ++++ 6 files changed, 338 insertions(+), 80 deletions(-) create mode 100644 gateway-api/scripts/test_call_gateway.py diff --git a/gateway-api/scripts/test_call_gateway.py b/gateway-api/scripts/test_call_gateway.py new file mode 100644 index 00000000..fed38ea6 --- /dev/null +++ b/gateway-api/scripts/test_call_gateway.py @@ -0,0 +1,201 @@ +"""Unit tests for call_gateway.py script.""" + +import json +import sys +from io import StringIO +from unittest.mock import MagicMock, patch + +import pytest +import requests +from call_gateway import main + + +class TestCallGateway: + """Test suite for the call_gateway script.""" + + def test_successful_request(self, monkeypatch: pytest.MonkeyPatch) -> None: + """Test successful POST request with valid NHS number.""" + nhs_number = "9690937278" + base_url = "https://example.com" + monkeypatch.setenv("BASE_URL", base_url) + monkeypatch.setattr(sys, "argv", ["call_gateway.py", nhs_number]) + + mock_response = MagicMock() + mock_response.status_code = 200 + mock_response.json.return_value = {"resourceType": "Bundle"} + + captured_output = StringIO() + + with ( + patch("requests.post", return_value=mock_response) as mock_post, + patch("sys.stdout", captured_output), + ): + main() + + mock_post.assert_called_once() + call_args = mock_post.call_args + assert call_args.args[0] == f"{base_url}/patient/$gpc.getstructuredrecord" + assert call_args.kwargs["headers"]["Content-Type"] == "application/fhir+json" + assert call_args.kwargs["headers"]["Accept"] == "application/fhir+json" + assert call_args.kwargs["headers"]["Ods-From"] == "S44444" + assert "Ssp-TraceID" in call_args.kwargs["headers"] + assert call_args.kwargs["json"]["resourceType"] == "Parameters" + assert ( + call_args.kwargs["json"]["parameter"][0]["valueIdentifier"]["value"] + == nhs_number + ) + assert call_args.kwargs["timeout"] == 10 + + output = captured_output.getvalue() + assert "Status Code: 200" in output + assert "Response:" in output + + def test_missing_base_url_environment_variable( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Test error handling when BASE_URL environment variable is not set.""" + monkeypatch.delenv("BASE_URL", raising=False) + monkeypatch.setattr(sys, "argv", ["call_gateway.py", "9690937278"]) + + captured_output = StringIO() + with ( + patch("sys.stdout", captured_output), + pytest.raises(SystemExit) as exc_info, + ): + main() + + assert exc_info.value.code == 1 + output = captured_output.getvalue() + assert "Error: BASE_URL environment variable is not set" in output + + def test_http_error_with_response_body( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Test handling of HTTP error with response body.""" + nhs_number = "9690937278" + monkeypatch.setenv("BASE_URL", "https://example.com") + monkeypatch.setattr(sys, "argv", ["call_gateway.py", nhs_number]) + + mock_response = MagicMock() + mock_response.status_code = 400 + mock_response.text = "Bad Request" + + http_error = requests.exceptions.HTTPError("HTTP Error") + http_error.response = mock_response + + captured_output = StringIO() + + with patch("requests.post") as mock_post: + mock_post.return_value.raise_for_status.side_effect = http_error + with ( + patch("sys.stdout", captured_output), + pytest.raises(SystemExit) as exc_info, + ): + main() + + assert exc_info.value.code == 1 + output = captured_output.getvalue() + assert "Error:" in output + assert "Status Code: 400" in output + assert "Response Body: Bad Request" in output + + def test_request_payload_structure(self, monkeypatch: pytest.MonkeyPatch) -> None: + """Test that request payload has correct FHIR structure.""" + nhs_number = "9690937278" + monkeypatch.setenv("BASE_URL", "https://example.com") + monkeypatch.setattr(sys, "argv", ["call_gateway.py", nhs_number]) + + mock_response = MagicMock() + mock_response.status_code = 200 + mock_response.json.return_value = {"resourceType": "Bundle"} + + with ( + patch("requests.post", return_value=mock_response) as mock_post, + patch("sys.stdout", StringIO()), + ): + main() + + call_args = mock_post.call_args + payload = call_args.kwargs["json"] + assert payload["resourceType"] == "Parameters" + assert len(payload["parameter"]) == 1 + assert payload["parameter"][0]["name"] == "patientNHSNumber" + assert ( + payload["parameter"][0]["valueIdentifier"]["system"] + == "https://fhir.nhs.uk/Id/nhs-number" + ) + assert payload["parameter"][0]["valueIdentifier"]["value"] == nhs_number + + def test_request_headers(self, monkeypatch: pytest.MonkeyPatch) -> None: + """Test that request headers are correctly set.""" + nhs_number = "9690937278" + monkeypatch.setenv("BASE_URL", "https://example.com") + monkeypatch.setattr(sys, "argv", ["call_gateway.py", nhs_number]) + + mock_response = MagicMock() + mock_response.status_code = 200 + mock_response.json.return_value = {"resourceType": "Bundle"} + + with ( + patch("requests.post", return_value=mock_response) as mock_post, + patch("sys.stdout", StringIO()), + ): + main() + + call_args = mock_post.call_args + headers = call_args.kwargs["headers"] + assert headers["Content-Type"] == "application/fhir+json" + assert headers["Accept"] == "application/fhir+json" + assert headers["Ods-From"] == "S44444" + assert "Ssp-TraceID" in headers + # Verify Ssp-TraceID is a valid UUID format + trace_id = headers["Ssp-TraceID"] + assert len(trace_id) == 36 + assert trace_id.count("-") == 4 + + def test_url_construction(self, monkeypatch: pytest.MonkeyPatch) -> None: + """Test that URL is correctly constructed from BASE_URL.""" + base_url = "https://test.example.com" + nhs_number = "9690937278" + monkeypatch.setenv("BASE_URL", base_url) + monkeypatch.setattr(sys, "argv", ["call_gateway.py", nhs_number]) + + mock_response = MagicMock() + mock_response.status_code = 200 + mock_response.json.return_value = {"resourceType": "Bundle"} + + with ( + patch("requests.post", return_value=mock_response) as mock_post, + patch("sys.stdout", StringIO()), + ): + main() + + call_args = mock_post.call_args + assert call_args.args[0] == f"{base_url}/patient/$gpc.getstructuredrecord" + + def test_output_format_for_successful_response( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Test output format for successful response includes status and JSON.""" + nhs_number = "9690937278" + monkeypatch.setenv("BASE_URL", "https://example.com") + monkeypatch.setattr(sys, "argv", ["call_gateway.py", nhs_number]) + + expected_json = {"resourceType": "Bundle", "id": "test-bundle"} + mock_response = MagicMock() + mock_response.status_code = 200 + mock_response.json.return_value = expected_json + + captured_output = StringIO() + + with ( + patch("requests.post", return_value=mock_response), + patch("sys.stdout", captured_output), + ): + main() + + output = captured_output.getvalue() + assert "Status Code: 200" in output + assert "Response:" in output + # Verify JSON is properly formatted + assert json.dumps(expected_json, indent=2) in output diff --git a/gateway-api/src/gateway_api/clinical_jwt/organization.py b/gateway-api/src/gateway_api/clinical_jwt/organization.py index 18f69354..4c2aa6d1 100644 --- a/gateway-api/src/gateway_api/clinical_jwt/organization.py +++ b/gateway-api/src/gateway_api/clinical_jwt/organization.py @@ -21,16 +21,3 @@ def to_dict(self) -> dict[str, Any]: ], "name": self.name, } - - @property - def json(self) -> dict[str, Any]: - """ - Return the Organization as a dictionary suitable for JWT payload. - Provided for backwards compatibility. - """ - return self.to_dict() - - def __str__(self) -> str: - import json - - return json.dumps(self.to_dict(), indent=2) diff --git a/gateway-api/src/gateway_api/clinical_jwt/test_validator.py b/gateway-api/src/gateway_api/clinical_jwt/test_validator.py index 914c307d..714ab796 100644 --- a/gateway-api/src/gateway_api/clinical_jwt/test_validator.py +++ b/gateway-api/src/gateway_api/clinical_jwt/test_validator.py @@ -17,6 +17,31 @@ def test_valid_jwt_passes(self, valid_jwt: JWT) -> None: except JWTValidationError as err: pytest.fail(f"JWT validation failed: {err}") + def test_all_missing_fields_reported_in_error(self) -> None: + """Test that all missing required fields are reported in a single error.""" + jwt = JWT( + issuer="", + subject="", + audience="", + requesting_device={}, + requesting_organization={}, + requesting_practitioner={}, + issued_at=None, # type: ignore + expiration=None, # type: ignore + ) + with pytest.raises(JWTValidationError) as exc_info: + JWTValidator.validate_required_fields(jwt) + error_message = str(exc_info.value) + # Verify all required fields are mentioned in the error + assert "issuer" in error_message + assert "subject" in error_message + assert "audience" in error_message + assert "requesting_device" in error_message + assert "requesting_organization" in error_message + assert "requesting_practitioner" in error_message + assert "issued_at" in error_message + assert "expiration" in error_message + class TestValidateTimestamps: """Tests for validate_timestamps method.""" @@ -83,29 +108,21 @@ def test_valid_device_passes(self, valid_jwt: JWT) -> None: """Test that a valid device passes validation.""" JWTValidator.validate_device(valid_jwt.requesting_device) # Should not raise - def test_missing_resource_type_raises_error(self, valid_jwt: JWT) -> None: - """Test that missing resourceType raises validation error.""" - device = valid_jwt.requesting_device.copy() - device["resourceType"] = "WrongType" + def test_invalid_device_reports_all_errors(self) -> None: + """Test that all device validation errors are reported.""" + device = { + "resourceType": "WrongType", + "identifier": [{"missing": "system_and_value"}], + # missing model and version + } with pytest.raises(JWTValidationError) as exc_info: JWTValidator.validate_device(device) - assert "resourceType must be 'Device'" in str(exc_info.value) - - def test_missing_identifier_raises_error(self, valid_jwt: JWT) -> None: - """Test that missing identifier raises validation error.""" - device = valid_jwt.requesting_device.copy() - del device["identifier"] - with pytest.raises(JWTValidationError) as exc_info: - JWTValidator.validate_device(device) - assert "identifier must be a non-empty list" in str(exc_info.value) - - def test_missing_model_raises_error(self, valid_jwt: JWT) -> None: - """Test that missing model raises validation error.""" - device = valid_jwt.requesting_device.copy() - del device["model"] - with pytest.raises(JWTValidationError) as exc_info: - JWTValidator.validate_device(device) - assert "model is required" in str(exc_info.value) + error_message = str(exc_info.value) + assert "resourceType must be 'Device'" in error_message + assert "identifier[0].system is required" in error_message + assert "identifier[0].value is required" in error_message + assert "model is required" in error_message + assert "version is required" in error_message class TestValidateOrganization: @@ -115,21 +132,20 @@ def test_valid_organization_passes(self, valid_jwt: JWT) -> None: """Test that a valid organization passes validation.""" JWTValidator.validate_organization(valid_jwt.requesting_organization) - def test_missing_resource_type_raises_error(self, valid_jwt: JWT) -> None: - """Test that missing resourceType raises validation error.""" - org = valid_jwt.requesting_organization.copy() - org["resourceType"] = "WrongType" - with pytest.raises(JWTValidationError) as exc_info: - JWTValidator.validate_organization(org) - assert "resourceType must be 'Organization'" in str(exc_info.value) - - def test_missing_name_raises_error(self, valid_jwt: JWT) -> None: - """Test that missing name raises validation error.""" - org = valid_jwt.requesting_organization.copy() - del org["name"] + def test_invalid_organization_reports_all_errors(self) -> None: + """Test that all organization validation errors are reported.""" + org = { + "resourceType": "WrongType", + "identifier": [{"missing": "system_and_value"}], + # missing name + } with pytest.raises(JWTValidationError) as exc_info: JWTValidator.validate_organization(org) - assert "name is required" in str(exc_info.value) + error_message = str(exc_info.value) + assert "resourceType must be 'Organization'" in error_message + assert "identifier[0].system is required" in error_message + assert "identifier[0].value is required" in error_message + assert "name is required" in error_message class TestValidatePractitioner: @@ -139,21 +155,26 @@ def test_valid_practitioner_passes(self, valid_jwt: JWT) -> None: """Test that a valid practitioner passes validation.""" JWTValidator.validate_practitioner(valid_jwt.requesting_practitioner) - def test_missing_resource_type_raises_error(self, valid_jwt: JWT) -> None: - """Test that missing resourceType raises validation error.""" - pract = valid_jwt.requesting_practitioner.copy() - pract["resourceType"] = "WrongType" - with pytest.raises(JWTValidationError) as exc_info: - JWTValidator.validate_practitioner(pract) - assert "resourceType must be 'Practitioner'" in str(exc_info.value) - - def test_missing_id_raises_error(self, valid_jwt: JWT) -> None: - """Test that missing id raises validation error.""" - pract = valid_jwt.requesting_practitioner.copy() - del pract["id"] + def test_invalid_practitioner_reports_all_errors(self) -> None: + """Test that all practitioner validation errors are reported.""" + pract = { + "resourceType": "WrongType", + # missing id + "identifier": [ + {"missing": "system_and_value"}, + {"system": "sys2", "value": "val2"}, + {"system": "sys3", "value": "val3"}, + ], # 3 items but first missing system/value + "name": [{"given": ["Test"]}], # Missing family + } with pytest.raises(JWTValidationError) as exc_info: JWTValidator.validate_practitioner(pract) - assert "id is required" in str(exc_info.value) + error_message = str(exc_info.value) + assert "resourceType must be 'Practitioner'" in error_message + assert "id is required" in error_message + assert "identifier[0].system is required" in error_message + assert "identifier[0].value is required" in error_message + assert "family is required" in error_message def test_insufficient_identifiers_raises_error(self, valid_jwt: JWT) -> None: """Test that less than 3 identifiers raises validation error.""" @@ -163,14 +184,6 @@ def test_insufficient_identifiers_raises_error(self, valid_jwt: JWT) -> None: JWTValidator.validate_practitioner(pract) assert "at least 3 items" in str(exc_info.value) - def test_missing_family_name_raises_error(self, valid_jwt: JWT) -> None: - """Test that missing family name raises validation error.""" - pract = valid_jwt.requesting_practitioner.copy() - pract["name"] = [{"given": ["Test"]}] # Missing family - with pytest.raises(JWTValidationError) as exc_info: - JWTValidator.validate_practitioner(pract) - assert "family is required" in str(exc_info.value) - class TestValidate: """Tests for the main validate method.""" @@ -179,21 +192,33 @@ def test_valid_jwt_passes_all_validation(self, valid_jwt: JWT) -> None: """Test that a completely valid JWT passes all validation.""" JWTValidator.validate(valid_jwt) # Should not raise - def test_invalid_jwt_reports_all_errors(self, valid_jwt: JWT) -> None: - """Test that validation reports all errors, not just the first one.""" + def test_multiple_validation_errors_collected(self, valid_jwt: JWT) -> None: + """ + Test that all validation errors from all validators are collected and + reported together. + """ jwt = JWT( - issuer="", # Invalid - missing required field + issuer="", # Invalid - missing issuer subject=valid_jwt.subject, audience=valid_jwt.audience, - requesting_device={"resourceType": "Wrong"}, # Invalid - wrong resourceType - requesting_organization=valid_jwt.requesting_organization, - requesting_practitioner=valid_jwt.requesting_practitioner, - issued_at=valid_jwt.issued_at, + requesting_device={ + "resourceType": "Wrong", # Invalid - wrong resourceType + }, + requesting_organization={ + "resourceType": "Wrong", # Invalid - wrong resourceType + }, + requesting_practitioner={ + "resourceType": "Wrong", # Invalid - wrong resourceType + }, + issued_at="not an int", # type: ignore # Invalid - not an integer expiration=valid_jwt.expiration, ) with pytest.raises(JWTValidationError) as exc_info: JWTValidator.validate(jwt) error_message = str(exc_info.value) - # Should contain both the missing issuer error and the device error + # Should contain errors from all validators assert "issuer" in error_message - assert "Device" in error_message or "device" in error_message + assert "timestamp" in error_message or "unix timestamp" in error_message + assert "Device" in error_message + assert "Organization" in error_message + assert "Practitioner" in error_message diff --git a/gateway-api/src/gateway_api/clinical_jwt/validator.py b/gateway-api/src/gateway_api/clinical_jwt/validator.py index d819541b..09c3e841 100644 --- a/gateway-api/src/gateway_api/clinical_jwt/validator.py +++ b/gateway-api/src/gateway_api/clinical_jwt/validator.py @@ -151,9 +151,6 @@ def _validate_practitioner_identifiers( def _validate_practitioner_name(names: list[dict[str, Any]]) -> list[str]: """Validate practitioner name list structure and contents.""" errors = [] - if len(names) == 0: - errors.append("name list cannot be empty") - return errors name = names[0] if not name.get("family"): diff --git a/gateway-api/src/gateway_api/common/test_common.py b/gateway-api/src/gateway_api/common/test_common.py index 5deea64f..e8026b20 100644 --- a/gateway-api/src/gateway_api/common/test_common.py +++ b/gateway-api/src/gateway_api/common/test_common.py @@ -2,6 +2,8 @@ Unit tests for :mod:`gateway_api.common.common`. """ +from http import HTTPStatus + import pytest from gateway_api.common import common @@ -53,3 +55,11 @@ def test_validate_nhs_number_check_edge_cases_10_and_11( # All zeros => weighted sum 0 => remainder 0 => check 11 => mapped to 0 => valid # with check digit 0 assert common.validate_nhs_number(nhs_number) is expected + + +def test_get_http_text() -> None: + """ + Validate that get_http_text returns the correct phrase for common HTTP status codes. + """ + for status in [200, 400, 500]: + assert common.get_http_text(status) == HTTPStatus(status).phrase diff --git a/gateway-api/src/gateway_api/provider/test_client.py b/gateway-api/src/gateway_api/provider/test_client.py index 56b891fb..16f41da0 100644 --- a/gateway-api/src/gateway_api/provider/test_client.py +++ b/gateway-api/src/gateway_api/provider/test_client.py @@ -280,3 +280,41 @@ def test_gpprovider_client_includes_authorization_header_with_bearer_token( assert "Authorization" in captured_headers assert captured_headers["Authorization"] == f"Bearer {valid_jwt}" assert result.status_code == 200 + + +def test_access_structured_record_debug_error_when_cdg_debug_set( + mock_request_post: dict[str, Any], # NOQA ARG001 (Mock not called directly) + valid_simple_request_payload: Parameters, + valid_jwt: JWT, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """ + Test that the `access_structured_record` method raises a detailed debug error + when the CDG_DEBUG environment variable is set to 'true'. + """ + monkeypatch.setenv("CDG_DEBUG", "true") + + provider_asid = "200000001154" + consumer_asid = "200000001152" + provider_endpoint = "https://test.com" + trace_id = "invalid for test" + request_body = json.dumps(valid_simple_request_payload) + + client = GpProviderClient( + provider_endpoint=provider_endpoint, + provider_asid=provider_asid, + consumer_asid=consumer_asid, + token=valid_jwt, + ) + + with pytest.raises(ProviderRequestFailedError) as exc_info: + client.access_structured_record(trace_id, request_body) + + error_message = str(exc_info.value) + # Verify detailed debug information is included + assert "GPProvider FHIR API request failed:" in error_message + assert "400:" in error_message + assert "Bad Request" in error_message + assert "Headers were:" in error_message + assert "Body payload was:" in error_message + assert request_body in error_message From 6db5298f88f7c1c34a11fa208d0f8fa45880e10d Mon Sep 17 00:00:00 2001 From: Ben Taylor <59649826+Vox-Ben@users.noreply.github.com> Date: Thu, 12 Mar 2026 15:53:52 +0000 Subject: [PATCH 16/18] Fix JWT default times and encoding value --- gateway-api/src/gateway_api/clinical_jwt/jwt.py | 6 +++++- gateway-api/src/gateway_api/provider/client.py | 6 +++--- gateway-api/src/gateway_api/provider/test_client.py | 4 ++-- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/gateway-api/src/gateway_api/clinical_jwt/jwt.py b/gateway-api/src/gateway_api/clinical_jwt/jwt.py index b0f3eb3e..085b1338 100644 --- a/gateway-api/src/gateway_api/clinical_jwt/jwt.py +++ b/gateway-api/src/gateway_api/clinical_jwt/jwt.py @@ -17,7 +17,7 @@ class JWT: # Time fields issued_at: int = field(default_factory=lambda: int(time())) - expiration: int = field(default_factory=lambda: int(time()) + 300) + expiration: int = 0 # These are here for future proofing but are not expected ever to be changed algorithm: str = "none" @@ -25,6 +25,10 @@ class JWT: reason_for_request: str = "directcare" requested_scope: str = "patient/*.read" + def __post_init__(self) -> None: + if self.expiration == 0: + object.__setattr__(self, "expiration", self.issued_at + 300) + @property def issue_time(self) -> str: return datetime.fromtimestamp(self.issued_at, tz=UTC).isoformat() diff --git a/gateway-api/src/gateway_api/provider/client.py b/gateway-api/src/gateway_api/provider/client.py index 305eb399..220cd979 100644 --- a/gateway-api/src/gateway_api/provider/client.py +++ b/gateway-api/src/gateway_api/provider/client.py @@ -62,7 +62,7 @@ class GpProviderClient: consumer_asid (str): The ASID for the consumer. token (JWT): JWT object for authentication with the provider API. endpoint_path (str): The endpoint path for the operation - (default: "FHIR/STU3/patient/$gpc.getstructuredrecord"). + (default: "Patient/$gpc.getstructuredrecord"). Methods: access_structured_record(trace_id: str, body: str) -> Response: @@ -89,8 +89,8 @@ def _build_headers(self, trace_id: str) -> dict[str, str]: """ # TODO: Post-steel-thread, probably check whether JWT is valid/not expired return { - "Content-Type": "application/fhir+json;charset=utf-8", - "Accept": "application/fhir+json;charset=utf-8", + "Content-Type": "application/fhir+json; charset=utf-8", + "Accept": "application/fhir+json; charset=utf-8", "Ssp-InteractionID": ACCESS_RECORD_STRUCTURED_INTERACTION_ID, "Ssp-To": self.provider_asid, "Ssp-From": self.consumer_asid, diff --git a/gateway-api/src/gateway_api/provider/test_client.py b/gateway-api/src/gateway_api/provider/test_client.py index 16f41da0..2c403219 100644 --- a/gateway-api/src/gateway_api/provider/test_client.py +++ b/gateway-api/src/gateway_api/provider/test_client.py @@ -116,8 +116,8 @@ def test_valid_gpprovider_access_structured_record_with_correct_headers_post_200 token=valid_jwt, ) expected_headers = { - "Content-Type": "application/fhir+json;charset=utf-8", - "Accept": "application/fhir+json;charset=utf-8", + "Content-Type": "application/fhir+json; charset=utf-8", + "Accept": "application/fhir+json; charset=utf-8", "Ssp-TraceID": str(trace_id), "Ssp-From": consumer_asid, "Ssp-To": provider_asid, From 42e28df73391f9d2567fd946ca166c3ed04984b6 Mon Sep 17 00:00:00 2001 From: Ben Taylor <59649826+Vox-Ben@users.noreply.github.com> Date: Thu, 12 Mar 2026 16:53:38 +0000 Subject: [PATCH 17/18] Move FHIR constants to single enum --- gateway-api/scripts/call_gateway.py | 3 +- gateway-api/scripts/test_call_gateway.py | 3 +- gateway-api/src/fhir/constants.py | 15 ++ .../gateway_api/clinical_jwt/organization.py | 4 +- .../gateway_api/clinical_jwt/practitioner.py | 6 +- gateway-api/src/gateway_api/conftest.py | 13 +- .../src/gateway_api/pds/test_client.py | 13 +- gateway-api/src/gateway_api/sds/client.py | 19 +- .../src/gateway_api/sds/test_client.py | 28 ++- gateway-api/src/scripts/__init__.py | 1 + gateway-api/src/scripts/call_gateway.py | 72 +++++++ gateway-api/src/scripts/test_call_gateway.py | 203 ++++++++++++++++++ gateway-api/stubs/stubs/sds/stub.py | 41 ++-- gateway-api/tests/conftest.py | 3 +- .../tests/contract/test_consumer_contract.py | 7 +- .../tests/integration/test_sds_search.py | 8 +- 16 files changed, 369 insertions(+), 70 deletions(-) create mode 100644 gateway-api/src/fhir/constants.py create mode 100644 gateway-api/src/scripts/__init__.py create mode 100644 gateway-api/src/scripts/call_gateway.py create mode 100644 gateway-api/src/scripts/test_call_gateway.py diff --git a/gateway-api/scripts/call_gateway.py b/gateway-api/scripts/call_gateway.py index 1f9ca25e..e75f1f52 100644 --- a/gateway-api/scripts/call_gateway.py +++ b/gateway-api/scripts/call_gateway.py @@ -7,6 +7,7 @@ from uuid import uuid4 import requests +from fhir.constants import FHIRSystem def main() -> None: @@ -43,7 +44,7 @@ def main() -> None: { "name": "patientNHSNumber", "valueIdentifier": { - "system": "https://fhir.nhs.uk/Id/nhs-number", + "system": FHIRSystem.NHS_NUMBER, "value": args.nhs_number, }, } diff --git a/gateway-api/scripts/test_call_gateway.py b/gateway-api/scripts/test_call_gateway.py index fed38ea6..d7b7670d 100644 --- a/gateway-api/scripts/test_call_gateway.py +++ b/gateway-api/scripts/test_call_gateway.py @@ -8,6 +8,7 @@ import pytest import requests from call_gateway import main +from fhir.constants import FHIRSystem class TestCallGateway: @@ -122,7 +123,7 @@ def test_request_payload_structure(self, monkeypatch: pytest.MonkeyPatch) -> Non assert payload["parameter"][0]["name"] == "patientNHSNumber" assert ( payload["parameter"][0]["valueIdentifier"]["system"] - == "https://fhir.nhs.uk/Id/nhs-number" + == FHIRSystem.NHS_NUMBER ) assert payload["parameter"][0]["valueIdentifier"]["value"] == nhs_number diff --git a/gateway-api/src/fhir/constants.py b/gateway-api/src/fhir/constants.py new file mode 100644 index 00000000..b6773e82 --- /dev/null +++ b/gateway-api/src/fhir/constants.py @@ -0,0 +1,15 @@ +from enum import StrEnum + + +class FHIRSystem(StrEnum): + """ + Enum for FHIR identifier systems used in the clinical data gateway. + """ + + NHS_NUMBER = "https://fhir.nhs.uk/Id/nhs-number" + ODS_CODE = "https://fhir.nhs.uk/Id/ods-organization-code" + SDS_USER_ID = "https://fhir.nhs.uk/Id/sds-user-id" + SDS_ROLE_PROFILE_ID = "https://fhir.nhs.uk/Id/sds-role-profile-id" + NHS_SERVICE_INTERACTION_ID = "https://fhir.nhs.uk/Id/nhsServiceInteractionId" + NHS_MHS_PARTY_KEY = "https://fhir.nhs.uk/Id/nhsMhsPartyKey" + NHS_SPINE_ASID = "https://fhir.nhs.uk/Id/nhsSpineASID" diff --git a/gateway-api/src/gateway_api/clinical_jwt/organization.py b/gateway-api/src/gateway_api/clinical_jwt/organization.py index 4c2aa6d1..8beb367f 100644 --- a/gateway-api/src/gateway_api/clinical_jwt/organization.py +++ b/gateway-api/src/gateway_api/clinical_jwt/organization.py @@ -1,6 +1,8 @@ from dataclasses import dataclass from typing import Any +from fhir.constants import FHIRSystem + @dataclass(frozen=True, kw_only=True) class Organization: @@ -15,7 +17,7 @@ def to_dict(self) -> dict[str, Any]: "resourceType": "Organization", "identifier": [ { - "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "system": FHIRSystem.ODS_CODE, "value": self.ods_code, } ], diff --git a/gateway-api/src/gateway_api/clinical_jwt/practitioner.py b/gateway-api/src/gateway_api/clinical_jwt/practitioner.py index 0475fa5e..0041dd59 100644 --- a/gateway-api/src/gateway_api/clinical_jwt/practitioner.py +++ b/gateway-api/src/gateway_api/clinical_jwt/practitioner.py @@ -1,6 +1,8 @@ from dataclasses import dataclass from typing import Any +from fhir.constants import FHIRSystem + @dataclass(frozen=True, kw_only=True) class Practitioner: @@ -26,8 +28,8 @@ def to_dict(self) -> dict[str, Any]: """ Return the Practitioner as a dictionary suitable for JWT payload. """ - user_id_system = "https://fhir.nhs.uk/Id/sds-user-id" - role_id_system = "https://fhir.nhs.uk/Id/sds-role-profile-id" + user_id_system = FHIRSystem.SDS_USER_ID + role_id_system = FHIRSystem.SDS_ROLE_PROFILE_ID return { "resourceType": "Practitioner", diff --git a/gateway-api/src/gateway_api/conftest.py b/gateway-api/src/gateway_api/conftest.py index 0757ea11..656785cc 100644 --- a/gateway-api/src/gateway_api/conftest.py +++ b/gateway-api/src/gateway_api/conftest.py @@ -7,6 +7,7 @@ import pytest import requests from fhir import Bundle, OperationOutcome, Patient +from fhir.constants import FHIRSystem from fhir.parameters import Parameters from flask import Request from requests.structures import CaseInsensitiveDict @@ -62,7 +63,7 @@ def valid_simple_request_payload() -> Parameters: { "name": "patientNHSNumber", "valueIdentifier": { - "system": "https://fhir.nhs.uk/Id/nhs-number", + "system": FHIRSystem.NHS_NUMBER, "value": "9999999999", }, }, @@ -103,7 +104,7 @@ def valid_simple_response_payload() -> Bundle: "identifier": { "value": "A12345", "period": {"start": "2020-01-01", "end": "9999-12-31"}, - "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "system": FHIRSystem.ODS_CODE, }, } ], @@ -143,7 +144,7 @@ def happy_path_pds_response_body() -> Patient: "identifier": { "value": "A12345", "period": {"start": "2020-01-01", "end": "9999-12-31"}, - "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "system": FHIRSystem.ODS_CODE, }, } ], @@ -174,7 +175,7 @@ def valid_jwt() -> JWT: "resourceType": "Organization", "identifier": [ { - "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "system": FHIRSystem.ODS_CODE, "value": "T1234", } ], @@ -184,9 +185,9 @@ def valid_jwt() -> JWT: "resourceType": "Practitioner", "id": "prac123", "identifier": [ - {"system": "https://fhir.nhs.uk/Id/sds-user-id", "value": "user123"}, + {"system": FHIRSystem.SDS_USER_ID, "value": "user123"}, { - "system": "https://fhir.nhs.uk/Id/sds-role-profile-id", + "system": FHIRSystem.SDS_ROLE_PROFILE_ID, "value": "role123", }, {"system": "https://example.com/userid", "value": "userid123"}, diff --git a/gateway-api/src/gateway_api/pds/test_client.py b/gateway-api/src/gateway_api/pds/test_client.py index d3571d43..95bade84 100644 --- a/gateway-api/src/gateway_api/pds/test_client.py +++ b/gateway-api/src/gateway_api/pds/test_client.py @@ -8,6 +8,7 @@ import pytest from fhir import Patient +from fhir.constants import FHIRSystem from pytest_mock import MockerFixture from gateway_api.common.error import PdsRequestFailedError @@ -144,7 +145,7 @@ def test_search_patient_by_nhs_number_finds_current_gp_ods_code_when_pds_returns "identifier": { "value": "OLDGP", "period": {"start": "2010-01-01", "end": "2012-01-01"}, - "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "system": FHIRSystem.ODS_CODE, }, } current_gp: GeneralPractitioner = { @@ -153,7 +154,7 @@ def test_search_patient_by_nhs_number_finds_current_gp_ods_code_when_pds_returns "identifier": { "value": "CURRGP", "period": {"start": "2020-01-01", "end": "9999-01-01"}, - "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "system": FHIRSystem.ODS_CODE, }, } pds_response_body_with_two_gps = happy_path_pds_response_body.copy() @@ -187,7 +188,7 @@ def test_find_current_gp_with_today_override() -> None: "identifier": { "value": "a", "period": {"start": "2020-01-01", "end": "2020-12-31"}, - "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "system": FHIRSystem.ODS_CODE, }, }, { @@ -196,7 +197,7 @@ def test_find_current_gp_with_today_override() -> None: "identifier": { "value": "b", "period": {"start": "2021-01-01", "end": "2021-12-31"}, - "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "system": FHIRSystem.ODS_CODE, }, }, ] @@ -349,7 +350,7 @@ def test_find_current_gp_ignore_dates_returns_last_or_none() -> None: "identifier": { "value": "GP-OLD", "period": {"start": "1900-01-01", "end": "1900-12-31"}, - "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "system": FHIRSystem.ODS_CODE, }, }, { @@ -358,7 +359,7 @@ def test_find_current_gp_ignore_dates_returns_last_or_none() -> None: "identifier": { "value": "GP-NEWER", "period": {"start": "1901-01-01", "end": "1901-12-31"}, - "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "system": FHIRSystem.ODS_CODE, }, }, ] diff --git a/gateway-api/src/gateway_api/sds/client.py b/gateway-api/src/gateway_api/sds/client.py index a2f7d640..01e2c283 100644 --- a/gateway-api/src/gateway_api/sds/client.py +++ b/gateway-api/src/gateway_api/sds/client.py @@ -12,6 +12,7 @@ from enum import StrEnum from typing import Any, cast +from fhir.constants import FHIRSystem from stubs import SdsFhirApiStub from gateway_api.get_structured_record import ACCESS_RECORD_STRUCTURED_INTERACTION_ID @@ -76,12 +77,6 @@ class SdsClient: SANDBOX_URL = "https://sandbox.api.service.nhs.uk/spine-directory/FHIR/R4" INT_URL = "https://int.api.service.nhs.uk/spine-directory/FHIR/R4" - # FHIR identifier systems - ODS_SYSTEM = "https://fhir.nhs.uk/Id/ods-organization-code" - INTERACTION_SYSTEM = "https://fhir.nhs.uk/Id/nhsServiceInteractionId" - PARTYKEY_SYSTEM = "https://fhir.nhs.uk/Id/nhsMhsPartyKey" - ASID_SYSTEM = "https://fhir.nhs.uk/Id/nhsSpineASID" - # Default service interaction ID for GP Connect DEFAULT_SERVICE_INTERACTION_ID = ACCESS_RECORD_STRUCTURED_INTERACTION_ID @@ -138,8 +133,8 @@ def get_org_details( # TODO: Post-steel-thread handle case where no device is found for ODS code - asid = self._extract_identifier(device, self.ASID_SYSTEM) - party_key = self._extract_identifier(device, self.PARTYKEY_SYSTEM) + asid = self._extract_identifier(device, FHIRSystem.NHS_SPINE_ASID) + party_key = self._extract_identifier(device, FHIRSystem.NHS_MHS_PARTY_KEY) # Step 2: Get Endpoint to obtain endpoint URL endpoint_url: str | None = None @@ -190,12 +185,14 @@ def _query_sds( url = f"{self.base_url}/{querytype.value}" params: dict[str, Any] = { - "organization": f"{self.ODS_SYSTEM}|{ods_code}", - "identifier": [f"{self.INTERACTION_SYSTEM}|{self.service_interaction_id}"], + "organization": f"{FHIRSystem.ODS_CODE}|{ods_code}", + "identifier": [ + f"{FHIRSystem.NHS_SERVICE_INTERACTION_ID}|{self.service_interaction_id}" + ], } if party_key is not None: - params["identifier"].append(f"{self.PARTYKEY_SYSTEM}|{party_key}") + params["identifier"].append(f"{FHIRSystem.NHS_MHS_PARTY_KEY}|{party_key}") response = get( url, diff --git a/gateway-api/src/gateway_api/sds/test_client.py b/gateway-api/src/gateway_api/sds/test_client.py index 3d7ff58c..d0c28088 100644 --- a/gateway-api/src/gateway_api/sds/test_client.py +++ b/gateway-api/src/gateway_api/sds/test_client.py @@ -5,6 +5,7 @@ from __future__ import annotations import pytest +from fhir.constants import FHIRSystem from stubs.sds.stub import SdsFhirApiStub from gateway_api.get_structured_record import ACCESS_RECORD_STRUCTURED_INTERACTION_ID @@ -70,17 +71,17 @@ def test_sds_client_get_org_details_with_endpoint( "id": "test-device-id", "identifier": [ { - "system": "https://fhir.nhs.uk/Id/nhsSpineASID", + "system": FHIRSystem.NHS_SPINE_ASID, "value": "999999999999", }, { - "system": "https://fhir.nhs.uk/Id/nhsMhsPartyKey", + "system": FHIRSystem.NHS_MHS_PARTY_KEY, "value": "TESTORG-123456", }, ], "owner": { "identifier": { - "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "system": FHIRSystem.ODS_CODE, "value": "TESTORG", } }, @@ -98,13 +99,13 @@ def test_sds_client_get_org_details_with_endpoint( "address": "https://testorg.example.com/fhir", "managingOrganization": { "identifier": { - "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "system": FHIRSystem.ODS_CODE, "value": "TESTORG", } }, "identifier": [ { - "system": "https://fhir.nhs.uk/Id/nhsMhsPartyKey", + "system": FHIRSystem.NHS_MHS_PARTY_KEY, "value": "TESTORG-123456", } ], @@ -179,13 +180,13 @@ def test_sds_client_custom_service_interaction_id( "id": "custom-device", "identifier": [ { - "system": "https://fhir.nhs.uk/Id/nhsSpineASID", + "system": FHIRSystem.NHS_SPINE_ASID, "value": "777777777777", } ], "owner": { "identifier": { - "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "system": FHIRSystem.ODS_CODE, "value": "CUSTOMINT", } }, @@ -226,16 +227,13 @@ def test_sds_client_builds_correct_device_query_params( params = stub.get_params # Check organization parameter - assert ( - params["organization"] - == "https://fhir.nhs.uk/Id/ods-organization-code|PROVIDER" - ) + assert params["organization"] == f"{FHIRSystem.ODS_CODE}|PROVIDER" # Check identifier list contains interaction ID identifiers = params["identifier"] assert isinstance(identifiers, list) assert any( - "https://fhir.nhs.uk/Id/nhsServiceInteractionId|" in str(ident) + f"{FHIRSystem.NHS_SERVICE_INTERACTION_ID}|" in str(ident) for ident in identifiers ) @@ -261,11 +259,11 @@ def test_sds_client_extract_party_key_from_device( "id": "device-with-party-key", "identifier": [ { - "system": "https://fhir.nhs.uk/Id/nhsSpineASID", + "system": FHIRSystem.NHS_SPINE_ASID, "value": "888888888888", }, { - "system": "https://fhir.nhs.uk/Id/nhsMhsPartyKey", + "system": FHIRSystem.NHS_MHS_PARTY_KEY, "value": "WITHPARTYKEY-654321", }, ], @@ -283,7 +281,7 @@ def test_sds_client_extract_party_key_from_device( "address": "https://withpartykey.example.com/fhir", "identifier": [ { - "system": "https://fhir.nhs.uk/Id/nhsMhsPartyKey", + "system": FHIRSystem.NHS_MHS_PARTY_KEY, "value": "WITHPARTYKEY-654321", } ], diff --git a/gateway-api/src/scripts/__init__.py b/gateway-api/src/scripts/__init__.py new file mode 100644 index 00000000..7af4f0b6 --- /dev/null +++ b/gateway-api/src/scripts/__init__.py @@ -0,0 +1 @@ +"""Scripts package for gateway-api utility scripts.""" diff --git a/gateway-api/src/scripts/call_gateway.py b/gateway-api/src/scripts/call_gateway.py new file mode 100644 index 00000000..e75f1f52 --- /dev/null +++ b/gateway-api/src/scripts/call_gateway.py @@ -0,0 +1,72 @@ +#!/usr/bin/env python3 + +import argparse +import json +import os +import sys +from uuid import uuid4 + +import requests +from fhir.constants import FHIRSystem + + +def main() -> None: + # Parse command-line arguments + parser = argparse.ArgumentParser( + description="POST request to GPC getstructuredrecord endpoint" + ) + parser.add_argument( + "nhs_number", help="NHS number to search for (e.g., 9690937278)" + ) + args = parser.parse_args() + + # Check if BASE_URL is set + base_url = os.environ.get("BASE_URL") + if not base_url: + print("Error: BASE_URL environment variable is not set") + sys.exit(1) + + # Endpoint URL + url = f"{base_url}/patient/$gpc.getstructuredrecord" + + # Request headers + headers = { + "Content-Type": "application/fhir+json", + "Accept": "application/fhir+json", + "Ssp-TraceID": str(uuid4()), + "Ods-From": "S44444", + } + + # Request body + payload = { + "resourceType": "Parameters", + "parameter": [ + { + "name": "patientNHSNumber", + "valueIdentifier": { + "system": FHIRSystem.NHS_NUMBER, + "value": args.nhs_number, + }, + } + ], + } + + # Make the POST request + try: + response = requests.post(url, headers=headers, json=payload, timeout=10) + response.raise_for_status() + + print(f"Status Code: {response.status_code}") + print(f"Response:\n{json.dumps(response.json(), indent=2)}") + + except requests.exceptions.RequestException as e: + errtext = f"Error: {e}\n" + if e.response is not None: + errtext += f"Status Code: {e.response.status_code}\n" + errtext += f"Response Body: {e.response.text}\n" + print(errtext) + sys.exit(1) + + +if __name__ == "__main__": + main() diff --git a/gateway-api/src/scripts/test_call_gateway.py b/gateway-api/src/scripts/test_call_gateway.py new file mode 100644 index 00000000..f54307e7 --- /dev/null +++ b/gateway-api/src/scripts/test_call_gateway.py @@ -0,0 +1,203 @@ +"""Unit tests for call_gateway.py script.""" + +import json +import sys +from io import StringIO +from unittest.mock import MagicMock, patch + +import pytest +import requests +from fhir.constants import FHIRSystem + +from scripts.call_gateway import main + + +class TestCallGateway: + """Test suite for the call_gateway script.""" + + def test_successful_request(self, monkeypatch: pytest.MonkeyPatch) -> None: + """Test successful POST request with valid NHS number.""" + nhs_number = "9690937278" + base_url = "https://example.com" + monkeypatch.setenv("BASE_URL", base_url) + monkeypatch.setattr(sys, "argv", ["call_gateway.py", nhs_number]) + + mock_response = MagicMock() + mock_response.status_code = 200 + mock_response.json.return_value = {"resourceType": "Bundle"} + + captured_output = StringIO() + + with ( + patch("requests.post", return_value=mock_response) as mock_post, + patch("sys.stdout", captured_output), + ): + main() + + mock_post.assert_called_once() + call_args = mock_post.call_args + assert call_args.args[0] == f"{base_url}/patient/$gpc.getstructuredrecord" + assert call_args.kwargs["headers"]["Content-Type"] == "application/fhir+json" + assert call_args.kwargs["headers"]["Accept"] == "application/fhir+json" + assert call_args.kwargs["headers"]["Ods-From"] == "S44444" + assert "Ssp-TraceID" in call_args.kwargs["headers"] + assert call_args.kwargs["json"]["resourceType"] == "Parameters" + assert ( + call_args.kwargs["json"]["parameter"][0]["valueIdentifier"]["value"] + == nhs_number + ) + assert call_args.kwargs["timeout"] == 10 + + output = captured_output.getvalue() + assert "Status Code: 200" in output + assert "Response:" in output + + def test_missing_base_url_environment_variable( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Test error handling when BASE_URL environment variable is not set.""" + monkeypatch.delenv("BASE_URL", raising=False) + monkeypatch.setattr(sys, "argv", ["call_gateway.py", "9690937278"]) + + captured_output = StringIO() + with ( + patch("sys.stdout", captured_output), + pytest.raises(SystemExit) as exc_info, + ): + main() + + assert exc_info.value.code == 1 + output = captured_output.getvalue() + assert "Error: BASE_URL environment variable is not set" in output + + def test_http_error_with_response_body( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Test handling of HTTP error with response body.""" + nhs_number = "9690937278" + monkeypatch.setenv("BASE_URL", "https://example.com") + monkeypatch.setattr(sys, "argv", ["call_gateway.py", nhs_number]) + + mock_response = MagicMock() + mock_response.status_code = 400 + mock_response.text = "Bad Request" + + http_error = requests.exceptions.HTTPError("HTTP Error") + http_error.response = mock_response + + captured_output = StringIO() + + with patch("requests.post") as mock_post: + mock_post.return_value.raise_for_status.side_effect = http_error + with ( + patch("sys.stdout", captured_output), + pytest.raises(SystemExit) as exc_info, + ): + main() + + assert exc_info.value.code == 1 + output = captured_output.getvalue() + assert "Error:" in output + assert "Status Code: 400" in output + assert "Response Body: Bad Request" in output + + def test_request_payload_structure(self, monkeypatch: pytest.MonkeyPatch) -> None: + """Test that request payload has correct FHIR structure.""" + nhs_number = "9690937278" + monkeypatch.setenv("BASE_URL", "https://example.com") + monkeypatch.setattr(sys, "argv", ["call_gateway.py", nhs_number]) + + mock_response = MagicMock() + mock_response.status_code = 200 + mock_response.json.return_value = {"resourceType": "Bundle"} + + with ( + patch("requests.post", return_value=mock_response) as mock_post, + patch("sys.stdout", StringIO()), + ): + main() + + call_args = mock_post.call_args + payload = call_args.kwargs["json"] + assert payload["resourceType"] == "Parameters" + assert len(payload["parameter"]) == 1 + assert payload["parameter"][0]["name"] == "patientNHSNumber" + assert ( + payload["parameter"][0]["valueIdentifier"]["system"] + == FHIRSystem.NHS_NUMBER + ) + assert payload["parameter"][0]["valueIdentifier"]["value"] == nhs_number + + def test_request_headers(self, monkeypatch: pytest.MonkeyPatch) -> None: + """Test that request headers are correctly set.""" + nhs_number = "9690937278" + monkeypatch.setenv("BASE_URL", "https://example.com") + monkeypatch.setattr(sys, "argv", ["call_gateway.py", nhs_number]) + + mock_response = MagicMock() + mock_response.status_code = 200 + mock_response.json.return_value = {"resourceType": "Bundle"} + + with ( + patch("requests.post", return_value=mock_response) as mock_post, + patch("sys.stdout", StringIO()), + ): + main() + + call_args = mock_post.call_args + headers = call_args.kwargs["headers"] + assert headers["Content-Type"] == "application/fhir+json" + assert headers["Accept"] == "application/fhir+json" + assert headers["Ods-From"] == "S44444" + assert "Ssp-TraceID" in headers + # Verify Ssp-TraceID is a valid UUID format + trace_id = headers["Ssp-TraceID"] + assert len(trace_id) == 36 + assert trace_id.count("-") == 4 + + def test_url_construction(self, monkeypatch: pytest.MonkeyPatch) -> None: + """Test that URL is correctly constructed from BASE_URL.""" + base_url = "https://test.example.com" + nhs_number = "9690937278" + monkeypatch.setenv("BASE_URL", base_url) + monkeypatch.setattr(sys, "argv", ["call_gateway.py", nhs_number]) + + mock_response = MagicMock() + mock_response.status_code = 200 + mock_response.json.return_value = {"resourceType": "Bundle"} + + with ( + patch("requests.post", return_value=mock_response) as mock_post, + patch("sys.stdout", StringIO()), + ): + main() + + call_args = mock_post.call_args + assert call_args.args[0] == f"{base_url}/patient/$gpc.getstructuredrecord" + + def test_output_format_for_successful_response( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Test output format for successful response includes status and JSON.""" + nhs_number = "9690937278" + monkeypatch.setenv("BASE_URL", "https://example.com") + monkeypatch.setattr(sys, "argv", ["call_gateway.py", nhs_number]) + + expected_json = {"resourceType": "Bundle", "id": "test-bundle"} + mock_response = MagicMock() + mock_response.status_code = 200 + mock_response.json.return_value = expected_json + + captured_output = StringIO() + + with ( + patch("requests.post", return_value=mock_response), + patch("sys.stdout", captured_output), + ): + main() + + output = captured_output.getvalue() + assert "Status Code: 200" in output + assert "Response:" in output + # Verify JSON is properly formatted + assert json.dumps(expected_json, indent=2) in output diff --git a/gateway-api/stubs/stubs/sds/stub.py b/gateway-api/stubs/stubs/sds/stub.py index fdd04f5c..17078d57 100644 --- a/gateway-api/stubs/stubs/sds/stub.py +++ b/gateway-api/stubs/stubs/sds/stub.py @@ -9,6 +9,7 @@ from collections import defaultdict from typing import TYPE_CHECKING, Any +from fhir.constants import FHIRSystem from gateway_api.get_structured_record import ACCESS_RECORD_STRUCTURED_INTERACTION_ID from stubs.base_stub import GetStub, StubBase @@ -39,10 +40,6 @@ class SdsFhirApiStub(StubBase, GetStub): https://github.com/NHSDigital/spine-directory-service-api """ - ODS_SYSTEM = "https://fhir.nhs.uk/Id/ods-organization-code" - INTERACTION_SYSTEM = "https://fhir.nhs.uk/Id/nhsServiceInteractionId" - PARTYKEY_SYSTEM = "https://fhir.nhs.uk/Id/nhsMhsPartyKey" - ASID_SYSTEM = "https://fhir.nhs.uk/Id/nhsSpineASID" CONNECTION_SYSTEM = ( "https://terminology.hl7.org/CodeSystem/endpoint-connection-type" ) @@ -205,7 +202,7 @@ def get_device_bundle( ) # Parse organization ODS code - org_ods = self._extract_param_value(organization, self.ODS_SYSTEM) + org_ods = self._extract_param_value(organization, FHIRSystem.ODS_CODE) # Parse identifier list (can be string or list) # if isinstance(identifier, str): @@ -217,12 +214,14 @@ def get_device_bundle( party_key: str | None = None for ident in identifier_list: - if self.INTERACTION_SYSTEM in ident: + if FHIRSystem.NHS_SERVICE_INTERACTION_ID in ident: service_interaction_id = self._extract_param_value( - ident, self.INTERACTION_SYSTEM + ident, FHIRSystem.NHS_SERVICE_INTERACTION_ID + ) + elif FHIRSystem.NHS_MHS_PARTY_KEY in ident: + party_key = self._extract_param_value( + ident, FHIRSystem.NHS_MHS_PARTY_KEY ) - elif self.PARTYKEY_SYSTEM in ident: - party_key = self._extract_param_value(ident, self.PARTYKEY_SYSTEM) # Always validate service interaction ID is present if not service_interaction_id: @@ -298,7 +297,7 @@ def get_endpoint_bundle( # Parse organization ODS code (optional) org_ods: str | None = None if organization: - org_ods = self._extract_param_value(organization, self.ODS_SYSTEM) + org_ods = self._extract_param_value(organization, FHIRSystem.ODS_CODE) # Parse identifier list (can be string or list) if isinstance(identifier, str): @@ -308,12 +307,14 @@ def get_endpoint_bundle( party_key: str | None = None for ident in identifier or []: - if self.INTERACTION_SYSTEM in ident: + if FHIRSystem.NHS_SERVICE_INTERACTION_ID in ident: service_interaction_id = self._extract_param_value( - ident, self.INTERACTION_SYSTEM + ident, FHIRSystem.NHS_SERVICE_INTERACTION_ID + ) + elif FHIRSystem.NHS_MHS_PARTY_KEY in ident: + party_key = self._extract_param_value( + ident, FHIRSystem.NHS_MHS_PARTY_KEY ) - elif self.PARTYKEY_SYSTEM in ident: - party_key = self._extract_param_value(ident, self.PARTYKEY_SYSTEM) # Look up endpoints endpoints = self._lookup_endpoints( @@ -497,17 +498,17 @@ def _create_device_resource( "id": device_id, "identifier": [ { - "system": self.ASID_SYSTEM, + "system": FHIRSystem.NHS_SPINE_ASID, "value": asid, }, { - "system": self.PARTYKEY_SYSTEM, + "system": FHIRSystem.NHS_MHS_PARTY_KEY, "value": party_key, }, ], "owner": { "identifier": { - "system": self.ODS_SYSTEM, + "system": FHIRSystem.ODS_CODE, "value": org_ods, }, "display": display, @@ -546,17 +547,17 @@ def _create_endpoint_resource( "address": address, "managingOrganization": { "identifier": { - "system": self.ODS_SYSTEM, + "system": FHIRSystem.ODS_CODE, "value": org_ods, } }, "identifier": [ { - "system": self.ASID_SYSTEM, + "system": FHIRSystem.NHS_SPINE_ASID, "value": asid, }, { - "system": self.PARTYKEY_SYSTEM, + "system": FHIRSystem.NHS_MHS_PARTY_KEY, "value": party_key, }, ], diff --git a/gateway-api/tests/conftest.py b/gateway-api/tests/conftest.py index 00783a77..db4b3a9e 100644 --- a/gateway-api/tests/conftest.py +++ b/gateway-api/tests/conftest.py @@ -7,6 +7,7 @@ import pytest import requests from dotenv import find_dotenv, load_dotenv +from fhir.constants import FHIRSystem from fhir.parameters import Parameters # Load environment variables from .env file in the workspace root @@ -80,7 +81,7 @@ def simple_request_payload() -> Parameters: { "name": "patientNHSNumber", "valueIdentifier": { - "system": "https://fhir.nhs.uk/Id/nhs-number", + "system": FHIRSystem.NHS_NUMBER, "value": "9999999999", }, }, diff --git a/gateway-api/tests/contract/test_consumer_contract.py b/gateway-api/tests/contract/test_consumer_contract.py index 2b7d7a30..1dfecad2 100644 --- a/gateway-api/tests/contract/test_consumer_contract.py +++ b/gateway-api/tests/contract/test_consumer_contract.py @@ -7,6 +7,7 @@ import json import requests +from fhir.constants import FHIRSystem from pact import Pact @@ -37,7 +38,7 @@ def test_get_structured_record(self) -> None: }, "identifier": [ { - "system": "https://fhir.nhs.uk/Id/nhs-number", + "system": FHIRSystem.NHS_NUMBER, "value": "9999999999", } ], @@ -86,7 +87,7 @@ def test_get_structured_record(self) -> None: { "name": "patientNHSNumber", "valueIdentifier": { - "system": "https://fhir.nhs.uk/Id/nhs-number", + "system": FHIRSystem.NHS_NUMBER, "value": "9999999999", }, }, @@ -110,7 +111,7 @@ def test_get_structured_record(self) -> None: { "name": "patientNHSNumber", "valueIdentifier": { - "system": "https://fhir.nhs.uk/Id/nhs-number", + "system": FHIRSystem.NHS_NUMBER, "value": "9999999999", }, }, diff --git a/gateway-api/tests/integration/test_sds_search.py b/gateway-api/tests/integration/test_sds_search.py index b241f432..4999e3d2 100644 --- a/gateway-api/tests/integration/test_sds_search.py +++ b/gateway-api/tests/integration/test_sds_search.py @@ -5,6 +5,8 @@ import json from typing import TYPE_CHECKING +from fhir.constants import FHIRSystem + if TYPE_CHECKING: from tests.conftest import Client @@ -23,7 +25,7 @@ def test_get_device_by_ods_code_returns_valid_asid(self, client: Client) -> None { "name": "patientNHSNumber", "valueIdentifier": { - "system": "https://fhir.nhs.uk/Id/nhs-number", + "system": FHIRSystem.NHS_NUMBER, "value": "9999999999", # Alice Jones with A12345 provider }, }, @@ -50,7 +52,7 @@ def test_consumer_organization_lookup(self, client: Client) -> None: { "name": "patientNHSNumber", "valueIdentifier": { - "system": "https://fhir.nhs.uk/Id/nhs-number", + "system": FHIRSystem.NHS_NUMBER, "value": "9999999999", # Alice Jones with A12345 provider }, }, @@ -82,7 +84,7 @@ def test_result_contains_both_asid_and_endpoint_when_available( { "name": "patientNHSNumber", "valueIdentifier": { - "system": "https://fhir.nhs.uk/Id/nhs-number", + "system": FHIRSystem.NHS_NUMBER, "value": "9999999999", # Alice Jones with A12345 provider }, }, From d34747c788b3ec7839e7213f8bbe6eb763b528bd Mon Sep 17 00:00:00 2001 From: Ben Taylor <59649826+Vox-Ben@users.noreply.github.com> Date: Thu, 12 Mar 2026 17:00:37 +0000 Subject: [PATCH 18/18] Remove now-duplicate scripts dir --- gateway-api/scripts/call_gateway.py | 72 -------- gateway-api/scripts/test_call_gateway.py | 202 ----------------------- 2 files changed, 274 deletions(-) delete mode 100644 gateway-api/scripts/call_gateway.py delete mode 100644 gateway-api/scripts/test_call_gateway.py diff --git a/gateway-api/scripts/call_gateway.py b/gateway-api/scripts/call_gateway.py deleted file mode 100644 index e75f1f52..00000000 --- a/gateway-api/scripts/call_gateway.py +++ /dev/null @@ -1,72 +0,0 @@ -#!/usr/bin/env python3 - -import argparse -import json -import os -import sys -from uuid import uuid4 - -import requests -from fhir.constants import FHIRSystem - - -def main() -> None: - # Parse command-line arguments - parser = argparse.ArgumentParser( - description="POST request to GPC getstructuredrecord endpoint" - ) - parser.add_argument( - "nhs_number", help="NHS number to search for (e.g., 9690937278)" - ) - args = parser.parse_args() - - # Check if BASE_URL is set - base_url = os.environ.get("BASE_URL") - if not base_url: - print("Error: BASE_URL environment variable is not set") - sys.exit(1) - - # Endpoint URL - url = f"{base_url}/patient/$gpc.getstructuredrecord" - - # Request headers - headers = { - "Content-Type": "application/fhir+json", - "Accept": "application/fhir+json", - "Ssp-TraceID": str(uuid4()), - "Ods-From": "S44444", - } - - # Request body - payload = { - "resourceType": "Parameters", - "parameter": [ - { - "name": "patientNHSNumber", - "valueIdentifier": { - "system": FHIRSystem.NHS_NUMBER, - "value": args.nhs_number, - }, - } - ], - } - - # Make the POST request - try: - response = requests.post(url, headers=headers, json=payload, timeout=10) - response.raise_for_status() - - print(f"Status Code: {response.status_code}") - print(f"Response:\n{json.dumps(response.json(), indent=2)}") - - except requests.exceptions.RequestException as e: - errtext = f"Error: {e}\n" - if e.response is not None: - errtext += f"Status Code: {e.response.status_code}\n" - errtext += f"Response Body: {e.response.text}\n" - print(errtext) - sys.exit(1) - - -if __name__ == "__main__": - main() diff --git a/gateway-api/scripts/test_call_gateway.py b/gateway-api/scripts/test_call_gateway.py deleted file mode 100644 index d7b7670d..00000000 --- a/gateway-api/scripts/test_call_gateway.py +++ /dev/null @@ -1,202 +0,0 @@ -"""Unit tests for call_gateway.py script.""" - -import json -import sys -from io import StringIO -from unittest.mock import MagicMock, patch - -import pytest -import requests -from call_gateway import main -from fhir.constants import FHIRSystem - - -class TestCallGateway: - """Test suite for the call_gateway script.""" - - def test_successful_request(self, monkeypatch: pytest.MonkeyPatch) -> None: - """Test successful POST request with valid NHS number.""" - nhs_number = "9690937278" - base_url = "https://example.com" - monkeypatch.setenv("BASE_URL", base_url) - monkeypatch.setattr(sys, "argv", ["call_gateway.py", nhs_number]) - - mock_response = MagicMock() - mock_response.status_code = 200 - mock_response.json.return_value = {"resourceType": "Bundle"} - - captured_output = StringIO() - - with ( - patch("requests.post", return_value=mock_response) as mock_post, - patch("sys.stdout", captured_output), - ): - main() - - mock_post.assert_called_once() - call_args = mock_post.call_args - assert call_args.args[0] == f"{base_url}/patient/$gpc.getstructuredrecord" - assert call_args.kwargs["headers"]["Content-Type"] == "application/fhir+json" - assert call_args.kwargs["headers"]["Accept"] == "application/fhir+json" - assert call_args.kwargs["headers"]["Ods-From"] == "S44444" - assert "Ssp-TraceID" in call_args.kwargs["headers"] - assert call_args.kwargs["json"]["resourceType"] == "Parameters" - assert ( - call_args.kwargs["json"]["parameter"][0]["valueIdentifier"]["value"] - == nhs_number - ) - assert call_args.kwargs["timeout"] == 10 - - output = captured_output.getvalue() - assert "Status Code: 200" in output - assert "Response:" in output - - def test_missing_base_url_environment_variable( - self, monkeypatch: pytest.MonkeyPatch - ) -> None: - """Test error handling when BASE_URL environment variable is not set.""" - monkeypatch.delenv("BASE_URL", raising=False) - monkeypatch.setattr(sys, "argv", ["call_gateway.py", "9690937278"]) - - captured_output = StringIO() - with ( - patch("sys.stdout", captured_output), - pytest.raises(SystemExit) as exc_info, - ): - main() - - assert exc_info.value.code == 1 - output = captured_output.getvalue() - assert "Error: BASE_URL environment variable is not set" in output - - def test_http_error_with_response_body( - self, monkeypatch: pytest.MonkeyPatch - ) -> None: - """Test handling of HTTP error with response body.""" - nhs_number = "9690937278" - monkeypatch.setenv("BASE_URL", "https://example.com") - monkeypatch.setattr(sys, "argv", ["call_gateway.py", nhs_number]) - - mock_response = MagicMock() - mock_response.status_code = 400 - mock_response.text = "Bad Request" - - http_error = requests.exceptions.HTTPError("HTTP Error") - http_error.response = mock_response - - captured_output = StringIO() - - with patch("requests.post") as mock_post: - mock_post.return_value.raise_for_status.side_effect = http_error - with ( - patch("sys.stdout", captured_output), - pytest.raises(SystemExit) as exc_info, - ): - main() - - assert exc_info.value.code == 1 - output = captured_output.getvalue() - assert "Error:" in output - assert "Status Code: 400" in output - assert "Response Body: Bad Request" in output - - def test_request_payload_structure(self, monkeypatch: pytest.MonkeyPatch) -> None: - """Test that request payload has correct FHIR structure.""" - nhs_number = "9690937278" - monkeypatch.setenv("BASE_URL", "https://example.com") - monkeypatch.setattr(sys, "argv", ["call_gateway.py", nhs_number]) - - mock_response = MagicMock() - mock_response.status_code = 200 - mock_response.json.return_value = {"resourceType": "Bundle"} - - with ( - patch("requests.post", return_value=mock_response) as mock_post, - patch("sys.stdout", StringIO()), - ): - main() - - call_args = mock_post.call_args - payload = call_args.kwargs["json"] - assert payload["resourceType"] == "Parameters" - assert len(payload["parameter"]) == 1 - assert payload["parameter"][0]["name"] == "patientNHSNumber" - assert ( - payload["parameter"][0]["valueIdentifier"]["system"] - == FHIRSystem.NHS_NUMBER - ) - assert payload["parameter"][0]["valueIdentifier"]["value"] == nhs_number - - def test_request_headers(self, monkeypatch: pytest.MonkeyPatch) -> None: - """Test that request headers are correctly set.""" - nhs_number = "9690937278" - monkeypatch.setenv("BASE_URL", "https://example.com") - monkeypatch.setattr(sys, "argv", ["call_gateway.py", nhs_number]) - - mock_response = MagicMock() - mock_response.status_code = 200 - mock_response.json.return_value = {"resourceType": "Bundle"} - - with ( - patch("requests.post", return_value=mock_response) as mock_post, - patch("sys.stdout", StringIO()), - ): - main() - - call_args = mock_post.call_args - headers = call_args.kwargs["headers"] - assert headers["Content-Type"] == "application/fhir+json" - assert headers["Accept"] == "application/fhir+json" - assert headers["Ods-From"] == "S44444" - assert "Ssp-TraceID" in headers - # Verify Ssp-TraceID is a valid UUID format - trace_id = headers["Ssp-TraceID"] - assert len(trace_id) == 36 - assert trace_id.count("-") == 4 - - def test_url_construction(self, monkeypatch: pytest.MonkeyPatch) -> None: - """Test that URL is correctly constructed from BASE_URL.""" - base_url = "https://test.example.com" - nhs_number = "9690937278" - monkeypatch.setenv("BASE_URL", base_url) - monkeypatch.setattr(sys, "argv", ["call_gateway.py", nhs_number]) - - mock_response = MagicMock() - mock_response.status_code = 200 - mock_response.json.return_value = {"resourceType": "Bundle"} - - with ( - patch("requests.post", return_value=mock_response) as mock_post, - patch("sys.stdout", StringIO()), - ): - main() - - call_args = mock_post.call_args - assert call_args.args[0] == f"{base_url}/patient/$gpc.getstructuredrecord" - - def test_output_format_for_successful_response( - self, monkeypatch: pytest.MonkeyPatch - ) -> None: - """Test output format for successful response includes status and JSON.""" - nhs_number = "9690937278" - monkeypatch.setenv("BASE_URL", "https://example.com") - monkeypatch.setattr(sys, "argv", ["call_gateway.py", nhs_number]) - - expected_json = {"resourceType": "Bundle", "id": "test-bundle"} - mock_response = MagicMock() - mock_response.status_code = 200 - mock_response.json.return_value = expected_json - - captured_output = StringIO() - - with ( - patch("requests.post", return_value=mock_response), - patch("sys.stdout", captured_output), - ): - main() - - output = captured_output.getvalue() - assert "Status Code: 200" in output - assert "Response:" in output - # Verify JSON is properly formatted - assert json.dumps(expected_json, indent=2) in output