search

ZF-Commons / ZfcUser

A generic user registration and authentication module for ZF2. Supports Zend\Db and Doctrine2. (Formerly EdpUser)
BSD 3-Clause "New" or "Revised" License
495 stars 343 forks source link

Auth\Event really need user_id as Identity after success? #412

Open ClemensSahs opened 10 years ago

ClemensSahs commented 10 years ago

In #327 we have this topic...

ZfcUser/Authentication/Adapter/Db.php#L108-L116

If there is a reason to holt the user_id as identity then we can add a userEntity additional? Or is there something else I don't see.

I write two patches $event->setIdentity($userObject) and $event->setEntity($userObject)

ping @EvanDotPro @Danielss89

adamlundrigan commented 9 years ago

Do we need to provide the resulting user entity at all here? From the listener you could pull an instance of the AuthenticationService and extract the user details from there.

ClemensSahs commented 9 years ago

My only point is that looks like a wrong behavior? I my mind AdapterChainEvent::getIdentity and \Zend\Authentication\AuthenticationService::getIdentity must follow the same behavior.

The have the same name so the must do the same...

And on the other Hand, why we need to hold the User-ID in the event, without other data? The only UseCase I see for a listener is to work with the User Date.

So you call pull AuthenticationService and call a result for a ID? Result of this 2 Database request... first for the login and second on the listener. If you have more that one listener... this example is a n+1

Currently I don't have this problem by my self.

adamlundrigan commented 9 years ago

AuthenticationService::getIdentity doesn't hit the database, it just returns the user entity that it was given during the bootstrap process and returns the same instance each time you call the method.

ClemensSahs commented 9 years ago

@adamlundrigan Yes sorry the n+1 request is not exists... but this was only a side point on this discussion...

API - inconsistency and the 2 request are already there...