Validate resource type on FHIR transaction updates (#6600)

* Validate resource type on FHIR transaction updates

* Add changelgo

* Spotless
This commit is contained in:
James Agnew 2025-01-09 14:14:39 -05:00 committed by GitHub
parent 544f83279c
commit ac38b42a45
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 55 additions and 18 deletions

View File

@ -107,6 +107,7 @@ ca.uhn.fhir.jpa.dao.BaseStorageDao.cantValidateWithNoResource=No resource suppli
ca.uhn.fhir.jpa.dao.BaseStorageDao.deleteBlockedBecauseDisabled=Resource deletion is not permitted on this server ca.uhn.fhir.jpa.dao.BaseStorageDao.deleteBlockedBecauseDisabled=Resource deletion is not permitted on this server
ca.uhn.fhir.jpa.dao.BaseStorageDao.duplicateCreateForcedId=Can not create entity with ID[{0}], a resource with this ID already exists ca.uhn.fhir.jpa.dao.BaseStorageDao.duplicateCreateForcedId=Can not create entity with ID[{0}], a resource with this ID already exists
ca.uhn.fhir.jpa.dao.BaseStorageDao.failedToCreateWithInvalidId=Can not process entity with ID[{0}], this is not a valid FHIR ID ca.uhn.fhir.jpa.dao.BaseStorageDao.failedToCreateWithInvalidId=Can not process entity with ID[{0}], this is not a valid FHIR ID
ca.uhn.fhir.jpa.dao.BaseStorageDao.failedToCreateWithInvalidIdWrongResourceType=Can not process entity with ID[{0}], this is not the correct resource type for this resource
ca.uhn.fhir.jpa.dao.BaseStorageDao.incorrectResourceType=Incorrect resource type detected for endpoint, found {0} but expected {1} ca.uhn.fhir.jpa.dao.BaseStorageDao.incorrectResourceType=Incorrect resource type detected for endpoint, found {0} but expected {1}
ca.uhn.fhir.jpa.dao.BaseStorageDao.failedToCreateWithClientAssignedNumericId=Can not create resource with ID[{0}], no resource with this ID exists and clients may only assign IDs which contain at least one non-numeric character ca.uhn.fhir.jpa.dao.BaseStorageDao.failedToCreateWithClientAssignedNumericId=Can not create resource with ID[{0}], no resource with this ID exists and clients may only assign IDs which contain at least one non-numeric character
ca.uhn.fhir.jpa.dao.BaseStorageDao.failedToCreateWithClientAssignedId=Can not create resource with ID[{0}], ID must not be supplied on a create (POST) operation ca.uhn.fhir.jpa.dao.BaseStorageDao.failedToCreateWithClientAssignedId=Can not create resource with ID[{0}], ID must not be supplied on a create (POST) operation

View File

@ -0,0 +1,4 @@
---
type: add
issue: 6600
title: "When submitting a FHIR transaction, we previously did not throw an error if a resource was submitted with a resource type in the URL that did not match the actual resource type (e.g. an entry containing a Patient, with the URL `Observation/123`). This will now correctly throw an error."

View File

@ -609,7 +609,8 @@ public class FhirResourceDaoDstu3UpdateTest extends BaseJpaDstu3Test {
myOrganizationDao.update(p2, mySrd); myOrganizationDao.update(p2, mySrd);
fail(""); fail("");
} catch (InvalidRequestException e) { } catch (InvalidRequestException e) {
assertThat(e.getMessage()).contains(Msg.code(996) + "Incorrect resource type"); assertThat(e.getMessage()).contains(Msg.code(2616));
assertThat(e.getMessage()).contains("this is not the correct resource type for this resource");
} }
} }

View File

@ -1,10 +1,5 @@
package ca.uhn.fhir.jpa.dao.dstu3; package ca.uhn.fhir.jpa.dao.dstu3;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.assertFalse;
import ca.uhn.fhir.i18n.Msg; import ca.uhn.fhir.i18n.Msg;
import ca.uhn.fhir.interceptor.api.IInterceptorService; import ca.uhn.fhir.interceptor.api.IInterceptorService;
import ca.uhn.fhir.jpa.api.config.JpaStorageSettings; import ca.uhn.fhir.jpa.api.config.JpaStorageSettings;
@ -33,6 +28,7 @@ import ca.uhn.fhir.util.ClasspathUtil;
import ca.uhn.fhir.util.StopWatch; import ca.uhn.fhir.util.StopWatch;
import com.google.common.base.Charsets; import com.google.common.base.Charsets;
import com.google.common.collect.Maps; import com.google.common.collect.Maps;
import jakarta.annotation.Nonnull;
import org.apache.commons.io.IOUtils; import org.apache.commons.io.IOUtils;
import org.hl7.fhir.dstu3.model.Appointment; import org.hl7.fhir.dstu3.model.Appointment;
import org.hl7.fhir.dstu3.model.Attachment; import org.hl7.fhir.dstu3.model.Attachment;
@ -74,7 +70,6 @@ import org.springframework.transaction.TransactionStatus;
import org.springframework.transaction.support.TransactionCallbackWithoutResult; import org.springframework.transaction.support.TransactionCallbackWithoutResult;
import org.springframework.transaction.support.TransactionTemplate; import org.springframework.transaction.support.TransactionTemplate;
import jakarta.annotation.Nonnull;
import java.io.IOException; import java.io.IOException;
import java.io.InputStream; import java.io.InputStream;
import java.math.BigDecimal; import java.math.BigDecimal;
@ -87,9 +82,12 @@ import java.util.Set;
import java.util.stream.Collectors; import java.util.stream.Collectors;
import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail; import static org.junit.jupiter.api.Assertions.fail;
import static org.junit.jupiter.api.Assertions.fail;
import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.when; import static org.mockito.Mockito.when;
@ -254,34 +252,51 @@ public class FhirSystemDaoDstu3Test extends BaseJpaDstu3SystemTest {
} }
@Test @Test
public void testBatchCreateWithBadRead() { public void testBatchCreateWithBadReadAndBadUpdate() {
Bundle request = new Bundle(); Bundle request = new Bundle();
request.setType(BundleType.BATCH); request.setType(BundleType.BATCH);
Patient p; Patient p0;
p = new Patient(); p0 = new Patient();
p.addIdentifier().setSystem("urn:system").setValue("FOO"); p0.addIdentifier().setSystem("urn:system").setValue("FOO");
Patient p1;
p1 = new Patient();
p1.addIdentifier().setSystem("urn:system").setValue("BAR");
request request
.addEntry() .addEntry()
.setResource(p) .setResource(p0)
.getRequest() .getRequest()
.setMethod(HTTPVerb.POST) .setMethod(HTTPVerb.POST)
.setUrl("Patient"); .setUrl("Patient");
request
.addEntry()
.setResource(p1)
.getRequest()
.setMethod(HTTPVerb.PUT)
.setUrl("Observation/123");
request request
.addEntry() .addEntry()
.getRequest() .getRequest()
.setMethod(HTTPVerb.GET) .setMethod(HTTPVerb.GET)
.setUrl("Patient/BABABABA"); .setUrl("Patient/BABABABA");
ourLog.info("Request:\n{}", myFhirContext.newJsonParser().setPrettyPrint(true).encodeResourceToString(request));
Bundle response = mySystemDao.transaction(mySrd, request); Bundle response = mySystemDao.transaction(mySrd, request);
assertThat(response.getEntry()).hasSize(2); assertThat(response.getEntry()).hasSize(3);
ourLog.info("Response:\n{}", myFhirContext.newJsonParser().setPrettyPrint(true).encodeResourceToString(response));
assertEquals("201 Created", response.getEntry().get(0).getResponse().getStatus()); assertEquals("201 Created", response.getEntry().get(0).getResponse().getStatus());
assertThat(response.getEntry().get(0).getResponse().getLocation()).matches(".*Patient/[0-9]+.*"); assertThat(response.getEntry().get(0).getResponse().getLocation()).matches(".*Patient/[0-9]+.*");
assertEquals("404 Not Found", response.getEntry().get(1).getResponse().getStatus()); assertEquals("400 Bad Request", response.getEntry().get(1).getResponse().getStatus());
assertEquals("404 Not Found", response.getEntry().get(2).getResponse().getStatus());
OperationOutcome oo = (OperationOutcome) response.getEntry().get(1).getResponse().getOutcome(); OperationOutcome oo = (OperationOutcome) response.getEntry().get(2).getResponse().getOutcome();
ourLog.debug(myFhirContext.newXmlParser().setPrettyPrint(true).encodeResourceToString(oo)); ourLog.debug(myFhirContext.newXmlParser().setPrettyPrint(true).encodeResourceToString(oo));
assertEquals(IssueSeverity.ERROR, oo.getIssue().get(0).getSeverity()); assertEquals(IssueSeverity.ERROR, oo.getIssue().get(0).getSeverity());
assertEquals(Msg.code(2001) + "Resource Patient/BABABABA is not known", oo.getIssue().get(0).getDiagnostics()); assertEquals(Msg.code(2001) + "Resource Patient/BABABABA is not known", oo.getIssue().get(0).getDiagnostics());
@ -516,6 +531,7 @@ public class FhirSystemDaoDstu3Test extends BaseJpaDstu3SystemTest {
.setMethod(HTTPVerb.PUT) .setMethod(HTTPVerb.PUT)
.setUrl("Patient/A"); .setUrl("Patient/A");
resp = mySystemDao.transaction(mySrd, bundle); resp = mySystemDao.transaction(mySrd, bundle);
ourLog.info(myFhirContext.newJsonParser().setPrettyPrint(true).encodeResourceToString(resp));
assertThat(resp.getEntry().get(0).getResponse().getLocation()).endsWith("Patient/A/_history/1"); assertThat(resp.getEntry().get(0).getResponse().getLocation()).endsWith("Patient/A/_history/1");
myPatientDao.read(new IdType("Patient/A")); myPatientDao.read(new IdType("Patient/A"));
@ -1115,7 +1131,9 @@ public class FhirSystemDaoDstu3Test extends BaseJpaDstu3SystemTest {
p.addIdentifier().setSystem("urn:system").setValue(methodName); p.addIdentifier().setSystem("urn:system").setValue(methodName);
request.addEntry().setResource(p).getRequest().setMethod(HTTPVerb.PUT).setUrl("Patient/" + methodName); request.addEntry().setResource(p).getRequest().setMethod(HTTPVerb.PUT).setUrl("Patient/" + methodName);
mySystemDao.transaction(mySrd, request); Bundle response = mySystemDao.transaction(mySrd, request);
ourLog.info(myFhirContext.newJsonParser().setPrettyPrint(true).encodeResourceToString(response));
assertEquals(2, response.getEntry().size());
myObservationDao.read(new IdType("Observation/a" + methodName), mySrd); myObservationDao.read(new IdType("Observation/a" + methodName), mySrd);
myPatientDao.read(new IdType("Patient/" + methodName), mySrd); myPatientDao.read(new IdType("Patient/" + methodName), mySrd);

View File

@ -189,6 +189,19 @@ public abstract class BaseStorageDao {
* Verify that the resource ID is actually valid according to FHIR's rules * Verify that the resource ID is actually valid according to FHIR's rules
*/ */
private void verifyResourceIdIsValid(IBaseResource theResource) { private void verifyResourceIdIsValid(IBaseResource theResource) {
if (theResource.getIdElement().hasResourceType()) {
String expectedType = getContext().getResourceType(theResource);
if (!expectedType.equals(theResource.getIdElement().getResourceType())) {
throw new InvalidRequestException(Msg.code(2616)
+ getContext()
.getLocalizer()
.getMessageSanitized(
BaseStorageDao.class,
"failedToCreateWithInvalidIdWrongResourceType",
theResource.getIdElement().toUnqualifiedVersionless()));
}
}
if (theResource.getIdElement().hasIdPart()) { if (theResource.getIdElement().hasIdPart()) {
if (!theResource.getIdElement().isIdPartValid()) { if (!theResource.getIdElement().isIdPartValid()) {
throw new InvalidRequestException(Msg.code(521) throw new InvalidRequestException(Msg.code(521)