From c3095ab1cdbacf0f443d3f6fbb3fd0aff4fff281 Mon Sep 17 00:00:00 2001 From: Sandy Forrester Date: Thu, 12 Mar 2026 17:35:40 +0000 Subject: [PATCH 1/3] NRL-1987 Update create and update endpoints to allow for v2 access control --- .../create_document_reference.py | 19 ++- .../tests/test_create_document_reference.py | 127 ++++++++++++++++-- .../tests/test_upsert_document_reference.py | 119 ++++++++++++++-- .../upsert_document_reference.py | 19 ++- 4 files changed, 255 insertions(+), 29 deletions(-) diff --git a/api/producer/createDocumentReference/create_document_reference.py b/api/producer/createDocumentReference/create_document_reference.py index 940e642b2..929f3e493 100644 --- a/api/producer/createDocumentReference/create_document_reference.py +++ b/api/producer/createDocumentReference/create_document_reference.py @@ -5,6 +5,7 @@ PERMISSION_AUDIT_DATES_FROM_PAYLOAD, PERMISSION_SUPERSEDE_IGNORE_DELETE_FAIL, TYPES_WITH_MULTIPLES, + AccessControls, ) from nrlf.core.decorators import request_handler from nrlf.core.dynamodb.repository import DocumentPointer, DocumentPointerRepository @@ -18,7 +19,9 @@ def _set_create_time_fields( - create_time: str, document_reference: DocumentReference, nrl_permissions: list[str] + create_time: str, + document_reference: DocumentReference, + metadata: ConnectionMetadata, ) -> DocumentReference: """ Set the date and lastUpdated timestamps on the provided DocumentReference @@ -27,10 +30,14 @@ def _set_create_time_fields( document_reference.meta = Meta() document_reference.meta.lastUpdated = create_time - if ( - document_reference.date - and PERMISSION_AUDIT_DATES_FROM_PAYLOAD in nrl_permissions - ): + can_override_creation_datetime = ( + AccessControls.ALLOW_OVERRIDE_CREATION_DATETIME.value + in metadata.nrl_permissions_policy.access_controls + if metadata.nrl_permissions_policy + else PERMISSION_AUDIT_DATES_FROM_PAYLOAD in metadata.nrl_permissions + ) + + if document_reference.date and can_override_creation_datetime: # Perserving the original date if it exists and the permission is set logger.log( LogReference.PROCREATE011, @@ -51,7 +58,7 @@ def _create_core_model(resource: DocumentReference, metadata: ConnectionMetadata document_reference = _set_create_time_fields( creation_time, document_reference=resource, - nrl_permissions=metadata.nrl_permissions, + metadata=metadata, ) return DocumentPointer.from_document_reference( diff --git a/api/producer/createDocumentReference/tests/test_create_document_reference.py b/api/producer/createDocumentReference/tests/test_create_document_reference.py index 13d422486..079069f97 100644 --- a/api/producer/createDocumentReference/tests/test_create_document_reference.py +++ b/api/producer/createDocumentReference/tests/test_create_document_reference.py @@ -10,8 +10,14 @@ _set_create_time_fields, handler, ) -from nrlf.core.constants import CLIENT_RP_DETAILS, SNOMED_SYSTEM_URL, V2Headers +from nrlf.core.constants import ( + CLIENT_RP_DETAILS, + SNOMED_SYSTEM_URL, + AccessControls, + V2Headers, +) from nrlf.core.dynamodb.repository import DocumentPointer, DocumentPointerRepository +from nrlf.core.model import ConnectionMetadata from nrlf.producer.fhir.r4.model import ( DocumentReferenceRelatesTo, Identifier, @@ -1806,9 +1812,19 @@ def test_create_document_reference_with_date_overridden( def test__set_create_time_fields(doc_ref_name: str): test_time = "2024-03-24T12:34:56.789Z" test_doc_ref = load_document_reference(doc_ref_name) - test_perms = [] + test_metadata = ConnectionMetadata.model_validate( + { + "nrl.ods-code": "Y05868", + "nrl.permissions": [], + "nrl.app-id": "Y05868-TestApp", + "client_rp_details": { + "developer.app.name": "TestApp", + "developer.app.id": "12345", + }, + } + ) - response = _set_create_time_fields(test_time, test_doc_ref, test_perms) + response = _set_create_time_fields(test_time, test_doc_ref, test_metadata) assert response.model_dump(exclude_none=True) == { **test_doc_ref.model_dump(exclude_none=True), @@ -1827,12 +1843,22 @@ def test__set_create_time_fields(doc_ref_name: str): "Y05868-736253002-Valid-with-date-and-meta-lastupdated", ], ) -def test__set_create_time_fields_when_doc_has_date_and_perms(doc_ref_name: str): +def test__set_create_time_fields_when_doc_has_date_and_v1_perms(doc_ref_name: str): test_time = "2024-03-24T12:34:56.789Z" test_doc_ref = load_document_reference(doc_ref_name) - test_perms = ["audit-dates-from-payload"] + test_metadata = ConnectionMetadata.model_validate( + { + "nrl.ods-code": "Y05868", + "nrl.permissions": ["audit-dates-from-payload"], + "nrl.app-id": "Y05868-TestApp", + "client_rp_details": { + "developer.app.name": "TestApp", + "developer.app.id": "12345", + }, + } + ) - response = _set_create_time_fields(test_time, test_doc_ref, test_perms) + response = _set_create_time_fields(test_time, test_doc_ref, test_metadata) assert response.model_dump(exclude_none=True) == { **test_doc_ref.model_dump(exclude_none=True), @@ -1844,12 +1870,95 @@ def test__set_create_time_fields_when_doc_has_date_and_perms(doc_ref_name: str): @freeze_time("2024-03-25") -def test__set_create_time_fields_when_no_date_but_perms(): +def test__set_create_time_fields_when_no_date_but_v1_perms(): test_time = "2024-03-24T12:34:56.789Z" test_doc_ref = load_document_reference("Y05868-736253002-Valid") - test_perms = ["audit-dates-from-payload"] + test_metadata = ConnectionMetadata.model_validate( + { + "nrl.ods-code": "Y05868", + "nrl.permissions": ["audit-dates-from-payload"], + "nrl.app-id": "Y05868-TestApp", + "client_rp_details": { + "developer.app.name": "TestApp", + "developer.app.id": "12345", + }, + } + ) + + response = _set_create_time_fields(test_time, test_doc_ref, test_metadata) + + assert response.model_dump(exclude_none=True) == { + **test_doc_ref.model_dump(exclude_none=True), + "meta": { + "lastUpdated": test_time, + }, + "date": test_time, + } + + +@freeze_time("2024-03-25") +@mark.parametrize( + "doc_ref_name", + [ + "Y05868-736253002-Valid-with-date", + "Y05868-736253002-Valid-with-date-and-meta-lastupdated", + ], +) +def test__set_create_time_fields_v2_when_doc_has_date_and_access_control( + doc_ref_name: str, +): + test_time = "2024-03-24T12:34:56.789Z" + test_doc_ref = load_document_reference(doc_ref_name) + test_metadata = ConnectionMetadata.model_validate( + { + "nrl.ods-code": "Y05868", + "nrl.permissions": [], + "nrl.app-id": "Y05868-TestApp", + "client_rp_details": { + "developer.app.name": "TestApp", + "developer.app.id": "12345", + }, + "nrl_permissions_policy": { + "access_controls": [ + AccessControls.ALLOW_OVERRIDE_CREATION_DATETIME.value + ] + }, + } + ) + + response = _set_create_time_fields(test_time, test_doc_ref, test_metadata) + + assert response.model_dump(exclude_none=True) == { + **test_doc_ref.model_dump(exclude_none=True), + "meta": { + "lastUpdated": test_time, + }, + "date": test_doc_ref.date, + } + + +@freeze_time("2024-03-25") +def test__set_create_time_fields_v2_when_no_date_but_access_control(): + test_time = "2024-03-24T12:34:56.789Z" + test_doc_ref = load_document_reference("Y05868-736253002-Valid") + test_metadata = ConnectionMetadata.model_validate( + { + "nrl.ods-code": "Y05868", + "nrl.permissions": [], + "nrl.app-id": "Y05868-TestApp", + "client_rp_details": { + "developer.app.name": "TestApp", + "developer.app.id": "12345", + }, + "nrl_permissions_policy": { + "access_controls": [ + AccessControls.ALLOW_OVERRIDE_CREATION_DATETIME.value + ] + }, + } + ) - response = _set_create_time_fields(test_time, test_doc_ref, test_perms) + response = _set_create_time_fields(test_time, test_doc_ref, test_metadata) assert response.model_dump(exclude_none=True) == { **test_doc_ref.model_dump(exclude_none=True), diff --git a/api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py b/api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py index 326dd581d..10f971db4 100644 --- a/api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py +++ b/api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py @@ -12,9 +12,11 @@ from nrlf.core.constants import ( CLIENT_RP_DETAILS, PERMISSION_SUPERSEDE_IGNORE_DELETE_FAIL, + AccessControls, V2Headers, ) from nrlf.core.dynamodb.repository import DocumentPointer, DocumentPointerRepository +from nrlf.core.model import ConnectionMetadata, PermissionsPolicy from nrlf.producer.fhir.r4.model import ( DocumentReferenceRelatesTo, Identifier, @@ -1773,9 +1775,19 @@ def test_upsert_document_reference_with_date_overridden( def test__set_create_time_fields(doc_ref_name: str): test_time = "2024-03-24T12:34:56.789Z" test_doc_ref = load_document_reference(doc_ref_name) - test_perms = [] + test_metadata = ConnectionMetadata.model_validate( + { + "nrl.ods-code": "Y05868", + "nrl.permissions": [], + "nrl.app-id": "Y05868-TestApp", + "client_rp_details": { + "developer.app.name": "TestApp", + "developer.app.id": "12345", + }, + } + ) - response = _set_upsert_time_fields(test_time, test_doc_ref, test_perms) + response = _set_upsert_time_fields(test_time, test_doc_ref, test_metadata) assert response.model_dump(exclude_none=True) == { **test_doc_ref.model_dump(exclude_none=True), @@ -1794,12 +1806,90 @@ def test__set_create_time_fields(doc_ref_name: str): "Y05868-736253002-Valid-with-date-and-meta-lastupdated", ], ) -def test__set_create_time_fields_when_doc_has_date_and_perms(doc_ref_name: str): +def test__set_create_time_fields_when_doc_has_date_and_v1_perms(doc_ref_name: str): + test_time = "2024-03-24T12:34:56.789Z" + test_doc_ref = load_document_reference(doc_ref_name) + test_metadata = ConnectionMetadata.model_validate( + { + "nrl.ods-code": "Y05868", + "nrl.permissions": ["audit-dates-from-payload"], + "nrl.app-id": "Y05868-TestApp", + "client_rp_details": { + "developer.app.name": "TestApp", + "developer.app.id": "12345", + }, + } + ) + + response = _set_upsert_time_fields(test_time, test_doc_ref, test_metadata) + + assert response.model_dump(exclude_none=True) == { + **test_doc_ref.model_dump(exclude_none=True), + "meta": { + "lastUpdated": test_time, + }, + "date": test_doc_ref.date, + } + + +@freeze_time("2024-03-25") +def test__set_create_time_fields_when_no_date_but_v1_perms(): + test_time = "2024-03-24T12:34:56.789Z" + test_doc_ref = load_document_reference("Y05868-736253002-Valid") + test_metadata = ConnectionMetadata.model_validate( + { + "nrl.ods-code": "Y05868", + "nrl.permissions": ["audit-dates-from-payload"], + "nrl.app-id": "Y05868-TestApp", + "client_rp_details": { + "developer.app.name": "TestApp", + "developer.app.id": "12345", + }, + } + ) + + response = _set_upsert_time_fields(test_time, test_doc_ref, test_metadata) + + assert response.model_dump(exclude_none=True) == { + **test_doc_ref.model_dump(exclude_none=True), + "meta": { + "lastUpdated": test_time, + }, + "date": test_time, + } + + +@freeze_time("2024-03-25") +@mark.parametrize( + "doc_ref_name", + [ + "Y05868-736253002-Valid-with-date", + "Y05868-736253002-Valid-with-date-and-meta-lastupdated", + ], +) +def test__set_upsert_time_fields_v2_when_doc_has_date_and_access_control( + doc_ref_name: str, +): test_time = "2024-03-24T12:34:56.789Z" test_doc_ref = load_document_reference(doc_ref_name) - test_perms = ["audit-dates-from-payload"] + test_metadata = ConnectionMetadata.model_validate( + { + "nrl.ods-code": "Y05868", + "nrl.permissions": [], + "nrl.app-id": "Y05868-TestApp", + "client_rp_details": { + "developer.app.name": "TestApp", + "developer.app.id": "12345", + }, + "nrl_permissions_policy": { + "access_controls": [ + AccessControls.ALLOW_OVERRIDE_CREATION_DATETIME.value + ] + }, + } + ) - response = _set_upsert_time_fields(test_time, test_doc_ref, test_perms) + response = _set_upsert_time_fields(test_time, test_doc_ref, test_metadata) assert response.model_dump(exclude_none=True) == { **test_doc_ref.model_dump(exclude_none=True), @@ -1811,12 +1901,25 @@ def test__set_create_time_fields_when_doc_has_date_and_perms(doc_ref_name: str): @freeze_time("2024-03-25") -def test__set_create_time_fields_when_no_date_but_perms(): +def test__set_upsert_time_fields_v2_when_no_date_but_access_control(): test_time = "2024-03-24T12:34:56.789Z" test_doc_ref = load_document_reference("Y05868-736253002-Valid") - test_perms = ["audit-dates-from-payload"] + test_metadata = ConnectionMetadata.model_validate( + { + "nrl.ods-code": "Y05868", + "nrl.permissions": [], + "nrl.app-id": "Y05868-TestApp", + "client_rp_details": { + "developer.app.name": "TestApp", + "developer.app.id": "12345", + }, + } + ) + test_metadata.nrl_permissions_policy = PermissionsPolicy( + access_controls=[AccessControls.ALLOW_OVERRIDE_CREATION_DATETIME.value] + ) - response = _set_upsert_time_fields(test_time, test_doc_ref, test_perms) + response = _set_upsert_time_fields(test_time, test_doc_ref, test_metadata) assert response.model_dump(exclude_none=True) == { **test_doc_ref.model_dump(exclude_none=True), diff --git a/api/producer/upsertDocumentReference/upsert_document_reference.py b/api/producer/upsertDocumentReference/upsert_document_reference.py index 3e5f31fc5..d2d5754cb 100644 --- a/api/producer/upsertDocumentReference/upsert_document_reference.py +++ b/api/producer/upsertDocumentReference/upsert_document_reference.py @@ -3,6 +3,7 @@ PERMISSION_AUDIT_DATES_FROM_PAYLOAD, PERMISSION_SUPERSEDE_IGNORE_DELETE_FAIL, TYPES_WITH_MULTIPLES, + AccessControls, ) from nrlf.core.decorators import request_handler from nrlf.core.dynamodb.repository import DocumentPointer, DocumentPointerRepository @@ -16,7 +17,9 @@ def _set_upsert_time_fields( - upsert_time: str, document_reference: DocumentReference, nrl_permissions: list[str] + upsert_time: str, + document_reference: DocumentReference, + metadata: ConnectionMetadata, ) -> DocumentReference: """ Set the date and lastUpdated timestamps on the provided DocumentReference @@ -25,10 +28,14 @@ def _set_upsert_time_fields( document_reference.meta = Meta() document_reference.meta.lastUpdated = upsert_time - if ( - document_reference.date - and PERMISSION_AUDIT_DATES_FROM_PAYLOAD in nrl_permissions - ): + can_override_creation_datetime = ( + AccessControls.ALLOW_OVERRIDE_CREATION_DATETIME.value + in metadata.nrl_permissions_policy.access_controls + if metadata.nrl_permissions_policy + else PERMISSION_AUDIT_DATES_FROM_PAYLOAD in metadata.nrl_permissions + ) + + if document_reference.date and can_override_creation_datetime: # Perserving the original date if it exists and the permission is set logger.log( LogReference.PROUPSERT011, @@ -49,7 +56,7 @@ def _create_core_model(resource: DocumentReference, metadata: ConnectionMetadata document_reference = _set_upsert_time_fields( creation_time, document_reference=resource, - nrl_permissions=metadata.nrl_permissions, + metadata=metadata, ) return DocumentPointer.from_document_reference( From 78e2257c57c62eb8a0f7f3521af0112a3d554bed Mon Sep 17 00:00:00 2001 From: Sandy Forrester Date: Fri, 13 Mar 2026 13:00:06 +0000 Subject: [PATCH 2/3] NRL-1987 Add feature tests for date override --- scripts/get_s3_permissions.py | 5 +- .../v2-permissions-access-controls.feature | 93 +++++++++++++++++++ tests/features/steps/3_assert.py | 30 +++++- tests/features/utils/data.py | 3 + 4 files changed, 128 insertions(+), 3 deletions(-) create mode 100644 tests/features/producer/v2-permissions-access-controls.feature diff --git a/scripts/get_s3_permissions.py b/scripts/get_s3_permissions.py index d4eeb7545..dbf2f2538 100644 --- a/scripts/get_s3_permissions.py +++ b/scripts/get_s3_permissions.py @@ -100,7 +100,10 @@ def add_feature_test_files(local_path): "z00z-y11y-x22x", "4LLTYP35P", [], - [AccessControls.ALLOW_ALL_TYPES.value], + [ + AccessControls.ALLOW_ALL_TYPES.value, + AccessControls.ALLOW_OVERRIDE_CREATION_DATETIME.value, + ], ), ], } diff --git a/tests/features/producer/v2-permissions-access-controls.feature b/tests/features/producer/v2-permissions-access-controls.feature new file mode 100644 index 000000000..716bc0966 --- /dev/null +++ b/tests/features/producer/v2-permissions-access-controls.feature @@ -0,0 +1,93 @@ +Feature: Producer v2 access_control permissions - Success and Failure Scenarios + + Scenario: Successfully create a DocumentReference with a specified date with the ALLOW_OVERRIDE_CREATION_DATETIME permission - createDocumentReference + Given the application 'DataShare' (ID 'z00z-y11y-x22x') is registered to access the API + When producer v2 '4LLTYP35P' creates a DocumentReference with values: + | property | value | + | subject | 9278693472 | + | status | current | + | type | 736253002 | + | category | 734163000 | + | custodian | 4LLTYP35P | + | author | HAR1 | + | url | https://example.org/my-doc.pdf | + | practiceSetting | 788002001 | + | date | 2024-06-01T12:00:00Z | + 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/4LLTYP35P-' + And the resource in the Location header exists with values: + | property | value | + | subject | 9278693472 | + | status | current | + | type | 736253002 | + | category | 734163000 | + | custodian | 4LLTYP35P | + | author | HAR1 | + | url | https://example.org/my-doc.pdf | + | practiceSetting | 788002001 | + | date | 2024-06-01T12:00:00Z | + + Scenario: Create a DocumentReference with a specified date WITHOUT ALLOW_OVERRIDE_CREATION_DATETIME - date should be overridden by the server + Given the application 'DataShare' (ID 'z00z-y11y-x22x') is registered to access the API + When producer v2 'RX898' creates a DocumentReference with values: + | property | value | + | subject | 9278693472 | + | status | current | + | type | 736373009 | + | category | 734163000 | + | custodian | RX898 | + | author | HAR1 | + | url | https://example.org/my-doc.pdf | + | practiceSetting | 788002001 | + | date | 2024-06-01T12:00:00Z | + 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 | 736373009 | + | category | 734163000 | + | custodian | RX898 | + | author | HAR1 | + | url | https://example.org/my-doc.pdf | + | practiceSetting | 788002001 | + And the date of the resource in the Location header is not '2024-06-01T12:00:00Z' diff --git a/tests/features/steps/3_assert.py b/tests/features/steps/3_assert.py index f8f32542a..5e76593b1 100644 --- a/tests/features/steps/3_assert.py +++ b/tests/features/steps/3_assert.py @@ -240,6 +240,14 @@ def assert_document_reference_matches_value( context.response.json(), ) + if date := items.get("date"): + assert doc_ref.date == date, format_error( + "DocumentReference date does not match", + date, + doc_ref.date, + context.response.json(), + ) + @then("the Bundle contains an DocumentReference with values") def assert_bundle_contains_documentreference_values_step(context: Context): @@ -369,8 +377,7 @@ def assert_header_starts_with(context: Context, header_name: str, starts_with: s ) -@then("the resource in the Location header exists with values") -def assert_resource_in_location_header_exists_with_values(context: Context): +def _get_resource_from_location_header(context: Context): location = context.response.headers.get("Location") assert location.startswith("/DocumentReference/"), format_error( @@ -390,6 +397,25 @@ def assert_resource_in_location_header_exists_with_values(context: Context): context.response.text, ) + return resource_id, resource + + +@then("the date of the resource in the Location header is not '{expected_date}'") +def assert_resource_date_not_equal(context: Context, expected_date: str): + _, resource = _get_resource_from_location_header(context) + doc_ref = DocumentReference.model_validate_json(resource.document) + assert doc_ref.date != expected_date, format_error( + "DocumentReference date was not overridden - date matches the payload value but should have been overwritten by the server", + f"anything except {expected_date}", + doc_ref.date, + context.response.text, + ) + + +@then("the resource in the Location header exists with values") +def assert_resource_in_location_header_exists_with_values(context: Context): + resource_id, resource = _get_resource_from_location_header(context) + if not context.table: raise ValueError("No DocumentReference table provided") diff --git a/tests/features/utils/data.py b/tests/features/utils/data.py index b335e26ac..67effbe8f 100644 --- a/tests/features/utils/data.py +++ b/tests/features/utils/data.py @@ -185,6 +185,9 @@ def create_test_document_reference(items: dict) -> DocumentReference: ) ] + if items.get("date"): + base_doc_ref.date = items["date"] + return base_doc_ref From 21d29c17b08d048b937bdbe0b7eed79f1d922032 Mon Sep 17 00:00:00 2001 From: Sandy Forrester Date: Fri, 13 Mar 2026 13:04:08 +0000 Subject: [PATCH 3/3] NRL-1987 Fix typo --- .../createDocumentReference/create_document_reference.py | 2 +- .../upsertDocumentReference/upsert_document_reference.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/api/producer/createDocumentReference/create_document_reference.py b/api/producer/createDocumentReference/create_document_reference.py index 929f3e493..4e0deba1c 100644 --- a/api/producer/createDocumentReference/create_document_reference.py +++ b/api/producer/createDocumentReference/create_document_reference.py @@ -38,7 +38,7 @@ def _set_create_time_fields( ) if document_reference.date and can_override_creation_datetime: - # Perserving the original date if it exists and the permission is set + # Preserving the original date if it exists and the permission is set logger.log( LogReference.PROCREATE011, id=document_reference.id, diff --git a/api/producer/upsertDocumentReference/upsert_document_reference.py b/api/producer/upsertDocumentReference/upsert_document_reference.py index d2d5754cb..c8b5a122c 100644 --- a/api/producer/upsertDocumentReference/upsert_document_reference.py +++ b/api/producer/upsertDocumentReference/upsert_document_reference.py @@ -36,7 +36,7 @@ def _set_upsert_time_fields( ) if document_reference.date and can_override_creation_datetime: - # Perserving the original date if it exists and the permission is set + # Preserving the original date if it exists and the permission is set logger.log( LogReference.PROUPSERT011, id=document_reference.id,