Paudi Moriarty (Migrated from SEC-1594) said:

The use of authenticationTrustResolver.isAnonymous(Authentication) to determine whether the context should be stored in the session does not allow for this to be decided based on other parts of the context or even other aspects of the Authentication!

Would be better to use a separate class to decide this based on the entire SecurityContext rather than reusing the AuthenticationTrustResolver in this way.

The default could still use authenticationTrustResolver.isAnonymous (via delegation) but I should be able to decide for myself when I want the context to be stored in the session.

Comment From: spring-projects-issues

Luke Taylor said:

HttpSessionContextIntegrationFilter no longer exists in the 3.1.0.M1 codebase, so please clarify what you think constitutes a major bug in the current code. If you have a particular problem you are trying to solve then please use the forum rather than the issue tracker.

Comment From: spring-projects-issues

Paudi Moriarty said:

Sorry for the confusion Luke, I'm actually using 2.0.4.

The bug still applies to 3.1 (and current trunk) though, having been carried through the refactoring to the SaveToSessionResponseWrapper.saveContext in HttpSessionSecurityContextRepository. (line 269 in current trunk)

I'll amend this issues description to reflect that.

My application has complex security requirements and this issue is a problem for me.

In particular, the authenticationTrustResolver field can't even be changed by dependency injection since it is both private and final.

private final AuthenticationTrustResolver authenticationTrustResolver = new AuthenticationTrustResolverImpl();

Spring Security has always provided us a robust and extensible framework and I think this bug is major because it makes assumptions about my implementation and doesn't provide a way to customise . To get around this, I'll have to patch it myself but I'd prefer to be able to use the official jars.

I can submit a patch if you like.

Comment From: spring-projects-issues

Paudi Moriarty said:

Sorry, I can't change the description due to permissions.

Comment From: spring-projects-issues

Luke Taylor said:

The current assumption is that an unauthenticated user has no need of a security context, which serves most requirements (in fact, all, up till now :) ). The use of the AuthenticationTrustResolver is an internal implementation detail. That's how the current code is intended to work, so this is not a bug. The introduction of the SecurityContextRepository gives you can extension point for controlling how (and if) the security context is stored, so you have the ability to change this behaviour if you really need to.

So essentially you can already can control when the context is saved. The SecurityContextRepository can also be injected through the namespace.

Comment From: spring-projects-issues

Paudi Moriarty said:

That's fair enough Luke!

Sorry for the confusion, I should have checked into 3.1 a little further before logging the issue. There was no such option available in 2.x other than replacing HttpSessionContextIntegrationFilter or avoiding AnonymousProcessingFilter. Even in 3.1 I have to subclass HttpSessionSecurityContextRepository just to override loadContext since SaveToSessionResponseWrapper is created with new:

        requestResponseHolder.setResponse(
                new SaveToSessionResponseWrapper(response, request, httpSession != null, context));

Then I have to create a new SaveContextOnUpdateOrErrorResponseWrapper or subclass on override saveContext.

All this just to remove the authenticationTrustResolver.isAnonymous check. If nothing else, a simple boolean property to disable the check would be very welcome. Otherwise my previous suggestion in the description would allow full flexibility without having to override, copy/paste or otherwise add unnecessary code to my security package.

I realise my requirements are a non-standard, but they are legitimate. I appreciate Spring Security's flexibility in allowing me to deviate from standard requirements but in this case the extension point is far too coarse for my needs.

Feel free to reclassify this as a minor improvement if you wish. I just think that a small change could add flexibility where it is currently lacking.

Comment From: rwinch

If you want to remove authenticationTrustResolver.isAnonymous check you can replace the AuthenticationTrustResolver in HttpSessionSecurityContextRepository. with a custom implementation that returns true all of the time.