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

Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net>

Author: mgaffigan

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<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);
                 }
             }
         }
@@ -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);
                 }
             }
         }

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

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 {
+}

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);
         }

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

@@ -1,64 +1,63 @@
-/*
- * 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 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 java.lang.reflect.Method;
+import java.util.List;
+
+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 void filter(ContainerRequestContext requestContext) throws IOException {
+        if (!isRequestedWithHeaderRequired) {
+            return;
+        }
 
-    @Override
-    public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException {
-        HttpServletResponse res = (HttpServletResponse) 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;
+            }
+        }
 
-        HttpServletRequest servletRequest = (HttpServletRequest)request;
-        String requestedWithHeader = (String) servletRequest.getHeader("X-Requested-With");
+        List<String> 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() {}
-}
+}