Compare commits

...

5 Commits

Author SHA1 Message Date
James Agnew 62c8f94f11
Merge 703924938f into 3f6d1eb29b 2024-09-26 02:08:11 +00:00
Thomas Papke 3f6d1eb29b
#5768 Upgrade to latest simple-java-mail (#6261) 2024-09-26 02:07:27 +00:00
Tadgh 377e44b6ca
attribution and pom change (#6309) 2024-09-25 20:38:22 +00:00
James Agnew 703924938f Cleanup 2024-09-10 10:02:52 -04:00
James Agnew 7ad2524272 Fix #6258 - Improve auth interceptor operation handling 2024-09-10 10:00:27 -04:00
17 changed files with 319 additions and 195 deletions

View File

@ -0,0 +1,10 @@
---
type: fix
issue: 6258
title: "The AuthorizationInterceptor handling for operations has been improved
so that operation rules now directly test the contents of response Bundle
or Parameters objects returned by the operation when configure to require
explicit response authorization. This fixes a regression in 7.4.0 where
operation responses could sometimes be denied even if appropriate
permissions were granted to view resources in a response bundle. Thanks to
Gijsbert van den Brink for reporting the issue with a sample test!"

View File

@ -0,0 +1,5 @@
---
type: fix
issue: 6290
title: "Previously, a specific migration task was using the `TRIM()` function, which does not exist in MSSQL 2012. This was causing migrations targeting MSSQL 2012 to fail.
This has been corrected and replaced with usage of a combination of LTRIM() and RTRIM(). Thanks to Primož Delopst at Better for the contribution!"

View File

@ -70,6 +70,11 @@
<artifactId>jakarta.servlet-api</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>jakarta.mail</groupId>
<artifactId>jakarta.mail-api</artifactId>
<optional>true</optional>
</dependency>
<!-- test dependencies -->
<dependency>

View File

@ -28,8 +28,8 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import javax.mail.internet.InternetAddress;
import javax.mail.internet.MimeMessage;
import jakarta.mail.internet.InternetAddress;
import jakarta.mail.internet.MimeMessage;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

View File

@ -17,8 +17,8 @@ import org.junit.jupiter.api.extension.RegisterExtension;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import javax.mail.internet.InternetAddress;
import javax.mail.internet.MimeMessage;
import jakarta.mail.internet.InternetAddress;
import jakarta.mail.internet.MimeMessage;
import java.util.Arrays;
import static org.assertj.core.api.Assertions.assertThat;

View File

@ -26,8 +26,8 @@ import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.springframework.beans.factory.annotation.Autowired;
import javax.mail.internet.InternetAddress;
import javax.mail.internet.MimeMessage;
import jakarta.mail.internet.InternetAddress;
import jakarta.mail.internet.MimeMessage;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

View File

@ -20,11 +20,15 @@ import ca.uhn.fhir.rest.api.Constants;
import ca.uhn.fhir.rest.api.MethodOutcome;
import ca.uhn.fhir.rest.api.server.RequestDetails;
import ca.uhn.fhir.rest.client.interceptor.SimpleRequestHeaderInterceptor;
import ca.uhn.fhir.rest.gclient.IOperation;
import ca.uhn.fhir.rest.gclient.IOperationUnnamed;
import ca.uhn.fhir.rest.gclient.IOperationUntypedWithInput;
import ca.uhn.fhir.rest.server.exceptions.ForbiddenOperationException;
import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
import ca.uhn.fhir.rest.server.exceptions.ResourceGoneException;
import ca.uhn.fhir.rest.server.interceptor.auth.AuthorizationInterceptor;
import ca.uhn.fhir.rest.server.interceptor.auth.IAuthRule;
import ca.uhn.fhir.rest.server.interceptor.auth.IAuthRuleBuilder;
import ca.uhn.fhir.rest.server.interceptor.auth.IAuthRuleTester;
import ca.uhn.fhir.rest.server.interceptor.auth.PolicyEnum;
import ca.uhn.fhir.rest.server.interceptor.auth.RuleBuilder;
@ -68,6 +72,7 @@ import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.CsvSource;
import org.junit.jupiter.params.provider.MethodSource;
import org.junit.jupiter.params.provider.NullSource;
import org.junit.jupiter.params.provider.ValueSource;
@ -897,8 +902,17 @@ public class AuthorizationInterceptorJpaR4Test extends BaseResourceProviderR4Tes
}
@Test
public void testDiffOperation_AllowedByType_Instance() {
@ParameterizedTest
@CsvSource({
// ResourceId , RequireExplicitResponseAuthorization , ShouldSucceed
"Patient/A , true , true",
"Patient/A/_history/2 , true , true",
"Observation/B , true , false",
"Patient/A , false , true",
"Patient/A/_history/2 , false , true",
"Observation/B , false , true"
})
public void testDiffOperation_AllowedByType_Instance(String theResourceId, boolean theRequireExplicitResponseAuthorization, boolean theShouldSucceed) {
createPatient(withId("A"), withActiveTrue());
createPatient(withId("A"), withActiveFalse());
createObservation(withId("B"), withStatus("final"));
@ -906,30 +920,42 @@ public class AuthorizationInterceptorJpaR4Test extends BaseResourceProviderR4Tes
myServer.getRestfulServer().registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) {
@Override
public List<IAuthRule> buildRuleList(RequestDetails theRequestDetails) {
return new RuleBuilder()
.allow().operation().named(ProviderConstants.DIFF_OPERATION_NAME).onAnyInstance().andAllowAllResponses().andThen()
.allow().operation().named(ProviderConstants.DIFF_OPERATION_NAME).onServer().andAllowAllResponses().andThen()
.allow().read().resourcesOfType(Patient.class).withAnyId().andThen()
.denyAll()
.build();
RuleBuilder ruleBuilder = new RuleBuilder();
if (theRequireExplicitResponseAuthorization) {
ruleBuilder.allow().operation().named(ProviderConstants.DIFF_OPERATION_NAME).onAnyInstance().andRequireExplicitResponseAuthorization().andThen();
ruleBuilder.allow().operation().named(ProviderConstants.DIFF_OPERATION_NAME).onServer().andRequireExplicitResponseAuthorization().andThen();
} else {
ruleBuilder.allow().operation().named(ProviderConstants.DIFF_OPERATION_NAME).onAnyInstance().andAllowAllResponses().andThen();
ruleBuilder.allow().operation().named(ProviderConstants.DIFF_OPERATION_NAME).onServer().andAllowAllResponses().andThen();
}
ruleBuilder.allow().read().resourcesOfType(Patient.class).withAnyId().andThen();
ruleBuilder.denyAll();
return ruleBuilder.build();
}
});
Parameters diff;
diff = myClient.operation().onInstance("Patient/A").named(ProviderConstants.DIFF_OPERATION_NAME).withNoParameters(Parameters.class).execute();
assertThat(diff.getParameter()).hasSize(1);
IOperation operation = myClient.operation();
IOperationUnnamed target;
if (theResourceId.contains("_history")) {
target = operation.onInstanceVersion(new IdType(theResourceId));
} else {
target = operation.onInstance(theResourceId);
}
IOperationUntypedWithInput<Parameters> executable = target.named(ProviderConstants.DIFF_OPERATION_NAME).withNoParameters(Parameters.class);
diff = myClient.operation().onInstanceVersion(new IdType("Patient/A/_history/2")).named(ProviderConstants.DIFF_OPERATION_NAME).withNoParameters(Parameters.class).execute();
if (theShouldSucceed) {
diff = executable.execute();
assertThat(diff.getParameter()).hasSize(1);
} else {
try {
myClient.operation().onInstance("Observation/B").named(ProviderConstants.DIFF_OPERATION_NAME).withNoParameters(Parameters.class).execute();
executable.execute();
fail();
} catch (ForbiddenOperationException e) {
// good
}
}
}
@Test
@ -943,8 +969,8 @@ public class AuthorizationInterceptorJpaR4Test extends BaseResourceProviderR4Tes
@Override
public List<IAuthRule> buildRuleList(RequestDetails theRequestDetails) {
return new RuleBuilder()
.allow().operation().named(ProviderConstants.DIFF_OPERATION_NAME).onAnyInstance().andAllowAllResponses().andThen()
.allow().operation().named(ProviderConstants.DIFF_OPERATION_NAME).onServer().andAllowAllResponses().andThen()
.allow().operation().named(ProviderConstants.DIFF_OPERATION_NAME).onAnyInstance().andRequireExplicitResponseAuthorization().andThen()
.allow().operation().named(ProviderConstants.DIFF_OPERATION_NAME).onServer().andRequireExplicitResponseAuthorization().andThen()
.allow().read().resourcesOfType(Patient.class).withAnyId().andThen()
.denyAll()
.build();
@ -977,6 +1003,72 @@ public class AuthorizationInterceptorJpaR4Test extends BaseResourceProviderR4Tes
}
@Test
public void testDocumentOperation_withExplicitAuthorization() {
IIdType patientId = createPatient();
IIdType compositionId = createResource("Composition", withSubject(patientId));
AuthorizationInterceptor interceptor = new AuthorizationInterceptor(PolicyEnum.DENY) {
@Override
public List<IAuthRule> buildRuleList(RequestDetails theRequestDetails) {
return new RuleBuilder()
.allow().read().instance(patientId).andThen()
.allow().operation().named("$document").onInstance(compositionId).andRequireExplicitResponseAuthorization().andThen()
.allow().read().instance(compositionId).andThen()
.denyAll()
.build();
}
};
myServer.getRestfulServer().registerInterceptor(interceptor);
Bundle bundle = myClient.operation().onInstanceVersion(compositionId).named("$document").withNoParameters(Parameters.class).returnResourceType(Bundle.class).execute();
assertEquals(2, bundle.getEntry().size());
}
@Test
public void testDocumentOperation_explicitAuthorizationNotNeeded() {
IIdType patientId = createPatient();
IIdType compositionId = createResource("Composition", withSubject(patientId));
AuthorizationInterceptor interceptor = new AuthorizationInterceptor(PolicyEnum.DENY) {
@Override
public List<IAuthRule> buildRuleList(RequestDetails theRequestDetails) {
return new RuleBuilder()
.allow().operation().named("$document").onInstance(compositionId).andAllowAllResponses().andThen()
.denyAll()
.build();
}
};
myServer.getRestfulServer().registerInterceptor(interceptor);
Bundle bundle = myClient.operation().onInstanceVersion(compositionId).named("$document").withNoParameters(Parameters.class).returnResourceType(Bundle.class).execute();
assertEquals(2, bundle.getEntry().size());
}
@Test
public void testDocumentOperation_withoutExplicitAuthorization() {
IIdType patientId = createPatient();
IIdType compositionId = createResource("Composition", withSubject(patientId));
AuthorizationInterceptor interceptor = new AuthorizationInterceptor(PolicyEnum.DENY) {
@Override
public List<IAuthRule> buildRuleList(RequestDetails theRequestDetails) {
return new RuleBuilder()
.allow().operation().named("$document").onInstance(compositionId).andRequireExplicitResponseAuthorization().andThen()
.allow().read().instance(compositionId).andThen()
.denyAll()
.build();
}
};
myServer.getRestfulServer().registerInterceptor(interceptor);
try {
myClient.operation().onInstanceVersion(compositionId).named("$document").withNoParameters(Parameters.class).returnResourceType(Bundle.class).execute();
fail();
} catch (ForbiddenOperationException e) {
// good
}
}
@Test
public void testGraphQL_AllowedByType_Instance() throws IOException {
@ -1205,8 +1297,17 @@ public class AuthorizationInterceptorJpaR4Test extends BaseResourceProviderR4Tes
ourLog.debug(myFhirContext.newJsonParser().setPrettyPrint(true).encodeResourceToString(resp));
}
@Test
public void testOperationEverything_SomeIncludedResourcesNotAuthorized() {
@ParameterizedTest
@CsvSource({
// RequireExplicitResponseAuthorization , AddNotExplicitlyAuthorizedResources , ShouldSucceed
"true , false , true",
"true , true , false",
"false , false , true",
"false , true , true",
})
public void testOperationEverything_SomeIncludedResourcesNotAuthorized(boolean theRequireExplicitResponseAuthorization, boolean theAddNotExplicitlyAuthorizedResources, boolean theShouldSucceed) {
int expectedCount = 2;
Patient pt1 = new Patient();
pt1.setActive(true);
final IIdType pid1 = myClient.create().resource(pt1).execute().getId().toUnqualifiedVersionless();
@ -1216,47 +1317,59 @@ public class AuthorizationInterceptorJpaR4Test extends BaseResourceProviderR4Tes
obs1.setSubject(new Reference(pid1));
myClient.create().resource(obs1).execute();
myServer.getRestfulServer().registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) {
@Override
public List<IAuthRule> buildRuleList(RequestDetails theRequestDetails) {
return new RuleBuilder()
.allow().operation().named(JpaConstants.OPERATION_EVERYTHING).onInstance(pid1).andRequireExplicitResponseAuthorization().andThen()
.allow().read().resourcesOfType(Patient.class).inCompartment("Patient", pid1).andThen()
.allow().read().resourcesOfType(Observation.class).inCompartment("Patient", pid1).andThen()
.allow().create().resourcesOfType(Encounter.class).withAnyId().andThen()
.build();
}
});
Bundle outcome = myClient
.operation()
.onInstance(pid1)
.named(JpaConstants.OPERATION_EVERYTHING)
.withNoParameters(Parameters.class)
.returnResourceType(Bundle.class)
.execute();
assertThat(outcome.getEntry()).hasSize(2);
if (theAddNotExplicitlyAuthorizedResources) {
// Add an Encounter, which will be returned by $everything but that hasn't been
// explicitly authorized
Encounter enc = new Encounter();
enc.setSubject(new Reference(pid1));
myClient.create().resource(enc).execute();
try {
myClient
Organization org = new Organization();
org.setName("Hello");
IIdType orgId = myClient.create().resource(org).execute().getId().toUnqualifiedVersionless();
expectedCount++;
pt1.setId(pid1);
pt1.setManagingOrganization(new Reference(orgId));
myClient.update().resource(pt1).execute();
expectedCount++;
}
myServer.getRestfulServer().registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) {
@SuppressWarnings("deprecation")
@Override
public List<IAuthRule> buildRuleList(RequestDetails theRequestDetails) {
IAuthRuleBuilder ruleBuilder = new RuleBuilder();
if (theRequireExplicitResponseAuthorization) {
ruleBuilder.allow().operation().named(JpaConstants.OPERATION_EVERYTHING).onInstance(pid1).andRequireExplicitResponseAuthorization().andThen();
} else {
ruleBuilder.allow().operation().named(JpaConstants.OPERATION_EVERYTHING).onInstance(pid1).andAllowAllResponsesWithAllResourcesAccess().andThen();
}
ruleBuilder.allow().read().resourcesOfType(Patient.class).inCompartment("Patient", pid1).andThen();
ruleBuilder.allow().read().resourcesOfType(Observation.class).inCompartment("Patient", pid1).andThen();
return ruleBuilder.build();
}
});
IOperationUntypedWithInput<Bundle> executable = myClient
.operation()
.onInstance(pid1)
.named(JpaConstants.OPERATION_EVERYTHING)
.withNoParameters(Parameters.class)
.returnResourceType(Bundle.class)
.execute();
.returnResourceType(Bundle.class);
if (theShouldSucceed) {
Bundle outcome = executable.execute();
assertThat(outcome.getEntry()).hasSize(expectedCount);
} else {
try {
executable.execute();
fail();
} catch (ForbiddenOperationException e) {
assertThat(e.getMessage()).contains("Access denied by default policy");
}
}
}
@Test
@ -2029,6 +2142,7 @@ public class AuthorizationInterceptorJpaR4Test extends BaseResourceProviderR4Tes
super(theResourceTypes);
}
@Override
public List<IAuthRule> buildRuleList(RequestDetails theRequestDetails) {
List<IAuthRule> rules = new ArrayList<>(super.buildRuleList(theRequestDetails));
List<IAuthRule> rulesToAdd = new RuleBuilder().allow().transaction().withAnyOperation().andApplyNormalRules().build();

View File

@ -79,25 +79,11 @@
<dependency>
<groupId>org.simplejavamail</groupId>
<artifactId>simple-java-mail</artifactId>
<!-- Excluded in favor of jakarta.activation:jakarta.activation-api -->
<exclusions>
<exclusion>
<groupId>com.sun.activation</groupId>
<artifactId>jakarta.activation</artifactId>
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>com.icegreen</groupId>
<artifactId>greenmail</artifactId>
<scope>compile</scope>
<!-- Excluded in favor of jakarta.activation:jakarta.activation-api -->
<exclusions>
<exclusion>
<groupId>com.sun.activation</groupId>
<artifactId>jakarta.activation</artifactId>
</exclusion>
</exclusions>
</dependency>
</dependencies>

View File

@ -164,7 +164,8 @@ public class AuthorizationInterceptor implements IRuleApplier {
rules.size(),
getPointcutNameOrEmpty(thePointcut),
getResourceTypeOrEmpty(theInputResource),
getResourceTypeOrEmpty(theOutputResource));
getResourceTypeOrEmpty(theOutputResource),
thePointcut);
Verdict verdict = null;
for (IAuthRule nextRule : rules) {
@ -528,7 +529,7 @@ public class AuthorizationInterceptor implements IRuleApplier {
case EXTENDED_OPERATION_TYPE:
case EXTENDED_OPERATION_INSTANCE: {
if (theResponseObject != null) {
resources = toListOfResourcesAndExcludeContainer(theResponseObject, fhirContext);
resources = toListOfResourcesAndExcludeContainerUnlessStandalone(theResponseObject, fhirContext);
}
break;
}
@ -575,7 +576,7 @@ public class AuthorizationInterceptor implements IRuleApplier {
OUT,
}
protected static List<IBaseResource> toListOfResourcesAndExcludeContainer(
protected static List<IBaseResource> toListOfResourcesAndExcludeContainerUnlessStandalone(
IBaseResource theResponseObject, FhirContext fhirContext) {
if (theResponseObject == null) {
return Collections.emptyList();
@ -588,6 +589,13 @@ public class AuthorizationInterceptor implements IRuleApplier {
return Collections.singletonList(theResponseObject);
}
return toListOfResourcesAndExcludeContainer(theResponseObject, fhirContext);
}
@Nonnull
public static List<IBaseResource> toListOfResourcesAndExcludeContainer(
IBaseResource theResponseObject, FhirContext fhirContext) {
List<IBaseResource> retVal;
retVal = fhirContext.newTerser().getAllPopulatedChildElementsOfType(theResponseObject, IBaseResource.class);
// Exclude the container

View File

@ -27,11 +27,9 @@ public interface IAuthRuleBuilderOperationNamedAndScoped {
IAuthRuleBuilderRuleOpClassifierFinished andAllowAllResponses();
/**
* Responses for this operation will not be checked and access to all resources is allowed. This
* is intended for operations which are known to fetch a graph of resources that is known to be
* safe, such as `$everything` which may access and fetch resources outside the patient's compartment
* but enforces safety in what it fetches via strict SQL queries.
* @deprecated This is a synonym for {@link #andAllowAllResponses()}, use that method instead
*/
@Deprecated(since = "7.6.0")
IAuthRuleBuilderRuleOpClassifierFinished andAllowAllResponsesWithAllResourcesAccess();
/**

View File

@ -85,6 +85,7 @@ class OperationRule extends BaseRule implements IAuthRule {
myAppliesToTypes = theAppliesToTypes;
}
@SuppressWarnings("EnumSwitchStatementWhichMissesCases")
@Override
public Verdict applyRule(
RestOperationTypeEnum theOperation,
@ -97,23 +98,8 @@ class OperationRule extends BaseRule implements IAuthRule {
Pointcut thePointcut) {
FhirContext ctx = theRequestDetails.getServer().getFhirContext();
// Operation rules apply to the execution of the operation itself, not to side effects like
// loading resources (that will presumably be reflected in the response). Those loads need
// to be explicitly authorized
if (!myAllowAllResourcesAccess && isResourceAccess(thePointcut)) {
return null;
}
boolean applies = false;
switch (theOperation) {
case ADD_TAGS:
case DELETE_TAGS:
case GET_TAGS:
case GET_PAGE:
case GRAPHQL_REQUEST:
// These things can't be tracked by the AuthorizationInterceptor
// at this time
return null;
case EXTENDED_OPERATION_SERVER:
if (myAppliesToServer || myAppliesAtAnyLevel) {
applies = true;
@ -138,10 +124,7 @@ class OperationRule extends BaseRule implements IAuthRule {
applies = true;
} else {
IIdType requestResourceId = null;
if (theInputResourceId != null) {
requestResourceId = theInputResourceId;
}
if (requestResourceId == null && myAllowAllResponses) {
if (theRequestDetails.getId() != null) {
requestResourceId = theRequestDetails.getId();
}
if (requestResourceId != null) {
@ -168,40 +151,6 @@ class OperationRule extends BaseRule implements IAuthRule {
}
}
break;
case CREATE:
break;
case DELETE:
break;
case HISTORY_INSTANCE:
break;
case HISTORY_SYSTEM:
break;
case HISTORY_TYPE:
break;
case READ:
break;
case SEARCH_SYSTEM:
break;
case SEARCH_TYPE:
break;
case TRANSACTION:
break;
case UPDATE:
break;
case VALIDATE:
break;
case VREAD:
break;
case METADATA:
break;
case META_ADD:
break;
case META:
break;
case META_DELETE:
break;
case PATCH:
break;
default:
return null;
}
@ -214,6 +163,8 @@ class OperationRule extends BaseRule implements IAuthRule {
return null;
}
if (theOutputResource == null) {
// This is the request part
return newVerdict(
theOperation,
theRequestDetails,
@ -221,6 +172,24 @@ class OperationRule extends BaseRule implements IAuthRule {
theInputResourceId,
theOutputResource,
theRuleApplier);
} else {
// This is the response part, so we might want to check all of the
// resources in the response
if (myAllowAllResponses) {
return newVerdict(
theOperation,
theRequestDetails,
theInputResource,
theInputResourceId,
theOutputResource,
theRuleApplier);
} else {
List<IBaseResource> outputResources = AuthorizationInterceptor.toListOfResourcesAndExcludeContainer(
theOutputResource, theRequestDetails.getFhirContext());
return RuleImplOp.applyRulesToResponseResources(
theRequestDetails, theRuleApplier, thePointcut, outputResources);
}
}
}
/**

View File

@ -828,10 +828,30 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ {
return verdict;
} else if (theOutputResource != null) {
return applyRulesToResponseBundle(theRequestDetails, theOutputResource, theRuleApplier, thePointcut);
} else {
return null;
}
}
List<IBaseResource> outputResources = AuthorizationInterceptor.toListOfResourcesAndExcludeContainer(
@Nullable
private static Verdict applyRulesToResponseBundle(
RequestDetails theRequestDetails,
IBaseResource theOutputResource,
IRuleApplier theRuleApplier,
Pointcut thePointcut) {
List<IBaseResource> outputResources =
AuthorizationInterceptor.toListOfResourcesAndExcludeContainerUnlessStandalone(
theOutputResource, theRequestDetails.getFhirContext());
return applyRulesToResponseResources(theRequestDetails, theRuleApplier, thePointcut, outputResources);
}
@Nullable
public static Verdict applyRulesToResponseResources(
RequestDetails theRequestDetails,
IRuleApplier theRuleApplier,
Pointcut thePointcut,
List<IBaseResource> outputResources) {
Verdict verdict = null;
for (IBaseResource nextResource : outputResources) {
if (nextResource == null) {
@ -848,9 +868,6 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ {
}
}
return verdict;
} else {
return null;
}
}
private boolean isInvalidNestedBundleRequest(BundleEntryParts theEntry) {

View File

@ -21,9 +21,9 @@ package ca.uhn.fhir.rest.server.mail;
import jakarta.annotation.Nonnull;
import org.simplejavamail.api.email.Email;
import org.simplejavamail.api.mailer.AsyncResponse;
import java.util.List;
import java.util.function.Consumer;
public interface IMailSvc {
void sendMail(@Nonnull List<Email> theEmails);
@ -31,7 +31,5 @@ public interface IMailSvc {
void sendMail(@Nonnull Email theEmail);
void sendMail(
@Nonnull Email theEmail,
@Nonnull Runnable theOnSuccess,
@Nonnull AsyncResponse.ExceptionConsumer theErrorHandler);
@Nonnull Email theEmail, @Nonnull Runnable theOnSuccess, @Nonnull Consumer<Throwable> theErrorHandler);
}

View File

@ -20,12 +20,9 @@
package ca.uhn.fhir.rest.server.mail;
import jakarta.annotation.Nonnull;
import org.apache.commons.lang3.Validate;
import org.simplejavamail.MailException;
import org.simplejavamail.api.email.Email;
import org.simplejavamail.api.email.Recipient;
import org.simplejavamail.api.mailer.AsyncResponse;
import org.simplejavamail.api.mailer.AsyncResponse.ExceptionConsumer;
import org.simplejavamail.api.mailer.Mailer;
import org.simplejavamail.api.mailer.config.TransportStrategy;
import org.simplejavamail.mailer.MailerBuilder;
@ -33,6 +30,8 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.util.List;
import java.util.Objects;
import java.util.function.Consumer;
import java.util.stream.Collectors;
public class MailSvc implements IMailSvc {
@ -42,14 +41,14 @@ public class MailSvc implements IMailSvc {
private final Mailer myMailer;
public MailSvc(@Nonnull MailConfig theMailConfig) {
Validate.notNull(theMailConfig);
Objects.requireNonNull(theMailConfig);
myMailConfig = theMailConfig;
myMailer = makeMailer(myMailConfig);
}
@Override
public void sendMail(@Nonnull List<Email> theEmails) {
Validate.notNull(theEmails);
Objects.requireNonNull(theEmails);
theEmails.forEach(theEmail -> send(theEmail, new OnSuccess(theEmail), new ErrorHandler(theEmail)));
}
@ -60,21 +59,23 @@ public class MailSvc implements IMailSvc {
@Override
public void sendMail(
@Nonnull Email theEmail, @Nonnull Runnable theOnSuccess, @Nonnull ExceptionConsumer theErrorHandler) {
@Nonnull Email theEmail, @Nonnull Runnable theOnSuccess, @Nonnull Consumer<Throwable> theErrorHandler) {
send(theEmail, theOnSuccess, theErrorHandler);
}
private void send(
@Nonnull Email theEmail, @Nonnull Runnable theOnSuccess, @Nonnull ExceptionConsumer theErrorHandler) {
Validate.notNull(theEmail);
Validate.notNull(theOnSuccess);
Validate.notNull(theErrorHandler);
@Nonnull Email theEmail, @Nonnull Runnable theOnSuccess, @Nonnull Consumer<Throwable> theErrorHandler) {
Objects.requireNonNull(theEmail);
Objects.requireNonNull(theOnSuccess);
Objects.requireNonNull(theErrorHandler);
try {
final AsyncResponse asyncResponse = myMailer.sendMail(theEmail, true);
if (asyncResponse != null) {
asyncResponse.onSuccess(theOnSuccess);
asyncResponse.onException(theErrorHandler);
myMailer.sendMail(theEmail, true).whenComplete((result, ex) -> {
if (ex != null) {
theErrorHandler.accept(ex);
} else {
theOnSuccess.run();
}
});
} catch (MailException e) {
theErrorHandler.accept(e);
}
@ -117,7 +118,7 @@ public class MailSvc implements IMailSvc {
}
}
private class ErrorHandler implements ExceptionConsumer {
private class ErrorHandler implements Consumer<Throwable> {
private final Email myEmail;
private ErrorHandler(@Nonnull Email theEmail) {
@ -125,7 +126,7 @@ public class MailSvc implements IMailSvc {
}
@Override
public void accept(Exception t) {
public void accept(Throwable t) {
ourLog.error("Email not sent" + makeMessage(myEmail), t);
}
}

View File

@ -4,6 +4,7 @@ import com.icegreen.greenmail.junit5.GreenMailExtension;
import com.icegreen.greenmail.util.GreenMailUtil;
import com.icegreen.greenmail.util.ServerSetupTest;
import jakarta.annotation.Nonnull;
import jakarta.mail.internet.MimeMessage;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
@ -11,7 +12,6 @@ import org.simplejavamail.MailException;
import org.simplejavamail.api.email.Email;
import org.simplejavamail.email.EmailBuilder;
import javax.mail.internet.MimeMessage;
import java.util.Arrays;
import java.util.List;
@ -86,13 +86,14 @@ public class MailSvcIT {
@Test
public void testSendMailWithInvalidToAddressExpectErrorHandler() {
// setup
final Email email = withEmail("xyz");
String invalidEmailAdress = "xyz";
final Email email = withEmail(invalidEmailAdress);
// execute
fixture.sendMail(email,
() -> fail("Should not execute on Success"),
(e) -> {
assertTrue(e instanceof MailException);
assertEquals("Invalid TO address: " + email, e.getMessage());
assertEquals("Invalid TO address: " + invalidEmailAdress, e.getMessage());
});
// validate
assertTrue(ourGreenMail.waitForIncomingEmail(1000, 0));

View File

@ -4230,7 +4230,7 @@ public class AuthorizationInterceptorR4Test extends BaseValidationTestWithInline
RequestDetails requestDetails = new SystemRequestDetails();
requestDetails.setResourceName("Bundle");
List<IBaseResource> resources = AuthorizationInterceptor.toListOfResourcesAndExcludeContainer(searchSet, ourCtx);
List<IBaseResource> resources = AuthorizationInterceptor.toListOfResourcesAndExcludeContainerUnlessStandalone(searchSet, ourCtx);
assertEquals(1, resources.size());
assertTrue(resources.contains(bundle));
}
@ -4247,7 +4247,7 @@ public class AuthorizationInterceptorR4Test extends BaseValidationTestWithInline
RequestDetails requestDetails = new SystemRequestDetails();
requestDetails.setResourceName("Patient");
List<IBaseResource> resources = AuthorizationInterceptor.toListOfResourcesAndExcludeContainer(searchSet, ourCtx);
List<IBaseResource> resources = AuthorizationInterceptor.toListOfResourcesAndExcludeContainerUnlessStandalone(searchSet, ourCtx);
assertEquals(2, resources.size());
assertTrue(resources.contains(patient1));
assertTrue(resources.contains(patient2));

34
pom.xml
View File

@ -869,6 +869,7 @@
<developer>
<id>delopst</id>
<name>Primož Delopst</name>
<organization>Better</organization>
</developer>
<developer>
<id>Zach Smith</id>
@ -1160,27 +1161,38 @@
<dependency>
<groupId>org.simplejavamail</groupId>
<artifactId>simple-java-mail</artifactId>
<version>6.6.1</version>
<version>8.11.2</version>
<exclusions>
<exclusion>
<groupId>com.sun.activation</groupId>
<artifactId>jakarta.activation-api</artifactId>
<groupId>com.github.bbottema</groupId>
<artifactId>jetbrains-runtime-annotations</artifactId>
</exclusion>
<exclusion>
<groupId>com.sun.activation</groupId>
<artifactId>jakarta.activation</artifactId>
<groupId>jakarta.mail</groupId>
<artifactId>jakarta.mail-api</artifactId>
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>jakarta.mail</groupId>
<artifactId>jakarta.mail-api</artifactId>
<version>2.1.3</version>
</dependency>
<dependency>
<groupId>com.icegreen</groupId>
<artifactId>greenmail</artifactId>
<version>2.1.0-rc-1</version>
<exclusions>
<exclusion>
<groupId>jakarta.mail</groupId>
<artifactId>jakarta.mail-api</artifactId>
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>com.icegreen</groupId>
<artifactId>greenmail</artifactId>
<version>1.6.4</version>
</dependency>
<dependency>
<groupId>com.icegreen</groupId>
<artifactId>greenmail-junit5</artifactId>
<version>1.6.4</version>
<version>2.1.0-rc-1</version>
<scope>compile</scope>
</dependency>
<!-- mail end -->