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.

Sign up for GitHub

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

Jump to bottom

Invalid azure oauth redirect when using prefix #7676

Open
MarcSkovMadsen opened this issue Jan 30, 2025 · 9 comments · May be fixed by #7680
Open

Invalid azure oauth redirect when using prefix #7676

MarcSkovMadsen opened this issue Jan 30, 2025 · 9 comments · May be fixed by #7680
Milestone
v1.6.1

Comments

@MarcSkovMadsen
Copy link
Collaborator

panel==1.6.0

In trying to get azure oauth working in combination with a --prefix. I'm running on my laptop.

I have the app

import panel as pn
import logging
import sys

pn.extension()

FORMAT = "%(asctime)s | %(levelname)s | %(name)s | %(message)s"

@pn.cache
def get_logger(name, format_=FORMAT, level=logging.DEBUG):
    logger = logging.getLogger(name)

    logger.handlers.clear()

    handler = logging.StreamHandler()
    handler.setStream(sys.stdout)
    formatter = logging.Formatter(format_)
    handler.setFormatter(formatter)
    logger.addHandler(handler)
    logger.propagate = False

    logger.setLevel(level)
    logger.info("Logger successfully configured")
    return logger

get_logger(name="panel.auth")

user = pn.state.user or "Guest User"

pn.panel(user).servable()

To serve the app I run

panel serve script.py --index script --oauth-provider=azure --oauth-key='*' --oauth-secret='*' --cookie-secret='*' --oauth-encryption-key='*' --oauth-extra-params "{'tenant': '*'}" --prefix=mt-devops/panel --index=script --oauth-redirect-uri=http://localhost:5006/mt-devops/panel/
  • If I use the /mt-devops/panel/logout url and click the login button I end up at /mt-devops/panel (not /mt-devops/panel/).
  • If I use the /mt-devops/panel/login url I end up at /mt-devops/panel (not /mt-devops/panel/).
  • If I use the /mt-devops/panel/ url I end up at /mt-devops/panel/. THIS WORKS.
@MarcSkovMadsen MarcSkovMadsen added TRIAGE Default label for untriaged issues and removed TRIAGE Default label for untriaged issues labels Jan 30, 2025
@MarcSkovMadsen
Copy link
Collaborator Author

This video shows the issues

redirect-problem.mp4

@MarcSkovMadsen
Copy link
Collaborator Author

I've also tried with --prefix=mt-devops/panel/. Does not change the behaviour.

@MarcSkovMadsen
Copy link
Collaborator Author

The cause/ problem is here

Image

@philippjfr philippjfr added this to the v1.6.1 milestone Jan 30, 2025
@philippjfr
Copy link
Member

Let's prioritize this fix along with the --index problem and get a new release out asap.

@philippjfr
Copy link
Member

Can you easily test, what happens when you change it to?

root_url = self.request.uri.replace(self._login_endpoint, '').split('?')[0]
if not root_url.endswith('/'):
    root_url += '/'

@MarcSkovMadsen
Copy link
Collaborator Author

MarcSkovMadsen commented Jan 31, 2025 •
edited
Loading

Yes. Every way of logging in and out works on laptop. I've tested with and without --prefix.

Some of the test cases would be

| `uri` | `_login_endpoint` | `root_url` |
| - | - | - | - |
| /login? | /login | / |
| /login?next=%2F | /login | / |
| /signin? | /signin | / |
| /signin?next=%2F | /signin | / |
| /signin?next=%2Fscript | /signin | / |
| /mt-devops/panel/login? | /login | /mt-devops/panel/ |

I'm not able to easily test this in JupyterHub or in Docker container on k8s.

@MarcSkovMadsen MarcSkovMadsen mentioned this issue Jan 31, 2025
Open
@Coderambling
Copy link
Contributor

Related? #6935

@philippjfr philippjfr linked a pull request Jan 31, 2025 that will close this issue
Draft
1 task
@philippjfr
Copy link
Member

philippjfr commented Jan 31, 2025 •
edited
Loading

No, unrelated. @MarcSkovMadsen Worked a little on setting up testing infra to make sure this doesn't regress after we fix it. One thing that I think we need to clarify (since some of what you said conflates the two) is that the root path and the --prefix are completely different things. The --prefix is honestly a little odd since it simply serves the apps on some subpath, this can be useful when you want to add other custom handlers to the server but rarely makes sense if you just want to panel serve. The --prefix has absolutely no relationship to what happens when you use a reverse proxy (like nginx), e.g. in a JupyterHub setup, where Panel is served on some URL on the server. FastAPI asks you to explicitly define the root_path you are serving on, while Panel has aimed to infer this automatically (with wildly varying success as you can tell). With the new testing setup I think we can make sure we can finally make sure we cover all these cases properly.

@MarcSkovMadsen
Copy link
Collaborator Author

I thought I was using and had to use --prefix when deploying to kubernetes. I looked at what we actually do and can see we don't use --prefix. Its always confusing because every framework I've tried works differently here and I've tried deploying them all 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.6.1
Development

Successfully merging a pull request may close this issue.

Ensure root_url is correctly determined during auth holoviz/panel
3 participants
@philippjfr @MarcSkovMadsen @Coderambling