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

Ensure Auth0 profile is availabe in all cases #374

Merged
merged 1 commit into from Jul 23, 2018

Conversation

@sholladay
Copy link
Contributor

sholladay commented Jul 16, 2018

This is a bugfix for Auth0’s “Last time you logged in with” flow. Previously, if a user clicked on their existing session instead of the "Not your account?" link, then request.auth.credentials would not contain the user's profile information. That was very confusing because bell normally provides the full profile. The bug was a hard one to track down, since it only happens to users who visit the login page while they are already logged in.

This PR fixes the problem by explicitly requesting all of the scopes necessary for bell to function as intended. Auth0's APIs behave themselves when you do this. 😄

Auth0’s “Last time you logged in with” UI for an existing session was not working, as it would fail to return any profile information. This fixes that by including the necessary OIDC scopes.
@AdriVanHoudt

This comment has been minimized.

Copy link
Contributor

AdriVanHoudt commented Jul 18, 2018

So normally you don't need these scopes unless you come on the login page while logged in and you try to log in with the current session?
Seems like a bug on Auth0's end?

I don't know Auth0's api but if you say this is a no-op for other situations then this one I will gladly merge this :D

@AdriVanHoudt AdriVanHoudt self-assigned this Jul 18, 2018
@AdriVanHoudt AdriVanHoudt added this to the 10.0.0 milestone Jul 18, 2018
@sholladay

This comment has been minimized.

Copy link
Contributor Author

sholladay commented Jul 18, 2018

I have a slight suspicion that it has to do with OIDC conformance, which Auth0 has been slowly moving towards over the past year, but I can't easily prove that. I just remember it working prior to some of their OIDC changes. Also, their scopes documentation seems to indicate these scopes are required to return the profile and email. But for some reason, in practice, it isn't necessary when creating a new session. Maybe someone made a mistake while trying to maintain backwards compatibility.

I will also note that the Okta provider uses these scopes, which seems relevant, since their product has a similar architecture.

It definitely doesn't adversely affect anything. Did lots of testing with and without it. Should be semver patch.

@AdriVanHoudt AdriVanHoudt merged commit 16a4911 into hapijs:master Jul 23, 2018
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@AdriVanHoudt

This comment has been minimized.

Copy link
Contributor

AdriVanHoudt commented Jul 23, 2018

Sounds good 👌
I asked in Slack for some Auth0 peeps to look at this but I think you are right. I'm waiting on some other PR's so I can do a release with some changes combined.
I expect eta to be somewhere this week.

@hueniverse hueniverse modified the milestones: 10.0.0, 9.3.2 Nov 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.