gaurav singh has posted comments on this change. ( http://gerrit.cloudera.org:8080/21382 )
Change subject: IMPALA-12559: Support x5c Parameter for RSA JSON Web Keys ...................................................................... Patch Set 21: (8 comments) http://gerrit.cloudera.org:8080/#/c/21382/20/be/src/util/jwt-util-test.cc File be/src/util/jwt-util-test.cc: http://gerrit.cloudera.org:8080/#/c/21382/20/be/src/util/jwt-util-test.cc@460 PS20, Line 460: kid_x5c) > This code is using the string "kid_x5c" but should be using the variable ki Done http://gerrit.cloudera.org:8080/#/c/21382/20/be/src/util/jwt-util.cc File be/src/util/jwt-util.cc: http://gerrit.cloudera.org:8080/#/c/21382/20/be/src/util/jwt-util.cc@157 PS20, Line 157: if (!values[0].empty()) { > This statement needs an "else" clause. Added. Thanks. http://gerrit.cloudera.org:8080/#/c/21382/20/be/src/util/jwt-util.cc@158 PS20, Line 158: = values[0]; > enclose this line with { } to follow coding style. Done http://gerrit.cloudera.org:8080/#/c/21382/20/be/src/util/jwt-util.cc@258 PS20, Line 258: > Line 251 allow different json type, here assume it's string. Please keep co In Line 251, we assert that the json_value is of type array. and here we are extracting a string since value is of type cpp_type which is *string or []string. http://gerrit.cloudera.org:8080/#/c/21382/20/be/src/util/jwt-util.cc@395 PS20, Line 395: algorithm = "rs384"; > If the certificate algorithm is not supported, then the users will get an e Added. Thanks. http://gerrit.cloudera.org:8080/#/c/21382/20/fe/src/test/java/org/apache/impala/customcluster/JwtHttpTest.java File fe/src/test/java/org/apache/impala/customcluster/JwtHttpTest.java: http://gerrit.cloudera.org:8080/#/c/21382/20/fe/src/test/java/org/apache/impala/customcluster/JwtHttpTest.java@110 PS20, Line 110: > This function currently use fixed json file. You may add new input paramete Reading the jwks directly from the local file instead of jwks url now. http://gerrit.cloudera.org:8080/#/c/21382/20/fe/src/test/java/org/apache/impala/customcluster/JwtHttpTest.java@575 PS20, Line 575: lient transport = new THttpClient("http://local > add a new parameter to pass jwks_x5c_rs256.json Reading the jwks from local file instead of the jwks url now. http://gerrit.cloudera.org:8080/#/c/21382/20/fe/src/test/java/org/apache/impala/customcluster/JwtHttpTest.java@600 PS20, Line 600: transport.setCustomHeaders(headers); : try { : openResp = client.OpenSession(openReq); : } catch (Exception e) { : verifyJwtAuthMetrics(3, 1); : assertEquals(e.getMessage(), "HTTP Response code: 401"); : } : } : : /** : * Asserts that the specified string is present in the impalad.ERROR file within the : * specified log directory. : * > This function block seems copying from testJwtAuthWithJwksHttpUrl(). In tha Removed. Now loading the jwks from local file directly. -- To view, visit http://gerrit.cloudera.org:8080/21382 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I70be6f9f54190544aa005b2644e2ed8db6f6bb74 Gerrit-Change-Number: 21382 Gerrit-PatchSet: 21 Gerrit-Owner: gaurav singh <gsi...@cloudera.com> Gerrit-Reviewer: Abhishek Rawat <ara...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com> Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com> Gerrit-Reviewer: gaurav singh <gsi...@cloudera.com> Gerrit-Comment-Date: Mon, 13 May 2024 22:23:15 +0000 Gerrit-HasComments: Yes
