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

getUser() returning null #524

Closed
bkilshaw opened this issue Jul 21, 2021 · 3 comments
Closed

getUser() returning null #524

bkilshaw opened this issue Jul 21, 2021 · 3 comments
Assignees

Comments

@bkilshaw
Copy link

So our customers experiencing a network outage and we ran into an edge case where getUser() returns null and doesn't throw an error.

From what I can tell, it's due to this line here:

    public function exchange()
    {
        $code = $this->getAuthorizationCode();
        if (! $code) {
            return false;
        }

       [...]
    }

public function exchange()

If Auth0 returns an error there is no authorization code. Instead of throwing an error it just returns false, causing getUser() to return the empty $user.

Would it make sense to throw an error here? It's cleaner to catch the error if there's no authorization code than it is to check what getUser() returned before using it.

Right now we have to do this:

    public function callback(): RedirectResponse
    {
        try {
            $auth0_profile = $this->auth0->getUser();
        } catch (CoreException $e) {
            Log::critical('Auth0 CoreException', ['error' => (array)$e, 'request' => request()]);
            return $this->logout();
        } catch (ApiException $e) {
            Log::critical('Auth0 ApiException', ['error' => (array)$e, 'request' => request()]);
            return $this->logout();
        }

        if($auth0_profile && is_array($auth0_profile)) {
            $user = $this->getLocalUser($auth0_profile);
        } else {
            Log::critical('Auth0Profile Empty', ['request' => request()]);
            return $this->logout();
        }

When it could be this

    public function callback(): RedirectResponse
    {
        try {
            $auth0_profile = $this->auth0->getUser();
        } catch (CoreException $e) {
            Log::critical('Auth0 CoreException', ['error' => (array)$e, 'request' => request()]);
            return $this->logout();
        } catch (ApiException $e) {
            Log::critical('Auth0 ApiException', ['error' => (array)$e, 'request' => request()]);
            return $this->logout();
        } catch (MissingProfile $e) {
            Log::critical('Auth0 MissingProfile', ['error' => (array)$e, 'request' => request()]);
            return $this->logout();
        }
@evansims
Copy link
Member

Hey, @bkilshaw 👋 Thanks for reporting this. Let me investigate this one and get back to you.

@evansims
Copy link
Member

evansims commented Sep 9, 2021

Hey @bkilshaw 👋

Sorry, it took me a bit to review this; busy times! I definitely see the point you're making, and I think it does represent a bit of a pitfall in the v7 branch that could be handled better. Unfortunately, there really isn't a way of improving this in that branch without introducing breaking changes, so the workaround you presented is probably the best case for handling it for now.

On the upside, the v8 branch is moving into stable status in the next week or so, and it uses a cleaner approach to this. In v8, getUser will return null if a session hasn't been established yet, but it won't try to establish one itself. Developers can opt in to establishing a session by calling exchange themselves.

As an example, roughly based off your code:

// Retrieve local session details, if available.
$session = $auth0->getCredentials();

// Is a session available?
if ($session === null) {
    // It is not; is the active request the callback of an authorization flow?
    if ($auth0->getExchangeParameters() !== null) {
        // It was, request our token and establish a session.
        try {
            $auth0->exchange();
        } catch (\Auth0\SDK\Exception\StateException $exception) {
            Log::critical('Auth0 StateException', ['error' => (array)$e, 'request' => request()]);
            die("Auth0 StateException was thrown; " . $e->getMessage());
        } catch (\Auth0\SDK\Exception\NetworkException $exception) {
            Log::critical('Auth0 NetworkException ', ['error' => (array)$e, 'request' => request()]);
            die("Auth0 NetworkException was thrown; " . $e->getMessage());
        }
    } else {
        // It is not a callback, so let's redirect to the authorization page.
        header("Location: " . $auth->login());
        exit;
    }
}

// If we've reached this point, a session is available.
echo "Authenticated.";

// Print the user data sturcture. (Alternatively, just use $session->user)
print_r($auth0->getUser());

Again, that's the upcoming v8, though, but just thought I'd express the differences in the API. The workaround you've shown in your example code is probably the best route you can go with for v7 for that scenario.

@github-actions
Copy link
Contributor

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 23, 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

No branches or pull requests

2 participants