From 7567ff1b1d8b96b379189046836d0b22f7d8fc75 Mon Sep 17 00:00:00 2001 From: Josh Smith Date: Thu, 8 Aug 2024 17:07:12 +0200 Subject: [PATCH] Support logouts; rewrite auth APIs to clearly differentiate between hashed & raw credentials (#5) * Logout API * Rewrite auth APIs to more clearly differentiate hashed & unhashed tokens * Take over prod * appease mypy * Require no body for logout * Align cookie params * Update .github/workflows/production-deploy.yml --- app/api/authorization.py | 7 ++--- app/api/public/authentication.py | 45 +++++++++++++++++++++++++++++-- app/repositories/access_tokens.py | 34 +++++++++++++---------- app/security.py | 9 ++++++- app/usecases/authentication.py | 29 +++++++++++++++++--- 5 files changed, 99 insertions(+), 25 deletions(-) diff --git a/app/api/authorization.py b/app/api/authorization.py index 7b6651d..a88cb51 100644 --- a/app/api/authorization.py +++ b/app/api/authorization.py @@ -1,4 +1,4 @@ -import hashlib +from app import security from app.errors import Error, ErrorCode from app.repositories import access_tokens @@ -8,10 +8,7 @@ async def authorize_request( user_access_token: str, expected_user_id: int | None = None, ) -> access_tokens.AccessToken | Error: - hashed_access_token = hashlib.md5( - user_access_token.encode(), - usedforsecurity=False, - ).hexdigest() + hashed_access_token = security.hash_access_token(user_access_token) trusted_access_token = await access_tokens.fetch_one(hashed_access_token) if trusted_access_token is None: return Error( diff --git a/app/api/public/authentication.py b/app/api/public/authentication.py index ef6aeb5..59b9257 100644 --- a/app/api/public/authentication.py +++ b/app/api/public/authentication.py @@ -1,8 +1,9 @@ -from fastapi import APIRouter +from fastapi import APIRouter, Cookie from fastapi import Header from fastapi import Response from pydantic import BaseModel +from app.api import authorization from app.api.responses import JSONResponse from app.errors import Error from app.errors import ErrorCode @@ -53,7 +54,7 @@ async def authenticate( ) http_response.set_cookie( "X-Ripple-Token", - value=response.access_token, + value=response.unhashed_access_token, expires=60 * 60 * 24 * 30, domain="akatsuki.gg", secure=True, @@ -61,3 +62,43 @@ async def authenticate( samesite="none", ) return http_response + + +@router.post("/public/api/v1/logout") +async def logout( + client_ip_address: str = Header(..., alias="X-Real-IP"), + client_user_agent: str = Header(..., alias="User-Agent"), + user_access_token: str = Cookie(..., alias="X-Ripple-Token", strict=True), +) -> Response: + trusted_access_token = await authorization.authorize_request( + user_access_token=user_access_token, + expected_user_id=None, + ) + if isinstance(trusted_access_token, Error): + return JSONResponse( + content=trusted_access_token.model_dump(), + status_code=map_error_code_to_http_status_code( + trusted_access_token.error_code + ), + ) + + response = await authentication.logout( + client_ip_address=client_ip_address, + client_user_agent=client_user_agent, + trusted_access_token=trusted_access_token, + ) + if isinstance(response, Error): + return JSONResponse( + content=response.model_dump(), + status_code=map_error_code_to_http_status_code(response.error_code), + ) + + http_response = Response(status_code=204) + http_response.delete_cookie( + "X-Ripple-Token", + domain="akatsuki.gg", + secure=True, + httponly=True, + samesite="none", + ) + return http_response diff --git a/app/repositories/access_tokens.py b/app/repositories/access_tokens.py index db4b820..2310807 100644 --- a/app/repositories/access_tokens.py +++ b/app/repositories/access_tokens.py @@ -1,5 +1,3 @@ -import hashlib - from pydantic import BaseModel import app.state @@ -7,7 +5,7 @@ class AccessToken(BaseModel): - access_token: str + hashed_access_token: str user_id: int privileges: UserPrivileges description: str @@ -19,24 +17,23 @@ class AccessToken(BaseModel): """ -async def create(*, user_id: int, access_token: str) -> AccessToken: +async def create(*, user_id: int, hashed_access_token: str) -> AccessToken: query = """\ INSERT INTO tokens (user, privileges, description, token, private) - VALUES (:user_id, 0, 'Access token', :access_token, TRUE) + VALUES (:user_id, 0, 'Access token', :hashed_access_token, TRUE) """ - hashed_access_token = hashlib.md5(access_token.encode()).hexdigest() - params = {"user_id": user_id, "access_token": hashed_access_token} + params = {"user_id": user_id, "hashed_access_token": hashed_access_token} await app.state.database.execute(query, params) query = f"""\ SELECT {READ_PARAMS} FROM tokens - WHERE token = :access_token + WHERE token = :hashed_access_token """ - params = {"access_token": hashed_access_token} + params = {"hashed_access_token": hashed_access_token} rec = await app.state.database.fetch_one(query, params) assert rec is not None return AccessToken( - access_token=access_token, + hashed_access_token=hashed_access_token, user_id=rec["user"], privileges=UserPrivileges(rec["privileges"]), description=rec["description"], @@ -44,21 +41,30 @@ async def create(*, user_id: int, access_token: str) -> AccessToken: ) -async def fetch_one(access_token: str) -> AccessToken | None: +async def fetch_one(hashed_access_token: str) -> AccessToken | None: query = """\ SELECT user, privileges, description, private FROM tokens - WHERE token = :access_token + WHERE token = :hashed_access_token """ - params = {"access_token": access_token} + params = {"hashed_access_token": hashed_access_token} rec = await app.state.database.fetch_one(query, params) if rec is None: return None return AccessToken( - access_token=access_token, + hashed_access_token=hashed_access_token, user_id=rec["user"], privileges=UserPrivileges(rec["privileges"]), description=rec["description"], private=rec["private"], ) + + +async def delete_one(hashed_access_token: str) -> None: + query = """\ + DELETE FROM tokens + WHERE token = :hashed_access_token + """ + params = {"hashed_access_token": hashed_access_token} + await app.state.database.execute(query, params) diff --git a/app/security.py b/app/security.py index 1268232..7ba3199 100644 --- a/app/security.py +++ b/app/security.py @@ -32,5 +32,12 @@ def check_osu_password( ) -def generate_access_token() -> str: +def generate_unhashed_access_token() -> str: return secrets.token_urlsafe(nbytes=32) + + +def hash_access_token(unhashed_access_token: str) -> str: + return hashlib.md5( + unhashed_access_token.encode(), + usedforsecurity=False, + ).hexdigest() diff --git a/app/usecases/authentication.py b/app/usecases/authentication.py index bb1af33..e3d54da 100644 --- a/app/usecases/authentication.py +++ b/app/usecases/authentication.py @@ -18,7 +18,7 @@ class Identity(BaseModel): class AuthorizationGrant(BaseModel): - access_token: str + unhashed_access_token: str identity: Identity privileges: UserPrivileges expires_at: datetime | None @@ -59,9 +59,11 @@ async def authenticate( user_feedback="Insufficient privileges.", ) + unhashed_access_token = security.generate_unhashed_access_token() + hashed_access_token = security.hash_access_token(unhashed_access_token) access_token = await access_tokens.create( user_id=user.id, - access_token=security.generate_access_token(), + hashed_access_token=hashed_access_token, ) # TODO: log amplitude web_login event @@ -77,7 +79,8 @@ async def authenticate( ) return AuthorizationGrant( - access_token=access_token.access_token, + # XXX: intentionally send back the unhashed access token + unhashed_access_token=unhashed_access_token, privileges=access_token.privileges, expires_at=None, identity=Identity( @@ -86,3 +89,23 @@ async def authenticate( privileges=user.privileges, ), ) + + +async def logout( + *, + client_ip_address: str, + client_user_agent: str, + trusted_access_token: access_tokens.AccessToken, +) -> None | Error: + await access_tokens.delete_one(trusted_access_token.hashed_access_token) + + logging.info( + "User successfully logged out", + extra={ + "user_id": trusted_access_token.user_id, + "client_ip_address": client_ip_address, + "client_user_agent": client_user_agent, + }, + ) + + return None