From 5da137233d1a12eba83e9de9d67a9bdb6c3f5cf9 Mon Sep 17 00:00:00 2001 From: Khris Richardson Date: Mon, 28 Sep 2026 20:51:39 -0700 Subject: [PATCH] s3/iam: persist IAM-managed OIDC providers in the filer, and trust them after a restart (#11510) * s3/iam: persist IAM-managed OIDC providers in the filer, and trust them after a restart The S3 server's IAM config loader never read the documented `oidcProviderStore` key, so the OIDC provider store was always in memory: a provider created with CreateOpenIDConnectProvider lived in one gateway's process, was lost on restart, and was never seen by peers. The /etc/iam/oidc-providers metadata subscription refreshed from that empty in-memory store. - Read `oidcProviderStore` and pass it to the IAM manager. With an IAM config file the default stays memory. With no config file (zero-config IAM, as `weed filer -s3` and operator-managed clusters run) it defaults to the filer: there is nothing static to shadow, and providers created at runtime otherwise vanish on restart. - With a store that outlives the process, load the STS runtime view from it at startup, so providers created on an earlier boot or on a peer are trusted without waiting for the next mutation. - If the store cannot be read at startup (a filer not up yet), the load is retried in the background with backoff until it succeeds: the metadata subscription reports only later changes, so providers already stored would otherwise stay unknown to STS until one of them changed. - Mark records mirrored from STS.Providers as `source: static-config`, and at startup delete such records whose provider has left the config, so removing a provider from the config file still revokes it. Records created through the IAM API are never pruned. - The filer store reported every failed lookup, an unreachable filer included, as ErrOIDCProviderNotFound, which CreateOIDCProvider reads as "free to create". Only a confirmed absence is now not-found. Co-Authored-By: Claude Opus 5.5 (1M context) * s3/iam: keep config-file OIDC providers out of a persistent store Review of the previous commit found that mirroring the IAM config file's providers into a persistent store, and pruning them when they leave the file, breaks as soon as S3 servers share a filer: - a server prunes stored config-file providers its own file does not list, including ones a peer's file still defines (a zero-config server prunes them all); - mirroring overwrites an API-created provider with the same ARN and marks it config-owned, so a later prune deletes it; - a failed mirror write or a failed prune leaves a stale record trusted; - a mirrored record is loaded into STS at startup as an IAM-managed provider and shadows the config-file provider, dropping the settings a record does not carry (jwksUri, roleMapping, policyClaim, ...). A persistent store now never receives the config file's providers. STS keeps serving them from its static configuration, as it always has; the IAM API lists and returns them from memory, refuses to change or delete them (UnmodifiableEntity; change them in the file) and to create another provider with their ARN (EntityAlreadyExists). The store holds only providers created through the IAM API, and those are what startup loads into STS. There is nothing to prune, so the source marker is gone. An in-memory store keeps its behaviour: the config file's providers are records in it, as before. buildOIDCProviderFromRecord also carries PolicyClaim and AllowedPrincipalTagKeys now; they were dropped whenever an API-created provider was loaded into STS. Co-Authored-By: Claude Opus 5.5 (1M context) * s3/iam: send UnmodifiableEntity as a 400, not an internal error The IAM API's error writer had no case for UnmodifiableEntity, which the previous commit returns for a change to a config-file provider, so it went out as a 500 ServiceFailure that clients retry. AWS sends it as a 400. Co-Authored-By: Claude Opus 5.5 (1M context) * s3/iam: document stored-over-config precedence, drop invented CreateDate, cancel superseded retries Follow-ups from review of b881982d2: - A provider stored under the same ARN as a config-file provider takes precedence in the IAM API, matching STS, which already prefers IAM-managed providers so that an API call can shadow a bootstrap entry. Deleting the stored provider brings the config-file one back. This was already the behaviour; it is now documented and tested. - A config-file provider no longer reports its server's start time as CreateDate, which changed on every restart; GetOpenIDConnectProvider now omits the date for it. An in-memory store still stamps its copies at load, as before. - The startup retry runs under a cancellable context, is cancelled when another store is installed, and retries the store it was started for rather than reading the manager's field, so replacing the store neither leaves the old retry running nor races with it (go test -race). Co-Authored-By: Claude Opus 5.5 (1M context) * s3/iam: serialize OIDC provider refreshes so an older snapshot cannot restore a deleted provider Refreshes run concurrently: after an IAM API change, on a peer's change and in the startup retry. Each lists the store and then hands STS the result, so a refresh that listed before a DeleteOIDCProvider could finish after that call's own refresh and keep the deleted provider trusted until the next change. Refreshes now hold a lock from the read to the hand-off, and a startup retry cancelled by installing another store drops its snapshot instead of applying it. The retry-cancellation test waits for the retry by polling instead of a fixed sleep. * s3/iam: route SetOIDCProviderStore through installOIDCProviderStore A store installed after Initialize skipped the static-provider overlay and startup hydration: config-file providers disappeared from the IAM API, ErrOIDCProviderStatic no longer protected them, and stored providers were never trusted until the next mutation or peer event. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Claude Opus 5.5 (1M context) Co-authored-by: Chris Lu Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- weed/iam/integration/iam_manager.go | 227 +++++++--- .../integration/oidc_provider_persist_test.go | 393 ++++++++++++++++++ weed/iam/integration/oidc_provider_store.go | 14 +- weed/s3api/iam_defaults_test.go | 29 ++ weed/s3api/s3api_embedded_iam.go | 2 + weed/s3api/s3api_embedded_iam_oidc.go | 39 +- weed/s3api/s3api_iam_oidc_test.go | 19 + weed/s3api/s3api_server.go | 15 +- 8 files changed, 667 insertions(+), 71 deletions(-) create mode 100644 weed/iam/integration/oidc_provider_persist_test.go diff --git a/weed/iam/integration/iam_manager.go b/weed/iam/integration/iam_manager.go index bff5a9c1c..4251587dc 100644 --- a/weed/iam/integration/iam_manager.go +++ b/weed/iam/integration/iam_manager.go @@ -27,11 +27,20 @@ const maxPoliciesForEvaluation = 1024 // IAMManager orchestrates all IAM components type IAMManager struct { - stsService *sts.STSService - policyEngine *policy.PolicyEngine - roleStore RoleStore - userStore UserStore - oidcProviderStore OIDCProviderStore + stsService *sts.STSService + policyEngine *policy.PolicyEngine + roleStore RoleStore + userStore UserStore + oidcProviderStore OIDCProviderStore + // staticOIDCProviders are the OIDC providers of this server's IAM config + // file, by ARN. With an in-memory store they are also written to the + // store; a persistent store never holds them (see installOIDCProviderStore). + staticOIDCProviders map[string]*OIDCProviderRecord + // cancelOIDCLoad stops a startup load still retrying against the store. + cancelOIDCLoad context.CancelFunc + // oidcRefreshMu serializes refreshes from reading the store to handing + // STS the result, so an older snapshot cannot replace a newer one. + oidcRefreshMu sync.Mutex oidcAuditSink OIDCProviderAuditSink revocationStore SessionRevocationStore filerAddressProvider func() string // Function to get current filer address @@ -108,12 +117,13 @@ func (m *IAMManager) PurgeRevokedSessions(ctx context.Context) (int, error) { } // SetOIDCProviderStore configures the IAM-managed OIDC provider store. When -// nil, OIDC provider IAM actions return ServiceNotReady. The store is the -// source of truth for AssumeRoleWithWebIdentity provider resolution once -// Phase 2b lands; in Phase 2a it is read-only and populated from static -// configuration at boot. +// nil, OIDC provider IAM actions return ServiceNotReady. func (m *IAMManager) SetOIDCProviderStore(store OIDCProviderStore) { - m.oidcProviderStore = store + var stsConfig *sts.STSConfig + if m.stsService != nil { + stsConfig = m.stsService.Config + } + m.installOIDCProviderStore(store, stsConfig) } // GetOIDCProviderStore returns the configured store (may be nil). @@ -127,7 +137,13 @@ func (m *IAMManager) GetOIDCProvider(ctx context.Context, arn string) (*OIDCProv if m.oidcProviderStore == nil { return nil, fmt.Errorf("OIDC provider store not configured") } - return m.oidcProviderStore.GetProviderByARN(ctx, m.getFilerAddress(), arn) + rec, err := m.oidcProviderStore.GetProviderByARN(ctx, m.getFilerAddress(), arn) + if errors.Is(err, ErrOIDCProviderNotFound) { + if static, ok := m.staticOIDCProviders[arn]; ok { + return copyOIDCProviderRecord(static), nil + } + } + return rec, err } // ListOIDCProviders enumerates all configured OIDC providers. @@ -135,7 +151,33 @@ func (m *IAMManager) ListOIDCProviders(ctx context.Context) ([]*OIDCProviderReco if m.oidcProviderStore == nil { return nil, fmt.Errorf("OIDC provider store not configured") } - return m.oidcProviderStore.ListProviders(ctx, m.getFilerAddress()) + records, err := m.oidcProviderStore.ListProviders(ctx, m.getFilerAddress()) + if err != nil { + return nil, err + } + // A persistent store does not hold the config file's providers. + seen := make(map[string]bool, len(records)) + for _, rec := range records { + seen[rec.ARN] = true + } + for arn, static := range m.staticOIDCProviders { + if !seen[arn] { + records = append(records, copyOIDCProviderRecord(static)) + } + } + return records, nil +} + +// mutableOIDCProvider loads a stored provider for a change. A provider only +// the IAM config file defines is refused with ErrOIDCProviderStatic. +func (m *IAMManager) mutableOIDCProvider(ctx context.Context, arn string) (*OIDCProviderRecord, error) { + rec, err := m.oidcProviderStore.GetProviderByARN(ctx, m.getFilerAddress(), arn) + if errors.Is(err, ErrOIDCProviderNotFound) { + if _, ok := m.staticOIDCProviders[arn]; ok { + return nil, fmt.Errorf("%w: %s", ErrOIDCProviderStatic, arn) + } + } + return rec, err } // CreateOIDCProvider persists a new IAM-managed OIDC provider record. Refuses @@ -150,6 +192,9 @@ func (m *IAMManager) CreateOIDCProvider(ctx context.Context, rec *OIDCProviderRe if err := validateOIDCProviderRecord(rec); err != nil { return err } + if _, static := m.staticOIDCProviders[rec.ARN]; static { + return fmt.Errorf("%w: %s", ErrOIDCProviderAlreadyExists, rec.ARN) + } existing, err := m.oidcProviderStore.GetProviderByARN(ctx, m.getFilerAddress(), rec.ARN) if err == nil && existing != nil { return fmt.Errorf("%w: %s", ErrOIDCProviderAlreadyExists, rec.ARN) @@ -173,6 +218,9 @@ func (m *IAMManager) DeleteOIDCProvider(ctx context.Context, arn string) error { if m.oidcProviderStore == nil { return fmt.Errorf("OIDC provider store not configured") } + if _, err := m.mutableOIDCProvider(ctx, arn); errors.Is(err, ErrOIDCProviderStatic) { + return err + } if err := m.oidcProviderStore.DeleteProvider(ctx, m.getFilerAddress(), arn); err != nil { return err } @@ -190,7 +238,7 @@ func (m *IAMManager) AddClientIDToOIDCProvider(ctx context.Context, arn, clientI if clientID == "" { return fmt.Errorf("ClientID cannot be empty") } - rec, err := m.oidcProviderStore.GetProviderByARN(ctx, m.getFilerAddress(), arn) + rec, err := m.mutableOIDCProvider(ctx, arn) if err != nil { return err } @@ -218,7 +266,7 @@ func (m *IAMManager) RemoveClientIDFromOIDCProvider(ctx context.Context, arn, cl if m.oidcProviderStore == nil { return fmt.Errorf("OIDC provider store not configured") } - rec, err := m.oidcProviderStore.GetProviderByARN(ctx, m.getFilerAddress(), arn) + rec, err := m.mutableOIDCProvider(ctx, arn) if err != nil { return err } @@ -255,7 +303,7 @@ func (m *IAMManager) UpdateOIDCProviderThumbprints(ctx context.Context, arn stri return fmt.Errorf("invalid thumbprint %q: must be 40-character SHA-1 hex", tp) } } - rec, err := m.oidcProviderStore.GetProviderByARN(ctx, m.getFilerAddress(), arn) + rec, err := m.mutableOIDCProvider(ctx, arn) if err != nil { return err } @@ -274,7 +322,7 @@ func (m *IAMManager) TagOIDCProvider(ctx context.Context, arn string, tags map[s if m.oidcProviderStore == nil { return fmt.Errorf("OIDC provider store not configured") } - rec, err := m.oidcProviderStore.GetProviderByARN(ctx, m.getFilerAddress(), arn) + rec, err := m.mutableOIDCProvider(ctx, arn) if err != nil { return err } @@ -297,7 +345,7 @@ func (m *IAMManager) UntagOIDCProvider(ctx context.Context, arn string, keys []s if m.oidcProviderStore == nil { return fmt.Errorf("OIDC provider store not configured") } - rec, err := m.oidcProviderStore.GetProviderByARN(ctx, m.getFilerAddress(), arn) + rec, err := m.mutableOIDCProvider(ctx, arn) if err != nil { return err } @@ -557,55 +605,116 @@ func (m *IAMManager) initOIDCProviderStore(config *IAMConfig) error { if err != nil { return err } - m.oidcProviderStore = store + m.installOIDCProviderStore(store, config.STS) + return nil +} - if config.STS == nil { - return nil +// installOIDCProviderStore makes store the manager's OIDC provider store. +// +// The IAM config file's providers are reported by the IAM API alongside the +// stored ones. An in-memory store holds them as records, as it always has. A +// persistent store never does: it outlives this process and may be shared by +// S3 servers with different config files, so a record written from one file +// would outlive its removal from that file and be trusted by every server. +// Those providers stay in memory (staticOIDCProviders) and STS keeps serving +// them from its static configuration; the store holds only providers created +// through the IAM API, and those are loaded into STS here. +// +// A provider stored under the same ARN as a config-file provider takes +// precedence, in the IAM API as in STS, which already prefers IAM-managed +// providers so that an API call can shadow a bootstrap entry. Deleting the +// stored provider brings the config-file one back. +func (m *IAMManager) installOIDCProviderStore(store OIDCProviderStore, stsConfig *sts.STSConfig) { + if m.cancelOIDCLoad != nil { + m.cancelOIDCLoad() + m.cancelOIDCLoad = nil } - for _, pc := range config.STS.Providers { + m.oidcProviderStore = store + m.staticOIDCProviders = staticOIDCProviderRecords(stsConfig) + if _, inMemory := store.(*MemoryOIDCProviderStore); inMemory { + ctx := context.Background() + now := time.Now().UTC() + for _, rec := range m.staticOIDCProviders { + mirrored := copyOIDCProviderRecord(rec) + mirrored.CreatedAt, mirrored.UpdatedAt = now, now + if err := store.StoreProvider(ctx, m.getFilerAddress(), mirrored); err != nil { + glog.Warningf("mirror static OIDC provider %s into store: %v", rec.ARN, err) + } + } + // The store now holds them and the API may change them, as before; + // the overlay is only for stores that must not hold them. + m.staticOIDCProviders = nil + return + } + if err := m.RefreshOIDCProvidersFromStore(context.Background()); err != nil { + // The metadata subscription only reports changes made from now on, so + // providers already in the store would stay unknown until one changes. + glog.Warningf("load OIDC providers from the store at startup: %v; retrying in the background", err) + ctx, cancel := context.WithCancel(context.Background()) + m.cancelOIDCLoad = cancel + go m.retryOIDCProviderLoad(ctx, store, oidcHydrateRetry) + } +} + +// staticOIDCProviderRecords describes the enabled OIDC providers of the IAM +// config file as provider records. +func staticOIDCProviderRecords(stsConfig *sts.STSConfig) map[string]*OIDCProviderRecord { + out := map[string]*OIDCProviderRecord{} + if stsConfig == nil { + return out + } + for _, pc := range stsConfig.Providers { if pc == nil || !pc.Enabled || pc.Type != sts.ProviderTypeOIDC { continue } issuer, _ := pc.Config["issuer"].(string) if issuer == "" { - glog.Warningf("OIDC provider %s in static config has empty issuer; skipping mirror to store", pc.Name) + glog.Warningf("OIDC provider %s in static config has empty issuer; skipping", pc.Name) continue } - accountID := "" - if config.STS != nil { - accountID = config.STS.AccountId - } - arn, err := DeriveOIDCProviderARN(accountID, issuer) + arn, err := DeriveOIDCProviderARN(stsConfig.AccountId, issuer) if err != nil { glog.Warningf("derive ARN for static OIDC provider %s: %v", pc.Name, err) continue } - clientIDs := extractClientIDs(pc.Config) - ctx := context.Background() - // Preserve CreatedAt across reboots when a persistent store already - // has this provider — IAM's GetOpenIDConnectProvider response - // shouldn't shift its CreateDate every time the server restarts. - now := time.Now().UTC() - createdAt := now - if existing, err := store.GetProviderByARN(ctx, m.getFilerAddress(), arn); err == nil && existing != nil && !existing.CreatedAt.IsZero() { - createdAt = existing.CreatedAt - } - rec := &OIDCProviderRecord{ - AccountID: accountID, + out[arn] = &OIDCProviderRecord{ + AccountID: stsConfig.AccountId, ARN: arn, URL: issuer, - ClientIDs: clientIDs, + ClientIDs: extractClientIDs(pc.Config), Thumbprints: extractStringList(pc.Config, "thumbprints"), AllowedPrincipalTagKeys: extractStringList(pc.Config, "allowedPrincipalTagKeys"), PolicyClaim: extractString(pc.Config, "policyClaim"), - CreatedAt: createdAt, - UpdatedAt: now, + // No CreatedAt: a config-file provider has no creation time the + // server could report consistently across restarts. } - if err := store.StoreProvider(ctx, m.getFilerAddress(), rec); err != nil { - glog.Warningf("mirror static OIDC provider %s into store: %v", pc.Name, err) + } + return out +} + +// oidcHydrateRetry bounds the backoff between startup load attempts. +var oidcHydrateRetry = struct{ initial, max time.Duration }{initial: time.Second, max: 30 * time.Second} + +// retryOIDCProviderLoad retries loading store, the store it was started for, +// until it succeeds or ctx is cancelled because another store was installed. +func (m *IAMManager) retryOIDCProviderLoad(ctx context.Context, store OIDCProviderStore, bounds struct{ initial, max time.Duration }) { + delay := bounds.initial + for { + select { + case <-ctx.Done(): + return + case <-time.After(delay): + } + err := m.refreshOIDCProvidersFrom(ctx, store) + if err == nil { + glog.V(0).Infof("loaded OIDC providers from the store after retrying") + return + } + glog.V(1).Infof("load OIDC providers from the store: %v; retrying in %v", err, delay) + if delay *= 2; delay > bounds.max { + delay = bounds.max } } - return nil } // refreshOIDCProvidersBestEffort calls RefreshOIDCProvidersFromStore and @@ -627,13 +736,29 @@ func (m *IAMManager) refreshOIDCProvidersBestEffort(ctx context.Context, op, arn // or invalid configuration are logged and skipped so a single bad entry // does not stop the rest from refreshing. func (m *IAMManager) RefreshOIDCProvidersFromStore(ctx context.Context) error { - if m.oidcProviderStore == nil || m.stsService == nil { + return m.refreshOIDCProvidersFrom(ctx, m.oidcProviderStore) +} + +// refreshOIDCProvidersFrom is RefreshOIDCProvidersFromStore for a given store. +func (m *IAMManager) refreshOIDCProvidersFrom(ctx context.Context, store OIDCProviderStore) error { + if store == nil || m.stsService == nil { return nil } - records, err := m.oidcProviderStore.ListProviders(ctx, m.getFilerAddress()) + // Refreshes run concurrently: after an IAM API change, on a peer's change + // and in the startup retry. Unserialized, a refresh that read the store + // before a DeleteOIDCProvider could finish after that call's own refresh + // and keep the deleted provider trusted. + m.oidcRefreshMu.Lock() + defer m.oidcRefreshMu.Unlock() + records, err := store.ListProviders(ctx, m.getFilerAddress()) if err != nil { return fmt.Errorf("list OIDC providers: %w", err) } + // A startup retry is cancelled when another store is installed; its + // snapshot is of the old store and must not replace the new one's. + if err := ctx.Err(); err != nil { + return err + } byIssuer := make(map[string][]sts.ScopedOIDCProvider, len(records)) for _, rec := range records { if rec == nil || rec.URL == "" { @@ -664,9 +789,11 @@ func buildOIDCProviderFromRecord(rec *OIDCProviderRecord) (*oidc.OIDCProvider, e return nil, fmt.Errorf("record cannot be nil") } cfg := &oidc.OIDCConfig{ - Issuer: rec.URL, - ClientIDs: append([]string(nil), rec.ClientIDs...), - Thumbprints: append([]string(nil), rec.Thumbprints...), + Issuer: rec.URL, + ClientIDs: append([]string(nil), rec.ClientIDs...), + Thumbprints: append([]string(nil), rec.Thumbprints...), + AllowedPrincipalTagKeys: append([]string(nil), rec.AllowedPrincipalTagKeys...), + PolicyClaim: rec.PolicyClaim, } provider := oidc.NewOIDCProvider(rec.ARN) if err := provider.Initialize(cfg); err != nil { diff --git a/weed/iam/integration/oidc_provider_persist_test.go b/weed/iam/integration/oidc_provider_persist_test.go new file mode 100644 index 000000000..6904f232c --- /dev/null +++ b/weed/iam/integration/oidc_provider_persist_test.go @@ -0,0 +1,393 @@ +package integration + +import ( + "context" + "errors" + "strings" + "sync" + "testing" + "time" + + "github.com/golang-jwt/jwt/v5" + "github.com/seaweedfs/seaweedfs/weed/iam/policy" + "github.com/seaweedfs/seaweedfs/weed/iam/sts" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// persistentTestStore stands in for a store that outlives the process and may +// be shared (the filer store): anything but *MemoryOIDCProviderStore is +// treated as persistent. +type persistentTestStore struct{ *MemoryOIDCProviderStore } + +const ( + persistTestStaticIssuer = "https://static.example" + persistTestAPIIssuer = "https://api.example" +) + +// persistTestConfig is an IAM config whose file defines the given OIDC +// issuers. +func persistTestConfig(issuers ...string) *IAMConfig { + var providers []*sts.ProviderConfig + for i, issuer := range issuers { + providers = append(providers, &sts.ProviderConfig{ + Name: "static-" + string(rune('a'+i)), + Type: sts.ProviderTypeOIDC, + Enabled: true, + Config: map[string]interface{}{"issuer": issuer, "clientId": "aud"}, + }) + } + return &IAMConfig{ + STS: &sts.STSConfig{ + TokenDuration: sts.FlexibleDuration{Duration: time.Hour}, + MaxSessionLength: sts.FlexibleDuration{Duration: 12 * time.Hour}, + Issuer: "test-sts", + SigningKey: []byte("test-signing-key-32-characters-long"), + Providers: providers, + }, + Policy: &policy.PolicyEngineConfig{DefaultEffect: "Deny", StoreType: "memory"}, + Roles: &RoleStoreConfig{StoreType: "memory"}, + } +} + +// startServer initializes a manager as an S3 server would at boot, with the +// given config file providers and OIDC provider store. +func startServer(t *testing.T, store OIDCProviderStore, issuers ...string) *IAMManager { + t.Helper() + cfg := persistTestConfig(issuers...) + mgr := NewIAMManager() + require.NoError(t, mgr.Initialize(cfg, func() string { return "localhost:8888" })) + mgr.installOIDCProviderStore(store, cfg.STS) + return mgr +} + +func arnOf(t *testing.T, issuer string) string { + t.Helper() + arn, err := DeriveOIDCProviderARN("", issuer) + require.NoError(t, err) + return arn +} + +func listedARNs(t *testing.T, mgr *IAMManager) map[string]bool { + t.Helper() + recs, err := mgr.ListOIDCProviders(context.Background()) + require.NoError(t, err) + out := map[string]bool{} + for _, r := range recs { + out[r.ARN] = true + } + return out +} + +func storedARNs(t *testing.T, store OIDCProviderStore) map[string]bool { + t.Helper() + recs, err := store.ListProviders(context.Background(), "") + require.NoError(t, err) + out := map[string]bool{} + for _, r := range recs { + out[r.ARN] = true + } + return out +} + +func createAPIProvider(t *testing.T, mgr *IAMManager, issuer string) { + t.Helper() + require.NoError(t, mgr.CreateOIDCProvider(context.Background(), &OIDCProviderRecord{ + ARN: arnOf(t, issuer), URL: issuer, ClientIDs: []string{"aud"}, + })) +} + +// stsKnowsIssuer reports whether STS resolves a provider for the issuer. The +// token is not validly signed; the only question is which error comes back. +func stsKnowsIssuer(t *testing.T, mgr *IAMManager, issuer string) bool { + t.Helper() + tok, err := jwt.NewWithClaims(jwt.SigningMethodHS256, jwt.MapClaims{ + "iss": issuer, "sub": "probe", "aud": "aud", "exp": time.Now().Add(time.Hour).Unix(), + }).SignedString([]byte("not-the-providers-key")) + require.NoError(t, err) + _, _, err = mgr.GetSTSService().ValidateWebIdentityToken(context.Background(), tok) + require.Error(t, err, "an unsigned-by-provider token for %s was accepted", issuer) + return !strings.Contains(err.Error(), "no identity provider registered") +} + +// A persistent store never receives the config file's providers: it outlives +// the file and may be shared with servers whose files differ. +func TestPersistentStoreNeverHoldsConfigFileProviders(t *testing.T) { + store := &persistentTestStore{NewMemoryOIDCProviderStore()} + mgr := startServer(t, store, persistTestStaticIssuer) + + assert.Empty(t, storedARNs(t, store), "a config-file provider was written to the persistent store") + assert.True(t, listedARNs(t, mgr)[arnOf(t, persistTestStaticIssuer)], "the IAM API no longer lists the config-file provider") + rec, err := mgr.GetOIDCProvider(context.Background(), arnOf(t, persistTestStaticIssuer)) + require.NoError(t, err) + assert.Equal(t, persistTestStaticIssuer, rec.URL) + assert.True(t, stsKnowsIssuer(t, mgr, persistTestStaticIssuer), "STS stopped trusting the config-file provider") +} + +// Servers sharing a store, with different config files, must not remove or +// replace each other's providers — including a zero-config server. +func TestServersSharingAStoreKeepEachOthersProviders(t *testing.T) { + store := &persistentTestStore{NewMemoryOIDCProviderStore()} + configured := startServer(t, store, persistTestStaticIssuer) + createAPIProvider(t, configured, persistTestAPIIssuer) + + zeroConfig := startServer(t, store) + + assert.True(t, storedARNs(t, store)[arnOf(t, persistTestAPIIssuer)], "a peer's start removed an API-created provider") + assert.True(t, listedARNs(t, configured)[arnOf(t, persistTestStaticIssuer)], "the configured server lost its config-file provider") + assert.False(t, listedARNs(t, zeroConfig)[arnOf(t, persistTestStaticIssuer)], "a server lists a provider only its peer's config file defines") + assert.True(t, stsKnowsIssuer(t, zeroConfig, persistTestAPIIssuer), "the peer does not trust the shared API-created provider") + assert.False(t, stsKnowsIssuer(t, zeroConfig, persistTestStaticIssuer), "the peer trusts a provider only another server's config file defines") +} + +// Removing a provider from the config file revokes it at the next start. +func TestRemovingAProviderFromTheConfigFileRevokesIt(t *testing.T) { + store := &persistentTestStore{NewMemoryOIDCProviderStore()} + startServer(t, store, persistTestStaticIssuer) + + restarted := startServer(t, store) // the file no longer lists it + + assert.False(t, listedARNs(t, restarted)[arnOf(t, persistTestStaticIssuer)]) + assert.False(t, stsKnowsIssuer(t, restarted, persistTestStaticIssuer), "a provider removed from the config file is still trusted") +} + +// The API cannot change a provider the config file defines, nor create one +// with its ARN; the file is where it changes. +func TestConfigFileProvidersCannotBeChangedThroughTheAPI(t *testing.T) { + store := &persistentTestStore{NewMemoryOIDCProviderStore()} + mgr := startServer(t, store, persistTestStaticIssuer) + ctx := context.Background() + arn := arnOf(t, persistTestStaticIssuer) + + for name, err := range map[string]error{ + "add client ID": mgr.AddClientIDToOIDCProvider(ctx, arn, "other"), + "remove client ID": mgr.RemoveClientIDFromOIDCProvider(ctx, arn, "aud"), + "update thumbprint": mgr.UpdateOIDCProviderThumbprints(ctx, arn, []string{"9e99a48a9960b14926bb7f3b02e22da2b0ab7280"}), + "tag": mgr.TagOIDCProvider(ctx, arn, map[string]string{"k": "v"}), + "untag": mgr.UntagOIDCProvider(ctx, arn, []string{"k"}), + "delete": mgr.DeleteOIDCProvider(ctx, arn), + } { + assert.ErrorIs(t, err, ErrOIDCProviderStatic, name) + } + err := mgr.CreateOIDCProvider(ctx, &OIDCProviderRecord{ARN: arn, URL: persistTestStaticIssuer, ClientIDs: []string{"aud"}}) + assert.ErrorIs(t, err, ErrOIDCProviderAlreadyExists) + assert.Empty(t, storedARNs(t, store), "a refused change still wrote to the store") +} + +// Providers created through the API on an earlier boot, or on a peer, are +// trusted at startup rather than after the next change. +func TestStartupLoadsStoredProvidersIntoSTS(t *testing.T) { + store := &persistentTestStore{NewMemoryOIDCProviderStore()} + createAPIProvider(t, startServer(t, store), persistTestAPIIssuer) + + restarted := startServer(t, store) + assert.True(t, stsKnowsIssuer(t, restarted, persistTestAPIIssuer)) +} + +// An in-memory store keeps its behaviour: the config file's providers are +// records in it, as before. +func TestInMemoryStoreStillHoldsConfigFileProviders(t *testing.T) { + store := NewMemoryOIDCProviderStore() + startServer(t, store, persistTestStaticIssuer) + assert.True(t, storedARNs(t, store)[arnOf(t, persistTestStaticIssuer)]) +} + +// unreachableThenReadyStore fails its first reads, as a filer that is not up +// yet when the S3 server starts. +type unreachableThenReadyStore struct { + *MemoryOIDCProviderStore + mu sync.Mutex + failsLeft int +} + +func (s *unreachableThenReadyStore) ListProviders(ctx context.Context, addr string) ([]*OIDCProviderRecord, error) { + s.mu.Lock() + if s.failsLeft > 0 { + s.failsLeft-- + s.mu.Unlock() + return nil, errors.New("filer unavailable") + } + s.mu.Unlock() + return s.MemoryOIDCProviderStore.ListProviders(ctx, addr) +} + +// A store that cannot be read at startup is retried: the metadata +// subscription only reports later changes, so without a retry the providers +// already stored would stay unknown to STS. +func TestStartupLoadRetriesUntilTheStoreIsReadable(t *testing.T) { + saved := oidcHydrateRetry + oidcHydrateRetry.initial, oidcHydrateRetry.max = time.Millisecond, 5*time.Millisecond + t.Cleanup(func() { oidcHydrateRetry = saved }) + + seeded := NewMemoryOIDCProviderStore() + require.NoError(t, seeded.StoreProvider(context.Background(), "", &OIDCProviderRecord{ + ARN: arnOf(t, persistTestAPIIssuer), URL: persistTestAPIIssuer, ClientIDs: []string{"aud"}, + })) + mgr := startServer(t, &unreachableThenReadyStore{MemoryOIDCProviderStore: seeded, failsLeft: 3}) + + deadline := time.Now().Add(2 * time.Second) + for !stsKnowsIssuer(t, mgr, persistTestAPIIssuer) { + if time.Now().After(deadline) { + t.Fatal("STS never learned the stored provider after the store became readable") + } + time.Sleep(5 * time.Millisecond) + } +} + +// A provider stored under a config-file provider's ARN takes precedence, as it +// does in STS; the API then changes the stored one, and deleting it brings the +// config-file provider back. +func TestStoredProviderTakesPrecedenceOverTheConfigFileOne(t *testing.T) { + ctx := context.Background() + store := &persistentTestStore{NewMemoryOIDCProviderStore()} + configured := startServer(t, store, persistTestStaticIssuer) + peer := startServer(t, store) + arn := arnOf(t, persistTestStaticIssuer) + require.NoError(t, peer.CreateOIDCProvider(ctx, &OIDCProviderRecord{ARN: arn, URL: persistTestStaticIssuer, ClientIDs: []string{"stored"}})) + + rec, err := configured.GetOIDCProvider(ctx, arn) + require.NoError(t, err) + assert.Equal(t, []string{"stored"}, rec.ClientIDs, "the config-file provider hides the stored one") + assert.NoError(t, configured.AddClientIDToOIDCProvider(ctx, arn, "more"), "the stored provider cannot be changed") + + require.NoError(t, configured.DeleteOIDCProvider(ctx, arn)) + rec, err = configured.GetOIDCProvider(ctx, arn) + require.NoError(t, err) + assert.Equal(t, []string{"aud"}, rec.ClientIDs, "deleting the stored provider did not bring the config-file one back") +} + +// A store installed through SetOIDCProviderStore behaves like one installed +// at startup: stored providers load into STS and config-file providers stay +// visible to the IAM API. +func TestSetOIDCProviderStoreInstallsLikeStartup(t *testing.T) { + store := &persistentTestStore{NewMemoryOIDCProviderStore()} + require.NoError(t, store.StoreProvider(context.Background(), "", &OIDCProviderRecord{ + ARN: arnOf(t, persistTestAPIIssuer), URL: persistTestAPIIssuer, ClientIDs: []string{"aud"}, + })) + + cfg := persistTestConfig(persistTestStaticIssuer) + mgr := NewIAMManager() + require.NoError(t, mgr.Initialize(cfg, func() string { return "localhost:8888" })) + mgr.SetOIDCProviderStore(store) + + assert.True(t, stsKnowsIssuer(t, mgr, persistTestAPIIssuer), "a stored provider was not trusted after install") + assert.True(t, listedARNs(t, mgr)[arnOf(t, persistTestStaticIssuer)], "the IAM API no longer lists the config-file provider") + assert.ErrorIs(t, mgr.DeleteOIDCProvider(context.Background(), arnOf(t, persistTestStaticIssuer)), ErrOIDCProviderStatic) +} + +// A config-file provider reports no creation time: a time taken at startup +// would change with every restart. +func TestConfigFileProvidersReportNoCreationTime(t *testing.T) { + store := &persistentTestStore{NewMemoryOIDCProviderStore()} + mgr := startServer(t, store, persistTestStaticIssuer) + rec, err := mgr.GetOIDCProvider(context.Background(), arnOf(t, persistTestStaticIssuer)) + require.NoError(t, err) + assert.True(t, rec.CreatedAt.IsZero()) +} + +// countingUnreadableStore never becomes readable and counts the attempts. +type countingUnreadableStore struct { + *MemoryOIDCProviderStore + mu sync.Mutex + reads int +} + +func (s *countingUnreadableStore) ListProviders(context.Context, string) ([]*OIDCProviderRecord, error) { + s.mu.Lock() + defer s.mu.Unlock() + s.reads++ + return nil, errors.New("filer unavailable") +} + +func (s *countingUnreadableStore) readCount() int { + s.mu.Lock() + defer s.mu.Unlock() + return s.reads +} + +// Installing another store stops the previous one's startup retry. +func TestInstallingAnotherStoreStopsThePreviousRetry(t *testing.T) { + saved := oidcHydrateRetry + oidcHydrateRetry.initial, oidcHydrateRetry.max = time.Millisecond, time.Millisecond + t.Cleanup(func() { oidcHydrateRetry = saved }) + + first := &countingUnreadableStore{MemoryOIDCProviderStore: NewMemoryOIDCProviderStore()} + mgr := startServer(t, first) + deadline := time.Now().Add(2 * time.Second) + for first.readCount() <= 1 { + if time.Now().After(deadline) { + t.Fatal("precondition: the first store is never retried") + } + time.Sleep(time.Millisecond) + } + + mgr.installOIDCProviderStore(&persistentTestStore{NewMemoryOIDCProviderStore()}, persistTestConfig().STS) + time.Sleep(10 * time.Millisecond) // let an in-flight attempt finish + settled := first.readCount() + time.Sleep(30 * time.Millisecond) + assert.Equal(t, settled, first.readCount(), "the superseded store is still being retried") +} + +// blockingListStore blocks the first ListProviders call after arm, having +// already read its snapshot, until release is closed. +type blockingListStore struct { + *MemoryOIDCProviderStore + mu sync.Mutex + armed bool + entered chan struct{} + release chan struct{} +} + +func (s *blockingListStore) arm() { + s.mu.Lock() + defer s.mu.Unlock() + s.armed, s.entered, s.release = true, make(chan struct{}), make(chan struct{}) +} + +func (s *blockingListStore) ListProviders(ctx context.Context, addr string) ([]*OIDCProviderRecord, error) { + records, err := s.MemoryOIDCProviderStore.ListProviders(ctx, addr) + s.mu.Lock() + block := s.armed + s.armed = false + s.mu.Unlock() + if block { + close(s.entered) + <-s.release + } + return records, err +} + +// A refresh that read the store before a provider was deleted cannot leave the +// deleted provider trusted by finishing after the deletion's own refresh. +func TestAnOlderRefreshCannotRestoreADeletedProvider(t *testing.T) { + store := &blockingListStore{MemoryOIDCProviderStore: NewMemoryOIDCProviderStore()} + mgr := startServer(t, store) + createAPIProvider(t, mgr, persistTestAPIIssuer) + require.True(t, stsKnowsIssuer(t, mgr, persistTestAPIIssuer), "precondition: the provider is trusted") + + store.arm() + stale := make(chan struct{}) + go func() { + defer close(stale) + _ = mgr.RefreshOIDCProvidersFromStore(context.Background()) + }() + <-store.entered // the stale refresh holds a snapshot with the provider + + deleted := make(chan error, 1) + go func() { deleted <- mgr.DeleteOIDCProvider(context.Background(), arnOf(t, persistTestAPIIssuer)) }() + // Serialized, the deletion's refresh waits for the stale one; otherwise + // let it finish first, which is the ordering that went wrong. + var deleteErr error + select { + case deleteErr = <-deleted: + deleted = nil + case <-time.After(200 * time.Millisecond): + } + close(store.release) + <-stale + if deleted != nil { + deleteErr = <-deleted + } + require.NoError(t, deleteErr) + assert.False(t, stsKnowsIssuer(t, mgr, persistTestAPIIssuer), "a refresh older than the deletion left the deleted provider trusted") +} diff --git a/weed/iam/integration/oidc_provider_store.go b/weed/iam/integration/oidc_provider_store.go index 1a139d78f..77eb1b8a5 100644 --- a/weed/iam/integration/oidc_provider_store.go +++ b/weed/iam/integration/oidc_provider_store.go @@ -27,6 +27,9 @@ import ( var ( ErrOIDCProviderNotFound = errors.New("OIDC provider not found") ErrOIDCProviderAlreadyExists = errors.New("OIDC provider already exists") + // ErrOIDCProviderStatic refuses a change to a provider defined in the + // server's IAM config file: change it there instead. + ErrOIDCProviderStatic = errors.New("OIDC provider is defined in the IAM config file") ) // OIDCProviderRecord is the persisted, IAM-managed view of an OIDC identity @@ -267,15 +270,20 @@ func (f *FilerOIDCProviderStore) GetProviderByARN(ctx context.Context, filerAddr var data []byte err := f.withFilerClient(filerAddress, func(client filer_pb.SeaweedFilerClient) error { - resp, err := client.LookupDirectoryEntry(ctx, &filer_pb.LookupDirectoryEntryRequest{ + resp, err := filer_pb.LookupEntry(ctx, client, &filer_pb.LookupDirectoryEntryRequest{ Directory: f.basePath, Name: f.fileName(arn), }) + // Only a confirmed absence is ErrOIDCProviderNotFound: callers create + // on it, so an unreachable filer must not read as "no such provider". + if errors.Is(err, filer_pb.ErrNotFound) { + return fmt.Errorf("%w: %s", ErrOIDCProviderNotFound, arn) + } if err != nil { - return fmt.Errorf("%w: %v", ErrOIDCProviderNotFound, err) + return fmt.Errorf("lookup OIDC provider %s: %w", arn, err) } if resp.Entry == nil { - return fmt.Errorf("OIDC provider not found: %s", arn) + return fmt.Errorf("%w: %s", ErrOIDCProviderNotFound, arn) } data = resp.Entry.Content return nil diff --git a/weed/s3api/iam_defaults_test.go b/weed/s3api/iam_defaults_test.go index 37aeeb482..8ce1febf9 100644 --- a/weed/s3api/iam_defaults_test.go +++ b/weed/s3api/iam_defaults_test.go @@ -277,3 +277,32 @@ func TestLoadIAMManagerFromConfig_ExplicitFileEnforcesUserScopedPolicy(t *testin assert.NoError(t, err) assert.True(t, allowed, "user-scoped bucket creation should be allowed") } + +func TestLoadIAMManagerFromConfig_HonorsOIDCProviderStore(t *testing.T) { + // The documented oidcProviderStore key must reach the IAM manager; without + // it, providers created through the IAM API live in one gateway's memory. + cases := []struct { + name string + store string + filer bool + }{ + {"absent keeps memory", ``, false}, + {"filer persists", `,"oidcProviderStore":{"storeType":"filer"}`, true}, + {"no config file persists", "no-file", true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + configPath := "" + if tc.store != "no-file" { + configPath = filepath.Join(t.TempDir(), "iam_config.json") + configContent := `{"sts":{"providers":[]},"policy":{"storeType":"memory","defaultEffect":"Deny"}` + tc.store + `}` + assert.NoError(t, os.WriteFile(configPath, []byte(configContent), 0644)) + } + + manager, err := loadIAMManagerFromConfig(configPath, func() string { return "localhost:8888" }, func() string { return "oidc-store-signing-key" }) + assert.NoError(t, err) + _, isFiler := manager.GetOIDCProviderStore().(*integration.FilerOIDCProviderStore) + assert.Equal(t, tc.filer, isFiler) + }) + } +} diff --git a/weed/s3api/s3api_embedded_iam.go b/weed/s3api/s3api_embedded_iam.go index 80dab1e87..9603dc05e 100644 --- a/weed/s3api/s3api_embedded_iam.go +++ b/weed/s3api/s3api_embedded_iam.go @@ -225,6 +225,8 @@ func (e *EmbeddedIamApi) writeIamErrorResponse(w http.ResponseWriter, r *http.Re s3err.WriteXMLResponse(w, r, http.StatusNotImplemented, errorResp) case iam.ErrCodeDeleteConflictException: s3err.WriteXMLResponse(w, r, http.StatusConflict, errorResp) + case iam.ErrCodeUnmodifiableEntityException: + s3err.WriteXMLResponse(w, r, http.StatusBadRequest, errorResp) default: s3err.WriteXMLResponse(w, r, http.StatusInternalServerError, internalErrorResponse) } diff --git a/weed/s3api/s3api_embedded_iam_oidc.go b/weed/s3api/s3api_embedded_iam_oidc.go index ccfdf9ee7..764bc56bc 100644 --- a/weed/s3api/s3api_embedded_iam_oidc.go +++ b/weed/s3api/s3api_embedded_iam_oidc.go @@ -161,7 +161,7 @@ func (e *EmbeddedIamApi) deleteOpenIDConnectProvider(ctx context.Context, mgr *i return nil, iamErr } if err := mgr.DeleteOIDCProvider(ctx, arn); err != nil { - return nil, &iamError{Code: iam.ErrCodeServiceFailureException, Error: err} + return nil, oidcMutationError(err) } return &iamlib.DeleteOpenIDConnectProviderResponse{}, nil } @@ -176,10 +176,7 @@ func (e *EmbeddedIamApi) addClientIDToOpenIDConnectProvider(ctx context.Context, return nil, &iamError{Code: iam.ErrCodeInvalidInputException, Error: errors.New("ClientID is required")} } if err := mgr.AddClientIDToOIDCProvider(ctx, arn, clientID); err != nil { - if errors.Is(err, integration.ErrOIDCProviderNotFound) { - return nil, &iamError{Code: iam.ErrCodeNoSuchEntityException, Error: err} - } - return nil, &iamError{Code: iam.ErrCodeServiceFailureException, Error: err} + return nil, oidcMutationError(err) } return &iamlib.AddClientIDToOpenIDConnectProviderResponse{}, nil } @@ -194,10 +191,7 @@ func (e *EmbeddedIamApi) removeClientIDFromOpenIDConnectProvider(ctx context.Con return nil, &iamError{Code: iam.ErrCodeInvalidInputException, Error: errors.New("ClientID is required")} } if err := mgr.RemoveClientIDFromOIDCProvider(ctx, arn, clientID); err != nil { - if errors.Is(err, integration.ErrOIDCProviderNotFound) { - return nil, &iamError{Code: iam.ErrCodeNoSuchEntityException, Error: err} - } - return nil, &iamError{Code: iam.ErrCodeServiceFailureException, Error: err} + return nil, oidcMutationError(err) } return &iamlib.RemoveClientIDFromOpenIDConnectProviderResponse{}, nil } @@ -212,6 +206,9 @@ func (e *EmbeddedIamApi) updateOpenIDConnectProviderThumbprint(ctx context.Conte return nil, &iamError{Code: iam.ErrCodeInvalidInputException, Error: errors.New("ThumbprintList must contain at least one entry")} } if err := mgr.UpdateOIDCProviderThumbprints(ctx, arn, thumbprints); err != nil { + if errors.Is(err, integration.ErrOIDCProviderStatic) { + return nil, oidcMutationError(err) + } if errors.Is(err, integration.ErrOIDCProviderNotFound) { return nil, &iamError{Code: iam.ErrCodeNoSuchEntityException, Error: err} } @@ -230,10 +227,7 @@ func (e *EmbeddedIamApi) tagOpenIDConnectProvider(ctx context.Context, mgr *inte return nil, &iamError{Code: iam.ErrCodeInvalidInputException, Error: errors.New("Tags must contain at least one Key/Value pair")} } if err := mgr.TagOIDCProvider(ctx, arn, tags); err != nil { - if errors.Is(err, integration.ErrOIDCProviderNotFound) { - return nil, &iamError{Code: iam.ErrCodeNoSuchEntityException, Error: err} - } - return nil, &iamError{Code: iam.ErrCodeServiceFailureException, Error: err} + return nil, oidcMutationError(err) } return &iamlib.TagOpenIDConnectProviderResponse{}, nil } @@ -248,10 +242,7 @@ func (e *EmbeddedIamApi) untagOpenIDConnectProvider(ctx context.Context, mgr *in return nil, &iamError{Code: iam.ErrCodeInvalidInputException, Error: errors.New("TagKeys must contain at least one entry")} } if err := mgr.UntagOIDCProvider(ctx, arn, keys); err != nil { - if errors.Is(err, integration.ErrOIDCProviderNotFound) { - return nil, &iamError{Code: iam.ErrCodeNoSuchEntityException, Error: err} - } - return nil, &iamError{Code: iam.ErrCodeServiceFailureException, Error: err} + return nil, oidcMutationError(err) } return &iamlib.UntagOpenIDConnectProviderResponse{}, nil } @@ -339,3 +330,17 @@ func (e *EmbeddedIamApi) getOpenIDConnectProvider(ctx context.Context, mgr *inte } return resp, nil } + +// oidcMutationError maps an IAMManager error from a provider change to its +// IAM error code. A provider defined in the IAM config file is changed there, +// not through the API. +func oidcMutationError(err error) *iamError { + switch { + case errors.Is(err, integration.ErrOIDCProviderStatic): + return &iamError{Code: iam.ErrCodeUnmodifiableEntityException, Error: err} + case errors.Is(err, integration.ErrOIDCProviderNotFound): + return &iamError{Code: iam.ErrCodeNoSuchEntityException, Error: err} + default: + return &iamError{Code: iam.ErrCodeServiceFailureException, Error: err} + } +} diff --git a/weed/s3api/s3api_iam_oidc_test.go b/weed/s3api/s3api_iam_oidc_test.go index cd503672c..70807ea80 100644 --- a/weed/s3api/s3api_iam_oidc_test.go +++ b/weed/s3api/s3api_iam_oidc_test.go @@ -2,6 +2,10 @@ package s3api import ( "context" + "fmt" + "github.com/aws/aws-sdk-go/service/iam" + "net/http" + "net/http/httptest" "net/url" "testing" "time" @@ -394,3 +398,18 @@ func TestUpdateThumbprintAndTags(t *testing.T) { t.Fatalf("Tags should be empty after untag, got: %v", gr.Tags) } } + +// A refusal to change a config-file provider reaches the client as AWS sends +// it (400 UnmodifiableEntity), not as an internal error clients retry. +func TestUnmodifiableEntityIsAClientError(t *testing.T) { + api := NewEmbeddedIamApiForTest() + rec := httptest.NewRecorder() + api.writeIamErrorResponse(rec, httptest.NewRequest(http.MethodPost, "/", nil), "req-1", + oidcMutationError(fmt.Errorf("%w: arn:aws:iam:::oidc-provider/static.example", integration.ErrOIDCProviderStatic))) + if rec.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400", rec.Code) + } + if code, _ := extractEmbeddedIamErrorCodeAndMessage(rec); code != iam.ErrCodeUnmodifiableEntityException { + t.Fatalf("code = %q, want UnmodifiableEntity", code) + } +} diff --git a/weed/s3api/s3api_server.go b/weed/s3api/s3api_server.go index b9c6205ee..e54d2ebfa 100644 --- a/weed/s3api/s3api_server.go +++ b/weed/s3api/s3api_server.go @@ -1168,7 +1168,10 @@ func loadIAMManagerFromConfig(configPath string, filerAddressProvider func() str Policy *policy.PolicyEngineConfig `json:"policy"` Providers []map[string]interface{} `json:"providers"` Roles []*integration.RoleDefinition `json:"roles"` - Policies []struct { + // OIDCProviderStore selects where IAM-managed OIDC providers persist. + // Absent, they live in memory and are lost on restart. + OIDCProviderStore *integration.OIDCProviderStoreConfig `json:"oidcProviderStore"` + Policies []struct { Name string `json:"name"` Document *policy.PolicyDocument `json:"document"` } `json:"policies"` @@ -1213,6 +1216,15 @@ func loadIAMManagerFromConfig(configPath string, filerAddressProvider func() str glog.V(1).Infof("Using policy defaults: DefaultEffect=%s, StoreType=%s", configRoot.Policy.DefaultEffect, configRoot.Policy.StoreType) } + // With no IAM config file there is nothing static for a persisted + // provider to shadow or outlive, so providers created at runtime default + // to the filer, where restarts and peer S3 servers see them. A config + // file keeps the in-memory default unless it sets oidcProviderStore. + oidcProviderStore := configRoot.OIDCProviderStore + if oidcProviderStore == nil && configPath == "" && filerAddressProvider != nil { + oidcProviderStore = &integration.OIDCProviderStoreConfig{StoreType: "filer"} + } + // Create IAM configuration iamConfig := &integration.IAMConfig{ STS: configRoot.STS, @@ -1220,6 +1232,7 @@ func loadIAMManagerFromConfig(configPath string, filerAddressProvider func() str Roles: &integration.RoleStoreConfig{ StoreType: sts.StoreTypeMemory, // Use memory store for JSON config-based setup }, + OIDCProviders: oidcProviderStore, } // Apply default signing key if not present in config