feat(ogc): add authenticated internal OGC mount (BDMS-985 A11)#783
Open
ksmuczynski wants to merge 5 commits into
Open
Conversation
pygeoapi is mounted via a raw Starlette Mount, so FastAPI's Depends() machinery never runs for it -- auth has to happen at the ASGI layer, in front of the mount, not as a route dependency. Added TokenInvalid and decode_token_payload() alongside the existing _get_token_payload so the same JWKS/jwt.decode logic can be reused from code with no FastAPI exception handler watching (raising HTTPException there would just be an unhandled exception, not a response). InternalOGCAuthMiddleware is a plain ASGI middleware class rather than BaseHTTPMiddleware (used elsewhere in this app for request logging and lazy admin init): BaseHTTPMiddleware buffers the full response body and breaks client-disconnect propagation, which matters here since this mount serves paginated GeoJSON up to max_items: 10000. Splits 401 (no/invalid token) from 403 (valid token, wrong group) per the acceptance criteria, rather than collapsing both into 401 like every other auth path in this app does today -- the underlying check is identical either way, so the split is a one-line branch, not meaningfully more code to maintain.
Generalized _mount_path()/_pygeoapi_dir()/_write_config() to take an env var and default rather than hardcoding the public mount's values, so the internal mount gets the same traversal/character safety and sensitive-file handling for free instead of a copy-pasted second implementation. mount_pygeoapi_internal() gets its own guard flag and runtime dir so it can't silently no-op against the public mount's state, and a startup check that the two configured mount paths actually differ -- Starlette doesn't error on duplicate Mounts, it just routes to whichever registered first, which would leave the internal mount silently unreachable rather than failing loudly. Also added _assert_server_settings_match(): pygeoapi.api.API mutates process-wide globals (CHARSET, FORMAT_TYPES) during __init__, so whichever of the two mounts is built last wins for both. Inert today since both configs agree on encoding/gzip, but this fails startup loudly instead of letting a future divergence between the two configs silently corrupt responses on whichever mount lost the race.
Mirrors all 22 ogc_* relations f4a5b6c7d8e9 filters, as ogc_internal_* counterparts with no release_status predicate, so /ogcapi-internal can serve private/draft records to authenticated staff. Reuses f4a5b6c7d8e9's parametrized builder functions (public_only kept in the signature and always passed False) for structural parity rather than a differently-shaped rewrite, since that's what the drift-detection test in the next commit diffs against. This repo keeps migrations self-contained with no cross-migration imports, so the two chemistry views' analyte-mapping CASE blocks are duplicated here rather than shared via a helper module -- a shared module would mean a future edit to one ticket's migration silently changes what the other replays from scratch. Added a one-line cross-reference comment in both files pointing at each other and the parity test. ogc_internal_locations has no release_status predicate at all (unlike ogc_locations, which is always public-only even on its own downgrade path) since it never existed in any filtered form before this migration. downgrade() simply drops all 22 relations rather than restoring a prior state, since none of them existed before this migration.
The major/minor chemistry CASE blocks now exist in two files (f4a5b6c7d8e9 and 2d3c3a268652) because migrations can't import from each other. Without a check, a future analyte-mapping fix applied to the public view (e.g. reported against a public-facing bug) has no reason to also touch the internal view sitting in a different file from a different ticket -- silently leaving internal staff looking at the stale mapping, which inverts who has the more reliable data. Compares AST source segments rather than raw text/regex so the comparison survives incidental formatting differences and only fails on a genuine logic change. Normalizes the one expected difference (the view name embedded in the CREATE statement) before comparing.
Tags the 6 A11 scenarios A1 already wrote into this shared feature file with @production, appended to their existing tag lines rather than at the feature level -- this file has no feature-level tag since it's shared across ~10 other tickets' scenarios, and tagging at that level would pull in every other ticket's undefined steps. The 401/403/200 auth scenarios neutralize the ambient AUTHENTIK_DISABLE_AUTHENTICATION dev-bypass for their own duration only (restored via context.add_cleanup) and patch core.permissions.decode_token_payload directly, since no real Authentik server is available in CI to issue a genuine JWT. Existing auth-testing infrastructure in this repo only reaches app.dependency_overrides, which has zero effect on ASGI middleware -- none of it exercises InternalOGCAuthMiddleware for free.
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
Stands up a second, authenticated pygeoapi mount at
/ogcapi-internal,mirroring all 22
ogc_*relations A1 restricts to public records asunfiltered
ogc_internal_*counterparts, so staff who need to seeprivate/draft records (pre-publication QA, internal review) can, without
touching the public catalog.
Why
Companion to A1 (restricts
/ogcapitorelease_status = 'public').Locking the public mount down means internal users still needs unfiltered access
for the records A1 now hides. This stands up that access as its own
authenticated mount rather than punching a hole back into the public one.
Changes
InternalOGCAuthMiddlewareis a plain ASGImiddleware (not
BaseHTTPMiddleware, which buffers the full responsebody and breaks client-disconnect propagation. This mount serves
paginated GeoJSON up to
max_items: 10000). Splits 401 (no/invalidtoken) from 403 (valid token, wrong group) per the acceptance criteria,
rather than collapsing both into 401 like the rest of the app does today.
mount_pygeoapi_internal()gets its ownguard flag and runtime dir, a startup check that the two mount paths
actually differ (Starlette silently routes to whichever registers
first otherwise), and an equality check on the two configs'
server.encoding/server.gzipsettings, since pygeoapi mutatesprocess-wide globals from those at construction time.
ogc_internal_*counterparts to all 22
ogc_*relations A1 touches, using the samedependency-ordering (
ogc_internal_actively_monitored_wellsbefore/afterogc_internal_water_well_summary) and unique-index treatment as thepublic side.
chemistry views' CASE-mapping logic byte-identical across both
migrations, since this repo's migrations can't import from each other.
ogc-cleanup-sprint1.featureare now tagged@productionandimplemented: auth (401/403/200), internal record exposure, DB relation
separation, and public-surface non-interference.
Verification
All against local
ocotilloapi_test, never staging/prod.uv run pytest tests/test_migration_view_parity.py tests/test_pygeoapi_mount.py -v— 10 passed.uv run pytest(full suite): 724 passed, 81 skipped, 6 xpassed, 0 failed.alembic upgrade head/downgrade -1/upgrade head: clean in bothdirections; confirmed
ogc_water_wellsandogc_internal_water_wellscoexist as 2 distinct relations, and downgrade fully drops the internal
set.
DROP_AND_REBUILD_DB=1 AUTHENTIK_DISABLE_AUTHENTICATION=1 uv run behave tests/features/ogc-cleanup-sprint1.feature --tags=@A11:6/6 scenarios passed (32 other scenarios sharing this feature file
correctly skipped, untouched).
Notes
f4a5b6c7d8e9(A1's migration file),adding a one-line cross-reference comment pointing at this migration
and the new parity test, not a functional change. Flagging since it's
the only diff here to a file introduced by a different ticket/PR.
/ogcapi-internalcollections share the same IDs as/ogcapi(e.g.both expose
water_wells); only the backing table differs(
ogc_water_wellsvsogc_internal_water_wells), following the plan'stable_prefix-only parametrization. Each internal collection'sself-link still resolves to
/ogcapi-internal, which is what theBehave "collections not available on the public endpoint" scenario
actually checks, rather than comparing id sets. This was a judgment
call, not an explicit instruction. Happy to switch to a distinct id
namespace (e.g.
internal_water_wells) if you'd rather have that.