Spring Security BasicAuthenticationFilter breaks SwitchUserFilter

Describe the bug When using the SwitchUserFilter with Basic auth, BasicAuthenticationFilter.authenticationIsRequired() fails when switched forcing a re-authentication.

To Reproduce Enable both basic auth and the SwitchUserFilter, switch to a new user and then make another request. BasicAuthenticationFilter.authenticationIsRequired() will return true since the current logged in user doesn't match the WWW-Authenticate header

Expected behavior BasicAuthenticationFilter.authenticationIsRequired() should also check if existingAuth has a GrantedAuthority of ROLE_PREVIOUS_ADMINISTRATOR, and check that username against username

        if (existingAuth instanceof UsernamePasswordAuthenticationToken
                && !existingAuth.getName().equals(username)) {
            return true;
        }

Comment From: smithtonson

An alternative fix (what I've implemented for myself locally in a CustomSwitchUserFilter) would be to replace creating a UsernamePasswordAuthenticationToken in SwitchUserFilter and instead create a new authentication token like SwitchUserToken this would bypass the existingAuth instanceof UsernamePasswordAuthenticationToken check.

Comment From: ianroberts

I've offered a possible fix in PR #9480, pretty much as described under "expected behaviour" above.

Comment From: eleftherias

Thanks for reaching out @smithtonson. I suspect this is an issue when using Basic auth and switching the user in the browser.

Typically with Basic auth, you would keep your application stateless and include the credentials in every request. In that case, it doesn't make sense to use the SwitchUserFilter, since the application doesn't have a way to remember that the user was switched.

You also have the option to provide the username and password using Basic auth, receive a Session ID and use the Session ID in all subsequent requests. In this case, it is sensible to use the SwitchUserFilter. This is the sequence of calls that I used and it worked successfully.

# get Session ID using basic auth
curl -v -u admin:password  http://localhost:8080/

# use Session ID when switching users
curl -v -b "JSESSIONID=..." -d "username=user1" http://localhost:8080/login/impersonate

# use same Session ID to impersonate user1
curl -v -b "JSESSIONID=..." http://localhost:8080/info

# received info for user1

A problem occurs if you are in a browser, because it will also send the Authorization: Basic ... header in the final request.

Your application will then have to deal with 2 conflicting ways that the user is identifying themselves, one being the Authorization header and the other being the JSESSIONID cookie.

The solution would be to not send both the Authorization header and the JSESSIONID cookie. In this specific case where you are switching users, you should only send the JSESSIONID cookie.

Does that make sense?

Comment From: spring-projects-issues

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

Comment From: eleftherias

@ianroberts Perhaps we can add something to the Javadoc to indicate that this is meant to be used with form login. I'm thinking "This filter assumes that the user performing the switch will be required to be logged in as normal, using form login". What do you think?

Comment From: ianroberts

I can see why you don't want to merge my PR and that's fine - I only contributed it because it was easy enough to do given I'd implemented it for my own use anyway in a modified copy of BasicAuthenticationFilter. I agree with you that a normal web app should use form login for the browser based parts and basic auth just for programmatic access, where you can properly control exactly which headers get sent by the client.

My specific situation is a bit of a weird special case, I have an existing app that uses normal form-based login, but I need to replace that with a special reverse proxy that hooks into our SSO infrastructure and then "fakes" a basic auth header to the backend (don't ask...). I'm personally happy to continue using my custom copy of the basic auth filter to achieve this.

One thing that would be nice though would be if authenticationIsRequired could be protected rather than private in BasicAuthenticationFilter, which would make it possible to apply customisations like this in a subclass rather than having to clone the entire class in my own package.

Comment From: smithtonson

With curl passing only the JSESSIONID cookie I can confirm the current SwitchUserFilter is fine.

However, what I ran into was that browsers by design continue to pass the Authorization header on subsequent requests along with the JSESSIONID.

So in a browser context, the url in setTargetUrl() will get executed with the correct switched user, but on subsequent requests the principal stored for a particular JSESSIONID no longer matches the user in the Authorization header. Which forces a logout (and ends up returning to the pre-switch user.)

@ianroberts implemented a fix for their code by replacing BasicAuthenticationFilter and checking the ROLE_PREVIOUS_ADMINISTRATOR authority matches; I did it with a custom SwitchUserFilter that uses a new SwitchUserAuthenticationToken (as opposed to UsernamePasswordAuthenticationToken).

Either way works, but I think the standard SwitchUserFilter is broken under reasonable use-cases when combined with basic auth.