diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/AuthorizationInterceptorResourceProviderDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/AuthorizationInterceptorResourceProviderDstu3Test.java index 48c68698726..7f6afcbea30 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/AuthorizationInterceptorResourceProviderDstu3Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/AuthorizationInterceptorResourceProviderDstu3Test.java @@ -1,39 +1,39 @@ package ca.uhn.fhir.jpa.provider.dstu3; +import ca.uhn.fhir.model.primitive.IdDt; import ca.uhn.fhir.rest.api.Constants; - -import static org.hamcrest.Matchers.startsWith; -import static org.junit.Assert.*; +import ca.uhn.fhir.rest.api.MethodOutcome; +import ca.uhn.fhir.rest.api.server.RequestDetails; +import ca.uhn.fhir.rest.server.exceptions.ForbiddenOperationException; +import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor; +import ca.uhn.fhir.rest.server.interceptor.auth.AuthorizationInterceptor; +import ca.uhn.fhir.rest.server.interceptor.auth.IAuthRule; +import ca.uhn.fhir.rest.server.interceptor.auth.PolicyEnum; +import ca.uhn.fhir.rest.server.interceptor.auth.RuleBuilder; +import ca.uhn.fhir.util.TestUtil; +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.client.methods.HttpDelete; +import org.apache.http.client.methods.HttpPost; +import org.apache.http.entity.ContentType; +import org.apache.http.entity.StringEntity; +import org.hl7.fhir.dstu3.model.IdType; +import org.hl7.fhir.dstu3.model.Observation; +import org.hl7.fhir.dstu3.model.Observation.ObservationStatus; +import org.hl7.fhir.dstu3.model.Patient; +import org.hl7.fhir.dstu3.model.Reference; +import org.hl7.fhir.instance.model.api.IIdType; +import org.junit.AfterClass; +import org.junit.Test; import java.io.IOException; import java.util.ArrayList; import java.util.List; -import org.apache.http.client.methods.*; -import org.apache.http.entity.ContentType; -import org.apache.http.entity.StringEntity; -import org.hl7.fhir.dstu3.model.*; -import org.hl7.fhir.dstu3.model.Observation.ObservationStatus; -import org.hl7.fhir.instance.model.api.IIdType; -import org.junit.AfterClass; -import org.junit.Test; - -import ca.uhn.fhir.model.primitive.IdDt; -import ca.uhn.fhir.rest.api.MethodOutcome; -import ca.uhn.fhir.rest.api.server.RequestDetails; -import ca.uhn.fhir.rest.server.exceptions.ForbiddenOperationException; -import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor; -import ca.uhn.fhir.rest.server.interceptor.auth.*; -import ca.uhn.fhir.util.TestUtil; +import static org.hamcrest.Matchers.startsWith; +import static org.junit.Assert.*; public class AuthorizationInterceptorResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { - @AfterClass - public static void afterClassClearContext() { - TestUtil.clearAllStaticFieldsForUnitTest(); - } - - @Override public void before() throws Exception { super.before(); @@ -41,96 +41,66 @@ public class AuthorizationInterceptorResourceProviderDstu3Test extends BaseResou unregisterInterceptors(); } - - private void unregisterInterceptors() { - for (IServerInterceptor next : new ArrayList(ourRestServer.getInterceptors())) { - if (next instanceof AuthorizationInterceptor) { - ourRestServer.unregisterInterceptor(next); - } - } - } - /** - * See #503 + * See #667 */ @Test - public void testDeleteIsBlocked() { - + public void testBlockUpdatingPatientUserDoesnNotHaveAccessTo() throws IOException { + Patient pt1 = new Patient(); + pt1.setActive(true); + final IIdType pid1 = ourClient.create().resource(pt1).execute().getId().toUnqualifiedVersionless(); + + Patient pt2 = new Patient(); + pt2.setActive(false); + final IIdType pid2 = ourClient.create().resource(pt2).execute().getId().toUnqualifiedVersionless(); + ourRestServer.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .deny().delete().allResources().withAnyId().andThen() - .allowAll() - .build(); + .allow().write().allResources().inCompartment("Patient", pid1).andThen() + .build(); } }); - - Patient patient = new Patient(); - patient.addIdentifier().setSystem("http://uhn.ca/mrns").setValue("100"); - patient.addName().setFamily("Tester").addGiven("Raghad"); - IIdType id = ourClient.create().resource(patient).execute().getId(); - try { - ourClient.delete().resourceById(id.toUnqualifiedVersionless()).execute(); - fail(); - } catch (ForbiddenOperationException e) { - // good - } - - patient = ourClient.read().resource(Patient.class).withId(id.toUnqualifiedVersionless()).execute(); - assertEquals(id.getValue(), patient.getId()); - } - + Observation obs = new Observation(); + obs.setStatus(ObservationStatus.FINAL); + obs.setSubject(new Reference(pid1)); + IIdType oid = ourClient.create().resource(obs).execute().getId().toUnqualified(); - /** - * See #503 #751 - */ - @Test - public void testDeleteIsAllowedForCompartment() { - - Patient patient = new Patient(); - patient.addIdentifier().setSystem("http://uhn.ca/mrns").setValue("100"); - patient.addName().setFamily("Tester").addGiven("Raghad"); - final IIdType id = ourClient.create().resource(patient).execute().getId(); - - Observation obsInCompartment = new Observation(); - obsInCompartment.setStatus(ObservationStatus.FINAL); - obsInCompartment.getSubject().setReferenceElement(id.toUnqualifiedVersionless()); - IIdType obsInCompartmentId = ourClient.create().resource(obsInCompartment).execute().getId().toUnqualifiedVersionless(); - - // create a 2nd observation to be deleted by url Observation?patient=id - ourClient.create().resource(obsInCompartment).execute().getId().toUnqualifiedVersionless(); - - Observation obsNotInCompartment = new Observation(); - obsNotInCompartment.setStatus(ObservationStatus.FINAL); - IIdType obsNotInCompartmentId = ourClient.create().resource(obsNotInCompartment).execute().getId().toUnqualifiedVersionless(); - + + unregisterInterceptors(); ourRestServer.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow().delete().resourcesOfType(Observation.class).inCompartment("Patient", id).andThen() - .deny().delete().allResources().withAnyId().andThen() - .allowAll() - .build(); + .allow().write().allResources().inCompartment("Patient", pid2).andThen() + .build(); } }); - - ourClient.delete().resourceById(obsInCompartmentId.toUnqualifiedVersionless()).execute(); - ourClient.delete().resourceConditionalByUrl("Observation?patient=" + id.toUnqualifiedVersionless()).execute(); + + /* + * Try to update to a new patient. The user has access to write to things in + * pid2's compartment, so this would normally be ok, but in this case they are overwriting + * a resource that is already in pid1's compartment, which shouldn't be allowed. + */ + obs = new Observation(); + obs.setId(oid); + obs.setStatus(ObservationStatus.FINAL); + obs.setSubject(new Reference(pid2)); try { - ourClient.delete().resourceById(obsNotInCompartmentId.toUnqualifiedVersionless()).execute(); + ourClient.update().resource(obs).execute(); fail(); } catch (ForbiddenOperationException e) { // good } + } @Test public void testCreateConditional() { - + Patient patient = new Patient(); patient.addIdentifier().setSystem("http://uhn.ca/mrns").setValue("100"); patient.addName().setFamily("Tester").addGiven("Raghad"); @@ -139,15 +109,13 @@ public class AuthorizationInterceptorResourceProviderDstu3Test extends BaseResou ourRestServer.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) { @Override public List buildRuleList(RequestDetails theRequestDetails) { - //@formatter:off return new RuleBuilder() .allow("Rule 2").write().allResources().inCompartment("Patient", new IdDt("Patient/" + output1.getId().getIdPart())).andThen() .allow().updateConditional().allResources() .build(); - //@formatter:on } }); - + patient = new Patient(); patient.setId(output1.getId().toUnqualifiedVersionless()); patient.addIdentifier().setSystem("http://uhn.ca/mrns").setValue("100"); @@ -155,7 +123,7 @@ public class AuthorizationInterceptorResourceProviderDstu3Test extends BaseResou MethodOutcome output2 = ourClient.update().resource(patient).conditionalByUrl("Patient?identifier=http://uhn.ca/mrns|100").execute(); assertEquals(output1.getId().getIdPart(), output2.getId().getIdPart()); - + patient = new Patient(); patient.addIdentifier().setSystem("http://uhn.ca/mrns").setValue("100"); patient.addName().setFamily("Tester").addGiven("Raghad"); @@ -180,62 +148,82 @@ public class AuthorizationInterceptorResourceProviderDstu3Test extends BaseResou } /** - * See #667 + * See #503 #751 */ @Test - public void testBlockUpdatingPatientUserDoesnNotHaveAccessTo() throws IOException { - Patient pt1 = new Patient(); - pt1.setActive(true); - final IIdType pid1 = ourClient.create().resource(pt1).execute().getId().toUnqualifiedVersionless(); + public void testDeleteIsAllowedForCompartment() { - Patient pt2 = new Patient(); - pt2.setActive(false); - final IIdType pid2 = ourClient.create().resource(pt2).execute().getId().toUnqualifiedVersionless(); + Patient patient = new Patient(); + patient.addIdentifier().setSystem("http://uhn.ca/mrns").setValue("100"); + patient.addName().setFamily("Tester").addGiven("Raghad"); + final IIdType id = ourClient.create().resource(patient).execute().getId(); + + Observation obsInCompartment = new Observation(); + obsInCompartment.setStatus(ObservationStatus.FINAL); + obsInCompartment.getSubject().setReferenceElement(id.toUnqualifiedVersionless()); + IIdType obsInCompartmentId = ourClient.create().resource(obsInCompartment).execute().getId().toUnqualifiedVersionless(); + + // create a 2nd observation to be deleted by url Observation?patient=id + ourClient.create().resource(obsInCompartment).execute().getId().toUnqualifiedVersionless(); + + Observation obsNotInCompartment = new Observation(); + obsNotInCompartment.setStatus(ObservationStatus.FINAL); + IIdType obsNotInCompartmentId = ourClient.create().resource(obsNotInCompartment).execute().getId().toUnqualifiedVersionless(); ourRestServer.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow().write().allResources().inCompartment("Patient", pid1).andThen() + .allow().delete().resourcesOfType(Observation.class).inCompartment("Patient", id).andThen() + .deny().delete().allResources().withAnyId().andThen() + .allowAll() .build(); } }); - - Observation obs = new Observation(); - obs.setStatus(ObservationStatus.FINAL); - obs.setSubject(new Reference(pid1)); - IIdType oid = ourClient.create().resource(obs).execute().getId().toUnqualified(); - - unregisterInterceptors(); - ourRestServer.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) { - @Override - public List buildRuleList(RequestDetails theRequestDetails) { - return new RuleBuilder() - .allow().write().allResources().inCompartment("Patient", pid2).andThen() - .build(); - } - }); - - /* - * Try to update to a new patient. The user has access to write to things in - * pid2's compartment, so this would normally be ok, but in this case they are overwriting - * a resource that is already in pid1's compartment, which shouldn't be allowed. - */ - obs = new Observation(); - obs.setId(oid); - obs.setStatus(ObservationStatus.FINAL); - obs.setSubject(new Reference(pid2)); - + ourClient.delete().resourceById(obsInCompartmentId.toUnqualifiedVersionless()).execute(); + ourClient.delete().resourceConditionalByUrl("Observation?patient=" + id.toUnqualifiedVersionless()).execute(); + try { - ourClient.update().resource(obs).execute(); + ourClient.delete().resourceById(obsNotInCompartmentId.toUnqualifiedVersionless()).execute(); fail(); } catch (ForbiddenOperationException e) { // good } - } - + + /** + * See #503 + */ + @Test + public void testDeleteIsBlocked() { + + ourRestServer.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) { + @Override + public List buildRuleList(RequestDetails theRequestDetails) { + return new RuleBuilder() + .deny().delete().allResources().withAnyId().andThen() + .allowAll() + .build(); + } + }); + + Patient patient = new Patient(); + patient.addIdentifier().setSystem("http://uhn.ca/mrns").setValue("100"); + patient.addName().setFamily("Tester").addGiven("Raghad"); + IIdType id = ourClient.create().resource(patient).execute().getId(); + + try { + ourClient.delete().resourceById(id.toUnqualifiedVersionless()).execute(); + fail(); + } catch (ForbiddenOperationException e) { + // good + } + + patient = ourClient.read().resource(Patient.class).withId(id.toUnqualifiedVersionless()).execute(); + assertEquals(id.getValue(), patient.getId()); + } + @Test public void testDeleteResourceConditional() throws IOException { String methodName = "testDeleteResourceConditional"; @@ -284,7 +272,7 @@ public class AuthorizationInterceptorResourceProviderDstu3Test extends BaseResou //@formatter:on } }); - + HttpDelete delete = new HttpDelete(ourServerBase + "/Patient?name=" + methodName); response = ourHttpClient.execute(delete); try { @@ -303,4 +291,17 @@ public class AuthorizationInterceptorResourceProviderDstu3Test extends BaseResou } + private void unregisterInterceptors() { + for (IServerInterceptor next : new ArrayList(ourRestServer.getInterceptors())) { + if (next instanceof AuthorizationInterceptor) { + ourRestServer.unregisterInterceptor(next); + } + } + } + + @AfterClass + public static void afterClassClearContext() { + TestUtil.clearAllStaticFieldsForUnitTest(); + } + }