hslavich / OneloginSamlBundle

OneLogin SAML Bundle for Symfony
MIT License
149 stars 94 forks source link

You cannot refresh a user from the EntityUserProvider that does not contain an identifier. The user object has to be serialized with its own identifier mapped by Doctrine. #178

Closed balazs92117 closed 3 years ago

balazs92117 commented 3 years ago

I saw #149 , but the issue still exists (or again). After the recent event based user persistence change (#172 ) the #174 changed the UserCreatedListener's default "needPersist" value to false. This causes the above error. If I change the false to true in Resorces/config/service.php:51 the login works well. So I guess, the system doesn't read the security.firewalls.main.saml.persist_user variable, just uses "false". It is important this error message appears only at first login, if the user does not yet exists in the DB. I would create a PR, but I'm not sure how (and where) should I read the config value.

a-menshchikov commented 3 years ago

@balazs92117 hi! Could you show your security.yaml, user mapping settings and a stack trace of the above error?

balazs92117 commented 3 years ago

Thank you for the answer @a-menshchikov! Here are the files:

security.yaml:

security:
    providers:
        user_provider:
            # Loads user from user repository
            entity:
                class: App\Entity\User
                property: elteId
    firewalls:
        dev:
            pattern: ^/(_(profiler|wdt)|css|images|js)/
            security: false
        main:
            # anonymous: ~
            saml:
                username_attribute: elteid
                # User factory service
                user_factory: my_user_factory
                # Persist new user. Doctrine is required.
                # always_use_default_target_path: true
                persist_user: true
            logout:
                path: /saml/logout
            provider: user_provider
    enable_authenticator_manager: true

services.yaml:

services:
    my_user_factory:
        class: App\Security\SAMLUserCreator

App\Security\SAMLUserCreator:


namespace App\Security;

use App\Entity\User;

use Hslavich\OneloginSamlBundle\Security\Authentication\Token\SamlTokenInterface;
use Hslavich\OneloginSamlBundle\Security\User\SamlUserFactoryInterface;
use Symfony\Component\Security\Core\User\UserInterface;

class SAMLUserCreator implements SamlUserFactoryInterface
{
    public function createUser($username, array $attributes = []): UserInterface
    {

        if(is_string($username)) {
            $idpAttributes = $attributes;
        } else {
            $idpAttributes = $username->getAttributes();
        }
        $user = new User();
        $user->setRoles(["ROLE_USER"]);
        $user->setElteId($idpAttributes["elteid"][0]);
        $user->setUniqueCode($idpAttributes["urn:oid:1.3.6.1.4.1.11914.0.1.154"][0] == "" ? $idpAttributes["username"][0] : $idpAttributes["urn:oid:1.3.6.1.4.1.11914.0.1.154"][0]);
        return $user;
    }
}

App\Entity\User:

<?php

namespace App\Entity;

use App\Repository\UserRepository;
use Doctrine\ORM\Mapping as ORM;
use Symfony\Component\Security\Core\User\UserInterface;

use Hslavich\OneloginSamlBundle\Security\User\SamlUserInterface;

/**
 * @ORM\Entity(repositoryClass="App\Repository\UserRepository")
 */
class User implements SamlUserInterface
{
    /**
     * @ORM\Id
     * @ORM\GeneratedValue
     * @ORM\Column(type="integer")
     */
    private $id;

    /**
     * @ORM\Column(type="string", length=180, unique=true)
     */
    private $uniqueCode;

    /**
     * @ORM\Column(type="json")
     */
    private $roles = [];

    /**
     * @ORM\Column(type="integer")
     */
    private $elteId;

    public function getId(): ?int
    {
        return $this->id;
    }

    public function getUniqueCode(): ?string
    {
        return $this->uniqueCode;
    }

    public function setUniqueCode(string $uniqueCode): self
    {
        $this->uniqueCode = $uniqueCode;

        return $this;
    }

    public function getUsername(): ?int
    {
        return $this->elteId;
    }
    public function getUserIdentifier(): ?int
    {
        return $this->elteId;
    }

    public function getRoles(): array
    {
        $roles = $this->roles;
        $roles[] = 'ROLE_USER';
        return array_unique($roles);
    }

    public function setRoles(array $roles): self
    {
        $this->roles = $roles;

        return $this;
    }

    public function getPassword(): ?string
    {
        return null;
    }

    public function getSalt(): ?string
    {
        return null;
    }

    public function eraseCredentials()
    {
        // If you store any temporary, sensitive data on the user, clear it here
        // $this->plainPassword = null;
    }

    public function getElteId(): ?int
    {
        return $this->elteId;
    }

    ....

    /**
     * @return string
     */
    public function serialize()
    {
        return serialize([$this->getId(), $this->getEltesId(), $this->getUniqueCode()]);
    }

    /**
     * @return void
     */
    public function unserialize($serialized)
    {
        list($this->id, $this->elteId, $this->uniqueCode) = unserialize($serialized);
    }

    public function setSamlAttributes(array $attributes)
    {
        $this->setRoles(array('ROLE_USER'));
    }

}
a-menshchikov commented 3 years ago

@balazs92117 could you also add the stack trace of error "You cannot refresh a user..."?

balazs92117 commented 3 years ago

Oh sorry, I forgot that:

InvalidArgumentException:
You cannot refresh a user from the EntityUserProvider that does not contain an identifier. The user object has to be serialized with its own identifier mapped by Doctrine.

  at vendor/symfony/doctrine-bridge/Security/User/EntityUserProvider.php:108
  at Symfony\Bridge\Doctrine\Security\User\EntityUserProvider->refreshUser(object(User))
     (vendor/symfony/security-http/Firewall/ContextListener.php:228)
  at Symfony\Component\Security\Http\Firewall\ContextListener->refreshUser(object(SamlToken))
     (vendor/symfony/security-http/Firewall/ContextListener.php:135)
  at Symfony\Component\Security\Http\Firewall\ContextListener->authenticate(object(RequestEvent))
     (vendor/symfony/security-bundle/Debug/WrappedLazyListener.php:49)
  at Symfony\Bundle\SecurityBundle\Debug\WrappedLazyListener->authenticate(object(RequestEvent))
     (vendor/symfony/security-http/Firewall/AbstractListener.php:26)
  at Symfony\Component\Security\Http\Firewall\AbstractListener->__invoke(object(RequestEvent))
     (vendor/symfony/security-bundle/Debug/TraceableFirewallListener.php:62)
  at Symfony\Bundle\SecurityBundle\Debug\TraceableFirewallListener->callListeners(object(RequestEvent), object(Generator))
     (vendor/symfony/security-http/Firewall.php:86)
  at Symfony\Component\Security\Http\Firewall->onKernelRequest(object(RequestEvent), 'kernel.request', object(TraceableEventDispatcher))
     (vendor/symfony/event-dispatcher/Debug/WrappedListener.php:117)
  at Symfony\Component\EventDispatcher\Debug\WrappedListener->__invoke(object(RequestEvent), 'kernel.request', object(TraceableEventDispatcher))
     (vendor/symfony/event-dispatcher/EventDispatcher.php:230)
  at Symfony\Component\EventDispatcher\EventDispatcher->callListeners(array(object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener)), 'kernel.request', object(RequestEvent))
     (vendor/symfony/event-dispatcher/EventDispatcher.php:59)
  at Symfony\Component\EventDispatcher\EventDispatcher->dispatch(object(RequestEvent), 'kernel.request')
     (vendor/symfony/event-dispatcher/Debug/TraceableEventDispatcher.php:151)
  at Symfony\Component\EventDispatcher\Debug\TraceableEventDispatcher->dispatch(object(RequestEvent), 'kernel.request')
     (vendor/symfony/http-kernel/HttpKernel.php:133)
  at Symfony\Component\HttpKernel\HttpKernel->handleRaw(object(Request), 1)
     (vendor/symfony/http-kernel/HttpKernel.php:79)
  at Symfony\Component\HttpKernel\HttpKernel->handle(object(Request), 1, true)
     (vendor/symfony/http-kernel/Kernel.php:199)
  at Symfony\Component\HttpKernel\Kernel->handle(object(Request))
     (public/index.php:20)        
a-menshchikov commented 3 years ago

@balazs92117 could you try this fix – https://github.com/a-menshchikov/OneloginSamlBundle/tree/fix_178_v2 ?

ls-philippe-gamache commented 3 years ago

Got the same problem.

a-menshchikov commented 3 years ago

Try the above branch one more time, please.

ls-philippe-gamache commented 3 years ago

There's an error :

$container->setDefinition('hslavich_onelogin_saml.user_created_listener'.$firewallName, new ChildDefinition('hslavich_onelogin_saml.user_created_listener'))

I think there's a dot missing : user_created_listener'.$firewallName, -> user_created_listener.'.$firewallName,

ls-philippe-gamache commented 3 years ago

Got this : When I correct the dot problem.

60 9.077 !! In ResolveChildDefinitionsPass.php line 76:

60 9.077 !!

60 9.077 !! Service "hslavich_onelogin_saml.user_created_listener.main": Parent definit

60 9.077 !! ion "hslavich_onelogin_saml.user_created_listener" does not exist.

60 9.077 !!

a-menshchikov commented 3 years ago

@ls-philippe-gamache :facepalm: it's my mistakes, sorry. Try again, please.

ls-philippe-gamache commented 3 years ago

Didn't change anything

a-menshchikov commented 3 years ago

@ls-philippe-gamache are there any errors? The above error about 'hslavich_onelogin_saml.user_created_listener' definition should have gone.

ls-philippe-gamache commented 3 years ago

It's not working if user not in the database. Still have the error. If the user is in the database, it's keep reloading the SAML call.

a-menshchikov commented 3 years ago

@ls-philippe-gamache can you debug the listener constructor call and show $needPersist value and call stack trace? I would be grateful if you also check that handleEvent of the listener called (or not) during a SamlAuthenticator::generateUser call.

ls-philippe-gamache commented 3 years ago

@balazs92117 did you try the fix yourself?

balazs92117 commented 3 years ago

Sorry, I had a really busy week, but now I had time to test it. For me it works! The new user is created and successfully persisted to DB. In the AbstractUserListener's constructor the $needPersist value is true now, as it should be. If I set "persist_user" to false in security.yaml, the original error shows. If I set it to "true", everything's ok. So I think the fix works!

ls-philippe-gamache commented 3 years ago

After reloading everything, look like it work.

a-menshchikov commented 3 years ago

Thank you folks! I will merge this fix ASAP.