bachmanity1 commented on code in PR #15475:
URL: https://github.com/apache/kafka/pull/15475#discussion_r1669337234
##########
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() {
- // See KAFKA-14496
- assertAuthorizationHeader("SOME_RANDOM_LONG_USER_01234",
"9Q|0`8i~ute-n9ksjLWb\\50\"AX@UUED5E");
+ public void testFormatAuthorizationHeaderEncoding() throws
UnsupportedEncodingException {
+ // according to RFC-7617, we need to use the *non-URL safe* base64
encoder. See KAFKA-14496.
+ assertAuthorizationHeader("SOME_RANDOM_LONG_USER_01234",
"9Q|0`8i~ute-n9ksjLWb\\50\"AX@UUED5E", false);
+ // according to RFC-6749 clientId & clientSecret must be urlencoded,
see https://tools.ietf.org/html/rfc6749#section-2.3.1
+ assertAuthorizationHeader("user!@~'", "secret-(*)!", true);
}
- private void assertAuthorizationHeader(String clientId, String
clientSecret) {
+ private void assertAuthorizationHeader(String clientId, String
clientSecret, boolean urlencode) throws UnsupportedEncodingException {
+ String actual =
HttpAccessTokenRetriever.formatAuthorizationHeader(clientId, clientSecret,
urlencode);
+ if (urlencode) {
+ clientId = URLEncoder.encode(clientId,
StandardCharsets.UTF_8.name());
+ clientSecret = URLEncoder.encode(clientSecret,
StandardCharsets.UTF_8.name());
+ }
String expected = "Basic " +
Base64.getEncoder().encodeToString(Utils.utf8(clientId + ":" + clientSecret));
- String actual =
HttpAccessTokenRetriever.formatAuthorizationHeader(clientId, clientSecret);
assertEquals(expected, actual, String.format("Expected the HTTP
Authorization header generated for client ID \"%s\" and client secret \"%s\" to
match", clientId, clientSecret));
}
@Test
public void testFormatAuthorizationHeaderMissingValues() {
- assertThrows(IllegalArgumentException.class, () ->
HttpAccessTokenRetriever.formatAuthorizationHeader(null, "secret"));
- assertThrows(IllegalArgumentException.class, () ->
HttpAccessTokenRetriever.formatAuthorizationHeader("id", null));
- assertThrows(IllegalArgumentException.class, () ->
HttpAccessTokenRetriever.formatAuthorizationHeader(null, null));
- assertThrows(IllegalArgumentException.class, () ->
HttpAccessTokenRetriever.formatAuthorizationHeader("", "secret"));
- assertThrows(IllegalArgumentException.class, () ->
HttpAccessTokenRetriever.formatAuthorizationHeader("id", ""));
- assertThrows(IllegalArgumentException.class, () ->
HttpAccessTokenRetriever.formatAuthorizationHeader("", ""));
- assertThrows(IllegalArgumentException.class, () ->
HttpAccessTokenRetriever.formatAuthorizationHeader(" ", "secret"));
- assertThrows(IllegalArgumentException.class, () ->
HttpAccessTokenRetriever.formatAuthorizationHeader("id", " "));
- assertThrows(IllegalArgumentException.class, () ->
HttpAccessTokenRetriever.formatAuthorizationHeader(" ", " "));
+ assertThrows(IllegalArgumentException.class, () ->
HttpAccessTokenRetriever.formatAuthorizationHeader(null, "secret", false));
+ assertThrows(IllegalArgumentException.class, () ->
HttpAccessTokenRetriever.formatAuthorizationHeader("id", null, false));
+ assertThrows(IllegalArgumentException.class, () ->
HttpAccessTokenRetriever.formatAuthorizationHeader(null, null, false));
+ assertThrows(IllegalArgumentException.class, () ->
HttpAccessTokenRetriever.formatAuthorizationHeader("", "secret", false));
+ assertThrows(IllegalArgumentException.class, () ->
HttpAccessTokenRetriever.formatAuthorizationHeader("id", "", false));
+ assertThrows(IllegalArgumentException.class, () ->
HttpAccessTokenRetriever.formatAuthorizationHeader("", "", false));
+ assertThrows(IllegalArgumentException.class, () ->
HttpAccessTokenRetriever.formatAuthorizationHeader(" ", "secret", false));
+ assertThrows(IllegalArgumentException.class, () ->
HttpAccessTokenRetriever.formatAuthorizationHeader("id", " ", false));
+ assertThrows(IllegalArgumentException.class, () ->
HttpAccessTokenRetriever.formatAuthorizationHeader(" ", " ", false));
Review Comment:
Since this method has 3 arguments and data types are different it's not very
straightforward to convert this method using `@ParameterizedTest`. I think it's
better to leave this method as it is because it's more readable this way.
--
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]
