Skip to content

#494 Implements Serializable interface in the User entity #495

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 25 additions & 1 deletion src/AppBundle/Entity/User.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
* @author Ryan Weaver <[email protected]>
* @author Javier Eguiluz <[email protected]>
*/
class User implements UserInterface
class User implements UserInterface, \Serializable
{
/**
* @var int
Expand Down Expand Up @@ -163,4 +163,28 @@ public function eraseCredentials()
// if you had a plainPassword property, you'd nullify it here
// $this->plainPassword = null;
}

/** @see \Serializable::serialize() */
public function serialize()
{
return serialize([
$this->id,
$this->username,
$this->password,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is adding the password here a recommended practice (from the security point of view)?

Copy link
Author

@apetitpa apetitpa Mar 10, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Author

@apetitpa apetitpa Mar 10, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quoted from http://symfony.com/doc/current/security/entity_provider.html#understanding-serialize-and-how-a-user-is-saved-in-the-session

First, the Serializable interface and its serialize() and unserialize() methods have been added to allow the User class to be serialized to the session. This may or may not be needed depending on your setup, but it's probably a good idea. In theory, only the id needs to be serialized, because the refreshUser() method refreshes the user on each request by using the id (as explained above). This gives us a "fresh" User object.

But Symfony also uses the username, salt, and password to verify that the User has not changed between requests (it also calls your AdvancedUserInterface methods if you implement it). Failing to serialize these may cause you to be logged out on each request.

Copy link
Member

@javiereguiluz javiereguiluz Mar 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This explanation is not very good 😕 Saying "maybe you need this ... maybe you don't" without explaining anything else doesn't look good. And then, saying "you can serialize just "id" if you want ... but maybe it doesn't work because Symfony wants more" is also confusing.

As usual, this is caused by the massive internal complexity of the security component ... so I don't know what can we do about this.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have to agree with you, it's quite unclear. I don't know what to do either maybe someone could enlight us on this particular topic ?

// see section on salt below
// $this->salt,
]);
}

/** @see \Serializable::unserialize() */
public function unserialize($serialized)
{
list(
$this->id,
$this->username,
$this->password,
// see section on salt below
// $this->salt
) = unserialize($serialized);
}
}