Yida Wu has posted comments on this change. ( http://gerrit.cloudera.org:8080/24472 )
Change subject: IMPALA-12232: Validate JWT aud/iss claims ...................................................................... Patch Set 6: (3 comments) http://gerrit.cloudera.org:8080/#/c/24472/6/be/src/util/jwt-util.cc File be/src/util/jwt-util.cc: http://gerrit.cloudera.org:8080/#/c/24472/6/be/src/util/jwt-util.cc@51 PS6, Line 51: DECLARE_int32(jwks_update_frequency_s); : DECLARE_int32(jwks_pulling_timeout_s); How about removing these, seems dead code http://gerrit.cloudera.org:8080/#/c/24472/6/be/src/util/oauth-servers-manager.cc File be/src/util/oauth-servers-manager.cc: http://gerrit.cloudera.org:8080/#/c/24472/6/be/src/util/oauth-servers-manager.cc@62 PS6, Line 62: const OAuthServerVerifier& verifier = jwt_helpers_->at(matched_server_idx); : RETURN_IF_ERROR(JWTHelper::VerifyJwtClaims( : decoded_token.get(), verifier.audience_claims, verifier.issuer_claims)); Should we move the VerifyJwtClaims() check inside the loop in OAuthServersManager::Verify() after L80? I am thinking would it be a case that the token signature matches but claims not. http://gerrit.cloudera.org:8080/#/c/24472/6/be/src/util/oauth-servers-manager.cc@81 PS6, Line 81: if (!verifier.validate_token_signature) { If move to here, maybe we can check like this, and this is lighter than checking jwt_helper.Verify() first: Status claim_status = JWTHelper::VerifyJwtClaims(decoded_token,verifier.audience_claims,verifier.issuer_claims); if (!claim_status.ok()) { last_error = claim_status; continue; } -- 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: 6 Gerrit-Owner: Anubhav Jindal <[email protected]> Gerrit-Reviewer: Abhishek Rawat <[email protected]> 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, 14 Aug 2026 21:45:02 +0000 Gerrit-HasComments: Yes
