Announcement Announcement Module
Collapse
No announcement yet.
AbstractPreAuthenticatedProcessingFilter: repeated getPreAuthenticatedPrincipal Page Title Module
Move Remove Collapse
X
Conversation Detail Module
Collapse
  • Filter
  • Time
  • Show
Clear All
new posts

  • AbstractPreAuthenticatedProcessingFilter: repeated getPreAuthenticatedPrincipal

    I'm integrating Facebook logins to my web app. After some research, I've decided to try to use a subclass of the AbstractPreAuthenticatedProcessingFilter to do this. In my subclass, I made an implementation of getPreAuthenticatedPrincipal that uses a Facebook REST client to validate that the user is logged into Facebook, and extract the user's Facebook ID. So far, so good.

    The confusing part is that this method gets invoked on every request, which is really bad for performance. Based on the AbstractPreAuthenticatedProcessingFilter's javadoc, I was expecting that once it had constructed an Authentication and put it into the security context, it would not need to do so on subsequent requests. I dug into the source code, and found what seems to be the reason. In the requiresAuthentication method, it calls getPreAuthenticatedPrincipal in order to compare it against the current Authentication's username, so that in case the "checkForPrincipalChanges" property is true, it can detect a change in the username. This happens on every filter invocation. It seems to me that a simple reorganization of the requiresAuthentication code to only run getPreAuthenticatedPrincipal if the checkForPrincipalChanges is true would fix this problem. The default is that checkForPrincipalChanges is false, so it seems like a waste to call getPreAuthenticatedPrincipal each time.

    I looked at the source code history, and this version has been around since about September of '09, so it seems very unlikely that I've uncovered a bug. Can anyone explain this logic to me?

  • #2
    Preauthentication assumes that something else has already authenticated the user and is managing its own state. Thus preauthentication will occur for every request. If you are looking for login with Facebook, I would recommend looking into using Spring Social which integrates Spring Security and Facebook very nicely. If you can't use Spring Social, you can create a custom authentication filter (in place of the UsernamePasswordAuthenticationFilter) which validates the user and then ensures to call SecurityContextHolder.getContext().setAuthenticati on(). Naturally you will need to ensure you have the SecurityContextPersistenceFilter properly configured (included with the http namespace configuration) to ensure that the authentication is stored.

    HTH,

    Comment


    • #3
      Thanks for the reply, and the tip about Spring Social. Not sure I have the appetite to learn a new Spring project, but it may be better than me monkeying around with Spring Security without being more knowledgeable.

      I am not sure I agree with you about the existing PreAuthenticated stuff though. According to my reading of the javadoc, and how the source code used to be before September 2009, there would only be one call to getPreAuthenticatedPrincipal per session. After that, due to the SecurityContextPersistenceFilter, there would be an Authentication object in the SecurityContext and so it would not call it again. It looks (to the untrained eye -- I realize I am not on firm ground here) like a simple artifact of the way that the requiresAuthentication / checkForPrincipalChanges logic was implemented.

      My solution so far has been to make my own version of AbstractPreAuthenticatedProcessingFilter, and make the logic only call getPreAuthenticatedPrincipal if checkForPrincipalChanges is true. This is not very elegant since I copied, pasted, and modified the source code, but it does have the desired effect of only invoking getPreAuthenticatedPrincipal once per session.

      Just to be clear: I think the original code has a bug. I don't like to declare it as such, because I am really not comfortable with my level of expertise in this area, and I've seen too many people declare "It's a bug!" when really they are just not fully understanding the situation.

      Comment


      • #4
        Rereading my response I can see it was worded rather poorly....perhaps it is one of those days. The Pre Authentication scenarios assume that the principal is maintained by some other external source. The filter is solely meant to create an Authentication with the external Principal and ensure they stay in synch. This makes sense for something like integrating with J2EE container security, Siteminder, etc.

        The authentication logic should not be placed in the getPreAuthenticatedPrincipal. It should be placed in the AuthenticationProvider. The getPreAuthenticatedPrincipal method should return the value required to perform the authentication. This value is not authenticated and thus should be able to return very quickly since it would be present in the request headers, cookies, etc. The AuthenticationProvider that is invoked indirectly through the doAuthenticate method does all the costly Authentication. This would only be invoked on requests that requiresAuthentication returns true.

        If you are still having problems feel free to post your code and I (or someone else) can provide tips on how to refactor it.

        Cheers,

        Comment


        • #5
          OK that makes sense -- it doesn't really matter if getPreAuthenticatedPrincipal is called each time, because it is supposed to be a lightweight method to get the identifier of the person to be authenticated. I'll look into making an appropriate AuthenticationProvider to do the expensive part. Thanks for the clarification.

          Here is the part of the javadoc that got me confused (for AbstractPreAuthenticatedProcessingFilter):

          "If the security context already contains an Authentication object (either from a invocation of the filter or because of some other authentication mechanism), the filter will do nothing by default."

          This did not turn out to be true.
          Last edited by jfrank; Mar 2nd, 2011, 08:07 AM.

          Comment

          Working...
          X