[GitHub] [tinkerpop] javeme commented on pull request #1289: TINKERPOP-2374 fix missing authorization with SaslAndHttpBasicAuthenticationHandler

[GitBox <gi...@apache.org>]

javeme commented on pull request #1289:
URL: https://github.com/apache/tinkerpop/pull/1289#issuecomment-636199113


   @spmallette @divijvaidya Thanks for your review.
   
   I don’t think this is a workaround, I'm sure there is no race condition here, let me explain the steps to reproduce this bug:
   
   1. Assume authentication is enabled with `WsAndHttpChannelizerHandler` and and `SaslAndHttpBasicAuthenticationHandler`，and keep http connection alive.
   2. Login user1 with username1+password1(in http basic auth), and gremlin-server can get user information from http header "Authorization"(through `HttpBasicAuthenticationHandler`).
   3. Login user2 with username2+password2 in the same way, the user information cannot be obtained, and this problem occurs when we keep using the same TCP connection.
   
   Detailed analysis:
   
   1. Login user1, the pipeline status is: http-response-encoder -> WebSocketServerProtocolHandshakeHandler -> ... -> request-close-decoder  -> authenticator
   2. After [WsAndHttpChannelizerHandler](https://github.com/apache/tinkerpop/blob/cc3c5cb83e253b9949076628a7cfaade7f86f40e/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/WsAndHttpChannelizerHandler.java#L66) updated the pipeline: http-response-encoder -> authenticator -> request-handler, note the request-handler is newly added.
   3. After [SaslAndHttpBasicAuthenticationHandler](https://github.com/apache/tinkerpop/blob/cc3c5cb83e253b9949076628a7cfaade7f86f40e/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/SaslAndHttpBasicAuthenticationHandler.java#L51) updated the pipeline: http-response-encoder -> authenticator -> http-authentication -> request-handler, note the http-authentication is newly added.
   4. Until now this is correct.
   5. Login user2, if the TCP connection is the same, netty will use the original channel, so the original pipline is also be used, the pipeline status is: http-response-encoder -> authenticator -> **http-authentication** -> request-handler.
   6. After  [WsAndHttpChannelizerHandler](https://github.com/apache/tinkerpop/blob/cc3c5cb83e253b9949076628a7cfaade7f86f40e/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/WsAndHttpChannelizerHandler.java#L66) updated the pipeline: http-response-encoder -> authenticator -> request-handler -> **http-authentication**, note http-authentication is behind request-handler.
   _(For the detailed process please refer to_ [how to remove request-handler](https://github.com/apache/tinkerpop/blob/cc3c5cb83e253b9949076628a7cfaade7f86f40e/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/WsAndHttpChannelizerHandler.java#L66) and [how to remove authenticator](https://github.com/apache/tinkerpop/blob/cc3c5cb83e253b9949076628a7cfaade7f86f40e/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/WsAndHttpChannelizerHandler.java#L68) and  [how to add authenticator again](https://github.com/apache/tinkerpop/blob/cc3c5cb83e253b9949076628a7cfaade7f86f40e/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/WsAndHttpChannelizerHandler.java#L69) and [how to add request-handler again](https://github.com/apache/tinkerpop/blob/cc3c5cb83e253b9949076628a7cfaade7f86f40e/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/WsAndHttpChannelizerHandler.java#L70))
   7. [SaslAndHttpBasicAuthenticationHandler](https://github.com/apache/tinkerpop/blob/cc3c5cb83e253b9949076628a7cfaade7f86f40e/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/SaslAndHttpBasicAuthenticationHandler.java#L51)  won't update the pipeline any more since http-authentication already exists.
   8. This caused us to be unable to get the information of user2 when executing gremlin through request-handler.
   
   Thie PR fix the bug by moving http-authentication in front of request-handler every time.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org