SEC-2054: BasicAuthenticationFilter should not invoke on ERROR dispatch

[spring-projects-issues]

Michael Osipov (Migrated from SEC-2054) said:

I have configured the following security element:

<security:http use-expressions="true" realm="My Application">

        <security:intercept-url
            pattern="/app/demo"
            access="hasRole('Info')" />

        <security:intercept-url
            pattern="/app/**"
            access="isAuthenticated()" />

        <security:intercept-url
            pattern="/admin/**"
            access="hasRole('Admin')" />

        <security:logout invalidate-session="true" delete-cookies="JSESSIONID"
            logout-url="/logout" logout-success-url="/logout-success" />

        <security:http-basic />
</security:http>

I have set up custom error pages in my web.xml:

!-- error pages -->
    <error-page>
        <error-code>401</error-code>
        <location>/errors/401</location>
    </error-page>

    <error-page>
        <error-code>403</error-code>
        <location>/errors/403</location>
    </error-page>

    <error-page>
        <error-code>404</error-code>
        <location>/errors/404</location>
    </error-page>

    <error-page>
        <error-code>500</error-code>
        <location>/errors/500</location>
    </error-page>

If for example an authentication fails the 401 page should be displayed.

Now, if a client sends an invalid Basic value the filter chain finds that and calls the 401 page through Tomcat. Unfortunately, the filter chain proxy does not know that this is one request with an error forward only.

It fires the entire auth chain again:

14:29:38.552 [http-8080-1] DEBUG o.s.s.web.util.AntPathRequestMatcher - Checking match of request : '/app'; against '/ext-resources/**'
14:29:38.552 [http-8080-1] DEBUG o.s.s.web.util.AntPathRequestMatcher - Checking match of request : '/app'; against '/resources/**'
14:29:38.552 [http-8080-1] DEBUG o.s.s.web.util.AntPathRequestMatcher - Checking match of request : '/app'; against '/rest/**'
14:29:38.552 [http-8080-1] DEBUG o.s.security.web.FilterChainProxy - /app at position 1 of 9 in additional filter chain; firing Filter: 'SecurityContextPersistenceFilter'
14:29:38.552 [http-8080-1] DEBUG o.s.s.w.c.HttpSessionSecurityContextRepository - No HttpSession currently exists
14:29:38.552 [http-8080-1] DEBUG o.s.s.w.c.HttpSessionSecurityContextRepository - No SecurityContext was available from the HttpSession: null. A new one will be created.
14:29:38.552 [http-8080-1] DEBUG o.s.security.web.FilterChainProxy - /app at position 2 of 9 in additional filter chain; firing Filter: 'LogoutFilter'
14:29:38.553 [http-8080-1] DEBUG o.s.security.web.FilterChainProxy - /app at position 3 of 9 in additional filter chain; firing Filter: 'BasicAuthenticationFilter'
14:29:38.553 [http-8080-1] DEBUG o.s.s.w.a.w.BasicAuthenticationFilter - Authentication request for failed: org.springframework.security.authentication.BadCredentialsException: Invalid basic authentication token
14:29:38.553 [http-8080-1] DEBUG o.s.s.w.c.HttpSessionSecurityContextRepository - SecurityContext is empty or contents are anonymous - context will not be stored in HttpSession.
14:29:38.553 [http-8080-1] DEBUG o.s.s.w.c.SecurityContextPersistenceFilter - SecurityContextHolder now cleared, as request processing completed
14:29:38.554 [http-8080-1] DEBUG o.s.s.web.util.AntPathRequestMatcher - Checking match of request : '/errors/401'; against '/ext-resources/**'
14:29:38.554 [http-8080-1] DEBUG o.s.s.web.util.AntPathRequestMatcher - Checking match of request : '/errors/401'; against '/resources/**'
14:29:38.554 [http-8080-1] DEBUG o.s.s.web.util.AntPathRequestMatcher - Checking match of request : '/errors/401'; against '/rest/**'
14:29:38.554 [http-8080-1] DEBUG o.s.security.web.FilterChainProxy - /errors/401 at position 1 of 9 in additional filter chain; firing Filter: 'SecurityContextPersistenceFilter'
14:29:38.554 [http-8080-1] DEBUG o.s.s.w.c.HttpSessionSecurityContextRepository - No HttpSession currently exists
14:29:38.555 [http-8080-1] DEBUG o.s.s.w.c.HttpSessionSecurityContextRepository - No SecurityContext was available from the HttpSession: null. A new one will be created.
14:29:38.555 [http-8080-1] DEBUG o.s.security.web.FilterChainProxy - /errors/401 at position 2 of 9 in additional filter chain; firing Filter: 'LogoutFilter'
14:29:38.555 [http-8080-1] DEBUG o.s.security.web.FilterChainProxy - /errors/401 at position 3 of 9 in additional filter chain; firing Filter: 'BasicAuthenticationFilter'
14:29:38.555 [http-8080-1] DEBUG o.s.s.w.a.w.BasicAuthenticationFilter - Authentication request for failed: org.springframework.security.authentication.BadCredentialsException: Invalid basic authentication token
14:29:38.555 [http-8080-1] DEBUG o.s.s.w.c.HttpSessionSecurityContextRepository - SecurityContext is empty or contents are anonymous - context will not be stored in HttpSession.
14:29:38.555 [http-8080-1] DEBUG o.s.s.w.c.SecurityContextPersistenceFilter - SecurityContextHolder now cleared, as request processing completed

Now the FilterSecurityInterceptor maintains an observeOncePerRequest property which tells him to observe checks only once. The FilterChainProxy does not have such a property which treats a subrequest a whole new request. The response gets reset twice and no output it written to the client.

I could set the error pages to security="none" but I want to maintain the security context on all pages whether they are protected or not.

[spring-projects-issues]

Michael Osipov said:

Forgot to mention:

My security filter is configured like this:

<filter>
        <filter-name>springSecurityFilterChain</filter-name>
        <filter-class>org.springframework.web.filter.DelegatingFilterProxy</filter-class>
    </filter>

    <filter-mapping>
        <filter-name>springSecurityFilterChain</filter-name>
        <url-pattern>/*</url-pattern>
        <dispatcher>REQUEST</dispatcher>
        <dispatcher>FORWARD</dispatcher>
        <dispatcher>ERROR</dispatcher>
    </filter-mapping>

[spring-projects-issues]

Rob Winch said:

Why would you map the springSecurityFilterChain to the ERROR dispatcher if you only want it to be invoked once? By adding an observeOncePerRequest option it is effectively the same thing as removing ERROR. Perhaps you are really wanting to add observeOncePerRequest to the BasicAuthenticationFilter?

[spring-projects-issues]

Michael Osipov said:

Removing ERROR is not the same as invoking once. As I have mentioned before, I want to maintain the security context on all pages whether they are protected or not. E.g., on a 403 page: the user is logged in but simply does not have the appropriate role.

I do not intend to modify the BasicAuthFilter, I think this can be done globally in the FilterChainProxy just as in the FilterSecurityInterceptor. I have found few discussions in the spring forums and stackoverlow with a resembling problem.

[spring-projects-issues]

Rob Winch said:

Only processing the REQUEST is effectively the same as stating that the springSecurityFilterChain should be invoked once when filtering on every URL. When the HTTP request is made, the FilterChainProxy will be invoked as normal. On the second invocation of the FilterChainProxy, the invocation of FilterChainProxy will be skipped (thus effectively removing it from the FilterChain). The SecurityContext will not be populated because, as a delegate of FilterChainProxy, the SecurityContextPersistenceFilter will not be invoked.

The attached sample application demonstrates that this is the case.

• Visit the URL http://localhost:8080/jira/admin/ as the user user/password will produce an empty SecurityContext (the response is Hello to an empty String).
• Update the web.xml to use springSecurityFilterChain (instead of onceSpringSecurityFilterChain). Now visit the same URL with the same user and you will see that it produces a non empty SecurityContext
• Now remove the ERROR dispatcher and you will see that it produces an empty SecurityContext again (same as the once per request configuration)

The reason you are not seeing a response written out is because when the BasicAuthenticationFilter is attempting to authenticate the second time it performs response.sendError a second time. Invoking sendError twice is not allowed and to prevent infinite recursion the container avoids processing the second invocation. You can fix this by:

• Changing the security.xml configuration to use onceBasicAuthenticationFilter instead of basicAuthenticationFilter
• Ensure to add the ERROR dispatcher element back and using springSecurityFilterChain as normal

You will find that the response body is written out when failing to authenticate. When the user authenticates and an error occurs the user is properly displayed.

As demonstrated above, adding observeOncePerRequest to FilterChainProxy will not fix the issue. The actual issue, the response not being written out, is not a bug since Spring Security does what it is being configured to do. At best this is an enhancement request. Anyone wanting a once per request filter can easily create a filter similar to the OncePerRequestDelegatingFilter in the sample application. Alternatively, the LoginUrlAuthenticationEntryPoint can be used which will allow the controller tier to write to the response without using the sendError that is causing this issue.

[spring-projects-issues]

Rob Winch said:

Updated to new feature per discussion on the JIRA

[spring-projects-issues]

Michael Osipov said:

I am sorry, I wasn't able to respond anytime sooner.

Though I am not an expert in configuring a Spring app, are you sure that this isn't a typo in the web.xml:

 <filter>
    <filter-name>springSecurityFilterChain</filter-name>
    <filter-class>org.springframework.web.filter.DelegatingFilterProxy</filter-class>
  </filter>
  <filter>
    <filter-name>onceSpringSecurityFilterChain</filter-name>
    <filter-class>org.springframework.web.filter.DelegatingFilterProxy</filter-class>
  </filter>

Both refer to the same class.

Are you sure that you have configured the application correctly? The security chain does not issue a WWW-Authenticate Basic because the 403EntryPoint does not do that.

The reason you are not seeing a response written out is because when the BasicAuthenticationFilter is attempting to authenticate the second time it performs response.sendError a second time. Invoking sendError twice is not allowed and to prevent infinite recursion the container avoids processing the second invocation.

Exactly, this is what I have observed. In this case, the second auth is just unnecessary.

I am fully satistified with a wrapper which will guarantee that my filter won't run several times without losing the security context. Since I am using a custom filter in my final app, I would wrap it. What I would appreciate is that the default auth filters would have appropriate out-of-the-box support for this with a mere xml attribute. This would save a lot of grief.

Again, brilliant analysis. Though, I still do not fully understand how the FilterChainProxy really works compared to the FilterSecurityInterceptor or how both interact. Deducing from this, it is still not completely clear to me why this cannot be added to the FilterChainProxy directly. I guess I am missing the big picture which I did not find in the Spring Security manual.

[spring-projects-issues]

Rob Winch said:

In reply to comment #6:

First of all, the project cannot be build. I guess, I am missing some spring repo

Everything (including org.slf4j:slf4j-api:jar:1.5.10) is available in maven central, so unless you have modified your settings.xml or you are behind a proxy it should not require any additional repositories. I confirmed that it builds for me with an empty maven repository and executing mvn clean package successfully.

Though I am not an expert in configuring a Spring app, are you sure that this isn't a typo in the web.xml:

<filter>
    <filter-name>springSecurityFilterChain</filter-name>
    <filter-class>org.springframework.web.filter.DelegatingFilterProxy</filter-class>
  </filter>
  <filter>
    <filter-name>onceSpringSecurityFilterChain</filter-name>
    <filter-class>org.springframework.web.filter.DelegatingFilterProxy</filter-class>
  </filter>

Both refer to the same class.

This is not a typo. DelegatingFilterProxy looks up a Spring bean by the name of the Filter and delegates all the work to it. So by changing the filter name we are pointing to a different object in the Spring configuration. You can find additional information on DelegatingFilterProxy in the Spring Security reference.

Though, I still do not fully understand how the FilterChainProxy really works compared to the FilterSecurityInterceptor or how both interact. Deducing from this, it is still not completely clear to me why this cannot be added to the FilterChainProxy directly. I guess I am missing the big picture which I did not find in the Spring Security manual.

I'm sorry you are still uncertain of how things work. If you haven't already read it, you might read the Security Filter Chain section of the reference which describes how it behaves. You can also sign up for the Spring Security Fundamentals webinar where I will talk about how the FilterChainProxy works in detail. Of course if you miss it, the webinar will be available on the SpringSourceDev YouTube channel.

[spring-projects-issues]

Michael Osipov said:

In reply to comment #6:

First of all, the project cannot be build. I guess, I am missing some spring repo

Everything (including org.slf4j:slf4j-api:jar:1.5.10) is available in maven central, so unless you have modified your settings.xml or you are behind a proxy it should not require any additional repositories. I confirmed that it builds for me with an empty maven repository and executing mvn clean package successfully.

I have already changed the my comment. This was some network problem with my local Nexus instance.

Everything (including org.slf4j:slf4j-api:jar:1.5.10) is available in maven central, so unless you have modified your settings.xml or you are behind a proxy it should not require any additional repositories. I confirmed that it builds for me with an empty maven repository and executing mvn clean package successfully.

Though I am not an expert in configuring a Spring app, are you sure that this isn't a typo in the web.xml:

    <filter>
        <filter-name>springSecurityFilterChain</filter-name>
        <filter-class>org.springframework.web.filter.DelegatingFilterProxy</filter-class>
      </filter>
      <filter>
        <filter-name>onceSpringSecurityFilterChain</filter-name>
        <filter-class>org.springframework.web.filter.DelegatingFilterProxy</filter-class>
      </filter>

Both refer to the same class.

This is not a typo. DelegatingFilterProxy looks up a Spring bean by the name of the Filter and delegates all the work to it. So by changing the filter name we are pointing to a different object in the Spring configuration. You can find additional information on DelegatingFilterProxy in the Spring Security reference.

There is too much voodoo happening inside the framework. That's why I did not understand the wiring in the security.xml. It did not look complete to me.

I'm sorry you are still uncertain of how things work. If you haven't already read it, you might read the Security Filter Chain section of the reference which describes how it behaves.

I have read this part twice. I got it but I still have the feeling that a state chart would help tremendously help to understand the complete wiring behind the scenes. I am in for the webinar too.

So, how can we continue to improve the Spring experience in this case?

[spring-projects-issues]

Michael Osipov said:

Rob, can wie reuse the org.springframework.web.filter.OncePerRequestFilter somehow?

[spring-projects-issues]

Rob Winch said:

No because it clears out the attribute after it executes (i.e. it would still occur twice). The call stack would look something like this:

Container start process request
   FilterChainProxy
   FilterChainProxy

This is different for FORWARD, INCLUDE, etc which is more like:

Container start process request
   FilterChainProxy
          FilterChainProxy

As you can see the clearing of the attribute would occur before the second invocation when processing an error and after invocation of a FORWARD, INCLUDE, etc. This means that it only prevents execution on FORWARD, INCLUDE, etc.

[spring-projects-issues]

Michael Osipov said:

OK, some custom wrapper or built-in support is necessary.

[spring-projects-issues]

Rob Winch said:

Thinking about this more, I really think it should be resolved in Spring Web. See SPR-9895

[spring-projects-issues]

Michael Osipov said:

Rob, issue SPR-9895 has been closed and there is method now. How does one now connect this with the DelegatingFilterProxy?

[spring-projects-issues]

Rob Winch said:

Spring Security 3.2 will be compiled against the new class and override that method.

[spring-projects-issues]

Michael Osipov said:

I guess this will include/patch INCLUDE, FORWARD, etc too?

[spring-projects-issues]

Rob Winch said:

Correct

[spring-projects-issues]

Michael Osipov said:

Great, looking forward too. You did change the summary to BasicAuthFiter. This should apply to every header-based filter, shouldn't it??

[spring-projects-issues]

Rob Winch said:

I will likely do some more evaluation at the time I address this issue. For now the scope is for the immediate problem, but again will likely change. The main point to updating the text was that it wasn't going to be a generic wrapper anymore

[spring-projects-issues]

Rob Winch said:

Pushing to 4.0 partially due to time lines and partially due to passivity concerns

[spring-projects-issues]

Rob Winch said:

This is now resolved in master. I decided to focus on this specific problem at hand since we did not have concrete use cases for other Filter's and dispatches.

[spring-projects-issues]

Michael Osipov said:

Thanks for the fix. Does it really suffice to extend from OncePerRequestFilter and it is guaranteed to be executed only once? Is it still valid to have this configuration though you responded this earlier?

[spring-projects-issues]

Rob Winch said:

Since SPR-9895 has been resolved, yes it is suffice to extend OncePerRequestFilter. Please go ahead and give the latest snapshot a try. You will need to ensure to update your dependencies to use Spring 4.1.2.RELEASE and Spring Security 4.0.0.CI-SNAPSHOT. You can resolve SNAPSHOTs with maven using the following maven repository:

  <repositories>
    <repository>
      <id>snapshot</id>
      <url>http://repo.spring.io/libs-snapshot</url>
    </repository>
  </repositories>

NOTE: You may notice that the commit contains a test to ensure this works. I also have ran this through a simple integration test to ensure the unit test is properly setup.

[spring-projects-issues]

Michael Osipov said:

Thanks a lot Rob, I will add this repo to our local Nexus instance and will give it a try next week. I will update the issue with the outcome of the test soon.

[spring-projects-issues]

This issue depends on https://jira.spring.io/browse/SPR-9895

[spring-projects-issues]

This issue depends on https://jira.spring.io/browse/SPR-9895

[spring-projects-issues]

This issue depends on https://jira.spring.io/browse/SPR-9895

[spring-projects-issues]

This issue depends on https://jira.spring.io/browse/SPR-9895