[5.5] Fixed null remember token error on EloquentUserProvider #21328
Conversation
…or its value before comparing it to
|
@Naxon I am just referencing another PR, with no intention to really comment on this PR. In other hand, It would be great if you could come up with failing test. I think issue with that file is spread across 3 releases already (just unfortunate file!). |
|
Are we actually even testing what is happening here? Has anyone actually run this code? /cc @mcordingley ... Or are we just throwing crap against the wall and hoping it sticks? |
|
@taylorotwell I ran the one I committed to check if it works, unfortunately no unit test. |
|
Sorry. 😢 I ran the tests and everything passed just fine. I should have double-checked them to ensure that null would be handled, too. |
| @@ -64,7 +64,9 @@ public function retrieveByToken($identifier, $token) | |||
|
|
|||
| $model = $model->where($model->getAuthIdentifierName(), $identifier)->first(); | |||
|
|
|||
| return $model && hash_equals($model->getRememberToken(), $token) ? $model : null; | |||
| $rememberToken = $model->getRememberToken(); | |||
There was a problem hiding this comment.
@Naxon What if $model == null here? There is a null-check for it two lines below, therefore I'm asking...
There was a problem hiding this comment.
This review makes sense. Why use $model to get rememberToken then right after it, check if the $model is null?
I added a check that
$model->getRememberToken()isn't null before comparing it to$tokenusing hash_equals so it doesn't throw an error.