From ffac599a308c9455614c7d44bba684b264bd566b Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Thu, 23 Nov 2017 06:42:10 -0500 Subject: [PATCH] Credit and tests for #762 --- ...tionInterceptorResourceProviderR4Test.java | 87 +++++++++++++++ .../interceptor/IServerInterceptor.java | 24 ++-- .../AuthorizationInterceptorR4Test.java | 103 ++++++++++++++++++ src/changes/changes.xml | 6 + 4 files changed, 211 insertions(+), 9 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/AuthorizationInterceptorResourceProviderR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/AuthorizationInterceptorResourceProviderR4Test.java index ab1a109eca2..dc9d4ff4572 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/AuthorizationInterceptorResourceProviderR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/AuthorizationInterceptorResourceProviderR4Test.java @@ -376,6 +376,93 @@ public class AuthorizationInterceptorResourceProviderR4Test extends BaseResource } + /** + * See #762 + */ + @Test + public void testInstanceRuleOkForResourceWithNoId2() { + + ourRestServer.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) { + @Override + public List buildRuleList(RequestDetails theRequestDetails) { + return new RuleBuilder() + .allow("transactions").transaction().withAnyOperation().andApplyNormalRules().andThen() + .allow("write patient").write().resourcesOfType(Patient.class).withAnyId().andThen() + .allow("write encounter").write().resourcesOfType(Encounter.class).withAnyId().andThen() + .allow("write condition").write().resourcesOfType(Condition.class).withAnyId().andThen() + .denyAll("deny all") + .build(); + } + }); + + + + // Create a bundle that will be used as a transaction + Bundle bundle = new Bundle(); + bundle.setType(Bundle.BundleType.TRANSACTION); + + + + String encounterId = "123-123"; + String encounterSystem = "http://our.internal.code.system/encounter"; + Encounter encounter = new Encounter(); + + encounter.addIdentifier(new Identifier().setValue(encounterId) + .setSystem(encounterSystem)); + + encounter.setStatus(Encounter.EncounterStatus.FINISHED); + + Patient p = new Patient() + .addIdentifier(new Identifier().setValue("321-321").setSystem("http://our.internal.code.system/patient")); + p.setId(IdDt.newRandomUuid()); + + // add patient to bundle so its created + bundle.addEntry() + .setFullUrl(p.getId()) + .setResource(p) + .getRequest() + .setUrl("Patient") + .setMethod(Bundle.HTTPVerb.POST); + + Reference patientRef = new Reference(p.getId()); + + encounter.setSubject(patientRef); + Condition condition = new Condition() + .setCode(new CodeableConcept().addCoding( + new Coding("http://hl7.org/fhir/icd-10", "S53.40", "FOREARM SPRAIN / STRAIN"))) + .setSubject(patientRef); + + condition.setId(IdDt.newRandomUuid()); + + // add condition to bundle so its created + bundle.addEntry() + .setFullUrl(condition.getId()) + .setResource(condition) + .getRequest() + .setUrl("Condition") + .setMethod(Bundle.HTTPVerb.POST); + + Encounter.DiagnosisComponent dc = new Encounter.DiagnosisComponent(); + + dc.setCondition(new Reference(condition.getId())); + encounter.addDiagnosis(dc); + CodeableConcept reason = new CodeableConcept(); + reason.setText("SLIPPED ON FLOOR,PAIN L) ELBOW"); + encounter.addReason(reason); + + // add encounter to bundle so its created + bundle.addEntry() + .setResource(encounter) + .getRequest() + .setUrl("Encounter") + .setIfNoneExist("identifier=" + encounterSystem + "|" + encounterId) + .setMethod(Bundle.HTTPVerb.POST); + + + Bundle resp = myClient.transaction().withBundle(bundle).execute(); + assertEquals(3, resp.getEntry().size()); + + } private void unregisterInterceptors() { for (IServerInterceptor next : new ArrayList(ourRestServer.getInterceptors())) { diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/IServerInterceptor.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/IServerInterceptor.java index f2ca2158cee..8b35f6413cb 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/IServerInterceptor.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/IServerInterceptor.java @@ -41,6 +41,8 @@ import ca.uhn.fhir.rest.server.exceptions.AuthenticationException; import ca.uhn.fhir.rest.server.exceptions.BaseServerResponseException; import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; +import static org.apache.commons.lang3.StringUtils.isBlank; + /** * Provides methods to intercept requests and responses. Note that implementations of this interface may wish to use * {@link InterceptorAdapter} in order to not need to implement every method. @@ -329,9 +331,9 @@ public interface IServerInterceptor { public static class ActionRequestDetails { private final FhirContext myContext; private final IIdType myId; + private final String myResourceType; private RequestDetails myRequestDetails; private IBaseResource myResource; - private final String myResourceType; public ActionRequestDetails(RequestDetails theRequestDetails) { myId = theRequestDetails.getId(); @@ -346,7 +348,11 @@ public interface IServerInterceptor { } public ActionRequestDetails(RequestDetails theRequestDetails, FhirContext theContext, String theResourceType, IIdType theId) { - myId = theId; + if (theId != null && isBlank(theId.getValue())) { + myId = null; + } else { + myId = theId; + } myResourceType = theResourceType; myContext = theContext; myRequestDetails = theRequestDetails; @@ -409,6 +415,13 @@ public interface IServerInterceptor { return myResource; } + /** + * This method should not be called by client code + */ + public void setResource(IBaseResource theObject) { + myResource = theObject; + } + /** * Returns the resource type this request pertains to, or null if this request is not type specific * (e.g. server-history) @@ -450,13 +463,6 @@ public interface IServerInterceptor { } } - /** - * This method should not be called by client code - */ - public void setResource(IBaseResource theObject) { - myResource = theObject; - } - } } diff --git a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/AuthorizationInterceptorR4Test.java b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/AuthorizationInterceptorR4Test.java index 4e40574d9c7..67a887a34f5 100644 --- a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/AuthorizationInterceptorR4Test.java +++ b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/AuthorizationInterceptorR4Test.java @@ -2,6 +2,7 @@ package ca.uhn.fhir.rest.server.interceptor; import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.context.api.AddProfileTagEnum; +import ca.uhn.fhir.model.primitive.IdDt; import ca.uhn.fhir.rest.annotation.*; import ca.uhn.fhir.rest.api.Constants; import ca.uhn.fhir.rest.api.*; @@ -263,6 +264,108 @@ public class AuthorizationInterceptorR4Test { assertEquals(403, status.getStatusLine().getStatusCode()); } + + /** + * See #762 + */ + @Test + public void testInstanceRuleOkForResourceWithNoId2() throws IOException { + + ourServlet.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) { + @Override + public List buildRuleList(RequestDetails theRequestDetails) { + return new RuleBuilder() + .allow("transactions").transaction().withAnyOperation().andApplyNormalRules().andThen() + .allow("write patient").write().resourcesOfType(Patient.class).withAnyId().andThen() + .allow("write encounter").write().resourcesOfType(Encounter.class).withAnyId().andThen() + .allow("write condition").write().resourcesOfType(Condition.class).withAnyId().andThen() + .denyAll("deny all") + .build(); + } + }); + + + + // Create a bundle that will be used as a transaction + Bundle bundle = new Bundle(); + bundle.setType(Bundle.BundleType.TRANSACTION); + + + + String encounterId = "123-123"; + String encounterSystem = "http://our.internal.code.system/encounter"; + Encounter encounter = new Encounter(); + + encounter.addIdentifier(new Identifier().setValue(encounterId) + .setSystem(encounterSystem)); + + encounter.setStatus(Encounter.EncounterStatus.FINISHED); + + Patient p = new Patient() + .addIdentifier(new Identifier().setValue("321-321").setSystem("http://our.internal.code.system/patient")); + p.setId(IdDt.newRandomUuid()); + + // add patient to bundle so its created + bundle.addEntry() + .setFullUrl(p.getId()) + .setResource(p) + .getRequest() + .setUrl("Patient") + .setMethod(Bundle.HTTPVerb.POST); + + Reference patientRef = new Reference(p.getId()); + + encounter.setSubject(patientRef); + Condition condition = new Condition() + .setCode(new CodeableConcept().addCoding( + new Coding("http://hl7.org/fhir/icd-10", "S53.40", "FOREARM SPRAIN / STRAIN"))) + .setSubject(patientRef); + + condition.setId(IdDt.newRandomUuid()); + + // add condition to bundle so its created + bundle.addEntry() + .setFullUrl(condition.getId()) + .setResource(condition) + .getRequest() + .setUrl("Condition") + .setMethod(Bundle.HTTPVerb.POST); + + Encounter.DiagnosisComponent dc = new Encounter.DiagnosisComponent(); + + dc.setCondition(new Reference(condition.getId())); + encounter.addDiagnosis(dc); + CodeableConcept reason = new CodeableConcept(); + reason.setText("SLIPPED ON FLOOR,PAIN L) ELBOW"); + encounter.addReason(reason); + + // add encounter to bundle so its created + bundle.addEntry() + .setResource(encounter) + .getRequest() + .setUrl("Encounter") + .setIfNoneExist("identifier=" + encounterSystem + "|" + encounterId) + .setMethod(Bundle.HTTPVerb.POST); + + Bundle output = new Bundle(); + output.setType(Bundle.BundleType.TRANSACTIONRESPONSE); + output.addEntry() + .setResource(new Patient().setActive(true)) // don't give this an ID + .getResponse().setLocation("/Patient/1"); + + + ourReturn = Collections.singletonList((Resource) output); + HttpPost httpPost = new HttpPost("http://localhost:" + ourPort + "/"); + httpPost.setEntity(createFhirResourceEntity(bundle)); + CloseableHttpResponse status = ourClient.execute(httpPost); + String resp = extractResponseAndClose(status); + assertEquals(200, status.getStatusLine().getStatusCode()); + + ourLog.info(resp); + + } + + @Test public void testBatchWhenTransactionReadDenied() throws Exception { ourServlet.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) { diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 9ef1e0a1fa8..a8e392dae62 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -240,6 +240,12 @@ The learn more links on the website home page had broken links. Thanks to James Daily for the pull request to fix this! + + Prevent a crash in AuthorizationInterceptor when processing transactions + if the interceptor has rules declared which allow resources to be read/written + by "any ID of a given type". Thanks to GitHub user @dconlan for the pull + request! +