From 6bba3cb45d21d37fe4f63ebb533f16f847ebf20c Mon Sep 17 00:00:00 2001 From: jdar Date: Fri, 17 Oct 2025 10:58:33 -0700 Subject: [PATCH 01/26] draft implementation - no support for typefilter yet --- .../ca/uhn/fhir/jpa/config/JpaConfig.java | 7 ++ .../jpa/interceptor/AuthResourceResolver.java | 38 +++++++ .../server/bulk/BulkExportJobParameters.java | 22 ++++ .../auth/AuthorizationInterceptor.java | 9 +- .../auth/FhirQueriesRuleTester.java | 83 ++++++++++++++ .../interceptor/auth/FhirQueryRuleTester.java | 4 + .../auth/IAuthResourceResolver.java | 14 +++ .../interceptor/auth/IAuthRuleFinished.java | 5 + .../server/interceptor/auth/IRuleApplier.java | 5 +- .../server/interceptor/auth/RuleBuilder.java | 11 +- .../interceptor/auth/RuleBulkExportImpl.java | 103 +++++++++++++++++- .../auth/RuleBulkExportImplTest.java | 21 +++- 12 files changed, 310 insertions(+), 12 deletions(-) create mode 100644 hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/AuthResourceResolver.java create mode 100644 hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/FhirQueriesRuleTester.java create mode 100644 hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthResourceResolver.java diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/JpaConfig.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/JpaConfig.java index c83230f0b8d8..111a77a7c580 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/JpaConfig.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/JpaConfig.java @@ -93,6 +93,7 @@ import ca.uhn.fhir.jpa.entity.TermValueSet; import ca.uhn.fhir.jpa.esr.ExternallyStoredResourceServiceRegistry; import ca.uhn.fhir.jpa.graphql.DaoRegistryGraphQLStorageServices; +import ca.uhn.fhir.jpa.interceptor.AuthResourceResolver; import ca.uhn.fhir.jpa.interceptor.CascadingDeleteInterceptor; import ca.uhn.fhir.jpa.interceptor.JpaConsentContextServices; import ca.uhn.fhir.jpa.interceptor.OverridePathBasedReferentialIntegrityForDeletesInterceptor; @@ -205,6 +206,7 @@ import ca.uhn.fhir.rest.api.server.storage.IResourcePersistentId; import ca.uhn.fhir.rest.server.interceptor.ResponseTerminologyTranslationInterceptor; import ca.uhn.fhir.rest.server.interceptor.ResponseTerminologyTranslationSvc; +import ca.uhn.fhir.rest.server.interceptor.auth.IAuthResourceResolver; import ca.uhn.fhir.rest.server.interceptor.consent.IConsentContextServices; import ca.uhn.fhir.rest.server.interceptor.partition.RequestTenantPartitionInterceptor; import ca.uhn.fhir.rest.server.util.ISearchParamRegistry; @@ -530,6 +532,11 @@ public IConsentContextServices consentContextServices() { return new JpaConsentContextServices(); } + @Bean + public IAuthResourceResolver authResourceResolver(DaoRegistry theDaoRegistry) { + return new AuthResourceResolver(theDaoRegistry); + } + @Bean @Lazy public DiffProvider diffProvider() { diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/AuthResourceResolver.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/AuthResourceResolver.java new file mode 100644 index 000000000000..a3c7a8018edc --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/AuthResourceResolver.java @@ -0,0 +1,38 @@ +package ca.uhn.fhir.jpa.interceptor; + +import ca.uhn.fhir.jpa.api.dao.DaoRegistry; +import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; +import ca.uhn.fhir.rest.api.Constants; +import ca.uhn.fhir.rest.api.server.SystemRequestDetails; +import ca.uhn.fhir.rest.param.TokenOrListParam; +import ca.uhn.fhir.rest.server.interceptor.auth.IAuthResourceResolver; + +import java.util.Arrays; +import java.util.List; + +import org.hl7.fhir.instance.model.api.IBaseResource; +import org.hl7.fhir.instance.model.api.IIdType; +import org.springframework.stereotype.Service; + +@Service +public class AuthResourceResolver implements IAuthResourceResolver { + private final DaoRegistry myDaoRegistry; + + public AuthResourceResolver(DaoRegistry myDaoRegistry) { + this.myDaoRegistry = myDaoRegistry; + } + + public IBaseResource resolveCompartmentById(IIdType theCompartmentId) { + return myDaoRegistry.getResourceDao(theCompartmentId.getResourceType()).read(theCompartmentId, new SystemRequestDetails()); + } + + public List resolveCompartmentByIds(List theCompartmentId, String theResourceType) { + TokenOrListParam t = new TokenOrListParam(null, theCompartmentId.toArray(String[]::new)); + + SearchParameterMap m = new SearchParameterMap(); + m.add(Constants.PARAM_ID, t); + return myDaoRegistry.getResourceDao(theResourceType).searchForResources(m, new SystemRequestDetails()); + } + //translateMatchUrl + +} diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/bulk/BulkExportJobParameters.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/bulk/BulkExportJobParameters.java index d9bee535488f..a26c3640e845 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/bulk/BulkExportJobParameters.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/bulk/BulkExportJobParameters.java @@ -127,6 +127,28 @@ public class BulkExportJobParameters extends BaseBatchJobParameters { @JsonProperty("includeHistory") private boolean myIncludeHistory; + /** + * This flag is set to true when the system has added narrowing filters to the bulk export parameters + * based on the permissions that the user has. + * + * For example, if a bulk export is performed with `_typeFilter=Patient?active=true` + * and a user has permissions for `Patient?_lastUpdated=gt2025 + * than the system will modify the filters in the parameters to Patient?active=true&_lastUpdated=gt2025 + * and this flag will be set to true + * + * See TODO JDJD add issue for more information. + */ + @JsonProperty(value = "systemAddedNarrowingFilters", access = JsonProperty.Access.READ_ONLY) + private boolean mySystemAddedNarrowingFilters; + + public boolean hasSystemAddedNarrowingFilters() { + return mySystemAddedNarrowingFilters; + } + + public void setSystemAddedNarrowingFilters(boolean theSystemAddedNarrowingFilters) { + this.mySystemAddedNarrowingFilters = theSystemAddedNarrowingFilters; + } + public String getExportIdentifier() { return myExportId; } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java index cfb5ac1ae6e0..1c9b7d29f509 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java @@ -25,6 +25,7 @@ import ca.uhn.fhir.interceptor.api.Hook; import ca.uhn.fhir.interceptor.api.Interceptor; import ca.uhn.fhir.interceptor.api.Pointcut; +import ca.uhn.fhir.model.primitive.IdDt; import ca.uhn.fhir.model.valueset.BundleTypeEnum; import ca.uhn.fhir.rest.api.RestOperationTypeEnum; import ca.uhn.fhir.rest.api.server.IPreResourceShowDetails; @@ -171,7 +172,7 @@ public Verdict applyRulesAndReturnDecision( Verdict verdict = null; for (IAuthRule nextRule : rules) { - ourLog.trace("Rule being applied - {}", nextRule); + ourLog.trace("Rule being applied - {}", nextRule);//todo jdjd 1007 does bulk export not go through here? verdict = nextRule.applyRule( theOperation, theRequestDetails, @@ -461,7 +462,11 @@ public void initiateBulkExport( if (theRequestDetails != null) { theRequestDetails.getUserData().put(REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS, theBulkExportOptions); } - applyRulesAndFailIfDeny(restOperationType, theRequestDetails, null, null, null, thePointcut); + //todo jdjd figure out what to do if we want to export patients, and patient is a list of ids in the bulk export options. so do you have to pass them in one by one? or how do we do this for other bulk operations? +// is this rule currently evaluated one by one for each patient included in the options? + // how do you do this? maybe a davinci export? https://smilecdr.com/docs/davinci_data_exchange/davinci_data_exchange.html#request-parameters + // or maybe with post w/ parameters resource and patient parameter in list of params? JpaConstants.PARAM_EXPORT_PATIENT and BulkDataExportProviderR4Test#testInitiatePatientExportRequest() + applyRulesAndFailIfDeny(restOperationType, theRequestDetails, null, new IdDt(theBulkExportOptions.getGroupId()), null, thePointcut); } /** diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/FhirQueriesRuleTester.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/FhirQueriesRuleTester.java new file mode 100644 index 000000000000..b264dcb66c58 --- /dev/null +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/FhirQueriesRuleTester.java @@ -0,0 +1,83 @@ +/*- + * #%L + * HAPI FHIR - Server Framework + * %% + * Copyright (C) 2014 - 2025 Smile CDR, Inc. + * %% + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * #L% + */ +package ca.uhn.fhir.rest.server.interceptor.auth; + +import java.util.ArrayList; +import java.util.List; + +import java.util.stream.Collectors; + +import org.apache.commons.lang3.builder.ToStringBuilder; +import org.apache.commons.lang3.builder.ToStringStyle; + +/** + * Tester that a resource matches any query filter in a list. + */ +public class FhirQueriesRuleTester implements IAuthRuleTester { + private final List myFilters; + private final String myResourceType; + + public FhirQueriesRuleTester(List theQueries, String theResourceType) { + myFilters = theQueries.stream() + .map(query -> query.contains("?") ? query.substring(query.indexOf("?") + 1) : query) + .map(FhirQueryRuleTester::new) + .collect(Collectors.toList()); + myResourceType = theResourceType; + } + + public void addFilter(FhirQueriesRuleTester theOtherRuleTester) { + //todo jdjd can i add to list? + if (theOtherRuleTester.getResourceType().equals(myResourceType)) { + myFilters.addAll(theOtherRuleTester.getFilters()); + } + } + + public List getFilters() { + return List.copyOf(myFilters); + } + + public List getFiltersAsString() { + return myFilters.stream().map(param -> myResourceType + '?' + param.getQueryParameters()).toList(); + } + + public String getResourceType() { + return myResourceType; + } + + @Override + public boolean matches(RuleTestRequest theRuleTestRequest) { + return myFilters.stream().anyMatch(t -> t.matches(theRuleTestRequest)); + } + + @Override + public boolean matchesOutput(RuleTestRequest theRuleTestRequest) { + return myFilters.stream().anyMatch(t -> t.matches(theRuleTestRequest)); + } + + //todo jdjds what is to string used for and what should the implementation look like for a list of queries + @Override + public String toString() { + return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE) + .append("filter", myFilters) + .toString(); + } + + +} diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/FhirQueryRuleTester.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/FhirQueryRuleTester.java index e9437d27719a..27c91930d4aa 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/FhirQueryRuleTester.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/FhirQueryRuleTester.java @@ -82,6 +82,10 @@ private boolean checkMatch(RuleTestRequest theRuleTestRequest) { } } + public String getQueryParameters() { + return myQueryParameters; + } + @Override public String toString() { return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE) diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthResourceResolver.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthResourceResolver.java new file mode 100644 index 000000000000..750f485e86c2 --- /dev/null +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthResourceResolver.java @@ -0,0 +1,14 @@ +package ca.uhn.fhir.rest.server.interceptor.auth; + +import ca.uhn.fhir.rest.api.server.SystemRequestDetails; + +import java.util.List; + +import org.hl7.fhir.instance.model.api.IBaseResource; +import org.hl7.fhir.instance.model.api.IIdType; + +public interface IAuthResourceResolver { + IBaseResource resolveCompartmentById(IIdType theCompartmentId); + + List resolveCompartmentByIds(List theCompartmentId, String theResourceType); +} diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleFinished.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleFinished.java index 353515ca8314..a837ae4ddb1e 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleFinished.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleFinished.java @@ -56,4 +56,9 @@ public interface IAuthRuleFinished { * @param theQueryParameters a FHIR query parameter string. E.g. {@code category=laboratory&date=ge2021} */ IAuthRuleFinished withFilterTester(String theQueryParameters); + + //todo jdjd docs + default IAuthRuleFinished withFilterOnExistingTester(String theQueryParameters) { + return this; + } } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IRuleApplier.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IRuleApplier.java index 8bfc67ad7274..28cc61eda24d 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IRuleApplier.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IRuleApplier.java @@ -50,5 +50,8 @@ Verdict applyRulesAndReturnDecision( default IAuthorizationSearchParamMatcher getSearchParamMatcher() { return null; } - ; + + default IAuthResourceResolver getAuthResourceResolver() { + return null; + } } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java index 4e444c09956c..fed246fae9d6 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java @@ -29,6 +29,7 @@ import ca.uhn.fhir.rest.server.provider.ProviderConstants; import com.google.common.collect.Lists; import jakarta.annotation.Nonnull; +import net.sf.saxon.trans.SymbolicName; import org.apache.commons.lang3.Validate; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; @@ -259,7 +260,7 @@ private class RuleBuilderRule implements IAuthRuleBuilderRule { private final String myRuleName; private RuleBuilderRuleOp myReadRuleBuilder; private RuleBuilderRuleOp myWriteRuleBuilder; - private RuleBuilderBulkExport ruleBuilderBulkExport; + private RuleBuilderBulkExport myRuleBuilderBulkExport; RuleBuilderRule(PolicyEnum theRuleMode, String theRuleName) { myRuleMode = theRuleMode; @@ -341,10 +342,10 @@ public IAuthRuleBuilderGraphQL graphQL() { @Override public IAuthRuleBuilderRuleBulkExport bulkExport() { - if (ruleBuilderBulkExport == null) { - ruleBuilderBulkExport = new RuleBuilderBulkExport(); + if (myRuleBuilderBulkExport == null) { + myRuleBuilderBulkExport = new RuleBuilderBulkExport(); } - return ruleBuilderBulkExport; + return myRuleBuilderBulkExport; } @Override @@ -901,6 +902,7 @@ public IAuthRuleBuilderRuleBulkExportWithTarget groupExportOnGroup(@Nonnull Stri @Override public IAuthRuleBuilderRuleBulkExportWithTarget patientExportOnAllPatients() { + //todo JDJD 1008 it's this ==null that is problematic, it prevents duplicate bulkExportRuleImpl if (myRuleBulkExport == null) { RuleBulkExportImpl rule = new RuleBulkExportImpl(myRuleName); rule.setMode(myRuleMode); @@ -938,6 +940,7 @@ public IAuthRuleBuilderRuleBulkExportWithTarget patientExportOnPatient(@Nonnull @Override public IAuthRuleBuilderRuleBulkExportWithTarget patientExportOnPatientStrings( @Nonnull Collection theFocusResourceIds) { + //todo JDJD 1008 it's this ==null that is problematic, it prevents duplicate bulkExportRuleImpl if (myRuleBulkExport == null) { RuleBulkExportImpl rule = new RuleBulkExportImpl(myRuleName); rule.setAppliesToPatientExport(theFocusResourceIds); diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBulkExportImpl.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBulkExportImpl.java index 2c8ab32974ff..2e35f853ca51 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBulkExportImpl.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBulkExportImpl.java @@ -24,7 +24,16 @@ import ca.uhn.fhir.rest.api.RestOperationTypeEnum; import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.api.server.bulk.BulkExportJobParameters; + +import static ca.uhn.fhir.rest.server.interceptor.auth.AuthorizationInterceptor.REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS; + import com.google.common.annotations.VisibleForTesting; + +import java.util.List; + +import java.util.Optional; + +import org.hl7.fhir.instance.model.api.IBase; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; @@ -52,6 +61,26 @@ public class RuleBulkExportImpl extends BaseRule { myPatientIds = new ArrayList<>(); } + @Override + public void addTester(IAuthRuleTester theNewTester) { + if (!getTesters().isEmpty()) { + // ensure only 1 tester with list of all filters in the rule + // this ensures the queries/filters are applied as an "OR" instead of "AND" + Optional queriesTester = getTesters().stream().filter(FhirQueriesRuleTester.class::isInstance).findFirst(); + if (queriesTester.isPresent() && theNewTester instanceof FhirQueriesRuleTester) { + FhirQueriesRuleTester existingTester = (FhirQueriesRuleTester) queriesTester.get(); + + if (!existingTester.getResourceType().equals(((FhirQueriesRuleTester) theNewTester).getResourceType())) { + //TODO JDJD refine error message + throw new IllegalArgumentException("All queries for compartments should apply to the same resource type"); + } + existingTester.addFilter((FhirQueriesRuleTester) theNewTester); + } + } else { + super.addTester(theNewTester); + } + } + @Override public AuthorizationInterceptor.Verdict applyRule( RestOperationTypeEnum theOperation, @@ -62,6 +91,8 @@ public AuthorizationInterceptor.Verdict applyRule( IRuleApplier theRuleApplier, Set theFlags, Pointcut thePointcut) { + //todo jdjd here you can access mypatientIds and myGroupId in the class field. what are these? + // ans - they are the IDs specified in the rule not the bulk export input if (thePointcut != Pointcut.STORAGE_INITIATE_BULK_EXPORT) { return null; } @@ -72,12 +103,13 @@ public AuthorizationInterceptor.Verdict applyRule( BulkExportJobParameters inboundBulkExportRequestOptions = (BulkExportJobParameters) theRequestDetails .getUserData() - .get(AuthorizationInterceptor.REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS); + .get(REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS); // if style doesn't match - abstain if (!myWantAnyStyle && inboundBulkExportRequestOptions.getExportStyle() != myWantExportStyle) { return null; } + // Do we only authorize some types? If so, make sure requested types are a subset if (isNotEmpty(myResourceTypes)) { if (isEmpty(inboundBulkExportRequestOptions.getResourceTypes())) { @@ -88,6 +120,21 @@ public AuthorizationInterceptor.Verdict applyRule( } } + boolean ruleUsesQueryToDetermineCompartment = false; + //todo jdjd what is the output resource? this time its null + // also is there a cleaner way other than 2 instanceof's + if (theInputResource == null && getTesters().stream().anyMatch(t->t instanceof FhirQueryRuleTester)) { + // The rule uses a filter query for eligible compartments + + // TODO jdjd you can actually resolve the ID from getting it from request details user data BulkDataExportOptions + IIdType theResourceToQuery = theInputResourceId != null ? theInputResourceId : getIdFromRequest(theRequestDetails); + if (theInputResourceId != null) { + theInputResource = theRuleApplier.getAuthResourceResolver().resolveCompartmentById(theInputResourceId); + ruleUsesQueryToDetermineCompartment = true; + } + + } + // system only supports filtering by resource type. So if we are system, or any(), then allow, since we have // done resource type checking // above @@ -99,7 +146,7 @@ public AuthorizationInterceptor.Verdict applyRule( theOutputResource, theRuleApplier); - if (myWantAnyStyle || myWantExportStyle == BulkExportJobParameters.ExportStyle.SYSTEM) { + if (myWantAnyStyle || myWantExportStyle == BulkExportJobParameters.ExportStyle.SYSTEM || (ruleUsesQueryToDetermineCompartment && myWantExportStyle.equals(BulkExportJobParameters.ExportStyle.GROUP))) { return allowVerdict; } @@ -120,11 +167,11 @@ public AuthorizationInterceptor.Verdict applyRule( // 1. If each of the requested resource IDs in the parameters are present in the users permissions, Approve // 2. If any requested ID is not present in the users permissions, Deny. - if (myWantExportStyle == BulkExportJobParameters.ExportStyle.PATIENT) - // Unfiltered Type Level + if (myWantExportStyle == BulkExportJobParameters.ExportStyle.PATIENT) {// Unfiltered Type Level if (myAppliesToAllPatients) { return allowVerdict; } + } // Instance level, or filtered type level if (isNotEmpty(myPatientIds)) { @@ -141,10 +188,58 @@ public AuthorizationInterceptor.Verdict applyRule( return new AuthorizationInterceptor.Verdict(PolicyEnum.DENY, this); } } + } else if (getTesters().stream().anyMatch(FhirQueriesRuleTester.class::isInstance)) { //todo jdjd refine this if condition + // We don't have any specified Patient IDs to resolve the compartments we are allowed to export + + if (inboundBulkExportRequestOptions.getPatientIds().isEmpty()) { + // There are no patient IDs requested in the bulk export. + // This is either a type level export, or export with _typeFilter + + FhirQueriesRuleTester tester = (FhirQueriesRuleTester) getTesters().stream().filter(FhirQueriesRuleTester.class::isInstance).findFirst().get(); + if (tester.getFiltersAsString().containsAll(inboundBulkExportRequestOptions.getFilters())) { + // If the queries in the tester exactly match the queries in the in the bulk export, then allow + return new AuthorizationInterceptor.Verdict(PolicyEnum.ALLOW, this); + } + } else { + // But we do have a FHIR query tester to resolve the compartment(s) + // Query the DB for the requested compartment(s) in the bulk export, + // and apply a matcher between the requested compartment, and the FHIR query tested included in the permission + + List requestedPatientsToExport = theRuleApplier.getAuthResourceResolver().resolveCompartmentByIds(inboundBulkExportRequestOptions.getPatientIds(), "Patient"); + + // directly call contents of new verdict instead? and construct ur own verdict here. + boolean allAllowed = requestedPatientsToExport.stream().map(patient -> newVerdict( + theOperation, + theRequestDetails, + patient, + theInputResourceId,//todo jdjd should this be patient.id + theOutputResource, + theRuleApplier)).allMatch(t->t != null && t.getDecision().equals(PolicyEnum.ALLOW)); + //todo jdjd 1008 any or all match?? 1015 -- all right? because all resources have to be allowed to export + // old reaonsing below + // well let's say you allow 2 different ruleBulkExportImpl - then you need to iterate through both and both must be allow (this needs to be done higher up). Or you can return null here so that it will iterate through them all. But assuming there's one bulkExportRule seems to be an assumption baked in to the existing implementation + // and let's say you only have 1 ruleBulkExportImpl - then you need to iterate through all your testers/filters and make sure at least one matches but currently, if one tester fails, the whole thing fails + // solution, probably when constructing the rule, if a test already exists, add to its filters for bulk export? + // i think it never makes sense to have an AND because + // - you have diff resource types Patient?name=Doe Patient and Patient?name=Doe Encounter --> then just combine them + // - you have diff queries (want Patient?name=Doe and patient?identifier=abc|def), then just use the & in the query + // - diff resource types (Patient?name=Doe, and Organization?identifier=abc|123), then it doesnt make sense because permission is only patients and it doesnt make snense for resource to match both conditions + + if (allAllowed) { + return new AuthorizationInterceptor.Verdict(PolicyEnum.ALLOW, this); + } + } } return null; } + private static IdDt getIdFromRequest(RequestDetails theRequestDetails) { + // TODO JDJD modify based on group/patient export + // also how do you handle patient list?? + // might also want to use export style (from bulk export params) intead of the resource type (from request details) + return new IdDt(((BulkExportJobParameters) theRequestDetails.getUserData().get(REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS)).getGroupId()); + } + private Set sanitizeIds(Collection myPatientIds) { return myPatientIds.stream() .map(id -> new IdDt(id).toUnqualifiedVersionless().getValue()) diff --git a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBulkExportImplTest.java b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBulkExportImplTest.java index b5e96a7eae65..3857aa144134 100644 --- a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBulkExportImplTest.java +++ b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBulkExportImplTest.java @@ -4,6 +4,11 @@ import ca.uhn.fhir.rest.api.RestOperationTypeEnum; import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.api.server.bulk.BulkExportJobParameters; + +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; + import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; @@ -450,6 +455,7 @@ public void testPatientExport_ruleAllowsId_requestsId_allow() { assertEquals(PolicyEnum.ALLOW, verdict.getDecision()); } + //todo jdjd test here?? @Test public void testPatientExport_ruleAllowsIds_requestsIds_allow() { //Given @@ -549,7 +555,7 @@ public void testPatientExportRulesOnTypeLevelExportWithPermittedAndUnpermittedPa myRule.setMode(PolicyEnum.ALLOW); final BulkExportJobParameters options = new BulkExportJobParameters(); options.setExportStyle(BulkExportJobParameters.ExportStyle.PATIENT); - options.setFilters(Set.of("Patient?_id=123","Patient?_id=456")); + options.setResourceTypes(Set.of("Patient")); when(myRequestDetails.getUserData()).thenReturn(Map.of(AuthorizationInterceptor.REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS, options)); @@ -561,6 +567,19 @@ public void testPatientExportRulesOnTypeLevelExportWithPermittedAndUnpermittedPa } + @Test + public void testAddTestersToBulkExportRule_addsQueryFiltersToExistingTester() { + //Given + FhirQueriesRuleTester queriesTester1 = new FhirQueriesRuleTester(List.of("Patient?name=Doe"), "Patient"); + FhirQueriesRuleTester queriesTester2 = new FhirQueriesRuleTester(List.of("Patient?identifier=system|value"), "Patient"); + final RuleBulkExportImpl myRule = new RuleBulkExportImpl("b"); + myRule.setAppliesToPatientExport(List.of()); + myRule.setMode(PolicyEnum.ALLOW); + myRule.addTesters(List.of(queriesTester1, queriesTester2)); + + assertThat(myRule.getTesters()).hasSize(1); + } + private static void assertAbstain(AuthorizationInterceptor.Verdict verdict) { assertNull(verdict, "Expect abstain"); } From f7951130a3df43008b80c6a467f957e7abb17864 Mon Sep 17 00:00:00 2001 From: jdar Date: Mon, 20 Oct 2025 10:04:58 -0700 Subject: [PATCH 02/26] revise and create draft of new permissions arg format w/ implied resource type --- .../rest/server/interceptor/auth/RuleBulkExportImpl.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBulkExportImpl.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBulkExportImpl.java index 2e35f853ca51..1312df4f32d9 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBulkExportImpl.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBulkExportImpl.java @@ -129,7 +129,7 @@ public AuthorizationInterceptor.Verdict applyRule( // TODO jdjd you can actually resolve the ID from getting it from request details user data BulkDataExportOptions IIdType theResourceToQuery = theInputResourceId != null ? theInputResourceId : getIdFromRequest(theRequestDetails); if (theInputResourceId != null) { - theInputResource = theRuleApplier.getAuthResourceResolver().resolveCompartmentById(theInputResourceId); + theInputResource = theRuleApplier.getAuthResourceResolver().resolveCompartmentById(theResourceToQuery); ruleUsesQueryToDetermineCompartment = true; } @@ -227,6 +227,10 @@ public AuthorizationInterceptor.Verdict applyRule( if (allAllowed) { return new AuthorizationInterceptor.Verdict(PolicyEnum.ALLOW, this); + } else { + //TODO JDJD should it be DENY or abstain? + // you have all the patients, not all of them match the matcher. Deny? + return new AuthorizationInterceptor.Verdict(PolicyEnum.DENY, this); } } } From 4d57ea85412ea294fe53a5bb5555295c8eb01232 Mon Sep 17 00:00:00 2001 From: jdar Date: Mon, 20 Oct 2025 17:43:33 -0700 Subject: [PATCH 03/26] add new rule for group query, with tests --- ...oupBulkExportByCompartmentMatcherImpl.java | 140 ++++++++++++++++++ ...ulkExportByCompartmentMatcherImplTest.java | 113 ++++++++++++++ 2 files changed, 253 insertions(+) create mode 100644 hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImpl.java create mode 100644 hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImplTest.java diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImpl.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImpl.java new file mode 100644 index 000000000000..202bf0c784ae --- /dev/null +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImpl.java @@ -0,0 +1,140 @@ +/*- + * #%L + * HAPI FHIR - Server Framework + * %% + * Copyright (C) 2014 - 2025 Smile CDR, Inc. + * %% + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * #L% + */ +package ca.uhn.fhir.rest.server.interceptor.auth; + +import ca.uhn.fhir.interceptor.api.Pointcut; +import ca.uhn.fhir.model.primitive.IdDt; +import ca.uhn.fhir.rest.api.RestOperationTypeEnum; +import ca.uhn.fhir.rest.api.server.RequestDetails; +import ca.uhn.fhir.rest.api.server.bulk.BulkExportJobParameters; + +import static ca.uhn.fhir.rest.server.interceptor.auth.AuthorizationInterceptor.REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS; + +import com.google.common.annotations.VisibleForTesting; + +import java.util.*; +import java.util.stream.Collectors; + +import static org.apache.commons.collections4.CollectionUtils.isEmpty; +import static org.apache.commons.collections4.CollectionUtils.isNotEmpty; +import static org.apache.commons.lang3.StringUtils.isNotBlank; + +import org.hl7.fhir.instance.model.api.IBaseResource; +import org.hl7.fhir.instance.model.api.IIdType; + +public class RuleGroupBulkExportByCompartmentMatcherImpl extends BaseRule { + private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(RuleGroupBulkExportByCompartmentMatcherImpl.class); + private static final BulkExportJobParameters.ExportStyle OUR_EXPORT_STYLE = BulkExportJobParameters.ExportStyle.GROUP; + private String myGroupMatcherFilter; + private Collection myResourceTypes; + + RuleGroupBulkExportByCompartmentMatcherImpl(String theRuleName) { + super(theRuleName); + } + + @Override + public AuthorizationInterceptor.Verdict applyRule( + RestOperationTypeEnum theOperation, + RequestDetails theRequestDetails, + IBaseResource theInputResource, + IIdType theInputResourceId, + IBaseResource theOutputResource, + IRuleApplier theRuleApplier, + Set theFlags, + Pointcut thePointcut) { + if (thePointcut != Pointcut.STORAGE_INITIATE_BULK_EXPORT) { + return null; + } + + if (theRequestDetails == null) { + return null; + } + + BulkExportJobParameters inboundBulkExportRequestOptions = (BulkExportJobParameters) theRequestDetails + .getUserData() + .get(REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS); + + if (inboundBulkExportRequestOptions.getExportStyle() != OUR_EXPORT_STYLE) { + // If the requested export style is not for a GROUP, then abstain + return null; + } + + + // Do we only authorize some types? If so, make sure requested types are a subset + if (isNotEmpty(myResourceTypes)) { + if (isEmpty(inboundBulkExportRequestOptions.getResourceTypes())) { + // Attempting an export on ALL resource types, but this rule restricts on a set of resource types + return new AuthorizationInterceptor.Verdict(PolicyEnum.DENY, this); + } + if (!myResourceTypes.containsAll(inboundBulkExportRequestOptions.getResourceTypes())) { + // The requested resource types is not a subset of the permitted resource types + return new AuthorizationInterceptor.Verdict(PolicyEnum.DENY, this); + } + } + + IBaseResource theGroupResource = theRuleApplier.getAuthResourceResolver().resolveCompartmentById(new IdDt(inboundBulkExportRequestOptions.getGroupId())); + + // Apply the FhirQueryTester (which contains a inMemoryResourceMatcher) to the found Group compartment resource, and return the verdict + return newVerdict( + theOperation, + theRequestDetails, + theGroupResource, + theInputResourceId, + theOutputResource, + theRuleApplier); + } + + private static IdDt getIdFromRequest(RequestDetails theRequestDetails) { + // TODO JDJD modify based on group/patient export + // also how do you handle patient list?? + // might also want to use export style (from bulk export params) intead of the resource type (from request details) + return new IdDt(((BulkExportJobParameters) theRequestDetails.getUserData().get(REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS)).getGroupId()); + } + + private Set sanitizeIds(Collection myPatientIds) { + return myPatientIds.stream() + .map(id -> new IdDt(id).toUnqualifiedVersionless().getValue()) + .collect(Collectors.toSet()); + } + + public void setResourceTypes(Collection theResourceTypes) { + myResourceTypes = theResourceTypes; + } + + // TODO jdjd do we need to clear the testers since we are replacing the filter string? + // but testers is not modifiable i think? + public void setAppliesToGroupExportOnGroup(String theGroupMatcherFilter) { + myGroupMatcherFilter = theGroupMatcherFilter; + addTester(new FhirQueryRuleTester(theGroupMatcherFilter)); + } + + String getGroupMatcherFilter() { + return myGroupMatcherFilter; + } + + BulkExportJobParameters.ExportStyle getWantExportStyle() { + return OUR_EXPORT_STYLE; + } + + @VisibleForTesting + Collection getResourceTypes() { + return myResourceTypes; + } +} diff --git a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImplTest.java b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImplTest.java new file mode 100644 index 000000000000..50f88ac902be --- /dev/null +++ b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImplTest.java @@ -0,0 +1,113 @@ +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.BulkExportJobParameters; + +import ca.uhn.fhir.rest.api.server.bulk.BulkExportJobParameters.ExportStyle; + +import java.util.Collection; + +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.stream.Stream; + +import org.hl7.fhir.instance.model.api.IBaseResource; + +import org.junit.jupiter.api.Assertions; + +import static org.junit.jupiter.api.Assertions.*; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; + +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import static org.mockito.ArgumentMatchers.any; + +import org.mockito.Mock; + +import static org.mockito.Mockito.when; + +import org.mockito.junit.jupiter.MockitoExtension; + +@ExtendWith(MockitoExtension.class) +class RuleGroupBulkExportByCompartmentMatcherImplTest { + + private final RestOperationTypeEnum myOperation = RestOperationTypeEnum.EXTENDED_OPERATION_SERVER; + private final Pointcut myPointcut = Pointcut.STORAGE_INITIATE_BULK_EXPORT; + + @Mock + private IRuleApplier myRuleApplier; + @Mock + private IAuthResourceResolver myAuthResourceResolver; + @Mock + private IBaseResource myResource; + @Mock + private IAuthorizationSearchParamMatcher mySearchParamMatcher; + @Mock + private RequestDetails myRequestDetails; + @Mock + private Set myFlags; + + + + + @ParameterizedTest + @MethodSource("params") + void testGroupRule_withCompartmentMatchers(IAuthorizationSearchParamMatcher.MatchResult theSearchParamMatcherMatchResult, Collection theAllowedResourceTypes, ExportStyle theExportStyle, Collection theRequestedResourceTypes, PolicyEnum theExpectedVerdict, String theMessage) { + RuleGroupBulkExportByCompartmentMatcherImpl rule = new RuleGroupBulkExportByCompartmentMatcherImpl("b"); + rule.setAppliesToGroupExportOnGroup("identifier=foo|bar"); + rule.setResourceTypes(theAllowedResourceTypes); + rule.setMode(PolicyEnum.ALLOW); + + BulkExportJobParameters options = new BulkExportJobParameters(); + options.setExportStyle(theExportStyle); + options.setResourceTypes(theRequestedResourceTypes); + options.setGroupId("Group/G1"); + + when(myRequestDetails.getUserData()).thenReturn(Map.of(AuthorizationInterceptor.REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS, options)); + + if (theSearchParamMatcherMatchResult != null) { + when(myRuleApplier.getAuthResourceResolver()).thenReturn(myAuthResourceResolver); + when(myAuthResourceResolver.resolveCompartmentById(any())).thenReturn(myResource); + when(myRuleApplier.getSearchParamMatcher()).thenReturn(mySearchParamMatcher); + when(myResource.fhirType()).thenReturn("Group"); + when(mySearchParamMatcher.match("Group?identifier=foo|bar", myResource)).thenReturn(theSearchParamMatcherMatchResult); + } + + if (theExpectedVerdict != null) { + // Expect a decision + AuthorizationInterceptor.Verdict verdict = rule.applyRule(myOperation, myRequestDetails, null, null, null, myRuleApplier, myFlags, myPointcut); + Assertions.assertNotNull(verdict, "Expected " + theExpectedVerdict + " but got abstain - " + theMessage); + assertEquals(theExpectedVerdict, verdict.getDecision(), "Expected " + theExpectedVerdict + " but got " + verdict.getDecision() + " - " + theMessage); + } else { + // Expect abstain + AuthorizationInterceptor.Verdict verdict = rule.applyRule(myOperation, myRequestDetails, null, null, null, myRuleApplier, myFlags, myPointcut); + assertNull(verdict, "Expected abstain - " + theMessage); + } + } + + static Stream params() { + IAuthorizationSearchParamMatcher.MatchResult match = IAuthorizationSearchParamMatcher.MatchResult.buildMatched(); + IAuthorizationSearchParamMatcher.MatchResult noMatch = IAuthorizationSearchParamMatcher.MatchResult.buildUnmatched(); + + return Stream.of( + // theSearchParamMatcherMatchResult, theAllowedResourceTypes, theExportStyle, theRequestedResourceTypes, theExpectedDecision, theMessage + Arguments.of(match, List.of(), ExportStyle.GROUP, List.of(), PolicyEnum.ALLOW, "Allow request for all types, allow all types"), + Arguments.of(match, List.of(), ExportStyle.GROUP, List.of("Patient", "Observation"), PolicyEnum.ALLOW, "Allow request for some types, allow all types"), + Arguments.of(match, List.of("Patient", "Observation"), ExportStyle.GROUP, List.of("Patient", "Observation"), PolicyEnum.ALLOW, "Allow request for exact set of allowable types"), + Arguments.of(match, List.of("Patient", "Observation"), ExportStyle.GROUP, List.of("Patient"), PolicyEnum.ALLOW, "Allow request for subset of allowable types"), + Arguments.of(noMatch, List.of("Patient", "Observation"), ExportStyle.GROUP, List.of("Patient", "Observation"), null, "Abstain when requesting some resource types, but no resources match the permission query"), + Arguments.of(noMatch, List.of(), ExportStyle.GROUP, List.of(), null, "Abstain when requesting all resource types, but no resources match the permission query"), + // TODO jdjd, below is the narrowing case. Narrowing should happen at the SecurityInterceptor layer + Arguments.of(null, List.of("Patient", "Observation"), ExportStyle.GROUP, List.of(), PolicyEnum.DENY, "Deny request for all types when allowing some types"), + Arguments.of(null, List.of("Patient", "Observation"), ExportStyle.GROUP, List.of("Patient", "Observation", "Encounter"), PolicyEnum.DENY, "Deny request for superset of allowable types"), + Arguments.of(null, List.of("Patient", "Observation"), ExportStyle.PATIENT, List.of(), null, "Abstain when export style is not Group") + ); + } +} From 384ff3155fa9aaf299642e0fca9509cdb65305c8 Mon Sep 17 00:00:00 2001 From: jdar Date: Tue, 21 Oct 2025 10:00:41 -0700 Subject: [PATCH 04/26] - formatting - add message.properties --- .../jpa/interceptor/AuthResourceResolver.java | 31 ++++- .../auth/AuthorizationInterceptor.java | 21 +++- .../auth/FhirQueriesRuleTester.java | 28 ++--- .../interceptor/auth/RuleBulkExportImpl.java | 116 +++++++++++------- ...oupBulkExportByCompartmentMatcherImpl.java | 38 +++--- 5 files changed, 143 insertions(+), 91 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/AuthResourceResolver.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/AuthResourceResolver.java index a3c7a8018edc..ac9fc84df61e 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/AuthResourceResolver.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/AuthResourceResolver.java @@ -1,3 +1,22 @@ +/*- + * #%L + * HAPI FHIR JPA Server + * %% + * Copyright (C) 2014 - 2025 Smile CDR, Inc. + * %% + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * #L% + */ package ca.uhn.fhir.jpa.interceptor; import ca.uhn.fhir.jpa.api.dao.DaoRegistry; @@ -6,14 +25,12 @@ import ca.uhn.fhir.rest.api.server.SystemRequestDetails; import ca.uhn.fhir.rest.param.TokenOrListParam; import ca.uhn.fhir.rest.server.interceptor.auth.IAuthResourceResolver; - -import java.util.Arrays; -import java.util.List; - import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; import org.springframework.stereotype.Service; +import java.util.List; + @Service public class AuthResourceResolver implements IAuthResourceResolver { private final DaoRegistry myDaoRegistry; @@ -23,7 +40,9 @@ public AuthResourceResolver(DaoRegistry myDaoRegistry) { } public IBaseResource resolveCompartmentById(IIdType theCompartmentId) { - return myDaoRegistry.getResourceDao(theCompartmentId.getResourceType()).read(theCompartmentId, new SystemRequestDetails()); + return myDaoRegistry + .getResourceDao(theCompartmentId.getResourceType()) + .read(theCompartmentId, new SystemRequestDetails()); } public List resolveCompartmentByIds(List theCompartmentId, String theResourceType) { @@ -33,6 +52,6 @@ public List resolveCompartmentByIds(List theCompartmentId m.add(Constants.PARAM_ID, t); return myDaoRegistry.getResourceDao(theResourceType).searchForResources(m, new SystemRequestDetails()); } - //translateMatchUrl + // translateMatchUrl } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java index 1c9b7d29f509..6e068eb9c8c6 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java @@ -172,7 +172,7 @@ public Verdict applyRulesAndReturnDecision( Verdict verdict = null; for (IAuthRule nextRule : rules) { - ourLog.trace("Rule being applied - {}", nextRule);//todo jdjd 1007 does bulk export not go through here? + ourLog.trace("Rule being applied - {}", nextRule); // todo jdjd 1007 does bulk export not go through here? verdict = nextRule.applyRule( theOperation, theRequestDetails, @@ -462,11 +462,20 @@ public void initiateBulkExport( if (theRequestDetails != null) { theRequestDetails.getUserData().put(REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS, theBulkExportOptions); } - //todo jdjd figure out what to do if we want to export patients, and patient is a list of ids in the bulk export options. so do you have to pass them in one by one? or how do we do this for other bulk operations? -// is this rule currently evaluated one by one for each patient included in the options? - // how do you do this? maybe a davinci export? https://smilecdr.com/docs/davinci_data_exchange/davinci_data_exchange.html#request-parameters - // or maybe with post w/ parameters resource and patient parameter in list of params? JpaConstants.PARAM_EXPORT_PATIENT and BulkDataExportProviderR4Test#testInitiatePatientExportRequest() - applyRulesAndFailIfDeny(restOperationType, theRequestDetails, null, new IdDt(theBulkExportOptions.getGroupId()), null, thePointcut); + // todo jdjd figure out what to do if we want to export patients, and patient is a list of ids in the bulk + // export options. so do you have to pass them in one by one? or how do we do this for other bulk operations? + // is this rule currently evaluated one by one for each patient included in the options? + // how do you do this? maybe a davinci export? + // https://smilecdr.com/docs/davinci_data_exchange/davinci_data_exchange.html#request-parameters + // or maybe with post w/ parameters resource and patient parameter in list of params? + // JpaConstants.PARAM_EXPORT_PATIENT and BulkDataExportProviderR4Test#testInitiatePatientExportRequest() + applyRulesAndFailIfDeny( + restOperationType, + theRequestDetails, + null, + new IdDt(theBulkExportOptions.getGroupId()), + null, + thePointcut); } /** diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/FhirQueriesRuleTester.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/FhirQueriesRuleTester.java index b264dcb66c58..3266a330d60f 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/FhirQueriesRuleTester.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/FhirQueriesRuleTester.java @@ -19,14 +19,12 @@ */ package ca.uhn.fhir.rest.server.interceptor.auth; -import java.util.ArrayList; -import java.util.List; - -import java.util.stream.Collectors; - import org.apache.commons.lang3.builder.ToStringBuilder; import org.apache.commons.lang3.builder.ToStringStyle; +import java.util.List; +import java.util.stream.Collectors; + /** * Tester that a resource matches any query filter in a list. */ @@ -36,14 +34,14 @@ public class FhirQueriesRuleTester implements IAuthRuleTester { public FhirQueriesRuleTester(List theQueries, String theResourceType) { myFilters = theQueries.stream() - .map(query -> query.contains("?") ? query.substring(query.indexOf("?") + 1) : query) - .map(FhirQueryRuleTester::new) - .collect(Collectors.toList()); + .map(query -> query.contains("?") ? query.substring(query.indexOf("?") + 1) : query) + .map(FhirQueryRuleTester::new) + .collect(Collectors.toList()); myResourceType = theResourceType; } public void addFilter(FhirQueriesRuleTester theOtherRuleTester) { - //todo jdjd can i add to list? + // todo jdjd can i add to list? if (theOtherRuleTester.getResourceType().equals(myResourceType)) { myFilters.addAll(theOtherRuleTester.getFilters()); } @@ -54,7 +52,9 @@ public List getFilters() { } public List getFiltersAsString() { - return myFilters.stream().map(param -> myResourceType + '?' + param.getQueryParameters()).toList(); + return myFilters.stream() + .map(param -> myResourceType + '?' + param.getQueryParameters()) + .toList(); } public String getResourceType() { @@ -71,13 +71,11 @@ public boolean matchesOutput(RuleTestRequest theRuleTestRequest) { return myFilters.stream().anyMatch(t -> t.matches(theRuleTestRequest)); } - //todo jdjds what is to string used for and what should the implementation look like for a list of queries + // todo jdjds what is to string used for and what should the implementation look like for a list of queries @Override public String toString() { return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE) - .append("filter", myFilters) - .toString(); + .append("filter", myFilters) + .toString(); } - - } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBulkExportImpl.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBulkExportImpl.java index 1312df4f32d9..0509ab7d8021 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBulkExportImpl.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBulkExportImpl.java @@ -24,25 +24,19 @@ import ca.uhn.fhir.rest.api.RestOperationTypeEnum; import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.api.server.bulk.BulkExportJobParameters; - -import static ca.uhn.fhir.rest.server.interceptor.auth.AuthorizationInterceptor.REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS; - import com.google.common.annotations.VisibleForTesting; - -import java.util.List; - -import java.util.Optional; - -import org.hl7.fhir.instance.model.api.IBase; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; import java.util.ArrayList; import java.util.Collection; +import java.util.List; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; +import static ca.uhn.fhir.rest.server.interceptor.auth.AuthorizationInterceptor.REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS; import static org.apache.commons.collections4.CollectionUtils.isEmpty; import static org.apache.commons.collections4.CollectionUtils.isNotEmpty; import static org.apache.commons.lang3.StringUtils.isNotBlank; @@ -66,13 +60,18 @@ public void addTester(IAuthRuleTester theNewTester) { if (!getTesters().isEmpty()) { // ensure only 1 tester with list of all filters in the rule // this ensures the queries/filters are applied as an "OR" instead of "AND" - Optional queriesTester = getTesters().stream().filter(FhirQueriesRuleTester.class::isInstance).findFirst(); + Optional queriesTester = getTesters().stream() + .filter(FhirQueriesRuleTester.class::isInstance) + .findFirst(); if (queriesTester.isPresent() && theNewTester instanceof FhirQueriesRuleTester) { FhirQueriesRuleTester existingTester = (FhirQueriesRuleTester) queriesTester.get(); - if (!existingTester.getResourceType().equals(((FhirQueriesRuleTester) theNewTester).getResourceType())) { - //TODO JDJD refine error message - throw new IllegalArgumentException("All queries for compartments should apply to the same resource type"); + if (!existingTester + .getResourceType() + .equals(((FhirQueriesRuleTester) theNewTester).getResourceType())) { + // TODO JDJD refine error message + throw new IllegalArgumentException( + "All queries for compartments should apply to the same resource type"); } existingTester.addFilter((FhirQueriesRuleTester) theNewTester); } @@ -91,7 +90,7 @@ public AuthorizationInterceptor.Verdict applyRule( IRuleApplier theRuleApplier, Set theFlags, Pointcut thePointcut) { - //todo jdjd here you can access mypatientIds and myGroupId in the class field. what are these? + // todo jdjd here you can access mypatientIds and myGroupId in the class field. what are these? // ans - they are the IDs specified in the rule not the bulk export input if (thePointcut != Pointcut.STORAGE_INITIATE_BULK_EXPORT) { return null; @@ -101,15 +100,13 @@ public AuthorizationInterceptor.Verdict applyRule( return null; } - BulkExportJobParameters inboundBulkExportRequestOptions = (BulkExportJobParameters) theRequestDetails - .getUserData() - .get(REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS); + BulkExportJobParameters inboundBulkExportRequestOptions = (BulkExportJobParameters) + theRequestDetails.getUserData().get(REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS); // if style doesn't match - abstain if (!myWantAnyStyle && inboundBulkExportRequestOptions.getExportStyle() != myWantExportStyle) { return null; } - // Do we only authorize some types? If so, make sure requested types are a subset if (isNotEmpty(myResourceTypes)) { if (isEmpty(inboundBulkExportRequestOptions.getResourceTypes())) { @@ -121,18 +118,19 @@ public AuthorizationInterceptor.Verdict applyRule( } boolean ruleUsesQueryToDetermineCompartment = false; - //todo jdjd what is the output resource? this time its null + // todo jdjd what is the output resource? this time its null // also is there a cleaner way other than 2 instanceof's - if (theInputResource == null && getTesters().stream().anyMatch(t->t instanceof FhirQueryRuleTester)) { + if (theInputResource == null && getTesters().stream().anyMatch(t -> t instanceof FhirQueryRuleTester)) { // The rule uses a filter query for eligible compartments - // TODO jdjd you can actually resolve the ID from getting it from request details user data BulkDataExportOptions - IIdType theResourceToQuery = theInputResourceId != null ? theInputResourceId : getIdFromRequest(theRequestDetails); + // TODO jdjd you can actually resolve the ID from getting it from request details user data + // BulkDataExportOptions + IIdType theResourceToQuery = + theInputResourceId != null ? theInputResourceId : getIdFromRequest(theRequestDetails); if (theInputResourceId != null) { theInputResource = theRuleApplier.getAuthResourceResolver().resolveCompartmentById(theResourceToQuery); ruleUsesQueryToDetermineCompartment = true; } - } // system only supports filtering by resource type. So if we are system, or any(), then allow, since we have @@ -146,7 +144,10 @@ public AuthorizationInterceptor.Verdict applyRule( theOutputResource, theRuleApplier); - if (myWantAnyStyle || myWantExportStyle == BulkExportJobParameters.ExportStyle.SYSTEM || (ruleUsesQueryToDetermineCompartment && myWantExportStyle.equals(BulkExportJobParameters.ExportStyle.GROUP))) { + if (myWantAnyStyle + || myWantExportStyle == BulkExportJobParameters.ExportStyle.SYSTEM + || (ruleUsesQueryToDetermineCompartment + && myWantExportStyle.equals(BulkExportJobParameters.ExportStyle.GROUP))) { return allowVerdict; } @@ -167,7 +168,7 @@ public AuthorizationInterceptor.Verdict applyRule( // 1. If each of the requested resource IDs in the parameters are present in the users permissions, Approve // 2. If any requested ID is not present in the users permissions, Deny. - if (myWantExportStyle == BulkExportJobParameters.ExportStyle.PATIENT) {// Unfiltered Type Level + if (myWantExportStyle == BulkExportJobParameters.ExportStyle.PATIENT) { // Unfiltered Type Level if (myAppliesToAllPatients) { return allowVerdict; } @@ -188,14 +189,18 @@ public AuthorizationInterceptor.Verdict applyRule( return new AuthorizationInterceptor.Verdict(PolicyEnum.DENY, this); } } - } else if (getTesters().stream().anyMatch(FhirQueriesRuleTester.class::isInstance)) { //todo jdjd refine this if condition + } else if (getTesters().stream() + .anyMatch(FhirQueriesRuleTester.class::isInstance)) { // todo jdjd refine this if condition // We don't have any specified Patient IDs to resolve the compartments we are allowed to export if (inboundBulkExportRequestOptions.getPatientIds().isEmpty()) { // There are no patient IDs requested in the bulk export. // This is either a type level export, or export with _typeFilter - FhirQueriesRuleTester tester = (FhirQueriesRuleTester) getTesters().stream().filter(FhirQueriesRuleTester.class::isInstance).findFirst().get(); + FhirQueriesRuleTester tester = (FhirQueriesRuleTester) getTesters().stream() + .filter(FhirQueriesRuleTester.class::isInstance) + .findFirst() + .get(); if (tester.getFiltersAsString().containsAll(inboundBulkExportRequestOptions.getFilters())) { // If the queries in the tester exactly match the queries in the in the bulk export, then allow return new AuthorizationInterceptor.Verdict(PolicyEnum.ALLOW, this); @@ -203,32 +208,48 @@ public AuthorizationInterceptor.Verdict applyRule( } else { // But we do have a FHIR query tester to resolve the compartment(s) // Query the DB for the requested compartment(s) in the bulk export, - // and apply a matcher between the requested compartment, and the FHIR query tested included in the permission + // and apply a matcher between the requested compartment, and the FHIR query tested included in the + // permission - List requestedPatientsToExport = theRuleApplier.getAuthResourceResolver().resolveCompartmentByIds(inboundBulkExportRequestOptions.getPatientIds(), "Patient"); + List requestedPatientsToExport = theRuleApplier + .getAuthResourceResolver() + .resolveCompartmentByIds(inboundBulkExportRequestOptions.getPatientIds(), "Patient"); // directly call contents of new verdict instead? and construct ur own verdict here. - boolean allAllowed = requestedPatientsToExport.stream().map(patient -> newVerdict( - theOperation, - theRequestDetails, - patient, - theInputResourceId,//todo jdjd should this be patient.id - theOutputResource, - theRuleApplier)).allMatch(t->t != null && t.getDecision().equals(PolicyEnum.ALLOW)); - //todo jdjd 1008 any or all match?? 1015 -- all right? because all resources have to be allowed to export + boolean allAllowed = requestedPatientsToExport.stream() + .map(patient -> newVerdict( + theOperation, + theRequestDetails, + patient, + theInputResourceId, // todo jdjd should this be patient.id + theOutputResource, + theRuleApplier)) + .allMatch(t -> t != null && t.getDecision().equals(PolicyEnum.ALLOW)); + // todo jdjd 1008 any or all match?? 1015 -- all right? because all resources have to be allowed to + // export // old reaonsing below - // well let's say you allow 2 different ruleBulkExportImpl - then you need to iterate through both and both must be allow (this needs to be done higher up). Or you can return null here so that it will iterate through them all. But assuming there's one bulkExportRule seems to be an assumption baked in to the existing implementation - // and let's say you only have 1 ruleBulkExportImpl - then you need to iterate through all your testers/filters and make sure at least one matches but currently, if one tester fails, the whole thing fails - // solution, probably when constructing the rule, if a test already exists, add to its filters for bulk export? + // well let's say you allow 2 different ruleBulkExportImpl - then you need to iterate through both and + // both must be allow (this needs to be done higher up). Or you can return null here so that it will + // iterate through them all. But assuming there's one bulkExportRule seems to be an assumption baked in + // to the existing implementation + // and let's say you only have 1 ruleBulkExportImpl - then you need to iterate through all your + // testers/filters and make sure at least one matches but currently, if one tester fails, the whole + // thing fails + // solution, probably when constructing the rule, if a test already exists, add to its filters for bulk + // export? // i think it never makes sense to have an AND because - // - you have diff resource types Patient?name=Doe Patient and Patient?name=Doe Encounter --> then just combine them - // - you have diff queries (want Patient?name=Doe and patient?identifier=abc|def), then just use the & in the query - // - diff resource types (Patient?name=Doe, and Organization?identifier=abc|123), then it doesnt make sense because permission is only patients and it doesnt make snense for resource to match both conditions + // - you have diff resource types Patient?name=Doe Patient and Patient?name=Doe Encounter --> then just + // combine them + // - you have diff queries (want Patient?name=Doe and patient?identifier=abc|def), then just use the & + // in the query + // - diff resource types (Patient?name=Doe, and Organization?identifier=abc|123), then it doesnt make + // sense because permission is only patients and it doesnt make snense for resource to match both + // conditions if (allAllowed) { return new AuthorizationInterceptor.Verdict(PolicyEnum.ALLOW, this); } else { - //TODO JDJD should it be DENY or abstain? + // TODO JDJD should it be DENY or abstain? // you have all the patients, not all of them match the matcher. Deny? return new AuthorizationInterceptor.Verdict(PolicyEnum.DENY, this); } @@ -240,8 +261,11 @@ public AuthorizationInterceptor.Verdict applyRule( private static IdDt getIdFromRequest(RequestDetails theRequestDetails) { // TODO JDJD modify based on group/patient export // also how do you handle patient list?? - // might also want to use export style (from bulk export params) intead of the resource type (from request details) - return new IdDt(((BulkExportJobParameters) theRequestDetails.getUserData().get(REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS)).getGroupId()); + // might also want to use export style (from bulk export params) intead of the resource type (from request + // details) + return new IdDt(((BulkExportJobParameters) + theRequestDetails.getUserData().get(REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS)) + .getGroupId()); } private Set sanitizeIds(Collection myPatientIds) { diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImpl.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImpl.java index 202bf0c784ae..89cec52973a0 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImpl.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImpl.java @@ -24,24 +24,22 @@ import ca.uhn.fhir.rest.api.RestOperationTypeEnum; import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.api.server.bulk.BulkExportJobParameters; - -import static ca.uhn.fhir.rest.server.interceptor.auth.AuthorizationInterceptor.REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS; - import com.google.common.annotations.VisibleForTesting; +import org.hl7.fhir.instance.model.api.IBaseResource; +import org.hl7.fhir.instance.model.api.IIdType; -import java.util.*; import java.util.stream.Collectors; +import java.util.*; +import static ca.uhn.fhir.rest.server.interceptor.auth.AuthorizationInterceptor.REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS; import static org.apache.commons.collections4.CollectionUtils.isEmpty; import static org.apache.commons.collections4.CollectionUtils.isNotEmpty; -import static org.apache.commons.lang3.StringUtils.isNotBlank; - -import org.hl7.fhir.instance.model.api.IBaseResource; -import org.hl7.fhir.instance.model.api.IIdType; public class RuleGroupBulkExportByCompartmentMatcherImpl extends BaseRule { - private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(RuleGroupBulkExportByCompartmentMatcherImpl.class); - private static final BulkExportJobParameters.ExportStyle OUR_EXPORT_STYLE = BulkExportJobParameters.ExportStyle.GROUP; + private static final org.slf4j.Logger ourLog = + org.slf4j.LoggerFactory.getLogger(RuleGroupBulkExportByCompartmentMatcherImpl.class); + private static final BulkExportJobParameters.ExportStyle OUR_EXPORT_STYLE = + BulkExportJobParameters.ExportStyle.GROUP; private String myGroupMatcherFilter; private Collection myResourceTypes; @@ -67,16 +65,14 @@ public AuthorizationInterceptor.Verdict applyRule( return null; } - BulkExportJobParameters inboundBulkExportRequestOptions = (BulkExportJobParameters) theRequestDetails - .getUserData() - .get(REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS); + BulkExportJobParameters inboundBulkExportRequestOptions = (BulkExportJobParameters) + theRequestDetails.getUserData().get(REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS); if (inboundBulkExportRequestOptions.getExportStyle() != OUR_EXPORT_STYLE) { // If the requested export style is not for a GROUP, then abstain return null; } - // Do we only authorize some types? If so, make sure requested types are a subset if (isNotEmpty(myResourceTypes)) { if (isEmpty(inboundBulkExportRequestOptions.getResourceTypes())) { @@ -89,9 +85,12 @@ public AuthorizationInterceptor.Verdict applyRule( } } - IBaseResource theGroupResource = theRuleApplier.getAuthResourceResolver().resolveCompartmentById(new IdDt(inboundBulkExportRequestOptions.getGroupId())); + IBaseResource theGroupResource = theRuleApplier + .getAuthResourceResolver() + .resolveCompartmentById(new IdDt(inboundBulkExportRequestOptions.getGroupId())); - // Apply the FhirQueryTester (which contains a inMemoryResourceMatcher) to the found Group compartment resource, and return the verdict + // Apply the FhirQueryTester (which contains a inMemoryResourceMatcher) to the found Group compartment resource, + // and return the verdict return newVerdict( theOperation, theRequestDetails, @@ -104,8 +103,11 @@ public AuthorizationInterceptor.Verdict applyRule( private static IdDt getIdFromRequest(RequestDetails theRequestDetails) { // TODO JDJD modify based on group/patient export // also how do you handle patient list?? - // might also want to use export style (from bulk export params) intead of the resource type (from request details) - return new IdDt(((BulkExportJobParameters) theRequestDetails.getUserData().get(REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS)).getGroupId()); + // might also want to use export style (from bulk export params) intead of the resource type (from request + // details) + return new IdDt(((BulkExportJobParameters) + theRequestDetails.getUserData().get(REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS)) + .getGroupId()); } private Set sanitizeIds(Collection myPatientIds) { From e070201e6fe1cd2b00f5f2488fbdb1c2d7dd8bd8 Mon Sep 17 00:00:00 2001 From: jdar Date: Tue, 21 Oct 2025 11:21:32 -0700 Subject: [PATCH 05/26] add Group rule builder code add unit tests --- .../auth/IAuthResourceResolver.java | 25 ++++++-- .../auth/IAuthRuleBuilderRule.java | 8 +++ ...RuleBuilderRuleGroupMatcherBulkExport.java | 34 +++++++++++ .../interceptor/auth/IAuthRuleFinished.java | 2 +- .../server/interceptor/auth/RuleBuilder.java | 57 ++++++++++++++++++- .../interceptor/auth/RuleBuilderTest.java | 32 +++++++++++ 6 files changed, 150 insertions(+), 8 deletions(-) create mode 100644 hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRuleGroupMatcherBulkExport.java diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthResourceResolver.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthResourceResolver.java index 750f485e86c2..53df3ccc1580 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthResourceResolver.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthResourceResolver.java @@ -1,12 +1,29 @@ +/*- + * #%L + * HAPI FHIR - Server Framework + * %% + * Copyright (C) 2014 - 2025 Smile CDR, Inc. + * %% + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * #L% + */ package ca.uhn.fhir.rest.server.interceptor.auth; -import ca.uhn.fhir.rest.api.server.SystemRequestDetails; - -import java.util.List; - import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; +import java.util.List; + public interface IAuthResourceResolver { IBaseResource resolveCompartmentById(IIdType theCompartmentId); diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRule.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRule.java index c2dfef0f3f4e..feb81ec11063 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRule.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRule.java @@ -127,6 +127,14 @@ public interface IAuthRuleBuilderRule { */ IAuthRuleBuilderRuleBulkExport bulkExport(); + /** + * This rule permits the user to initiate a FHIR bulk export + * by providing a filter matcher on Group compartment(s). + * + * @since 8.5.0 + */ + IAuthRuleBuilderRuleGroupMatcherBulkExport bulkExportGroupCompartmentMatcher(); + /** * This rule specifically allows a user to perform a FHIR update on the historical version of a resource * diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRuleGroupMatcherBulkExport.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRuleGroupMatcherBulkExport.java new file mode 100644 index 000000000000..74ce3bbaafb4 --- /dev/null +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRuleGroupMatcherBulkExport.java @@ -0,0 +1,34 @@ +/*- + * #%L + * HAPI FHIR - Server Framework + * %% + * Copyright (C) 2014 - 2025 Smile CDR, Inc. + * %% + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * #L% + */ +package ca.uhn.fhir.rest.server.interceptor.auth; + +import jakarta.annotation.Nonnull; + +/** + * @since 8.5.0 + */ +public interface IAuthRuleBuilderRuleGroupMatcherBulkExport { + /** + * Allow/deny group-level export rule applies to the Group with the given resource ID, e.g. Group/123 + * + * @since 8.5.0 + */ + IAuthRuleBuilderRuleBulkExportWithTarget groupExportOnGroup(@Nonnull String theCompartmentFilterMatcher); +} diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleFinished.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleFinished.java index a837ae4ddb1e..c513d26941f3 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleFinished.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleFinished.java @@ -57,7 +57,7 @@ public interface IAuthRuleFinished { */ IAuthRuleFinished withFilterTester(String theQueryParameters); - //todo jdjd docs + // todo jdjd docs default IAuthRuleFinished withFilterOnExistingTester(String theQueryParameters) { return this; } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java index fed246fae9d6..9f7acbc09811 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java @@ -29,7 +29,6 @@ import ca.uhn.fhir.rest.server.provider.ProviderConstants; import com.google.common.collect.Lists; import jakarta.annotation.Nonnull; -import net.sf.saxon.trans.SymbolicName; import org.apache.commons.lang3.Validate; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; @@ -261,6 +260,7 @@ private class RuleBuilderRule implements IAuthRuleBuilderRule { private RuleBuilderRuleOp myReadRuleBuilder; private RuleBuilderRuleOp myWriteRuleBuilder; private RuleBuilderBulkExport myRuleBuilderBulkExport; + private RuleBuilderGroupMatcherBulkExport myRuleBuilderGroupMatcherBulkExport; RuleBuilderRule(PolicyEnum theRuleMode, String theRuleName) { myRuleMode = theRuleMode; @@ -348,6 +348,14 @@ public IAuthRuleBuilderRuleBulkExport bulkExport() { return myRuleBuilderBulkExport; } + @Override + public IAuthRuleBuilderRuleGroupMatcherBulkExport bulkExportGroupCompartmentMatcher() { + if (myRuleBuilderGroupMatcherBulkExport == null) { + myRuleBuilderGroupMatcherBulkExport = new RuleBuilderGroupMatcherBulkExport(); + } + return myRuleBuilderGroupMatcherBulkExport; + } + @Override public IAuthRuleBuilderUpdateHistoryRewrite updateHistoryRewrite() { return new UpdateHistoryRewriteBuilder(); @@ -902,7 +910,7 @@ public IAuthRuleBuilderRuleBulkExportWithTarget groupExportOnGroup(@Nonnull Stri @Override public IAuthRuleBuilderRuleBulkExportWithTarget patientExportOnAllPatients() { - //todo JDJD 1008 it's this ==null that is problematic, it prevents duplicate bulkExportRuleImpl + // todo JDJD 1008 it's this ==null that is problematic, it prevents duplicate bulkExportRuleImpl if (myRuleBulkExport == null) { RuleBulkExportImpl rule = new RuleBulkExportImpl(myRuleName); rule.setMode(myRuleMode); @@ -940,7 +948,7 @@ public IAuthRuleBuilderRuleBulkExportWithTarget patientExportOnPatient(@Nonnull @Override public IAuthRuleBuilderRuleBulkExportWithTarget patientExportOnPatientStrings( @Nonnull Collection theFocusResourceIds) { - //todo JDJD 1008 it's this ==null that is problematic, it prevents duplicate bulkExportRuleImpl + // todo JDJD 1008 it's this ==null that is problematic, it prevents duplicate bulkExportRuleImpl if (myRuleBulkExport == null) { RuleBulkExportImpl rule = new RuleBulkExportImpl(myRuleName); rule.setAppliesToPatientExport(theFocusResourceIds); @@ -1004,6 +1012,49 @@ public IAuthRuleBuilderRuleBulkExportWithTarget withResourceTypes(Collection theResourceTypes) { + myRule.setResourceTypes(theResourceTypes); + return this; + } + } + } } private String toTypeName(Class theType) { diff --git a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilderTest.java b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilderTest.java index 9f9d01ee2ac4..a798a4cd35e4 100644 --- a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilderTest.java +++ b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilderTest.java @@ -84,6 +84,37 @@ public void testBulkExportPermitsIfASingleGroupMatches() { } + @ParameterizedTest + @MethodSource("groupMatcherBulkExportParams") + public void testBulkExportByGroupMatcher(String theCompartmentMatcherFilter, Collection theResourceTypes) { + // Given + RuleBuilder builder = new RuleBuilder(); + List resourceTypes = new ArrayList<>(theResourceTypes); + + // When + builder.allow().bulkExportGroupCompartmentMatcher().groupExportOnGroup(theCompartmentMatcherFilter).withResourceTypes(resourceTypes); + final List rules = builder.build(); + + // Then + assertEquals(1, rules.size()); + final IAuthRule authRule = rules.get(0); + assertInstanceOf(RuleGroupBulkExportByCompartmentMatcherImpl.class, authRule); + + final RuleGroupBulkExportByCompartmentMatcherImpl ruleGroupBulkExport = (RuleGroupBulkExportByCompartmentMatcherImpl) authRule; + assertEquals(theCompartmentMatcherFilter, ruleGroupBulkExport.getGroupMatcherFilter()); + assertEquals(theResourceTypes, ruleGroupBulkExport.getResourceTypes()); + assertEquals(PolicyEnum.ALLOW, ruleGroupBulkExport.getMode()); + } + + private static Stream groupMatcherBulkExportParams() { + return Stream.of( + Arguments.of("?identifier=foo|bar", List.of()), + Arguments.of("?identifier=foo|bar", List.of("Patient", "Observation")) + ); + } + + + @Test public void testBulkExport_PatientExportOnPatient_MultiplePatientsSingleRule() { RuleBuilder builder = new RuleBuilder(); @@ -126,6 +157,7 @@ public void testBulkExport_PatientExportOnPatients_MultiplePatientsSingleRule(Co assertEquals(theExpectedResourceTypes, ruleBulkExport.getResourceTypes()); assertEquals(thePolicyEnum, ruleBulkExport.getMode()); } + //todo jdjd test like above public static Stream owners() { return Stream.of( From 33d0df77f3112e849700683e57b03d8616d4b22a Mon Sep 17 00:00:00 2001 From: jdar Date: Thu, 23 Oct 2025 11:04:35 -0700 Subject: [PATCH 06/26] implement basic Patient rule with tests --- ...entBulkExportByCompartmentMatcherImpl.java | 251 ++++++++++++++++++ ...ulkExportByCompartmentMatcherImplTest.java | 4 +- ...ulkExportByCompartmentMatcherImplTest.java | 215 +++++++++++++++ 3 files changed, 468 insertions(+), 2 deletions(-) create mode 100644 hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java create mode 100644 hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImplTest.java diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java new file mode 100644 index 000000000000..0b0cc239017e --- /dev/null +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java @@ -0,0 +1,251 @@ +/*- + * #%L + * HAPI FHIR - Server Framework + * %% + * Copyright (C) 2014 - 2025 Smile CDR, Inc. + * %% + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * #L% + */ +package ca.uhn.fhir.rest.server.interceptor.auth; + +import ca.uhn.fhir.interceptor.api.Pointcut; +import ca.uhn.fhir.model.primitive.IdDt; +import ca.uhn.fhir.rest.api.RestOperationTypeEnum; +import ca.uhn.fhir.rest.api.server.RequestDetails; +import ca.uhn.fhir.rest.api.server.bulk.BulkExportJobParameters; +import com.google.common.annotations.VisibleForTesting; + +import java.util.ArrayList; +import java.util.List; + +import java.util.Objects; + +import org.hl7.fhir.instance.model.api.IBaseResource; +import org.hl7.fhir.instance.model.api.IIdType; + +import java.util.Collection; +import java.util.Set; +import java.util.stream.Collectors; + +import static ca.uhn.fhir.rest.server.interceptor.auth.AuthorizationInterceptor.REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS; +import static org.apache.commons.collections4.CollectionUtils.isEmpty; +import static org.apache.commons.collections4.CollectionUtils.isNotEmpty; + +public class RulePatientBulkExportByCompartmentMatcherImpl extends BaseRule { + private static final org.slf4j.Logger ourLog = + org.slf4j.LoggerFactory.getLogger(RulePatientBulkExportByCompartmentMatcherImpl.class); + private static final BulkExportJobParameters.ExportStyle OUR_EXPORT_STYLE = + BulkExportJobParameters.ExportStyle.PATIENT; + private List myPatientMatcherFilter; + private List> myTokenizedPatientMatcherFilter; + private Collection myResourceTypes; + + RulePatientBulkExportByCompartmentMatcherImpl(String theRuleName) { + super(theRuleName); + } + + @Override + public AuthorizationInterceptor.Verdict applyRule( + RestOperationTypeEnum theOperation, + RequestDetails theRequestDetails, + IBaseResource theInputResource, + IIdType theInputResourceId, + IBaseResource theOutputResource, + IRuleApplier theRuleApplier, + Set theFlags, + Pointcut thePointcut) { + if (thePointcut != Pointcut.STORAGE_INITIATE_BULK_EXPORT) { + return null; + } + + if (theRequestDetails == null) { + return null; + } + + BulkExportJobParameters inboundBulkExportRequestOptions = (BulkExportJobParameters) + theRequestDetails.getUserData().get(REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS); + + if (inboundBulkExportRequestOptions.getExportStyle() != OUR_EXPORT_STYLE) { + // If the requested export style is not for a GROUP, then abstain + return null; + } + + // Do we only authorize some types? If so, make sure requested types are a subset + if (isNotEmpty(myResourceTypes)) { + if (isEmpty(inboundBulkExportRequestOptions.getResourceTypes())) { + // Attempting an export on ALL resource types, but this rule restricts on a set of resource types + return new AuthorizationInterceptor.Verdict(PolicyEnum.DENY, this); + } + if (!myResourceTypes.containsAll(inboundBulkExportRequestOptions.getResourceTypes())) { + // The requested resource types is not a subset of the permitted resource types + return new AuthorizationInterceptor.Verdict(PolicyEnum.DENY, this); + } + } + + // TODO JDJD you need to handle + // export with list of ids + // export with list of ids (matching on filter) + // export with list of ids (non-matching on filter) + // export at type with no list of ids (matcher by type filter) + // permission contains OR + + List patientIdOptions = inboundBulkExportRequestOptions.getPatientIds(); + List filterOptions = inboundBulkExportRequestOptions.getFilters(); + + if (!filterOptions.isEmpty()) { + // Export with a _typeFilter + boolean allFiltersMatch = matchOnFilterOptions(filterOptions); + + if (patientIdOptions.isEmpty()) { + // This is a type-level export with a _typeFilter + // All filters must be permitted to return an ALLOW verdict + return allFiltersMatch ? + new AuthorizationInterceptor.Verdict(PolicyEnum.ALLOW, this) : + new AuthorizationInterceptor.Verdict(PolicyEnum.DENY, this); + } else if (!allFiltersMatch) { + // This is an instance-level export with a _typeFilter + // Where at least one filter didn't match the permitted filters + return new AuthorizationInterceptor.Verdict(PolicyEnum.DENY, this); + } + } + + List thePatientResources = theRuleApplier + .getAuthResourceResolver() + .resolveCompartmentByIds(patientIdOptions, "Patient"); + + // Apply the FhirQueryTester (which contains a inMemoryResourceMatcher) to the found Patient compartment resource, + // and return the verdict + // All requested Patient IDs must be permitted to return an ALLOW verdict. + List verdicts = thePatientResources.stream() + .map(patient -> newVerdict( + theOperation, + theRequestDetails, + patient, + theInputResourceId, // todo jdjd should this be patient.id? + theOutputResource, + theRuleApplier)) + .toList(); + + if (verdicts.stream().allMatch(t -> t != null && t.getDecision().equals(PolicyEnum.ALLOW))) { + // All the resources match at least 1 permission query filter --> ALLOW + return new AuthorizationInterceptor.Verdict(PolicyEnum.ALLOW, this); + } else if (verdicts.stream().allMatch(Objects::isNull)) { + // All the resources do not match any permission query filters + // This rule must not apply --> abstain + return null; + } else { + // We have a mixture of ALLOW and abstain + // Default to DENY + return new AuthorizationInterceptor.Verdict(PolicyEnum.DENY, this); + } + } + + /** + * See if ALL the requested _typeFilters match at least one of the permitted filters as defined in the permission. + * + * In order for the export to be allowed, at least one permission argument filter must exactly match all search parameters included in the query + * The search parameters in the filters are tokenized so that parameter ordering does not matter + * + * Example 1: Patient?name=Doe&active=true == Patient?active=true&name=Doe + * Example 2: Patient?name=Doe != Patient?active=True&name=Doe + * + * @param theFilterOptions The inbound export _typeFilter options. + * As per the spec, these filters should have a resource type. + * (https://build.fhir.org/ig/HL7/bulk-data/en/export.html#_typefilter-query-parameter) + * + * @return true if the all _typeFilters are permitted, false otherwise + */ + private boolean matchOnFilterOptions(List theFilterOptions) { + for (String filter : theFilterOptions) { + String query = sanitizeQueryFilter(filter); + + Set tokenizedQuery = Set.of(query.split("&")); + + if (!myTokenizedPatientMatcherFilter.contains(tokenizedQuery)) { + return false; + } + } + return true; + } + + /** + * Remove the resource type and "?" prefix, if present + * @param theFilter + * @return + */ + private static String sanitizeQueryFilter(String theFilter) { + if (theFilter.contains("?")) { + + return theFilter.substring(theFilter.indexOf("?")); + } + return theFilter; + } + + private static IdDt getIdFromRequest(RequestDetails theRequestDetails) { + // TODO JDJD modify based on group/patient export + // also how do you handle patient list?? + // might also want to use export style (from bulk export params) intead of the resource type (from request + // details) + return new IdDt(((BulkExportJobParameters) + theRequestDetails.getUserData().get(REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS)) + .getGroupId()); + } + + private Set sanitizeIds(Collection myPatientIds) { + return myPatientIds.stream() + .map(id -> new IdDt(id).toUnqualifiedVersionless().getValue()) + .collect(Collectors.toSet()); + } + + public void setResourceTypes(Collection theResourceTypes) { + myResourceTypes = theResourceTypes; + } + + // TODO jdjd do we need to clear the testers since we are replacing the filter string? + // but testers is not modifiable i think? + + /** + * @param thePatientMatcherFilter the matcher filter for the permitted Patient + * Note that resource type is implied as this queries for the compartment + * So this filter should start with a "?" + */ + public void addAppliesToPatientExportOnPatient(String thePatientMatcherFilter) { + //todo jdjd what does the string look like here? should i have a ? or resource type? + if (myPatientMatcherFilter == null){ + myPatientMatcherFilter = new ArrayList<>(); + } + myPatientMatcherFilter.add(sanitizeQueryFilter(thePatientMatcherFilter)); + + if (myTokenizedPatientMatcherFilter == null){ + myTokenizedPatientMatcherFilter = new ArrayList<>(); + } + myTokenizedPatientMatcherFilter.add(Set.of(thePatientMatcherFilter.split("&"))); + + + addTester(new FhirQueryRuleTester(thePatientMatcherFilter)); + } + + List getPatientMatcherFilter() { + return myPatientMatcherFilter; + } + + BulkExportJobParameters.ExportStyle getWantExportStyle() { + return OUR_EXPORT_STYLE; + } + + @VisibleForTesting + Collection getResourceTypes() { + return myResourceTypes; + } +} diff --git a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImplTest.java b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImplTest.java index 50f88ac902be..60f85f711228 100644 --- a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImplTest.java +++ b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImplTest.java @@ -80,14 +80,14 @@ void testGroupRule_withCompartmentMatchers(IAuthorizationSearchParamMatcher.Matc when(mySearchParamMatcher.match("Group?identifier=foo|bar", myResource)).thenReturn(theSearchParamMatcherMatchResult); } + AuthorizationInterceptor.Verdict verdict = rule.applyRule(myOperation, myRequestDetails, null, null, null, myRuleApplier, myFlags, myPointcut); + if (theExpectedVerdict != null) { // Expect a decision - AuthorizationInterceptor.Verdict verdict = rule.applyRule(myOperation, myRequestDetails, null, null, null, myRuleApplier, myFlags, myPointcut); Assertions.assertNotNull(verdict, "Expected " + theExpectedVerdict + " but got abstain - " + theMessage); assertEquals(theExpectedVerdict, verdict.getDecision(), "Expected " + theExpectedVerdict + " but got " + verdict.getDecision() + " - " + theMessage); } else { // Expect abstain - AuthorizationInterceptor.Verdict verdict = rule.applyRule(myOperation, myRequestDetails, null, null, null, myRuleApplier, myFlags, myPointcut); assertNull(verdict, "Expected abstain - " + theMessage); } } diff --git a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImplTest.java b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImplTest.java new file mode 100644 index 000000000000..af50f0185a82 --- /dev/null +++ b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImplTest.java @@ -0,0 +1,215 @@ +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.BulkExportJobParameters; + +import static ca.uhn.fhir.rest.api.server.bulk.BulkExportJobParameters.ExportStyle.*; + +import java.util.Collection; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.stream.Stream; + +import org.hl7.fhir.instance.model.api.IBaseResource; +import org.junit.jupiter.api.Assertions; + +import static org.junit.jupiter.api.Assertions.*; + +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import static org.mockito.ArgumentMatchers.any; + +import static org.mockito.ArgumentMatchers.eq; + +import org.mockito.Mock; + +import static org.mockito.Mockito.when; + +import org.mockito.junit.jupiter.MockitoExtension; + +@ExtendWith(MockitoExtension.class) +class RulePatientBulkExportByCompartmentMatcherImplTest { + + private final RestOperationTypeEnum myOperation = RestOperationTypeEnum.EXTENDED_OPERATION_SERVER; + private final Pointcut myPointcut = Pointcut.STORAGE_INITIATE_BULK_EXPORT; + + @Mock + private IRuleApplier myRuleApplier; + @Mock + private IAuthResourceResolver myAuthResourceResolver; + @Mock + private IBaseResource myResource; + @Mock + private IBaseResource myResource2; + @Mock + private IAuthorizationSearchParamMatcher mySearchParamMatcher; + @Mock + private RequestDetails myRequestDetails; + @Mock + private Set myFlags; + + + @ParameterizedTest + @MethodSource("paramsInstanceLevel") + void testPatientRule_instanceLevelExport_withCompartmentMatchers(Collection theAllowedResourceTypes, + BulkExportJobParameters theBulkExportJobParams, + List theSearchParamMatcherMatchResults, + PolicyEnum theExpectedVerdict, + String theMessage) { + RulePatientBulkExportByCompartmentMatcherImpl rule = new RulePatientBulkExportByCompartmentMatcherImpl("b"); + rule.addAppliesToPatientExportOnPatient("identifier=foo|bar"); + rule.setResourceTypes(theAllowedResourceTypes); + rule.setMode(PolicyEnum.ALLOW); + + when(myRequestDetails.getUserData()).thenReturn(Map.of(AuthorizationInterceptor.REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS, theBulkExportJobParams)); + + if (theSearchParamMatcherMatchResults != null) { + when(myRuleApplier.getAuthResourceResolver()).thenReturn(myAuthResourceResolver); + if (theSearchParamMatcherMatchResults.size() == 1) { + when(myAuthResourceResolver.resolveCompartmentByIds(any(), eq("Patient"))).thenReturn(List.of(myResource)); + } else if (theSearchParamMatcherMatchResults.size() == 2) { + when(myAuthResourceResolver.resolveCompartmentByIds(any(), eq("Patient"))).thenReturn(List.of(myResource, myResource2)); + when(myResource2.fhirType()).thenReturn("Patient"); + when(mySearchParamMatcher.match("Patient?identifier=foo|bar", myResource2)).thenReturn(theSearchParamMatcherMatchResults.get(1)); + } + when(myRuleApplier.getSearchParamMatcher()).thenReturn(mySearchParamMatcher); + when(myResource.fhirType()).thenReturn("Patient"); + when(mySearchParamMatcher.match("Patient?identifier=foo|bar", myResource)).thenReturn(theSearchParamMatcherMatchResults.get(0)); + } + + AuthorizationInterceptor.Verdict verdict = rule.applyRule(myOperation, myRequestDetails, null, null, null, myRuleApplier, myFlags, myPointcut); + + if (theExpectedVerdict != null) { + // Expect a decision + Assertions.assertNotNull(verdict, "Expected " + theExpectedVerdict + " but got abstain - " + theMessage); + assertEquals(theExpectedVerdict, verdict.getDecision(), "Expected " + theExpectedVerdict + " but got " + verdict.getDecision() + " - " + theMessage); + } else { + // Expect abstain + assertNull(verdict, "Expected abstain - " + theMessage); + } + } + + static Stream paramsInstanceLevel() { + IAuthorizationSearchParamMatcher.MatchResult match = IAuthorizationSearchParamMatcher.MatchResult.buildMatched(); + IAuthorizationSearchParamMatcher.MatchResult noMatch = IAuthorizationSearchParamMatcher.MatchResult.buildUnmatched(); + + // theAllowedResourceTypes, theBulkExportJobParams, theSearchParamMatcherMatchResults, theExpectedVerdict, theMessage + return Stream.of( + // One patient cases + Arguments.of(List.of(), new BulkExportParamsBuilder().exportOnePatient().build(), List.of(match), PolicyEnum.ALLOW, "Allow request for all types, permit all types"), + Arguments.of(List.of(), new BulkExportParamsBuilder().exportOnePatient().withResourceTypes("Patient", "Observation").build(), List.of(match), PolicyEnum.ALLOW, "Allow request for some types, permit all types"), + Arguments.of(List.of("Patient", "Observation"), new BulkExportParamsBuilder().exportOnePatient().withResourceTypes("Patient","Observation").build(), List.of(match), PolicyEnum.ALLOW, "Allow request for exact set of allowable types"), + Arguments.of(List.of("Patient", "Observation"), new BulkExportParamsBuilder().exportOnePatient().withResourceTypes("Patient").build(), List.of(match), PolicyEnum.ALLOW, "Allow request for subset of allowable types"), + Arguments.of(List.of("Patient"), new BulkExportParamsBuilder().exportOnePatient().withResourceTypes("Patient").build(), List.of(noMatch), null, "Abstain when requesting some resource types, but no resources match the permission query"), + Arguments.of(List.of(), new BulkExportParamsBuilder().exportOnePatient().build(), List.of(noMatch), null, "Abstain when requesting all resource types, but no resources match the permission query"), + // Below is the narrowing case. Narrowing should happen at the SecurityInterceptor layer. Here, we expect deny + Arguments.of(List.of("Patient", "Observation"), new BulkExportParamsBuilder().exportOnePatient().build(), null, PolicyEnum.DENY, "Deny request for all types when allowing some types"), + Arguments.of(List.of("Patient", "Observation"), new BulkExportParamsBuilder().exportOnePatient().withResourceTypes("Patient","Observation","Encounter").build(), null, PolicyEnum.DENY, "Deny request for superset of allowable types"), + Arguments.of(List.of(), new BulkExportParamsBuilder().exportOnePatient().withExportStyle(GROUP).build(), null, null, "Abstain when export style is not Group"), + + // Two patient cases + Arguments.of(List.of(), new BulkExportParamsBuilder().exportTwoPatients().build(), List.of(match, match), PolicyEnum.ALLOW, "Allow request for all types on 2 patients, permit all types"), + Arguments.of(List.of(), new BulkExportParamsBuilder().exportTwoPatients().withResourceTypes("Patient", "Observation").build(), List.of(match, match), PolicyEnum.ALLOW, "Allow request for some types on 2 patients, permit all types"), + Arguments.of(List.of("Patient", "Observation"), new BulkExportParamsBuilder().exportTwoPatients().withResourceTypes("Patient","Observation").build(), List.of(match, match), PolicyEnum.ALLOW, "Allow request for exact set of allowable types on 2 patients"), + Arguments.of(List.of("Patient", "Observation"), new BulkExportParamsBuilder().exportTwoPatients().withResourceTypes("Patient").build(), List.of(match, match), PolicyEnum.ALLOW, "Allow request on 2 patients for subset of allowable types"), + Arguments.of(List.of("Patient"), new BulkExportParamsBuilder().exportTwoPatients().withResourceTypes("Patient").build(), List.of(noMatch), null, "Abstain when requesting some resource types on 2 patients, but no resources match the permission query"), + Arguments.of(List.of("Patient"), new BulkExportParamsBuilder().exportTwoPatients().withResourceTypes("Patient").build(), List.of(match, noMatch), PolicyEnum.DENY, "Deny when requesting some resource types on 2 patients, but only one Patient match the permission query"), + + // Instance-level with _typeFilter + Arguments.of(List.of("Patient", "Observation"), new BulkExportParamsBuilder().exportOnePatient().withTypeFilters("identifier=foo|bar").withResourceTypes("Patient","Observation").build(), List.of(match), PolicyEnum.ALLOW, "Allow request with typeFilter for exact set of allowable types"), + Arguments.of(List.of("Patient", "Observation"), new BulkExportParamsBuilder().exportOnePatient().withTypeFilters("identifier=abc|def").withResourceTypes("Patient","Observation").build(), null, PolicyEnum.DENY, "Deny request with typeFilter when all don't match permissible filter"), + Arguments.of(List.of("Patient", "Observation"), new BulkExportParamsBuilder().exportOnePatient().withTypeFilters("identifier=foo|bar", "name=Doe").withResourceTypes("Patient","Observation").build(), null, PolicyEnum.DENY, "Deny request with typeFilter when one doesn't match permissible filter"), + Arguments.of(List.of("Patient", "Observation"), new BulkExportParamsBuilder().exportTwoPatients().withTypeFilters("identifier=foo|bar").withResourceTypes("Patient","Observation").build(), List.of(match, noMatch), PolicyEnum.DENY, "Deny request with typeFilter for exact set of allowable types, but matcher doesn't match") + ); + } + + @ParameterizedTest + @MethodSource("paramsTypeLevel") + void testPatientRule_typeLevelExport_withCompartmentMatchers(Collection theAllowedResourceTypes, + Collection thePermissionFilters, + BulkExportJobParameters theBulkExportJobParams, + PolicyEnum theExpectedVerdict, + String theMessage) { + RulePatientBulkExportByCompartmentMatcherImpl rule = new RulePatientBulkExportByCompartmentMatcherImpl("b"); + for (String filter : thePermissionFilters) { + rule.addAppliesToPatientExportOnPatient(filter); + } + rule.setResourceTypes(theAllowedResourceTypes); + rule.setMode(PolicyEnum.ALLOW); + + when(myRequestDetails.getUserData()).thenReturn(Map.of(AuthorizationInterceptor.REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS, theBulkExportJobParams)); + + AuthorizationInterceptor.Verdict verdict = rule.applyRule(myOperation, myRequestDetails, null, null, null, myRuleApplier, myFlags, myPointcut); + + if (theExpectedVerdict != null) { + // Expect a decision + Assertions.assertNotNull(verdict, "Expected " + theExpectedVerdict + " but got abstain - " + theMessage); + assertEquals(theExpectedVerdict, verdict.getDecision(), "Expected " + theExpectedVerdict + " but got " + verdict.getDecision() + " - " + theMessage); + } else { + // Expect abstain + assertNull(verdict, "Expected abstain - " + theMessage); + } + } + + static Stream paramsTypeLevel() { + // theAllowedResourceTypes, thePermissionFilters, theBulkExportJobParams, theExpectedVerdict, theMessage + return Stream.of( + // Type-Level export with _typeFilter + Arguments.of(List.of(), List.of("identifier=foo|bar"), new BulkExportParamsBuilder().withTypeFilters("identifier=foo|bar").build(), PolicyEnum.ALLOW, "Allow request when filters match"), + Arguments.of(List.of(), List.of("identifier=foo|bar", "name=Doe"), new BulkExportParamsBuilder().withTypeFilters("identifier=foo|bar").build(), PolicyEnum.ALLOW, "Allow request with subset of permitted filters"), + Arguments.of(List.of(), List.of("identifier=foo|bar", "name=Doe"), new BulkExportParamsBuilder().withTypeFilters("identifier=foo|bar", "name=Doe").build(), PolicyEnum.ALLOW, "Allow request when multiple filters match"), + Arguments.of(List.of("Patient"), List.of("identifier=foo|bar", "name=Doe"), new BulkExportParamsBuilder().withTypeFilters("identifier=foo|bar", "name=Doe").withResourceTypes("Patient").build(), PolicyEnum.ALLOW, "Allow request when multiple filters match including resource type"), + Arguments.of(List.of("Observation"), List.of("identifier=foo|bar", "name=Doe"), new BulkExportParamsBuilder().withTypeFilters("identifier=foo|bar", "name=Doe").withResourceTypes("Patient").build(), PolicyEnum.DENY, "Deny request when multiple filters match, but resource type doesn't"), + Arguments.of(List.of(), List.of("identifier=foo|bar&active=true", "name=Doe"), new BulkExportParamsBuilder().withTypeFilters("identifier=foo|bar&active=true", "name=Doe").build(), PolicyEnum.ALLOW, "Allow request when multiple filters match"), + Arguments.of(List.of(), List.of("identifier=foo|bar", "name=Doe"), new BulkExportParamsBuilder().withTypeFilters("identifier=foo|bar&active=true", "name=Doe").build(), PolicyEnum.DENY, "Deny request when filters don't match"), + Arguments.of(List.of(), List.of("identifier=abc|def"), new BulkExportParamsBuilder().withTypeFilters("identifier=foo|bar").build(), PolicyEnum.DENY, "Deny request when filters do not match"), + Arguments.of(List.of(), List.of("identifier=foo|bar"), new BulkExportParamsBuilder().withTypeFilters("identifier=foo|bar","name=Doe").build(), PolicyEnum.DENY, "Deny request when requesting more filters than permitted") + ); + } + + private static class BulkExportParamsBuilder { + + private final BulkExportJobParameters myBulkExportJobParameters; + + public BulkExportParamsBuilder() { + myBulkExportJobParameters = new BulkExportJobParameters(); + myBulkExportJobParameters.setExportStyle(PATIENT); + } + + public BulkExportParamsBuilder withExportStyle(BulkExportJobParameters.ExportStyle theStyle) { + myBulkExportJobParameters.setExportStyle(theStyle); + return this; + } + + public BulkExportParamsBuilder withResourceTypes(String... theResourceTypes) { + myBulkExportJobParameters.setResourceTypes(List.of(theResourceTypes)); + return this; + } + + public BulkExportParamsBuilder exportOnePatient() { + myBulkExportJobParameters.setPatientIds(List.of("Patient/1")); + return this; + } + + public BulkExportParamsBuilder exportTwoPatients() { + myBulkExportJobParameters.setPatientIds(List.of("Patient/1", "Patient/2")); + return this; + } + + public BulkExportParamsBuilder withTypeFilters(String... theFilters) { + myBulkExportJobParameters.setFilters(List.of(theFilters)); + return this; + } + + public BulkExportJobParameters build() { + return myBulkExportJobParameters; + } + } + +} From 5575871aca0d0314a9af7fe13e3271fea3286f17 Mon Sep 17 00:00:00 2001 From: jdar Date: Fri, 24 Oct 2025 10:44:29 -0700 Subject: [PATCH 07/26] Implement rule building for patient by matcher --- .../auth/IAuthRuleBuilderRule.java | 8 +++ ...leBuilderRulePatientMatcherBulkExport.java | 34 ++++++++++++ .../server/interceptor/auth/RuleBuilder.java | 53 +++++++++++++++++++ .../interceptor/auth/RuleBuilderTest.java | 32 +++++++++++ 4 files changed, 127 insertions(+) create mode 100644 hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRulePatientMatcherBulkExport.java diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRule.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRule.java index feb81ec11063..896d5336a244 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRule.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRule.java @@ -135,6 +135,14 @@ public interface IAuthRuleBuilderRule { */ IAuthRuleBuilderRuleGroupMatcherBulkExport bulkExportGroupCompartmentMatcher(); + /** + * This rule permits the user to initiate a FHIR bulk export + * by providing a filter matcher on Patient compartment(s). + * + * @since 8.5.0 + */ + IAuthRuleBuilderRulePatientMatcherBulkExport bulkExportPatientCompartmentMatcher(); + /** * This rule specifically allows a user to perform a FHIR update on the historical version of a resource * diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRulePatientMatcherBulkExport.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRulePatientMatcherBulkExport.java new file mode 100644 index 000000000000..ef48c2ef4962 --- /dev/null +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRulePatientMatcherBulkExport.java @@ -0,0 +1,34 @@ +/*- + * #%L + * HAPI FHIR - Server Framework + * %% + * Copyright (C) 2014 - 2025 Smile CDR, Inc. + * %% + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * #L% + */ +package ca.uhn.fhir.rest.server.interceptor.auth; + +import jakarta.annotation.Nonnull; + +/** + * @since 8.5.0 + */ +public interface IAuthRuleBuilderRulePatientMatcherBulkExport { + /** + * Allow/deny group-level export rule applies to the Group with the given resource ID, e.g. Group/123 + * + * @since 8.5.0 + */ + IAuthRuleBuilderRuleBulkExportWithTarget patientExportOnPatient(@Nonnull String theCompartmentFilterMatcher); +} diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java index 9f7acbc09811..0f86b1758cd5 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java @@ -261,6 +261,7 @@ private class RuleBuilderRule implements IAuthRuleBuilderRule { private RuleBuilderRuleOp myWriteRuleBuilder; private RuleBuilderBulkExport myRuleBuilderBulkExport; private RuleBuilderGroupMatcherBulkExport myRuleBuilderGroupMatcherBulkExport; + private RuleBuilderPatientMatcherBulkExport myRuleBuilderPatientMatcherBulkExport; RuleBuilderRule(PolicyEnum theRuleMode, String theRuleName) { myRuleMode = theRuleMode; @@ -356,6 +357,14 @@ public IAuthRuleBuilderRuleGroupMatcherBulkExport bulkExportGroupCompartmentMatc return myRuleBuilderGroupMatcherBulkExport; } + @Override + public IAuthRuleBuilderRulePatientMatcherBulkExport bulkExportPatientCompartmentMatcher() { + if (myRuleBuilderPatientMatcherBulkExport == null) { + myRuleBuilderPatientMatcherBulkExport = new RuleBuilderPatientMatcherBulkExport(); + } + return myRuleBuilderPatientMatcherBulkExport; + } + @Override public IAuthRuleBuilderUpdateHistoryRewrite updateHistoryRewrite() { return new UpdateHistoryRewriteBuilder(); @@ -1055,6 +1064,50 @@ public IAuthRuleBuilderRuleBulkExportWithTarget withResourceTypes(Collection theResourceTypes) { + myRule.setResourceTypes(theResourceTypes); + return this; + } + } + } } private String toTypeName(Class theType) { diff --git a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilderTest.java b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilderTest.java index a798a4cd35e4..3e59ab6916da 100644 --- a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilderTest.java +++ b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilderTest.java @@ -113,7 +113,39 @@ private static Stream groupMatcherBulkExportParams() { ); } + @ParameterizedTest + @MethodSource("patientMatcherBulkExportParams") + public void testBulkExportByPatientMatcher(List theCompartmentMatcherFilter, Collection theResourceTypes) { + // Given + RuleBuilder builder = new RuleBuilder(); + List resourceTypes = new ArrayList<>(theResourceTypes); + // When + for (String filter : theCompartmentMatcherFilter) { + builder.allow().bulkExportPatientCompartmentMatcher().patientExportOnPatient(filter).withResourceTypes(resourceTypes); + } + + final List rules = builder.build(); + + // Then + assertEquals(1, rules.size()); + final IAuthRule authRule = rules.get(0); + assertInstanceOf(RulePatientBulkExportByCompartmentMatcherImpl.class, authRule); + + final RulePatientBulkExportByCompartmentMatcherImpl rulePatientExport = (RulePatientBulkExportByCompartmentMatcherImpl) authRule; + assertThat(rulePatientExport.getPatientMatcherFilter()).containsExactlyInAnyOrderElementsOf(theCompartmentMatcherFilter); + assertEquals(theResourceTypes, rulePatientExport.getResourceTypes()); + assertEquals(PolicyEnum.ALLOW, rulePatientExport.getMode()); + } + + private static Stream patientMatcherBulkExportParams() { + return Stream.of( + Arguments.of(List.of("?identifier=foo|bar"), List.of()), + Arguments.of(List.of("?identifier=foo|bar"), List.of("Patient", "Observation")), + Arguments.of(List.of("?identifier=foo|bar", "?name=Doe"), List.of()), + Arguments.of(List.of("?identifier=foo|bar", "?name=Doe&active=true"), List.of("Patient", "Observation")) + ); + } @Test public void testBulkExport_PatientExportOnPatient_MultiplePatientsSingleRule() { From 6ff269b0956af37952d6d680a6d2c104fa452e51 Mon Sep 17 00:00:00 2001 From: jdar Date: Fri, 24 Oct 2025 10:46:24 -0700 Subject: [PATCH 08/26] spotless --- .../server/interceptor/auth/RuleBuilder.java | 10 ++-- ...entBulkExportByCompartmentMatcherImpl.java | 46 +++++++++---------- 2 files changed, 26 insertions(+), 30 deletions(-) diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java index 0f86b1758cd5..40095e9f4cf6 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java @@ -1070,17 +1070,17 @@ private class RuleBuilderPatientMatcherBulkExport implements IAuthRuleBuilderRul @Override public IAuthRuleBuilderRuleBulkExportWithTarget patientExportOnPatient( - @Nonnull String theCompartmentFilterMatcher) {//todo jdjd change to list + @Nonnull String theCompartmentFilterMatcher) { // todo jdjd change to list if (myRulePatientBulkExportByCompartmentMatcher == null) { RulePatientBulkExportByCompartmentMatcherImpl rule = - new RulePatientBulkExportByCompartmentMatcherImpl(myRuleName); + new RulePatientBulkExportByCompartmentMatcherImpl(myRuleName); rule.addAppliesToPatientExportOnPatient(theCompartmentFilterMatcher); rule.setMode(myRuleMode); myRulePatientBulkExportByCompartmentMatcher = rule; } else { myRulePatientBulkExportByCompartmentMatcher.addAppliesToPatientExportOnPatient( - theCompartmentFilterMatcher); + theCompartmentFilterMatcher); } // prevent duplicate rules being added @@ -1089,11 +1089,11 @@ public IAuthRuleBuilderRuleBulkExportWithTarget patientExportOnPatient( } return new RuleBuilderPatientMatcherBulkExport.RuleBuilderBulkExportWithTarget( - myRulePatientBulkExportByCompartmentMatcher); + myRulePatientBulkExportByCompartmentMatcher); } private class RuleBuilderBulkExportWithTarget extends RuleBuilderFinished - implements IAuthRuleBuilderRuleBulkExportWithTarget { + implements IAuthRuleBuilderRuleBulkExportWithTarget { private final RulePatientBulkExportByCompartmentMatcherImpl myRule; private RuleBuilderBulkExportWithTarget(RulePatientBulkExportByCompartmentMatcherImpl theRule) { diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java index 0b0cc239017e..9630008f8d04 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java @@ -25,16 +25,13 @@ import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.api.server.bulk.BulkExportJobParameters; import com.google.common.annotations.VisibleForTesting; - -import java.util.ArrayList; -import java.util.List; - -import java.util.Objects; - import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; +import java.util.ArrayList; import java.util.Collection; +import java.util.List; +import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; @@ -110,9 +107,9 @@ public AuthorizationInterceptor.Verdict applyRule( if (patientIdOptions.isEmpty()) { // This is a type-level export with a _typeFilter // All filters must be permitted to return an ALLOW verdict - return allFiltersMatch ? - new AuthorizationInterceptor.Verdict(PolicyEnum.ALLOW, this) : - new AuthorizationInterceptor.Verdict(PolicyEnum.DENY, this); + return allFiltersMatch + ? new AuthorizationInterceptor.Verdict(PolicyEnum.ALLOW, this) + : new AuthorizationInterceptor.Verdict(PolicyEnum.DENY, this); } else if (!allFiltersMatch) { // This is an instance-level export with a _typeFilter // Where at least one filter didn't match the permitted filters @@ -120,22 +117,22 @@ public AuthorizationInterceptor.Verdict applyRule( } } - List thePatientResources = theRuleApplier - .getAuthResourceResolver() - .resolveCompartmentByIds(patientIdOptions, "Patient"); + List thePatientResources = + theRuleApplier.getAuthResourceResolver().resolveCompartmentByIds(patientIdOptions, "Patient"); - // Apply the FhirQueryTester (which contains a inMemoryResourceMatcher) to the found Patient compartment resource, + // Apply the FhirQueryTester (which contains a inMemoryResourceMatcher) to the found Patient compartment + // resource, // and return the verdict // All requested Patient IDs must be permitted to return an ALLOW verdict. List verdicts = thePatientResources.stream() - .map(patient -> newVerdict( - theOperation, - theRequestDetails, - patient, - theInputResourceId, // todo jdjd should this be patient.id? - theOutputResource, - theRuleApplier)) - .toList(); + .map(patient -> newVerdict( + theOperation, + theRequestDetails, + patient, + theInputResourceId, // todo jdjd should this be patient.id? + theOutputResource, + theRuleApplier)) + .toList(); if (verdicts.stream().allMatch(t -> t != null && t.getDecision().equals(PolicyEnum.ALLOW))) { // All the resources match at least 1 permission query filter --> ALLOW @@ -221,18 +218,17 @@ public void setResourceTypes(Collection theResourceTypes) { * So this filter should start with a "?" */ public void addAppliesToPatientExportOnPatient(String thePatientMatcherFilter) { - //todo jdjd what does the string look like here? should i have a ? or resource type? - if (myPatientMatcherFilter == null){ + // todo jdjd what does the string look like here? should i have a ? or resource type? + if (myPatientMatcherFilter == null) { myPatientMatcherFilter = new ArrayList<>(); } myPatientMatcherFilter.add(sanitizeQueryFilter(thePatientMatcherFilter)); - if (myTokenizedPatientMatcherFilter == null){ + if (myTokenizedPatientMatcherFilter == null) { myTokenizedPatientMatcherFilter = new ArrayList<>(); } myTokenizedPatientMatcherFilter.add(Set.of(thePatientMatcherFilter.split("&"))); - addTester(new FhirQueryRuleTester(thePatientMatcherFilter)); } From 6c5d8601faaab0fac9001d770fc59f0be4196e01 Mon Sep 17 00:00:00 2001 From: jdar Date: Mon, 27 Oct 2025 23:50:17 -0700 Subject: [PATCH 09/26] type level ITs, fix some tests and bugs --- .../server/interceptor/auth/BaseRule.java | 21 +++++++++++++++++++ ...entBulkExportByCompartmentMatcherImpl.java | 16 +++++++------- ...ulkExportByCompartmentMatcherImplTest.java | 20 ++++++++++-------- 3 files changed, 40 insertions(+), 17 deletions(-) diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRule.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRule.java index 5ce7e1529342..dc3952fc841b 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRule.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRule.java @@ -93,6 +93,27 @@ private boolean applyTesters( return retVal; } + boolean applyTestersAtLeastOneMatch( + RestOperationTypeEnum theOperation, + RequestDetails theRequestDetails, + IBaseResource theInputResource, + IRuleApplier theRuleApplier) { + + boolean retVal = false; + + IAuthRuleTester.RuleTestRequest inputRequest = new IAuthRuleTester.RuleTestRequest( + myMode, theOperation, theRequestDetails, null, theInputResource, theRuleApplier); + + for (IAuthRuleTester next : getTesters()) { + if (next.matches(inputRequest)) { + retVal = true; + break; + } + } + + return retVal; + } + PolicyEnum getMode() { return myMode; } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java index 9630008f8d04..272962c753b4 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java @@ -124,24 +124,25 @@ public AuthorizationInterceptor.Verdict applyRule( // resource, // and return the verdict // All requested Patient IDs must be permitted to return an ALLOW verdict. - List verdicts = thePatientResources.stream() - .map(patient -> newVerdict( + List verdicts = thePatientResources.stream() + .map(patient -> applyTestersAtLeastOneMatch( theOperation, theRequestDetails, patient, - theInputResourceId, // todo jdjd should this be patient.id? - theOutputResource, theRuleApplier)) .toList(); - if (verdicts.stream().allMatch(t -> t != null && t.getDecision().equals(PolicyEnum.ALLOW))) { + if (verdicts.stream().allMatch(Boolean::booleanValue)) { + // Then the testers evaluated to true on all Patients // All the resources match at least 1 permission query filter --> ALLOW return new AuthorizationInterceptor.Verdict(PolicyEnum.ALLOW, this); - } else if (verdicts.stream().allMatch(Objects::isNull)) { + } else if (verdicts.stream().noneMatch(Boolean::booleanValue)) { + // Then the testers evaluated to false on all Patients // All the resources do not match any permission query filters // This rule must not apply --> abstain return null; } else { + // Then the testers evaluated to true on some Patients, and false on others. // We have a mixture of ALLOW and abstain // Default to DENY return new AuthorizationInterceptor.Verdict(PolicyEnum.DENY, this); @@ -183,8 +184,7 @@ private boolean matchOnFilterOptions(List theFilterOptions) { */ private static String sanitizeQueryFilter(String theFilter) { if (theFilter.contains("?")) { - - return theFilter.substring(theFilter.indexOf("?")); + return theFilter.substring(theFilter.indexOf("?") + 1); } return theFilter; } diff --git a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImplTest.java b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImplTest.java index af50f0185a82..0d897b622791 100644 --- a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImplTest.java +++ b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImplTest.java @@ -161,15 +161,17 @@ static Stream paramsTypeLevel() { // theAllowedResourceTypes, thePermissionFilters, theBulkExportJobParams, theExpectedVerdict, theMessage return Stream.of( // Type-Level export with _typeFilter - Arguments.of(List.of(), List.of("identifier=foo|bar"), new BulkExportParamsBuilder().withTypeFilters("identifier=foo|bar").build(), PolicyEnum.ALLOW, "Allow request when filters match"), - Arguments.of(List.of(), List.of("identifier=foo|bar", "name=Doe"), new BulkExportParamsBuilder().withTypeFilters("identifier=foo|bar").build(), PolicyEnum.ALLOW, "Allow request with subset of permitted filters"), - Arguments.of(List.of(), List.of("identifier=foo|bar", "name=Doe"), new BulkExportParamsBuilder().withTypeFilters("identifier=foo|bar", "name=Doe").build(), PolicyEnum.ALLOW, "Allow request when multiple filters match"), - Arguments.of(List.of("Patient"), List.of("identifier=foo|bar", "name=Doe"), new BulkExportParamsBuilder().withTypeFilters("identifier=foo|bar", "name=Doe").withResourceTypes("Patient").build(), PolicyEnum.ALLOW, "Allow request when multiple filters match including resource type"), - Arguments.of(List.of("Observation"), List.of("identifier=foo|bar", "name=Doe"), new BulkExportParamsBuilder().withTypeFilters("identifier=foo|bar", "name=Doe").withResourceTypes("Patient").build(), PolicyEnum.DENY, "Deny request when multiple filters match, but resource type doesn't"), - Arguments.of(List.of(), List.of("identifier=foo|bar&active=true", "name=Doe"), new BulkExportParamsBuilder().withTypeFilters("identifier=foo|bar&active=true", "name=Doe").build(), PolicyEnum.ALLOW, "Allow request when multiple filters match"), - Arguments.of(List.of(), List.of("identifier=foo|bar", "name=Doe"), new BulkExportParamsBuilder().withTypeFilters("identifier=foo|bar&active=true", "name=Doe").build(), PolicyEnum.DENY, "Deny request when filters don't match"), - Arguments.of(List.of(), List.of("identifier=abc|def"), new BulkExportParamsBuilder().withTypeFilters("identifier=foo|bar").build(), PolicyEnum.DENY, "Deny request when filters do not match"), - Arguments.of(List.of(), List.of("identifier=foo|bar"), new BulkExportParamsBuilder().withTypeFilters("identifier=foo|bar","name=Doe").build(), PolicyEnum.DENY, "Deny request when requesting more filters than permitted") + Arguments.of(List.of(), List.of("identifier=foo|bar"), new BulkExportParamsBuilder().withTypeFilters("Patient?identifier=foo|bar").build(), PolicyEnum.ALLOW, "Allow request when filters match"), + Arguments.of(List.of(), List.of("identifier=foo|bar", "name=Doe"), new BulkExportParamsBuilder().withTypeFilters("Patient?identifier=foo|bar").build(), PolicyEnum.ALLOW, "Allow request with subset of permitted filters"), + Arguments.of(List.of(), List.of("identifier=foo|bar", "name=Doe"), new BulkExportParamsBuilder().withTypeFilters("Patient?identifier=foo|bar", "Patient?name=Doe").build(), PolicyEnum.ALLOW, "Allow request when multiple filters match"), + Arguments.of(List.of("Patient"), List.of("identifier=foo|bar", "name=Doe"), new BulkExportParamsBuilder().withTypeFilters("Patient?identifier=foo|bar", "Patient?name=Doe").withResourceTypes("Patient").build(), PolicyEnum.ALLOW, "Allow request when multiple filters match including resource type"), + Arguments.of(List.of("Observation"), List.of("identifier=foo|bar", "name=Doe"), new BulkExportParamsBuilder().withTypeFilters("Patient?identifier=foo|bar", "Patient?name=Doe").withResourceTypes("Patient").build(), PolicyEnum.DENY, "Deny request when multiple filters match, but resource type doesn't"), + Arguments.of(List.of(), List.of("identifier=foo|bar&active=true", "name=Doe"), new BulkExportParamsBuilder().withTypeFilters("Patient?identifier=foo|bar&active=true", "Patient?name=Doe").build(), PolicyEnum.ALLOW, "Allow request when multiple filters match"), + Arguments.of(List.of(), List.of("active=true&identifier=foo|bar", "name=Doe"), new BulkExportParamsBuilder().withTypeFilters("Patient?identifier=foo|bar&active=true", "Patient?name=Doe").build(), PolicyEnum.ALLOW, "Allow request when multiple filters match"), + Arguments.of(List.of(), List.of("identifier=foo|bar&active=true", "name=Doe"), new BulkExportParamsBuilder().withTypeFilters("Patient?identifier=foo|bar", "Patient?name=Doe").build(), PolicyEnum.DENY, "Deny request when some tokenized filters match"), + Arguments.of(List.of(), List.of("identifier=foo|bar", "name=Doe"), new BulkExportParamsBuilder().withTypeFilters("Patient?identifier=foo|bar&active=true", "Patient?name=Doe").build(), PolicyEnum.DENY, "Deny request when filters don't match"), + Arguments.of(List.of(), List.of("identifier=abc|def"), new BulkExportParamsBuilder().withTypeFilters("Patient?identifier=foo|bar").build(), PolicyEnum.DENY, "Deny request when filters do not match"), + Arguments.of(List.of(), List.of("identifier=foo|bar"), new BulkExportParamsBuilder().withTypeFilters("Patient?identifier=foo|bar","Patient?name=Doe").build(), PolicyEnum.DENY, "Deny request when requesting more filters than permitted") ); } From 640c9ef519a95dc29dca7d7e8020b8a238b5a977 Mon Sep 17 00:00:00 2001 From: jdar Date: Tue, 28 Oct 2025 00:06:51 -0700 Subject: [PATCH 10/26] spotless --- .../fhir/rest/server/interceptor/auth/BaseRule.java | 10 +++++----- .../RulePatientBulkExportByCompartmentMatcherImpl.java | 7 +------ 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRule.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRule.java index dc3952fc841b..503d706433ff 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRule.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRule.java @@ -94,15 +94,15 @@ private boolean applyTesters( } boolean applyTestersAtLeastOneMatch( - RestOperationTypeEnum theOperation, - RequestDetails theRequestDetails, - IBaseResource theInputResource, - IRuleApplier theRuleApplier) { + RestOperationTypeEnum theOperation, + RequestDetails theRequestDetails, + IBaseResource theInputResource, + IRuleApplier theRuleApplier) { boolean retVal = false; IAuthRuleTester.RuleTestRequest inputRequest = new IAuthRuleTester.RuleTestRequest( - myMode, theOperation, theRequestDetails, null, theInputResource, theRuleApplier); + myMode, theOperation, theRequestDetails, null, theInputResource, theRuleApplier); for (IAuthRuleTester next : getTesters()) { if (next.matches(inputRequest)) { diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java index 272962c753b4..3f9ebcadd409 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java @@ -31,7 +31,6 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; -import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; @@ -125,11 +124,7 @@ public AuthorizationInterceptor.Verdict applyRule( // and return the verdict // All requested Patient IDs must be permitted to return an ALLOW verdict. List verdicts = thePatientResources.stream() - .map(patient -> applyTestersAtLeastOneMatch( - theOperation, - theRequestDetails, - patient, - theRuleApplier)) + .map(patient -> applyTestersAtLeastOneMatch(theOperation, theRequestDetails, patient, theRuleApplier)) .toList(); if (verdicts.stream().allMatch(Boolean::booleanValue)) { From 2f02a09cbd77970a820a310ba40b7b7f882214db Mon Sep 17 00:00:00 2001 From: jdar Date: Tue, 28 Oct 2025 17:17:54 -0700 Subject: [PATCH 11/26] add javadocs, remove unused code after some design changes, updated some tests --- .../jpa/interceptor/AuthResourceResolver.java | 16 ++++--- .../server/bulk/BulkExportJobParameters.java | 22 --------- .../auth/AuthorizationInterceptor.java | 17 +------ .../server/interceptor/auth/BaseRule.java | 4 ++ .../auth/FhirQueriesRuleTester.java | 1 + .../interceptor/auth/FhirQueryRuleTester.java | 1 + .../auth/IAuthResourceResolver.java | 14 +++++- .../auth/IAuthRuleBuilderRule.java | 4 +- ...RuleBuilderRuleGroupMatcherBulkExport.java | 4 +- ...leBuilderRulePatientMatcherBulkExport.java | 4 +- .../interceptor/auth/IAuthRuleFinished.java | 5 -- .../server/interceptor/auth/RuleBuilder.java | 8 ++-- ...oupBulkExportByCompartmentMatcherImpl.java | 36 +++++--------- ...entBulkExportByCompartmentMatcherImpl.java | 47 +++---------------- .../interceptor/auth/RuleBuilderTest.java | 19 ++++---- ...ulkExportByCompartmentMatcherImplTest.java | 6 +-- 16 files changed, 68 insertions(+), 140 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/AuthResourceResolver.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/AuthResourceResolver.java index ac9fc84df61e..6e67e24a89bd 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/AuthResourceResolver.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/AuthResourceResolver.java @@ -31,6 +31,10 @@ import java.util.List; +/** + * Small service class to inject DB access into an interceptor + * For example, used in bulk export security to allow querying for resource to match against permission argument filters + */ @Service public class AuthResourceResolver implements IAuthResourceResolver { private final DaoRegistry myDaoRegistry; @@ -39,19 +43,17 @@ public AuthResourceResolver(DaoRegistry myDaoRegistry) { this.myDaoRegistry = myDaoRegistry; } - public IBaseResource resolveCompartmentById(IIdType theCompartmentId) { + public IBaseResource resolveCompartmentById(IIdType theResourceId) { return myDaoRegistry - .getResourceDao(theCompartmentId.getResourceType()) - .read(theCompartmentId, new SystemRequestDetails()); + .getResourceDao(theResourceId.getResourceType()) + .read(theResourceId, new SystemRequestDetails()); } - public List resolveCompartmentByIds(List theCompartmentId, String theResourceType) { - TokenOrListParam t = new TokenOrListParam(null, theCompartmentId.toArray(String[]::new)); + public List resolveCompartmentByIds(List theResourceIds, String theResourceType) { + TokenOrListParam t = new TokenOrListParam(null, theResourceIds.toArray(String[]::new)); SearchParameterMap m = new SearchParameterMap(); m.add(Constants.PARAM_ID, t); return myDaoRegistry.getResourceDao(theResourceType).searchForResources(m, new SystemRequestDetails()); } - // translateMatchUrl - } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/bulk/BulkExportJobParameters.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/bulk/BulkExportJobParameters.java index a26c3640e845..d9bee535488f 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/bulk/BulkExportJobParameters.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/bulk/BulkExportJobParameters.java @@ -127,28 +127,6 @@ public class BulkExportJobParameters extends BaseBatchJobParameters { @JsonProperty("includeHistory") private boolean myIncludeHistory; - /** - * This flag is set to true when the system has added narrowing filters to the bulk export parameters - * based on the permissions that the user has. - * - * For example, if a bulk export is performed with `_typeFilter=Patient?active=true` - * and a user has permissions for `Patient?_lastUpdated=gt2025 - * than the system will modify the filters in the parameters to Patient?active=true&_lastUpdated=gt2025 - * and this flag will be set to true - * - * See TODO JDJD add issue for more information. - */ - @JsonProperty(value = "systemAddedNarrowingFilters", access = JsonProperty.Access.READ_ONLY) - private boolean mySystemAddedNarrowingFilters; - - public boolean hasSystemAddedNarrowingFilters() { - return mySystemAddedNarrowingFilters; - } - - public void setSystemAddedNarrowingFilters(boolean theSystemAddedNarrowingFilters) { - this.mySystemAddedNarrowingFilters = theSystemAddedNarrowingFilters; - } - public String getExportIdentifier() { return myExportId; } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java index 6e068eb9c8c6..c714e6bef52c 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java @@ -172,7 +172,7 @@ public Verdict applyRulesAndReturnDecision( Verdict verdict = null; for (IAuthRule nextRule : rules) { - ourLog.trace("Rule being applied - {}", nextRule); // todo jdjd 1007 does bulk export not go through here? + ourLog.trace("Rule being applied - {}", nextRule); verdict = nextRule.applyRule( theOperation, theRequestDetails, @@ -462,20 +462,7 @@ public void initiateBulkExport( if (theRequestDetails != null) { theRequestDetails.getUserData().put(REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS, theBulkExportOptions); } - // todo jdjd figure out what to do if we want to export patients, and patient is a list of ids in the bulk - // export options. so do you have to pass them in one by one? or how do we do this for other bulk operations? - // is this rule currently evaluated one by one for each patient included in the options? - // how do you do this? maybe a davinci export? - // https://smilecdr.com/docs/davinci_data_exchange/davinci_data_exchange.html#request-parameters - // or maybe with post w/ parameters resource and patient parameter in list of params? - // JpaConstants.PARAM_EXPORT_PATIENT and BulkDataExportProviderR4Test#testInitiatePatientExportRequest() - applyRulesAndFailIfDeny( - restOperationType, - theRequestDetails, - null, - new IdDt(theBulkExportOptions.getGroupId()), - null, - thePointcut); + applyRulesAndFailIfDeny(restOperationType, theRequestDetails, null, null, null, thePointcut); } /** diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRule.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRule.java index 503d706433ff..789533cbe407 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRule.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRule.java @@ -93,6 +93,10 @@ private boolean applyTesters( return retVal; } + /** + * Apply testers, and return true if at least 1 tester matches. + * Returns false if all testers do not match. + */ boolean applyTestersAtLeastOneMatch( RestOperationTypeEnum theOperation, RequestDetails theRequestDetails, diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/FhirQueriesRuleTester.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/FhirQueriesRuleTester.java index 3266a330d60f..f22055e4c3af 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/FhirQueriesRuleTester.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/FhirQueriesRuleTester.java @@ -26,6 +26,7 @@ import java.util.stream.Collectors; /** + * TODO JDJD 1028 likely remove this class * Tester that a resource matches any query filter in a list. */ public class FhirQueriesRuleTester implements IAuthRuleTester { diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/FhirQueryRuleTester.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/FhirQueryRuleTester.java index 27c91930d4aa..8a3d5239759f 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/FhirQueryRuleTester.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/FhirQueryRuleTester.java @@ -82,6 +82,7 @@ private boolean checkMatch(RuleTestRequest theRuleTestRequest) { } } +// TODO JDJD 1028 likely remove this method public String getQueryParameters() { return myQueryParameters; } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthResourceResolver.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthResourceResolver.java index 53df3ccc1580..3fb2a4e075ea 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthResourceResolver.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthResourceResolver.java @@ -24,8 +24,18 @@ import java.util.List; +/** + * Small service class to inject DB access into an interceptor + * For example, used in bulk export security to allow querying for resource to match against permission argument filters + */ public interface IAuthResourceResolver { - IBaseResource resolveCompartmentById(IIdType theCompartmentId); + IBaseResource resolveCompartmentById(IIdType theResourceId); - List resolveCompartmentByIds(List theCompartmentId, String theResourceType); + /** + * Resolve a list of resources by ID. All resources should be the same type. + * @param theResourceIds the FHIR id of the resource(s) + * @param theResourceType the type of resource + * @return A list of resources resolved by ID + */ + List resolveCompartmentByIds(List theResourceIds, String theResourceType); } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRule.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRule.java index 896d5336a244..3a194eb012dd 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRule.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRule.java @@ -131,7 +131,7 @@ public interface IAuthRuleBuilderRule { * This rule permits the user to initiate a FHIR bulk export * by providing a filter matcher on Group compartment(s). * - * @since 8.5.0 + * @since 8.6.0 */ IAuthRuleBuilderRuleGroupMatcherBulkExport bulkExportGroupCompartmentMatcher(); @@ -139,7 +139,7 @@ public interface IAuthRuleBuilderRule { * This rule permits the user to initiate a FHIR bulk export * by providing a filter matcher on Patient compartment(s). * - * @since 8.5.0 + * @since 8.6.0 */ IAuthRuleBuilderRulePatientMatcherBulkExport bulkExportPatientCompartmentMatcher(); diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRuleGroupMatcherBulkExport.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRuleGroupMatcherBulkExport.java index 74ce3bbaafb4..37d73e4dde0b 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRuleGroupMatcherBulkExport.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRuleGroupMatcherBulkExport.java @@ -22,13 +22,13 @@ import jakarta.annotation.Nonnull; /** - * @since 8.5.0 + * @since 8.6.0 */ public interface IAuthRuleBuilderRuleGroupMatcherBulkExport { /** * Allow/deny group-level export rule applies to the Group with the given resource ID, e.g. Group/123 * - * @since 8.5.0 + * @since 8.6.0 */ IAuthRuleBuilderRuleBulkExportWithTarget groupExportOnGroup(@Nonnull String theCompartmentFilterMatcher); } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRulePatientMatcherBulkExport.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRulePatientMatcherBulkExport.java index ef48c2ef4962..17de6fef8d57 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRulePatientMatcherBulkExport.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRulePatientMatcherBulkExport.java @@ -22,13 +22,13 @@ import jakarta.annotation.Nonnull; /** - * @since 8.5.0 + * @since 8.6.0 */ public interface IAuthRuleBuilderRulePatientMatcherBulkExport { /** * Allow/deny group-level export rule applies to the Group with the given resource ID, e.g. Group/123 * - * @since 8.5.0 + * @since 8.6.0 */ IAuthRuleBuilderRuleBulkExportWithTarget patientExportOnPatient(@Nonnull String theCompartmentFilterMatcher); } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleFinished.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleFinished.java index c513d26941f3..353515ca8314 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleFinished.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleFinished.java @@ -56,9 +56,4 @@ public interface IAuthRuleFinished { * @param theQueryParameters a FHIR query parameter string. E.g. {@code category=laboratory&date=ge2021} */ IAuthRuleFinished withFilterTester(String theQueryParameters); - - // todo jdjd docs - default IAuthRuleFinished withFilterOnExistingTester(String theQueryParameters) { - return this; - } } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java index 40095e9f4cf6..8afe0ab16e90 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java @@ -919,7 +919,6 @@ public IAuthRuleBuilderRuleBulkExportWithTarget groupExportOnGroup(@Nonnull Stri @Override public IAuthRuleBuilderRuleBulkExportWithTarget patientExportOnAllPatients() { - // todo JDJD 1008 it's this ==null that is problematic, it prevents duplicate bulkExportRuleImpl if (myRuleBulkExport == null) { RuleBulkExportImpl rule = new RuleBulkExportImpl(myRuleName); rule.setMode(myRuleMode); @@ -957,7 +956,6 @@ public IAuthRuleBuilderRuleBulkExportWithTarget patientExportOnPatient(@Nonnull @Override public IAuthRuleBuilderRuleBulkExportWithTarget patientExportOnPatientStrings( @Nonnull Collection theFocusResourceIds) { - // todo JDJD 1008 it's this ==null that is problematic, it prevents duplicate bulkExportRuleImpl if (myRuleBulkExport == null) { RuleBulkExportImpl rule = new RuleBulkExportImpl(myRuleName); rule.setAppliesToPatientExport(theFocusResourceIds); @@ -1039,7 +1037,7 @@ public IAuthRuleBuilderRuleBulkExportWithTarget groupExportOnGroup( theCompartmentFilterMatcher); } - // prevent duplicate rules being added + // prevent duplicate rules from being added if (!myRules.contains(myRuleGroupBulkExportByCompartmentMatcher)) { myRules.add(myRuleGroupBulkExportByCompartmentMatcher); } @@ -1070,7 +1068,7 @@ private class RuleBuilderPatientMatcherBulkExport implements IAuthRuleBuilderRul @Override public IAuthRuleBuilderRuleBulkExportWithTarget patientExportOnPatient( - @Nonnull String theCompartmentFilterMatcher) { // todo jdjd change to list + @Nonnull String theCompartmentFilterMatcher) { if (myRulePatientBulkExportByCompartmentMatcher == null) { RulePatientBulkExportByCompartmentMatcherImpl rule = new RulePatientBulkExportByCompartmentMatcherImpl(myRuleName); @@ -1083,7 +1081,7 @@ public IAuthRuleBuilderRuleBulkExportWithTarget patientExportOnPatient( theCompartmentFilterMatcher); } - // prevent duplicate rules being added + // prevent duplicate rules from being added if (!myRules.contains(myRulePatientBulkExportByCompartmentMatcher)) { myRules.add(myRulePatientBulkExportByCompartmentMatcher); } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImpl.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImpl.java index 89cec52973a0..7967f2ddf03d 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImpl.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImpl.java @@ -36,8 +36,6 @@ import static org.apache.commons.collections4.CollectionUtils.isNotEmpty; public class RuleGroupBulkExportByCompartmentMatcherImpl extends BaseRule { - private static final org.slf4j.Logger ourLog = - org.slf4j.LoggerFactory.getLogger(RuleGroupBulkExportByCompartmentMatcherImpl.class); private static final BulkExportJobParameters.ExportStyle OUR_EXPORT_STYLE = BulkExportJobParameters.ExportStyle.GROUP; private String myGroupMatcherFilter; @@ -100,39 +98,29 @@ public AuthorizationInterceptor.Verdict applyRule( theRuleApplier); } - private static IdDt getIdFromRequest(RequestDetails theRequestDetails) { - // TODO JDJD modify based on group/patient export - // also how do you handle patient list?? - // might also want to use export style (from bulk export params) intead of the resource type (from request - // details) - return new IdDt(((BulkExportJobParameters) - theRequestDetails.getUserData().get(REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS)) - .getGroupId()); - } - - private Set sanitizeIds(Collection myPatientIds) { - return myPatientIds.stream() - .map(id -> new IdDt(id).toUnqualifiedVersionless().getValue()) - .collect(Collectors.toSet()); - } - public void setResourceTypes(Collection theResourceTypes) { myResourceTypes = theResourceTypes; } - // TODO jdjd do we need to clear the testers since we are replacing the filter string? - // but testers is not modifiable i think? public void setAppliesToGroupExportOnGroup(String theGroupMatcherFilter) { - myGroupMatcherFilter = theGroupMatcherFilter; - addTester(new FhirQueryRuleTester(theGroupMatcherFilter)); + String sanitizedFilter = sanitizeQueryFilter(theGroupMatcherFilter); + myGroupMatcherFilter = sanitizedFilter; + addTester(new FhirQueryRuleTester(sanitizedFilter)); } String getGroupMatcherFilter() { return myGroupMatcherFilter; } - BulkExportJobParameters.ExportStyle getWantExportStyle() { - return OUR_EXPORT_STYLE; + /** + * Remove the resource type and "?" prefix, if present + * since resource type is implied for the rule based on the permission (Patient in this case) + */ + private static String sanitizeQueryFilter(String theFilter) { + if (theFilter.contains("?")) { + return theFilter.substring(theFilter.indexOf("?") + 1); + } + return theFilter; } @VisibleForTesting diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java index 3f9ebcadd409..5e3a1e49b2a9 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java @@ -39,8 +39,6 @@ import static org.apache.commons.collections4.CollectionUtils.isNotEmpty; public class RulePatientBulkExportByCompartmentMatcherImpl extends BaseRule { - private static final org.slf4j.Logger ourLog = - org.slf4j.LoggerFactory.getLogger(RulePatientBulkExportByCompartmentMatcherImpl.class); private static final BulkExportJobParameters.ExportStyle OUR_EXPORT_STYLE = BulkExportJobParameters.ExportStyle.PATIENT; private List myPatientMatcherFilter; @@ -89,13 +87,6 @@ public AuthorizationInterceptor.Verdict applyRule( } } - // TODO JDJD you need to handle - // export with list of ids - // export with list of ids (matching on filter) - // export with list of ids (non-matching on filter) - // export at type with no list of ids (matcher by type filter) - // permission contains OR - List patientIdOptions = inboundBulkExportRequestOptions.getPatientIds(); List filterOptions = inboundBulkExportRequestOptions.getFilters(); @@ -174,8 +165,7 @@ private boolean matchOnFilterOptions(List theFilterOptions) { /** * Remove the resource type and "?" prefix, if present - * @param theFilter - * @return + * since resource type is implied for the rule based on the permission (Patient in this case) */ private static String sanitizeQueryFilter(String theFilter) { if (theFilter.contains("?")) { @@ -184,57 +174,34 @@ private static String sanitizeQueryFilter(String theFilter) { return theFilter; } - private static IdDt getIdFromRequest(RequestDetails theRequestDetails) { - // TODO JDJD modify based on group/patient export - // also how do you handle patient list?? - // might also want to use export style (from bulk export params) intead of the resource type (from request - // details) - return new IdDt(((BulkExportJobParameters) - theRequestDetails.getUserData().get(REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS)) - .getGroupId()); - } - - private Set sanitizeIds(Collection myPatientIds) { - return myPatientIds.stream() - .map(id -> new IdDt(id).toUnqualifiedVersionless().getValue()) - .collect(Collectors.toSet()); - } - public void setResourceTypes(Collection theResourceTypes) { myResourceTypes = theResourceTypes; } - // TODO jdjd do we need to clear the testers since we are replacing the filter string? - // but testers is not modifiable i think? - /** * @param thePatientMatcherFilter the matcher filter for the permitted Patient - * Note that resource type is implied as this queries for the compartment - * So this filter should start with a "?" */ public void addAppliesToPatientExportOnPatient(String thePatientMatcherFilter) { - // todo jdjd what does the string look like here? should i have a ? or resource type? + if (myPatientMatcherFilter == null) { myPatientMatcherFilter = new ArrayList<>(); } - myPatientMatcherFilter.add(sanitizeQueryFilter(thePatientMatcherFilter)); + + String sanitizedFilter = sanitizeQueryFilter(thePatientMatcherFilter); + myPatientMatcherFilter.add(sanitizedFilter); + addTester(new FhirQueryRuleTester(sanitizedFilter)); if (myTokenizedPatientMatcherFilter == null) { myTokenizedPatientMatcherFilter = new ArrayList<>(); } - myTokenizedPatientMatcherFilter.add(Set.of(thePatientMatcherFilter.split("&"))); - addTester(new FhirQueryRuleTester(thePatientMatcherFilter)); + myTokenizedPatientMatcherFilter.add(Set.of(thePatientMatcherFilter.split("&"))); } List getPatientMatcherFilter() { return myPatientMatcherFilter; } - BulkExportJobParameters.ExportStyle getWantExportStyle() { - return OUR_EXPORT_STYLE; - } - @VisibleForTesting Collection getResourceTypes() { return myResourceTypes; diff --git a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilderTest.java b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilderTest.java index 3e59ab6916da..473110d5fd76 100644 --- a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilderTest.java +++ b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilderTest.java @@ -92,7 +92,7 @@ public void testBulkExportByGroupMatcher(String theCompartmentMatcherFilter, Col List resourceTypes = new ArrayList<>(theResourceTypes); // When - builder.allow().bulkExportGroupCompartmentMatcher().groupExportOnGroup(theCompartmentMatcherFilter).withResourceTypes(resourceTypes); + builder.allow().bulkExportGroupCompartmentMatcher().groupExportOnGroup("?" + theCompartmentMatcherFilter).withResourceTypes(resourceTypes); final List rules = builder.build(); // Then @@ -108,8 +108,8 @@ public void testBulkExportByGroupMatcher(String theCompartmentMatcherFilter, Col private static Stream groupMatcherBulkExportParams() { return Stream.of( - Arguments.of("?identifier=foo|bar", List.of()), - Arguments.of("?identifier=foo|bar", List.of("Patient", "Observation")) + Arguments.of("identifier=foo|bar", List.of()), + Arguments.of("identifier=foo|bar", List.of("Patient", "Observation")) ); } @@ -122,9 +122,8 @@ public void testBulkExportByPatientMatcher(List theCompartmentMatcherFil // When for (String filter : theCompartmentMatcherFilter) { - builder.allow().bulkExportPatientCompartmentMatcher().patientExportOnPatient(filter).withResourceTypes(resourceTypes); + builder.allow().bulkExportPatientCompartmentMatcher().patientExportOnPatient("?" + filter).withResourceTypes(resourceTypes); } - final List rules = builder.build(); // Then @@ -140,10 +139,12 @@ public void testBulkExportByPatientMatcher(List theCompartmentMatcherFil private static Stream patientMatcherBulkExportParams() { return Stream.of( - Arguments.of(List.of("?identifier=foo|bar"), List.of()), - Arguments.of(List.of("?identifier=foo|bar"), List.of("Patient", "Observation")), - Arguments.of(List.of("?identifier=foo|bar", "?name=Doe"), List.of()), - Arguments.of(List.of("?identifier=foo|bar", "?name=Doe&active=true"), List.of("Patient", "Observation")) + Arguments.of(List.of("identifier=foo|bar"), List.of()), + Arguments.of(List.of("identifier=foo|bar"), List.of("Patient", "Observation")), + // Multiple arguments may be added to the filter when multiple FHIR_OP_INITIATE_BULK_DATA_EXPORT_PATIENTS_MATCHING permissions + // are added to the same user, even when the permission does not accept multiple (a list of) arguments by itself. + Arguments.of(List.of("identifier=foo|bar", "name=Doe"), List.of()), + Arguments.of(List.of("identifier=foo|bar", "name=Doe&active=true"), List.of("Patient", "Observation")) ); } diff --git a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImplTest.java b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImplTest.java index 60f85f711228..d06348cb6304 100644 --- a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImplTest.java +++ b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImplTest.java @@ -20,7 +20,6 @@ import static org.junit.jupiter.api.Assertions.*; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.params.ParameterizedTest; @@ -54,9 +53,6 @@ class RuleGroupBulkExportByCompartmentMatcherImplTest { @Mock private Set myFlags; - - - @ParameterizedTest @MethodSource("params") void testGroupRule_withCompartmentMatchers(IAuthorizationSearchParamMatcher.MatchResult theSearchParamMatcherMatchResult, Collection theAllowedResourceTypes, ExportStyle theExportStyle, Collection theRequestedResourceTypes, PolicyEnum theExpectedVerdict, String theMessage) { @@ -104,7 +100,7 @@ static Stream params() { Arguments.of(match, List.of("Patient", "Observation"), ExportStyle.GROUP, List.of("Patient"), PolicyEnum.ALLOW, "Allow request for subset of allowable types"), Arguments.of(noMatch, List.of("Patient", "Observation"), ExportStyle.GROUP, List.of("Patient", "Observation"), null, "Abstain when requesting some resource types, but no resources match the permission query"), Arguments.of(noMatch, List.of(), ExportStyle.GROUP, List.of(), null, "Abstain when requesting all resource types, but no resources match the permission query"), - // TODO jdjd, below is the narrowing case. Narrowing should happen at the SecurityInterceptor layer + // The case below is the narrowing case. Narrowing should happen at the SecurityInterceptor layer Arguments.of(null, List.of("Patient", "Observation"), ExportStyle.GROUP, List.of(), PolicyEnum.DENY, "Deny request for all types when allowing some types"), Arguments.of(null, List.of("Patient", "Observation"), ExportStyle.GROUP, List.of("Patient", "Observation", "Encounter"), PolicyEnum.DENY, "Deny request for superset of allowable types"), Arguments.of(null, List.of("Patient", "Observation"), ExportStyle.PATIENT, List.of(), null, "Abstain when export style is not Group") From 10fd8858061ab549a4b18e4dcc465acf6a31948a Mon Sep 17 00:00:00 2001 From: jdar Date: Tue, 28 Oct 2025 17:19:07 -0700 Subject: [PATCH 12/26] spotless --- .../rest/server/interceptor/auth/AuthorizationInterceptor.java | 1 - .../fhir/rest/server/interceptor/auth/FhirQueryRuleTester.java | 2 +- .../auth/RuleGroupBulkExportByCompartmentMatcherImpl.java | 1 - .../auth/RulePatientBulkExportByCompartmentMatcherImpl.java | 2 -- 4 files changed, 1 insertion(+), 5 deletions(-) diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java index c714e6bef52c..cfb5ac1ae6e0 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java @@ -25,7 +25,6 @@ import ca.uhn.fhir.interceptor.api.Hook; import ca.uhn.fhir.interceptor.api.Interceptor; import ca.uhn.fhir.interceptor.api.Pointcut; -import ca.uhn.fhir.model.primitive.IdDt; import ca.uhn.fhir.model.valueset.BundleTypeEnum; import ca.uhn.fhir.rest.api.RestOperationTypeEnum; import ca.uhn.fhir.rest.api.server.IPreResourceShowDetails; diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/FhirQueryRuleTester.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/FhirQueryRuleTester.java index 8a3d5239759f..7196cb76c2f9 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/FhirQueryRuleTester.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/FhirQueryRuleTester.java @@ -82,7 +82,7 @@ private boolean checkMatch(RuleTestRequest theRuleTestRequest) { } } -// TODO JDJD 1028 likely remove this method + // TODO JDJD 1028 likely remove this method public String getQueryParameters() { return myQueryParameters; } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImpl.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImpl.java index 7967f2ddf03d..d4a60b19b4a3 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImpl.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImpl.java @@ -28,7 +28,6 @@ import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; -import java.util.stream.Collectors; import java.util.*; import static ca.uhn.fhir.rest.server.interceptor.auth.AuthorizationInterceptor.REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS; diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java index 5e3a1e49b2a9..1cf01700e3de 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java @@ -20,7 +20,6 @@ package ca.uhn.fhir.rest.server.interceptor.auth; import ca.uhn.fhir.interceptor.api.Pointcut; -import ca.uhn.fhir.model.primitive.IdDt; import ca.uhn.fhir.rest.api.RestOperationTypeEnum; import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.api.server.bulk.BulkExportJobParameters; @@ -32,7 +31,6 @@ import java.util.Collection; import java.util.List; import java.util.Set; -import java.util.stream.Collectors; import static ca.uhn.fhir.rest.server.interceptor.auth.AuthorizationInterceptor.REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS; import static org.apache.commons.collections4.CollectionUtils.isEmpty; From af123a9ad7a60f835862f3c1c08750c56b251850 Mon Sep 17 00:00:00 2001 From: jdar Date: Wed, 29 Oct 2025 09:48:42 -0700 Subject: [PATCH 13/26] round 2 - add javadocs, remove unused code after some design changes, updated some tests --- .../auth/RuleGroupBulkExportByCompartmentMatcherImpl.java | 2 +- .../RulePatientBulkExportByCompartmentMatcherImpl.java | 7 ++++++- .../fhir/rest/server/interceptor/auth/RuleBuilderTest.java | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImpl.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImpl.java index d4a60b19b4a3..1db56fd8e647 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImpl.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImpl.java @@ -107,7 +107,7 @@ public void setAppliesToGroupExportOnGroup(String theGroupMatcherFilter) { addTester(new FhirQueryRuleTester(sanitizedFilter)); } - String getGroupMatcherFilter() { + public String getGroupMatcherFilter() { return myGroupMatcherFilter; } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java index 1cf01700e3de..54713d8831f2 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java @@ -196,7 +196,7 @@ public void addAppliesToPatientExportOnPatient(String thePatientMatcherFilter) { myTokenizedPatientMatcherFilter.add(Set.of(thePatientMatcherFilter.split("&"))); } - List getPatientMatcherFilter() { + public List getPatientMatcherFilters() { return myPatientMatcherFilter; } @@ -204,4 +204,9 @@ List getPatientMatcherFilter() { Collection getResourceTypes() { return myResourceTypes; } + + @VisibleForTesting + public List> getTokenizedPatientMatcherFilter() { + return myTokenizedPatientMatcherFilter; + } } diff --git a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilderTest.java b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilderTest.java index 473110d5fd76..b7978d4e5e6a 100644 --- a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilderTest.java +++ b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilderTest.java @@ -132,7 +132,7 @@ public void testBulkExportByPatientMatcher(List theCompartmentMatcherFil assertInstanceOf(RulePatientBulkExportByCompartmentMatcherImpl.class, authRule); final RulePatientBulkExportByCompartmentMatcherImpl rulePatientExport = (RulePatientBulkExportByCompartmentMatcherImpl) authRule; - assertThat(rulePatientExport.getPatientMatcherFilter()).containsExactlyInAnyOrderElementsOf(theCompartmentMatcherFilter); + assertThat(rulePatientExport.getPatientMatcherFilters()).containsExactlyInAnyOrderElementsOf(theCompartmentMatcherFilter); assertEquals(theResourceTypes, rulePatientExport.getResourceTypes()); assertEquals(PolicyEnum.ALLOW, rulePatientExport.getMode()); } From bd3ca500104b574990020549ba3920835871723c Mon Sep 17 00:00:00 2001 From: jdar Date: Wed, 29 Oct 2025 11:12:55 -0700 Subject: [PATCH 14/26] round 4 - add javadocs, remove unused code after some design changes, updated some tests --- .../auth/FhirQueriesRuleTester.java | 82 ----------- .../interceptor/auth/FhirQueryRuleTester.java | 5 - .../interceptor/auth/RuleBulkExportImpl.java | 128 +----------------- .../auth/RuleBulkExportImplTest.java | 19 +-- 4 files changed, 4 insertions(+), 230 deletions(-) delete mode 100644 hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/FhirQueriesRuleTester.java diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/FhirQueriesRuleTester.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/FhirQueriesRuleTester.java deleted file mode 100644 index f22055e4c3af..000000000000 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/FhirQueriesRuleTester.java +++ /dev/null @@ -1,82 +0,0 @@ -/*- - * #%L - * HAPI FHIR - Server Framework - * %% - * Copyright (C) 2014 - 2025 Smile CDR, Inc. - * %% - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * #L% - */ -package ca.uhn.fhir.rest.server.interceptor.auth; - -import org.apache.commons.lang3.builder.ToStringBuilder; -import org.apache.commons.lang3.builder.ToStringStyle; - -import java.util.List; -import java.util.stream.Collectors; - -/** - * TODO JDJD 1028 likely remove this class - * Tester that a resource matches any query filter in a list. - */ -public class FhirQueriesRuleTester implements IAuthRuleTester { - private final List myFilters; - private final String myResourceType; - - public FhirQueriesRuleTester(List theQueries, String theResourceType) { - myFilters = theQueries.stream() - .map(query -> query.contains("?") ? query.substring(query.indexOf("?") + 1) : query) - .map(FhirQueryRuleTester::new) - .collect(Collectors.toList()); - myResourceType = theResourceType; - } - - public void addFilter(FhirQueriesRuleTester theOtherRuleTester) { - // todo jdjd can i add to list? - if (theOtherRuleTester.getResourceType().equals(myResourceType)) { - myFilters.addAll(theOtherRuleTester.getFilters()); - } - } - - public List getFilters() { - return List.copyOf(myFilters); - } - - public List getFiltersAsString() { - return myFilters.stream() - .map(param -> myResourceType + '?' + param.getQueryParameters()) - .toList(); - } - - public String getResourceType() { - return myResourceType; - } - - @Override - public boolean matches(RuleTestRequest theRuleTestRequest) { - return myFilters.stream().anyMatch(t -> t.matches(theRuleTestRequest)); - } - - @Override - public boolean matchesOutput(RuleTestRequest theRuleTestRequest) { - return myFilters.stream().anyMatch(t -> t.matches(theRuleTestRequest)); - } - - // todo jdjds what is to string used for and what should the implementation look like for a list of queries - @Override - public String toString() { - return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE) - .append("filter", myFilters) - .toString(); - } -} diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/FhirQueryRuleTester.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/FhirQueryRuleTester.java index 7196cb76c2f9..e9437d27719a 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/FhirQueryRuleTester.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/FhirQueryRuleTester.java @@ -82,11 +82,6 @@ private boolean checkMatch(RuleTestRequest theRuleTestRequest) { } } - // TODO JDJD 1028 likely remove this method - public String getQueryParameters() { - return myQueryParameters; - } - @Override public String toString() { return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE) diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBulkExportImpl.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBulkExportImpl.java index 0509ab7d8021..2c9f7742e360 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBulkExportImpl.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBulkExportImpl.java @@ -30,9 +30,7 @@ import java.util.ArrayList; import java.util.Collection; -import java.util.List; import java.util.Objects; -import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; @@ -55,31 +53,6 @@ public class RuleBulkExportImpl extends BaseRule { myPatientIds = new ArrayList<>(); } - @Override - public void addTester(IAuthRuleTester theNewTester) { - if (!getTesters().isEmpty()) { - // ensure only 1 tester with list of all filters in the rule - // this ensures the queries/filters are applied as an "OR" instead of "AND" - Optional queriesTester = getTesters().stream() - .filter(FhirQueriesRuleTester.class::isInstance) - .findFirst(); - if (queriesTester.isPresent() && theNewTester instanceof FhirQueriesRuleTester) { - FhirQueriesRuleTester existingTester = (FhirQueriesRuleTester) queriesTester.get(); - - if (!existingTester - .getResourceType() - .equals(((FhirQueriesRuleTester) theNewTester).getResourceType())) { - // TODO JDJD refine error message - throw new IllegalArgumentException( - "All queries for compartments should apply to the same resource type"); - } - existingTester.addFilter((FhirQueriesRuleTester) theNewTester); - } - } else { - super.addTester(theNewTester); - } - } - @Override public AuthorizationInterceptor.Verdict applyRule( RestOperationTypeEnum theOperation, @@ -90,8 +63,6 @@ public AuthorizationInterceptor.Verdict applyRule( IRuleApplier theRuleApplier, Set theFlags, Pointcut thePointcut) { - // todo jdjd here you can access mypatientIds and myGroupId in the class field. what are these? - // ans - they are the IDs specified in the rule not the bulk export input if (thePointcut != Pointcut.STORAGE_INITIATE_BULK_EXPORT) { return null; } @@ -117,22 +88,6 @@ public AuthorizationInterceptor.Verdict applyRule( } } - boolean ruleUsesQueryToDetermineCompartment = false; - // todo jdjd what is the output resource? this time its null - // also is there a cleaner way other than 2 instanceof's - if (theInputResource == null && getTesters().stream().anyMatch(t -> t instanceof FhirQueryRuleTester)) { - // The rule uses a filter query for eligible compartments - - // TODO jdjd you can actually resolve the ID from getting it from request details user data - // BulkDataExportOptions - IIdType theResourceToQuery = - theInputResourceId != null ? theInputResourceId : getIdFromRequest(theRequestDetails); - if (theInputResourceId != null) { - theInputResource = theRuleApplier.getAuthResourceResolver().resolveCompartmentById(theResourceToQuery); - ruleUsesQueryToDetermineCompartment = true; - } - } - // system only supports filtering by resource type. So if we are system, or any(), then allow, since we have // done resource type checking // above @@ -144,10 +99,7 @@ public AuthorizationInterceptor.Verdict applyRule( theOutputResource, theRuleApplier); - if (myWantAnyStyle - || myWantExportStyle == BulkExportJobParameters.ExportStyle.SYSTEM - || (ruleUsesQueryToDetermineCompartment - && myWantExportStyle.equals(BulkExportJobParameters.ExportStyle.GROUP))) { + if (myWantAnyStyle || myWantExportStyle == BulkExportJobParameters.ExportStyle.SYSTEM) { return allowVerdict; } @@ -168,7 +120,8 @@ public AuthorizationInterceptor.Verdict applyRule( // 1. If each of the requested resource IDs in the parameters are present in the users permissions, Approve // 2. If any requested ID is not present in the users permissions, Deny. - if (myWantExportStyle == BulkExportJobParameters.ExportStyle.PATIENT) { // Unfiltered Type Level + if (myWantExportStyle == BulkExportJobParameters.ExportStyle.PATIENT) { + // Unfiltered Type Level if (myAppliesToAllPatients) { return allowVerdict; } @@ -189,85 +142,10 @@ public AuthorizationInterceptor.Verdict applyRule( return new AuthorizationInterceptor.Verdict(PolicyEnum.DENY, this); } } - } else if (getTesters().stream() - .anyMatch(FhirQueriesRuleTester.class::isInstance)) { // todo jdjd refine this if condition - // We don't have any specified Patient IDs to resolve the compartments we are allowed to export - - if (inboundBulkExportRequestOptions.getPatientIds().isEmpty()) { - // There are no patient IDs requested in the bulk export. - // This is either a type level export, or export with _typeFilter - - FhirQueriesRuleTester tester = (FhirQueriesRuleTester) getTesters().stream() - .filter(FhirQueriesRuleTester.class::isInstance) - .findFirst() - .get(); - if (tester.getFiltersAsString().containsAll(inboundBulkExportRequestOptions.getFilters())) { - // If the queries in the tester exactly match the queries in the in the bulk export, then allow - return new AuthorizationInterceptor.Verdict(PolicyEnum.ALLOW, this); - } - } else { - // But we do have a FHIR query tester to resolve the compartment(s) - // Query the DB for the requested compartment(s) in the bulk export, - // and apply a matcher between the requested compartment, and the FHIR query tested included in the - // permission - - List requestedPatientsToExport = theRuleApplier - .getAuthResourceResolver() - .resolveCompartmentByIds(inboundBulkExportRequestOptions.getPatientIds(), "Patient"); - - // directly call contents of new verdict instead? and construct ur own verdict here. - boolean allAllowed = requestedPatientsToExport.stream() - .map(patient -> newVerdict( - theOperation, - theRequestDetails, - patient, - theInputResourceId, // todo jdjd should this be patient.id - theOutputResource, - theRuleApplier)) - .allMatch(t -> t != null && t.getDecision().equals(PolicyEnum.ALLOW)); - // todo jdjd 1008 any or all match?? 1015 -- all right? because all resources have to be allowed to - // export - // old reaonsing below - // well let's say you allow 2 different ruleBulkExportImpl - then you need to iterate through both and - // both must be allow (this needs to be done higher up). Or you can return null here so that it will - // iterate through them all. But assuming there's one bulkExportRule seems to be an assumption baked in - // to the existing implementation - // and let's say you only have 1 ruleBulkExportImpl - then you need to iterate through all your - // testers/filters and make sure at least one matches but currently, if one tester fails, the whole - // thing fails - // solution, probably when constructing the rule, if a test already exists, add to its filters for bulk - // export? - // i think it never makes sense to have an AND because - // - you have diff resource types Patient?name=Doe Patient and Patient?name=Doe Encounter --> then just - // combine them - // - you have diff queries (want Patient?name=Doe and patient?identifier=abc|def), then just use the & - // in the query - // - diff resource types (Patient?name=Doe, and Organization?identifier=abc|123), then it doesnt make - // sense because permission is only patients and it doesnt make snense for resource to match both - // conditions - - if (allAllowed) { - return new AuthorizationInterceptor.Verdict(PolicyEnum.ALLOW, this); - } else { - // TODO JDJD should it be DENY or abstain? - // you have all the patients, not all of them match the matcher. Deny? - return new AuthorizationInterceptor.Verdict(PolicyEnum.DENY, this); - } - } } return null; } - private static IdDt getIdFromRequest(RequestDetails theRequestDetails) { - // TODO JDJD modify based on group/patient export - // also how do you handle patient list?? - // might also want to use export style (from bulk export params) intead of the resource type (from request - // details) - return new IdDt(((BulkExportJobParameters) - theRequestDetails.getUserData().get(REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS)) - .getGroupId()); - } - private Set sanitizeIds(Collection myPatientIds) { return myPatientIds.stream() .map(id -> new IdDt(id).toUnqualifiedVersionless().getValue()) diff --git a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBulkExportImplTest.java b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBulkExportImplTest.java index 3857aa144134..a0fed1081628 100644 --- a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBulkExportImplTest.java +++ b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBulkExportImplTest.java @@ -5,10 +5,6 @@ import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.api.server.bulk.BulkExportJobParameters; -import java.util.List; - -import static org.assertj.core.api.Assertions.assertThat; - import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; @@ -455,7 +451,6 @@ public void testPatientExport_ruleAllowsId_requestsId_allow() { assertEquals(PolicyEnum.ALLOW, verdict.getDecision()); } - //todo jdjd test here?? @Test public void testPatientExport_ruleAllowsIds_requestsIds_allow() { //Given @@ -555,6 +550,7 @@ public void testPatientExportRulesOnTypeLevelExportWithPermittedAndUnpermittedPa myRule.setMode(PolicyEnum.ALLOW); final BulkExportJobParameters options = new BulkExportJobParameters(); options.setExportStyle(BulkExportJobParameters.ExportStyle.PATIENT); + options.setFilters(Set.of("Patient?_id=123","Patient?_id=456")); options.setResourceTypes(Set.of("Patient")); when(myRequestDetails.getUserData()).thenReturn(Map.of(AuthorizationInterceptor.REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS, options)); @@ -567,19 +563,6 @@ public void testPatientExportRulesOnTypeLevelExportWithPermittedAndUnpermittedPa } - @Test - public void testAddTestersToBulkExportRule_addsQueryFiltersToExistingTester() { - //Given - FhirQueriesRuleTester queriesTester1 = new FhirQueriesRuleTester(List.of("Patient?name=Doe"), "Patient"); - FhirQueriesRuleTester queriesTester2 = new FhirQueriesRuleTester(List.of("Patient?identifier=system|value"), "Patient"); - final RuleBulkExportImpl myRule = new RuleBulkExportImpl("b"); - myRule.setAppliesToPatientExport(List.of()); - myRule.setMode(PolicyEnum.ALLOW); - myRule.addTesters(List.of(queriesTester1, queriesTester2)); - - assertThat(myRule.getTesters()).hasSize(1); - } - private static void assertAbstain(AuthorizationInterceptor.Verdict verdict) { assertNull(verdict, "Expect abstain"); } From 5873e184936a5a5e2b23cf026512aa7b679ca522 Mon Sep 17 00:00:00 2001 From: jdar Date: Wed, 29 Oct 2025 11:36:31 -0700 Subject: [PATCH 15/26] round 5 - remove todos --- .../uhn/fhir/rest/server/interceptor/auth/RuleBuilderTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilderTest.java b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilderTest.java index b7978d4e5e6a..dd8bd36f3759 100644 --- a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilderTest.java +++ b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilderTest.java @@ -190,7 +190,6 @@ public void testBulkExport_PatientExportOnPatients_MultiplePatientsSingleRule(Co assertEquals(theExpectedResourceTypes, ruleBulkExport.getResourceTypes()); assertEquals(thePolicyEnum, ruleBulkExport.getMode()); } - //todo jdjd test like above public static Stream owners() { return Stream.of( From 0d906c47c9ad124bb1ccb7da3b82c750b6c549ab Mon Sep 17 00:00:00 2001 From: jdar Date: Wed, 29 Oct 2025 13:00:01 -0700 Subject: [PATCH 16/26] changelogs, more java docs --- ...ce-rulebuilder-to-support-compartments-by-matchers.yaml | 4 ++++ .../fhir/rest/server/interceptor/auth/IRuleApplier.java | 7 +++++++ 2 files changed, 11 insertions(+) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/8_6_0/7342-enhance-rulebuilder-to-support-compartments-by-matchers.yaml diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/8_6_0/7342-enhance-rulebuilder-to-support-compartments-by-matchers.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/8_6_0/7342-enhance-rulebuilder-to-support-compartments-by-matchers.yaml new file mode 100644 index 000000000000..44e57a16b360 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/8_6_0/7342-enhance-rulebuilder-to-support-compartments-by-matchers.yaml @@ -0,0 +1,4 @@ +--- +type: add +issue: 7342 +title: "Enhanced the bulk export RuleBuilder code to support the identification of allowable Groups/Patients to export by a FHIR query matcher." diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IRuleApplier.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IRuleApplier.java index 28cc61eda24d..84e2ff18856b 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IRuleApplier.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IRuleApplier.java @@ -51,6 +51,13 @@ default IAuthorizationSearchParamMatcher getSearchParamMatcher() { return null; } + /** + * The auth resource resolve is a service that allows you to query the DB for a resource, given a resource ID + * WARNING: This is slow, and should have limited use in authorization. + * + * It is currently used for bulk-export, to support permissible Group/Patient exports by matching a FHIR query + * This is ok, since bulk-export is a slow and (relatively) rare operation + */ default IAuthResourceResolver getAuthResourceResolver() { return null; } From 9727733ac577d06e119284b0b43a71f603215e20 Mon Sep 17 00:00:00 2001 From: jdar Date: Thu, 30 Oct 2025 15:55:20 -0700 Subject: [PATCH 17/26] round 1 of addressing review comments --- ...entBulkExportByCompartmentMatcherImpl.java | 58 +++++++++++-------- .../interceptor/auth/RuleBuilderTest.java | 3 +- ...ulkExportByCompartmentMatcherImplTest.java | 8 +-- ...ulkExportByCompartmentMatcherImplTest.java | 10 +--- 4 files changed, 39 insertions(+), 40 deletions(-) diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java index 54713d8831f2..c6709adae10b 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java @@ -23,7 +23,16 @@ import ca.uhn.fhir.rest.api.RestOperationTypeEnum; import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.api.server.bulk.BulkExportJobParameters; + +import static ca.uhn.fhir.rest.server.interceptor.auth.PolicyEnum.ALLOW; + +import static ca.uhn.fhir.rest.server.interceptor.auth.PolicyEnum.DENY; + import com.google.common.annotations.VisibleForTesting; + +import java.util.HashMap; +import java.util.Map; + import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; @@ -69,7 +78,7 @@ public AuthorizationInterceptor.Verdict applyRule( theRequestDetails.getUserData().get(REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS); if (inboundBulkExportRequestOptions.getExportStyle() != OUR_EXPORT_STYLE) { - // If the requested export style is not for a GROUP, then abstain + // If the requested export style is not for a PATIENT, then abstain return null; } @@ -77,11 +86,11 @@ public AuthorizationInterceptor.Verdict applyRule( if (isNotEmpty(myResourceTypes)) { if (isEmpty(inboundBulkExportRequestOptions.getResourceTypes())) { // Attempting an export on ALL resource types, but this rule restricts on a set of resource types - return new AuthorizationInterceptor.Verdict(PolicyEnum.DENY, this); + return new AuthorizationInterceptor.Verdict(DENY, this); } if (!myResourceTypes.containsAll(inboundBulkExportRequestOptions.getResourceTypes())) { // The requested resource types is not a subset of the permitted resource types - return new AuthorizationInterceptor.Verdict(PolicyEnum.DENY, this); + return new AuthorizationInterceptor.Verdict(DENY, this); } } @@ -96,12 +105,12 @@ public AuthorizationInterceptor.Verdict applyRule( // This is a type-level export with a _typeFilter // All filters must be permitted to return an ALLOW verdict return allFiltersMatch - ? new AuthorizationInterceptor.Verdict(PolicyEnum.ALLOW, this) - : new AuthorizationInterceptor.Verdict(PolicyEnum.DENY, this); + ? new AuthorizationInterceptor.Verdict(ALLOW, this) + : new AuthorizationInterceptor.Verdict(DENY, this); } else if (!allFiltersMatch) { // This is an instance-level export with a _typeFilter // Where at least one filter didn't match the permitted filters - return new AuthorizationInterceptor.Verdict(PolicyEnum.DENY, this); + return new AuthorizationInterceptor.Verdict(DENY, this); } } @@ -112,25 +121,26 @@ public AuthorizationInterceptor.Verdict applyRule( // resource, // and return the verdict // All requested Patient IDs must be permitted to return an ALLOW verdict. - List verdicts = thePatientResources.stream() - .map(patient -> applyTestersAtLeastOneMatch(theOperation, theRequestDetails, patient, theRuleApplier)) - .toList(); - - if (verdicts.stream().allMatch(Boolean::booleanValue)) { - // Then the testers evaluated to true on all Patients - // All the resources match at least 1 permission query filter --> ALLOW - return new AuthorizationInterceptor.Verdict(PolicyEnum.ALLOW, this); - } else if (verdicts.stream().noneMatch(Boolean::booleanValue)) { - // Then the testers evaluated to false on all Patients - // All the resources do not match any permission query filters - // This rule must not apply --> abstain - return null; - } else { - // Then the testers evaluated to true on some Patients, and false on others. - // We have a mixture of ALLOW and abstain - // Default to DENY - return new AuthorizationInterceptor.Verdict(PolicyEnum.DENY, this); + Map counts = new HashMap<>(); + counts.put(true, 0); + counts.put(false, 0); + + for (IBaseResource patient : thePatientResources) { + boolean applies = applyTestersAtLeastOneMatch(theOperation, theRequestDetails, patient, theRuleApplier); + + counts.put(applies, counts.get(applies) + 1); + + if (counts.get(applies) > 0 && counts.get(!applies) > 0) { + // Then the testers evaluated to true on some Patients, and false on others - no need to evaluate the rest + // We have a mixture of ALLOW and abstain + // Default to DENY + return new AuthorizationInterceptor.Verdict(PolicyEnum.DENY, this); + } } + + // If all testers evaluated to match, then ALLOW. If they all evaluated to false, then abstain. + // It's impossible to have a mixture due to the early-return in the for loop + return counts.get(true) > 0 ? new AuthorizationInterceptor.Verdict(PolicyEnum.ALLOW, this) : null; } /** diff --git a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilderTest.java b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilderTest.java index dd8bd36f3759..c0a8fbfb5af0 100644 --- a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilderTest.java +++ b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilderTest.java @@ -144,7 +144,8 @@ private static Stream patientMatcherBulkExportParams() { // Multiple arguments may be added to the filter when multiple FHIR_OP_INITIATE_BULK_DATA_EXPORT_PATIENTS_MATCHING permissions // are added to the same user, even when the permission does not accept multiple (a list of) arguments by itself. Arguments.of(List.of("identifier=foo|bar", "name=Doe"), List.of()), - Arguments.of(List.of("identifier=foo|bar", "name=Doe&active=true"), List.of("Patient", "Observation")) + Arguments.of(List.of("identifier=foo|bar", "name=Doe&active=true"), List.of("Patient", "Observation")), + Arguments.of(List.of("identifier=foo|bar", "active=true&name=Doe"), List.of("Patient", "Observation")) ); } diff --git a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImplTest.java b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImplTest.java index d06348cb6304..7e73cc815247 100644 --- a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImplTest.java +++ b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImplTest.java @@ -36,10 +36,6 @@ @ExtendWith(MockitoExtension.class) class RuleGroupBulkExportByCompartmentMatcherImplTest { - - private final RestOperationTypeEnum myOperation = RestOperationTypeEnum.EXTENDED_OPERATION_SERVER; - private final Pointcut myPointcut = Pointcut.STORAGE_INITIATE_BULK_EXPORT; - @Mock private IRuleApplier myRuleApplier; @Mock @@ -50,8 +46,6 @@ class RuleGroupBulkExportByCompartmentMatcherImplTest { private IAuthorizationSearchParamMatcher mySearchParamMatcher; @Mock private RequestDetails myRequestDetails; - @Mock - private Set myFlags; @ParameterizedTest @MethodSource("params") @@ -76,7 +70,7 @@ void testGroupRule_withCompartmentMatchers(IAuthorizationSearchParamMatcher.Matc when(mySearchParamMatcher.match("Group?identifier=foo|bar", myResource)).thenReturn(theSearchParamMatcherMatchResult); } - AuthorizationInterceptor.Verdict verdict = rule.applyRule(myOperation, myRequestDetails, null, null, null, myRuleApplier, myFlags, myPointcut); + AuthorizationInterceptor.Verdict verdict = rule.applyRule(RestOperationTypeEnum.EXTENDED_OPERATION_SERVER, myRequestDetails, null, null, null, myRuleApplier, Set.of(), Pointcut.STORAGE_INITIATE_BULK_EXPORT); if (theExpectedVerdict != null) { // Expect a decision diff --git a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImplTest.java b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImplTest.java index 0d897b622791..8477f485b529 100644 --- a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImplTest.java +++ b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImplTest.java @@ -35,10 +35,6 @@ @ExtendWith(MockitoExtension.class) class RulePatientBulkExportByCompartmentMatcherImplTest { - - private final RestOperationTypeEnum myOperation = RestOperationTypeEnum.EXTENDED_OPERATION_SERVER; - private final Pointcut myPointcut = Pointcut.STORAGE_INITIATE_BULK_EXPORT; - @Mock private IRuleApplier myRuleApplier; @Mock @@ -51,8 +47,6 @@ class RulePatientBulkExportByCompartmentMatcherImplTest { private IAuthorizationSearchParamMatcher mySearchParamMatcher; @Mock private RequestDetails myRequestDetails; - @Mock - private Set myFlags; @ParameterizedTest @@ -83,7 +77,7 @@ void testPatientRule_instanceLevelExport_withCompartmentMatchers(Collection when(myRequestDetails.getUserData()).thenReturn(Map.of(AuthorizationInterceptor.REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS, theBulkExportJobParams)); - AuthorizationInterceptor.Verdict verdict = rule.applyRule(myOperation, myRequestDetails, null, null, null, myRuleApplier, myFlags, myPointcut); + AuthorizationInterceptor.Verdict verdict = rule.applyRule(RestOperationTypeEnum.EXTENDED_OPERATION_SERVER, myRequestDetails, null, null, null, myRuleApplier, Set.of(), Pointcut.STORAGE_INITIATE_BULK_EXPORT); if (theExpectedVerdict != null) { // Expect a decision From f8b3d41e88c877248f17ba1fc28219c4a49a57b3 Mon Sep 17 00:00:00 2001 From: jdar Date: Thu, 30 Oct 2025 15:56:05 -0700 Subject: [PATCH 18/26] spotless --- ...atientBulkExportByCompartmentMatcherImpl.java | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java index c6709adae10b..95f72aea01fa 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java @@ -23,25 +23,20 @@ import ca.uhn.fhir.rest.api.RestOperationTypeEnum; import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.api.server.bulk.BulkExportJobParameters; - -import static ca.uhn.fhir.rest.server.interceptor.auth.PolicyEnum.ALLOW; - -import static ca.uhn.fhir.rest.server.interceptor.auth.PolicyEnum.DENY; - import com.google.common.annotations.VisibleForTesting; - -import java.util.HashMap; -import java.util.Map; - import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; import java.util.ArrayList; import java.util.Collection; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Set; import static ca.uhn.fhir.rest.server.interceptor.auth.AuthorizationInterceptor.REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS; +import static ca.uhn.fhir.rest.server.interceptor.auth.PolicyEnum.ALLOW; +import static ca.uhn.fhir.rest.server.interceptor.auth.PolicyEnum.DENY; import static org.apache.commons.collections4.CollectionUtils.isEmpty; import static org.apache.commons.collections4.CollectionUtils.isNotEmpty; @@ -131,7 +126,8 @@ public AuthorizationInterceptor.Verdict applyRule( counts.put(applies, counts.get(applies) + 1); if (counts.get(applies) > 0 && counts.get(!applies) > 0) { - // Then the testers evaluated to true on some Patients, and false on others - no need to evaluate the rest + // Then the testers evaluated to true on some Patients, and false on others - no need to evaluate the + // rest // We have a mixture of ALLOW and abstain // Default to DENY return new AuthorizationInterceptor.Verdict(PolicyEnum.DENY, this); From 7fdc3a40eeeaa82cf27a9aadd3ce52f17db6e4dd Mon Sep 17 00:00:00 2001 From: jdar Date: Thu, 30 Oct 2025 16:54:38 -0700 Subject: [PATCH 19/26] refactor rule classes to have a base class for common logic --- ...aseRuleBulkExportByCompartmentMatcher.java | 111 ++++++++++++++++++ ...oupBulkExportByCompartmentMatcherImpl.java | 57 ++------- ...entBulkExportByCompartmentMatcherImpl.java | 60 ++-------- 3 files changed, 127 insertions(+), 101 deletions(-) create mode 100644 hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRuleBulkExportByCompartmentMatcher.java diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRuleBulkExportByCompartmentMatcher.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRuleBulkExportByCompartmentMatcher.java new file mode 100644 index 000000000000..a8200ce4dd93 --- /dev/null +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRuleBulkExportByCompartmentMatcher.java @@ -0,0 +1,111 @@ +/*- + * #%L + * HAPI FHIR - Server Framework + * %% + * Copyright (C) 2014 - 2025 Smile CDR, Inc. + * %% + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * #L% + */ +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.BulkExportJobParameters; + +import static ca.uhn.fhir.rest.server.interceptor.auth.AuthorizationInterceptor.REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS; + +import com.google.common.annotations.VisibleForTesting; + +import java.util.Collection; +import java.util.Set; + +import static org.apache.commons.collections4.CollectionUtils.isEmpty; +import static org.apache.commons.collections4.CollectionUtils.isNotEmpty; + +import org.hl7.fhir.instance.model.api.IBaseResource; +import org.hl7.fhir.instance.model.api.IIdType; + +abstract class BaseRuleBulkExportByCompartmentMatcher extends BaseRule { + private final BulkExportJobParameters.ExportStyle myExportStyle; + private Collection myResourceTypes; + + BaseRuleBulkExportByCompartmentMatcher(String theRuleName, BulkExportJobParameters.ExportStyle theExportStyle) { + super(theRuleName); + myExportStyle = theExportStyle; + } + + @Override + public AuthorizationInterceptor.Verdict applyRule( + RestOperationTypeEnum theOperation, + RequestDetails theRequestDetails, + IBaseResource theInputResource, + IIdType theInputResourceId, + IBaseResource theOutputResource, + IRuleApplier theRuleApplier, + Set theFlags, + Pointcut thePointcut) { + if (thePointcut != Pointcut.STORAGE_INITIATE_BULK_EXPORT) { + return null; + } + + if (theRequestDetails == null) { + return null; + } + + BulkExportJobParameters inboundBulkExportRequestOptions = (BulkExportJobParameters) + theRequestDetails.getUserData().get(REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS); + + if (inboundBulkExportRequestOptions.getExportStyle() != myExportStyle) { + // If the requested export style is not the implied style by the implementer, then abstain + return null; + } + + // Do we only authorize some types? If so, make sure requested types are a subset + if (isNotEmpty(myResourceTypes)) { + if (isEmpty(inboundBulkExportRequestOptions.getResourceTypes())) { + // Attempting an export on ALL resource types, but this rule restricts on a set of resource types + return new AuthorizationInterceptor.Verdict(PolicyEnum.DENY, this); + } + if (!myResourceTypes.containsAll(inboundBulkExportRequestOptions.getResourceTypes())) { + // The requested resource types is not a subset of the permitted resource types + return new AuthorizationInterceptor.Verdict(PolicyEnum.DENY, this); + } + } + + // We passed the first few checks, so we'll ALLOW for now... + // Further checks are required by the implementation. + return new AuthorizationInterceptor.Verdict(PolicyEnum.ALLOW, this); + } + + public void setResourceTypes(Collection theResourceTypes) { + myResourceTypes = theResourceTypes; + } + + /** + * Remove the resource type and "?" prefix, if present + * since resource type is implied for the rule based on the permission (Patient in this case) + */ + static String sanitizeQueryFilter(String theFilter) { + if (theFilter.contains("?")) { + return theFilter.substring(theFilter.indexOf("?") + 1); + } + return theFilter; + } + + @VisibleForTesting + Collection getResourceTypes() { + return myResourceTypes; + } +} diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImpl.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImpl.java index 1db56fd8e647..4714f71c0eff 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImpl.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImpl.java @@ -34,14 +34,11 @@ import static org.apache.commons.collections4.CollectionUtils.isEmpty; import static org.apache.commons.collections4.CollectionUtils.isNotEmpty; -public class RuleGroupBulkExportByCompartmentMatcherImpl extends BaseRule { - private static final BulkExportJobParameters.ExportStyle OUR_EXPORT_STYLE = - BulkExportJobParameters.ExportStyle.GROUP; +public class RuleGroupBulkExportByCompartmentMatcherImpl extends BaseRuleBulkExportByCompartmentMatcher { private String myGroupMatcherFilter; - private Collection myResourceTypes; RuleGroupBulkExportByCompartmentMatcherImpl(String theRuleName) { - super(theRuleName); + super(theRuleName, BulkExportJobParameters.ExportStyle.GROUP); } @Override @@ -54,33 +51,15 @@ public AuthorizationInterceptor.Verdict applyRule( IRuleApplier theRuleApplier, Set theFlags, Pointcut thePointcut) { - if (thePointcut != Pointcut.STORAGE_INITIATE_BULK_EXPORT) { - return null; - } - - if (theRequestDetails == null) { - return null; + // Apply the base checks for invalid inputs, requested resource types + AuthorizationInterceptor.Verdict result = super.applyRule(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier, theFlags, thePointcut); + if (result == null || result.getDecision().equals(PolicyEnum.DENY)) { + // The base checks have already decided we should abstain, or deny + return result; } BulkExportJobParameters inboundBulkExportRequestOptions = (BulkExportJobParameters) - theRequestDetails.getUserData().get(REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS); - - if (inboundBulkExportRequestOptions.getExportStyle() != OUR_EXPORT_STYLE) { - // If the requested export style is not for a GROUP, then abstain - return null; - } - - // Do we only authorize some types? If so, make sure requested types are a subset - if (isNotEmpty(myResourceTypes)) { - if (isEmpty(inboundBulkExportRequestOptions.getResourceTypes())) { - // Attempting an export on ALL resource types, but this rule restricts on a set of resource types - return new AuthorizationInterceptor.Verdict(PolicyEnum.DENY, this); - } - if (!myResourceTypes.containsAll(inboundBulkExportRequestOptions.getResourceTypes())) { - // The requested resource types is not a subset of the permitted resource types - return new AuthorizationInterceptor.Verdict(PolicyEnum.DENY, this); - } - } + theRequestDetails.getUserData().get(REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS); IBaseResource theGroupResource = theRuleApplier .getAuthResourceResolver() @@ -97,10 +76,6 @@ public AuthorizationInterceptor.Verdict applyRule( theRuleApplier); } - public void setResourceTypes(Collection theResourceTypes) { - myResourceTypes = theResourceTypes; - } - public void setAppliesToGroupExportOnGroup(String theGroupMatcherFilter) { String sanitizedFilter = sanitizeQueryFilter(theGroupMatcherFilter); myGroupMatcherFilter = sanitizedFilter; @@ -110,20 +85,4 @@ public void setAppliesToGroupExportOnGroup(String theGroupMatcherFilter) { public String getGroupMatcherFilter() { return myGroupMatcherFilter; } - - /** - * Remove the resource type and "?" prefix, if present - * since resource type is implied for the rule based on the permission (Patient in this case) - */ - private static String sanitizeQueryFilter(String theFilter) { - if (theFilter.contains("?")) { - return theFilter.substring(theFilter.indexOf("?") + 1); - } - return theFilter; - } - - @VisibleForTesting - Collection getResourceTypes() { - return myResourceTypes; - } } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java index 95f72aea01fa..e2d46505270e 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java @@ -28,7 +28,6 @@ import org.hl7.fhir.instance.model.api.IIdType; import java.util.ArrayList; -import java.util.Collection; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -37,18 +36,13 @@ import static ca.uhn.fhir.rest.server.interceptor.auth.AuthorizationInterceptor.REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS; import static ca.uhn.fhir.rest.server.interceptor.auth.PolicyEnum.ALLOW; import static ca.uhn.fhir.rest.server.interceptor.auth.PolicyEnum.DENY; -import static org.apache.commons.collections4.CollectionUtils.isEmpty; -import static org.apache.commons.collections4.CollectionUtils.isNotEmpty; -public class RulePatientBulkExportByCompartmentMatcherImpl extends BaseRule { - private static final BulkExportJobParameters.ExportStyle OUR_EXPORT_STYLE = - BulkExportJobParameters.ExportStyle.PATIENT; +public class RulePatientBulkExportByCompartmentMatcherImpl extends BaseRuleBulkExportByCompartmentMatcher { private List myPatientMatcherFilter; private List> myTokenizedPatientMatcherFilter; - private Collection myResourceTypes; RulePatientBulkExportByCompartmentMatcherImpl(String theRuleName) { - super(theRuleName); + super(theRuleName, BulkExportJobParameters.ExportStyle.PATIENT); } @Override @@ -61,33 +55,15 @@ public AuthorizationInterceptor.Verdict applyRule( IRuleApplier theRuleApplier, Set theFlags, Pointcut thePointcut) { - if (thePointcut != Pointcut.STORAGE_INITIATE_BULK_EXPORT) { - return null; - } - - if (theRequestDetails == null) { - return null; + // Apply the base checks for invalid inputs, requested resource types + AuthorizationInterceptor.Verdict result = super.applyRule(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier, theFlags, thePointcut); + if (result == null || result.getDecision().equals(PolicyEnum.DENY)) { + // The base checks have already decided we should abstain, or deny + return result; } BulkExportJobParameters inboundBulkExportRequestOptions = (BulkExportJobParameters) - theRequestDetails.getUserData().get(REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS); - - if (inboundBulkExportRequestOptions.getExportStyle() != OUR_EXPORT_STYLE) { - // If the requested export style is not for a PATIENT, then abstain - return null; - } - - // Do we only authorize some types? If so, make sure requested types are a subset - if (isNotEmpty(myResourceTypes)) { - if (isEmpty(inboundBulkExportRequestOptions.getResourceTypes())) { - // Attempting an export on ALL resource types, but this rule restricts on a set of resource types - return new AuthorizationInterceptor.Verdict(DENY, this); - } - if (!myResourceTypes.containsAll(inboundBulkExportRequestOptions.getResourceTypes())) { - // The requested resource types is not a subset of the permitted resource types - return new AuthorizationInterceptor.Verdict(DENY, this); - } - } + theRequestDetails.getUserData().get(REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS); List patientIdOptions = inboundBulkExportRequestOptions.getPatientIds(); List filterOptions = inboundBulkExportRequestOptions.getFilters(); @@ -167,21 +143,6 @@ private boolean matchOnFilterOptions(List theFilterOptions) { return true; } - /** - * Remove the resource type and "?" prefix, if present - * since resource type is implied for the rule based on the permission (Patient in this case) - */ - private static String sanitizeQueryFilter(String theFilter) { - if (theFilter.contains("?")) { - return theFilter.substring(theFilter.indexOf("?") + 1); - } - return theFilter; - } - - public void setResourceTypes(Collection theResourceTypes) { - myResourceTypes = theResourceTypes; - } - /** * @param thePatientMatcherFilter the matcher filter for the permitted Patient */ @@ -206,11 +167,6 @@ public List getPatientMatcherFilters() { return myPatientMatcherFilter; } - @VisibleForTesting - Collection getResourceTypes() { - return myResourceTypes; - } - @VisibleForTesting public List> getTokenizedPatientMatcherFilter() { return myTokenizedPatientMatcherFilter; From 31d143cbbe0ddb39d57097f995f1d14b418cf260 Mon Sep 17 00:00:00 2001 From: jdar Date: Thu, 30 Oct 2025 17:00:58 -0700 Subject: [PATCH 20/26] spotless --- .../BaseRuleBulkExportByCompartmentMatcher.java | 9 +++------ ...leGroupBulkExportByCompartmentMatcherImpl.java | 15 ++++++++++----- ...PatientBulkExportByCompartmentMatcherImpl.java | 12 ++++++++++-- 3 files changed, 23 insertions(+), 13 deletions(-) diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRuleBulkExportByCompartmentMatcher.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRuleBulkExportByCompartmentMatcher.java index a8200ce4dd93..c21ccb3fb2bd 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRuleBulkExportByCompartmentMatcher.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRuleBulkExportByCompartmentMatcher.java @@ -23,20 +23,17 @@ import ca.uhn.fhir.rest.api.RestOperationTypeEnum; import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.api.server.bulk.BulkExportJobParameters; - -import static ca.uhn.fhir.rest.server.interceptor.auth.AuthorizationInterceptor.REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS; - import com.google.common.annotations.VisibleForTesting; +import org.hl7.fhir.instance.model.api.IBaseResource; +import org.hl7.fhir.instance.model.api.IIdType; import java.util.Collection; import java.util.Set; +import static ca.uhn.fhir.rest.server.interceptor.auth.AuthorizationInterceptor.REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS; import static org.apache.commons.collections4.CollectionUtils.isEmpty; import static org.apache.commons.collections4.CollectionUtils.isNotEmpty; -import org.hl7.fhir.instance.model.api.IBaseResource; -import org.hl7.fhir.instance.model.api.IIdType; - abstract class BaseRuleBulkExportByCompartmentMatcher extends BaseRule { private final BulkExportJobParameters.ExportStyle myExportStyle; private Collection myResourceTypes; diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImpl.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImpl.java index 4714f71c0eff..b65a0a8557a0 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImpl.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImpl.java @@ -24,15 +24,12 @@ import ca.uhn.fhir.rest.api.RestOperationTypeEnum; import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.api.server.bulk.BulkExportJobParameters; -import com.google.common.annotations.VisibleForTesting; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; import java.util.*; import static ca.uhn.fhir.rest.server.interceptor.auth.AuthorizationInterceptor.REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS; -import static org.apache.commons.collections4.CollectionUtils.isEmpty; -import static org.apache.commons.collections4.CollectionUtils.isNotEmpty; public class RuleGroupBulkExportByCompartmentMatcherImpl extends BaseRuleBulkExportByCompartmentMatcher { private String myGroupMatcherFilter; @@ -52,14 +49,22 @@ public AuthorizationInterceptor.Verdict applyRule( Set theFlags, Pointcut thePointcut) { // Apply the base checks for invalid inputs, requested resource types - AuthorizationInterceptor.Verdict result = super.applyRule(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier, theFlags, thePointcut); + AuthorizationInterceptor.Verdict result = super.applyRule( + theOperation, + theRequestDetails, + theInputResource, + theInputResourceId, + theOutputResource, + theRuleApplier, + theFlags, + thePointcut); if (result == null || result.getDecision().equals(PolicyEnum.DENY)) { // The base checks have already decided we should abstain, or deny return result; } BulkExportJobParameters inboundBulkExportRequestOptions = (BulkExportJobParameters) - theRequestDetails.getUserData().get(REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS); + theRequestDetails.getUserData().get(REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS); IBaseResource theGroupResource = theRuleApplier .getAuthResourceResolver() diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java index e2d46505270e..ed4d56a6c70d 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java @@ -56,14 +56,22 @@ public AuthorizationInterceptor.Verdict applyRule( Set theFlags, Pointcut thePointcut) { // Apply the base checks for invalid inputs, requested resource types - AuthorizationInterceptor.Verdict result = super.applyRule(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier, theFlags, thePointcut); + AuthorizationInterceptor.Verdict result = super.applyRule( + theOperation, + theRequestDetails, + theInputResource, + theInputResourceId, + theOutputResource, + theRuleApplier, + theFlags, + thePointcut); if (result == null || result.getDecision().equals(PolicyEnum.DENY)) { // The base checks have already decided we should abstain, or deny return result; } BulkExportJobParameters inboundBulkExportRequestOptions = (BulkExportJobParameters) - theRequestDetails.getUserData().get(REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS); + theRequestDetails.getUserData().get(REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS); List patientIdOptions = inboundBulkExportRequestOptions.getPatientIds(); List filterOptions = inboundBulkExportRequestOptions.getFilters(); From 17ed1ad728e143e6ec47b4011301796b36c7a850 Mon Sep 17 00:00:00 2001 From: jdar Date: Thu, 30 Oct 2025 17:13:58 -0700 Subject: [PATCH 21/26] Revert "spotless" This reverts commit 31d143cbbe0ddb39d57097f995f1d14b418cf260. --- .../BaseRuleBulkExportByCompartmentMatcher.java | 9 ++++++--- ...leGroupBulkExportByCompartmentMatcherImpl.java | 15 +++++---------- ...PatientBulkExportByCompartmentMatcherImpl.java | 12 ++---------- 3 files changed, 13 insertions(+), 23 deletions(-) diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRuleBulkExportByCompartmentMatcher.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRuleBulkExportByCompartmentMatcher.java index c21ccb3fb2bd..a8200ce4dd93 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRuleBulkExportByCompartmentMatcher.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRuleBulkExportByCompartmentMatcher.java @@ -23,17 +23,20 @@ import ca.uhn.fhir.rest.api.RestOperationTypeEnum; import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.api.server.bulk.BulkExportJobParameters; + +import static ca.uhn.fhir.rest.server.interceptor.auth.AuthorizationInterceptor.REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS; + import com.google.common.annotations.VisibleForTesting; -import org.hl7.fhir.instance.model.api.IBaseResource; -import org.hl7.fhir.instance.model.api.IIdType; import java.util.Collection; import java.util.Set; -import static ca.uhn.fhir.rest.server.interceptor.auth.AuthorizationInterceptor.REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS; import static org.apache.commons.collections4.CollectionUtils.isEmpty; import static org.apache.commons.collections4.CollectionUtils.isNotEmpty; +import org.hl7.fhir.instance.model.api.IBaseResource; +import org.hl7.fhir.instance.model.api.IIdType; + abstract class BaseRuleBulkExportByCompartmentMatcher extends BaseRule { private final BulkExportJobParameters.ExportStyle myExportStyle; private Collection myResourceTypes; diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImpl.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImpl.java index b65a0a8557a0..4714f71c0eff 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImpl.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImpl.java @@ -24,12 +24,15 @@ import ca.uhn.fhir.rest.api.RestOperationTypeEnum; import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.api.server.bulk.BulkExportJobParameters; +import com.google.common.annotations.VisibleForTesting; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; import java.util.*; import static ca.uhn.fhir.rest.server.interceptor.auth.AuthorizationInterceptor.REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS; +import static org.apache.commons.collections4.CollectionUtils.isEmpty; +import static org.apache.commons.collections4.CollectionUtils.isNotEmpty; public class RuleGroupBulkExportByCompartmentMatcherImpl extends BaseRuleBulkExportByCompartmentMatcher { private String myGroupMatcherFilter; @@ -49,22 +52,14 @@ public AuthorizationInterceptor.Verdict applyRule( Set theFlags, Pointcut thePointcut) { // Apply the base checks for invalid inputs, requested resource types - AuthorizationInterceptor.Verdict result = super.applyRule( - theOperation, - theRequestDetails, - theInputResource, - theInputResourceId, - theOutputResource, - theRuleApplier, - theFlags, - thePointcut); + AuthorizationInterceptor.Verdict result = super.applyRule(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier, theFlags, thePointcut); if (result == null || result.getDecision().equals(PolicyEnum.DENY)) { // The base checks have already decided we should abstain, or deny return result; } BulkExportJobParameters inboundBulkExportRequestOptions = (BulkExportJobParameters) - theRequestDetails.getUserData().get(REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS); + theRequestDetails.getUserData().get(REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS); IBaseResource theGroupResource = theRuleApplier .getAuthResourceResolver() diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java index ed4d56a6c70d..e2d46505270e 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java @@ -56,22 +56,14 @@ public AuthorizationInterceptor.Verdict applyRule( Set theFlags, Pointcut thePointcut) { // Apply the base checks for invalid inputs, requested resource types - AuthorizationInterceptor.Verdict result = super.applyRule( - theOperation, - theRequestDetails, - theInputResource, - theInputResourceId, - theOutputResource, - theRuleApplier, - theFlags, - thePointcut); + AuthorizationInterceptor.Verdict result = super.applyRule(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier, theFlags, thePointcut); if (result == null || result.getDecision().equals(PolicyEnum.DENY)) { // The base checks have already decided we should abstain, or deny return result; } BulkExportJobParameters inboundBulkExportRequestOptions = (BulkExportJobParameters) - theRequestDetails.getUserData().get(REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS); + theRequestDetails.getUserData().get(REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS); List patientIdOptions = inboundBulkExportRequestOptions.getPatientIds(); List filterOptions = inboundBulkExportRequestOptions.getFilters(); From 5f52b36580dd3158f6859d51284a6c4c829f492f Mon Sep 17 00:00:00 2001 From: jdar Date: Thu, 30 Oct 2025 17:13:58 -0700 Subject: [PATCH 22/26] Revert "refactor rule classes to have a base class for common logic" This reverts commit 7fdc3a40eeeaa82cf27a9aadd3ce52f17db6e4dd. --- ...aseRuleBulkExportByCompartmentMatcher.java | 111 ------------------ ...oupBulkExportByCompartmentMatcherImpl.java | 57 +++++++-- ...entBulkExportByCompartmentMatcherImpl.java | 60 ++++++++-- 3 files changed, 101 insertions(+), 127 deletions(-) delete mode 100644 hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRuleBulkExportByCompartmentMatcher.java diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRuleBulkExportByCompartmentMatcher.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRuleBulkExportByCompartmentMatcher.java deleted file mode 100644 index a8200ce4dd93..000000000000 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRuleBulkExportByCompartmentMatcher.java +++ /dev/null @@ -1,111 +0,0 @@ -/*- - * #%L - * HAPI FHIR - Server Framework - * %% - * Copyright (C) 2014 - 2025 Smile CDR, Inc. - * %% - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * #L% - */ -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.BulkExportJobParameters; - -import static ca.uhn.fhir.rest.server.interceptor.auth.AuthorizationInterceptor.REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS; - -import com.google.common.annotations.VisibleForTesting; - -import java.util.Collection; -import java.util.Set; - -import static org.apache.commons.collections4.CollectionUtils.isEmpty; -import static org.apache.commons.collections4.CollectionUtils.isNotEmpty; - -import org.hl7.fhir.instance.model.api.IBaseResource; -import org.hl7.fhir.instance.model.api.IIdType; - -abstract class BaseRuleBulkExportByCompartmentMatcher extends BaseRule { - private final BulkExportJobParameters.ExportStyle myExportStyle; - private Collection myResourceTypes; - - BaseRuleBulkExportByCompartmentMatcher(String theRuleName, BulkExportJobParameters.ExportStyle theExportStyle) { - super(theRuleName); - myExportStyle = theExportStyle; - } - - @Override - public AuthorizationInterceptor.Verdict applyRule( - RestOperationTypeEnum theOperation, - RequestDetails theRequestDetails, - IBaseResource theInputResource, - IIdType theInputResourceId, - IBaseResource theOutputResource, - IRuleApplier theRuleApplier, - Set theFlags, - Pointcut thePointcut) { - if (thePointcut != Pointcut.STORAGE_INITIATE_BULK_EXPORT) { - return null; - } - - if (theRequestDetails == null) { - return null; - } - - BulkExportJobParameters inboundBulkExportRequestOptions = (BulkExportJobParameters) - theRequestDetails.getUserData().get(REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS); - - if (inboundBulkExportRequestOptions.getExportStyle() != myExportStyle) { - // If the requested export style is not the implied style by the implementer, then abstain - return null; - } - - // Do we only authorize some types? If so, make sure requested types are a subset - if (isNotEmpty(myResourceTypes)) { - if (isEmpty(inboundBulkExportRequestOptions.getResourceTypes())) { - // Attempting an export on ALL resource types, but this rule restricts on a set of resource types - return new AuthorizationInterceptor.Verdict(PolicyEnum.DENY, this); - } - if (!myResourceTypes.containsAll(inboundBulkExportRequestOptions.getResourceTypes())) { - // The requested resource types is not a subset of the permitted resource types - return new AuthorizationInterceptor.Verdict(PolicyEnum.DENY, this); - } - } - - // We passed the first few checks, so we'll ALLOW for now... - // Further checks are required by the implementation. - return new AuthorizationInterceptor.Verdict(PolicyEnum.ALLOW, this); - } - - public void setResourceTypes(Collection theResourceTypes) { - myResourceTypes = theResourceTypes; - } - - /** - * Remove the resource type and "?" prefix, if present - * since resource type is implied for the rule based on the permission (Patient in this case) - */ - static String sanitizeQueryFilter(String theFilter) { - if (theFilter.contains("?")) { - return theFilter.substring(theFilter.indexOf("?") + 1); - } - return theFilter; - } - - @VisibleForTesting - Collection getResourceTypes() { - return myResourceTypes; - } -} diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImpl.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImpl.java index 4714f71c0eff..1db56fd8e647 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImpl.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImpl.java @@ -34,11 +34,14 @@ import static org.apache.commons.collections4.CollectionUtils.isEmpty; import static org.apache.commons.collections4.CollectionUtils.isNotEmpty; -public class RuleGroupBulkExportByCompartmentMatcherImpl extends BaseRuleBulkExportByCompartmentMatcher { +public class RuleGroupBulkExportByCompartmentMatcherImpl extends BaseRule { + private static final BulkExportJobParameters.ExportStyle OUR_EXPORT_STYLE = + BulkExportJobParameters.ExportStyle.GROUP; private String myGroupMatcherFilter; + private Collection myResourceTypes; RuleGroupBulkExportByCompartmentMatcherImpl(String theRuleName) { - super(theRuleName, BulkExportJobParameters.ExportStyle.GROUP); + super(theRuleName); } @Override @@ -51,15 +54,33 @@ public AuthorizationInterceptor.Verdict applyRule( IRuleApplier theRuleApplier, Set theFlags, Pointcut thePointcut) { - // Apply the base checks for invalid inputs, requested resource types - AuthorizationInterceptor.Verdict result = super.applyRule(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier, theFlags, thePointcut); - if (result == null || result.getDecision().equals(PolicyEnum.DENY)) { - // The base checks have already decided we should abstain, or deny - return result; + if (thePointcut != Pointcut.STORAGE_INITIATE_BULK_EXPORT) { + return null; + } + + if (theRequestDetails == null) { + return null; } BulkExportJobParameters inboundBulkExportRequestOptions = (BulkExportJobParameters) - theRequestDetails.getUserData().get(REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS); + theRequestDetails.getUserData().get(REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS); + + if (inboundBulkExportRequestOptions.getExportStyle() != OUR_EXPORT_STYLE) { + // If the requested export style is not for a GROUP, then abstain + return null; + } + + // Do we only authorize some types? If so, make sure requested types are a subset + if (isNotEmpty(myResourceTypes)) { + if (isEmpty(inboundBulkExportRequestOptions.getResourceTypes())) { + // Attempting an export on ALL resource types, but this rule restricts on a set of resource types + return new AuthorizationInterceptor.Verdict(PolicyEnum.DENY, this); + } + if (!myResourceTypes.containsAll(inboundBulkExportRequestOptions.getResourceTypes())) { + // The requested resource types is not a subset of the permitted resource types + return new AuthorizationInterceptor.Verdict(PolicyEnum.DENY, this); + } + } IBaseResource theGroupResource = theRuleApplier .getAuthResourceResolver() @@ -76,6 +97,10 @@ public AuthorizationInterceptor.Verdict applyRule( theRuleApplier); } + public void setResourceTypes(Collection theResourceTypes) { + myResourceTypes = theResourceTypes; + } + public void setAppliesToGroupExportOnGroup(String theGroupMatcherFilter) { String sanitizedFilter = sanitizeQueryFilter(theGroupMatcherFilter); myGroupMatcherFilter = sanitizedFilter; @@ -85,4 +110,20 @@ public void setAppliesToGroupExportOnGroup(String theGroupMatcherFilter) { public String getGroupMatcherFilter() { return myGroupMatcherFilter; } + + /** + * Remove the resource type and "?" prefix, if present + * since resource type is implied for the rule based on the permission (Patient in this case) + */ + private static String sanitizeQueryFilter(String theFilter) { + if (theFilter.contains("?")) { + return theFilter.substring(theFilter.indexOf("?") + 1); + } + return theFilter; + } + + @VisibleForTesting + Collection getResourceTypes() { + return myResourceTypes; + } } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java index e2d46505270e..95f72aea01fa 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java @@ -28,6 +28,7 @@ import org.hl7.fhir.instance.model.api.IIdType; import java.util.ArrayList; +import java.util.Collection; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -36,13 +37,18 @@ import static ca.uhn.fhir.rest.server.interceptor.auth.AuthorizationInterceptor.REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS; import static ca.uhn.fhir.rest.server.interceptor.auth.PolicyEnum.ALLOW; import static ca.uhn.fhir.rest.server.interceptor.auth.PolicyEnum.DENY; +import static org.apache.commons.collections4.CollectionUtils.isEmpty; +import static org.apache.commons.collections4.CollectionUtils.isNotEmpty; -public class RulePatientBulkExportByCompartmentMatcherImpl extends BaseRuleBulkExportByCompartmentMatcher { +public class RulePatientBulkExportByCompartmentMatcherImpl extends BaseRule { + private static final BulkExportJobParameters.ExportStyle OUR_EXPORT_STYLE = + BulkExportJobParameters.ExportStyle.PATIENT; private List myPatientMatcherFilter; private List> myTokenizedPatientMatcherFilter; + private Collection myResourceTypes; RulePatientBulkExportByCompartmentMatcherImpl(String theRuleName) { - super(theRuleName, BulkExportJobParameters.ExportStyle.PATIENT); + super(theRuleName); } @Override @@ -55,15 +61,33 @@ public AuthorizationInterceptor.Verdict applyRule( IRuleApplier theRuleApplier, Set theFlags, Pointcut thePointcut) { - // Apply the base checks for invalid inputs, requested resource types - AuthorizationInterceptor.Verdict result = super.applyRule(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier, theFlags, thePointcut); - if (result == null || result.getDecision().equals(PolicyEnum.DENY)) { - // The base checks have already decided we should abstain, or deny - return result; + if (thePointcut != Pointcut.STORAGE_INITIATE_BULK_EXPORT) { + return null; + } + + if (theRequestDetails == null) { + return null; } BulkExportJobParameters inboundBulkExportRequestOptions = (BulkExportJobParameters) - theRequestDetails.getUserData().get(REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS); + theRequestDetails.getUserData().get(REQUEST_ATTRIBUTE_BULK_DATA_EXPORT_OPTIONS); + + if (inboundBulkExportRequestOptions.getExportStyle() != OUR_EXPORT_STYLE) { + // If the requested export style is not for a PATIENT, then abstain + return null; + } + + // Do we only authorize some types? If so, make sure requested types are a subset + if (isNotEmpty(myResourceTypes)) { + if (isEmpty(inboundBulkExportRequestOptions.getResourceTypes())) { + // Attempting an export on ALL resource types, but this rule restricts on a set of resource types + return new AuthorizationInterceptor.Verdict(DENY, this); + } + if (!myResourceTypes.containsAll(inboundBulkExportRequestOptions.getResourceTypes())) { + // The requested resource types is not a subset of the permitted resource types + return new AuthorizationInterceptor.Verdict(DENY, this); + } + } List patientIdOptions = inboundBulkExportRequestOptions.getPatientIds(); List filterOptions = inboundBulkExportRequestOptions.getFilters(); @@ -143,6 +167,21 @@ private boolean matchOnFilterOptions(List theFilterOptions) { return true; } + /** + * Remove the resource type and "?" prefix, if present + * since resource type is implied for the rule based on the permission (Patient in this case) + */ + private static String sanitizeQueryFilter(String theFilter) { + if (theFilter.contains("?")) { + return theFilter.substring(theFilter.indexOf("?") + 1); + } + return theFilter; + } + + public void setResourceTypes(Collection theResourceTypes) { + myResourceTypes = theResourceTypes; + } + /** * @param thePatientMatcherFilter the matcher filter for the permitted Patient */ @@ -167,6 +206,11 @@ public List getPatientMatcherFilters() { return myPatientMatcherFilter; } + @VisibleForTesting + Collection getResourceTypes() { + return myResourceTypes; + } + @VisibleForTesting public List> getTokenizedPatientMatcherFilter() { return myTokenizedPatientMatcherFilter; From 5390b9bd0aedb1c44bb4faf3ca2cb6fa2ca5dbbd Mon Sep 17 00:00:00 2001 From: jdar Date: Thu, 30 Oct 2025 18:19:03 -0700 Subject: [PATCH 23/26] round 2 - address code review comments --- .../ca/uhn/fhir/jpa/interceptor/AuthResourceResolver.java | 5 ++--- .../ca/uhn/fhir/rest/server/interceptor/auth/BaseRule.java | 2 +- .../rest/server/interceptor/auth/IAuthResourceResolver.java | 4 ++-- .../auth/IAuthRuleBuilderRuleGroupMatcherBulkExport.java | 4 +++- .../auth/IAuthRuleBuilderRulePatientMatcherBulkExport.java | 4 +++- .../auth/RuleGroupBulkExportByCompartmentMatcherImpl.java | 2 +- .../auth/RulePatientBulkExportByCompartmentMatcherImpl.java | 4 ++-- .../RuleGroupBulkExportByCompartmentMatcherImplTest.java | 2 +- .../RulePatientBulkExportByCompartmentMatcherImplTest.java | 4 ++-- 9 files changed, 17 insertions(+), 14 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/AuthResourceResolver.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/AuthResourceResolver.java index 6e67e24a89bd..7b31eb9f2180 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/AuthResourceResolver.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/AuthResourceResolver.java @@ -35,7 +35,6 @@ * Small service class to inject DB access into an interceptor * For example, used in bulk export security to allow querying for resource to match against permission argument filters */ -@Service public class AuthResourceResolver implements IAuthResourceResolver { private final DaoRegistry myDaoRegistry; @@ -43,13 +42,13 @@ public AuthResourceResolver(DaoRegistry myDaoRegistry) { this.myDaoRegistry = myDaoRegistry; } - public IBaseResource resolveCompartmentById(IIdType theResourceId) { + public IBaseResource resolveResourceById(IIdType theResourceId) { return myDaoRegistry .getResourceDao(theResourceId.getResourceType()) .read(theResourceId, new SystemRequestDetails()); } - public List resolveCompartmentByIds(List theResourceIds, String theResourceType) { + public List resolveResourcesByIds(List theResourceIds, String theResourceType) { TokenOrListParam t = new TokenOrListParam(null, theResourceIds.toArray(String[]::new)); SearchParameterMap m = new SearchParameterMap(); diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRule.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRule.java index 789533cbe407..249ac7b37973 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRule.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRule.java @@ -97,7 +97,7 @@ private boolean applyTesters( * Apply testers, and return true if at least 1 tester matches. * Returns false if all testers do not match. */ - boolean applyTestersAtLeastOneMatch( + boolean atLeastOneTesterMatches( RestOperationTypeEnum theOperation, RequestDetails theRequestDetails, IBaseResource theInputResource, diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthResourceResolver.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthResourceResolver.java index 3fb2a4e075ea..2fe7909ccd07 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthResourceResolver.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthResourceResolver.java @@ -29,7 +29,7 @@ * For example, used in bulk export security to allow querying for resource to match against permission argument filters */ public interface IAuthResourceResolver { - IBaseResource resolveCompartmentById(IIdType theResourceId); + IBaseResource resolveResourceById(IIdType theResourceId); /** * Resolve a list of resources by ID. All resources should be the same type. @@ -37,5 +37,5 @@ public interface IAuthResourceResolver { * @param theResourceType the type of resource * @return A list of resources resolved by ID */ - List resolveCompartmentByIds(List theResourceIds, String theResourceType); + List resolveResourcesByIds(List theResourceIds, String theResourceType); } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRuleGroupMatcherBulkExport.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRuleGroupMatcherBulkExport.java index 37d73e4dde0b..702913cfdbb0 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRuleGroupMatcherBulkExport.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRuleGroupMatcherBulkExport.java @@ -26,7 +26,9 @@ */ public interface IAuthRuleBuilderRuleGroupMatcherBulkExport { /** - * Allow/deny group-level export rule applies to the Group with the given resource ID, e.g. Group/123 + * Allow/deny group-level export rule applies to the Group by matching on the provided FHIR query filter, + * e.g. ?identifier=foo|bar + * Note that resource type is implied to be Group * * @since 8.6.0 */ diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRulePatientMatcherBulkExport.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRulePatientMatcherBulkExport.java index 17de6fef8d57..a8eeae9d693c 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRulePatientMatcherBulkExport.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRulePatientMatcherBulkExport.java @@ -26,7 +26,9 @@ */ public interface IAuthRuleBuilderRulePatientMatcherBulkExport { /** - * Allow/deny group-level export rule applies to the Group with the given resource ID, e.g. Group/123 + * Allow/deny patient-level export rule applies to the Patient by matching on the provided FHIR query filter, + * e.g. ?identifier=foo|bar + * Note that resource type is implied to be Patient * * @since 8.6.0 */ diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImpl.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImpl.java index 1db56fd8e647..450366b27a60 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImpl.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImpl.java @@ -84,7 +84,7 @@ public AuthorizationInterceptor.Verdict applyRule( IBaseResource theGroupResource = theRuleApplier .getAuthResourceResolver() - .resolveCompartmentById(new IdDt(inboundBulkExportRequestOptions.getGroupId())); + .resolveResourceById(new IdDt(inboundBulkExportRequestOptions.getGroupId())); // Apply the FhirQueryTester (which contains a inMemoryResourceMatcher) to the found Group compartment resource, // and return the verdict diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java index 95f72aea01fa..19481d55c620 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImpl.java @@ -110,7 +110,7 @@ public AuthorizationInterceptor.Verdict applyRule( } List thePatientResources = - theRuleApplier.getAuthResourceResolver().resolveCompartmentByIds(patientIdOptions, "Patient"); + theRuleApplier.getAuthResourceResolver().resolveResourcesByIds(patientIdOptions, "Patient"); // Apply the FhirQueryTester (which contains a inMemoryResourceMatcher) to the found Patient compartment // resource, @@ -121,7 +121,7 @@ public AuthorizationInterceptor.Verdict applyRule( counts.put(false, 0); for (IBaseResource patient : thePatientResources) { - boolean applies = applyTestersAtLeastOneMatch(theOperation, theRequestDetails, patient, theRuleApplier); + boolean applies = atLeastOneTesterMatches(theOperation, theRequestDetails, patient, theRuleApplier); counts.put(applies, counts.get(applies) + 1); diff --git a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImplTest.java b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImplTest.java index 7e73cc815247..c07dd97d5661 100644 --- a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImplTest.java +++ b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleGroupBulkExportByCompartmentMatcherImplTest.java @@ -64,7 +64,7 @@ void testGroupRule_withCompartmentMatchers(IAuthorizationSearchParamMatcher.Matc if (theSearchParamMatcherMatchResult != null) { when(myRuleApplier.getAuthResourceResolver()).thenReturn(myAuthResourceResolver); - when(myAuthResourceResolver.resolveCompartmentById(any())).thenReturn(myResource); + when(myAuthResourceResolver.resolveResourceById(any())).thenReturn(myResource); when(myRuleApplier.getSearchParamMatcher()).thenReturn(mySearchParamMatcher); when(myResource.fhirType()).thenReturn("Group"); when(mySearchParamMatcher.match("Group?identifier=foo|bar", myResource)).thenReturn(theSearchParamMatcherMatchResult); diff --git a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImplTest.java b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImplTest.java index 8477f485b529..291a175b4fef 100644 --- a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImplTest.java +++ b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RulePatientBulkExportByCompartmentMatcherImplTest.java @@ -66,9 +66,9 @@ void testPatientRule_instanceLevelExport_withCompartmentMatchers(Collection Date: Thu, 30 Oct 2025 18:20:47 -0700 Subject: [PATCH 24/26] spotless --- .../java/ca/uhn/fhir/jpa/interceptor/AuthResourceResolver.java | 1 - 1 file changed, 1 deletion(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/AuthResourceResolver.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/AuthResourceResolver.java index 7b31eb9f2180..c59a4415398d 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/AuthResourceResolver.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/AuthResourceResolver.java @@ -27,7 +27,6 @@ import ca.uhn.fhir.rest.server.interceptor.auth.IAuthResourceResolver; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; -import org.springframework.stereotype.Service; import java.util.List; From b50a5aa9019fd7c30ac9cfc22ee942eb9bbce2a2 Mon Sep 17 00:00:00 2001 From: jdar Date: Mon, 17 Nov 2025 11:39:28 -0800 Subject: [PATCH 25/26] draft implementation of combining RuleBuilder apis --- .../auth/IAuthRuleBuilderRule.java | 16 --- .../auth/IAuthRuleBuilderRuleBulkExport.java | 18 +++ ...RuleBuilderRuleGroupMatcherBulkExport.java | 36 ------ ...leBuilderRulePatientMatcherBulkExport.java | 36 ------ .../server/interceptor/auth/RuleBuilder.java | 116 +++++++----------- .../interceptor/auth/RuleBuilderTest.java | 4 +- 6 files changed, 65 insertions(+), 161 deletions(-) delete mode 100644 hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRuleGroupMatcherBulkExport.java delete mode 100644 hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRulePatientMatcherBulkExport.java diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRule.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRule.java index 3a194eb012dd..c2dfef0f3f4e 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRule.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRule.java @@ -127,22 +127,6 @@ public interface IAuthRuleBuilderRule { */ IAuthRuleBuilderRuleBulkExport bulkExport(); - /** - * This rule permits the user to initiate a FHIR bulk export - * by providing a filter matcher on Group compartment(s). - * - * @since 8.6.0 - */ - IAuthRuleBuilderRuleGroupMatcherBulkExport bulkExportGroupCompartmentMatcher(); - - /** - * This rule permits the user to initiate a FHIR bulk export - * by providing a filter matcher on Patient compartment(s). - * - * @since 8.6.0 - */ - IAuthRuleBuilderRulePatientMatcherBulkExport bulkExportPatientCompartmentMatcher(); - /** * This rule specifically allows a user to perform a FHIR update on the historical version of a resource * diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRuleBulkExport.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRuleBulkExport.java index 1719cd0c8632..f69ce28c3c52 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRuleBulkExport.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRuleBulkExport.java @@ -46,6 +46,24 @@ default IAuthRuleBuilderRuleBulkExportWithTarget groupExportOnGroup(@Nonnull IId */ IAuthRuleBuilderRuleBulkExportWithTarget groupExportOnGroup(@Nonnull String theFocusResourceId); + /** + * Allow/deny group-level export rule applies to the Group by matching on the provided FHIR query filter, + * e.g. ?identifier=foo|bar + * Note that resource type is implied to be Group + * + * @since 8.6.0 + */ + IAuthRuleBuilderRuleBulkExportWithTarget groupExportOnFilter(@Nonnull String theCompartmentFilterMatcher); + + /** + * Allow/deny patient-level export rule applies to the Patient by matching on the provided FHIR query filter, + * e.g. ?identifier=foo|bar + * Note that resource type is implied to be Patient + * + * @since 8.6.0 + */ + IAuthRuleBuilderRuleBulkExportWithTarget patientExportOnFilter(@Nonnull String theCompartmentFilterMatcher); + /** * Allow/deny patient-level export rule applies to the Group with the given resource ID, e.g. Group/123 * diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRuleGroupMatcherBulkExport.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRuleGroupMatcherBulkExport.java deleted file mode 100644 index 702913cfdbb0..000000000000 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRuleGroupMatcherBulkExport.java +++ /dev/null @@ -1,36 +0,0 @@ -/*- - * #%L - * HAPI FHIR - Server Framework - * %% - * Copyright (C) 2014 - 2025 Smile CDR, Inc. - * %% - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * #L% - */ -package ca.uhn.fhir.rest.server.interceptor.auth; - -import jakarta.annotation.Nonnull; - -/** - * @since 8.6.0 - */ -public interface IAuthRuleBuilderRuleGroupMatcherBulkExport { - /** - * Allow/deny group-level export rule applies to the Group by matching on the provided FHIR query filter, - * e.g. ?identifier=foo|bar - * Note that resource type is implied to be Group - * - * @since 8.6.0 - */ - IAuthRuleBuilderRuleBulkExportWithTarget groupExportOnGroup(@Nonnull String theCompartmentFilterMatcher); -} diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRulePatientMatcherBulkExport.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRulePatientMatcherBulkExport.java deleted file mode 100644 index a8eeae9d693c..000000000000 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRulePatientMatcherBulkExport.java +++ /dev/null @@ -1,36 +0,0 @@ -/*- - * #%L - * HAPI FHIR - Server Framework - * %% - * Copyright (C) 2014 - 2025 Smile CDR, Inc. - * %% - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * #L% - */ -package ca.uhn.fhir.rest.server.interceptor.auth; - -import jakarta.annotation.Nonnull; - -/** - * @since 8.6.0 - */ -public interface IAuthRuleBuilderRulePatientMatcherBulkExport { - /** - * Allow/deny patient-level export rule applies to the Patient by matching on the provided FHIR query filter, - * e.g. ?identifier=foo|bar - * Note that resource type is implied to be Patient - * - * @since 8.6.0 - */ - IAuthRuleBuilderRuleBulkExportWithTarget patientExportOnPatient(@Nonnull String theCompartmentFilterMatcher); -} diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java index 8afe0ab16e90..1fb4dfabdc0e 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java @@ -260,8 +260,6 @@ private class RuleBuilderRule implements IAuthRuleBuilderRule { private RuleBuilderRuleOp myReadRuleBuilder; private RuleBuilderRuleOp myWriteRuleBuilder; private RuleBuilderBulkExport myRuleBuilderBulkExport; - private RuleBuilderGroupMatcherBulkExport myRuleBuilderGroupMatcherBulkExport; - private RuleBuilderPatientMatcherBulkExport myRuleBuilderPatientMatcherBulkExport; RuleBuilderRule(PolicyEnum theRuleMode, String theRuleName) { myRuleMode = theRuleMode; @@ -349,22 +347,6 @@ public IAuthRuleBuilderRuleBulkExport bulkExport() { return myRuleBuilderBulkExport; } - @Override - public IAuthRuleBuilderRuleGroupMatcherBulkExport bulkExportGroupCompartmentMatcher() { - if (myRuleBuilderGroupMatcherBulkExport == null) { - myRuleBuilderGroupMatcherBulkExport = new RuleBuilderGroupMatcherBulkExport(); - } - return myRuleBuilderGroupMatcherBulkExport; - } - - @Override - public IAuthRuleBuilderRulePatientMatcherBulkExport bulkExportPatientCompartmentMatcher() { - if (myRuleBuilderPatientMatcherBulkExport == null) { - myRuleBuilderPatientMatcherBulkExport = new RuleBuilderPatientMatcherBulkExport(); - } - return myRuleBuilderPatientMatcherBulkExport; - } - @Override public IAuthRuleBuilderUpdateHistoryRewrite updateHistoryRewrite() { return new UpdateHistoryRewriteBuilder(); @@ -906,6 +888,8 @@ public IAuthRuleFinished any() { private class RuleBuilderBulkExport implements IAuthRuleBuilderRuleBulkExport { private RuleBulkExportImpl myRuleBulkExport; + private RuleGroupBulkExportByCompartmentMatcherImpl myRuleGroupBulkExportByCompartmentMatcher; + private RulePatientBulkExportByCompartmentMatcherImpl myRulePatientBulkExportByCompartmentMatcher; @Override public IAuthRuleBuilderRuleBulkExportWithTarget groupExportOnGroup(@Nonnull String theFocusResourceId) { @@ -1003,38 +987,18 @@ public IAuthRuleBuilderRuleBulkExportWithTarget any() { return new RuleBuilderBulkExportWithTarget(rule); } - private class RuleBuilderBulkExportWithTarget extends RuleBuilderFinished - implements IAuthRuleBuilderRuleBulkExportWithTarget { - private final RuleBulkExportImpl myRule; - - private RuleBuilderBulkExportWithTarget(RuleBulkExportImpl theRule) { - super(theRule); - myRule = theRule; - } - - @Override - public IAuthRuleBuilderRuleBulkExportWithTarget withResourceTypes(Collection theResourceTypes) { - myRule.setResourceTypes(theResourceTypes); - return this; - } - } - } - - private class RuleBuilderGroupMatcherBulkExport implements IAuthRuleBuilderRuleGroupMatcherBulkExport { - private RuleGroupBulkExportByCompartmentMatcherImpl myRuleGroupBulkExportByCompartmentMatcher; - @Override - public IAuthRuleBuilderRuleBulkExportWithTarget groupExportOnGroup( - @Nonnull String theCompartmentFilterMatcher) { + public IAuthRuleBuilderRuleBulkExportWithTarget groupExportOnFilter( + @Nonnull String theCompartmentFilterMatcher) { if (myRuleGroupBulkExportByCompartmentMatcher == null) { RuleGroupBulkExportByCompartmentMatcherImpl rule = - new RuleGroupBulkExportByCompartmentMatcherImpl(myRuleName); + new RuleGroupBulkExportByCompartmentMatcherImpl(myRuleName); rule.setAppliesToGroupExportOnGroup(theCompartmentFilterMatcher); rule.setMode(myRuleMode); myRuleGroupBulkExportByCompartmentMatcher = rule; } else { myRuleGroupBulkExportByCompartmentMatcher.setAppliesToGroupExportOnGroup( - theCompartmentFilterMatcher); + theCompartmentFilterMatcher); } // prevent duplicate rules from being added @@ -1042,43 +1006,22 @@ public IAuthRuleBuilderRuleBulkExportWithTarget groupExportOnGroup( myRules.add(myRuleGroupBulkExportByCompartmentMatcher); } - return new RuleBuilderGroupMatcherBulkExport.RuleBuilderBulkExportWithTarget( - myRuleGroupBulkExportByCompartmentMatcher); - } - - private class RuleBuilderBulkExportWithTarget extends RuleBuilderFinished - implements IAuthRuleBuilderRuleBulkExportWithTarget { - private final RuleGroupBulkExportByCompartmentMatcherImpl myRule; - - private RuleBuilderBulkExportWithTarget(RuleGroupBulkExportByCompartmentMatcherImpl theRule) { - super(theRule); - myRule = theRule; - } - - @Override - public IAuthRuleBuilderRuleBulkExportWithTarget withResourceTypes(Collection theResourceTypes) { - myRule.setResourceTypes(theResourceTypes); - return this; - } + return new RuleBuilderGroupBulkExportWithFilter(myRuleGroupBulkExportByCompartmentMatcher); } - } - - private class RuleBuilderPatientMatcherBulkExport implements IAuthRuleBuilderRulePatientMatcherBulkExport { - private RulePatientBulkExportByCompartmentMatcherImpl myRulePatientBulkExportByCompartmentMatcher; @Override - public IAuthRuleBuilderRuleBulkExportWithTarget patientExportOnPatient( - @Nonnull String theCompartmentFilterMatcher) { + public IAuthRuleBuilderRuleBulkExportWithTarget patientExportOnFilter( + @Nonnull String theCompartmentFilterMatcher) { if (myRulePatientBulkExportByCompartmentMatcher == null) { RulePatientBulkExportByCompartmentMatcherImpl rule = - new RulePatientBulkExportByCompartmentMatcherImpl(myRuleName); + new RulePatientBulkExportByCompartmentMatcherImpl(myRuleName); rule.addAppliesToPatientExportOnPatient(theCompartmentFilterMatcher); rule.setMode(myRuleMode); myRulePatientBulkExportByCompartmentMatcher = rule; } else { myRulePatientBulkExportByCompartmentMatcher.addAppliesToPatientExportOnPatient( - theCompartmentFilterMatcher); + theCompartmentFilterMatcher); } // prevent duplicate rules from being added @@ -1086,15 +1029,46 @@ public IAuthRuleBuilderRuleBulkExportWithTarget patientExportOnPatient( myRules.add(myRulePatientBulkExportByCompartmentMatcher); } - return new RuleBuilderPatientMatcherBulkExport.RuleBuilderBulkExportWithTarget( - myRulePatientBulkExportByCompartmentMatcher); + return new RuleBuilderPatientBulkExportWithFilter(myRulePatientBulkExportByCompartmentMatcher); } private class RuleBuilderBulkExportWithTarget extends RuleBuilderFinished implements IAuthRuleBuilderRuleBulkExportWithTarget { + private final RuleBulkExportImpl myRule; + + private RuleBuilderBulkExportWithTarget(RuleBulkExportImpl theRule) { + super(theRule); + myRule = theRule; + } + + @Override + public IAuthRuleBuilderRuleBulkExportWithTarget withResourceTypes(Collection theResourceTypes) { + myRule.setResourceTypes(theResourceTypes); + return this; + } + } + + private class RuleBuilderGroupBulkExportWithFilter extends RuleBuilderFinished + implements IAuthRuleBuilderRuleBulkExportWithTarget { + private final RuleGroupBulkExportByCompartmentMatcherImpl myRule; + + private RuleBuilderGroupBulkExportWithFilter(RuleGroupBulkExportByCompartmentMatcherImpl theRule) { + super(theRule); + myRule = theRule; + } + + @Override + public IAuthRuleBuilderRuleBulkExportWithTarget withResourceTypes(Collection theResourceTypes) { + myRule.setResourceTypes(theResourceTypes); + return this; + } + } + + private class RuleBuilderPatientBulkExportWithFilter extends RuleBuilderFinished + implements IAuthRuleBuilderRuleBulkExportWithTarget { private final RulePatientBulkExportByCompartmentMatcherImpl myRule; - private RuleBuilderBulkExportWithTarget(RulePatientBulkExportByCompartmentMatcherImpl theRule) { + private RuleBuilderPatientBulkExportWithFilter(RulePatientBulkExportByCompartmentMatcherImpl theRule) { super(theRule); myRule = theRule; } diff --git a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilderTest.java b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilderTest.java index c0a8fbfb5af0..70e7189fa51f 100644 --- a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilderTest.java +++ b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilderTest.java @@ -92,7 +92,7 @@ public void testBulkExportByGroupMatcher(String theCompartmentMatcherFilter, Col List resourceTypes = new ArrayList<>(theResourceTypes); // When - builder.allow().bulkExportGroupCompartmentMatcher().groupExportOnGroup("?" + theCompartmentMatcherFilter).withResourceTypes(resourceTypes); + builder.allow().bulkExport().groupExportOnFilter("?" + theCompartmentMatcherFilter).withResourceTypes(resourceTypes); final List rules = builder.build(); // Then @@ -122,7 +122,7 @@ public void testBulkExportByPatientMatcher(List theCompartmentMatcherFil // When for (String filter : theCompartmentMatcherFilter) { - builder.allow().bulkExportPatientCompartmentMatcher().patientExportOnPatient("?" + filter).withResourceTypes(resourceTypes); + builder.allow().bulkExport().patientExportOnFilter("?" + filter).withResourceTypes(resourceTypes); } final List rules = builder.build(); From d558587c2a23e632c36e522794b5256b65811285 Mon Sep 17 00:00:00 2001 From: jdar Date: Mon, 17 Nov 2025 11:42:14 -0800 Subject: [PATCH 26/26] spotless --- .../server/interceptor/auth/RuleBuilder.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java index 1fb4dfabdc0e..2eb2c9e86745 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java @@ -989,16 +989,16 @@ public IAuthRuleBuilderRuleBulkExportWithTarget any() { @Override public IAuthRuleBuilderRuleBulkExportWithTarget groupExportOnFilter( - @Nonnull String theCompartmentFilterMatcher) { + @Nonnull String theCompartmentFilterMatcher) { if (myRuleGroupBulkExportByCompartmentMatcher == null) { RuleGroupBulkExportByCompartmentMatcherImpl rule = - new RuleGroupBulkExportByCompartmentMatcherImpl(myRuleName); + new RuleGroupBulkExportByCompartmentMatcherImpl(myRuleName); rule.setAppliesToGroupExportOnGroup(theCompartmentFilterMatcher); rule.setMode(myRuleMode); myRuleGroupBulkExportByCompartmentMatcher = rule; } else { myRuleGroupBulkExportByCompartmentMatcher.setAppliesToGroupExportOnGroup( - theCompartmentFilterMatcher); + theCompartmentFilterMatcher); } // prevent duplicate rules from being added @@ -1011,17 +1011,17 @@ public IAuthRuleBuilderRuleBulkExportWithTarget groupExportOnFilter( @Override public IAuthRuleBuilderRuleBulkExportWithTarget patientExportOnFilter( - @Nonnull String theCompartmentFilterMatcher) { + @Nonnull String theCompartmentFilterMatcher) { if (myRulePatientBulkExportByCompartmentMatcher == null) { RulePatientBulkExportByCompartmentMatcherImpl rule = - new RulePatientBulkExportByCompartmentMatcherImpl(myRuleName); + new RulePatientBulkExportByCompartmentMatcherImpl(myRuleName); rule.addAppliesToPatientExportOnPatient(theCompartmentFilterMatcher); rule.setMode(myRuleMode); myRulePatientBulkExportByCompartmentMatcher = rule; } else { myRulePatientBulkExportByCompartmentMatcher.addAppliesToPatientExportOnPatient( - theCompartmentFilterMatcher); + theCompartmentFilterMatcher); } // prevent duplicate rules from being added @@ -1049,7 +1049,7 @@ public IAuthRuleBuilderRuleBulkExportWithTarget withResourceTypes(Collection