feat(api_key): user-issued API keys for /ogcapi-internal - #903
Open
jirhiker wants to merge 3 commits into
Open
Conversation
Companion to OcotilloUI#360, whose settings page ships a generate / rename / revoke / one-time-reveal card against local component state with no endpoints behind it. Design only -- no model, routes, or migration yet. Records three decisions worth settling before implementation: - Key creation must be gated on the OGCInternal group, not a general role. A key is a pre-authorized stand-in for that group, so gating creation on viewer_dependency would let any Viewer mint themselves access to the unfiltered internal collections. - v1 keys authorize /ogcapi-internal only. Making a key act as its owner would require authenticated() to accept a non-JWT credential and synthesize a claims payload, putting every *_dependency behind a static string. - The ADR5 access layer is not on staging, so a key cannot be modeled as a grant principal yet. That is the reason to keep v1 narrow. Also notes that env-var keys in INTERNAL_OGC_API_KEYS stay live alongside the table, since existing holders have them saved in ArcGIS Pro connection dialogs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both open questions answered: - Scope is internal-only. Keys authorize /ogcapi-internal and nothing else, as drafted. - Every key expires, 365 days after creation. expires_at becomes NOT NULL and 365 days is also the maximum, so there is no way to request a key that never expires; operator-issued INTERNAL_OGC_API_KEYS entries stay the escape hatch for a longer-lived credential. Records two consequences of a lifetime that long. It is a backstop against abandoned keys rather than a security control -- the real control is revocation, which the table makes instant. And with no notification infrastructure a key dies silently, so expires_at has to reach the UI with a warning as it approaches. Open questions section replaced with the resulting asks on OcotilloUI#360: a new expiresAt field, and an empty state for accounts without the OGCInternal group. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The internal OGC mount already accepted a static API key, but those live
as label:sha256hex entries in the INTERNAL_OGC_API_KEYS environment
variable, rendered from Secret Manager at deploy time. Revoking one
requires a redeploy, and a key is tied to a person only by an unenforced
label. This adds keys that users issue for themselves and that revoke on
the next request.
Backend for the settings card in OcotilloUI#360, which currently runs the
whole generate / rename / revoke / reveal flow against local component
state.
domain/api_key.py token shape, digesting, expiry, usability
db/api_key.py the api_key table, plus its migration
services/ resolve_api_key(), and the variant that opens its
own session for the ASGI middleware
api/, schemas/ POST / GET / PATCH / DELETE on /api_key
Three decisions worth knowing:
Creation is gated on the OGCInternal group, not on a general role. A key
is a pre-authorized stand-in for that group, so minting one is exactly as
privileged as holding it; gating on viewer_dependency would let any
Viewer issue themselves access to the unfiltered internal collections.
This is the first Depends()-shaped consumer of INTERNAL_OGC_GROUP.
Keys authorize /ogcapi-internal and nothing else. Making a key act as its
owner would mean authenticated() accepting a non-JWT credential and
synthesizing a claims payload, which puts every *_dependency behind a
static string. The scope column makes widening a value rather than a
migration.
Every key expires, 365 days out by default and by ceiling. That length is
a backstop against keys nobody remembers holding, not a security control
-- the control is revocation, which is now immediate. Tokens are stored
as unsalted SHA-256: the secret is 256 bits of CSPRNG output, so a salt
buys nothing, and verification stays one indexed lookup rather than a KDF
per candidate row.
The middleware checks the environment-variable keys first, then the
table, then falls through to the JWT path. A database failure logs and
falls through rather than 500ing, so a bad pool degrades the mount to
bearer-JWT-only instead of taking it down. The session is closed before
the downstream call, since the mount streams paginated GeoJSON up to
max_items 10000 and a held session would pin a pool connection for the
whole response. last_used_at is written at most once every 15 minutes so
paging does not write per page.
Responses are snake_case, unlike the camelCase ApiKey type in
OcotilloUI's src/utils/apiKeys.ts -- that file was written with no server
behind it, and mapping it there beats one router disagreeing with the
rest of the API. The UI also needs expires_at rendered with a warning as
it nears, and an empty state for accounts without OGCInternal.
41 new tests. Full suite green: 1200 passed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Coverage✅ 79.66% total — gate is 75%. Coverage for the Python files changed in this PR
|
Contributor
There was a problem hiding this comment.
Pull request overview
Implements user-issued API keys for the /ogcapi-internal mount by adding an api_key database table, a small domain rules module for token/digest/expiry behavior, an authentication resolver used by the ASGI middleware, and FastAPI routes under /api_key for users to issue/list/rename/revoke their own keys.
Changes:
- Add
api_keypersistence (SQLAlchemy model + Alembic migration) and domain rules for key generation/digests/expiry/last-used semantics. - Extend
/ogcapi-internalASGI middleware to accept database-backed keys (after env-var operator keys, before JWT decode). - Add
/api_keyCRUD-lite routes plus tests and authorization allowlist updates.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
api/api_key.py |
New /api_key routes to issue, list, rename, and revoke user-owned API keys gated by internal_ogc_dependency. |
core/dependencies.py |
Adds internal_ogc_function/internal_ogc_dependency as the Depends()-shaped gate for OGCInternal. |
core/internal_ogc_auth.py |
Adds database-backed key validation to the /ogcapi-internal ASGI auth middleware. |
core/initializers.py |
Registers the new api_key router with the FastAPI app. |
db/api_key.py |
Introduces the ApiKey SQLAlchemy model + indexes for digest lookup and owner filtering. |
db/__init__.py |
Exposes the new ApiKey model via the db package import surface. |
domain/api_key.py |
Domain-layer rules for token shape, digesting, previewing, expiry, and usability checks. |
schemas/api_key.py |
Pydantic request/response models for the /api_key endpoints (snake_case fields). |
services/api_key_auth.py |
Resolver for authenticating a presented secret against the api_key table (incl. middleware-safe session variant). |
alembic/versions/d0e1f2a3b4c5_add_api_key_table.py |
Migration creating the api_key table and required indexes. |
tests/test_api_key.py |
New test suite covering domain rules, routes, and resolver behavior (one-time token reveal, ownership, expiry, revocation, etc.). |
tests/test_internal_ogc_auth.py |
Extends middleware-path tests to cover database-backed keys across transports and DB failure fallback behavior. |
tests/test_authorization.py |
Registers internal_ogc_function so the authorization test correctly recognizes the new routes as gated. |
docs/api-key-management.md |
Design/implementation documentation for API key management and security decisions. |
docs/internal-ogc-desktop-gis.md |
Updates operational docs to describe the two key sources and their lifecycle differences. |
CLAUDE.md |
Updates repository architecture guidance to document the new API key subsystem and gating. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+242
to
+246
| elif key.revoked_at is None: | ||
| # Expired but never revoked. Stamp it so the list stops showing it as | ||
| # merely aged and the reason it stopped working is unambiguous. | ||
| key.revoked_at = now | ||
| session.commit() |
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
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.
What
Adds an
api_keytable and four routes under/api_key, so a user can issue, name, revoke, and list their own API keys for the/ogcapi-internalmount.This is the backend for the API keys card in OcotilloUI#360, which currently runs the whole generate / rename / revoke / one-time-reveal flow against local component state because no endpoints existed.
domain/api_key.pydb/api_key.py+ migrationapi_keytable.services/api_key_auth.pyresolve_api_key(), plus the variant that opens its own session for the ASGI middleware.core/internal_ogc_auth.py_database_api_key_valid(), checked after the environment-variable keys and before the JWT decode.core/dependencies.pyinternal_ogc_dependency— the firstDepends()-shaped consumer ofINTERNAL_OGC_GROUP.api/api_key.py,schemas/api_key.pyPOST/GET/PATCH/DELETEon/api_key.Why
The mount already accepts a static API key, but those live as
label:sha256hexentries inINTERNAL_OGC_API_KEYS, rendered from Secret Manager at deploy time. Two consequences motivated this:docs/internal-ogc-desktop-gis.mdalready named the exit — "or to a keys table in Postgres". This is it. Both sources stay live: existing holders have keys saved in ArcGIS Pro connection dialogs, and breaking those to land a table buys nothing.Three decisions worth reviewing
Creation is gated on
OGCInternal, not on a general role. A key is a pre-authorized stand-in for that group — it reaches the unfiltered, draft-inclusive internal collections. So minting one is exactly as privileged as holding the group. Gating creation onviewer_dependencywould have let any Viewer issue themselves internal-mount access with a button.Keys authorize
/ogcapi-internaland nothing else. Making a key act as its owner across the API would meanauthenticated()accepting a non-JWT credential and synthesizing a claims payload, which puts every*_dependencybehind a static string. Thescopecolumn makes widening a value rather than a migration. Related: the ADR5 access layer is not onstaging, so a key cannot be modeled as a grant principal yet — which is the reason to keep v1 narrow.Every key expires, 365 days out by default and by ceiling. A lifetime that long is a backstop against keys nobody remembers holding — a decommissioned laptop, someone who left — not a security control. The control is revocation, which is now immediate.
expires_atisNOT NULL, and a NULL reads as expired so a row written around the create path fails closed.Implementation notes
/ogcapi-internalstreams paginated GeoJSON up tomax_items: 10000; a session held for the response body would pin a pool connection for the whole stream.last_used_atis written at most once per 15 minutes, so a client paging through a collection does not write once per page.last_used_atis what you want after revoking a key you think was leaked.Verification
tests/test_api_key.py— rules, routes, and the resolver: the token is returned exactly once and never again, no column holds it, another user's key 404s on read/rename/revoke, expiry defaults and clamps, and a revoked key stops resolving on the next call.tests/test_internal_ogc_auth.py— eight more for the middleware path: every transport (bearer, Basic,?token=, with the query parameter still stripped), revoked, expired, unissued, a database failure falling through instead of erroring, and operator-issued keys still working with an empty table.alembic revision --autogenerateproduced noapi_keydrift — model and migration agree. That check caught a duplicate unique index ontoken_digest, since removed.tests/test_authorization.pyinitially reported all four routes as anonymous. Not a routing bug:AUTH_DEPENDENCY_CALLABLESis an explicit inventory andinternal_ogc_functionwas not in it. Registered. Any future tier needs that line or the guard reports false positives.Follow-ups, not in this PR
Response fields are snake_case, unlike the camelCase
ApiKeytype in OcotilloUI'ssrc/utils/apiKeys.ts. That file is camelCase only because it was written with no server behind it; mapping it there is smaller than making one router disagree with the rest of the API.idis a number, not a string.So OcotilloUI#360 needs: snake_case field names,
expires_atrendered with a warning as it nears (there is no notification infrastructure, so a key otherwise dies silently a year after someone set it up), and an empty state for accounts withoutOGCInternal— the card is meaningless to them, and a Generate button that 403s is worse than an explanation.No admin list-all route: it is a different gate (general
Admin, which confers nothing in this family) and deserves its own change.Read
docs/api-key-management.mdbefore widening the scope.🤖 Generated with Claude Code