diff --git a/infrastructure/modules/lambda/lambda.tf b/infrastructure/modules/lambda/lambda.tf index 717fbb0a..1e6e16d2 100644 --- a/infrastructure/modules/lambda/lambda.tf +++ b/infrastructure/modules/lambda/lambda.tf @@ -23,6 +23,7 @@ resource "aws_lambda_function" "eligibility_signposting_lambda" { ENV = var.environment LOG_LEVEL = var.log_level ENABLE_XRAY_PATCHING = var.enable_xray_patching + API_DOMAIN_NAME = var.api_domain_name } } @@ -36,7 +37,9 @@ resource "aws_lambda_function" "eligibility_signposting_lambda" { } layers = compact([ - var.environment == "prod" ? "arn:aws:lambda:${var.region}:580247275435:layer:LambdaInsightsExtension:${var.lambda_insights_extension_version}" : null + var.environment == "prod" ? + "arn:aws:lambda:${var.region}:580247275435:layer:LambdaInsightsExtension:${var.lambda_insights_extension_version}" + : null ]) tracing_config { diff --git a/infrastructure/modules/lambda/variables.tf b/infrastructure/modules/lambda/variables.tf index 92d9d081..dd70823e 100644 --- a/infrastructure/modules/lambda/variables.tf +++ b/infrastructure/modules/lambda/variables.tf @@ -73,3 +73,8 @@ variable "lambda_insights_extension_version" { description = "version number of LambdaInsightsExtension" type = number } + +variable "api_domain_name" { + description = "api domain name - env variable for status endpoint response" + type = string +} diff --git a/infrastructure/stacks/api-layer/api_gateway.tf b/infrastructure/stacks/api-layer/api_gateway.tf index 285d2d08..77e5801f 100644 --- a/infrastructure/stacks/api-layer/api_gateway.tf +++ b/infrastructure/stacks/api-layer/api_gateway.tf @@ -8,12 +8,6 @@ module "eligibility_signposting_api_gateway" { tags = local.tags } -resource "aws_api_gateway_resource" "_status" { - rest_api_id = module.eligibility_signposting_api_gateway.rest_api_id - parent_id = module.eligibility_signposting_api_gateway.root_resource_id - path_part = "_status" -} - resource "aws_api_gateway_resource" "patient_check" { rest_api_id = module.eligibility_signposting_api_gateway.rest_api_id parent_id = module.eligibility_signposting_api_gateway.root_resource_id @@ -26,6 +20,12 @@ resource "aws_api_gateway_resource" "patient" { path_part = "{id}" } +resource "aws_api_gateway_resource" "patient_check_status" { + rest_api_id = module.eligibility_signposting_api_gateway.rest_api_id + parent_id = aws_api_gateway_resource.patient_check.id + path_part = "_status" +} + # deployment resource "aws_api_gateway_deployment" "eligibility_signposting_api" { @@ -34,7 +34,7 @@ resource "aws_api_gateway_deployment" "eligibility_signposting_api" { triggers = { redeployment = sha1(jsonencode([ aws_api_gateway_integration.get_patient_check.id, - aws_api_gateway_integration._status.id, + aws_api_gateway_integration.get_patient_check_status.id ])) } diff --git a/infrastructure/stacks/api-layer/healthcheck_status.tf b/infrastructure/stacks/api-layer/healthcheck_status.tf deleted file mode 100644 index f3e75b65..00000000 --- a/infrastructure/stacks/api-layer/healthcheck_status.tf +++ /dev/null @@ -1,66 +0,0 @@ -resource "aws_api_gateway_method" "_status" { - #checkov:skip=CKV_AWS_59: API is secured via Apigee proxy with mTLS, API keys are not used - #checkov:skip=CKV2_AWS_53: No request parameters to validate for static healthcheck endpoint - rest_api_id = module.eligibility_signposting_api_gateway.rest_api_id - resource_id = aws_api_gateway_resource._status.id - http_method = "GET" - authorization = "NONE" - - depends_on = [ - aws_api_gateway_resource._status, - ] -} - -resource "aws_api_gateway_integration" "_status" { - rest_api_id = module.eligibility_signposting_api_gateway.rest_api_id - resource_id = aws_api_gateway_resource._status.id - http_method = aws_api_gateway_method._status.http_method - type = "MOCK" - - request_templates = { - "application/json" = jsonencode({ - statusCode = 200 - }) - } -} - -resource "aws_api_gateway_method_response" "_status" { - rest_api_id = module.eligibility_signposting_api_gateway.rest_api_id - resource_id = aws_api_gateway_resource._status.id - http_method = aws_api_gateway_method._status.http_method - status_code = "200" - - response_models = { - "application/json" = "Empty" - } -} - -resource "aws_api_gateway_integration_response" "_status" { - rest_api_id = module.eligibility_signposting_api_gateway.rest_api_id - resource_id = aws_api_gateway_resource._status.id - http_method = aws_api_gateway_method._status.http_method - status_code = aws_api_gateway_method_response._status.status_code - - response_templates = { - "application/json" = jsonencode({ - status = "pass", - version = "", - revision = "", - releaseId = "", - commitId = "", - checks = { - "healthcheckService:status" = [ - { - status = "pass", - timeout = false, - responseCode = 200, - outcome = "

Ok

", - links = { - self = "http://healthcheckService.example.com/_status" - } - } - ] - } - }) - } -} diff --git a/infrastructure/stacks/api-layer/lambda.tf b/infrastructure/stacks/api-layer/lambda.tf index 5c71a278..8576a19d 100644 --- a/infrastructure/stacks/api-layer/lambda.tf +++ b/infrastructure/stacks/api-layer/lambda.tf @@ -18,7 +18,7 @@ module "eligibility_signposting_lambda_function" { environment = var.environment runtime = "python3.13" lambda_func_name = "${terraform.workspace == "default" ? "" : "${terraform.workspace}-"}eligibility_signposting_api" - security_group_ids = [data.aws_security_group.main_sg.id] + security_group_ids = [data.aws_security_group.main_sg.id] vpc_intra_subnets = [for v in data.aws_subnet.private_subnets : v.id] file_name = "../../../dist/lambda.zip" handler = "eligibility_signposting_api.app.lambda_handler" @@ -30,4 +30,5 @@ module "eligibility_signposting_lambda_function" { enable_xray_patching = "true" stack_name = local.stack_name provisioned_concurrency_count = 5 + api_domain_name = local.api_domain_name } diff --git a/infrastructure/stacks/api-layer/patient_check.tf b/infrastructure/stacks/api-layer/patient_check.tf index 5130e18b..bd86d45c 100644 --- a/infrastructure/stacks/api-layer/patient_check.tf +++ b/infrastructure/stacks/api-layer/patient_check.tf @@ -38,6 +38,34 @@ resource "aws_api_gateway_integration" "get_patient_check" { ] } +resource "aws_api_gateway_method" "get_patient_check_status" { + #checkov:skip=CKV_AWS_59: API is secured via Apigee proxy with mTLS, API keys are not used + #checkov:skip=CKV2_AWS_53: No request parameters to validate for static healthcheck endpoint + rest_api_id = module.eligibility_signposting_api_gateway.rest_api_id + resource_id = aws_api_gateway_resource.patient_check_status.id + http_method = "GET" + authorization = "NONE" + api_key_required = false + + depends_on = [ + aws_api_gateway_resource.patient_check_status, + aws_api_gateway_resource.patient_check, + ] +} + +resource "aws_api_gateway_integration" "get_patient_check_status" { + rest_api_id = module.eligibility_signposting_api_gateway.rest_api_id + resource_id = aws_api_gateway_resource.patient_check_status.id + http_method = aws_api_gateway_method.get_patient_check_status.http_method + integration_http_method = "POST" # Needed for lambda proxy integration + type = "AWS_PROXY" + uri = module.eligibility_signposting_lambda_function.aws_lambda_invoke_arn + + depends_on = [ + aws_api_gateway_method.get_patient_check_status + ] +} + resource "aws_lambda_permission" "get_patient_check" { statement_id = "AllowExecutionFromAPIGateway" action = "lambda:InvokeFunction" diff --git a/src/eligibility_signposting_api/app.py b/src/eligibility_signposting_api/app.py index 97adea76..cda8ac4c 100644 --- a/src/eligibility_signposting_api/app.py +++ b/src/eligibility_signposting_api/app.py @@ -12,8 +12,8 @@ from eligibility_signposting_api import audit, repos, services from eligibility_signposting_api.common.cache_manager import FLASK_APP_CACHE_KEY, cache_manager from eligibility_signposting_api.common.error_handler import handle_exception -from eligibility_signposting_api.common.request_validator import validate_request_params from eligibility_signposting_api.config.config import config +from eligibility_signposting_api.config.contants import URL_PREFIX from eligibility_signposting_api.logging.logs_helper import log_request_ids_from_headers from eligibility_signposting_api.logging.logs_manager import add_lambda_request_id_to_logger, init_logging from eligibility_signposting_api.logging.tracing_helper import tracing_setup @@ -49,7 +49,6 @@ def get_or_create_app() -> Flask: @add_lambda_request_id_to_logger() @tracing_setup() @log_request_ids_from_headers() -@validate_request_params() def lambda_handler(event: LambdaEvent, context: LambdaContext) -> dict[str, Any]: # pragma: no cover """Run the Flask app as an AWS Lambda.""" app = get_or_create_app() @@ -64,7 +63,7 @@ def create_app() -> Flask: logger.info("app created") # Register views & error handler - app.register_blueprint(eligibility_blueprint, url_prefix="/patient-check") + app.register_blueprint(eligibility_blueprint, url_prefix=f"/{URL_PREFIX}") app.register_error_handler(Exception, handle_exception) # Set up dependency injection using wireup diff --git a/src/eligibility_signposting_api/common/api_error_response.py b/src/eligibility_signposting_api/common/api_error_response.py index 63246dc5..67bef1f3 100644 --- a/src/eligibility_signposting_api/common/api_error_response.py +++ b/src/eligibility_signposting_api/common/api_error_response.py @@ -4,9 +4,10 @@ from datetime import UTC, datetime from enum import Enum from http import HTTPStatus -from typing import Any from fhir.resources.operationoutcome import OperationOutcome, OperationOutcomeIssue +from flask import make_response +from flask.typing import ResponseReturnValue logger = logging.getLogger(__name__) @@ -65,7 +66,7 @@ def build_operation_outcome_issue(self, diagnostics: str, location: list[str] | details=details, ) # pyright: ignore[reportCallIssue] - def generate_response(self, diagnostics: str, location_param: str | None = None) -> dict[str, Any]: + def generate_response(self, diagnostics: str, location_param: str | None = None) -> ResponseReturnValue: issue_location = [f"parameters/{location_param}"] if location_param else None problem = OperationOutcome( @@ -75,16 +76,11 @@ def generate_response(self, diagnostics: str, location_param: str | None = None) ) # pyright: ignore[reportCallIssue] response_body = json.dumps(problem.model_dump(by_alias=True, mode="json")) - - return { - "statusCode": self.status_code, - "headers": {"Content-Type": "application/fhir+json"}, - "body": response_body, - } + return make_response(response_body, self.status_code, {"Content-Type": "application/fhir+json"}) def log_and_generate_response( self, log_message: str, diagnostics: str, location_param: str | None = None - ) -> dict[str, Any]: + ) -> ResponseReturnValue: logger.error(log_message) return self.generate_response(diagnostics, location_param) diff --git a/src/eligibility_signposting_api/common/error_handler.py b/src/eligibility_signposting_api/common/error_handler.py index 6a5166a3..d0f2e258 100644 --- a/src/eligibility_signposting_api/common/error_handler.py +++ b/src/eligibility_signposting_api/common/error_handler.py @@ -1,7 +1,6 @@ import logging import traceback -from flask import make_response from flask.typing import ResponseReturnValue from werkzeug.exceptions import HTTPException @@ -11,7 +10,7 @@ logger = logging.getLogger(__name__) -def handle_exception(e: Exception) -> ResponseReturnValue | HTTPException: +def handle_exception(e: Exception) -> ResponseReturnValue: if isinstance(e, HTTPException): return e @@ -25,7 +24,6 @@ def handle_exception(e: Exception) -> ResponseReturnValue | HTTPException: logger.exception("Unexpected Exception", exc_info=e) log_msg = f"An unexpected error occurred: {full_traceback}" - response = INTERNAL_SERVER_ERROR.log_and_generate_response( + return INTERNAL_SERVER_ERROR.log_and_generate_response( log_message=log_msg, diagnostics="An unexpected error occurred." ) - return make_response(response.get("body"), response.get("statusCode"), response.get("headers")) diff --git a/src/eligibility_signposting_api/common/request_validator.py b/src/eligibility_signposting_api/common/request_validator.py index 7e4cc97a..9573c838 100644 --- a/src/eligibility_signposting_api/common/request_validator.py +++ b/src/eligibility_signposting_api/common/request_validator.py @@ -2,9 +2,9 @@ import re from collections.abc import Callable from functools import wraps -from typing import Any -from mangum.types import LambdaContext, LambdaEvent +from flask import request +from flask.typing import ResponseReturnValue from eligibility_signposting_api.common.api_error_response import ( INVALID_CATEGORY_ERROR, @@ -21,7 +21,7 @@ include_actions_pattern = re.compile(r"^\s*([YN])\s*$", re.IGNORECASE) -def validate_query_params(query_params: dict[str, str]) -> tuple[bool, dict[str, Any] | None]: +def validate_query_params(query_params: dict[str, str]) -> tuple[bool, ResponseReturnValue | None]: conditions = query_params.get("conditions", "ALL").split(",") for condition in conditions: search = re.search(condition_pattern, condition) @@ -39,7 +39,7 @@ def validate_query_params(query_params: dict[str, str]) -> tuple[bool, dict[str, return True, None -def validate_nhs_number(path_nhs: str, header_nhs: str) -> bool: +def validate_nhs_number(path_nhs: str | None, header_nhs: str | None) -> bool: logger.info("NHS numbers from the request", extra={"header_nhs": header_nhs, "path_nhs": path_nhs}) if not header_nhs or not path_nhs: @@ -55,28 +55,28 @@ def validate_nhs_number(path_nhs: str, header_nhs: str) -> bool: def validate_request_params() -> Callable: def decorator(func: Callable) -> Callable: @wraps(func) - def wrapper(event: LambdaEvent, context: LambdaContext) -> dict[str, Any] | None: - path_nhs_no = event.get("pathParameters", {}).get("id") - header_nhs_no = event.get("headers", {}).get(NHS_NUMBER_HEADER) + def wrapper(*args, **kwargs) -> ResponseReturnValue: # noqa:ANN002,ANN003 + path_nhs_number = str(kwargs.get("nhs_number")) + header_nhs_no = str(request.headers.get(NHS_NUMBER_HEADER)) - if not validate_nhs_number(path_nhs_no, header_nhs_no): + if not validate_nhs_number(path_nhs_number, header_nhs_no): message = "You are not authorised to request information for the supplied NHS Number" return NHS_NUMBER_MISMATCH_ERROR.log_and_generate_response(log_message=message, diagnostics=message) - query_params = event.get("queryStringParameters") + query_params = request.args if query_params: is_valid, problem = validate_query_params(query_params) - if not is_valid: + if not is_valid and problem is not None: return problem - return func(event, context) + return func(*args, **kwargs) return wrapper return decorator -def get_include_actions_error_response(include_actions: str) -> dict[str, Any]: +def get_include_actions_error_response(include_actions: str) -> ResponseReturnValue: diagnostics = f"{include_actions} is not a value that is supported by the API" return INVALID_INCLUDE_ACTIONS_ERROR.log_and_generate_response( log_message=f"Invalid include actions query param: '{include_actions}'", @@ -85,14 +85,14 @@ def get_include_actions_error_response(include_actions: str) -> dict[str, Any]: ) -def get_category_error_response(category: str) -> dict[str, Any]: +def get_category_error_response(category: str) -> ResponseReturnValue: diagnostics = f"{category} is not a category that is supported by the API" return INVALID_CATEGORY_ERROR.log_and_generate_response( log_message=f"Invalid category query param: '{category}'", diagnostics=diagnostics, location_param="category" ) -def get_condition_error_response(condition: str) -> dict[str, Any]: +def get_condition_error_response(condition: str) -> ResponseReturnValue: diagnostics = ( f"{condition} should be a single or comma separated list of condition " f"strings with no other punctuation or special characters" diff --git a/src/eligibility_signposting_api/config/config.py b/src/eligibility_signposting_api/config/config.py index 58be7025..546863e9 100644 --- a/src/eligibility_signposting_api/config/config.py +++ b/src/eligibility_signposting_api/config/config.py @@ -14,6 +14,7 @@ AwsAccessKey = NewType("AwsAccessKey", str) AwsSecretAccessKey = NewType("AwsSecretAccessKey", str) AwsKinesisFirehoseStreamName = NewType("AwsKinesisFirehoseStreamName", str) +ApiDomainName = NewType("ApiDomainName", str) @cache diff --git a/src/eligibility_signposting_api/config/contants.py b/src/eligibility_signposting_api/config/contants.py index 0e92962d..3aa45fd3 100644 --- a/src/eligibility_signposting_api/config/contants.py +++ b/src/eligibility_signposting_api/config/contants.py @@ -1,5 +1,6 @@ from typing import Literal +URL_PREFIX = "patient-check" RULE_STOP_DEFAULT = False NHS_NUMBER_HEADER = "nhs-login-nhs-number" ALLOWED_CONDITIONS = Literal["COVID", "FLU", "MMR", "RSV"] diff --git a/src/eligibility_signposting_api/views/eligibility.py b/src/eligibility_signposting_api/views/eligibility.py index 383d73d1..94319d4e 100644 --- a/src/eligibility_signposting_api/views/eligibility.py +++ b/src/eligibility_signposting_api/views/eligibility.py @@ -1,4 +1,5 @@ import logging +import os import uuid from datetime import UTC, datetime from http import HTTPStatus @@ -11,6 +12,8 @@ from eligibility_signposting_api.audit.audit_context import AuditContext from eligibility_signposting_api.audit.audit_service import AuditService from eligibility_signposting_api.common.api_error_response import NHS_NUMBER_NOT_FOUND_ERROR +from eligibility_signposting_api.common.request_validator import validate_request_params +from eligibility_signposting_api.config.contants import URL_PREFIX from eligibility_signposting_api.model.eligibility_status import Condition, EligibilityStatus, NHSNumber, Status from eligibility_signposting_api.services import EligibilityService, UnknownPersonError from eligibility_signposting_api.views.response_model import eligibility_response @@ -32,8 +35,14 @@ def before_request() -> None: AuditContext.add_request_details(request) +@eligibility_blueprint.get("/_status") +def api_status() -> ResponseReturnValue: + return make_response(build_status_payload(), HTTPStatus.OK, {"Content-Type": "application/json"}) + + @eligibility_blueprint.get("/", defaults={"nhs_number": ""}) @eligibility_blueprint.get("/") +@validate_request_params() def check_eligibility( nhs_number: NHSNumber, eligibility_service: Injected[EligibilityService], audit_service: Injected[AuditService] ) -> ResponseReturnValue: @@ -88,10 +97,9 @@ def get_or_default_query_params() -> dict[str, Any]: def handle_unknown_person_error(nhs_number: NHSNumber) -> ResponseReturnValue: diagnostics = f"NHS Number '{nhs_number}' was not recognised by the Eligibility Signposting API" - response = NHS_NUMBER_NOT_FOUND_ERROR.log_and_generate_response( + return NHS_NUMBER_NOT_FOUND_ERROR.log_and_generate_response( log_message=diagnostics, diagnostics=diagnostics, location_param="id" ) - return make_response(response.get("body"), response.get("statusCode"), response.get("headers")) def build_eligibility_response(eligibility_status: EligibilityStatus) -> eligibility_response.EligibilityResponse: @@ -170,3 +178,25 @@ def build_suitability_results(condition: Condition) -> list[eligibility_response for reason in condition.suitability_rules if reason.rule_description ] + + +def build_status_payload() -> dict: + api_domain_name = os.getenv("API_DOMAIN_NAME", "localhost") + return { + "status": "pass", + "version": "", + "revision": "", + "releaseId": "", + "commitId": "", + "checks": { + "healthcheckService:status": [ + { + "status": "pass", + "timeout": False, + "responseCode": HTTPStatus.OK, + "outcome": "

Ok

", + "links": {"self": f"https://{api_domain_name}/{URL_PREFIX}/_status"}, + } + ] + }, + } diff --git a/tests/integration/in_process/test_eligibility_endpoint.py b/tests/integration/in_process/test_eligibility_endpoint.py index 2acb52a9..c1a2f9a4 100644 --- a/tests/integration/in_process/test_eligibility_endpoint.py +++ b/tests/integration/in_process/test_eligibility_endpoint.py @@ -5,6 +5,7 @@ from flask.testing import FlaskClient from hamcrest import ( assert_that, + contains_exactly, equal_to, has_entries, has_entry, @@ -25,9 +26,10 @@ def test_nhs_number_given( campaign_config: CampaignConfig, # noqa: ARG002 ): # Given + headers = {"nhs-login-nhs-number": str(persisted_person)} # When - response = client.get(f"/patient-check/{persisted_person}") + response = client.get(f"/patient-check/{persisted_person}", headers=headers) # Then assert_that( @@ -45,7 +47,27 @@ def test_no_nhs_number_given(self, client: FlaskClient): assert_that( response, is_response() - .with_status_code(HTTPStatus.NOT_FOUND) + .with_status_code(HTTPStatus.FORBIDDEN) + .and_text(is_json_that(has_entries(resourceType="OperationOutcome"))), + ) + + def test_no_nhs_number_given_but_header_given( + self, + client: FlaskClient, + persisted_person: NHSNumber, + campaign_config: CampaignConfig, # noqa: ARG002 + ): + # Given + headers = {"nhs-login-nhs-number": str(persisted_person)} + + # When + response = client.get("/patient-check/", headers=headers) + + # Then + assert_that( + response, + is_response() + .with_status_code(HTTPStatus.FORBIDDEN) .and_text(is_json_that(has_entries(resourceType="OperationOutcome"))), ) @@ -58,9 +80,10 @@ def test_not_base_eligible( campaign_config: CampaignConfig, # noqa: ARG002 ): # Given + headers = {"nhs-login-nhs-number": str(persisted_person_no_cohorts)} # When - response = client.get(f"/patient-check/{persisted_person_no_cohorts}?includeActions=Y") + response = client.get(f"/patient-check/{persisted_person_no_cohorts}?includeActions=Y", headers=headers) # Then assert_that( @@ -101,9 +124,10 @@ def test_not_eligible_by_rule( campaign_config: CampaignConfig, # noqa: ARG002 ): # Given + headers = {"nhs-login-nhs-number": str(persisted_person_pc_sw19)} # When - response = client.get(f"/patient-check/{persisted_person_pc_sw19}?includeActions=Y") + response = client.get(f"/patient-check/{persisted_person_pc_sw19}?includeActions=Y", headers=headers) # Then assert_that( @@ -144,9 +168,10 @@ def test_not_actionable( campaign_config: CampaignConfig, # noqa: ARG002 ): # Given + headers = {"nhs-login-nhs-number": str(persisted_person)} # When - response = client.get(f"/patient-check/{persisted_person}?includeActions=Y") + response = client.get(f"/patient-check/{persisted_person}?includeActions=Y", headers=headers) # Then assert_that( @@ -192,10 +217,10 @@ def test_actionable( persisted_77yo_person: NHSNumber, campaign_config: CampaignConfig, # noqa: ARG002 ): - # Given + headers = {"nhs-login-nhs-number": str(persisted_77yo_person)} # When - response = client.get(f"/patient-check/{persisted_77yo_person}?includeActions=Y") + response = client.get(f"/patient-check/{persisted_77yo_person}?includeActions=Y", headers=headers) # Then assert_that( @@ -244,9 +269,10 @@ def test_actionable_with_and_rule( campaign_config_with_and_rule: CampaignConfig, # noqa: ARG002 ): # Given + headers = {"nhs-login-nhs-number": str(persisted_person)} # When - response = client.get(f"/patient-check/{persisted_person}?includeActions=Y") + response = client.get(f"/patient-check/{persisted_person}?includeActions=Y", headers=headers) # Then assert_that( @@ -297,9 +323,10 @@ def test_not_eligible_by_rule_when_only_virtual_cohort_is_present( campaign_config_with_virtual_cohort: CampaignConfig, # noqa: ARG002 ): # Given + headers = {"nhs-login-nhs-number": str(persisted_person_pc_sw19)} # When - response = client.get(f"/patient-check/{persisted_person_pc_sw19}?includeActions=Y") + response = client.get(f"/patient-check/{persisted_person_pc_sw19}?includeActions=Y", headers=headers) # Then assert_that( @@ -340,9 +367,10 @@ def test_not_actionable_when_only_virtual_cohort_is_present( campaign_config_with_virtual_cohort: CampaignConfig, # noqa: ARG002 ): # Given + headers = {"nhs-login-nhs-number": str(persisted_person)} # When - response = client.get(f"/patient-check/{persisted_person}?includeActions=Y") + response = client.get(f"/patient-check/{persisted_person}?includeActions=Y", headers=headers) # Then assert_that( @@ -389,9 +417,10 @@ def test_actionable_when_only_virtual_cohort_is_present( campaign_config_with_virtual_cohort: CampaignConfig, # noqa: ARG002 ): # Given + headers = {"nhs-login-nhs-number": str(persisted_77yo_person)} # When - response = client.get(f"/patient-check/{persisted_77yo_person}?includeActions=Y") + response = client.get(f"/patient-check/{persisted_77yo_person}?includeActions=Y", headers=headers) # Then assert_that( @@ -442,9 +471,10 @@ def test_not_base_eligible( campaign_config_with_missing_descriptions_missing_rule_text: CampaignConfig, # noqa: ARG002 ): # Given + headers = {"nhs-login-nhs-number": str(persisted_person_no_cohorts)} # When - response = client.get(f"/patient-check/{persisted_person_no_cohorts}?includeActions=Y") + response = client.get(f"/patient-check/{persisted_person_no_cohorts}?includeActions=Y", headers=headers) # Then assert_that( @@ -479,9 +509,10 @@ def test_not_eligible_by_rule( campaign_config_with_missing_descriptions_missing_rule_text: CampaignConfig, # noqa: ARG002 ): # Given + headers = {"nhs-login-nhs-number": str(persisted_person_pc_sw19)} # When - response = client.get(f"/patient-check/{persisted_person_pc_sw19}?includeActions=Y") + response = client.get(f"/patient-check/{persisted_person_pc_sw19}?includeActions=Y", headers=headers) # Then assert_that( @@ -516,9 +547,10 @@ def test_not_actionable( campaign_config_with_missing_descriptions_missing_rule_text: CampaignConfig, # noqa: ARG002 ): # Given + headers = {"nhs-login-nhs-number": str(persisted_person)} # When - response = client.get(f"/patient-check/{persisted_person}?includeActions=Y") + response = client.get(f"/patient-check/{persisted_person}?includeActions=Y", headers=headers) # Then assert_that( @@ -559,9 +591,10 @@ def test_actionable( campaign_config_with_missing_descriptions_missing_rule_text: CampaignConfig, # noqa: ARG002 ): # Given + headers = {"nhs-login-nhs-number": str(persisted_77yo_person)} # When - response = client.get(f"/patient-check/{persisted_77yo_person}?includeActions=Y") + response = client.get(f"/patient-check/{persisted_77yo_person}?includeActions=Y", headers=headers) # Then assert_that( @@ -604,9 +637,10 @@ def test_actionable_no_actions( campaign_config_with_missing_descriptions_missing_rule_text: CampaignConfig, # noqa: ARG002 ): # Given + headers = {"nhs-login-nhs-number": str(persisted_77yo_person)} # When - response = client.get(f"/patient-check/{persisted_77yo_person}?includeActions=N") + response = client.get(f"/patient-check/{persisted_77yo_person}?includeActions=N", headers=headers) # Then assert_that( @@ -632,3 +666,38 @@ def test_actionable_no_actions( ) ), ) + + def test_status_endpoint(self, client: FlaskClient): + # When + response = client.get("/patient-check/_status") + + # Then + assert_that( + response, + is_response() + .with_status_code(HTTPStatus.OK) + .and_json( + has_entries( + { + "status": "pass", + "checks": has_entries( + { + "healthcheckService:status": contains_exactly( + has_entries( + { + "status": "pass", + "timeout": False, + "responseCode": HTTPStatus.OK, + "outcome": "

Ok

", + "links": has_entries({"self": "https://localhost/patient-check/_status"}), + } + ) + ) + } + ), + } + ) + ), + ) + + assert_that(response.headers, has_entry("Content-Type", "application/json")) diff --git a/tests/integration/lambda/test_app_running_as_lambda.py b/tests/integration/lambda/test_app_running_as_lambda.py index 62343a32..9c473696 100644 --- a/tests/integration/lambda/test_app_running_as_lambda.py +++ b/tests/integration/lambda/test_app_running_as_lambda.py @@ -9,7 +9,6 @@ from botocore.exceptions import ClientError from brunns.matchers.data import json_matching as is_json_that from brunns.matchers.response import is_response -from faker import Faker from freezegun import freeze_time from hamcrest import ( assert_that, @@ -108,14 +107,14 @@ def test_install_and_call_flask_lambda_over_http( def test_install_and_call_flask_lambda_with_unknown_nhs_number( flask_function: str, + persisted_person: NHSNumber, campaign_config: CampaignConfig, # noqa: ARG001 logs_client: BaseClient, api_gateway_endpoint: URL, - faker: Faker, ): """Given lambda installed into localstack, run it via http, with a nonexistent NHS number specified""" # Given - nhs_number = NHSNumber(faker.nhs_number()) + nhs_number = f"123{persisted_person}" # When invoke_url = f"{api_gateway_endpoint}/patient-check/{nhs_number}" @@ -691,3 +690,44 @@ def test_incorrect_token_causes_internal_server_error( # noqa: PLR0913 get_log_messages(flask_function, logs_client), has_item(contains_string("Invalid attribute name 'ICECREAM' in token '[[PERSON.ICECREAM]]'.")), ) + + +def test_status_end_point(api_gateway_endpoint: URL): + """Given api-gateway and lambda installed into localstack, run it via http""" + # Given + # When + invoke_url = f"{api_gateway_endpoint}/patient-check/_status" + response = httpx.get( + invoke_url, + timeout=10, + ) + + # Then + assert_that( + response, + is_response() + .with_status_code(HTTPStatus.OK) + .with_headers(has_entries({"Content-Type": "application/json"})) + .and_json( + has_entries( + { + "status": "pass", + "checks": has_entries( + { + "healthcheckService:status": contains_exactly( + has_entries( + { + "status": "pass", + "timeout": False, + "responseCode": HTTPStatus.OK, + "outcome": "

Ok

", + "links": has_entries({"self": "https://localhost/patient-check/_status"}), + } + ) + ) + } + ), + } + ) + ), + ) diff --git a/tests/unit/common/test_request_validator.py b/tests/unit/common/test_request_validator.py index d00c9768..7de1c776 100644 --- a/tests/unit/common/test_request_validator.py +++ b/tests/unit/common/test_request_validator.py @@ -1,9 +1,9 @@ -import json import logging from http import HTTPStatus from unittest.mock import MagicMock import pytest +from flask import request from eligibility_signposting_api.common import request_validator from eligibility_signposting_api.common.request_validator import logger @@ -40,48 +40,49 @@ def test_validate_nhs_number(self, path_nhs, header_nhs, expected_result, expect class TestValidateRequestParams: - def test_validate_request_params_success(self, caplog): - mock_handler = MagicMock() - mock_handler.__name__ = "mock_handler" - - mock_event_valid = { - "pathParameters": {"id": "1234567890"}, - "headers": {"nhs-login-nhs-number": "1234567890"}, - } - mock_context = {} + def test_validate_request_params_success(self, app, caplog): + mock_api = MagicMock(return_value="success") decorator = request_validator.validate_request_params() - wrapped_handler = decorator(mock_handler) - with caplog.at_level(logging.INFO): - wrapped_handler(mock_event_valid, mock_context) + dummy_route = decorator(mock_api) + + with app.test_request_context( + "/dummy?id=1234567890", + headers={"nhs-login-nhs-number": "1234567890"}, + method="GET", + ): + with caplog.at_level(logging.INFO): + response = dummy_route(nhs_number=request.args.get("id")) - assert any("NHS numbers from the request" in record.message for record in caplog.records) - assert not any(record.levelname == "ERROR" for record in caplog.records) + assert response == "success" + assert any("NHS numbers from the request" in record.message for record in caplog.records) + assert not any(record.levelname == "ERROR" for record in caplog.records) - def test_validate_request_params_nhs_mismatch(self, caplog): - mock_handler = MagicMock() - mock_context = {} - event = { - "pathParameters": {"id": "0987654321"}, - "headers": {"nhs-login-nhs-number": "1234567890"}, - } + def test_validate_request_params_nhs_mismatch(self, app, caplog): + mock_api = MagicMock() decorator = request_validator.validate_request_params() - wrapped_handler = decorator(mock_handler) + dummy_route = decorator(mock_api) - with caplog.at_level(logging.ERROR): - response = wrapped_handler(event, mock_context) + with app.test_request_context( + "/dummy?id=1234567890", + headers={"nhs-login-nhs-number": "0987654321"}, + method="GET", + ): + with caplog.at_level(logging.INFO): + response = dummy_route(nhs_number=request.args.get("id")) - mock_handler.assert_not_called() + mock_api.assert_not_called() - assert response is not None - assert response["statusCode"] == HTTPStatus.FORBIDDEN - response_body = json.loads(response["body"]) - issue = response_body["issue"][0] - assert issue["code"] == "forbidden" - assert issue["details"]["coding"][0]["code"] == "ACCESS_DENIED" - assert issue["details"]["coding"][0]["display"] == "Access has been denied to process this request." - assert issue["diagnostics"] == "You are not authorised to request information for the supplied NHS Number" + assert response is not None + assert response.status_code == HTTPStatus.FORBIDDEN + response_json = response.json + issue = response_json["issue"][0] + assert issue["code"] == "forbidden" + assert issue["details"]["coding"][0]["code"] == "ACCESS_DENIED" + assert issue["details"]["coding"][0]["display"] == "Access has been denied to process this request." + assert issue["diagnostics"] == "You are not authorised to request information for the supplied NHS Number" + assert response.headers["Content-Type"] == "application/fhir+json" class TestValidateQueryParameters: @@ -259,18 +260,18 @@ def test_validate_query_params_returns_correct_problem_details_for_conditions_er assert is_valid is False assert problem is not None - assert problem["statusCode"] == HTTPStatus.BAD_REQUEST - assert problem["headers"]["Content-Type"] == "application/fhir+json" + assert problem.status_code == HTTPStatus.BAD_REQUEST + assert problem.headers["Content-Type"] == "application/fhir+json" - response_body = json.loads(problem["body"]) + response_json = problem.json - assert response_body["resourceType"] == "OperationOutcome" - assert "id" in response_body - assert "meta" in response_body - assert "lastUpdated" in response_body["meta"] + assert response_json["resourceType"] == "OperationOutcome" + assert "id" in response_json + assert "meta" in response_json + assert "lastUpdated" in response_json["meta"] - assert len(response_body["issue"]) == 1 - issue = response_body["issue"][0] + assert len(response_json["issue"]) == 1 + issue = response_json["issue"][0] assert issue["severity"] == "error" assert issue["code"] == "value" @@ -296,18 +297,18 @@ def test_validate_query_params_returns_correct_problem_details_for_category_erro assert is_valid is False assert problem is not None - assert problem["statusCode"] == HTTPStatus.UNPROCESSABLE_ENTITY - assert problem["headers"]["Content-Type"] == "application/fhir+json" + assert problem.status_code == HTTPStatus.UNPROCESSABLE_ENTITY + assert problem.headers["Content-Type"] == "application/fhir+json" - response_body = json.loads(problem["body"]) + response_json = problem.json - assert response_body["resourceType"] == "OperationOutcome" - assert "id" in response_body - assert "meta" in response_body - assert "lastUpdated" in response_body["meta"] + assert response_json["resourceType"] == "OperationOutcome" + assert "id" in response_json + assert "meta" in response_json + assert "lastUpdated" in response_json["meta"] - assert len(response_body["issue"]) == 1 - issue = response_body["issue"][0] + assert len(response_json["issue"]) == 1 + issue = response_json["issue"][0] assert issue["severity"] == "error" assert issue["code"] == "value" @@ -330,18 +331,18 @@ def test_validate_query_params_returns_correct_problem_details_for_include_actio assert is_valid is False assert problem is not None - assert problem["statusCode"] == HTTPStatus.UNPROCESSABLE_ENTITY - assert problem["headers"]["Content-Type"] == "application/fhir+json" + assert problem.status_code == HTTPStatus.UNPROCESSABLE_ENTITY + assert problem.headers["Content-Type"] == "application/fhir+json" - response_body = json.loads(problem["body"]) + response_json = problem.json - assert response_body["resourceType"] == "OperationOutcome" - assert "id" in response_body - assert "meta" in response_body - assert "lastUpdated" in response_body["meta"] + assert response_json["resourceType"] == "OperationOutcome" + assert "id" in response_json + assert "meta" in response_json + assert "lastUpdated" in response_json["meta"] - assert len(response_body["issue"]) == 1 - issue = response_body["issue"][0] + assert len(response_json["issue"]) == 1 + issue = response_json["issue"][0] assert issue["severity"] == "error" assert issue["code"] == "value" diff --git a/tests/unit/views/test_eligibility.py b/tests/unit/views/test_eligibility.py index 04d22323..40d26c6d 100644 --- a/tests/unit/views/test_eligibility.py +++ b/tests/unit/views/test_eligibility.py @@ -98,8 +98,11 @@ def test_nhs_number_given(app: Flask, client: FlaskClient): get_app_container(app).override.service(EligibilityService, new=FakeEligibilityService()), get_app_container(app).override.service(AuditService, new=FakeAuditService()), ): + # Given + headers = {"nhs-login-nhs-number": str(12345)} + # When - response = client.get("/patient-check/12345") + response = client.get("/patient-check/12345", headers=headers) # Then assert_that(response, is_response().with_status_code(HTTPStatus.OK)) @@ -108,14 +111,17 @@ def test_nhs_number_given(app: Flask, client: FlaskClient): def test_no_nhs_number_given(app: Flask, client: FlaskClient): # Given with get_app_container(app).override.service(EligibilityService, new=FakeUnknownPersonEligibilityService()): + # Given + headers = {"nhs-login-nhs-number": str(12345)} + # When - response = client.get("/patient-check/") + response = client.get("/patient-check/", headers=headers) # Then assert_that( response, is_response() - .with_status_code(HTTPStatus.NOT_FOUND) + .with_status_code(HTTPStatus.FORBIDDEN) .with_headers(has_entries({"Content-Type": "application/fhir+json"})) .and_text( is_json_that( @@ -124,16 +130,14 @@ def test_no_nhs_number_given(app: Flask, client: FlaskClient): issue=contains_exactly( has_entries( severity="error", - code="processing", - diagnostics="NHS Number '' was not recognised by the Eligibility Signposting API", + code="forbidden", + diagnostics="You are not authorised to request information for the supplied NHS Number", details={ "coding": [ { "system": "https://fhir.nhs.uk/STU3/ValueSet/Spine-ErrorOrWarningCode-1", - "code": "REFERENCE_NOT_FOUND", - "display": "The given NHS number was not found in our datasets. " - "This could be because the number is incorrect or " - "some other reason we cannot process that number.", + "code": "ACCESS_DENIED", + "display": "Access has been denied to process this request.", } ] }, @@ -147,8 +151,10 @@ def test_no_nhs_number_given(app: Flask, client: FlaskClient): def test_unexpected_error(app: Flask, client: FlaskClient): # Given + headers = {"nhs-login-nhs-number": str(12345)} + with get_app_container(app).override.service(EligibilityService, new=FakeUnexpectedErrorEligibilityService()): - response = client.get("/patient-check/12345") + response = client.get("/patient-check/12345", headers=headers) assert_that( response, @@ -355,7 +361,7 @@ def test_excludes_nulls_via_build_response(client: FlaskClient): return_value=mocked_response, ), ): - response = client.get("/patient-check/12345") + response = client.get("/patient-check/12345", headers={"nhs-login-nhs-number": str(12345)}) assert response.status_code == HTTPStatus.OK payload = json.loads(response.data) @@ -407,7 +413,7 @@ def test_build_response_include_values_that_are_not_null(client: FlaskClient): return_value=mocked_response, ), ): - response = client.get("/patient-check/12345") + response = client.get("/patient-check/12345", headers={"nhs-login-nhs-number": str(12345)}) assert response.status_code == HTTPStatus.OK payload = json.loads(response.data) @@ -461,3 +467,39 @@ def test_get_or_default_query_params_missing_include_actions(app: Flask): expected = {"includeActions": "Y", "category": "SCREENING", "conditions": ["COVID19", "FLU"]} assert_that(result, is_(expected)) + + +def test_status_endpoint(app: Flask, client: FlaskClient): + with get_app_container(app).override.service(EligibilityService, new=FakeEligibilityService()): + response = client.get("/patient-check/_status") + + assert_that(response, is_response().with_status_code(HTTPStatus.OK)) + + assert_that( + response, + is_response() + .with_status_code(HTTPStatus.OK) + .with_headers(has_entries({"Content-Type": "application/json"})) + .and_json( + has_entries( + { + "status": "pass", + "checks": has_entries( + { + "healthcheckService:status": contains_exactly( + has_entries( + { + "status": "pass", + "timeout": False, + "responseCode": HTTPStatus.OK, + "outcome": "

Ok

", + "links": has_entries({"self": "https://localhost/patient-check/_status"}), + } + ) + ) + } + ), + } + ) + ), + )