Add optional OIDC bearer auth on remote/virtual config writes #136

Merged

benvin merged 4 commits from benvin/oidc-write-auth into master 2026-10-16 22:08:21 +11:00

Conversation 4 Commits 4 Files Changed 11

+618 -3

unkin-agent commented 2026-10-14 22:23:11 +11:00

Member

Copy Link

Copy Source

Remote and virtual configuration writes are unauthenticated, so anything that can reach the API can repoint or delete repos. Vault identity OIDC tokens give callers a verifiable identity.

• add ARTIFACTAPI_OIDC_ISSUER/ARTIFACTAPI_OIDC_AUDIENCE; unset issuer keeps auth off
• require a JWKS-verified bearer token on POST/PUT/DELETE of /api/v2/remotes and /api/v2/virtuals, 401 otherwise
• leave uploads, object deletes, probe, reads and /v2 open
• discover the issuer lazily with a 10s client timeout; fail fast with 503 for 15s after a failed discovery
• log the verified sub; allow Authorization in CORS

Remote and virtual configuration writes are unauthenticated, so anything that can reach the API can repoint or delete repos. Vault identity OIDC tokens give callers a verifiable identity. - add `ARTIFACTAPI_OIDC_ISSUER`/`ARTIFACTAPI_OIDC_AUDIENCE`; unset issuer keeps auth off - require a JWKS-verified bearer token on POST/PUT/DELETE of `/api/v2/remotes` and `/api/v2/virtuals`, 401 otherwise - leave uploads, object deletes, probe, reads and `/v2` open - discover the issuer lazily with a 10s client timeout; fail fast with 503 for 15s after a failed discovery - log the verified `sub`; allow `Authorization` in CORS

unkin-agent added 1 commit 2026-10-14 22:23:12 +11:00

Require OIDC bearer tokens on remote/virtual config writes when an issuer is set 638931d7ed

unkin-agent commented 2026-10-14 22:25:11 +11:00

Author

Member

Copy Link

Copy Source

• internal/auth/oidc.go:42 — context.WithoutCancel(ctx) is applied to the already-timeboxed ctx, so it drops the 10s deadline too; http.DefaultClient has no timeout, so a hung issuer blocks discovery indefinitely while holding b.mu, wedging every gated write (reproduced: 25s hang against a stalled issuer) → build the long-lived ctx from context.Background() for the provider, and put the 10s timeout on a dedicated http.Client (via oidc.ClientContext) or on a separate request-scoped discovery attempt.
• internal/auth/oidc.go:58 — failed discovery is retried on every request under the mutex, so concurrent writes queue serially and each re-hits the issuer → add a short failure backoff (e.g. cache last error for a few seconds) so a down issuer is not hammered.
• nit: internal/auth/oidc.go:63 — CutPrefix(..., "Bearer ") is case-sensitive and does not trim whitespace (RFC 7235 scheme is case-insensitive) → use strings.EqualFold on the scheme and TrimSpace the token.
• nit: internal/auth/oidc_test.go — no test for alg=none/HS256 tokens, wrong iss, or a lowercase/empty Bearer header → add negative cases.

- internal/auth/oidc.go:42 — `context.WithoutCancel(ctx)` is applied to the already-timeboxed ctx, so it drops the 10s deadline too; `http.DefaultClient` has no timeout, so a hung issuer blocks discovery indefinitely while holding `b.mu`, wedging every gated write (reproduced: 25s hang against a stalled issuer) → build the long-lived ctx from `context.Background()` for the provider, and put the 10s timeout on a dedicated `http.Client` (via `oidc.ClientContext`) or on a separate request-scoped discovery attempt. - internal/auth/oidc.go:58 — failed discovery is retried on every request under the mutex, so concurrent writes queue serially and each re-hits the issuer → add a short failure backoff (e.g. cache last error for a few seconds) so a down issuer is not hammered. - nit: internal/auth/oidc.go:63 — `CutPrefix(..., "Bearer ")` is case-sensitive and does not trim whitespace (RFC 7235 scheme is case-insensitive) → use `strings.EqualFold` on the scheme and `TrimSpace` the token. - nit: internal/auth/oidc_test.go — no test for alg=none/HS256 tokens, wrong `iss`, or a lowercase/empty `Bearer` header → add negative cases.

unkin-agent added 1 commit 2026-10-14 22:27:23 +11:00

Bound OIDC discovery with a client timeout and failure backoff 51a77e3446

unkin-agent commented 2026-10-14 22:29:25 +11:00

Author

Member

Copy Link

Copy Source

• internal/auth/oidc.go:48 — callers arriving while the first discovery is in flight get an instant 503 (|| b.discovering returns errIssuerUnavailable); terraform writes in parallel (default 10), so the first apply on a fresh pod sees spurious 503s → make concurrent callers wait for the in-flight discovery (singleflight or a done channel, bounded by the 10s client timeout) and fail fast only during post-failure backoff; do not hold b.mu while waiting.
• nit: internal/auth/oidc_test.go — no test for concurrent first requests against a slow-but-healthy issuer → add one asserting all N get 2xx/401 (not 503) with a single discovery hit, under -race.

- internal/auth/oidc.go:48 — callers arriving while the first discovery is in flight get an instant 503 (`|| b.discovering` returns errIssuerUnavailable); terraform writes in parallel (default 10), so the first apply on a fresh pod sees spurious 503s → make concurrent callers wait for the in-flight discovery (singleflight or a done channel, bounded by the 10s client timeout) and fail fast only during post-failure backoff; do not hold b.mu while waiting. - nit: internal/auth/oidc_test.go — no test for concurrent first requests against a slow-but-healthy issuer → add one asserting all N get 2xx/401 (not 503) with a single discovery hit, under -race.

unkin-agent added 1 commit 2026-10-14 22:31:37 +11:00

Wait on in-flight OIDC discovery instead of returning 503 98ad145b6f

unkin-agent commented 2026-10-14 22:34:47 +11:00

Author

Member

Copy Link

Copy Source

• internal/auth/oidc.go:61 — inflight is only cleared after oidc.NewProvider returns, with no defer/recover; a panic in discovery (chi Recoverer swallows it) leaves inflight set and done never closed, so every later caller blocks until its own request ctx ends and the backoff path is never reached, until restart → run the discovery in a func with defer that clears inflight, closes done and records failedAt on any exit.

- internal/auth/oidc.go:61 — `inflight` is only cleared after `oidc.NewProvider` returns, with no defer/recover; a panic in discovery (chi Recoverer swallows it) leaves `inflight` set and `done` never closed, so every later caller blocks until its own request ctx ends and the backoff path is never reached, until restart → run the discovery in a func with `defer` that clears `inflight`, closes `done` and records `failedAt` on any exit.

unkin-agent added 1 commit 2026-10-14 22:36:04 +11:00

Release OIDC discovery waiters when discovery panics 4d9f2bb693

unkin-agent commented 2026-10-14 22:37:49 +11:00

Author

Member

Copy Link

Copy Source

No findings.

No findings.

benvin merged commit 9161558334 into master 2026-10-16 22:08:21 +11:00

benvin deleted branch benvin/oidc-write-auth 2026-10-16 22:08:21 +11:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

No items

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

benvin (BenVincent) unkin-agent (Unkin Agent) unkinben (Ben Vincent)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: unkin/artifactapi#136