Allow patches to be authorized for DSTU3 transactions (#1529)
* Allow patches to be authorized for DSTU3 transactions * Add changelog
This commit is contained in:
James Agnew
GitHub
parent
a4c9258d38
commit
861ed36f00
|
|
@ -1,16 +1,15 @@
|
|||
package ca.uhn.fhir.util;
|
||||
|
||||
import ca.uhn.fhir.context.*;
|
||||
import ca.uhn.fhir.rest.api.PatchTypeEnum;
|
||||
import ca.uhn.fhir.rest.api.RequestTypeEnum;
|
||||
import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
|
||||
import ca.uhn.fhir.util.bundle.BundleEntryMutator;
|
||||
import ca.uhn.fhir.util.bundle.BundleEntryParts;
|
||||
import ca.uhn.fhir.util.bundle.EntryListAccumulator;
|
||||
import ca.uhn.fhir.util.bundle.ModifiableBundleEntry;
|
||||
import org.apache.commons.lang3.tuple.Pair;
|
||||
import org.hl7.fhir.instance.model.api.IBase;
|
||||
import org.hl7.fhir.instance.model.api.IBaseBundle;
|
||||
import org.hl7.fhir.instance.model.api.IBaseResource;
|
||||
import org.hl7.fhir.instance.model.api.IPrimitiveType;
|
||||
import org.hl7.fhir.instance.model.api.*;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
|
|
@ -253,4 +252,25 @@ public class BundleUtil {
|
|||
}
|
||||
return retVal;
|
||||
}
|
||||
|
||||
/**
|
||||
* DSTU3 did not allow the PATCH verb for transaction bundles- so instead we infer that a bundle
|
||||
* is a patch if the payload is a binary resource containing a patch. This method
|
||||
* tests whether a resource (which should have come from
|
||||
* <code>Bundle.entry.resource</code> is a Binary resource with a patch
|
||||
* payload type.
|
||||
*/
|
||||
public static boolean isDstu3TransactionPatch(IBaseResource thePayloadResource) {
|
||||
boolean isPatch = false;
|
||||
if (thePayloadResource instanceof IBaseBinary) {
|
||||
String contentType = ((IBaseBinary) thePayloadResource).getContentType();
|
||||
try {
|
||||
PatchTypeEnum.forContentTypeOrThrowInvalidRequestException(contentType);
|
||||
isPatch = true;
|
||||
} catch (InvalidRequestException e) {
|
||||
// ignore
|
||||
}
|
||||
}
|
||||
return isPatch;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -21,16 +21,13 @@ package ca.uhn.fhir.jpa.dao.dstu3;
|
|||
*/
|
||||
|
||||
import ca.uhn.fhir.jpa.dao.TransactionProcessor;
|
||||
import ca.uhn.fhir.rest.api.PatchTypeEnum;
|
||||
import ca.uhn.fhir.rest.server.exceptions.BaseServerResponseException;
|
||||
import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
|
||||
import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
|
||||
import ca.uhn.fhir.util.BundleUtil;
|
||||
import org.hl7.fhir.dstu3.model.Bundle;
|
||||
import org.hl7.fhir.dstu3.model.OperationOutcome;
|
||||
import org.hl7.fhir.dstu3.model.Resource;
|
||||
import org.hl7.fhir.dstu3.model.codesystems.IssueType;
|
||||
import org.hl7.fhir.exceptions.FHIRException;
|
||||
import org.hl7.fhir.instance.model.api.IBaseBinary;
|
||||
import org.hl7.fhir.instance.model.api.IBaseOperationOutcome;
|
||||
import org.hl7.fhir.instance.model.api.IBaseResource;
|
||||
|
||||
|
|
@ -117,14 +114,11 @@ public class TransactionProcessorVersionAdapterDstu3 implements TransactionProce
|
|||
* DSTU3 Bundle.entry.request.method (it was added in R4)
|
||||
*/
|
||||
if (isBlank(retVal)) {
|
||||
if (theEntry.getResource() instanceof IBaseBinary) {
|
||||
String contentType = ((IBaseBinary) theEntry.getResource()).getContentType();
|
||||
try {
|
||||
PatchTypeEnum.forContentTypeOrThrowInvalidRequestException(contentType);
|
||||
retVal = "PATCH";
|
||||
} catch (InvalidRequestException e) {
|
||||
// ignore
|
||||
}
|
||||
Resource resource = theEntry.getResource();
|
||||
boolean isPatch = BundleUtil.isDstu3TransactionPatch(resource);
|
||||
|
||||
if (isPatch) {
|
||||
retVal = "PATCH";
|
||||
}
|
||||
}
|
||||
return retVal;
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
package ca.uhn.fhir.rest.server.interceptor.auth;
|
||||
|
||||
import ca.uhn.fhir.context.FhirContext;
|
||||
import ca.uhn.fhir.context.FhirVersionEnum;
|
||||
import ca.uhn.fhir.context.RuntimeResourceDefinition;
|
||||
import ca.uhn.fhir.context.RuntimeSearchParam;
|
||||
import ca.uhn.fhir.interceptor.api.Pointcut;
|
||||
|
|
@ -203,6 +204,14 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ {
|
|||
appliesToResourceId = Collections.singletonList(theInputResourceId);
|
||||
}
|
||||
break;
|
||||
case PATCH:
|
||||
appliesToResource = null;
|
||||
if (theInputResourceId != null) {
|
||||
appliesToResourceId = Collections.singletonList(theInputResourceId);
|
||||
} else {
|
||||
return null;
|
||||
}
|
||||
break;
|
||||
default:
|
||||
return null;
|
||||
}
|
||||
|
|
@ -285,7 +294,14 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ {
|
|||
operation = RestOperationTypeEnum.UPDATE;
|
||||
} else if (nextPart.getRequestType() == RequestTypeEnum.DELETE) {
|
||||
operation = RestOperationTypeEnum.DELETE;
|
||||
} else if (nextPart.getRequestType() == RequestTypeEnum.PATCH) {
|
||||
operation = RestOperationTypeEnum.PATCH;
|
||||
} else if (nextPart.getRequestType() == null && theRequestDetails.getServer().getFhirContext().getVersion().getVersion() == FhirVersionEnum.DSTU3 && BundleUtil.isDstu3TransactionPatch(nextPart.getResource())) {
|
||||
// This is a workaround for the fact that there is no PATCH verb in DSTU3's bundle entry verb type ValueSet.
|
||||
// See BundleUtil#isDstu3TransactionPatch
|
||||
operation = RestOperationTypeEnum.PATCH;
|
||||
} else {
|
||||
|
||||
throw new InvalidRequestException("Can not handle transaction with operation of type " + nextPart.getRequestType());
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,283 @@
|
|||
package ca.uhn.fhir.rest.server.interceptor.auth;
|
||||
|
||||
import ca.uhn.fhir.context.FhirContext;
|
||||
import ca.uhn.fhir.context.api.AddProfileTagEnum;
|
||||
import ca.uhn.fhir.interceptor.api.HookParams;
|
||||
import ca.uhn.fhir.interceptor.api.IInterceptorBroadcaster;
|
||||
import ca.uhn.fhir.interceptor.api.Pointcut;
|
||||
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.server.RequestDetails;
|
||||
import ca.uhn.fhir.rest.param.ReferenceParam;
|
||||
import ca.uhn.fhir.rest.param.TokenAndListParam;
|
||||
import ca.uhn.fhir.rest.server.FifoMemoryPagingProvider;
|
||||
import ca.uhn.fhir.rest.server.IResourceProvider;
|
||||
import ca.uhn.fhir.rest.server.RestfulServer;
|
||||
import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException;
|
||||
import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor.ActionRequestDetails;
|
||||
import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails;
|
||||
import ca.uhn.fhir.rest.server.tenant.UrlBaseTenantIdentificationStrategy;
|
||||
import ca.uhn.fhir.test.utilities.JettyUtil;
|
||||
import ca.uhn.fhir.util.TestUtil;
|
||||
import ca.uhn.fhir.util.UrlUtil;
|
||||
import com.google.common.base.Charsets;
|
||||
import com.google.common.collect.Lists;
|
||||
import org.apache.commons.io.IOUtils;
|
||||
import org.apache.http.HttpEntity;
|
||||
import org.apache.http.HttpResponse;
|
||||
import org.apache.http.client.methods.*;
|
||||
import org.apache.http.entity.ContentType;
|
||||
import org.apache.http.entity.StringEntity;
|
||||
import org.apache.http.impl.client.CloseableHttpClient;
|
||||
import org.apache.http.impl.client.HttpClientBuilder;
|
||||
import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
|
||||
import org.eclipse.jetty.server.Server;
|
||||
import org.eclipse.jetty.servlet.ServletHandler;
|
||||
import org.eclipse.jetty.servlet.ServletHolder;
|
||||
import org.hl7.fhir.dstu3.model.*;
|
||||
import org.hl7.fhir.instance.model.api.IBaseResource;
|
||||
import org.hl7.fhir.instance.model.api.IIdType;
|
||||
import org.junit.*;
|
||||
import org.slf4j.Logger;
|
||||
import org.slf4j.LoggerFactory;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.nio.charset.Charset;
|
||||
import java.nio.charset.StandardCharsets;
|
||||
import java.util.*;
|
||||
import java.util.concurrent.TimeUnit;
|
||||
|
||||
import static org.apache.commons.lang3.StringUtils.isNotBlank;
|
||||
import static org.hamcrest.Matchers.containsString;
|
||||
import static org.junit.Assert.*;
|
||||
|
||||
public class AuthorizationInterceptorDstu3Test {
|
||||
|
||||
private static final String ERR403 = "{\"resourceType\":\"OperationOutcome\",\"issue\":[{\"severity\":\"error\",\"code\":\"processing\",\"diagnostics\":\"Access denied by default policy (no applicable rules)\"}]}";
|
||||
private static final Logger ourLog = LoggerFactory.getLogger(AuthorizationInterceptorDstu3Test.class);
|
||||
private static CloseableHttpClient ourClient;
|
||||
private static String ourConditionalCreateId;
|
||||
private static FhirContext ourCtx = FhirContext.forDstu3();
|
||||
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;
|
||||
|
||||
@Before
|
||||
public void before() {
|
||||
ourCtx.setAddProfileTagWhenEncoding(AddProfileTagEnum.NEVER);
|
||||
ourServlet.getInterceptorService().unregisterAllInterceptors();
|
||||
ourServlet.setTenantIdentificationStrategy(null);
|
||||
ourReturn = null;
|
||||
ourDeleted = null;
|
||||
ourHitMethod = false;
|
||||
ourConditionalCreateId = "1123";
|
||||
}
|
||||
|
||||
private Resource createCarePlan(Integer theId, String theSubjectId) {
|
||||
CarePlan retVal = new CarePlan();
|
||||
if (theId != null) {
|
||||
retVal.setId(new IdType("CarePlan", (long) theId));
|
||||
}
|
||||
retVal.setSubject(new Reference("Patient/" + theSubjectId));
|
||||
return retVal;
|
||||
}
|
||||
|
||||
private Resource createDiagnosticReport(Integer theId, String theSubjectId) {
|
||||
DiagnosticReport retVal = new DiagnosticReport();
|
||||
if (theId != null) {
|
||||
retVal.setId(new IdType("DiagnosticReport", (long) theId));
|
||||
}
|
||||
retVal.getCode().setText("OBS");
|
||||
retVal.setSubject(new Reference(theSubjectId));
|
||||
return retVal;
|
||||
}
|
||||
|
||||
private HttpEntity createFhirResourceEntity(IBaseResource theResource) {
|
||||
String out = ourCtx.newJsonParser().encodeResourceToString(theResource);
|
||||
return new StringEntity(out, ContentType.create(Constants.CT_FHIR_JSON, "UTF-8"));
|
||||
}
|
||||
|
||||
private Resource createObservation(Integer theId, String theSubjectId) {
|
||||
Observation retVal = new Observation();
|
||||
if (theId != null) {
|
||||
retVal.setId(new IdType("Observation", (long) theId));
|
||||
}
|
||||
retVal.getCode().setText("OBS");
|
||||
retVal.setSubject(new Reference(theSubjectId));
|
||||
|
||||
if (theSubjectId != null && theSubjectId.startsWith("#")) {
|
||||
Patient p = new Patient();
|
||||
p.setId(theSubjectId);
|
||||
p.setActive(true);
|
||||
retVal.addContained(p);
|
||||
}
|
||||
|
||||
return retVal;
|
||||
}
|
||||
|
||||
private Organization createOrganization(int theIndex) {
|
||||
Organization retVal = new Organization();
|
||||
retVal.setId("" + theIndex);
|
||||
retVal.setName("Org " + theIndex);
|
||||
return retVal;
|
||||
}
|
||||
|
||||
private Resource createPatient(Integer theId) {
|
||||
Patient retVal = new Patient();
|
||||
if (theId != null) {
|
||||
retVal.setId(new IdType("Patient", (long) theId));
|
||||
}
|
||||
retVal.addName().setFamily("FAM");
|
||||
return retVal;
|
||||
}
|
||||
|
||||
private Resource createPatient(Integer theId, int theVersion) {
|
||||
Resource retVal = createPatient(theId);
|
||||
retVal.setId(retVal.getIdElement().withVersion(Integer.toString(theVersion)));
|
||||
return retVal;
|
||||
}
|
||||
|
||||
private Bundle createTransactionWithPlaceholdersResponseBundle() {
|
||||
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");
|
||||
return output;
|
||||
}
|
||||
|
||||
private String extractResponseAndClose(HttpResponse status) throws IOException {
|
||||
if (status.getEntity() == null) {
|
||||
return null;
|
||||
}
|
||||
String responseContent;
|
||||
responseContent = IOUtils.toString(status.getEntity().getContent(), StandardCharsets.UTF_8);
|
||||
IOUtils.closeQuietly(status.getEntity().getContent());
|
||||
return responseContent;
|
||||
}
|
||||
|
||||
|
||||
@Test
|
||||
public void testTransactionWithPatch() throws IOException {
|
||||
|
||||
ourServlet.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) {
|
||||
@Override
|
||||
public List<IAuthRule> buildRuleList(RequestDetails theRequestDetails) {
|
||||
return new RuleBuilder()
|
||||
.allow("transactions").transaction().withAnyOperation().andApplyNormalRules().andThen()
|
||||
.allow("read patient").patch().allRequests().andThen()
|
||||
.allow().write().resourcesOfType("Patient").withAnyId().andThen()
|
||||
.build();
|
||||
}
|
||||
});
|
||||
|
||||
// Payload
|
||||
Binary binary = new Binary();
|
||||
binary.setContent("{}".getBytes(Charset.defaultCharset()));
|
||||
binary.setContentType(Constants.CT_JSON_PATCH);
|
||||
|
||||
// Request is a transaction with 1 search
|
||||
Bundle requestBundle = new Bundle();
|
||||
requestBundle.setType(Bundle.BundleType.TRANSACTION);
|
||||
requestBundle.addEntry()
|
||||
.setResource(binary)
|
||||
.getRequest()
|
||||
.setUrl(ResourceType.Patient.name() + "/123");
|
||||
|
||||
/*
|
||||
* Response is a transaction response containing the search results
|
||||
*/
|
||||
Bundle responseBundle = new Bundle();
|
||||
responseBundle.setType(Bundle.BundleType.TRANSACTION);
|
||||
ourReturn = Collections.singletonList(responseBundle);
|
||||
|
||||
HttpPost httpPost = new HttpPost("http://localhost:" + ourPort + "/");
|
||||
httpPost.setEntity(createFhirResourceEntity(requestBundle));
|
||||
CloseableHttpResponse status = ourClient.execute(httpPost);
|
||||
String resp = extractResponseAndClose(status);
|
||||
assertEquals(200, status.getStatusLine().getStatusCode());
|
||||
|
||||
}
|
||||
|
||||
|
||||
public static class PlainProvider {
|
||||
|
||||
@History()
|
||||
public List<Resource> history() {
|
||||
ourHitMethod = true;
|
||||
return (ourReturn);
|
||||
}
|
||||
|
||||
@Operation(name = "opName", idempotent = true)
|
||||
public Parameters operation() {
|
||||
ourHitMethod = true;
|
||||
return (Parameters) new Parameters().setId("1");
|
||||
}
|
||||
|
||||
@Operation(name = "process-message", idempotent = true)
|
||||
public Parameters processMessage(@OperationParam(name = "content") Bundle theInput) {
|
||||
ourHitMethod = true;
|
||||
return (Parameters) new Parameters().setId("1");
|
||||
}
|
||||
|
||||
@GraphQL
|
||||
public String processGraphQlRequest(ServletRequestDetails theRequestDetails, @IdParam IIdType theId, @GraphQLQuery String theQuery) {
|
||||
ourHitMethod = true;
|
||||
return "{'foo':'bar'}";
|
||||
}
|
||||
|
||||
@Transaction()
|
||||
public Bundle search(ServletRequestDetails theRequestDetails, IInterceptorBroadcaster theInterceptorBroadcaster, @TransactionParam Bundle theInput) {
|
||||
ourHitMethod = true;
|
||||
if (ourDeleted != null) {
|
||||
for (IBaseResource next : ourDeleted) {
|
||||
HookParams params = new HookParams()
|
||||
.add(IBaseResource.class, next)
|
||||
.add(RequestDetails.class, theRequestDetails)
|
||||
.add(ServletRequestDetails.class, theRequestDetails);
|
||||
theInterceptorBroadcaster.callHooks(Pointcut.STORAGE_PRESTORAGE_RESOURCE_DELETED, params);
|
||||
}
|
||||
}
|
||||
return (Bundle) ourReturn.get(0);
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@AfterClass
|
||||
public static void afterClassClearContext() throws Exception {
|
||||
JettyUtil.closeServer(ourServer);
|
||||
TestUtil.clearAllStaticFieldsForUnitTest();
|
||||
}
|
||||
|
||||
@BeforeClass
|
||||
public static void beforeClass() throws Exception {
|
||||
ourServer = new Server(0);
|
||||
|
||||
PlainProvider plainProvider = new PlainProvider();
|
||||
|
||||
ServletHandler proxyHandler = new ServletHandler();
|
||||
ourServlet = new RestfulServer(ourCtx);
|
||||
ourServlet.setFhirContext(ourCtx);
|
||||
ourServlet.registerProvider(plainProvider);
|
||||
ourServlet.setPagingProvider(new FifoMemoryPagingProvider(100));
|
||||
ourServlet.setDefaultResponseEncoding(EncodingEnum.JSON);
|
||||
ServletHolder servletHolder = new ServletHolder(ourServlet);
|
||||
proxyHandler.addServletWithMapping(servletHolder, "/*");
|
||||
ourServer.setHandler(proxyHandler);
|
||||
JettyUtil.startServer(ourServer);
|
||||
ourPort = JettyUtil.getPortForStartedServer(ourServer);
|
||||
|
||||
PoolingHttpClientConnectionManager connectionManager = new PoolingHttpClientConnectionManager(5000, TimeUnit.MILLISECONDS);
|
||||
HttpClientBuilder builder = HttpClientBuilder.create();
|
||||
builder.setConnectionManager(connectionManager);
|
||||
ourClient = builder.build();
|
||||
|
||||
}
|
||||
|
||||
|
||||
}
|
||||
|
|
@ -340,6 +340,12 @@
|
|||
it hasn't been permitted in the spec for a very long time. Hopefully this change will not
|
||||
impact anyone!
|
||||
</action>
|
||||
<action type="fix" issue="1529">
|
||||
HAPI FHIR allows transactions in DSTU3 to contain a JSON/XML Patch in a Binary resource without
|
||||
specifying a verb in Bundle.entry.request.method, since the valueset defined in DSTU3 for that
|
||||
field does not include the PATCH verb. The AuthorizationInterceptor however did not understand
|
||||
this and would reject these requests. This is now corrected.
|
||||
</action>
|
||||
<action type="fix" issue="1530">
|
||||
A potential XXE vulnerability in the validator was corrected. The XML parser used for validating
|
||||
XML payloads (i.e. FHIR resources) will no longer read from DTD declarations.
|
||||
|
|
|
|||
Loading…
Reference in New Issue