auth0 / auth0-PHP

PHP SDK for Auth0 Authentication and Management APIs.
https://auth0.com/docs/libraries/auth0-php
MIT License
381 stars 212 forks source link

User is null not false #41

Closed adamdama closed 8 years ago

adamdama commented 8 years ago

I have found an issue using custom session storage. I am unable to successfully get a user from Auth0 because the exchangeCode() method is never being called.

I think I have narrowed this down to a combination of line 198 and line 276 in the Auth0.php file.

Line 198:

$this->user = $this->store->get("user");

When a key is returned from my session storage and there was nothing stored against that key, I get the value null. This is then assigned to $this->user.

On line 276 the following check is made:

if ($this->user === false) {
        $this->exchangeCode();
}

Seeing as the user variable is set to null and not false this method is never called and I get returned false for my user.

I have resolved this by changing the check to:

if (!$this->user) {
        $this->exchangeCode();
}

Which is allowing me to get the user.

glena commented 8 years ago

Can you share your session store? I think it is easier for you to change it to return false in this case since it can break BC for users using this package already.

adamdama commented 8 years ago

My session store class is a wrapper for the Phalcon Framework Session class. How would be best to share it?

I agree breaking BC would be bad. However the reason I flagged this as an issue is that if people are trying to use a generic session store with this package then flase is a valid value for something stored in the session and therefore a generic class will return null when a key is not present.

I suppose in my wrapper class I can change the return to be false if null seeing aas that wrapper is only being used with the Auth0 package.

glena commented 8 years ago

Yes, make sense. I am planing in releasing a new version (not sure if a new minor or mayor) with Guzzle 6.1 (latest). I think I can push this change too since (not a big deal if it is a mayor). How does it sounds?

adamdama commented 8 years ago

Sounds fair enough! :)

! thing I might add though, if you are thinking of using a new version have you considered unirest (http://unirest.io/) instead of Guzzle?

glena commented 8 years ago

never hear of it, will take a look but I doubt I will change it, I think I will just bump the guzzle version. Are you using it? why unirest over guzzle?

adamdama commented 8 years ago

I find Guzzle to be very long winded and not very nice to work with. Unirest is much simpler and cleaner.

I use Guzzle in the project I am working with Auth0 on, but I am using unirest in a more recently started project and am finding that much more enjoyable.

glena commented 8 years ago

ok, I will have it in mind. Based on how much it changed from guzzle 5 to 6, I will evaluate the change :)

glena commented 8 years ago

It will be ready in v2.0 I will release it today (uses guzzle 6 sorry)

github-actions[bot] commented 2 years ago

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.