From f47ad6711457b30b2e24d27e47ef111d24881c7b Mon Sep 17 00:00:00 2001 From: Kate Bobyn Date: Thu, 12 Mar 2026 12:42:50 +0000 Subject: [PATCH 1/8] NRL-1933 use app level permissions if defined --- layer/nrlf/core/authoriser.py | 11 +- layer/nrlf/core/tests/test_authoriser.py | 31 +++- scripts/get_s3_permissions.py | 39 ++++- .../producer/v2-permissions-app-level | 152 ++++++++++++++++++ 4 files changed, 227 insertions(+), 6 deletions(-) create mode 100644 tests/features/producer/v2-permissions-app-level diff --git a/layer/nrlf/core/authoriser.py b/layer/nrlf/core/authoriser.py index f6b95b970..271f74a00 100644 --- a/layer/nrlf/core/authoriser.py +++ b/layer/nrlf/core/authoriser.py @@ -22,9 +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: # use org level + key = f"{producer_or_consumer}/{app_id}/{ods_code}.json" + logger.log(LogReference.V2PERMISSIONS011, key=key) file_path = f"/opt/python/nrlf_permissions/{key}" pointer_permissions = {} 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/scripts/get_s3_permissions.py b/scripts/get_s3_permissions.py index bbd08a66b..3baec50d1 100644 --- a/scripts/get_s3_permissions.py +++ b/scripts/get_s3_permissions.py @@ -62,13 +62,14 @@ def add_feature_test_files(local_path): """ print("Adding feature test v2 permissions to temporary directory...") - permissions = { + org_permissions = { "consumer": [ ( "z00z-y11y-x22x", "RX898", [PointerTypes.MENTAL_HEALTH_PLAN.value], ), # http://snomed.info/sct|736253002 + ("app-t004", "ODS1", [PointerTypes.PERSONALISED_CARE_AND_SUPPORT_PLAN]), ], "producer": [ ( @@ -76,15 +77,49 @@ def add_feature_test_files(local_path): "RX898", [PointerTypes.EOL_CARE_PLAN.value], ), # http://snomed.info/sct|736373009 + ("app-t004", "ODS1", [PointerTypes.PERSONALISED_CARE_AND_SUPPORT_PLAN]), ], } [ _write_permission_file( Path.joinpath(local_path, actor_type, app_id), ods_code, pointer_types ) - for actor_type, entries in permissions.items() + for actor_type, entries in org_permissions.items() for app_id, ods_code, pointer_types in entries ] + app_permissions = { + "consumer": [ + ("app-t001", [PointerTypes.MENTAL_HEALTH_PLAN]), + ( + "app-t002", + [ + PointerTypes.ADVANCE_CARE_PLAN, + PointerTypes.EMERGENCY_HEALTHCARE_PLAN, + PointerTypes.NEWS2_CHART, + ], + ), + ("app-t004", [PointerTypes.APPOINTMENT]), + ], + "producer": [ + ("app-t001", [PointerTypes.EOL_COORDINATION_SUMMARY]), + ( + "app-t003", + [ + PointerTypes.ADVANCE_CARE_PLAN, + PointerTypes.EMERGENCY_HEALTHCARE_PLAN, + PointerTypes.NEWS2_CHART, + ], + ), + ("app-t004", [PointerTypes.APPOINTMENT]), + ], + } + [ + _write_permission_file( + Path.joinpath(local_path, actor_type), app_id, pointer_types + ) + for actor_type, entries in app_permissions.items() + for app_id, pointer_types in entries + ] def download_files(s3_client, bucket_name, local_path, file_names, folders): diff --git a/tests/features/producer/v2-permissions-app-level b/tests/features/producer/v2-permissions-app-level new file mode 100644 index 000000000..cdf25939f --- /dev/null +++ b/tests/features/producer/v2-permissions-app-level @@ -0,0 +1,152 @@ +Feature: Producer v2 permissions by pointer type - Success and Failure Scenarios + For the v2 permissions model, permissions are resolved from a JSON file stored in the + nrlf_permissions Lambda layer. Permissions for the feature tests are baked into the layer by + `scripts/get_s3_permissions.py` at build time, so no dynamic seeding step is required for + success scenarios. + + + Scenario: HAPPY PATH V2 Permissions with access for pointer type - createDocumentReference + Given the application 'ProducerTest001' (ID 'app-t001') is registered to access the API + When producer v2 'ORGA' creates a DocumentReference with values: + | property | value | + | subject | 9278693472 | + | status | current | + | type | 861421000000109 | + | category | 734163000 | + | custodian | ORGA | + | 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/RX898-' + And the resource in the Location header exists with values: + | property | value | + | subject | 9278693472 | + | status | current | + | type | 861421000000109 | + | category | 734163000 | + | custodian | ORGA | + | author | HAR1 | + | url | https://example.org/my-doc.pdf | + | practiceSetting | 788002001 | + + Scenario: V2 Permissions with no producer access at all (but app level consumer access for specified type) + Given the application 'ProducerTest002' (ID 'app-t002') is registered to access the API + When producer v2 'ORGA' creates a DocumentReference with values: + | property | value | + | subject | 9278693472 | + | status | current | + | type | 736366004 | + | category | 734163000 | + | custodian | ORGA | + | author | HAR1 | + | url | https://example.org/my-doc.pdf | + | practiceSetting | 788002001 | + 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 'ORGA' does not have permission to access this resource. Contact the onboarding team." + } + """ + + Scenario: V2 Permissions with no access to specified type + Given the application 'ProducerTest003' (ID 'app-t003') is registered to access the API + When producer v2 'ORGA' creates a DocumentReference with values: + | property | value | + | subject | 9278693472 | + | status | current | + | type | 736366004 | + | category | 734163000 | + | custodian | ORGA | + | author | HAR1 | + | url | https://example.org/my-doc.pdf | + | practiceSetting | 788002001 | + 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": "AUTHOR_CREDENTIALS_ERROR", + "display": "Author credentials error" + } + ] + }, + "diagnostics": "The type of the provided DocumentReference is not in the list of allowed types for this organisation", + "expression": [ + "type.coding[0].code" + ] + } + """ + + Scenario: V2 Permissions with org-level permissions for requested type but app level permissions for other types + Given the application 'ProducerTest004' (ID 'app-t004') is registered to access the API + When producer v2 'ORGA' creates a DocumentReference with values: + | property | value | + | subject | 9278693472 | + | status | current | + | type | 749001000000101 | + | category | 734163000 | + | custodian | ODS1 | + | author | HAR1 | + | url | https://example.org/my-doc.pdf | + | practiceSetting | 788002001 | + 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": "AUTHOR_CREDENTIALS_ERROR", + "display": "Author credentials error" + } + ] + }, + "diagnostics": "The type of the provided DocumentReference is not in the list of allowed types for this organisation", + "expression": [ + "type.coding[0].code" + ] + } + """ From 43f54641c7c169c8cc2cd0202bc67e996bbe9c6e Mon Sep 17 00:00:00 2001 From: Kate Bobyn Date: Thu, 12 Mar 2026 12:51:16 +0000 Subject: [PATCH 2/8] NRL-1933 fix merge conflict --- scripts/get_s3_permissions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/get_s3_permissions.py b/scripts/get_s3_permissions.py index d5996d139..b8ab36d2b 100644 --- a/scripts/get_s3_permissions.py +++ b/scripts/get_s3_permissions.py @@ -101,7 +101,7 @@ def add_feature_test_files(local_path): pointer_types, access_controls, ) - for actor_type, entries in permissions.items() + for actor_type, entries in org_permissions.items() for app_id, ods_code, pointer_types, access_controls in entries ] app_permissions = { From f7381a89631c5998f06b590a59af166701792094 Mon Sep 17 00:00:00 2001 From: Kate Bobyn Date: Thu, 12 Mar 2026 13:05:10 +0000 Subject: [PATCH 3/8] NRL-1933 fix merge conflicts --- scripts/get_s3_permissions.py | 12 +++++++----- tests/features/producer/v2-permissions-app-level | 4 ++-- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/scripts/get_s3_permissions.py b/scripts/get_s3_permissions.py index b8ab36d2b..b01b020dc 100644 --- a/scripts/get_s3_permissions.py +++ b/scripts/get_s3_permissions.py @@ -85,7 +85,7 @@ def add_feature_test_files(local_path): [PointerTypes.EOL_CARE_PLAN.value], [], ), # http://snomed.info/sct|736373009 - ("app-t004", "ODS1", [PointerTypes.PERSONALISED_CARE_AND_SUPPORT_PLAN]), + ("app-t004", "ODS1", [PointerTypes.PERSONALISED_CARE_AND_SUPPORT_PLAN], []), ( "z00z-y11y-x22x", "4LLTYP35P", @@ -106,7 +106,7 @@ def add_feature_test_files(local_path): ] app_permissions = { "consumer": [ - ("app-t001", [PointerTypes.MENTAL_HEALTH_PLAN]), + ("app-t001", [PointerTypes.MENTAL_HEALTH_PLAN], []), ( "app-t002", [ @@ -114,8 +114,9 @@ def add_feature_test_files(local_path): PointerTypes.EMERGENCY_HEALTHCARE_PLAN, PointerTypes.NEWS2_CHART, ], + [], ), - ("app-t004", [PointerTypes.APPOINTMENT]), + ("app-t004", [PointerTypes.APPOINTMENT], []), ], "producer": [ ("app-t001", [PointerTypes.EOL_COORDINATION_SUMMARY]), @@ -126,8 +127,9 @@ def add_feature_test_files(local_path): PointerTypes.EMERGENCY_HEALTHCARE_PLAN, PointerTypes.NEWS2_CHART, ], + [], ), - ("app-t004", [PointerTypes.APPOINTMENT]), + ("app-t004", [PointerTypes.APPOINTMENT], []), ], } [ @@ -135,7 +137,7 @@ def add_feature_test_files(local_path): Path.joinpath(local_path, actor_type), app_id, pointer_types ) for actor_type, entries in app_permissions.items() - for app_id, pointer_types in entries + for app_id, pointer_types, access_controls in entries ] diff --git a/tests/features/producer/v2-permissions-app-level b/tests/features/producer/v2-permissions-app-level index cdf25939f..0e4cd07ad 100644 --- a/tests/features/producer/v2-permissions-app-level +++ b/tests/features/producer/v2-permissions-app-level @@ -118,11 +118,11 @@ Feature: Producer v2 permissions by pointer type - Success and Failure Scenarios Scenario: V2 Permissions with org-level permissions for requested type but app level permissions for other types Given the application 'ProducerTest004' (ID 'app-t004') is registered to access the API - When producer v2 'ORGA' creates a DocumentReference with values: + When producer v2 'ODS1' creates a DocumentReference with values: | property | value | | subject | 9278693472 | | status | current | - | type | 749001000000101 | + | type | 2181441000000107 | | category | 734163000 | | custodian | ODS1 | | author | HAR1 | From 00475f4774725e0032e27ea8c3a0ea45d3d6c9b0 Mon Sep 17 00:00:00 2001 From: Kate Bobyn Date: Thu, 12 Mar 2026 13:14:13 +0000 Subject: [PATCH 4/8] NRL-1933 fix merge conflicts --- scripts/get_s3_permissions.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/scripts/get_s3_permissions.py b/scripts/get_s3_permissions.py index b01b020dc..33889e245 100644 --- a/scripts/get_s3_permissions.py +++ b/scripts/get_s3_permissions.py @@ -70,7 +70,7 @@ def add_feature_test_files(local_path): [PointerTypes.MENTAL_HEALTH_PLAN.value], [], ), # http://snomed.info/sct|736253002 - ("app-t004", "ODS1", [PointerTypes.PERSONALISED_CARE_AND_SUPPORT_PLAN]), + ("app-t004", "ODS1", [PointerTypes.PERSONALISED_CARE_AND_SUPPORT_PLAN], []), ( "z00z-y11y-x22x", "4LLTYP35C", @@ -134,7 +134,10 @@ def add_feature_test_files(local_path): } [ _write_permission_file( - Path.joinpath(local_path, actor_type), app_id, pointer_types + Path.joinpath(local_path, actor_type), + app_id, + pointer_types, + access_controls, ) for actor_type, entries in app_permissions.items() for app_id, pointer_types, access_controls in entries From 593e3770d526aaf5ca51444129f6f4db2573e986 Mon Sep 17 00:00:00 2001 From: Kate Bobyn Date: Thu, 12 Mar 2026 13:29:23 +0000 Subject: [PATCH 5/8] NRL-1933 fix pointer type values for permissions test seed script --- scripts/get_s3_permissions.py | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/scripts/get_s3_permissions.py b/scripts/get_s3_permissions.py index 33889e245..0c4c6e472 100644 --- a/scripts/get_s3_permissions.py +++ b/scripts/get_s3_permissions.py @@ -70,7 +70,12 @@ def add_feature_test_files(local_path): [PointerTypes.MENTAL_HEALTH_PLAN.value], [], ), # http://snomed.info/sct|736253002 - ("app-t004", "ODS1", [PointerTypes.PERSONALISED_CARE_AND_SUPPORT_PLAN], []), + ( + "app-t004", + "ODS1", + [PointerTypes.PERSONALISED_CARE_AND_SUPPORT_PLAN.value], + [], + ), ( "z00z-y11y-x22x", "4LLTYP35C", @@ -85,7 +90,12 @@ def add_feature_test_files(local_path): [PointerTypes.EOL_CARE_PLAN.value], [], ), # http://snomed.info/sct|736373009 - ("app-t004", "ODS1", [PointerTypes.PERSONALISED_CARE_AND_SUPPORT_PLAN], []), + ( + "app-t004", + "ODS1", + [PointerTypes.PERSONALISED_CARE_AND_SUPPORT_PLAN.value], + [], + ), ( "z00z-y11y-x22x", "4LLTYP35P", @@ -106,30 +116,30 @@ def add_feature_test_files(local_path): ] app_permissions = { "consumer": [ - ("app-t001", [PointerTypes.MENTAL_HEALTH_PLAN], []), + ("app-t001", [PointerTypes.MENTAL_HEALTH_PLAN.value], []), ( "app-t002", [ - PointerTypes.ADVANCE_CARE_PLAN, - PointerTypes.EMERGENCY_HEALTHCARE_PLAN, - PointerTypes.NEWS2_CHART, + PointerTypes.ADVANCE_CARE_PLAN.value, + PointerTypes.EMERGENCY_HEALTHCARE_PLAN.value, + PointerTypes.NEWS2_CHART.value, ], [], ), - ("app-t004", [PointerTypes.APPOINTMENT], []), + ("app-t004", [PointerTypes.APPOINTMENT.value], []), ], "producer": [ - ("app-t001", [PointerTypes.EOL_COORDINATION_SUMMARY]), + ("app-t001", [PointerTypes.EOL_COORDINATION_SUMMARY.value]), ( "app-t003", [ - PointerTypes.ADVANCE_CARE_PLAN, - PointerTypes.EMERGENCY_HEALTHCARE_PLAN, - PointerTypes.NEWS2_CHART, + PointerTypes.ADVANCE_CARE_PLAN.value, + PointerTypes.EMERGENCY_HEALTHCARE_PLAN.value, + PointerTypes.NEWS2_CHART.value, ], [], ), - ("app-t004", [PointerTypes.APPOINTMENT], []), + ("app-t004", [PointerTypes.APPOINTMENT.value], []), ], } [ From 69124982f63503da6c9c874c4bf03991879e04c2 Mon Sep 17 00:00:00 2001 From: Kate Bobyn Date: Thu, 12 Mar 2026 13:37:41 +0000 Subject: [PATCH 6/8] NRL-1933 fix merge conflicts --- scripts/get_s3_permissions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/get_s3_permissions.py b/scripts/get_s3_permissions.py index 0c4c6e472..d4eeb7545 100644 --- a/scripts/get_s3_permissions.py +++ b/scripts/get_s3_permissions.py @@ -129,7 +129,7 @@ def add_feature_test_files(local_path): ("app-t004", [PointerTypes.APPOINTMENT.value], []), ], "producer": [ - ("app-t001", [PointerTypes.EOL_COORDINATION_SUMMARY.value]), + ("app-t001", [PointerTypes.EOL_COORDINATION_SUMMARY.value], []), ( "app-t003", [ From a2054bc8c05ec7f0798cc3ffda3370bf52925933 Mon Sep 17 00:00:00 2001 From: Kate Bobyn Date: Thu, 12 Mar 2026 15:37:00 +0000 Subject: [PATCH 7/8] NRL-1993 fix app level feature test file --- ...missions-app-level => v2-permissions-app-level.feature} | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) rename tests/features/producer/{v2-permissions-app-level => v2-permissions-app-level.feature} (96%) diff --git a/tests/features/producer/v2-permissions-app-level b/tests/features/producer/v2-permissions-app-level.feature similarity index 96% rename from tests/features/producer/v2-permissions-app-level rename to tests/features/producer/v2-permissions-app-level.feature index 0e4cd07ad..e6b53bc6b 100644 --- a/tests/features/producer/v2-permissions-app-level +++ b/tests/features/producer/v2-permissions-app-level.feature @@ -1,10 +1,9 @@ -Feature: Producer v2 permissions by pointer type - Success and Failure Scenarios +Feature: Producer v2 APP-LEVEL permissions by pointer type - Success and Failure Scenarios For the v2 permissions model, permissions are resolved from a JSON file stored in the nrlf_permissions Lambda layer. Permissions for the feature tests are baked into the layer by `scripts/get_s3_permissions.py` at build time, so no dynamic seeding step is required for success scenarios. - Scenario: HAPPY PATH V2 Permissions with access for pointer type - createDocumentReference Given the application 'ProducerTest001' (ID 'app-t001') is registered to access the API When producer v2 'ORGA' creates a DocumentReference with values: @@ -37,7 +36,7 @@ Feature: Producer v2 permissions by pointer type - Success and Failure Scenarios } """ And the response has a Location header - And the Location header starts with '/DocumentReference/RX898-' + And the Location header starts with '/DocumentReference/ORGA-' And the resource in the Location header exists with values: | property | value | | subject | 9278693472 | @@ -87,7 +86,7 @@ Feature: Producer v2 permissions by pointer type - Success and Failure Scenarios | property | value | | subject | 9278693472 | | status | current | - | type | 736366004 | + | type | 749001000000101 | | category | 734163000 | | custodian | ORGA | | author | HAR1 | From 62461055fb3a150e235e4e147ea2ae0c395467df Mon Sep 17 00:00:00 2001 From: Kate Bobyn Date: Thu, 12 Mar 2026 15:50:58 +0000 Subject: [PATCH 8/8] NRL-1993 fix test data for integration test scenario --- tests/features/producer/v2-permissions-app-level.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/features/producer/v2-permissions-app-level.feature b/tests/features/producer/v2-permissions-app-level.feature index e6b53bc6b..1fcf7c6d3 100644 --- a/tests/features/producer/v2-permissions-app-level.feature +++ b/tests/features/producer/v2-permissions-app-level.feature @@ -87,7 +87,7 @@ Feature: Producer v2 APP-LEVEL permissions by pointer type - Success and Failure | subject | 9278693472 | | status | current | | type | 749001000000101 | - | category | 734163000 | + | category | 419891008 | | custodian | ORGA | | author | HAR1 | | url | https://example.org/my-doc.pdf |