Skip to content

Commit dbdadd3

Browse files
mgaffiganmgaffigan
committed
Refactor to allow CSRF bypass
Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net>
1 parent a063f75 commit dbdadd3

File tree

5 files changed

+105
-57
lines changed

5 files changed

+105
-57
lines changed

‎server/src/com/mirth/connect/server/MirthWebServer.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@
101101
import com.mirth.connect.client.core.api.Replaces;
102102
import com.mirth.connect.model.ApiProvider;
103103
import com.mirth.connect.model.MetaData;
104+
import com.mirth.connect.server.api.DontRequireRequestedWith;
104105
import com.mirth.connect.server.api.MirthServlet;
105106
import com.mirth.connect.server.api.providers.ApiOriginFilter;
106107
import com.mirth.connect.server.api.providers.ClickjackingFilter;
@@ -454,7 +455,7 @@ private ServletContextHandler createApiServletContextHandler(String contextPath,
454455
apiServletContextHandler.setContextPath(contextPath + baseAPI + apiPath);
455456
apiServletContextHandler.addFilter(new FilterHolder(new ApiOriginFilter(mirthProperties)), "/*", EnumSet.of(DispatcherType.REQUEST));
456457
apiServletContextHandler.addFilter(new FilterHolder(new ClickjackingFilter(mirthProperties)), "/*", EnumSet.of(DispatcherType.REQUEST));
457-
apiServletContextHandler.addFilter(new FilterHolder(new RequestedWithFilter(mirthProperties)), "/*", EnumSet.of(DispatcherType.REQUEST));
458+
RequestedWithFilter.configure(mirthProperties);
458459
apiServletContextHandler.addFilter(new FilterHolder(new MethodFilter()), "/*", EnumSet.of(DispatcherType.REQUEST));
459460
apiServletContextHandler.addFilter(new FilterHolder(new StrictTransportSecurityFilter(mirthProperties)), "/*", EnumSet.of(DispatcherType.REQUEST));
460461
setConnectorNames(apiServletContextHandler, apiAllowHTTP);
@@ -608,6 +609,7 @@ private ApiProviders getApiProviders(Version version) {
608609
providerClasses.addAll(serverProviderClasses);
609610
providerClasses.add(OpenApiResource.class);
610611
providerClasses.add(AcceptHeaderOpenApiResource.class);
612+
providerClasses.add(DontRequireRequestedWith.class);
611613

612614
return new ApiProviders(servletInterfacePackages, servletInterfaces, providerPackages, providerClasses);
613615
}

‎server/src/com/mirth/connect/server/api/DontRequireRequestedWith.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// SPDX-License-Identifier: MPL-2.0
2+
// SPDX-FileCopyrightText: 2025 Mitch Gaffigan <mitch.gaffigan@comcast.net>
3+
4+
package com.mirth.connect.server.api;
5+
6+
import java.lang.annotation.ElementType;
7+
import java.lang.annotation.Retention;
8+
import java.lang.annotation.RetentionPolicy;
9+
import java.lang.annotation.Target;
10+
11+
/**
12+
* If this annotation is present on a method or class, the X-Requested-With header
13+
* requirement will not be enforced for that resource.
14+
*/
15+
@Target({ElementType.METHOD, ElementType.TYPE})
16+
@Retention(RetentionPolicy.RUNTIME)
17+
public @interface DontRequireRequestedWith {
18+
}

‎server/src/com/mirth/connect/server/api/providers/RequestedWithFilter.java‎

Lines changed: 47 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
// SPDX-License-Identifier: MPL-2.0
2+
// SPDX-FileCopyrightText: Mirth Corporation
3+
// SPDX-FileCopyrightText: 2025 Mitch Gaffigan <mitch.gaffigan@comcast.net>
14
/*
25
* Copyright (c) Mirth Corporation. All rights reserved.
36
*
@@ -10,55 +13,65 @@
1013
package com.mirth.connect.server.api.providers;
1114

1215
import java.io.IOException;
16+
import java.lang.reflect.Method;
17+
import java.util.List;
1318

14-
import javax.servlet.Filter;
15-
import javax.servlet.FilterChain;
16-
import javax.servlet.FilterConfig;
17-
import javax.servlet.ServletException;
18-
import javax.servlet.ServletRequest;
19-
import javax.servlet.ServletResponse;
20-
import javax.servlet.http.HttpServletRequest;
21-
import javax.servlet.http.HttpServletResponse;
22-
import javax.ws.rs.ext.Provider;
19+
import javax.annotation.Priority;
20+
import javax.ws.rs.Priorities;
21+
import javax.ws.rs.container.ContainerRequestContext;
22+
import javax.ws.rs.container.ContainerRequestFilter;
23+
import javax.ws.rs.container.ResourceInfo;
24+
import javax.ws.rs.core.Context;
25+
import javax.ws.rs.core.Response;
2326

2427
import org.apache.commons.configuration2.PropertiesConfiguration;
2528
import org.apache.commons.lang3.StringUtils;
2629

27-
@Provider
28-
public class RequestedWithFilter implements Filter {
30+
import com.mirth.connect.server.api.DontRequireRequestedWith;
2931

30-
private boolean isRequestedWithHeaderRequired = true;
32+
@Priority(Priorities.AUTHENTICATION + 100)
33+
public class RequestedWithFilter implements ContainerRequestFilter {
3134

35+
@Context
36+
private ResourceInfo resourceInfo;
3237

33-
public RequestedWithFilter(PropertiesConfiguration mirthProperties) {
34-
38+
private static boolean isRequestedWithHeaderRequired = true;
39+
40+
// Jax requires a no-arg constructor to instantiate providers via classpath scanning.
41+
public RequestedWithFilter() {
42+
}
43+
44+
public static void configure(PropertiesConfiguration mirthProperties) {
3545
isRequestedWithHeaderRequired = mirthProperties.getBoolean("server.api.require-requested-with", true);
3646
}
3747

38-
@Override
39-
public void init(FilterConfig filterConfig) throws ServletException {}
48+
public static boolean isRequestedWithHeaderRequired() {
49+
return isRequestedWithHeaderRequired;
50+
}
4051

4152
@Override
42-
public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException {
43-
HttpServletResponse res = (HttpServletResponse) response;
53+
public void filter(ContainerRequestContext requestContext) throws IOException {
54+
if (!isRequestedWithHeaderRequired) {
55+
return;
56+
}
57+
58+
// If the resource method or class is annotated with DontRequireRequestedWith, skip the check
59+
if (resourceInfo != null) {
60+
Method method = resourceInfo.getResourceMethod();
61+
if (method != null && method.getAnnotation(DontRequireRequestedWith.class) != null) {
62+
return;
63+
}
64+
Class<?> resourceClass = resourceInfo.getResourceClass();
65+
if (resourceClass != null && resourceClass.getAnnotation(DontRequireRequestedWith.class) != null) {
66+
return;
67+
}
68+
}
4469

45-
HttpServletRequest servletRequest = (HttpServletRequest)request;
46-
String requestedWithHeader = (String) servletRequest.getHeader("X-Requested-With");
70+
List<String> header = requestContext.getHeaders().get("X-Requested-With");
4771

4872
//if header is required and not present, send an error
49-
if(isRequestedWithHeaderRequired && StringUtils.isBlank(requestedWithHeader)) {
50-
res.sendError(400, "All requests must have 'X-Requested-With' header");
73+
if (header == null || header.isEmpty() || StringUtils.isBlank(header.get(0))) {
74+
requestContext.abortWith(Response.status(400).entity("All requests must have 'X-Requested-With' header").build());
5175
}
52-
else {
53-
chain.doFilter(request, response);
54-
}
55-
5676
}
57-
58-
public boolean isRequestedWithHeaderRequired() {
59-
return isRequestedWithHeaderRequired;
60-
}
61-
62-
@Override
63-
public void destroy() {}
64-
}
77+
}

‎server/src/com/mirth/connect/server/api/servlets/UserServlet.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import com.mirth.connect.model.User;
3737
import com.mirth.connect.server.api.CheckAuthorizedUserId;
3838
import com.mirth.connect.server.api.DontCheckAuthorized;
39+
import com.mirth.connect.server.api.DontRequireRequestedWith;
3940
import com.mirth.connect.server.api.MirthServlet;
4041
import com.mirth.connect.server.controllers.ConfigurationController;
4142
import com.mirth.connect.server.controllers.ControllerFactory;
@@ -153,6 +154,7 @@ public LoginStatus login(String username, String password) {
153154

154155
@Override
155156
@DontCheckAuthorized
157+
@DontRequireRequestedWith
156158
public void logout() {
157159
// Audit the logout request but don't block it
158160
isUserAuthorized();

‎server/test/com/mirth/connect/server/api/providers/RequestedWithFilterTest.java‎

Lines changed: 35 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,19 @@
1+
// SPDX-License-Identifier: MPL-2.0
2+
// SPDX-FileCopyrightText: Mirth Corporation
3+
// SPDX-FileCopyrightText: 2025 Mitch Gaffigan <mitch.gaffigan@comcast.net>
14
package com.mirth.connect.server.api.providers;
25

36
import static org.mockito.Mockito.never;
47
import static org.mockito.Mockito.verify;
8+
import static org.mockito.Mockito.when;
59

6-
import javax.servlet.FilterChain;
7-
import javax.servlet.http.HttpServletRequest;
8-
import javax.servlet.http.HttpServletResponse;
10+
import javax.ws.rs.container.ContainerRequestContext;
11+
import javax.ws.rs.core.Response;
912

1013
import org.apache.commons.configuration2.PropertiesConfiguration;
1114
import org.junit.Test;
15+
import org.mockito.ArgumentCaptor;
16+
import org.mockito.ArgumentMatchers;
1217
import org.mockito.Mockito;
1318

1419
import com.mirth.connect.client.core.PropertiesConfigurationUtil;
@@ -22,26 +27,34 @@ public class RequestedWithFilterTest extends TestCase {
2227
@Test
2328
//assert that if property is set to false, isRequestedWithHeaderRequired = false
2429
public void testConstructor() {
25-
30+
mirthProperties.clearProperty("server.api.require-requested-with");
31+
RequestedWithFilter.configure(mirthProperties);
32+
assertEquals(true, RequestedWithFilter.isRequestedWithHeaderRequired());
33+
2634
mirthProperties.setProperty("server.api.require-requested-with", "false");
27-
RequestedWithFilter requestedWithFilter = new RequestedWithFilter(mirthProperties);
28-
assertEquals(requestedWithFilter.isRequestedWithHeaderRequired(), false);
35+
RequestedWithFilter.configure(mirthProperties);
36+
assertEquals(false, RequestedWithFilter.isRequestedWithHeaderRequired());
2937
}
3038

3139
@Test
3240
//assert that HttpServletResponse.sendError() is called when X-Requested-With is required but not present
3341
public void testDoFilterRequestedWithTrue() {
3442

3543
mirthProperties.setProperty("server.api.require-requested-with", "true");
36-
RequestedWithFilter testFilter = new RequestedWithFilter(mirthProperties);
37-
38-
HttpServletRequest mockReq = Mockito.mock(HttpServletRequest.class);
39-
HttpServletResponse mockResp = Mockito.mock(HttpServletResponse.class);
40-
FilterChain mockFilterChain = Mockito.mock(FilterChain.class);
41-
44+
RequestedWithFilter.configure(mirthProperties);
45+
46+
ContainerRequestContext mockCtx = Mockito.mock(ContainerRequestContext.class);
47+
when(mockCtx.getHeaders()).thenReturn(new javax.ws.rs.core.MultivaluedHashMap<String, String>());
48+
4249
try {
43-
testFilter.doFilter(mockReq, mockResp, mockFilterChain);
44-
verify(mockResp).sendError(HttpServletResponse.SC_BAD_REQUEST, "All requests must have 'X-Requested-With' header");
50+
RequestedWithFilter filter = new RequestedWithFilter();
51+
filter.filter(mockCtx);
52+
ArgumentCaptor<Response> responseCaptor =
53+
ArgumentCaptor.forClass(Response.class);
54+
verify(mockCtx).abortWith(responseCaptor.capture());
55+
Response response = responseCaptor.getValue();
56+
assertEquals(400, response.getStatus());
57+
assertEquals("All requests must have 'X-Requested-With' header", response.getEntity());
4558
} catch (Exception e) {
4659
e.printStackTrace();
4760
}
@@ -52,15 +65,15 @@ public void testDoFilterRequestedWithTrue() {
5265
public void testDoFilterRequestedWithFalse() {
5366

5467
mirthProperties.setProperty("server.api.require-requested-with", "false");
55-
RequestedWithFilter testFilter = new RequestedWithFilter(mirthProperties);
56-
57-
HttpServletRequest mockReq = Mockito.mock(HttpServletRequest.class);
58-
HttpServletResponse mockResp = Mockito.mock(HttpServletResponse.class);
59-
FilterChain mockFilterChain = Mockito.mock(FilterChain.class);
60-
68+
RequestedWithFilter.configure(mirthProperties);
69+
70+
ContainerRequestContext mockCtx = Mockito.mock(ContainerRequestContext.class);
71+
when(mockCtx.getHeaders()).thenReturn(new javax.ws.rs.core.MultivaluedHashMap<String, String>());
72+
6173
try {
62-
testFilter.doFilter(mockReq, mockResp, mockFilterChain);
63-
verify(mockResp, never()).sendError(HttpServletResponse.SC_BAD_REQUEST, "All requests must have 'X-Requested-With' header");
74+
RequestedWithFilter filter = new RequestedWithFilter();
75+
filter.filter(mockCtx);
76+
verify(mockCtx, never()).abortWith(ArgumentMatchers.any(javax.ws.rs.core.Response.class));
6477
} catch (Exception e) {
6578
e.printStackTrace();
6679
}

0 commit comments

Comments
 (0)