Spring Security
  1. Spring Security
  2. SEC-1499

SessionFixationProtectionStrategy reused destroyed Spring session bean as session attribute when migrateSessionAttributes is true

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Won't Fix
    • Affects Version/s: 3.0.2
    • Fix Version/s: 3.1.0.M1
    • Component/s: Web
    • Labels:
      None

      Description

      Scenario
      SessionFixationProtectionStrategy saves bean with the method createMigratedAttributeMap
      SessionFixationProtectionStrategy invalidate the session.
      The bean destroy's methods are called.
      The destroyed bean are injected into the new session

        Activity

        Hide
        Luke Taylor added a comment - - edited

        I don't think this is really a bug. It's a consequence of using this functionality with attributes which implement HttpSessionBindingListener, as Spring's ServletRequestAttributes does. There is no other way of modifying the session ID through the servet API, so I don't see any way round it, other than disabling the functionality. If you have any other suggestions, please post them.

        Show
        Luke Taylor added a comment - - edited I don't think this is really a bug. It's a consequence of using this functionality with attributes which implement HttpSessionBindingListener, as Spring's ServletRequestAttributes does. There is no other way of modifying the session ID through the servet API, so I don't see any way round it, other than disabling the functionality. If you have any other suggestions, please post them.
        Hide
        Nicolas Labrot added a comment -

        I'm afraid I don't have suggestion using traditionnal servlet API.

        I think a part of the need is to migrate bean states from the anonymous session to the authentified one (for example search fields).

        A part of the solution could be to :

        • save state from the old bean
        • invalidate the session
        • create the bean in the new session
        • restore state
        Show
        Nicolas Labrot added a comment - I'm afraid I don't have suggestion using traditionnal servlet API. I think a part of the need is to migrate bean states from the anonymous session to the authentified one (for example search fields). A part of the solution could be to : save state from the old bean invalidate the session create the bean in the new session restore state
        Hide
        Luke Taylor added a comment -

        I don't really see any way that Spring Security can restore the states of beans that are stored in the session and have been destroyed.

        Show
        Luke Taylor added a comment - I don't really see any way that Spring Security can restore the states of beans that are stored in the session and have been destroyed.
        Hide
        Nicolas Labrot added a comment -

        For example an interface that can be implemented by spring bean (hope the text formatting works) :

        public interface ISpringBeanSerializableState {
           Object saveState();
           void restoreState(Object state);
        }

        With the following algorithm :

        1. Get the names of all instanciated Spring beans with session scope that implement ISpringBeanSerializableState
        2. Save the state for this beans
        3. Invalidate the session
        4. Create the previous instanciated Spring beans with session scope (with call to ApplicationContext)
        5. Restore the state for this beans
        Show
        Nicolas Labrot added a comment - For example an interface that can be implemented by spring bean (hope the text formatting works) : public interface ISpringBeanSerializableState { Object saveState(); void restoreState(Object state); } With the following algorithm : Get the names of all instanciated Spring beans with session scope that implement ISpringBeanSerializableState Save the state for this beans Invalidate the session Create the previous instanciated Spring beans with session scope (with call to ApplicationContext) Restore the state for this beans
        Hide
        Luke Taylor added a comment -

        I'd recommend that you implement a custom SessionAuthenticationStrategy if you want to use this approach, either that or avoid using session-scoped beans which rely on the DisposableBean lifecycle. It's not just a problem with Spring beans. It's is a general problem which will apply to any object which implements HttpSessionBindingListener and assumes that when it is unbound it is no longer required. It's certainly something that should be documented though.

        Show
        Luke Taylor added a comment - I'd recommend that you implement a custom SessionAuthenticationStrategy if you want to use this approach, either that or avoid using session-scoped beans which rely on the DisposableBean lifecycle. It's not just a problem with Spring beans. It's is a general problem which will apply to any object which implements HttpSessionBindingListener and assumes that when it is unbound it is no longer required. It's certainly something that should be documented though.
        Hide
        Luke Taylor added a comment -

        I've added some docs and Javadoc to clarify that this may cause issues, but I think the best option is to implement a custom strategy which deals with the application's life-cycle requirements for specific beans.

        Show
        Luke Taylor added a comment - I've added some docs and Javadoc to clarify that this may cause issues, but I think the best option is to implement a custom strategy which deals with the application's life-cycle requirements for specific beans.
        Hide
        Alexander Zagumennikov added a comment -

        Hi, I faced with this issue too and I have some considerations about it.
        When migrating attributes from one session to another we can set some flag in session attributes that indicates that it's a migration, not an actual session descruction. And we can check this flag in DestructionCallbackBindingListener#valueUnbound and not call destructionCallback.

        Show
        Alexander Zagumennikov added a comment - Hi, I faced with this issue too and I have some considerations about it. When migrating attributes from one session to another we can set some flag in session attributes that indicates that it's a migration, not an actual session descruction. And we can check this flag in DestructionCallbackBindingListener#valueUnbound and not call destructionCallback.
        Hide
        Alexander Zagumennikov added a comment -

        If anyone interested, here's my solution:

        public class SessionMigratingSwitchUserFilter extends SwitchUserFilter {
         
            private static final String SAVED_SESSION_ATTRIBUTES_KEY = SessionMigratingSwitchUserFilter.class.getName() + ".SAVED_SESSION_ATTRIBUTES";
            private static final String SAVED_SESSION_ATTRIBUTES_DESTRUCTION_CALLBACK_KEY = SAVED_SESSION_ATTRIBUTES_KEY + ".DESTRUCTION_CALLBACK";
         
            @Override
            protected Authentication attemptSwitchUser(HttpServletRequest request) throws AuthenticationException {
                Authentication result = super.attemptSwitchUser(request);
         
                if (result == null) {
                    //switch user not successful
                    return result;
                }
         
                HttpSession session = request.getSession(false);
         
                if (session == null) {
                    return result;
                }
         
                synchronized (WebUtils.getSessionMutex(session)) {
                    SessionMigrationUtils.setSessionMigrating(session);
         
                    //save security context
                    Object sessionContextAttribute = session.getAttribute(HttpSessionSecurityContextRepository.SPRING_SECURITY_CONTEXT_KEY);
                    session.removeAttribute(HttpSessionSecurityContextRepository.SPRING_SECURITY_CONTEXT_KEY);
         
                    final Map<String, Object> sessionAttributesToSave = new HashMap<String, Object>();
                    Enumeration attrNames = session.getAttributeNames();
                    while (attrNames.hasMoreElements()) {
                        String attrName = (String) attrNames.nextElement();
         
                        if (SessionMigrationUtils.SESSION_MIGRATING_ATTRIBUTE_NAME.equals(attrName)) {
                            continue;
                        }
         
                        sessionAttributesToSave.put(attrName, session.getAttribute(attrName));
                        session.removeAttribute(attrName);
                    }
         
                    SessionMigrationUtils.unsetSessionMigrating(session);
         
                    session.invalidate();
         
                    //create new session
                    session = request.getSession();
         
                    SessionMigrationUtils.setSessionMigrating(session);
         
                    //set new session security context
                    session.setAttribute(HttpSessionSecurityContextRepository.SPRING_SECURITY_CONTEXT_KEY, sessionContextAttribute);
         
                    //set saved session attributes
                    session.setAttribute(SAVED_SESSION_ATTRIBUTES_KEY, sessionAttributesToSave);
         
                    //if attribute removed (in case of session invalidation), we should call valueUnbound callbacks of saved attributes
                    session.setAttribute(SAVED_SESSION_ATTRIBUTES_DESTRUCTION_CALLBACK_KEY, new HttpSessionBindingListener() {
                        public void valueBound(HttpSessionBindingEvent event) {
                        }
         
                        public void valueUnbound(HttpSessionBindingEvent event) {
                            if (SessionMigrationUtils.isSessionMigrating(event.getSession())) {
                                return;
                            }
                            for (Object savedSessionAttribute : sessionAttributesToSave.values()) {
                                if (savedSessionAttribute instanceof HttpSessionBindingListener) {
                                    ((HttpSessionBindingListener) savedSessionAttribute).valueUnbound(event);
                                }
                            }
                        }
                    });
         
                    SessionMigrationUtils.unsetSessionMigrating(session);
                }
         
                return result;
            }
         
            @Override
            protected Authentication attemptExitUser(HttpServletRequest request) throws AuthenticationCredentialsNotFoundException {
                Authentication result = super.attemptExitUser(request);
         
                if (result == null) {
                    //exit user not successful
                    return result;
                }
         
                HttpSession session = request.getSession(false);
         
                if (session == null) {
                    return result;
                }
         
                synchronized (WebUtils.getSessionMutex(session)) {
                    SessionMigrationUtils.setSessionMigrating(session);
         
                    //noinspection unchecked
                    Map<String, Object> sessionAttributesToRestore = (Map<String, Object>) session.getAttribute(SAVED_SESSION_ATTRIBUTES_KEY);
                    session.removeAttribute(SAVED_SESSION_ATTRIBUTES_DESTRUCTION_CALLBACK_KEY);
                    session.removeAttribute(SAVED_SESSION_ATTRIBUTES_KEY);
         
                    Object sessionContextAttribute = session.getAttribute(HttpSessionSecurityContextRepository.SPRING_SECURITY_CONTEXT_KEY);
                    session.removeAttribute(HttpSessionSecurityContextRepository.SPRING_SECURITY_CONTEXT_KEY);
         
                    SessionMigrationUtils.unsetSessionMigrating(session);
         
                    session.invalidate();
         
                    //create new session
                    session = request.getSession();
         
                    SessionMigrationUtils.setSessionMigrating(session);
         
                    if (sessionAttributesToRestore != null) {
                        //restore attributes
                        for (Map.Entry<String, Object> sessionAttributeToRestore : sessionAttributesToRestore.entrySet()) {
                            session.setAttribute(sessionAttributeToRestore.getKey(), sessionAttributeToRestore.getValue());
                        }
                    }
         
                    if (sessionContextAttribute != null) {
                        //restore security context
                        session.setAttribute(HttpSessionSecurityContextRepository.SPRING_SECURITY_CONTEXT_KEY, sessionContextAttribute);
                    }
         
                    SessionMigrationUtils.unsetSessionMigrating(session);
                }
         
                return result;
            }
        }
        

        public abstract class SessionMigrationUtils {
         
            public  static final String SESSION_MIGRATING_ATTRIBUTE_NAME = SessionMigrationUtils .class.getName() + ".MIGRATING";
         
            public static boolean isSessionValid(HttpSession session) {
                try {
                    session.getCreationTime();
                    return true;
                } catch (IllegalStateException e) {
                    return false;
                }
            }
         
            public static boolean isSessionMigrating(HttpSession session) {
                if (!isSessionValid(session)) {
                    return false;
                }
                Boolean migrating = (Boolean) session.getAttribute(SESSION_MIGRATING_ATTRIBUTE_NAME);
                return migrating != null && migrating;
            }
         
            public static void setSessionMigrating(HttpSession session) {
                session.setAttribute(SESSION_MIGRATING_ATTRIBUTE_NAME, true);
            }
         
            public static void unsetSessionMigrating(HttpSession session) {
                session.removeAttribute(SESSION_MIGRATING_ATTRIBUTE_NAME);
            }
        }
        
        

        Show
        Alexander Zagumennikov added a comment - If anyone interested, here's my solution: public class SessionMigratingSwitchUserFilter extends SwitchUserFilter {   private static final String SAVED_SESSION_ATTRIBUTES_KEY = SessionMigratingSwitchUserFilter. class .getName() + ".SAVED_SESSION_ATTRIBUTES" ; private static final String SAVED_SESSION_ATTRIBUTES_DESTRUCTION_CALLBACK_KEY = SAVED_SESSION_ATTRIBUTES_KEY + ".DESTRUCTION_CALLBACK" ;   @Override protected Authentication attemptSwitchUser(HttpServletRequest request) throws AuthenticationException { Authentication result = super .attemptSwitchUser(request);   if (result == null ) { //switch user not successful return result; }   HttpSession session = request.getSession( false );   if (session == null ) { return result; }   synchronized (WebUtils.getSessionMutex(session)) { SessionMigrationUtils.setSessionMigrating(session);   //save security context Object sessionContextAttribute = session.getAttribute(HttpSessionSecurityContextRepository.SPRING_SECURITY_CONTEXT_KEY); session.removeAttribute(HttpSessionSecurityContextRepository.SPRING_SECURITY_CONTEXT_KEY);   final Map<String, Object> sessionAttributesToSave = new HashMap<String, Object>(); Enumeration attrNames = session.getAttributeNames(); while (attrNames.hasMoreElements()) { String attrName = (String) attrNames.nextElement();   if (SessionMigrationUtils.SESSION_MIGRATING_ATTRIBUTE_NAME.equals(attrName)) { continue ; }   sessionAttributesToSave.put(attrName, session.getAttribute(attrName)); session.removeAttribute(attrName); }   SessionMigrationUtils.unsetSessionMigrating(session);   session.invalidate();   //create new session session = request.getSession();   SessionMigrationUtils.setSessionMigrating(session);   //set new session security context session.setAttribute(HttpSessionSecurityContextRepository.SPRING_SECURITY_CONTEXT_KEY, sessionContextAttribute);   //set saved session attributes session.setAttribute(SAVED_SESSION_ATTRIBUTES_KEY, sessionAttributesToSave);   //if attribute removed (in case of session invalidation), we should call valueUnbound callbacks of saved attributes session.setAttribute(SAVED_SESSION_ATTRIBUTES_DESTRUCTION_CALLBACK_KEY, new HttpSessionBindingListener() { public void valueBound(HttpSessionBindingEvent event) { }   public void valueUnbound(HttpSessionBindingEvent event) { if (SessionMigrationUtils.isSessionMigrating(event.getSession())) { return ; } for (Object savedSessionAttribute : sessionAttributesToSave.values()) { if (savedSessionAttribute instanceof HttpSessionBindingListener) { ((HttpSessionBindingListener) savedSessionAttribute).valueUnbound(event); } } } });   SessionMigrationUtils.unsetSessionMigrating(session); }   return result; }   @Override protected Authentication attemptExitUser(HttpServletRequest request) throws AuthenticationCredentialsNotFoundException { Authentication result = super .attemptExitUser(request);   if (result == null ) { //exit user not successful return result; }   HttpSession session = request.getSession( false );   if (session == null ) { return result; }   synchronized (WebUtils.getSessionMutex(session)) { SessionMigrationUtils.setSessionMigrating(session);   //noinspection unchecked Map<String, Object> sessionAttributesToRestore = (Map<String, Object>) session.getAttribute(SAVED_SESSION_ATTRIBUTES_KEY); session.removeAttribute(SAVED_SESSION_ATTRIBUTES_DESTRUCTION_CALLBACK_KEY); session.removeAttribute(SAVED_SESSION_ATTRIBUTES_KEY);   Object sessionContextAttribute = session.getAttribute(HttpSessionSecurityContextRepository.SPRING_SECURITY_CONTEXT_KEY); session.removeAttribute(HttpSessionSecurityContextRepository.SPRING_SECURITY_CONTEXT_KEY);   SessionMigrationUtils.unsetSessionMigrating(session);   session.invalidate();   //create new session session = request.getSession();   SessionMigrationUtils.setSessionMigrating(session);   if (sessionAttributesToRestore != null ) { //restore attributes for (Map.Entry<String, Object> sessionAttributeToRestore : sessionAttributesToRestore.entrySet()) { session.setAttribute(sessionAttributeToRestore.getKey(), sessionAttributeToRestore.getValue()); } }   if (sessionContextAttribute != null ) { //restore security context session.setAttribute(HttpSessionSecurityContextRepository.SPRING_SECURITY_CONTEXT_KEY, sessionContextAttribute); }   SessionMigrationUtils.unsetSessionMigrating(session); }   return result; } } public abstract class SessionMigrationUtils {   public static final String SESSION_MIGRATING_ATTRIBUTE_NAME = SessionMigrationUtils . class .getName() + ".MIGRATING" ;   public static boolean isSessionValid(HttpSession session) { try { session.getCreationTime(); return true ; } catch (IllegalStateException e) { return false ; } }   public static boolean isSessionMigrating(HttpSession session) { if (!isSessionValid(session)) { return false ; } Boolean migrating = (Boolean) session.getAttribute(SESSION_MIGRATING_ATTRIBUTE_NAME); return migrating != null && migrating; }   public static void setSessionMigrating(HttpSession session) { session.setAttribute(SESSION_MIGRATING_ATTRIBUTE_NAME, true ); }   public static void unsetSessionMigrating(HttpSession session) { session.removeAttribute(SESSION_MIGRATING_ATTRIBUTE_NAME); } }
        Hide
        Alexander Zagumennikov added a comment -

        oops, wrong issue

        Show
        Alexander Zagumennikov added a comment - oops, wrong issue

          People

          • Assignee:
            Luke Taylor
            Reporter:
            Nicolas Labrot
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: