C0urante commented on a change in pull request #8357:
URL: https://github.com/apache/kafka/pull/8357#discussion_r428260801
##########
File path:
connect/basic-auth-extension/src/main/java/org/apache/kafka/connect/rest/basic/auth/extension/JaasBasicAuthFilter.java
##########
@@ -67,36 +87,60 @@ 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 used {} authentication, but
only {} supported; ignoring", method, BASIC);
+ 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 {
+ List<Callback> unsupportedCallbacks = new ArrayList<>();
for (Callback callback : callbacks) {
if (callback instanceof NameCallback) {
((NameCallback) callback).setName(username);
} else if (callback instanceof PasswordCallback) {
- ((PasswordCallback)
callback).setPassword(password.toCharArray());
+ ((PasswordCallback) callback).setPassword(password != null
+ ? password.toCharArray()
+ : null
+ );
} else {
- throw new UnsupportedCallbackException(callback, "Supports
only NameCallback "
- + "and
PasswordCallback");
+ unsupportedCallbacks.add(callback);
}
}
+ if (!unsupportedCallbacks.isEmpty())
+ throw new ConnectException(String.format(
+ "Unsupported callbacks %s; request authentication will
fail. "
+ + "This indicates the Connect worker was configured
with a JAAS "
+ + "LoginModule that is incompatible with the %s, and
will need to be "
+ + "corrected and restarted.",
+ unsupportedCallbacks,
+ BasicAuthSecurityRestExtension.class.getSimpleName()
+ ));
Review comment:
Ack, added a new test for an unsupported callback. We have tests already
for the green path, so it didn't seem necessary to add more than that.
----------------------------------------------------------------
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:
us...@infra.apache.org
