-
Notifications
You must be signed in to change notification settings - Fork 310
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 an option to have authentication enabled for all endpoints by default #1392
Changes from all commits
3f65147
a9cff16
646739e
4cbb504
faee488
204d29f
dccc423
8e13727
e6a8d9a
2882516
4e1d664
cd84175
5f6bc16
0facfec
90d7044
54c2ea2
319a4d1
5e7615d
4265f4e
c3f5d8f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -14,7 +14,7 @@ | |||||||||||||||||||||||||
import warnings | ||||||||||||||||||||||||||
from http.client import responses | ||||||||||||||||||||||||||
from logging import Logger | ||||||||||||||||||||||||||
from typing import TYPE_CHECKING, Any, Awaitable, Sequence, cast | ||||||||||||||||||||||||||
from typing import TYPE_CHECKING, Any, Awaitable, Coroutine, Sequence, cast | ||||||||||||||||||||||||||
from urllib.parse import urlparse | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
import prometheus_client | ||||||||||||||||||||||||||
|
@@ -29,7 +29,7 @@ | |||||||||||||||||||||||||
from jupyter_server import CallContext | ||||||||||||||||||||||||||
from jupyter_server._sysinfo import get_sys_info | ||||||||||||||||||||||||||
from jupyter_server._tz import utcnow | ||||||||||||||||||||||||||
from jupyter_server.auth.decorator import authorized | ||||||||||||||||||||||||||
from jupyter_server.auth.decorator import allow_unauthenticated, authorized | ||||||||||||||||||||||||||
from jupyter_server.auth.identity import User | ||||||||||||||||||||||||||
from jupyter_server.i18n import combine_translations | ||||||||||||||||||||||||||
from jupyter_server.services.security import csp_report_uri | ||||||||||||||||||||||||||
|
@@ -589,7 +589,7 @@ def check_host(self) -> bool: | |||||||||||||||||||||||||
) | ||||||||||||||||||||||||||
return allow | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
async def prepare(self) -> Awaitable[None] | None: # type:ignore[override] | ||||||||||||||||||||||||||
async def prepare(self, *, _redirect_to_login=True) -> Awaitable[None] | None: # type:ignore[override] | ||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. private keyword arg makes this hard for subclasses, e.g. extensions with websockets, but I suppose they will inherit from our own websocket classes, too? I wonder if this should be a Handler class attribute, rather than a private argument. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a check for inheritance from jupyter_server/jupyter_server/base/websocket.py Lines 110 to 121 in c3f5d8f
A Handler class attribute is certainly a reasonable alternative that I briefly considered. My thinking was along: if you pass |
||||||||||||||||||||||||||
"""Prepare a response.""" | ||||||||||||||||||||||||||
# Set the current Jupyter Handler context variable. | ||||||||||||||||||||||||||
CallContext.set(CallContext.JUPYTER_HANDLER, self) | ||||||||||||||||||||||||||
|
@@ -630,6 +630,25 @@ async def prepare(self) -> Awaitable[None] | None: # type:ignore[override] | |||||||||||||||||||||||||
self.set_cors_headers() | ||||||||||||||||||||||||||
if self.request.method not in {"GET", "HEAD", "OPTIONS"}: | ||||||||||||||||||||||||||
self.check_xsrf_cookie() | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
if not self.settings.get("allow_unauthenticated_access", False): | ||||||||||||||||||||||||||
if not self.request.method: | ||||||||||||||||||||||||||
raise HTTPError(403) | ||||||||||||||||||||||||||
method = getattr(self, self.request.method.lower()) | ||||||||||||||||||||||||||
if not getattr(method, "__allow_unauthenticated", False): | ||||||||||||||||||||||||||
if _redirect_to_login: | ||||||||||||||||||||||||||
# reuse `web.authenticated` logic, which redirects to the login | ||||||||||||||||||||||||||
# page on GET and HEAD and otherwise raises 403 | ||||||||||||||||||||||||||
return web.authenticated(lambda _: super().prepare())(self) | ||||||||||||||||||||||||||
else: | ||||||||||||||||||||||||||
# raise 403 if user is not known without redirecting to login page | ||||||||||||||||||||||||||
user = self.current_user | ||||||||||||||||||||||||||
if user is None: | ||||||||||||||||||||||||||
self.log.warning( | ||||||||||||||||||||||||||
f"Couldn't authenticate {self.__class__.__name__} connection" | ||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||
raise web.HTTPError(403) | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
return super().prepare() | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
# --------------------------------------------------------------- | ||||||||||||||||||||||||||
|
@@ -726,7 +745,7 @@ def write_error(self, status_code: int, **kwargs: Any) -> None: | |||||||||||||||||||||||||
class APIHandler(JupyterHandler): | ||||||||||||||||||||||||||
"""Base class for API handlers""" | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
async def prepare(self) -> None: | ||||||||||||||||||||||||||
async def prepare(self) -> None: # type:ignore[override] | ||||||||||||||||||||||||||
"""Prepare an API response.""" | ||||||||||||||||||||||||||
await super().prepare() | ||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Something for later - API handlers should probably 403, not redirect. Now that we have logic for this. |
||||||||||||||||||||||||||
if not self.check_origin(): | ||||||||||||||||||||||||||
|
@@ -794,6 +813,7 @@ def finish(self, *args: Any, **kwargs: Any) -> Future[Any]: | |||||||||||||||||||||||||
self.set_header("Content-Type", set_content_type) | ||||||||||||||||||||||||||
return super().finish(*args, **kwargs) | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
@allow_unauthenticated | ||||||||||||||||||||||||||
def options(self, *args: Any, **kwargs: Any) -> None: | ||||||||||||||||||||||||||
"""Get the options.""" | ||||||||||||||||||||||||||
if "Access-Control-Allow-Headers" in self.settings.get("headers", {}): | ||||||||||||||||||||||||||
|
@@ -837,7 +857,7 @@ def options(self, *args: Any, **kwargs: Any) -> None: | |||||||||||||||||||||||||
class Template404(JupyterHandler): | ||||||||||||||||||||||||||
"""Render our 404 template""" | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
async def prepare(self) -> None: | ||||||||||||||||||||||||||
async def prepare(self) -> None: # type:ignore[override] | ||||||||||||||||||||||||||
"""Prepare a 404 response.""" | ||||||||||||||||||||||||||
await super().prepare() | ||||||||||||||||||||||||||
raise web.HTTPError(404) | ||||||||||||||||||||||||||
|
@@ -1002,6 +1022,18 @@ def compute_etag(self) -> str | None: | |||||||||||||||||||||||||
"""Compute the etag.""" | ||||||||||||||||||||||||||
return None | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
# access is allowed as this class is used to serve static assets on login page | ||||||||||||||||||||||||||
# TODO: create an allow-list of files used on login page and remove this decorator | ||||||||||||||||||||||||||
@allow_unauthenticated | ||||||||||||||||||||||||||
def get(self, path: str, include_body: bool = True) -> Coroutine[Any, Any, None]: | ||||||||||||||||||||||||||
return super().get(path, include_body) | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
# access is allowed as this class is used to serve static assets on login page | ||||||||||||||||||||||||||
# TODO: create an allow-list of files used on login page and remove this decorator | ||||||||||||||||||||||||||
@allow_unauthenticated | ||||||||||||||||||||||||||
def head(self, path: str) -> Awaitable[None]: | ||||||||||||||||||||||||||
return super().head(path) | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
@classmethod | ||||||||||||||||||||||||||
def get_absolute_path(cls, roots: Sequence[str], path: str) -> str: | ||||||||||||||||||||||||||
"""locate a file to serve on our static file search path""" | ||||||||||||||||||||||||||
|
@@ -1036,6 +1068,7 @@ class APIVersionHandler(APIHandler): | |||||||||||||||||||||||||
|
||||||||||||||||||||||||||
_track_activity = False | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
@allow_unauthenticated | ||||||||||||||||||||||||||
def get(self) -> None: | ||||||||||||||||||||||||||
"""Get the server version info.""" | ||||||||||||||||||||||||||
# not authenticated, so give as few info as possible | ||||||||||||||||||||||||||
|
@@ -1048,6 +1081,7 @@ class TrailingSlashHandler(web.RequestHandler): | |||||||||||||||||||||||||
This should be the first, highest priority handler. | ||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
@allow_unauthenticated | ||||||||||||||||||||||||||
def get(self) -> None: | ||||||||||||||||||||||||||
"""Handle trailing slashes in a get.""" | ||||||||||||||||||||||||||
assert self.request.uri is not None | ||||||||||||||||||||||||||
|
@@ -1064,6 +1098,7 @@ def get(self) -> None: | |||||||||||||||||||||||||
class MainHandler(JupyterHandler): | ||||||||||||||||||||||||||
"""Simple handler for base_url.""" | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
@allow_unauthenticated | ||||||||||||||||||||||||||
def get(self) -> None: | ||||||||||||||||||||||||||
"""Get the main template.""" | ||||||||||||||||||||||||||
html = self.render_template("main.html") | ||||||||||||||||||||||||||
|
@@ -1104,18 +1139,20 @@ async def redirect_to_files(self: Any, path: str) -> None: | |||||||||||||||||||||||||
self.log.debug("Redirecting %s to %s", self.request.path, url) | ||||||||||||||||||||||||||
self.redirect(url) | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
@allow_unauthenticated | ||||||||||||||||||||||||||
async def get(self, path: str = "") -> None: | ||||||||||||||||||||||||||
return await self.redirect_to_files(self, path) | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
class RedirectWithParams(web.RequestHandler): | ||||||||||||||||||||||||||
"""Sam as web.RedirectHandler, but preserves URL parameters""" | ||||||||||||||||||||||||||
"""Same as web.RedirectHandler, but preserves URL parameters""" | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
def initialize(self, url: str, permanent: bool = True) -> None: | ||||||||||||||||||||||||||
"""Initialize a redirect handler.""" | ||||||||||||||||||||||||||
self._url = url | ||||||||||||||||||||||||||
self._permanent = permanent | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
@allow_unauthenticated | ||||||||||||||||||||||||||
def get(self) -> None: | ||||||||||||||||||||||||||
"""Get a redirect.""" | ||||||||||||||||||||||||||
sep = "&" if "?" in self._url else "?" | ||||||||||||||||||||||||||
|
@@ -1128,6 +1165,7 @@ class PrometheusMetricsHandler(JupyterHandler): | |||||||||||||||||||||||||
Return prometheus metrics for this server | ||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
@allow_unauthenticated | ||||||||||||||||||||||||||
def get(self) -> None: | ||||||||||||||||||||||||||
"""Get prometheus metrics.""" | ||||||||||||||||||||||||||
if self.settings["authenticate_prometheus"] and not self.logged_in: | ||||||||||||||||||||||||||
|
@@ -1137,6 +1175,18 @@ def get(self) -> None: | |||||||||||||||||||||||||
self.write(prometheus_client.generate_latest(prometheus_client.REGISTRY)) | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
class PublicStaticFileHandler(web.StaticFileHandler): | ||||||||||||||||||||||||||
"""Same as web.StaticFileHandler, but decorated to acknowledge that auth is not required.""" | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
@allow_unauthenticated | ||||||||||||||||||||||||||
def head(self, path: str) -> Awaitable[None]: | ||||||||||||||||||||||||||
return super().head(path) | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
@allow_unauthenticated | ||||||||||||||||||||||||||
def get(self, path: str, include_body: bool = True) -> Coroutine[Any, Any, None]: | ||||||||||||||||||||||||||
return super().get(path, include_body) | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
# ----------------------------------------------------------------------------- | ||||||||||||||||||||||||||
# URL pattern fragments for reuse | ||||||||||||||||||||||||||
# ----------------------------------------------------------------------------- | ||||||||||||||||||||||||||
|
@@ -1152,6 +1202,6 @@ def get(self) -> None: | |||||||||||||||||||||||||
default_handlers = [ | ||||||||||||||||||||||||||
(r".*/", TrailingSlashHandler), | ||||||||||||||||||||||||||
(r"api", APIVersionHandler), | ||||||||||||||||||||||||||
(r"/(robots\.txt|favicon\.ico)", web.StaticFileHandler), | ||||||||||||||||||||||||||
(r"/(robots\.txt|favicon\.ico)", PublicStaticFileHandler), | ||||||||||||||||||||||||||
(r"/metrics", PrometheusMetricsHandler), | ||||||||||||||||||||||||||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps something even more aggressive, e.g.
@UNAUTHENTICATED
.