Yida Wu has posted comments on this change. ( http://gerrit.cloudera.org:8080/24448 )
Change subject: IMPALA-14799: Add oauth_servers support and tests ...................................................................... Patch Set 20: (2 comments) http://gerrit.cloudera.org:8080/#/c/24448/20/be/src/util/webserver.cc File be/src/util/webserver.cc: http://gerrit.cloudera.org:8080/#/c/24448/20/be/src/util/webserver.cc@763 PS20, Line 763: // Try authenticating with JWT token first, if enabled. We can remove this comment since we are using the unified interface now. And the order in which the tokens are checked belongs in the documentation for the OAuthServersManager implementation http://gerrit.cloudera.org:8080/#/c/24448/20/be/src/util/webserver.cc@782 PS20, Line 782: // Preserve existing JWT-first compatibility semantics when both modes : // are enabled while avoiding duplicate token verification calls. This comment is a bit confusing because it mixes the metrics logic with the authentication logic. Can we simply show that there is a bug here in the comment like: // When both modes are enabled, this can incorrectly attribute successes to JWT because OAuthTokenAuth() may use OAuth internally. A follow-up should return the exact provider from OAuthTokenAuth() to fix these metrics. -- To view, visit http://gerrit.cloudera.org:8080/24448 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib29ff36600406ba59c10f29d79cc632020f4a3f7 Gerrit-Change-Number: 24448 Gerrit-PatchSet: 20 Gerrit-Owner: Anubhav Jindal <[email protected]> Gerrit-Reviewer: Anubhav Jindal <[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, 28 Aug 2026 15:24:22 +0000 Gerrit-HasComments: Yes
