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 19: (1 comment) http://gerrit.cloudera.org:8080/#/c/24448/18/be/src/util/webserver.cc File be/src/util/webserver.cc: http://gerrit.cloudera.org:8080/#/c/24448/18/be/src/util/webserver.cc@777 PS18, Line 777: if (use_jwt_) { : if (OAuthTokenAuth(bearer_token, request_info, &response_headers)) { : total_jwt_token_auth_success_->Increment(1); : authenticated = true; : check_csrf_protection = false; : // TODO: cookies are not added, but are not needed right now : } : } : if (!authenticated && use_oauth_) { : if (OAuthTokenAuth(bearer_token, request_info, &response_headers)) { : total_oauth_token_auth_success_->Increment(1); : authenticated = true; : check_csrf_protection = false; : // TODO: cookies are not added, but are not needed right now : } : } : if (!authenticated) { : if (use_jwt_) { : LOG(INFO) << "Invalid JWT token provided"; : total_jwt_token_auth_failure_->Increment(1); : } : if (use_oauth_) { : LOG(INFO) << "Invalid OAuth token provided"; : total_oauth_token_auth_failure_->Increment(1); : } : } > Good question. In the current flow, if both are enabled we intentionally tr If my understanding is correct, the problem is that, the if use_jwt_ block succeeding doesn't actually guarantee that a JWT config was used as it can fall back to OAuth internally. Therefore, the separated metrics and logging based purely on use_jwt_ and use_oauth_ can be incorrect here. I also think this is too confusing because the two calls to OAuthTokenAuth() are now completely identical. The only difference is which metrics and logging get triggered around them. Previously this structure made sense because JWT and OAuth were calling different interfaces, but now that they are using the same one, we probably need a better way to know what was actually used inside OAuthTokenAuth() to ensure the accuracy of the 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: 19 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: Tue, 25 Aug 2026 04:44:23 +0000 Gerrit-HasComments: Yes
