[Security] [Bug] User switching is not available for pre-authenticated users #2172

Closed
lisachenko opened this Issue Sep 14, 2011 · 11 comments

Projects

None yet

8 participants

@lisachenko

I have non-stateless authentication with x509 and want to enable user switching.
Expected that user switching should work.
Actual result: current user remains active

Bug detailed description:
Current user is authenticated with x509 certificate, so in the security context there is a PreAuthenticatedToken and session store it. Next step is to switch user. When user is switching then SwitchUserListener replaces existing PreAuthenticatedToken with UsernamePasswordToken and stores it in the security context and in the session, after that we have redirect to our page. We have valid token on the next request, however x509 authentication listener will be invoked again and it overrides existing UsernamePasswordToken.

How to fix that bug:
AbstractPreAuthenticatedListener class should not override existing authenticated tokens in the security context.

    // AbstractPreAuthenticatedListener:64-68
    if (null !== $token = $this->securityContext->getToken()) {
        if ($token->isAuthenticated()) {
            return;
        }
    }
Contributor

Is your code snippet the problem or the solution?

For quick reference, here is the file now:

https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Security/Http/Firewall/AbstractPreAuthenticatedListener.php#L65

My code snippet is solution, but I'm not sure that everything will be OK with that patch. Why do we need to check that $token is instance of PreAuthenticatedToken only and verify a user name to skip authentication?

Member
stof commented Apr 4, 2012
JHGitty commented Sep 14, 2014

There is a fix, but it was never merged? What happened?

Member
stof commented Sep 25, 2014

Well, we are not sure it was a fix. teh guy submitting it closed the PR before it got reviewed, and without explaining why.

dlancea commented Oct 29, 2014

I'm running into this same problem. I tried the code given by lisachenko and the submitted PR, but neither are working for me.

Forgive my naive attempt to fix this, I'm certainly not an expert at the Security Component.

One problem I've found is that the SwitchUserListener only uses UsernamePasswordToken tokens. This seems to mean the token never gets set in the securityContext in the PreAuthenticatedListener. If I change the SwitchUserListener to only produce PreAuthenticatedToken tokens the token at least shows up in the SecurityContext in the AbstractPreAuthenticatedListener handle function.

At this point the problem is that the PreAuthenticatedToken generated by the SwitchUserListener isn't authenticated, so it doesn't get returned by this line of code and accepted by the system as the "real" token.

Long story short, I think the problem is two fold, one being that SwitchUserListener only creates UsernamePasswordToken tokens and that AbstractPreAuthenticatedListener handler function doesn't handle existing tokens that aren't authenticated and don't match the user returned by getPreAuthenticatedData very well.

Contributor
daum commented May 13, 2015

+1

Contributor
daum commented May 13, 2015

FYI - it does appear that @lisachenko solution works still, the lines are slightly different but as far as I can tell it looks like it is working. @stof you see any reason that his solution should not work? I can make an updated PR if you'd like.

JHGitty commented Dec 24, 2015

This is currently the oldest still open bug of Symfony 😄
@daum Do you like to investigate it?

Contributor

I worked on it using your different comments but the new behaviour breaks an existing test.
Feedbacks about my solution are welcome.

Contributor

According to both @stof and @fabpot this issue should not be fixed and closed.
I submitted a PR for documentation about it #6673

@fabpot fabpot closed this Jun 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment