RequestHeaderAuthenticationFilter creates a session even if not configured to do so

[obourgain]

Describe the bug
Spring Security, even if configured with SessionCreationPolicy.NEVER or SessionCreationPolicy.STATELESS creates a session when using RequestHeaderAuthenticationFilter.

It may be caused by the change here 4479cef#diff-b9376389ef77383ad282c387359020ed122ad52d641cf25de70f104deae213d8R113
that changed AbstractPreAuthenticatedProcessingFilter to use a HttpSessionSecurityContextRepository by default, but fails to configure it properly with allowSessionCreation=false when using a SessionCreationPolicy that should not create sessions.

Also, when using NEVER the credentials may be read from the session (as expected) but with precedence over the headers sent in the request, which should be the source of truth.

This may affect all subclasses of AbstractPreAuthenticatedProcessingFilter and also other classes that started to use HttpSessionSecurityContextRepository by default in the commit mentionned above, but I didn't test this.

To Reproduce

• Configure an application with a RequestHeaderAuthenticationFilter and SessionCreationPolicy.NEVER or SessionCreationPolicy.STATELESS in the SecurityFilterChain.
• Make an HTTP call with a SM_USER header.

Note that using MockMvc still creates the session, as we can see with a debugger, but doesn't seem to set the Set-Cookie header.

Expected behavior

Srping Security should not create a session when configured to not create one.

Sample

I created a reproducer there:
https://github.com/obourgain/session-issue

[marcusdacoregio]

Hi, @obourgain. I think this is similar to #13840.

This is indeed affected by the change in Spring Security about who is responsible for persisting the security context. However, this has nothing to do with the DSL since you are not using it to configure the RequestHeaderAuthenticationFilter as you do with formLogin(), for example. Therefore, in that case, you should do:

var filter = new RequestHeaderAuthenticationFilter();
filter.setSecurityContextRepository(new RequestAttributeSecurityContextRepository());
// ...

[obourgain]

It is indeed similar to #13840 (comment)

I agree with the fact that as I created the filter, it's not Spring Security's role to change its configuration.
Yet, it is very surprising that the RequestHeaderAuthenticationFilter triggers the creation of a session by default.

Moreover, it may be a security issue, because with SessionCreationPolicy.NEVER, the Authentication stored in the session will be used even if in subsequent requests the header have changed, as you can see in the test in the reproducer I linked.

[marcusdacoregio]

Moreover, it may be a security issue, because with SessionCreationPolicy.NEVER, the Authentication stored in the session will be used even if in subsequent requests the header have changed, as you can see in the test in the reproducer I linked.

You can set RequestHeaderAuthenticationFilter#setCheckForPrincipalChanges(true) to check if the header is not equal to the current authentication principal, forcing it to re-authenticate.

[obourgain]

Yes, we can fix the configuration by calling setters explicitly.
My point is that the default behavior is very surprising, and seems to not follow the javadocs:

• from RequestHeaderAuthenticationFilter, which states that it will obtains username from the request and use it.
  It even indicates that the header is mandatory by default. It is therefore very surprising that this header may be ignored and the authentication read from the session.
• for AbstractPreAuthenticatedProcessingFilter, it mentionned the setCheckForPrincipalChanges, but I would think that security components should be secure by default and avoid surprise as much as possible.

The reason for using a HttpSessionSecurityContextRepository seems to be performance #6125 but I think this change in defaults is weakening the security level and probably exposing some applications that won't notice this change in behavior.

[marcusdacoregio]

for AbstractPreAuthenticatedProcessingFilter, it mentionned the setCheckForPrincipalChanges, but I would think that security components should be secure by default and avoid surprise as much as possible.

I don't follow how it does not have a secure by default posture. The Javadoc mentions how it behaves and guides you on how to change it if needed.

I agree that the following paragraph of the javadoc could be improved to be clear that the header is only required if authentication is required and link to the proper section of AbstractPreAuthenticatedProcessingFilter javadoc.

If the header is missing from the request, getPreAuthenticatedPrincipal will throw an exception. You can override this behaviour by setting the exceptionIfHeaderMissing property.

[obourgain]

I find the doc on both AbstractPreAuthenticatedProcessingFilter and RequestHeaderAuthenticationFilter misleading and not describing the real behavior.

RequestHeaderAuthenticationFilter's doc starts with

 A simple pre-authenticated filter which obtains the username from a request header, for use with systems such as CA Siteminder.

and do not mention storing things in a session.

In AbstractPreAuthenticatedProcessingFilter's doc, there is no mention of storing things in a session, but once we know that it wil do it, then we can infer that from the paragraph starting with

If the security context already contains an {@code Authentication} object...

but at first read, I would assume that the second paragraph

 The purpose is then only to extract the necessary information on the principal from the incoming request, rather than to authenticate them.

would apply and that by default the filters would just "read from the request" and not "read from the request and store that in a cookie and then for the next request use the value stored in the cookie".

That's my point, now we can close the issue.
Thank you.

[marcusdacoregio]

It would be great if you could provide a PR that improves the documentation, would you be interested in doing that?

[marbon87]

Hi @marcusdacoregio ,

we also stumble upon this change lately in production.

I agree with @obourgain that the current behaviour of the AbstractPreAuthenticatedProcessingFilter is not very intuitive and another big problem is that this change is not mentioned in the migration guide.

Furthermore the javadoc of HttpSecurity#sessionManagement() does not give any information that this configuration is only relevant for a few filters and not all.