KAFKA-18573: Add support for OAuth jwt-bearer grant type by kirktrue · Pull Request #19754 · apache/kafka

kirktrue · 2025-05-18T19:23:48Z

Adding support for the urn:ietf:params:oauth:grant-type:jwt-bearer
grant type (AKA jwt-bearer). Includes further refactoring of the
existing OAuth layer and addition of generic JWT assertion layer that
can be leveraged in the future.

This constitutes the main piece of the JWT Bearer grant type support.

Forthcoming commits/PRs will include improvements for both the
client_credentials and jwt-bearer grant types in the following
areas:

Integration test coverage (KAFKA-19153)
Unit test coverage (KAFKA-19308)
Top-level documentation (KAFKA-19152)
Improvements to and documentation for OAuthCompatibilityTool
(KAFKA-19307)

Reviewers: Manikumar Reddy manikumar@confluent.io, Lianet Magrans lmagrans@confluent.io, Chia-Ping Tsai chia7712@gmail.com

Adding support for the "urn:ietf:params:oauth:grant-type:jwt-bearer" grant type (AKA "jwt-bearer"). Includes further refactoring of the existing OAuth layer and addition of generic JWT assertion layer that can be leveraged in the future. Co-Authored-By: Zachary Hamilton <77027819+zacharydhamilton@users.noreply.github.com>

kirktrue · 2025-05-18T19:50:56Z

cc @lianetm @omkreddy @zacharydhamilton

github-actions · 2025-05-26T03:28:04Z

A label of 'needs-attention' was automatically added to this PR in order to raise the
attention of the committers. Once this issue has been triaged, the triage label
should be removed to prevent this automation from happening again.

github-actions · 2025-05-28T03:26:16Z

A label of 'needs-attention' was automatically added to this PR in order to raise the
attention of the committers. Once this issue has been triaged, the triage label
should be removed to prevent this automation from happening again.

emasab · 2025-05-28T15:02:16Z

+        if (algorithm.equalsIgnoreCase(TOKEN_SIGNING_ALGORITHM_RS256)) {
+            return Signature.getInstance("SHA256withRSA");
+        } else if (algorithm.equalsIgnoreCase(TOKEN_SIGNING_ALGORITHM_ES256)) {
+            return Signature.getInstance("SHA256withECDSA");
+        } else {
+            throw new NoSuchAlgorithmException(String.format("Unsupported signing algorithm: %s", algorithm));


To be future proof, could we support all JOSE hash lengths for RS and ES: RS256, RS384, RS512, ES256, ES384, ES512.

has this been addressed?

Yes. I spoke with @emasab offline and he said he's OK postponing additional algorithms until a later release.

….files

…ded a unit test for passphrases

lianetm

Thanks for the PR @kirktrue! Here are some comments from a partial review

lianetm · 2025-05-29T17:46:18Z

+    public static final String DEFAULT_CLIENT_SASL_OAUTHBEARER_JWT_VALIDATOR_CLASS = "org.apache.kafka.common.security.oauthbearer.ClientJwtValidator";
+    public static final String SASL_OAUTHBEARER_JWT_VALIDATOR_CLASS_DOC = "<p>The fully-qualified class name of a <code>JwtValidator</code> implementation used to"
+        + " validate the JWT from the identity provider.</p>"
+        + "<p>The default configuration value represents a class that maintains backward compatibility with previous versions of"


do we want to refer to the "default validator" here instead of "default configuration value"?? (referring to the "default configuration value" is confusing, because there is really no config for the DefaultJwtValidator right? (only for DEFAULT_BROKER.. and DEFAULT_CLIENT...)

lianetm · 2025-05-29T18:42:46Z

+        + "<p>The default configuration value represents a class that maintains backward compatibility with previous versions of"
+        + " Apache Kafka. The default implementation uses the configuration to determine which concrete implementation to create."
+        + "<p>Other implementations that are provided include:</p>"
+        + "<ul>"


Doesn't look like we use tags in config docs like these elsewhere (I may be missing something), and it surely shows polluted with the literal tags when looking at the java doc for this var usages. Is this intentional?

It is rare; there are only nine occurrences other than those in this PR. These two client configuration were my inspiration:

ConsumerConfig.PARTITION_ASSIGNMENT_STRATEGY_DOC

ProducerConfig.BATCH_SIZE_DOC

lianetm · 2025-05-29T18:50:00Z

+        + " included in the token request. If provided, it should match one or more scopes configured in the identity provider.</p>"
+        + "<p>"
+        + "The scope was previously stored as part of the <code>sasl.jaas.config</code> configuration with the key <code>scope</code>."
+        + " For backward compatibility, the <code>scope</code> JAAS option can still be used, but it is deprecated and will be removed in a future version."


sasl.jaas.config is not really marked as deprecated yet, right? Do we plan to do it as follow-up?

sasl.jaas.config itself is not deprecated. Just the OAuth layer's use of JAAS options.

I'm not entirely sure how to deprecate JAAS options because they're not listed anywhere explicitly like configuration are 🤔

I see, interesting case then. Should we then add a note on the docs of the sasl.jaas.config config itself? (so users know that the oauth options of it are deprecated and they should use the sasl.oauthbearer properties instead?

This is missing (Do we want to add the note the sasl.jaas.config? about the deprecated options and how the new props will take precedence)

There is a statement about that under that line:

Order of precedence:

sasl.oauthbearer.client.credentials.client.id from configuration

clientId from JAAS

Or are you referring to something else?

I was referring to a note on the jaas.config doc, to add the note on the one being deprecated, not only the one being added

lianetm · 2025-05-29T19:48:24Z

+ * should be specified with a listener-based property:
+ *
+ * <pre>
+ * listener.name.<listener name>.oauthbearer.sasl.oauthbearer.jwt.retriever.class=org.apache.kafka.common.security.oauthbearer.JwtBearerJwtRetriever


uhm having unescaped "<listener name>" within the <pre> will probably mess it up?

(ha ha, I had to escape the tags myself on this comment to show properly)

lianetm · 2025-05-29T20:10:07Z

+         * @param file     File containing the source data
+         * @param contents Data from file; could be zero length but not {@code null}
+         */
+        T transform(File file, String contents);


seems redundant that we need to pass the File and the contents, is it needed? (or could we consider passing the file only, and retrieve the contents from it?)

The File is used for metadata (size, last modified time, etc.) and the String is used for the actual contents.

Because CachedFile.snapshot() performs the file loading step and provides the result to transform(), it's less work for the different Transformer implementations.

…gs.java Co-authored-by: Lianet Magrans <98415067+lianetm@users.noreply.github.com>

…arer/DefaultJwtRetriever.java Co-authored-by: Lianet Magrans <98415067+lianetm@users.noreply.github.com>

…gs.java Co-authored-by: Lianet Magrans <98415067+lianetm@users.noreply.github.com>

…T_CLIENT_SASL_OAUTHBEARER_JWT_VALIDATOR_CLASS into DEFAULT_SASL_OAUTHBEARER_JWT_VALIDATOR_CLASS

…ssages

lianetm

Thanks @kirktrue ! A few more comments

lianetm · 2025-06-03T13:08:40Z

+        + " included in the token request. If provided, it should match one or more scopes configured in the identity provider.</p>"
+        + "<p>"
+        + "The scope was previously stored as part of the <code>sasl.jaas.config</code> configuration with the key <code>scope</code>."
+        + " For backward compatibility, the <code>scope</code> JAAS option can still be used, but it is deprecated and will be removed in a future version."


This is missing (Do we want to add the note the sasl.jaas.config? about the deprecated options and how the new props will take precedence)

lianetm · 2025-06-03T13:18:11Z

    public static final String SASL_OAUTHBEARER_TOKEN_ENDPOINT_URL = "sasl.oauthbearer.token.endpoint.url";
    public static final String SASL_OAUTHBEARER_TOKEN_ENDPOINT_URL_DOC = "The URL for the OAuth/OIDC identity provider. If the URL is HTTP(S)-based, it is the issuer's token"
-            + " endpoint URL to which requests will be made to login based on the configuration in " + SASL_JAAS_CONFIG + ". If the URL is file-based, it"
+            + " endpoint URL to which requests will be made to login based on the configured <code>JwtRetriever</code>. If the URL is file-based, it"


should we reference the SASL_OAUTHBEARER_JWT_RETRIEVER_CLASS config here instead of the class name? Seeing the related config name is probably more helpful (similar to what was done before for the old config)

Agreed. I made the change.

lianetm · 2025-06-03T13:36:40Z

+                                Function<String, String> configValueGetter,
+                                Function<String, String> jaasValueGetter) {
+            if (cu.containsKey(configName)) {
+                return configValueGetter.apply(configName);


did we consider logging a warn in case cu.containsKey(configName) && jou.containsKey(jaasName)?
Otherwise it will go silently that the user has 2 conflicting properties, and that the jaas oauth option will be ignored

Added more logging in this case. I struggled with the wording a bit, so happy to take suggestions.

lianetm · 2025-06-03T13:50:39Z

+    }
+
+    @Override
+    public void configure(Map<String, ?> configs, String saslMechanism, List<AppConfigurationEntry> jaasConfigEntries) {


I don't see any test for this configure logic on the JwtBearerJwtRetriever, just checking it is coming in the following PR?

I added a few unit tests for the configure().

lianetm · 2025-06-03T14:42:02Z

+        if (algorithm.equalsIgnoreCase(TOKEN_SIGNING_ALGORITHM_RS256)) {
+            return Signature.getInstance("SHA256withRSA");
+        } else if (algorithm.equalsIgnoreCase(TOKEN_SIGNING_ALGORITHM_ES256)) {
+            return Signature.getInstance("SHA256withECDSA");
+        } else {
+            throw new NoSuchAlgorithmException(String.format("Unsupported signing algorithm: %s", algorithm));


has this been addressed?

…plate to its configure() method

lianetm · 2025-06-03T20:33:42Z

+
+public class JwtResponseParser {
+
+    private static final String[] JSON_PATHS = new String[] {"/access_token", "/id_token"};


do we intend to use id_tokens? I was expecting only access_token since those are the ones intended for API authorization (but I could be missing why we are expecting to use ID tokens?)

According to the specs, access_token is correct. However, in my testing with GCP, the access_token was not present and only id_token was there as a JWT. Let me perform my testing against GCP again and see if I can get more clarity.

ok, let's get to the bottom of it, but we can take it as follow-up if it needs more investigation

This will need some more investigation. I tested against GCP and, depending on the claims that I include in the assertion JWT, the response includes either access_token or id_token.

According to the RFC it should return an access token, here it says:

This document defines how a JWT Bearer Token can be used to request
an access token when a client wishes to utilize an existing trust
relationship

so I think it's fine to look first for access_token and then for the id_token.

…he deprecated JAAS option

omkreddy

@kirktrue Thanks for the PR. LGTM

lianetm

Thanks! LGTM.

Adding support for the `urn:ietf:params:oauth:grant-type:jwt-bearer` grant type (AKA `jwt-bearer`). Includes further refactoring of the existing OAuth layer and addition of generic JWT assertion layer that can be leveraged in the future. This constitutes the main piece of the JWT Bearer grant type support. Forthcoming commits/PRs will include improvements for both the `client_credentials` and `jwt-bearer` grant types in the following areas: * Integration test coverage (KAFKA-19153) * Unit test coverage (KAFKA-19308) * Top-level documentation (KAFKA-19152) * Improvements to and documentation for `OAuthCompatibilityTool` (KAFKA-19307) Reviewers: Manikumar Reddy <manikumar@confluent.io>, Lianet Magrans <lmagrans@confluent.io> --------- Co-authored-by: Zachary Hamilton <77027819+zacharydhamilton@users.noreply.github.com> Co-authored-by: Lianet Magrans <98415067+lianetm@users.noreply.github.com>

chia7712 · 2026-07-02T20:09:13Z

    @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 = verificationKeyResolverOpt.orElseGet(
            () -> VerificationKeyResolverFactory.get(configs, saslMechanism, jaasConfigEntries)
        );

Sorry for bumping this old PR :)

Should we close verificationKeyResolver ? I observed it in reviewing #22520

github-actions Bot added triage PRs from the community tools clients labels May 18, 2025

m1a2st added the ci-approved label May 19, 2025

kirktrue added 4 commits May 20, 2025 07:41

Update OAuthCompatibilityTool.java

e2bbeaa

Merge branch 'trunk' into KAFKA-18573-add-oauth-jwt-bearer-support

b2bf48d

Hopefully this fixes whatever boneheaded thing I did before :(

371abe9

Added configuration to KafkaConfigTest.testFromPropsInvalid()

24e3bb7

github-actions Bot added the core Kafka Broker label May 20, 2025

github-actions Bot added the needs-attention label May 26, 2025

lianetm removed the needs-attention label May 27, 2025

github-actions Bot added the needs-attention label May 28, 2025

lianetm removed the needs-attention label May 28, 2025

emasab reviewed May 28, 2025

View reviewed changes

kirktrue added 2 commits May 28, 2025 13:59

Merge branch 'trunk' into KAFKA-18573-add-oauth-jwt-bearer-support

558292e

Added support and tests for org.apache.kafka.sasl.oauthbearer.allowed…

6d2b0e0

….files

github-actions Bot removed the triage PRs from the community label May 29, 2025

omkreddy reviewed May 29, 2025

View reviewed changes

Comment thread .../java/org/apache/kafka/common/security/oauthbearer/internals/secured/ConfigurationUtils.java Outdated

omkreddy reviewed May 29, 2025

View reviewed changes

Comment thread clients/src/main/java/org/apache/kafka/common/config/SaslConfigs.java Outdated

omkreddy reviewed May 29, 2025

View reviewed changes

Comment thread clients/src/main/java/org/apache/kafka/common/config/internals/BrokerSecurityConfigs.java Outdated

marcin-krystianc mentioned this pull request May 29, 2025

Add Support for OAuth JWT-Bearer Grant Type (aka Client Assertions) in librdkafka confluentinc/librdkafka#5096

Closed

Made the private key passphrase configuration of type PASSWORD and ad…

a0445a4

…ded a unit test for passphrases

lianetm reviewed May 29, 2025

View reviewed changes

kirktrue and others added 3 commits May 29, 2025 14:49

Update clients/src/main/java/org/apache/kafka/common/config/SaslConfi…

d63231e

…gs.java Co-authored-by: Lianet Magrans <98415067+lianetm@users.noreply.github.com>

Update clients/src/main/java/org/apache/kafka/common/security/oauthbe…

7f54250

…arer/DefaultJwtRetriever.java Co-authored-by: Lianet Magrans <98415067+lianetm@users.noreply.github.com>

Update clients/src/main/java/org/apache/kafka/common/config/SaslConfi…

3b03911

…gs.java Co-authored-by: Lianet Magrans <98415067+lianetm@users.noreply.github.com>

kirktrue added 7 commits May 29, 2025 14:55

Fixes for comments & documentation

6a67818

Merged DEFAULT_BROKER_SASL_OAUTHBEARER_JWT_VALIDATOR_CLASS and DEFAUL…

0c0b717

…T_CLIENT_SASL_OAUTHBEARER_JWT_VALIDATOR_CLASS into DEFAULT_SASL_OAUTHBEARER_JWT_VALIDATOR_CLASS

CloseableVerificationKeyResolver refactoring in BrokerJwtValidator

7d4b45b

Moved getConfiguredInstance to internals package and revised error me…

1a7f7ee

…ssages

Escaping < and > in JavaDoc

1fca538

Removed superfluous sasl.oauthbearer.grant.type configuration

36210c6

Semi-temporarily removing tests for private keys with passphrases

1b62c92

kirktrue requested review from lianetm and omkreddy June 3, 2025 00:20

lianetm reviewed Jun 3, 2025

View reviewed changes

kirktrue added 4 commits June 3, 2025 12:39

Updates to SASL config message

9eaf96a

Added/improved logging when both JAAS option and configuration present

abbc9b9

Split out HTTP response parsing into separate class

a60d3ca

Changing HttpJwtRetriever to implement JwtRetriever and moving boiler…

efd1aec

…plate to its configure() method

lianetm reviewed Jun 3, 2025

View reviewed changes

kirktrue and others added 3 commits June 3, 2025 13:47

Added a few unit tests for JwtBearerJwtRetriever's configure()

419d4b7

Make sure to remove any PRIVATE KEY header and footer

4ff56e4

Use the correct sasl.oauthbearer.scope configuration for scope, not t…

862121e

…he deprecated JAAS option

omkreddy approved these changes Jun 4, 2025

View reviewed changes

lianetm approved these changes Jun 4, 2025

View reviewed changes

lianetm merged commit 1e91790 into apache:trunk Jun 4, 2025
25 checks passed

kirktrue deleted the KAFKA-18573-add-oauth-jwt-bearer-support branch June 18, 2025 15:25


		public class JwtResponseParser {

		private static final String[] JSON_PATHS = new String[] {"/access_token", "/id_token"};

Uh oh!

Conversation

kirktrue commented May 18, 2025 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kirktrue commented May 18, 2025

Uh oh!

github-actions Bot commented May 26, 2025

Uh oh!

github-actions Bot commented May 28, 2025

Uh oh!

emasab May 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lianetm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lianetm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

kirktrue commented May 18, 2025 •

edited by github-actions Bot

Loading

emasab May 28, 2025 •

edited

Loading