Stale operator-declared OAuth clients are never removed from durable storage
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 2.2k
- Forks
- 300
- Avg merge
- 1d 15h
- Merged PRs (30d)
- 184
Description
Summary
Operator-declared (configured) OAuth clients — delegate clients today, and static SPIFFE clients as of #6474 — are registered/reconciled into durable storage at server startup, but nothing ever removes a client's durable record when it is later dropped from RunConfig. The record (and, for SPIFFE, the durable client-ID claim added in #6474 to close a rolling-deployment race) persists forever until an operator manually deletes it from the storage backend.
This is pre-existing behavior for delegate clients (registerDelegateClients in pkg/authserver/server_impl.go only ever creates/reconciles, never deletes) and was inherited, not introduced, by the SPIFFE static-client work.
Impact
- Redis: a stale record survives indefinitely (no TTL) since it's operator-declared and therefore exempt from the DCR-issued eviction/TTL path.
- In-memory: not affected across restarts (state is ephemeral), but persists for the lifetime of a running process even after a config reload that would otherwise remove the association.
- A previously-configured client ID stays permanently reserved/unusable for DCR (SPIFFE case) or keeps its old secret/scopes/grants live (delegate-client case) even after the operator believes they removed it.
Possible approach
- Persist a marker distinguishing "configured/reconciled" rows (needed for other reasons too — see #6474 review).
- Add a startup (or periodic) reconciliation step that diffs currently-claimed configured client IDs against the current RunConfig and removes ones no longer configured, scoped per storage backend (Redis:
SCANover the client keyspace filtered to the marker; in-memory: iterate the map). - Needs care around blast radius/perf on
SCANat production scale, and around not deleting anything for an in-flight config validation error (only reconcile after successful validation).
References
- pkg/authserver/server_impl.go (
registerDelegateClients) - pkg/authserver/storage/spiffe_decorator.go (
preflightDurableCollisions, added in #6474) - pkg/authserver/storage/{memory,redis}.go (
RegisterClient/ReconcileConfiguredClient) - Discussed in PR #6474 review (JAORMX finding #1) and its follow-up fix
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in pkg/authserver/server_impl.go at registerDelegateClients, then read pkg/authserver/storage/spiffe_decorator.go and the RegisterClient/ReconcileConfiguredClient implementations in pkg/authserver/storage/memory.go and redis.go. Trace reconciliation after successful RunConfig validation, and compare configured client IDs with durable records in both backends. Done means removed configured clients no longer remain reserved or active, while DCR-issued records and invalid configurations are left untouched.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, redis
- Domain
- authentication, databases
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 52/100