From 1f89bdd0909120ad785fccee7beab4f3a3ed8012 Mon Sep 17 00:00:00 2001 From: David Brochart Date: Tue, 21 Sep 2021 12:08:34 +0200 Subject: [PATCH 1/4] Rework noauth --- plugins/auth/fps_auth/backends.py | 103 ++++++++++++++++++++ plugins/auth/fps_auth/routes.py | 86 +--------------- plugins/contents/fps_contents/routes.py | 14 +-- plugins/jupyterlab/fps_jupyterlab/routes.py | 16 +-- plugins/kernels/fps_kernels/routes.py | 10 +- plugins/nbconvert/fps_nbconvert/routes.py | 4 +- plugins/terminals/fps_terminals/routes.py | 6 +- 7 files changed, 132 insertions(+), 107 deletions(-) create mode 100644 plugins/auth/fps_auth/backends.py diff --git a/plugins/auth/fps_auth/backends.py b/plugins/auth/fps_auth/backends.py new file mode 100644 index 00000000..07a39d21 --- /dev/null +++ b/plugins/auth/fps_auth/backends.py @@ -0,0 +1,103 @@ +from typing import Optional + +import httpx +from fastapi import Depends, status +from fastapi.security.base import SecurityBase +from fastapi_users.authentication import BaseAuthentication, CookieAuthentication # type: ignore +from fastapi_users import FastAPIUsers, BaseUserManager # type: ignore +from starlette.requests import Request + +from .models import ( + User, + UserCreate, + UserUpdate, + UserDB, +) +from .config import get_auth_config +from .db import secret, get_user_db + + +class NoAuthAuthentication(BaseAuthentication): + def __init__(self, user: UserDB, name: str = "noauth"): + super().__init__(name, logout=False) + self.user = user + self.scheme = SecurityBase + + async def __call__(self, credentials, user_manager): + # always return the user no matter what + return self.user + + +noauth_email = "noauth_user@jupyter.com" +no_auth_user = UserDB(email=noauth_email, hashed_password="") +no_auth_authentication = NoAuthAuthentication(no_auth_user) +auth_config = get_auth_config() + + +class UserManager(BaseUserManager[UserCreate, UserDB]): + user_db_model = UserDB + + async def on_after_register(self, user: UserDB, request: Optional[Request] = None): + user.initialized = True + for oauth_account in user.oauth_accounts: + print(oauth_account) + if oauth_account.oauth_name == "github": + r = httpx.get( + f"https://api.github.com/user/{oauth_account.account_id}" + ).json() + user.anonymous = False + user.username = r["login"] + user.name = r["name"] + user.color = None + user.avatar = r["avatar_url"] + await self.user_db.update(user) + + +class LoginCookieAuthentication(CookieAuthentication): + async def get_login_response(self, user, response, user_manager): + await super().get_login_response(user, response, user_manager) + # set user as logged in + user.logged_in = True + await user_manager.user_db.update(user) + # auto redirect + response.status_code = status.HTTP_302_FOUND + response.headers["Location"] = "/lab" + + async def get_logout_response(self, user, response, user_manager): + await super().get_logout_response(user, response, user_manager) + # set user as logged out + user.logged_in = False + await user_manager.user_db.update(user) + + +cookie_authentication = LoginCookieAuthentication( + cookie_secure=auth_config.cookie_secure, secret=secret +) + + +def get_user_manager(user_db=Depends(get_user_db)): + yield UserManager(user_db) + + +if auth_config.mode == "noauth": + auth_backend = no_auth_authentication +else: + auth_backend = cookie_authentication + +users = FastAPIUsers( + get_user_manager, + [auth_backend], + User, + UserCreate, + UserUpdate, + UserDB, +) + + +async def get_enabled_backends(auth_config=Depends(get_auth_config)): + if auth_config.mode == "noauth": + return [no_auth_authentication] + return [cookie_authentication] + + +current_user = users.current_user(get_enabled_backends=get_enabled_backends) diff --git a/plugins/auth/fps_auth/routes.py b/plugins/auth/fps_auth/routes.py index 6f5cf46a..cc4847dc 100644 --- a/plugins/auth/fps_auth/routes.py +++ b/plugins/auth/fps_auth/routes.py @@ -1,22 +1,16 @@ from uuid import uuid4 -from typing import Optional -import httpx # type: ignore from httpx_oauth.clients.github import GitHubOAuth2 # type: ignore from fps.hooks import register_router # type: ignore from fps.config import get_config, FPSConfig # type: ignore -from fastapi_users.authentication import CookieAuthentication # type: ignore -from fastapi import APIRouter, Depends, HTTPException, status -from fastapi_users import FastAPIUsers, BaseUserManager # type: ignore -from starlette.requests import Request +from fastapi import APIRouter, Depends from sqlalchemy.orm import sessionmaker # type: ignore from .config import get_auth_config -from .db import get_user_db, user_db, secret, database, engine, UserTable +from .db import user_db, secret, database, engine, UserTable +from .backends import users, current_user, cookie_authentication from .models import ( User, - UserCreate, - UserUpdate, UserDB, ) @@ -28,61 +22,6 @@ auth_config = get_auth_config() -class UserManager(BaseUserManager[UserCreate, UserDB]): - user_db_model = UserDB - - async def on_after_register(self, user: UserDB, request: Optional[Request] = None): - user.initialized = True - for oauth_account in user.oauth_accounts: - print(oauth_account) - if oauth_account.oauth_name == "github": - r = httpx.get( - f"https://api.github.com/user/{oauth_account.account_id}" - ).json() - user.anonymous = False - user.username = r["login"] - user.name = r["name"] - user.color = None - user.avatar = r["avatar_url"] - await self.user_db.update(user) - - -def get_user_manager(user_db=Depends(get_user_db)): - yield UserManager(user_db) - - -class LoginCookieAuthentication(CookieAuthentication): - async def get_login_response(self, user, response, user_manager): - await super().get_login_response(user, response, user_manager) - # set user as logged in - user.logged_in = True - await user_manager.user_db.update(user) - # auto redirect - response.status_code = status.HTTP_302_FOUND - response.headers["Location"] = "/lab" - - async def get_logout_response(self, user, response, user_manager): - await super().get_logout_response(user, response, user_manager) - # set user as logged out - user.logged_in = False - await user_manager.user_db.update(user) - - -cookie_authentication = LoginCookieAuthentication( - cookie_secure=auth_config.cookie_secure, secret=secret -) - -auth_backends = [cookie_authentication] - -users = FastAPIUsers( - get_user_manager, - auth_backends, - User, - UserCreate, - UserUpdate, - UserDB, -) - github_oauth_client = GitHubOAuth2( auth_config.client_id, auth_config.client_secret.get_secret_value() ) @@ -149,25 +88,8 @@ async def create_token_user(): await user_db.create(TOKEN_USER) -def current_user(optional: bool = False): - async def _( - auth_config=Depends(get_auth_config), - user: User = Depends(users.current_user(optional=True)), - user_db=Depends(get_user_db), - ): - if auth_config.mode == "noauth": - return await user_db.get_by_email(noauth_email) - elif user is None and not optional: - # FIXME: could be 403 - raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED) - else: - return user - - return _ - - @router.get("/auth/users") -async def get_users(user: User = Depends(current_user())): +async def get_users(user: User = Depends(current_user)): users = session.query(UserTable).all() return [user for user in users if user.logged_in] diff --git a/plugins/contents/fps_contents/routes.py b/plugins/contents/fps_contents/routes.py index 39a75464..f5d1f519 100644 --- a/plugins/contents/fps_contents/routes.py +++ b/plugins/contents/fps_contents/routes.py @@ -9,7 +9,7 @@ from fastapi import APIRouter, Depends from starlette.requests import Request # type: ignore -from fps_auth.routes import current_user # type: ignore +from fps_auth.backends import current_user # type: ignore from fps_auth.models import User # type: ignore from .models import Checkpoint, Content, SaveContent @@ -23,7 +23,7 @@ ) async def create_content( request: Request, - user: User = Depends(current_user()), + user: User = Depends(current_user), ): create_content = await request.json() path = Path(create_content["path"]) @@ -53,13 +53,13 @@ async def create_content( @router.get("/api/contents") async def get_root_content( content: int, - user: User = Depends(current_user()), + user: User = Depends(current_user), ): return Content(**get_path_content(Path(""), bool(content))) @router.get("/api/contents/{path:path}/checkpoints") -async def get_checkpoint(path, user: User = Depends(current_user())): +async def get_checkpoint(path, user: User = Depends(current_user)): src_path = Path(path) dst_path = ( Path(".ipynb_checkpoints") / f"{src_path.stem}-checkpoint{src_path.suffix}" @@ -74,7 +74,7 @@ async def get_checkpoint(path, user: User = Depends(current_user())): async def get_content( path: str, content: int, - user: User = Depends(current_user()), + user: User = Depends(current_user), ): return Content(**get_path_content(Path(path), bool(content))) @@ -82,7 +82,7 @@ async def get_content( @router.put("/api/contents/{path:path}") async def save_content( request: Request, - user: User = Depends(current_user()), + user: User = Depends(current_user), ): save_content = SaveContent(**(await request.json())) try: @@ -110,7 +110,7 @@ async def save_content( "/api/contents/{path:path}/checkpoints", status_code=201, ) -async def create_checkpoint(path, user: User = Depends(current_user())): +async def create_checkpoint(path, user: User = Depends(current_user)): src_path = Path(path) dst_path = ( Path(".ipynb_checkpoints") / f"{src_path.stem}-checkpoint{src_path.suffix}" diff --git a/plugins/jupyterlab/fps_jupyterlab/routes.py b/plugins/jupyterlab/fps_jupyterlab/routes.py index 0e26aea4..c6cd1a50 100644 --- a/plugins/jupyterlab/fps_jupyterlab/routes.py +++ b/plugins/jupyterlab/fps_jupyterlab/routes.py @@ -17,7 +17,7 @@ from fps.hooks import register_router # type: ignore from fps_auth.db import get_user_db # type: ignore -from fps_auth.routes import ( # type: ignore +from fps_auth.backends import ( # type: ignore current_user, cookie_authentication, LoginCookieAuthentication, @@ -91,7 +91,7 @@ async def get_root( @router.get("/lab") async def get_lab( - user: User = Depends(current_user()), jlab_config=Depends(get_jlab_config) + user: User = Depends(current_user), jlab_config=Depends(get_jlab_config) ): return HTMLResponse( get_index("default", jlab_config.collaborative, jlab_config.base_url) @@ -128,7 +128,7 @@ async def get_listings(): @router.get("/lab/api/workspaces/{name}") -async def get_workspace_data(user: User = Depends(current_user(optional=True))): +async def get_workspace_data(user: User = Depends(current_user)): if user: return json.loads(user.workspace) return {} @@ -140,7 +140,7 @@ async def get_workspace_data(user: User = Depends(current_user(optional=True))): ) async def set_workspace( request: Request, - user: User = Depends(current_user()), + user: User = Depends(current_user), user_db=Depends(get_user_db), ): user.workspace = await request.body() @@ -150,7 +150,7 @@ async def set_workspace( @router.get("/lab/workspaces/{name}", response_class=HTMLResponse) async def get_workspace( - name, user: User = Depends(current_user()), jlab_config=Depends(get_jlab_config) + name, user: User = Depends(current_user), jlab_config=Depends(get_jlab_config) ): return get_index(name, jlab_config.collaborative, jlab_config.base_url) @@ -202,7 +202,7 @@ async def get_setting( name0, name1, name2, - user: User = Depends(current_user(optional=True)), + user: User = Depends(current_user), ): with open( prefix_dir / "share" / "jupyter" / "lab" / "static" / "package.json" @@ -248,7 +248,7 @@ async def change_setting( request: Request, name0, name1, - user: User = Depends(current_user()), + user: User = Depends(current_user), user_db=Depends(get_user_db), ): settings = json.loads(user.settings) @@ -259,7 +259,7 @@ async def change_setting( @router.get("/lab/api/settings") -async def get_settings(user: User = Depends(current_user(optional=True))): +async def get_settings(user: User = Depends(current_user)): with open( prefix_dir / "share" / "jupyter" / "lab" / "static" / "package.json" ) as f: diff --git a/plugins/kernels/fps_kernels/routes.py b/plugins/kernels/fps_kernels/routes.py index 8a117d80..d1c2b4f5 100644 --- a/plugins/kernels/fps_kernels/routes.py +++ b/plugins/kernels/fps_kernels/routes.py @@ -9,7 +9,7 @@ from fastapi.responses import FileResponse from starlette.requests import Request # type: ignore -from fps_auth.routes import cookie_authentication, current_user # type: ignore +from fps_auth.backends import cookie_authentication, current_user # type: ignore from fps_auth.models import User # type: ignore from fps_auth.db import get_user_db # type: ignore from fps_auth.config import get_auth_config # type: ignore @@ -75,7 +75,7 @@ async def get_kernels(): @router.delete("/api/sessions/{session_id}", status_code=204) async def delete_session( session_id: str, - user: User = Depends(current_user()), + user: User = Depends(current_user), ): kernel_id = sessions[session_id]["kernel"]["id"] kernel_server = kernels[kernel_id]["server"] @@ -88,7 +88,7 @@ async def delete_session( @router.patch("/api/sessions/{session_id}") async def rename_session( request: Request, - user: User = Depends(current_user()), + user: User = Depends(current_user), ): rename_session = await request.json() session_id = rename_session.pop("id") @@ -116,7 +116,7 @@ async def get_sessions(): ) async def create_session( request: Request, - user: User = Depends(current_user()), + user: User = Depends(current_user), ): create_session = await request.json() kernel_name = create_session["kernel"]["name"] @@ -150,7 +150,7 @@ async def create_session( @router.post("/api/kernels/{kernel_id}/restart") async def restart_kernel( kernel_id, - user: User = Depends(current_user()), + user: User = Depends(current_user), ): if kernel_id in kernels: kernel = kernels[kernel_id] diff --git a/plugins/nbconvert/fps_nbconvert/routes.py b/plugins/nbconvert/fps_nbconvert/routes.py index 0f6e305f..2a583929 100644 --- a/plugins/nbconvert/fps_nbconvert/routes.py +++ b/plugins/nbconvert/fps_nbconvert/routes.py @@ -6,7 +6,7 @@ from fastapi.responses import FileResponse from fps.hooks import register_router # type: ignore -from fps_auth.routes import current_user # type: ignore +from fps_auth.backends import current_user # type: ignore from fps_auth.models import User # type: ignore router = APIRouter() @@ -27,7 +27,7 @@ async def get_nbconvert_document( format: str, path: str, download: bool, - user: User = Depends(current_user()), + user: User = Depends(current_user), ): exporter = nbconvert.exporters.get_exporter(format) if download: diff --git a/plugins/terminals/fps_terminals/routes.py b/plugins/terminals/fps_terminals/routes.py index 531cf0b6..997f2b80 100644 --- a/plugins/terminals/fps_terminals/routes.py +++ b/plugins/terminals/fps_terminals/routes.py @@ -5,7 +5,7 @@ from fps.hooks import register_router # type: ignore from fastapi import APIRouter, WebSocket, Response, Depends, status -from fps_auth.routes import cookie_authentication, current_user # type: ignore +from fps_auth.backends import cookie_authentication, current_user # type: ignore from fps_auth.models import User # type: ignore from fps_auth.db import get_user_db # type: ignore from fps_auth.config import get_auth_config # type: ignore @@ -25,7 +25,7 @@ async def get_terminals(): @router.post("/api/terminals") async def create_terminal( - user: User = Depends(current_user()), + user: User = Depends(current_user), ): name = str(len(TERMINALS) + 1) terminal = Terminal( @@ -42,7 +42,7 @@ async def create_terminal( @router.delete("/api/terminals/{name}", status_code=204) async def delete_terminal( name: str, - user: User = Depends(current_user()), + user: User = Depends(current_user), ): TERMINALS[name]["server"].quit() del TERMINALS[name] From 7b1c9c7d0d0376ad57d02e26da5a3d8814feb7be Mon Sep 17 00:00:00 2001 From: David Brochart Date: Tue, 21 Sep 2021 14:09:05 +0200 Subject: [PATCH 2/4] Changes according to review --- plugins/auth/fps_auth/backends.py | 33 ++++++++++++++++--------------- plugins/auth/fps_auth/routes.py | 3 +-- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/plugins/auth/fps_auth/backends.py b/plugins/auth/fps_auth/backends.py index 07a39d21..91af217d 100644 --- a/plugins/auth/fps_auth/backends.py +++ b/plugins/auth/fps_auth/backends.py @@ -17,11 +17,16 @@ from .db import secret, get_user_db +class NoAuth(SecurityBase): + def __call__(self): + return + + class NoAuthAuthentication(BaseAuthentication): def __init__(self, user: UserDB, name: str = "noauth"): super().__init__(name, logout=False) self.user = user - self.scheme = SecurityBase + self.scheme = NoAuth() async def __call__(self, credentials, user_manager): # always return the user no matter what @@ -29,9 +34,8 @@ async def __call__(self, credentials, user_manager): noauth_email = "noauth_user@jupyter.com" -no_auth_user = UserDB(email=noauth_email, hashed_password="") -no_auth_authentication = NoAuthAuthentication(no_auth_user) -auth_config = get_auth_config() +noauth_user = UserDB(email=noauth_email, hashed_password="") +noauth_authentication = NoAuthAuthentication(noauth_user) class UserManager(BaseUserManager[UserCreate, UserDB]): @@ -40,11 +44,13 @@ class UserManager(BaseUserManager[UserCreate, UserDB]): async def on_after_register(self, user: UserDB, request: Optional[Request] = None): user.initialized = True for oauth_account in user.oauth_accounts: - print(oauth_account) if oauth_account.oauth_name == "github": - r = httpx.get( - f"https://api.github.com/user/{oauth_account.account_id}" - ).json() + async with httpx.AsyncClient() as client: + r = ( + await client.get( + f"https://api.github.com/user/{oauth_account.account_id}" + ) + ).json() user.anonymous = False user.username = r["login"] user.name = r["name"] @@ -71,7 +77,7 @@ async def get_logout_response(self, user, response, user_manager): cookie_authentication = LoginCookieAuthentication( - cookie_secure=auth_config.cookie_secure, secret=secret + cookie_secure=get_auth_config().cookie_secure, secret=secret ) @@ -79,14 +85,9 @@ def get_user_manager(user_db=Depends(get_user_db)): yield UserManager(user_db) -if auth_config.mode == "noauth": - auth_backend = no_auth_authentication -else: - auth_backend = cookie_authentication - users = FastAPIUsers( get_user_manager, - [auth_backend], + [noauth_authentication, cookie_authentication], User, UserCreate, UserUpdate, @@ -96,7 +97,7 @@ def get_user_manager(user_db=Depends(get_user_db)): async def get_enabled_backends(auth_config=Depends(get_auth_config)): if auth_config.mode == "noauth": - return [no_auth_authentication] + return [noauth_authentication] return [cookie_authentication] diff --git a/plugins/auth/fps_auth/routes.py b/plugins/auth/fps_auth/routes.py index cc4847dc..094dcd24 100644 --- a/plugins/auth/fps_auth/routes.py +++ b/plugins/auth/fps_auth/routes.py @@ -8,7 +8,7 @@ from .config import get_auth_config from .db import user_db, secret, database, engine, UserTable -from .backends import users, current_user, cookie_authentication +from .backends import users, current_user, cookie_authentication, noauth_email from .models import ( User, UserDB, @@ -38,7 +38,6 @@ TOKEN_USER = None USER_TOKEN = None -noauth_email = "noauth_user@jupyter.com" def set_user_token(user_token): From ba6a2ba4b3741ff884ee021a4f10797d4510e76d Mon Sep 17 00:00:00 2001 From: David Brochart Date: Tue, 21 Sep 2021 16:21:50 +0200 Subject: [PATCH 3/4] Fix noauth security scheme --- plugins/auth/fps_auth/backends.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/auth/fps_auth/backends.py b/plugins/auth/fps_auth/backends.py index 91af217d..009b8914 100644 --- a/plugins/auth/fps_auth/backends.py +++ b/plugins/auth/fps_auth/backends.py @@ -19,7 +19,7 @@ class NoAuth(SecurityBase): def __call__(self): - return + return "noauth" class NoAuthAuthentication(BaseAuthentication): From 9431b9475c3cd4fa22c44f8d2b95489faa680b3f Mon Sep 17 00:00:00 2001 From: David Brochart Date: Tue, 21 Sep 2021 17:47:45 +0200 Subject: [PATCH 4/4] Get noauth user from DB --- plugins/auth/fps_auth/backends.py | 16 +++++++++++----- plugins/auth/fps_auth/routes.py | 19 ++++++++++--------- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/plugins/auth/fps_auth/backends.py b/plugins/auth/fps_auth/backends.py index 009b8914..69dee169 100644 --- a/plugins/auth/fps_auth/backends.py +++ b/plugins/auth/fps_auth/backends.py @@ -17,6 +17,14 @@ from .db import secret, get_user_db +NOAUTH_EMAIL = "noauth_user@jupyter.com" +NOAUTH_USER = UserDB( + id="d4ded46b-a4df-4b51-8d83-ae19010272a7", + email=NOAUTH_EMAIL, + hashed_password="", +) + + class NoAuth(SecurityBase): def __call__(self): return "noauth" @@ -29,13 +37,11 @@ def __init__(self, user: UserDB, name: str = "noauth"): self.scheme = NoAuth() async def __call__(self, credentials, user_manager): - # always return the user no matter what - return self.user + noauth_user = await user_manager.user_db.get_by_email(NOAUTH_EMAIL) + return noauth_user or self.user -noauth_email = "noauth_user@jupyter.com" -noauth_user = UserDB(email=noauth_email, hashed_password="") -noauth_authentication = NoAuthAuthentication(noauth_user) +noauth_authentication = NoAuthAuthentication(NOAUTH_USER) class UserManager(BaseUserManager[UserCreate, UserDB]): diff --git a/plugins/auth/fps_auth/routes.py b/plugins/auth/fps_auth/routes.py index 094dcd24..e49166b1 100644 --- a/plugins/auth/fps_auth/routes.py +++ b/plugins/auth/fps_auth/routes.py @@ -8,7 +8,13 @@ from .config import get_auth_config from .db import user_db, secret, database, engine, UserTable -from .backends import users, current_user, cookie_authentication, noauth_email +from .backends import ( + users, + current_user, + cookie_authentication, + NOAUTH_USER, + NOAUTH_EMAIL, +) from .models import ( User, UserDB, @@ -68,14 +74,9 @@ async def shutdown(): async def create_noauth_user(): - user = await user_db.get_by_email(noauth_email) - if user is None: - user = UserDB( - id="d4ded46b-a4df-4b51-8d83-ae19010272a7", - email=noauth_email, - hashed_password="", - ) - await user_db.create(user) + noauth_user = await user_db.get_by_email(NOAUTH_EMAIL) + if noauth_user is None: + await user_db.create(NOAUTH_USER) async def create_token_user():