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