From 8c5d5a7a6e42aa112c0fdeb3859291b874d8852f 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 cb4290475d..91ceb6d894 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 d387729be03f6f3bebe500f274be9f394a76b448 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 | 3 +- .../server/api/DontRequireRequestedWith.java | 18 ++++ .../api/providers/RequestedWithFilter.java | 83 +++++++++++-------- .../providers/RequestedWithFilterTest.java | 57 ++++++++----- 4 files changed, 103 insertions(+), 58 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 d18f2f1ea0..aa600817e8 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); @@ -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); } 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..ed74bf762b --- /dev/null +++ b/server/src/com/mirth/connect/server/api/DontRequireRequestedWith.java @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: MPL-2.0 +// SPDX-FileCopyrightText: 2025 Mitch Gaffigan + +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..486004149a 100644 --- a/server/src/com/mirth/connect/server/api/providers/RequestedWithFilter.java +++ b/server/src/com/mirth/connect/server/api/providers/RequestedWithFilter.java @@ -1,3 +1,6 @@ +// SPDX-License-Identifier: MPL-2.0 +// SPDX-FileCopyrightText: Mirth Corporation +// SPDX-FileCopyrightText: 2025 Mitch Gaffigan /* * Copyright (c) Mirth Corporation. All rights reserved. * @@ -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 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() {} -} \ 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..d302978d5f 100644 --- a/server/test/com/mirth/connect/server/api/providers/RequestedWithFilterTest.java +++ b/server/test/com/mirth/connect/server/api/providers/RequestedWithFilterTest.java @@ -1,14 +1,19 @@ +// SPDX-License-Identifier: MPL-2.0 +// SPDX-FileCopyrightText: Mirth Corporation +// SPDX-FileCopyrightText: 2025 Mitch Gaffigan 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; @@ -22,10 +27,13 @@ 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 @@ -33,15 +41,20 @@ 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); + ArgumentCaptor 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(); } @@ -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()); + 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(); }