Treat a SerializationException retrieving a session as though there w…

[dbyron-sf]

…ere no session.

This makes zero-downtime / seamless upgrades possible in more cases. Currently, upgrading spring-security across versions that change SpringSecurityCoreVersion.SERIAL_VERSION_ID requires manual deletion of cached sessions.

See also spring-projects/spring-security#9204 and spring-projects/spring-security#3737

[quaff]

Customize bean springSessionDefaultRedisSerializer as workaround if you are using redis as session store

@Configuration(proxyBeanMethods = false)
@EnableSpringHttpSession
public class SessionConfiguration {

	@Bean
	public RedisSerializer<?> springSessionDefaultRedisSerializer() {
		return new JdkSerializationRedisSerializer(getClass().getClassLoader()) {
			@Override
			public Object deserialize(byte[] bytes) throws SerializationException {
				try {
					return super.deserialize(bytes);
				}
				catch (SerializationException ex) {
					if (ex.getCause() instanceof SerializationFailedException) {
						return null; // ignore SerializationFailedException
					}
					throw ex;
				}
			}
		};
	}

}

[dbyron-sf]

@marcusdacoregio Hope it's OK that I'm tagging you here as things have been quiet on this PR for awhile. I see some activity elsewhere and would love a review.

Thanks much!

[marcusdacoregio]

Hi @dbyron-sf, thanks for the ping.

I'll tag @rwinch so we can talk about this issue and get back to you. It may take a couple of weeks for us to take a look at this so I will ask you for a little bit more patience.

[dbyron-sf]

Thanks so much @marcusdacoregio for taking a look!

[dbyron-sf]

Hey @marcusdacoregio (or maybe @rwinch). Sorry to nag, but it's been awhile and I'm still hoping to get some traction here. Thanks for your help.

[vpavic]

I don't think this should be addressed as proposed here. The PR targets only a single SessionRepository implementation (and the one that isn't a default anymore in 3.0) while in reality this is a concern that affects all session repositories that support Java based session serialization (and that's most if not all). Duplicating the try-catch block proposed here across different SessionRepository implementations isn't an attractive option either, for obvious reasons.

Note that this concern (safe session deserialization) in general is tracked under #529.

My opinion is that this should be addressed using a centralized strategy that would deal with deserialization failures and would then be reused across all session repositories that support Java serialization.

However, taking a step back, my suggestion to anyone affected by this is to strongly consider opting into a different (non-default) session serialization mechanism. JSON serialization is the most attractive option there, and is already supported by all Redis based SessionRepository implementations. This way, you shouldn't be affected by Spring Security's changes of serial version id while also standing a much better chance of doing major upgrades without affecting persisted sessions.

Note that there's also #1913 tracking the effort to move away from Java serialization.

[dbyron-sf]

Thanks for the comments @vpavic. What you're saying makes sense...and especially if it wouldn't be backported to some probably consider ancient/closed versions like 2.5/2.6/2.7 it won't help those upgrades. And if this isn't the default in 3.0 it won't help that upgrade either.

Any chance you can give some pointers on how to select JSON serialization?

PS I'm following both those issues, but they've been quiet for quite some time.

[vpavic]

There's a sample application within the Spring Session codebase that shows how to set up JSON based serialization. It also uses Spring Security's Jackson mixins.

[dbyron-sf]

Just saw #2124 fly by which looks related.

[marcusdacoregio]

Based on the comment from @vpavic, I'll close this as duplicate of #529