Re: [PR] RANGER-5749: Limit concurrent Ranger Admin UI sessions per user [ranger]

["vyommani (via GitHub)" <gi...@apache.org>]

vyommani commented on code in PR #1200:
URL: https://github.com/apache/ranger/pull/1200#discussion_r3901335321


##########
security-admin/src/main/webapp/WEB-INF/web.xml:
##########
@@ -33,6 +33,9 @@
   <listener>
     <listener-class>org.springframework.web.context.request.RequestContextListener</listener-class>
   </listener>
+  <listener>
+    <listener-class>org.apache.ranger.security.listener.RangerHttpSessionListener</listener-class>

Review Comment:
   RangerHttpSessionListener is now registered twice: via this explicit `<listener>` entry and via the @WebListener annotation added in the same commit (RangerHttpSessionListener.java). `web-app` here doesn't set `metadata-complete="true"`, so a Servlet 3.0+ container (Tomcat) will pick the class up through both annotation scanning and this XML declaration, instantiating two listener instances.
   
   Since sessionCreated/sessionDestroyed both write into the same static CopyOnWriteArrayList<HttpSession> listOfSession, every session create/destroy event fires twice, so each login adds the session to the list twice. That inflates the count enforceConcurrentSessionLimit() compares against ranger.session.limit.concurrency, making the limit trip early/incorrectly, and also affects the existing consumer of getActiveSessionOnServer() in SessionMgr.java.
   
   Please pick one registration mechanism — either drop this `<listener>` block (the annotation alone is sufficient) or drop `@WebListener` and keep this explicit entry.



-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@ranger.apache.org

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