[GitHub] [kafka] C0urante commented on a change in pull request #8357: KAFKA-9767: Add logging to basic auth rest extension

[GitBox <gi...@apache.org>]

C0urante commented on a change in pull request #8357:
URL: https://github.com/apache/kafka/pull/8357#discussion_r421859386



##########
File path: connect/basic-auth-extension/src/main/java/org/apache/kafka/connect/rest/basic/auth/extension/JaasBasicAuthFilter.java
##########
@@ -67,36 +84,61 @@ public void filter(ContainerRequestContext requestContext) throws IOException {
         private String password;
 
         public BasicAuthCallBackHandler(String credentials) {
-            if (credentials != null) {
-                int space = credentials.indexOf(SPACE);
-                if (space > 0) {
-                    String method = credentials.substring(0, space);
-                    if (BASIC.equalsIgnoreCase(method)) {
-                        credentials = credentials.substring(space + 1);
-                        credentials = new String(Base64.getDecoder().decode(credentials),
-                                                 StandardCharsets.UTF_8);
-                        int i = credentials.indexOf(COLON);
-                        if (i > 0) {
-                            username = credentials.substring(0, i);
-                            password = credentials.substring(i + 1);
-                        }
-                    }
-                }
+            if (credentials == null) {
+                log.trace("No credentials were provided with the request");
+                return;
             }
+
+            int space = credentials.indexOf(SPACE);
+            if (space <= 0) {
+                log.trace("Request credentials were malformed; no space present in value for authorization header");
+                return;
+            }
+
+            String method = credentials.substring(0, space);
+            if (!BASIC.equalsIgnoreCase(method)) {
+                log.trace("Request credentials did not use basic authentication; ignoring");
+                return;
+            }
+
+            credentials = credentials.substring(space + 1);
+            credentials = new String(Base64.getDecoder().decode(credentials),
+                                     StandardCharsets.UTF_8);
+            int i = credentials.indexOf(COLON);
+            if (i <= 0) {
+                log.trace("Request credentials were malformed; no colon present between username and password");
+                return;
+            }
+
+            username = credentials.substring(0, i);
+            password = credentials.substring(i + 1);
         }
 
         @Override
         public void handle(Callback[] callbacks) throws UnsupportedCallbackException {
+            Callback unsupportedCallback = null;
             for (Callback callback : callbacks) {
                 if (callback instanceof NameCallback) {
                     ((NameCallback) callback).setName(username);
                 } else if (callback instanceof PasswordCallback) {
                     ((PasswordCallback) callback).setPassword(password.toCharArray());
                 } else {
-                    throw new UnsupportedCallbackException(callback, "Supports only NameCallback "
-                                                                     + "and PasswordCallback");
+                    // Log at WARN level here as this indicates incompatibility between the Connect basic auth
+                    // extension and the JAAS login module that the user has configured and it is likely that the
+                    // worker will need to be reconfigured and restarted
+                    log.warn(
+                        "Asked to handle unsupported callback '{}' of type {}; request authentication will fail",
+                        callback,
+                        callback.getClass()
+                    );
+                    if (unsupportedCallback == null)
+                        unsupportedCallback = callback;
                 }
             }
+            if (unsupportedCallback != null)
+                throw new UnsupportedCallbackException(
+                    unsupportedCallback,
+                    "Supports only NameCallback and PasswordCallback");

Review comment:
       I wanted to preserve the content of the original exception message here. If it's fine to change it, then yes, it'd definitely be an improvement to include the whole list here.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org