SEC-1499: SessionFixationProtectionStrategy reused destroyed Spring session bean as session attribute when migrateSessionAttributes is true #1742

Closed
spring-issuemaster opened this Issue Jun 14, 2010 · 9 comments

1 participant

@spring-issuemaster

Nicolas Labrot (Migrated from SEC-1499) said:

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

@spring-issuemaster

Luke Taylor said:

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.

@spring-issuemaster

Nicolas Labrot said:

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
@spring-issuemaster

Luke Taylor said:

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.

@spring-issuemaster

Nicolas Labrot said:

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

@spring-issuemaster

Luke Taylor said:

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.

@spring-issuemaster

Luke Taylor said:

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.

@spring-issuemaster

Alexander Zagumennikov said:

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.

@spring-issuemaster

Alexander Zagumennikov said:

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);
    }
}
@spring-issuemaster

Alexander Zagumennikov said:

oops, wrong issue

@spring-issuemaster spring-issuemaster added this to the 3.1.0.M1 milestone Feb 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment