feat(admin): HTTP API for request-line fingerprint bans#153
Merged
Conversation
Three operator endpoints under /admin/bans, gated by ADMIN_TOKEN bearer auth (mirror of LML's /admin/upload-library-db pattern). request-o-matic is a thin proxy: every write forwards to Backend-Service's /internal/banned-fingerprints CRUD (BS#1261). No local ban state, no JWT verification — rom's value-add is being co-located with the request-line surface operators already think about. The shared mutation surface lives in services/ban_service.py so the future Slack-native router (#152) can plug into the same ban/unban/list functions without spawning a second codepath. End-to-end idempotency is preserved: re-banning returns 200 with the updated row, unbanning a non-existent fingerprint returns 204. Operator-facing 4xx upstream errors forward verbatim; 5xx upstream becomes 502 so a BS outage doesn't look like a rom bug. Operator runbook in docs/admin-bans.md. Closes #151
…d bad input Addresses the substantive bucket of the max-effort review (PR #153): * `_map_upstream_error`: BS 401/403/429 now remap to 502 (these reflect the rom->BS hop, not the operator->rom request — forwarding them verbatim made an X-Internal-Key rotation indistinguishable from an ADMIN_TOKEN typo). * `BanAdminClient` wraps httpx.HTTPError (connect, read, timeout, protocol) as `BanAdminClientError(0, ...)` so transport failures render as 502 instead of escaping as unhandled 500. The router's `responses=` table promised 502; it now delivers. * `require_admin_token` switches to `hmac.compare_digest` (encoded to bytes so non-ASCII bearer values don't TypeError into a 500) and tolerates RFC 7235-allowed whitespace (`Bearer token`, leading/trailing space, tabs). * `BanCreateRequest`: `extra='forbid'` + UUID validator + reason length bounds + `gt=0` on expires_in_seconds. Operator typos now surface as a clean 422 instead of a nested BS 400. * DELETE path-param validates as UUID before reaching BS. * `BanAdminClient.unban` URL-quotes the fingerprint so a future Slack-native caller (#152) bypassing the FastAPI segment validator can't inject `?`/`#`/`/`. * Success-path `response.json()` runs through `_safe_json` so a 200-with-non-JSON body (HTML from a reverse proxy, content-type drift) becomes a normal upstream-error 502 instead of an unhandled JSONDecodeError. * `ban_service.ban/unban` log only the first 8 chars of the fingerprint (it's a stable per-device identifier; full UUID shouldn't sit in Railway/Sentry logs). * Status-code check widens from `!= 200` to `200 <= status < 300` so a future BS refinement to 201-Created-vs-200-OK doesn't break rom. Tests: +13 cases across the four affected files (102 -> 115 in the admin-bans scope; 437 total unit tests still passing).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three operator endpoints under
/admin/bans, gated byADMIN_TOKENbearer auth (mirror of LML's/admin/upload-library-dbpattern). request-o-matic is a thin proxy: every write forwards to Backend-Service's/internal/banned-fingerprintsCRUD (BS#1261). No local ban state, no JWT verification — rom's value-add is being co-located with the request-line surface operators already think about.The shared mutation surface lives in
services/ban_service.pyso the future Slack-native router (#152) plugs into the sameban/unban/list_bansfunctions without spawning a second codepath.End-to-end idempotency is preserved:
Endpoints
POST/admin/bansDELETE/admin/bans/{fingerprint}GET/admin/bans?limit=&cursor=Files
routers/admin.py(new) — FastAPI router.services/ban_service.py(new) — shared mutation surface for Admin HTTP API for managing request-line fingerprint bans #151 + Slack-native ban: button + modal on request posts, restricted to authorized Slack users #152.services/ban_admin_client.py(new) — httpx client for BS admin endpoints.core/dependencies.py—require_admin_token+get_ban_admin_client.config/settings.py—admin_token,bs_internal_bans_url,bs_internal_key.main.py— wire admin router at root (operators expect/admin/bans, not/api/v1/admin/bans).docs/admin-bans.md(new) — operator runbook with curl examples.docs/env-vars.md,README.md,.env.example,CLAUDE.md— env-var reference and topic-router updates.Test plan
ban_admin_client(httpx-mock-based: header forwarding, camelCase serialization, success + 4xx + 5xx + non-JSON body)ban_service(passthrough,actor→banned_by_user_idtranslation)BS_INTERNAL_*unset)require_admin_token+get_ban_admin_clientdependenciestest_main.pypin that/admin/bansmounts at root, not under/api/v1pytest tests/unit/— 413 passing locallyruff check .,ruff format --check .,mypy . --ignore-missing-imports— all cleanADMIN_TOKEN+BS_INTERNAL_BANS_URL+BS_INTERNAL_KEYon the Railway service, then curl the runbook examples indocs/admin-bans.mdRelated
BS_INTERNAL_KEY)services/ban_service.py)Closes #151