logging(auth): adding optional logging to authentication exceptions (d…

…atahub-project#7929)

(cherry picked from commit b71baac)

metadata-service/auth-filter/src/main/java/com/datahub/auth/authentication/filter/AuthenticationFilter.java

@@ -44,6 +44,7 @@
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import lombok.extern.slf4j.Slf4j;
+import org.springframework.beans.factory.annotation.Value;
 import org.springframework.web.context.support.SpringBeanAutowiringSupport;
 
 import static com.datahub.authentication.AuthenticationConstants.*;
@@ -67,6 +68,9 @@ public class AuthenticationFilter implements Filter {
   @Named("dataHubTokenService")
   private StatefulTokenService _tokenService;
 
+  @Value("#{new Boolean('${authentication.logAuthenticatorExceptions}')}")
+  private boolean _logAuthenticatorExceptions;
+
   private AuthenticatorChain authenticatorChain;
 
   @Override
@@ -81,7 +85,7 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
     AuthenticationRequest context = buildAuthContext((HttpServletRequest) request);
     Authentication authentication = null;
     try {
-      authentication = this.authenticatorChain.authenticate(context);
+      authentication = this.authenticatorChain.authenticate(context, _logAuthenticatorExceptions);
     } catch (AuthenticationException e) {
       // For AuthenticationExpiredExceptions, terminate and provide that feedback to the user
       log.debug("Failed to authenticate request. Received an AuthenticationExpiredException from authenticator chain.",

metadata-service/auth-impl/src/main/java/com/datahub/authentication/authenticator/AuthenticatorChain.java

@@ -12,6 +12,8 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Objects;
+import java.util.stream.Collectors;
+
 import lombok.extern.slf4j.Slf4j;
 
 
@@ -43,10 +45,10 @@ public void register(@Nonnull final Authenticator authenticator) {
    * Returns null if {@link Authentication} cannot be resolved for the incoming request.
    */
   @Nullable
-  public Authentication authenticate(@Nonnull final AuthenticationRequest context) throws AuthenticationException {
+  public Authentication authenticate(@Nonnull final AuthenticationRequest context, boolean logExceptions) throws AuthenticationException {
     Objects.requireNonNull(context);
     ClassLoader contextClassLoader = Thread.currentThread().getContextClassLoader();
-    List<Pair<String, String>> authenticationFailures = new ArrayList<>();
+    List<Pair<String, Exception>> authenticationFailures = new ArrayList<>();
     for (final Authenticator authenticator : this.authenticators) {
       try {
         log.debug(String.format("Executing Authenticator with class name %s", authenticator.getClass().getCanonicalName()));
@@ -67,17 +69,24 @@ public Authentication authenticate(@Nonnull final AuthenticationRequest context)
         throw e;
       } catch (Exception e) {
         // Log as a normal error otherwise.
-        authenticationFailures.add(new Pair<>(authenticator.getClass().getCanonicalName(), e.getMessage()));
         log.debug(String.format(
-            "Caught exception while attempting to authenticate request using Authenticator %s",
-            authenticator.getClass().getCanonicalName()), e);
+                "Caught exception while attempting to authenticate request using Authenticator %s",
+                authenticator.getClass().getCanonicalName()), e);
+        authenticationFailures.add(new Pair<>(authenticator.getClass().getCanonicalName(), e));
       } finally {
         Thread.currentThread().setContextClassLoader(contextClassLoader);
       }
     }
     // No authentication resolved. Return null.
     if (!authenticationFailures.isEmpty()) {
-      log.warn("Authentication chain failed to resolve a valid authentication. Errors: {}", authenticationFailures);
+      List<Pair<String, String>> shortMessage = authenticationFailures.stream()
+              .peek(p -> {
+                if (logExceptions) {
+                  log.error("Error during {} authentication: ", p.getFirst(), p.getSecond());
+                }
+              })
+              .map(p -> Pair.of(p.getFirst(), p.getSecond().getMessage())).collect(Collectors.toList());
+      log.warn("Authentication chain failed to resolve a valid authentication. Errors: {}", shortMessage);
     }
     return null;
   }

metadata-service/auth-impl/src/test/java/com/datahub/authentication/authenticator/AuthenticatorChainTest.java

@@ -31,7 +31,7 @@ public void testAuthenticateSuccess() throws Exception {
     // Verify that the mock authentication is returned on Authenticate.
     final AuthenticationRequest mockContext = Mockito.mock(AuthenticationRequest.class);
 
-    Authentication result = authenticatorChain.authenticate(mockContext);
+    Authentication result = authenticatorChain.authenticate(mockContext, false);
 
     // Verify that the authentication matches the mock returned by authenticator1
     assertSame(result, mockAuthentication);
@@ -53,7 +53,7 @@ public void testAuthenticateFailure() throws Exception {
     // Verify that the mock authentication is returned on Authenticate.
     final AuthenticationRequest mockContext = Mockito.mock(AuthenticationRequest.class);
 
-    Authentication result = authenticatorChain.authenticate(mockContext);
+    Authentication result = authenticatorChain.authenticate(mockContext, false);
 
     // If the authenticator throws, verify that null is returned to indicate failure to authenticate.
     assertNull(result);
@@ -71,6 +71,6 @@ public void testAuthenticateThrows() throws Exception {
     // Verify that the mock authentication is returned on Authenticate.
     final AuthenticationRequest mockContext = Mockito.mock(AuthenticationRequest.class);
 
-    assertThrows(AuthenticationExpiredException.class, () -> authenticatorChain.authenticate(mockContext));
+    assertThrows(AuthenticationExpiredException.class, () -> authenticatorChain.authenticate(mockContext, false));
   }
 }

metadata-service/factories/src/main/resources/application.yml

@@ -12,6 +12,9 @@ authentication:
         signingKey: ${DATAHUB_TOKEN_SERVICE_SIGNING_KEY:WnEdIeTG/VVCLQqGwC/BAkqyY0k+H8NEAtWGejrBI94=}
         salt: ${DATAHUB_TOKEN_SERVICE_SALT:ohDVbJBvHHVJh9S/UA4BYF9COuNnqqVhr9MLKEGXk1O=}
 
+  # Normally failures are only warnings, enable this to throw them.
+  logAuthenticatorExceptions: ${METADATA_SERVICE_AUTHENTICATOR_EXCEPTIONS_ENABLED:false}
+
   # Required separately from the DataHubTokenAuthenticator as this is used by the AuthServiceController to authorize token generation
   # at user login time.
   systemClientId: ${DATAHUB_SYSTEM_CLIENT_ID:__datahub_system}