Mihaly Szjatinya 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 16:

(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:
> Oh, yes, those two statements are rather ambiguous.  My intention was to te
Oh, ok. As clients will gradually get to adopt this, there's definitely some 
work on client side.


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@851
PS13, Line 851:   bool use_saml = use_saml_ && !sq_get_header(connection, 
"X-Impala-EETest");
> Ah, that is helpful.  It still appears to me that any client specifying the
Hmm, we can do this only for FLAGS_saml2_ee_test_mode, but in that case we 
cannot start the cluster with './start-impala-cluster.py' and use it with real 
SAML validation.

This approach seems like a hack to me as well. Maybe it's better to skip the 
entire Webserver polling within '/start-impala-cluster.py' in case SAML is 
turned on? Or other ideas?


http://gerrit.cloudera.org:8080/#/c/23237/13/tests/custom_cluster/test_saml2_sso.py
File tests/custom_cluster/test_saml2_sso.py:

http://gerrit.cloudera.org:8080/#/c/23237/13/tests/custom_cluster/test_saml2_sso.py@145
PS13, Line 145:                                          
assertation_id=str(uuid.uuid4()),
> Nice tests!  Thanks for adding.
Yeah, makes sense. Added the missing invalidation for Webserver and updated the 
tests.

Also fixed some misplaced DCHECKS, which were causing a crash on 'Invalid relay 
state'.



--
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: 16
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: Thu, 05 Feb 2026 16:13:38 +0000
Gerrit-HasComments: Yes

Reply via email to