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

Add OAuth Guest for selected endpoints #5743

Merged
merged 4 commits into from
Oct 30, 2023
Merged

Conversation

tupui
Copy link
Contributor

@tupui tupui commented Oct 25, 2023

This PR adds the possibility to set PANEL_OAUTH_GUEST_ENDPOINTS (or --oauth-guest-endpoints) to register endpoints where we skip the OAuth workflow (if not authenticated) and return a guest user.

Once we have a guest, we can use authorize_callback and for instance detect that we have a guest and show the oauth-template. I am using this to present a landing page before OAuth.

See some background discussion in https://discord.com/channels/1075331058024861767/1166025760138600478

panel/auth.py Outdated Show resolved Hide resolved
@tupui
Copy link
Contributor Author

tupui commented Oct 26, 2023

Not sure what is happening but now I get this error locally:

2023-10-26 13:57:34,675 Uncaught exception GET /app/ws (::1)
HTTPServerRequest(protocol='http', host='localhost:5006', method='GET', uri='/app/ws', version='HTTP/1.1', remote_ip='::1')
Traceback (most recent call last):
  File "/opt/homebrew/Caskroom/mambaforge/base/envs/panel/lib/python3.10/site-packages/tornado/websocket.py", line 942, in _accept_connection
    open_result = handler.open(*handler.open_args, **handler.open_kwargs)
  File "/opt/homebrew/Caskroom/mambaforge/base/envs/panel/lib/python3.10/site-packages/tornado/web.py", line 3205, in wrapper
    self.redirect(url)
  File "/opt/homebrew/Caskroom/mambaforge/base/envs/panel/lib/python3.10/site-packages/tornado/websocket.py", line 614, in _raise_not_supported_for_websockets
    raise RuntimeError("Method not supported for Web Sockets")
RuntimeError: Method not supported for Web Sockets

@philippjfr
Copy link
Member

I'll try to play around.

@philippjfr
Copy link
Member

@tupui I made a bunch of changes and added some docs. If you get a chance please review.

@codecov
Copy link

codecov bot commented Oct 27, 2023

Codecov Report

Merging #5743 (0c6cb68) into main (49e1cd6) will decrease coverage by 8.62%.
Report is 8 commits behind head on main.
The diff coverage is 16.98%.

@@            Coverage Diff             @@
##             main    #5743      +/-   ##
==========================================
- Coverage   82.23%   73.62%   -8.62%     
==========================================
  Files         289      289              
  Lines       42111    42160      +49     
==========================================
- Hits        34630    31040    -3590     
- Misses       7481    11120    +3639     
Flag Coverage Δ
ui-tests ?
unitexamples-tests 73.62% <16.98%> (+0.17%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
panel/io/server.py 66.32% <50.00%> (-4.75%) ⬇️
panel/command/serve.py 14.90% <0.00%> (-0.23%) ⬇️
panel/config.py 66.99% <46.15%> (-0.54%) ⬇️
panel/io/state.py 69.74% <0.00%> (-1.05%) ⬇️
panel/auth.py 28.08% <4.16%> (-0.58%) ⬇️

... and 77 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor Author

@tupui tupui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on that @philippjfr 🚀 I see I had a few missing bits 😅

doc/how_to/authentication/guest_users.md Show resolved Hide resolved
panel/io/state.py Outdated Show resolved Hide resolved

if pn.state.user == 'guest':
button = pn.widgets.Button(name='Login').servable(target='header')
button.js_on_click(code='window.location.href="/login"')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure I understand how that works. What if I want to hide everything, shall I just put the template.servable() in the else (my attempt did not work. Also during my tries I got this error: 2023-10-27 15:24:49,470 DocumentLifeCycleHandler on_session_destroyed callback <bound method OAuthProvider._remove_user of <panel.auth.OAuthProvider object at 0x168aaf220>> failed with following error: 'NoneType' object has no attribute 'decode' )? Or do I need to check the user on every pane?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That should work fine and does when I try:

import panel as pn

pn.extension(template='material')

pn.state.template.title = 'Optional Auth'


pn.Column(f'# Hello {pn.state.user}!', pn.state.user_info).servable()

if pn.state.user == 'guest':
    button = pn.widgets.Button(name='Login').servable(target='header')
    button.js_on_click(code='window.location.href="/login"')
else:
    pn.Column('My super secret content that I can only see if I am logged in.').servable()
Screenshot 2023-10-30 at 12 05 17

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be doing something wrong on my side, but I still cannot make it work.

print(pn.state.user)
if pn.state.user == 'guest':
    prompt = pn.pane.Markdown("Login to access App")

    login_button = pn.widgets.Button(name="Login", width=100)
    login_button.js_on_click(code="""window.location.href = './login'""")

    template.main[:, :] = pn.Column(prompt, login_button)

    template.servable()
else:
    template.header.append(...)
    template.main[:, :] = ...
    template.servable()
    ...

Here print(pn.state.user) starts with None, then goes to "guest" and after I click the button and authenticate with Google, I am back on the same page and it again prints "guest". Did I miss something on the OAuth setup part?

Also when a connection is closed, I get the following:

2023-10-31 21:41:12,873 DocumentLifeCycleHandler on_session_destroyed callback <bound method OAuthProvider._remove_user of <panel.auth.OAuthProvider object at 0x17cdebd10>> failed with following error: 'NoneType' object has no attribute 'decode'

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any chance you still have an older development version sticking around?

Copy link
Contributor Author

@tupui tupui Oct 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought that and re-created an env from scratch but got the same results. My env and PATH looks normal too. (Will re-check everything once more.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have a more complete example? I just tried to reproduce again and couldn't.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will try to make a more minimal example tomorrow.

My app is here otherwise: https://github.com/Simulation-Decomposition/simdec-python/blob/main/panel/app.py

There is a make serve-oauth that I modified like so:

	PANEL_OAUTH_SCOPE=email panel serve panel/app.py \
		--show \
		--cookie-secret panel_cookie_secret_oauth \
		--oauth-optional \
		--basic-login-template panel/login.html \
		--logout-template panel/logout.html \
		--oauth-provider google \
		--static-dirs _static=docs/_static \
		--reuse-sessions --warm

And on the app itself I was trying to add the if/else as above.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I confirm that even your example does not work on my side. I am on a fresh mamba env with 1.3.1. The user remains guest.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I haven't checked back in here, could you file an issue so this doesn't fall through the cracks?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure no problem, done 😃

@philippjfr philippjfr merged commit 0bad9c4 into holoviz:main Oct 30, 2023
7 of 13 checks passed
@tupui tupui deleted the oauth_guest branch October 30, 2023 13:48
@tupui
Copy link
Contributor Author

tupui commented Oct 30, 2023

Thanks @philippjfr 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants