From 6bbb403fd19b76de03cfc932760f58dd8b2d24f3 Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Fri, 23 Jul 2021 19:10:19 -0400 Subject: [PATCH] When Client Id Strategy is set to NOT_ALLOWED, permit system requests to create resources (#2827) * resolved 2826 * fix regression --- ...s-when-client-id-strategy-not-allowed.yaml | 5 ++ .../fhir/jpa/dao/BaseHapiFhirResourceDao.java | 50 +++++++----- .../jpa/dao/BaseHapiFhirResourceDaoTest.java | 77 +++++++++++++++++++ 3 files changed, 114 insertions(+), 18 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_5_0/2826-system-create-resources-when-client-id-strategy-not-allowed.yaml create mode 100644 hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDaoTest.java diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_5_0/2826-system-create-resources-when-client-id-strategy-not-allowed.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_5_0/2826-system-create-resources-when-client-id-strategy-not-allowed.yaml new file mode 100644 index 00000000000..8e8ce970217 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_5_0/2826-system-create-resources-when-client-id-strategy-not-allowed.yaml @@ -0,0 +1,5 @@ +--- +type: fix +issue: 2826 +title: "When Client Id Strategy is set to NOT_ALLOWED, permit system requests to create resources, e.g. SearchParameter +and Subscription resources required by the system." diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java index 0ac5d4ab11b..5d96742e857 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java @@ -47,6 +47,7 @@ import ca.uhn.fhir.jpa.model.entity.TagTypeEnum; import ca.uhn.fhir.jpa.model.search.SearchRuntimeDetails; import ca.uhn.fhir.jpa.model.util.JpaConstants; import ca.uhn.fhir.jpa.partition.IRequestPartitionHelperSvc; +import ca.uhn.fhir.jpa.partition.SystemRequestDetails; import ca.uhn.fhir.jpa.patch.FhirPatch; import ca.uhn.fhir.jpa.patch.JsonPatchUtils; import ca.uhn.fhir.jpa.patch.XmlPatchUtils; @@ -126,7 +127,6 @@ import org.springframework.transaction.support.TransactionSynchronizationManager import org.springframework.transaction.support.TransactionTemplate; import javax.annotation.Nonnull; -import javax.annotation.Nullable; import javax.annotation.PostConstruct; import javax.persistence.NoResultException; import javax.persistence.TypedQuery; @@ -228,7 +228,7 @@ public abstract class BaseHapiFhirResourceDao extends B if (isNotBlank(theResource.getIdElement().getIdPart())) { if (getContext().getVersion().getVersion().isOlderThan(FhirVersionEnum.DSTU3)) { - String message = getContext().getLocalizer().getMessageSanitized(BaseHapiFhirResourceDao.class, "failedToCreateWithClientAssignedId", theResource.getIdElement().getIdPart()); + String message = getMessageSanitized("failedToCreateWithClientAssignedId", theResource.getIdElement().getIdPart()); throw new InvalidRequestException(message, createErrorOperationOutcome(message, "processing")); } else { // As of DSTU3, ID and version in the body should be ignored for a create/update @@ -305,21 +305,9 @@ public abstract class BaseHapiFhirResourceDao extends B createForcedIdIfNeeded(entity, theResource.getIdElement(), true); serverAssignedId = true; } else { - switch (getConfig().getResourceClientIdStrategy()) { - case NOT_ALLOWED: - throw new ResourceNotFoundException( - getContext().getLocalizer().getMessageSanitized(BaseHapiFhirResourceDao.class, "failedToCreateWithClientAssignedIdNotAllowed", theResource.getIdElement().getIdPart())); - case ALPHANUMERIC: - if (theResource.getIdElement().isIdPartValidLong()) { - throw new InvalidRequestException( - getContext().getLocalizer().getMessageSanitized(BaseHapiFhirResourceDao.class, "failedToCreateWithClientAssignedNumericId", theResource.getIdElement().getIdPart())); - } - createForcedIdIfNeeded(entity, theResource.getIdElement(), false); - break; - case ANY: - createForcedIdIfNeeded(entity, theResource.getIdElement(), true); - break; - } + validateResourceIdCreation(theResource, theRequest); + boolean createForPureNumericIds = getConfig().getResourceClientIdStrategy() != DaoConfig.ClientIdStrategyEnum.ALPHANUMERIC; + createForcedIdIfNeeded(entity, theResource.getIdElement(), createForPureNumericIds); serverAssignedId = false; } } else { @@ -407,6 +395,32 @@ public abstract class BaseHapiFhirResourceDao extends B return outcome; } + void validateResourceIdCreation(T theResource, RequestDetails theRequest) { + DaoConfig.ClientIdStrategyEnum strategy = getConfig().getResourceClientIdStrategy(); + + if (strategy == DaoConfig.ClientIdStrategyEnum.NOT_ALLOWED) { + if (!isSystemRequest(theRequest)) { + throw new ResourceNotFoundException( + getMessageSanitized("failedToCreateWithClientAssignedIdNotAllowed", theResource.getIdElement().getIdPart())); + } + } + + if (strategy == DaoConfig.ClientIdStrategyEnum.ALPHANUMERIC) { + if (theResource.getIdElement().isIdPartValidLong()) { + throw new InvalidRequestException( + getMessageSanitized("failedToCreateWithClientAssignedNumericId", theResource.getIdElement().getIdPart())); + } + } + } + + protected String getMessageSanitized(String theKey, String theIdPart) { + return getContext().getLocalizer().getMessageSanitized(BaseHapiFhirResourceDao.class, theKey, theIdPart); + } + + private boolean isSystemRequest(RequestDetails theRequest) { + return theRequest instanceof SystemRequestDetails; + } + private IInstanceValidatorModule getInstanceValidator() { return myInstanceValidator; } @@ -632,7 +646,7 @@ public abstract class BaseHapiFhirResourceDao extends B IBaseOperationOutcome oo; if (deletedResources.isEmpty()) { oo = OperationOutcomeUtil.newInstance(getContext()); - String message = getContext().getLocalizer().getMessageSanitized(BaseHapiFhirResourceDao.class, "unableToDeleteNotFound", theUrl); + String message = getMessageSanitized("unableToDeleteNotFound", theUrl); String severity = "warning"; String code = "not-found"; OperationOutcomeUtil.addIssue(getContext(), oo, severity, message, null, code); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDaoTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDaoTest.java new file mode 100644 index 00000000000..bc984843ff4 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDaoTest.java @@ -0,0 +1,77 @@ +package ca.uhn.fhir.jpa.dao; + +import ca.uhn.fhir.jpa.api.config.DaoConfig; +import ca.uhn.fhir.jpa.partition.SystemRequestDetails; +import ca.uhn.fhir.rest.api.server.RequestDetails; +import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; +import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; +import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; +import org.hl7.fhir.r4.model.Patient; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.fail; + +class BaseHapiFhirResourceDaoTest { + TestResourceDao mySvc = new TestResourceDao(); + + @Test + public void validateResourceIdCreation_asSystem() { + Patient patient = new Patient(); + RequestDetails sysRequest = new SystemRequestDetails(); + mySvc.getConfig().setResourceClientIdStrategy(DaoConfig.ClientIdStrategyEnum.NOT_ALLOWED); + mySvc.validateResourceIdCreation(patient, sysRequest); + // no exception is thrown + } + + @Test + public void validateResourceIdCreation_asUser() { + Patient patient = new Patient(); + RequestDetails sysRequest = new ServletRequestDetails(); + mySvc.getConfig().setResourceClientIdStrategy(DaoConfig.ClientIdStrategyEnum.NOT_ALLOWED); + try { + mySvc.validateResourceIdCreation(patient, sysRequest); + fail(); + } catch (ResourceNotFoundException e) { + assertEquals("failedToCreateWithClientAssignedIdNotAllowed", e.getMessage()); + } + } + + @Test + public void validateResourceIdCreationAlpha_withNumber() { + Patient patient = new Patient(); + patient.setId("2401"); + RequestDetails sysRequest = new ServletRequestDetails(); + mySvc.getConfig().setResourceClientIdStrategy(DaoConfig.ClientIdStrategyEnum.ALPHANUMERIC); + try { + mySvc.validateResourceIdCreation(patient, sysRequest); + fail(); + } catch (InvalidRequestException e) { + assertEquals("failedToCreateWithClientAssignedNumericId", e.getMessage()); + } + } + + @Test + public void validateResourceIdCreationAlpha_withAlpha() { + Patient patient = new Patient(); + patient.setId("P2401"); + RequestDetails sysRequest = new ServletRequestDetails(); + mySvc.getConfig().setResourceClientIdStrategy(DaoConfig.ClientIdStrategyEnum.ALPHANUMERIC); + mySvc.validateResourceIdCreation(patient, sysRequest); + // no exception is thrown + } + + static class TestResourceDao extends BaseHapiFhirResourceDao { + private final DaoConfig myDaoConfig = new DaoConfig(); + + @Override + public DaoConfig getConfig() { + return myDaoConfig; + } + + @Override + protected String getMessageSanitized(String theKey, String theIdPart) { + return theKey; + } + } +}