aledsage commented on a change in pull request #1033: Share sessions among all
bundles so we can share auth
URL: https://github.com/apache/brooklyn-server/pull/1033#discussion_r251413877
##########
File path:
rest/rest-resources/src/main/java/org/apache/brooklyn/rest/filter/BrooklynSecurityProviderFilterHelper.java
##########
@@ -73,43 +83,90 @@
*/
public static final String AUTHENTICATED_USER_SESSION_ATTRIBUTE =
"brooklyn.user";
- public static Set<SessionHandler> SESSION_MANAGER_CACHE = MutableSet.of();
-
private static final Logger log =
LoggerFactory.getLogger(BrooklynSecurityProviderFilterHelper.class);
// TODO this should be parametrisable
public static final String BASIC_REALM_NAME = "brooklyn";
public static final String BASIC_REALM_HEADER_VALUE = "BASIC
realm="+StringEscapes.JavaStringEscapes.wrapJavaString(BASIC_REALM_NAME);
+ /** The first session handler encountered becomes the shared handler that
replaces all others encountered. */
+ private static SessionHandler sharedSessionHandler;
+
/* check all contexts for sessions; surprisingly hard to configure session
management for karaf/pax web container.
* they _really_ want each servlet to have their own sessions. how you're
meant to do oauth for multiple servlets i don't know! */
- public HttpSession getSession(HttpServletRequest webRequest,
ManagementContext mgmt, boolean create) {
- String requestedSessionId = webRequest.getRequestedSessionId();
-
- log.trace("SESSION for {}, wants session {}",
webRequest.getRequestURI(), requestedSessionId);
-
- if (webRequest instanceof Request) {
- SessionHandler sm = ((Request)webRequest).getSessionHandler();
- boolean added = SESSION_MANAGER_CACHE.add( sm );
- log.trace("SESSION MANAGER found for {}: {} (added={})",
webRequest.getRequestURI(), sm, added);
- } else {
- log.trace("SESSION MANAGER NOT found for {}: {}",
webRequest.getRequestURI(), webRequest);
+ public HttpSession getSession(HttpServletRequest webRequest,
ManagementContext mgmt, boolean create) throws
SecurityProviderDeniedAuthentication {
+ if (webRequest instanceof ThreadLocalHttpServletRequest) {
+ // CXF requests get this, unwrap to get the jetty request
+ webRequest = ((ThreadLocalHttpServletRequest)webRequest).get();
}
-
- if (requestedSessionId!=null) {
- for (SessionHandler m: SESSION_MANAGER_CACHE) {
- HttpSession s = m.getHttpSession(requestedSessionId);
- if (s!=null) {
- log.trace("SESSION found for {}: {} (valid={})",
webRequest.getRequestURI(), s, m.isValid(s));
- return s;
+ if (webRequest instanceof ServletRequestWrapper) {
+ webRequest = (HttpServletRequest)
((ServletRequestWrapper)webRequest).getRequest();
+ }
+ if (webRequest instanceof Request) {
+ if (sharedSessionHandler==null ||
!sharedSessionHandler.isRunning()) {
+ synchronized (BrooklynSecurityProviderFilterHelper.class) {
Review comment:
Double-checked locking in java is broken. You have to declare the field
volatile, but that pretty-much removes the speed benefit of double-checked
locking.
(https://stackoverflow.com/questions/3578604/how-to-solve-the-double-checked-locking-is-broken-declaration-in-java)
But one will normally get away with it!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
