diff --git a/weed/s3api/auth_credentials.go b/weed/s3api/auth_credentials.go index 1a11b490c..044c4a6c4 100644 --- a/weed/s3api/auth_credentials.go +++ b/weed/s3api/auth_credentials.go @@ -503,7 +503,9 @@ func (iam *IdentityAccessManagement) loadS3ApiConfigurationFromFile(fileName str glog.Warningf("KMS initialization failed: %v", err) } - config, err := iam.loadS3ApiConfigurationFromBytes(content) + // fromStaticFile=true: this file is the source of truth for its static + // identities, so a reload overwrites them (e.g. a rotated secretKey). + config, err := iam.loadS3ApiConfigurationFromBytes(content, true) if err != nil { return err } @@ -518,11 +520,11 @@ func (iam *IdentityAccessManagement) loadS3ApiConfigurationFromFile(fileName str } func (iam *IdentityAccessManagement) LoadS3ApiConfigurationFromBytes(content []byte) error { - _, err := iam.loadS3ApiConfigurationFromBytes(content) + _, err := iam.loadS3ApiConfigurationFromBytes(content, false) return err } -func (iam *IdentityAccessManagement) loadS3ApiConfigurationFromBytes(content []byte) (*iam_pb.S3ApiConfiguration, error) { +func (iam *IdentityAccessManagement) loadS3ApiConfigurationFromBytes(content []byte, fromStaticFile bool) (*iam_pb.S3ApiConfiguration, error) { s3ApiConfiguration := &iam_pb.S3ApiConfiguration{} if err := filer.ParseS3ConfigurationFromBytes(content, s3ApiConfiguration); err != nil { glog.Warningf("unmarshal error: %v", err) @@ -533,13 +535,19 @@ func (iam *IdentityAccessManagement) loadS3ApiConfigurationFromBytes(content []b return nil, err } - if err := iam.loadS3ApiConfiguration(s3ApiConfiguration); err != nil { + if err := iam.loadS3ApiConfigurationWithSource(s3ApiConfiguration, fromStaticFile); err != nil { return nil, err } return s3ApiConfiguration, nil } func (iam *IdentityAccessManagement) loadS3ApiConfiguration(config *iam_pb.S3ApiConfiguration) error { + return iam.loadS3ApiConfigurationWithSource(config, false) +} + +// fromStaticFile lets a config-file reload overwrite its static identities; +// dynamic updates keep them immutable. +func (iam *IdentityAccessManagement) loadS3ApiConfigurationWithSource(config *iam_pb.S3ApiConfiguration, fromStaticFile bool) error { // Check if we need to merge with existing static configuration iam.m.RLock() hasStaticConfig := iam.useStaticConfig && len(iam.staticIdentityNames) > 0 @@ -547,7 +555,7 @@ func (iam *IdentityAccessManagement) loadS3ApiConfiguration(config *iam_pb.S3Api if hasStaticConfig { // Merge mode: preserve static identities, add/update dynamic ones - return iam.MergeS3ApiConfiguration(config) + return iam.MergeS3ApiConfiguration(config, fromStaticFile) } // Normal mode: completely replace configuration @@ -766,10 +774,9 @@ func (iam *IdentityAccessManagement) ReplaceS3ApiConfiguration(config *iam_pb.S3 return nil } -// MergeS3ApiConfiguration merges dynamic configuration with existing static configuration -// Static identities (from file) are preserved and cannot be updated -// Dynamic identities (from filer/admin) can be added or updated -func (iam *IdentityAccessManagement) MergeS3ApiConfiguration(config *iam_pb.S3ApiConfiguration) error { +// MergeS3ApiConfiguration adds/updates dynamic identities while preserving static +// ones. A config-file reload (fromStaticFile) may also overwrite its static identities. +func (iam *IdentityAccessManagement) MergeS3ApiConfiguration(config *iam_pb.S3ApiConfiguration, fromStaticFile bool) error { // Start with current configuration (which includes static identities) iam.m.RLock() identities := make([]*Identity, len(iam.identities)) @@ -834,15 +841,14 @@ func (iam *IdentityAccessManagement) MergeS3ApiConfiguration(config *iam_pb.S3Ap emailAccount[AccountAnonymous.EmailAddress] = accounts[AccountAnonymous.Id] } - // Process identities from dynamic config for _, ident := range config.Identities { - // Skip static identities - they cannot be updated - if staticNames[ident.Name] { + // Static identities are immutable to dynamic updates, but the config file can update them. + if !fromStaticFile && staticNames[ident.Name] { glog.V(3).Infof("skipping static identity %s (immutable)", ident.Name) continue } - glog.V(3).Infof("loading/updating dynamic identity %s (disabled=%v)", ident.Name, ident.Disabled) + glog.V(3).Infof("loading/updating identity %s (disabled=%v)", ident.Name, ident.Disabled) t := &Identity{ Name: ident.Name, Credentials: nil, @@ -850,6 +856,9 @@ func (iam *IdentityAccessManagement) MergeS3ApiConfiguration(config *iam_pb.S3Ap PrincipalArn: generatePrincipalArn(ident.Name), Disabled: ident.Disabled, PolicyNames: ident.PolicyNames, + // File identities are static; set it here so the published identity is + // never briefly observable as non-static (RemoveIdentity guards on it). + IsStatic: fromStaticFile, } switch { @@ -936,8 +945,8 @@ func (iam *IdentityAccessManagement) MergeS3ApiConfiguration(config *iam_pb.S3Ap continue } - // Skip if parent is a static identity (we don't modify static identities) - if staticNames[sa.ParentUser] { + // Same static-parent rule as identities above. + if !fromStaticFile && staticNames[sa.ParentUser] { glog.V(3).Infof("Skipping service account %s for static parent %s", sa.Id, sa.ParentUser) continue } @@ -1106,7 +1115,7 @@ func (iam *IdentityAccessManagement) UpsertIdentity(ident *iam_pb.Identity) erro glog.V(1).Infof("IAM: upsert identity %s", ident.Name) return iam.MergeS3ApiConfiguration(&iam_pb.S3ApiConfiguration{ Identities: []*iam_pb.Identity{ident}, - }) + }, false) } // isEnabled reports whether S3 auth should be enforced for this server. diff --git a/weed/s3api/auth_credentials_static_config_test.go b/weed/s3api/auth_credentials_static_config_test.go index b08203778..7f4e8d71a 100644 --- a/weed/s3api/auth_credentials_static_config_test.go +++ b/weed/s3api/auth_credentials_static_config_test.go @@ -109,6 +109,64 @@ func TestReloadStaticConfigMarksNewIdentitiesWithoutFreezingDynamic(t *testing.T } } +// A config-file reload must apply an edited secretKey to its static identity. +func TestReloadStaticConfigUpdatesExistingSecretKey(t *testing.T) { + s3a := newTestS3ApiServerWithMemoryIAM(t, []*iam_pb.Identity{}) + + p1 := writeTempIamConfig(t, `{"identities":[{"name":"static-admin","credentials":[{"accessKey":"AKADMIN0","secretKey":"b2xkc2VjcmV0"}],"actions":["Admin"]}]}`) + if err := s3a.iam.loadS3ApiConfigurationFromFile(p1); err != nil { + t.Fatalf("failed to load initial config: %v", err) + } + + _, cred, found := s3a.iam.lookupByAccessKey("AKADMIN0") + if !found || cred.SecretKey != "b2xkc2VjcmV0" { + t.Fatalf("expected initial secretKey to load, got found=%v cred=%+v", found, cred) + } + + // Rotate the secretKey in the file and reload. + p2 := writeTempIamConfig(t, `{"identities":[{"name":"static-admin","credentials":[{"accessKey":"AKADMIN0","secretKey":"bmV3c2VjcmV0"}],"actions":["Admin"]}]}`) + if err := s3a.iam.loadS3ApiConfigurationFromFile(p2); err != nil { + t.Fatalf("failed to reload config: %v", err) + } + + _, cred, found = s3a.iam.lookupByAccessKey("AKADMIN0") + if !found { + t.Fatalf("static-admin access key disappeared after reload") + } + if cred.SecretKey != "bmV3c2VjcmV0" { + t.Fatalf("expected reloaded secretKey bmV3c2VjcmV0, got %q", cred.SecretKey) + } + if !isStaticName(s3a.iam, "static-admin") { + t.Fatalf("static-admin must stay marked static after reload") + } +} + +// A reload must also reapply a service-account credential under a static parent. +func TestReloadStaticConfigUpdatesServiceAccountSecret(t *testing.T) { + s3a := newTestS3ApiServerWithMemoryIAM(t, []*iam_pb.Identity{}) + + p1 := writeTempIamConfig(t, `{"identities":[{"name":"static-admin","credentials":[{"accessKey":"AKADMIN0","secretKey":"YWRtaW4="}],"actions":["Admin"]}],"serviceAccounts":[{"id":"sa-1","parentUser":"static-admin","credential":{"accessKey":"AKSA0001","secretKey":"b2xkc2E="}}]}`) + if err := s3a.iam.loadS3ApiConfigurationFromFile(p1); err != nil { + t.Fatalf("failed to load initial config: %v", err) + } + if _, cred, found := s3a.iam.lookupByAccessKey("AKSA0001"); !found || cred.SecretKey != "b2xkc2E=" { + t.Fatalf("expected service account secret to load, got found=%v cred=%+v", found, cred) + } + + // Rotate the service account secret in the file and reload. + p2 := writeTempIamConfig(t, `{"identities":[{"name":"static-admin","credentials":[{"accessKey":"AKADMIN0","secretKey":"YWRtaW4="}],"actions":["Admin"]}],"serviceAccounts":[{"id":"sa-1","parentUser":"static-admin","credential":{"accessKey":"AKSA0001","secretKey":"bmV3c2E="}}]}`) + if err := s3a.iam.loadS3ApiConfigurationFromFile(p2); err != nil { + t.Fatalf("failed to reload config: %v", err) + } + _, cred, found := s3a.iam.lookupByAccessKey("AKSA0001") + if !found { + t.Fatalf("service account access key disappeared after reload") + } + if cred.SecretKey != "bmV3c2E=" { + t.Fatalf("expected reloaded service account secret bmV3c2E=, got %q", cred.SecretKey) + } +} + func isStaticName(iam *IdentityAccessManagement, name string) bool { iam.m.RLock() defer iam.m.RUnlock()