Skip to content
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

[5.0] Guard method onceUsingId not returning false but throwing exception #8836

Merged
merged 3 commits into from
May 21, 2015
Merged

Conversation

HolgerW1
Copy link
Contributor

Fixed Guard implementation not accepting a null value for the setUser call leading to an exception in onceUsingId instead of returning false if the provider returns null for the specified user ID.

… call leading to an exception in onceUsingId instead of returning false if the provider for the specified ID returns null
@@ -710,7 +710,7 @@ public function getUser()
* @param \Illuminate\Contracts\Auth\Authenticatable $user
* @return void
*/
public function setUser(UserContract $user)
public function setUser(UserContract $user = null)
Copy link
Member

Choose a reason for hiding this comment

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

You need to update the phpdoc.

@GrahamCampbell GrahamCampbell changed the title Guard method onceUsingId not returning false but throwing exception [5.0] Guard method onceUsingId not returning false but throwing exception May 21, 2015
@HolgerW1
Copy link
Contributor Author

I've updated the solution a little to not touch the setUser method but simply checking for a null value inside the onceUsingId. Setting the user property to null wouldn't make much sense imho unless it would behave like a logout.

…ole method to fail if the provider doesn't return an instance implementing Illuminate\Contracts\Auth\Authenticatable
taylorotwell added a commit that referenced this pull request May 21, 2015
[5.0] Guard method onceUsingId not returning false but throwing exception
@taylorotwell taylorotwell merged commit fc7cc85 into laravel:5.0 May 21, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants