GUACAMOLE-611: Merge selectively fall through to other extensions when authentication fails

guacamole/src/main/java/org/apache/guacamole/extension/AuthenticationProviderFacade.java

@@ -19,14 +19,14 @@
 
 package org.apache.guacamole.extension;
 
+import java.util.Set;
 import java.util.UUID;
 import org.apache.guacamole.GuacamoleException;
 import org.apache.guacamole.net.auth.AuthenticatedUser;
 import org.apache.guacamole.net.auth.AuthenticationProvider;
 import org.apache.guacamole.net.auth.Credentials;
 import org.apache.guacamole.net.auth.UserContext;
-import org.apache.guacamole.net.auth.credentials.CredentialsInfo;
+import org.apache.guacamole.net.auth.credentials.GuacamoleCredentialsException;
-import org.apache.guacamole.net.auth.credentials.GuacamoleInvalidCredentialsException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -48,6 +48,16 @@ public class AuthenticationProviderFacade implements AuthenticationProvider {
      */
     private final AuthenticationProvider authProvider;
 
+    /**
+     * The set of identifiers of all authentication providers whose internal
+     * failures should be tolerated during the authentication process. If the
+     * identifier of this authentication provider is within this set, errors
+     * during authentication will result in the authentication provider being
+     * ignored for that authentication attempt. By default, errors during
+     * authentication halt the authentication process entirely.
+     */
+    private final Set<String> tolerateFailures;
+
     /**
      * The identifier to provide for the underlying authentication provider if
      * the authentication provider could not be loaded.
@@ -63,9 +73,21 @@ public class AuthenticationProviderFacade implements AuthenticationProvider {
      *
      * @param authProviderClass
      *     The AuthenticationProvider subclass to instantiate.
+     *
+     * @param tolerateFailures
+     *     The set of identifiers of all authentication providers whose
+     *     internal failures should be tolerated during the authentication
+     *     process. If the identifier of this authentication provider is within
+     *     this set, errors during authentication will result in the
+     *     authentication provider being ignored for that authentication
+     *     attempt. By default, errors during authentication halt the
+     *     authentication process entirely.
      */
-    public AuthenticationProviderFacade(Class<? extends AuthenticationProvider> authProviderClass) {
+    public AuthenticationProviderFacade(
-        authProvider = ProviderFactory.newInstance("authentication provider",
+            Class<? extends AuthenticationProvider> authProviderClass,
+            Set<String> tolerateFailures) {
+        this.tolerateFailures = tolerateFailures;
+        this.authProvider = ProviderFactory.newInstance("authentication provider",
             authProviderClass);
     }
 
@@ -97,18 +119,124 @@ public class AuthenticationProviderFacade implements AuthenticationProvider {
 
     }
 
+    /**
+     * Returns whether this authentication provider should tolerate internal
+     * failures during the authentication process, allowing other
+     * authentication providers to continue operating as if this authentication
+     * provider simply is not present.
+     *
+     * @return
+     *     true if this authentication provider should tolerate internal
+     *     failures during the authentication process, false otherwise.
+     */
+    private boolean isFailureTolerated() {
+        return tolerateFailures.contains(getIdentifier());
+    }
+
+    /**
+     * Logs a warning that this authentication provider is being skipped due to
+     * an internal error. If debug-level logging is enabled, the full details
+     * of the internal error are also logged.
+     *
+     * @param e
+     *     The internal error that occurred which has resulted in this
+     *     authentication provider being skipped.
+     */
+    private void warnAuthProviderSkipped(Throwable e) {
+
+        logger.warn("The \"{}\" authentication provider has been skipped due "
+                + "to an internal error. If this is unexpected or you are the "
+                + "developer of this authentication provider, you may wish to "
+                + "enable debug-level logging: {}",
+                getIdentifier(), e.getMessage());
+
+        logger.debug("Authentication provider skipped due to an internal failure.", e);
+
+    }
+
+    /**
+     * Logs a warning that the authentication process will be entirely aborted
+     * due to an internal error, advising the administrator to set the
+     * "skip-if-unavailable" property if error encountered is expected and
+     * should be tolerated.
+     */
+    private void warnAuthAborted() {
+        String identifier = getIdentifier();
+        logger.warn("The \"{}\" authentication provider has encountered an "
+                + "internal error which will halt the authentication "
+                + "process. If this is unexpected or you are the developer of "
+                + "this authentication provider, you may wish to enable "
+                + "debug-level logging. If this is expected and you wish to "
+                + "ignore such failures in the future, please set \"{}: {}\" "
+                + "within your guacamole.properties.",
+                identifier, ExtensionModule.SKIP_IF_UNAVAILABLE.getName(),
+                identifier);
+    }
+
     @Override
     public AuthenticatedUser authenticateUser(Credentials credentials)
             throws GuacamoleException {
 
         // Ignore auth attempts if no auth provider could be loaded
         if (authProvider == null) {
-            logger.warn("Authentication attempt denied because the authentication system could not be loaded. Please check for errors earlier in the logs.");
+            logger.warn("Authentication attempt ignored because the relevant "
-            throw new GuacamoleInvalidCredentialsException("Permission denied.", CredentialsInfo.USERNAME_PASSWORD);
+                    + "authentication provider could not be loaded. Please "
+                    + "check for errors earlier in the logs.");
+            return null;
         }
 
         // Delegate to underlying auth provider
-        return authProvider.authenticateUser(credentials);
+        try {
+            return authProvider.authenticateUser(credentials);
+        }
+
+        // Pass through credential exceptions untouched, as these are not
+        // internal failures
+        catch (GuacamoleCredentialsException e) {
+            throw e;
+        }
+
+        // Pass through all other exceptions (aborting authentication entirely)
+        // only if not configured to ignore such failures
+        catch (GuacamoleException e) {
+
+            // Skip using this authentication provider if configured to ignore
+            // internal failures during auth
+            if (isFailureTolerated()) {
+                warnAuthProviderSkipped(e);
+                return null;
+            }
+
+            warnAuthAborted();
+            throw e;
+
+        }
+        catch (RuntimeException e) {
+
+            // Skip using this authentication provider if configured to ignore
+            // internal failures during auth
+            if (isFailureTolerated()) {
+                warnAuthProviderSkipped(e);
+                return null;
+            }
+
+            warnAuthAborted();
+            throw e;
+
+        }
+        catch (Error e) {
+
+            // Skip using this authentication provider if configured to ignore
+            // internal failures during auth
+            if (isFailureTolerated()) {
+                warnAuthProviderSkipped(e);
+                return null;
+            }
+
+            warnAuthAborted();
+            throw e;
+
+        }
 
     }
 
@@ -118,8 +246,10 @@ public class AuthenticationProviderFacade implements AuthenticationProvider {
 
         // Ignore auth attempts if no auth provider could be loaded
         if (authProvider == null) {
-            logger.warn("Reauthentication attempt denied because the authentication system could not be loaded. Please check for errors earlier in the logs.");
+            logger.warn("Reauthentication attempt ignored because the relevant "
-            throw new GuacamoleInvalidCredentialsException("Permission denied.", CredentialsInfo.USERNAME_PASSWORD);
+                    + "authentication provider could not be loaded. Please "
+                    + "check for errors earlier in the logs.");
+            return null;
         }
 
         // Delegate to underlying auth provider
@@ -133,13 +263,65 @@ public class AuthenticationProviderFacade implements AuthenticationProvider {
 
         // Ignore auth attempts if no auth provider could be loaded
         if (authProvider == null) {
-            logger.warn("User data retrieval attempt denied because the authentication system could not be loaded. Please check for errors earlier in the logs.");
+            logger.warn("User data retrieval attempt ignored because the "
+                    + "relevant authentication provider could not be loaded. "
+                    + "Please check for errors earlier in the logs.");
             return null;
         }
 
         // Delegate to underlying auth provider
-        return authProvider.getUserContext(authenticatedUser);
+        try {
-        
+            return authProvider.getUserContext(authenticatedUser);
+        }
+
+        // Pass through credential exceptions untouched, as these are not
+        // internal failures
+        catch (GuacamoleCredentialsException e) {
+            throw e;
+        }
+
+        // Pass through all other exceptions (aborting authentication entirely)
+        // only if not configured to ignore such failures
+        catch (GuacamoleException e) {
+
+            // Skip using this authentication provider if configured to ignore
+            // internal failures during auth
+            if (isFailureTolerated()) {
+                warnAuthProviderSkipped(e);
+                return null;
+            }
+
+            warnAuthAborted();
+            throw e;
+
+        }
+        catch (RuntimeException e) {
+
+            // Skip using this authentication provider if configured to ignore
+            // internal failures during auth
+            if (isFailureTolerated()) {
+                warnAuthProviderSkipped(e);
+                return null;
+            }
+
+            warnAuthAborted();
+            throw e;
+
+        }
+        catch (Error e) {
+
+            // Skip using this authentication provider if configured to ignore
+            // internal failures during auth
+            if (isFailureTolerated()) {
+                warnAuthProviderSkipped(e);
+                return null;
+            }
+
+            warnAuthAborted();
+            throw e;
+
+        }
+
     }
 
     @Override
@@ -149,7 +331,9 @@ public class AuthenticationProviderFacade implements AuthenticationProvider {
 
         // Ignore auth attempts if no auth provider could be loaded
         if (authProvider == null) {
-            logger.warn("User data refresh attempt denied because the authentication system could not be loaded. Please check for errors earlier in the logs.");
+            logger.warn("User data refresh attempt ignored because the "
+                    + "relevant authentication provider could not be loaded. "
+                    + "Please check for errors earlier in the logs.");
             return null;
         }
 

guacamole/src/main/java/org/apache/guacamole/extension/ExtensionModule.java

@@ -29,12 +29,14 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import org.apache.guacamole.auth.file.FileAuthenticationProvider;
 import org.apache.guacamole.GuacamoleException;
 import org.apache.guacamole.GuacamoleServerException;
 import org.apache.guacamole.environment.Environment;
 import org.apache.guacamole.net.auth.AuthenticationProvider;
 import org.apache.guacamole.net.event.listener.Listener;
+import org.apache.guacamole.properties.StringSetProperty;
 import org.apache.guacamole.resource.Resource;
 import org.apache.guacamole.resource.ResourceServlet;
 import org.apache.guacamole.resource.SequenceResource;
@@ -81,6 +83,25 @@ public class ExtensionModule extends ServletModule {
      */
     private static final String EXTENSION_SUFFIX = ".jar";
 
+    /**
+     * A comma-separated list of the identifiers of all authentication
+     * providers whose internal failures should be tolerated during the
+     * authentication process. If an authentication provider within this list
+     * encounters an internal error during the authentication process, it will
+     * simply be skipped, allowing other authentication providers to continue
+     * trying to authenticate the user. Internal errors within authentication
+     * providers that are not within this list will halt the authentication
+     * process entirely.
+     */
+    public static final StringSetProperty SKIP_IF_UNAVAILABLE = new StringSetProperty() {
+
+        @Override
+        public String getName() {
+            return "skip-if-unavailable";
+        }
+
+    };
+
     /**
      * The Guacamole server environment.
      */
@@ -156,13 +177,26 @@ public class ExtensionModule extends ServletModule {
      *
      * @param authenticationProvider
      *     The AuthenticationProvider class to bind.
+     *
+     * @param tolerateFailures
+     *     The set of identifiers of all authentication providers whose
+     *     internal failures should be tolerated during the authentication
+     *     process. If the identifier of an authentication provider is within
+     *     this set, errors during authentication will result in the
+     *     authentication provider being ignored for that authentication
+     *     attempt, with the authentication process proceeding as if that
+     *     authentication provider were not present. By default, errors during
+     *     authentication halt the authentication process entirely.
      */
-    private void bindAuthenticationProvider(Class<? extends AuthenticationProvider> authenticationProvider) {
+    private void bindAuthenticationProvider(
+            Class<? extends AuthenticationProvider> authenticationProvider,
+            Set<String> tolerateFailures) {
 
         // Bind authentication provider
         logger.debug("[{}] Binding AuthenticationProvider \"{}\".",
                 boundAuthenticationProviders.size(), authenticationProvider.getName());
-        boundAuthenticationProviders.add(new AuthenticationProviderFacade(authenticationProvider));
+        boundAuthenticationProviders.add(new AuthenticationProviderFacade(
+                authenticationProvider, tolerateFailures));
 
     }
 
@@ -173,12 +207,24 @@ public class ExtensionModule extends ServletModule {
      *
      * @param authProviders
      *     The AuthenticationProvider classes to bind.
+     *
+     * @param tolerateFailures
+     *     The set of identifiers of all authentication providers whose
+     *     internal failures should be tolerated during the authentication
+     *     process. If the identifier of an authentication provider is within
+     *     this set, errors during authentication will result in the
+     *     authentication provider being ignored for that authentication
+     *     attempt, with the authentication process proceeding as if that
+     *     authentication provider were not present. By default, errors during
+     *     authentication halt the authentication process entirely.
      */
-    private void bindAuthenticationProviders(Collection<Class<AuthenticationProvider>> authProviders) {
+    private void bindAuthenticationProviders(
+            Collection<Class<AuthenticationProvider>> authProviders,
+            Set<String> tolerateFailures) {
 
         // Bind each authentication provider within extension
         for (Class<AuthenticationProvider> authenticationProvider : authProviders)
-            bindAuthenticationProvider(authenticationProvider);
+            bindAuthenticationProvider(authenticationProvider, tolerateFailures);
 
     }
 
@@ -313,6 +359,38 @@ public class ExtensionModule extends ServletModule {
         return ALLOWED_GUACAMOLE_VERSIONS.contains(guacamoleVersion);
     }
 
+    /**
+     * Returns the set of identifiers of all authentication providers whose
+     * internal failures should be tolerated during the authentication process.
+     * If the identifier of an authentication provider is within this set,
+     * errors during authentication will result in the authentication provider
+     * being ignored for that authentication attempt, with the authentication
+     * process proceeding as if that authentication provider were not present.
+     * By default, errors during authentication halt the authentication process
+     * entirely.
+     *
+     * @return
+     *     The set of identifiers of all authentication providers whose
+     *     internal failures should be tolerated during the authentication
+     *     process.
+     */
+    private Set<String> getToleratedAuthenticationProviders() {
+
+        // Parse list of auth providers whose internal failures should be
+        // tolerated
+        try {
+            return environment.getProperty(SKIP_IF_UNAVAILABLE, Collections.<String>emptySet());
+        }
+
+        // Use empty set by default if property cannot be parsed
+        catch (GuacamoleException e) {
+            logger.warn("The list of authentication providers specified via the \"{}\" property could not be parsed: {}", SKIP_IF_UNAVAILABLE.getName(), e.getMessage());
+            logger.debug("Unable to parse \"{}\" property.", SKIP_IF_UNAVAILABLE.getName(), e);
+            return Collections.<String>emptySet();
+        }
+
+    }
+
     /**
      * Loads all extensions within the GUACAMOLE_HOME/extensions directory, if
      * any, adding their static resource to the given resoure collections.
@@ -324,9 +402,20 @@ public class ExtensionModule extends ServletModule {
      * @param cssResources
      *     A modifiable collection of static CSS resources which may receive
      *     new CSS resources from extensions.
+     *
+     * @param toleratedAuthProviders
+     *     The set of identifiers of all authentication providers whose
+     *     internal failures should be tolerated during the authentication
+     *     process. If the identifier of an authentication provider is within
+     *     this set, errors during authentication will result in the
+     *     authentication provider being ignored for that authentication
+     *     attempt, with the authentication process proceeding as if that
+     *     authentication provider were not present. By default, errors during
+     *     authentication halt the authentication process entirely.
      */
     private void loadExtensions(Collection<Resource> javaScriptResources,
-            Collection<Resource> cssResources) {
+            Collection<Resource> cssResources,
+            Set<String> toleratedAuthProviders) {
 
         // Retrieve and validate extensions directory
         File extensionsDir = new File(environment.getGuacamoleHome(), EXTENSIONS_DIRECTORY);
@@ -375,7 +464,7 @@ public class ExtensionModule extends ServletModule {
                 cssResources.addAll(extension.getCSSResources().values());
 
                 // Attempt to load all authentication providers
-                bindAuthenticationProviders(extension.getAuthenticationProviderClasses());
+                bindAuthenticationProviders(extension.getAuthenticationProviderClasses(), toleratedAuthProviders);
 
                 // Attempt to load all listeners
                 bindListeners(extension.getListenerClasses());
@@ -430,10 +519,11 @@ public class ExtensionModule extends ServletModule {
         cssResources.add(new WebApplicationResource(getServletContext(), "/guacamole.min.css"));
 
         // Load all extensions
-        loadExtensions(javaScriptResources, cssResources);
+        final Set<String> toleratedAuthProviders = getToleratedAuthenticationProviders();
+        loadExtensions(javaScriptResources, cssResources, toleratedAuthProviders);
 
         // Always bind default file-driven auth last
-        bindAuthenticationProvider(FileAuthenticationProvider.class);
+        bindAuthenticationProvider(FileAuthenticationProvider.class, toleratedAuthProviders);
 
         // Dynamically generate app.js and app.css from extensions
         serve("/app.js").with(new ResourceServlet(new SequenceResource(javaScriptResources)));