Allow authorizing delete operations via a transaction in

AuthorizationInterceptor
This commit is contained in:
James Agnew 2019-01-07 15:43:36 -05:00
parent a8a97ae6b2
commit 62ae71c1c6
4 changed files with 141 additions and 11 deletions

View File

@ -239,6 +239,52 @@ public class AuthorizationInterceptorResourceProviderR4Test extends BaseResource
}
}
@Test
public void testDeleteIsAllowedForCompartmentUsingTransaction() {
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();
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<IAuthRule> buildRuleList(RequestDetails theRequestDetails) {
return new RuleBuilder()
.allow().delete().resourcesOfType(Observation.class).inCompartment("Patient", id).andThen()
.allow().transaction().withAnyOperation().andApplyNormalRules().andThen()
.denyAll()
.build();
}
});
Bundle bundle;
bundle = new Bundle();
bundle.setType(Bundle.BundleType.TRANSACTION);
bundle.addEntry().getRequest().setMethod(Bundle.HTTPVerb.DELETE).setUrl(obsInCompartmentId.toUnqualifiedVersionless().getValue());
ourClient.transaction().withBundle(bundle).execute();
try {
bundle = new Bundle();
bundle.setType(Bundle.BundleType.TRANSACTION);
bundle.addEntry().getRequest().setMethod(Bundle.HTTPVerb.DELETE).setUrl(obsNotInCompartmentId.toUnqualifiedVersionless().getValue());
ourClient.transaction().withBundle(bundle).execute();
fail();
} catch (ForbiddenOperationException e) {
// good
}
}
/**
* See #503
*/

View File

@ -7,7 +7,6 @@ import ca.uhn.fhir.rest.api.RequestTypeEnum;
import ca.uhn.fhir.rest.api.RestOperationTypeEnum;
import ca.uhn.fhir.rest.api.server.RequestDetails;
import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
import ca.uhn.fhir.rest.server.exceptions.PreconditionFailedException;
import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException;
import ca.uhn.fhir.rest.server.interceptor.auth.AuthorizationInterceptor.Verdict;
import ca.uhn.fhir.util.BundleUtil;
@ -38,9 +37,9 @@ import static org.apache.commons.lang3.StringUtils.isNotBlank;
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
*
* http://www.apache.org/licenses/LICENSE-2.0
*
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@ -198,7 +197,7 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ {
for (BundleEntryParts nextPart : inputResources) {
IBaseResource inputResource = nextPart.getResource();
RestOperationTypeEnum operation = null;
RestOperationTypeEnum operation;
if (nextPart.getRequestType() == RequestTypeEnum.GET) {
continue;
} else {
@ -208,18 +207,22 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ {
operation = RestOperationTypeEnum.CREATE;
} else if (nextPart.getRequestType() == RequestTypeEnum.PUT) {
operation = RestOperationTypeEnum.UPDATE;
} else if (nextPart.getRequestType() == RequestTypeEnum.DELETE) {
operation = RestOperationTypeEnum.DELETE;
} else {
throw new InvalidRequestException("Can not handle transaction with operation of type " + nextPart.getRequestType());
}
/*
* This is basically just being conservative - Be careful of transactions containing
* nested operations and nested transactions. We block the by default. At some point
* nested operations and nested transactions. We block them by default. At some point
* it would be nice to be more nuanced here.
*/
RuntimeResourceDefinition resourceDef = ctx.getResourceDefinition(nextPart.getResource());
if ("Parameters".equals(resourceDef.getName()) || "Bundle".equals(resourceDef.getName())) {
throw new InvalidRequestException("Can not handle transaction with nested resource of type " + resourceDef.getName());
if (nextPart.getResource() != null) {
RuntimeResourceDefinition resourceDef = ctx.getResourceDefinition(nextPart.getResource());
if ("Parameters".equals(resourceDef.getName()) || "Bundle".equals(resourceDef.getName())) {
throw new InvalidRequestException("Can not handle transaction with nested resource of type " + resourceDef.getName());
}
}
Verdict newVerdict = theRuleApplier.applyRulesAndReturnDecision(operation, theRequestDetails, inputResource, null, null);

View File

@ -4,8 +4,8 @@ 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.*;
import ca.uhn.fhir.rest.api.Constants;
import ca.uhn.fhir.rest.api.*;
import ca.uhn.fhir.rest.api.server.IRequestOperationCallback;
import ca.uhn.fhir.rest.api.server.RequestDetails;
import ca.uhn.fhir.rest.param.ReferenceParam;
@ -46,7 +46,6 @@ import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
import static org.apache.commons.lang3.StringUtils.isNotBlank;
import static org.hamcrest.Matchers.containsString;
@ -62,6 +61,7 @@ public class AuthorizationInterceptorDstu3Test {
private static boolean ourHitMethod;
private static int ourPort;
private static List<Resource> ourReturn;
private static List<IBaseResource> ourDeleted;
private static Server ourServer;
private static RestfulServer ourServlet;
@ -73,6 +73,7 @@ public class AuthorizationInterceptorDstu3Test {
}
ourServlet.setTenantIdentificationStrategy(null);
ourReturn = null;
ourDeleted = null;
ourHitMethod = false;
ourConditionalCreateId = "1123";
}
@ -598,6 +599,75 @@ public class AuthorizationInterceptorDstu3Test {
assertTrue(ourHitMethod);
}
@Test
public void testDeleteByCompartmentUsingTransaction() throws Exception {
ourServlet.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) {
@Override
public List<IAuthRule> buildRuleList(RequestDetails theRequestDetails) {
return new RuleBuilder()
.allow("Rule 1").delete().resourcesOfType(Patient.class).inCompartment("Patient", new IdType("Patient/1")).andThen()
.allow("Rule 2").delete().resourcesOfType(Observation.class).inCompartment("Patient", new IdType("Patient/1")).andThen()
.allow().transaction().withAnyOperation().andApplyNormalRules().andThen()
.build();
}
});
HttpPost httpPost;
HttpResponse status;
String responseString;
Bundle responseBundle = new Bundle();
responseBundle.setType(Bundle.BundleType.TRANSACTIONRESPONSE);
ourHitMethod = false;
Bundle bundle = new Bundle();
ourReturn = Collections.singletonList(responseBundle);
ourDeleted = Collections.singletonList(createPatient(2));
bundle.setType(Bundle.BundleType.TRANSACTION);
bundle.addEntry().getRequest().setMethod(Bundle.HTTPVerb.DELETE).setUrl("Patient/2");
httpPost = new HttpPost("http://localhost:" + ourPort + "/");
httpPost.setEntity(new StringEntity(ourCtx.newJsonParser().encodeResourceToString(bundle), ContentType.create(Constants.CT_FHIR_JSON_NEW, Charsets.UTF_8)));
status = ourClient.execute(httpPost);
responseString = extractResponseAndClose(status);
assertEquals(responseString,403, status.getStatusLine().getStatusCode());
assertTrue(ourHitMethod);
bundle.getEntry().clear();
bundle.addEntry().getRequest().setMethod(Bundle.HTTPVerb.DELETE).setUrl("Patient/1");
ourReturn = Collections.singletonList(responseBundle);
ourDeleted = Collections.singletonList(createPatient(1));
httpPost = new HttpPost("http://localhost:" + ourPort + "/");
httpPost.setEntity(new StringEntity(ourCtx.newJsonParser().encodeResourceToString(bundle), ContentType.create(Constants.CT_FHIR_JSON_NEW, Charsets.UTF_8)));
status = ourClient.execute(httpPost);
responseString = extractResponseAndClose(status);
assertEquals(responseString,200, status.getStatusLine().getStatusCode());
assertTrue(ourHitMethod);
ourHitMethod = false;
bundle.getEntry().clear();
bundle.addEntry().getRequest().setMethod(Bundle.HTTPVerb.DELETE).setUrl("Observation?subject=Patient/2");
ourReturn = Collections.singletonList(responseBundle);
ourDeleted = Collections.singletonList(createObservation(99, "Patient/2"));
httpPost = new HttpPost("http://localhost:" + ourPort + "/");
httpPost.setEntity(new StringEntity(ourCtx.newJsonParser().encodeResourceToString(bundle), ContentType.create(Constants.CT_FHIR_JSON_NEW, Charsets.UTF_8)));
status = ourClient.execute(httpPost);
responseString = extractResponseAndClose(status);
assertEquals(responseString,403, status.getStatusLine().getStatusCode());
assertTrue(ourHitMethod);
ourHitMethod = false;
bundle.getEntry().clear();
bundle.addEntry().getRequest().setMethod(Bundle.HTTPVerb.DELETE).setUrl("Observation?subject=Patient/1");
ourReturn = Collections.singletonList(responseBundle);
ourDeleted = Collections.singletonList(createObservation(99, "Patient/1"));
httpPost = new HttpPost("http://localhost:" + ourPort + "/");
httpPost.setEntity(new StringEntity(ourCtx.newJsonParser().encodeResourceToString(bundle), ContentType.create(Constants.CT_FHIR_JSON_NEW, Charsets.UTF_8)));
status = ourClient.execute(httpPost);
responseString = extractResponseAndClose(status);
assertEquals(responseString,200, status.getStatusLine().getStatusCode());
assertTrue(ourHitMethod);
}
/**
* #528
*/
@ -3519,11 +3589,18 @@ public class AuthorizationInterceptorDstu3Test {
}
@Transaction()
public Bundle search(@TransactionParam Bundle theInput) {
public Bundle search(IRequestOperationCallback theRequestOperationCallback, @TransactionParam Bundle theInput) {
ourHitMethod = true;
if (ourDeleted != null){
for (IBaseResource next : ourDeleted) {
theRequestOperationCallback.resourceDeleted(next);
}
}
return (Bundle) ourReturn.get(0);
}
}
}

View File

@ -247,6 +247,10 @@
AuthorizationInterceptor now rejects transactions with an invalid or unset request
using an HTTP 422 response Bundle type instead of silently refusing to authorize them.
</action>
<action type="add">
AuthorizationInterceptor is now able to authorize DELETE operations performed via a
transaction operation. Previously these were always denied.
</action>
</release>
<release version="3.6.0" date="2018-11-12" description="Food">
<action type="add">