GUACAMOLE-1224: Migrate existing auth-related logging to global event listener.

extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/status/InMemoryAuthenticationFailureTracker.java

@@ -21,7 +21,6 @@ package org.apache.guacamole.auth.ban.status;
 
 import com.github.benmanes.caffeine.cache.Cache;
 import com.github.benmanes.caffeine.cache.Caffeine;
-import javax.servlet.http.HttpServletRequest;
 import org.apache.guacamole.GuacamoleException;
 import org.apache.guacamole.GuacamoleServerException;
 import org.apache.guacamole.language.TranslatableGuacamoleClientTooManyException;
@@ -88,40 +87,6 @@ public class InMemoryAuthenticationFailureTracker implements AuthenticationFailu
 
     }
 
-    /**
-     * Returns whether the given Credentials do not contain any specific
-     * authentication parameters, including HTTP parameters. An authentication
-     * request that contains no parameters whatsoever will tend to be the
-     * first, anonymous, credential-less authentication attempt that results in
-     * the initial login screen rendering.
-     *
-     * @param credentials
-     *     The Credentials object to test.
-     *
-     * @return
-     *     true if the given Credentials contain no authentication parameters
-     *     whatsoever, false otherwise.
-     */
-    private boolean isEmpty(Credentials credentials) {
-
-        // An authentication request that contains an explicit username or
-        // password (even if blank) is non-empty, regardless of how the values
-        // were passed
-        if (credentials.getUsername() != null || credentials.getPassword() != null)
-            return false;
-
-        // All further tests depend on HTTP request details
-        HttpServletRequest request = credentials.getRequest();
-        if (request == null)
-            return true;
-
-        // An authentication request is non-empty if it contains any HTTP
-        // parameters at all or contains an authentication token
-        return !request.getParameterNames().hasMoreElements()
-                && request.getHeader("Guacamole-Token") == null;
-
-    }
-
     /**
      * Reports that the given address has just failed to authenticate and
      * returns the AuthenticationFailureStatus that represents that failure. If
@@ -168,7 +133,7 @@ public class InMemoryAuthenticationFailureTracker implements AuthenticationFailu
             boolean failed) throws GuacamoleException {
 
         // Ignore requests that do not contain explicit parameters of any kind
-        if (isEmpty(credentials))
+        if (credentials.isEmpty())
             return;
 
         // Determine originating address of the authentication request

guacamole-ext/src/main/java/org/apache/guacamole/net/auth/Credentials.java

@@ -236,4 +236,36 @@ public class Credentials implements Serializable {
         this.remoteHostname = remoteHostname;
     }
 
+    /**
+     * Returns whether this Credentials object does not contain any specific
+     * authentication parameters, including HTTP parameters and the HTTP header
+     * used for the authentication token. An authentication request that
+     * contains no parameters whatsoever will tend to be the first, anonymous,
+     * credential-less authentication attempt that results in the initial login
+     * screen rendering.
+     *
+     * @return
+     *     true if this Credentials object contains no authentication
+     *     parameters whatsoever, false otherwise.
+     */
+    public boolean isEmpty() {
+
+        // An authentication request that contains an explicit username or
+        // password (even if blank) is non-empty, regardless of how the values
+        // were passed
+        if (getUsername() != null || getPassword() != null)
+            return false;
+
+        // All further tests depend on HTTP request details
+        HttpServletRequest httpRequest = getRequest();
+        if (httpRequest == null)
+            return true;
+
+        // An authentication request is non-empty if it contains any HTTP
+        // parameters at all or contains an authentication token
+        return !httpRequest.getParameterNames().hasMoreElements()
+                && httpRequest.getHeader("Guacamole-Token") == null;
+
+    }
+
 }

guacamole-ext/src/main/java/org/apache/guacamole/net/event/AuthenticationSuccessEvent.java

@@ -42,17 +42,47 @@ public class AuthenticationSuccessEvent implements UserEvent, CredentialEvent,
      */
     private final AuthenticatedUser authenticatedUser;
 
+    /**
+     * Whether the successful authentication attempt represented by this event
+     * is related to an established Guacamole session.
+     */
+    private final boolean existingSession;
+    
     /**
      * Creates a new AuthenticationSuccessEvent which represents a successful
      * authentication attempt by the user identified by the given
-     * AuthenticatedUser object.
+     * AuthenticatedUser object. The authentication attempt is presumed to be
+     * a fresh authentication attempt unrelated to an established session (a
+     * login attempt).
      *
      * @param authenticatedUser
      *     The AuthenticatedUser identifying the user that successfully
      *     authenticated.
      */
     public AuthenticationSuccessEvent(AuthenticatedUser authenticatedUser) {
+        this(authenticatedUser, false);
+    }
+
+    /**
+     * Creates a new AuthenticationSuccessEvent which represents a successful
+     * authentication attempt by the user identified by the given
+     * AuthenticatedUser object. Whether the authentication attempt is
+     * related to an established session (a periodic re-authentication attempt
+     * that updates session status) or not (a fresh login attempt) is
+     * determined by the value of the provided flag.
+     *
+     * @param authenticatedUser
+     *     The AuthenticatedUser identifying the user that successfully
+     *     authenticated.
+     *
+     * @param existingSession
+     *     Whether this AuthenticationSuccessEvent represents an
+     *     re-authentication attempt that updates the status of an established
+     *     Guacamole session.
+     */
+    public AuthenticationSuccessEvent(AuthenticatedUser authenticatedUser, boolean existingSession) {
         this.authenticatedUser = authenticatedUser;
+        this.existingSession = existingSession;
     }
 
     @Override
@@ -70,4 +100,21 @@ public class AuthenticationSuccessEvent implements UserEvent, CredentialEvent,
         return getAuthenticatedUser().getAuthenticationProvider();
     }
 
+    /**
+     * Returns whether the successful authentication attempt represented by
+     * this event is related to an established Guacamole session. During normal
+     * operation, the Guacamole web application will periodically
+     * re-authenticate with the server to verify its authentication token and
+     * update the session state, in which case the value returned by this
+     * function will be true. If the user was not already authenticated and has
+     * just initially logged in, false is returned.
+     *
+     * @return
+     *     true if this AuthenticationSuccessEvent is related to a Guacamole
+     *     session that was already established, false otherwise.
+     */
+    public boolean isExistingSession() {
+        return existingSession;
+    }
+
 }

guacamole/src/main/java/org/apache/guacamole/event/EventLoggingListener.java

@@ -22,8 +22,12 @@ package org.apache.guacamole.event;
 import javax.annotation.Nonnull;
 import org.apache.guacamole.GuacamoleException;
 import org.apache.guacamole.GuacamoleResourceNotFoundException;
+import org.apache.guacamole.net.auth.Credentials;
+import org.apache.guacamole.net.auth.credentials.GuacamoleInsufficientCredentialsException;
 import org.apache.guacamole.net.event.ApplicationShutdownEvent;
 import org.apache.guacamole.net.event.ApplicationStartedEvent;
+import org.apache.guacamole.net.event.AuthenticationFailureEvent;
+import org.apache.guacamole.net.event.AuthenticationSuccessEvent;
 import org.apache.guacamole.net.event.DirectoryEvent;
 import org.apache.guacamole.net.event.DirectoryFailureEvent;
 import org.apache.guacamole.net.event.DirectorySuccessEvent;
@@ -115,6 +119,52 @@ public class EventLoggingListener implements Listener {
         }
     }
 
+    /**
+     * Logs that authentication succeeded for a user.
+     *
+     * @param event
+     *     The event describing the authentication attempt that succeeded.
+     */
+    private void logSuccess(AuthenticationSuccessEvent event) {
+        if (!event.isExistingSession())
+            logger.info("{} successfully authenticated from {}",
+                    new RequestingUser(event),
+                    new RemoteAddress(event.getCredentials()));
+        else
+            logger.debug("{} successfully re-authenticated their existing "
+                    + "session from {}", new RequestingUser(event),
+                    new RemoteAddress(event.getCredentials()));
+    }
+
+    /**
+     * Logs that authentication failed for a user.
+     *
+     * @param event
+     *     The event describing the authentication attempt that failed.
+     */
+    private void logFailure(AuthenticationFailureEvent event) {
+
+        Credentials creds = event.getCredentials();
+        String username = creds.getUsername();
+
+        if (creds.isEmpty())
+            logger.debug("Empty authentication attempt (login screen "
+                    + "initialization) from {} failed: {}",
+                    new RemoteAddress(creds), new Failure(event));
+        else if (username == null || username.isEmpty())
+            logger.debug("Anonymous authentication attempt from {} failed: {}",
+                    new RemoteAddress(creds), new Failure(event));
+        else if (event.getFailure() instanceof GuacamoleInsufficientCredentialsException)
+            logger.debug("Authentication attempt from {} for user \"{}\" "
+                    + "requires additional credentials to continue: {}",
+                    new RemoteAddress(creds), username, new Failure(event));
+        else
+            logger.warn("Authentication attempt from {} for user \"{}\" "
+                    + "failed: {}", new RemoteAddress(creds), username,
+                    new Failure(event));
+
+    }
+
     @Override
     public void handleEvent(@Nonnull Object event) throws GuacamoleException {
 
@@ -124,7 +174,11 @@ public class EventLoggingListener implements Listener {
         else if (event instanceof DirectoryFailureEvent)
             logFailure((DirectoryFailureEvent<?>) event);
 
-        // Logout / session expiration
+        // Login / logout / session expiration
+        else if (event instanceof AuthenticationSuccessEvent)
+            logSuccess((AuthenticationSuccessEvent) event);
+        else if (event instanceof AuthenticationFailureEvent)
+            logFailure((AuthenticationFailureEvent) event);
         else if (event instanceof UserSessionInvalidatedEvent)
             logger.info("{} has logged out, or their session has expired or "
                     + "been terminated.", new RequestingUser((UserSessionInvalidatedEvent) event));

guacamole/src/main/java/org/apache/guacamole/event/RemoteAddress.java

@@ -0,0 +1,95 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.guacamole.event;
+
+import java.util.regex.Pattern;
+import javax.servlet.http.HttpServletRequest;
+import org.apache.guacamole.net.auth.Credentials;
+
+/**
+ * Loggable representation of the remote address of a user, including any
+ * intervening proxies noted by "X-Forwarded-For". This representation takes
+ * into account the fact that "X-Forwarded-For" may come from an untrusted
+ * source, logging such addresses within square brackets alongside the trusted
+ * source IP.
+ */
+public class RemoteAddress implements LoggableDetail {
+
+    /**
+     * Regular expression which matches any IPv4 address.
+     */
+    private static final String IPV4_ADDRESS_REGEX = "([0-9]{1,3}\\.[0-9]{1,3}\\.[0-9]{1,3}\\.[0-9]{1,3})";
+
+    /**
+     * Regular expression which matches any IPv6 address.
+     */
+    private static final String IPV6_ADDRESS_REGEX = "([0-9a-fA-F]*(:[0-9a-fA-F]*){0,7})";
+
+    /**
+     * Regular expression which matches any IP address, regardless of version.
+     */
+    private static final String IP_ADDRESS_REGEX = "(" + IPV4_ADDRESS_REGEX + "|" + IPV6_ADDRESS_REGEX + ")";
+
+    /**
+     * Regular expression which matches any Port Number.
+     */
+    private static final String PORT_NUMBER_REGEX = "(:[0-9]{1,5})?";
+
+    /**
+     * Pattern which matches valid values of the de-facto standard
+     * "X-Forwarded-For" header.
+     */
+    private static final Pattern X_FORWARDED_FOR = Pattern.compile("^" + IP_ADDRESS_REGEX + PORT_NUMBER_REGEX + "(, " + IP_ADDRESS_REGEX + PORT_NUMBER_REGEX + ")*$");
+
+    /**
+     * The credentials supplied by the user when they authenticated.
+     */
+    private final Credentials creds;
+
+    /**
+     * Creates a new RemoteAddress representing the source address of the HTTP
+     * request that provided the given Credentials.
+     *
+     * @param creds
+     *     The Credentials associated with the request whose source address
+     *     should be represented by this RemoteAddress.
+     */
+    public RemoteAddress(Credentials creds) {
+        this.creds = creds;
+    }
+
+    @Override
+    public String toString() {
+
+        HttpServletRequest request = creds.getRequest();
+        if (request == null)
+            return creds.getRemoteAddress();
+
+        // Log X-Forwarded-For, if present and valid
+        String header = request.getHeader("X-Forwarded-For");
+        if (header != null && X_FORWARDED_FOR.matcher(header).matches())
+            return "[" + header + ", " + request.getRemoteAddr() + "]";
+
+        // If header absent or invalid, just use source IP
+        return request.getRemoteAddr();
+
+    }
+
+}

guacamole/src/main/java/org/apache/guacamole/rest/auth/AuthenticationService.java

@@ -21,9 +21,7 @@ package org.apache.guacamole.rest.auth;
 
 import java.util.ArrayList;
 import java.util.List;
-import java.util.regex.Pattern;
 import javax.inject.Inject;
-import javax.servlet.http.HttpServletRequest;
 
 import org.apache.guacamole.GuacamoleException;
 import org.apache.guacamole.GuacamoleSecurityException;
@@ -98,57 +96,6 @@ public class AuthenticationService {
      */
     public static final String TOKEN_PARAMETER_NAME = "token";
 
-    /**
-     * Regular expression which matches any IPv4 address.
-     */
-    private static final String IPV4_ADDRESS_REGEX = "([0-9]{1,3}\\.[0-9]{1,3}\\.[0-9]{1,3}\\.[0-9]{1,3})";
-
-    /**
-     * Regular expression which matches any IPv6 address.
-     */
-    private static final String IPV6_ADDRESS_REGEX = "([0-9a-fA-F]*(:[0-9a-fA-F]*){0,7})";
-
-    /**
-     * Regular expression which matches any IP address, regardless of version.
-     */
-    private static final String IP_ADDRESS_REGEX = "(" + IPV4_ADDRESS_REGEX + "|" + IPV6_ADDRESS_REGEX + ")";
-
-    /**
-     * Regular expression which matches any Port Number.
-     */
-    private static final String PORT_NUMBER_REGEX = "(:[0-9]{1,5})?";
-    
-    /**
-     * Pattern which matches valid values of the de-facto standard
-     * "X-Forwarded-For" header.
-     */
-    private static final Pattern X_FORWARDED_FOR = Pattern.compile("^" + IP_ADDRESS_REGEX + PORT_NUMBER_REGEX + "(, " + IP_ADDRESS_REGEX + PORT_NUMBER_REGEX + ")*$");
-
-    /**
-     * Returns a formatted string containing an IP address, or list of IP
-     * addresses, which represent the HTTP client and any involved proxies. As
-     * the headers used to determine proxies can easily be forged, this data is
-     * superficially validated to ensure that it at least looks like a list of
-     * IPs.
-     *
-     * @param request
-     *     The HTTP request to format.
-     *
-     * @return
-     *     A formatted string containing one or more IP addresses.
-     */
-    private String getLoggableAddress(HttpServletRequest request) {
-
-        // Log X-Forwarded-For, if present and valid
-        String header = request.getHeader("X-Forwarded-For");
-        if (header != null && X_FORWARDED_FOR.matcher(header).matches())
-            return "[" + header + ", " + request.getRemoteAddr() + "]";
-
-        // If header absent or invalid, just use source IP
-        return request.getRemoteAddr();
-
-    }
-
     /**
      * Attempts authentication against all AuthenticationProviders, in order,
      * using the provided credentials. The first authentication failure takes
@@ -437,12 +384,12 @@ public class AuthenticationService {
             else {
                 authToken = authTokenGenerator.getToken();
                 tokenSessionMap.put(authToken, new GuacamoleSession(listenerService, authenticatedUser, userContexts));
-                logger.debug("Login was successful for user \"{}\".", authenticatedUser.getIdentifier());
             }
 
             // Report authentication success
             try {
-                listenerService.handleEvent(new AuthenticationSuccessEvent(authenticatedUser));
+                listenerService.handleEvent(new AuthenticationSuccessEvent(authenticatedUser,
+                        existingSession != null));
             }
             catch (GuacamoleException e) {
                 throw new GuacamoleAuthenticationProcessException("User "
@@ -454,25 +401,9 @@ public class AuthenticationService {
         // Log and rethrow any authentication errors
         catch (GuacamoleAuthenticationProcessException e) {
 
-            // Get request and username for sake of logging
-            HttpServletRequest request = credentials.getRequest();
-            String username = credentials.getUsername();
-
             listenerService.handleEvent(new AuthenticationFailureEvent(credentials,
                     e.getAuthenticationProvider(), e.getCause()));
 
-            // Log authentication failures with associated usernames
-            if (username != null) {
-                if (logger.isWarnEnabled())
-                    logger.warn("Authentication attempt from {} for user \"{}\" failed.",
-                            getLoggableAddress(request), username);
-            }
-
-            // Log anonymous authentication failures
-            else if (logger.isDebugEnabled())
-                logger.debug("Anonymous authentication attempt from {} failed.",
-                        getLoggableAddress(request));
-
             // Rethrow exception
             e.rethrowCause();
 
@@ -490,11 +421,6 @@ public class AuthenticationService {
 
         }
 
-        if (logger.isInfoEnabled())
-            logger.info("User \"{}\" successfully authenticated from {}.",
-                    authenticatedUser.getIdentifier(),
-                    getLoggableAddress(credentials.getRequest()));
-
         return authToken;
 
     }