chia7712 commented on code in PR #23557:
URL: https://github.com/apache/kafka/pull/23557#discussion_r4206274065
##########
clients/src/main/java/org/apache/kafka/common/security/oauthbearer/internals/unsecured/OAuthBearerUnsecuredLoginCallbackHandler.java:
##########
@@ -180,12 +180,20 @@ public void close() {
private void handleTokenCallback(OAuthBearerTokenCallback callback) {
if (callback.token() != null)
throw new IllegalArgumentException("Callback had a token already");
- if (moduleOptions.isEmpty()) {
- log.debug("Token not provided, this login cannot be used to
establish client connections");
+ if (!hasUnsecuredLoginOptions()) {
+ if (moduleOptions.isEmpty()) {
+ log.debug("Token not provided, this login cannot be used to
establish client connections");
+ } else {
+ // WARN (not DEBUG) so a mistyped unsecuredLogin* option isn't
silently ignored.
+ log.warn("Token not provided (no {}* options; ignoring options
{}), this login cannot be used to establish client connections",
Review Comment:
This path is what KAFKA-21143 describes, right? If so, that is a normal path
before we have an official tokenless login handler (KAFKA-21221). Hence, a
warning log is a bit too much I think. Maybe INFO is enough to catch a mistyped
option.
##########
clients/src/main/java/org/apache/kafka/common/security/oauthbearer/OAuthBearerLoginModule.java:
##########
@@ -318,7 +318,10 @@ private void identifyToken() throws LoginException {
callbackHandler.handle(new Callback[] {tokenCallback});
Review Comment:
`OAuthBearerLoginModule` is a public API, so we could enhance its docs to
make it clear that `OAuthBearerUnsecuredLoginCallbackHandler` is a
testing-purpose class, even when it is used for tokenless login.
--
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]
