Maks3w / FR3DLdapBundle

This bundle integrates LDAP Authentication with any user manager (Ex: FOSUserBundle)
119 stars 77 forks source link

Update user on every login? #53

Open Mika56 opened 10 years ago

Mika56 commented 10 years ago

Hi,

I use a LDAP directory as a central login system. Your bundle seems to be the only one that could help me authenticate my users with LDAP, so I gave it a try.

It works pretty well, but, sadly, my users aren't updated every time the login.

It seems to me like my users are first loaded from LDAP, then saved in local database (without the password). Then, on second login, LDAP is still used, but only to retrieve the user's username, then FOSUserBundle takes over, and logs the user.

My problem is, for example, if I update a field on my LDAP server, I would like that field to be updated on the application, but that doesn't seems to be the case.

Is this the expected behaviour?

I tried something like that, but that doesn't work either:

$service = $this->get('fr3d_ldap.security.user.provider');
$em = $this->getDoctrine()->getManager();
$em->persist($service->refreshUser($this->getUser()));
$em->flush();

(Doctrine recognizes this as a new user and tries to INSERT it (throwing an Integrity constraint violation) instead of UPDATE-ing the user)

Couldn't find a "updateEverytime" on fr3d_ldap.user.attributes in config either

Fraifrai commented 10 years ago

I had a similar problem : first login OK and second login KO because the bundle tried to create a new user with the same login. I extended baseldapmanager to handle this issue (I wish it may help you) : http://pastealacon.com/34168

pmithrandir commented 10 years ago

Fraifrai, I don't think you have the same issue.

I'm also looking for a way to force the bundle to update all field in the attribute list to be refresh on eavery login.

For example, a woman can change name, I may change the email address automaticaly, etc...

Right now, I can log in and my user password is tested on each connection, but the only update performed is :

UPDATE fos_user SET last_login = ? WHERE id = ?

If anyone found how to do it, I will be glad. Pierre

dkisselev commented 10 years ago

+1 on this. I might work on implementing it myself at some point.

Maks3w commented 9 years ago

I think this can be resolved with the new Hydrator step proposed in #89

You can add a custom hydrator and flush the changes in the hydrator() method.

Tell me if this is enough.

paoloavezzano commented 9 years ago

How can I persist the $user entity in the redefined hydrate() method? I can't find an easy way to inject the EntityManager.

Maks3w commented 9 years ago

Ough. http://symfony.com/doc/current/book/service_container.html#service-parameters

paoloavezzano commented 9 years ago

Yeah, I knew that, but when I call the construct with parent:__construct() I have issues with the arguments. I've set up other injections in the past where the service (not an HydratorInteface as in this case) didn't need any specific argument, like array $ldapUserAttributes in this case.

Basically I can't figure out how to keep everything as it is, just adding the @doctrine.orm.default_entity_manager argument in service.yml. In any case my goal is to persist the entity, so any different yet useful solution is still appreciated.

movsky commented 9 years ago

Did you find any solution on this?

mhnrm commented 8 years ago

Did anybody succeed in this issue? I'm also struggling at this...

I already use a custom UserHydrator class, as described in the Cookbook (just implementing HydratorInterface, not extending AbstractHydrator as suggested in #89) - but the problem is that hydrate() is called only on initial login, but not anymore if the user was already created locally. Overriding LdapManager obviously doesn't help either.

I appreciate any hint how to achieve hydration refresh on every login.

bertoost commented 8 years ago

@mhnrm This is also my problem. We're using AD's group structure in our application to give the user the right permissions (roles). When a session is expired, I would like to see the user is being hydrated again. This way we can clear sessions to renew everyone's permissions.

mhnrm commented 8 years ago

@bertoost: I was finally able to solve this by overriding the LdapAuthenticationProvider class following @baldurrensch's approach posted above. So the local user's permissions (and mail address) are sync'd to the LDAP on every login.

Here's what I did:

parameters:
    fr3d_ldap.security.authentication.provider.class: AppBundle\Security\Authentication\LdapAuthenticationProvider

services:
    fr3d_ldap.security.authentication.provider:
        class: "%fr3d_ldap.security.authentication.provider.class%"
        arguments:
            - '@security.user_checker'
            - ''
            - ''
            - '@fr3d_ldap.ldap_manager'
            - '@fos_user.user_manager'
            - "%security.authentication.hide_user_not_found%"
<?php

namespace AppBundle\Security\Authentication;

use AppBundle\Entity\User;
use FOS\UserBundle\Model\UserInterface;
use FOS\UserBundle\Model\UserManagerInterface;
use FR3D\LdapBundle\Ldap\LdapManagerInterface;
use FR3D\LdapBundle\Security\Authentication\LdapAuthenticationProvider as BaseProvider;
use FR3D\LdapBundle\Security\User\LdapUserProvider;
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
use Symfony\Component\Security\Core\Exception\AuthenticationServiceException;
use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
use Symfony\Component\Security\Core\User\ChainUserProvider;
use Symfony\Component\Security\Core\User\UserCheckerInterface;
use Symfony\Component\Security\Core\User\UserProviderInterface;

class LdapAuthenticationProvider extends BaseProvider
{
    /**
     * @var UserProviderInterface
     */
    private $userProvider;

    /**
     * @var LdapManagerInterface
     */
    private $ldapManager;

    /**
     * @var mixed
     */
    private $userManager;

    /**
     * Constructor.
     *
     * @param UserCheckerInterface $userChecker An UserCheckerInterface interface
     * @param string $providerKey A provider key
     * @param UserProviderInterface $userProvider An UserProviderInterface interface
     * @param LdapManagerInterface $ldapManager An LdapProviderInterface interface
     * @param UserManagerInterface $userManager
     * @param bool $hideUserNotFoundExceptions Whether to hide user not found exception or not
     */
    public function __construct(UserCheckerInterface $userChecker, $providerKey, UserProviderInterface $userProvider, LdapManagerInterface $ldapManager, UserManagerInterface $userManager, $hideUserNotFoundExceptions = true)
    {
        parent::__construct($userChecker, $providerKey, $userProvider, $ldapManager, $hideUserNotFoundExceptions);

        $this->userProvider = $userProvider;
        $this->ldapManager = $ldapManager;
        $this->userManager = $userManager;
    }

    /**
     * {@inheritdoc}
     */
    protected function retrieveUser($username, UsernamePasswordToken $token)
    {
        $user = $token->getUser();
        if ($user instanceof UserInterface) {
            return $user;
        }

        try {
            /** @var User $user */
            $user = $this->userProvider->loadUserByUsername($username);

            if ($this->userProvider instanceof ChainUserProvider) {

                /** @var ChainUserProvider $userProvider */
                $userProvider = $this->userProvider;
                foreach ($userProvider->getProviders() as $provider) {
                    if ($provider instanceof LdapUserProvider) {
                        /** @var User $ldapUser */
                        $ldapUser = $provider->loadUserByUsername($username);
                        $user->setEmail($ldapUser->getEmail());
                        $user->setRoles($ldapUser->getRoles());
                        // ...some additional syncing...

                        $this->userManager->updateUser($user);
                    }
                }
            }

            return $user;
        } catch (UsernameNotFoundException $notFound) {
            throw $notFound;
        } catch (\Exception $repositoryProblem) {
            $e = new AuthenticationServiceException($repositoryProblem->getMessage(), (int) $repositoryProblem->getCode(), $repositoryProblem);
            $e->setToken($token);

            throw $e;
        }
    }
}

hth, µ.

bertoost commented 8 years ago

@mhnrm Is this still working with FOSUserBundle? Since my provider-chain is configured to use that as first...

chain_provider:
  chain:
    providers: [fos_userbundle, fr3d_ldapbundle]
...

When the user is found locally, it will not traverse down to the LDAP bundle as far I understand :) And I really would like to use my custom hydrator again, since it contains a lot of logic to map AD's usergroups to roles etc.

Thanks anyway for sharing!

Maks3w commented 8 years ago

@bertoost In the example posted the loop traverse every user provider. A better option could be inject fr3d_ldap.security.user.provider in the constructor.

@mhnrm Thx for share your example. Unfortunatelly the provided solution is not valid for to be included in this project, it couple too much the UserManager, even UserManagerInterface not longer exists

mhnrm commented 8 years ago

@bertoost: Yes, same configuration here:

    providers:
        chain_provider:
            chain:
                providers: [fos_userbundle, fr3d_ldapbundle]

It took me some hours of serious debugging to understand what was happening, but this is where the override kicks in:

The original LdapAuthenticationProvider runs

 $user = $this->userProvider->loadUserByUsername($username);

which is satisfied by getting the local copy of the user from FOSUserBundle.

What I'm doing (and credits go to @baldurrensch, as this is exactly what he did before) is that after that, we anyway iterate over all UserProviders registered in the ChainUserProvider until we find our LdapUserPovider and call loadByUsername on it, which in turn will hydrate the user from LDAP again. So in the end, we just need to sync our exisiting local copy with the newly hydrated user and update/persist it.

No other logic needed, the custom Hydrator is reused.

This may not be the most beautiful solution, but it works flawlessly. Anyhow, I wish that the bundle would provide an easier way to achive this - maybe just adding some event hooks in retrieveUser?

mhnrm commented 8 years ago

@Maks3w Thanks for your reply. I'm aware of my solution being strongly coupled strongly to FOSUserBundle (which is, after what I understood, what you guys are trying to get rid of), therefore I did not bother with building a generic (and configurable) solution yet and offer it as a pull request. But it works for my (and, probably, @bertoost's) setup.

Anyway, I'm thinking about coming back to this later - I'm just a bit busy atm.

bertoost commented 8 years ago

@mhnrm Sounds good! Started to get it implemented an tested.. But one question; you're doing $user->setEmail(...) etc. in your provider-chain loop.. I do a lot more inside the Hydrator; like creating custom profile records (name, phone numbers etc. etc.). I do not want to do this twice.. I understand that $ldapUser is the hydrated version, but how can I merge that with the local user record ($user), without calling all different methods again ;-)

mhnrm commented 8 years ago

@bertoost I don't know how your User class is structured, but if those profile records are stored in an associated entity, you could just copy this from $ldapUser, as it is exactly what your hydrate() method returns. In my case, next to mail address and roles, there are some tenant specifc roles stored in a collection on the User class - this is what I'm actually copying in the line replaced by

// ...some additional syncing...

in the code above.

If all of your custom attributes are members of the User class, you'll probably have to copy them one by one. At least, you won't have to duplicate the logic for extracting them from the LDAP record - you can just use

$user->setMyAttribute($ldapUser->getMyAttribute();
bertoost commented 8 years ago

@mhnrm Yeah I understand. Was just wondering if there isn't an easier way.. Because I need to set some attributes on the user itself, the profile record needs to be replaced (simple indeed), user groups are replaced, since I created a setGroups() method to replace all and some others are easily overwritten now.

But the provider isn't being used yet.. somehow the service definition is not overriding the LDAP bundle defintion. Since my bundle is not set as child from the LDAP bundle, but from FOSUserBundle.. Maybe something to do with the order of loading bundles in the kernel class?

mhnrm commented 8 years ago

Did you also add the lines to services.yml as I posted above? This is crucial, especially the parameters: part.

bertoost commented 8 years ago

Yes I did. In my user bundle services.yml (which is configured to be child of FOSUserBundle)..

mhnrm commented 8 years ago

Hm, I have it in my AppBundle's services.yml. Maybe you're right - it may have something to do with loading order... You could try to put these lines inside your app/config/services.yml, or just move your user bundle below FR3DLdapBundle in AppKernel.php.

bertoost commented 8 years ago

My user bundle is already loaded after FOS and after the FR3D bundle. Moving it to the app/config/services.yml doesn't work either. It seems the authentication provider is not being triggered at all. I am resetting the session every time I retry to get into the provider, but that doesn't seems to be the trick either

mhnrm commented 8 years ago

How are you resetting the session? The AuthenticationProvider is triggered on every login (and logout, fwiw). So the user will have to login to get updated.

bertoost commented 8 years ago

For this I need to share some application details; I am working on an internal web application, where the authentication is arranged with the REMOTE_USER as per CookBook and a PreAuthenticatedToken. The first time the user is loaded via LDAP and second time it's the local user record. The user should always be authenticated in my application.

To reset the session; I have implemented the logout route from the FOS bundle and I hit the logout action from the bottom toolbar. Before I delete the session records from the table (only my ones of course). Then there is a new PHPSESSID and the user is authenticated immediately again. But the data is not refreshed.

mhnrm commented 8 years ago

Ok, that differs a little bit from my setup - we always require the user to login interactively. I'm afraid that I can't help you any further - you'll have to do some digging for yourself, sorry.

bertoost commented 8 years ago

Still think it should work the same. I tried to override the supports() method but nothing changed. It's hard to debug if the process is passing the custom provider or not

sirbaconjr commented 7 years ago

Hi, I know this is an old issue, but I hope my comment helps someone in the future. The best way I found to do this is:

  1. Set the chain like this: [fr3d_ldapbundle, fos_userbundle] (this will make the bundle call the hydrator on every login)
  2. Override the Hydrator
  3. Inject the EntityManager inside the Hydrator
  4. Inside the hydrate function get the user from database based on some attribute. If there is no user, create a new one.
  5. Fill the properties you need
if ($fosUser == null) {
    $fosUser = new User();
}
//...
$fosUser->setUsername($ldapEntry['uid'][0]);
//...
Paktusin commented 6 years ago

I make this solution in config.yml

parameters:
  fr3d_ldap.security.authentication.provider.class: 'Application\Sonata\MyLdapAuthenticationProvider'

in src\Application\Sonata\MyLdapAuthenticationProvider.php

<?php

namespace Application\Sonata;

use FR3D\LdapBundle\Ldap\LdapManagerInterface;
use FR3D\LdapBundle\Security\Authentication\LdapAuthenticationProvider;
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
use Symfony\Component\Security\Core\User\UserCheckerInterface;
use Symfony\Component\Security\Core\User\UserProviderInterface;

class MyLdapAuthenticationProvider extends LdapAuthenticationProvider
{
    private $ldapManager;
    private $userProvider;

    public function __construct(UserCheckerInterface $userChecker, $providerKey, UserProviderInterface $userProvider, LdapManagerInterface $ldapManager, $hideUserNotFoundExceptions = true)
    {
        $this->ldapManager = $ldapManager;
        $this->userProvider = $userProvider;
        parent::__construct($userChecker, $providerKey, $userProvider, $ldapManager, $hideUserNotFoundExceptions);
    }

    protected function retrieveUser($username, UsernamePasswordToken $token)
    {
        $user = parent::retrieveUser($username, $token);
        $ldap_user = $this->ldapManager->findUserByUsername('123');
        if($ldap_user) {
            $user->setPhone($ldap_user->getPhone());
            $this->userProvider->refreshUser($user);
        }
        return $user;
    }
}