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) <noreply@anthropic.com>

weed/iam/integration/iam_manager.go

@@ -32,6 +32,10 @@ type IAMManager struct {
 	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
 	oidcAuditSink        OIDCProviderAuditSink
 	revocationStore      SessionRevocationStore
 	filerAddressProvider func() string // Function to get current filer address
@@ -127,7 +131,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 +145,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 +186,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 +212,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 +232,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 +260,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 +297,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 +316,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 +339,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,92 +599,88 @@ func (m *IAMManager) initOIDCProviderStore(config *IAMConfig) error {
 	if err != nil {
 		return err
 	}
-	m.oidcProviderStore = store
+	m.installOIDCProviderStore(store, config.STS)
-
-	mirrored := map[string]bool{}
-	defer m.pruneAndHydrateOIDCProviders(context.Background(), mirrored)
-	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.
+func (m *IAMManager) installOIDCProviderStore(store OIDCProviderStore, stsConfig *sts.STSConfig) {
+	m.oidcProviderStore = store
+	m.staticOIDCProviders = staticOIDCProviderRecords(stsConfig)
+	if _, inMemory := store.(*MemoryOIDCProviderStore); inMemory {
+		ctx := context.Background()
+		for _, rec := range m.staticOIDCProviders {
+			if err := store.StoreProvider(ctx, m.getFilerAddress(), copyOIDCProviderRecord(rec)); err != nil {
+				glog.Warningf("mirror static OIDC provider %s into store: %v", rec.ARN, err)
 			}
-	for _, pc := range config.STS.Providers {
+		}
+		// 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)
+		go m.retryOIDCProviderLoad()
+	}
+}
+
+// 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
+	}
+	now := time.Now().UTC()
+	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 := ""
+		arn, err := DeriveOIDCProviderARN(stsConfig.AccountId, issuer)
-		if config.STS != nil {
-			accountID = config.STS.AccountId
-		}
-		arn, err := DeriveOIDCProviderARN(accountID, issuer)
 		if err != nil {
 			glog.Warningf("derive ARN for static OIDC provider %s: %v", pc.Name, err)
 			continue
 		}
-		clientIDs := extractClientIDs(pc.Config)
+		out[arn] = &OIDCProviderRecord{
-		ctx := context.Background()
+			AccountID:               stsConfig.AccountId,
-		// 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,
 			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"),
-			Source:                  OIDCProviderSourceStaticConfig,
+			CreatedAt:               now,
-			CreatedAt:               createdAt,
 			UpdatedAt:               now,
 		}
-		if err := store.StoreProvider(ctx, m.getFilerAddress(), rec); err != nil {
-			glog.Warningf("mirror static OIDC provider %s into store: %v", pc.Name, err)
-		}
-		mirrored[arn] = true
-	}
-	return nil
-}
-
-// pruneAndHydrateOIDCProviders runs after the static mirror when the store
-// outlives the process. A record the static config seeded on an earlier boot
-// and no longer lists is deleted, so removing a provider from the config file
-// still revokes it. The runtime STS view is then loaded from the store, since
-// providers created through the IAM API on an earlier boot, or on a peer, are
-// otherwise unknown until the next mutation. An in-memory store holds nothing
-// from before this boot, so it needs neither.
-func (m *IAMManager) pruneAndHydrateOIDCProviders(ctx context.Context, mirrored map[string]bool) {
-	if _, inMemory := m.oidcProviderStore.(*MemoryOIDCProviderStore); inMemory {
-		return
-	}
-	if err := m.pruneAndHydrateOnce(ctx, mirrored); 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)
-		keep := make(map[string]bool, len(mirrored))
-		for arn := range mirrored {
-			keep[arn] = true
-		}
-		go m.retryPruneAndHydrate(keep)
 	}
+	return out
 }
 
 // oidcHydrateRetry bounds the backoff between startup load attempts.
 var oidcHydrateRetry = struct{ initial, max time.Duration }{initial: time.Second, max: 30 * time.Second}
 
-func (m *IAMManager) retryPruneAndHydrate(mirrored map[string]bool) {
+func (m *IAMManager) retryOIDCProviderLoad() {
 	delay := oidcHydrateRetry.initial
 	for {
 		time.Sleep(delay)
-		err := m.pruneAndHydrateOnce(context.Background(), mirrored)
+		err := m.RefreshOIDCProvidersFromStore(context.Background())
 		if err == nil {
 			glog.V(0).Infof("loaded OIDC providers from the store after retrying")
 			return
@@ -654,26 +692,6 @@ func (m *IAMManager) retryPruneAndHydrate(mirrored map[string]bool) {
 	}
 }
 
-// pruneAndHydrateOnce deletes static-config records the config no longer
-// lists, then loads the store into STS. It fails when the store cannot be read.
-func (m *IAMManager) pruneAndHydrateOnce(ctx context.Context, mirrored map[string]bool) error {
-	records, err := m.oidcProviderStore.ListProviders(ctx, m.getFilerAddress())
-	if err != nil {
-		return fmt.Errorf("list OIDC providers: %w", err)
-	}
-	for _, rec := range records {
-		if rec == nil || rec.Source != OIDCProviderSourceStaticConfig || mirrored[rec.ARN] {
-			continue
-		}
-		if err := m.oidcProviderStore.DeleteProvider(ctx, m.getFilerAddress(), rec.ARN); err != nil {
-			glog.Warningf("prune OIDC provider %s removed from static config: %v", rec.ARN, err)
-			continue
-		}
-		glog.V(1).Infof("pruned OIDC provider %s: no longer in static config", rec.ARN)
-	}
-	return m.RefreshOIDCProvidersFromStore(ctx)
-}
-
 // refreshOIDCProvidersBestEffort calls RefreshOIDCProvidersFromStore and
 // logs a warning on failure. The IAM API call has already succeeded by the
 // time we get here, so a refresh failure must not surface to the caller —
@@ -733,6 +751,8 @@ func buildOIDCProviderFromRecord(rec *OIDCProviderRecord) (*oidc.OIDCProvider, e
 		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 {

weed/iam/integration/oidc_provider_persist_test.go

@@ -12,75 +12,66 @@ import (
 	"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 (the
+// persistentTestStore stands in for a store that outlives the process and may
-// filer store): anything but *MemoryOIDCProviderStore is treated as persistent.
+// be shared (the filer store): anything but *MemoryOIDCProviderStore is
+// treated as persistent.
 type persistentTestStore struct{ *MemoryOIDCProviderStore }
 
 const (
-	persistTestCurrentIssuer = "https://current.example"
+	persistTestStaticIssuer = "https://static.example"
-	persistTestStaleIssuer   = "https://stale.example"
 	persistTestAPIIssuer    = "https://api.example"
 )
 
-func newPersistTestManager(t *testing.T) *IAMManager {
+// persistTestConfig is an IAM config whose file defines the given OIDC
-	t.Helper()
+// issuers.
-	mgr := NewIAMManager()
+func persistTestConfig(issuers ...string) *IAMConfig {
-	cfg := &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: []*sts.ProviderConfig{{
+			Providers:        providers,
-				Name:    "current",
-				Type:    sts.ProviderTypeOIDC,
-				Enabled: true,
-				Config:  map[string]interface{}{"issuer": persistTestCurrentIssuer, "clientId": "aud"},
-			}},
 		},
 		Policy: &policy.PolicyEngineConfig{DefaultEffect: "Deny", StoreType: "memory"},
 		Roles:  &RoleStoreConfig{StoreType: "memory"},
 	}
-	if err := mgr.Initialize(cfg, func() string { return "localhost:8888" }); err != nil {
+}
-		t.Fatalf("Initialize: %v", err)
+
-	}
+// 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
 }
 
-// seedEarlierBoot fills a store as an earlier boot would have left it: the
+func arnOf(t *testing.T, issuer string) string {
-// provider the config still lists, one it has since dropped, and one created
-// through the IAM API.
-func seedEarlierBoot(t *testing.T, store OIDCProviderStore) (current, stale, api string) {
 	t.Helper()
-	ctx := context.Background()
+	arn, err := DeriveOIDCProviderARN("", issuer)
-	for _, r := range []struct{ issuer, source string }{
+	require.NoError(t, err)
-		{persistTestCurrentIssuer, OIDCProviderSourceStaticConfig},
+	return arn
-		{persistTestStaleIssuer, OIDCProviderSourceStaticConfig},
-		{persistTestAPIIssuer, ""},
-	} {
-		arn, err := DeriveOIDCProviderARN("", r.issuer)
-		if err != nil {
-			t.Fatalf("derive ARN: %v", err)
-		}
-		rec := &OIDCProviderRecord{ARN: arn, URL: r.issuer, ClientIDs: []string{"aud"}, Source: r.source}
-		if err := store.StoreProvider(ctx, "", rec); err != nil {
-			t.Fatalf("seed %s: %v", r.issuer, err)
-		}
-	}
-	current, _ = DeriveOIDCProviderARN("", persistTestCurrentIssuer)
-	stale, _ = DeriveOIDCProviderARN("", persistTestStaleIssuer)
-	api, _ = DeriveOIDCProviderARN("", persistTestAPIIssuer)
-	return current, stale, api
 }
 
-func storedARNs(t *testing.T, mgr *IAMManager) map[string]bool {
+func listedARNs(t *testing.T, mgr *IAMManager) map[string]bool {
 	t.Helper()
 	recs, err := mgr.ListOIDCProviders(context.Background())
-	if err != nil {
+	require.NoError(t, err)
-		t.Fatalf("ListOIDCProviders: %v", err)
-	}
 	out := map[string]bool{}
 	for _, r := range recs {
 		out[r.ARN] = true
@@ -88,6 +79,24 @@ func storedARNs(t *testing.T, mgr *IAMManager) map[string]bool {
 	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 {
@@ -95,71 +104,92 @@ func stsKnowsIssuer(t *testing.T, mgr *IAMManager, issuer string) bool {
 	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"))
-	if err != nil {
+	require.NoError(t, err)
-		t.Fatalf("mint token: %v", err)
-	}
 	_, _, err = mgr.GetSTSService().ValidateWebIdentityToken(context.Background(), tok)
-	if err == nil {
+	require.Error(t, err, "an unsigned-by-provider token for %s was accepted", issuer)
-		t.Fatalf("an unsigned-by-provider token for %s was accepted", issuer)
-	}
 	return !strings.Contains(err.Error(), "no identity provider registered")
 }
 
-func TestPersistentStorePrunesDroppedStaticProvidersAndHydratesSTS(t *testing.T) {
+// A persistent store never receives the config file's providers: it outlives
-	mgr := newPersistTestManager(t)
+// the file and may be shared with servers whose files differ.
+func TestPersistentStoreNeverHoldsConfigFileProviders(t *testing.T) {
 	store := &persistentTestStore{NewMemoryOIDCProviderStore()}
-	current, stale, api := seedEarlierBoot(t, store)
+	mgr := startServer(t, store, persistTestStaticIssuer)
-	mgr.SetOIDCProviderStore(store)
 
-	if stsKnowsIssuer(t, mgr, persistTestAPIIssuer) {
+	assert.Empty(t, storedARNs(t, store), "a config-file provider was written to the persistent store")
-		t.Fatal("precondition: STS already knows the API-created issuer before hydration")
+	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)
-	mgr.pruneAndHydrateOIDCProviders(context.Background(), map[string]bool{current: true})
+	assert.Equal(t, persistTestStaticIssuer, rec.URL)
-
+	assert.True(t, stsKnowsIssuer(t, mgr, persistTestStaticIssuer), "STS stopped trusting the config-file provider")
-	got := storedARNs(t, mgr)
-	if got[stale] {
-		t.Errorf("provider dropped from static config is still stored: %s", stale)
-	}
-	if !got[current] {
-		t.Errorf("provider still in static config was pruned: %s", current)
-	}
-	if !got[api] {
-		t.Errorf("provider created through the IAM API was pruned: %s", api)
-	}
-	if !stsKnowsIssuer(t, mgr, persistTestAPIIssuer) {
-		t.Error("STS does not trust the API-created provider found in the store at startup")
-	}
-	if stsKnowsIssuer(t, mgr, persistTestStaleIssuer) {
-		t.Error("STS still trusts the provider dropped from static config")
-	}
 }
 
-func TestInMemoryStoreIsNeitherPrunedNorHydrated(t *testing.T) {
+// Servers sharing a store, with different config files, must not remove or
-	mgr := newPersistTestManager(t)
+// 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()
-	current, stale, _ := seedEarlierBoot(t, store)
+	startServer(t, store, persistTestStaticIssuer)
-	mgr.SetOIDCProviderStore(store)
+	assert.True(t, storedARNs(t, store)[arnOf(t, persistTestStaticIssuer)])
-
-	mgr.pruneAndHydrateOIDCProviders(context.Background(), map[string]bool{current: true})
-
-	if !storedARNs(t, mgr)[stale] {
-		t.Error("an in-memory store was pruned; it holds nothing from an earlier boot")
-	}
-	if stsKnowsIssuer(t, mgr, persistTestAPIIssuer) {
-		t.Error("an in-memory store was hydrated into STS at startup")
-	}
-}
-
-func TestStaticMirrorMarksItsRecords(t *testing.T) {
-	mgr := newPersistTestManager(t)
-	recs, err := mgr.ListOIDCProviders(context.Background())
-	if err != nil {
-		t.Fatalf("ListOIDCProviders: %v", err)
-	}
-	if len(recs) != 1 || recs[0].Source != OIDCProviderSourceStaticConfig {
-		t.Fatalf("static mirror did not mark its record: %+v", recs)
-	}
 }
 
 // unreachableThenReadyStore fails its first reads, as a filer that is not up
@@ -189,12 +219,11 @@ func TestStartupLoadRetriesUntilTheStoreIsReadable(t *testing.T) {
 	oidcHydrateRetry.initial, oidcHydrateRetry.max = time.Millisecond, 5*time.Millisecond
 	t.Cleanup(func() { oidcHydrateRetry = saved })
 
-	mgr := newPersistTestManager(t)
+	seeded := NewMemoryOIDCProviderStore()
-	store := &unreachableThenReadyStore{MemoryOIDCProviderStore: NewMemoryOIDCProviderStore(), failsLeft: 3}
+	require.NoError(t, seeded.StoreProvider(context.Background(), "", &OIDCProviderRecord{
-	current, stale, api := seedEarlierBoot(t, store)
+		ARN: arnOf(t, persistTestAPIIssuer), URL: persistTestAPIIssuer, ClientIDs: []string{"aud"},
-	mgr.SetOIDCProviderStore(store)
+	}))
-
+	mgr := startServer(t, &unreachableThenReadyStore{MemoryOIDCProviderStore: seeded, failsLeft: 3})
-	mgr.pruneAndHydrateOIDCProviders(context.Background(), map[string]bool{current: true})
 
 	deadline := time.Now().Add(2 * time.Second)
 	for !stsKnowsIssuer(t, mgr, persistTestAPIIssuer) {
@@ -203,7 +232,4 @@ func TestStartupLoadRetriesUntilTheStoreIsReadable(t *testing.T) {
 		}
 		time.Sleep(5 * time.Millisecond)
 	}
-	got := storedARNs(t, mgr)
-	assert.False(t, got[stale], "the provider dropped from static config was not pruned on retry")
-	assert.True(t, got[api])
 }

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
@@ -71,20 +74,10 @@ type OIDCProviderRecord struct {
 	// (audit/inventory metadata, not propagated into sessions).
 	Tags map[string]string `json:"tags,omitempty"`
 
-	// Source records where the entry came from. OIDCProviderSourceStaticConfig
-	// marks a record mirrored from STS.Providers at boot; empty means it was
-	// created through the IAM API. Only static-config records are pruned when
-	// their provider leaves the configuration.
-	Source string `json:"source,omitempty"`
-
 	CreatedAt time.Time `json:"createdAt"`
 	UpdatedAt time.Time `json:"updatedAt"`
 }
 
-// OIDCProviderSourceStaticConfig is the Source of a record mirrored from the
-// static STS provider configuration.
-const OIDCProviderSourceStaticConfig = "static-config"
-
 // OIDCProviderStore stores OIDCProviderRecord entries. Implementations are
 // expected to be safe for concurrent use.
 type OIDCProviderStore interface {

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, oidcMutationError(err)
-			return nil, &iamError{Code: iam.ErrCodeNoSuchEntityException, Error: err}
-		}
-		return nil, &iamError{Code: iam.ErrCodeServiceFailureException, Error: 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, oidcMutationError(err)
-			return nil, &iamError{Code: iam.ErrCodeNoSuchEntityException, Error: err}
-		}
-		return nil, &iamError{Code: iam.ErrCodeServiceFailureException, Error: 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, oidcMutationError(err)
-			return nil, &iamError{Code: iam.ErrCodeNoSuchEntityException, Error: err}
-		}
-		return nil, &iamError{Code: iam.ErrCodeServiceFailureException, Error: 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, oidcMutationError(err)
-			return nil, &iamError{Code: iam.ErrCodeNoSuchEntityException, Error: err}
-		}
-		return nil, &iamError{Code: iam.ErrCodeServiceFailureException, Error: 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}
+	}
+}