diff --git a/weed/s3api/auth_credentials.go b/weed/s3api/auth_credentials.go index 6cb19d241..a95094211 100644 --- a/weed/s3api/auth_credentials.go +++ b/weed/s3api/auth_credentials.go @@ -267,17 +267,6 @@ func NewIdentityAccessManagementWithStore(option *S3ApiServerOption, filerClient if err := iam.loadS3ApiConfigurationFromFile(startConfigFile); err != nil { glog.Fatalf("fail to load config file %s: %v", startConfigFile, err) } - - // Track identity names from static config to protect them from dynamic updates - // Must be done under lock to avoid race conditions - iam.m.Lock() - iam.useStaticConfig = true - iam.staticIdentityNames = make(map[string]bool) - for _, identity := range iam.identities { - iam.staticIdentityNames[identity.Name] = true - identity.IsStatic = true - } - iam.m.Unlock() } // Always try to load/merge config from credential manager (filer/db) @@ -332,6 +321,30 @@ func NewIdentityAccessManagementWithStore(option *S3ApiServerOption, filerClient return iam } +// markStaticIdentities marks the identities declared in a static config file +// (-config, or -iam.config when it carries inline identities) as immutable, so +// dynamic filer reloads can't overwrite them. It is additive and scoped to the +// file's identities: a reload protects newly added ones without un-protecting +// the existing set or freezing dynamic filer-managed identities. useStaticConfig +// stays gated on whether any static identity exists, so an advanced-IAM file +// with no inline identities (OIDC/STS only) keeps the dynamic store live. +func (iam *IdentityAccessManagement) markStaticIdentities(config *iam_pb.S3ApiConfiguration) { + iam.m.Lock() + defer iam.m.Unlock() + if iam.staticIdentityNames == nil { + iam.staticIdentityNames = make(map[string]bool) + } + for _, ident := range config.Identities { + iam.staticIdentityNames[ident.Name] = true + } + for _, identity := range iam.identities { + if iam.staticIdentityNames[identity.Name] { + identity.IsStatic = true + } + } + iam.useStaticConfig = len(iam.staticIdentityNames) > 0 +} + func (iam *IdentityAccessManagement) pollIamConfigChanges(interval time.Duration) { ticker := time.NewTicker(interval) defer ticker.Stop() @@ -477,24 +490,40 @@ func (iam *IdentityAccessManagement) loadS3ApiConfigurationFromFile(fileName str glog.Warningf("KMS initialization failed: %v", err) } - return iam.LoadS3ApiConfigurationFromBytes(content) + config, err := iam.loadS3ApiConfigurationFromBytes(content) + if err != nil { + return err + } + // Identities listed in a config file are static (immutable). Mark them on + // every load so a reload protects newly added identities too, not just the + // set present at startup, and push the updated set into the credential + // manager so reloaded identities still show up in listings and survive + // later dynamic merges. + iam.markStaticIdentities(config) + iam.updateCredentialManagerStaticIdentities() + return nil } func (iam *IdentityAccessManagement) LoadS3ApiConfigurationFromBytes(content []byte) error { + _, err := iam.loadS3ApiConfigurationFromBytes(content) + return err +} + +func (iam *IdentityAccessManagement) loadS3ApiConfigurationFromBytes(content []byte) (*iam_pb.S3ApiConfiguration, error) { s3ApiConfiguration := &iam_pb.S3ApiConfiguration{} if err := filer.ParseS3ConfigurationFromBytes(content, s3ApiConfiguration); err != nil { glog.Warningf("unmarshal error: %v", err) - return fmt.Errorf("unmarshal error: %w", err) + return nil, fmt.Errorf("unmarshal error: %w", err) } if err := filer.CheckDuplicateAccessKey(s3ApiConfiguration); err != nil { - return err + return nil, err } if err := iam.loadS3ApiConfiguration(s3ApiConfiguration); err != nil { - return err + return nil, err } - return nil + return s3ApiConfiguration, nil } func (iam *IdentityAccessManagement) loadS3ApiConfiguration(config *iam_pb.S3ApiConfiguration) error { diff --git a/weed/s3api/auth_credentials_static_config_test.go b/weed/s3api/auth_credentials_static_config_test.go new file mode 100644 index 000000000..b08203778 --- /dev/null +++ b/weed/s3api/auth_credentials_static_config_test.go @@ -0,0 +1,125 @@ +package s3api + +import ( + "context" + "os" + "path/filepath" + "testing" + + _ "github.com/seaweedfs/seaweedfs/weed/credential/memory" + "github.com/seaweedfs/seaweedfs/weed/filer" + "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" + "github.com/seaweedfs/seaweedfs/weed/pb/iam_pb" +) + +// An advanced -iam.config file (STS/OIDC/roles) carries no inline identities, so the +// server must not enter static-config mode. Otherwise it freezes live reloads and +// filer-backed identities created at runtime (e.g. by the operator's IAM CRDs) never +// take effect. +func TestIamConfigWithoutIdentitiesIsNotStatic(t *testing.T) { + s3a := newTestS3ApiServerWithMemoryIAM(t, []*iam_pb.Identity{}) + + path := writeTempIamConfig(t, `{"sts":{"signingKey":"dGVzdC1zaWduaW5nLWtleQ=="}}`) + if err := s3a.iam.loadS3ApiConfigurationFromFile(path); err != nil { + t.Fatalf("failed to load advanced iam config: %v", err) + } + + if s3a.iam.IsStaticConfig() { + t.Fatalf("advanced iam config without identities must not be treated as static") + } + + // A filer change (operator creating a user) must still reload at runtime. + if err := s3a.iam.credentialManager.CreateUser(context.Background(), &iam_pb.Identity{Name: "alice"}); err != nil { + t.Fatalf("failed to create alice: %v", err) + } + if err := s3a.onIamConfigChange(filer.IamConfigDirectory+"/identities", nil, &filer_pb.Entry{Name: "alice.json"}); err != nil { + t.Fatalf("onIamConfigChange returned error: %v", err) + } + if !hasIdentity(s3a.iam, "alice") { + t.Fatalf("expected alice to load after filer change with -iam.config-only setup") + } +} + +// A -config identity file marks its identities static, protecting them and keeping the +// established behavior of not live-reloading those from the filer. +func TestConfigWithIdentitiesIsStatic(t *testing.T) { + s3a := newTestS3ApiServerWithMemoryIAM(t, []*iam_pb.Identity{}) + + path := writeTempIamConfig(t, `{"identities":[{"name":"static-admin","credentials":[{"accessKey":"AKIAITEST","secretKey":"c2VjcmV0"}],"actions":["Admin"]}]}`) + if err := s3a.iam.loadS3ApiConfigurationFromFile(path); err != nil { + t.Fatalf("failed to load identity config: %v", err) + } + + if !s3a.iam.IsStaticConfig() { + t.Fatalf("config file with inline identities must be treated as static") + } + + s3a.iam.m.RLock() + id := s3a.iam.nameToIdentity["static-admin"] + s3a.iam.m.RUnlock() + if id == nil || !id.IsStatic { + t.Fatalf("expected static-admin to be marked static") + } + + // A static identity file does not live-reload dynamic identities from the filer. + if err := s3a.iam.credentialManager.CreateUser(context.Background(), &iam_pb.Identity{Name: "alice"}); err != nil { + t.Fatalf("failed to create alice: %v", err) + } + if err := s3a.onIamConfigChange(filer.IamConfigDirectory+"/identities", nil, &filer_pb.Entry{Name: "alice.json"}); err != nil { + t.Fatalf("onIamConfigChange returned error: %v", err) + } + if hasIdentity(s3a.iam, "alice") { + t.Fatalf("did not expect alice to load while running off a static identity file") + } +} + +// Reloading the static config file (grace.OnReload) must mark newly added +// identities as static so dynamic filer updates can't overwrite them, while +// leaving already-loaded dynamic (filer-managed) identities untouched. +func TestReloadStaticConfigMarksNewIdentitiesWithoutFreezingDynamic(t *testing.T) { + s3a := newTestS3ApiServerWithMemoryIAM(t, []*iam_pb.Identity{}) + + p1 := writeTempIamConfig(t, `{"identities":[{"name":"static-admin","credentials":[{"accessKey":"AKADMIN0","secretKey":"c2VjcmV0"}],"actions":["Admin"]}]}`) + if err := s3a.iam.loadS3ApiConfigurationFromFile(p1); err != nil { + t.Fatalf("failed to load initial config: %v", err) + } + + // A dynamic identity arrives from the filer; merge mode keeps it dynamic. + if err := s3a.iam.credentialManager.CreateUser(context.Background(), &iam_pb.Identity{Name: "alice"}); err != nil { + t.Fatalf("failed to create alice: %v", err) + } + if err := s3a.iam.LoadS3ApiConfigurationFromCredentialManager(); err != nil { + t.Fatalf("failed to load from credential manager: %v", err) + } + if !hasIdentity(s3a.iam, "alice") { + t.Fatalf("expected alice to load dynamically") + } + + // Reload the static file with a new identity bob. + p2 := writeTempIamConfig(t, `{"identities":[{"name":"static-admin","credentials":[{"accessKey":"AKADMIN0","secretKey":"c2VjcmV0"}],"actions":["Admin"]},{"name":"bob","credentials":[{"accessKey":"AKBOB000","secretKey":"c2VjcmV0"}],"actions":["Read"]}]}`) + if err := s3a.iam.loadS3ApiConfigurationFromFile(p2); err != nil { + t.Fatalf("failed to reload config: %v", err) + } + + if !isStaticName(s3a.iam, "bob") { + t.Fatalf("expected reloaded identity bob to be marked static") + } + if isStaticName(s3a.iam, "alice") { + t.Fatalf("dynamic identity alice must not be frozen as static by a config reload") + } +} + +func isStaticName(iam *IdentityAccessManagement, name string) bool { + iam.m.RLock() + defer iam.m.RUnlock() + return iam.staticIdentityNames[name] +} + +func writeTempIamConfig(t *testing.T, content string) string { + t.Helper() + path := filepath.Join(t.TempDir(), "iam.json") + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + t.Fatalf("failed to write temp config: %v", err) + } + return path +} diff --git a/weed/s3api/s3api_server.go b/weed/s3api/s3api_server.go index 88029d70e..dcd84d396 100644 --- a/weed/s3api/s3api_server.go +++ b/weed/s3api/s3api_server.go @@ -392,7 +392,6 @@ func NewS3ApiServerWithStore(router *mux.Router, option *S3ApiServerOption, expl glog.Errorf("fail to load config file %s: %v", option.Config, err) } else { glog.V(1).Infof("Loaded %d identities from config file %s", len(s3ApiServer.iam.identities), option.Config) - s3ApiServer.iam.updateCredentialManagerStaticIdentities() } }) }