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

Google auth redirect loop with hapi v18 #394

Closed
JoshGlazebrook opened this issue Jan 28, 2019 · 7 comments · Fixed by #397
Closed

Google auth redirect loop with hapi v18 #394

JoshGlazebrook opened this issue Jan 28, 2019 · 7 comments · Fixed by #397
Assignees
Labels
bug Bug or defect
Milestone

Comments

@JoshGlazebrook
Copy link

JoshGlazebrook commented Jan 28, 2019

Upgrading from v17.8.1 -> v18.0.0 seems to cause a redirect loop. Following the same exact configuration pattern as https://github.com/hapijs/bell/blob/master/examples/google.js

My router handler is never actually called, instead the route is redirected to the google SSO login page, and when the correct google account is selected, it redirects back, but the handler is never called. Instead it redirects back to Google. Downgrading to 17.8.1 it works fine.

@chrisshiplet
Copy link

chrisshiplet commented Feb 5, 2019

Also seeing this with the FB provider. From what I can tell when the OAuth client code gets to here: https://github.com/hapijs/bell/blob/master/lib/oauth.js#L176

request.query is set to {} even when the raw URL in the request object shows ?code=whatever in the URL string. I don't have more time today to dig into this, will update if I'm able to confirm a fix

@framerate
Copy link

framerate commented Feb 16, 2019

I'm seeing this behavior too on my first time using Bell. Is it looking like a module error or user error?

EDIT: I'm using Discord

@AdriVanHoudt AdriVanHoudt added the bug Bug or defect label Feb 25, 2019
@AdriVanHoudt AdriVanHoudt self-assigned this Feb 25, 2019
@AdriVanHoudt
Copy link
Contributor

I can confirm this is happening with hapi@18 🙊
It seems that the cookie (in my case bell-google) is not there when the flow comes back from the google oauth flow to bell.
Running the tests under hapi@18 breaks a lot of them, most of them due to the redirect not adding a port anymore. I have not figured out why but I will look at this tonight.

@pdxmholmes
Copy link

I'm seeing the same issue with the Twitch provider. I've rolled back to Hapi@17 for now and the problem goes away.

@AdriVanHoudt
Copy link
Contributor

I think this is an issue with just the oAuth flow code and hapi@18.
I'm still looking into it but if you want to help -> npm i hapi@18 && npm t to see what is currently failing for hapi@18 :D

@pdxmholmes
Copy link

Some of the errors appear to be because of a change to server.inject in hapi@18. Specifically it seems to eat the port now if it's port 80 (and I assume 443 though I haven't tested it). That doesn't explain the loop being caused in real code, but does cover some of the broken tests.

AdriVanHoudt pushed a commit that referenced this issue Feb 27, 2019
This PR includes two fixes related to [hapi 18](hapijs/hapi#3871):

1. Many tests broke because hapi now strips the default port from `request.info.host` (a side effect of using the WHATWG URL API) – causing lots of assertions to fail. I decided to use a non-default port in the tests instead of removing the default port from the assertions. This verifies that port info is still propagated correctly.
2. `request.url.query` is no longer available. I’ve changed it to `request.query` which works for hapi 18 and older versions. (I’ve tested it with hapi 17 and 18). This fixes #394.
@AdriVanHoudt AdriVanHoudt added this to the 10.0.0 milestone Mar 5, 2019
@lock
Copy link

lock bot commented Jan 9, 2020

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Bug or defect
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants