Spring Security
  1. Spring Security
  2. SEC-1763

Deal with 'nested switches' in SwitchUserFilter

    Details

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

      Description

      We have a system where administrators can 'impersonate' end-users. We use the SwitchUserFilter to accomplish this. However while impersonating, we want the administrators to have access to both the end-user views and the administration views. This was easy to accomplish, but it does mean that while impersonating an end-user, the administrator can use the administration views to impersonate another user. This currently results in 'nested' impersonations, e.g. admin1 -> user1 -> user2 -> user3. This works fine, except when the administrator attempts to exit the impersonation. Since the 'switches' are nested, they have to visit the 'exit' URL multiple times before they get back to their default admin-only view.

      My suggestion for addressing this is to add an ability to prevent the nested switches. When attempting a switch, the code would check to see if a switch is already in place, and if so, it would exit the existing switch before processing the new switch request.

      Alternatively, there could be an ability to perform an 'exit all' instead of just 'exit', which would exit all nested switches in a single operation.

        Activity

        Hide
        Luke Taylor added a comment -

        I've added a call to attemptExitUser() before switching, which will prevent nesting of switches. If the use tries to switch again, the previous switch will be discarded first.

        Show
        Luke Taylor added a comment - I've added a call to attemptExitUser() before switching, which will prevent nesting of switches. If the use tries to switch again, the previous switch will be discarded first.
        Hide
        Morley Howell added a comment -

        Thanks Luke! Haven't tried it yet, but it looks good.

        One question - what happens in this case:

        1. admin impersonates user1
        2. admin attempts to impersonate user2, but the impersonation of user2 fails

        Does the attempt to exit the previous impersonation in step 2 happen before or after the second impersonation is completed? If the implicit exit happens before the actual 2nd impersonation, then won't this scenario mean that attempting the 2nd impersonation but then having a failure after the implicit exit will boot you out of the 1st impersonation as well?

        I'm not sure this is really a problem, but perhaps an alternate solution would be to do the 2nd impersonation, and if it succeeds, then overwrite the 1st impersonation. That way if the 2nd one fails, your state won't have changed - you'll still be in the 1st one.

        Show
        Morley Howell added a comment - Thanks Luke! Haven't tried it yet, but it looks good. One question - what happens in this case: 1. admin impersonates user1 2. admin attempts to impersonate user2, but the impersonation of user2 fails Does the attempt to exit the previous impersonation in step 2 happen before or after the second impersonation is completed? If the implicit exit happens before the actual 2nd impersonation, then won't this scenario mean that attempting the 2nd impersonation but then having a failure after the implicit exit will boot you out of the 1st impersonation as well? I'm not sure this is really a problem, but perhaps an alternate solution would be to do the 2nd impersonation, and if it succeeds, then overwrite the 1st impersonation. That way if the 2nd one fails, your state won't have changed - you'll still be in the 1st one.
        Hide
        Luke Taylor added a comment -

        I don't think that should be a problem, since the attemptExitUser is called in the createSwitchUserToken, after the target UserDetails has been loaded and validated.

        Show
        Luke Taylor added a comment - I don't think that should be a problem, since the attemptExitUser is called in the createSwitchUserToken, after the target UserDetails has been loaded and validated.
        Hide
        Russell Schwager added a comment -

        This fix causes extraneous logging. Here's the call path:

        try

        { // SEC-1763. Check first if we are already switched. currentAuth = attemptExitUser(request); }

        catch (AuthenticationCredentialsNotFoundException e)

        { currentAuth = SecurityContextHolder.getContext().getAuthentication(); }

        and then within attemptExitUser():

        if (original == null)

        { logger.error("Could not find original user Authentication object!"); throw new AuthenticationCredentialsNotFoundException(messages.getMessage( "SwitchUserFilter.noOriginalAuthentication", "Could not find original Authentication object")); }

        So the method is intentionally throwing an exception on the first loginAs call because original is null, yet an error is logged. Subsequent loginAs don't have this issue as original is null.

        Show
        Russell Schwager added a comment - This fix causes extraneous logging. Here's the call path: try { // SEC-1763. Check first if we are already switched. currentAuth = attemptExitUser(request); } catch (AuthenticationCredentialsNotFoundException e) { currentAuth = SecurityContextHolder.getContext().getAuthentication(); } and then within attemptExitUser(): if (original == null) { logger.error("Could not find original user Authentication object!"); throw new AuthenticationCredentialsNotFoundException(messages.getMessage( "SwitchUserFilter.noOriginalAuthentication", "Could not find original Authentication object")); } So the method is intentionally throwing an exception on the first loginAs call because original is null, yet an error is logged. Subsequent loginAs don't have this issue as original is null.

          People

          • Assignee:
            Luke Taylor
            Reporter:
            Morley Howell
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: