Test passing identity instance to persistIdentity() and setIdentity() #449
Conversation
|
|
||
| $service = new AuthenticationService(); | ||
|
|
||
| $result = $service->persistIdentity($request, $response, $identity); |
There was a problem hiding this comment.
Passing an Identity instance here would mean SessionAuthenticate would store that instance in the session. But when one uses form login the instance stored in session is an Entity (or array), not an IdentityInterface instance.
IMO it's better to avoid such discrepancy. The authenticators should always deal with the "raw" identity object and only AuthenticationService should deal with IdentityInterface instances.
There was a problem hiding this comment.
That said a clean way to fix the regression would be make buildIdentity() simply return the argument if it's already an IdentityInterface instance. That would also avoid the object type checks at multiple places.
There was a problem hiding this comment.
The authenticators should always deal with the "raw" identity object and decorating it with IdentityInterface should be left to AuthenticationService.
The thing is that sometimes the "raw" identity object is an identity instance itself. If your User entity already implements IdentityInterface that is. And this test covers that case.
There was a problem hiding this comment.
If your User entity already implements IdentityInterface that is.
Ah, I had overlooked that case. Could you please add a comment stating this info in the relevant test case.
This adds missing tests for calling persistIdentity() and setIdentity() with identity instance.
Refs #448
PS. I've also removed deprecated method calls form tests.