diff --git a/weed/iam/integration/iam_manager.go b/weed/iam/integration/iam_manager.go index dd825db67..8d73906b2 100644 --- a/weed/iam/integration/iam_manager.go +++ b/weed/iam/integration/iam_manager.go @@ -27,11 +27,15 @@ 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 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) + return nil +} - 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) + } + } + // 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 } - for _, pc := range config.STS.Providers { + 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 := "" - 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"), - Source: OIDCProviderSourceStaticConfig, - CreatedAt: createdAt, + CreatedAt: now, 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 — @@ -730,9 +748,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 index 5011b03a7..31a2241b8 100644 --- a/weed/iam/integration/oidc_provider_persist_test.go +++ b/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 -// filer store): anything but *MemoryOIDCProviderStore is treated as persistent. +// 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 ( - persistTestCurrentIssuer = "https://current.example" - persistTestStaleIssuer = "https://stale.example" - persistTestAPIIssuer = "https://api.example" + persistTestStaticIssuer = "https://static.example" + persistTestAPIIssuer = "https://api.example" ) -func newPersistTestManager(t *testing.T) *IAMManager { - t.Helper() - mgr := NewIAMManager() - cfg := &IAMConfig{ +// 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: []*sts.ProviderConfig{{ - Name: "current", - Type: sts.ProviderTypeOIDC, - Enabled: true, - Config: map[string]interface{}{"issuer": persistTestCurrentIssuer, "clientId": "aud"}, - }}, + Providers: providers, }, 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 -// 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) { +func arnOf(t *testing.T, issuer string) string { t.Helper() - ctx := context.Background() - for _, r := range []struct{ issuer, source string }{ - {persistTestCurrentIssuer, OIDCProviderSourceStaticConfig}, - {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 + arn, err := DeriveOIDCProviderARN("", issuer) + require.NoError(t, err) + return arn } -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 { - t.Fatalf("ListOIDCProviders: %v", err) - } + require.NoError(t, 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 { - t.Fatalf("mint token: %v", err) - } + require.NoError(t, err) _, _, err = mgr.GetSTSService().ValidateWebIdentityToken(context.Background(), tok) - if err == nil { - t.Fatalf("an unsigned-by-provider token for %s was accepted", issuer) - } + require.Error(t, err, "an unsigned-by-provider token for %s was accepted", issuer) return !strings.Contains(err.Error(), "no identity provider registered") } -func TestPersistentStorePrunesDroppedStaticProvidersAndHydratesSTS(t *testing.T) { - mgr := newPersistTestManager(t) +// 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()} - current, stale, api := seedEarlierBoot(t, store) - mgr.SetOIDCProviderStore(store) + mgr := startServer(t, store, persistTestStaticIssuer) - if stsKnowsIssuer(t, mgr, persistTestAPIIssuer) { - t.Fatal("precondition: STS already knows the API-created issuer before hydration") - } - - mgr.pruneAndHydrateOIDCProviders(context.Background(), map[string]bool{current: true}) - - 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") - } + 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") } -func TestInMemoryStoreIsNeitherPrunedNorHydrated(t *testing.T) { - mgr := newPersistTestManager(t) +// 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() - current, stale, _ := seedEarlierBoot(t, store) - mgr.SetOIDCProviderStore(store) - - 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) - } + 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 @@ -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) - store := &unreachableThenReadyStore{MemoryOIDCProviderStore: NewMemoryOIDCProviderStore(), failsLeft: 3} - current, stale, api := seedEarlierBoot(t, store) - mgr.SetOIDCProviderStore(store) - - mgr.pruneAndHydrateOIDCProviders(context.Background(), map[string]bool{current: true}) + 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) { @@ -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]) } diff --git a/weed/iam/integration/oidc_provider_store.go b/weed/iam/integration/oidc_provider_store.go index 3b23b40a5..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 @@ -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 { 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} + } +}