Skip to content

Commit afa5750

Browse files
yuvipandaconsideRatio
authored andcommitted
Check authentication when websockets open
1 parent 1b9f84b commit afa5750

File tree

2 files changed

+50
-8
lines changed

2 files changed

+50
-8
lines changed

jupyter_server_proxy/handlers.py

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,39 @@ def check_origin(self, origin=None):
130130
async def open(self, port, proxied_path):
131131
raise NotImplementedError("Subclasses of ProxyHandler should implement open")
132132

133+
async def prepare(self, *args, **kwargs):
134+
"""
135+
Enforce authentication on *all* requests.
136+
137+
This method is called *before* any other method for all requests.
138+
See https://www.tornadoweb.org/en/stable/web.html#tornado.web.RequestHandler.prepare.
139+
"""
140+
# Due to https://github.com/jupyter-server/jupyter_server/issues/1012,
141+
# we can not decorate `prepare` with `@web.authenticated`.
142+
# `super().prepare`, which calls `JupyterHandler.prepare`, *must* be called
143+
# before `@web.authenticated` can work. Since `@web.authenticated` is a decorator
144+
# that relies on the decorated method to get access to request information, we can
145+
# not call it directly. Instead, we create an empty lambda that takes a request_handler,
146+
# decorate that with web.authenticated, and call the decorated function.
147+
# super().prepare became async with jupyter_server v2
148+
_prepared = super().prepare(*args, **kwargs)
149+
if _prepared is not None:
150+
await _prepared
151+
152+
# If this is a GET request that wants to be upgraded to a websocket, users not
153+
# already authenticated gets a straightforward 403. Everything else is dealt
154+
# with by `web.authenticated`, which does a 302 to the appropriate login url.
155+
# Websockets are purely API calls made by JS rather than a direct user facing page,
156+
# so redirects do not make sense for them.
157+
if (
158+
self.request.method == "GET"
159+
and self.request.headers.get("Upgrade", "").lower() == "websocket"
160+
):
161+
if not self.current_user:
162+
raise web.HTTPError(403)
163+
else:
164+
web.authenticated(lambda request_handler: None)(self)
165+
133166
async def http_get(self, host, port, proxy_path=""):
134167
"""Our non-websocket GET."""
135168
raise NotImplementedError(
@@ -280,7 +313,6 @@ def _check_host_allowlist(self, host):
280313
else:
281314
return host in self.host_allowlist
282315

283-
@web.authenticated
284316
async def proxy(self, host, port, proxied_path):
285317
"""
286318
This serverextension handles:
@@ -682,7 +714,6 @@ def _realize_rendered_template(self, attribute):
682714
attribute = call_with_asked_args(attribute, self.process_args)
683715
return self._render_template(attribute)
684716

685-
@web.authenticated
686717
async def proxy(self, port, path):
687718
if not path.startswith("/"):
688719
path = "/" + path
@@ -866,7 +897,6 @@ async def ensure_process(self):
866897
del self.state["proc"]
867898
raise
868899

869-
@web.authenticated
870900
async def proxy(self, port, path):
871901
await self.ensure_process()
872902
return await ensure_async(super().proxy(port, path))

tests/test_proxies.py

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
from urllib.parse import quote
88

99
import pytest
10+
from tornado.httpclient import HTTPClientError
1011
from tornado.websocket import websocket_connect
1112

1213
# use ipv4 for CI, etc.
@@ -334,8 +335,8 @@ def test_server_content_encoding_header(
334335
async def test_server_proxy_websocket_messages(
335336
a_server_port_and_token: Tuple[int, str]
336337
) -> None:
337-
PORT = a_server_port_and_token[0]
338-
url = f"ws://{LOCALHOST}:{PORT}/python-websocket/echosocket"
338+
PORT, TOKEN = a_server_port_and_token
339+
url = f"ws://{LOCALHOST}:{PORT}/python-websocket/echosocket?token={TOKEN}"
339340
conn = await websocket_connect(url)
340341
expected_msg = "Hello, world!"
341342
await conn.write_message(expected_msg)
@@ -344,8 +345,8 @@ async def test_server_proxy_websocket_messages(
344345

345346

346347
async def test_server_proxy_websocket_headers(a_server_port_and_token: Tuple[int, str]):
347-
PORT = a_server_port_and_token[0]
348-
url = f"ws://{LOCALHOST}:{PORT}/python-websocket/headerssocket"
348+
PORT, TOKEN = a_server_port_and_token
349+
url = f"ws://{LOCALHOST}:{PORT}/python-websocket/headerssocket?token={TOKEN}"
349350
conn = await websocket_connect(url)
350351
await conn.write_message("Hello")
351352
msg = await conn.read_message()
@@ -394,7 +395,7 @@ async def test_server_proxy_websocket_subprotocols(
394395
proxy_responded,
395396
):
396397
PORT, TOKEN = a_server_port_and_token
397-
url = f"ws://{LOCALHOST}:{PORT}/python-websocket/subprotocolsocket"
398+
url = f"ws://{LOCALHOST}:{PORT}/python-websocket/subprotocolsocket?token={TOKEN}"
398399
conn = await websocket_connect(url, subprotocols=client_requested)
399400
await conn.write_message("Hello, world!")
400401

@@ -418,6 +419,17 @@ async def test_server_proxy_websocket_subprotocols(
418419
assert "Sec-Websocket-Protocol" in conn.headers
419420

420421

422+
async def test_websocket_no_auth_failure(
423+
a_server_port_and_token: Tuple[int, str]
424+
) -> None:
425+
PORT = a_server_port_and_token[0]
426+
# Intentionally do not pass an appropriate token, which should cause a 403
427+
url = f"ws://{LOCALHOST}:{PORT}/python-websocket/headerssocket"
428+
429+
with pytest.raises(HTTPClientError, match=r".*HTTP 403: Forbidden.*"):
430+
await websocket_connect(url)
431+
432+
421433
@pytest.mark.parametrize(
422434
"proxy_path, status",
423435
[

0 commit comments

Comments
 (0)