SocalNick / ScnSocialAuth

Uses the HybridAuth PHP library to Enable authentication via Google, Facebook, Twitter, Yahoo!, etc for the ZfcUser ZF2 module.
BSD 3-Clause "New" or "Revised" License
216 stars 110 forks source link

How to (correctly) extend ScnSocialAuth\Authentication\Adapter\HybridAuth::authenticate() method? #202

Closed diogodomanski closed 9 years ago

diogodomanski commented 9 years ago

Hi,

I'm using ScnSocialAuth + Doctrine in a ZF2 application. I need to add some fields to "user" and "user_provider" tables and populate them during authentication process.

What I've done up to now was:

  1. alter these tables structures (adding the fields I need)
  2. create two custom entities to override the default ones (ZfcUser\Entity\User and ScnSocialAuth\Entity\UserProvider) with the needed fields (and their getters and setters methods)
  3. modify config files to use these new entities (zfcuser.global.php and scn-social-auth.global.php) instead the default ones

What I need now is to use the user's profile data (for example, from Facebook) to hydrate my custom User and UserProvider entities, so they populate the records to be inserted/updated into "user" and "user_provider" tables.

Looking into ScnSocialAuth\Authentication\Adapter\HybridAuth class I've realized that there are some methods that aims to hydrate these entities with the values from user's profile data. These methods have the name like ToLocalUser().

The problem is that they set only a few attributes of the entities. For example, the facebookToLocalUser do the following:

protected function facebookToLocalUser($userProfile)
    {
        ...

        $localUser = $this->instantiateLocalUser();
        $localUser->setEmail($userProfile->emailVerified)
            ->setDisplayName($userProfile->displayName)
            ->setPassword(__FUNCTION__);
        $result = $this->insert($localUser, 'facebook', $userProfile);

        return $localUser;
    }

My question is: how can I make the authentication process populate custom User and UserProvider entities correctly?

Thanks

ltouro commented 9 years ago

I think you should register an event handler at ScnSocialAuth\Authentication\Adapter\HydridAuth registerViaProvider or registerViaProvider.post.

In these events, you should have the user object, provider as string and the userProfile, which is the user object on the provider

diogodomanski commented 9 years ago

Hi Itouro,

I think this is the way. I've tried to write the following test code into my Module's onBootstrap() method

public function onBootstrap(MvcEvent $e) {
        $app = $e->getApplication();
        $em = $app->getEventManager();

        ...

        $em->attach("register.post", function() {
            \Doctrine\Common\Util\Debug::dump(func_get_args());
            die("register.post");
        }, 1);

        $em->attach("registerViaProvider", function() {
            \Doctrine\Common\Util\Debug::dump(func_get_args());
            die("registerViaProvider");
        }, 1);

        $em->attach("registerViaProvider.post", function() {
            \Doctrine\Common\Util\Debug::dump(func_get_args());
            die("registerViaProvider.post");
        }, 1);
        ...
}

but none of the callbacks was called. Can you guide me? Thanks

ltouro commented 9 years ago

I think the problem is that you are attaching the event handlers on the application event manager, which is not the correct one.

You have two choices: Get an Static Event Manager and attach the events there, providing the identifier for the event manager you want (see here https://github.com/zendframework/zf2/blob/master/library/Zend/EventManager/StaticEventManager.php)

From Matthew's blog (https://mwop.net/blog/266-Using-the-ZF2-EventManager.html):

use Zend\EventManager\StaticEventManager;

$events = StaticEventManager::getInstance();
$events->attach('Example', 'do', function($e) {
    $event  = $e->getName();
    $target = get_class($e->getTarget()); // \"Example\"
    $params = $e->getParams();
    printf(
        'Handled event \"%s\" on target \"%s\", with parameters %s',
        $event,
        $target,
        json_encode($params)
    );
});

Or, you can grab the HydridAuth Auth Adapter through the Service Locator and attach the events directly on it. The downside of this approach is that you instantiate the HydridAuth on every request, which may not be optimal.

diogodomanski commented 9 years ago

You are right. I've changed the onBootstrap to the following, but it still didn't work.

public function onBootstrap(MvcEvent $e) {
        ...

        $event = \Zend\EventManager\StaticEventManager::getInstance();

        $event->attach("ScnSocialAuth\Authentication\Adapter\HybridAuth", "register.post", function($e) {
            die("register.post");
        });

        $event->attach("ScnSocialAuth\Authentication\Adapter\HybridAuth", "registerViaProvider", function($e) {
            die("registerViaProvider");
        });

        $event->attach("ScnSocialAuth\Authentication\Adapter\HybridAuth", "registerViaProvider.post", function($e) {
            die("registerViaProvider.post");
        });

        ...
}
ltouro commented 9 years ago

Before diving into why the Shared Event Manager is not working, could you please try to attach the events directly on ScnSocialAuth\Authentication\Adapter\HybridAuth and check if your event is being triggered?

I took a look at the HydridAuthFactory and there is no much complex logic there, reason I think the overhead for instantiating it should be negligible

diogodomanski commented 9 years ago

The code is now the following:

        $hybridAuth = $sm->get("ScnSocialAuth\Authentication\Adapter\HybridAuth");
        $event = $hybridAuth->getEventManager();

        $event->attach("register.post", function($e) {
            die("register.post");
        });

        $event->attach("registerViaProvider", function($e) {
            die("registerViaProvider");
        });

        $event->attach("registerViaProvider.post", function($e) {
            die("registerViaProvider.post");
        });

But it still didn't work (at least, none of "die()" instructions are being executed).

If I dump the returned value by $hybridAuth->getEventManager(), I get this:

object(stdClass)#345 (5) {
  ["__CLASS__"]=>
  string(30) "Zend\EventManager\EventManager"
  ["events"]=>
  array(0) {
  }
  ["eventClass"]=>
  string(23) "Zend\EventManager\Event"
  ["identifiers"]=>
  array(1) {
    [0]=>
    string(47) "ScnSocialAuth\Authentication\Adapter\HybridAuth"
  }
  ["sharedManager"]=>
  object(stdClass)#351 (2) {
    ["__CLASS__"]=>
    string(36) "Zend\EventManager\SharedEventManager"
    ["identifiers"]=>
    array(3) {
      ["Zend\Mvc\Application"]=>
      string(30) "Zend\EventManager\EventManager"
      ["doctrine"]=>
      string(30) "Zend\EventManager\EventManager"
      ["Zend\Stdlib\DispatchableInterface"]=>
      string(30) "Zend\EventManager\EventManager"
    }
  }
}
ltouro commented 9 years ago

Hmm.. Lets see. Are you creating a new local user or just linking an already created local user to a provider?

You could also try to attach to those two events: scnUpdateUser.pre' and scnUpdateUser.post'

diogodomanski commented 9 years ago

Itouro,

Hopeless, I just run php composer.phar update. For my surprise, hybridauth was updated from v2.2.2 to v2.4.1 and scn-social-auth from v1.16.0 to v1.17.0.

Without any modifications in the source code, I run the tests again and now the events are being listened. It is also working with the StaticEventManager way.

Where did you find these scnUpdateUser.pre and scnUpdateUser.post events? I searched for them in hybridauth and scn-social-auth libraries but I didn't find. Is there more events like these?

Thank you very much for your time and help

ltouro commented 9 years ago

diogodomanski,

great! You can find these events here: https://github.com/SocalNick/ScnSocialAuth/blob/master/src/ScnSocialAuth/Authentication/Adapter/HybridAuth.php#L581

I dont think there is any other events besides these four or at least, not in that event manager.

And don't worry, helping you is also a great opportunity for me to learn also

diogodomanski commented 9 years ago

After all, I still needed to modify the HybridAuth::authenticate() method to get everything done the way I need (that is, can set all custom attributes of User and UserProvider entities).

By using "registerViaProvider" and "scnUpdateUser.pre" I was able to set User's attributes. But the UserProvider was still being set partially (and "hardcoded") by authenticate() method.

What I had to do was to trigger a new event (register.pre) before the UserProvider entity is persisted into the DB. My solution was the following:

public function authenticate(AuthEvent $authEvent) {
        ...
        $localUserProvider = $this->getMapper()->findUserByProviderId($userProfile->identifier, $provider);
        if (false == $localUserProvider) {
            if (!$this->getOptions()->getEnableSocialRegistration()) {
                $authEvent->setCode(Result::FAILURE_IDENTITY_NOT_FOUND)
                        ->setMessages(array('A record with the supplied identity could not be found.'));
                $this->setSatisfied(false);

                return false;
            }
            $method = $provider . 'ToLocalUser';
            if (method_exists($this, $method)) {
                try {
                    $localUser = $this->$method($userProfile);
                } catch (Exception\RuntimeException $ex) {
                    $authEvent->setCode($ex->getCode())
                            ->setMessages(array($ex->getMessage()))
                            ->stopPropagation();
                    $this->setSatisfied(false);

                    return false;
                }
            } else {
                $localUser = $this->instantiateLocalUser();
                $localUser->setDisplayName($userProfile->displayName)
                        ->setPassword($provider);
                if (isset($userProfile->emailVerified) && !empty($userProfile->emailVerified)) {
                    $localUser->setEmail($userProfile->emailVerified);
                }
                $result = $this->insert($localUser, $provider, $userProfile);
            }
            $localUserProvider = clone($this->getMapper()->getEntityPrototype());
            $localUserProvider->setUserId($localUser->getId())
                    ->setProviderId($userProfile->identifier)
                    ->setProvider($provider);

            // Trigger register.pre event 
            $this->getEventManager()->trigger('register.pre', $this, array('user' => $localUser, 'userProvider' => $localUserProvider, 'userProfile' => $userProfile)); 

            $this->getMapper()->insert($localUserProvider);

            // Trigger register.post event
            $this->getEventManager()->trigger('register.post', $this, array('user' => $localUser, 'userProvider' => $localUserProvider, 'userProfile' => $userProfile));
        } else {
            $mapper = $this->getZfcUserMapper();
            $localUser = $mapper->findById($localUserProvider->getUserId());

            if ($localUser instanceof UserInterface) {
                $this->update($localUser, $provider, $userProfile);
            }
        }
        ...
}

It would be nice if this modification was added to HybridAuth::authenticate() officially

SocalNick commented 9 years ago

@ltouro - thanks for helping out on this!

@diogodomanski - couple ideas:

1) Do you really need to modify the user provider before it is saved? You could just modify the user provider model after it has been saved. 2) You could also create your own domain models to capture the metadata that you are adding to User / UserProvider. You could even have a User domain model that is made up of several underlying data models, a YourApp\Entity\UserAggregate that contains a ZfcUser\Entity\User, ScnSocialAuth\Entity\UserProvider, and a YourApp\Entity\UserMetadata.

If you are just adding a register.pre event, you could always submit that as a pull request.

diogodomanski commented 9 years ago

Hi @SocalNick,

My idea is to take advantage of the fact that, when the user authenticates using his/her social media account, the variable $userProfile holds many user's profile data. So I could use the already existing "insert" operation to populate the user_provider table record with the maximum possible data.

Thank for your help

ltouro commented 9 years ago

@SocalNick,

I like the idea of having an Aggregate entity. But how one could effectively compose this aggregate with another entitties? Is that like an CQRS Aggregate, which hold many different typed events or just an entity that holds the relations for the other objects as FKs? I'm trying to figure how this would work for a relational database. Thank you!

SocalNick commented 9 years ago

@ltouro - I'm not familiar with CQRS, but it may be similar to the concept of domain aggregates from domain driven design: http://martinfowler.com/bliki/DDD_Aggregate.html