Skip to content

Refactor to allow CSRF bypass, improve error logging for API authoring #176

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mgaffigan wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Refactor to allow CSRF bypass, improve error logging for API authoring #176

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8c5d5a7
Improve error messsages for a few sharp edges
mgaffigan Sep 3, 2025
d387729
Refactor to allow CSRF bypass
mgaffigan Sep 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions server/src/com/mirth/connect/client/core/Client.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ public Connector getConnector(javax.ws.rs.client.Client client, Configuration ru
try {
config.register(Class.forName(apiProviderClass));
} catch (Throwable t) {
logger.error("Error registering API provider class: " + apiProviderClass);
logger.error("Error registering API provider class: {}", apiProviderClass, t);
}
}
}
Expand All @@ -219,7 +219,7 @@ public void registerApiProviders(Set<String> packageNames, Set<String> classes)
client.register(clazz);
}
} catch (Throwable t) {
logger.error("Error registering API provider package: " + packageName);
logger.error("Error registering API provider package: {}", packageName, t);
}
}
}
Expand All @@ -229,7 +229,7 @@ public void registerApiProviders(Set<String> packageNames, Set<String> classes)
try {
client.register(Class.forName(clazz));
} catch (Throwable t) {
logger.error("Error registering API provider class: " + clazz);
logger.error("Error registering API provider class: {}", clazz, t);
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion server/src/com/mirth/connect/server/MirthWebServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,7 @@ private ServletContextHandler createApiServletContextHandler(String contextPath,
apiServletContextHandler.setContextPath(contextPath + baseAPI + apiPath);
apiServletContextHandler.addFilter(new FilterHolder(new ApiOriginFilter(mirthProperties)), "/*", EnumSet.of(DispatcherType.REQUEST));
apiServletContextHandler.addFilter(new FilterHolder(new ClickjackingFilter(mirthProperties)), "/*", EnumSet.of(DispatcherType.REQUEST));
apiServletContextHandler.addFilter(new FilterHolder(new RequestedWithFilter(mirthProperties)), "/*", EnumSet.of(DispatcherType.REQUEST));
RequestedWithFilter.configure(mirthProperties);
apiServletContextHandler.addFilter(new FilterHolder(new MethodFilter()), "/*", EnumSet.of(DispatcherType.REQUEST));
apiServletContextHandler.addFilter(new FilterHolder(new StrictTransportSecurityFilter(mirthProperties)), "/*", EnumSet.of(DispatcherType.REQUEST));
setConnectorNames(apiServletContextHandler, apiAllowHTTP);
Expand Down Expand Up @@ -608,6 +608,7 @@ private ApiProviders getApiProviders(Version version) {
providerClasses.addAll(serverProviderClasses);
providerClasses.add(OpenApiResource.class);
providerClasses.add(AcceptHeaderOpenApiResource.class);
providerClasses.add(RequestedWithFilter.class);

return new ApiProviders(servletInterfacePackages, servletInterfaces, providerPackages, providerClasses);
}
Expand Down
18 changes: 18 additions & 0 deletions server/src/com/mirth/connect/server/api/DontRequireRequestedWith.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// SPDX-License-Identifier: MPL-2.0
// SPDX-FileCopyrightText: 2025 Mitch Gaffigan <mitch.gaffigan@comcast.net>

package com.mirth.connect.server.api;

import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

/**
* If this annotation is present on a method or class, the X-Requested-With header
* requirement will not be enforced for that resource.
*/
@Target({ElementType.METHOD, ElementType.TYPE})
@Retention(RetentionPolicy.RUNTIME)
public @interface DontRequireRequestedWith {
}
3 changes: 3 additions & 0 deletions server/src/com/mirth/connect/server/api/MirthServlet.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,9 @@ private void setContext() {
}

public void setOperation(Operation operation) {
if (operation == null) {
throw new MirthApiException("Method operation cannot be null.");
}
if (extensionName != null) {
operation = new ExtensionOperation(extensionName, operation);
}
Expand Down
83 changes: 48 additions & 35 deletions server/src/com/mirth/connect/server/api/providers/RequestedWithFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// SPDX-License-Identifier: MPL-2.0
// SPDX-FileCopyrightText: Mirth Corporation
// SPDX-FileCopyrightText: 2025 Mitch Gaffigan <mitch.gaffigan@comcast.net>
/*
* Copyright (c) Mirth Corporation. All rights reserved.
*
Expand All @@ -10,55 +13,65 @@
package com.mirth.connect.server.api.providers;

import java.io.IOException;
import java.lang.reflect.Method;
import java.util.List;

import javax.servlet.Filter;
import javax.servlet.FilterChain;
import javax.servlet.FilterConfig;
import javax.servlet.ServletException;
import javax.servlet.ServletRequest;
import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import javax.ws.rs.ext.Provider;
import javax.annotation.Priority;
import javax.ws.rs.Priorities;
import javax.ws.rs.container.ContainerRequestContext;
import javax.ws.rs.container.ContainerRequestFilter;
import javax.ws.rs.container.ResourceInfo;
import javax.ws.rs.core.Context;
import javax.ws.rs.core.Response;

import org.apache.commons.configuration2.PropertiesConfiguration;
import org.apache.commons.lang3.StringUtils;

@Provider
public class RequestedWithFilter implements Filter {
import com.mirth.connect.server.api.DontRequireRequestedWith;

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

@Context
private ResourceInfo resourceInfo;

public RequestedWithFilter(PropertiesConfiguration mirthProperties) {

private static boolean isRequestedWithHeaderRequired = true;

// Jax requires a no-arg constructor to instantiate providers via classpath scanning.
public RequestedWithFilter() {
}

public static void configure(PropertiesConfiguration mirthProperties) {
isRequestedWithHeaderRequired = mirthProperties.getBoolean("server.api.require-requested-with", true);
}

@Override
public void init(FilterConfig filterConfig) throws ServletException {}
public static boolean isRequestedWithHeaderRequired() {
return isRequestedWithHeaderRequired;
}

@Override
public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException {
HttpServletResponse res = (HttpServletResponse) response;

HttpServletRequest servletRequest = (HttpServletRequest)request;
String requestedWithHeader = (String) servletRequest.getHeader("X-Requested-With");

//if header is required and not present, send an error
if(isRequestedWithHeaderRequired && StringUtils.isBlank(requestedWithHeader)) {
res.sendError(400, "All requests must have 'X-Requested-With' header");
public void filter(ContainerRequestContext requestContext) throws IOException {
if (!isRequestedWithHeaderRequired) {
return;
}
else {
chain.doFilter(request, response);

// If the resource method or class is annotated with DontRequireRequestedWith, skip the check
if (resourceInfo != null) {
Method method = resourceInfo.getResourceMethod();
if (method != null && method.getAnnotation(DontRequireRequestedWith.class) != null) {
return;
}
Class<?> resourceClass = resourceInfo.getResourceClass();
if (resourceClass != null && resourceClass.getAnnotation(DontRequireRequestedWith.class) != null) {
return;
}
}

List<String> header = requestContext.getHeaders().get("X-Requested-With");

// if header is required and not present, send an error
if (header == null || header.isEmpty() || StringUtils.isBlank(header.get(0))) {
requestContext.abortWith(Response.status(400).entity("All requests must have 'X-Requested-With' header").build());
}
}

public boolean isRequestedWithHeaderRequired() {
return isRequestedWithHeaderRequired;
}

@Override
public void destroy() {}
}
}
57 changes: 35 additions & 22 deletions server/test/com/mirth/connect/server/api/providers/RequestedWithFilterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,19 @@
// SPDX-License-Identifier: MPL-2.0
// SPDX-FileCopyrightText: Mirth Corporation
// SPDX-FileCopyrightText: 2025 Mitch Gaffigan <mitch.gaffigan@comcast.net>
package com.mirth.connect.server.api.providers;

import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import javax.servlet.FilterChain;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import javax.ws.rs.container.ContainerRequestContext;
import javax.ws.rs.core.Response;

import org.apache.commons.configuration2.PropertiesConfiguration;
import org.junit.Test;
import org.mockito.ArgumentCaptor;
import org.mockito.ArgumentMatchers;
import org.mockito.Mockito;

import com.mirth.connect.client.core.PropertiesConfigurationUtil;
Expand All @@ -22,26 +27,34 @@ public class RequestedWithFilterTest extends TestCase {
@Test
//assert that if property is set to false, isRequestedWithHeaderRequired = false
public void testConstructor() {

mirthProperties.clearProperty("server.api.require-requested-with");
RequestedWithFilter.configure(mirthProperties);
assertEquals(true, RequestedWithFilter.isRequestedWithHeaderRequired());

mirthProperties.setProperty("server.api.require-requested-with", "false");
RequestedWithFilter requestedWithFilter = new RequestedWithFilter(mirthProperties);
assertEquals(requestedWithFilter.isRequestedWithHeaderRequired(), false);
RequestedWithFilter.configure(mirthProperties);
assertEquals(false, RequestedWithFilter.isRequestedWithHeaderRequired());
}

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

mirthProperties.setProperty("server.api.require-requested-with", "true");
RequestedWithFilter testFilter = new RequestedWithFilter(mirthProperties);

HttpServletRequest mockReq = Mockito.mock(HttpServletRequest.class);
HttpServletResponse mockResp = Mockito.mock(HttpServletResponse.class);
FilterChain mockFilterChain = Mockito.mock(FilterChain.class);

RequestedWithFilter.configure(mirthProperties);

ContainerRequestContext mockCtx = Mockito.mock(ContainerRequestContext.class);
when(mockCtx.getHeaders()).thenReturn(new javax.ws.rs.core.MultivaluedHashMap<String, String>());

try {
testFilter.doFilter(mockReq, mockResp, mockFilterChain);
verify(mockResp).sendError(HttpServletResponse.SC_BAD_REQUEST, "All requests must have 'X-Requested-With' header");
RequestedWithFilter filter = new RequestedWithFilter();
filter.filter(mockCtx);
ArgumentCaptor<Response> responseCaptor =
ArgumentCaptor.forClass(Response.class);
verify(mockCtx).abortWith(responseCaptor.capture());
Response response = responseCaptor.getValue();
assertEquals(400, response.getStatus());
assertEquals("All requests must have 'X-Requested-With' header", response.getEntity());
} catch (Exception e) {
e.printStackTrace();
}
Expand All @@ -52,15 +65,15 @@ public void testDoFilterRequestedWithTrue() {
public void testDoFilterRequestedWithFalse() {

mirthProperties.setProperty("server.api.require-requested-with", "false");
RequestedWithFilter testFilter = new RequestedWithFilter(mirthProperties);

HttpServletRequest mockReq = Mockito.mock(HttpServletRequest.class);
HttpServletResponse mockResp = Mockito.mock(HttpServletResponse.class);
FilterChain mockFilterChain = Mockito.mock(FilterChain.class);

RequestedWithFilter.configure(mirthProperties);

ContainerRequestContext mockCtx = Mockito.mock(ContainerRequestContext.class);
when(mockCtx.getHeaders()).thenReturn(new javax.ws.rs.core.MultivaluedHashMap<String, String>());

try {
testFilter.doFilter(mockReq, mockResp, mockFilterChain);
verify(mockResp, never()).sendError(HttpServletResponse.SC_BAD_REQUEST, "All requests must have 'X-Requested-With' header");
RequestedWithFilter filter = new RequestedWithFilter();
filter.filter(mockCtx);
verify(mockCtx, never()).abortWith(ArgumentMatchers.any(javax.ws.rs.core.Response.class));
} catch (Exception e) {
e.printStackTrace();
}
Expand Down