Abhishek Rawat has posted comments on this change. ( http://gerrit.cloudera.org:8080/22986 )
Change subject: IMPALA-14083: Connected user and session user mismatch when cookie based authentication is used with SPNEGO ...................................................................... Patch Set 4: (5 comments) http://gerrit.cloudera.org:8080/#/c/22986/3/be/src/rpc/authentication.cc File be/src/rpc/authentication.cc: http://gerrit.cloudera.org:8080/#/c/22986/3/be/src/rpc/authentication.cc@672 PS3, Line 672: GetShortUsernameFromKerberosPrincipal > Maybe add a logging here for the username and short name here like line 904 Added debug logs for logging username and kerberos_user_short. GetShortUsernameFromKerberosPrincipal(username) should return 'username' if the 'username' is not kerberos principal. I think the 'username' cannot be empty because NegotiateAuth will fail and not generate a cookie. https://github.com/apache/impala/blob/master/be/src/rpc/authentication.cc#L881 http://gerrit.cloudera.org:8080/#/c/22986/3/fe/src/test/java/org/apache/impala/customcluster/SpnegoAuthTest.java File fe/src/test/java/org/apache/impala/customcluster/SpnegoAuthTest.java: http://gerrit.cloudera.org:8080/#/c/22986/3/fe/src/test/java/org/apache/impala/customcluster/SpnegoAuthTest.java@249 PS3, Line 249: assertNotNull(authMech); > Nit: debugging line that should be removed. Done http://gerrit.cloudera.org:8080/#/c/22986/3/fe/src/test/java/org/apache/impala/customcluster/SpnegoAuthTest.java@401 PS3, Line 401: String[] authMech = cookieValueFields[4].trim().split("="); > Since Impala expects the cookie value to be in a specific format, it would Done http://gerrit.cloudera.org:8080/#/c/22986/3/fe/src/test/java/org/apache/impala/customcluster/SpnegoTokenGenerator.java File fe/src/test/java/org/apache/impala/customcluster/SpnegoTokenGenerator.java: http://gerrit.cloudera.org:8080/#/c/22986/3/fe/src/test/java/org/apache/impala/customcluster/SpnegoTokenGenerator.java@24 PS3, Line 24: > not used in the code Remove the import. http://gerrit.cloudera.org:8080/#/c/22986/3/fe/src/test/java/org/apache/impala/customcluster/THttpClientWithHeaders.java File fe/src/test/java/org/apache/impala/customcluster/THttpClientWithHeaders.java: http://gerrit.cloudera.org:8080/#/c/22986/3/fe/src/test/java/org/apache/impala/customcluster/THttpClientWithHeaders.java@77 PS3, Line 77: THttpClientWithHeaders > I'm not in favor of adding "Test" to the class name since having that word I'll probably keep the class name as 'THttpClientWithHeaders' for now. -- To view, visit http://gerrit.cloudera.org:8080/22986 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id7223e449c32484bfd2295f7a9e728b7c02637e9 Gerrit-Change-Number: 22986 Gerrit-PatchSet: 4 Gerrit-Owner: Abhishek Rawat <[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: Yida Wu <[email protected]> Gerrit-Comment-Date: Wed, 11 Jun 2025 02:24:31 +0000 Gerrit-HasComments: Yes
