fixes #246: Require custom REST API authentication filters to be annotated as such

changelog.html

@@ -47,6 +47,7 @@ <h1>
 <p><b>1.12.1</b> (to be determined)</p>
 <ul>
     <li>[<a href="https://github.com/igniterealtime/openfire-restAPI-plugin/issues/251">#251</a>] - Enable JUnit 5 tests</li>
+    <li>[<a href="https://github.com/igniterealtime/openfire-restAPI-plugin/issues/246">#246</a>] - Require custom authenticator plugin to be annotated as an authenticator</li>
     <li>[<a href="https://github.com/igniterealtime/openfire-restAPI-plugin/issues/244">#244</a>] - Prevent REST API plugin from exposing its own configuration (including authentication) via its own endpoints</li>
     <li>[<a href="https://github.com/igniterealtime/openfire-restAPI-plugin/issues/242">#242</a>] - Fix individual System Property GETs returning HTTP/404</li>
     <li>[<a href="https://github.com/igniterealtime/openfire-restAPI-plugin/issues/213">#213</a>] - Improve setting a subject in a chat room</li>

readme.md

@@ -80,6 +80,34 @@ The secret key can be defined in Openfire Admin console under Server > Server Se
 E.g.
 >**Header:** Authorization: s3cretKey
 
+### Custom authentication filter
+
+In addition to Basic HTTP Authentication and the shared secret key, the REST API plugin can delegate authentication
+to a custom implementation, for deployments that need to integrate with an external identity provider, a
+different credential store, or additional checks beyond what the built-in mechanisms provide.
+
+This is configured in the Openfire Admin console under Server > Server Settings > REST API, by setting the
+authentication type to "custom" and providing the fully qualified class name of your implementation. The class
+must already be present on Openfire's classpath (e.g. provided as a JAR file in Openfire's LIB folder) before it can be
+selected.
+
+#### Requirements for a custom implementation
+
+A class used as a custom authentication filter must:
+
+1. Implement `javax.ws.rs.container.ContainerRequestFilter`.
+2. Be annotated with `@javax.annotation.Priority(javax.ws.rs.Priorities.AUTHENTICATION)`.
+3. Reject any request that does not carry valid credentials (for example by throwing a `WebApplicationException` with an
+   appropriate `Response.Status`, or by calling `requestContext.abortWith(...)` with an appropriate `4xx` response)
+   before returning from `filter()`.
+
+The first two requirements are enforced by the plugin: a class that does not satisfy both will be rejected when
+you attempt to save the configuration. The REST API will continue using its default authentication filter.
+
+The third requirement cannot be verified automatically and is the implementer's responsibility. A filter that
+implements the interface and carries the annotation, but returns without calling `abortWith(...)` on an
+unauthenticated request, will be loaded successfully and will silently grant unauthenticated access.
+
 # User related REST Endpoints
 
 ## Retrieve users

src/java/org/jivesoftware/openfire/plugin/rest/AuthFilter.java

@@ -41,7 +41,7 @@
  * The Class AuthFilter.
  */
 @PreMatching
-@Priority(Priorities.AUTHORIZATION)
+@Priority(Priorities.AUTHENTICATION)
 public class AuthFilter implements ContainerRequestFilter {
 
     /** The log. */

src/java/org/jivesoftware/openfire/plugin/rest/RESTServicePlugin.java

@@ -149,18 +149,13 @@ public void destroyPlugin() {
     }
 
     /**
-     * Returns the loading status message.
+     * Validates the custom authentication filter class name.
      *
-     * @return the loading status message.
+     * @param customAuthFilterClassName the custom authentication filter class name.
+     * @return the validation message, or null when the class name is valid.
      */
-    public String getLoadingStatusMessage() {
-        return JerseyWrapper.getLoadingStatusMessage();
-    }
-
-    /**
-     * Reloads the Jersey wrapper.
-     */
-    public String loadAuthenticationFilter(String customAuthFilterClassName) {
-        return JerseyWrapper.tryLoadingAuthenticationFilter(customAuthFilterClassName);
+    public String validateCustomAuthenticationFilter(String customAuthFilterClassName)
+    {
+        return JerseyWrapper.validateCustomAuthFilterClassName(customAuthFilterClassName);
     }
 }

src/java/org/jivesoftware/openfire/plugin/rest/service/JerseyWrapper.java

@@ -16,68 +16,135 @@
 
 package org.jivesoftware.openfire.plugin.rest.service;
 
+import com.google.common.annotations.VisibleForTesting;
 import org.glassfish.jersey.server.ResourceConfig;
 import org.jivesoftware.openfire.plugin.rest.*;
 import org.jivesoftware.openfire.plugin.rest.exceptions.RESTExceptionMapper;
 
+import javax.annotation.Nonnull;
+import javax.annotation.Priority;
 import javax.servlet.ServletConfig;
+import javax.ws.rs.Priorities;
+import javax.ws.rs.container.ContainerRequestFilter;
+import javax.ws.rs.container.DynamicFeature;
 import javax.ws.rs.core.Context;
+import javax.ws.rs.core.Feature;
+import java.util.Set;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import java.util.stream.Collectors;
 
 /**
  * The Class JerseyWrapper.
  */
 public class JerseyWrapper extends ResourceConfig {
 
+    private static final org.slf4j.Logger Log = org.slf4j.LoggerFactory.getLogger(JerseyWrapper.class);
+
+    /**
+     * A collection of classes that are acceptable for use as a custom REST API authentication implementation.
+     */
+    public static final Set<Class<?>> CUSTOM_AUTH_ACCEPTABLE_CONTRACT_SHAPES = Set.of(
+        ContainerRequestFilter.class, // Implementations are expected to be a ContainerRequestFilter
+        Feature.class,                // Theoretically, Jersey can use this too - allowing/keeping this for backwards compatibility.
+        DynamicFeature.class          // Theoretically, Jersey can use this too - allowing/keeping this for backwards compatibility.
+    );
+
     /** The Constant SERVLET_URL. */
     public static final String SERVLET_URL = "restapi/*";
 
     /** The Constant JERSEY_LOGGER. */
     private final static Logger JERSEY_LOGGER = Logger.getLogger("org.glassfish.jersey");
 
-    private static String loadingStatusMessage = null;
-
     static {
         JERSEY_LOGGER.setLevel(Level.SEVERE);
     }
 
-    public static String tryLoadingAuthenticationFilter(String customAuthFilterClassName) {
-
-        try {
-            if(customAuthFilterClassName != null) {
-                Class.forName(customAuthFilterClassName, false, JerseyWrapper.class.getClassLoader());
+    public static String validateCustomAuthFilterClassName(String customAuthFilterClassName)
+    {
+        String loadingStatusMessage;
+        if (customAuthFilterClassName == null || customAuthFilterClassName.isEmpty()) {
+            loadingStatusMessage = "Classname field can't be empty!";
+        } else {
+            try {
+                getCustomAuthFilterClassObject(customAuthFilterClassName);
                 loadingStatusMessage = null;
+            } catch (IllegalArgumentException e) {
+                loadingStatusMessage = "Unable to use the custom auth filter class name '" + customAuthFilterClassName + "': " + e.getMessage();
             }
-        } catch (ClassNotFoundException e) {
-            loadingStatusMessage = "No custom auth filter found for restAPI plugin with name " + customAuthFilterClassName;
         }
-
-        if(customAuthFilterClassName == null || customAuthFilterClassName.isEmpty())
-            loadingStatusMessage = "Classname field can't be empty!";
+
         return loadingStatusMessage;
     }
 
-    public String loadAuthenticationFilter() {
-
+    public String loadAuthenticationFilter()
+    {
         // Check if custom AuthFilter is available
         String customAuthFilterClassName = RESTServicePlugin.CUSTOM_AUTH_FILTER.getValue();
         RESTServicePlugin.AuthType restAuthType = RESTServicePlugin.AUTH_TYPE.getValue();
         Class<?> pickedAuthFilter = AuthFilter.class;
-
+
+        String loadingStatusMessage = null;
         try {
             if(customAuthFilterClassName != null && RESTServicePlugin.AuthType.custom.equals(restAuthType)) {
-                pickedAuthFilter = Class.forName(customAuthFilterClassName, false, JerseyWrapper.class.getClassLoader());
+                pickedAuthFilter = getCustomAuthFilterClassObject(customAuthFilterClassName);
                 loadingStatusMessage = null;
             }
-        } catch (ClassNotFoundException e) {
-            loadingStatusMessage = "No custom auth filter found for restAPI plugin! " + customAuthFilterClassName + " " + restAuthType;
+        } catch (IllegalArgumentException e) {
+            loadingStatusMessage = "Unable to use the custom auth filter class name '" + customAuthFilterClassName + "': " + e.getMessage();
+            Log.error("Unable to load custom auth filter: {}", customAuthFilterClassName, e);
         }
 
         register(pickedAuthFilter);
         return loadingStatusMessage;
     }
-
+
+    /**
+     * Resolves the given class name and validates that it is suitable for use as a custom REST API authentication
+     * filter.
+     *
+     * To be accepted, the resolved class must satisfy both of the following:
+     * <ul>
+     *     <li>It must implement one of {@link ContainerRequestFilter}, {@link Feature}, or {@link DynamicFeature}.
+     *     {@link ContainerRequestFilter} is the expected, documented contract; {@link Feature} and
+     *     {@link DynamicFeature} are accepted only for backwards compatibility with any implementation that wires
+     *     up its filter indirectly through one of these.</li>
+     *     <li>It must be annotated with {@code @}{@link Priority}{@code (}{@link Priorities#AUTHENTICATION}{@code )},
+     *     signaling that the class is specifically intended to perform authentication. This is a necessary, but not
+     *     sufficient, safeguard: it prevents an incidental class (e.g. a logging or metrics filter) from being
+     *     mistaken for an authenticator, but it cannot verify that the class actually rejects unauthenticated
+     *     requests.</li>
+     * </ul>
+     * This method does not instantiate or invoke the resolved class; it only inspects its type and annotations.
+     *
+     * @param className the fully qualified name of the class to resolve and validate. Must not be {@code null}.
+     * @return the resolved, validated class. Never {@code null}.
+     * @throws IllegalArgumentException if the class cannot be resolved on the classpath, or if it is resolved but
+     *         does not satisfy both validation requirements above. The exception message identifies the specific
+     *         reason for rejection.
+     */
+    @VisibleForTesting
+    static Class<?> getCustomAuthFilterClassObject(@Nonnull final String className) throws IllegalArgumentException
+    {
+        final Class<?> candidate;
+        try {
+            candidate = Class.forName(className, false, JerseyWrapper.class.getClassLoader());
+        } catch (ClassNotFoundException e) {
+            throw new IllegalArgumentException("Class not found: " + className, e);
+        }
+
+        if (CUSTOM_AUTH_ACCEPTABLE_CONTRACT_SHAPES.stream().noneMatch(c -> c.isAssignableFrom(candidate))) {
+            throw new IllegalArgumentException("Class " + className + " does not implement an acceptable contract shape (one of " + CUSTOM_AUTH_ACCEPTABLE_CONTRACT_SHAPES.stream().map(Class::getName).collect(Collectors.joining(", ")) + ").");
+        }
+
+        final Priority priority = candidate.getAnnotation(Priority.class);
+        if (priority == null || priority.value() != Priorities.AUTHENTICATION) {
+            throw new IllegalArgumentException("Class " + className + " is not a valid authentication filter (it lacks the @javax.annotation.Priority(javax.ws.rs.Priorities.AUTHENTICATION) annotation).");
+        }
+
+        return candidate;
+    }
+
     /**
      * Instantiates a new jersey wrapper.
      */
@@ -118,14 +185,4 @@ public JerseyWrapper(@Context ServletConfig servletConfig) {
         // Documentation (Swagger)
         register( new CustomOpenApiResource() );
     }
-
-    /*
-     * Returns the loading status message.
-     *
-     * @return the loading status message.
-     */
-    public static String getLoadingStatusMessage() {
-        return loadingStatusMessage;
-    }
-
 }