Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/23237 )
Change subject: IMPALA-14285: Add SAML2 authentication support for Coordinator Web UI ...................................................................... Patch Set 14: (3 comments) http://gerrit.cloudera.org:8080/#/c/23237/13/be/src/rpc/authentication-util.cc File be/src/rpc/authentication-util.cc: http://gerrit.cloudera.org:8080/#/c/23237/13/be/src/rpc/authentication-util.cc@326 PS13, Line 326: ValidateSaml2Response > IMO in the webserver case the target should be always a specific coordinato Good point -- we don't want the user to get bounced to a different coordinator UI. The tests in test_saml2_sso.py should verify a new SAML assertion is requested if the user presents an invalid one. http://gerrit.cloudera.org:8080/#/c/23237/13/be/src/util/webserver.cc File be/src/util/webserver.cc: http://gerrit.cloudera.org:8080/#/c/23237/13/be/src/util/webserver.cc@373 PS13, Line 373: || use_saml_ > > to enforce that SAML must be used by itself I wasn't thinking about the machine use case. I agree Impala needs more work around supporting multiple auth methods, but that is far beyond the scope of this patch. Given that use case, supporting multiple auth methods at the same time is ok with me. We should still test one or two of the more likely combinations (such as SAML+LDAP or SAML+Keberos). http://gerrit.cloudera.org:8080/#/c/23237/13/fe/src/main/java/org/apache/impala/authentication/saml/HiveSamlRelayStateStoreBase.java File fe/src/main/java/org/apache/impala/authentication/saml/HiveSamlRelayStateStoreBase.java: http://gerrit.cloudera.org:8080/#/c/23237/13/fe/src/main/java/org/apache/impala/authentication/saml/HiveSamlRelayStateStoreBase.java@38 PS13, Line 38: T > Yes, good idea. Done -- To view, visit http://gerrit.cloudera.org:8080/23237 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I12540300529f9c240abf7196141ecb0ae6e37995 Gerrit-Change-Number: 23237 Gerrit-PatchSet: 14 Gerrit-Owner: Mihaly Szjatinya <[email protected]> Gerrit-Reviewer: Abhishek Rawat <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Mihaly Szjatinya <[email protected]> Gerrit-Reviewer: Nandor Kollar <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Comment-Date: Tue, 03 Feb 2026 22:10:45 +0000 Gerrit-HasComments: Yes
