Merge pull request #1172 from NHSDigital/NRL-2015-option-4

NRL-2015 option 4 - choose v2 if v2 permissions file in lambda layer

Author: anjalitrace2-nhs

Makefile

@@ -8,7 +8,7 @@ SHELL := /bin/bash
 DIST_PATH ?= ./dist
 TEST_ARGS ?= --cov --cov-report=term-missing --cov-report=xml:$(DIST_PATH)/test-coverage.xml
 SMOKE_TEST_ARGS ?=
-FEATURE_TEST_ARGS ?= ./tests/features --format progress2
+FEATURE_TEST_ARGS ?= ./tests/features
 TF_WORKSPACE_NAME ?= $(shell terraform -chdir=terraform/infrastructure workspace show)
 ENV ?= dev
 ACCOUNT ?= dev
@@ -117,6 +117,7 @@ test-features-integration: check-warn ## Run the BDD feature tests in the integr
 		--define="env=$(TF_WORKSPACE_NAME)" \
 		--define="account_name=$(ENV)" \
 		--define="use_shared_resources=${USE_SHARED_RESOURCES}" \
+		-v --format progress2 \
 		$(FEATURE_TEST_ARGS)
 
 integration-test-with-custom_tag:

layer/nrlf/core/authoriser.py

@@ -25,17 +25,24 @@ def get_pointer_permissions_v2(
     # check for app-wide permissions
     app_wide_key = f"{producer_or_consumer}/{app_id}.json"
     if path.isfile(f"/opt/python/nrlf_permissions/{app_wide_key}"):
-        logger.log(LogReference.V2PERMISSIONS011, key=app_wide_key)
         key = app_wide_key
     else:  # use org level
         key = f"{producer_or_consumer}/{app_id}/{ods_code}.json"
-        logger.log(LogReference.V2PERMISSIONS011, key=key)
+
+    logger.log(LogReference.V2PERMISSIONS011, key=key)
     file_path = f"/opt/python/nrlf_permissions/{key}"
 
     pointer_permissions = {}
     try:
         with open(file_path) as file:
             pointer_permissions = json.load(file)
+    except FileNotFoundError as exc:
+        logger.log(
+            LogReference.V2PERMISSIONS013,
+            exc_info=sys.exc_info(),
+            error=str(exc),
+        )
+        raise exc
     except Exception as exc:
         logger.log(
             LogReference.S3PERMISSIONS005,

layer/nrlf/core/decorators.py

@@ -9,7 +9,7 @@
     event_source,
 )
 from aws_lambda_powertools.utilities.typing import LambdaContext
-from pydantic import BaseModel
+from pydantic import BaseModel, ValidationError
 
 from nrlf.core.authoriser import (
     get_pointer_permissions_v2,
@@ -19,8 +19,6 @@
 from nrlf.core.codes import SpineErrorConcept
 from nrlf.core.config import Config
 from nrlf.core.constants import (
-    CLIENT_RP_DETAILS,
-    CONNECTION_METADATA,
     NHSD_CORRELATION_ID_HEADER,
     PERMISSION_ALLOW_ALL_POINTER_TYPES,
     X_CORRELATION_ID_HEADER,
@@ -31,7 +29,7 @@
 from nrlf.core.dynamodb.repository import DocumentPointerRepository
 from nrlf.core.errors import OperationOutcomeError, ParseError
 from nrlf.core.logger import LogReference, logger
-from nrlf.core.model import PermissionsPolicy
+from nrlf.core.model import ConnectionMetadata, PermissionsPolicy
 from nrlf.core.request import parse_body, parse_headers, parse_params, parse_path
 from nrlf.core.response import Response
 
@@ -74,7 +72,7 @@ def wrapper(*args, **kwargs) -> Dict[str, Any]:
 
 
 def header_handler(
-    wrapped_func: Callable[..., Dict[str, Any]]
+    wrapped_func: Callable[..., Dict[str, Any]],
 ) -> Callable[..., Dict[str, Any]]:
     """
     Wraps the function to set the specific headers in the request and response
@@ -118,7 +116,7 @@ def wrapper(*args, **kwargs) -> Dict[str, Any]:
 
 
 def logger_initialiser(
-    wrapper_func: Callable[..., Dict[str, Any]]
+    wrapper_func: Callable[..., Dict[str, Any]],
 ) -> Callable[..., Dict[str, Any]]:
     """
     Wraps the function and initialises the request logger
@@ -145,27 +143,48 @@ def wrapper(*args, **kwargs) -> Dict[str, Any]:
 RepositoryType = Union[Type[DocumentPointerRepository], None]
 
 
-def _use_v2_permissions_model(headers: Dict[str, str]) -> bool:
-    case_insensitive_headers = {key.lower(): value for key, value in headers.items()}
-    # if either or both headers are missing
-    return (
-        CLIENT_RP_DETAILS not in case_insensitive_headers.keys()
-        or CONNECTION_METADATA not in case_insensitive_headers.keys()
-    )
+def v1_perms_stuff(metadata: ConnectionMetadata, config: Config):
+    if PERMISSION_ALLOW_ALL_POINTER_TYPES in metadata.nrl_permissions:
+        logger.log(LogReference.HANDLER004a)
+        metadata.pointer_types = PointerTypes.list()
+        return metadata
+
+    logger.log(LogReference.HANDLER004b)
+    pointer_types = parse_permissions_file(metadata)
+    if not pointer_types and not metadata.is_test_event:
+        logger.log(LogReference.HANDLER004)
+        pointer_types = get_pointer_types(metadata, config)
 
+    metadata.pointer_types = pointer_types
+    logger.log(LogReference.HANDLER004c, pointer_types=pointer_types)
 
-def _load_v2_connection_metadata(headers: Dict[str, str], path: str):
-    logger.log(LogReference.HANDLER004d)
+    return metadata
 
-    metadata = parse_headers(headers, use_v2_permissions=True)
-    logger.log(LogReference.HANDLER003, metadata=metadata.model_dump())
 
-    logger.log(LogReference.HANDLER004b)
+def v2_perms_stuff(metadata: ConnectionMetadata, path=""):
     pointer_permissions = get_pointer_permissions_v2(metadata, path)
 
-    metadata.nrl_permissions_policy = PermissionsPolicy.model_validate(
-        pointer_permissions
-    )
+    try:
+        metadata.nrl_permissions_policy = PermissionsPolicy.model_validate(
+            pointer_permissions
+        )
+    except ValidationError as err:
+        logger.log(
+            LogReference.HANDLER004e,
+            pointer_permissions=pointer_permissions,
+            path=path,
+            validation_errors=err.errors(),
+        )
+        raise OperationOutcomeError(
+            status_code="401",
+            severity="error",
+            code="invalid",
+            details=SpineErrorConcept.from_code("MISSING_OR_INVALID_HEADER"),
+            diagnostics=(
+                "Unable to parse metadata about the requesting application. "
+                "Contact the onboarding team."
+            ),
+        ) from None
 
     if (
         AccessControls.ALLOW_ALL_TYPES.value
@@ -189,27 +208,16 @@ def _load_v2_connection_metadata(headers: Dict[str, str], path: str):
 def load_connection_metadata(headers: Dict[str, str], config: Config, path=""):
     logger.log(LogReference.HANDLER002, headers=headers)
 
-    if _use_v2_permissions_model(headers):
-        return _load_v2_connection_metadata(headers, path)
-
-    metadata = parse_headers(headers, use_v2_permissions=False)
+    metadata = parse_headers(headers)
     logger.log(LogReference.HANDLER003, metadata=metadata.model_dump())
 
-    if PERMISSION_ALLOW_ALL_POINTER_TYPES in metadata.nrl_permissions:
-        logger.log(LogReference.HANDLER004a)
-        metadata.pointer_types = PointerTypes.list()
-        return metadata
-
-    logger.log(LogReference.HANDLER004b)
-    pointer_types = parse_permissions_file(metadata)
-    if not pointer_types and not metadata.is_test_event:
-        logger.log(LogReference.HANDLER004)
-        pointer_types = get_pointer_types(metadata, config)
-
-    metadata.pointer_types = pointer_types
-    logger.log(LogReference.HANDLER004c, pointer_types=pointer_types)
+    try:
+        return v2_perms_stuff(metadata, path)
+    except FileNotFoundError:
+        # No v2 perms file found, so try v1 instead
+        pass
 
-    return metadata
+    return v1_perms_stuff(metadata, config)
 
 
 def filter_kwargs(handler_func: RequestHandler, kwargs: Dict[str, Any]):

layer/nrlf/core/log_references.py

@@ -33,6 +33,7 @@ class LogReference(Enum):
     HANDLER004b = _Reference("INFO", "Parsing embedded permissions file")
     HANDLER004c = _Reference("INFO", "Parsed embedded permissions file")
     HANDLER004d = _Reference("INFO", "Using v2 permissions model")
+    HANDLER004e = _Reference("ERROR", "Unable to validate PermissionsPolicy")
     HANDLER005 = _Reference("WARN", "Rejecting request due to missing pointer types")
     HANDLER006 = _Reference("DEBUG", "Attempting to parse request parameters")
     HANDLER007 = _Reference("INFO", "Parsed request parameters")
@@ -88,7 +89,7 @@ class LogReference(Enum):
         "INFO", "Retrieved v2 pointer permissions from lambda layer"
     )
     V2PERMISSIONS013 = _Reference(
-        "WARN", "No v2 permissions file found in lambda layer"
+        "INFO", "No v2 permissions file found in lambda layer"
     )
 
     # Parse Logs

layer/nrlf/core/request.py

@@ -11,12 +11,10 @@
 from nrlf.core.model import ClientRpDetails, ConnectionMetadata
 
 
-def _fetch_ods_app_id_headers(headers: dict[str, str]):
-
+def _fetch_v2_ods_app_id_headers(headers: dict[str, str]):
     case_insensitive_headers = {key.lower(): value for key, value in headers.items()}
 
     ods_code = case_insensitive_headers.get(V2Headers.NHSD_END_USER_ORGANISATION_ODS)
-
     if not ods_code or len(ods_code.strip()) == 0:
         logger.log(
             LogReference.HANDLER003a,
@@ -33,9 +31,7 @@ def _fetch_ods_app_id_headers(headers: dict[str, str]):
     return ods_code, nrl_app_id
 
 
-def parse_headers(
-    headers: Dict[str, str], use_v2_permissions=False
-) -> ConnectionMetadata:
+def parse_headers(headers: Dict[str, str]) -> ConnectionMetadata:
     """
     Parses the connection metadata and client rp details from the headers passed from Apigee
     """
@@ -49,8 +45,8 @@ def parse_headers(
             case_insensitive_headers.get(CONNECTION_METADATA, "{}")
         )
 
-        if use_v2_permissions:
-            ods_code, nrl_app_id = _fetch_ods_app_id_headers(case_insensitive_headers)
+        ods_code, nrl_app_id = _fetch_v2_ods_app_id_headers(case_insensitive_headers)
+        if ods_code and nrl_app_id:
             raw_connection_metadata["nrl.ods-code"] = ods_code
             raw_connection_metadata["nrl.app-id"] = nrl_app_id
             raw_client_rp_details["developer.app.id"] = nrl_app_id

layer/nrlf/core/tests/test_authoriser.py

@@ -1,5 +1,7 @@
 from unittest.mock import mock_open, patch
 
+import pytest
+
 from nrlf.core.authoriser import get_pointer_permissions_v2, parse_permissions_file
 from nrlf.core.logger import LogReference, logger
 from nrlf.core.request import parse_headers
@@ -76,17 +78,16 @@ def test_authoriser_get_v2_permissions_with_app_pointer_types(
     spy.assert_called_with(LogReference.V2PERMISSIONS011, key=expected_lookup_key)
 
 
-def test_authoriser_parse_v2_permission_file_with_no_permission_file(mocker):
-    spy = mocker.spy(logger, "log")
-    expected_lookup_key = "consumer/NotAnApp/NotFound.json"
+def test_authoriser_parse_v2_permission_file_with_no_permission_file():
+    with pytest.raises(FileNotFoundError) as error:
+        get_pointer_permissions_v2(
+            connection_metadata=parse_headers(
+                create_headers(ods_code="NotFound", nrl_app_id="NotAnApp")
+            ),
+            request_path="/consumer/_status",
+        )
 
-    metadata_result = get_pointer_permissions_v2(
-        connection_metadata=parse_headers(
-            create_headers(ods_code="NotFound", nrl_app_id="NotAnApp")
-        ),
-        request_path="/consumer/_status",
+    assert (
+        f"No such file or directory: '/opt/python/nrlf_permissions/consumer/NotAnApp/NotFound.json'"
+        in str(error.value)
     )
-
-    assert metadata_result == {}
-
-    spy.assert_any_call(LogReference.V2PERMISSIONS011, key=expected_lookup_key)