Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/24472 )
Change subject: IMPALA-12232: Validate JWT aud/iss claims ...................................................................... Patch Set 12: (3 comments) http://gerrit.cloudera.org:8080/#/c/24472/10/be/src/util/oauth-servers-manager-test.cc File be/src/util/oauth-servers-manager-test.cc: http://gerrit.cloudera.org:8080/#/c/24472/10/be/src/util/oauth-servers-manager-test.cc@127 PS10, Line 127: TEST(OAuthServersManagerTest, VerifyTokenFromSecondServerWhenFirstAudienceDoesNotMatch) { > Good catch. I don't see any negative tests in JwtHttpTest.java that prove Done http://gerrit.cloudera.org:8080/#/c/24472/12/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/24472/12/fe/src/test/java/org/apache/impala/customcluster/JwtHttpTest.java@364 PS12, Line 364: assertEquals(e.getMessage(), "HTTP Response code: 401"); There needs to be another assertion that checks that auth failed because the JWT issuer did not match the JWKS issuer. Otherwise, the auth could fail for another reason, and this test would not actually be testing what it is supposed to be testing. http://gerrit.cloudera.org:8080/#/c/24472/12/fe/src/test/java/org/apache/impala/customcluster/JwtHttpTest.java@439 PS12, Line 439: assertEquals(e.getMessage(), "HTTP Response code: 401"); There needs to be another assertion that checks that auth failed because the JWT audience did not match the JWKS issuer. Otherwise, the auth could fail for another reason, and this test would not actually be testing what it is supposed to be testing. -- To view, visit http://gerrit.cloudera.org:8080/24472 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0a00b126359f2bc7e2f73d894cebc2b9014c7375 Gerrit-Change-Number: 24472 Gerrit-PatchSet: 12 Gerrit-Owner: Anubhav Jindal <[email protected]> Gerrit-Reviewer: Abhishek Rawat <[email protected]> Gerrit-Reviewer: Anonymous Coward (934) Gerrit-Reviewer: Anubhav Jindal <[email protected]> Gerrit-Reviewer: Gokul Kolady <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Yida Wu <[email protected]> Gerrit-Comment-Date: Fri, 02 Oct 2026 18:21:39 +0000 Gerrit-HasComments: Yes
