From e304cd66c00d945315602b21f6bb911d0fe0c915 Mon Sep 17 00:00:00 2001 From: Mitch Gaffigan Date: Wed, 3 Sep 2025 10:01:05 -0500 Subject: [PATCH 1/2] Improve error messsages for a few sharp edges Signed-off-by: Mitch Gaffigan --- server/src/com/mirth/connect/client/core/Client.java | 6 +++--- server/src/com/mirth/connect/server/api/MirthServlet.java | 3 +++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/server/src/com/mirth/connect/client/core/Client.java b/server/src/com/mirth/connect/client/core/Client.java index a0ba3f57bb..ade7635141 100644 --- a/server/src/com/mirth/connect/client/core/Client.java +++ b/server/src/com/mirth/connect/client/core/Client.java @@ -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); } } } @@ -219,7 +219,7 @@ public void registerApiProviders(Set packageNames, Set classes) client.register(clazz); } } catch (Throwable t) { - logger.error("Error registering API provider package: " + packageName); + logger.error("Error registering API provider package: {}", packageName, t); } } } @@ -229,7 +229,7 @@ public void registerApiProviders(Set packageNames, Set 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); } } } diff --git a/server/src/com/mirth/connect/server/api/MirthServlet.java b/server/src/com/mirth/connect/server/api/MirthServlet.java index b153b52759..cede2344e3 100644 --- a/server/src/com/mirth/connect/server/api/MirthServlet.java +++ b/server/src/com/mirth/connect/server/api/MirthServlet.java @@ -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); } From 8620766392af768ad199c1a94dd16f9b5c1032af Mon Sep 17 00:00:00 2001 From: Mitch Gaffigan Date: Wed, 3 Sep 2025 10:01:35 -0500 Subject: [PATCH 2/2] Refactor to allow CSRF bypass Signed-off-by: Mitch Gaffigan --- .../mirth/connect/server/MirthWebServer.java | 2 +- .../server/api/DontRequireRequestedWith.java | 15 ++++ .../api/providers/RequestedWithFilter.java | 85 ++++++++++--------- .../providers/RequestedWithFilterTest.java | 42 ++++----- 4 files changed, 81 insertions(+), 63 deletions(-) create mode 100644 server/src/com/mirth/connect/server/api/DontRequireRequestedWith.java diff --git a/server/src/com/mirth/connect/server/MirthWebServer.java b/server/src/com/mirth/connect/server/MirthWebServer.java index b4ab2b3bbf..3709e2b7b1 100644 --- a/server/src/com/mirth/connect/server/MirthWebServer.java +++ b/server/src/com/mirth/connect/server/MirthWebServer.java @@ -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); diff --git a/server/src/com/mirth/connect/server/api/DontRequireRequestedWith.java b/server/src/com/mirth/connect/server/api/DontRequireRequestedWith.java new file mode 100644 index 0000000000..1fd1c92c71 --- /dev/null +++ b/server/src/com/mirth/connect/server/api/DontRequireRequestedWith.java @@ -0,0 +1,15 @@ +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 { +} diff --git a/server/src/com/mirth/connect/server/api/providers/RequestedWithFilter.java b/server/src/com/mirth/connect/server/api/providers/RequestedWithFilter.java index 778f17baab..3bc06e9ca3 100644 --- a/server/src/com/mirth/connect/server/api/providers/RequestedWithFilter.java +++ b/server/src/com/mirth/connect/server/api/providers/RequestedWithFilter.java @@ -1,64 +1,67 @@ -/* - * Copyright (c) Mirth Corporation. All rights reserved. - * - * http://www.mirthcorp.com - * - * The software in this package is published under the terms of the MPL license a copy of which has - * been included with this distribution in the LICENSE.txt file. - */ - 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.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 javax.ws.rs.ext.Provider; import org.apache.commons.configuration2.PropertiesConfiguration; import org.apache.commons.lang3.StringUtils; +import com.mirth.connect.server.api.DontRequireRequestedWith; + @Provider -public class RequestedWithFilter implements Filter { +@Priority(Priorities.AUTHENTICATION + 100) +public class RequestedWithFilter implements ContainerRequestFilter { - private boolean isRequestedWithHeaderRequired = true; + @Context + private ResourceInfo resourceInfo; + private static boolean isRequestedWithHeaderRequired = true; - public RequestedWithFilter(PropertiesConfiguration mirthProperties) { - + // 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; + public void filter(ContainerRequestContext requestContext) throws IOException { + if (!isRequestedWithHeaderRequired) { + return; + } + + // 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; + } + } - HttpServletRequest servletRequest = (HttpServletRequest)request; - String requestedWithHeader = (String) servletRequest.getHeader("X-Requested-With"); + List header = requestContext.getHeaders().get("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"); + 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()); } - else { - chain.doFilter(request, response); - } - } - - public boolean isRequestedWithHeaderRequired() { - return isRequestedWithHeaderRequired; - } - - @Override - public void destroy() {} -} \ No newline at end of file +} diff --git a/server/test/com/mirth/connect/server/api/providers/RequestedWithFilterTest.java b/server/test/com/mirth/connect/server/api/providers/RequestedWithFilterTest.java index b699ee6f50..a7782868ba 100644 --- a/server/test/com/mirth/connect/server/api/providers/RequestedWithFilterTest.java +++ b/server/test/com/mirth/connect/server/api/providers/RequestedWithFilterTest.java @@ -2,10 +2,9 @@ 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 org.apache.commons.configuration2.PropertiesConfiguration; import org.junit.Test; @@ -24,8 +23,9 @@ public class RequestedWithFilterTest extends TestCase { public void testConstructor() { mirthProperties.setProperty("server.api.require-requested-with", "false"); - RequestedWithFilter requestedWithFilter = new RequestedWithFilter(mirthProperties); - assertEquals(requestedWithFilter.isRequestedWithHeaderRequired(), false); + assertEquals(RequestedWithFilter.isRequestedWithHeaderRequired(), true); + RequestedWithFilter.configure(mirthProperties); + assertEquals(RequestedWithFilter.isRequestedWithHeaderRequired(), false); } @Test @@ -33,15 +33,15 @@ public void testConstructor() { 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()); + 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); + verify(mockCtx).abortWith(org.mockito.Matchers.any(javax.ws.rs.core.Response.class)); } catch (Exception e) { e.printStackTrace(); } @@ -52,15 +52,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()); + 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(org.mockito.Matchers.any(javax.ws.rs.core.Response.class)); } catch (Exception e) { e.printStackTrace(); }