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

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/decorators.py

@@ -19,14 +19,13 @@
 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,
     X_REQUEST_ID_HEADER,
     AccessControls,
     PointerTypes,
+    V2Headers,
 )
 from nrlf.core.dynamodb.repository import DocumentPointerRepository
 from nrlf.core.errors import OperationOutcomeError, ParseError
@@ -145,13 +144,20 @@ def wrapper(*args, **kwargs) -> Dict[str, Any]:
 RepositoryType = Union[Type[DocumentPointerRepository], None]
 
 
-def _use_v2_permissions_model(headers: Dict[str, str]) -> bool:
+def _use_v2_permissions_model(headers: Dict[str, str], path: 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()
+
+    v2_headers_provided = (
+        V2Headers.NHSD_END_USER_ORGANISATION_ODS in case_insensitive_headers.keys()
+        and V2Headers.NHSD_NRL_APP_ID in case_insensitive_headers.keys()
     )
+    if not v2_headers_provided:
+        return False
+
+    metadata = parse_headers(headers, use_v2_permissions=True)
+    v2_permissions_configured = get_pointer_permissions_v2(metadata, path) != {}
+
+    return v2_permissions_configured
 
 
 def _load_v2_connection_metadata(headers: Dict[str, str], path: str):
@@ -189,7 +195,7 @@ 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):
+    if _use_v2_permissions_model(headers, path):
         return _load_v2_connection_metadata(headers, path)
 
     metadata = parse_headers(headers, use_v2_permissions=False)

layer/nrlf/core/tests/test_decorators.py

@@ -807,45 +807,138 @@ def test_request_load_connection_metadata_with_no_permission_lookup_or_file():
     assert expected_metadata.pointer_types == []
 
 
-missing_headers = [
-    ["nhsd-connection-metadata"],
-    ["nhsd-connection-metadata", "nhsd-client-rp-details"],
-    ["nhsd-client-rp-details"],
-]
+def _create_v2_headers() -> dict:
+    """Create headers that trigger the v2 permissions model (missing nhsd-client-rp-details)."""
+    headers = create_headers(
+        additional_headers={
+            V2Headers.NHSD_END_USER_ORGANISATION_ODS: "Y05868",
+            V2Headers.NHSD_NRL_APP_ID: "Y05868-TestApp-12345678",
+        }
+    )
+    headers.pop("nhsd-client-rp-details")
+    return headers
 
 
-@pytest.mark.parametrize("headers_missing_from_request", missing_headers)
-def test_request_load_connection_with_missing_headers_gets_v2_permissions(
-    headers_missing_from_request,
+def test_load_connection_metadata_v2_happy_path(
+    mocker,
 ):
-    headers = create_headers(
+    mocker.patch(
+        "nrlf.core.decorators.get_pointer_permissions_v2",
+        return_value={
+            "types": [
+                "http://snomed.info/sct|749001000000101",
+                "https://nicip.nhs.uk|MAULR",
+            ]
+        },
+    )
+
+    metadata = load_connection_metadata(
+        headers=_create_v2_headers(),
+        config=Config(),
+        path="/producer/DocumentReference",
+    )
+
+    assert metadata.nrl_permissions_policy.types == [
+        "http://snomed.info/sct|749001000000101",
+        "https://nicip.nhs.uk|MAULR",
+    ]
+    assert metadata.pointer_types == []  # no v1 permissions
+    assert metadata.ods_code == "Y05868"
+    assert metadata.nrl_app_id == "Y05868-TestApp-12345678"
+
+
+def test_load_connection_metadata_gets_v2_permissions_when_v1_headers_also_provided(
+    mocker,
+):
+    v1_permissions = [
+        "http://snomed.info/sct|749001000000101",
+        "https://nicip.nhs.uk|MAULR",
+    ]
+    mocker.patch(
+        "nrlf.core.decorators.parse_permissions_file",
+        return_value=v1_permissions,
+    )
+    v2_permissions = {"access_controls": [AccessControls.ALLOW_ALL_TYPES.value]}
+    mocker.patch(
+        "nrlf.core.decorators.get_pointer_permissions_v2",
+        return_value=v2_permissions,
+    )
+
+    v1_plus_v2_headers = create_headers(
         additional_headers={
             V2Headers.NHSD_END_USER_ORGANISATION_ODS: "Y05868",
             V2Headers.NHSD_NRL_APP_ID: "Y05868-TestApp-12345678",
         }
     )
-    for header_name in headers_missing_from_request:
-        headers.pop(header_name)
 
-    expected_metadata = load_connection_metadata(
-        headers=headers, config=Config(), path="/producer/DocumentReference"
+    metadata = load_connection_metadata(
+        headers=v1_plus_v2_headers, config=Config(), path="/producer/DocumentReference"
     )
 
-    assert expected_metadata.pointer_types == []
-    assert expected_metadata.ods_code == "Y05868"
-    assert expected_metadata.nrl_app_id == "Y05868-TestApp-12345678"
+    assert metadata.nrl_permissions_policy.types == PointerTypes.list()
+    assert metadata.pointer_types == []  # no v1 permissions
 
 
-def _create_v2_headers() -> dict:
-    """Create headers that trigger the v2 permissions model (missing nhsd-client-rp-details)."""
-    headers = create_headers(
+def test_load_connection_metadata_gets_v1_permissions_when_v2_permission_file_missing(
+    mocker,
+):
+    v1_permissions = [
+        "http://snomed.info/sct|749001000000101",
+        "https://nicip.nhs.uk|MAULR",
+    ]
+    mocker.patch(
+        "nrlf.core.decorators.parse_permissions_file",
+        return_value=v1_permissions,
+    )
+    mocker.patch(
+        "nrlf.core.decorators.get_pointer_permissions_v2",
+        return_value={},
+    )
+
+    v1_plus_v2_headers = create_headers(
         additional_headers={
             V2Headers.NHSD_END_USER_ORGANISATION_ODS: "Y05868",
             V2Headers.NHSD_NRL_APP_ID: "Y05868-TestApp-12345678",
         }
     )
-    headers.pop("nhsd-client-rp-details")
-    return headers
+
+    metadata = load_connection_metadata(
+        headers=v1_plus_v2_headers, config=Config(), path="/producer/DocumentReference"
+    )
+
+    assert metadata.pointer_types == v1_permissions
+    assert metadata.nrl_permissions_policy == None  # no v2 permissions
+
+
+def test_load_connection_metadata_gets_v1_permissions_when_v2_headers_missing(
+    mocker,
+):
+    v1_permissions = [
+        "http://snomed.info/sct|749001000000101",
+        "https://nicip.nhs.uk|MAULR",
+    ]
+    mocker.patch(
+        "nrlf.core.decorators.parse_permissions_file",
+        return_value=v1_permissions,
+    )
+    v2_permissions = {"access_controls": [AccessControls.ALLOW_ALL_TYPES.value]}
+    mocker.patch(
+        "nrlf.core.decorators.get_pointer_permissions_v2",
+        return_value=v2_permissions,
+    )
+
+    v1_headers_only = create_headers(
+        additional_headers={
+            V2Headers.NHSD_END_USER_ORGANISATION_ODS: "Y05868",
+        }
+    )
+
+    metadata = load_connection_metadata(
+        headers=v1_headers_only, config=Config(), path="/producer/DocumentReference"
+    )
+
+    assert metadata.pointer_types == v1_permissions
+    assert metadata.nrl_permissions_policy == None  # no v2 permissions
 
 
 def test_load_v2_connection_metadata_allow_all_types(mocker: MockerFixture):

scripts/get_s3_permissions.py

@@ -65,7 +65,7 @@ def add_feature_test_files(local_path):
     org_permissions = {
         "consumer": [
             (
-                "z00z-y11y-x22x",
+                "v2-z00z-y11y-x22x",
                 "RX898",
                 [PointerTypes.MENTAL_HEALTH_PLAN.value],
                 [],
@@ -77,15 +77,15 @@ def add_feature_test_files(local_path):
                 [],
             ),
             (
-                "z00z-y11y-x22x",
+                "v2-z00z-y11y-x22x",
                 "4LLTYP35C",
                 [],
                 [AccessControls.ALLOW_ALL_TYPES.value],
             ),
         ],
         "producer": [
             (
-                "z00z-y11y-x22x",
+                "v2-z00z-y11y-x22x",
                 "RX898",
                 [PointerTypes.EOL_CARE_PLAN.value],
                 [],
@@ -97,7 +97,7 @@ def add_feature_test_files(local_path):
                 [],
             ),
             (
-                "z00z-y11y-x22x",
+                "v2-z00z-y11y-x22x",
                 "4LLTYP35P",
                 [],
                 [

tests/features/consumer/readDocumentReference-failure.feature

@@ -6,7 +6,7 @@ Feature: Consumer - readDocumentReference - Failure Scenarios
     And the organisation 'RX898' is authorised to access pointer types:
       | system                 | value     |
       | http://snomed.info/sct | 736253002 |
-    When consumer v1 'RX898' reads a DocumentReference with ID 'X26-000000000-000000000'
+    When consumer 'RX898' reads a DocumentReference with ID 'X26-000000000-000000000'
     Then the response status code is 404
     And the response is an OperationOutcome with 1 issue
     And the OperationOutcome contains the issue:
@@ -33,7 +33,7 @@ Feature: Consumer - readDocumentReference - Failure Scenarios
     And the organisation 'RX898' is authorised to access pointer types:
       | system                 | value     |
       | http://snomed.info/sct | 736253002 |
-    When consumer v1 'RX898' reads a DocumentReference with ID 'X26`DROP TABLE 'pointers';--Something-000000000-000000000'
+    When consumer 'RX898' reads a DocumentReference with ID 'X26`DROP TABLE 'pointers';--Something-000000000-000000000'
     Then the response status code is 404
     And the response is an OperationOutcome with 1 issue
     And the OperationOutcome contains the issue:
@@ -58,7 +58,7 @@ Feature: Consumer - readDocumentReference - Failure Scenarios
     Given the application 'DataShare' (ID 'z00z-y11y-x22x') is registered to access the API
     And the organisation 'RX898' is authorised to access pointer types:
       | system | value |
-    When consumer v1 'RX898' reads a DocumentReference with ID 'X26-000000000-000000000'
+    When consumer 'RX898' reads a DocumentReference with ID 'X26-000000000-000000000'
     Then the response status code is 403
     And the response is an OperationOutcome with 1 issue
     And the OperationOutcome contains the issue:
@@ -95,7 +95,7 @@ Feature: Consumer - readDocumentReference - Failure Scenarios
       | url         | https://example.org/my-doc.pdf         |
       | custodian   | 02V                                    |
       | author      | 02V                                    |
-    When consumer v1 'RX898' reads a DocumentReference with ID '02V-1111111111-ReadDocRefNoAuthForType'
+    When consumer 'RX898' reads a DocumentReference with ID '02V-1111111111-ReadDocRefNoAuthForType'
     Then the response status code is 403
     And the response is an OperationOutcome with 1 issue
     And the OperationOutcome contains the issue:
@@ -120,7 +120,7 @@ Feature: Consumer - readDocumentReference - Failure Scenarios
     Given the application 'DataShare' (ID 'z00z-y11y-x22x') is registered to access the API
     And the organisation 'RX898' is authorised to access pointer types:
       | system | value |
-    When consumer v1 'RX898' reads a DocumentReference with ID 'X26-000000000-000000000'
+    When consumer 'RX898' reads a DocumentReference with ID 'X26-000000000-000000000'
     Then the response status code is 403
     And the response is an OperationOutcome with 1 issue
     And the OperationOutcome contains the issue:
@@ -157,7 +157,7 @@ Feature: Consumer - readDocumentReference - Failure Scenarios
       | url         | https://example.org/my-doc.pdf           |
       | custodian   | 02V                                      |
       | author      | 02V                                      |
-    When consumer v1 'RX898' reads a DocumentReference with ID '02V-1111111111-ReadDocRefNoAuthForTypeS3'
+    When consumer 'RX898' reads a DocumentReference with ID '02V-1111111111-ReadDocRefNoAuthForTypeS3'
     Then the response status code is 403
     And the response is an OperationOutcome with 1 issue
     And the OperationOutcome contains the issue:

tests/features/consumer/readDocumentReference-success.feature

@@ -16,7 +16,7 @@ Feature: Consumer - readDocumentReference - Success Scenarios
       | url         | https://example.org/my-doc.pdf           |
       | custodian   | RX898                                    |
       | author      | RX898                                    |
-    When consumer v1 'RX898' reads a DocumentReference with ID 'RX898-9999999999-ReadDocRefSameCustodian'
+    When consumer 'RX898' reads a DocumentReference with ID 'RX898-9999999999-ReadDocRefSameCustodian'
     Then the response status code is 200
     And the response is a DocumentReference with JSON value:
       """
@@ -133,7 +133,7 @@ Feature: Consumer - readDocumentReference - Success Scenarios
       | url         | https://example.org/my-doc.pdf         |
       | custodian   | X26                                    |
       | author      | RX898                                  |
-    When consumer v1 'RX898' reads a DocumentReference with ID 'X26-9999999999-ReadDocRefDiffCustodian'
+    When consumer 'RX898' reads a DocumentReference with ID 'X26-9999999999-ReadDocRefDiffCustodian'
     Then the response status code is 200
     And the response is a DocumentReference with JSON value:
       """
@@ -250,5 +250,5 @@ Feature: Consumer - readDocumentReference - Success Scenarios
       | url         | https://example.org/my-doc.pdf             |
       | custodian   | RX898\|001                                 |
       | author      | RX898                                      |
-    When consumer v1 'RX898' reads a DocumentReference with ID 'RX898%7C001-1234567890-ReadDocRefUrlEncoded'
+    When consumer 'RX898' reads a DocumentReference with ID 'RX898%7C001-1234567890-ReadDocRefUrlEncoded'
     Then the response status code is 200