Spring Security
  1. Spring Security
  2. SEC-1064

Modify AbstractSecurityInterceptor.authenticateIfRequired() to use a getAuthenticationManager() call

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Invalid
    • Affects Version/s: 2.0.4
    • Fix Version/s: 3.0.0 M1
    • Component/s: Core
    • Labels:
      None

      Description

      line 319: http://static.springframework.org/spring-security/site/xref/index.html

      Would it be possible to modify the line in the AbstractSecurityInterceptor.authenticateIfRequired() method from:

      authentication = authenticationManager.authenticate(authentication);

      to:

      authentication = getAuthenticationManager().authenticate(authentication);

      This is similar to how the AuthenticationProcessingFilter works and it allows us to extend the class, inject multiple AuthenticationManager properties, and then override the getAuthenticationManager() method to return the one to use for this request. We have 3 different authenticationManagers and we don't want to have to configure a FilterSecurityInterceptor for each one....where the only difference is the authenticationManager property.

      Thoughts?

        Activity

        Hide
        Luke Taylor added a comment - - edited

        Can you provide an explanation of the requirement for multiple AuthenticationManager instances in one interceptor please? How would you know which one to return from your getAuthenticationManager() method without any contextual information?

        Also, can you explain why the existing AuthenticationProvider architecture doesn't meet your needs, or why the same requirements can't be implemented through delegation (i.e. with a delegating AuthenticationManager which selected the appropriate instance).

        The situation is different with AbstractProcessingFilter in that the method is used to provide access to the AuthenticationManager for subclasses to make use of it. It's not intended as an extension point.

        Show
        Luke Taylor added a comment - - edited Can you provide an explanation of the requirement for multiple AuthenticationManager instances in one interceptor please? How would you know which one to return from your getAuthenticationManager() method without any contextual information? Also, can you explain why the existing AuthenticationProvider architecture doesn't meet your needs, or why the same requirements can't be implemented through delegation (i.e. with a delegating AuthenticationManager which selected the appropriate instance). The situation is different with AbstractProcessingFilter in that the method is used to provide access to the AuthenticationManager for subclasses to make use of it. It's not intended as an extension point.
        Hide
        Jason Arndt added a comment -

        The multiple AuthenticationManagers is because we have 3 different providers, but we cannot just register them all in one manager because it can't just spin through them all (user Ids are not unique) and attempt a valid login...

        which is what causes the need to have multiple AuthenticationManagers, which in turn causes the need to have multiple SecurityInterceptors. I was trying to simplify the spring-security-config by only having 1 AuthenticationProcessingFilter and 1 FilterSecurityInterceptor, which could be injected with the 3 AuthenticationManagers and dynamically switched.

        You are correct in that I would have an issue knowing which one to return from my getAuthenticationManager() method without any contextual information. I actually discovered that during the time I opened the issue and the time you commented on it and started down a slightly different path. I think the way I would need to do it is to override the beforeInvocationMethod(), which has a FilterInvocation and thus the Request object (for contextual), and then set the AuthenticationManager before delegating back to the super impl. However, I found another problem that prevents that from working:

        FilterSecurityInterceptor line 106: http://static.springframework.org/spring-security/site/xref/index.html
        It forces the "super" method to be called:

        InterceptorStatusToken token = super.beforeInvocation(fi);

        I don't think we can delegate to the AccessDecisionManager because I am trying to intercept the "authentication" calls.

        I understand your point about not being extension points and would welcome any other suggestions. Worst case we could just configure 3 of everything and then make sure all the FilterChainProxy configuration handles them correctly, but it was starting to get a bit messy.

        Anyway, I think that the line 106 of FilterSecurityInterceptor is still a bug, since that does seem like a valid extension point??...even if we end up throwing my craziness away...

        Thoughts?

        and thanks Luke!

        Show
        Jason Arndt added a comment - The multiple AuthenticationManagers is because we have 3 different providers, but we cannot just register them all in one manager because it can't just spin through them all (user Ids are not unique) and attempt a valid login... which is what causes the need to have multiple AuthenticationManagers, which in turn causes the need to have multiple SecurityInterceptors. I was trying to simplify the spring-security-config by only having 1 AuthenticationProcessingFilter and 1 FilterSecurityInterceptor, which could be injected with the 3 AuthenticationManagers and dynamically switched. You are correct in that I would have an issue knowing which one to return from my getAuthenticationManager() method without any contextual information. I actually discovered that during the time I opened the issue and the time you commented on it and started down a slightly different path. I think the way I would need to do it is to override the beforeInvocationMethod(), which has a FilterInvocation and thus the Request object (for contextual), and then set the AuthenticationManager before delegating back to the super impl. However, I found another problem that prevents that from working: FilterSecurityInterceptor line 106: http://static.springframework.org/spring-security/site/xref/index.html It forces the "super" method to be called: InterceptorStatusToken token = super.beforeInvocation(fi); I don't think we can delegate to the AccessDecisionManager because I am trying to intercept the "authentication" calls. I understand your point about not being extension points and would welcome any other suggestions. Worst case we could just configure 3 of everything and then make sure all the FilterChainProxy configuration handles them correctly, but it was starting to get a bit messy. Anyway, I think that the line 106 of FilterSecurityInterceptor is still a bug, since that does seem like a valid extension point??...even if we end up throwing my craziness away... Thoughts? and thanks Luke!
        Hide
        Luke Taylor added a comment - - edited

        AccessDecisionManager was a typo - I meant AuthenticationManager. Why can't you use a custom Authentication token - extend an existing implementation and add the contextual information you need in the authentication mechanism (e.g. the AuthenticationProcessingFilter). Then the AuthenticationManager can extract it and delegate appropriately. Alternatively, add it through a custom AuthenticationDetailsSource (which will set the "details" property of the Authentication request when it is created in the web tier).

        I don't get what you mean by a bug at line 106. The beforeInvocation and afterInvocation methods are intended to be called by subclasses, passing the appropriate type of secure object (MethodInvocation or FilterInvocation). Note also that the URLs your pasting are to the xref index. It uses frames so you need to bookmark a line number (they are links) or classname.

        Show
        Luke Taylor added a comment - - edited AccessDecisionManager was a typo - I meant AuthenticationManager. Why can't you use a custom Authentication token - extend an existing implementation and add the contextual information you need in the authentication mechanism (e.g. the AuthenticationProcessingFilter). Then the AuthenticationManager can extract it and delegate appropriately. Alternatively, add it through a custom AuthenticationDetailsSource (which will set the "details" property of the Authentication request when it is created in the web tier). I don't get what you mean by a bug at line 106. The beforeInvocation and afterInvocation methods are intended to be called by subclasses, passing the appropriate type of secure object (MethodInvocation or FilterInvocation). Note also that the URLs your pasting are to the xref index. It uses frames so you need to bookmark a line number (they are links) or classname.
        Hide
        Jason Arndt added a comment -

        I think that is a better way and will look into it.... Please resolve this bug as INVALID. Thanks!

        Show
        Jason Arndt added a comment - I think that is a better way and will look into it.... Please resolve this bug as INVALID. Thanks!

          People

          • Assignee:
            Luke Taylor
            Reporter:
            Jason Arndt
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: