Rel 5 7 mergeback (#3401)

* fix handling of common search parameters

* Revert "fix handling of common search parameters"

This reverts commit 89c45eebdc.

* Fix implementation, add test (#3378)

* Fix implementation, add test

* Tighten test

* Rip out dead modules

* Add changelog

* Jr 20220210 handle common search params in contained searches (#3377)

* fix handling of common search parameters

* add support for reference search parameters with multiple paths

* Issue 3357

* Version bump

* Fixed null pointer exception for re-loading subscription on cdr restart and when there's no partition id in the request, and added tests

* added changelogs for this fix

* Fix broken changelog file

* Can't specify specific resource type permissions for bulk export (#3376)

* deny user from exporting without perms

* add unit tests

* add changelog

Co-authored-by: olivia-you <olivia.you@smilecdr.com>

* Make migration donothing as it was added in error

* Remove ehcache

* Add version.yaml:

* Add to sources/javadocs for dist

* Fix typo

* fix up pom

* wip test removing checkstyle plugin from deployable pom

* remove test pom changes, instead just dont deploy to sonatype

* typo

* Remove ehcache

Co-authored-by: Jason Roberts <jason.roberts@smilecdr.com>
Co-authored-by: JasonRoberts-smile <85363818+JasonRoberts-smile@users.noreply.github.com>
Co-authored-by: Mark Iantorno <markiantorno@gmail.com>
Co-authored-by: Steven Li <steven@smilecdr.com>
Co-authored-by: Olivia You <46392181+oliviayou@users.noreply.github.com>
Co-authored-by: olivia-you <olivia.you@smilecdr.com>
This commit is contained in:
Tadgh 2022-02-18 11:22:44 -08:00 committed by GitHub
parent 0c5d868f44
commit 3561672fe7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
21 changed files with 515 additions and 90 deletions

View File

@ -21,8 +21,13 @@
<artifactId>hapi-fhir-base</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>${project.groupId}</groupId>
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>hapi-fhir-checkstyle</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>hapi-fhir-utilities</artifactId>
<version>${project.version}</version>
</dependency>

View File

@ -42,7 +42,7 @@
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-deploy-plugin</artifactId>
<configuration>
<skip>false</skip>
<skip>true</skip>
</configuration>
</plugin>
</plugins>

View File

@ -92,8 +92,6 @@
<classifier>javadoc</classifier>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>ca.uhn.hapi.fhir</groupId>
<artifactId>hapi-fhir-structures-dstu2</artifactId>

View File

@ -0,0 +1,5 @@
---
type: fix
issue: 2791
jira: 3710
title: "Users were able to access data on partitions they did not have access to by using pagination links generated by users who did have access."

View File

@ -0,0 +1,4 @@
---
type: fix
issue: 3374
title: "Chained searches will handle common search parameters correctly when the `Index Contained Resources` configuration parameter is enabled."

View File

@ -0,0 +1,6 @@
---
type: fix
issue: 3392
title: "When cross-partition reference Mode is used, the rest-hook subscriptions on a partition enabled server would
cause a NPE. Cause of this is from the reloading of the subscription when the server is restarted.
This issue has been fixed. Also fixed issue with revinclude for rest-hook subscription not working."

View File

@ -0,0 +1,4 @@
---
type: fix
issue: 3394
title: "Bulk export permissions were over-permissive in RuleBulkExportImpl. This has been corrected."

View File

@ -0,0 +1,3 @@
---
release-date: "2022-02-17"
codename: "Sojourner"

View File

@ -41,16 +41,6 @@
<groupId>org.hibernate</groupId>
<artifactId>hibernate-java8</artifactId>
</dependency>
<dependency>
<groupId>org.hibernate</groupId>
<artifactId>hibernate-ehcache</artifactId>
<exclusions>
<exclusion>
<groupId>net.sf.ehcache</groupId>
<artifactId>ehcache-core</artifactId>
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>org.hibernate.validator</groupId>
<artifactId>hibernate-validator</artifactId>

View File

@ -177,6 +177,7 @@ public class HapiFhirJpaMigrationTasks extends BaseMigrationTasks<VersionEnum> {
.addIndex("20211210.4", "FK_FORCEDID_RESOURCE")
.unique(true)
.withColumns("RESOURCE_PID")
.doNothing()//This migration was added in error, as this table already has a unique constraint on RESOURCE_PID and every database creates an index on anything that is unique.
.onlyAppliesToPlatforms(NON_AUTOMATIC_FK_INDEX_PLATFORMS);
}

View File

@ -22,6 +22,7 @@ package ca.uhn.fhir.jpa.search;
import ca.uhn.fhir.jpa.api.dao.DaoRegistry;
import ca.uhn.fhir.jpa.dao.SearchBuilderFactory;
import ca.uhn.fhir.jpa.partition.IRequestPartitionHelperSvc;
import ca.uhn.fhir.rest.api.server.IBundleProvider;
import ca.uhn.fhir.rest.api.server.RequestDetails;
import ca.uhn.fhir.rest.server.BasePagingProvider;
@ -38,6 +39,8 @@ public class DatabaseBackedPagingProvider extends BasePagingProvider {
private SearchBuilderFactory mySearchBuilderFactory;
@Autowired
private PersistedJpaBundleProviderFactory myPersistedJpaBundleProviderFactory;
@Autowired
private IRequestPartitionHelperSvc myRequestPartitionHelperSvc;
/**
* Constructor
@ -57,6 +60,7 @@ public class DatabaseBackedPagingProvider extends BasePagingProvider {
@Override
public synchronized IBundleProvider retrieveResultList(RequestDetails theRequestDetails, String theId) {
myRequestPartitionHelperSvc.determineReadPartitionForRequestForSearchType(theRequestDetails, "Bundle", null, null);
PersistedJpaBundleProvider provider = myPersistedJpaBundleProviderFactory.newInstance(theRequestDetails, theId);
if (!provider.ensureSearchEntityLoaded()) {
return null;

View File

@ -730,32 +730,34 @@ public class QueryStack {
private class ChainElement {
private final String myResourceType;
private final RuntimeSearchParam mySearchParam;
private final String mySearchParameterName;
private final String myPath;
public ChainElement(String theResourceType, RuntimeSearchParam theSearchParam) {
public ChainElement(String theResourceType, String theSearchParameterName, String thePath) {
this.myResourceType = theResourceType;
this.mySearchParam = theSearchParam;
this.mySearchParameterName = theSearchParameterName;
this.myPath = thePath;
}
public String getResourceType() {
return myResourceType;
}
public RuntimeSearchParam getSearchParam() {
return mySearchParam;
}
public String getPath() { return myPath; }
public String getSearchParameterName() { return mySearchParameterName; }
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
ChainElement that = (ChainElement) o;
return myResourceType.equals(that.myResourceType) && mySearchParam.equals(that.mySearchParam);
return myResourceType.equals(that.myResourceType) && mySearchParameterName.equals(that.mySearchParameterName) && myPath.equals(that.myPath);
}
@Override
public int hashCode() {
return Objects.hash(myResourceType, mySearchParam);
return Objects.hash(myResourceType, mySearchParameterName, myPath);
}
}
@ -768,25 +770,40 @@ public class QueryStack {
return split(theReferenceParam.getChain(), '.').length <= 3;
}
private List<String> extractPaths(String theResourceType, RuntimeSearchParam theSearchParam) {
List<String> pathsForType = theSearchParam.getPathsSplit().stream()
.map(String::trim)
.filter(t -> t.startsWith(theResourceType))
.collect(Collectors.toList());
if (pathsForType.isEmpty()) {
ourLog.warn("Search parameter {} does not have a path for resource type {}.", theSearchParam.getName(), theResourceType);
}
return pathsForType;
}
public void deriveChains(String theResourceType, RuntimeSearchParam theSearchParam, List<? extends IQueryParameterType> theList) {
List<ChainElement> searchParams = Lists.newArrayList();
searchParams.add(new ChainElement(theResourceType, theSearchParam));
for (IQueryParameterType nextOr : theList) {
String targetValue = nextOr.getValueAsQueryToken(myFhirContext);
if (nextOr instanceof ReferenceParam) {
ReferenceParam referenceParam = (ReferenceParam) nextOr;
List<String> paths = extractPaths(theResourceType, theSearchParam);
for (String path : paths) {
List<ChainElement> searchParams = Lists.newArrayList();
searchParams.add(new ChainElement(theResourceType, theSearchParam.getName(), path));
for (IQueryParameterType nextOr : theList) {
String targetValue = nextOr.getValueAsQueryToken(myFhirContext);
if (nextOr instanceof ReferenceParam) {
ReferenceParam referenceParam = (ReferenceParam) nextOr;
if (!isReferenceParamValid(referenceParam)) {
throw new InvalidRequestException(Msg.code(2007) +
"The search chain " + theSearchParam.getName() + "." + referenceParam.getChain() +
if (!isReferenceParamValid(referenceParam)) {
throw new InvalidRequestException(Msg.code(2007) +
"The search chain " + theSearchParam.getName() + "." + referenceParam.getChain() +
" is too long. Only chains up to three references are supported.");
}
String targetChain = referenceParam.getChain();
List<String> qualifiers = Lists.newArrayList(referenceParam.getResourceType());
processNextLinkInChain(searchParams, theSearchParam, targetChain, targetValue, qualifiers, referenceParam.getResourceType());
}
String targetChain = referenceParam.getChain();
List<String> qualifiers = Lists.newArrayList(referenceParam.getResourceType());
processNextLinkInChain(searchParams, theSearchParam, targetChain, targetValue, qualifiers, referenceParam.getResourceType());
}
}
}
@ -822,9 +839,6 @@ public class QueryStack {
searchParamFound = true;
// If we find a search param on this resource type for this parameter name, keep iterating
// Otherwise, abandon this branch and carry on to the next one
List<ChainElement> searchParamBranch = Lists.newArrayList();
searchParamBranch.addAll(theSearchParams);
if (StringUtils.isEmpty(nextChain)) {
// We've reached the end of the chain
ArrayList<IQueryParameterType> orValues = Lists.newArrayList();
@ -837,16 +851,21 @@ public class QueryStack {
orValues.add(qp);
}
Set<LeafNodeDefinition> leafNodes = myChains.get(searchParamBranch);
Set<LeafNodeDefinition> leafNodes = myChains.get(theSearchParams);
if (leafNodes == null) {
leafNodes = Sets.newHashSet();
myChains.put(searchParamBranch, leafNodes);
myChains.put(theSearchParams, leafNodes);
}
leafNodes.add(new LeafNodeDefinition(nextSearchParam, orValues, nextTarget, nextParamName, "", qualifiersBranch));
} else {
List<String> nextPaths = extractPaths(nextTarget, nextSearchParam);
for (String nextPath : nextPaths) {
List<ChainElement> searchParamBranch = Lists.newArrayList();
searchParamBranch.addAll(theSearchParams);
searchParamBranch.add(new ChainElement(nextTarget, nextSearchParam));
processNextLinkInChain(searchParamBranch, nextSearchParam, nextChain, theTargetValue, qualifiersBranch, nextQualifier);
searchParamBranch.add(new ChainElement(nextTarget, nextSearchParam.getName(), nextPath));
processNextLinkInChain(searchParamBranch, nextSearchParam, nextChain, theTargetValue, qualifiersBranch, nextQualifier);
}
}
}
}
@ -988,65 +1007,65 @@ public class QueryStack {
// Note: the first element in each chain is assumed to be discrete. This may need to change when we add proper support for `_contained`
if (nextChain.size() == 1) {
// discrete -> discrete
updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(nextChain.get(0).getSearchParam().getPath()), leafNodes);
updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(nextChain.get(0).getPath()), leafNodes);
// discrete -> contained
updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(),
leafNodes
.stream()
.map(t -> t.withPathPrefix(nextChain.get(0).getResourceType(), nextChain.get(0).getSearchParam().getName()))
.map(t -> t.withPathPrefix(nextChain.get(0).getResourceType(), nextChain.get(0).getSearchParameterName()))
.collect(Collectors.toSet()));
} else if (nextChain.size() == 2) {
// discrete -> discrete -> discrete
updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(nextChain.get(0).getSearchParam().getPath(), nextChain.get(1).getSearchParam().getPath()), leafNodes);
updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(nextChain.get(0).getPath(), nextChain.get(1).getPath()), leafNodes);
// discrete -> discrete -> contained
updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(nextChain.get(0).getSearchParam().getPath()),
updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(nextChain.get(0).getPath()),
leafNodes
.stream()
.map(t -> t.withPathPrefix(nextChain.get(1).getResourceType(), nextChain.get(1).getSearchParam().getName()))
.map(t -> t.withPathPrefix(nextChain.get(1).getResourceType(), nextChain.get(1).getSearchParameterName()))
.collect(Collectors.toSet()));
// discrete -> contained -> discrete
updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(mergePaths(nextChain.get(0).getSearchParam().getPath(), nextChain.get(1).getSearchParam().getPath())), leafNodes);
updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(mergePaths(nextChain.get(0).getPath(), nextChain.get(1).getPath())), leafNodes);
if (myModelConfig.isIndexOnContainedResourcesRecursively()) {
// discrete -> contained -> contained
updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(),
leafNodes
.stream()
.map(t -> t.withPathPrefix(nextChain.get(0).getResourceType(), nextChain.get(0).getSearchParam().getName() + "." + nextChain.get(1).getSearchParam().getName()))
.map(t -> t.withPathPrefix(nextChain.get(0).getResourceType(), nextChain.get(0).getSearchParameterName() + "." + nextChain.get(1).getSearchParameterName()))
.collect(Collectors.toSet()));
}
} else if (nextChain.size() == 3) {
// discrete -> discrete -> discrete -> discrete
updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(nextChain.get(0).getSearchParam().getPath(), nextChain.get(1).getSearchParam().getPath(), nextChain.get(2).getSearchParam().getPath()), leafNodes);
updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(nextChain.get(0).getPath(), nextChain.get(1).getPath(), nextChain.get(2).getPath()), leafNodes);
// discrete -> discrete -> discrete -> contained
updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(nextChain.get(0).getSearchParam().getPath(), nextChain.get(1).getSearchParam().getPath()),
updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(nextChain.get(0).getPath(), nextChain.get(1).getPath()),
leafNodes
.stream()
.map(t -> t.withPathPrefix(nextChain.get(2).getResourceType(), nextChain.get(2).getSearchParam().getName()))
.map(t -> t.withPathPrefix(nextChain.get(2).getResourceType(), nextChain.get(2).getSearchParameterName()))
.collect(Collectors.toSet()));
// discrete -> discrete -> contained -> discrete
updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(nextChain.get(0).getSearchParam().getPath(), mergePaths(nextChain.get(1).getSearchParam().getPath(), nextChain.get(2).getSearchParam().getPath())), leafNodes);
updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(nextChain.get(0).getPath(), mergePaths(nextChain.get(1).getPath(), nextChain.get(2).getPath())), leafNodes);
// discrete -> contained -> discrete -> discrete
updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(mergePaths(nextChain.get(0).getSearchParam().getPath(), nextChain.get(1).getSearchParam().getPath()), nextChain.get(2).getSearchParam().getPath()), leafNodes);
updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(mergePaths(nextChain.get(0).getPath(), nextChain.get(1).getPath()), nextChain.get(2).getPath()), leafNodes);
// discrete -> contained -> discrete -> contained
updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(mergePaths(nextChain.get(0).getSearchParam().getPath(), nextChain.get(1).getSearchParam().getPath())),
updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(mergePaths(nextChain.get(0).getPath(), nextChain.get(1).getPath())),
leafNodes
.stream()
.map(t -> t.withPathPrefix(nextChain.get(2).getResourceType(), nextChain.get(2).getSearchParam().getName()))
.map(t -> t.withPathPrefix(nextChain.get(2).getResourceType(), nextChain.get(2).getSearchParameterName()))
.collect(Collectors.toSet()));
if (myModelConfig.isIndexOnContainedResourcesRecursively()) {
// discrete -> contained -> contained -> discrete
updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(mergePaths(nextChain.get(0).getSearchParam().getPath(), nextChain.get(1).getSearchParam().getPath(), nextChain.get(2).getSearchParam().getPath())), leafNodes);
updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(mergePaths(nextChain.get(0).getPath(), nextChain.get(1).getPath(), nextChain.get(2).getPath())), leafNodes);
// discrete -> discrete -> contained -> contained
updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(nextChain.get(0).getSearchParam().getPath()),
updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(nextChain.get(0).getPath()),
leafNodes
.stream()
.map(t -> t.withPathPrefix(nextChain.get(1).getResourceType(), nextChain.get(1).getSearchParam().getName() + "." + nextChain.get(2).getSearchParam().getName()))
.map(t -> t.withPathPrefix(nextChain.get(1).getResourceType(), nextChain.get(1).getSearchParameterName() + "." + nextChain.get(2).getSearchParameterName()))
.collect(Collectors.toSet()));
// discrete -> contained -> contained -> contained
updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(),
leafNodes
.stream()
.map(t -> t.withPathPrefix(nextChain.get(0).getResourceType(), nextChain.get(0).getSearchParam().getName() + "." + nextChain.get(1).getSearchParam().getName() + "." + nextChain.get(2).getSearchParam().getName()))
.map(t -> t.withPathPrefix(nextChain.get(0).getResourceType(), nextChain.get(0).getSearchParameterName() + "." + nextChain.get(1).getSearchParameterName() + "." + nextChain.get(2).getSearchParameterName()))
.collect(Collectors.toSet()));
}
} else {

View File

@ -2,6 +2,7 @@ package ca.uhn.fhir.jpa.dao.r4;
import ca.uhn.fhir.i18n.Msg;
import ca.uhn.fhir.jpa.api.config.DaoConfig;
import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao;
import ca.uhn.fhir.jpa.model.entity.ModelConfig;
import ca.uhn.fhir.jpa.searchparam.MatchUrlService;
import ca.uhn.fhir.jpa.searchparam.ResourceSearch;
@ -11,12 +12,17 @@ import ca.uhn.fhir.parser.StrictErrorHandler;
import ca.uhn.fhir.rest.api.server.IBundleProvider;
import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
import org.hl7.fhir.instance.model.api.IIdType;
import org.hl7.fhir.r4.model.AuditEvent;
import org.hl7.fhir.r4.model.Device;
import org.hl7.fhir.r4.model.DomainResource;
import org.hl7.fhir.r4.model.Encounter;
import org.hl7.fhir.r4.model.IdType;
import org.hl7.fhir.r4.model.Location;
import org.hl7.fhir.r4.model.Observation;
import org.hl7.fhir.r4.model.Organization;
import org.hl7.fhir.r4.model.Patient;
import org.hl7.fhir.r4.model.Quantity;
import org.hl7.fhir.r4.model.Reference;
import org.hl7.fhir.r4.model.StringType;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
@ -130,6 +136,144 @@ public class ChainingR4SearchTest extends BaseJpaR4Test {
assertThat(oids, contains(oid1.getIdPart()));
}
@Test
public void testShouldResolveATwoLinkChainWithStandAloneResources_CommonReference() throws Exception {
// setup
myModelConfig.setIndexOnContainedResources(true);
IIdType oid1;
{
Patient p = new Patient();
p.setId(IdType.newRandomUuid());
p.addName().setFamily("Smith").addGiven("John");
myPatientDao.create(p, mySrd);
Encounter encounter = new Encounter();
encounter.setId(IdType.newRandomUuid());
encounter.addIdentifier().setSystem("foo").setValue("bar");
myEncounterDao.create(encounter, mySrd);
Observation obs = new Observation();
obs.getCode().setText("Body Weight");
obs.getCode().addCoding().setCode("obs2").setSystem("Some System").setDisplay("Body weight as measured by me");
obs.setStatus(Observation.ObservationStatus.FINAL);
obs.setValue(new Quantity(81));
obs.setSubject(new Reference(p.getId()));
obs.setEncounter(new Reference(encounter.getId()));
oid1 = myObservationDao.create(obs, mySrd).getId().toUnqualifiedVersionless();
// Create a dummy record so that an unconstrained query doesn't pass the test due to returning the only record
myObservationDao.create(new Observation(), mySrd);
}
String url = "/Observation?encounter.identifier=foo|bar";
// execute
myCaptureQueriesListener.clear();
List<String> oids = searchAndReturnUnqualifiedVersionlessIdValues(url);
myCaptureQueriesListener.logSelectQueries();
// validate
assertEquals(1L, oids.size());
assertThat(oids, contains(oid1.getIdPart()));
}
@Test
public void testShouldResolveATwoLinkChainWithStandAloneResources_CompoundReference() throws Exception {
// setup
myModelConfig.setIndexOnContainedResources(true);
IIdType oid1;
IIdType oid2;
// AuditEvent should match both AuditEvent.agent.who and AuditEvent.entity.what
{
Patient p = new Patient();
p.setId(IdType.newRandomUuid());
p.addName().setFamily("Smith").addGiven("John");
myPatientDao.create(p, mySrd);
AuditEvent auditEvent = new AuditEvent();
auditEvent.addAgent().setWho(new Reference(p.getId()));
oid1 = myAuditEventDao.create(auditEvent, mySrd).getId().toUnqualifiedVersionless();
}
{
Patient p = new Patient();
p.setId(IdType.newRandomUuid());
p.addName().setFamily("Smith").addGiven("John");
myPatientDao.create(p, mySrd);
AuditEvent auditEvent = new AuditEvent();
auditEvent.addEntity().setWhat(new Reference(p.getId()));
oid2 = myAuditEventDao.create(auditEvent, mySrd).getId().toUnqualifiedVersionless();
}
{
// Create a dummy record so that an unconstrained query doesn't pass the test due to returning all the records
myAuditEventDao.create(new AuditEvent(), mySrd);
}
String url = "/AuditEvent?patient.name=Smith";
// execute
myCaptureQueriesListener.clear();
List<String> oids = searchAndReturnUnqualifiedVersionlessIdValues(url, myAuditEventDao);
myCaptureQueriesListener.logSelectQueries();
// validate
assertEquals(2L, oids.size());
assertThat(oids, contains(oid1.getIdPart(), oid2.getIdPart()));
}
@Test
public void testShouldResolveATwoLinkChainWithContainedResources_CompoundReference() throws Exception {
// setup
myModelConfig.setIndexOnContainedResources(true);
IIdType oid1;
IIdType oid2;
// AuditEvent should match both AuditEvent.agent.who and AuditEvent.entity.what
{
Patient p = new Patient();
p.setId("p1");
p.addName().setFamily("Smith").addGiven("John");
AuditEvent auditEvent = new AuditEvent();
auditEvent.addContained(p);
auditEvent.addAgent().setWho(new Reference("#p1"));
oid1 = myAuditEventDao.create(auditEvent, mySrd).getId().toUnqualifiedVersionless();
}
{
Patient p = new Patient();
p.setId("p2");
p.addName().setFamily("Smith").addGiven("John");
AuditEvent auditEvent = new AuditEvent();
auditEvent.addContained(p);
auditEvent.addEntity().setWhat(new Reference("#p2"));
oid2 = myAuditEventDao.create(auditEvent, mySrd).getId().toUnqualifiedVersionless();
}
{
// Create a dummy record so that an unconstrained query doesn't pass the test due to returning all the records
myAuditEventDao.create(new AuditEvent(), mySrd);
}
String url = "/AuditEvent?patient.name=Smith";
// execute
myCaptureQueriesListener.clear();
List<String> oids = searchAndReturnUnqualifiedVersionlessIdValues(url, myAuditEventDao);
myCaptureQueriesListener.logSelectQueries();
// validate
assertEquals(2L, oids.size());
assertThat(oids, contains(oid1.getIdPart(), oid2.getIdPart()));
}
@Test
public void testShouldResolveATwoLinkChainWithAContainedResource() throws Exception {
// setup
@ -330,6 +474,46 @@ public class ChainingR4SearchTest extends BaseJpaR4Test {
assertThat(oids, contains(oid1.getIdPart()));
}
@Test
public void testShouldResolveATwoLinkChainWithAContainedResource_CommonReference() throws Exception {
// setup
myModelConfig.setIndexOnContainedResources(true);
IIdType oid1;
{
Encounter encounter = new Encounter();
encounter.setId("enc");
encounter.addIdentifier().setSystem("foo").setValue("bar");
Observation obs = new Observation();
obs.getCode().setText("Body Weight");
obs.getCode().addCoding().setCode("obs2").setSystem("Some System").setDisplay("Body weight as measured by me");
obs.setStatus(Observation.ObservationStatus.FINAL);
obs.setValue(new Quantity(81));
obs.addContained(encounter);
obs.setEncounter(new Reference("#enc"));
oid1 = myObservationDao.create(obs, mySrd).getId().toUnqualifiedVersionless();
// Create a dummy record so that an unconstrained query doesn't pass the test due to returning the only record
myObservationDao.create(new Observation(), mySrd);
}
String url = "/Observation?encounter.identifier=foo|bar";
// execute
myCaptureQueriesListener.clear();
List<String> oids = searchAndReturnUnqualifiedVersionlessIdValues(url);
myCaptureQueriesListener.logSelectQueries();
// validate
assertEquals(1L, oids.size());
assertThat(oids, contains(oid1.getIdPart()));
}
@Test
public void testShouldResolveAThreeLinkChainWhereAllResourcesStandAloneWithoutContainedResourceIndexing() throws Exception {
@ -477,6 +661,51 @@ public class ChainingR4SearchTest extends BaseJpaR4Test {
assertThat(oids, contains(oid1.getIdPart()));
}
@Test
public void testShouldResolveAThreeLinkChainWithAContainedResourceAtTheEndOfTheChain_CommonReference() throws Exception {
// setup
myModelConfig.setIndexOnContainedResources(true);
IIdType oid1;
{
Patient p = new Patient();
p.setId("pat");
p.addName().setFamily("Smith").addGiven("John");
Encounter encounter = new Encounter();
encounter.addContained(p);
encounter.setId(IdType.newRandomUuid());
encounter.addIdentifier().setSystem("foo").setValue("bar");
encounter.setSubject(new Reference("#pat"));
myEncounterDao.create(encounter, mySrd);
Observation obs = new Observation();
obs.getCode().setText("Body Weight");
obs.getCode().addCoding().setCode("obs2").setSystem("Some System").setDisplay("Body weight as measured by me");
obs.setStatus(Observation.ObservationStatus.FINAL);
obs.setValue(new Quantity(81));
obs.setSubject(new Reference(p.getId()));
obs.setEncounter(new Reference(encounter.getId()));
oid1 = myObservationDao.create(obs, mySrd).getId().toUnqualifiedVersionless();
// Create a dummy record so that an unconstrained query doesn't pass the test due to returning the only record
myObservationDao.create(new Observation(), mySrd);
}
String url = "/Observation?encounter.patient.name=Smith";
// execute
myCaptureQueriesListener.clear();
List<String> oids = searchAndReturnUnqualifiedVersionlessIdValues(url);
myCaptureQueriesListener.logSelectQueries();
// validate
assertEquals(1L, oids.size());
assertThat(oids, contains(oid1.getIdPart()));
}
@Test
public void testShouldResolveAThreeLinkChainWithAContainedResourceAtTheBeginningOfTheChain() throws Exception {
// Adding support for this case in SMILE-3151
@ -518,6 +747,50 @@ public class ChainingR4SearchTest extends BaseJpaR4Test {
assertThat(oids, contains(oid1.getIdPart()));
}
@Test
public void testShouldResolveAThreeLinkChainWithAContainedResourceAtTheBeginningOfTheChain_CommonReference() throws Exception {
// setup
myModelConfig.setIndexOnContainedResources(true);
IIdType oid1;
{
Patient p = new Patient();
p.setId(IdType.newRandomUuid());
p.addName().setFamily("Smith").addGiven("John");
myPatientDao.create(p, mySrd);
Encounter encounter = new Encounter();
encounter.setId("enc");
encounter.addIdentifier().setSystem("foo").setValue("bar");
encounter.setSubject(new Reference(p.getId()));
Observation obs = new Observation();
obs.addContained(encounter);
obs.getCode().setText("Body Weight");
obs.getCode().addCoding().setCode("obs2").setSystem("Some System").setDisplay("Body weight as measured by me");
obs.setStatus(Observation.ObservationStatus.FINAL);
obs.setValue(new Quantity(81));
obs.setEncounter(new Reference("#enc"));
oid1 = myObservationDao.create(obs, mySrd).getId().toUnqualifiedVersionless();
// Create a dummy record so that an unconstrained query doesn't pass the test due to returning the only record
myObservationDao.create(new Observation(), mySrd);
}
String url = "/Observation?encounter.identifier=foo|bar";
// execute
myCaptureQueriesListener.clear();
List<String> oids = searchAndReturnUnqualifiedVersionlessIdValues(url);
myCaptureQueriesListener.logSelectQueries();
// validate
assertEquals(1L, oids.size());
assertThat(oids, contains(oid1.getIdPart()));
}
@Test
public void testShouldNotResolveAThreeLinkChainWithAllContainedResourcesWhenRecursiveContainedIndexesAreDisabled() throws Exception {
@ -1265,12 +1538,16 @@ public class ChainingR4SearchTest extends BaseJpaR4Test {
}
private List<String> searchAndReturnUnqualifiedVersionlessIdValues(String theUrl) throws IOException {
return searchAndReturnUnqualifiedVersionlessIdValues(theUrl, myObservationDao);
}
private List<String> searchAndReturnUnqualifiedVersionlessIdValues(String theUrl, IFhirResourceDao<? extends DomainResource> theObservationDao) {
List<String> ids = new ArrayList<>();
ResourceSearch search = myMatchUrlService.getResourceSearch(theUrl);
SearchParameterMap map = search.getSearchParameterMap();
map.setLoadSynchronous(true);
IBundleProvider result = myObservationDao.search(map);
IBundleProvider result = theObservationDao.search(map);
return result.getAllResourceIds();
}

View File

@ -11,6 +11,7 @@ import org.hl7.fhir.instance.model.api.IIdType;
import org.hl7.fhir.r4.model.Bundle;
import org.hl7.fhir.r4.model.Observation;
import org.hl7.fhir.r4.model.Patient;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@ -19,7 +20,9 @@ import java.util.List;
import java.util.Optional;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.blankOrNullString;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.not;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
@ -31,7 +34,7 @@ public class AuthorizationInterceptorMultitenantJpaR4Test extends BaseMultitenan
@Test
public void testCreateInTenant_Allowed() {
enableAuthorizationInterceptor(() -> new RuleBuilder()
setupAuthorizationInterceptorWithRules(() -> new RuleBuilder()
.allow().create().allResources().withAnyId().forTenantIds(TENANT_A)
.build());
@ -48,7 +51,7 @@ public class AuthorizationInterceptorMultitenantJpaR4Test extends BaseMultitenan
createPatient(withTenant(TENANT_A), withActiveTrue());
IIdType idB = createPatient(withTenant(TENANT_B), withActiveFalse());
enableAuthorizationInterceptor(() -> new RuleBuilder()
setupAuthorizationInterceptorWithRules(() -> new RuleBuilder()
.allow().create().allResources().withAnyId().forTenantIds(TENANT_A)
.build());
@ -66,7 +69,7 @@ public class AuthorizationInterceptorMultitenantJpaR4Test extends BaseMultitenan
IIdType idA = createPatient(withTenant(TENANT_A), withActiveTrue());
createPatient(withTenant(TENANT_B), withActiveFalse());
enableAuthorizationInterceptor(() -> new RuleBuilder()
setupAuthorizationInterceptorWithRules(() -> new RuleBuilder()
.allow().read().allResources().withAnyId().forTenantIds(TENANT_A)
.build());
@ -80,7 +83,7 @@ public class AuthorizationInterceptorMultitenantJpaR4Test extends BaseMultitenan
createPatient(withTenant(TENANT_A), withActiveTrue());
IIdType idB = createPatient(withTenant(TENANT_B), withActiveFalse());
enableAuthorizationInterceptor(() -> new RuleBuilder()
setupAuthorizationInterceptorWithRules(() -> new RuleBuilder()
.allow().read().allResources().withAnyId().forTenantIds(TENANT_A)
.build());
@ -97,7 +100,7 @@ public class AuthorizationInterceptorMultitenantJpaR4Test extends BaseMultitenan
public void testReadInDefaultTenant_Allowed() {
IIdType idA = createPatient(withTenant("DEFAULT"), withActiveTrue());
enableAuthorizationInterceptor(() -> new RuleBuilder()
setupAuthorizationInterceptorWithRules(() -> new RuleBuilder()
.allow().read().allResources().withAnyId().forTenantIds("DEFAULT")
.build());
@ -110,7 +113,7 @@ public class AuthorizationInterceptorMultitenantJpaR4Test extends BaseMultitenan
public void testReadInDefaultTenant_Blocked() {
IIdType idA = createPatient(withTenant(TENANT_A), withActiveTrue());
enableAuthorizationInterceptor(() -> new RuleBuilder()
setupAuthorizationInterceptorWithRules(() -> new RuleBuilder()
.allow().read().allResources().withAnyId().forTenantIds("DEFAULT")
.build());
@ -130,7 +133,7 @@ public class AuthorizationInterceptorMultitenantJpaR4Test extends BaseMultitenan
IIdType patientId = createPatient(withTenant(TENANT_A), withActiveTrue());
createObservation(withTenant(TENANT_B), withSubject(patientId.toUnqualifiedVersionless()));
enableAuthorizationInterceptor(() -> new RuleBuilder()
setupAuthorizationInterceptorWithRules(() -> new RuleBuilder()
.allow().read().allResources().withAnyId().forTenantIds(TENANT_A, TENANT_B)
.build());
@ -152,7 +155,7 @@ public class AuthorizationInterceptorMultitenantJpaR4Test extends BaseMultitenan
IIdType patientId = createPatient(withTenant(TENANT_A), withActiveTrue());
createObservation(withTenant(TENANT_B), withSubject(patientId.toUnqualifiedVersionless()));
enableAuthorizationInterceptor(() -> new RuleBuilder()
setupAuthorizationInterceptorWithRules(() -> new RuleBuilder()
.allow().read().allResources().withAnyId().forTenantIds(TENANT_A)
.build());
@ -185,7 +188,7 @@ public class AuthorizationInterceptorMultitenantJpaR4Test extends BaseMultitenan
observationIds.add(id);
}
enableAuthorizationInterceptor(() -> new RuleBuilder()
setupAuthorizationInterceptorWithRules(() -> new RuleBuilder()
.allow().read().allResources().withAnyId().forTenantIds(TENANT_A)
.build());
@ -221,7 +224,47 @@ public class AuthorizationInterceptorMultitenantJpaR4Test extends BaseMultitenan
} catch (ForbiddenOperationException e) {
// good
}
}
@Test
public void testPaginNextUrl_Blocked() {
// We're going to create 4 patients, then request all patients, giving us two pages of results
myPagingProvider.setMaximumPageSize(2);
createPatient(withTenant(TENANT_A), withActiveTrue());
createPatient(withTenant(TENANT_A), withActiveTrue());
createPatient(withTenant(TENANT_A), withActiveTrue());
createPatient(withTenant(TENANT_A), withActiveTrue());
setupAuthorizationInterceptorWithRules(() -> new RuleBuilder()
.allow().read().allResources().withAnyId().forTenantIds(TENANT_A)
.build());
myTenantClientInterceptor.setTenantId(TENANT_A);
Bundle patientBundle = myClient
.search()
.forResource("Patient")
.include(Observation.INCLUDE_ALL)
.returnBundle(Bundle.class)
.execute();
Assertions.assertTrue(patientBundle.hasLink());
Assertions.assertTrue(patientBundle.getLink().stream().anyMatch(link -> link.hasRelation() && link.getRelation().equals("next")));
String nextLink = patientBundle.getLink().stream().filter(link -> link.hasRelation() && link.getRelation().equals("next")).findFirst().get().getUrl();
assertThat(nextLink, not(blankOrNullString()));
// Now come in as an imposter from a diff tenant with a stolen next link
// Request as a user with only access to TENANT_B
setupAuthorizationInterceptorWithRules(() -> new RuleBuilder()
.allow().read().allResources().withAnyId().forTenantIds(TENANT_B)
.build());
try {
Bundle resp2 = myClient.search().byUrl(nextLink).returnBundle(Bundle.class).execute();
fail();
} catch (ForbiddenOperationException e) {
Assertions.assertEquals("HTTP 403 Forbidden: HAPI-0334: Access denied by default policy (no applicable rules)", e.getMessage());
}
}
}

View File

@ -108,7 +108,10 @@ public abstract class BaseMultitenantResourceProviderR4Test extends BaseResource
.execute();
}
public void enableAuthorizationInterceptor(Supplier<List<IAuthRule>> theRuleSupplier) {
public void setupAuthorizationInterceptorWithRules(Supplier<List<IAuthRule>> theRuleSupplier) {
if(myAuthorizationInterceptor != null) {
ourRestServer.unregisterInterceptor(myAuthorizationInterceptor);
}
myAuthorizationInterceptor = new AuthorizationInterceptor() {
@Override
public List<IAuthRule> buildRuleList(RequestDetails theRequestDetails) {

View File

@ -341,5 +341,4 @@ public class MultitenantServerR4Test extends BaseMultitenantResourceProviderR4Te
}
}
}

View File

@ -33,10 +33,7 @@
<property name="hibernate.cache.use_query_cache" value="false" />
<property name="hibernate.cache.use_second_level_cache" value="false" />
<property name="hibernate.cache.use_structured_entries" value="false" />
<!--
<property name="hibernate.cache.region.factory_class" value="org.hibernate.cache.ehcache.SingletonEhCacheRegionFactory" />
-->
</properties>
</properties>
</persistence-unit>
</persistence>

View File

@ -66,7 +66,7 @@ public class RuleBulkExportImpl extends BaseRule {
}
for (String next : options.getResourceTypes()) {
if (!myResourceTypes.contains(next)) {
return null;
return new AuthorizationInterceptor.Verdict(PolicyEnum.DENY,this);
}
}
}

View File

@ -0,0 +1,75 @@
package ca.uhn.fhir.rest.server.interceptor.auth;
import ca.uhn.fhir.interceptor.api.Pointcut;
import ca.uhn.fhir.rest.api.RestOperationTypeEnum;
import ca.uhn.fhir.rest.api.server.RequestDetails;
import ca.uhn.fhir.rest.api.server.bulk.BulkDataExportOptions;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import java.util.HashSet;
import java.util.Set;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.when;
@ExtendWith(MockitoExtension.class)
public class RuleBulkExportImplTest {
@Mock
private RequestDetails myRequestDetails;
@Mock
private IRuleApplier myRuleApplier;
@Mock
private Set<AuthorizationFlagsEnum> myFlags;
@Test
public void testDenyBulkRequestWithInvalidResourcesTypes() {
RuleBulkExportImpl myRule = new RuleBulkExportImpl("a");
RestOperationTypeEnum myOperation = RestOperationTypeEnum.EXTENDED_OPERATION_SERVER;
Pointcut myPointcut = Pointcut.STORAGE_INITIATE_BULK_EXPORT;
Set<String> myTypes = new HashSet<>();
myTypes.add("Patient");
myTypes.add("Practitioner");
myRule.setResourceTypes(myTypes);
Set<String> myWantTypes = new HashSet<>();
myWantTypes.add("Questionnaire");
BulkDataExportOptions options = new BulkDataExportOptions();
options.setResourceTypes(myWantTypes);
when(myRequestDetails.getAttribute(any())).thenReturn(options);
AuthorizationInterceptor.Verdict verdict = myRule.applyRule(myOperation, myRequestDetails, null, null, null, myRuleApplier, myFlags, myPointcut);
assertEquals(PolicyEnum.DENY, verdict.getDecision());
}
@Test
public void testBulkRequestWithValidResourcesTypes() {
RuleBulkExportImpl myRule = new RuleBulkExportImpl("a");
RestOperationTypeEnum myOperation = RestOperationTypeEnum.EXTENDED_OPERATION_SERVER;
Pointcut myPointcut = Pointcut.STORAGE_INITIATE_BULK_EXPORT;
Set<String> myTypes = new HashSet<>();
myTypes.add("Patient");
myTypes.add("Practitioner");
myRule.setResourceTypes(myTypes);
Set<String> myWantTypes = new HashSet<>();
myWantTypes.add("Patient");
myWantTypes.add("Practitioner");
BulkDataExportOptions options = new BulkDataExportOptions();
options.setResourceTypes(myWantTypes);
when(myRequestDetails.getAttribute(any())).thenReturn(options);
AuthorizationInterceptor.Verdict verdict = myRule.applyRule(myOperation, myRequestDetails, null, null, null, myRuleApplier, myFlags, myPointcut);
assertNull(verdict);
}
}

View File

@ -43,10 +43,7 @@
<!--
<property name="hibernate.ejb.naming_strategy" value="ca.uhn.fhir.jpa.util.CustomNamingStrategy" />
-->
<!--
<property name="hibernate.cache.region.factory_class" value="org.hibernate.cache.ehcache.SingletonEhCacheRegionFactory" />
-->
</properties>
</properties>
</persistence-unit>
</persistence>

View File

@ -1584,11 +1584,6 @@
<artifactId>hibernate-java8</artifactId>
<version>${hibernate_version}</version>
</dependency>
<dependency>
<groupId>org.hibernate</groupId>
<artifactId>hibernate-ehcache</artifactId>
<version>${hibernate_version}</version>
</dependency>
<dependency>
<groupId>org.hibernate</groupId>
<artifactId>hibernate-entitymanager</artifactId>