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

User is null not false #41

Closed
adamdama opened this issue Nov 16, 2015 · 9 comments
Closed

User is null not false #41

adamdama opened this issue Nov 16, 2015 · 9 comments

Comments

@adamdama
Copy link

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
Copy link
Contributor

glena commented Nov 17, 2015

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
Copy link
Author

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
Copy link
Contributor

glena commented Nov 19, 2015

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
Copy link
Author

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
Copy link
Contributor

glena commented Nov 19, 2015

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
Copy link
Author

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
Copy link
Contributor

glena commented Nov 19, 2015

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

@glena
Copy link
Contributor

glena commented Nov 23, 2015

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

@glena glena closed this as completed Nov 23, 2015
@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