kirktrue commented on code in PR #19754:
URL: https://github.com/apache/kafka/pull/19754#discussion_r2114876525


##########
clients/src/main/java/org/apache/kafka/common/security/oauthbearer/BrokerJwtValidator.java:
##########
@@ -61,63 +77,50 @@
  *     </li>
  * </ol>
  */
-
 public class BrokerJwtValidator implements JwtValidator {
 
     private static final Logger log = 
LoggerFactory.getLogger(BrokerJwtValidator.class);
 
-    private final JwtConsumer jwtConsumer;
+    private final Optional<CloseableVerificationKeyResolver> 
verificationKeyResolverOpt;
 
-    private final String scopeClaimName;
+    private JwtConsumer jwtConsumer;
 
-    private final String subClaimName;
+    private String scopeClaimName;
+
+    private String subClaimName;
 
     /**
-     * Creates a new {@code BrokerJwtValidator} that will be used by the 
broker for more
-     * thorough validation of the JWT.
-     *
-     * @param clockSkew               The optional value (in seconds) to allow 
for differences
-     *                                between the time of the OAuth/OIDC 
identity provider and
-     *                                the broker. If <code>null</code> is 
provided, the broker
-     *                                and the OAUth/OIDC identity provider are 
assumed to have
-     *                                very close clock settings.
-     * @param expectedAudiences       The (optional) set the broker will use 
to verify that
-     *                                the JWT was issued for one of the 
expected audiences.
-     *                                The JWT will be inspected for the 
standard OAuth
-     *                                <code>aud</code> claim and if this value 
is set, the
-     *                                broker will match the value from JWT's 
<code>aud</code>
-     *                                claim to see if there is an <b>exact</b> 
match. If there is no
-     *                                match, the broker will reject the JWT 
and authentication
-     *                                will fail. May be <code>null</code> to 
not perform any
-     *                                check to verify the JWT's 
<code>aud</code> claim matches any
-     *                                fixed set of known/expected audiences.
-     * @param expectedIssuer          The (optional) value for the broker to 
use to verify that
-     *                                the JWT was created by the expected 
issuer. The JWT will
-     *                                be inspected for the standard OAuth 
<code>iss</code> claim
-     *                                and if this value is set, the broker 
will match it
-     *                                <b>exactly</b> against what is in the 
JWT's <code>iss</code>
-     *                                claim. If there is no match, the broker 
will reject the JWT
-     *                                and authentication will fail. May be 
<code>null</code> to not
-     *                                perform any check to verify the JWT's 
<code>iss</code> claim
-     *                                matches a specific issuer.
-     * @param verificationKeyResolver jose4j-based {@link 
VerificationKeyResolver} that is used
-     *                                to validate the signature matches the 
contents of the header
-     *                                and payload
-     * @param scopeClaimName          Name of the scope claim to use; must be 
non-<code>null</code>
-     * @param subClaimName            Name of the subject claim to use; must be
-     *                                non-<code>null</code>
-     *
-     * @see JwtConsumerBuilder
-     * @see JwtConsumer
-     * @see VerificationKeyResolver
+     * A public, no-args constructor is necessary for instantiation via 
configuration.
      */
+    public BrokerJwtValidator() {
+        this.verificationKeyResolverOpt = Optional.empty();
+    }
+
+    /*
+     * Package-visible for testing.
+     */
+    BrokerJwtValidator(CloseableVerificationKeyResolver 
verificationKeyResolver) {
+        this.verificationKeyResolverOpt = Optional.of(verificationKeyResolver);
+    }
+
+    @Override
+    public void configure(Map<String, ?> configs, String saslMechanism, 
List<AppConfigurationEntry> jaasConfigEntries) {
+        ConfigurationUtils cu = new ConfigurationUtils(configs, saslMechanism);
+        List<String> expectedAudiencesList = 
cu.get(SASL_OAUTHBEARER_EXPECTED_AUDIENCE);
+        Set<String> expectedAudiences = expectedAudiencesList != null ? 
Set.copyOf(expectedAudiencesList) : null;
+        Integer clockSkew = 
cu.validateInteger(SASL_OAUTHBEARER_CLOCK_SKEW_SECONDS, false);
+        String expectedIssuer = 
cu.validateString(SASL_OAUTHBEARER_EXPECTED_ISSUER, false);
+        String scopeClaimName = 
cu.validateString(SASL_OAUTHBEARER_SCOPE_CLAIM_NAME);
+        String subClaimName = 
cu.validateString(SASL_OAUTHBEARER_SUB_CLAIM_NAME);
+
+        CloseableVerificationKeyResolver verificationKeyResolver = null;
+
+        if (verificationKeyResolverOpt.isPresent()) {
+            verificationKeyResolver = verificationKeyResolverOpt.get();
+        } else {
+            verificationKeyResolver = 
VerificationKeyResolverFactory.get(configs, saslMechanism, jaasConfigEntries);
+        }

Review Comment:
   Fixed.



-- 
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: jira-unsubscr...@kafka.apache.org

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

Reply via email to