Skip to content

Commit 24a1ba5

Browse files
lmsurprelmsurpre
authored
issue #4101 - handle patch in fhir-smart interceptor (#4103)
* issue #4101 - handle patch in fhir-smart interceptor and add corresponding tests to AuthzPolicyEnforcementTest.java Signed-off-by: Lee Surprenant <lmsurpre@merative.com> * updated comment as suggested Signed-off-by: Lee Surprenant <lmsurpre@merative.com> * add clarifying comments as requested in review Signed-off-by: Lee Surprenant <lmsurpre@merative.com> Signed-off-by: Lee Surprenant <lmsurpre@merative.com>
1 parent b8adb1b commit 24a1ba5

2 files changed

Lines changed: 89 additions & 1 deletion

File tree

‎fhir-smart/src/main/java/org/linuxforhealth/fhir/smart/AuthzPolicyEnforcementPersistenceInterceptor.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,15 @@ public void beforeDelete(FHIRPersistenceEvent event) throws FHIRPersistenceInter
458458

459459
@Override
460460
public void beforeUpdate(FHIRPersistenceEvent event) throws FHIRPersistenceInterceptorException {
461+
beforeUpdateOrPatch(event);
462+
}
463+
464+
@Override
465+
public void beforePatch(FHIRPersistenceEvent event) throws FHIRPersistenceInterceptorException {
466+
beforeUpdateOrPatch(event);
467+
}
468+
469+
private void beforeUpdateOrPatch(FHIRPersistenceEvent event) throws FHIRPersistenceInterceptorException {
461470
DecodedJWT jwt = JWT.decode(getAccessToken());
462471
Set<String> patientIdFromToken = getPatientIdFromToken(jwt);
463472
Map<ContextType, List<Scope>> scopesFromToken = getScopesFromToken(jwt);

‎fhir-smart/src/test/java/org/linuxforhealth/fhir/smart/test/AuthzPolicyEnforcementTest.java‎

Lines changed: 80 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,10 @@ public void testCreate(String scopeString, List<String> contextIds, Set<Resource
202202
}
203203
}
204204

205+
// For the update interaction, both read and write permissions are needed.
206+
// * In the "allowed" case both READ and WRITE "shouldSucceed" (or else it should not have been allowed)
207+
// * In the "disallowed" (Exception) case, then either the READ or WRITE interaction was expected
208+
// to not succeed (or else it should have been allowed)
205209
@Test(dataProvider = "scopeStringProvider")
206210
public void testUpdate(String scopeString, List<String> contextIds, Set<ResourceType> resourceTypesPermittedByScope, Permission permission) {
207211
FHIRRequestContext.get().setHttpHeaders(buildRequestHeaders(scopeString, contextIds));
@@ -255,7 +259,8 @@ public void testUpdate(String scopeString, List<String> contextIds, Set<Resource
255259
shouldSucceed(resourceTypesPermittedByScope, BINARY, WRITE_APPROVED, permission));
256260
}
257261

258-
// Test update Binary Resource which has a securityContext. Should Fail since securityContext is not supported.
262+
// Test beforePatch Binary Resource which has a securityContext.
263+
// It should be an exception since securityContext is not supported.
259264
try {
260265
properties.put(FHIRPersistenceEvent.PROPNAME_RESOURCE_TYPE, "Binary");
261266
FHIRPersistenceEvent event = new FHIRPersistenceEvent(binaryWithSecurityContext, properties);
@@ -267,9 +272,82 @@ public void testUpdate(String scopeString, List<String> contextIds, Set<Resource
267272
shouldSucceed(resourceTypesPermittedByScope, BINARY, WRITE_APPROVED, permission)) {
268273
assertTrue(e.getMessage().equals("securityContext is not supported for resource type Binary"));
269274
}
275+
// else beforeUpdate was rejected due to normal fhir-smart behavior (non-securityContext-related)
276+
}
277+
}
278+
279+
// The patch interaction is just like the update interaction; both read and write permissions are needed.
280+
// * In the "allowed" case both READ and WRITE "shouldSucceed" (or else it should not have been allowed)
281+
// * In the "disallowed" (Exception) case, then either the READ or WRITE interaction was expected
282+
// to not succeed (or else it should have been allowed)
283+
@Test(dataProvider = "scopeStringProvider")
284+
public void testPatch(String scopeString, List<String> contextIds, Set<ResourceType> resourceTypesPermittedByScope, Permission permission) {
285+
FHIRRequestContext.get().setHttpHeaders(buildRequestHeaders(scopeString, contextIds));
286+
287+
try {
288+
properties.put(FHIRPersistenceEvent.PROPNAME_RESOURCE_TYPE, "Patient");
289+
FHIRPersistenceEvent event = new FHIRPersistenceEvent(patient, properties);
290+
event.setPrevFhirResource(patient);
291+
interceptor.beforePatch(event);
292+
assertTrue(shouldSucceed(resourceTypesPermittedByScope, PATIENT, READ_APPROVED, permission) &&
293+
shouldSucceed(resourceTypesPermittedByScope, PATIENT, WRITE_APPROVED, permission));
294+
} catch (FHIRPersistenceInterceptorException e) {
295+
assertFalse(shouldSucceed(resourceTypesPermittedByScope, PATIENT, READ_APPROVED, permission) &&
296+
shouldSucceed(resourceTypesPermittedByScope, PATIENT, WRITE_APPROVED, permission));
297+
}
298+
299+
try {
300+
properties.put(FHIRPersistenceEvent.PROPNAME_RESOURCE_TYPE, "Observation");
301+
FHIRPersistenceEvent event = new FHIRPersistenceEvent(observation, properties);
302+
event.setPrevFhirResource(observation);
303+
interceptor.beforePatch(event);
304+
assertTrue(shouldSucceed(resourceTypesPermittedByScope, OBSERVATION, READ_APPROVED, permission) &&
305+
shouldSucceed(resourceTypesPermittedByScope, OBSERVATION, WRITE_APPROVED, permission));
306+
} catch (FHIRPersistenceInterceptorException e) {
307+
assertFalse(shouldSucceed(resourceTypesPermittedByScope, OBSERVATION, READ_APPROVED, permission) &&
308+
shouldSucceed(resourceTypesPermittedByScope, OBSERVATION, WRITE_APPROVED, permission));
309+
}
310+
311+
try {
312+
properties.put(FHIRPersistenceEvent.PROPNAME_RESOURCE_TYPE, "Condition");
313+
FHIRPersistenceEvent event = new FHIRPersistenceEvent(condition, properties);
314+
event.setPrevFhirResource(condition);
315+
interceptor.beforePatch(event);
316+
assertTrue(shouldSucceed(resourceTypesPermittedByScope, CONDITION, READ_APPROVED, permission) &&
317+
shouldSucceed(resourceTypesPermittedByScope, CONDITION, WRITE_APPROVED, permission));
318+
} catch (FHIRPersistenceInterceptorException e) {
319+
assertFalse(shouldSucceed(resourceTypesPermittedByScope, CONDITION, READ_APPROVED, permission) &&
320+
shouldSucceed(resourceTypesPermittedByScope, CONDITION, WRITE_APPROVED, permission));
270321
}
271322

323+
// Test beforePatch Binary Resource which does not have a securityContext. Should Succeed
324+
try {
325+
properties.put(FHIRPersistenceEvent.PROPNAME_RESOURCE_TYPE, "Binary");
326+
FHIRPersistenceEvent event = new FHIRPersistenceEvent(binary, properties);
327+
event.setPrevFhirResource(binary);
328+
interceptor.beforePatch(event);
329+
assertTrue(shouldSucceed(resourceTypesPermittedByScope, BINARY, READ_APPROVED, permission) &&
330+
shouldSucceed(resourceTypesPermittedByScope, BINARY, WRITE_APPROVED, permission));
331+
} catch (FHIRPersistenceInterceptorException e) {
332+
assertFalse(shouldSucceed(resourceTypesPermittedByScope, BINARY, READ_APPROVED, permission) &&
333+
shouldSucceed(resourceTypesPermittedByScope, BINARY, WRITE_APPROVED, permission));
334+
}
272335

336+
// Test beforePatch Binary Resource which has a securityContext.
337+
// It should be an exception since securityContext is not supported.
338+
try {
339+
properties.put(FHIRPersistenceEvent.PROPNAME_RESOURCE_TYPE, "Binary");
340+
FHIRPersistenceEvent event = new FHIRPersistenceEvent(binaryWithSecurityContext, properties);
341+
event.setPrevFhirResource(binaryWithSecurityContext);
342+
interceptor.beforePatch(event);
343+
fail("Did not receive the expected FHIRPersistenceInterceptorException");
344+
} catch (FHIRPersistenceInterceptorException e) {
345+
if (shouldSucceed(resourceTypesPermittedByScope, BINARY, READ_APPROVED, permission) &&
346+
shouldSucceed(resourceTypesPermittedByScope, BINARY, WRITE_APPROVED, permission)) {
347+
assertTrue(e.getMessage().equals("securityContext is not supported for resource type Binary"));
348+
}
349+
// else beforePatch was rejected due to normal fhir-smart behavior (non-securityContext-related)
350+
}
273351
}
274352

275353
@Test(dataProvider = "scopeStringProvider")
@@ -1559,6 +1637,7 @@ private Map<String, List<String>> buildRequestHeaders(String scopeString, List<S
15591637

15601638
/**
15611639
* @return true if the interaction should succeed, otherwise false
1640+
* @implNote this method is used to help establish the "expected outcome" of a given interaction
15621641
*/
15631642
private boolean shouldSucceed(Set<ResourceType> resourceTypesPermittedByScope, ResourceType requiredResourceType,
15641643
List<Permission> permissionsPermittedByScope, Permission requiredPermission) {

0 commit comments

Comments
 (0)