-
-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
Token was deauthenticated Error Symfony 4 #9914
Conversation
We should no long used Serializable interface in Symfony 4 it's caused error when you log with a user 'Token was deauthenticated after trying to refresh it' we should use EquatableInterface and isEqualTo method
Implementing the |
We also have: |
I am not sure that I understand what you are trying to say. Can you elaborate a bit on why you think we should drop the |
If you're curious about the importance of the serialize() method inside the User class or how the User object is serialized or deserialized, then this section is for you. If not, feel free to skip this. Once the user is logged in, the entire User object is serialized into the session. On the next request, the User object is deserialized. Then, the value of the id property is used to re-query for a fresh User object from the database. Finally, the fresh User object is compared to the deserialized User object to make sure that they represent the same user. For example, if the username on the 2 User objects doesn't match for some reason, then the user will be logged out for security reasons. Even though this all happens automatically, there are a few important side-effects. 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. If your user implements the EquatableInterface, then instead of these properties being checked, your isEqualTo() method is called, and you can check whatever properties you want. Unless you understand this, you probably won't need to implement this interface or worry about it. So the problem if you use Serializable interface, for that it works it should contains $id, $email or $username and $password
But in symfony 4 Serializable interface isn't really useful, because you can implements EquatableInterface and put isEqualTo method that make the job and you can pass it what do you want compare (you can put only password if you want or add attributes ...) and it compare at each request if user have changed or not, for exemple:
To resume we can use Serializable Interface in User class, but is not usefull in this context of a User class because we can implements EquatableInterface (and it's useless to keep the both interfaces) |
Well, my point is that when not implementing |
@xabbuh Ok but we should add Equatable interface exemple maybe ? |
Yes, adding |
@xabbuh Ok, I added it :) |
security/entity_provider.rst
Outdated
} | ||
|
||
/** | ||
* if you want to keep the control on what attributes are are compared at each request to know if user have changed, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Double are 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@maxhelias thanks it fix
security/entity_provider.rst
Outdated
@@ -42,12 +42,14 @@ with the following fields: ``id``, ``username``, ``password``, | |||
|
|||
use Doctrine\ORM\Mapping as ORM; | |||
use Symfony\Component\Security\Core\User\UserInterface; | |||
use Serializable; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be reverted. We follow the Symfony code style which does not add use
statements for classes from the global namespace.
security/entity_provider.rst
Outdated
/** @see \Serializable::serialize() */ | ||
public function serialize() | ||
{ | ||
return serialize(array( | ||
$this->id, | ||
$this->username, | ||
$this->username, // you should use $this->email if you don't use username but email to log user |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO we should remove this comment. Similar things apply to all other attributes as well if you change their names.
security/entity_provider.rst
Outdated
return false; | ||
} | ||
|
||
if ($this->username !== $user->getUsername()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not look realistic to me. Users are refreshed by their username. So this should always be the same.
As explained in #10151, we're refactoring Security docs completely. So let's close this pull request and once the refactor is finished, let's rethink about this. Thanks! |
We should no long used Serializable interface without username/email, password, id attributes it's caused error when you log with a user 'Token was deauthenticated after trying to refresh it'
Here I propose to add EquatableInterface and isEqualTo method to choose what attributes do you want compare for each request.