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

Return null instead of false in Auth0UserProvider. #128

Merged
merged 1 commit into from
Jun 17, 2019

Conversation

afreakk
Copy link
Contributor

@afreakk afreakk commented Jun 4, 2019

Changes

Return null instead of false from validateCredentials, retrieveByToken and retrieveByCredentials, in Auth0UserProvider.

Why

Because of this function in Illuminate\Auth\GuardHelpers:

    /**
     * Determine if the current user is authenticated.
     *
     * @return bool
     */
    public function check()
    {
        return ! is_null($this->user());
    }

This caused issues for us when we migrated to auth0, as some of our users utilized remember_token in previous login system.
Which caused Auth0UserProvider retrieveByToken to be called, and returned false user, which laravel counts as a "logged in user".
Which then caused exception when code asked for $user->stuff on a boolean.

In our case, only retrieveByToken was called, but it makes sense to me that retrieveByCredentials and validateCredentials also should return null instead of false.

Illuminate\Auth\GuardHelpers check/guest function checks if user `is_null`.
To determine if there is a logged in user.
@afreakk afreakk requested a review from a team June 4, 2019 14:27
@joshcanhelp joshcanhelp requested review from joshcanhelp and removed request for a team June 4, 2019 14:56
@joshcanhelp
Copy link
Contributor

Thank you @afreakk! We'll review this as soon as we can. Appreciate the contribution!

Copy link
Contributor

@joshcanhelp joshcanhelp left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you for this 🙇

@joshcanhelp joshcanhelp added this to the v5-Next milestone Jun 17, 2019
@joshcanhelp joshcanhelp merged commit f9cb30a into auth0:master Jun 17, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants