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
v4.1.5: oauth redirect loop #4800
Comments
Thank you for opening your first issue in this project! Engagement like this is essential for open source projects! 🤗 |
Update: I added some print statements in here: jupyterhub/jupyterhub/_xsrf_utils.py Lines 203 to 204 in b405361
and found that this function is now called in 4.1.5 on the GET requests for the JS and CSS static files, when it wasn't in 4.1.4 (Note to self: only difference between the two versions is this diff f395acd from #4771) |
Further update: When inspecting the requests that were getting redirected and ultimately failing, they included the XSRF token in the If I make this edit then everything works as expected: Hidden
def check_xsrf_cookie(handler):
"""Check that xsrf cookie matches xsrf token in request"""
# overrides tornado's implementation
# because we changed what a correct value should be in xsrf_token
if not _needs_check_xsrf(handler):
# don't require XSRF for regular page views
return
token = (
handler.get_argument("_xsrf", None)
or handler.request.headers.get("X-Xsrftoken")
or handler.request.headers.get("X-Csrftoken")
+ or handler.get_cookie("_xsrf")
) However I suspect this is defeating the point of |
I am having a very similar issue where check_xsrf_cookie returns a 403 during login. Our cookie header contains the correct _xsrf token but the X-Xsrftoken/X-Csrftoken header is not set so the check_xsrf_cookie method returns the 403 '_xsrf' argument missing from POST. |
Yes, that is precisely defeating the purpose. The goal of the XSRF token is establishing that the sender has access to the xsrf cookie. The XSRF token must always be sent in two places:
The XSRF check passes if these two match. The presence of the
This generally shouldn't happen on static files, and doesn't in my tests. I will try to investigate why yours are getting caught up in this. @ibh1127 it sounds like your login form doesn't set the xsrf token input. The explanation is above, that presence in |
@MetRonnie can you explain why you have added |
Many thanks for looking into this.
This seems to be done by Vite when building the web app. Removing them from the built HTML file does not solve the problem unfortunately. However, a colleague suggested we could remove Tornado's and this does allow the JS/CSS to be fetched without a problem. But unfortunately we still get the redirect loop problem for a XHR GET to our Log
Edit: just noticed on page refresh after this the app loads successfully. But after closing the browser and re-opening, I get the problem again. |
One thing that would clean up and simplify things is if we only did the login redirect on The underlying cause is that 4.1 applies consistent XSRF checks to authenticated GET requests, which is required to protect user servers from each other, whereas 4.0 did not strictly protect GET requests, only POST and others. In general, these are the changes required to work in this situation:
This makes sense e.g. for shared static assets, but probably not I would recommend going with token approach, if possible, but copying the xsrf header code you already have to the requests missing them is probably the smallest change to get things working in the short term. I've made the following PRs:
which together made cylc work for me in 4.1.5 and should be fully backward compatible. |
Much appreciated! And thank you for the explanation! It now works after logging in for the first time. However, if I close the browser, then re-open the browser, I find that I get the oauth redirect loop again. I think this is because the XSRF cookie has session lifetime, and when re-opening the browser it is no-longer set in time for the userprofile GET request. I think this is why the redirect loop is happening: After the first redirect to the oauth URLs, the XSRF cookie is set, but when it redirects back to the userprofile, the GET request still does not contain the XSRF token header (because it is still the original request? (It is not re-executing our This can be worked around by retrying the userprofile GET after the redirect loop fails. But it sounds like this could be solved by using the JHub API token that you've mentioned, is there somewhere in the docs you can point me to for this? Or is there another way to solve this by somehow going through the oauth before the userprofile GET request? |
Ah, the missing xsrf token is probably because the request to serve That way, when GET |
That's almost got it, however I think when the
Edit: see cylc/cylc-uiserver#592 (comment):
|
Awesome, thanks for the fixes! |
Bug description
Hello, here at Cylc we're having an issue with Jupyterhub 4.1.5. When launching the hub and trying to access our web app, I get 403 errors for the JS and CSS files linked in the
index.html
of the app. This seems related to a series of bugfix releases from 4.1.0-4.1.5 regarding XSRF cookies - @minrk do you have any ideas as to what's going on from a quick look at this snippet?Sanitised log snippet:
I also get
ERR_TOO_MANY_REDIRECTS
in the browser.How to reproduce
pip install cylc-uiserver[hub]==1.4.4 jupyterhub==4.1.5
andconfigurable-http-proxy
via conda or npmcylc hub
, or equivalently:CYLC_HUB_VERSION=1.4.4 jupyterhub --config ~/.conda/envs/envname/lib/python3.9/site-packages/cylc/uiserver/jupyterhub_config.py
Expected behaviour
In Jupyterhub 4.1.4, the app loads after logging in.
Actual behaviour
403 error as described above.
Your personal set up
Full environment
Configuration
This:
https://github.com/cylc/cylc-uiserver/blob/1.4.4/cylc/uiserver/jupyterhub_config.py
which is used to load this:
Logs
The text was updated successfully, but these errors were encountered: