lightSAML / SpBundle

SAML2 SP Symfony Bundle based on LightSAML
https://www.lightsaml.com/SP-Bundle/
MIT License
66 stars 70 forks source link

Lost LightSaml\Model\Protocol\Response on token serialization #76

Open NaxYo opened 6 years ago

NaxYo commented 6 years ago

I'm setting lightSAML SpBundle v1.2.0 (Symfony 2.8.44) for the first time and it seems to be partially working, but I'm having the following issue after getting my login_check redirection (IdP user exists and it does match user resolution):

Type error: Argument 1 passed to blah\Security\UsernameMapper::getUsername() must be an instance of LightSaml\Model\Protocol\Response, null given, called in blah/vendor/lightsaml/sp-bundle/src/LightSaml/SpBundle/Security/Authentication/Provider/LightsSamlSpAuthenticationProvider.php on line 188

The problem doesn't seem to come from the custom username mapper since the same thing happens with the default one.

Interesting fact that when the framework sets the token on the token storage, I have the response attribute there (vendor/symfony/symfony/src/Symfony/Component/Security/Http/Firewall/AbstractAuthenticationListener.php:209), but when it recovers from the session for unserializing, I get:

string(121)"C:68:"LightSaml\SpBundle\Security\Authentication\Token\SamlSpResponseToken":40:{a:4:{i:0;N;i:1;b:0;i:2;a:0:{}i:3;a:0:{}}}"
/home/wedesygn/Dev/webrand/vendor/symfony/symfony/src/Symfony/Component/Security/Http/Firewall/ContextListener.php:79

Where there is clearly no reference to the Response (or any other valuable information). I couldn't find much more by debugging, so not sure if it's a configuration issue on my end or a legit bug. Thanks either way!

NaxYo commented 6 years ago

I keep feeling like I'm losing something on the flow... it doesn't look right to be serializing the SamlSpResponseToken instead of resolving the user straight away.

I can work around the issue by adding:

// LightSaml\SpBundle\Security\Authentication\Token\SamlSpResponseToken

    public function serialize() {
        return serialize(
            array(
                \is_object($this->response) ? clone $this->response : $this->response
            )
        );
    }

    public function unserialize($serialized) {
        list($this->response) = unserialize($serialized);
    }

And it does work, but yeah, it doesn't feel right and it should be failing for everyone, what doesn't makes any sense to me.