Skip to content

OIDC pending-login state is per-process, so a second API replica breaks redirect logins #109

Description

@eth-man

Latent today — the shipped compose file runs one API container — but it constrains any future scale-out, and it would present as an intermittent login failure rather than an obvious one.

The mechanism

OidcAuthProvider holds in-flight logins in a per-instance Map: the PKCE verifier and the nonce, keyed by state. Core's half of the correlation is fine — that state lives in a browser cookie — but the verifier and nonce exist only in the process that began the login.

Behind two API replicas and a load balancer, a callback routed to the other replica finds no pending entry and refuses with "an unknown or expired state". Roughly half of logins fail, non-deterministically, and retrying sometimes works — which is the worst shape a bug can have.

The bounded-size eviction has a milder version of the same problem: under load, PENDING_MAX evicts oldest-first, so a login begun during a burst can be dropped before its callback arrives even on a single replica.

Why file it now

Two reasons beyond scale-out:

  • Any future SAML provider needs an assertion replay cache with exactly this shape, so the storage decision wants making once rather than twice.
  • The audit delivery worker and the ENC materializer already solve single-flight across replicas with Postgres advisory locks, so the codebase has an established answer to "shared state across replicas" that this does not use.

Options

  1. Document single-replica as a constraint for redirect-mode auth, and fail loudly if more than one replica is detected (hard to detect honestly).
  2. Move pending logins to Postgres — a small table keyed by state with a TTL and a sweeper, mirroring the delivery outbox's shape. Costs one write and one read per login, which against a network round trip to the identity provider is nothing.
  3. Sticky sessions at the proxy — works, but pushes a correctness requirement into deployment configuration where nothing enforces it.

My inclination is (2): it is small, it matches an existing pattern in this codebase, and it removes the constraint rather than documenting it. But it is a schema change, so it deserves a decision rather than a drive-by.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingneeds-triageMaintainer needs to evaluate this issue

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions