Skip to content

Commit

Permalink
Support logouts; rewrite auth APIs to clearly differentiate between h…
Browse files Browse the repository at this point in the history
…ashed & 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
  • Loading branch information
cmyui authored Aug 8, 2024
1 parent 884ded6 commit 7567ff1
Show file tree
Hide file tree
Showing 5 changed files with 99 additions and 25 deletions.
7 changes: 2 additions & 5 deletions app/api/authorization.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import hashlib
from app import security
from app.errors import Error, ErrorCode
from app.repositories import access_tokens

Expand All @@ -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(
Expand Down
45 changes: 43 additions & 2 deletions app/api/public/authentication.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -53,11 +54,51 @@ 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,
httponly=True,
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
34 changes: 20 additions & 14 deletions app/repositories/access_tokens.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
import hashlib

from pydantic import BaseModel

import app.state
from app.common_types import UserPrivileges


class AccessToken(BaseModel):
access_token: str
hashed_access_token: str
user_id: int
privileges: UserPrivileges
description: str
Expand All @@ -19,46 +17,54 @@ 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"],
private=rec["private"],
)


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)
9 changes: 8 additions & 1 deletion app/security.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
29 changes: 26 additions & 3 deletions app/usecases/authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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(
Expand All @@ -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

0 comments on commit 7567ff1

Please sign in to comment.