diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java index 961e7747982..4a280f68e57 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java @@ -159,12 +159,6 @@ public class AuthorizationInterceptor implements IRuleApplier { rules = buildRuleList(theRequestDetails); theRequestDetails.getUserData().put(myRequestRuleListKey, rules); } - // LUKETODO: rules do not contain any reference to PATCH - /* - 0 = {RuleImplOp@40767} "RuleImplOp[testers=,op=TRANSACTION,transactionAppliesToOp=ANY_OPERATION,appliesTo=,appliesToTypes=,classifierCompartmentName=,classifierCompartmentOwners=,classifierType=]" - 1 = {RuleImplOp@40768} "RuleImplOp[testers=,op=WRITE,transactionAppliesToOp=,appliesTo=TYPES,appliesToTypes=[Patient],classifierCompartmentName=,classifierCompartmentOwners=,classifierType=ANY_ID]" - */ - Set flags = getFlags(); ourLog.trace( @@ -175,39 +169,20 @@ public class AuthorizationInterceptor implements IRuleApplier { getResourceTypeOrEmpty(theOutputResource)); Verdict verdict = null; - - // LUKETODO: explicitly rule out superuser - if (theOperation == RestOperationTypeEnum.PATCH) { - // if (rules.stream() - // .filter(RuleImplOp.class::isInstance) - // .map(RuleImplOp.class::cast) - // .noneMatch(rule -> rule.getOp() == RuleOpEnum.PATCH)) { - if (rules.stream().noneMatch(RuleImplPatch.class::isInstance) - && rules.stream() - .filter(RuleImplOp.class::isInstance) - .map(RuleImplOp.class::cast) - .noneMatch(rule -> rule.getOp() == RuleOpEnum.ALL)) { - // LUKETODO: this results in a 403 but is that what we want? - verdict = new Verdict(PolicyEnum.DENY, null); - } - } - - if (verdict == null) { - for (IAuthRule nextRule : rules) { - ourLog.trace("Rule being applied - {}", nextRule); - verdict = nextRule.applyRule( - theOperation, - theRequestDetails, - theInputResource, - theInputResourceId, - theOutputResource, - this, - flags, - thePointcut); - if (verdict != null) { - ourLog.trace("Rule {} returned decision {}", nextRule, verdict.getDecision()); - break; - } + for (IAuthRule nextRule : rules) { + ourLog.trace("Rule being applied - {}", nextRule); + verdict = nextRule.applyRule( + theOperation, + theRequestDetails, + theInputResource, + theInputResourceId, + theOutputResource, + this, + flags, + thePointcut); + if (verdict != null) { + ourLog.trace("Rule {} returned decision {}", nextRule, verdict.getDecision()); + break; } } @@ -422,6 +397,7 @@ public class AuthorizationInterceptor implements IRuleApplier { @Hook(Pointcut.SERVER_INCOMING_REQUEST_PRE_HANDLED) public void incomingRequestPreHandled(RequestDetails theRequest, Pointcut thePointcut) { + ourLog.info("5688: PATCH: SERVER_INCOMING_REQUEST_PRE_HANDLED"); IBaseResource inputResource = null; IIdType inputResourceId = null; @@ -590,6 +566,7 @@ public class AuthorizationInterceptor implements IRuleApplier { IBaseResource theOldResource, IBaseResource theNewResource, Pointcut thePointcut) { + ourLog.info("5688: PATCH: STORAGE_PRESTORAGE_RESOURCE_UPDATED"); if (theOldResource != null) { handleUserOperation(theRequest, theOldResource, RestOperationTypeEnum.UPDATE, thePointcut); } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplPatch.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplPatch.java index f8e22373650..fb7597f155e 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplPatch.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplPatch.java @@ -47,7 +47,7 @@ class RuleImplPatch extends BaseRule { if (myAllRequests) { if (theOperation == RestOperationTypeEnum.PATCH) { - if (theInputResource == null && theOutputResource == null) { +// if (theInputResource == null && theOutputResource == null) { return newVerdict( theOperation, theRequestDetails, @@ -55,7 +55,7 @@ class RuleImplPatch extends BaseRule { theInputResourceId, theOutputResource, theRuleApplier); - } +// } } } diff --git a/hapi-fhir-validation/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptorR4Test.java b/hapi-fhir-validation/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptorR4Test.java index d85cf5e4ee9..3ea29e40388 100644 --- a/hapi-fhir-validation/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptorR4Test.java +++ b/hapi-fhir-validation/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptorR4Test.java @@ -2592,8 +2592,26 @@ public class AuthorizationInterceptorR4Test extends BaseValidationTestWithInline @Test public void testPatchAllowed() throws IOException { Observation obs = new Observation(); + obs.setId("123"); obs.setSubject(new Reference("Patient/999")); + final HttpPut put = new HttpPut(ourServer.getBaseUrl() + "/Observation/123"); + + final String obsAsJson = ourCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(obs); + + put.setEntity(new StringEntity(obsAsJson, ContentType.create(Constants.CT_JSON, Charsets.UTF_8))); + + CloseableHttpResponse status1 = ourClient.execute(put); + final String response = extractResponseAndClose(status1); + + ourLog.info("response: {}", response); + +// final HttpGet httpGet = new HttpGet(ourServer.getBaseUrl() + "/Observation/123"); +// +// CloseableHttpResponse status2 = ourClient.execute(httpGet); +// final String response2 = extractResponseAndClose(status2); +// ourLog.info("response2: {}", response2); + ourServer.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) { @Override public List buildRuleList(RequestDetails theRequestDetails) { @@ -2608,6 +2626,56 @@ public class AuthorizationInterceptorR4Test extends BaseValidationTestWithInline " ]"; HttpPatch patch = new HttpPatch(ourServer.getBaseUrl() + "/Observation/123"); patch.setEntity(new StringEntity(patchBody, ContentType.create(Constants.CT_JSON_PATCH, Charsets.UTF_8))); + ourLog.info("BEFORE execute PATCH"); + CloseableHttpResponse status = ourClient.execute(patch); + ourLog.info("AFTER execute PATCH"); + extractResponseAndClose(status); + assertEquals(200, status.getStatusLine().getStatusCode()); + assertTrue(ourHitMethod); + } + + @Test + public void testPatchAllowedFhirJson() throws IOException { + Observation obs = new Observation(); + obs.setSubject(new Reference("Patient/999")); + + ourServer.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) { + @Override + public List buildRuleList(RequestDetails theRequestDetails) { + return new RuleBuilder() + .allow().patch().allRequests().andThen() + .build(); + } + }); + + String patchBody = + """ + { + "resourceType": "Parameters", + "parameter": [ + { + "name": "operation", + "part": [ + { + "name": "type", + "valueCode": "replace" + }, + { + "name": "path", + "valueString": "Observation/status" + }, + { + "name": "value", + "value": "amended" + } + ] + } + ] + } + """; + + HttpPatch patch = new HttpPatch(ourServer.getBaseUrl() + "/Observation/123"); + patch.setEntity(new StringEntity(patchBody, ContentType.create(Constants.CT_FHIR_JSON_NEW, Charsets.UTF_8))); CloseableHttpResponse status = ourClient.execute(patch); extractResponseAndClose(status); assertEquals(200, status.getStatusLine().getStatusCode());