Commit Graph
2 Commits
Author SHA1 Message Date
Chris LuandGitHub f8973b3ed6 feat(iam): OIDC provider mutations + multi-client + TLS thumbprints (Phase 2b) (#9320)
* feat(iam): OIDC provider mutations + multi-client + TLS thumbprints

- Mutating IAM actions: CreateOpenIDConnectProvider,
  DeleteOpenIDConnectProvider, AddClientIDToOpenIDConnectProvider,
  RemoveClientIDFromOpenIDConnectProvider,
  UpdateOpenIDConnectProviderThumbprint, TagOpenIDConnectProvider,
  UntagOpenIDConnectProvider. Each enforces AWS-shape input bounds and
  the read-only mode rejects all mutations.
- Multiple client_ids per provider in OIDCConfig (clientIds list, plural)
  with full backward compatibility — singular clientId still works and is
  merged into the audience allowlist. Provider factory accepts both.
- AWS-compatible TLS thumbprint pinning: when OIDCConfig.Thumbprints is
  non-empty, JWKS fetches enforce that the negotiated TLS chain contains
  a certificate whose SHA-1 hex matches the allowlist. Empty list keeps
  the existing system-trust path.

* fix(iam): factor Tags.member.N parser into a helper

CreateOpenIDConnectProvider and TagOpenIDConnectProvider were both
walking the AWS Tags.member.N.Key / Tags.member.N.Value query-string
convention with copy-pasted loops. Factor into extractTags so the
parsing rules and the "no tags present" semantics live in one place.

Addresses gemini medium review on PR #9320.

* fix(iam): sentinel errors for OIDC provider not-found / already-exists

The s3api dispatcher was using strings.Contains(err.Error(), "not found")
and "already exists" to map IAM-manager errors back to AWS error codes.
Substring matching on a formatted message couples the API error code
to the exact wording of the upstream message — touching the message
silently changes the IAM API contract.

Define ErrOIDCProviderNotFound and ErrOIDCProviderAlreadyExists in the
integration package, fmt.Errorf("%w: ...") them at the four return
sites in iam_manager.go and oidc_provider_store.go, and use errors.Is
at the s3api call sites. Same control flow, no string-match fragility.

Addresses gemini medium review on PR #9320.

* fix(iam): surface non-NotFound errors from CreateOIDCProvider lookup

Previously CreateOIDCProvider only treated GetProviderByARN's success path
as "exists" and silently fell through on any error, including transient
backend failures. That hid real problems and still attempted a write.
Distinguish ErrOIDCProviderNotFound (the only "safe to create" case) from
other errors so we don't mask filer outages or partition issues.

* fix(iam): enforce 100-client-ID cap on AddClientIDToOIDCProvider

CreateOIDCProvider and the implicit update path through validateOIDC-
ProviderRecord both reject lists with more than 100 client IDs, but
AddClientIDToOIDCProvider could grow the list past that bound one
ID at a time. Refuse the add when the list is already at the cap so
the invariant holds across every mutation entry point.

* feat(iam): IAM-managed OIDC provider live view in STS service

Add a separate, mutex-guarded map of admin-managed OIDC providers on
the STS service. The map can be atomically replaced via
SetIAMManagedOIDCProvidersByIssuer; AssumeRoleWithWebIdentity lookups
consult it first and fall back to the existing static-config map, so
records persisted through the IAM API can shadow bootstrap entries
without a restart.

This is the runtime hook the IAM API and the metadata-subscribe path
will both call when the OIDCProviderStore changes (next two commits).

* feat(iam): refresh STS service runtime view after OIDC mutations

Add IAMManager.RefreshOIDCProvidersFromStore: lists every persisted
OIDCProviderRecord, builds a runtime OIDCProvider for each, and atomically
publishes the issuer-keyed map into the STS service. Each mutating IAM API
call (Create / Delete / AddClientID / RemoveClientID / UpdateThumbprints)
now triggers this refresh inline so the local instance picks up the change
without waiting for a metadata-subscribe round trip. Tag mutations skip
the refresh because tags do not affect token validation.

Refresh failures only log; the persisted write has already succeeded by
that point, so a transient list error must not surface to the API caller.
The peer-instance update path (filer metadata subscription) is added in a
follow-up commit.

* feat(iam): subscribe to OIDC provider changes on the filer

Watch /etc/iam/oidc-providers under the existing s3 metadata-subscribe
loop and call RefreshOIDCProvidersFromStore on any create / update /
delete / rename. This is the cross-instance update path: S3 server A
writes via the IAM API, the filer fans out the metadata change, and S3
servers B..N pick up the new runtime view without a restart.

Mirrors the existing onIamConfigChange / onCircuitBreakerConfigChange
pattern. The handler short-circuits when the path is unrelated, and
when no IAMManager is wired in (static-only configurations).
2026-05-05 11:26:08 -07:00
Chris LuandGitHub 4ded97a321 feat(iam): OIDC provider store + read-only IAM API (Phase 2a) (#9319)
* feat(iam): STS web-identity AWS-fidelity polish

- OIDC discovery via .well-known/openid-configuration; falls back to
  /.well-known/jwks.json when discovery is absent. Reject discovery docs
  whose issuer claim does not match the configured issuer to defend
  against issuer-substitution.
- ComputeParentUser derives a stable per-identity hash from (sub, iss).
  Surface as aws:userid in the request context and as a parent_user
  claim in the session JWT so per-user state survives token rotation.
- Per-role MaxSessionDuration (3600..43200) clamps requested
  DurationSeconds before the STS service applies its own caps.
- Tighten RoleSessionName to the AWS contract: 2..64 chars from
  [\w+=,.@-].
- Populate PackedPolicySize in AssumeRole / AssumeRoleWithWebIdentity /
  AssumeRoleWithLDAPIdentity responses as a percentage of the 2048-byte
  inline session policy budget.

* fix(iam): leave omitted DurationSeconds nil so STS default applies

capDurationByRole was substituting the role's MaxSessionDuration
when the caller omitted DurationSeconds entirely. AWS returns the
configured default (typically 1 hour) in that case, not the role's
upper bound — a 12h MaxSessionDuration shouldn't silently make every
no-duration assume-role mint a 12h session.

Return nil when requested is nil; let the downstream
calculateSessionDuration in the STS service apply its TokenDuration
default. The role-max upper bound still clamps when the request
arrives with a concrete value above the cap.

Addresses gemini high-priority review on PR #9318.

* fix(iam): synchronize OIDCProvider JWKS cache fields

jwksCache, jwksFetchedAt, resolvedJWKSUri, and discoveryFailed are
mutated lazily on the first token-validate call and refreshed
afterwards on TTL expiry. Multiple S3 requests can land here in
parallel, so the writes were racing against subsequent reads on
every other goroutine. resolvedJWKSUri/discoveryFailed inherited
the same un-protected pattern when discovery shipped.

Add sync.RWMutex; getPublicKey takes the read lock for the
common cache-hit path and promotes to the write lock for misses
+ refreshes. fetchJWKSLocked / resolveJWKSUriLocked assume the
write lock is held by the caller; fetchJWKS keeps the
test-friendly entry point that acquires the lock itself.

Addresses gemini high-priority review on PR #9318.

* fix(iam): trim trailing slash + retry discovery after transient failure

Two OIDC discovery edge cases reviewers flagged:

1. Issuer comparison was sensitive to trailing slashes. resolveJWKSUri
   trims them when building the discovery URL, but the doc.Issuer ↔
   p.config.Issuer check did not, so an IDP whose issuer claim drops or
   adds the slash relative to the configured value would be falsely
   rejected. Trim a single trailing slash on each side before comparing.

2. discoveryFailed flipped to true on any error and stayed there for the
   process lifetime. A transient 5xx at startup permanently locked the
   provider into the /.well-known/jwks.json fallback. Reset the flag at
   the top of fetchJWKSLocked when no URI has been cached yet, so each
   JWKS refresh (typically once per TTL = 1h) reattempts discovery.
   Successful discovery remains cached via resolvedJWKSUri so we don't
   pay the discovery RTT on every refresh.

Addresses gemini security-medium + medium reviews on PR #9318.

* fix(iam): require non-empty issuer in OIDC discovery doc

The previous "doc.Issuer != "" && ..." guard let a discovery document
that omitted the issuer field bypass the issuer-mismatch check
entirely, letting the doc steer fetchJWKS at any URL it provided.
OIDC Discovery 1.0 §3 mandates the issuer field; treat missing as a
hard failure same as mismatched. Trailing-slash equivalence still
applies.

Adds TestDiscoveryRejectsMissingIssuer alongside the existing
TestDiscoveryRejectsIssuerMismatch via a new omitDiscoveryIssuer
toggle on fakeIDP.

* feat(iam): OIDC provider store + read-only IAM API

Add OIDCProviderRecord — the persisted, IAM-managed view of an OIDC
identity provider — and an OIDCProviderStore interface with memory and
filer implementations mirroring the existing role-store pattern.

The store is hydrated at boot from the static STS.Providers list so the
new IAM API surfaces the same set the STS service already validates
against. Two read-only actions land now:

- ListOpenIDConnectProviders -> ARN-only list, AWS-shape XML.
- GetOpenIDConnectProvider   -> URL, ClientIDList, ThumbprintList,
                                Tags, CreateDate.

Mutations (Create/Delete/Add-Remove ClientID/Update Thumbprint), multiple
client_ids per provider, and TLS thumbprint pinning come in Phase 2b.

* fix(iam): preserve CreatedAt across boots + paginate ListProviders

Two medium-priority issues gemini flagged on the read-only IAM API:

1. The static-config bootstrap was setting CreatedAt = time.Now() on
   every server start, so the IAM GetOpenIDConnectProvider response's
   CreateDate shifted on each restart even when backed by a persistent
   store. Look up the existing record via GetProviderByARN first and
   preserve its CreatedAt; only the UpdatedAt advances.

2. FilerOIDCProviderStore.ListProviders had a hardcoded Limit: 1000
   that silently truncated above that. Stream-paginate via
   StartFromFileName, returning io.EOF naturally and surfacing all
   other errors instead of swallowing them.

Addresses two gemini medium reviews on PR #9319.
2026-05-04 22:15:03 -07:00