feat(iam): account-scoped OIDC providers (Phase 3c) (#9323)

* feat(iam): claim-based policy mode for AssumeRoleWithWebIdentity

When the caller passes the sentinel RoleArn arn:aws:iam:::role/sts-claim-based
(or omits it entirely) and the matched OIDC provider has policyClaim set,
mint a session whose effective policies come from that JWT claim instead
of from a server-side role mapping. Accepts string, comma-separated
string, or array shapes — MinIO-compatible behaviour for IDPs that
already attach policies to the user.

Trust-policy validation is skipped in claim-mode: the IDP is the sole
authority for both authentication and authorization, mirroring the
contract MinIO documents for its DummyRoleARN flow. Concrete-role mode
is unchanged and still requires the role definition + trust policy.

* fix(iam): trim policy-claim array elements + clean up stale comments

Three medium-priority cleanups gemini flagged on the claim-based path:

- extractClaimPolicies's array branch was leaving whitespace on each
  element while the string/comma-separated branch trimmed via
  splitPolicyClaimString. An IDP that emits ["readonly", " billing "]
  would create a "billing" policy lookup that didn't match the stored
  name. Trim every array element, drop empties.
- The "synthetic ARN keyed on the session name" comment was wrong —
  effectiveRoleArn here is the literal sentinel; it's the assumed-role
  ARN generated downstream that's session-keyed. Reword.
- The empty if/else block at the start of
  validateAssumeRoleWithWebIdentityRequest existed only to host a
  comment about deferred validation; the comment now lives in the
  function godoc and the empty branch is gone.

Addresses three gemini medium reviews on PR #9322.

* feat(iam): account-scoped OIDC providers

Add OIDCProviderRecord.AccountID enforcement: when a role lives in
account A, the OIDC provider validating the assume-role token must be
either global (AccountID="") or also live in account A. Cross-account
use is rejected at the IAM-manager layer before reaching the trust
policy validator.

OIDCProviderStore gains GetProviderByIssuerAndAccount; both the in-
memory and filer-backed stores implement it. Static-config-only
deployments are unaffected since they don't populate the store.

* fix(iam): account-scoped lookup for cross-account check

enforceProviderAccountScope was calling GetProviderByIssuer, which
returns the first match arbitrarily when multiple providers share an
issuer (one global + one per tenant is the canonical setup). On a
two-record collision the wrong record could come back first and
falsely reject a valid same-account or global-provider request.

Use GetProviderByIssuerAndAccount as the primary lookup so the
filter happens in the store. On miss, fall back to
GetProviderByIssuer purely to distinguish "issuer entirely unknown"
(let the STS layer reject) from "issuer registered in a different
account" (surface a precise cross-account error).

Addresses gemini high-priority review on PR #9323.
This commit is contained in:
Chris Lu
2026-05-05 13:06:53 -07:00
committed by GitHub
parent bc1d458fe6
commit 6483583491
3 changed files with 188 additions and 0 deletions
@@ -0,0 +1,72 @@
package integration
import (
"context"
"testing"
)
func TestGetProviderByIssuerAndAccount(t *testing.T) {
ctx := context.Background()
store := NewMemoryOIDCProviderStore()
// Two providers for the same issuer, scoped to different accounts.
must := func(rec *OIDCProviderRecord) {
if err := store.StoreProvider(ctx, "", rec); err != nil {
t.Fatalf("store: %v", err)
}
}
must(&OIDCProviderRecord{
AccountID: "111",
ARN: "arn:aws:iam::111:oidc-provider/idp.example",
URL: "https://idp.example",
ClientIDs: []string{"a"},
})
must(&OIDCProviderRecord{
AccountID: "222",
ARN: "arn:aws:iam::222:oidc-provider/idp.example",
URL: "https://idp.example",
ClientIDs: []string{"b"},
})
t.Run("matching account returns scoped record", func(t *testing.T) {
rec, err := store.GetProviderByIssuerAndAccount(ctx, "", "https://idp.example", "111")
if err != nil {
t.Fatalf("err: %v", err)
}
if rec.AccountID != "111" {
t.Fatalf("unexpected record: %+v", rec)
}
})
t.Run("unknown account is rejected", func(t *testing.T) {
if _, err := store.GetProviderByIssuerAndAccount(ctx, "", "https://idp.example", "999"); err == nil {
t.Fatal("expected error for cross-account use")
}
})
t.Run("global provider satisfies any account", func(t *testing.T) {
must(&OIDCProviderRecord{
AccountID: "",
ARN: "arn:aws:iam:::oidc-provider/global.example",
URL: "https://global.example",
ClientIDs: []string{"x"},
})
rec, err := store.GetProviderByIssuerAndAccount(ctx, "", "https://global.example", "anything")
if err != nil {
t.Fatalf("err: %v", err)
}
if rec.AccountID != "" {
t.Fatalf("expected global record, got AccountID=%q", rec.AccountID)
}
})
t.Run("empty caller account accepts any record", func(t *testing.T) {
rec, err := store.GetProviderByIssuerAndAccount(ctx, "", "https://idp.example", "")
if err != nil {
t.Fatalf("err: %v", err)
}
if rec == nil {
t.Fatal("expected a record")
}
})
}
+73
View File
@@ -744,6 +744,14 @@ func (m *IAMManager) AssumeRoleWithWebIdentity(ctx context.Context, request *sts
return nil, fmt.Errorf("IAM manager not initialized")
}
// Claim-based mode bypasses role lookup and trust policy entirely; the
// STS service handles the policy resolution from the JWT itself. Account
// scoping still applies but at the provider-resolution layer in Phase 3c
// (we'll plug that in once a multi-account assume path lands).
if sts.IsClaimBasedPolicyRoleArn(request.RoleArn) {
return m.stsService.AssumeRoleWithWebIdentity(ctx, request)
}
// Extract role name from ARN
roleName := utils.ExtractRoleNameFromArn(request.RoleArn)
@@ -753,6 +761,14 @@ func (m *IAMManager) AssumeRoleWithWebIdentity(ctx context.Context, request *sts
return nil, fmt.Errorf("role not found: %s", roleName)
}
// Account scoping: when the role lives in account A, the OIDC provider
// validating the token must be either global (AccountID="") or also live
// in account A. Skip when we can't resolve account context; fall through
// to the existing trust-policy enforcement.
if err := m.enforceProviderAccountScope(ctx, request); err != nil {
return nil, err
}
// Validate trust policy before allowing STS to assume the role
if err := m.validateTrustPolicyForWebIdentity(ctx, roleDef, request.WebIdentityToken, request.DurationSeconds); err != nil {
return nil, fmt.Errorf("trust policy validation failed: %w", err)
@@ -767,6 +783,63 @@ func (m *IAMManager) AssumeRoleWithWebIdentity(ctx context.Context, request *sts
return m.stsService.AssumeRoleWithWebIdentity(ctx, request)
}
// enforceProviderAccountScope checks that the OIDC provider matching the
// token's issuer is registered in either the role's account or as a global
// (account-less) provider. Returns nil when no provider store is configured
// (preserves the static-config-only path) or when the issuer is not known to
// the store (the existing STS-layer issuer→provider map handles that case
// during validation).
func (m *IAMManager) enforceProviderAccountScope(ctx context.Context, request *sts.AssumeRoleWithWebIdentityRequest) error {
if m.oidcProviderStore == nil {
return nil
}
roleAccount := utils.ParseRoleARN(request.RoleArn).AccountID
if roleAccount == "" {
// Legacy ARN form (no account): nothing to enforce.
return nil
}
issuer, err := extractIssuerFromJWT(request.WebIdentityToken)
if err != nil {
return nil // signature validation will reject, no need to fail twice
}
// Look for a provider that is either global or scoped to the role's
// account. Multiple providers may share an issuer (e.g. one global plus
// one per tenant), and GetProviderByIssuer returns the first match
// arbitrarily — using it here would falsely reject a valid request
// whenever the wrong record happens to come back first.
if _, err := m.oidcProviderStore.GetProviderByIssuerAndAccount(ctx, m.getFilerAddress(), issuer, roleAccount); err == nil {
return nil
}
// No allowed match. Distinguish "issuer entirely unknown" (let the STS
// layer reject with the existing not-registered error) from "issuer is
// registered but only in a different account" (surface a precise error
// so the operator knows the call was cross-account).
if other, err := m.oidcProviderStore.GetProviderByIssuer(ctx, m.getFilerAddress(), issuer); err == nil {
return fmt.Errorf("OIDC provider for issuer %s is registered in account %s; cannot be used to assume a role in account %s", issuer, other.AccountID, roleAccount)
}
return nil
}
// extractIssuerFromJWT returns the iss claim of a JWT without verifying its
// signature. Safe here because the caller still goes through full signature
// + issuer validation in the STS service; this is purely for routing.
func extractIssuerFromJWT(token string) (string, error) {
parser := new(jwt.Parser)
parsed, _, err := parser.ParseUnverified(token, jwt.MapClaims{})
if err != nil {
return "", err
}
claims, ok := parsed.Claims.(jwt.MapClaims)
if !ok {
return "", fmt.Errorf("invalid claims")
}
iss, _ := claims["iss"].(string)
if iss == "" {
return "", fmt.Errorf("token has no iss claim")
}
return iss, nil
}
// capDurationByRole returns the requested duration clamped to the role's
// MaxSessionDuration. A nil requested duration is left nil so the STS
// service's calculateSessionDuration applies the global default (typically
@@ -81,6 +81,7 @@ type OIDCProviderStore interface {
StoreProvider(ctx context.Context, filerAddress string, record *OIDCProviderRecord) error
GetProviderByARN(ctx context.Context, filerAddress string, arn string) (*OIDCProviderRecord, error)
GetProviderByIssuer(ctx context.Context, filerAddress string, issuer string) (*OIDCProviderRecord, error)
GetProviderByIssuerAndAccount(ctx context.Context, filerAddress string, issuer, accountID string) (*OIDCProviderRecord, error)
ListProviders(ctx context.Context, filerAddress string) ([]*OIDCProviderRecord, error)
DeleteProvider(ctx context.Context, filerAddress string, arn string) error
}
@@ -137,6 +138,29 @@ func (m *MemoryOIDCProviderStore) GetProviderByIssuer(ctx context.Context, _ str
return nil, fmt.Errorf("no OIDC provider registered for issuer: %s", issuer)
}
// GetProviderByIssuerAndAccount returns the record matching `issuer` whose
// AccountID is empty (global) or equals `accountID`. AWS-style account
// scoping: cross-account references must go through a shared/global
// provider, which is why an empty AccountID is treated as a wildcard.
//
// `accountID` may itself be empty when the caller has no specific account
// context (e.g. claim-based mode); in that case any registered provider
// matching the issuer is acceptable.
func (m *MemoryOIDCProviderStore) GetProviderByIssuerAndAccount(ctx context.Context, _ string, issuer, accountID string) (*OIDCProviderRecord, error) {
want := normalizeIssuer(issuer)
m.mu.RLock()
defer m.mu.RUnlock()
for _, rec := range m.providers {
if normalizeIssuer(rec.URL) != want {
continue
}
if rec.AccountID == "" || accountID == "" || rec.AccountID == accountID {
return copyOIDCProviderRecord(rec), nil
}
}
return nil, fmt.Errorf("no OIDC provider registered for issuer %s in account %s", issuer, accountID)
}
// ListProviders returns every record sorted by ARN for stable output.
func (m *MemoryOIDCProviderStore) ListProviders(ctx context.Context, _ string) ([]*OIDCProviderRecord, error) {
m.mu.RLock()
@@ -283,6 +307,25 @@ func (f *FilerOIDCProviderStore) GetProviderByIssuer(ctx context.Context, filerA
return nil, fmt.Errorf("no OIDC provider registered for issuer: %s", issuer)
}
// GetProviderByIssuerAndAccount filters by AccountID on top of the issuer scan.
// See the MemoryOIDCProviderStore equivalent for semantics.
func (f *FilerOIDCProviderStore) GetProviderByIssuerAndAccount(ctx context.Context, filerAddress string, issuer, accountID string) (*OIDCProviderRecord, error) {
records, err := f.ListProviders(ctx, filerAddress)
if err != nil {
return nil, err
}
want := normalizeIssuer(issuer)
for _, rec := range records {
if normalizeIssuer(rec.URL) != want {
continue
}
if rec.AccountID == "" || accountID == "" || rec.AccountID == accountID {
return rec, nil
}
}
return nil, fmt.Errorf("no OIDC provider registered for issuer %s in account %s", issuer, accountID)
}
// ListProviders enumerates every record in the directory.
func (f *FilerOIDCProviderStore) ListProviders(ctx context.Context, filerAddress string) ([]*OIDCProviderRecord, error) {
filerAddress = f.resolveFilerAddress(filerAddress)