Spring Security
  1. Spring Security
  2. SEC-1788

AbstractPreAuthenticatedProcessingFilter: Unnecessary calls to getPreAuthenticatedPrincipal() from requiresAuthentication()

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Complete
    • Affects Version/s: 3.0.5, 3.1.0.RC2
    • Fix Version/s: 3.1.0.RC3
    • Component/s: Web
    • Labels:
      None

      Description

      The requiresAuthentication(...) method in the AbstractPreAuthenticatedProcessingFilter class creates a principal object before it is known whether it will be needed. If checkForPrincipalChanges is set to false, the getPreAuthenticatedPrincipal(...) call is unnecessary.

      The current code:

      Object principal = getPreAuthenticatedPrincipal(request);
      if (checkForPrincipalChanges &&
      ! currentUser.getName().equals(principal))

      { // Check session... return true; }

      return false;

      Could be more efficiently written:

      if (checkForPrincipalChanges) {
      Object principal = getPreAuthenticatedPrincipal(request);
      if (! currentUser.getName().equals(principal))

      { // Check session... return true; }

      }
      return false;

      The unnecessary call to getPreAuthenticatedPrincipal(...) may be trivial for most implementations but I'm in a situation where I'm working with a corporate security service which saves the username of the preauthenticated user inside an encrypted cookie. Each call to getPreAuthenticatedPrincipal(...) requires that the cookie be decrypted and validated. Consequently this is occurs multiple times for each ServletRequest (once for each resource associated with the page: image, css, js, etc.)

      This difficulty could be worked around by subclassing AbstractPreAuthenticatedProcessingFilter, but, alas, requiresAuthentication(...) is private: see SEC-1759.

        Activity

        Hide
        Luke Taylor added a comment -

        I've modified the class to only make the call to getPreAuthenticationPrincipal if checkForPrincipalChanges is true.

        Show
        Luke Taylor added a comment - I've modified the class to only make the call to getPreAuthenticationPrincipal if checkForPrincipalChanges is true.

          People

          • Assignee:
            Luke Taylor
            Reporter:
            Andrew Lamb
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: