diff --git a/.env.example b/.env.example index 5fa1ad8ba..54c576b32 100644 --- a/.env.example +++ b/.env.example @@ -15,8 +15,21 @@ PYGEOAPI_POSTGRES_USER=your_username # above; only the mount path, runtime dir, and advertised server URL differ. PYGEOAPI_INTERNAL_MOUNT_PATH=/ogcapi-internal PYGEOAPI_INTERNAL_RUNTIME_DIR=/tmp/pygeoapi-internal +# Leave blank to derive from PYGEOAPI_SERVER_URL's application root. Only set +# this when the internal mount is served from a different host than /ogcapi. PYGEOAPI_INTERNAL_SERVER_URL= +# Static API keys for /ogcapi-internal, for desktop GIS clients that cannot +# refresh an Authentik access token (ArcGIS Pro, QGIS). Comma- or +# whitespace-separated `label:sha256hex` entries; the label is bookkeeping +# only. Blank means bearer-JWT access only. Mint one with: +# python -c "import secrets,hashlib;k=secrets.token_urlsafe(32);print(k,hashlib.sha256(k.encode()).hexdigest())" +# Give the first value to the user, put `label:` here. +# Deployed environments source this from the Secret Manager secret +# `internal-ogc-api-keys`, not from a GitHub secret. +# See docs/internal-ogc-desktop-gis.md. +INTERNAL_OGC_API_KEYS= + # Connection pool configuration for parallel transfers # pool_size: number of persistent connections to maintain # max_overflow: additional connections allowed during peak usage diff --git a/.github/app.template.yaml b/.github/app.template.yaml index bb44e584c..a0f67c7b9 100644 --- a/.github/app.template.yaml +++ b/.github/app.template.yaml @@ -34,6 +34,13 @@ env_variables: PYGEOAPI_POSTGRES_PASSWORD: |- ${PYGEOAPI_POSTGRES_PASSWORD} PYGEOAPI_SERVER_URL: "${PYGEOAPI_SERVER_URL}" + # Hashed static API keys for the authenticated /ogcapi-internal mount, as + # `label:sha256hex` entries, sourced from the Secret Manager secret + # `internal-ogc-api-keys`. Needed because ArcGIS Pro and QGIS cannot refresh + # an Authentik access token; see core/internal_ogc_auth.py and + # docs/internal-ogc-desktop-gis.md. Unset means bearer-JWT access only. + INTERNAL_OGC_API_KEYS: |- + ${INTERNAL_OGC_API_KEYS} CLOUD_SQL_IAM_AUTH: "${CLOUD_SQL_IAM_AUTH}" GCS_SERVICE_ACCOUNT_KEY: |- ${GCS_SERVICE_ACCOUNT_KEY} diff --git a/.github/skills/code-review/SKILL.md b/.github/skills/code-review/SKILL.md new file mode 100644 index 000000000..723969783 --- /dev/null +++ b/.github/skills/code-review/SKILL.md @@ -0,0 +1,84 @@ +--- +name: code-review +description: Repository-specific review rules for OcotilloAPI pull requests. Use this when reviewing a pull request in this repository, so review comments account for the authorization, schema, domain-layer, and migration conventions that are easy to violate silently. +--- + +# Reviewing OcotilloAPI pull requests + +OcotilloAPI is a FastAPI + PostgreSQL/PostGIS geospatial service for the New Mexico +Bureau of Geology and Mineral Resources. Read `CLAUDE.md` at the repository root for +the full architecture; this skill lists the mistakes worth flagging in review because +they fail silently rather than breaking a test. + +Use the GitHub MCP server tools (`list_workflow_runs`, `summarize_job_log_failures`, +`get_job_logs`) to check whether the `Test Suite` workflow is failing before commenting +on behavior — a failing `unit-tests` job often explains the diff better than the diff does. + +## Authorization is opt-in, so omissions are invisible + +Authorization is applied per endpoint via a parameter in the route signature, not by a +router-level `dependencies=[...]`. Two failure modes to flag: + +1. A new route with no `user: _dependency` parameter is fully public and raises no + error. If the pull request adds a route, check whether it belongs in the anonymous-route + allowlist in `tests/test_authorization.py`. If it does not, it needs a role dependency. +2. The dependency must be a **type annotation** (`user: viewer_dependency`), never a default + value (`user=viewer_dependency`). The latter silently disables the dependency, and FastAPI + reinterprets it as a query parameter. Flag this every time. + +Role families are orthogonal: general `Admin` confers nothing in the `AMP*` or `Lexicon*` +families. Only tiers within one family nest. A diff that treats `Admin` as a superset of +`AMPEditor` is wrong. + +`@in_public_schema` controls anonymous OpenAPI visibility only. It grants no access and +removes no dependency; flag any use that appears to be standing in for authorization. + +`/ogcapi-internal` is a raw Starlette Mount and is gated at the ASGI layer in +`core/internal_ogc_auth.py`, outside `Depends()`. Changes to its credential paths should +cite `docs/internal-ogc-desktop-gis.md`. + +The development auth bypass (`AUTHENTIK_DISABLE_AUTHENTICATION=1`) is honored only when +`MODE=development`. Any change that widens that condition is a security finding. + +## Model changes are a five-step workflow + +A pull request that edits a model in `db/` is incomplete unless it also covers the matching +Pydantic schemas in `schemas/`, an Alembic migration, test fixtures and payloads in `tests/`, +and the field mappings in `transfers/` when the field is populated from the legacy AMPAPI +data. Flag whichever step is missing. + +Schema conventions: `Create` schemas use `` for non-nullable and ` | None = None` +for nullable; `Update` schemas make every field optional with a `None` default; `Response` +schemas use `` for non-nullable and ` | None` for nullable. + +Validation split: input validation belongs in Pydantic validators and produces 422s. Database +constraint checks are manual in the endpoint and produce 409s. Custom exceptions should use +`PydanticStyleException` from `services/exceptions_helper.py` so error bodies stay consistent. + +## Layer boundaries + +`domain/` holds business rules as plain functions over plain values. Modules there must not +import from `api/`, `db/`, `schemas/`, or `services/`, and must not import `fastapi`, +`sqlalchemy`, `pydantic`, or `httpx`. Flag any new import that breaks this — it is what keeps +the rules testable without a database. Domain errors subclass `ValueError` because the CSV +importers treat a `ValueError` on a row as a per-row validation failure; an exception type +that does not subclass `ValueError` will escape that handling. See `ADR4.md`. + +`services/` is the layer that loads data, calls the domain rule, and persists the result. + +## Spatial and query specifics + +All geometries are WGS84 (SRID 4326). Legacy transfer scripts convert from UTM (SRID 26913); +a missing transformation puts points in the wrong hemisphere rather than raising. + +List filters arrive from the Refine UI as repeated `filter` query parameters containing JSON. +Association-backed columns are virtual and map to EXISTS subqueries in +`services/query_helper.py`, not to `ILIKE` on an ORM proxy. Sorting by monitoring status or +well status must use SQL subqueries on `StatusHistory`, because `ORDER BY` cannot see a Python +`@property`. See `docs/refine-json-filters-and-virtual-fields.md`. + +## Migrations + +Alembic schema migrations run automatically in the deployment pipeline. Registered *data* +migrations do not — they sit unapplied until someone runs them by hand. If a pull request adds +a data migration, ask how and when it will be run. diff --git a/.github/workflows/CD_production.yml b/.github/workflows/CD_production.yml index 39c5ff6b4..2df5ed439 100644 --- a/.github/workflows/CD_production.yml +++ b/.github/workflows/CD_production.yml @@ -71,11 +71,14 @@ jobs: with: credentials_json: ${{ secrets.CLOUD_DEPLOY_SERVICE_ACCOUNT_KEY }} - # Feedback endpoint credentials live in Google Secret Manager, not - # GitHub secrets. The deploy service account needs - # roles/secretmanager.secretAccessor on these secrets. - - name: Fetch feedback secrets from Secret Manager - id: feedback-secrets + # Application credentials live in Google Secret Manager, not GitHub + # secrets. The deploy service account needs + # roles/secretmanager.secretAccessor on these secrets. Every secret + # listed here must already exist in the target project or the deploy + # fails -- see docs/internal-ogc-desktop-gis.md for the placeholder + # value to seed internal-ogc-api-keys with. + - name: Fetch application secrets from Secret Manager + id: app-secrets uses: 'google-github-actions/get-secretmanager-secrets@v3' with: secrets: |- @@ -83,6 +86,7 @@ jobs: jira_api_token:${{ vars.GCP_PROJECT_ID }}/jira-api-token slack_feedback_webhook_url:${{ vars.GCP_PROJECT_ID }}/slack-feedback-webhook-url slack_edits_webhook_url:${{ vars.GCP_PROJECT_ID }}/slack-edits-webhook-url + internal_ogc_api_keys:${{ vars.GCP_PROJECT_ID }}/internal-ogc-api-keys - name: Run Alembic migrations on production database env: @@ -119,6 +123,7 @@ jobs: PYGEOAPI_POSTGRES_PORT: "${{ vars.PYGEOAPI_POSTGRES_PORT || '5432' }}" PYGEOAPI_POSTGRES_PASSWORD: "${{ secrets.PYGEOAPI_POSTGRES_PASSWORD }}" PYGEOAPI_SERVER_URL: "${{ vars.PYGEOAPI_SERVER_URL }}" + INTERNAL_OGC_API_KEYS: "${{ steps.app-secrets.outputs.internal_ogc_api_keys }}" CLOUD_SQL_IAM_AUTH: "true" GCS_SERVICE_ACCOUNT_KEY: "${{ secrets.GCS_SERVICE_ACCOUNT_KEY }}" GCS_BUCKET_NAME: "${{ vars.GCS_BUCKET_NAME }}" @@ -128,11 +133,11 @@ jobs: AUTHENTIK_TOKEN_URL: "${{ vars.AUTHENTIK_TOKEN_URL }}" APITALLY_CLIENT_ID: "${{ vars.APITALLY_CLIENT_ID }}" JIRA_BASE_URL: "${{ vars.JIRA_BASE_URL || 'https://nmbgmr.atlassian.net' }}" - JIRA_EMAIL: "${{ steps.feedback-secrets.outputs.jira_email }}" - JIRA_API_TOKEN: "${{ steps.feedback-secrets.outputs.jira_api_token }}" + JIRA_EMAIL: "${{ steps.app-secrets.outputs.jira_email }}" + JIRA_API_TOKEN: "${{ steps.app-secrets.outputs.jira_api_token }}" JIRA_DEFAULT_PROJECT: "${{ vars.JIRA_DEFAULT_PROJECT || 'BDMS' }}" - SLACK_FEEDBACK_WEBHOOK_URL: "${{ steps.feedback-secrets.outputs.slack_feedback_webhook_url }}" - SLACK_EDITS_WEBHOOK_URL: "${{ steps.feedback-secrets.outputs.slack_edits_webhook_url }}" + SLACK_FEEDBACK_WEBHOOK_URL: "${{ steps.app-secrets.outputs.slack_feedback_webhook_url }}" + SLACK_EDITS_WEBHOOK_URL: "${{ steps.app-secrets.outputs.slack_edits_webhook_url }}" OCOTILLO_UI_BASE_URL: "${{ vars.OCOTILLO_UI_BASE_URL || 'https://ocotillo.newmexicowaterdata.org' }}" run: | export MAX_INSTANCES="10" diff --git a/.github/workflows/CD_staging.yml b/.github/workflows/CD_staging.yml index c1cbd243d..c5bf72b4a 100644 --- a/.github/workflows/CD_staging.yml +++ b/.github/workflows/CD_staging.yml @@ -36,11 +36,14 @@ jobs: with: credentials_json: ${{ secrets.CLOUD_DEPLOY_SERVICE_ACCOUNT_KEY }} - # Feedback endpoint credentials live in Google Secret Manager, not - # GitHub secrets. The deploy service account needs - # roles/secretmanager.secretAccessor on these secrets. - - name: Fetch feedback secrets from Secret Manager - id: feedback-secrets + # Application credentials live in Google Secret Manager, not GitHub + # secrets. The deploy service account needs + # roles/secretmanager.secretAccessor on these secrets. Every secret + # listed here must already exist in the target project or the deploy + # fails -- see docs/internal-ogc-desktop-gis.md for the placeholder + # value to seed internal-ogc-api-keys with. + - name: Fetch application secrets from Secret Manager + id: app-secrets uses: 'google-github-actions/get-secretmanager-secrets@v3' with: secrets: |- @@ -48,6 +51,7 @@ jobs: jira_api_token:${{ vars.GCP_PROJECT_ID }}/jira-api-token slack_feedback_webhook_url:${{ vars.GCP_PROJECT_ID }}/slack-feedback-webhook-url slack_edits_webhook_url:${{ vars.GCP_PROJECT_ID }}/slack-edits-webhook-url + internal_ogc_api_keys:${{ vars.GCP_PROJECT_ID }}/internal-ogc-api-keys - name: Run Alembic migrations on staging database env: @@ -79,6 +83,7 @@ jobs: PYGEOAPI_POSTGRES_PORT: "${{ vars.PYGEOAPI_POSTGRES_PORT || '5432' }}" PYGEOAPI_POSTGRES_PASSWORD: "${{ secrets.PYGEOAPI_POSTGRES_PASSWORD }}" PYGEOAPI_SERVER_URL: "${{ vars.PYGEOAPI_SERVER_URL }}" + INTERNAL_OGC_API_KEYS: "${{ steps.app-secrets.outputs.internal_ogc_api_keys }}" CLOUD_SQL_IAM_AUTH: "true" GCS_SERVICE_ACCOUNT_KEY: "${{ secrets.GCS_SERVICE_ACCOUNT_KEY }}" GCS_BUCKET_NAME: "${{ vars.GCS_BUCKET_NAME }}" @@ -88,11 +93,11 @@ jobs: AUTHENTIK_TOKEN_URL: "${{ vars.AUTHENTIK_TOKEN_URL }}" APITALLY_CLIENT_ID: "${{ vars.APITALLY_CLIENT_ID }}" JIRA_BASE_URL: "${{ vars.JIRA_BASE_URL || 'https://nmbgmr.atlassian.net' }}" - JIRA_EMAIL: "${{ steps.feedback-secrets.outputs.jira_email }}" - JIRA_API_TOKEN: "${{ steps.feedback-secrets.outputs.jira_api_token }}" + JIRA_EMAIL: "${{ steps.app-secrets.outputs.jira_email }}" + JIRA_API_TOKEN: "${{ steps.app-secrets.outputs.jira_api_token }}" JIRA_DEFAULT_PROJECT: "${{ vars.JIRA_DEFAULT_PROJECT || 'BDMS' }}" - SLACK_FEEDBACK_WEBHOOK_URL: "${{ steps.feedback-secrets.outputs.slack_feedback_webhook_url }}" - SLACK_EDITS_WEBHOOK_URL: "${{ steps.feedback-secrets.outputs.slack_edits_webhook_url }}" + SLACK_FEEDBACK_WEBHOOK_URL: "${{ steps.app-secrets.outputs.slack_feedback_webhook_url }}" + SLACK_EDITS_WEBHOOK_URL: "${{ steps.app-secrets.outputs.slack_edits_webhook_url }}" OCOTILLO_UI_BASE_URL: "${{ vars.OCOTILLO_UI_BASE_URL || 'https://ocotillo-staging.newmexicowaterdata.org' }}" run: | export MAX_INSTANCES="10" diff --git a/.github/workflows/CD_testing.yml b/.github/workflows/CD_testing.yml index 0ed101d15..2a9a9dd2e 100644 --- a/.github/workflows/CD_testing.yml +++ b/.github/workflows/CD_testing.yml @@ -36,11 +36,14 @@ jobs: with: credentials_json: ${{ secrets.CLOUD_DEPLOY_SERVICE_ACCOUNT_KEY }} - # Feedback endpoint credentials live in Google Secret Manager, not - # GitHub secrets. The deploy service account needs - # roles/secretmanager.secretAccessor on these secrets. - - name: Fetch feedback secrets from Secret Manager - id: feedback-secrets + # Application credentials live in Google Secret Manager, not GitHub + # secrets. The deploy service account needs + # roles/secretmanager.secretAccessor on these secrets. Every secret + # listed here must already exist in the target project or the deploy + # fails -- see docs/internal-ogc-desktop-gis.md for the placeholder + # value to seed internal-ogc-api-keys with. + - name: Fetch application secrets from Secret Manager + id: app-secrets uses: 'google-github-actions/get-secretmanager-secrets@v3' with: secrets: |- @@ -48,6 +51,7 @@ jobs: jira_api_token:${{ vars.GCP_PROJECT_ID }}/jira-api-token slack_feedback_webhook_url:${{ vars.GCP_PROJECT_ID }}/slack-feedback-webhook-url slack_edits_webhook_url:${{ vars.GCP_PROJECT_ID }}/slack-edits-webhook-url + internal_ogc_api_keys:${{ vars.GCP_PROJECT_ID }}/internal-ogc-api-keys - name: Run Alembic migrations on staging database env: @@ -79,6 +83,7 @@ jobs: PYGEOAPI_POSTGRES_PORT: "${{ vars.PYGEOAPI_POSTGRES_PORT || '5432' }}" PYGEOAPI_POSTGRES_PASSWORD: "${{ secrets.PYGEOAPI_POSTGRES_PASSWORD }}" PYGEOAPI_SERVER_URL: "${{ vars.PYGEOAPI_SERVER_URL }}" + INTERNAL_OGC_API_KEYS: "${{ steps.app-secrets.outputs.internal_ogc_api_keys }}" CLOUD_SQL_IAM_AUTH: "true" GCS_SERVICE_ACCOUNT_KEY: "${{ secrets.GCS_SERVICE_ACCOUNT_KEY }}" GCS_BUCKET_NAME: "${{ vars.GCS_BUCKET_NAME }}" @@ -88,11 +93,11 @@ jobs: AUTHENTIK_TOKEN_URL: "${{ vars.AUTHENTIK_TOKEN_URL }}" APITALLY_CLIENT_ID: "${{ vars.APITALLY_CLIENT_ID }}" JIRA_BASE_URL: "${{ vars.JIRA_BASE_URL || 'https://nmbgmr.atlassian.net' }}" - JIRA_EMAIL: "${{ steps.feedback-secrets.outputs.jira_email }}" - JIRA_API_TOKEN: "${{ steps.feedback-secrets.outputs.jira_api_token }}" + JIRA_EMAIL: "${{ steps.app-secrets.outputs.jira_email }}" + JIRA_API_TOKEN: "${{ steps.app-secrets.outputs.jira_api_token }}" JIRA_DEFAULT_PROJECT: "${{ vars.JIRA_DEFAULT_PROJECT || 'BDMS' }}" - SLACK_FEEDBACK_WEBHOOK_URL: "${{ steps.feedback-secrets.outputs.slack_feedback_webhook_url }}" - SLACK_EDITS_WEBHOOK_URL: "${{ steps.feedback-secrets.outputs.slack_edits_webhook_url }}" + SLACK_FEEDBACK_WEBHOOK_URL: "${{ steps.app-secrets.outputs.slack_feedback_webhook_url }}" + SLACK_EDITS_WEBHOOK_URL: "${{ steps.app-secrets.outputs.slack_edits_webhook_url }}" OCOTILLO_UI_BASE_URL: "${{ vars.OCOTILLO_UI_BASE_URL || 'https://ocotillo-staging.newmexicowaterdata.org' }}" run: | export MAX_INSTANCES="10" diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 9f8b0fcb0..7c2b24364 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -14,7 +14,12 @@ jobs: unit-tests: runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: write + env: + COVERAGE_FAIL_UNDER: "75" MODE: development POSTGRES_HOST: localhost POSTGRES_PORT: 5432 @@ -93,13 +98,50 @@ jobs: PGPASSWORD=postgres psql -h localhost -p 5432 -U postgres -d ocotilloapi_test -c "CREATE EXTENSION IF NOT EXISTS postgis" - name: Run tests - run: uv run pytest -vv --durations=20 --cov --cov-report=xml --junitxml=junit.xml --ignore=tests/transfers - - - name: Upload results to Codecov - uses: codecov/codecov-action@v6 + # --cov-fail-under is set here rather than in pyproject so that running a + # single test file locally does not fail on the whole-project total. + run: uv run pytest -vv --durations=20 --cov --cov-report=xml --cov-report=html --cov-report=term-missing --cov-fail-under="$COVERAGE_FAIL_UNDER" --junitxml=junit.xml --ignore=tests/transfers + + - name: Write coverage summary + # Runs even when the coverage gate above fails, so the job summary shows + # which modules dropped rather than only the failing total. + if: ${{ !cancelled() }} + run: | + { + echo "## Coverage" + echo + echo '```' + uv run coverage report + echo '```' + } >> "$GITHUB_STEP_SUMMARY" + + - name: Comment coverage summary on the pull request + # A comment failure must not red the build, and the GITHUB_TOKEN is + # read-only for pull requests opened from a fork. + if: ${{ !cancelled() }} + continue-on-error: true + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ github.event.pull_request.number }} + run: | + gh pr diff "$PR_NUMBER" --name-only > changed-files.txt + uv run python scripts/coverage_pr_comment.py \ + --changed-files changed-files.txt \ + --fail-under "$COVERAGE_FAIL_UNDER" > coverage-comment.md + gh pr comment "$PR_NUMBER" \ + --body-file coverage-comment.md \ + --edit-last --create-if-none + + - name: Upload coverage reports + if: ${{ !cancelled() }} + uses: actions/upload-artifact@v4 with: - report_type: test_results - token: ${{ secrets.CODECOV_TOKEN }} + name: coverage-${{ github.run_id }} + path: | + coverage.xml + htmlcov/ + junit.xml + retention-days: 14 bdd-tests: runs-on: ubuntu-latest diff --git a/.gitignore b/.gitignore index b001e6f5e..eb6f7c340 100644 --- a/.gitignore +++ b/.gitignore @@ -12,6 +12,7 @@ wheels/ .coverage.* htmlcov/ coverage.xml +junit.xml # Virtual environments .venv diff --git a/CLAUDE.md b/CLAUDE.md index 88802a2a0..30549235f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -180,6 +180,18 @@ dependency and FastAPI treats it as a query parameter. only — it grants no access and removes no dependency. Apply it only to routes that genuinely have none. +**`/ogcapi-internal` is gated outside `Depends()`.** It is a raw Starlette +Mount, so `core/internal_ogc_auth.py` gates it at the ASGI layer instead. It +accepts a bearer Authentik JWT carrying `OGCInternal`, **or** a static API key +presented as a bearer token, as the Basic password, or as `?token=`. Only the +key digests are stored, as `label:sha256hex` entries in `INTERNAL_OGC_API_KEYS` +— sourced in deployed environments from the Secret Manager secret +`internal-ogc-api-keys` at deploy time, so revoking a key needs a redeploy. +Never a GitHub secret. The static keys exist because +ArcGIS Pro cannot send a bearer token at all and neither desktop client can +refresh an Authentik token. Read **`docs/internal-ogc-desktop-gis.md`** before +changing the credential paths. + ### Database Configuration The application supports two database modes (configured via `DB_DRIVER` in `.env`): diff --git a/README.md b/README.md index 656d47556..47a178e95 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,6 @@ [![Dependabot Updates](https://github.com/DataIntegrationGroup/NMSampleLocations/actions/workflows/dependabot/dependabot-updates/badge.svg)](https://github.com/DataIntegrationGroup/NMSampleLocations/actions/workflows/dependabot/dependabot-updates) [![Sentry Release](https://github.com/DataIntegrationGroup/NMSampleLocations/actions/workflows/release.yml/badge.svg)](https://github.com/DataIntegrationGroup/NMSampleLocations/actions/workflows/release.yml) [![Tests](https://github.com/DataIntegrationGroup/NMSampleLocations/actions/workflows/tests.yml/badge.svg)](https://github.com/DataIntegrationGroup/NMSampleLocations/actions/workflows/tests.yml) -[![codecov](https://codecov.io/gh/DataIntegrationGroup/NMSampleLocations/graph/badge.svg?token=Y20QB357OO)](https://codecov.io/gh/DataIntegrationGroup/NMSampleLocations) **Geospatial Sample Data Management System** _New Mexico Bureau of Geology and Mineral Resources_ diff --git a/core/internal_ogc_auth.py b/core/internal_ogc_auth.py index 85f1ee3dd..3d3631982 100644 --- a/core/internal_ogc_auth.py +++ b/core/internal_ogc_auth.py @@ -26,42 +26,177 @@ Kept separate from core/permissions.py to avoid a circular import with core/pygeoapi.py. + +Three credential transports are accepted, because the desktop GIS clients +this mount exists for cannot all carry an Authentik bearer token: + + * ``Authorization: Bearer `` -- QGIS's OAuth2 authentication method, + scripts, and anything that can talk to Authentik directly. + * ``Authorization: Basic `` -- the only scheme ArcGIS + Pro's "Add OGC API connection" dialog supports with saved credentials + (Authentication > Server Authentication). Esri does not support + token-secured OGC service connections at all. + * ``?token=`` -- ArcGIS Pro's "Custom request parameters", which the + client re-appends to every request it issues. Also a workaround for the + QGIS regression where OGC API - Features requests dropped the + Authorization header (qgis/QGIS#60473). + +The Basic and query-parameter transports carry a *static API key* (see +`api_key_label`) rather than a JWT, since neither ArcGIS nor QGIS can refresh +an Authentik access token before it expires. A bearer JWT is still accepted +and still checked for INTERNAL_OGC_GROUP membership; an API key is a +pre-authorized stand-in for that same group. + +The query-parameter transport puts the secret in the request URL, which App +Engine's request log records. Prefer Basic where the client supports it, and +treat keys handed out for ArcGIS as log-exposed when rotating. """ +import base64 +import binascii +import hashlib +import hmac import json +import os +from urllib.parse import parse_qsl, urlencode from starlette.types import ASGIApp, Receive, Scope, Send from core import permissions from core.settings import settings +# Comma- or whitespace-separated `label:sha256hex` entries. The label is for +# operator bookkeeping (who holds this key) and never appears in a response. +API_KEYS_ENV = "INTERNAL_OGC_API_KEYS" + +# Query parameter carrying a credential. Stripped before the request reaches +# pygeoapi so it cannot trip pygeoapi's unknown-parameter handling or leak +# into a provider's filter parsing. +TOKEN_QUERY_PARAM = "token" + +# Sent on 401 so ArcGIS Pro and QGIS surface a credential prompt instead of a +# bare failure. +WWW_AUTHENTICATE = 'Basic realm="Ocotillo Internal OGC API", charset="UTF-8"' + + +def _configured_api_keys() -> dict[str, str]: + """Parse API_KEYS_ENV into {label: sha256hex}. + + Read fresh on every call for the same reason + permissions.authentication_disabled() is: an import-time snapshot diverges + from a value changed after import, and the two checks disagreeing is how + the earlier auth bugs in this codebase presented. + + Malformed entries are skipped rather than raising. A typo in one entry + must not take the whole mount down for every other key holder. + """ + raw = os.environ.get(API_KEYS_ENV) or "" + keys: dict[str, str] = {} + for entry in raw.replace(",", " ").split(): + label, sep, digest = entry.partition(":") + digest = digest.strip().lower() + if not sep or not label.strip() or len(digest) != 64: + continue + try: + int(digest, 16) + except ValueError: + continue + keys[label.strip()] = digest + return keys + + +def api_key_label(secret: str) -> str | None: + """Return the configured label for `secret`, or None if it matches none. + + Compared as SHA-256 hex with hmac.compare_digest so neither the stored + material nor the comparison timing reveals a valid key. + """ + configured = _configured_api_keys() + if not configured: + return None + presented = hashlib.sha256(secret.encode("utf-8")).hexdigest() + for label, expected in configured.items(): + if hmac.compare_digest(presented, expected): + return label + return None + -def _extract_bearer_token(scope: Scope) -> str | None: +def _extract_credential(scope: Scope) -> str | None: + """Pull a credential out of the Authorization header or ?token=. + + Header wins over query parameter, and within the header both Bearer and + Basic are accepted. For Basic, the password half carries the secret + (username ignored, conventionally "apikey"); a Basic credential with an + empty password falls back to the username so pasting a key into either + field of a connection dialog works. + """ headers = dict(scope.get("headers") or []) authorization = headers.get(b"authorization") - if not authorization: + if authorization: + scheme, _, param = authorization.decode("latin-1").partition(" ") + scheme = scheme.lower() + param = param.strip() + if scheme == "bearer" and param: + return param + if scheme == "basic" and param: + try: + decoded = base64.b64decode(param, validate=True).decode("utf-8") + except (binascii.Error, UnicodeDecodeError, ValueError): + return None + username, sep, password = decoded.partition(":") + if not sep: + return None + return password or username or None return None - scheme, _, param = authorization.decode("latin-1").partition(" ") - if scheme.lower() != "bearer" or not param: - return None - return param + + for key, value in parse_qsl( + (scope.get("query_string") or b"").decode("latin-1"), keep_blank_values=True + ): + if key == TOKEN_QUERY_PARAM and value: + return value + return None -async def _send_json(send: Send, status_code: int, detail: str) -> None: +def _strip_token_query_param(scope: Scope) -> Scope: + """Return `scope` with any ?token= removed, copied only if it was present. + + pygeoapi echoes the incoming query string into the `self` and `next` links + it emits; leaving the secret in place would publish it in every response + body as well as in the request log. + """ + query_string = scope.get("query_string") or b"" + if TOKEN_QUERY_PARAM.encode("latin-1") not in query_string: + return scope + pairs = parse_qsl(query_string.decode("latin-1"), keep_blank_values=True) + remaining = [(k, v) for k, v in pairs if k != TOKEN_QUERY_PARAM] + if len(remaining) == len(pairs): + return scope + scope = dict(scope) + scope["query_string"] = urlencode(remaining).encode("latin-1") + return scope + + +async def _send_json( + send: Send, status_code: int, detail: str, *, challenge: bool = False +) -> None: body = json.dumps({"detail": detail}).encode("utf-8") + headers = [(b"content-type", b"application/json")] + if challenge: + headers.append((b"www-authenticate", WWW_AUTHENTICATE.encode("latin-1"))) await send( { "type": "http.response.start", "status": status_code, - "headers": [(b"content-type", b"application/json")], + "headers": headers, } ) await send({"type": "http.response.body", "body": body}) class InternalOGCAuthMiddleware: - """Gates every request under `mount_path` behind INTERNAL_OGC_GROUP - membership; requests to any other path pass straight through untouched. + """Gates every request under `mount_path` behind an API key or + INTERNAL_OGC_GROUP membership; requests to any other path pass straight + through untouched. Registered via app.add_middleware(), which wraps the whole app -- the path check below is what keeps this scoped to the internal mount only. @@ -71,8 +206,13 @@ def __init__(self, app: ASGIApp, mount_path: str) -> None: self.app = app self.mount_path = mount_path + def _covers(self, path: str) -> bool: + # Segment-boundary match, not a bare startswith: with mount_path + # "/ogcapi" a plain prefix test would also swallow "/ogcapi-internal". + return path == self.mount_path or path.startswith(f"{self.mount_path}/") + async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: - if scope["type"] != "http" or not scope["path"].startswith(self.mount_path): + if scope["type"] != "http" or not self._covers(scope["path"]): await self.app(scope, receive, send) return @@ -87,25 +227,29 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: send, 424, permissions.bypass_misconfiguration_detail() ) return - await self.app(scope, receive, send) + await self.app(_strip_token_query_param(scope), receive, send) return - token = _extract_bearer_token(scope) - if not token: - await _send_json(send, 401, "Unauthorized") + secret = _extract_credential(scope) + if not secret: + await _send_json(send, 401, "Unauthorized", challenge=True) return - try: - payload = permissions.decode_token_payload(token) - except permissions.TokenInvalid: - await _send_json(send, 401, "Could not validate credentials") - return + if api_key_label(secret) is None: + # Not a static key, so it has to be an Authentik access token. + try: + payload = permissions.decode_token_payload(secret) + except permissions.TokenInvalid: + await _send_json( + send, 401, "Could not validate credentials", challenge=True + ) + return - if permissions.INTERNAL_OGC_GROUP not in payload.get("groups", []): - await _send_json(send, 403, "Forbidden") - return + if permissions.INTERNAL_OGC_GROUP not in payload.get("groups", []): + await _send_json(send, 403, "Forbidden") + return - await self.app(scope, receive, send) + await self.app(_strip_token_query_param(scope), receive, send) # ============= EOF ============================================= diff --git a/core/pygeoapi.py b/core/pygeoapi.py index 377d77c0f..017af4588 100644 --- a/core/pygeoapi.py +++ b/core/pygeoapi.py @@ -196,7 +196,16 @@ def _internal_server_url() -> str: configured = os.environ.get("PYGEOAPI_INTERNAL_SERVER_URL") if configured: return configured.rstrip("/") - return f"http://localhost:8000{_internal_mount_path()}" + # Derived from the application root rather than hardcoded to localhost. + # PYGEOAPI_INTERNAL_SERVER_URL is set in no deploy config -- not + # app.template.yaml, not any of the three CD workflows -- so every + # deployed environment fell into this branch and pygeoapi stamped + # "http://localhost:8000/ogcapi-internal" into the `self` and `next` links + # of every collection and items response. QGIS and ArcGIS Pro follow those + # links to page, so both walked off to localhost after the first page. + # _app_base_url() reads PYGEOAPI_SERVER_URL, which every deploy already + # sets, and still resolves to http://localhost:8000 for local development. + return f"{_app_base_url()}{_internal_mount_path()}" def _app_base_url() -> str: diff --git a/docs/internal-ogc-desktop-gis.md b/docs/internal-ogc-desktop-gis.md new file mode 100644 index 000000000..7efa66403 --- /dev/null +++ b/docs/internal-ogc-desktop-gis.md @@ -0,0 +1,139 @@ +# Connecting ArcGIS Pro and QGIS to `/ogcapi-internal` + +The internal OGC API mount serves the unfiltered (private- and draft-inclusive) +collections. It is gated by `core/internal_ogc_auth.py`, an ASGI middleware that +runs in front of the raw Starlette Mount — FastAPI's `Depends()` machinery never +sees these requests, so none of the `*_dependency` role parameters apply here. + +## Why there are static API keys at all + +The mount originally accepted only `Authorization: Bearer `. +Neither desktop client can sustain that: + +- **ArcGIS Pro** cannot send a bearer token to an OGC API connection. Its + connection dialog offers Basic ("Server Authentication"), Esri-portal OAuth, + and "Custom request parameters" (appended to the request URL). Esri + [does not support token-secured OGC service connections](https://pro.arcgis.com/en/pro-app/latest/help/data/services/add-ogc-api-services.htm). +- **QGIS** can send one via its OAuth2 or API Header authentication methods, but + shipped a regression where OGC API - Features requests dropped the + Authorization header entirely ([qgis/QGIS#60473](https://github.com/qgis/QGIS/issues/60473)). + +Neither client can refresh an Authentik access token before it expires, so even +a working bearer flow means re-pasting a token every hour. A static key issued +per user solves both problems. + +## Accepted credentials + +| Transport | Carries | Used by | +| --- | --- | --- | +| `Authorization: Bearer ` | Authentik JWT **or** API key | QGIS OAuth2 / API Header, scripts | +| `Authorization: Basic ` | API key (or JWT) as the password | ArcGIS Pro, QGIS Basic | +| `?token=` | API key (or JWT) | ArcGIS Pro custom request parameters | + +A JWT must additionally carry the `OGCInternal` group (`INTERNAL_OGC_GROUP` in +`core/permissions.py`); a valid JWT without it gets 403. An API key is a +pre-authorized stand-in for that group and carries no per-user claims. + +The `?token=` value is stripped from the query string before the request reaches +pygeoapi, so it never lands in the `self`/`next` links pygeoapi echoes into +response bodies. It is still recorded in App Engine's request log — prefer Basic +where the client supports it. + +## Where the keys live + +Only the **SHA-256 digests** are stored, never the keys themselves. The digest +list lives in a Google Secret Manager secret named `internal-ogc-api-keys`, one +per GCP project (production, staging, testing) — the same place the Jira and +Slack credentials live, not a GitHub secret. + +CD reads it at deploy time (`Fetch application secrets from Secret Manager` in +each `.github/workflows/CD_*.yml`) and `envsubst` renders it into `app.yaml` as +the `INTERNAL_OGC_API_KEYS` environment variable, which +`core/internal_ogc_auth.py` parses. The app makes no Secret Manager call at +runtime. + +Consequences worth knowing: + +- **The secret must exist before the next deploy of any environment.** + `get-secretmanager-secrets` fails the whole job on a missing secret. Seed each + project with a placeholder that parses to zero keys: + + ```bash + printf 'placeholder:none' | gcloud secrets create internal-ogc-api-keys --data-file=- --project + ``` + + The parser skips any entry whose digest is not 64 hex characters, so that + value is inert and means "bearer-JWT access only". + +- **Revoking a key requires a redeploy.** Adding a secret version does not + affect a running instance. If revocation ever needs to be immediate, that is + the point to switch to a runtime fetch with a TTL cache (same shape as the + JWKS cache in `core/permissions.py`) or to a keys table in Postgres. + +- The deploy service account needs `roles/secretmanager.secretAccessor` on + `internal-ogc-api-keys` in each project, alongside the four it already has. + +## Issuing a key + +```bash +python -c "import secrets,hashlib;k=secrets.token_urlsafe(32);print('key: ',k);print('digest:',hashlib.sha256(k.encode()).hexdigest())" +``` + +Give the **key** to the user over a secure channel and keep only the digest. +Append `