Round 4 review (PR #910), no blockers:
- poll(): a 2xx body with no `status` now fails fast (logged) instead of being
treated as perpetually pending until the deadline; defensive page index
(`p.get("index", i)`) so a malformed page degrades rather than KeyError-ing.
- Document on poll() that job_id is namespaced (embeds "/") so the gateway route
must be a path-capture param (GET /v1/ocr/batch/{job_id:path}).
- Drop the vestigial `status` + `updated_at` columns from batch_ocr_jobs: a row
only ever exists while pending (terminal jobs are deleted) and the live status
comes from a fresh poll, so a stored mirror was permanently "pending" /
redundant with submitted_at. Simplifies the migration, store, and dataclass.
- Tests: submit() ValueError on missing job_id; poll() missing-status → failed.
1653 unit tests pass; ruff + ty green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Round 1 review (PR #910):
- BLOCKING: add document_ocr_mode / _batch_poll_seconds / _batch_max_wait_seconds
to config._field_map — without it dynaconf silently ignored the env vars and
DOCUMENT_OCR_MODE=batch could never be enabled in production. Add a regression
test asserting the three round-trip from env.
- migration 008: give batch_ocr_jobs a composite PRIMARY KEY on
(user_id, doc_id, doc_type, etag) instead of a bare UniqueConstraint (N1).
- OcrProcessor: use a dedicated _batch_client_lock instead of sharing the sync
backend lock (N3).
- tests: use https:// gateway URLs in the new fixtures to clear SonarCloud's
"insecure http" security hotspots (all 14 were test-only http://gw literals).
1653 unit tests pass; ruff + ty green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add DOCUMENT_OCR_MODE=sync|batch (default sync). In batch mode the tier-3 OCR
processor submits documents to the embedding gateway's async Batch OCR routes
(POST /v1/ocr/batch + GET /v1/ocr/batch/{job_id}, astrolabe-cloud-website#372)
for ~50% cheaper large-corpus backfill. The direct Mistral OCR path is left
untouched. Tracked on Deck #332.
Batch jobs run minutes-hours, so the OCR tier cannot block (the procrastinate
worker reclaims jobs in `doing` after INGEST_STALLED_JOB_SECONDS). Instead it
submits, records the gateway job id in a new per-tenant `batch_ocr_jobs` table
(procrastinate args are immutable across retries), and raises a BatchPending
signal that TieredEscalationStrategy turns into a same-queue deferred re-poll —
releasing the worker slot between polls. On completion the per-page markdown is
indexed like the sync path; a failure or a job past
DOCUMENT_OCR_BATCH_MAX_WAIT_SECONDS marks the document parse-failed.
Batch is opt-in and gateway-only: with the direct mistral backend, no gateway
URL, or the inline/memory pipeline (which can't defer), it falls back to sync.
One batch job per document (coalescing N docs/job is a follow-up).
- embedding/gateway_batch_client.py: submit/poll client (reuses GatewayTokenProvider).
- vector/batch_ocr_store.py + migration 008: job tracking (portable SQLite+PG).
- document_processors/escalation.py: BatchPending control-flow signal.
- document_processors/ocr.py: batch state machine + sync fallback.
- vector/processor.py: thread doc identity to the OCR tier; raise BatchPending
from the pending sentinel; propagate it as control flow (not a failure).
- vector/queue/procrastinate.py: BatchPending -> same-queue retry_in, exempt
from the transient cap (bounded by the processor's deadline).
- config + docs; tests across client/store/processor/strategy/parse-tier.
1653 unit tests pass; ruff + ty green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Billing product model finalized (Deck #281): bill pages externally, record
tokens internally. Rename the data-plane metric literals to match the now-
canonical contract (Deck #284) — the control plane's METRIC_EVENT_NAMES is
already renamed, so the old names would be unmapped and never sync to Stripe.
Rename (values unchanged):
- embeddings_queries → tokens_embedded (value = real token count, already
emitted by this PR; the unit upstream providers bill on).
- pages_chunks → pages_embedded (value kept as len(chunk_texts) interim;
TODO(#282): real normalized "pages indexed" count — real pages for paginated
types, chars/tokens-per-page constant otherwise — is deferred to the
instrumentation card, this only lands the name/contract).
- All literals, log strings, docstrings, comments, the migration comment, and
tests renamed; grep confirms zero old strings remain.
Observability (new): export embedding token cost to Prometheus as
astrolabe_embedding_tokens_total{provider,operation} (operation = index|query)
so the billed cost unit is visible in Grafana, not just the per-tenant billing
DB. Dedicated counter (doesn't inflate the existing chunk/request metrics) and
always-on (independent of USAGE_METERING_ENABLED, so OSS/self-host gets it).
Wired on both the indexing batch embed and the search query embed (query inside
the per-request cache-miss branch, so reused embeddings aren't double-counted).
Note: the rename orphans any pre-existing embeddings_queries/pages_chunks rows
in tenant app DBs (CP no longer maps them) — acceptable; pipeline is inert with
throwaway dev/sandbox data.
Deck #284 (folded into PR #875).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Non-blocking follow-ups from the merge-ready review:
- semantic.py: bound the doc_types copied into embeddings_queries metadata
to _USAGE_METADATA_MAX_DOC_TYPES (16). doc_types is caller-supplied with
no max_length on the tool signature; capping the stored copy keeps one
JSONB row from ballooning (not a billing/injection risk — CP ignores
metadata, binds are parameterized).
- migration: note that `metric` is intentionally unconstrained Text and
that adding a third metric requires keeping the CP-side catalog in sync,
else the rollup silently ignores the new rows.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- hooks: document why user_id in metadata is safe — it stays tenant-local
(the CP rollup aggregates GROUP BY (day, metric) into usage_daily, which
has no metadata column, so it never reaches Stripe) and is retained to
keep Deck #67's future per-user attribution derivable from the app DB.
- migration: instantiate the SQLite-side column types (sa.Text() etc.) for
visual parity with the instantiated Postgres types.
- tests: assert the WARNING contract in the unserializable-metadata test
too; add an autouse fixture that resets UsageEventStore._shared_instance
so a stray shared() call can't leak across tests.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- store: guard UsageEventStore.shared() with a class-level anyio.Lock so
two concurrent cold-start callers don't both build (and one silently
overwrite) the cached instance — mirrors get_shared_storage(). Document
that tests should construct the store directly to avoid singleton leak.
- migration: rename 20260610 -> 20260607 and fix Create Date to today so
`alembic history` isn't future-dated (revision id 007 / down_revision
006 unchanged; single head verified).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- remove accidentally-committed .claude/scheduled_tasks.lock (Claude Code
runtime artifact swept in by `git add -A`) and gitignore it; the rest
of .claude/ stays tracked.
- store: cache UsageEventStore.shared() as a process-wide instance so the
hot search path doesn't allocate a fresh wrapper per metered query (the
wrapper is stateless beyond its storage handle).
- hooks: pass enabled=True directly (the outer guard already confirmed
the flag) instead of re-reading settings.usage_metering_enabled.
- migration: document the no-TTL retention design (control-plane rollup
owns the lifecycle; the data plane only appends).
- tests: assert the best-effort error path logs at WARNING (observability
contract).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- store: add optional `enabled` param to record_usage_event so hot-path
callers (nc_semantic_search) pass the already-resolved flag instead of
forcing a second uncached Settings build (ADR-024); falls back to
get_settings() when None so the store stays self-gating for standalone
use.
- hooks: thread enabled= through both call sites; bump the outer
shared()/construction failure log from debug → warning so "metering
enabled but no billing data" is visible at the default INFO level.
- migration: instantiate postgresql.JSONB() to match the sibling
TIMESTAMP(timezone=True) column.
- tests: fix the misleading "asyncpg returns JSONB as a JSON string"
comment; add occurred_at dialect round-trip test and an enabled-param
short-circuit test.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Deck #67 data-plane slice: tenant Pods record billable operations
(embedding queries, pages/chunks embedded) into an app-DB usage_events
table that the control plane later pulls read-only into the billing
ledger and syncs to Stripe Meter Events.
- migration 007: usage_events table (Postgres TIMESTAMPTZ/JSONB/UUID
with portable SQLite fallbacks), indexed (occurred_at, metric) for the
CP rollup's per-day range scan + GROUP BY metric.
- UsageEventStore: best-effort, flag-gated writer reusing the shared
RefreshTokenStorage engine; ON CONFLICT (event_id) DO NOTHING for
idempotent retries; dialect-branched occurred_at bind. All work
(incl. metadata JSON encode) is swallowed so a metering failure never
surfaces to the user op.
- USAGE_METERING_ENABLED flag (default off) wired through Settings +
env map; off-path touches no storage, so OSS self-hosters get an empty
table and zero write overhead.
- two recording hooks: embeddings_queries (per nc_semantic_search, which
nc_semantic_search_answer reuses) and pages_chunks (after dense
embedding succeeds, covering both in-process and procrastinate paths).
- storage.acquire()/.dialect public seams so the sibling store doesn't
reach into the underscored internal.
- tests parametrized over SQLite + Postgres: flag-off no-op, roundtrip,
ON CONFLICT dedup, JSON/NULL metadata, and the best-effort swallow of
both DB errors and unserializable metadata.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Round-3 fixes. Two threads:
- 9 OPEN SonarQube issues caused the "E Security Rating on New Code"
gate failure. The bot's diagnosis (sa.text(text_sql) → SQL injection)
was a wrong guess; the actual SQ rules firing were different.
- Bot's substantive concerns: pool defaults too aggressive,
delete_browser_session RETURNING path untested on Postgres,
schema_version legacy table created on Postgres, stale module
docstring.
- User's underlying question on the pool: "isn't 1 connection enough?"
Right-sized to 2+5 and documented the concurrency model in ADR-026
so the rationale is durable.
SonarQube quality-gate fixes (clears all 9 OPEN issues)
-------------------------------------------------------
- BLOCKER S6418: rename `SECRET` constant in test_storage_logging.py
to `SENTINEL_PASSWORD_FRAGMENT` + NOSONAR with rationale.
- CRITICAL S3776: extract `_build_postgres_engine()` from
`initialize()` (was complexity 26 > 15); incidentally creates a
clean unit-test seam for engine args.
- CRITICAL S4423: `ssl.create_default_context(cafile=...)` is flagged
as "weak protocol" — Python 3.10+ already negotiates the strongest
available protocol. Explicitly pass `purpose=ssl.Purpose.SERVER_AUTH`
and NOSONAR with the Python-version rationale.
- MAJOR S3358: split the TLS-mode nested ternary in the engine
factory into a `_describe_ssl_arg()` helper.
- MAJOR S2068 ×3: bind test app-password literals to local vars and
put `# NOSONAR S2068` on the same line as the literal (anchoring
requirement) instead of on the closing paren.
- MINOR S7503 ×2: `# NOSONAR S7503` on `_Cursor.__aenter__/__aexit__`
— they MUST be `async` per the context-manager protocol.
Pool sizing right-sized (answers "why so many connections?")
------------------------------------------------------------
- `DATABASE_POOL_SIZE` default 10 → **2**.
- `DATABASE_MAX_OVERFLOW` default 20 → **5**.
- Per-pod max drops from 30 to 7. With 3 replicas, total = 21
connections (was 90) — well under managed-Postgres
`max_connections=100`.
- New INFO log at startup: `Postgres engine ready: pool_size=N
max_overflow=M (per-pod max K connections)`. Surfaces the active
sizing without grepping config.
- New ADR-026 § "Concurrency model and pool sizing" explains
asyncpg's single-flight connection semantics, the MCP workload
shape (read-mostly point lookups), why-not-1 (multi-user
serialization), and the tune-up/tune-down recipe.
- `docs/configuration.md` table updated with new defaults +
homelab-vs-prod tuning guidance, linking the ADR.
RETURNING path covered on Postgres
----------------------------------
- New `test_browser_session_delete_returning` exercises the
`DELETE … RETURNING user_id` path — the only RETURNING clause in
the storage layer and the most dialect-sensitive SQL in this PR.
Asserts both present-row (returns True, row gone) and absent-row
(returns False) branches.
Schema portability polish
-------------------------
- `alembic 001`: gate `schema_version` table creation on
`op.get_bind().dialect.name == "sqlite"`. The table exists purely
to match the fingerprint of pre-Alembic SQLite databases; fresh
Postgres installs no longer carry the dead legacy table.
Misc polish
-----------
- Module docstring: "SQLite-based" → "SQL-backed", with a sentence
on the DATABASE_URL opt-in and an ADR-026 link.
- Comment on `_wrap_row` noting `row._mapping` is the documented
RowMapping accessor in SQLAlchemy 2.x despite the underscore.
Skipped (rationale in PR reply)
-------------------------------
- `_qmark_to_named` SQL-comment handling: docstring already notes
the limitation; no `?` in storage SQL comments today.
- Module-level `anyio.Lock()`: established precedent confirmed by
the bot itself.
- `get_audit_logs` `SELECT *`: pre-existing pattern, out of scope.
Verification
------------
- `uv run pytest tests/unit/` — 1025 passed.
- `TEST_DATABASE_URL=… uv run pytest tests/integration/test_storage_postgres.py -m postgres` — 7 passed.
- `ruff check && ruff format --check && ty check` — clean.
- Confirmed `schema_version` absent on fresh Postgres, still present
on fresh SQLite.
Tracked on Astrolabe Cloud POC board, card #99.
---
_This PR was generated with the help of AI, and reviewed by a Human_
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Round-2 fixes after the bot review on PR #798 plus two user follow-ups
(self-signed Postgres support; asyncpg should be a PyPI extra). Folded
into the same PR rather than a follow-up since the work is still
unmerged.
Security
--------
- Mask database credentials in all 5 log call sites (storage.py × 4,
migrations.py × 1) via a new `mask_db_password()` helper in config.py.
Uses SQLAlchemy's `make_url(...).render_as_string(hide_password=True)`
with a regex fallback so the masking path never raises.
- New `tests/unit/test_storage_logging.py` asserts a sentinel password
never appears in `caplog` during `RefreshTokenStorage.initialize()`.
Distribution
------------
- `asyncpg` moved to `[project.optional-dependencies] postgres` so a
vanilla `pip install nextcloud-mcp-server` no longer pulls in the
~5 MB C extension. The Docker image runs `uv sync --extra postgres`,
so containerized deployments are unchanged.
- When `DATABASE_URL=postgresql+asyncpg://...` is set on a venv missing
the extra, `RefreshTokenStorage.initialize()` raises a friendly
RuntimeError pointing at `[postgres]` rather than the generic
ModuleNotFoundError.
TLS for the Postgres backend
----------------------------
- New `DATABASE_VERIFY_SSL` + `DATABASE_CA_BUNDLE` env vars mirror the
existing `NEXTCLOUD_VERIFY_SSL` / `NEXTCLOUD_CA_BUNDLE` pattern
(validators in Settings.__post_init__, `get_database_ssl()` helper
alongside `get_nextcloud_ssl_verify()`). `DATABASE_VERIFY_SSL=false`
wins over `DATABASE_CA_BUNDLE` for incident-response convenience.
- Default is **None** rather than True — keeps PR #798's behavior
intact for cluster-internal Postgres that runs without TLS. Operators
opt into verify-full or supply a private CA. ADR-026 records the
reasoning vs the Nextcloud HTTPS default.
- Engine factory in `storage.py` passes `ssl` via `connect_args` only
when `get_database_ssl()` returns non-None; otherwise asyncpg's
default (`prefer`) applies.
- Storage logs which TLS mode is active at INFO (no secret material).
Configurable connection pool
----------------------------
- `DATABASE_POOL_SIZE` (default 10) and `DATABASE_MAX_OVERFLOW`
(default 20) replace the hardcoded engine values. With many replicas
this can blow past managed-Postgres `max_connections=100`; tune down
for large fleets.
- gte-1 / gte-0 validators in __post_init__ reject 0/negative pool
sizes at startup with the offending value in the error.
Consistency polish
------------------
- Migration 006: convert raw `op.execute("ALTER TABLE ... ADD COLUMN")`
to `op.batch_alter_table(...).add_column(sa.Column("nonce", sa.Text))`
for stylistic consistency with the rewritten 001-005. Downgrade now
drops the column instead of being a no-op.
- `registered_webhooks.created_at` standardized from `sa.Float` to
`sa.BigInteger` (all other `*_at` columns); `store_webhook()` casts
`time.time()` → `int`.
- `is_sqlite_url()` made case-insensitive.
Testing
-------
- New `tests/integration/test_storage_postgres.py::test_cleanup_expired_roundtrip`
exercises `cleanup_expired_tokens`, `cleanup_expired_sessions`, and
`cleanup_expired_browser_sessions` — relies on DELETE rowcount,
historically dialect-tricky.
- `tests/unit/test_ssl_config.py` extended with `TestDatabaseSSLSettings`
+ `TestGetDatabaseSSL` classes (9 new tests) mirroring the existing
Nextcloud SSL tests one-for-one.
Docs
----
- `docs/configuration.md` Centralized-Storage section grew the four new
env vars + a homelab example with a private CA.
- `docs/ADR-026` grew Distribution, TLS, and `alembic/env.py` async-pattern
subsections explaining the non-obvious design choices.
Helm chart counterpart in cbcoutinho/helm-charts PR #34 (separate
commit on `feat/nextcloud-mcp-server-database-url`).
Verification
------------
- `uv run pytest tests/unit/` — 1025 passed.
- `TEST_DATABASE_URL=... uv run pytest tests/integration/test_storage_postgres.py -m postgres` — 6 passed (including new cleanup test).
- `uv run ruff check && uv run ruff format --check && uv run ty check -- nextcloud_mcp_server` — clean.
Tracked on Astrolabe Cloud POC board, card #99.
---
_This PR was generated with the help of AI, and reviewed by a Human_
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a `DATABASE_URL` setting that lets `RefreshTokenStorage` run against
any SQLAlchemy async backend, primarily `postgresql+asyncpg://...` for
HA k8s deployments. Default behavior is unchanged: when `DATABASE_URL` is
unset the server falls back to the existing `TOKEN_STORAGE_DB` path /
ephemeral SQLite tempfile.
Why
---
Today every MCP pod needs its own PVC to hold the SQLite file, which
pins the Deployment to one replica and blocks horizontal scaling. With
this change, operators can point all replicas at a shared Postgres
(CNPG, RDS, etc.) and the pods become stateless. Encryption stays in
Python (Fernet); the database only sees ciphertext.
What changed
------------
- `config.get_database_url()` resolves DATABASE_URL → TOKEN_STORAGE_DB →
ephemeral tempfile in that priority order.
- `RefreshTokenStorage` builds a process-shared `AsyncEngine` in
`initialize()`. SQLite gets NullPool; Postgres gets pool_size=10,
max_overflow=20, pool_pre_ping=True. 30 aiosqlite call sites adapted
via a thin `_DBConn` / `_Cursor` / `_Row` / `_ExecuteCtx` shim so
existing method bodies need no churn beyond the connection
context-manager swap.
- 7 `INSERT OR REPLACE` statements rewritten as portable
`INSERT ... ON CONFLICT (...) DO UPDATE` (SQLite ≥ 3.24, Postgres ≥ 9.5).
- `sqlite_master` legacy-detection lookup replaced with SQLAlchemy
inspector so the path works against either backend.
- File-permission hardening + parent-dir creation gated on
`is_sqlite_url(...)` — centralized backends manage their own filesystem.
- Alembic migrations 001/002/003/005 converted from raw `op.execute(SQL)`
to portable `op.create_table()` / `op.create_index()` with SQLAlchemy
types. All timestamp columns are `sa.BigInteger` so Postgres allocates
BIGINT (unix epochs don't fit in INT4). SQLite treats BIGINT as
INTEGER, so existing deployments at revision 006 see no schema drift.
- `migrations.py` + CLI take URLs; `db {upgrade,downgrade,current,history}`
gain `--database-url / -u` alongside the legacy `--database-path / -d`.
`get_current_revision()` uses SQLAlchemy inspector instead of raw
sqlite3, so the CLI works against Postgres too.
- `docker-compose.yml` adds a `postgres-test` service under the
`postgres` profile (pinned `postgres:16-alpine` digest) for
integration testing.
- Unit storage tests parametrized over backends via shared
`tests/fixtures/storage_backend.py` — every test in
`test_app_password_storage.py` and `test_webhook_storage.py` runs
once per backend that is available. Postgres is opted in by
`TEST_DATABASE_URL`.
- New `tests/integration/test_storage_postgres.py` (5 tests, marked
`postgres` + `integration`) covers refresh-token, app-password,
OAuth-session, webhook, and audit-log paths end-to-end on Postgres.
- New `docs/ADR-026-pluggable-database-backend.md` records the decision;
`docs/configuration.md` documents `DATABASE_URL` with examples.
Out of scope
------------
- No SQLite → Postgres data migration tool (clean cutover; tokens reissue
on next login, webhooks re-register on next sync tick).
- This repo does not provision Postgres. The matching helm chart change
lives in cbcoutinho/helm-charts (database.url / existingSecret values).
Verification
------------
- `uv run pytest tests/unit/` — 1012 passed, SQLite path unchanged.
- `docker compose --profile postgres up -d postgres-test`
- `TEST_DATABASE_URL=... uv run pytest tests/integration/test_storage_postgres.py -m postgres -v`
— 5 passed.
- `TEST_DATABASE_URL=... uv run pytest tests/unit/test_app_password_storage.py
tests/unit/test_webhook_storage.py` — 50 passed (25 per backend).
- `uv run ruff check && uv run ruff format --check && uv run ty check -- nextcloud_mcp_server` — clean.
Tracked on Astrolabe Cloud POC board, card #99.
---
_This PR was generated with the help of AI, and reviewed by a Human_
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Blocking:
- AS proxy callback now calls verify_id_token before caching the proxy
code so a tampered IdP response can't smuggle identity claims.
Important:
- Browser OAuth flow generates and verifies an OIDC nonce; new alembic
migration 006 adds the nonce column to oauth_sessions.
- _origin_matches_self logs a warning when CSRF check is bypassed.
- oauth_tools.py uses get_shared_storage instead of fresh handles.
Nits:
- New token_utils.get_oidc_discovery shares the 5-minute cache with
verify_id_token; oauth_login (integrated) and _revoke_refresh_token_at_idp
now use it instead of issuing fresh discovery fetches.
- Drop typing.Optional from oauth_tools.py in favour of X | None.
CI:
- test.yml generates an ephemeral Fernet TOKEN_ENCRYPTION_KEY per run
with openssl, removing the dependency on a missing repo secret.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pre-launch hardening for the hosted Astrolabe Cloud offering. Addresses
all five findings raised in #626 (Tim Kaufmann, code review of v0.65.0).
Re-verified against master before fixing.
Finding 3 (LLM-controllable user_id) — drop user_id from the public
signatures of provision_nextcloud_access, revoke_nextcloud_access,
check_provisioning_status, check_logged_in. Tool wrappers now always
derive identity from the verified AccessToken; user_id is no longer
accepted as MCP input. Adds parameterized CI-guard test that locks the
schema.
Finding 2 (predictable session cookie) — replace mcp_session=<user_id>
cookie with a cryptographically random session_id mapped server-side
(new browser_sessions table, alembic 005). Cookie value is opaque,
expires, revocable. SessionAuthBackend looks up user_id via the new
mapping and additionally requires a refresh token to fail closed.
Finding 4 (logout doesn't revoke refresh token) — oauth_logout now
calls the IdP revocation_endpoint (RFC 7009) when advertised, deletes
the stored refresh token regardless, and clears the browser_sessions
row. Cleanup is best-effort: logout always 302s.
Finding 1 (unverified ID token decodes) — verify_id_token helper does
JWKS signature + issuer + audience + exp + nonce checks per OIDC core
3.1.3.7. Used by both OAuth callback handlers (browser + MCP). Removes
the four "verify_signature: False" decodes that previously trusted IdP
claims unconditionally. Drops dead-code _validate_token_audience in
token_broker. Refactors token_utils + provisioning_decorator to read
user_id from the verified AccessToken instead of re-decoding the JWT.
Finding 5 (hardcoded Fernet keys in docker-compose.yml) — replace the
three inline TOKEN_ENCRYPTION_KEY values with required env var
interpolation; document in env.sample.
Test coverage: 4 new unit test modules (signature pinning, browser
sessions, ID-token verification, logout + revoke + session backend).
693 unit tests pass; ruff/format/ty clean.
Migration note: existing browser admin-UI sessions become invalid on
rollout (cookies are looked up against the new browser_sessions table,
which starts empty). Users re-login. MCP API access is unaffected.
Tracked on Astrolabe Cloud POC board card #37.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Many identity providers (AWS Cognito, Okta, Azure AD) reject or mishandle
colons in OAuth scope names. This migrates all custom scopes from
`resource:action` to `resource.action` format (e.g., `notes:read` →
`notes.read`), which is universally accepted and aligns with industry
conventions (Microsoft, Google).
Includes Alembic migration 004 for stored scope strings and ADR-024
documenting the rationale and RFC references.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Previously, the multi-user BasicAuth mode attempted to retrieve app passwords
via OAuth client_credentials grant, which Nextcloud OIDC doesn't support.
This fix implements local storage for app passwords:
- Add app_passwords table via Alembic migration (002)
- Add store/get/delete methods to RefreshTokenStorage
- Add management API endpoints for app password provisioning:
- POST /api/v1/users/{user_id}/app-password
- GET /api/v1/users/{user_id}/app-password
- DELETE /api/v1/users/{user_id}/app-password
- Update oauth_sync.py to read from local storage
- Update Astrolabe to send app passwords to MCP server after validation
- Add app-hook to configure mcp_server_url in Nextcloud
The flow is now:
1. User creates app password in Nextcloud Security settings
2. User enters it in Astrolabe Personal Settings
3. Astrolabe validates against Nextcloud, then sends to MCP server
4. MCP server stores encrypted app password locally
5. Background sync uses locally stored password
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Move alembic/ directory to nextcloud_mcp_server/alembic/ subpackage
- Update migrations.py to use package location instead of alembic.ini
- Update env.py to set script_location dynamically
- Update alembic.ini for development CLI usage
- Fix Dockerfile typo: .vnev -> .venv
This fixes FileNotFoundError when running in Docker with non-editable
install. The alembic package is now installed with the main package,
making it work in both development and production environments.
Resolves: Docker startup error 'alembic.ini not found at
/app/.venv/lib/python3.12/site-packages/alembic.ini'
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
