diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_4_0/4441-bad-resource-refs-should-not-create.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_4_0/4441-bad-resource-refs-should-not-create.yaml new file mode 100644 index 00000000000..70f54ca34b1 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_4_0/4441-bad-resource-refs-should-not-create.yaml @@ -0,0 +1,7 @@ +--- +type: fix +issue: 4441 +title: "Creating a resource with an invalid embedded resource reference + would not fail. Even if IsEnforceReferentialIntegrityOnWrite was enabled. + This has been fixed, and invalid references will throw. + " diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/ExtractInlineReferenceParams.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/ExtractInlineReferenceParams.java new file mode 100644 index 00000000000..74fcda0ac53 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/ExtractInlineReferenceParams.java @@ -0,0 +1,54 @@ +package ca.uhn.fhir.jpa.dao.index; + +import ca.uhn.fhir.rest.api.server.RequestDetails; +import ca.uhn.fhir.rest.api.server.storage.TransactionDetails; +import org.hl7.fhir.instance.model.api.IBaseResource; + +public class ExtractInlineReferenceParams { + private IBaseResource myResource; + private TransactionDetails myTransactionDetails; + private RequestDetails myRequestDetails; + private boolean myFailOnInvalidReferences; + + public ExtractInlineReferenceParams( + IBaseResource theResource, + TransactionDetails theTransactionDetails, + RequestDetails theRequest + ) { + myResource = theResource; + myTransactionDetails = theTransactionDetails; + myRequestDetails = theRequest; + } + + public IBaseResource getResource() { + return myResource; + } + + public void setResource(IBaseResource theResource) { + myResource = theResource; + } + + public TransactionDetails getTransactionDetails() { + return myTransactionDetails; + } + + public void setTransactionDetails(TransactionDetails theTransactionDetails) { + myTransactionDetails = theTransactionDetails; + } + + public RequestDetails getRequestDetails() { + return myRequestDetails; + } + + public void setRequestDetails(RequestDetails theRequestDetails) { + myRequestDetails = theRequestDetails; + } + + public boolean isFailOnInvalidReferences() { + return myFailOnInvalidReferences; + } + + public void setFailOnInvalidReferences(boolean theFailOnInvalidReferences) { + myFailOnInvalidReferences = theFailOnInvalidReferences; + } +} diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/SearchParamWithInlineReferencesExtractor.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/SearchParamWithInlineReferencesExtractor.java index 8d61c90a944..eef691e37e2 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/SearchParamWithInlineReferencesExtractor.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/SearchParamWithInlineReferencesExtractor.java @@ -113,7 +113,9 @@ public class SearchParamWithInlineReferencesExtractor { } public void populateFromResource(RequestPartitionId theRequestPartitionId, ResourceIndexedSearchParams theParams, TransactionDetails theTransactionDetails, ResourceTable theEntity, IBaseResource theResource, ResourceIndexedSearchParams theExistingParams, RequestDetails theRequest, boolean theFailOnInvalidReference) { - extractInlineReferences(theResource, theTransactionDetails, theRequest); + ExtractInlineReferenceParams theExtractParams = new ExtractInlineReferenceParams(theResource, theTransactionDetails, theRequest); + theExtractParams.setFailOnInvalidReferences(theFailOnInvalidReference); + extractInlineReferences(theExtractParams); mySearchParamExtractorService.extractFromResource(theRequestPartitionId, theRequest, theParams, theExistingParams, theEntity, theResource, theTransactionDetails, theFailOnInvalidReference); @@ -188,16 +190,32 @@ public class SearchParamWithInlineReferencesExtractor { myContext = theContext; } + @Deprecated + public void extractInlineReferences( + IBaseResource theResource, + TransactionDetails theTransactionDetails, + RequestDetails theRequest + ) { + extractInlineReferences(new ExtractInlineReferenceParams(theResource, theTransactionDetails, theRequest)); + } + /** - * Handle references within the resource that are match URLs, for example references like "Patient?identifier=foo". These match URLs are resolved and replaced with the ID of the + * Handle references within the resource that are match URLs, for example references like "Patient?identifier=foo". + * These match URLs are resolved and replaced with the ID of the * matching resource. + * + * This method is *only* called from UPDATE path */ - public void extractInlineReferences(IBaseResource theResource, TransactionDetails theTransactionDetails, RequestDetails theRequest) { + public void extractInlineReferences(ExtractInlineReferenceParams theParams) { if (!myDaoConfig.isAllowInlineMatchUrlReferences()) { return; } + IBaseResource resource = theParams.getResource(); + RequestDetails theRequest = theParams.getRequestDetails(); + TransactionDetails theTransactionDetails = theParams.getTransactionDetails(); + FhirTerser terser = myContext.newTerser(); - List allRefs = terser.getAllPopulatedChildElementsOfType(theResource, IBaseReference.class); + List allRefs = terser.getAllPopulatedChildElementsOfType(resource, IBaseReference.class); for (IBaseReference nextRef : allRefs) { IIdType nextId = nextRef.getReferenceElement(); String nextIdText = nextId.getValue(); @@ -229,7 +247,6 @@ public class SearchParamWithInlineReferencesExtractor { JpaPid match; if (matches.isEmpty()) { - Optional placeholderOpt = myDaoResourceLinkResolver.createPlaceholderTargetIfConfiguredToDoSo(matchResourceType, nextRef, null, theRequest, theTransactionDetails); if (placeholderOpt.isPresent()) { match = (JpaPid) placeholderOpt.get().getPersistentId(); diff --git a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/BaseSearchParamExtractor.java b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/BaseSearchParamExtractor.java index 1741f11d8ea..69d1a968cf4 100644 --- a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/BaseSearchParamExtractor.java +++ b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/BaseSearchParamExtractor.java @@ -1818,10 +1818,11 @@ public abstract class BaseSearchParamExtractor implements ISearchParamExtractor nextId = valueRef.getResource().getIdElement(); } - if (nextId == null || - nextId.isEmpty() || - nextId.getValue().startsWith("urn:")) { - // Ignore placeholder references + if ( + nextId == null || + nextId.isEmpty() + ) { + // Ignore placeholder references that are blank } else if (!theWantLocalReferences && nextId.getValue().startsWith("#")) { // Ignore local refs unless we specifically want them } else { diff --git a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/SearchParamExtractorService.java b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/SearchParamExtractorService.java index d647ca527cc..425f78fc52f 100644 --- a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/SearchParamExtractorService.java +++ b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/SearchParamExtractorService.java @@ -105,8 +105,10 @@ public class SearchParamExtractorService { } /** - * This method is responsible for scanning a resource for all of the search parameter instances. I.e. for all search parameters defined for - * a given resource type, it extracts the associated indexes and populates {@literal theParams}. + * This method is responsible for scanning a resource for all of the search parameter instances. + * I.e. for all search parameters defined for + * a given resource type, it extracts the associated indexes and populates + * {@literal theParams}. */ public void extractFromResource(RequestPartitionId theRequestPartitionId, RequestDetails theRequestDetails, ResourceIndexedSearchParams theNewParams, ResourceIndexedSearchParams theExistingParams, ResourceTable theEntity, IBaseResource theResource, TransactionDetails theTransactionDetails, boolean theFailOnInvalidReference) { diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/bulk/BulkExportUseCaseTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/bulk/BulkExportUseCaseTest.java index 0ee9854bc5d..d510c26665d 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/bulk/BulkExportUseCaseTest.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/bulk/BulkExportUseCaseTest.java @@ -54,6 +54,7 @@ import java.util.Set; import java.util.stream.Collectors; import java.util.concurrent.TimeUnit; +import static org.apache.commons.lang3.StringUtils.isNotBlank; import static org.awaitility.Awaitility.await; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.notNullValue; @@ -164,7 +165,9 @@ public class BulkExportUseCaseTest extends BaseResourceProviderR4Test { HttpGet statusGet = new HttpGet(pollingLocation); String expectedOriginalUrl = myClient.getServerBase() + "/$export"; try (CloseableHttpResponse status = ourHttpClient.execute(statusGet)) { + assertEquals(200, status.getStatusLine().getStatusCode()); String responseContent = IOUtils.toString(status.getEntity().getContent(), StandardCharsets.UTF_8); + assertTrue(isNotBlank(responseContent), responseContent); ourLog.info(responseContent); diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4Test.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4Test.java index 9bc5c5a4fbf..d9e37b30660 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4Test.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4Test.java @@ -3,6 +3,7 @@ package ca.uhn.fhir.jpa.provider.r4; import ca.uhn.fhir.i18n.HapiLocalizer; import ca.uhn.fhir.i18n.Msg; import ca.uhn.fhir.jpa.api.config.DaoConfig; +import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome; import ca.uhn.fhir.jpa.dao.data.ISearchDao; import ca.uhn.fhir.jpa.entity.Search; import ca.uhn.fhir.jpa.model.entity.ModelConfig; @@ -12,6 +13,7 @@ import ca.uhn.fhir.jpa.model.util.JpaConstants; import ca.uhn.fhir.jpa.model.util.UcumServiceUtil; import ca.uhn.fhir.jpa.provider.BaseResourceProviderR4Test; import ca.uhn.fhir.jpa.search.SearchCoordinatorSvcImpl; +import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.jpa.term.ZipCollectionBuilder; import ca.uhn.fhir.jpa.test.config.TestR4Config; import ca.uhn.fhir.jpa.util.QueryParameterUtils; @@ -27,6 +29,7 @@ import ca.uhn.fhir.rest.api.MethodOutcome; import ca.uhn.fhir.rest.api.PreferReturnEnum; import ca.uhn.fhir.rest.api.SearchTotalModeEnum; import ca.uhn.fhir.rest.api.SummaryEnum; +import ca.uhn.fhir.rest.api.server.SystemRequestDetails; import ca.uhn.fhir.rest.client.apache.ResourceEntity; import ca.uhn.fhir.rest.client.api.IClientInterceptor; import ca.uhn.fhir.rest.client.api.IGenericClient; @@ -309,12 +312,10 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test { assertNotNull(id); assertEquals("resource-security", id.getIdPart()); - } @Test public void createSearchParameter_with2Expressions_succeeds() { - SearchParameter searchParameter = new SearchParameter(); searchParameter.setStatus(Enumerations.PublicationStatus.ACTIVE); @@ -326,7 +327,6 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test { MethodOutcome result = myClient.create().resource(searchParameter).execute(); assertEquals(true, result.getCreated()); - } @Test @@ -454,7 +454,6 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test { assertThat(output, containsString(MSG_PREFIX_INVALID_FORMAT + "">"")); assertEquals(400, resp.getStatusLine().getStatusCode()); } - } @@ -911,7 +910,7 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test { @Test @Disabled - public void test() throws IOException { + public void testMakingQuery() throws IOException { HttpGet get = new HttpGet(myServerBase + "/QuestionnaireResponse?_count=50&status=completed&questionnaire=ARIncenterAbsRecord&_lastUpdated=%3E" + UrlUtil.escapeUrlParam("=2018-01-01") + "&context.organization=O3435"); ourLog.info("*** MAKING QUERY"); ourHttpClient.execute(get); @@ -7556,6 +7555,113 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test { assertTrue(resultIds.contains("Patient/" + patientId + "/_history/2")); } + + private static class CreateResourceInput { + boolean IsEnforceRefOnWrite; + boolean IsEnforceRefOnType; + boolean IsAutoCreatePlaceholderReferences; + + public CreateResourceInput( + boolean theEnforceRefOnWrite, + boolean theEnforceRefOnType, + boolean theAutoCreatePlaceholders + ) { + IsEnforceRefOnWrite = theEnforceRefOnWrite; + IsEnforceRefOnType = theEnforceRefOnType; + IsAutoCreatePlaceholderReferences = theAutoCreatePlaceholders; + } + + @Override + public String toString() { + return "IsEnforceReferentialIntegrityOnWrite : " + + IsEnforceRefOnWrite + "\n" + + "IsEnforceReferenceTargetTypes : " + + IsEnforceRefOnType + "\n" + + "IsAutoCreatePlaceholderReferenceTargets : " + + IsAutoCreatePlaceholderReferences + "\n"; + } + } + + private static List createResourceParameters() { + boolean[] bools = new boolean[] { true, false }; + List input = new ArrayList<>(); + for (boolean bool : bools) { + for (boolean bool2 : bools) { + for (boolean bool3 : bools) { + input.add(new CreateResourceInput(bool, bool2, bool3)); + } + } + } + return input; + } + + @ParameterizedTest + @MethodSource("createResourceParameters") + public void createResource_refIntegrityOnWriteAndRefTargetTypes_throws(CreateResourceInput theInput) { + ourLog.info( + String.format("Test case : \n%s", theInput.toString()) + ); + + String patientStr = """ + { + "resourceType": "Patient", + "managingOrganization": { + "reference": "urn:uuid:d8080e87-1842-46b4-aea0-b65803bc2897" + } + } + """; + IParser parser = myFhirContext.newJsonParser(); + Patient patient = parser.parseResource(Patient.class, patientStr); + + { + List orgs = myOrganizationDao + .search(new SearchParameterMap(), new SystemRequestDetails()) + .getAllResources(); + + assertTrue(orgs == null || orgs.isEmpty()); + } + + boolean isEnforceRefOnWrite = myDaoConfig.isEnforceReferentialIntegrityOnWrite(); + boolean isEnforceRefTargetTypes = myDaoConfig.isEnforceReferenceTargetTypes(); + boolean isAutoCreatePlaceholderReferences = myDaoConfig.isAutoCreatePlaceholderReferenceTargets(); + + try { + // allows resources to be created even if they have local resources that do not exist + myDaoConfig.setEnforceReferentialIntegrityOnWrite(theInput.IsEnforceRefOnWrite); + // ensures target references are using the correct resource type + myDaoConfig.setEnforceReferenceTargetTypes(theInput.IsEnforceRefOnType); + // will create the resource if it does not already exist + myDaoConfig.setAutoCreatePlaceholderReferenceTargets(theInput.IsAutoCreatePlaceholderReferences); + + // should fail + DaoMethodOutcome result = myPatientDao.create(patient, new SystemRequestDetails()); + + // a bad reference can never create a new resource + { + List orgs = myOrganizationDao + .search(new SearchParameterMap(), new SystemRequestDetails()) + .getAllResources(); + + assertTrue(orgs == null || orgs.isEmpty()); + } + + // only if all 3 are true do we expect this to fail + assertFalse( + theInput.IsAutoCreatePlaceholderReferences + && theInput.IsEnforceRefOnType + && theInput.IsEnforceRefOnWrite + ); + } catch (InvalidRequestException ex) { + assertTrue(ex.getMessage().contains( + "Invalid resource reference" + ), ex.getMessage()); + } finally { + myDaoConfig.setEnforceReferentialIntegrityOnWrite(isEnforceRefOnWrite); + myDaoConfig.setEnforceReferenceTargetTypes(isEnforceRefTargetTypes); + myDaoConfig.setAutoCreatePlaceholderReferenceTargets(isAutoCreatePlaceholderReferences); + } + } + @Test public void searchResource_bySourceWithPreserveRequestIdDisabled_isSuccess() { String sourceUri = "http://acme.org"; @@ -8085,5 +8191,4 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test { ); } } - } diff --git a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/api/config/DaoConfig.java b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/api/config/DaoConfig.java index 3dacf4d2d19..d229b057ab6 100644 --- a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/api/config/DaoConfig.java +++ b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/api/config/DaoConfig.java @@ -1630,7 +1630,7 @@ public class DaoConfig { *

* For example, if a patient contains a reference to managing organization Organization/FOO * but FOO is not a valid ID for an organization on the server, the operation will be blocked unless - * this propery has been set to false + * this property has been set to false *

*

* This property can cause confusing results for clients of the server since searches, includes, @@ -1648,7 +1648,7 @@ public class DaoConfig { *

* For example, if a patient contains a reference to managing organization Organization/FOO * but FOO is not a valid ID for an organization on the server, the operation will be blocked unless - * this propery has been set to false + * this property has been set to false *

*

* This property can cause confusing results for clients of the server since searches, includes, diff --git a/pom.xml b/pom.xml index 0a109e6579f..2f3a7bcef3c 100644 --- a/pom.xml +++ b/pom.xml @@ -2,6 +2,7 @@ + 4.0.0 ca.uhn.hapi.fhir hapi-fhir