When authenticating with HF OAuth, stay in same tab #7887
Conversation
🪼 branch checks and previews
Install Gradio from this PR pip install https://gradio-builds.s3.amazonaws.com/7cdb53fee862c584ec26b7d6834fdce0d810a196/gradio-4.26.0-py3-none-any.whlInstall Gradio Python Client from this PR pip install "gradio-client @ git+https://github.com/gradio-app/gradio@7cdb53fee862c584ec26b7d6834fdce0d810a196#subdirectory=client/python" |
🦄 change detectedThis Pull Request includes changes to the following packages.
With the following changelog entry.
Maintainers or the PR author can modify the PR title to modify this entry.
|
|
This LGTM. But just a heads up, one issue I noticed in the test space was that I couldn't scroll down to give permissions to specific organizations and then click accept: 
I think this is due to way that the iframe is set up. Until we fix it, we may want to keep opening in a new tab. |
|
@abidlabs @freddyaboulton I also noticed this issue and reported it on slack (internal). |
There was a problem hiding this comment.
Everything looks good left a comment about the iframe options.
We also need to 'reset' the scrolling when we go back to gradio. This should be straightforward because when we go back to gradio it is a fresh page load, so we can do it somewhere like this:
https://github.com/gradio-app/gradio/blob/main/js/app/index.html#L60-L65
The code to disable scrolling is like so:
window.parent?.postMessage({type: "SET_SCROLLING", enabled: false})
gradio/components/login_button.py
Outdated
| uri = buttonValue === BUTTON_DEFAULT_VALUE ? '/login/huggingface' : '/logout'; | ||
| window.location.assign(uri + window.location.search); |
There was a problem hiding this comment.
This is where we will need to set the scrolling of the iframe as mention in @coyotte508's PR.
We should be able to add this as the first line of this function:
window.parent?.postMessage({type: "SET_SCROLLING", enabled: true})
|
I think it is still not scrolling. lmk if/ when you want me to take a look @Wauplin. |
This comment was marked as resolved.
This comment was marked as resolved.
|
@pete @coyotte508 Thanks for your help! Scrolling now works on this demo Space: https://huggingface.co/spaces/templates/gradio-oauth. I still had to add a 500ms timeout on the redirection to make sure the message is posted (see here). I don't think it downgrades UX but it is not 100% safe on slow connections. Should be ok though? 🙄 |
|
Posting the message shouldn't be related to the speed of the internet connection, it's child iframe communicating with parent iframe, so I think slow connections will be ok |
|
I'm surprised that such a long timeout is required. I don't think it will cause issues for slow connections. Technically a low machine might have issues but I don't think it is a major concern. A wait of some amount of time is expected though as the message is sent and forgotten about, it doesn't wait for the parent to process it. |
There was a problem hiding this comment.
Looks great to me, and I have tested the demo space and it seems to work great, even on tiny screens! Thanks for battling with this one @Wauplin! And thanks for your help on the hub side @coyotte508!
TBH I only tried with this timeout. If you think it's too long I can update. It's just that it's annoying to test different values (need to push commit, wait for gradio build, then update the Space, wait for rebuild, then test). So if 500ms is fine, I'll keep it 😄 |
You are welcome, but I cannot take credit. |
it's fine, we'll keep an eye on it. |
pngwn
changed the title
|
Great, then feel free to merge whenever it's ready/approved :) |
* OAuth: stay in same tab * add changeset * add changeset * typo * scroll * add changeset * lint * test * test with timeout * log * new test * fix origin in postMessage * with timeout * shoud be fine * lint * lint * remove logs * add changeset --------- Co-authored-by: gradio-pr-bot <gradio-pr-bot@users.noreply.github.com>
Related to this slack thread (internal), https://github.com/huggingface/moon-landing/issues/9054 (internal) and https://github.com/huggingface/moon-landing/pull/9164 (internal).
Thanks to @coyotte508's work in moon-landing , we do not need to open a new tab to login with OAuth in a Gradio Space anymore! This PR simply updates the JS code in
LoginButtonto stay in the same tab during the process. I've also updated the cookie key to invalidate all cookies (necessary). Not a big problem anyway since it will only impact Spaces that will upgrade their Gradio version.Test Space: https://huggingface.co/spaces/templates/gradio-oauth. To test the full OAuth workflow, don't forget to revoke the permission first in https://huggingface.co/settings/connected-applications.