Skip to content

Commit 9b6b9f1

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

File tree

4 files changed

+101
-55
lines changed

4 files changed

+101
-55
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -454,7 +454,7 @@ private ServletContextHandler createApiServletContextHandler(String contextPath,
454454
apiServletContextHandler.setContextPath(contextPath + baseAPI + apiPath);
455455
apiServletContextHandler.addFilter(new FilterHolder(new ApiOriginFilter(mirthProperties)), "/*", EnumSet.of(DispatcherType.REQUEST));
456456
apiServletContextHandler.addFilter(new FilterHolder(new ClickjackingFilter(mirthProperties)), "/*", EnumSet.of(DispatcherType.REQUEST));
457-
apiServletContextHandler.addFilter(new FilterHolder(new RequestedWithFilter(mirthProperties)), "/*", EnumSet.of(DispatcherType.REQUEST));
457+
RequestedWithFilter.configure(mirthProperties);
458458
apiServletContextHandler.addFilter(new FilterHolder(new MethodFilter()), "/*", EnumSet.of(DispatcherType.REQUEST));
459459
apiServletContextHandler.addFilter(new FilterHolder(new StrictTransportSecurityFilter(mirthProperties)), "/*", EnumSet.of(DispatcherType.REQUEST));
460460
setConnectorNames(apiServletContextHandler, apiAllowHTTP);

‎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 & 32 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,67 @@
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;
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;
2226
import javax.ws.rs.ext.Provider;
2327

2428
import org.apache.commons.configuration2.PropertiesConfiguration;
2529
import org.apache.commons.lang3.StringUtils;
2630

31+
import com.mirth.connect.server.api.DontRequireRequestedWith;
32+
2733
@Provider
28-
public class RequestedWithFilter implements Filter {
34+
@Priority(Priorities.AUTHENTICATION + 100)
35+
public class RequestedWithFilter implements ContainerRequestFilter {
2936

30-
private boolean isRequestedWithHeaderRequired = true;
37+
@Context
38+
private ResourceInfo resourceInfo;
3139

40+
private static boolean isRequestedWithHeaderRequired = true;
3241

33-
public RequestedWithFilter(PropertiesConfiguration mirthProperties) {
34-
42+
// Jax requires a no-arg constructor to instantiate providers via classpath scanning.
43+
public RequestedWithFilter() {
44+
}
45+
46+
public static void configure(PropertiesConfiguration mirthProperties) {
3547
isRequestedWithHeaderRequired = mirthProperties.getBoolean("server.api.require-requested-with", true);
3648
}
3749

38-
@Override
39-
public void init(FilterConfig filterConfig) throws ServletException {}
50+
public static boolean isRequestedWithHeaderRequired() {
51+
return isRequestedWithHeaderRequired;
52+
}
4053

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

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

4874
//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");
75+
if (header == null || header.isEmpty() || StringUtils.isBlank(header.get(0))) {
76+
requestContext.abortWith(Response.status(400).entity("All requests must have 'X-Requested-With' header").build());
5177
}
52-
else {
53-
chain.doFilter(request, response);
54-
}
55-
5678
}
57-
58-
public boolean isRequestedWithHeaderRequired() {
59-
return isRequestedWithHeaderRequired;
60-
}
61-
62-
@Override
63-
public void destroy() {}
64-
}
79+
}

‎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)