Skip to content

Commit 20c1b74

Browse files
sandyforresternhssandyforresternhs
authored
NRL-1995 Access control for allow_supersede_with_delete_failure (#1171)
- Create and upsert endpoints now factor the v2 access control for allow_supersede_with_delete_failure into their checks - Added a before_scenario to the feature tests to delete the table contents and avoid clashes with leftover data
1 parent 78bb5fd commit 20c1b74

File tree

11 files changed

+410
-28
lines changed

11 files changed

+410
-28
lines changed

‎api/producer/createDocumentReference/create_document_reference.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,10 @@ def handler(
254254
return error_response
255255

256256
can_ignore_delete_fail = (
257-
PERMISSION_SUPERSEDE_IGNORE_DELETE_FAIL in metadata.nrl_permissions
257+
AccessControls.ALLOW_SUPERSEDE_WITH_DELETE_FAILURE.value
258+
in metadata.nrl_permissions_policy.access_controls
259+
if metadata.nrl_permissions_policy
260+
else PERMISSION_SUPERSEDE_IGNORE_DELETE_FAIL in metadata.nrl_permissions
258261
)
259262

260263
if ids_to_delete := _get_document_ids_to_supersede(

‎api/producer/createDocumentReference/tests/test_create_document_reference.py‎

Lines changed: 139 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1435,7 +1435,7 @@ def test_create_document_reference_supersede_deletes_old_pointers_replace(
14351435
@mock_aws
14361436
@mock_repository
14371437
@freeze_uuid("00000000-0000-0000-0000-000000000001")
1438-
def test_create_document_reference_supersede_succeeds_with_toggle(
1438+
def test_supersede_non_existent_pointer_succeeds_with_v1_ignore_delete_fail(
14391439
repository: DocumentPointerRepository,
14401440
):
14411441
doc_ref = load_document_reference("Y05868-736253002-Valid")
@@ -1493,7 +1493,7 @@ def test_create_document_reference_supersede_succeeds_with_toggle(
14931493

14941494
@mock_aws
14951495
@mock_repository
1496-
def test_create_document_reference_supersede_fails_without_toggle(
1496+
def test_supersede_non_existent_pointer_fails_without_v1_ignore_delete_fail(
14971497
repository: DocumentPointerRepository,
14981498
):
14991499
doc_ref = load_document_reference("Y05868-736253002-Valid")
@@ -1544,6 +1544,143 @@ def test_create_document_reference_supersede_fails_without_toggle(
15441544
}
15451545

15461546

1547+
@mock_aws
1548+
@mock_repository
1549+
@freeze_uuid("00000000-0000-0000-0000-000000000001")
1550+
@patch("nrlf.core.decorators.get_pointer_permissions_v2")
1551+
def test_supersede_non_existent_pointer_succeeds_with_v2_access_control(
1552+
get_pointer_permissions_mock, repository: DocumentPointerRepository
1553+
):
1554+
doc_ref = load_document_reference("Y05868-736253002-Valid")
1555+
1556+
# Add reference to a non-existing pointer
1557+
doc_ref.relatesTo = [
1558+
DocumentReferenceRelatesTo(
1559+
code="replaces",
1560+
target=Reference(identifier=Identifier(value="Y05868-99999-99999-000000")),
1561+
)
1562+
]
1563+
1564+
v2_headers = create_headers(
1565+
additional_headers={
1566+
V2Headers.NHSD_END_USER_ORGANISATION_ODS: "Y05868",
1567+
V2Headers.NHSD_NRL_APP_ID: "Y05868-TestApp-12345678",
1568+
}
1569+
)
1570+
v2_headers.pop(CLIENT_RP_DETAILS)
1571+
1572+
get_pointer_permissions_mock.return_value = {
1573+
"access_controls": [AccessControls.ALLOW_SUPERSEDE_WITH_DELETE_FAILURE.value],
1574+
"types": ["http://snomed.info/sct|736253002"],
1575+
}
1576+
1577+
event = create_test_api_gateway_event(
1578+
headers=v2_headers,
1579+
body=doc_ref.model_dump_json(exclude_none=True),
1580+
)
1581+
1582+
result = handler(event, create_mock_context())
1583+
body = result.pop("body")
1584+
1585+
assert result == {
1586+
"statusCode": "201",
1587+
"headers": {
1588+
"Location": "/DocumentReference/Y05868-00000000-0000-0000-0000-000000000001",
1589+
**default_response_headers(),
1590+
},
1591+
"isBase64Encoded": False,
1592+
}
1593+
1594+
parsed_body = json.loads(body)
1595+
1596+
assert parsed_body == {
1597+
"resourceType": "OperationOutcome",
1598+
"issue": [
1599+
{
1600+
"severity": "information",
1601+
"code": "informational",
1602+
"details": {
1603+
"coding": [
1604+
{
1605+
"code": "RESOURCE_SUPERSEDED",
1606+
"display": "Resource created and resource(s) deleted",
1607+
"system": "https://fhir.nhs.uk/ValueSet/NRL-ResponseCode",
1608+
}
1609+
]
1610+
},
1611+
"diagnostics": "The document has been superseded by a new version",
1612+
}
1613+
],
1614+
}
1615+
1616+
1617+
@mock_aws
1618+
@mock_repository
1619+
@patch("nrlf.core.decorators.get_pointer_permissions_v2")
1620+
def test_supersede_fails_without_v2_access_control(
1621+
get_pointer_permissions_mock, repository: DocumentPointerRepository
1622+
):
1623+
doc_ref = load_document_reference("Y05868-736253002-Valid")
1624+
1625+
# Add reference to a non-existing pointer
1626+
doc_ref.relatesTo = [
1627+
DocumentReferenceRelatesTo(
1628+
code="replaces",
1629+
target=Reference(identifier=Identifier(value="Y05868-99999-99999-000000")),
1630+
)
1631+
]
1632+
1633+
v2_headers = create_headers(
1634+
additional_headers={
1635+
V2Headers.NHSD_END_USER_ORGANISATION_ODS: "Y05868",
1636+
V2Headers.NHSD_NRL_APP_ID: "Y05868-TestApp-12345678",
1637+
}
1638+
)
1639+
v2_headers.pop(CLIENT_RP_DETAILS)
1640+
1641+
get_pointer_permissions_mock.return_value = {
1642+
"access_controls": [],
1643+
"types": ["http://snomed.info/sct|736253002"],
1644+
}
1645+
1646+
event = create_test_api_gateway_event(
1647+
headers=v2_headers,
1648+
body=doc_ref.model_dump_json(exclude_none=True),
1649+
)
1650+
1651+
result = handler(event, create_mock_context())
1652+
body = result.pop("body")
1653+
1654+
assert result == {
1655+
"statusCode": "422",
1656+
"headers": default_response_headers(),
1657+
"isBase64Encoded": False,
1658+
}
1659+
1660+
parsed_body = json.loads(body)
1661+
1662+
assert parsed_body == {
1663+
"resourceType": "OperationOutcome",
1664+
"issue": [
1665+
{
1666+
"severity": "error",
1667+
"code": "business-rule",
1668+
"details": {
1669+
"coding": [
1670+
{
1671+
"code": "UNPROCESSABLE_ENTITY",
1672+
"display": "Unprocessable Entity",
1673+
"system": "https://fhir.nhs.uk/CodeSystem/Spine-ErrorOrWarningCode",
1674+
}
1675+
]
1676+
},
1677+
"diagnostics": "The relatesTo target document does not exist",
1678+
"expression": ["relatesTo[0].target.identifier.value"],
1679+
}
1680+
],
1681+
}
1682+
1683+
15471684
@mock_aws
15481685
@mock_repository
15491686
@freeze_uuid("00000000-0000-0000-0000-000000000001")

‎api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py‎

Lines changed: 138 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1411,7 +1411,7 @@ def test_upsert_document_reference_supersede_deletes_old_pointers_replace(
14111411

14121412
@mock_aws
14131413
@mock_repository
1414-
def test_upsert_document_reference_supersede_succeeds_with_toggle(
1414+
def test_supersede_non_existent_pointer_succeeds_with_v1_ignore_delete_fail(
14151415
repository: DocumentPointerRepository,
14161416
):
14171417
doc_ref = load_document_reference("Y05868-736253002-Valid")
@@ -1463,13 +1463,10 @@ def test_upsert_document_reference_supersede_succeeds_with_toggle(
14631463
],
14641464
}
14651465

1466-
non_existent_pointer = repository.get_by_id("Y05868-99999-99999-000000")
1467-
assert non_existent_pointer is None
1468-
14691466

14701467
@mock_aws
14711468
@mock_repository
1472-
def test_upsert_document_reference_supersede_fails_without_toggle(
1469+
def test_supersede_non_existent_pointer_fails_without_v1_ignore_delete_fail(
14731470
repository: DocumentPointerRepository,
14741471
):
14751472
doc_ref = load_document_reference("Y05868-736253002-Valid")
@@ -1520,6 +1517,142 @@ def test_upsert_document_reference_supersede_fails_without_toggle(
15201517
}
15211518

15221519

1520+
@mock_aws
1521+
@mock_repository
1522+
@patch("nrlf.core.decorators.get_pointer_permissions_v2")
1523+
def test_supersede_non_existent_pointer_succeeds_with_v2_access_control(
1524+
get_pointer_permissions_mock, repository: DocumentPointerRepository
1525+
):
1526+
doc_ref = load_document_reference("Y05868-736253002-Valid")
1527+
1528+
# Add reference to a non-existing pointer
1529+
doc_ref.relatesTo = [
1530+
DocumentReferenceRelatesTo(
1531+
code="replaces",
1532+
target=Reference(identifier=Identifier(value="Y05868-99999-99999-000000")),
1533+
)
1534+
]
1535+
1536+
v2_headers = create_headers(
1537+
additional_headers={
1538+
V2Headers.NHSD_END_USER_ORGANISATION_ODS: "Y05868",
1539+
V2Headers.NHSD_NRL_APP_ID: "Y05868-TestApp-12345678",
1540+
}
1541+
)
1542+
v2_headers.pop(CLIENT_RP_DETAILS)
1543+
1544+
get_pointer_permissions_mock.return_value = {
1545+
"access_controls": [AccessControls.ALLOW_SUPERSEDE_WITH_DELETE_FAILURE.value],
1546+
"types": ["http://snomed.info/sct|736253002"],
1547+
}
1548+
1549+
event = create_test_api_gateway_event(
1550+
headers=v2_headers,
1551+
body=doc_ref.model_dump_json(exclude_none=True),
1552+
)
1553+
1554+
result = handler(event, create_mock_context())
1555+
body = result.pop("body")
1556+
1557+
assert result == {
1558+
"statusCode": "201",
1559+
"headers": {
1560+
"Location": "/DocumentReference/Y05868-99999-99999-999999",
1561+
**default_response_headers(),
1562+
},
1563+
"isBase64Encoded": False,
1564+
}
1565+
1566+
parsed_body = json.loads(body)
1567+
1568+
assert parsed_body == {
1569+
"resourceType": "OperationOutcome",
1570+
"issue": [
1571+
{
1572+
"severity": "information",
1573+
"code": "informational",
1574+
"details": {
1575+
"coding": [
1576+
{
1577+
"code": "RESOURCE_SUPERSEDED",
1578+
"display": "Resource created and resource(s) deleted",
1579+
"system": "https://fhir.nhs.uk/ValueSet/NRL-ResponseCode",
1580+
}
1581+
]
1582+
},
1583+
"diagnostics": "The document has been superseded by a new version",
1584+
}
1585+
],
1586+
}
1587+
1588+
1589+
@mock_aws
1590+
@mock_repository
1591+
@patch("nrlf.core.decorators.get_pointer_permissions_v2")
1592+
def test_supersede_fails_without_v2_access_control(
1593+
get_pointer_permissions_mock, repository: DocumentPointerRepository
1594+
):
1595+
doc_ref = load_document_reference("Y05868-736253002-Valid")
1596+
1597+
# Add reference to a non-existing pointer
1598+
doc_ref.relatesTo = [
1599+
DocumentReferenceRelatesTo(
1600+
code="replaces",
1601+
target=Reference(identifier=Identifier(value="Y05868-99999-99999-000000")),
1602+
)
1603+
]
1604+
1605+
v2_headers = create_headers(
1606+
additional_headers={
1607+
V2Headers.NHSD_END_USER_ORGANISATION_ODS: "Y05868",
1608+
V2Headers.NHSD_NRL_APP_ID: "Y05868-TestApp-12345678",
1609+
}
1610+
)
1611+
v2_headers.pop(CLIENT_RP_DETAILS)
1612+
1613+
get_pointer_permissions_mock.return_value = {
1614+
"access_controls": [],
1615+
"types": ["http://snomed.info/sct|736253002"],
1616+
}
1617+
1618+
event = create_test_api_gateway_event(
1619+
headers=v2_headers,
1620+
body=doc_ref.model_dump_json(exclude_none=True),
1621+
)
1622+
1623+
result = handler(event, create_mock_context())
1624+
body = result.pop("body")
1625+
1626+
assert result == {
1627+
"statusCode": "422",
1628+
"headers": default_response_headers(),
1629+
"isBase64Encoded": False,
1630+
}
1631+
1632+
parsed_body = json.loads(body)
1633+
1634+
assert parsed_body == {
1635+
"resourceType": "OperationOutcome",
1636+
"issue": [
1637+
{
1638+
"severity": "error",
1639+
"code": "business-rule",
1640+
"details": {
1641+
"coding": [
1642+
{
1643+
"code": "UNPROCESSABLE_ENTITY",
1644+
"display": "Unprocessable Entity",
1645+
"system": "https://fhir.nhs.uk/CodeSystem/Spine-ErrorOrWarningCode",
1646+
}
1647+
]
1648+
},
1649+
"diagnostics": "The relatesTo target document does not exist",
1650+
"expression": ["relatesTo[0].target.identifier.value"],
1651+
}
1652+
],
1653+
}
1654+
1655+
15231656
@mock_aws
15241657
@mock_repository
15251658
def test_upsert_document_reference_create_relatesto_not_replaces(

‎api/producer/upsertDocumentReference/upsert_document_reference.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,10 @@ def handler(
258258
return error_response
259259

260260
can_ignore_delete_fail = (
261-
PERMISSION_SUPERSEDE_IGNORE_DELETE_FAIL in metadata.nrl_permissions
261+
AccessControls.ALLOW_SUPERSEDE_WITH_DELETE_FAILURE.value
262+
in metadata.nrl_permissions_policy.access_controls
263+
if metadata.nrl_permissions_policy
264+
else PERMISSION_SUPERSEDE_IGNORE_DELETE_FAIL in metadata.nrl_permissions
262265
)
263266

264267
if ids_to_delete := _get_document_ids_to_supersede(

‎layer/nrlf/core/constants.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ class AccessControls(Enum):
5858
ALLOW_PRODUCE_FOR_ANY_AUTHOR = "allow_produce_for_any_author"
5959
ALLOW_PRODUCE_FOR_ANY_CUSTODIAN = "allow_produce_for_any_custodian"
6060
ALLOW_OVERRIDE_CREATION_DATETIME = "allow_override_creation_datetime"
61+
ALLOW_SUPERSEDE_WITH_DELETE_FAILURE = "allow_supersede_with_delete_failure"
6162

6263
@staticmethod
6364
def list():

‎scripts/get_s3_permissions.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ def add_feature_test_files(local_path):
103103
[
104104
AccessControls.ALLOW_ALL_TYPES.value,
105105
AccessControls.ALLOW_OVERRIDE_CREATION_DATETIME.value,
106+
AccessControls.ALLOW_SUPERSEDE_WITH_DELETE_FAILURE.value,
106107
],
107108
),
108109
],

0 commit comments

Comments
 (0)