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 2: (8 comments) http://gerrit.cloudera.org:8080/#/c/22986/1/be/src/service/impala-hs2-server.cc File be/src/service/impala-hs2-server.cc: http://gerrit.cloudera.org:8080/#/c/22986/1/be/src/service/impala-hs2-server.cc@374 PS1, Line 374: connection_context->server_name == HS2_HTTP_SERVER_NAME) > Can't we do this somewhere else, e.g. in CookieAuth? https://github.com/apa Yes, setting 'connection_context->kerberos_user_short' in CookieAuth() is also an alternative. The only issue would be we will be setting it for every RPC and not just OpenSession, which is still okay I think since we anyways set 'connection_context->username' in every call to CookieAuth. I will explore setting the original Auth Mechanism in the Cookie. http://gerrit.cloudera.org:8080/#/c/22986/1/be/src/service/impala-hs2-server.cc@383 PS1, Line 383: threads > The threads being important here is strange to me. My understanding is that Each thread caches ConnectionContext as a thread local variable and the SessionState object is created based on the cached ConnectionContext. Here is an example: Thread_A: New connection from proxy client using SPNEGO Thread_B: New connection from proxy client using Auth Cookie sessions created by Thread_A will have 'connected_user_short' populated in the SessionState object. Any future RPCs from this session can be serviced by either Thread_A or Thread_B. sessions created by Thread_B will not have 'connected_user_short' populated in the SessionState object. If the RPCs from this session is serviced by the same thread i.e., Thread_B then all good. However, if some RPC gets serviced by Thread_A then there is a mismatch. http://gerrit.cloudera.org:8080/#/c/22986/1/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/1/fe/src/test/java/org/apache/impala/customcluster/SpnegoAuthTest.java@182 PS1, Line 182: interestin > nit: interesting Done http://gerrit.cloudera.org:8080/#/c/22986/1/fe/src/test/java/org/apache/impala/customcluster/SpnegoAuthTest.java@206 PS1, Line 206: assertEquals(0, ret); // cluster should start up > should be assertEquals(0, ret) since the assertEquals function's first para Done http://gerrit.cloudera.org:8080/#/c/22986/1/fe/src/test/java/org/apache/impala/customcluster/SpnegoAuthTest.java@222 PS1, Line 222: assertEquals(TStatusCode.ERROR_STATUS, openResp.getStatus().getStatusCode()); > should be assertEquals(TStatusCode.ERROR_STATUS, openResp.getStatus().getSt Done http://gerrit.cloudera.org:8080/#/c/22986/1/fe/src/test/java/org/apache/impala/customcluster/SpnegoAuthTest.java@238 PS1, Line 238: assertEquals(TStatusCode.SUCCESS_STATUS, openResp.getStatus().getStatusCode()); > should be assertEquals(TStatusCode.SUCCESS_STATUS, openResp.getStatus().get Done http://gerrit.cloudera.org:8080/#/c/22986/1/fe/src/test/java/org/apache/impala/customcluster/SpnegoAuthTest.java@250 PS1, Line 250: } else { > even though it *shouldn't* happen, please consider adding an else here to f Done http://gerrit.cloudera.org:8080/#/c/22986/1/fe/src/test/java/org/apache/impala/customcluster/SpnegoAuthTest.java@319 PS1, Line 319: int exitCode = process.wai > Should be assertEquals(0, exitCode) Done -- 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: 2 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-Comment-Date: Fri, 06 Jun 2025 21:25:34 +0000 Gerrit-HasComments: Yes
