Spring Security
  1. Spring Security
  2. SEC-2054

BasicAuthenticationFilter should extend OncePerRequestFilter

    Details

    • Type: New Feature New Feature
    • Status: Open
    • Priority: Trivial Trivial
    • Resolution: Unresolved
    • Affects Version/s: 3.1.2
    • Fix Version/s: 4.0 Backlog
    • Component/s: Core
    • Labels:
      None

      Description

      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.

        Issue Links

          Activity

          Hide
          Michael Osipov added a comment -

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

          Show
          Michael Osipov added a comment - I guess this will include/patch INCLUDE, FORWARD, etc too?
          Hide
          Rob Winch added a comment -

          Correct

          Show
          Rob Winch added a comment - Correct
          Hide
          Michael Osipov added a comment -

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

          Show
          Michael Osipov added a comment - Great, looking forward too. You did change the summary to BasicAuthFiter. This should apply to every header-based filter, shouldn't it??
          Hide
          Rob Winch added a comment -

          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

          Show
          Rob Winch added a comment - 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
          Hide
          Rob Winch added a comment -

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

          Show
          Rob Winch added a comment - Pushing to 4.0 partially due to time lines and partially due to passivity concerns

            People

            • Assignee:
              Rob Winch
              Reporter:
              Michael Osipov
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated: