Spring issue 10032 causing User can't use own implementation of SecurityContext

[chiqiu]

Describe the bug
in issue #10032
After authentication, a new SecurityContextImpl always replace user's own implementation of SecurityContext.

To Reproduce
Put a customized SecurityContext object before authentication. It will always be replaced by a SecurityContextImpl object after authentication

Expected behavior
User customized object isn't replaced

[SpiReCZ]

We tried to upgrade to Spring 5 and Spring Security 5 and we have encountered this issue as well.

Our filter creates instance of our custom implementation of SecurityContext and it gets scrapped in AnonymousAuthenticationFilter, because user is not yet authenticated (Authentication is null).

spring-security/web/src/main/java/org/springframework/security/web/authentication/AnonymousAuthenticationFilter.java

Line 94 in 9d37810

SecurityContextHolder.setContext(context);

It seems like a wanted behavior according to: #8323

[jzheaux]

Thanks for the report, @chiqiu.

Initially, this sounds like what SecurityContextHolderStrategy is intended to assist with. Have you already tried implementing your custom SecurityContext construction using that API?

With an instance of SecurityContextHolderStrategy, you can call SecurityContextHolder.setContextHolderStrategy(strategy) at startup time, and then this filter and others will construct your custom SecurityContext.

[spring-projects-issues]

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

[chiqiu]

I have not implemented my own SecurityContextHolderStrategy , do you mean that I should implement my own one and also assign a new filter so the [AnonymousAuthenticationFilter] will not be used?

[jzheaux]

@chiqiu, you should be able to have your own SecurityContextHolderStrategy and not change anything else. Since AnonymousAuthenticationFilter will call your implementation of createNewContext, it should be possible to get your custom SecurityContext set by the filter.

[spring-projects-issues]

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

[spring-projects-issues]

Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open the issue.

[timo-eloranta]

We ran into this very same issue when trying to migrate to Spring Security 5.

For the last 10 years or so we have had our own SecurityContextHolderStrategy, named ExtendedSecurityContextHolderStrategy. With this strategy we use our own ExtendedSecurityContextImpl which implements SecurityContext, but also has additional/custom data included in a Map<String, List<String>> additionalContextData.

Now then, when observing the calls to ExtendedSecurityContextHolderStrategy during request processing with Spring Security 5.6.0-M2 or later, we can see:
1.) createEmptyContext() is called via Spring's NullSecurityContextRepository.loadContext -- OK
2.) setContext is called via SecurityContextPersistenceFilter.doFilter -- OK
3.) getContext is called from our custom ServletAuthenticationFilter. In this filter we populate additionalContextData inside ExtendedSecurityContextImpl, but as the test case is anonymous, we do not (always) set Authentication. -- OK
4.) getContext is called from Spring's AnonymousAuthenticationFilter. At this point the previously added additionalContextData is still there in the returned ExtendedSecurityContextImpl. -- OK
5.) createEmptyContext() is called via Spring's AnonymousAuthenticationFilter, after it notices that SecurityContextHolder.getContext().getAuthentication() == null
=> New ExtendedSecurityContextImpl gets created, and we lose our additionalContextData. -- NOT OK

So, you are right @jzheaux that by implementing one's own SecurityContextHolderStrategy, Spring Security's Filters - including AnonymousAuthenticationFilter - do call one's own methods.

However, if we look at ...

spring-security/core/src/main/java/org/springframework/security/core/context/SecurityContextHolderStrategy.java

Lines 49 to 55 in 9d37810

	/**
	* Creates a new, empty context implementation, for use by
	* <tt>SecurityContextRepository</tt> implementations, when creating a new context for
	* the first time.
	* @return the empty context.
	*/
	SecurityContext createEmptyContext();

... it seems quite Wrong to me that a method that is described to be one that is

for use by SecurityContextRepository implementations

and

when creating a new context for the first time.

is now called again by the default filters after the initial call from *SecurityContextRepository.loadContext

Requesting this issue to be re-opened.

[jzheaux]

@timo-eloranta Thanks for the detailed report.

3.) getContext is called from our custom ServletAuthenticationFilter. In this filter we populate additionalContextData inside ExtendedSecurityContextImpl, but as the test case is anonymous, we do not (always) set Authentication. -- OK

It's not clear to me why you'd want ServletAuthenticationFilter to populate anything other than authentication data; that is, data tied to the authenticated user. If the test case is anonymous, I'd imagine ServletAuthenticationFilter would do nothing and AnonymousAuthenticationFilter would take over to add an anonymous user.

There are three courses of action that I see:

• If you can populate additionalContextData even when no user is present, then I'd recommend not adding this in an authentication filter. Instead, add it after all authentication filters have weighed in. For example, you could do ServletAuthenticationFilter -> AnonymousAuthenticationFilter -> SecurityContextAdditionalDataFilter.

• If you really need to add the additional context data in ServletAuthenticationFilter, consider also adding it in AnonymousAuthenticationFilter by extending #createAuthentication. In this way, it gets added by every authentication filter.

• Or, another option may be to have your SecurityContextHolderStrategy#createNewToken perform a copy. If a security context exists already, then copy the additional context data from it into the new instance before returning it.

I believe the first option to be the cleanest separation of the two concerns, but it also comes from a place of not knowing what exactly is happening in your code base and how the user and the additional context data interact.

[spring-projects-issues]

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

[spring-projects-issues]

Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open the issue.

[SHxKM]

This doesn't work for me:

http
                // Disable session cookies  creation on Spring Security
                .sessionManagement().sessionCreationPolicy(SessionCreationPolicy.STATELESS).and()
                // Disable CSRF (cross site request forgery)
                .csrf().disable()
                .authorizeHttpRequests(request -> request
                        .requestMatchers(HttpMethod.GET, HEALTH_CHECK_PATH).permitAll()
                        .anyRequest()
                        .authenticated()
                )
                .authenticationProvider(new CustomTokenAuthenticationProvider())
                .addFilterAfter(new IntegrationAuthenticationFilter(), AnonymousAuthenticationFilter.class);



        return http.build();

@jzheaux wasn't this your suggestion? AnonymousAuthenticationFilter is still getting called after my IntegrationAuthenticationFilter...

I basically want something like this:

Authentication authentication = customTokenAuthenticationProvider.authenticate(pairAuthenticationRequest);
SecurityContext context = SecurityContextHolder.createEmptyContext();
context.setAuthentication(authentication);
SecurityContextHolder.setContext(context);
SecurityContextHolder.getContextHolderStrategy().setContext(context);            

And have AnonymousAuthenticationFilter find that authentication is indeed not null.