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
497 stars 343 forks source link

Messages get overridden #357

Open Thinkscape opened 10 years ago

Thinkscape commented 10 years ago

Because of https://github.com/ZF-Commons/ZfcUser/blob/master/src/ZfcUser/Authentication/Adapter/Db.php#L78 the Db adapter will override any previously set messages (i.e. when there are other adapters in the chain before it).

We should think about AdapterChainEvent::addMessage() and s/setMessages/addMessage/g.

adamlundrigan commented 9 years ago

Is there a use case for why it shouldn't overwrite?

My concern isthat if you have two adapters, A and B, and A doesn't find a matching user but B does you'll end up with a messages array that looks like this:

// AdapterChainEvent->getMessages()
array(
  'A record with the supplied identity could not be found.',
  'Authentication successful.',
);

Or, if they both fail, this:

// AdapterChainEvent->getMessages()
array(
  'A record with the supplied identity could not be found.',
  'A record with the supplied identity could not be found.',
);
Thinkscape commented 9 years ago

i.e.

  1. You have plaintext and facebook adapters.
  2. You log in via facebook.
  3. Because login/password are empty, plaintext does nothing and returns early.
  4. Facebook got creds, but they are invalid - setting "you have not authorized login via facebook" message.