goauthentik / goauthentik/authentik

providers/proxy: startup reconcile full-row re-saves every ProxyProvider from a stale snapshot, silently reverting concurrent writes

Open
#26,032 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
25.6k
Forks
2k
Avg merge
1d 2h
Merged PRs (30d)
659

Description

### Summary
`AuthentikProviderProxyConfig.proxy_set_defaults` (a `reconcile_tenant` method) iterates every `ProxyProvider` and calls `provider.save()` — a full-row save of an object loaded during iteration, with no `update_fields`, no per-row re-fetch, no lock, and no epoch guard. Because the reconcile runs on every gunicorn worker-id-1 boot and every dramatiq worker process boot, and a pass over a large provider set takes many minutes, passes overlap in normal operation and any write landing mid-pass is silently reverted with **zero events**.

### Environment
- authentik **2026.8.2** (latest stable at time of writing; same behavior reproduced in `main`)
- Deployment: server + worker containers, Postgres backend
- Scale where this manifests: ~190 proxy providers

### Mechanism (code references, all verified against `version/2026.8.2` and `main`)
- `authentik/providers/proxy/apps.py` — `proxy_set_defaults` is marked `@ManagedAppConfig.reconcile_tenant` and does:
```python
for provider in ProxyProvider.objects.all():
provider.set_oauth_defaults()
provider.save()
```
(this file is **byte-identical in `version/2026.8.2` and `main`** — blob SHA `4e9055b586b4cb528797033599a8e745b14e7c21`)
- `authentik/providers/proxy/models.py` — `set_oauth_defaults()` overwrites `grant_types`, `client_type`, `signing_key`, `include_claims_in_id_token`; **re-adds** the five managed scope mappings via `property_mappings.add(...)` (resurrecting operator-removed mappings; no signal if already present); and recomputes `redirect_uris` from `external_host`. The default queryset iteration fetches rows in chunks, so rows early in the loop are saved from snapshots that are minutes to tens-of-minutes old by the time they are written.
- `authentik/blueprints/apps.py` — `_reconcile_tenant` iterates ready tenants and invokes every `reconcile_tenant` method; **no lock, no epoch, no dirty-check**.
- `lifecycle/gunicorn.conf.py` — `post_worker_init` calls `root_app.call_startup()` for worker id 1 only (guard from #9081), but worker IDs are **reused** (`_next_worker_id`), and `max_requests` (default 1000, +50 jitter) recycles workers regularly, so each recycle of the id-1 worker re-runs the full pass.
- `authentik/tasks/middleware.py` — `StartupSignalsMiddleware.after_process_boot` runs the same startup signals for every dramatiq worker process.
- `authentik/root/asgi.py` — `call_startup` sends `startup`, which is what triggers the reconcile.

### Regression window
- `version/2025.6.0`: `providers/proxy/apps.py` contains no reconcile method.
- `version/2025.8.0`: the `reconcile_tenant` loop is present.
- Introduced by PR #14875 (commit `d65b8ae0295251b43e66a04cc2bce457b723575e`, 2025-06-05, "providers/proxy: set_oauth_defaults in reconcile instead of task") — this moved the work from a one-shot background task into a synchronous startup reconcile that re-runs per process boot.

### Observed behavior (production, 2026.8.2, ~190 proxy providers)
- Provider field writes (e.g. `authorization_flow`, custom redirect URIs, removed scope mappings) land successfully and then **revert minutes later**, matching a reconcile pass; no authentik event accompanies the revert.
- Full-row `UPDATE` statements on the provider rows from stale values; `pg_stat_user_tables.n_tup_upd` on the provider tables rises continuously **without corresponding events** — writes with no attributable action.
- `PATCH`/provider-update API calls intermittently take **2–13.5 s** (row-lock waits against the in-flight save pass).
- Pass duration scales with provider count: **~15–30 minutes for ~190 providers** — far beyond the cost of the work itself (per provider: one scope-mapping SELECT, one full-row UPDATE, M2M existence checks — milliseconds on a healthy database). The duration is dominated by lock contention among overlapping passes and concurrent API writes.
- Worker recycling under probe traffic: **~7–10 worker boots/hour**; with passes that outlive the recycling interval, passes are effectively always overlapping.
- Secondary effect: because `property_mappings.add()` re-adds the managed scope mappings and `redirect_uris` is recomputed from `external_host`, any deliberate narrowing of scopes or custom redirect URIs is undone by the next pass (see related issue #17074, which describes this subset for blueprint-applied providers).

### Reproduction
1. Any deployment with a few hundred proxy providers on 2026.8.2 (or main).
2. PATCH a proxy provider field that `set_oauth_defaults` does not itself set (e.g. `authorization_flow`, or a custom `redirect_uris` entry, or remove one of the five managed scope mappings).
3. Observe the write succeed, then revert within one reconcile cycle (≤ ~35 min at this scale), with no event emitted. Repeat the PATCH while a pass is running: latency spikes (seconds) as the save blocks on row locks.

Minimal code-level repro: the loop above is sufficient — any row-modified-after-fetch is lost at `.save()`.

### Suggested fix
- Save only the fields this function owns: `provider.save(update_fields=[...])` for the fields set by `set_oauth_defaults` (and replace the blanket `property_mappings.add` with a check that only adds genuinely missing defaults, or gate it entirely).
- Make the pass safe to overlap or impossible to overlap: take a lock (e.g. advisory lock) or use an epoch/updated-at guard so a pass cannot write rows modified after it fetched them; alternatively re-fetch each row (`select_for_update` or a fresh `.get()`) before save.
- Consider the TODO in the code: "figure out if this can be in pre_save + post_save signals" — moving to signals would eliminate the full-table pass entirely.
- At minimum, skip providers whose stored values already match the computed defaults.

### Related
- #17074 — "Proxy providers created by blueprint silently force the default oauth scope mappings" (open) — same function, M2M subset.
- #5015 — "Refactor Proxy outpost OAuth logic" (open) — umbrella.
- PR #14875 — the change that moved `set_oauth_defaults` into the reconcile path.

Contributor guide

Open the contributing guide

Research direction

Start in authentik/providers/proxy/apps.py and models.py, then trace the startup path through authentik/blueprints/apps.py, lifecycle/gunicorn.conf.py, authentik/tasks/middleware.py, and authentik/root/asgi.py. Reproduce the stale-save behavior with the stated PATCH steps and inspect the existing reconcile flow. Done means concurrent provider changes are not silently reverted, managed defaults remain correct, and overlapping startup passes no longer cause the reported write and lock issues.

Written by the indexing model from the issue text.

Assessment

Tech stack
django, postgresql, python
Domain
authentication, backend, databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.