diff --git a/layer/nrlf/core/authoriser.py b/layer/nrlf/core/authoriser.py index f6b95b970..c8a21bddf 100644 --- a/layer/nrlf/core/authoriser.py +++ b/layer/nrlf/core/authoriser.py @@ -22,8 +22,14 @@ def get_pointer_permissions_v2( ods_code = connection_metadata.ods_code app_id = connection_metadata.nrl_app_id - key = f"{producer_or_consumer}/{app_id}/{ods_code}.json" - logger.log(LogReference.V2PERMISSIONS011, key=key) + # 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: + key = f"{producer_or_consumer}/{app_id}/{ods_code}.json" + logger.log(LogReference.V2PERMISSIONS011, key=key) file_path = f"/opt/python/nrlf_permissions/{key}" diff --git a/layer/nrlf/core/tests/test_authoriser.py b/layer/nrlf/core/tests/test_authoriser.py index 308157c15..a6673836c 100644 --- a/layer/nrlf/core/tests/test_authoriser.py +++ b/layer/nrlf/core/tests/test_authoriser.py @@ -27,7 +27,7 @@ def test_authoriser_parse_permission_file_with_permission_file(): new_callable=mock_open, read_data='{"types": ["http://snomed.info/sct|736253001"]}', ) -def test_authoriser_get_v2_permissions_with_pointer_types(mock_file, mocker): +def test_authoriser_get_v2_permissions_with_org_pointer_types(mock_file, mocker): spy = mocker.spy(logger, "log") expected_lookup_key = "producer/ODS123-app-id/ODS123.json" @@ -47,6 +47,35 @@ def test_authoriser_get_v2_permissions_with_pointer_types(mock_file, mocker): spy.assert_called_with(LogReference.V2PERMISSIONS011, key=expected_lookup_key) +@patch( + "builtins.open", + new_callable=mock_open, + read_data='{"types": ["http://snomed.info/sct|736253001"]}', +) +@patch("os.path.isfile") +def test_authoriser_get_v2_permissions_with_app_pointer_types( + mock_isfile, mock_file, mocker +): + spy = mocker.spy(logger, "log") + mock_isfile.return_value = True + + expected_lookup_key = "producer/ODS123-app-id.json" + connection_metadata = parse_headers( + create_headers(ods_code="ODS123", nrl_app_id="ODS123-app-id") + ) + result = get_pointer_permissions_v2( + connection_metadata=connection_metadata, + request_path="/producer/DocumentReference/_search", + ) + + mock_file.assert_called_once_with( + f"/opt/python/nrlf_permissions/{expected_lookup_key}" + ) + assert result.get("types") == ["http://snomed.info/sct|736253001"] + + 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" diff --git a/tests/features/consumer/searchPostDocumentReference-success.feature b/tests/features/consumer/searchPostDocumentReference-success.feature index c8fcecaa5..3c9b40451 100644 --- a/tests/features/consumer/searchPostDocumentReference-success.feature +++ b/tests/features/consumer/searchPostDocumentReference-success.feature @@ -232,7 +232,7 @@ Feature: Consumer - searchDocumentReference - Success Scenarios | url | https://example.org/my-doc.pdf | | custodian | DK94 | | author | N64 | - When producer 'DK94' requests to delete DocumentReference with id 'DK94-111-DeleteDocRefTest1' + When producer v1 'DK94' requests to delete DocumentReference with id 'DK94-111-DeleteDocRefTest1' And consumer 'RX898' searches for DocumentReferences using POST with request body: | key | value | | subject | 9278693472 | diff --git a/tests/features/producer/createDocumentReference-failure.feature b/tests/features/producer/createDocumentReference-failure.feature index ff1d2bfda..a3ee056bb 100644 --- a/tests/features/producer/createDocumentReference-failure.feature +++ b/tests/features/producer/createDocumentReference-failure.feature @@ -16,7 +16,7 @@ Feature: Producer - createDocumentReference - Failure Scenarios And the organisation 'ANGY1' is authorised to access pointer types: | system | value | | http://snomed.info/sct | 736253002 | - When producer 'ANGY1' creates a DocumentReference with values: + When producer v1 'ANGY1' creates a DocumentReference with values: | property | value | | subject | 9999999999 | | status | current | @@ -53,7 +53,7 @@ Feature: Producer - createDocumentReference - Failure Scenarios And the organisation 'ANGY1' is authorised to access pointer types: | system | value | | http://snomed.info/sct | 736253002 | - When producer 'ANGY1' creates a DocumentReference with values: + When producer v1 'ANGY1' creates a DocumentReference with values: | property | value | | subject | 1234567890 | | status | current | @@ -212,7 +212,7 @@ Feature: Producer - createDocumentReference - Failure Scenarios | url | https://example.org/my-doc.pdf | | custodian | N0TANGY | | author | HAR1 | - When producer 'ANGY1' creates a DocumentReference with values: + When producer v1 'ANGY1' creates a DocumentReference with values: | property | value | | subject | 9278693472 | | status | current | @@ -263,7 +263,7 @@ Feature: Producer - createDocumentReference - Failure Scenarios Given the application 'DataShare' (ID 'z00z-y11y-x22x') is registered to access the API And the organisation 'ANGY1' is authorised to access pointer types: | system | value | - When producer 'ANGY1' creates a DocumentReference with values: + When producer v1 'ANGY1' creates a DocumentReference with values: | property | value | | subject | 9999999999 | | status | current | @@ -292,13 +292,83 @@ Feature: Producer - createDocumentReference - Failure Scenarios } """ + # Credentials - type not allowed at application level (v2 permissions) + Scenario: Producer lacks permissions to create specified type at the application level + Given the application 'DataShare' (ID 'z00z-y11y-x22x') is registered to access the API + And the application has 'producer' permissions for pointer types: + | system | value | + | http://snomed.info/sct | 736253002 | + When producer v2 'ANGY1' creates a DocumentReference with values: + | property | value | + | subject | 9999999999 | + | status | current | + | type | 887701000000100 | + | category | 734163000 | + | custodian | ANGY1 | + | author | HAR1 | + | url | https://example.org/my-doc.pdf | + Then the response status code is 403 + And the response is an OperationOutcome with 1 issue + And the OperationOutcome contains the issue: + """ + { + "severity": "error", + "code": "forbidden", + "details": { + "coding": [ + { + "system": "https://fhir.nhs.uk/CodeSystem/Spine-ErrorOrWarningCode", + "code": "ACCESS DENIED", + "display": "Access has been denied to process this request" + } + ] + }, + "diagnostics": "Your organisation 'ANGY1' does not have permission to access this resource. Contact the onboarding team." + } + """ + + # Credentials - type not allowed at org level (v2 permissions) + Scenario: Producer lacks permissions to create specified type at the org level + Given the application 'DataShare' (ID 'z00z-y11y-x22x') is registered to access the API + And the organisation 'ANGY1' is authorised as a Producer for pointer types: + | system | value | + | http://snomed.info/sct | 736253002 | + When producer v2 'ANGY1' creates a DocumentReference with values: + | property | value | + | subject | 9999999999 | + | status | current | + | type | 887701000000100 | + | category | 734163000 | + | custodian | ANGY1 | + | author | HAR1 | + | url | https://example.org/my-doc.pdf | + Then the response status code is 403 + And the response is an OperationOutcome with 1 issue + And the OperationOutcome contains the issue: + """ + { + "severity": "error", + "code": "forbidden", + "details": { + "coding": [ + { + "system": "https://fhir.nhs.uk/CodeSystem/Spine-ErrorOrWarningCode", + "code": "ACCESS DENIED", + "display": "Access has been denied to process this request" + } + ] + }, + "diagnostics": "Your organisation 'ANGY1' does not have permission to access this resource. Contact the onboarding team." + } + """ + Scenario: Invalid status Given the application 'DataShare' (ID 'z00z-y11y-x22x') is registered to access the API And the organisation 'X26' is authorised to access pointer types: | system | value | | http://snomed.info/sct | 1363501000000100 | | http://snomed.info/sct | 736253002 | - When producer 'X26' creates a DocumentReference with values: + When producer v1 'X26' creates a DocumentReference with values: | property | value | | subject | 9999999999 | | type | 736253002 | @@ -335,7 +405,7 @@ Feature: Producer - createDocumentReference - Failure Scenarios And the organisation 'ANGY1' is authorised to access pointer types: | system | value | | http://snomed.info/sct | 736253002 | - When producer 'ANGY1' creates a DocumentReference with values: + When producer v1 'ANGY1' creates a DocumentReference with values: | property | value | | subject | 9999999999 | | status | current | @@ -373,7 +443,7 @@ Feature: Producer - createDocumentReference - Failure Scenarios | system | value | | http://snomed.info/sct | 1363501000000100 | | http://snomed.info/sct | 736253002 | - When producer 'X26' creates a DocumentReference with values: + When producer v1 'X26' creates a DocumentReference with values: | property | value | | subject | 9999999999 | | status | current | @@ -544,7 +614,7 @@ Feature: Producer - createDocumentReference - Failure Scenarios And the organisation 'ANGY1' is authorised to access pointer types: | system | value | | http://snomed.info/sct | 736253002 | - When producer 'ANGY1' creates a DocumentReference with values: + When producer v1 'ANGY1' creates a DocumentReference with values: | property | value | | subject | 9278693472 | | status | current | @@ -581,7 +651,7 @@ Feature: Producer - createDocumentReference - Failure Scenarios And the organisation 'ANGY1' is authorised to access pointer types: | system | value | | http://snomed.info/sct | 736253002 | - When producer 'ANGY1' creates a DocumentReference with values: + When producer v1 'ANGY1' creates a DocumentReference with values: | property | value | | subject | 9999999999 | | status | current | @@ -620,7 +690,7 @@ Feature: Producer - createDocumentReference - Failure Scenarios | system | value | | https://nicip.nhs.uk | MAULR | | https://nicip.nhs.uk | MAXIB | - When producer 'ANGY1' creates a DocumentReference with values: + When producer v1 'ANGY1' creates a DocumentReference with values: | property | value | | subject | 9999999999 | | status | current | @@ -665,7 +735,7 @@ Feature: Producer - createDocumentReference - Failure Scenarios | system | value | | http://snomed.info/sct | 1363501000000100 | | http://snomed.info/sct | 736253002 | - When producer 'X26' creates a DocumentReference with values: + When producer v1 'X26' creates a DocumentReference with values: | property | value | | subject | 9999999999 | | status | current | @@ -814,7 +884,7 @@ Feature: Producer - createDocumentReference - Failure Scenarios And the organisation 'ANGY1' is authorised to access pointer types: | system | value | | http://snomed.info/sct | 736253002 | - When producer 'ANGY1' creates a DocumentReference with values: + When producer v1 'ANGY1' creates a DocumentReference with values: | property | value | | subject | 9999999999 | | status | current | diff --git a/tests/features/producer/createDocumentReference-success.feature b/tests/features/producer/createDocumentReference-success.feature index e7ea87d8c..589572bf7 100644 --- a/tests/features/producer/createDocumentReference-success.feature +++ b/tests/features/producer/createDocumentReference-success.feature @@ -5,7 +5,101 @@ Feature: Producer - createDocumentReference - Success Scenarios And the organisation 'ANGY1' is authorised to access pointer types: | system | value | | http://snomed.info/sct | 736253002 | - When producer 'ANGY1' creates a DocumentReference with values: + When producer v1 'ANGY1' creates a DocumentReference with values: + | property | value | + | subject | 9278693472 | + | status | current | + | type | 736253002 | + | category | 734163000 | + | custodian | ANGY1 | + | author | HAR1 | + | url | https://example.org/my-doc.pdf | + | practiceSetting | 788002001 | + Then the response status code is 201 + And the response is an OperationOutcome with 1 issue + And the OperationOutcome contains the issue: + """ + { + "severity": "information", + "code": "informational", + "details": { + "coding": [ + { + "system": "https://fhir.nhs.uk/ValueSet/NRL-ResponseCode", + "code": "RESOURCE_CREATED", + "display": "Resource created" + } + ] + }, + "diagnostics": "The document has been created" + } + """ + And the response has a Location header + And the Location header starts with '/DocumentReference/ANGY1-' + And the resource in the Location header exists with values: + | property | value | + | subject | 9278693472 | + | status | current | + | type | 736253002 | + | category | 734163000 | + | custodian | ANGY1 | + | author | HAR1 | + | url | https://example.org/my-doc.pdf | + | practiceSetting | 788002001 | + + Scenario: Successfully create a Document Pointer with org-level permissions (v2 permissions model) + Given the application 'DataShare' (ID 'z00z-y11y-x22x') is registered to access the API + And the organisation 'ANGY1' is authorised as a Producer for pointer types: + | system | value | + | http://snomed.info/sct | 736253002 | + When producer v2 'ANGY1' creates a DocumentReference with values: + | property | value | + | subject | 9278693472 | + | status | current | + | type | 736253002 | + | category | 734163000 | + | custodian | ANGY1 | + | author | HAR1 | + | url | https://example.org/my-doc.pdf | + | practiceSetting | 788002001 | + Then the response status code is 201 + And the response is an OperationOutcome with 1 issue + And the OperationOutcome contains the issue: + """ + { + "severity": "information", + "code": "informational", + "details": { + "coding": [ + { + "system": "https://fhir.nhs.uk/ValueSet/NRL-ResponseCode", + "code": "RESOURCE_CREATED", + "display": "Resource created" + } + ] + }, + "diagnostics": "The document has been created" + } + """ + And the response has a Location header + And the Location header starts with '/DocumentReference/ANGY1-' + And the resource in the Location header exists with values: + | property | value | + | subject | 9278693472 | + | status | current | + | type | 736253002 | + | category | 734163000 | + | custodian | ANGY1 | + | author | HAR1 | + | url | https://example.org/my-doc.pdf | + | practiceSetting | 788002001 | + + Scenario: Successfully create a Document Pointer with app-level permissions (v2 permissions model) + Given the application 'DataShare' (ID 'z00z-y11y-x22x') is registered to access the API + And the application has 'producer' permissions for pointer types: + | system | value | + | http://snomed.info/sct | 736253002 | + When producer v2 'ANGY1' creates a DocumentReference with values: | property | value | | subject | 9278693472 | | status | current | @@ -110,7 +204,7 @@ Feature: Producer - createDocumentReference - Success Scenarios | custodian | ANGY1 | | author | HAR1 | | practiceSetting | 788002001 | - When producer 'ANGY1' creates a DocumentReference with values: + When producer v1 'ANGY1' creates a DocumentReference with values: | property | value | | subject | 9278693472 | | status | current | @@ -161,7 +255,7 @@ Feature: Producer - createDocumentReference - Success Scenarios And the organisation 'ANGY1' is authorised to access pointer types: | system | value | | http://snomed.info/sct | | - When producer 'ANGY1' creates a DocumentReference with values: + When producer v1 'ANGY1' creates a DocumentReference with values: | property | value | | subject | 9278693472 | | status | current | @@ -239,7 +333,7 @@ Feature: Producer - createDocumentReference - Success Scenarios | system | value | | https://nicip.nhs.uk | MAULR | | https://nicip.nhs.uk | MAXIB | - When producer 'ANGY1' creates a DocumentReference with values: + When producer v1 'ANGY1' creates a DocumentReference with values: | property | value | | subject | 9278693472 | | status | current | @@ -282,7 +376,7 @@ Feature: Producer - createDocumentReference - Success Scenarios | author | HAR1 | | url | https://example.org/my-doc.pdf | | practiceSetting | 788002001 | - When producer 'ANGY1' creates a DocumentReference with values: + When producer v1 'ANGY1' creates a DocumentReference with values: | property | value | | subject | 9278693472 | | status | current | @@ -330,7 +424,7 @@ Feature: Producer - createDocumentReference - Success Scenarios And the organisation 'BARS1' is authorised to access pointer types: | system | value | | http://snomed.info/sct | 749001000000101 | - When producer 'BARS1' creates a DocumentReference with values: + When producer v1 'BARS1' creates a DocumentReference with values: | property | value | | subject | 9278693472 | | status | current | diff --git a/tests/features/steps/1_setup.py b/tests/features/steps/1_setup.py index 2a0a51e3b..7463ebc0e 100644 --- a/tests/features/steps/1_setup.py +++ b/tests/features/steps/1_setup.py @@ -1,4 +1,5 @@ import json +from typing import Optional from behave import * # noqa from behave.runner import Context @@ -25,6 +26,24 @@ def add_pointer_types(self, ods_code: str, context: Context): s3_client.put_object(Bucket=bucket, Key=key, Body=json.dumps(pointer_types)) context.add_cleanup(lambda: s3_client.delete_object(Bucket=bucket, Key=key)) + def add_v2_permissions( + self, api_side: str, context: Context, ods_code: Optional[str] = None + ): + if not context.table: + raise ValueError("No permissions table provided") + + pointer_types = [f"{system}|{value}" for system, value in context.table] + perms_body = {"types": pointer_types} + bucket = f"nhsd-nrlf--{context.stack_name}-authorization-store" + if ods_code: # org-level permissions + key = f"{api_side}/{self.app_id}/{ods_code}.json" + else: # app-level permissions + key = f"{api_side}/{self.app_id}.json" + + s3_client = get_s3_client() + s3_client.put_object(Bucket=bucket, Key=key, Body=json.dumps(pointer_types)) + context.add_cleanup(lambda: s3_client.delete_object(Bucket=bucket, Key=key)) + @given("the application '{app_name}' (ID '{app_id}') is registered to access the API") def register_application_step(context: Context, app_name: str, app_id: str): @@ -39,6 +58,31 @@ def register_org_permissions_step(context: Context, ods_code: str): context.application.add_pointer_types(ods_code, context) +@given("the organisation '{ods_code}' is authorised as a Producer for pointer types") +def register_v2_producer_org_permissions_step(context: Context, ods_code: str): + if not context.table: + raise ValueError("No permissions table provided") + + context.application.add_v2_permissions("producer", context, ods_code) + + +@given("the organisation '{ods_code}' is authorised as a Consumer for pointer types") +def register_v2_consumer_org_permissions_step(context: Context, ods_code: str): + if not context.table: + raise ValueError("No permissions table provided") + + context.application.add_v2_permissions("consumer", context, ods_code) + + +@given("the application has '{use_type}' permissions for pointer types") +def register_app_level_permissions_step(context: Context, use_type: str): + if not context.table: + raise ValueError("No permissions table provided") + if use_type not in {"producer", "consumer"}: + raise ValueError("Producer or Consumer side must be specified") + context.application.add_v2_permissions(use_type, context) + + @given("a DocumentReference resource exists with values") def create_document_reference_step(context: Context): if not context.table: diff --git a/tests/features/steps/2_request.py b/tests/features/steps/2_request.py index 304228fec..3f461992e 100644 --- a/tests/features/steps/2_request.py +++ b/tests/features/steps/2_request.py @@ -76,9 +76,9 @@ def consumer_read_document_reference_step( context.response = client.read(doc_ref_id) -@when("producer '{ods_code}' creates a DocumentReference with values") -def create_post_document_reference_step(context: Context, ods_code: str): - client = producer_client_from_context(context, ods_code) +@when("producer {version} '{ods_code}' creates a DocumentReference with values") +def create_post_document_reference_step(context: Context, version: str, ods_code: str): + client = producer_client_from_context(context, ods_code, v2=(version == "v2")) if not context.table: raise ValueError("No document reference data table provided") diff --git a/tests/features/utils/api_client.py b/tests/features/utils/api_client.py index 4c8f8a8cc..3b00c5938 100644 --- a/tests/features/utils/api_client.py +++ b/tests/features/utils/api_client.py @@ -33,11 +33,11 @@ def _config_from_context(context: Context, ods_code: str): ) -def consumer_client_from_context(context: Context, ods_code: str): +def consumer_client_from_context(context: Context, ods_code: str, v2: bool = False): client_config = _config_from_context(context, ods_code) - return ConsumerTestClient(config=client_config) + return ConsumerTestClient(config=client_config, use_v2=v2) -def producer_client_from_context(context: Context, ods_code: str): +def producer_client_from_context(context: Context, ods_code: str, v2: bool = False): client_config = _config_from_context(context, ods_code) - return ProducerTestClient(config=client_config) + return ProducerTestClient(config=client_config, use_v2=v2) diff --git a/tests/utilities/api_clients.py b/tests/utilities/api_clients.py index bb522cbe3..85cf41d36 100644 --- a/tests/utilities/api_clients.py +++ b/tests/utilities/api_clients.py @@ -68,7 +68,7 @@ def wrapper(*args: Any, **kwargs: Any) -> Any: class ConsumerTestClient: - def __init__(self, config: ClientConfig): + def __init__(self, config: ClientConfig, use_v2: bool = False): self.config = config self.api_url = f"{self.config.base_url}consumer{self.config.api_path}" @@ -78,17 +78,26 @@ def __init__(self, config: ClientConfig): } if self.config.client_cert: - connection_metadata = self.config.connection_metadata.model_dump( - by_alias=True - ) - client_rp_details = connection_metadata.pop("client_rp_details") - self.request_headers.update( - { - "NHSD-Connection-Metadata": json.dumps(connection_metadata), - "NHSD-Client-RP-Details": json.dumps(client_rp_details), - "NHSD-Correlation-Id": "test-correlation-id", - } - ) + if use_v2: + self.request_headers.update( + { + "NHSD-End-User-Organisation-ODS": self.config.connection_metadata.ods_code, + "NHSD-NRL-App-Id": self.config.connection_metadata.nrl_app_id, + "NHSD-Correlation-Id": "test-correlation-id", + } + ) + else: + connection_metadata = self.config.connection_metadata.model_dump( + by_alias=True + ) + client_rp_details = connection_metadata.pop("client_rp_details") + self.request_headers.update( + { + "NHSD-Connection-Metadata": json.dumps(connection_metadata), + "NHSD-Client-RP-Details": json.dumps(client_rp_details), + "NHSD-Correlation-Id": "test-correlation-id", + } + ) self.request_headers.update(self.config.custom_headers) @@ -210,7 +219,7 @@ def read_capability_statement(self) -> Response: class ProducerTestClient: - def __init__(self, config: ClientConfig): + def __init__(self, config: ClientConfig, use_v2: bool = False): self.config = config self.api_url = f"{self.config.base_url}producer{self.config.api_path}" @@ -220,17 +229,26 @@ def __init__(self, config: ClientConfig): } if self.config.client_cert: - connection_metadata = self.config.connection_metadata.model_dump( - by_alias=True - ) - client_rp_details = connection_metadata.pop("client_rp_details") - self.request_headers.update( - { - "NHSD-Connection-Metadata": json.dumps(connection_metadata), - "NHSD-Client-RP-Details": json.dumps(client_rp_details), - "NHSD-Correlation-Id": "test-correlation-id", - } - ) + if use_v2: + self.request_headers.update( + { + "NHSD-End-User-Organisation-ODS": self.config.connection_metadata.ods_code, + "NHSD-NRL-App-Id": self.config.connection_metadata.nrl_app_id, + "NHSD-Correlation-Id": "test-correlation-id", + } + ) + else: + connection_metadata = self.config.connection_metadata.model_dump( + by_alias=True + ) + client_rp_details = connection_metadata.pop("client_rp_details") + self.request_headers.update( + { + "NHSD-Connection-Metadata": json.dumps(connection_metadata), + "NHSD-Client-RP-Details": json.dumps(client_rp_details), + "NHSD-Correlation-Id": "test-correlation-id", + } + ) self.request_headers.update(self.config.custom_headers)