From cf327858461eae4d1e6fa7c4b2d427a49e056365 Mon Sep 17 00:00:00 2001 From: Philipp Rudiger Date: Wed, 12 Feb 2025 15:05:31 +0100 Subject: [PATCH] Ensure root_url is correctly determined during auth (#7680) --- .github/workflows/test.yaml | 9 ++- doc/how_to/server/proxy.md | 26 ++++++++- panel/auth.py | 57 ++++++++++--------- panel/command/serve.py | 18 +++++- panel/io/server.py | 17 ++++-- panel/tests/conftest.py | 2 +- panel/tests/ui/test_auth.py | 110 +++++++++++++++++++++++++++++++++++- panel/tests/util.py | 43 ++++++++++++-- pixi.toml | 2 +- 9 files changed, 233 insertions(+), 51 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 46d0538740..6e89326521 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -181,11 +181,6 @@ jobs: AUTH0_OAUTH_EXTRA_PARAMS: ${{ secrets.AUTH0_OAUTH_EXTRA_PARAMS }} AUTH0_OAUTH_USER: ${{ secrets.AUTH0_OAUTH_USER }} AUTH0_OAUTH_PASSWORD: ${{ secrets.AUTH0_OAUTH_PASSWORD }} - AZURE_PORT: "5702" - AZURE_OAUTH_KEY: ${{ secrets.AZURE_OAUTH_KEY }} - AZURE_OAUTH_SECRET: ${{ secrets.AZURE_OAUTH_SECRET }} - AZURE_OAUTH_USER: ${{ secrets.AZURE_OAUTH_USER }} - AZURE_OAUTH_PASSWORD: ${{ secrets.AZURE_OAUTH_PASSWORD }} OKTA_PORT: "5703" OKTA_OAUTH_KEY: ${{ secrets.OKTA_OAUTH_KEY }} OKTA_OAUTH_SECRET: ${{ secrets.OKTA_OAUTH_SECRET }} @@ -212,6 +207,10 @@ jobs: with: resource: http-get://localhost:8887/lab timeout: 180000 + - name: Check if auth should run + if: '!github.event.pull_request.head.repo.fork' + run: | + echo "PANEL_TEST_AUTH=1" >> $GITHUB_ENV - name: Test UI run: | # Create a .uicoveragerc file to set the concurrency library to greenlet diff --git a/doc/how_to/server/proxy.md b/doc/how_to/server/proxy.md index 3e3bf44485..986184615b 100644 --- a/doc/how_to/server/proxy.md +++ b/doc/how_to/server/proxy.md @@ -1,3 +1,27 @@ # Configuring a reverse proxy -If the goal is to serve an web application to the general Internet, it is often desirable to host the application on an internal network, and proxy connections to it through some dedicated HTTP server. For some basic configurations to set up a Bokeh server behind some common reverse proxies, including Nginx and Apache, refer to the [Bokeh documentation](https://bokeh.pydata.org/en/latest/docs/user_guide/server.html#basic-reverse-proxy-setup). +If the goal is to serve an web application to the general Internet, it is often desirable to host the application on an internal network, and proxy connections to it through some dedicated HTTP server. For some basic configurations to set up a Bokeh server behind some common reverse proxies, including Nginx and Apache, refer to the [Bokeh documentation](https://docs.bokeh.org/en/latest/docs/user_guide/server/deploy.html#basic-reverse-proxy-setup). + +Two important things that can cause issues when deploying a Panel app behind a reverse proxy are issues with large client headers and handling of the root path. + +## Client Headers + +To communicate cookies and headers across processes, Panel may include this information in a JSON web token, sending it via a WebSocket. In certain cases this token can grow very large causing Nginx (or another reverse proxy) to drop the request. You may have to work around this by overriding the default Nginx setting `large_client_header_buffers`: + +``` +large_client_header_buffers 4 24k; +``` + +### Proxy with a stripped path prefixes + +Configuring a proxy with a stripped path prefix, which is one common use of a proxy, means that you might serve a Panel app on `/app`, but then configure the proxy to serve the app under a path prefix like `/api/v1`. Unless you configure the proxy to forward appropriate headers Panel will have no idea that it is being served on a path prefix and may incorrectly configure internal redirects. To avoid this provide `panel` with a `root_path` when serving: + +```bash +panel serve app.py --root-path /proxy/ +``` + +Additionally, should you have configured an OAuth provider you **must** also declare an explicit `--oauth-redirect-uri` that includes the proxied path, e.g.: + +```bash +panel serve app.py --root-path /api/v1/ ... --oauth-redirect-uri https:///api/v1/login +``` diff --git a/panel/auth.py b/panel/auth.py index d9a55e18b8..a02b7bc82c 100644 --- a/panel/auth.py +++ b/panel/auth.py @@ -119,6 +119,12 @@ def _SCOPE(self): return self._DEFAULT_SCOPES return [scope for scope in os.environ['PANEL_OAUTH_SCOPE'].split(',')] + @property + def _redirect_uri(self): + if config.oauth_redirect_uri: + return config.oauth_redirect_uri + return f"{self.request.protocol}://{self.request.host}{state.base_url[:-1]}" + async def get_authenticated_user(self, redirect_uri, client_id, state, client_secret=None, code=None): """ @@ -306,8 +312,13 @@ def set_state_cookie(self, state): ) def get_state(self): - root_url = self.request.uri.replace(self._login_endpoint, '') + # Determine root url by removing login subpath and query parameters + root_url = self.request.uri.replace(self._login_endpoint, '').split('?')[0] + if not root_url.endswith('/'): + root_url += '/' next_url = original_next_url = self.get_argument('next', root_url) + if state.base_url and not next_url.startswith(state.base_url): + next_url = original_next_url = next_url.replace('/', state.base_url, 1) if next_url: # avoid browsers treating \ as / next_url = next_url.replace('\\', urlparse.quote('\\')) @@ -322,7 +333,7 @@ def get_state(self): "Ignoring next_url %r, using %r", original_next_url, next_url ) return _serialize_state( - {'state_id': uuid.uuid4().hex, 'next_url': next_url or '/'} + {'state_id': uuid.uuid4().hex, 'next_url': next_url or state.base_url} ) def get_code(self): @@ -343,12 +354,8 @@ def set_code_cookie(self, code): async def get(self): log.debug("%s received login request", type(self).__name__) - if config.oauth_redirect_uri: - redirect_uri = config.oauth_redirect_uri - else: - redirect_uri = f"{self.request.protocol}://{self.request.host}" params = { - 'redirect_uri': redirect_uri, + 'redirect_uri': self._redirect_uri, 'client_id': config.oauth_key, } @@ -381,7 +388,7 @@ async def get(self): log.warning("OAuth state mismatch: %s != %s", cookie_state, url_state) raise HTTPError(401, "OAuth state mismatch. Please restart the authentication flow.", reason='state mismatch') - state = _deserialize_state(url_state) + decoded_state = _deserialize_state(url_state) # For security reason, the state value (cross-site token) will be # retrieved from the query string. params.update({ @@ -393,11 +400,11 @@ async def get(self): if user is None: raise HTTPError(403, "Permissions unknown.") log.debug("%s authorized user, redirecting to app.", type(self).__name__) - self.redirect(state.get('next_url', '/')) + self.redirect(decoded_state.get('next_url', state.base_url)) else: # Redirect for user authentication - params['state'] = state = self.get_state() - self.set_state_cookie(state) + params['state'] = decoded_state = self.get_state() + self.set_state_cookie(decoded_state) await self.get_authenticated_user(**params) @staticmethod @@ -528,6 +535,8 @@ def get(self): next_url = self.get_argument('next', None) if next_url: + if state.base_url and not next_url.startswith(state.base_url): + next_url = next_url.replace('/', state.base_url, 1) self.set_cookie("next_url", next_url) html = self._login_template.render( errormessage=errormessage, @@ -538,19 +547,14 @@ def get(self): async def post(self): username = self.get_argument("username", "") password = self.get_argument("password", "") - if config.oauth_redirect_uri: - redirect_uri = config.oauth_redirect_uri - else: - redirect_uri = f"{self.request.protocol}://{self.request.host}{self._login_endpoint}" user, _, _, _ = await self._fetch_access_token( client_id=config.oauth_key, - redirect_uri=redirect_uri, + redirect_uri=self._redirect_uri, username=username, password=password ) - if not user: - return - self.redirect('/') + next_url = self.get_cookie("next_url", state.base_url) + self.redirect(next_url) class CodeChallengeLoginHandler(GenericLoginHandler): @@ -558,11 +562,7 @@ class CodeChallengeLoginHandler(GenericLoginHandler): async def get(self): code = self.get_argument("code", "") url_state = self.get_argument("state", "") - if config.oauth_redirect_uri: - redirect_uri = config.oauth_redirect_uri - else: - redirect_uri = f"{self.request.protocol}://{self.request.host}{self._login_endpoint}" - + redirect_uri = self._redirect_uri if not code or not url_state: self._authorize_redirect(redirect_uri) return @@ -577,7 +577,7 @@ async def get(self): if user is None: raise HTTPError(403) log.debug("%s authorized user, redirecting to app.", type(self).__name__) - self.redirect(state.get('next_url', '/')) + self.redirect(state.get('next_url', state.base_url)) def _authorize_redirect(self, redirect_uri): state = self.get_state() @@ -814,9 +814,10 @@ def get(self): errormessage = self.get_argument("error") except Exception: errormessage = "" - - next_url = self.get_argument('next', None) + next_url = self.get_argument('next', state.base_url) if next_url: + if state.base_url and not next_url.startswith(state.base_url): + next_url = next_url.replace('/', state.base_url, 1) self.set_cookie("next_url", next_url) html = self._login_template.render( errormessage=errormessage, @@ -846,7 +847,7 @@ def post(self): auth = self._validate(username, password) if auth: self.set_current_user(username) - next_url = self.get_cookie("next_url", "/") + next_url = self.get_cookie("next_url", state.base_url) self.redirect(next_url) else: error_msg = "?error=" + tornado.escape.url_escape("Invalid username or password!") diff --git a/panel/command/serve.py b/panel/command/serve.py index 3abdb7d418..49251d038f 100644 --- a/panel/command/serve.py +++ b/panel/command/serve.py @@ -40,7 +40,7 @@ from ..io.rest import REST_PROVIDERS from ..io.server import INDEX_HTML, get_static_routes, set_curdoc from ..io.state import state -from ..util import fullpath +from ..util import edit_readonly, fullpath log = logging.getLogger(__name__) @@ -177,6 +177,11 @@ class Serve(_BkServe): "or if they can access all applications as a guest." ) )), + ('--root-path', Argument( + action = 'store', + type = str, + help = "The root path can be used to handle cases where Panel is served behind a proxy." + )), ('--login-endpoint', Argument( action = 'store', type = str, @@ -380,6 +385,17 @@ def customize_kwargs(self, args, server_kwargs): config.global_loading_spinner = args.global_loading_spinner config.reuse_sessions = args.reuse_sessions + if args.root_path: + root_path = args.root_path + if not root_path.endswith('/'): + root_path += '/' + if not root_path.startswith('/'): + raise ValueError( + '--root-path must start with a leading slash (`/`).' + ) + with edit_readonly(state): + state.base_url = args.root_path + if config.autoreload: for f in files: watch(f) diff --git a/panel/io/server.py b/panel/io/server.py index c5ddc5bfe6..2e851a48ad 100644 --- a/panel/io/server.py +++ b/panel/io/server.py @@ -57,7 +57,7 @@ # Internal imports from ..config import config -from ..util import fullpath +from ..util import edit_readonly, fullpath from ..util.warnings import warn from .application import build_applications from .document import ( # noqa @@ -879,6 +879,7 @@ def get_server( oauth_refresh_tokens: str | None = None, oauth_guest_endpoints: list[str] | None = None, oauth_optional: bool | None = None, + root_path: str | None = None, login_endpoint: str | None = None, logout_endpoint: str | None = None, login_template: str | None = None, @@ -937,8 +938,6 @@ def get_server( The client secret for the OAuth provider oauth_redirect_uri: Optional[str] = None, Overrides the default OAuth redirect URI - oauth_jwt_user: Optional[str] = None, - Key that identifies the user in the JWT id_token. oauth_extra_params: dict (optional, default={}) Additional information for the OAuth provider oauth_error_template: str (optional, default=None) @@ -948,13 +947,18 @@ def get_server( oauth_encryption_key: str (optional, default=None) A random encryption key used for encrypting OAuth user information and access tokens. + oauth_jwt_user: Optional[str] = None, + Key that identifies the user in the JWT id_token. + oauth_refresh_tokens: bool (optional, default=None) + Whether to automatically refresh OAuth access tokens when they expire. oauth_guest_endpoints: list (optional, default=None) List of endpoints that can be accessed as a guest without authenticating. oauth_optional: bool (optional, default=None) Whether the user will be forced to go through login flow or if they can access all applications as a guest. - oauth_refresh_tokens: bool (optional, default=None) - Whether to automatically refresh OAuth access tokens when they expire. + root_path: str (optional, default=None) + Root path the application is being served on when behind + a reverse proxy. login_endpoint: str (optional, default=None) Overrides the default login endpoint `/login` logout_endpoint: str (optional, default=None) @@ -1094,6 +1098,9 @@ def flask_handler(slug, app): config.oauth_guest_endpoints = oauth_guest_endpoints # type: ignore if oauth_jwt_user is not None: config.oauth_jwt_user = oauth_jwt_user # type: ignore + if root_path: + with edit_readonly(state): + state.base_url = root_path # type: ignore opts['cookie_secret'] = config.cookie_secret server = Server(apps, port=port, **opts) diff --git a/panel/tests/conftest.py b/panel/tests/conftest.py index 735304e906..7ca241cbe3 100644 --- a/panel/tests/conftest.py +++ b/panel/tests/conftest.py @@ -52,7 +52,7 @@ for e in os.environ: - if e.startswith(('BOKEH_', "PANEL_")) and e not in ("PANEL_LOG_LEVEL", ): + if e.startswith(('BOKEH_', "PANEL_")) and e not in ("PANEL_LOG_LEVEL", "PANEL_TEST_AUTH"): os.environ.pop(e, None) @cache diff --git a/panel/tests/ui/test_auth.py b/panel/tests/ui/test_auth.py index f5b82b2395..34f1562bce 100644 --- a/panel/tests/ui/test_auth.py +++ b/panel/tests/ui/test_auth.py @@ -12,14 +12,13 @@ from panel.io.state import state from panel.pane import Markdown from panel.tests.util import ( - linux_only, run_panel_serve, serve_component, wait_for_port, wait_until, - write_file, + linux_only, reverse_proxy, run_panel_serve, serve_component, wait_for_port, + wait_until, write_file, ) pytestmark = pytest.mark.ui auth_check = pytest.mark.skipif('PANEL_TEST_AUTH' not in os.environ, reason='PANEL_TEST_AUTH environment variable is required to run this test') - @linux_only @pytest.mark.parametrize('prefix', ['', 'prefix']) def test_basic_auth(py_file, page, prefix): @@ -42,9 +41,36 @@ def test_basic_auth(py_file, page, prefix): expect(page.locator('.markdown')).to_have_text('test_user', timeout=10000) +@linux_only +@pytest.mark.parametrize('prefix', ['', 'prefix']) +def test_basic_auth_via_proxy(py_file, page, prefix, reverse_proxy): + app = "import panel as pn; pn.pane.Markdown(pn.state.user).servable(title='A')" + write_file(app, py_file.file) + + app_name = os.path.basename(py_file.name)[:-3] + + port, proxy = reverse_proxy + cmd = [ + "--port", str(port), "--basic-auth", "my_password", "--cookie-secret", "secret", py_file.name, + "--root-path", "/proxy/", "--allow-websocket-origin", f"localhost:{proxy}" + ] + if prefix: + app_name = f'{prefix}/{app_name}' + cmd += ['--prefix', prefix] + with run_panel_serve(cmd) as p: + wait_for_port(p.stdout) + page.goto(f"http://localhost:{proxy}/proxy/{app_name}") + page.locator('input[name="username"]').fill("test_user") + page.locator('input[name="password"]').fill("my_password") + page.get_by_role("button").click(force=True) + + expect(page.locator('.markdown')).to_have_text('test_user', timeout=10000) + @linux_only @auth_check +@pytest.mark.skipif('OKTA_OAUTH_KEY' not in os.environ, reason='OKTA_OAUTH_KEY environment variable is required to run this test') +@pytest.mark.xdist_group("auth") def test_okta_oauth(py_file, page): app = "import panel as pn; pn.pane.Markdown(pn.state.user).servable(title='A')" write_file(app, py_file.file) @@ -76,6 +102,8 @@ def test_okta_oauth(py_file, page): @linux_only @auth_check +@pytest.mark.skipif('AZURE_OAUTH_KEY' not in os.environ, reason='AZURE_OAUTH_KEY environment variable is required to run this test') +@pytest.mark.xdist_group("auth") def test_azure_oauth(py_file, page): app = "import panel as pn; pn.pane.Markdown(pn.state.user).servable(title='A')" write_file(app, py_file.file) @@ -110,6 +138,8 @@ def test_azure_oauth(py_file, page): @linux_only @auth_check +@pytest.mark.skipif('AUTH0_OAUTH_KEY' not in os.environ, reason='AUTH0_OAUTH_KEY environment variable is required to run this test') +@pytest.mark.xdist_group("auth") def test_auth0_oauth(py_file, page): app = "import panel as pn; pn.pane.Markdown(pn.state.user).servable(title='A')" write_file(app, py_file.file) @@ -139,6 +169,41 @@ def test_auth0_oauth(py_file, page): expect(page.locator('.markdown')).to_have_text(auth0_user, timeout=10000) +@linux_only +@auth_check +@pytest.mark.skipif('AUTH0_OAUTH_KEY' not in os.environ, reason='AUTH0_OAUTH_KEY environment variable is required to run this test') +@pytest.mark.xdist_group("auth") +def test_auth0_oauth_via_proxy(py_file, page): + app = "import panel as pn; pn.pane.Markdown(pn.state.user).servable(title='A')" + write_file(app, py_file.file) + + proxy = os.environ.get('AUTH0_PORT', '5701') + cookie_secret = os.environ['OAUTH_COOKIE_SECRET'] + encryption_key = os.environ['OAUTH_ENCRYPTION_KEY'] + oauth_key = os.environ['AUTH0_OAUTH_KEY'] + oauth_secret = os.environ['AUTH0_OAUTH_SECRET'] + extra_params = os.environ['AUTH0_OAUTH_EXTRA_PARAMS'] + auth0_user = os.environ['AUTH0_OAUTH_USER'] + auth0_password = os.environ['AUTH0_OAUTH_PASSWORD'] + with reverse_proxy(proxy_port=proxy) as (port, _): + cmd = [ + "--port", port, "--oauth-provider", "auth0", "--oauth-key", oauth_key, + "--oauth-secret", oauth_secret, "--cookie-secret", cookie_secret, + "--oauth-encryption-key", encryption_key, "--oauth-extra-params", extra_params, + "--allow-websocket-origin", f"localhost:{proxy}", "--root-path", "/proxy/", + "--oauth-redirect-uri", f"http://localhost:{proxy}/proxy/login", + py_file.name + ] + with run_panel_serve(cmd) as p: + port = wait_for_port(p.stdout) + page.goto(f"http://localhost:{port}") + + page.locator('input[name="username"]').fill(auth0_user) + page.locator('input[name="password"]').fill(auth0_password) + page.get_by_role("button", name="Continue", exact=True).click(force=True) + + expect(page.locator('.markdown')).to_have_text(auth0_user, timeout=10000) + @linux_only @pytest.mark.parametrize('logout_template', [None, (pathlib.Path(__file__).parent / 'logout.html').absolute()]) def test_basic_auth_logout(py_file, page, logout_template): @@ -173,6 +238,45 @@ def test_basic_auth_logout(py_file, page, logout_template): assert 'id_token' not in cookies +@linux_only +@pytest.mark.parametrize('logout_template', [None, (pathlib.Path(__file__).parent / 'logout.html').absolute()]) +def test_basic_auth_logout_via_proxy(py_file, page, logout_template, reverse_proxy): + app = "import panel as pn; pn.pane.Markdown(pn.state.user).servable(title='A')" + write_file(app, py_file.file) + + app_name = os.path.basename(py_file.name)[:-3] + + port, proxy = reverse_proxy + cmd = [ + "--port", str(port), "--basic-auth", "my_password", "--cookie-secret", + "secret", py_file.name, "--root-path", "/proxy/", "--allow-websocket-origin", + f"localhost:{proxy}" + ] + if logout_template: + cmd += ['--logout-template', str(logout_template)] + with run_panel_serve(cmd) as p: + wait_for_port(p.stdout) + page.goto(f"http://localhost:{proxy}/proxy/{app_name}") + + page.locator('input[name="username"]').fill("test_user") + page.locator('input[name="password"]').fill("my_password") + page.get_by_role("button").click(force=True) + + expect(page.locator('.markdown')).to_have_text('test_user', timeout=10000) + + cookies = [cookie['name'] for cookie in page.context.cookies()] + assert 'user' in cookies + assert 'id_token' in cookies + + page.goto(f"http://localhost:{proxy}/proxy/logout") + + assert page.title() == ('Test Logout Page' if logout_template else 'Panel App | Logout') + + cookies = [cookie['name'] for cookie in page.context.cookies()] + assert 'user' not in cookies + assert 'id_token' not in cookies + + @linux_only def test_authorize_callback_redirect(page): diff --git a/panel/tests/util.py b/panel/tests/util.py index 59c46d60dc..9d4b1e819c 100644 --- a/panel/tests/util.py +++ b/panel/tests/util.py @@ -3,6 +3,7 @@ import asyncio import contextlib import http.server +import json import os import platform import re @@ -506,28 +507,58 @@ def end_headers(self): @contextlib.contextmanager -def reverse_proxy(): - port, proxy_port = get_open_ports(2) +def reverse_proxy(port=None, proxy_port=None): + if port is None and proxy_port is None: + port, proxy_port = get_open_ports(2) + elif proxy_port is None: + proxy_port, = get_open_ports(1) + elif port is None: + port, = get_open_ports(1) + headers = { + "request": { + "set": { + "Connection": ["Upgrade"], + "Upgrade": ["websocket"] + } + } + } route_config = { - "match": [{"path": ["/proxy/*"]}], + "match": [ + {"path": ["/proxy/*"]}, + {"path_regexp": { + "name": "proxy_path", + "pattern": "^/proxy/([^/]+)" + }} + ], "handle": [ {"handler": "rewrite", "strip_path_prefix": "/proxy"}, {"handler": "reverse_proxy", "upstreams": [{"dial": f"localhost:{port}"}]} ] } + ws_config = { + "match": [ + {"path_regexp": { + "name": "ws_path", + "pattern": "^/proxy/([^/]+)/ws" + }} + ], + "handle": [ + {"handler": "rewrite", "strip_path_prefix": "/proxy"}, + {"handler": "reverse_proxy", "upstreams": [{"dial": f"localhost:{port}"}], "headers": headers} + ] + } proxy_config = { "listen": [f":{proxy_port}"], - "routes": [route_config] + "routes": [route_config, ws_config] } config = { "admin": {"disabled": True}, - "apps": {"http": {"servers": {"srv0": proxy_config}}} + "apps": {"http": {"servers": {"srv0": proxy_config}}}, } process = subprocess.Popen( ['caddy', 'run', '--config', '-'], stdin=subprocess.PIPE, close_fds=ON_POSIX, text=True ) - import json process.stdin.write(json.dumps(config)) process.stdin.close() try: diff --git a/pixi.toml b/pixi.toml index 871f3aa834..2aefc6cb7a 100644 --- a/pixi.toml +++ b/pixi.toml @@ -183,7 +183,7 @@ packaging = "*" _install-ui = 'playwright install chromium' [feature.test-ui.tasks.test-ui] -cmd = 'pytest panel/tests/ui --ui --browser chromium -n logical --dist loadgroup' +cmd = 'pytest panel/tests/ui --ui --browser chromium -n logical --dist loadgroup -v' depends-on = ["_install-ui"] # =============================================