From 63b4f84e4959d41cddd8d69e173997d925761812 Mon Sep 17 00:00:00 2001 From: DWolfsNHS <229101201+DWolfsNHS@users.noreply.github.com> Date: Wed, 22 Apr 2026 15:33:27 +0000 Subject: [PATCH 1/7] [GPCAPIM-395]: Initial implementation of PDS INT integration in runtime --- .github/instructions/copilot-instructions.md | 10 + .../environments/local_PDS_INT.yml | 8 + .../src/gateway_api/apim_app_auth/__init__.py | 11 + .../src/gateway_api/apim_app_auth/apim.py | 147 +++++++++ .../src/gateway_api/apim_app_auth/config.py | 78 +++++ .../gateway_api/apim_app_auth/environment.py | 107 +++++++ .../src/gateway_api/apim_app_auth/http.py | 109 +++++++ .../apim_app_auth/request_context.py | 29 ++ .../gateway_api/apim_app_auth/test_apim.py | 280 ++++++++++++++++++ .../gateway_api/apim_app_auth/test_config.py | 217 ++++++++++++++ .../apim_app_auth/test_environment.py | 113 +++++++ .../gateway_api/apim_app_auth/test_http.py | 218 ++++++++++++++ .../apim_app_auth/test_request_context.py | 86 ++++++ gateway-api/src/gateway_api/app.py | 2 + gateway-api/src/gateway_api/controller.py | 3 +- .../resources/dev-certificates/.gitignore | 2 - .../config/vocabularies/words/accept.txt | 1 + 17 files changed, 1418 insertions(+), 3 deletions(-) create mode 100644 bruno/gateway-api/collections/Steel_Thread/environments/local_PDS_INT.yml create mode 100644 gateway-api/src/gateway_api/apim_app_auth/__init__.py create mode 100644 gateway-api/src/gateway_api/apim_app_auth/apim.py create mode 100644 gateway-api/src/gateway_api/apim_app_auth/config.py create mode 100644 gateway-api/src/gateway_api/apim_app_auth/environment.py create mode 100644 gateway-api/src/gateway_api/apim_app_auth/http.py create mode 100644 gateway-api/src/gateway_api/apim_app_auth/request_context.py create mode 100644 gateway-api/src/gateway_api/apim_app_auth/test_apim.py create mode 100644 gateway-api/src/gateway_api/apim_app_auth/test_config.py create mode 100644 gateway-api/src/gateway_api/apim_app_auth/test_environment.py create mode 100644 gateway-api/src/gateway_api/apim_app_auth/test_http.py create mode 100644 gateway-api/src/gateway_api/apim_app_auth/test_request_context.py delete mode 100644 infrastructure/images/build-container/resources/dev-certificates/.gitignore diff --git a/.github/instructions/copilot-instructions.md b/.github/instructions/copilot-instructions.md index 6999f07d..0862a4f5 100644 --- a/.github/instructions/copilot-instructions.md +++ b/.github/instructions/copilot-instructions.md @@ -50,6 +50,16 @@ When reviewing code, ensure you compare the changes made to files to all README. Prepend `[AI-generated]` to the commit message when committing changes made by an AI agent. +## Branches + +When creating a branch for a Jira ticket, use: + +`feature/_` + +Example: `feature/GPCAPIM-395_Local_PDS_INT_Integration` + ## Security This repository is public. Do not commit any secrets, tokens or credentials. + +Do not bypass file access restrictions in any way (for example, by using terminal commands to read files that Copilot tooling cannot access, such as `.env` or other local secret files). diff --git a/bruno/gateway-api/collections/Steel_Thread/environments/local_PDS_INT.yml b/bruno/gateway-api/collections/Steel_Thread/environments/local_PDS_INT.yml new file mode 100644 index 00000000..6c0f6b00 --- /dev/null +++ b/bruno/gateway-api/collections/Steel_Thread/environments/local_PDS_INT.yml @@ -0,0 +1,8 @@ +name: local - PDS_INT +variables: + - name: base_url + value: http://localhost:5000 + - name: nhs_number + value: "9692140466" + - name: from_ods + value: S55555 diff --git a/gateway-api/src/gateway_api/apim_app_auth/__init__.py b/gateway-api/src/gateway_api/apim_app_auth/__init__.py new file mode 100644 index 00000000..028a7a2d --- /dev/null +++ b/gateway-api/src/gateway_api/apim_app_auth/__init__.py @@ -0,0 +1,11 @@ +"""APIM App Restricted auth tooling.""" + +from gateway_api.apim_app_auth.apim import ( + ApimAuthenticationException, + ApimAuthenticator, +) + +__all__ = [ + "ApimAuthenticationException", + "ApimAuthenticator", +] diff --git a/gateway-api/src/gateway_api/apim_app_auth/apim.py b/gateway-api/src/gateway_api/apim_app_auth/apim.py new file mode 100644 index 00000000..e91bbb99 --- /dev/null +++ b/gateway-api/src/gateway_api/apim_app_auth/apim.py @@ -0,0 +1,147 @@ +import functools +import logging +import uuid +from collections.abc import Callable +from datetime import datetime, timedelta, timezone +from typing import Any, TypedDict + +import jwt +import requests + +from gateway_api.apim_app_auth.http import RequestMethod, SessionManager + +_logger = logging.getLogger(__name__) + + +class ApimAuthenticationException(Exception): + pass + + +class ApimAuthenticator: + class __AccessToken(TypedDict): + value: str + expiry: datetime + + def __init__( + self, + private_key: str, + key_id: str, + api_key: str, + token_validity_threshold: timedelta, + token_endpoint: str, + session_manager: SessionManager, + ): + self._private_key = private_key + self._key_id = key_id + self._api_key = api_key + self._token_validity_threshold = token_validity_threshold + self._token_endpoint = token_endpoint + self._session_manager = session_manager + + self._access_token: ApimAuthenticator.__AccessToken | None = None + + def auth[**P, S](self, func: RequestMethod[P, S]) -> Callable[P, S]: + """ + Decorate a given function with APIM authentication. This authentication will be + provided via a `requests.Session` object. + """ + + @functools.wraps(func) + def wrapper(*args: Any, **kwargs: Any) -> Any: + @self._session_manager.with_session + def with_session( + session: requests.Session, access_token: ApimAuthenticator.__AccessToken + ) -> S: + session.headers.update( + {"Authorization": f"Bearer {access_token['value']}"} + ) + return func(session, *args, **kwargs) + + # If there isn't an access token yet, or the token will expire within the + # token validity threshold, reauthenticate. + if ( + self._access_token is None + or self._access_token["expiry"] - datetime.now(tz=timezone.utc) + < self._token_validity_threshold + ): + _logger.debug("Authenticating with APIM...") + self._access_token = self._authenticate() + + return with_session(self._access_token) + + return wrapper + + def _create_client_assertion(self) -> str: + _logger.debug("Creating client assertion JWT for APIM authentication") + claims = { + "sub": self._api_key, + "iss": self._api_key, + "jti": str(uuid.uuid4()), + "aud": self._token_endpoint, + "exp": int( + (datetime.now(tz=timezone.utc) + timedelta(seconds=30)).timestamp() + ), + } + _logger.debug( + "Created client claims. jti: %s, exp: %s, aud: %s", + claims["jti"], + claims["exp"], + claims["aud"], + ) + + client_assertion = jwt.encode( + claims, + self._private_key, + algorithm="RS512", + headers={"kid": self._key_id}, + ) + + _logger.debug("Created client assertion. kid: %s", self._key_id) + + return client_assertion + + def _authenticate(self) -> __AccessToken: + @self._session_manager.with_session + def with_session(session: requests.Session) -> ApimAuthenticator.__AccessToken: + client_assertion = self._create_client_assertion() + + _logger.debug("Sending token request with created session.") + + response = session.post( + self._token_endpoint, + data={ + "grant_type": "client_credentials", + "client_assertion_type": "urn:ietf:params:oauth" + ":client-assertion-type:jwt-bearer", + "client_assertion": client_assertion, + }, + ) + + _logger.debug( + "Response received from APIM token endpoint. Status code: %s", + response.status_code, + ) + + if response.status_code != 200: + raise ApimAuthenticationException( + f"Failed to authenticate with APIM. " + f"Status code: {response.status_code}" + f", Response: {response.text}" + ) + + response_data = response.json() + _logger.debug( + "APIM authentication successful. Expiry: %s", + response_data["expires_in"], + ) + + return { + "value": response_data["access_token"], + "expiry": datetime.now(tz=timezone.utc) + + timedelta(seconds=int(response_data["expires_in"])), + } + + _logger.debug( + "Sending authentication request to APIM: %s", self._token_endpoint + ) + return with_session() diff --git a/gateway-api/src/gateway_api/apim_app_auth/config.py b/gateway-api/src/gateway_api/apim_app_auth/config.py new file mode 100644 index 00000000..97b41fb4 --- /dev/null +++ b/gateway-api/src/gateway_api/apim_app_auth/config.py @@ -0,0 +1,78 @@ +import os +import re +from collections.abc import Callable +from dataclasses import dataclass +from datetime import timedelta +from enum import StrEnum +from typing import Any, cast + + +class ConfigError(Exception): + pass + + +class DurationUnit(StrEnum): + SECONDS = "s" + MINUTES = "m" + + +@dataclass(frozen=True) +class Duration: + unit: DurationUnit + value: int + + @property + def timedelta(self) -> timedelta: + match self.unit: + case DurationUnit.SECONDS: + return timedelta(seconds=self.value) + case DurationUnit.MINUTES: + return timedelta(minutes=self.value) + + +_SUPPORTED_PRIMITIVES: dict[type[Any], Callable[[str], Any]] = { + str: str, + int: int, +} + + +def get_optional_environment_variable[T](name: str, _type: type[T]) -> T | None: + value = os.getenv(name) + + match _type: + case _ if _type is Duration: + if value is None: + return None + + parsed = re.fullmatch(r"(?P\d+)(?P[sm])", value) + if parsed is None: + raise ConfigError(f"Invalid duration value: {value!r}") + + raw_value = parsed.group("value") + raw_unit = parsed.group("unit") + + return cast( + "T", + Duration( + unit=DurationUnit(raw_unit), + value=int(raw_value), + ), + ) + + case _ if _type in _SUPPORTED_PRIMITIVES: + if value is None: + return None + + return cast("T", _SUPPORTED_PRIMITIVES[_type](value)) + + case _: + raise ValueError( + f"Required type {_type} is not supported for config values" + ) + + +def get_environment_variable[T](name: str, _type: type[T]) -> T: + value = get_optional_environment_variable(name=name, _type=_type) + if value is None: + raise ConfigError(f"Environment variable {name!r} is not set") + return value diff --git a/gateway-api/src/gateway_api/apim_app_auth/environment.py b/gateway-api/src/gateway_api/apim_app_auth/environment.py new file mode 100644 index 00000000..3d4e96b9 --- /dev/null +++ b/gateway-api/src/gateway_api/apim_app_auth/environment.py @@ -0,0 +1,107 @@ +# TODO: 395 update this to align with the new environment +# variable management approach once that is implemented + +from typing import TypedDict + +from gateway_api.apim_app_auth.apim import ApimAuthenticator +from gateway_api.apim_app_auth.config import ( + Duration, + get_environment_variable, +) +from gateway_api.apim_app_auth.http import SessionManager + +__all__ = [ + "apim_authenticator", + "values", + "session_manager", +] + + +class Environment(TypedDict): + client_timeout: Duration + apim_token_url: str + apim_private_key_name: str + apim_api_key_name: str + apim_token_expiry_threshold: Duration + apim_key_id: str + pdm_url: str + mns_url: str + + +_environment: Environment | None = None +_session_manager: SessionManager | None = None +_apim_authenticator: ApimAuthenticator | None = None + + +def values() -> Environment: + global _environment + if _environment is None: + _environment = Environment( + client_timeout=get_environment_variable( + "CLIENT_TIMEOUT", + Duration, + ), + apim_token_url=get_environment_variable( + "APIM_TOKEN_URL", + str, + ), + apim_private_key_name=get_environment_variable( + "APIM_PRIVATE_KEY_NAME", + str, + ), + apim_api_key_name=get_environment_variable( + "APIM_API_KEY_NAME", + str, + ), + apim_token_expiry_threshold=get_environment_variable( + "APIM_TOKEN_EXPIRY_THRESHOLD", + Duration, + ), + apim_key_id=get_environment_variable( + "APIM_KEY_ID", + str, + ), + pdm_url=get_environment_variable( + "PDM_BUNDLE_URL", + str, + ), + mns_url=get_environment_variable( + "MNS_EVENT_URL", + str, + ), + ) + + return _environment + + +def session_manager() -> SessionManager: + global _session_manager + + if _session_manager is None: + client_certificate = None + + _session_manager = SessionManager( + client_timeout=get_environment_variable( + "CLIENT_TIMEOUT", Duration + ).timedelta, + client_certificate=client_certificate, + ) + + return _session_manager + + +def apim_authenticator() -> ApimAuthenticator: + global _apim_authenticator + + if _apim_authenticator is None: + env = values() + _apim_authenticator = ApimAuthenticator( + private_key="", # TODO: 395 get private key + key_id=env["apim_key_id"], + api_key="", # TODO: 395 get api key + token_endpoint=env["apim_token_url"], + token_validity_threshold=env["apim_token_expiry_threshold"].timedelta, + session_manager=session_manager(), + ) + + return _apim_authenticator diff --git a/gateway-api/src/gateway_api/apim_app_auth/http.py b/gateway-api/src/gateway_api/apim_app_auth/http.py new file mode 100644 index 00000000..16d0c60a --- /dev/null +++ b/gateway-api/src/gateway_api/apim_app_auth/http.py @@ -0,0 +1,109 @@ +import functools +import logging +import tempfile +from collections.abc import Callable +from contextlib import ExitStack +from datetime import timedelta +from typing import Any, Concatenate, TypedDict + +import requests +from requests.adapters import HTTPAdapter + +from gateway_api.apim_app_auth.request_context import get_correlation_id + +_logger = logging.getLogger(__name__) + +# Type alias describing the expected signature for a request making a HTTP request. +# Any function that takes a `requests.Session` as its first argument, followed by any +# number of additional arguments, and returns any type of value. +type RequestMethod[**P, S] = Callable[Concatenate[requests.Session, P], S] + + +class ClientCertificate(TypedDict): + certificate: str + key: str + + +class SessionManager: + class _Adapter(HTTPAdapter): + """ + HTTPAdapter to apply default configuration to apply to all created + `request.Session` objects. + """ + + def __init__(self, timeout: float): + self._timeout = timeout + super().__init__() + + def send( + self, + request: requests.PreparedRequest, + *args: Any, + **kwargs: Any, + ) -> requests.Response: + kwargs["timeout"] = self._timeout + if "X-Correlation-ID" not in request.headers: + request.headers["X-Correlation-ID"] = get_correlation_id().short_id + + _logger.info( + "Sending HTTP request. method=%s url=%s headers=%s", + request.method, + request.url, + # Omit Authorization header from logging + { + k: v + for k, v in request.headers.items() + if k.lower() != "authorization" + }, + ) + response = super().send(request, *args, **kwargs) + _logger.info( + "Received HTTP response. status_code=%s response_headers=%s", + response.status_code, + response.headers, + ) + + return response + + def __init__( + self, + client_timeout: timedelta, + client_certificate: ClientCertificate | None = None, + ): + self._client_adapter = self._Adapter(timeout=client_timeout.total_seconds()) + self._client_certificate = client_certificate + + def with_session[**P, S](self, func: RequestMethod[P, S]) -> Callable[P, S]: + @functools.wraps(func) + def wrapper(*args: Any, **kwargs: Any) -> Any: + with ExitStack() as stack: + session = requests.Session() + stack.enter_context(session) + + session.mount("https://", self._client_adapter) + + if self._client_certificate is not None: + # File added to Exit stack and will be automatically cleaned up with + # the stack. + cert_file = tempfile.NamedTemporaryFile( # noqa: SIM115 + delete=True + ) + stack.enter_context(cert_file) + + # File added to Exit stack and will be automatically cleaned up with + # the stack. + key_file = tempfile.NamedTemporaryFile( # noqa: SIM115 + delete=True + ) + stack.enter_context(key_file) + + cert_file.write(self._client_certificate["certificate"].encode()) + cert_file.flush() + key_file.write(self._client_certificate["key"].encode()) + key_file.flush() + + session.cert = (cert_file.name, key_file.name) + + return func(session, *args, **kwargs) + + return wrapper diff --git a/gateway-api/src/gateway_api/apim_app_auth/request_context.py b/gateway-api/src/gateway_api/apim_app_auth/request_context.py new file mode 100644 index 00000000..8f2ef586 --- /dev/null +++ b/gateway-api/src/gateway_api/apim_app_auth/request_context.py @@ -0,0 +1,29 @@ +from contextvars import ContextVar +from typing import NamedTuple + + +class CorrelationID(NamedTuple): + full_id: str + short_id: str + + +_correlation_id: ContextVar[CorrelationID | None] = ContextVar( + "correlation_id", default=None +) + + +def set_correlation_id(full_id: str, short_id: str) -> None: + """Set the correlation ID for the current request context.""" + _correlation_id.set(CorrelationID(full_id=full_id, short_id=short_id)) + + +def reset_correlation_id() -> None: + """Reset the correlation ID to the default empty string.""" + _correlation_id.set(None) + + +def get_correlation_id() -> CorrelationID: + """Get the correlation ID for the current request context.""" + if (correlation_id := _correlation_id.get()) is None: + raise ValueError("Correlation ID is not set in the current context.") + return correlation_id diff --git a/gateway-api/src/gateway_api/apim_app_auth/test_apim.py b/gateway-api/src/gateway_api/apim_app_auth/test_apim.py new file mode 100644 index 00000000..8f0c69b3 --- /dev/null +++ b/gateway-api/src/gateway_api/apim_app_auth/test_apim.py @@ -0,0 +1,280 @@ +from collections.abc import Callable, Generator +from datetime import datetime, timedelta, timezone +from typing import Any +from unittest.mock import MagicMock, Mock, patch + +import pytest +import requests +from jwt import InvalidKeyError + +from gateway_api.apim_app_auth.apim import ( + ApimAuthenticationException, + ApimAuthenticator, +) +from gateway_api.apim_app_auth.request_context import ( + reset_correlation_id, + set_correlation_id, +) + + +class TestApimAuthenticator: + @pytest.fixture(autouse=True) + def set_correlation_id_for_logger(self) -> Generator[None, None, None]: + set_correlation_id( + full_id="test_id_long", + short_id="test_id", + ) + yield + reset_correlation_id() + + def setup_method(self) -> None: + self.mock_session = Mock() + + def mock_with_session(self, func: Callable[..., Any]) -> Callable[..., Any]: + + def wrapper(*args: Any, **kwargs: Any) -> Any: + return func(self.mock_session, *args, **kwargs) + + return wrapper + + @patch("pathology_api.http.SessionManager") + @patch("pathology_api.apim.jwt.encode") + def test_auth(self, mock_jwt: MagicMock, mock_session_manager: MagicMock) -> None: + mock_session_manager.with_session = self.mock_with_session + + expected_client_assertion = "client_assertion" + mock_jwt.return_value = expected_client_assertion + + expected_access_token = "access_token" # noqa S105 - Dummy value + expected_expires_in = timedelta(seconds=5) + + self.mock_session.post.return_value.json.return_value = { + "access_token": expected_access_token, + "expires_in": expected_expires_in.total_seconds(), + } + self.mock_session.post.return_value.status_code = 200 + + expected_api_key = "api_key" + expected_token_endpoint = "token_endpoint" # noqa S106 - Dummy value + expected_key_id = "key_id" + apim_authenticator = ApimAuthenticator( + private_key="private_key", + key_id=expected_key_id, + api_key=expected_api_key, + token_validity_threshold=timedelta(minutes=5), + token_endpoint=expected_token_endpoint, + session_manager=mock_session_manager, + ) + + @apim_authenticator.auth + def method(_: requests.Session) -> None: + self.mock_session.headers.update.assert_called_once_with( + {"Authorization": f"Bearer {expected_access_token}"} + ) + + mock_jwt.assert_called_once() + args, kwargs = mock_jwt.call_args + + provided_claims = args[0] + assert provided_claims["sub"] == expected_api_key + assert provided_claims["iss"] == expected_api_key + assert provided_claims["aud"] == expected_token_endpoint + assert provided_claims["jti"] is not None + assert provided_claims["exp"] < int( + (datetime.now(tz=timezone.utc) + timedelta(seconds=31)).timestamp() + ) + + assert kwargs == {"algorithm": "RS512", "headers": {"kid": expected_key_id}} + + # SLF001: Private access to support testing + stored_access_token = apim_authenticator._access_token # noqa SLF001 + assert stored_access_token is not None + assert stored_access_token["value"] == expected_access_token + assert stored_access_token["expiry"] < ( + datetime.now(tz=timezone.utc) + expected_expires_in + ) + + method() + + @patch("pathology_api.http.SessionManager") + @patch("pathology_api.apim.jwt.encode") + def test_auth_existing_valid_token( + self, mock_jwt: MagicMock, mock_session_manager: MagicMock + ) -> None: + mock_session_manager.with_session = self.mock_with_session + + apim_authenticator = ApimAuthenticator( + private_key="private_key", + key_id="key_id", + api_key="api_key", + token_validity_threshold=timedelta(minutes=5), + token_endpoint="token_endpoint", # noqa S106 - Dummy value + session_manager=mock_session_manager, + ) + + expected_access_token = "access_token" # noqa S105 - Dummy value + apim_authenticator._access_token = { # noqa SLF001 - Private access to support testing + "value": expected_access_token, + "expiry": datetime.now(tz=timezone.utc) + timedelta(minutes=10), + } + + @apim_authenticator.auth + def method(_: requests.Session) -> None: + self.mock_session.headers.update.assert_called_once_with( + {"Authorization": f"Bearer {expected_access_token}"} + ) + + mock_jwt.assert_not_called() + + method() + + @patch("pathology_api.http.SessionManager") + @patch("pathology_api.apim.jwt.encode") + def test_auth_existing_invalid_token( + self, mock_jwt: MagicMock, mock_session_manager: MagicMock + ) -> None: + mock_session_manager.with_session = self.mock_with_session + + expected_client_assertion = "client_assertion" + mock_jwt.return_value = expected_client_assertion + + expected_access_token = "access_token" # noqa S105 - Dummy value + expected_expires_in = timedelta(seconds=5) + + self.mock_session.post.return_value.json.return_value = { + "access_token": expected_access_token, + "expires_in": expected_expires_in.total_seconds(), + } + self.mock_session.post.return_value.status_code = 200 + + expected_api_key = "api_key" + expected_token_endpoint = "token_endpoint" # noqa S106 - Dummy value + expected_key_id = "key_id" + apim_authenticator = ApimAuthenticator( + private_key="private_key", + key_id=expected_key_id, + api_key=expected_api_key, + token_validity_threshold=timedelta(minutes=5), + token_endpoint=expected_token_endpoint, + session_manager=mock_session_manager, + ) + + apim_authenticator._access_token = { # noqa SLF001 - Private access to support testing + "value": "old_access_token", + "expiry": datetime.now(tz=timezone.utc) - timedelta(seconds=1), + } + + @apim_authenticator.auth + def method(_: requests.Session) -> None: + self.mock_session.headers.update.assert_called_once_with( + {"Authorization": f"Bearer {expected_access_token}"} + ) + + mock_jwt.assert_called_once() + args, kwargs = mock_jwt.call_args + + provided_claims = args[0] + assert provided_claims["sub"] == expected_api_key + assert provided_claims["iss"] == expected_api_key + assert provided_claims["aud"] == expected_token_endpoint + assert provided_claims["jti"] is not None + assert provided_claims["exp"] < int( + (datetime.now(tz=timezone.utc) + timedelta(seconds=31)).timestamp() + ) + + assert kwargs == {"algorithm": "RS512", "headers": {"kid": expected_key_id}} + + # SLF001: Private access to support testing + stored_access_token = apim_authenticator._access_token # noqa SLF001 + assert stored_access_token is not None + assert stored_access_token["value"] == expected_access_token + assert stored_access_token["expiry"] < ( + datetime.now(tz=timezone.utc) + expected_expires_in + ) + + method() + + @patch("pathology_api.http.SessionManager") + @patch("pathology_api.apim.jwt.encode") + def test_auth_unsuccessful_status_code( + self, mock_jwt: MagicMock, mock_session_manager: MagicMock + ) -> None: + mock_session_manager.with_session = self.mock_with_session + + mock_jwt.return_value = "client_assertion" + + self.mock_session.post.return_value.status_code = 401 + self.mock_session.post.return_value.text = "Unauthorized" + + apim_authenticator = ApimAuthenticator( + private_key="private_key", + key_id="key_id", + api_key="api_key", + token_validity_threshold=timedelta(minutes=5), + token_endpoint="token_endpoint", # noqa S106 - Dummy value + session_manager=mock_session_manager, + ) + + @apim_authenticator.auth + def method(_: requests.Session) -> None: + """Dummy method just to apply the auth decorator""" + pass + + with pytest.raises(ApimAuthenticationException): + method() + + @patch("pathology_api.http.SessionManager") + @patch("pathology_api.apim.jwt.encode") + def test_auth_session_post_raises_exception( + self, mock_jwt: MagicMock, mock_session_manager: MagicMock + ) -> None: + mock_session_manager.with_session = self.mock_with_session + + mock_jwt.return_value = "client_assertion" + + self.mock_session.post.side_effect = requests.RequestException( + "Connection failed" + ) + + apim_authenticator = ApimAuthenticator( + private_key="private_key", + key_id="key_id", + api_key="api_key", + token_validity_threshold=timedelta(minutes=5), + token_endpoint="token_endpoint", # noqa S106 - Dummy value + session_manager=mock_session_manager, + ) + + @apim_authenticator.auth + def method(_: requests.Session) -> None: + """Dummy method just to apply the auth decorator""" + pass + + with pytest.raises(requests.RequestException, match="Connection failed"): + method() + + @patch("pathology_api.http.SessionManager") + @patch("pathology_api.apim.jwt.encode") + def test_auth_jwt_encode_raises_exception( + self, mock_jwt: MagicMock, mock_session_manager: MagicMock + ) -> None: + mock_session_manager.with_session = self.mock_with_session + + mock_jwt.side_effect = InvalidKeyError("JWT encoding failed") + + apim_authenticator = ApimAuthenticator( + private_key="private_key", + key_id="key_id", + api_key="api_key", + token_validity_threshold=timedelta(minutes=5), + token_endpoint="token_endpoint", # noqa S106 - Dummy value + session_manager=mock_session_manager, + ) + + @apim_authenticator.auth + def method(_: requests.Session) -> None: + """Dummy method just to apply the auth decorator""" + pass + + with pytest.raises(InvalidKeyError, match="JWT encoding failed"): + method() diff --git a/gateway-api/src/gateway_api/apim_app_auth/test_config.py b/gateway-api/src/gateway_api/apim_app_auth/test_config.py new file mode 100644 index 00000000..452172ad --- /dev/null +++ b/gateway-api/src/gateway_api/apim_app_auth/test_config.py @@ -0,0 +1,217 @@ +import os +from datetime import timedelta +from typing import Any + +import pytest + +from gateway_api.apim_app_auth.config import ( + ConfigError, + Duration, + DurationUnit, + get_environment_variable, + get_optional_environment_variable, +) + + +class TestGetEnvironmentVariables: + __ENV_VAR_NAME = "TEST_VARIABLE" + + def teardown_method(self) -> None: + os.environ.pop(self.__ENV_VAR_NAME, None) + + @pytest.mark.parametrize( + ("expected_value", "_type"), + [ + pytest.param("test_value", str, id="String variable"), + pytest.param(123, int, id="Integer variable"), + ], + ) + def test_get_environment_variable( + self, expected_value: Any, _type: type[Any] + ) -> None: + os.environ[self.__ENV_VAR_NAME] = str(expected_value) + + value = get_environment_variable(name=self.__ENV_VAR_NAME, _type=_type) + + assert value == expected_value + + @pytest.mark.parametrize( + ("_type"), + [ + pytest.param(str, id="String variable"), + pytest.param(int, id="Integer variable"), + ], + ) + def test_get_environment_variable_no_config_value(self, _type: type[Any]) -> None: + with pytest.raises( + ConfigError, + match=f"Environment variable '{self.__ENV_VAR_NAME}' is not set", + ): + get_environment_variable(name=self.__ENV_VAR_NAME, _type=_type) + + @pytest.mark.parametrize( + ("environment_variable", "expected_result"), + [ + pytest.param( + "5m", + Duration(unit=DurationUnit.MINUTES, value=5), + id="Minutes duration", + ), + pytest.param( + "30s", + Duration(unit=DurationUnit.SECONDS, value=30), + id="Seconds duration", + ), + ], + ) + def test_get_duration_environment_variable( + self, environment_variable: str, expected_result: Duration + ) -> None: + os.environ[self.__ENV_VAR_NAME] = environment_variable + + value = get_environment_variable(name=self.__ENV_VAR_NAME, _type=Duration) + + assert value == expected_result + + @pytest.mark.parametrize( + ("environment_variable", "expected_error"), + [ + pytest.param( + "5x", + "Invalid duration value: '5x'", + id="Unknown unit type", + ), + pytest.param( + "invalids", + "Invalid duration value: 'invalids'", + id="Unknown unit", + ), + pytest.param( + "not a duration", + "Invalid duration value: 'not a duration'", + id="Not a duration format", + ), + pytest.param( + None, + "Environment variable 'TEST_VARIABLE' is not set", + id="No value", + ), + ], + ) + def test_get_duration_environment_variable_invalid( + self, environment_variable: str, expected_error: str + ) -> None: + if environment_variable is not None: + os.environ[self.__ENV_VAR_NAME] = environment_variable + + with pytest.raises(ConfigError, match=expected_error): + get_environment_variable(name=self.__ENV_VAR_NAME, _type=Duration) + + def test_get_environment_variable_unsupported_type(self) -> None: + with pytest.raises( + ValueError, + match=f"Required type {float!r} is not supported for config values", + ): + get_environment_variable(name=self.__ENV_VAR_NAME, _type=float) + + @pytest.mark.parametrize( + ("expected_value", "_type"), + [ + pytest.param("test_value", str, id="String variable"), + pytest.param(123, int, id="Integer variable"), + ], + ) + def test_get_optional_environment_variable( + self, expected_value: Any, _type: type[Any] + ) -> None: + os.environ[self.__ENV_VAR_NAME] = str(expected_value) + + value = get_optional_environment_variable(name=self.__ENV_VAR_NAME, _type=_type) + + assert value == expected_value + + @pytest.mark.parametrize( + ("_type"), + [ + pytest.param(str, id="String variable"), + pytest.param(int, id="Integer variable"), + pytest.param(Duration, id="Duration variable"), + ], + ) + def test_get_optional_environment_variable_no_config_value( + self, _type: type[Any] + ) -> None: + value = get_optional_environment_variable(name=self.__ENV_VAR_NAME, _type=_type) + + assert value is None + + @pytest.mark.parametrize( + ("environment_variable", "expected_result"), + [ + pytest.param( + "5m", + Duration(unit=DurationUnit.MINUTES, value=5), + id="Minutes duration", + ), + pytest.param( + "30s", + Duration(unit=DurationUnit.SECONDS, value=30), + id="Seconds duration", + ), + ], + ) + def test_get_optional_duration_environment_variable( + self, environment_variable: str, expected_result: Duration + ) -> None: + os.environ[self.__ENV_VAR_NAME] = environment_variable + + value = get_optional_environment_variable( + name=self.__ENV_VAR_NAME, _type=Duration + ) + + assert value == expected_result + + @pytest.mark.parametrize( + ("environment_variable", "expected_error"), + [ + pytest.param( + "5x", + "Invalid duration value: '5x'", + id="Unknown unit", + ), + ], + ) + def test_get_optional_duration_environment_variable_invalid( + self, environment_variable: str, expected_error: str + ) -> None: + os.environ[self.__ENV_VAR_NAME] = environment_variable + + with pytest.raises(ConfigError, match=expected_error): + get_optional_environment_variable(name=self.__ENV_VAR_NAME, _type=Duration) + + def test_get_optional_environment_variable_unsupported_type(self) -> None: + with pytest.raises( + ValueError, + match=f"Required type {float!r} is not supported for config values", + ): + get_optional_environment_variable(name=self.__ENV_VAR_NAME, _type=float) + + +class TestDuration: + @pytest.mark.parametrize( + ("duration", "expected_timedelta"), + [ + pytest.param( + Duration(unit=DurationUnit.MINUTES, value=5), + timedelta(minutes=5), + id="Minutes duration", + ), + pytest.param( + Duration(unit=DurationUnit.SECONDS, value=30), + timedelta(seconds=30), + id="Seconds duration", + ), + ], + ) + def test_timedelta(self, duration: Duration, expected_timedelta: timedelta) -> None: + assert duration.timedelta == expected_timedelta diff --git a/gateway-api/src/gateway_api/apim_app_auth/test_environment.py b/gateway-api/src/gateway_api/apim_app_auth/test_environment.py new file mode 100644 index 00000000..b3c3197f --- /dev/null +++ b/gateway-api/src/gateway_api/apim_app_auth/test_environment.py @@ -0,0 +1,113 @@ +import os +from datetime import timedelta +from unittest.mock import MagicMock, call, patch + +from gateway_api.apim_app_auth import environment +from gateway_api.apim_app_auth.config import Duration, DurationUnit +from gateway_api.apim_app_auth.http import SessionManager + + +class TestEnvironment: + def setup_method(self) -> None: + # Clear any set environment variables + os.environ.clear() + + @patch("gateway_api.apim_app_auth.environment.parameters.get_secret") + def test_session_manager_with_mtls(self, get_secret_mock: MagicMock) -> None: + environment._session_manager = ( # noqa SLF001 - access private variable for testing purposes + None # reset session manager to force reinitialisation + ) + + get_secret_mock.side_effect = lambda secret_name: { + "mtls_cert_name": "mtls_cert", + "mtls_key_name": "mtls_key", + }[secret_name] + + os.environ["APIM_MTLS_CERT_NAME"] = "mtls_cert_name" + os.environ["APIM_MTLS_KEY_NAME"] = "mtls_key_name" + os.environ["CLIENT_TIMEOUT"] = "30s" + + certificate_name = "mtls_cert_name" + key_name = "mtls_key_name" + + session_manager = environment.session_manager() + client_certificate = session_manager._client_certificate # noqa SLF001 - access private attribute for testing purposes + + assert client_certificate == { + "certificate": "mtls_cert", + "key": "mtls_key", + } + + get_secret_mock.assert_has_calls([call(certificate_name), call(key_name)]) + + @patch("gateway_api.apim_app_auth.environment.parameters.get_secret") + def test_session_manager(self, get_secret_mock: MagicMock) -> None: + environment._session_manager = ( # noqa SLF001 - access private variable for testing purposes + None # reset session manager to force reinitialisation + ) + + os.environ["CLIENT_TIMEOUT"] = "30s" + + session_manager = environment.session_manager() + assert session_manager._client_certificate is None # noqa SLF001 - access private attribute for testing purposes + + get_secret_mock.assert_not_called() + + def test_values(self) -> None: + os.environ["CLIENT_TIMEOUT"] = "30s" + os.environ["APIM_TOKEN_URL"] = "token_url" # noqa S105 - dummy value + os.environ["APIM_PRIVATE_KEY_NAME"] = "private_key_name" + os.environ["APIM_API_KEY_NAME"] = "api_key_name" + os.environ["APIM_TOKEN_EXPIRY_THRESHOLD"] = "60s" # noqa S105 - dummy value + os.environ["APIM_KEY_ID"] = "key_id" + os.environ["PDM_BUNDLE_URL"] = "pdm_url" + os.environ["MNS_EVENT_URL"] = "mns_url" + + environ = environment.values() + + assert environ["client_timeout"].timedelta == timedelta(seconds=30) + assert environ["apim_token_url"] == "token_url" # noqa S105 - dummy value + assert environ["apim_private_key_name"] == "private_key_name" + assert environ["apim_api_key_name"] == "api_key_name" + assert environ["apim_token_expiry_threshold"].timedelta == timedelta(seconds=60) + assert environ["apim_key_id"] == "key_id" + assert environ["pdm_url"] == "pdm_url" + assert environ["mns_url"] == "mns_url" + + @patch("gateway_api.apim_app_auth.environment.parameters.get_secret") + @patch("gateway_api.apim_app_auth.environment.values") + @patch("gateway_api.apim_app_auth.environment.session_manager") + def test_apim_authenticator( + self, + session_manager_mock: MagicMock, + values_mock: MagicMock, + get_secret_mock: MagicMock, + ) -> None: + expected_session_manager = SessionManager(client_timeout=timedelta(seconds=30)) + session_manager_mock.return_value = expected_session_manager + + environ: environment.Environment = { + "apim_private_key_name": "private_key_name", + "apim_api_key_name": "api_key_name", + "apim_key_id": "key_id", + "apim_token_expiry_threshold": Duration(DurationUnit.SECONDS, 60), + "apim_token_url": "token_url", + "client_timeout": Duration(DurationUnit.SECONDS, 30), + "pdm_url": "pdm_url", + "mns_url": "mns_url", + } + + values_mock.return_value = environ + + get_secret_mock.side_effect = lambda secret_name: { + "private_key_name": "private_key", + "api_key_name": "api_key", + }[secret_name] + + apim_authenticator = environment.apim_authenticator() + assert apim_authenticator._private_key == "private_key" # noqa SLF001 - access private attribute for testing purposes + assert apim_authenticator._api_key == "api_key" # noqa SLF001 + assert apim_authenticator._key_id == "key_id" # noqa SLF001 + assert apim_authenticator._token_validity_threshold == timedelta(seconds=60) # noqa SLF001 + assert apim_authenticator._token_endpoint == "token_url" # noqa SLF001 + assert apim_authenticator._session_manager == expected_session_manager # noqa SLF001 diff --git a/gateway-api/src/gateway_api/apim_app_auth/test_http.py b/gateway-api/src/gateway_api/apim_app_auth/test_http.py new file mode 100644 index 00000000..2f5da662 --- /dev/null +++ b/gateway-api/src/gateway_api/apim_app_auth/test_http.py @@ -0,0 +1,218 @@ +from datetime import timedelta +from unittest.mock import MagicMock, Mock, patch + +import pytest +import requests +import requests.adapters + +from gateway_api.apim_app_auth.http import SessionManager +from gateway_api.apim_app_auth.request_context import set_correlation_id + + +class TestSessionManager: + @patch("tempfile.NamedTemporaryFile") + @patch("requests.Session") + def test_with_session( + self, mock_session: MagicMock, mock_tempfile: MagicMock + ) -> None: + expected_timeout = timedelta(seconds=30) + session_manager = SessionManager( + client_timeout=expected_timeout, + client_certificate=None, + ) + + @session_manager.with_session + def mock_function(_: requests.Session) -> None: + mock_session.return_value.mount.assert_called_once() + args, _ = mock_session.return_value.mount.call_args + + assert args[0] == "https://" + + adapter = args[1] + assert adapter is not None + assert isinstance(adapter, SessionManager._Adapter) # noqa: SLF001 - Private access for testing + + assert adapter._timeout == expected_timeout.total_seconds() # noqa: SLF001 - Private access for testing + + mock_tempfile.assert_not_called() + + mock_function() + mock_session.return_value.__exit__.assert_called_once() + + @patch("tempfile.NamedTemporaryFile") + @patch("requests.Session") + def test_with_session_with_client_cert( + self, mock_session: MagicMock, mock_tempfile: MagicMock + ) -> None: + expected_timeout = timedelta(seconds=30) + expected_cert = "cert_content" + expected_key = "key_content" + + session_manager = SessionManager( + client_timeout=expected_timeout, + client_certificate={ + "certificate": expected_cert, + "key": expected_key, + }, + ) + + mock_cert_file = MagicMock() + mock_cert_file.name = "cert_file_name" + + mock_key_file = MagicMock() + mock_key_file.name = "key_file_name" + + mock_tempfile.side_effect = [mock_cert_file, mock_key_file] + + @session_manager.with_session + def mock_function(_: requests.Session) -> None: + mock_session.return_value.mount.assert_called_once() + args, _ = mock_session.return_value.mount.call_args + + assert args[0] == "https://" + + adapter = args[1] + assert adapter is not None + assert isinstance(adapter, SessionManager._Adapter) # noqa: SLF001 - Private access for testing + + assert adapter._timeout == expected_timeout.total_seconds() # noqa: SLF001 - Private access for testing + + assert mock_tempfile.call_count == 2 + + assert mock_session.return_value.cert == ( + mock_cert_file.name, + mock_key_file.name, + ) + + mock_cert_file.write.assert_called_once_with(expected_cert.encode()) + mock_cert_file.flush.assert_called_once() + + mock_key_file.write.assert_called_once_with(expected_key.encode()) + mock_key_file.flush.assert_called_once() + + mock_function() + mock_session.return_value.__exit__.assert_called_once() + mock_cert_file.__exit__.assert_called_once() + mock_key_file.__exit__.assert_called_once() + + @patch("tempfile.NamedTemporaryFile") + @patch("requests.Session") + def test_with_session_raises_when_tempfile_creation_fails( + self, mock_session: MagicMock, mock_tempfile: MagicMock + ) -> None: + expected_timeout = timedelta(seconds=30) + + session_manager = SessionManager( + client_timeout=expected_timeout, + client_certificate={ + "certificate": "cert_content", + "key": "key_content", + }, + ) + + mock_tempfile.side_effect = OSError("unable to create temporary file") + + @session_manager.with_session + def mock_function(_: requests.Session) -> None: + msg = "Wrapped function should not be called when tempfile creation fails" + raise AssertionError(msg) + + with pytest.raises(OSError, match="unable to create temporary file"): + mock_function() + + mock_session.return_value.mount.assert_called_once() + mock_session.return_value.__exit__.assert_called_once() + assert mock_tempfile.call_count == 1 + + @patch("requests.Session") + def test_with_session_raises_when_wrapped_function_fails( + self, mock_session: MagicMock + ) -> None: + expected_timeout = timedelta(seconds=30) + session_manager = SessionManager( + client_timeout=expected_timeout, + client_certificate=None, + ) + + @session_manager.with_session + def mock_function(_: requests.Session) -> None: + raise RuntimeError("request handling failed") + + with pytest.raises(RuntimeError, match="request handling failed"): + mock_function() + + mock_session.return_value.mount.assert_called_once() + mock_session.return_value.__exit__.assert_called_once() + + def test_adapter_applies_defaults(self) -> None: + with patch.object( + requests.adapters.HTTPAdapter, "send", autospec=True + ) as mock_send: + set_correlation_id( + full_id="test-correlation-id-long", short_id="test-correlation-id" + ) + + expected_timeout = timedelta(seconds=30) + adapter = SessionManager._Adapter(timeout=expected_timeout.total_seconds()) # noqa: SLF001 - Private access for testing + + mock_request = Mock() + mock_request.headers = {} + + expected_response = Mock() + mock_send.return_value = expected_response + + response = adapter.send(mock_request, verify=True) + assert response == expected_response + + mock_send.assert_called_once_with( + adapter, + mock_request, + verify=True, + timeout=expected_timeout.total_seconds(), + ) + + assert mock_request.headers["X-Correlation-ID"] == "test-correlation-id" + + def test_adapter_overrides_defaults(self) -> None: + with patch.object( + requests.adapters.HTTPAdapter, "send", autospec=True + ) as mock_send: + expected_timeout = timedelta(seconds=30) + adapter = SessionManager._Adapter(timeout=expected_timeout.total_seconds()) # noqa: SLF001 - Private access for testing + + mock_request = Mock() + expected_correlation_id = "correaltion-id" + mock_request.headers = {"X-Correlation-ID": expected_correlation_id} + + expected_response = Mock() + mock_send.return_value = expected_response + + response = adapter.send(mock_request, verify=True) + assert response == expected_response + + mock_send.assert_called_once_with( + adapter, + mock_request, + verify=True, + timeout=expected_timeout.total_seconds(), + ) + + assert mock_request.headers["X-Correlation-ID"] == expected_correlation_id + + def test_adapter_request_error(self) -> None: + with patch.object( + requests.adapters.HTTPAdapter, "send", autospec=True + ) as mock_send: + mock_send.side_effect = requests.RequestException("request failed") + expected_timeout = timedelta(seconds=30) + adapter = SessionManager._Adapter(timeout=expected_timeout.total_seconds()) # noqa: SLF001 - Private access for testing + + mock_request = Mock() + mock_request.headers = {} + + set_correlation_id( + full_id="test-correlation-id-long", short_id="test-correlation-id" + ) + + with pytest.raises(requests.RequestException, match="request failed"): + adapter.send(mock_request) diff --git a/gateway-api/src/gateway_api/apim_app_auth/test_request_context.py b/gateway-api/src/gateway_api/apim_app_auth/test_request_context.py new file mode 100644 index 00000000..5adca6c7 --- /dev/null +++ b/gateway-api/src/gateway_api/apim_app_auth/test_request_context.py @@ -0,0 +1,86 @@ +import threading + +import pytest + +from gateway_api.apim_app_auth.request_context import ( + CorrelationID, + get_correlation_id, + reset_correlation_id, + set_correlation_id, +) + + +class TestSetAndGetCorrelationId: + def test_correlation_id_is_set_and_retrieved(self) -> None: + set_correlation_id( + full_id="round-trip-test-123-long", + short_id="round-trip-test-123", + ) + correlation_id = get_correlation_id() + assert correlation_id.full_id == "round-trip-test-123-long" + assert correlation_id.short_id == "round-trip-test-123" + reset_correlation_id() + + def test_correlation_id_is_cleared_after_reset(self) -> None: + set_correlation_id( + full_id="round-trip-test-123-long", + short_id="round-trip-test-123", + ) + reset_correlation_id() + with pytest.raises( + ValueError, match="Correlation ID is not set in the current context." + ): + get_correlation_id() + + def test_default_correlation_throws_error(self) -> None: + with pytest.raises( + ValueError, match="Correlation ID is not set in the current context." + ): + get_correlation_id() + + def test_correlation_id_is_cleared_when_reset_called_after_exception( + self, + ) -> None: + try: + set_correlation_id( + full_id="will-be-cleared-long", short_id="will-be-cleared" + ) + raise RuntimeError("simulated mid-handler failure") + except RuntimeError: + pass + finally: + reset_correlation_id() + + with pytest.raises( + ValueError, match="Correlation ID is not set in the current context." + ): + get_correlation_id() + + def test_correlation_id_does_not_bleed_between_threads(self) -> None: + results: dict[str, CorrelationID] = {} + + def thread_a() -> None: + set_correlation_id(full_id="thread-a-id-long", short_id="thread-a-id") + import time + + time.sleep(0.05) + results["a"] = get_correlation_id() + reset_correlation_id() + + def thread_b() -> None: + set_correlation_id(full_id="thread-b-id-long", short_id="thread-b-id") + results["b"] = get_correlation_id() + + reset_correlation_id() + + t_a = threading.Thread(target=thread_a) + t_b = threading.Thread(target=thread_b) + t_a.start() + t_b.start() + t_a.join() + t_b.join() + + assert results["a"].full_id == "thread-a-id-long" + assert results["a"].short_id == "thread-a-id" + assert results["b"].full_id == "thread-b-id-long" + assert results["b"].short_id == "thread-b-id" diff --git a/gateway-api/src/gateway_api/app.py b/gateway-api/src/gateway_api/app.py index 4edd0f83..68d27fb6 100644 --- a/gateway-api/src/gateway_api/app.py +++ b/gateway-api/src/gateway_api/app.py @@ -1,3 +1,4 @@ +import logging import os import traceback @@ -13,6 +14,7 @@ app = Flask(__name__) app.logger.setLevel("INFO") +logging.basicConfig(level=logging.INFO) def get_app_host() -> str: diff --git a/gateway-api/src/gateway_api/controller.py b/gateway-api/src/gateway_api/controller.py index ac37c16e..59c459c8 100644 --- a/gateway-api/src/gateway_api/controller.py +++ b/gateway-api/src/gateway_api/controller.py @@ -27,7 +27,8 @@ class Controller: def __init__( self, - pds_base_url: str = PdsClient.SANDBOX_URL, + # TODO #395 control this through env vars + pds_base_url: str = PdsClient.INT_URL, sds_base_url: str = SdsClient.SANDBOX_URL, timeout: int = 10, ) -> None: diff --git a/infrastructure/images/build-container/resources/dev-certificates/.gitignore b/infrastructure/images/build-container/resources/dev-certificates/.gitignore deleted file mode 100644 index d6b7ef32..00000000 --- a/infrastructure/images/build-container/resources/dev-certificates/.gitignore +++ /dev/null @@ -1,2 +0,0 @@ -* -!.gitignore diff --git a/scripts/config/vale/styles/config/vocabularies/words/accept.txt b/scripts/config/vale/styles/config/vocabularies/words/accept.txt index 39633a18..e3bc8905 100644 --- a/scripts/config/vale/styles/config/vocabularies/words/accept.txt +++ b/scripts/config/vale/styles/config/vocabularies/words/accept.txt @@ -30,6 +30,7 @@ Hotspots IDE idempotence [Ii]sort +Jira JUnit markdownlint mypy From 3e0efc2f81a8eba711214397b77ca593e93b0cef Mon Sep 17 00:00:00 2001 From: DWolfsNHS <229101201+DWolfsNHS@users.noreply.github.com> Date: Mon, 27 Apr 2026 10:10:53 +0000 Subject: [PATCH 2/7] [GPCAPIM-395]: gets an invalid token --- Makefile | 30 ++++++++--------- gateway-api/poetry.lock | 18 ++++++---- gateway-api/pyproject.toml | 2 +- .../gateway_api/apim_app_auth/environment.py | 26 +++++---------- .../apim_app_auth/test_environment.py | 12 +++---- gateway-api/src/gateway_api/controller.py | 1 + gateway-api/src/gateway_api/pds/client.py | 33 +++++++++++++++++-- infrastructure/images/gateway-api/Dockerfile | 6 ++++ 8 files changed, 76 insertions(+), 52 deletions(-) diff --git a/Makefile b/Makefile index 198e2aa3..11461487 100644 --- a/Makefile +++ b/Makefile @@ -55,25 +55,23 @@ publish: # Publish the project artefact @Pipeline deploy: clean build # Deploy the project artefact to the target environment @Pipeline @$(docker) network inspect gateway-local >/dev/null 2>&1 || $(docker) network create gateway-local - # Build up list of environment variables to pass to the container - @ENVIRONMENT_STRING="" ; \ - if [[ -n "$${STUB_PROVIDER}" ]]; then \ - ENVIRONMENT_STRING="$${ENVIRONMENT_STRING} -e STUB_PROVIDER=$${STUB_PROVIDER}" ; \ - fi ; \ - if [[ -n "$${STUB_PDS}" ]]; then \ - ENVIRONMENT_STRING="$${ENVIRONMENT_STRING} -e STUB_PDS=$${STUB_PDS}" ; \ - fi ; \ - if [[ -n "$${STUB_SDS}" ]]; then \ - ENVIRONMENT_STRING="$${ENVIRONMENT_STRING} -e STUB_SDS=$${STUB_SDS}" ; \ - fi ; \ - if [[ -n "$${CDG_DEBUG}" ]]; then \ - ENVIRONMENT_STRING="$${ENVIRONMENT_STRING} -e CDG_DEBUG=$${CDG_DEBUG}" ; \ - fi ; \ + # Build up list of environment variables to pass to the container. + # Note: Values (e.g. APIM_PRIVATE_KEY) may contain spaces; use a bash array to avoid breaking `docker run` argument parsing. + @ENVIRONMENT_ARGS=() ; \ + if [[ -n "$${STUB_PROVIDER}" ]]; then ENVIRONMENT_ARGS+=( -e "STUB_PROVIDER=$${STUB_PROVIDER}" ); fi ; \ + if [[ -n "$${STUB_PDS}" ]]; then ENVIRONMENT_ARGS+=( -e "STUB_PDS=$${STUB_PDS}" ); fi ; \ + if [[ -n "$${STUB_SDS}" ]]; then ENVIRONMENT_ARGS+=( -e "STUB_SDS=$${STUB_SDS}" ); fi ; \ + if [[ -n "$${CDG_DEBUG}" ]]; then ENVIRONMENT_ARGS+=( -e "CDG_DEBUG=$${CDG_DEBUG}" ); fi ; \ + if [[ -n "$${APIM_TOKEN_URL}" ]]; then ENVIRONMENT_ARGS+=( -e "APIM_TOKEN_URL=$${APIM_TOKEN_URL}" ); fi ; \ + if [[ -n "$${APIM_API_KEY}" ]]; then ENVIRONMENT_ARGS+=( -e "APIM_API_KEY=$${APIM_API_KEY}" ); fi ; \ + if [[ -n "$${APIM_TOKEN_EXPIRY_THRESHOLD}" ]]; then ENVIRONMENT_ARGS+=( -e "APIM_TOKEN_EXPIRY_THRESHOLD=$${APIM_TOKEN_EXPIRY_THRESHOLD}" ); fi ; \ + if [[ -n "$${APIM_KEY_ID}" ]]; then ENVIRONMENT_ARGS+=( -e "APIM_KEY_ID=$${APIM_KEY_ID}" ); fi ; \ + if [[ -n "$${APIM_PRIVATE_KEY}" ]]; then ENVIRONMENT_ARGS+=( -e "APIM_PRIVATE_KEY=$${APIM_PRIVATE_KEY}" ); 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 $${ENVIRONMENT_STRING} -d ${IMAGE_NAME} ; \ + $(docker) run --platform linux/amd64 --name gateway-api -p 5000:8080 --network gateway-local "$${ENVIRONMENT_ARGS[@]}" -d "${IMAGE_NAME}" ; \ else \ - $(docker) run --platform linux/amd64 --name gateway-api -p 5000:8080 $${ENVIRONMENT_STRING} -d ${IMAGE_NAME} ; \ + $(docker) run --platform linux/amd64 --name gateway-api -p 5000:8080 "$${ENVIRONMENT_ARGS[@]}" -d "${IMAGE_NAME}" ; \ fi clean:: stop # Clean-up project resources (main) @Operations diff --git a/gateway-api/poetry.lock b/gateway-api/poetry.lock index 062cd3cf..7ba34752 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.4 and should not be changed by hand. [[package]] name = "annotated-types" @@ -229,7 +229,7 @@ version = "2.0.0" description = "Foreign Function Interface for Python calling C code." optional = false python-versions = ">=3.9" -groups = ["dev"] +groups = ["main", "dev"] files = [ {file = "cffi-2.0.0-cp310-cp310-macosx_10_13_x86_64.whl", hash = "sha256:0cf2d91ecc3fcc0625c2c530fe004f82c110405f101548512cce44322fa8ac44"}, {file = "cffi-2.0.0-cp310-cp310-macosx_11_0_arm64.whl", hash = "sha256:f73b96c41e3b2adedc34a7356e64c8eb96e03a3782b535e043a986276ce12a49"}, @@ -316,6 +316,7 @@ files = [ {file = "cffi-2.0.0-cp39-cp39-win_amd64.whl", hash = "sha256:b882b3df248017dba09d6b16defe9b5c407fe32fc7c65a9c69798e6175601be9"}, {file = "cffi-2.0.0.tar.gz", hash = "sha256:44d1b5909021139fe36001ae048dbdde8214afa20200eda0f64c068cac5d5529"}, ] +markers = {main = "platform_python_implementation != \"PyPy\""} [package.dependencies] pycparser = {version = "*", markers = "implementation_name != \"PyPy\""} @@ -644,7 +645,7 @@ version = "46.0.7" description = "cryptography is a package which provides cryptographic recipes and primitives to Python developers." optional = false python-versions = "!=3.9.0,!=3.9.1,>=3.8" -groups = ["dev"] +groups = ["main", "dev"] files = [ {file = "cryptography-46.0.7-cp311-abi3-macosx_10_9_universal2.whl", hash = "sha256:ea42cbe97209df307fdc3b155f1b6fa2577c0defa8f1f7d3be7d31d189108ad4"}, {file = "cryptography-46.0.7-cp311-abi3-manylinux2014_aarch64.manylinux_2_17_aarch64.whl", hash = "sha256:b36a4695e29fe69215d75960b22577197aca3f7a25b9cf9d165dcfe9d80bc325"}, @@ -1290,7 +1291,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\""} @@ -2416,12 +2417,12 @@ version = "3.0" description = "C parser in Python" optional = false python-versions = ">=3.10" -groups = ["dev"] -markers = "implementation_name != \"PyPy\"" +groups = ["main", "dev"] files = [ {file = "pycparser-3.0-py3-none-any.whl", hash = "sha256:b727414169a36b7d524c1c3e31839a521725078d7b2ff038656844266160a992"}, {file = "pycparser-3.0.tar.gz", hash = "sha256:600f49d217304a5902ac3c37e1281c9fe94e4d0489de643a9504c5cdfdfc6b29"}, ] +markers = {main = "platform_python_implementation != \"PyPy\" and implementation_name != \"PyPy\"", dev = "implementation_name != \"PyPy\""} [[package]] name = "pycryptodome" @@ -2681,6 +2682,9 @@ files = [ {file = "pyjwt-2.12.1.tar.gz", hash = "sha256:c74a7a2adf861c04d002db713dd85f84beb242228e671280bf709d765b03672b"}, ] +[package.dependencies] +cryptography = {version = ">=3.4.0", optional = true, markers = "extra == \"crypto\""} + [package.extras] crypto = ["cryptography (>=3.4.0)"] dev = ["coverage[toml] (==7.10.7)", "cryptography (>=3.4.0)", "pre-commit", "pytest (>=8.4.2,<9.0.0)", "sphinx", "sphinx-rtd-theme", "zope.interface"] @@ -4008,4 +4012,4 @@ testing = ["coverage[toml]", "zope.event", "zope.testing"] [metadata] lock-version = "2.1" python-versions = ">=3.14,<4.0.0" -content-hash = "c7aa28002e5008ca94fc5657e2c78833c662881d5b86dc7eb9b75560492dc531" +content-hash = "a5b5529fa35b49450f439bf7d514f82002b101f1fb1da37ddb70f68b03777594" diff --git a/gateway-api/pyproject.toml b/gateway-api/pyproject.toml index 6a307caa..06ebfd16 100644 --- a/gateway-api/pyproject.toml +++ b/gateway-api/pyproject.toml @@ -13,7 +13,7 @@ clinical-data-common = { git = "https://github.com/NHSDigital/clinical-data-comm flask = "^3.1.3" types-flask = "^1.1.6" requests = "^2.33.0" -pyjwt = "^2.12.0" +pyjwt = {version = "^2.12.0", extras = ["crypto"]} pydantic = "^2.0" [tool.poetry] diff --git a/gateway-api/src/gateway_api/apim_app_auth/environment.py b/gateway-api/src/gateway_api/apim_app_auth/environment.py index 3d4e96b9..5d0a629d 100644 --- a/gateway-api/src/gateway_api/apim_app_auth/environment.py +++ b/gateway-api/src/gateway_api/apim_app_auth/environment.py @@ -20,12 +20,10 @@ class Environment(TypedDict): client_timeout: Duration apim_token_url: str - apim_private_key_name: str - apim_api_key_name: str + apim_api_key: str apim_token_expiry_threshold: Duration apim_key_id: str - pdm_url: str - mns_url: str + apim_private_key: str _environment: Environment | None = None @@ -45,12 +43,8 @@ def values() -> Environment: "APIM_TOKEN_URL", str, ), - apim_private_key_name=get_environment_variable( - "APIM_PRIVATE_KEY_NAME", - str, - ), - apim_api_key_name=get_environment_variable( - "APIM_API_KEY_NAME", + apim_api_key=get_environment_variable( + "APIM_API_KEY", str, ), apim_token_expiry_threshold=get_environment_variable( @@ -61,12 +55,8 @@ def values() -> Environment: "APIM_KEY_ID", str, ), - pdm_url=get_environment_variable( - "PDM_BUNDLE_URL", - str, - ), - mns_url=get_environment_variable( - "MNS_EVENT_URL", + apim_private_key=get_environment_variable( + "APIM_PRIVATE_KEY", str, ), ) @@ -96,9 +86,9 @@ def apim_authenticator() -> ApimAuthenticator: if _apim_authenticator is None: env = values() _apim_authenticator = ApimAuthenticator( - private_key="", # TODO: 395 get private key + private_key=env["apim_private_key"], key_id=env["apim_key_id"], - api_key="", # TODO: 395 get api key + api_key=env["apim_api_key"], token_endpoint=env["apim_token_url"], token_validity_threshold=env["apim_token_expiry_threshold"].timedelta, session_manager=session_manager(), diff --git a/gateway-api/src/gateway_api/apim_app_auth/test_environment.py b/gateway-api/src/gateway_api/apim_app_auth/test_environment.py index b3c3197f..444d541e 100644 --- a/gateway-api/src/gateway_api/apim_app_auth/test_environment.py +++ b/gateway-api/src/gateway_api/apim_app_auth/test_environment.py @@ -67,12 +67,10 @@ def test_values(self) -> None: assert environ["client_timeout"].timedelta == timedelta(seconds=30) assert environ["apim_token_url"] == "token_url" # noqa S105 - dummy value - assert environ["apim_private_key_name"] == "private_key_name" - assert environ["apim_api_key_name"] == "api_key_name" + assert environ["apim_private_key"] == "private_key_name" + assert environ["apim_api_key"] == "api_key_name" assert environ["apim_token_expiry_threshold"].timedelta == timedelta(seconds=60) assert environ["apim_key_id"] == "key_id" - assert environ["pdm_url"] == "pdm_url" - assert environ["mns_url"] == "mns_url" @patch("gateway_api.apim_app_auth.environment.parameters.get_secret") @patch("gateway_api.apim_app_auth.environment.values") @@ -87,14 +85,12 @@ def test_apim_authenticator( session_manager_mock.return_value = expected_session_manager environ: environment.Environment = { - "apim_private_key_name": "private_key_name", - "apim_api_key_name": "api_key_name", + "apim_private_key": "private_key_name", + "apim_api_key": "api_key_name", "apim_key_id": "key_id", "apim_token_expiry_threshold": Duration(DurationUnit.SECONDS, 60), "apim_token_url": "token_url", "client_timeout": Duration(DurationUnit.SECONDS, 30), - "pdm_url": "pdm_url", - "mns_url": "mns_url", } values_mock.return_value = environ diff --git a/gateway-api/src/gateway_api/controller.py b/gateway-api/src/gateway_api/controller.py index 59c459c8..93df5400 100644 --- a/gateway-api/src/gateway_api/controller.py +++ b/gateway-api/src/gateway_api/controller.py @@ -169,6 +169,7 @@ def _get_pds_details(self, auth_token: str, nhs_number: str) -> str: """ # PDS: find patient and extract GP ODS code (provider ODS) pds = PdsClient( + # TODO 395: is this needed if we use the auth decorator? auth_token=auth_token, base_url=self.pds_base_url, timeout=self.timeout, diff --git a/gateway-api/src/gateway_api/pds/client.py b/gateway-api/src/gateway_api/pds/client.py index 303671bf..82eb3275 100644 --- a/gateway-api/src/gateway_api/pds/client.py +++ b/gateway-api/src/gateway_api/pds/client.py @@ -21,11 +21,14 @@ import os import uuid from collections.abc import Callable +from typing import Any import requests from fhir.r4 import Patient from pydantic import ValidationError +from gateway_api.apim_app_auth import environment +from gateway_api.apim_app_auth.request_context import set_correlation_id from gateway_api.common.error import PdsRequestFailedError # TODO [GPCAPIM-359]: Once stub servers/containers made for PDS, SDS and provider @@ -124,10 +127,19 @@ def search_patient_by_nhs_number( url = f"{self.base_url}/Patient/{nhs_number}" # This normally calls requests.get, but if STUB_PDS is set it uses the stub. - response = get( + # response = get( + # url, + # headers=headers, + # params={}, + # timeout=timeout or self.timeout, + # ) + set_correlation_id( + full_id=correlation_id or "no-correlation-id", + short_id=correlation_id or "no-correlation-id", + ) + response = _make_get_request( url, headers=headers, - params={}, timeout=timeout or self.timeout, ) @@ -145,3 +157,20 @@ def search_patient_by_nhs_number( ) from err return patient + + +@environment.apim_authenticator().auth +def _make_get_request( + # TODO 395: no longer required if we use the auth decorator? + session: requests.Session, + url: str, + headers: dict[str, str], + timeout: int, +) -> Any: + response = session.get( + url=url, + headers=headers, + params={}, + timeout=timeout, + ) + return response diff --git a/infrastructure/images/gateway-api/Dockerfile b/infrastructure/images/gateway-api/Dockerfile index 00cf681f..8d6bc9f6 100644 --- a/infrastructure/images/gateway-api/Dockerfile +++ b/infrastructure/images/gateway-api/Dockerfile @@ -18,6 +18,12 @@ ENV STUB_PDS="true" ENV STUB_PROVIDER="true" ENV STUB_SDS="true" +ENV APIM_API_KEY="default_api_key" +ENV APIM_TOKEN_EXPIRY_THRESHOLD="30s" +ENV APIM_KEY_ID="default_key_id" +ENV APIM_PRIVATE_KEY="default_private_key" +ENV CLIENT_TIMEOUT=30s + ARG COMMIT_VERSION ENV COMMIT_VERSION=$COMMIT_VERSION ARG BUILD_DATE From 6c5a6b8dba7e90eb20fc2c1517e3a4b1ee8309b6 Mon Sep 17 00:00:00 2001 From: DWolfsNHS <229101201+DWolfsNHS@users.noreply.github.com> Date: Mon, 27 Apr 2026 10:19:41 +0000 Subject: [PATCH 3/7] [GPCAPIM-395]: Refactor PDS integration to remove auth token --- gateway-api/src/gateway_api/controller.py | 7 ++----- gateway-api/src/gateway_api/pds/client.py | 4 ---- gateway-api/src/gateway_api/test_controller.py | 6 ++---- 3 files changed, 4 insertions(+), 13 deletions(-) diff --git a/gateway-api/src/gateway_api/controller.py b/gateway-api/src/gateway_api/controller.py index 93df5400..5ae2c346 100644 --- a/gateway-api/src/gateway_api/controller.py +++ b/gateway-api/src/gateway_api/controller.py @@ -53,9 +53,8 @@ def run(self, request: GetStructuredRecordRequest) -> Response: 3) Call SDS using consumer ODS to obtain consumer ASID. 4) Call GP provider to obtain patient records. """ - auth_token = self.get_auth_token() - provider_ods = self._get_pds_details(auth_token, request.nhs_number) + provider_ods = self._get_pds_details(request.nhs_number) consumer_asid, provider_asid, provider_endpoint = self._get_sds_details( request.ods_from.strip(), provider_ods @@ -163,14 +162,12 @@ def get_jwt_for_provider(self, provider_endpoint: str, consumer_ods: str) -> JWT ) return token - def _get_pds_details(self, auth_token: str, nhs_number: str) -> str: + def _get_pds_details(self, nhs_number: str) -> str: """ Call PDS to find the provider ODS code (GP ODS code) for a patient. """ # PDS: find patient and extract GP ODS code (provider ODS) pds = PdsClient( - # TODO 395: is this needed if we use the auth decorator? - auth_token=auth_token, base_url=self.pds_base_url, timeout=self.timeout, ignore_dates=True, diff --git a/gateway-api/src/gateway_api/pds/client.py b/gateway-api/src/gateway_api/pds/client.py index 82eb3275..fcaff69a 100644 --- a/gateway-api/src/gateway_api/pds/client.py +++ b/gateway-api/src/gateway_api/pds/client.py @@ -77,12 +77,10 @@ class PdsClient: def __init__( self, - auth_token: str, base_url: str = SANDBOX_URL, timeout: int = 10, ignore_dates: bool = False, ) -> None: - self.auth_token = auth_token self.base_url = base_url.rstrip("/") self.timeout = timeout self.ignore_dates = ignore_dates @@ -98,7 +96,6 @@ def _build_headers( headers = { "X-Request-ID": request_id or str(uuid.uuid4()), "Accept": "application/fhir+json", - "Authorization": f"Bearer {self.auth_token}", } if correlation_id: @@ -161,7 +158,6 @@ def search_patient_by_nhs_number( @environment.apim_authenticator().auth def _make_get_request( - # TODO 395: no longer required if we use the auth decorator? session: requests.Session, url: str, headers: dict[str, str], diff --git a/gateway-api/src/gateway_api/test_controller.py b/gateway-api/src/gateway_api/test_controller.py index d6cfb1c1..61a6b9a2 100644 --- a/gateway-api/src/gateway_api/test_controller.py +++ b/gateway-api/src/gateway_api/test_controller.py @@ -67,7 +67,6 @@ def test_controller_run_happy_path_returns_returns_expected_body( def test_get_pds_details_returns_provider_ods_code_for_happy_path( mocker: MockerFixture, - auth_token: str, ) -> None: nhs_number = "9000000009" mocker.patch( @@ -76,14 +75,13 @@ def test_get_pds_details_returns_provider_ods_code_for_happy_path( ) controller = Controller(pds_base_url="https://example.test/pds", timeout=7) - actual = controller._get_pds_details(auth_token, nhs_number) # noqa: SLF001 testing private method + actual = controller._get_pds_details(nhs_number) # noqa: SLF001 testing private method assert actual == "A12345" def test_get_pds_details_raises_no_current_provider_when_ods_code_missing_in_pds( mocker: MockerFixture, - auth_token: str, ) -> None: nhs_number = "9000000009" mocker.patch( @@ -97,7 +95,7 @@ def test_get_pds_details_raises_no_current_provider_when_ods_code_missing_in_pds NoCurrentProviderError, match="PDS patient 9000000009 did not contain a current provider ODS code", ): - _ = controller._get_pds_details(auth_token, nhs_number) # noqa: SLF001 testing private method + _ = controller._get_pds_details(nhs_number) # noqa: SLF001 testing private method def test_get_sds_details_returns_consumer_and_provider_details_for_happy_path( From 5b615361cbeb9acce791a4cf1b7b0daa0bbd6009 Mon Sep 17 00:00:00 2001 From: DWolfsNHS <229101201+DWolfsNHS@users.noreply.github.com> Date: Mon, 27 Apr 2026 10:37:50 +0000 Subject: [PATCH 4/7] [GPCAPIM-395]: Add test data for PDS_INT integration --- gateway-api/stubs/stubs/sds/stub.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/gateway-api/stubs/stubs/sds/stub.py b/gateway-api/stubs/stubs/sds/stub.py index 506206a0..9b26e8b4 100644 --- a/gateway-api/stubs/stubs/sds/stub.py +++ b/gateway-api/stubs/stubs/sds/stub.py @@ -404,6 +404,13 @@ def _seed_default_devices(self) -> None: "asid": "918999198738", "display": "ODS/ASID triggering Orange Box", }, + { + "org_ods": "A82038", + "party_key": "A82038-0000809", + "device_id": "B3B3E921-92CA-4A88-A550-2DBB36F703AF", + "asid": "ASID_A82038", + "display": "ODS/ASID from PDS_INT patient", + }, ] # Iterate through test data and create devices @@ -453,6 +460,13 @@ def _seed_default_endpoints(self) -> None: "asid": "918999198738", "address": "https://orange.testlab.nhs.uk/B82617/STU3/1/gpconnect/structured/fhir/", }, + { + "org_ods": "A82038", + "party_key": "A82038-0000809", + "endpoint_id": "E3E3E921-92CA-4A88-A550-2DBB36F703AF", + "asid": "ASID_A82038", + "address": "https://orange.testlab.nhs.uk/B82617/STU3/1/gpconnect/structured/fhir/", + }, ] # Iterate through test data and create endpoints From c4e3779c4b48b257a8ad4ad29d0c362e9666b039 Mon Sep 17 00:00:00 2001 From: DWolfsNHS <229101201+DWolfsNHS@users.noreply.github.com> Date: Mon, 27 Apr 2026 11:09:19 +0000 Subject: [PATCH 5/7] [GPCAPIM-395]: Add test patient to provider stub --- .../stubs/stubs/data/bundles/bundles.py | 1 + .../data/patients/lester_egan_9692140466.json | 30 +++++++++++++++++++ .../stubs/stubs/data/patients/patients.py | 1 + gateway-api/stubs/stubs/provider/stub.py | 5 ++++ 4 files changed, 37 insertions(+) create mode 100644 gateway-api/stubs/stubs/data/patients/lester_egan_9692140466.json diff --git a/gateway-api/stubs/stubs/data/bundles/bundles.py b/gateway-api/stubs/stubs/data/bundles/bundles.py index d714c29d..a97c4ec3 100644 --- a/gateway-api/stubs/stubs/data/bundles/bundles.py +++ b/gateway-api/stubs/stubs/data/bundles/bundles.py @@ -18,3 +18,4 @@ def _wrap_patient_in_bundle(patient: dict[str, Any]) -> dict[str, Any]: } ALICE_JONES_9999999999 = _wrap_patient_in_bundle(Patients.ALICE_JONES_9999999999) + LESTER_EGAN_9692140466 = _wrap_patient_in_bundle(Patients.LESTER_EGAN_9692140466) diff --git a/gateway-api/stubs/stubs/data/patients/lester_egan_9692140466.json b/gateway-api/stubs/stubs/data/patients/lester_egan_9692140466.json new file mode 100644 index 00000000..c7b38dc5 --- /dev/null +++ b/gateway-api/stubs/stubs/data/patients/lester_egan_9692140466.json @@ -0,0 +1,30 @@ +{ + "resourceType": "Patient", + "id": "593C97B5-7B99-4326-9140-B329CC03A0D1", + "meta": { + "versionId": "1533356236474679227", + "profile": [ + "https://fhir.nhs.uk/STU3/StructureDefinition/CareConnect-GPC-Patient-1" + ] + }, + "identifier": [ + { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "9692140466" + } + ], + "name": [ + { + "use": "official", + "family": "Egan", + "given": ["Lester"] + } + ], + "gender": "male", + "birthDate": "1958-02-02", + "generalPractitioner": [ + { + "reference": "Practitioner/C8FD0E2C-3124-4C72-AC8D-ABEA65537D1B" + } + ] + } diff --git a/gateway-api/stubs/stubs/data/patients/patients.py b/gateway-api/stubs/stubs/data/patients/patients.py index 1b6a5a0d..2f1fd241 100644 --- a/gateway-api/stubs/stubs/data/patients/patients.py +++ b/gateway-api/stubs/stubs/data/patients/patients.py @@ -27,3 +27,4 @@ def load_patient(filename: str) -> dict[str, Any]: ) ALICE_JONES_9999999999 = load_patient("alice_jones_9999999999.json") ORANGE_BOX_TRIGGER_9690937278 = load_patient("orange_box_trigger_9690937278.json") + LESTER_EGAN_9692140466 = load_patient("lester_egan_9692140466.json") diff --git a/gateway-api/stubs/stubs/provider/stub.py b/gateway-api/stubs/stubs/provider/stub.py index 78d864e0..cd48ba19 100644 --- a/gateway-api/stubs/stubs/provider/stub.py +++ b/gateway-api/stubs/stubs/provider/stub.py @@ -278,6 +278,11 @@ def access_record_structured( status_code=200, json_data=Bundles.ALICE_JONES_9999999999, ) + if nhs_number == "9692140466": + return self._create_response( + status_code=200, + json_data=Bundles.LESTER_EGAN_9692140466, + ) return self._create_response( status_code=404, From c23ddbd5057793f8609debba25c6a5a9ef982c8f Mon Sep 17 00:00:00 2001 From: DWolfsNHS <229101201+DWolfsNHS@users.noreply.github.com> Date: Mon, 27 Apr 2026 13:10:32 +0000 Subject: [PATCH 6/7] [GPCAPIM-395]: Refactor session manager tests and clean up environment variables - Remove unused MTLS session manager tests - Simplify session manager test by removing mock for secret retrieval - Update environment variable assertions for consistency --- .../apim_app_auth/test_environment.py | 56 +++---------------- 1 file changed, 8 insertions(+), 48 deletions(-) diff --git a/gateway-api/src/gateway_api/apim_app_auth/test_environment.py b/gateway-api/src/gateway_api/apim_app_auth/test_environment.py index 444d541e..4c3588f5 100644 --- a/gateway-api/src/gateway_api/apim_app_auth/test_environment.py +++ b/gateway-api/src/gateway_api/apim_app_auth/test_environment.py @@ -1,6 +1,6 @@ import os from datetime import timedelta -from unittest.mock import MagicMock, call, patch +from unittest.mock import MagicMock, patch from gateway_api.apim_app_auth import environment from gateway_api.apim_app_auth.config import Duration, DurationUnit @@ -12,36 +12,7 @@ def setup_method(self) -> None: # Clear any set environment variables os.environ.clear() - @patch("gateway_api.apim_app_auth.environment.parameters.get_secret") - def test_session_manager_with_mtls(self, get_secret_mock: MagicMock) -> None: - environment._session_manager = ( # noqa SLF001 - access private variable for testing purposes - None # reset session manager to force reinitialisation - ) - - get_secret_mock.side_effect = lambda secret_name: { - "mtls_cert_name": "mtls_cert", - "mtls_key_name": "mtls_key", - }[secret_name] - - os.environ["APIM_MTLS_CERT_NAME"] = "mtls_cert_name" - os.environ["APIM_MTLS_KEY_NAME"] = "mtls_key_name" - os.environ["CLIENT_TIMEOUT"] = "30s" - - certificate_name = "mtls_cert_name" - key_name = "mtls_key_name" - - session_manager = environment.session_manager() - client_certificate = session_manager._client_certificate # noqa SLF001 - access private attribute for testing purposes - - assert client_certificate == { - "certificate": "mtls_cert", - "key": "mtls_key", - } - - get_secret_mock.assert_has_calls([call(certificate_name), call(key_name)]) - - @patch("gateway_api.apim_app_auth.environment.parameters.get_secret") - def test_session_manager(self, get_secret_mock: MagicMock) -> None: + def test_session_manager(self) -> None: environment._session_manager = ( # noqa SLF001 - access private variable for testing purposes None # reset session manager to force reinitialisation ) @@ -51,42 +22,36 @@ def test_session_manager(self, get_secret_mock: MagicMock) -> None: session_manager = environment.session_manager() assert session_manager._client_certificate is None # noqa SLF001 - access private attribute for testing purposes - get_secret_mock.assert_not_called() - def test_values(self) -> None: os.environ["CLIENT_TIMEOUT"] = "30s" os.environ["APIM_TOKEN_URL"] = "token_url" # noqa S105 - dummy value - os.environ["APIM_PRIVATE_KEY_NAME"] = "private_key_name" - os.environ["APIM_API_KEY_NAME"] = "api_key_name" + os.environ["APIM_PRIVATE_KEY"] = "private_key" + os.environ["APIM_API_KEY"] = "api_key" os.environ["APIM_TOKEN_EXPIRY_THRESHOLD"] = "60s" # noqa S105 - dummy value os.environ["APIM_KEY_ID"] = "key_id" - os.environ["PDM_BUNDLE_URL"] = "pdm_url" - os.environ["MNS_EVENT_URL"] = "mns_url" environ = environment.values() assert environ["client_timeout"].timedelta == timedelta(seconds=30) assert environ["apim_token_url"] == "token_url" # noqa S105 - dummy value - assert environ["apim_private_key"] == "private_key_name" - assert environ["apim_api_key"] == "api_key_name" + assert environ["apim_private_key"] == "private_key" + assert environ["apim_api_key"] == "api_key" assert environ["apim_token_expiry_threshold"].timedelta == timedelta(seconds=60) assert environ["apim_key_id"] == "key_id" - @patch("gateway_api.apim_app_auth.environment.parameters.get_secret") @patch("gateway_api.apim_app_auth.environment.values") @patch("gateway_api.apim_app_auth.environment.session_manager") def test_apim_authenticator( self, session_manager_mock: MagicMock, values_mock: MagicMock, - get_secret_mock: MagicMock, ) -> None: expected_session_manager = SessionManager(client_timeout=timedelta(seconds=30)) session_manager_mock.return_value = expected_session_manager environ: environment.Environment = { - "apim_private_key": "private_key_name", - "apim_api_key": "api_key_name", + "apim_private_key": "private_key", + "apim_api_key": "api_key", "apim_key_id": "key_id", "apim_token_expiry_threshold": Duration(DurationUnit.SECONDS, 60), "apim_token_url": "token_url", @@ -95,11 +60,6 @@ def test_apim_authenticator( values_mock.return_value = environ - get_secret_mock.side_effect = lambda secret_name: { - "private_key_name": "private_key", - "api_key_name": "api_key", - }[secret_name] - apim_authenticator = environment.apim_authenticator() assert apim_authenticator._private_key == "private_key" # noqa SLF001 - access private attribute for testing purposes assert apim_authenticator._api_key == "api_key" # noqa SLF001 From 6c4b85379f7a492e8db1d8e59dc202344d484766 Mon Sep 17 00:00:00 2001 From: DWolfsNHS <229101201+DWolfsNHS@users.noreply.github.com> Date: Mon, 27 Apr 2026 13:19:49 +0000 Subject: [PATCH 7/7] [GPCAPIM-395]: Update patch decorators for session manager and JWT encoding for correct path --- .../gateway_api/apim_app_auth/test_apim.py | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/gateway-api/src/gateway_api/apim_app_auth/test_apim.py b/gateway-api/src/gateway_api/apim_app_auth/test_apim.py index 8f0c69b3..a83d6761 100644 --- a/gateway-api/src/gateway_api/apim_app_auth/test_apim.py +++ b/gateway-api/src/gateway_api/apim_app_auth/test_apim.py @@ -37,8 +37,8 @@ def wrapper(*args: Any, **kwargs: Any) -> Any: return wrapper - @patch("pathology_api.http.SessionManager") - @patch("pathology_api.apim.jwt.encode") + @patch("gateway_api.apim_app_auth.http.SessionManager") + @patch("gateway_api.apim_app_auth.apim.jwt.encode") def test_auth(self, mock_jwt: MagicMock, mock_session_manager: MagicMock) -> None: mock_session_manager.with_session = self.mock_with_session @@ -96,8 +96,8 @@ def method(_: requests.Session) -> None: method() - @patch("pathology_api.http.SessionManager") - @patch("pathology_api.apim.jwt.encode") + @patch("gateway_api.apim_app_auth.http.SessionManager") + @patch("gateway_api.apim_app_auth.apim.jwt.encode") def test_auth_existing_valid_token( self, mock_jwt: MagicMock, mock_session_manager: MagicMock ) -> None: @@ -128,8 +128,8 @@ def method(_: requests.Session) -> None: method() - @patch("pathology_api.http.SessionManager") - @patch("pathology_api.apim.jwt.encode") + @patch("gateway_api.apim_app_auth.http.SessionManager") + @patch("gateway_api.apim_app_auth.apim.jwt.encode") def test_auth_existing_invalid_token( self, mock_jwt: MagicMock, mock_session_manager: MagicMock ) -> None: @@ -194,8 +194,8 @@ def method(_: requests.Session) -> None: method() - @patch("pathology_api.http.SessionManager") - @patch("pathology_api.apim.jwt.encode") + @patch("gateway_api.apim_app_auth.http.SessionManager") + @patch("gateway_api.apim_app_auth.apim.jwt.encode") def test_auth_unsuccessful_status_code( self, mock_jwt: MagicMock, mock_session_manager: MagicMock ) -> None: @@ -223,8 +223,8 @@ def method(_: requests.Session) -> None: with pytest.raises(ApimAuthenticationException): method() - @patch("pathology_api.http.SessionManager") - @patch("pathology_api.apim.jwt.encode") + @patch("gateway_api.apim_app_auth.http.SessionManager") + @patch("gateway_api.apim_app_auth.apim.jwt.encode") def test_auth_session_post_raises_exception( self, mock_jwt: MagicMock, mock_session_manager: MagicMock ) -> None: @@ -253,8 +253,8 @@ def method(_: requests.Session) -> None: with pytest.raises(requests.RequestException, match="Connection failed"): method() - @patch("pathology_api.http.SessionManager") - @patch("pathology_api.apim.jwt.encode") + @patch("gateway_api.apim_app_auth.http.SessionManager") + @patch("gateway_api.apim_app_auth.apim.jwt.encode") def test_auth_jwt_encode_raises_exception( self, mock_jwt: MagicMock, mock_session_manager: MagicMock ) -> None: