Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/16542 )
Change subject: IMPALA-10210: Skip Authentication for connection from a trusted domain ...................................................................... Patch Set 2: (6 comments) http://gerrit.cloudera.org:8080/#/c/16542/2/be/src/rpc/authentication-util.h File be/src/rpc/authentication-util.h: http://gerrit.cloudera.org:8080/#/c/16542/2/be/src/rpc/authentication-util.h@40 PS2, Line 40: a list comma separated http://gerrit.cloudera.org:8080/#/c/16542/2/be/src/rpc/authentication-util.h@40 PS2, Line 40: picks the first one on the : // list what's the rationale for only checking the first one? http://gerrit.cloudera.org:8080/#/c/16542/2/be/src/rpc/authentication-util.h@48 PS2, Line 48: false nit: an error http://gerrit.cloudera.org:8080/#/c/16542/2/be/src/util/webserver.h File be/src/util/webserver.h: http://gerrit.cloudera.org:8080/#/c/16542/2/be/src/util/webserver.h@201 PS2, Line 201: std::string const&? http://gerrit.cloudera.org:8080/#/c/16542/2/be/src/util/webserver.cc File be/src/util/webserver.cc: http://gerrit.cloudera.org:8080/#/c/16542/2/be/src/util/webserver.cc@610 PS2, Line 610: // Connections originating from trusted domains should not require authentication. Might be good to mention that we're doing this after checking cookies to avoid the reverse DNS lookup, here and in THttpServer http://gerrit.cloudera.org:8080/#/c/16542/2/common/thrift/metrics.json File common/thrift/metrics.json: http://gerrit.cloudera.org:8080/#/c/16542/2/common/thrift/metrics.json@1299 PS2, Line 1299: Failure I wonder if "Failure" is too strong of a word here and below, since this will be a normal thing any time a regular user tries to connect. Maybe just "...Connections from Non-Trusted Domains" Or might even be fine to just drop the "failure" metrics, since these connections will end up counted in the SPNEGO/BASIC auth success/failure metrics too. -- To view, visit http://gerrit.cloudera.org:8080/16542 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I09234078e2314dbc3177d0e869ae028e216ca699 Gerrit-Change-Number: 16542 Gerrit-PatchSet: 2 Gerrit-Owner: Bikramjeet Vig <bikramjeet....@cloudera.com> Gerrit-Reviewer: Bikramjeet Vig <bikramjeet....@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Thomas Tauber-Marshall <tmarsh...@cloudera.com> Gerrit-Comment-Date: Tue, 20 Oct 2020 20:53:30 +0000 Gerrit-HasComments: Yes
