Skip to content

Commit b592c25

Browse files
mrdnctrkmrdnctrk
authored
enforce fhirServer is https for cdsHooks version 2 (#6806)
1 parent 5b639ee commit b592c25

File tree

5 files changed

+161
-2
lines changed

5 files changed

+161
-2
lines changed

‎hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/8_2_0/6805-cds-hooks-v2-enforce-http-for-fhirServer.yaml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
type: add
3+
issue: 6805
4+
title: "CDS Hooks now requires the `fhirServer` field to use HTTPS in requests when using
5+
[CDS Hooks version 2.0](https://cds-hooks.hl7.org/2.0/), as per
6+
the specification. You can configure the CDS Hooks version by providing a bean that returns a CDSHooksVersion enum,
7+
introduced in this update. By default, the version is set to 1.1 to maintain compatibility with existing
8+
implementations."

‎hapi-fhir-server-cds-hooks/src/main/java/ca/uhn/hapi/fhir/cdshooks/api/CDSHooksVersion.java‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/*-
2+
* #%L
3+
* HAPI FHIR - CDS Hooks
4+
* %%
5+
* Copyright (C) 2014 - 2025 Smile CDR, Inc.
6+
* %%
7+
* Licensed under the Apache License, Version 2.0 (the "License");
8+
* you may not use this file except in compliance with the License.
9+
* You may obtain a copy of the License at
10+
*
11+
* http://www.apache.org/licenses/LICENSE-2.0
12+
*
13+
* Unless required by applicable law or agreed to in writing, software
14+
* distributed under the License is distributed on an "AS IS" BASIS,
15+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
16+
* See the License for the specific language governing permissions and
17+
* limitations under the License.
18+
* #L%
19+
*/
20+
package ca.uhn.hapi.fhir.cdshooks.api;
21+
22+
import jakarta.annotation.Nullable;
23+
24+
public enum CDSHooksVersion {
25+
V_1_1,
26+
V_2_0;
27+
28+
/**
29+
* Using V_1_1 as the default for not breaking existing implementations
30+
*/
31+
public static final CDSHooksVersion DEFAULT = V_1_1;
32+
33+
public static CDSHooksVersion getOrDefault(@Nullable CDSHooksVersion theVersion) {
34+
if (theVersion == null) {
35+
return DEFAULT;
36+
}
37+
return theVersion;
38+
}
39+
}

‎hapi-fhir-server-cds-hooks/src/main/java/ca/uhn/hapi/fhir/cdshooks/config/CdsHooksConfig.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
2929
import ca.uhn.fhir.rest.api.server.RequestDetails;
3030
import ca.uhn.fhir.rest.server.RestfulServer;
31+
import ca.uhn.hapi.fhir.cdshooks.api.CDSHooksVersion;
3132
import ca.uhn.hapi.fhir.cdshooks.api.ICdsConfigService;
3233
import ca.uhn.hapi.fhir.cdshooks.api.ICdsHooksDaoAuthorizationSvc;
3334
import ca.uhn.hapi.fhir.cdshooks.api.ICdsServiceRegistry;
@@ -102,7 +103,8 @@ public ICdsServiceRegistry cdsServiceRegistry(
102103
@Qualifier(CDS_HOOKS_OBJECT_MAPPER_FACTORY) ObjectMapper theObjectMapper,
103104
ICdsCrServiceFactory theCdsCrServiceFactory,
104105
ICrDiscoveryServiceFactory theCrDiscoveryServiceFactory,
105-
FhirContext theFhirContext) {
106+
FhirContext theFhirContext,
107+
@Nullable CDSHooksVersion theCDSHooksVersion) {
106108
final CdsServiceRequestJsonDeserializer cdsServiceRequestJsonDeserializer =
107109
new CdsServiceRequestJsonDeserializer(theFhirContext, theObjectMapper);
108110
return new CdsServiceRegistryImpl(
@@ -111,7 +113,8 @@ public ICdsServiceRegistry cdsServiceRegistry(
111113
theObjectMapper,
112114
theCdsCrServiceFactory,
113115
theCrDiscoveryServiceFactory,
114-
cdsServiceRequestJsonDeserializer);
116+
cdsServiceRequestJsonDeserializer,
117+
CDSHooksVersion.getOrDefault(theCDSHooksVersion));
115118
}
116119

117120
@Bean

‎hapi-fhir-server-cds-hooks/src/main/java/ca/uhn/hapi/fhir/cdshooks/svc/CdsServiceRegistryImpl.java‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,9 @@
2323
import ca.uhn.fhir.i18n.Msg;
2424
import ca.uhn.fhir.rest.api.server.cdshooks.CdsHooksExtension;
2525
import ca.uhn.fhir.rest.api.server.cdshooks.CdsServiceRequestJson;
26+
import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
2627
import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException;
28+
import ca.uhn.hapi.fhir.cdshooks.api.CDSHooksVersion;
2729
import ca.uhn.hapi.fhir.cdshooks.api.ICdsMethod;
2830
import ca.uhn.hapi.fhir.cdshooks.api.ICdsServiceMethod;
2931
import ca.uhn.hapi.fhir.cdshooks.api.ICdsServiceRegistry;
@@ -56,6 +58,7 @@ public class CdsServiceRegistryImpl implements ICdsServiceRegistry {
5658
private final ObjectMapper myObjectMapper;
5759
private final ICdsCrServiceFactory myCdsCrServiceFactory;
5860
private final ICrDiscoveryServiceFactory myCrDiscoveryServiceFactory;
61+
private final CDSHooksVersion myCdsHooksVersion;
5962

6063
public CdsServiceRegistryImpl(
6164
CdsHooksContextBooter theCdsHooksContextBooter,
@@ -64,12 +67,31 @@ public CdsServiceRegistryImpl(
6467
ICdsCrServiceFactory theCdsCrServiceFactory,
6568
ICrDiscoveryServiceFactory theCrDiscoveryServiceFactory,
6669
CdsServiceRequestJsonDeserializer theCdsServiceRequestJsonDeserializer) {
70+
this(
71+
theCdsHooksContextBooter,
72+
theCdsPrefetchSvc,
73+
theObjectMapper,
74+
theCdsCrServiceFactory,
75+
theCrDiscoveryServiceFactory,
76+
theCdsServiceRequestJsonDeserializer,
77+
CDSHooksVersion.DEFAULT);
78+
}
79+
80+
public CdsServiceRegistryImpl(
81+
CdsHooksContextBooter theCdsHooksContextBooter,
82+
CdsPrefetchSvc theCdsPrefetchSvc,
83+
ObjectMapper theObjectMapper,
84+
ICdsCrServiceFactory theCdsCrServiceFactory,
85+
ICrDiscoveryServiceFactory theCrDiscoveryServiceFactory,
86+
CdsServiceRequestJsonDeserializer theCdsServiceRequestJsonDeserializer,
87+
CDSHooksVersion theCDSHooksVersion) {
6788
myCdsHooksContextBooter = theCdsHooksContextBooter;
6889
myCdsPrefetchSvc = theCdsPrefetchSvc;
6990
myObjectMapper = theObjectMapper;
7091
myCdsCrServiceFactory = theCdsCrServiceFactory;
7192
myCrDiscoveryServiceFactory = theCrDiscoveryServiceFactory;
7293
myCdsServiceRequestJsonDeserializer = theCdsServiceRequestJsonDeserializer;
94+
myCdsHooksVersion = theCDSHooksVersion;
7395
}
7496

7597
@PostConstruct
@@ -87,6 +109,7 @@ public CdsServiceResponseJson callService(String theServiceId, Object theCdsServ
87109
final CdsServiceJson cdsServiceJson = getCdsServiceJson(theServiceId);
88110
final CdsServiceRequestJson deserializedRequest =
89111
myCdsServiceRequestJsonDeserializer.deserialize(cdsServiceJson, theCdsServiceRequestJson);
112+
validateHookRequestFhirServer(deserializedRequest);
90113
ICdsServiceMethod serviceMethod = (ICdsServiceMethod) getCdsServiceMethodOrThrowException(theServiceId);
91114
myCdsPrefetchSvc.augmentRequest(deserializedRequest, serviceMethod);
92115
Object response = serviceMethod.invoke(myObjectMapper, deserializedRequest, theServiceId);
@@ -245,4 +268,15 @@ private CdsServiceFeedbackJson buildFeedbackFromString(String theServiceId, Stri
245268
void setServiceCache(CdsServiceCache theCdsServiceCache) {
246269
myServiceCache = theCdsServiceCache;
247270
}
271+
272+
private void validateHookRequestFhirServer(CdsServiceRequestJson theCdsServiceRequestJson) {
273+
if (myCdsHooksVersion != CDSHooksVersion.V_1_1) {
274+
// for a version greater than V_1_1 (which is the base version supported),
275+
// the fhirServer is required to use https scheme
276+
String fhirServer = theCdsServiceRequestJson.getFhirServer();
277+
if (fhirServer != null && !fhirServer.startsWith("https")) {
278+
throw new InvalidRequestException(Msg.code(2632) + "The scheme for the fhirServer must be https");
279+
}
280+
}
281+
}
248282
}

‎hapi-fhir-server-cds-hooks/src/test/java/ca/uhn/hapi/fhir/cdshooks/svc/CdsServiceRegistryImplTest.java‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
package ca.uhn.hapi.fhir.cdshooks.svc;
22

33
import ca.uhn.fhir.context.ConfigurationException;
4+
import ca.uhn.fhir.rest.api.server.cdshooks.CdsServiceRequestJson;
5+
import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
6+
import ca.uhn.hapi.fhir.cdshooks.api.CDSHooksVersion;
7+
import ca.uhn.hapi.fhir.cdshooks.api.ICdsServiceMethod;
48
import ca.uhn.hapi.fhir.cdshooks.api.json.CdsServiceFeedbackJson;
59
import ca.uhn.hapi.fhir.cdshooks.api.json.CdsServiceJson;
610
import ca.uhn.hapi.fhir.cdshooks.api.json.CdsServiceResponseJson;
@@ -10,6 +14,7 @@
1014
import ca.uhn.hapi.fhir.cdshooks.svc.prefetch.CdsPrefetchSvc;
1115
import com.fasterxml.jackson.core.JsonProcessingException;
1216
import com.fasterxml.jackson.databind.ObjectMapper;
17+
import jakarta.annotation.Nullable;
1318
import org.junit.jupiter.api.BeforeEach;
1419
import org.junit.jupiter.api.Test;
1520
import org.junit.jupiter.api.extension.ExtendWith;
@@ -18,7 +23,13 @@
1823

1924
import static org.assertj.core.api.Assertions.assertThat;
2025
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
26+
import static org.junit.jupiter.api.Assertions.assertThrows;
27+
import static org.mockito.ArgumentMatchers.any;
28+
import static org.mockito.ArgumentMatchers.anyString;
29+
import static org.mockito.ArgumentMatchers.eq;
2130
import static org.mockito.Mockito.doReturn;
31+
import static org.mockito.Mockito.mock;
32+
import static org.mockito.Mockito.when;
2233

2334
@ExtendWith(MockitoExtension.class)
2435
class CdsServiceRegistryImplTest {
@@ -177,4 +188,68 @@ void getCdsServiceJsonWhenServiceIsNotPresent() {
177188
myFixture.getCdsServiceJson(serviceId);
178189
}).withMessage("HAPI-2536: No service with " + serviceId + " is registered.");
179190
}
191+
192+
@Test
193+
void testCallService_doesNotEnforceFhirServerIsHttpsForCDSHooksV1() {
194+
String fhirServer = "http://localhost:8000/remote_fhir";
195+
String response = "{\"cards\": []}";
196+
CdsServiceRegistryImpl serviceRegistryImpl = createAndSetupCdsServiceRegistryImplForCallService(CDSHooksVersion.V_1_1, fhirServer, response);
197+
CdsServiceResponseJson responseJson = serviceRegistryImpl.callService(SERVICE_ID, "");
198+
assertThat(responseJson).isNotNull();
199+
}
200+
201+
@Test
202+
void testCallService_noValidationIfFhirServerIsNullForCDSHooksV2() {
203+
String response = "{\"cards\": []}";
204+
CdsServiceRegistryImpl serviceRegistryImpl = createAndSetupCdsServiceRegistryImplForCallService(CDSHooksVersion.V_2_0, null, response);
205+
CdsServiceResponseJson responseJson = serviceRegistryImpl.callService(SERVICE_ID, "");
206+
assertThat(responseJson).isNotNull();
207+
}
208+
209+
@Test
210+
void testCallService_enforcesFhirServerIsHttpsForCDSHooksV2_InvalidInput() {
211+
String fhirServer = "http://localhost:8000/remote_fhir";
212+
CdsServiceRegistryImpl serviceRegistryImpl = createAndSetupCdsServiceRegistryImplForCallService(CDSHooksVersion.V_2_0, fhirServer, null);
213+
InvalidRequestException ex = assertThrows(InvalidRequestException.class, () -> serviceRegistryImpl.callService(SERVICE_ID, ""));
214+
assertThat(ex.getMessage()).isEqualTo("HAPI-2632: The scheme for the fhirServer must be https");
215+
}
216+
217+
@Test
218+
void testCallService_enforcesFhirServerIsHttpsForCDSHooksV2_ValidInput() {
219+
String fhirServer = "https://localhost:8000/remote_fhir";
220+
String response = "{\"cards\": []}";
221+
CdsServiceRegistryImpl serviceRegistryImpl = createAndSetupCdsServiceRegistryImplForCallService(CDSHooksVersion.V_2_0, fhirServer, response);
222+
CdsServiceResponseJson responseJson = serviceRegistryImpl.callService(SERVICE_ID, "");
223+
assertThat(responseJson).isNotNull();
224+
}
225+
226+
private CdsServiceRegistryImpl createAndSetupCdsServiceRegistryImplForCallService(CDSHooksVersion theCdsHooksVersion, @Nullable String theFhirServer, @Nullable String theSuccessResponse) {
227+
CdsServiceRegistryImpl serviceRegistryImpl = new CdsServiceRegistryImpl(
228+
myCdsHooksContextBooter,
229+
myCdsPrefetchSvc,
230+
myObjectMapper,
231+
myCdsCrServiceFactory,
232+
myCrDiscoveryServiceFactory,
233+
myCdsServiceRequestJsonDeserializer,
234+
theCdsHooksVersion);
235+
236+
final CdsServiceJson cdsService = new CdsServiceJson();
237+
cdsService.setId(SERVICE_ID);
238+
serviceRegistryImpl.setServiceCache(myCdsServiceCache);
239+
doReturn(cdsService).when(myCdsServiceCache).getCdsServiceJson(SERVICE_ID);
240+
241+
CdsServiceRequestJson requestJson = new CdsServiceRequestJson();
242+
requestJson.setFhirServer(theFhirServer);
243+
doReturn(requestJson).when(myCdsServiceRequestJsonDeserializer).deserialize(eq(cdsService), anyString());
244+
245+
if (theSuccessResponse != null) {
246+
ICdsServiceMethod theMethodMock= mock(ICdsServiceMethod.class);
247+
when(theMethodMock.invoke(myObjectMapper, requestJson, SERVICE_ID)).thenReturn(theSuccessResponse);
248+
when(myCdsServiceCache.getServiceMethod(SERVICE_ID)).thenReturn(theMethodMock);
249+
}
250+
251+
252+
return serviceRegistryImpl;
253+
}
254+
180255
}

0 commit comments

Comments
 (0)