gharris1727 commented on code in PR #12846:
URL: https://github.com/apache/kafka/pull/12846#discussion_r1024519136
##########
connect/basic-auth-extension/src/main/java/org/apache/kafka/connect/rest/basic/auth/extension/JaasBasicAuthFilter.java:
##########
@@ -174,4 +153,84 @@ public void handle(Callback[] callbacks) throws
UnsupportedCallbackException {
));
}
}
+
+ private void setSecurityContextForRequest(ContainerRequestContext
requestContext, BasicAuthenticationCredential credential) {
+ requestContext.setSecurityContext(new SecurityContext() {
+ @Override
+ public Principal getUserPrincipal() {
+ return () -> credential.getUsername();
+ }
+
+ @Override
+ public boolean isUserInRole(String role) {
+ return false;
+ }
+
+ @Override
+ public boolean isSecure() {
+ return
requestContext.getUriInfo().getRequestUri().getScheme().equalsIgnoreCase("https");
+ }
+
+ @Override
+ public String getAuthenticationScheme() {
+ return BASIC_AUTH;
+ }
+ });
+ }
+
+
+ public static class BasicAuthenticationCredential {
+ private String username;
+ private String password;
+
+ public BasicAuthenticationCredential(String authorizationHeader) {
+ final char colon = ':';
+ final char space = ' ';
+ final String authType = "basic";
+
+ initializeEmptyCredentials();
+
+ if (authorizationHeader == null) {
+ log.trace("No credentials were provided with the request");
+ return;
+ }
+
+ int spaceIndex = authorizationHeader.indexOf(space);
+ if (spaceIndex <= 0) {
+ log.trace("Request credentials were malformed; no space
present in value for authorization header");
+ return;
+ }
+
+ String method = authorizationHeader.substring(0, spaceIndex);
+ if (!authType.equalsIgnoreCase(method)) {
+ log.trace("Request credentials used {} authentication, but
only {} supported; ignoring", method, authType);
+ return;
+ }
+
+ authorizationHeader = authorizationHeader.substring(spaceIndex +
1);
+ authorizationHeader = new
String(Base64.getDecoder().decode(authorizationHeader),
+ StandardCharsets.UTF_8);
+ int i = authorizationHeader.indexOf(colon);
+ if (i <= 0) {
+ log.trace("Request credentials were malformed; no colon
present between username and password");
+ return;
+ }
+
+ this.username = authorizationHeader.substring(0, i);
+ this.password = authorizationHeader.substring(i + 1);
+ }
+
+ private void initializeEmptyCredentials() {
+ this.username = "";
+ this.password = "";
+ }
Review Comment:
This is different than the previous behavior. Previously, if the
authorizationHeader was null, the username/password would be null as well. Now
they are empty string.
Is this an intentional change?
##########
connect/basic-auth-extension/src/main/java/org/apache/kafka/connect/rest/basic/auth/extension/JaasBasicAuthFilter.java:
##########
@@ -174,4 +153,84 @@ public void handle(Callback[] callbacks) throws
UnsupportedCallbackException {
));
}
}
+
+ private void setSecurityContextForRequest(ContainerRequestContext
requestContext, BasicAuthenticationCredential credential) {
+ requestContext.setSecurityContext(new SecurityContext() {
+ @Override
+ public Principal getUserPrincipal() {
+ return () -> credential.getUsername();
+ }
+
+ @Override
+ public boolean isUserInRole(String role) {
+ return false;
+ }
+
+ @Override
+ public boolean isSecure() {
+ return
requestContext.getUriInfo().getRequestUri().getScheme().equalsIgnoreCase("https");
+ }
+
+ @Override
+ public String getAuthenticationScheme() {
+ return BASIC_AUTH;
+ }
+ });
+ }
+
+
+ public static class BasicAuthenticationCredential {
+ private String username;
+ private String password;
+
+ public BasicAuthenticationCredential(String authorizationHeader) {
+ final char colon = ':';
+ final char space = ' ';
+ final String authType = "basic";
Review Comment:
nit: these can remain static constants. Maybe we can even pull from
SecurityContext.BASIC_AUTH.
##########
connect/basic-auth-extension/src/main/java/org/apache/kafka/connect/rest/basic/auth/extension/JaasBasicAuthFilter.java:
##########
@@ -174,4 +153,84 @@ public void handle(Callback[] callbacks) throws
UnsupportedCallbackException {
));
}
}
+
+ private void setSecurityContextForRequest(ContainerRequestContext
requestContext, BasicAuthenticationCredential credential) {
+ requestContext.setSecurityContext(new SecurityContext() {
+ @Override
+ public Principal getUserPrincipal() {
+ return () -> credential.getUsername();
+ }
+
+ @Override
+ public boolean isUserInRole(String role) {
+ return false;
+ }
+
+ @Override
+ public boolean isSecure() {
+ return
requestContext.getUriInfo().getRequestUri().getScheme().equalsIgnoreCase("https");
Review Comment:
nit: I think getScheme() can return null in the case of relative URIs, in
which case we cannot determine from the URI whether the request is secure. I'm
not sure if this is a realistic situation or a contrived case.
Looking through the rest of these calls, I think everything else is non-null.
##########
connect/basic-auth-extension/src/main/java/org/apache/kafka/connect/rest/basic/auth/extension/JaasBasicAuthFilter.java:
##########
@@ -174,4 +153,84 @@ public void handle(Callback[] callbacks) throws
UnsupportedCallbackException {
));
}
}
+
+ private void setSecurityContextForRequest(ContainerRequestContext
requestContext, BasicAuthenticationCredential credential) {
+ requestContext.setSecurityContext(new SecurityContext() {
+ @Override
+ public Principal getUserPrincipal() {
Review Comment:
Note: The Javadoc indicates that this method should only return a non-null
Principal if authentication has already been set. This invariant is preserved
when `setSecurityContextForRequest` is called explicitly only after the
`LoginContext::login` method successfully exits, which it is.
:+1:
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
