You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "lh0156 (via GitHub)" <gi...@apache.org> on 2026/05/16 03:02:44 UTC
[PR] KAFKA-19790: Parse space-delimited OAuth scope claim strings [kafka]
lh0156 opened a new pull request, #22298:
URL: https://github.com/apache/kafka/pull/22298
### Summary
- Parse string-valued OAuth scope claims as space-delimited scope tokens in broker and client JWT validators.
- Reuse the same parsing for unsecured JWS string scope claims.
- Preserve existing behavior for collection-valued scope claims and existing blank-scope validation semantics.
- Add focused unit coverage for configured scope claim names, blank string scopes, parser behavior, and unsecured JWS scope immutability.
### Motivation
OAuth scope values are commonly represented as a space-delimited string such as `"email profile phone"`. Previously, the secured validators wrapped a string-valued scope claim as a single collection item, so Kafka treated that example as one scope instead of three.
This patch normalizes only string-valued scope claims before the existing validation path. Collection-valued scope claims continue to use the existing behavior.
### Testing
- `./gradlew :clients:unitTest --tests org.apache.kafka.common.security.oauthbearer.BrokerJwtValidatorTest --tests org.apache.kafka.common.security.oauthbearer.ClientJwtValidatorTest --tests org.apache.kafka.common.security.oauthbearer.internals.unsecured.OAuthBearerUnsecuredJwsTest --tests org.apache.kafka.common.security.oauthbearer.internals.OAuthBearerScopeClaimUtilsTest`
- `./gradlew :clients:checkstyleMain :clients:checkstyleTest`
- `git diff --check`
- `./gradlew :clients:unitTest`
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19790: Parse space-delimited OAuth scope claim strings [kafka]
Posted by "lh0156 (via GitHub)" <gi...@apache.org>.
lh0156 commented on PR #22298:
URL: https://github.com/apache/kafka/pull/22298#issuecomment-5159309030
Follow-up on the blank scope behavior:
I adopted the suggested RFC-consistent behavior. Whitespace-only string-valued scope claims now produce an empty scope set in both the broker and client JWT validators, matching the parser and unsecured-JWS behavior. The regression tests were changed first and reproduced the previous validation failure (RED); implementation commit `d91a575747` makes both tests pass (GREEN).
Verification:
- targeted blank-scope tests for BrokerJwtValidator and ClientJwtValidator passed
- the existing parser, secured-validator, and unsecured-JWS coverage remains in place
@kirktrue, please re-review the updated behavior.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19790: Parse space-delimited OAuth scope claim strings [kafka]
Posted by "lh0156 (via GitHub)" <gi...@apache.org>.
lh0156 commented on PR #22298:
URL: https://github.com/apache/kafka/pull/22298#issuecomment-4585372879
Hi @kirktrue, I updated the blank string scope handling and CI is green. Could you take another look when you have a chance?
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19790: Parse space-delimited OAuth scope claim strings [kafka]
Posted by "ezhou413 (via GitHub)" <gi...@apache.org>.
ezhou413 commented on code in PR #22298:
URL: https://github.com/apache/kafka/pull/22298#discussion_r3815743530
##########
clients/src/main/java/org/apache/kafka/common/security/oauthbearer/internals/OAuthBearerScopeClaimUtils.java:
##########
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kafka.common.security.oauthbearer.internals;
Review Comment:
I see `OAuthBearerScopeUtils.parseScope` in the same package as this new file, so to me this seems to be duplicating existing code. Is there a reason for adding this that I don't see?
##########
clients/src/main/java/org/apache/kafka/common/security/oauthbearer/BrokerJwtValidator.java:
##########
@@ -19,6 +19,7 @@
import org.apache.kafka.common.annotation.InterfaceAudience;
import org.apache.kafka.common.config.ConfigException;
+import org.apache.kafka.common.security.oauthbearer.internals.OAuthBearerScopeClaimUtils;
Review Comment:
Similar to my last comment, I think we should use existing code: `OAuthBearerScopeUtils.parseScope`
##########
clients/src/test/java/org/apache/kafka/common/security/oauthbearer/ClientJwtValidatorTest.java:
##########
@@ -55,4 +59,100 @@ void testJwtRequiresBase64UrlDecoding() throws Exception {
);
}
}
+
+ @Test
+ void testSpaceDelimitedStringScopesProcessedAccordingToRfc6749() throws Exception {
Review Comment:
It looks like you have duplicated tests in `BrokerJwtValidatorTest` and `ClientJwtValidatorTest`. Could we consolidate the tests into `JwtValidatorTest`? That file seems like the right place for these
##########
clients/src/main/java/org/apache/kafka/common/security/oauthbearer/ClientJwtValidator.java:
##########
@@ -18,6 +18,7 @@
package org.apache.kafka.common.security.oauthbearer;
import org.apache.kafka.common.annotation.InterfaceAudience;
+import org.apache.kafka.common.security.oauthbearer.internals.OAuthBearerScopeClaimUtils;
Review Comment:
Same here
##########
clients/src/main/java/org/apache/kafka/common/security/oauthbearer/internals/unsecured/OAuthBearerUnsecuredJws.java:
##########
@@ -345,10 +346,9 @@ private Set<String> calculateScope() {
if (isClaimType(scopeClaimName, String.class)) {
String scopeClaimValue = claim(scopeClaimName, String.class);
if (Utils.isBlank(scopeClaimValue))
- return Set.of();
- else {
- return Set.of(scopeClaimValue.trim());
- }
+ return Collections.emptySet();
Review Comment:
What's the reason for this change? `Set.of` is preferred over the `Collections` methods
##########
clients/src/test/java/org/apache/kafka/common/security/oauthbearer/internals/unsecured/OAuthBearerUnsecuredJwsTest.java:
##########
@@ -144,6 +232,14 @@ private static String scopeJson(List<String> scope) {
return scopeJsonBuilder.toString();
}
+ private static String stringScopeJson(String scope) {
Review Comment:
this seems to be dead code
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19790: Parse space-delimited OAuth scope claim strings [kafka]
Posted by "lh0156 (via GitHub)" <gi...@apache.org>.
lh0156 commented on code in PR #22298:
URL: https://github.com/apache/kafka/pull/22298#discussion_r3288100451
##########
clients/src/test/java/org/apache/kafka/common/security/oauthbearer/internals/OAuthBearerScopeClaimUtilsTest.java:
##########
@@ -0,0 +1,56 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kafka.common.security.oauthbearer.internals;
+
+import org.junit.jupiter.api.Test;
+
+import java.util.List;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+public class OAuthBearerScopeClaimUtilsTest {
+
+ @Test
+ public void testParseSpaceDelimitedScopeClaimSplitsScopeClaim() {
+ assertEquals(List.of("email", "profile", "phone"),
+ OAuthBearerScopeClaimUtils.parseSpaceDelimitedScopeClaim("email profile phone"));
+ }
+
+ @Test
+ public void testParseSpaceDelimitedScopeClaimPreservesSingleScope() {
+ assertEquals(List.of("email"),
+ OAuthBearerScopeClaimUtils.parseSpaceDelimitedScopeClaim("email"));
+ }
+
+ @Test
+ public void testParseSpaceDelimitedScopeClaimTrimsClaimBeforeSplitting() {
+ assertEquals(List.of("email", "profile", "phone"),
+ OAuthBearerScopeClaimUtils.parseSpaceDelimitedScopeClaim(" email profile phone "));
+ }
+
+ @Test
+ public void testParseSpaceDelimitedScopeClaimDoesNotEmitEmptyScopeItems() {
+ assertEquals(List.of("email", "profile", "phone"),
+ OAuthBearerScopeClaimUtils.parseSpaceDelimitedScopeClaim("email profile phone"));
+ }
+
+ @Test
+ public void testParseSpaceDelimitedScopeClaimPreservesBlankClaimForCallerValidation() {
+ assertEquals(List.of(" "), OAuthBearerScopeClaimUtils.parseSpaceDelimitedScopeClaim(" "));
Review Comment:
Thanks for catching this. I verified the RFC 6749 scope grammar and agree that a whitespace-only scope claim does not contain any scope-token, so the parser should not emit the original blank string as a scope item.
I updated `parseSpaceDelimitedScopeClaim(" ")` to return an empty list and adjusted the unit test accordingly. I also kept the secured validator behavior explicit: blank string-valued scope claims are still rejected instead of being silently treated as a valid no-scope claim.
I also corrected the related test names to reference RFC 6749.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19790: Parse space-delimited OAuth scope claim strings [kafka]
Posted by "ezhou413 (via GitHub)" <gi...@apache.org>.
ezhou413 commented on PR #22298:
URL: https://github.com/apache/kafka/pull/22298#issuecomment-5685938617
@lh0156 any updates for this PR?
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19790: Parse space-delimited OAuth scope claim strings [kafka]
Posted by "ezhou413 (via GitHub)" <gi...@apache.org>.
ezhou413 commented on PR #22298:
URL: https://github.com/apache/kafka/pull/22298#issuecomment-5799944251
Hi @lh0156 since there has been no activity on this PR for over a month, I'll be taking over this ticket
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19790: Parse space-delimited OAuth scope claim strings [kafka]
Posted by "kirktrue (via GitHub)" <gi...@apache.org>.
kirktrue commented on code in PR #22298:
URL: https://github.com/apache/kafka/pull/22298#discussion_r3261337521
##########
clients/src/test/java/org/apache/kafka/common/security/oauthbearer/internals/OAuthBearerScopeClaimUtilsTest.java:
##########
@@ -0,0 +1,56 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kafka.common.security.oauthbearer.internals;
+
+import org.junit.jupiter.api.Test;
+
+import java.util.List;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+public class OAuthBearerScopeClaimUtilsTest {
+
+ @Test
+ public void testParseSpaceDelimitedScopeClaimSplitsScopeClaim() {
+ assertEquals(List.of("email", "profile", "phone"),
+ OAuthBearerScopeClaimUtils.parseSpaceDelimitedScopeClaim("email profile phone"));
+ }
+
+ @Test
+ public void testParseSpaceDelimitedScopeClaimPreservesSingleScope() {
+ assertEquals(List.of("email"),
+ OAuthBearerScopeClaimUtils.parseSpaceDelimitedScopeClaim("email"));
+ }
+
+ @Test
+ public void testParseSpaceDelimitedScopeClaimTrimsClaimBeforeSplitting() {
+ assertEquals(List.of("email", "profile", "phone"),
+ OAuthBearerScopeClaimUtils.parseSpaceDelimitedScopeClaim(" email profile phone "));
+ }
+
+ @Test
+ public void testParseSpaceDelimitedScopeClaimDoesNotEmitEmptyScopeItems() {
+ assertEquals(List.of("email", "profile", "phone"),
+ OAuthBearerScopeClaimUtils.parseSpaceDelimitedScopeClaim("email profile phone"));
+ }
+
+ @Test
+ public void testParseSpaceDelimitedScopeClaimPreservesBlankClaimForCallerValidation() {
+ assertEquals(List.of(" "), OAuthBearerScopeClaimUtils.parseSpaceDelimitedScopeClaim(" "));
Review Comment:
Is this the desired behavior per the RFC? I would think we would want it to be an empty list. Please verify.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19790: Parse space-delimited OAuth scope claim strings [kafka]
Posted by "smjn (via GitHub)" <gi...@apache.org>.
smjn commented on PR #22298:
URL: https://github.com/apache/kafka/pull/22298#issuecomment-4467988044
looks valid per https://datatracker.ietf.org/doc/html/rfc6749#section-3.3
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19790: Parse space-delimited OAuth scope claim strings [kafka]
Posted by "lh0156 (via GitHub)" <gi...@apache.org>.
lh0156 commented on code in PR #22298:
URL: https://github.com/apache/kafka/pull/22298#discussion_r3699762262
##########
clients/src/test/java/org/apache/kafka/common/security/oauthbearer/internals/OAuthBearerScopeClaimUtilsTest.java:
##########
@@ -0,0 +1,56 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kafka.common.security.oauthbearer.internals;
+
+import org.junit.jupiter.api.Test;
+
+import java.util.List;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+public class OAuthBearerScopeClaimUtilsTest {
+
+ @Test
+ public void testParseSpaceDelimitedScopeClaimSplitsScopeClaim() {
+ assertEquals(List.of("email", "profile", "phone"),
+ OAuthBearerScopeClaimUtils.parseSpaceDelimitedScopeClaim("email profile phone"));
+ }
+
+ @Test
+ public void testParseSpaceDelimitedScopeClaimPreservesSingleScope() {
+ assertEquals(List.of("email"),
+ OAuthBearerScopeClaimUtils.parseSpaceDelimitedScopeClaim("email"));
+ }
+
+ @Test
+ public void testParseSpaceDelimitedScopeClaimTrimsClaimBeforeSplitting() {
+ assertEquals(List.of("email", "profile", "phone"),
+ OAuthBearerScopeClaimUtils.parseSpaceDelimitedScopeClaim(" email profile phone "));
+ }
+
+ @Test
+ public void testParseSpaceDelimitedScopeClaimDoesNotEmitEmptyScopeItems() {
+ assertEquals(List.of("email", "profile", "phone"),
+ OAuthBearerScopeClaimUtils.parseSpaceDelimitedScopeClaim("email profile phone"));
+ }
+
+ @Test
+ public void testParseSpaceDelimitedScopeClaimPreservesBlankClaimForCallerValidation() {
+ assertEquals(List.of(" "), OAuthBearerScopeClaimUtils.parseSpaceDelimitedScopeClaim(" "));
Review Comment:
Follow-up after rechecking the RFC behavior: the secured broker and client validators now treat a whitespace-only string scope claim as an empty scope set, matching the parser and unsecured JWS behavior. The RED test reproduced the previous rejection; the GREEN implementation and focused OAuth unit tests now pass. Please resolve this thread if the updated behavior is acceptable.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org