mimaison commented on code in PR #15475:
URL: https://github.com/apache/kafka/pull/15475#discussion_r1663905062
##########
clients/src/main/java/org/apache/kafka/common/config/SaslConfigs.java:
##########
@@ -192,6 +192,10 @@ public class SaslConfigs {
+ " be inspected for the standard OAuth \"iss\" claim and if this
value is set, the broker will match it exactly against what is in the JWT's
\"iss\" claim. If there is no"
+ " match, the broker will reject the JWT and authentication will
fail.";
+ public static final String SASL_OAUTHBEARER_HEADER_URLENCODE =
"sasl.oauthbearer.header.urlencode";
+ public static final boolean DEFAULT_SASL_OAUTHBEARER_HEADER_URLENCODE =
false;
+ public static final String SASL_OAUTHBEARER_HEADER_URLENCODE_DOC = "The
(optional) setting to enable the OAuth client to URL-encode the client_id and
client_secret in the authorization header"
+ + " in accordance with RFC6749, see
https://datatracker.ietf.org/doc/html/rfc6749#section-2.3.1 for more detail.
The default value is set to 'false' for backward compatibility";
Review Comment:
Configuration docs are rendered in HTML on the website. Can you wrap the
link into an `a` HTML tag?
##########
clients/src/test/java/org/apache/kafka/common/security/oauthbearer/internals/secured/HttpAccessTokenRetrieverTest.java:
##########
@@ -174,33 +176,39 @@ public void testParseAccessTokenInvalidJson() {
}
@Test
- public void testFormatAuthorizationHeader() {
- assertAuthorizationHeader("id", "secret");
+ public void testFormatAuthorizationHeader() throws
UnsupportedEncodingException {
+ assertAuthorizationHeader("id", "secret", false);
}
@Test
- public void testFormatAuthorizationHeaderEncoding() {
+ public void testFormatAuthorizationHeaderEncoding() throws
UnsupportedEncodingException {
// See KAFKA-14496
- assertAuthorizationHeader("SOME_RANDOM_LONG_USER_01234",
"9Q|0`8i~ute-n9ksjLWb\\50\"AX@UUED5E");
+ assertAuthorizationHeader("SOME_RANDOM_LONG_USER_01234",
"9Q|0`8i~ute-n9ksjLWb\\50\"AX@UUED5E", false);
+ // See KAFKA-16345
+ assertAuthorizationHeader("user!@~'", "secret-(*)!", true);
}
- private void assertAuthorizationHeader(String clientId, String
clientSecret) {
+ private void assertAuthorizationHeader(String clientId, String
clientSecret, boolean urlencode) throws UnsupportedEncodingException {
Review Comment:
This is a bit weird that this duplicates
`HttpAccessTokenRetriever.formatAuthorizationHeader()` as this simply asserts
that running the same logic produces the same output
##########
clients/src/test/java/org/apache/kafka/common/security/oauthbearer/internals/secured/HttpAccessTokenRetrieverTest.java:
##########
@@ -174,33 +176,39 @@ public void testParseAccessTokenInvalidJson() {
}
@Test
- public void testFormatAuthorizationHeader() {
- assertAuthorizationHeader("id", "secret");
+ public void testFormatAuthorizationHeader() throws
UnsupportedEncodingException {
+ assertAuthorizationHeader("id", "secret", false);
}
@Test
- public void testFormatAuthorizationHeaderEncoding() {
+ public void testFormatAuthorizationHeaderEncoding() throws
UnsupportedEncodingException {
// See KAFKA-14496
- assertAuthorizationHeader("SOME_RANDOM_LONG_USER_01234",
"9Q|0`8i~ute-n9ksjLWb\\50\"AX@UUED5E");
+ assertAuthorizationHeader("SOME_RANDOM_LONG_USER_01234",
"9Q|0`8i~ute-n9ksjLWb\\50\"AX@UUED5E", false);
+ // See KAFKA-16345
Review Comment:
I see you're copying what is already done before but it's better to put the
relevant details directly in the comment instead of mentioning a Jira.
--
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
