spring-projects / spring-security

Spring Security
http://spring.io/projects/spring-security
Apache License 2.0
8.72k stars 5.86k forks source link

SEC-3163: SessionFixationProtectionStrategy. When migrateSessionAttributes = false retains attributes with SPRING_SECURITY_ prefix #3345

Closed spring-projects-issues closed 8 years ago

spring-projects-issues commented 8 years ago

Krzysztof Szymko (Migrated from SEC-3163) said:

When springsecurity.sessionFixationPrevention.migrate = false, we do not want to copy the session attributes of the existing session to the new session after login.

Existing code allowed to copy SPRING_SECURITY_SAVED_REQUEST attribute to the new session:

    private HashMap<String, Object> createMigratedAttributeMap(HttpSession session) {
        HashMap<String, Object> attributesToMigrate = null;

        if (migrateSessionAttributes || retainedAttributes == null) {
            attributesToMigrate = new HashMap<String, Object>();

            Enumeration enumer = session.getAttributeNames();

            while (enumer.hasMoreElements()) {
                String key = (String) enumer.nextElement();
                if (!migrateSessionAttributes && !key.startsWith("SPRING_SECURITY_")) {
                    // Only retain Spring Security attributes
                    continue;
                }
                attributesToMigrate.put(key, session.getAttribute(key));
            }
        }

It works correctly when:

     if (!migrateSessionAttributes && !key.startsWith("SPRING_SECURITY_")) {

is changed to:

     if (!migrateSessionAttributes || !key.startsWith("SPRING_SECURITY_")) {

P.S. Javadoc is missing for createMigratedAttributeMap(HttpSession session).

spring-projects-issues commented 8 years ago

Rob Winch said:

Thank you for submitting this issue!

The behavior is intentional and documented in at the class level and on the setMigrateSessionAttributes. While I agree it is somewhat misleading, this has been the case since the property was introduced. This means we cannot change the behavior without breaking existing users.

If you want no attributes to be migrated, you can extend the class and override extractAttributes instead.

If you don't want Spring Security's attribute migrated, you can also instruct Spring Security to not persist the attribute in the first place. For example, you can configure Spring Security to use the NullRequestCache and SPRING_SECURITY_SAVED_REQUEST will not be set in the first place.

Does this help?

spring-projects-issues commented 8 years ago

Krzysztof Szymko said:

Yes, it is more than enough for me.

I ended up with extending SessionFixationProtectionStrategy.

At least now, I know to look for class level Javadoc before submitting an issue ;)

Thank you for detailed explanation.

spring-projects-issues commented 8 years ago

Rob Winch said:

kszymko Thanks for the follow up. I'm glad you go this working :)