Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,6 @@ certs/
*.crt
*.key
*.pem

# Litestar file-backed stores (server-side sessions, response cache)
/api/stores/
4 changes: 2 additions & 2 deletions api/docs/adr/002-no-global-mutable-state.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ Chosen option: a single frozen `AppState` dataclass built once in the lifespan.

- Good: initialisation order and the full dependency set are explicit in one place
- Good: missing dependency results in construction error at startup, not a `None` at request time.
- Good: tests build `AppState` with fakes (a mock MyMdC client, an in-memory token store)
- Good: tests build `AppState` with fakes (a mock MyMdC client, a stub OAuth client)
- Good: removes/reduces need to monkeypatch modules and clear cache between tests.
- Bad: (ish?) dependencies must be specified through signatures instead of imported where needed
- This is kind of the whole point, but it does mean there is more code to do the same thing (importing a global is easier/shorter)
Expand All @@ -43,7 +43,7 @@ Chosen option: a single frozen `AppState` dataclass built once in the lifespan.
### The rules

1. All runtime dependencies stored in a single frozen `AppState` dataclass (`state.py`) which is constructed once in the application lifespan and attached to `app.state`. This currently contains:
- App DB engine/sessionmaker, MyMdC client, OAuth client (`None` when auth is disabled), DAMNIT DB registry, token store, subscription cursors.
- App DB engine/sessionmaker, MyMdC client, OAuth client (`None` when auth is disabled), DAMNIT DB registry, subscription cursors.
2. Each field is built by a pure factory function (`create_*`) taking `Settings` as explicit arguments. No factory reads module state or has side effects beyond constructing its object.
3. There is exactly one composition/setup root: the app entrypoint and its lifespan (target shape: `create_app(settings)`, see the ADR-000 layout).
- This is the only place that reads settings to select implementations.
Expand Down
7 changes: 1 addition & 6 deletions api/src/damnit_api/auth/dependencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,9 @@
from sqlmodel.ext.asyncio.session import AsyncSession

from .._mymdc.dependencies import MyMdCClient
from ..state import OAuthClient
from .models import OAuthUserInfo as _OAuthUserInfo
from .models import User as _User
from .token_store import TokenStore
from .oauth import OAuthClient


def get_oauth_client(state: State) -> OAuthClient:
Expand Down Expand Up @@ -41,10 +40,6 @@ async def get_oauth_http_client(
await client.aclose()


def get_token_store(state: State) -> TokenStore:
return state.app_state.token_store # type: ignore[attr-defined]


def get_oauth_user_info(request: Request) -> _OAuthUserInfo:
"""Litestar dependency: resolve OAuthUserInfo from the session."""
return _OAuthUserInfo.from_connection(request) # type: ignore[arg-type]
Expand Down
46 changes: 46 additions & 0 deletions api/src/damnit_api/auth/oauth.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
"""OAuth2/OIDC client configuration.

Owned by the auth slice; the composition root builds it via
`create_oauth_client` and holds it on `AppState`.
"""

from dataclasses import dataclass, field
from typing import TYPE_CHECKING

if TYPE_CHECKING:
from ..shared.settings import Settings

# Session cookie name, shared by `main.py`'s session config and the logout
# handlers in `auth/routers.py` so the two cannot drift.
SESSION_COOKIE_KEY = "session"


@dataclass
class OAuthClient:
"""OAuth2/OIDC client configuration with lazily loaded server metadata."""

client_id: str
client_secret: str
scope: str
server_metadata_url: str
server_metadata: dict = field(default_factory=dict)

async def load_server_metadata(self) -> None:
import httpx

async with httpx.AsyncClient() as http:
resp = await http.get(self.server_metadata_url)
resp.raise_for_status()
self.server_metadata = resp.json()


def create_oauth_client(settings: "Settings") -> OAuthClient | None:
if settings.auth is None:
return None

return OAuthClient(
client_id=settings.auth.client_id,
client_secret=settings.auth.client_secret.get_secret_value(),
scope="openid email groups",
server_metadata_url=str(settings.auth.server_metadata_url),
)
16 changes: 8 additions & 8 deletions api/src/damnit_api/auth/routers.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,8 @@
from .._db.dependencies import DBSession
from .._mymdc.dependencies import MyMdCClient
from ..runs.dependencies import Repositories
from ..state import SESSION_COOKIE_KEY, OAuthClient
from . import dependencies, models
from .token_store import TokenStore
from .oauth import SESSION_COOKIE_KEY, OAuthClient

logger = get_logger()

Expand Down Expand Up @@ -122,7 +121,6 @@ async def callback(
self,
request: Request,
oauth_config: OAuthClient,
token_store: TokenStore,
oauth_http_client: Annotated[
AsyncOAuth2Client, Dependency(skip_validation=True)
],
Expand Down Expand Up @@ -159,8 +157,10 @@ async def callback(
# session and is re-validated against the relative-path allow-list.
target = _sanitize_redirect_target(request.session.pop("_login_redirect", None))

# Tokens live in the session: server-side only, keyed by the session
# id, with no parallel store to keep consistent.
request.session["user"] = user
token_store.store(str(user["sub"]), token)
request.session["tokens"] = token

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooooh this is a bit scary. Do we have encryption on the file store or something similar?


return Redirect(path=target)

Expand All @@ -169,15 +169,15 @@ async def logout(
self,
request: Request,
oauth_config: OAuthClient,
token_store: TokenStore,
) -> Response:
"""Clear the session; revoke tokens in the background."""
user_sub = request.session.get("user", {}).get("sub")
revocation_endpoint = oauth_config.server_metadata.get("revocation_endpoint")
end_session_endpoint = oauth_config.server_metadata.get(
"end_session_endpoint"
)

tokens = request.session.pop("tokens", None) or {}

tokens_to_revoke: list[tuple[str, str]] = []
if (
revocation_endpoint
Expand All @@ -187,10 +187,10 @@ async def logout(
tokens_to_revoke = [
(k, token)
for k in ("refresh_token", "access_token")
if (token := token_store.pop_token_field(user_sub, k))
if (token := tokens.get(k))
]

token_id = token_store.pop_token_field(user_sub, "id_token")
token_id = tokens.get("id_token")
logout_url = None
if token_id and end_session_endpoint:
params = {"id_token_hint": token_id}
Expand Down
29 changes: 0 additions & 29 deletions api/src/damnit_api/auth/token_store.py

This file was deleted.

3 changes: 0 additions & 3 deletions api/src/damnit_api/contextfile/models.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
from typing import Self

import async_lru
from anyio import Path as APath
from pydantic import BaseModel

Expand All @@ -9,7 +8,6 @@ class ModifiedTime(BaseModel):
lastModified: float # noqa: N815

@classmethod
@async_lru.alru_cache(ttl=5)
async def from_file(cls, path: APath) -> Self:
stat = await path.stat()
return cls(lastModified=stat.st_mtime)
Expand All @@ -20,7 +18,6 @@ class ContextFile(BaseModel):
fileContent: str # noqa: N815

@classmethod
@async_lru.alru_cache(ttl=5)
async def from_file(cls, path: APath) -> Self:
content = await path.read_text()
modified_timestamp = await ModifiedTime.from_file(path)
Expand Down
12 changes: 9 additions & 3 deletions api/src/damnit_api/contextfile/routers.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,19 @@
from anyio import Path as APath
from litestar import Router, get
from litestar import Request, Router, get
from litestar.di import Provide

from ..metadata.models import ProposalMeta
from ..metadata.routers import get_proposal_meta
from . import models


@get("/content")
def _proposal_cache_key(request: Request) -> str:
"""Key response-cache entries per proposal so entries stay isolated."""
proposal_number = request.query_params.get("proposal_number", "")
return f"{request.url.path}:{proposal_number}"


@get("/content", cache=5, cache_key_builder=_proposal_cache_key)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to confirm: is this behind auth? I just wonder on the case when the cache is already built by an authorized user who just accessed it, and then subsequenltly accessed by unauthorized user. From reading the code, the endpoint will return the content regardless.

async def get_content(proposal: ProposalMeta) -> models.ContextFile | None:
if proposal.damnit_path is None:
return None
Expand All @@ -16,7 +22,7 @@ async def get_content(proposal: ProposalMeta) -> models.ContextFile | None:
)


@get("/last_modified")
@get("/last_modified", cache=5, cache_key_builder=_proposal_cache_key)
async def get_modified(proposal: ProposalMeta) -> models.ModifiedTime | None:
if proposal.damnit_path is None:
return None
Expand Down
50 changes: 28 additions & 22 deletions api/src/damnit_api/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,42 +14,39 @@


def create_app():
import hashlib

from litestar import Request, Response
from litestar.middleware.session.client_side import CookieBackendConfig
from litestar.middleware.session.server_side import ServerSideSessionConfig
from litestar.openapi import OpenAPIConfig
from litestar.response import Redirect
from litestar.stores.file import FileStore
from litestar.stores.memory import MemoryStore
from litestar.stores.registry import StoreRegistry

from . import _logging, auth, get_logger
from .auth.oauth import SESSION_COOKIE_KEY, create_oauth_client
from .graphql.dependencies import get_subscription_cursors
from .runs.dependencies import get_repositories
from .shared.errors import DamnitWebError
from .shared.gql import get_gql_controller
from .shared.settings import settings
from .state import (
SESSION_COOKIE_KEY,
AppState,
create_db_engine,
create_db_sessionmaker,
create_mymdc_client,
create_oauth_client,
create_repositories,
create_subscription_cursors,
create_token_store,
provide_app_state,
)

logger = get_logger("lifespan")

# ── Session middleware ────────────────────────────────────────────────────
# Derive a 32-byte AES key from the session secret via SHA-256.
session_secret = settings.session_secret
assert session_secret is not None # enforced by Settings validator # noqa: S101
session_config = CookieBackendConfig(
secret=hashlib.sha256(session_secret.get_secret_value().encode()).digest(),
key=SESSION_COOKIE_KEY,
)
# ── Stores + server-side sessions ─────────────────────────────────────────
# Sessions are server-side: the cookie carries only an opaque session id;
# session data lives in a Litestar store. The same registry backs every
# named store (sessions, response cache); the backend is mode-dependent
# (in-memory locally, file-backed otherwise) and selected below.
session_config = ServerSideSessionConfig(key=SESSION_COOKIE_KEY)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can add secure=... here? We can use/introduce an env var to denote if we're prod or dev (or local with is_local).


# ── Exception handlers ────────────────────────────────────────────────────
def dw_error_handler(request: Request, exc: DamnitWebError) -> Response:
Expand Down Expand Up @@ -100,7 +97,6 @@ async def lifespan(app: Litestar):
db_sessionmaker=create_db_sessionmaker(engine),
mymdc_client=create_mymdc_client(settings),
oauth_client=oauth_client,
token_store=create_token_store(),
repositories=create_repositories(),
subscription_cursors=create_subscription_cursors(),
)
Expand All @@ -110,10 +106,22 @@ async def lifespan(app: Litestar):
finally:
await engine.dispose()

# ── Auth controller (mode-dependent, ADR-008 composition) ───────────────
auth_controller = (
auth.NoAuthOAuthController if settings.is_local else auth.OAuthController
)
def _file_store(name: str) -> FileStore:
# FileStore does not create its directory on write; the session read
# path (and forged test writes) need it to exist up front.
path = settings.store_path / name
path.mkdir(parents=True, exist_ok=True)
return FileStore(path)

# ── Mode-dependent composition: controller and stores ───────────────────
# In-memory stores are process-local: local mode is single-worker, and the
# file-backed stores serve the deployed (potentially multi-worker) case.
if settings.is_local:
auth_controller = auth.NoAuthOAuthController
stores = StoreRegistry(default_factory=lambda name: MemoryStore())
else:
auth_controller = auth.OAuthController
stores = StoreRegistry(default_factory=_file_store)

# ── GraphQL controller ────────────────────────────────────────────────────
gql_controller = get_gql_controller()
Expand All @@ -132,9 +140,6 @@ async def lifespan(app: Litestar):
"oauth_config": Provide(
auth.dependencies.get_oauth_client, sync_to_thread=False
),
"token_store": Provide(
auth.dependencies.get_token_store, sync_to_thread=False
),
"session": Provide(get_session),
"mymdc": Provide(get_mymdc_client, sync_to_thread=False),
"user": Provide(get_user),
Expand All @@ -144,6 +149,7 @@ async def lifespan(app: Litestar):
),
"repositories": Provide(get_repositories, sync_to_thread=False),
},
stores=stores,
middleware=[
session_config.middleware,
_logging.RequestLoggingMiddleware,
Expand Down
4 changes: 4 additions & 0 deletions api/src/damnit_api/shared/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ class Settings(BaseSettings):

db_path: Path = Path(__file__).parents[3] / "dw_api.sqlite"

# Directory for file-backed Litestar stores (server-side sessions, response
# cache) outside local mode; each named store gets a subdirectory.
store_path: Path = Path(__file__).parents[3] / "stores"

debug: bool = True

log_level: str = "DEBUG"
Expand Down
Loading
Loading