fix(ui): fail closed on auth, export the required metrics, finish the workflow
Closes SECURITY_ANALYSIS.md M-08, M-09, M-10, M-11, M-17, the UI half of M-18, and the lower-severity §10 drift. M-08/M-09: signed tokens with exp NaN or Infinity passed (any int/float was accepted and non-finite comparisons never expire), sub was not required, and a multi-audience token with a mismatched azp was admitted. A scalar group claim was coerced into a list where §14.21 requires 403. M-10: with configured origins the Host header was ignored, so an allowed Origin paired with Host: evil.example was accepted. M-11: JWKS was cached forever, so logins failed after a PocketID key rotation until the pod restarted; an unknown kid now forces one refresh. M-17: §13a.7 requires an embedding error counter, a search latency histogram and MCP calls by tool and outcome; none existed, and monotonic totals were Gauges. M-18: state=inbox returned active and inbox, state=archive returned everything. Each filter now returns exactly that state, narrowing a wider candidate pool so a match below the first page is not silently dropped. §10 drift: "edit & approve" turned an intentionally emptied body into None and approved the old text; history compared one snapshot to current instead of any two revisions; browse could not sort by tags; the health page showed no audit source and had no run-eval action. The eval action writes to the data volume, not the working directory, which readOnlyRootFilesystem makes unwritable. Route-level OIDC coverage is new: login/callback/logout, PKCE verifier against its challenge, nonce carriage, forged and missing state, cookie flags, open redirect confinement and provider-down handling. The session test named "round trip and expiry" now actually advances the clock. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0139vHnNpS2sLNELjrAHX5Cu
This commit is contained in:
1 parent
6477a7a862
commit
862113b482
8 files changed
+873
-34
No files matched your search
+189
-17
@@ -11,15 +11,24 @@ import difflib
|
||||
import json
|
||||
import logging
|
||||
import secrets
|
||||
import time
|
||||
from pathlib import Path
|
||||
from types import SimpleNamespace
|
||||
from typing import Any
|
||||
from urllib.parse import urlencode, urlparse
|
||||
|
||||
from fastapi import FastAPI, Form, Query, Request
|
||||
from fastapi import BackgroundTasks, FastAPI, Form, Query, Request
|
||||
from fastapi.responses import HTMLResponse, JSONResponse, PlainTextResponse, RedirectResponse, Response
|
||||
from fastapi.staticfiles import StaticFiles
|
||||
from fastapi.templating import Jinja2Templates
|
||||
from prometheus_client import CONTENT_TYPE_LATEST, CollectorRegistry, Gauge, generate_latest
|
||||
from prometheus_client import (
|
||||
CONTENT_TYPE_LATEST,
|
||||
CollectorRegistry,
|
||||
Counter,
|
||||
Gauge,
|
||||
Histogram,
|
||||
generate_latest,
|
||||
)
|
||||
|
||||
from mdmem.errors import MdmemError, VersionConflict
|
||||
from mdmem.notes import parse as parse_note
|
||||
@@ -40,6 +49,7 @@ from mdmem.ui.render import render_markdown
|
||||
log = logging.getLogger("mdmem.ui")
|
||||
|
||||
HERE = Path(__file__).parent
|
||||
SEARCH_PAGE_K = 20
|
||||
CSP = (
|
||||
"default-src 'self'; img-src 'self' data:; style-src 'self' 'unsafe-inline'; "
|
||||
"script-src 'self'; frame-ancestors 'none'; base-uri 'none'; form-action 'self'"
|
||||
@@ -72,15 +82,48 @@ def build_ui_app(svc: MemoryService) -> FastAPI: # noqa: C901 - one place, many
|
||||
app = FastAPI(title="mdmem", docs_url=None, redoc_url=None, openapi_url=None)
|
||||
app.mount("/ui/static", StaticFiles(directory=str(HERE / "static")), name="static")
|
||||
|
||||
@app.on_event("shutdown")
|
||||
async def close_oidc() -> None:
|
||||
await oidc.aclose()
|
||||
|
||||
registry = CollectorRegistry()
|
||||
g_notes = Gauge("mdmem_notes", "notes by lifecycle state", ["state"], registry=registry)
|
||||
g_chunks = Gauge("mdmem_chunks", "indexed chunks", registry=registry)
|
||||
g_pending = Gauge("mdmem_vec_pending", "chunks waiting for an embedding", registry=registry)
|
||||
g_embed_ok = Gauge("mdmem_embed_ok", "1 when the embedding backend answered", registry=registry)
|
||||
g_embed_ms = Gauge("mdmem_embed_last_ms", "latency of the last embedding call", registry=registry)
|
||||
g_searches = Gauge("mdmem_searches_total", "searches served", registry=registry)
|
||||
g_writes = Gauge("mdmem_writes_total", "notes written", registry=registry)
|
||||
c_searches = Counter("mdmem_searches_total", "searches served", registry=registry)
|
||||
c_writes = Counter("mdmem_writes_total", "notes written", registry=registry)
|
||||
g_unresolved = Gauge("mdmem_unresolved_links", "wikilinks with no target", registry=registry)
|
||||
c_embed_errors = Counter("mdmem_embed_errors_total", "embedding calls that failed", registry=registry)
|
||||
h_search_latency = Histogram(
|
||||
"mdmem_search_latency_seconds",
|
||||
"latency of a UI search request",
|
||||
registry=registry,
|
||||
buckets=(0.01, 0.025, 0.05, 0.1, 0.25, 0.5, 1, 2.5, 5, 10),
|
||||
)
|
||||
# tool is one of a fixed, small set of MCP tool names (never a note id or
|
||||
# query string) and outcome is one of {"ok", "error"} — bounded cardinality
|
||||
# by construction (§13a.7).
|
||||
c_mcp_calls = Counter(
|
||||
"mdmem_mcp_calls_total",
|
||||
"MCP tool calls by tool and outcome",
|
||||
["tool", "outcome"],
|
||||
registry=registry,
|
||||
)
|
||||
# prometheus_client Counters only support .inc(); the service and the audit
|
||||
# log hand us plain running totals (they never decrease within a process
|
||||
# lifetime), so each scrape advances the exported counter by the observed
|
||||
# delta instead of ever setting it backwards.
|
||||
_last_totals = {"search": 0, "writes": 0, "embed_errors": 0}
|
||||
_last_mcp_counts: dict[tuple[str, str], int] = {}
|
||||
|
||||
def _advance(counter: Counter, key: str, current: Any) -> None:
|
||||
current = int(current or 0)
|
||||
delta = current - _last_totals[key]
|
||||
if delta > 0:
|
||||
counter.inc(delta)
|
||||
_last_totals[key] = max(_last_totals[key], current)
|
||||
|
||||
# -- middleware --------------------------------------------------------
|
||||
@app.middleware("http")
|
||||
@@ -147,9 +190,30 @@ def build_ui_app(svc: MemoryService) -> FastAPI: # noqa: C901 - one place, many
|
||||
g_pending.set(h.vec_pending)
|
||||
g_embed_ok.set(1 if h.embed_ok else 0)
|
||||
g_embed_ms.set(h.embed_last_ms or 0)
|
||||
g_searches.set(svc.metrics["search"])
|
||||
g_writes.set(svc.metrics["writes"])
|
||||
g_unresolved.set(h.unresolved_links)
|
||||
_advance(c_searches, "search", svc.metrics["search"])
|
||||
_advance(c_writes, "writes", svc.metrics["writes"])
|
||||
_advance(c_embed_errors, "embed_errors", h.embed_errors)
|
||||
|
||||
# MCP calls by tool and outcome, sourced from the audit log — never
|
||||
# note bodies, titles or query strings (§13a.7). The audit window is
|
||||
# bounded, so a call that scrolls out of it simply stops contributing
|
||||
# further deltas; the exported counter still never goes backwards.
|
||||
window_counts: dict[tuple[str, str], int] = {}
|
||||
for entry in svc.audit.recent(1000):
|
||||
if entry.get("source") != "mcp":
|
||||
continue
|
||||
tool = str(entry.get("tool") or "unknown")[:64]
|
||||
outcome = "ok" if entry.get("ok") else "error"
|
||||
key = (tool, outcome)
|
||||
window_counts[key] = window_counts.get(key, 0) + 1
|
||||
for key, current in window_counts.items():
|
||||
prev = _last_mcp_counts.get(key, 0)
|
||||
delta = current - prev
|
||||
if delta > 0:
|
||||
c_mcp_calls.labels(tool=key[0], outcome=key[1]).inc(delta)
|
||||
_last_mcp_counts[key] = max(prev, current)
|
||||
|
||||
return Response(generate_latest(registry), media_type=CONTENT_TYPE_LATEST)
|
||||
|
||||
# -- auth --------------------------------------------------------------
|
||||
@@ -241,9 +305,18 @@ def build_ui_app(svc: MemoryService) -> FastAPI: # noqa: C901 - one place, many
|
||||
return templates.TemplateResponse(request, "inbox.html", _ctx(request, items=items))
|
||||
|
||||
@app.post("/ui/n/{note_id}/approve")
|
||||
async def approve(request: Request, note_id: str, body: str = Form(default="")) -> Response:
|
||||
async def approve(request: Request, note_id: str) -> Response:
|
||||
# The plain "Übernehmen" form submits no `body` field at all — approve
|
||||
# as-is. The "Bearbeiten & übernehmen" form always submits one, even
|
||||
# when the textarea was cleared, and an intentionally emptied body
|
||||
# must be saved as empty rather than silently falling back to the old
|
||||
# body (lower-severity plan drift, §10). `Form(default="")` can't tell
|
||||
# "field absent" from "field present but empty", so read the raw form.
|
||||
form = await request.form()
|
||||
raw_body = form.get("body")
|
||||
edited_body = str(raw_body) if raw_body is not None else None
|
||||
try:
|
||||
await svc.approve(note_id, edited_body=body or None)
|
||||
await svc.approve(note_id, edited_body=edited_body)
|
||||
except MdmemError as exc:
|
||||
return _error(request, templates, exc)
|
||||
return RedirectResponse("/ui/inbox", status_code=303)
|
||||
@@ -277,9 +350,17 @@ def build_ui_app(svc: MemoryService) -> FastAPI: # noqa: C901 - one place, many
|
||||
) -> Response:
|
||||
resp = None
|
||||
if q.strip():
|
||||
# search() always keeps active-state rows and only ever *adds*
|
||||
# inbox/archive on top of that, so a single-state filter (as opposed
|
||||
# to the "inbox + active" and "everything" presets) has to be applied
|
||||
# to the results (M-18). Ask for more candidates first: filtering a
|
||||
# 20-row page down would otherwise show an empty result whenever the
|
||||
# top 20 happen to be in another state.
|
||||
narrowing = state in ("inbox", "active", "archive")
|
||||
started = time.monotonic()
|
||||
resp = await svc.search(
|
||||
q,
|
||||
k=20,
|
||||
k=SEARCH_PAGE_K * 5 if narrowing else SEARCH_PAGE_K,
|
||||
types=[type] if type else None,
|
||||
tags=[tag] if tag else None,
|
||||
since=since or None,
|
||||
@@ -290,6 +371,11 @@ def build_ui_app(svc: MemoryService) -> FastAPI: # noqa: C901 - one place, many
|
||||
debug=True,
|
||||
max_chars=100_000,
|
||||
)
|
||||
h_search_latency.observe(time.monotonic() - started)
|
||||
if narrowing:
|
||||
kept = [r for r in resp.results if r.state == state]
|
||||
resp.truncated = resp.truncated or len(kept) > SEARCH_PAGE_K
|
||||
resp.results = kept[:SEARCH_PAGE_K]
|
||||
return templates.TemplateResponse(
|
||||
request,
|
||||
"search.html",
|
||||
@@ -318,11 +404,25 @@ def build_ui_app(svc: MemoryService) -> FastAPI: # noqa: C901 - one place, many
|
||||
request, "note.html", _ctx(request, note=note, history=history, resolve=resolve)
|
||||
)
|
||||
|
||||
def _revision_text(note_id: str, stamp: str) -> tuple[str, str]:
|
||||
"""Read one side of a history compare. "current" means the live file."""
|
||||
if stamp == "current":
|
||||
return svc.get_note(note_id).to_text(), "jetzt"
|
||||
return svc.vault.read_history(note_id, stamp), stamp
|
||||
|
||||
@app.get("/ui/n/{note_id}/history/{stamp}", response_class=HTMLResponse)
|
||||
async def history_diff(request: Request, note_id: str, stamp: str) -> Response:
|
||||
async def history_diff(
|
||||
request: Request, note_id: str, stamp: str, other: str = Query(default="current")
|
||||
) -> Response:
|
||||
"""Diff `stamp` against `other` — any two revisions, not just vs. current (§10).
|
||||
|
||||
`other` defaults to the literal "current" for backward compatibility
|
||||
with the per-revision "compare to now" buttons; any other value is
|
||||
read as a second history stamp.
|
||||
"""
|
||||
try:
|
||||
old = svc.vault.read_history(note_id, stamp)
|
||||
current = svc.get_note(note_id).to_text()
|
||||
old, _ = _revision_text(note_id, stamp)
|
||||
new, other_label = _revision_text(note_id, other)
|
||||
except Exception as exc:
|
||||
# A note can have history but no current file (unlinked outside
|
||||
# hard_delete); svc.get_note then raises NotFound after the path
|
||||
@@ -331,7 +431,25 @@ def build_ui_app(svc: MemoryService) -> FastAPI: # noqa: C901 - one place, many
|
||||
return templates.TemplateResponse(
|
||||
request,
|
||||
"partial_diff.html",
|
||||
{"request": request, "diff": _diff(old, current, f"{stamp} → jetzt")},
|
||||
{"request": request, "diff": _diff(old, new, f"{stamp} → {other_label}")},
|
||||
)
|
||||
|
||||
@app.get("/ui/n/{note_id}/history-diff", response_class=HTMLResponse)
|
||||
async def history_diff_pair(
|
||||
request: Request, note_id: str, a: str = Query(...), b: str = Query(default="current")
|
||||
) -> Response:
|
||||
"""Same diff as `history_diff`, driven by the note page's two-revision
|
||||
compare form (a plain htmx GET form, so both sides come in as query
|
||||
params rather than one being a path segment)."""
|
||||
try:
|
||||
old, a_label = _revision_text(note_id, a)
|
||||
new, b_label = _revision_text(note_id, b)
|
||||
except Exception as exc:
|
||||
return PlainTextResponse(str(exc), status_code=404)
|
||||
return templates.TemplateResponse(
|
||||
request,
|
||||
"partial_diff.html",
|
||||
{"request": request, "diff": _diff(old, new, f"{a_label} → {b_label}")},
|
||||
)
|
||||
|
||||
@app.get("/ui/n/{note_id}/edit", response_class=HTMLResponse)
|
||||
@@ -414,9 +532,14 @@ def build_ui_app(svc: MemoryService) -> FastAPI: # noqa: C901 - one place, many
|
||||
order: str = Query(default="updated"),
|
||||
tag: str = Query(default=""),
|
||||
) -> Response:
|
||||
rows = svc.list_notes(state=state or None, limit=2000, order=order)
|
||||
# "tags" isn't a sortable SQL column (tags are stored as a JSON blob),
|
||||
# so fetch in the service's default order and sort by tags here (§10).
|
||||
sql_order = order if order != "tags" else "updated"
|
||||
rows = svc.list_notes(state=state or None, limit=2000, order=sql_order)
|
||||
if tag:
|
||||
rows = [r for r in rows if tag in r["tags"]]
|
||||
if order == "tags":
|
||||
rows = sorted(rows, key=lambda r: ", ".join(sorted(r["tags"], key=str.lower)))
|
||||
cloud: dict[str, int] = {}
|
||||
for r in svc.list_notes(limit=5000):
|
||||
for t in r["tags"]:
|
||||
@@ -435,7 +558,7 @@ def build_ui_app(svc: MemoryService) -> FastAPI: # noqa: C901 - one place, many
|
||||
)
|
||||
|
||||
@app.get("/ui/health", response_class=HTMLResponse)
|
||||
async def health_page(request: Request) -> Response:
|
||||
async def health_page(request: Request, eval: str = Query(default="")) -> Response:
|
||||
return templates.TemplateResponse(
|
||||
request,
|
||||
"health.html",
|
||||
@@ -445,6 +568,7 @@ def build_ui_app(svc: MemoryService) -> FastAPI: # noqa: C901 - one place, many
|
||||
unresolved=svc.unresolved_links()[:100],
|
||||
calls=svc.audit.recent(100),
|
||||
settings=settings,
|
||||
eval_status=eval[:300],
|
||||
),
|
||||
)
|
||||
|
||||
@@ -458,6 +582,44 @@ def build_ui_app(svc: MemoryService) -> FastAPI: # noqa: C901 - one place, many
|
||||
await svc.reembed_all()
|
||||
return RedirectResponse("/ui/health", status_code=303)
|
||||
|
||||
@app.post("/ui/health/eval")
|
||||
async def run_eval(request: Request, background_tasks: BackgroundTasks) -> Response:
|
||||
"""Kick off the eval harness (§10) without blocking the request.
|
||||
|
||||
`run_eval_cli` drives its own `asyncio.run(...)`, so it cannot be
|
||||
awaited from inside this request's event loop. FastAPI's
|
||||
BackgroundTasks runs a plain (non-async) callable in a worker thread
|
||||
via `run_in_threadpool`, which gives it a loop-free thread to call
|
||||
`asyncio.run` from and keeps the response fast.
|
||||
|
||||
Both the query set and the report live on the data volume, not in the
|
||||
working directory: the pod runs with `readOnlyRootFilesystem: true`
|
||||
(§13a.3), so a relative `eval/results/...` path would fail at the write
|
||||
with nothing but a log line to show for it.
|
||||
"""
|
||||
from mdmem.evalharness import run_eval_cli
|
||||
|
||||
data_dir = settings.index_db.parent
|
||||
queries = data_dir / "eval" / "queries.yaml"
|
||||
if not queries.exists():
|
||||
# §11: a run needs 50-100 labelled queries. Say so on the page
|
||||
# instead of starting a job that prints a hint into the log and
|
||||
# exits 2.
|
||||
return RedirectResponse(f"/ui/health?eval=missing:{queries}", status_code=303)
|
||||
out = data_dir / "eval" / f"{time.strftime('%Y-%m-%dT%H-%M-%SZ', time.gmtime())}.json"
|
||||
args = SimpleNamespace(
|
||||
queries=str(queries), config="ui", out=str(out), ablate=False, fake_embeddings=False
|
||||
)
|
||||
|
||||
def _run() -> None:
|
||||
try:
|
||||
run_eval_cli(settings, args)
|
||||
except Exception:
|
||||
log.exception("background eval run failed")
|
||||
|
||||
background_tasks.add_task(_run)
|
||||
return RedirectResponse(f"/ui/health?eval=started:{out.name}", status_code=303)
|
||||
|
||||
return app
|
||||
|
||||
|
||||
@@ -472,15 +634,25 @@ def _origin_from_referer(request: Request) -> str | None:
|
||||
|
||||
|
||||
def _origin_allowed(origin: str, settings: Any, request: Request) -> bool:
|
||||
parsed_origin = urlparse(origin)
|
||||
if not parsed_origin.scheme or not parsed_origin.netloc:
|
||||
return False
|
||||
host = _request_host(request)
|
||||
if host != parsed_origin.netloc:
|
||||
return False
|
||||
allowed = set(settings.ui_allowed_origins)
|
||||
if settings.ui_base_url:
|
||||
allowed.add(settings.ui_base_url)
|
||||
if not allowed: # not configured: fall back to same-host
|
||||
host = request.headers.get("host", "")
|
||||
return urlparse(origin).netloc == host
|
||||
return True
|
||||
return origin.rstrip("/") in {a.rstrip("/") for a in allowed}
|
||||
|
||||
|
||||
def _request_host(request: Request) -> str:
|
||||
forwarded = request.headers.get("x-forwarded-host", "").split(",", 1)[0].strip()
|
||||
return forwarded or request.headers.get("host", "")
|
||||
|
||||
|
||||
def _diff(old: str, new: str, label: str) -> str:
|
||||
return "\n".join(
|
||||
difflib.unified_diff(
|
||||
|
||||
+46
-8
@@ -10,6 +10,7 @@ from __future__ import annotations
|
||||
import base64
|
||||
import hashlib
|
||||
import logging
|
||||
import math
|
||||
import secrets
|
||||
import time
|
||||
from dataclasses import dataclass
|
||||
@@ -61,6 +62,8 @@ def authorize_claims(claims: dict[str, Any], settings: Settings) -> Principal:
|
||||
rejections — never 'the token is valid, so allow' (§14.21).
|
||||
"""
|
||||
sub = str(claims.get("sub") or "")
|
||||
if not sub:
|
||||
raise AuthError("id_token subject is required")
|
||||
claim_name = settings.oidc_groups_claim
|
||||
required = settings.oidc_required_group
|
||||
seen = sorted(claims.keys())
|
||||
@@ -75,8 +78,6 @@ def authorize_claims(claims: dict[str, Any], settings: Settings) -> Principal:
|
||||
raise AuthError(f"the {claim_name!r} claim is absent — is the groups scope configured?")
|
||||
|
||||
groups = claims.get(claim_name)
|
||||
if isinstance(groups, str):
|
||||
groups = [groups]
|
||||
if not isinstance(groups, list) or not all(isinstance(g, str) for g in groups):
|
||||
log.warning(
|
||||
"login rejected: group claim has the wrong type",
|
||||
@@ -160,8 +161,8 @@ class OidcClient:
|
||||
self._meta = r.json()
|
||||
return self._meta
|
||||
|
||||
async def jwks(self) -> dict[str, Any]:
|
||||
if self._jwks is None:
|
||||
async def jwks(self, *, refresh: bool = False) -> dict[str, Any]:
|
||||
if self._jwks is None or refresh:
|
||||
meta = await self.metadata()
|
||||
r = await self._client.get(meta["jwks_uri"])
|
||||
r.raise_for_status()
|
||||
@@ -212,16 +213,31 @@ class OidcClient:
|
||||
id_token = token_response.get("id_token")
|
||||
if not id_token:
|
||||
raise AuthError("the token response carried no id_token", status=502)
|
||||
keys = JsonWebKey.import_key_set(await self.jwks())
|
||||
jwt = JsonWebToken(["RS256", "ES256"])
|
||||
claims = jwt.decode(id_token, keys)
|
||||
header = _jwt_header(id_token)
|
||||
jwks = await self.jwks()
|
||||
refreshed = False
|
||||
if isinstance(header.get("kid"), str) and not _jwks_has_kid(jwks, header["kid"]):
|
||||
jwks = await self.jwks(refresh=True)
|
||||
refreshed = True
|
||||
keys = JsonWebKey.import_key_set(jwks)
|
||||
try:
|
||||
claims = jwt.decode(id_token, keys)
|
||||
except Exception:
|
||||
if refreshed:
|
||||
raise
|
||||
keys = JsonWebKey.import_key_set(await self.jwks(refresh=True))
|
||||
claims = jwt.decode(id_token, keys)
|
||||
data = dict(claims)
|
||||
exp = data.get("exp")
|
||||
if not isinstance(exp, int | float):
|
||||
if not isinstance(exp, int | float) or not math.isfinite(exp):
|
||||
raise AuthError("id_token has no valid exp claim")
|
||||
iat = data.get("iat")
|
||||
if not isinstance(iat, int | float):
|
||||
if not isinstance(iat, int | float) or not math.isfinite(iat):
|
||||
raise AuthError("id_token has no valid iat claim")
|
||||
sub = data.get("sub")
|
||||
if not isinstance(sub, str) or not sub:
|
||||
raise AuthError("id_token subject is required")
|
||||
if iat > time.time() + IAT_MAX_FUTURE_SKEW_S:
|
||||
raise AuthError("id_token iat is implausibly far in the future")
|
||||
# Our own iat check above must run before authlib's, and with a leeway
|
||||
@@ -238,7 +254,29 @@ class OidcClient:
|
||||
auds = aud if isinstance(aud, list) else [aud]
|
||||
if self.settings.oidc_client_id not in auds:
|
||||
raise AuthError("id_token audience mismatch")
|
||||
if len(auds) > 1:
|
||||
azp = data.get("azp")
|
||||
if not isinstance(azp, str) or azp != self.settings.oidc_client_id:
|
||||
raise AuthError("id_token authorized party mismatch")
|
||||
return data
|
||||
|
||||
async def aclose(self) -> None:
|
||||
await self._client.aclose()
|
||||
|
||||
|
||||
def _jwt_header(token: str) -> dict[str, Any]:
|
||||
try:
|
||||
raw = token.split(".", 1)[0]
|
||||
raw += "=" * (-len(raw) % 4)
|
||||
data = base64.urlsafe_b64decode(raw.encode())
|
||||
import json
|
||||
|
||||
parsed = json.loads(data)
|
||||
except Exception:
|
||||
return {}
|
||||
return parsed if isinstance(parsed, dict) else {}
|
||||
|
||||
|
||||
def _jwks_has_kid(jwks: dict[str, Any], kid: str) -> bool:
|
||||
keys = jwks.get("keys")
|
||||
return isinstance(keys, list) and any(isinstance(k, dict) and k.get("kid") == kid for k in keys)
|
||||
@@ -16,7 +16,7 @@
|
||||
<th><a href="/ui/browse?order=title&state={{ state }}&tag={{ tag }}">Titel</a></th>
|
||||
<th><a href="/ui/browse?order=type&state={{ state }}&tag={{ tag }}">Typ</a></th>
|
||||
<th><a href="/ui/browse?order=state&state={{ state }}&tag={{ tag }}">Zustand</a></th>
|
||||
<th>Tags</th>
|
||||
<th><a href="/ui/browse?order=tags&state={{ state }}&tag={{ tag }}">Tags</a></th>
|
||||
<th><a href="/ui/browse?order=updated&state={{ state }}&tag={{ tag }}">Aktualisiert</a></th>
|
||||
</tr></thead>
|
||||
<tbody>
|
||||
|
||||
@@ -41,6 +41,14 @@
|
||||
<form method="post" action="/ui/health/reindex"><button type="submit">Index auffrischen</button></form>
|
||||
<form method="post" action="/ui/health/reindex"><input type="hidden" name="full" value="1"><button type="submit">Index neu bauen</button></form>
|
||||
<form method="post" action="/ui/health/reembed"><button type="submit">Alles neu einbetten</button></form>
|
||||
<form method="post" action="/ui/health/eval"><button type="submit">Eval starten</button></form>
|
||||
</div>
|
||||
<div>
|
||||
{% if eval_status.startswith("started:") %}
|
||||
<p class="ok">Eval gestartet — Bericht: {{ eval_status[8:] }}</p>
|
||||
{% elif eval_status.startswith("missing:") %}
|
||||
<p class="warn">Kein Query-Set unter {{ eval_status[8:] }} — erst 50–100 Queries labeln (§11).</p>
|
||||
{% endif %}
|
||||
</div>
|
||||
|
||||
<h3>Unaufgelöste Links</h3>
|
||||
@@ -48,12 +56,12 @@
|
||||
|
||||
<h3>Letzte MCP-Aufrufe</h3>
|
||||
<table>
|
||||
<thead><tr><th>Zeit</th><th>Tool</th><th>ok</th><th>ms</th><th>ids</th></tr></thead>
|
||||
<thead><tr><th>Zeit</th><th>Tool</th><th>Quelle</th><th>ok</th><th>ms</th><th>ids</th></tr></thead>
|
||||
<tbody>
|
||||
{% for c in calls %}
|
||||
<tr><td>{{ c.ts }}</td><td>{{ c.tool }}</td><td>{{ c.get('ok', '') }}</td><td>{{ c.get('ms', '') }}</td>
|
||||
<tr><td>{{ c.ts }}</td><td>{{ c.tool }}</td><td>{{ c.get('source', '') }}</td><td>{{ c.get('ok', '') }}</td><td>{{ c.get('ms', '') }}</td>
|
||||
<td>{% for i in c.get('ids', []) %}<a href="/ui/n/{{ i }}">{{ i[:8] }}</a> {% endfor %}</td></tr>
|
||||
{% else %}<tr><td colspan="5" class="empty">keine</td></tr>{% endfor %}
|
||||
{% else %}<tr><td colspan="6" class="empty">keine</td></tr>{% endfor %}
|
||||
</tbody>
|
||||
</table>
|
||||
{% endblock %}
|
||||
@@ -45,6 +45,19 @@
|
||||
<div id="diff-{{ loop.index }}"></div></li>
|
||||
{% else %}<li class="empty">keine früheren Fassungen</li>{% endfor %}
|
||||
</ul>
|
||||
{% if history %}
|
||||
<form class="history-compare" hx-get="/ui/n/{{ note.id }}/history-diff" hx-target="#history-compare-diff">
|
||||
<label>von <select name="a">
|
||||
{% for h in history %}<option value="{{ h }}" {% if loop.first %}selected{% endif %}>{{ h }}</option>{% endfor %}
|
||||
</select></label>
|
||||
<label>bis <select name="b">
|
||||
<option value="current" selected>aktuell</option>
|
||||
{% for h in history|reverse %}<option value="{{ h }}">{{ h }}</option>{% endfor %}
|
||||
</select></label>
|
||||
<button type="submit">Vergleichen</button>
|
||||
</form>
|
||||
<div id="history-compare-diff"></div>
|
||||
{% endif %}
|
||||
|
||||
<div class="actions">
|
||||
<a class="button" href="/ui/n/{{ note.id }}/edit">Bearbeiten</a>
|
||||
|
||||
+86
-5
@@ -54,10 +54,8 @@ def test_claim_of_the_wrong_type_is_rejected(auth_settings):
|
||||
authorize_claims({"sub": "u1", "groups": {"a": 1}}, auth_settings)
|
||||
with pytest.raises(AuthError, match="list of strings"):
|
||||
authorize_claims({"sub": "u1", "groups": [1, 2]}, auth_settings)
|
||||
|
||||
|
||||
def test_a_single_string_claim_is_accepted_as_one_group(auth_settings):
|
||||
assert authorize_claims({"sub": "u1", "groups": "mdmem_admins"}, auth_settings).sub == "u1"
|
||||
with pytest.raises(AuthError, match="list of strings"):
|
||||
authorize_claims({"sub": "u1", "groups": "mdmem_admins"}, auth_settings)
|
||||
|
||||
|
||||
def test_unconfigured_required_group_refuses_everything(auth_settings):
|
||||
@@ -75,13 +73,24 @@ def test_rejection_logs_the_sub_and_claim_names_but_not_the_values(auth_settings
|
||||
assert "secret_group_name" not in str(record.fields)
|
||||
|
||||
|
||||
def test_session_cookie_round_trip_and_expiry(auth_settings):
|
||||
def test_session_cookie_round_trip_and_expiry(auth_settings, monkeypatch):
|
||||
import time as _time
|
||||
|
||||
codec = SessionCodec(auth_settings.session_secret, auth_settings.session_ttl_hours)
|
||||
token = codec.issue(Principal(sub="u1", name="M", email="m@x"))
|
||||
assert codec.read(token).sub == "u1"
|
||||
assert codec.read("garbage") is None
|
||||
assert codec.read(None) is None
|
||||
|
||||
# The name promised expiry but nothing ever advanced the clock: a session
|
||||
# that outlives SESSION_TTL_HOURS must stop being accepted.
|
||||
real_time = _time.time
|
||||
ttl = auth_settings.session_ttl_hours * 3600
|
||||
monkeypatch.setattr("itsdangerous.timed.time.time", lambda: real_time() + ttl - 60)
|
||||
assert codec.read(token).sub == "u1", "a session inside its TTL must still be valid"
|
||||
monkeypatch.setattr("itsdangerous.timed.time.time", lambda: real_time() + ttl + 60)
|
||||
assert codec.read(token) is None, "a session past its TTL must be refused"
|
||||
|
||||
|
||||
def test_a_session_signed_with_another_secret_is_refused(auth_settings):
|
||||
other = SessionCodec("y" * 32, 12).issue(Principal(sub="u1", name="M", email=""))
|
||||
@@ -121,6 +130,23 @@ async def test_id_token_requires_exp_and_matching_nonce(auth_settings):
|
||||
try:
|
||||
with pytest.raises(AuthError, match="exp"):
|
||||
await oidc.claims({"id_token": token()}, "expected")
|
||||
with pytest.raises(AuthError, match="exp"):
|
||||
await oidc.claims({"id_token": token(exp=float("nan"))}, "expected")
|
||||
with pytest.raises(AuthError, match="exp"):
|
||||
await oidc.claims({"id_token": token(exp=float("inf"))}, "expected")
|
||||
with pytest.raises(AuthError, match="subject"):
|
||||
await oidc.claims({"id_token": token(exp=int(time.time()) + 300, sub="")}, "expected")
|
||||
with pytest.raises(AuthError, match="authorized party"):
|
||||
await oidc.claims(
|
||||
{
|
||||
"id_token": token(
|
||||
exp=int(time.time()) + 300,
|
||||
aud=[auth_settings.oidc_client_id, "other-client"],
|
||||
azp="other-client",
|
||||
)
|
||||
},
|
||||
"expected",
|
||||
)
|
||||
with pytest.raises(AuthError, match="nonce"):
|
||||
await oidc.claims({"id_token": token(exp=int(time.time()) + 300, nonce="wrong")}, "expected")
|
||||
accepted = await oidc.claims({"id_token": token(exp=int(time.time()) + 300)}, "expected")
|
||||
@@ -165,6 +191,43 @@ async def test_id_token_requires_iat(auth_settings):
|
||||
await oidc.aclose()
|
||||
|
||||
|
||||
async def test_unknown_kid_forces_one_jwks_refresh(auth_settings):
|
||||
import time
|
||||
|
||||
from authlib.jose import JsonWebKey, JsonWebToken
|
||||
|
||||
old_key = JsonWebKey.generate_key("RSA", 2048, is_private=True, options={"kid": "old"})
|
||||
new_key = JsonWebKey.generate_key("RSA", 2048, is_private=True, options={"kid": "new"})
|
||||
oidc = OidcClient(auth_settings)
|
||||
jwt = JsonWebToken(["RS256"])
|
||||
token = jwt.encode(
|
||||
{"alg": "RS256", "kid": "new"},
|
||||
{
|
||||
"iss": auth_settings.oidc_issuer,
|
||||
"aud": auth_settings.oidc_client_id,
|
||||
"sub": "u1",
|
||||
"nonce": "expected",
|
||||
"exp": int(time.time()) + 300,
|
||||
"iat": int(time.time()),
|
||||
},
|
||||
new_key,
|
||||
).decode()
|
||||
calls = []
|
||||
|
||||
async def counted_jwks(*, refresh=False):
|
||||
calls.append(refresh)
|
||||
key = new_key if refresh else old_key
|
||||
return {"keys": [key.as_dict(is_private=False)]}
|
||||
|
||||
oidc.jwks = counted_jwks
|
||||
try:
|
||||
claims = await oidc.claims({"id_token": token}, "expected")
|
||||
finally:
|
||||
await oidc.aclose()
|
||||
assert claims["sub"] == "u1"
|
||||
assert calls == [False, True]
|
||||
|
||||
|
||||
# -- routing ---------------------------------------------------------------
|
||||
@pytest.fixture()
|
||||
async def app_and_svc(auth_settings):
|
||||
@@ -262,6 +325,24 @@ async def test_post_from_the_configured_origin_is_allowed(app_and_svc):
|
||||
assert svc.get_note(res["id"]).state == "active"
|
||||
|
||||
|
||||
async def test_post_with_good_origin_but_bad_host_is_refused(app_and_svc):
|
||||
app, svc = app_and_svc
|
||||
cookie = SessionCodec(svc.settings.session_secret, 12).issue(Principal("u1", "M", ""))
|
||||
res = await svc.write_note(title="T", body="b", type="note")
|
||||
async with httpx.AsyncClient(
|
||||
transport=httpx.ASGITransport(app=app),
|
||||
base_url="https://evil.example",
|
||||
cookies={SESSION_COOKIE: cookie},
|
||||
) as c:
|
||||
r = await c.post(
|
||||
f"/ui/n/{res['id']}/approve",
|
||||
headers={"origin": "https://memory.cloud.traunseenet.com"},
|
||||
data={},
|
||||
)
|
||||
assert r.status_code == 403
|
||||
assert svc.get_note(res["id"]).state == "inbox"
|
||||
|
||||
|
||||
async def test_post_without_origin_or_referer_is_refused(app_and_svc):
|
||||
"""A state-changing POST with neither header must fail closed (§10, §14.21).
|
||||
|
||||
|
||||
@@ -266,3 +266,213 @@ async def test_unknown_note_returns_404(ui):
|
||||
client, _ = ui
|
||||
r = await client.get("/ui/n/01JZZZZZZZZZZZZZZZZZZZZZZZ")
|
||||
assert r.status_code == 404 and "NOT_FOUND" in r.text
|
||||
|
||||
|
||||
# -- M-17: required Prometheus signals ---------------------------------------
|
||||
|
||||
|
||||
async def test_metrics_exports_required_families_with_bounded_labels(ui):
|
||||
client, svc = ui
|
||||
note = await svc.write_note(title="Geheimer Titel", body="streng geheimer Inhalt", type="fact")
|
||||
await svc.approve(note["id"])
|
||||
await client.get("/ui/search?q=Geheimer")
|
||||
svc.audit.record(tool="memory_search", args="deadbeef", source="mcp", ok=True, ids=[note["id"]], ms=1.2)
|
||||
svc.audit.record(tool="memory_write", args="c0ffee", source="mcp", ok=False, error="INVALID", ms=0.5)
|
||||
|
||||
r = await client.get("/metrics")
|
||||
assert r.status_code == 200
|
||||
body = r.text
|
||||
|
||||
for line in (
|
||||
"# TYPE mdmem_notes gauge",
|
||||
"# TYPE mdmem_searches_total counter",
|
||||
"# TYPE mdmem_writes_total counter",
|
||||
"# TYPE mdmem_embed_errors_total counter",
|
||||
"# TYPE mdmem_search_latency_seconds histogram",
|
||||
"# TYPE mdmem_mcp_calls_total counter",
|
||||
):
|
||||
assert line in body, line
|
||||
|
||||
assert "mdmem_search_latency_seconds_count 1.0" in body
|
||||
assert 'mdmem_mcp_calls_total{outcome="ok",tool="memory_search"} 1.0' in body
|
||||
assert 'mdmem_mcp_calls_total{outcome="error",tool="memory_write"} 1.0' in body
|
||||
# bounded cardinality: exactly the (tool, outcome) pairs recorded above, never a per-note label
|
||||
assert body.count("mdmem_mcp_calls_total{") == 2
|
||||
|
||||
assert "Geheimer Titel" not in body and "streng geheimer Inhalt" not in body
|
||||
assert note["id"] not in body
|
||||
|
||||
|
||||
async def test_metrics_totals_only_ever_advance_across_scrapes(ui):
|
||||
client, svc = ui
|
||||
await client.get("/metrics")
|
||||
await svc.write_note(title="Noch einer", body="x", type="note")
|
||||
body = (await client.get("/metrics")).text
|
||||
assert "mdmem_writes_total 1.0" in body
|
||||
|
||||
|
||||
# -- M-18 (UI half): search state filter exactness ---------------------------
|
||||
|
||||
|
||||
async def test_search_state_filter_returns_exactly_that_lifecycle_state(ui):
|
||||
client, svc = ui
|
||||
await svc.write_note(title="Ampel Inbox", body="Ampel steht", type="note")
|
||||
active_note = await svc.write_note(title="Ampel Aktiv", body="Ampel steht", type="note")
|
||||
await svc.approve(active_note["id"])
|
||||
archive_note = await svc.write_note(title="Ampel Archiv", body="Ampel steht", type="note")
|
||||
await svc.approve(archive_note["id"])
|
||||
await svc.set_state(archive_note["id"], "archive")
|
||||
|
||||
r_inbox = await client.get("/ui/search?q=Ampel&state=inbox")
|
||||
assert "Ampel Inbox" in r_inbox.text
|
||||
assert "Ampel Aktiv" not in r_inbox.text and "Ampel Archiv" not in r_inbox.text
|
||||
|
||||
r_active = await client.get("/ui/search?q=Ampel&state=active")
|
||||
assert "Ampel Aktiv" in r_active.text
|
||||
assert "Ampel Inbox" not in r_active.text and "Ampel Archiv" not in r_active.text
|
||||
|
||||
r_archive = await client.get("/ui/search?q=Ampel&state=archive")
|
||||
assert "Ampel Archiv" in r_archive.text
|
||||
assert "Ampel Inbox" not in r_archive.text and "Ampel Aktiv" not in r_archive.text
|
||||
|
||||
r_default = await client.get("/ui/search?q=Ampel")
|
||||
assert "Ampel Inbox" in r_default.text and "Ampel Aktiv" in r_default.text
|
||||
assert "Ampel Archiv" not in r_default.text
|
||||
|
||||
|
||||
# -- lower-severity plan drift ------------------------------------------------
|
||||
|
||||
|
||||
async def test_edit_and_approve_with_emptied_body_saves_empty_body(ui):
|
||||
client, svc = ui
|
||||
res = await svc.write_note(title="Leerbar", body="ursprünglicher Text", type="note")
|
||||
r = await client.post(f"/ui/n/{res['id']}/approve", data={"body": ""})
|
||||
assert r.status_code == 303
|
||||
note = svc.get_note(res["id"])
|
||||
assert note.state == "active"
|
||||
assert note.body.strip() == ""
|
||||
|
||||
|
||||
async def test_approve_without_a_body_field_keeps_the_original_body(ui):
|
||||
client, svc = ui
|
||||
res = await svc.write_note(title="Unverändert", body="bleibt", type="note")
|
||||
r = await client.post(f"/ui/n/{res['id']}/approve", data={})
|
||||
assert r.status_code == 303
|
||||
assert svc.get_note(res["id"]).body.strip() == "bleibt"
|
||||
|
||||
|
||||
async def test_history_compare_between_any_two_revisions(ui):
|
||||
client, svc = ui
|
||||
res = await svc.write_note(title="Verlaufstest", body="v1", type="note")
|
||||
await svc.edit(res["id"], body="v2", expected_version=svc.get_note(res["id"]).file_hash)
|
||||
await svc.edit(res["id"], body="v3", expected_version=svc.get_note(res["id"]).file_hash)
|
||||
stamps = svc.vault.history(res["id"])
|
||||
assert len(stamps) == 2
|
||||
|
||||
r = await client.get(f"/ui/n/{res['id']}/history/{stamps[0]}?other={stamps[1]}")
|
||||
assert r.status_code == 200
|
||||
assert "-v1" in r.text and "+v2" in r.text
|
||||
|
||||
r2 = await client.get(f"/ui/n/{res['id']}/history-diff?a={stamps[0]}&b={stamps[1]}")
|
||||
assert r2.status_code == 200
|
||||
assert "-v1" in r2.text and "+v2" in r2.text
|
||||
|
||||
page = await client.get(f"/ui/n/{res['id']}")
|
||||
assert 'name="a"' in page.text and 'name="b"' in page.text and "history-diff" in page.text
|
||||
|
||||
|
||||
async def test_browse_sorts_by_tags(ui):
|
||||
client, svc = ui
|
||||
await svc.write_note(title="Zeta Note", body="x", type="note", tags=["zzz"])
|
||||
await svc.write_note(title="Beta Note", body="x", type="note", tags=["aaa"])
|
||||
await svc.write_note(title="Kein Tag", body="x", type="note")
|
||||
r = await client.get("/ui/browse?order=tags")
|
||||
text = r.text
|
||||
assert text.index("Kein Tag") < text.index("Beta Note") < text.index("Zeta Note")
|
||||
|
||||
|
||||
async def test_health_page_shows_the_audit_source_and_eval_action(ui):
|
||||
client, svc = ui
|
||||
svc.audit.record(tool="memory_write", args="deadbeef", source="mcp", ok=True, ids=[], ms=3.0)
|
||||
r = await client.get("/ui/health")
|
||||
assert "mcp" in r.text
|
||||
assert "Eval starten" in r.text
|
||||
|
||||
|
||||
async def test_run_eval_is_auth_and_csrf_protected_like_the_other_health_actions(ui):
|
||||
client, svc = ui
|
||||
ok = await client.post("/ui/health/eval", data={})
|
||||
assert ok.status_code == 303
|
||||
|
||||
app = client._transport.app
|
||||
async with httpx.AsyncClient(
|
||||
transport=httpx.ASGITransport(app=app),
|
||||
base_url="https://memory.test",
|
||||
follow_redirects=False,
|
||||
headers={"origin": "https://memory.test"},
|
||||
) as anon:
|
||||
no_session = await anon.post("/ui/health/eval", data={})
|
||||
assert no_session.status_code == 302 and "/ui/auth/login" in no_session.headers["location"]
|
||||
|
||||
cookie = SessionCodec(svc.settings.session_secret, 12).issue(Principal("u1", "M", ""))
|
||||
async with httpx.AsyncClient(
|
||||
transport=httpx.ASGITransport(app=app),
|
||||
base_url="https://memory.test",
|
||||
cookies={SESSION_COOKIE: cookie},
|
||||
follow_redirects=False,
|
||||
) as bad_origin:
|
||||
refused = await bad_origin.post(
|
||||
"/ui/health/eval", data={}, headers={"origin": "https://evil.example.com"}
|
||||
)
|
||||
assert refused.status_code == 403
|
||||
|
||||
|
||||
async def test_run_eval_keeps_its_files_on_the_data_volume_and_reports_status(ui, monkeypatch):
|
||||
"""The pod runs with readOnlyRootFilesystem, so a relative eval/results path
|
||||
would fail at the write with only a log line. Both sides of the run must sit
|
||||
next to index.db on the mounted volume (§13a.3, §11)."""
|
||||
client, svc = ui
|
||||
data_dir = svc.settings.index_db.parent
|
||||
|
||||
# no labelled query set yet: say so on the page instead of starting a job
|
||||
# that exits 2 into the log
|
||||
missing = await client.post("/ui/health/eval", data={})
|
||||
assert missing.status_code == 303
|
||||
assert "eval=missing" in missing.headers["location"]
|
||||
assert str(data_dir) in missing.headers["location"]
|
||||
|
||||
queries = data_dir / "eval" / "queries.yaml"
|
||||
queries.parent.mkdir(parents=True, exist_ok=True)
|
||||
queries.write_text("- q: irgendwas\n relevant: []\n", encoding="utf-8")
|
||||
|
||||
called: dict[str, object] = {}
|
||||
monkeypatch.setattr(
|
||||
"mdmem.evalharness.run_eval_cli",
|
||||
lambda settings, args: called.setdefault("args", args) and 0,
|
||||
)
|
||||
started = await client.post("/ui/health/eval", data={})
|
||||
assert started.status_code == 303
|
||||
assert "eval=started" in started.headers["location"]
|
||||
|
||||
args = called["args"]
|
||||
assert args.queries == str(queries)
|
||||
assert args.out is not None and args.out.startswith(str(data_dir))
|
||||
|
||||
# and the health page renders the status rather than swallowing it
|
||||
page = await client.get("/ui/health", params={"eval": f"started:{data_dir.name}.json"})
|
||||
assert page.status_code == 200 and "Eval gestartet" in page.text
|
||||
|
||||
|
||||
async def test_search_state_filter_still_finds_matches_below_the_first_page(ui):
|
||||
"""Filtering a 20-row page down to one lifecycle state would hide a matching
|
||||
inbox note whenever 20 active notes outrank it — the filter has to narrow a
|
||||
wider candidate pool, not the page (M-18)."""
|
||||
client, svc = ui
|
||||
for i in range(25):
|
||||
w = await svc.write_note(title=f"Bifrost Notiz {i}", body="Bifrost routet Modelle", type="fact")
|
||||
await svc.approve(w["id"])
|
||||
needle = await svc.write_note(title="Bifrost Posteingang", body="Bifrost routet Modelle", type="fact")
|
||||
|
||||
r = await client.get("/ui/search", params={"q": "Bifrost routet Modelle", "state": "inbox"})
|
||||
assert r.status_code == 200
|
||||
assert needle["id"] in r.text
|
||||
@@ -0,0 +1,317 @@
|
||||
"""Route-level OIDC login flow (PLAN §10): discovery, state, PKCE, nonce, cookies, logout.
|
||||
|
||||
The claim-validation rules are covered in test_ui_auth.py; what was missing was any
|
||||
test that drives the actual /ui/auth/* routes, so an error in how the routes carry
|
||||
state, verifier and nonce — or in the cookie flags — went unnoticed.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import base64
|
||||
import hashlib
|
||||
from dataclasses import replace
|
||||
from urllib.parse import parse_qs, urlparse
|
||||
|
||||
import httpx
|
||||
import pytest
|
||||
|
||||
from mdmem.config import Settings
|
||||
from mdmem.embed import FakeEmbedder
|
||||
from mdmem.service import MemoryService
|
||||
from mdmem.ui import app as ui_app
|
||||
from mdmem.ui.auth import FLOW_COOKIE, SESSION_COOKIE, AuthError, Principal, SessionCodec
|
||||
|
||||
BASE = "https://memory.cloud.traunseenet.com"
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
def auth_settings(settings) -> Settings:
|
||||
return replace(
|
||||
settings,
|
||||
session_secret="x" * 32,
|
||||
oidc_issuer="https://pocketid.example",
|
||||
oidc_client_id="client-uuid",
|
||||
oidc_groups_claim="groups",
|
||||
oidc_required_group="mdmem_admins",
|
||||
ui_base_url=BASE,
|
||||
ui_allowed_origins=(BASE,),
|
||||
)
|
||||
|
||||
|
||||
class FakeOidc:
|
||||
"""Stands in for OidcClient: records what the routes hand it."""
|
||||
|
||||
def __init__(self, settings: Settings) -> None:
|
||||
self.settings = settings
|
||||
self.calls: dict[str, object] = {}
|
||||
self.authorize_error: Exception | None = None
|
||||
self.exchange_error: Exception | None = None
|
||||
self.claims_error: Exception | None = None
|
||||
self.claims_payload: dict[str, object] = {
|
||||
"sub": "u1",
|
||||
"name": "Markus",
|
||||
"email": "m@x",
|
||||
"groups": ["mdmem_admins"],
|
||||
}
|
||||
self.closed = False
|
||||
|
||||
async def authorize_url(self, state: str, challenge: str, nonce: str) -> str:
|
||||
if self.authorize_error:
|
||||
raise self.authorize_error
|
||||
self.calls["authorize"] = {"state": state, "challenge": challenge, "nonce": nonce}
|
||||
return f"https://pocketid.example/authorize?state={state}&code_challenge={challenge}&nonce={nonce}"
|
||||
|
||||
async def exchange(self, code: str, verifier: str) -> dict[str, object]:
|
||||
if self.exchange_error:
|
||||
raise self.exchange_error
|
||||
self.calls["exchange"] = {"code": code, "verifier": verifier}
|
||||
return {"id_token": "signed.id.token"}
|
||||
|
||||
async def claims(self, token_response: dict[str, object], expected_nonce: str) -> dict[str, object]:
|
||||
if self.claims_error:
|
||||
raise self.claims_error
|
||||
self.calls["claims"] = {"tokens": token_response, "nonce": expected_nonce}
|
||||
return dict(self.claims_payload)
|
||||
|
||||
async def aclose(self) -> None:
|
||||
self.closed = True
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
async def app_oidc(auth_settings, monkeypatch):
|
||||
fake: dict[str, FakeOidc] = {}
|
||||
|
||||
def _build(settings: Settings) -> FakeOidc:
|
||||
fake["client"] = FakeOidc(settings)
|
||||
return fake["client"]
|
||||
|
||||
monkeypatch.setattr(ui_app, "OidcClient", _build)
|
||||
svc = MemoryService.build(auth_settings, embedder=FakeEmbedder(auth_settings.embed_dim))
|
||||
await svc.start(watch=False)
|
||||
yield ui_app.build_ui_app(svc), fake["client"], svc
|
||||
await svc.stop()
|
||||
|
||||
|
||||
def _client(app, cookies: dict[str, str] | None = None) -> httpx.AsyncClient:
|
||||
return httpx.AsyncClient(
|
||||
transport=httpx.ASGITransport(app=app),
|
||||
base_url=BASE,
|
||||
cookies=cookies or None,
|
||||
)
|
||||
|
||||
|
||||
def _cookie_attrs(response: httpx.Response, name: str) -> str:
|
||||
for raw in response.headers.get_list("set-cookie"):
|
||||
if raw.startswith(f"{name}="):
|
||||
return raw
|
||||
raise AssertionError(f"{name} cookie was not set: {response.headers.get_list('set-cookie')}")
|
||||
|
||||
|
||||
async def _login(client: httpx.AsyncClient, next_url: str = "/ui/browse") -> httpx.Response:
|
||||
return await client.get("/ui/auth/login", params={"next": next_url})
|
||||
|
||||
|
||||
# -- login -------------------------------------------------------------------
|
||||
async def test_login_redirects_to_the_provider_and_carries_state_nonce_and_pkce(app_oidc):
|
||||
app, oidc, _ = app_oidc
|
||||
async with _client(app) as c:
|
||||
r = await _login(c)
|
||||
|
||||
assert r.status_code == 302
|
||||
sent = oidc.calls["authorize"]
|
||||
query = parse_qs(urlparse(r.headers["location"]).query)
|
||||
assert query["state"] == [sent["state"]]
|
||||
assert query["nonce"] == [sent["nonce"]]
|
||||
assert query["code_challenge"] == [sent["challenge"]]
|
||||
# state and nonce must be unguessable, not reused constants
|
||||
assert len(sent["state"]) >= 16 and len(sent["nonce"]) >= 16
|
||||
assert sent["state"] != sent["nonce"]
|
||||
|
||||
|
||||
async def test_login_flow_cookie_is_httponly_secure_lax_and_scoped_to_the_auth_path(app_oidc):
|
||||
app, _, _ = app_oidc
|
||||
async with _client(app) as c:
|
||||
r = await _login(c)
|
||||
|
||||
cookie = _cookie_attrs(r, FLOW_COOKIE)
|
||||
assert "HttpOnly" in cookie
|
||||
assert "Secure" in cookie
|
||||
assert "SameSite=lax" in cookie.replace("samesite", "SameSite")
|
||||
assert "Path=/ui/auth" in cookie
|
||||
assert "Max-Age=600" in cookie
|
||||
|
||||
|
||||
async def test_login_confines_the_next_parameter_to_this_ui(app_oidc):
|
||||
"""An open redirect would let a phisher bounce a freshly authenticated owner
|
||||
off to their own host."""
|
||||
app, _, svc = app_oidc
|
||||
async with _client(app) as c:
|
||||
r = await _login(c, next_url="https://evil.example/steal")
|
||||
flow_cookie = c.cookies[FLOW_COOKIE]
|
||||
assert r.status_code == 302
|
||||
|
||||
from mdmem.ui.auth import FlowCodec
|
||||
|
||||
flow = FlowCodec(svc.settings.session_secret).read(flow_cookie)
|
||||
assert flow["next"] == "/ui/inbox"
|
||||
|
||||
|
||||
async def test_login_reports_an_unreachable_identity_provider_as_503(app_oidc):
|
||||
app, oidc, _ = app_oidc
|
||||
oidc.authorize_error = RuntimeError("discovery timed out")
|
||||
async with _client(app) as c:
|
||||
r = await _login(c)
|
||||
assert r.status_code == 503
|
||||
assert "identity provider" in r.text
|
||||
|
||||
|
||||
async def test_login_is_skipped_entirely_when_auth_is_disabled(auth_settings, monkeypatch):
|
||||
monkeypatch.setattr(ui_app, "OidcClient", FakeOidc)
|
||||
svc = MemoryService.build(replace(auth_settings, ui_auth_disabled=True), embedder=None)
|
||||
await svc.start(watch=False, embed=False)
|
||||
try:
|
||||
app = ui_app.build_ui_app(svc)
|
||||
async with _client(app) as c:
|
||||
r = await c.get("/ui/auth/login", params={"next": "/ui/health"})
|
||||
assert r.status_code == 302 and r.headers["location"] == "/ui/health"
|
||||
finally:
|
||||
await svc.stop()
|
||||
|
||||
|
||||
# -- callback ----------------------------------------------------------------
|
||||
async def test_callback_without_a_flow_cookie_is_refused(app_oidc):
|
||||
app, _, _ = app_oidc
|
||||
async with _client(app) as c:
|
||||
r = await c.get("/ui/auth/callback", params={"code": "abc", "state": "whatever"})
|
||||
assert r.status_code == 400
|
||||
assert SESSION_COOKIE not in r.headers.get("set-cookie", "")
|
||||
|
||||
|
||||
async def test_callback_with_a_mismatched_state_is_refused(app_oidc):
|
||||
app, oidc, _ = app_oidc
|
||||
async with _client(app) as c:
|
||||
await _login(c)
|
||||
r = await c.get("/ui/auth/callback", params={"code": "abc", "state": "forged-state"})
|
||||
assert r.status_code == 400
|
||||
assert "exchange" not in oidc.calls, "a forged state must never reach the token endpoint"
|
||||
|
||||
|
||||
async def test_callback_without_a_code_is_refused(app_oidc):
|
||||
app, oidc, _ = app_oidc
|
||||
async with _client(app) as c:
|
||||
await _login(c)
|
||||
state = oidc.calls["authorize"]["state"]
|
||||
r = await c.get("/ui/auth/callback", params={"code": "", "state": state})
|
||||
assert r.status_code == 400
|
||||
assert "exchange" not in oidc.calls
|
||||
|
||||
|
||||
async def test_callback_hands_the_exchange_the_matching_verifier_and_nonce(app_oidc):
|
||||
app, oidc, _ = app_oidc
|
||||
async with _client(app) as c:
|
||||
await _login(c)
|
||||
issued = oidc.calls["authorize"]
|
||||
r = await c.get("/ui/auth/callback", params={"code": "auth-code", "state": issued["state"]})
|
||||
|
||||
assert r.status_code == 302
|
||||
assert oidc.calls["exchange"]["code"] == "auth-code"
|
||||
verifier = oidc.calls["exchange"]["verifier"]
|
||||
expected = base64.urlsafe_b64encode(hashlib.sha256(verifier.encode()).digest()).rstrip(b"=").decode()
|
||||
assert expected == issued["challenge"], "the verifier must match the challenge sent to the provider"
|
||||
assert oidc.calls["claims"]["nonce"] == issued["nonce"]
|
||||
|
||||
|
||||
async def test_callback_issues_a_hardened_session_cookie_and_drops_the_flow_cookie(app_oidc):
|
||||
app, oidc, svc = app_oidc
|
||||
async with _client(app) as c:
|
||||
await _login(c, next_url="/ui/health")
|
||||
r = await c.get("/ui/auth/callback", params={"code": "c", "state": oidc.calls["authorize"]["state"]})
|
||||
|
||||
assert r.status_code == 302 and r.headers["location"] == "/ui/health"
|
||||
session = _cookie_attrs(r, SESSION_COOKIE)
|
||||
assert "HttpOnly" in session and "Secure" in session and "Path=/" in session
|
||||
assert "SameSite=lax" in session.replace("samesite", "SameSite")
|
||||
assert f"Max-Age={svc.settings.session_ttl_hours * 3600}" in session
|
||||
# the single-use flow cookie is cleared
|
||||
assert 'mdmem_oidc_flow=""' in _cookie_attrs(r, FLOW_COOKIE)
|
||||
|
||||
codec = SessionCodec(svc.settings.session_secret, svc.settings.session_ttl_hours)
|
||||
token = r.headers.get_list("set-cookie")[0].split("=", 1)[1].split(";", 1)[0]
|
||||
assert codec.read(token).sub == "u1"
|
||||
|
||||
|
||||
async def test_callback_refuses_a_user_outside_the_required_group(app_oidc):
|
||||
app, oidc, _ = app_oidc
|
||||
oidc.claims_payload = {"sub": "u2", "groups": ["some_other_group"]}
|
||||
async with _client(app) as c:
|
||||
await _login(c)
|
||||
r = await c.get("/ui/auth/callback", params={"code": "c", "state": oidc.calls["authorize"]["state"]})
|
||||
assert r.status_code == 403 and "access denied" in r.text
|
||||
assert SESSION_COOKIE not in r.headers.get("set-cookie", "")
|
||||
|
||||
|
||||
async def test_callback_surfaces_an_auth_error_with_its_own_status(app_oidc):
|
||||
app, oidc, _ = app_oidc
|
||||
oidc.claims_error = AuthError("id_token has no valid exp claim")
|
||||
async with _client(app) as c:
|
||||
await _login(c)
|
||||
r = await c.get("/ui/auth/callback", params={"code": "c", "state": oidc.calls["authorize"]["state"]})
|
||||
assert r.status_code == 403 and "access denied" in r.text
|
||||
|
||||
|
||||
async def test_callback_turns_an_unexpected_failure_into_502_without_leaking_details(app_oidc):
|
||||
app, oidc, _ = app_oidc
|
||||
oidc.exchange_error = RuntimeError("connection reset by peer at 10.0.0.7:8443")
|
||||
async with _client(app) as c:
|
||||
await _login(c)
|
||||
r = await c.get("/ui/auth/callback", params={"code": "c", "state": oidc.calls["authorize"]["state"]})
|
||||
assert r.status_code == 502
|
||||
assert "10.0.0.7" not in r.text
|
||||
|
||||
|
||||
async def test_a_flow_cookie_cannot_be_replayed_after_a_successful_login(app_oidc):
|
||||
"""The flow cookie is deleted on success; a captured one must not mint a second
|
||||
session on its own, because the httpx client no longer holds it."""
|
||||
app, oidc, _ = app_oidc
|
||||
async with _client(app) as c:
|
||||
await _login(c)
|
||||
state = oidc.calls["authorize"]["state"]
|
||||
assert (await c.get("/ui/auth/callback", params={"code": "c", "state": state})).status_code == 302
|
||||
again = await c.get("/ui/auth/callback", params={"code": "c", "state": state})
|
||||
assert again.status_code == 400
|
||||
|
||||
|
||||
# -- logout ------------------------------------------------------------------
|
||||
async def test_logout_clears_the_session_and_sends_the_owner_back_to_login(app_oidc):
|
||||
app, _, svc = app_oidc
|
||||
codec = SessionCodec(svc.settings.session_secret, svc.settings.session_ttl_hours)
|
||||
cookie = codec.issue(Principal(sub="u1", name="M", email=""))
|
||||
async with _client(app, {SESSION_COOKIE: cookie}) as c:
|
||||
assert (await c.get("/ui/inbox")).status_code == 200
|
||||
r = await c.get("/ui/auth/logout")
|
||||
|
||||
assert r.status_code == 302 and r.headers["location"] == "/ui/auth/login"
|
||||
cleared = _cookie_attrs(r, SESSION_COOKIE)
|
||||
# The browser only drops the cookie if the clearing Set-Cookie matches the path
|
||||
# it was issued on and actually expires it.
|
||||
assert 'mdmem_session=""' in cleared
|
||||
assert "Path=/" in cleared
|
||||
assert "Max-Age=0" in cleared or "expires=Thu, 01 Jan 1970" in cleared
|
||||
|
||||
# and a visitor arriving without the cookie is back at the login redirect
|
||||
async with _client(app) as anon:
|
||||
assert (await anon.get("/ui/inbox")).status_code == 302
|
||||
|
||||
|
||||
async def test_an_expired_session_is_sent_back_to_login(app_oidc, monkeypatch):
|
||||
import time as _time
|
||||
|
||||
app, _, svc = app_oidc
|
||||
codec = SessionCodec(svc.settings.session_secret, svc.settings.session_ttl_hours)
|
||||
cookie = codec.issue(Principal(sub="u1", name="M", email=""))
|
||||
real_time = _time.time
|
||||
ttl = svc.settings.session_ttl_hours * 3600
|
||||
monkeypatch.setattr("itsdangerous.timed.time.time", lambda: real_time() + ttl + 60)
|
||||
async with _client(app, {SESSION_COOKIE: cookie}) as c:
|
||||
r = await c.get("/ui/inbox")
|
||||
assert r.status_code == 302 and r.headers["location"].startswith("/ui/auth/login")
|
||||
Reference in new issue
Block a user