Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

SEC-1763: Deal with 'nested switches' in SwitchUserFilter #1997

spring-issuemaster opened this Issue Jun 13, 2011 · 4 comments


None yet
1 participant

Morley Howell (Migrated from SEC-1763) said:

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.

Luke Taylor said:

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.

Morley Howell said:

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.

Luke Taylor said:

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.

Russell Schwager said:

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(
                "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.

@spring-issuemaster spring-issuemaster added this to the 3.1.0.RC3 milestone Feb 5, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment