From 7aaa431bb480f701a02d43384db7e56f3cd54929 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Mon, 13 Apr 2026 12:13:38 -0700 Subject: [PATCH] s3api: prune bucket-scoped IAM actions on DeleteBucket (#9054) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * s3api: prune bucket-scoped IAM actions on DeleteBucket DeleteBucket removed the bucket directory and collection but left behind any identity actions configured via s3.configure that were scoped to that bucket (e.g. Read:bucket, Write:bucket/prefix), leaving stale auth metadata that users expected to be cleaned up along with the bucket. After a successful delete, strip actions whose resource is exactly the bucket or a prefix under it, save via the credential manager, and let the existing filer metadata subscription fan the reload out to every S3 server. Wildcarded resources and global actions are preserved since they may cover other buckets; static identities are left untouched. Fixes #5310 * s3api: address review feedback on bucket IAM prune - Apply per-identity updates via credentialManager.UpdateUser instead of a full LoadConfiguration/SaveConfiguration round-trip, so the prune no longer clobbers concurrent IAM edits made by s3.configure or the IAM API during a DeleteBucket. - Use a 30s bounded background context for the post-delete cleanup so it survives client disconnect — the bucket is already gone by then and this is best-effort bookkeeping. - Skip static identities via IsStaticIdentity, since the credential store never persists them and UpdateUser would return NotFound. --- weed/s3api/auth_credentials.go | 77 +++++++++++++++++++++++++++++ weed/s3api/s3api_bucket_handlers.go | 11 +++++ 2 files changed, 88 insertions(+) diff --git a/weed/s3api/auth_credentials.go b/weed/s3api/auth_credentials.go index cc0670c02..3a924c44d 100644 --- a/weed/s3api/auth_credentials.go +++ b/weed/s3api/auth_credentials.go @@ -1803,6 +1803,83 @@ func (iam *IdentityAccessManagement) syncRuntimePoliciesToIAMManager(ctx context return manager.SyncRuntimePolicies(ctx, policies) } +// PruneBucketFromConfiguration removes any identity actions scoped to the given +// bucket (e.g. "Read:bucket", "Write:bucket/prefix") from the persisted S3 IAM +// configuration. Wildcarded resources and global actions are preserved because +// they may cover other buckets. Static (read-only) identities are not touched. +// +// Updates are applied per-identity via the credential store's UpdateUser path, +// which rewrites only the affected user record. This avoids a full-config +// read-modify-write cycle that could clobber unrelated concurrent IAM edits. +// Returns true if any identity was updated. +func (iam *IdentityAccessManagement) PruneBucketFromConfiguration(ctx context.Context, bucket string) (bool, error) { + if iam == nil || iam.credentialManager == nil || bucket == "" { + return false, nil + } + usernames, err := iam.credentialManager.ListUsers(ctx) + if err != nil { + return false, err + } + changed := false + var firstErr error + for _, username := range usernames { + if iam.IsStaticIdentity(username) { + continue + } + ident, err := iam.credentialManager.GetUser(ctx, username) + if err != nil { + if errors.Is(err, credential.ErrUserNotFound) { + continue + } + if firstErr == nil { + firstErr = err + } + continue + } + if ident == nil || ident.IsStatic { + continue + } + kept := ident.Actions[:0] + pruned := false + for _, a := range ident.Actions { + if actionScopedToBucket(a, bucket) { + pruned = true + continue + } + kept = append(kept, a) + } + if !pruned { + continue + } + ident.Actions = kept + if err := iam.credentialManager.UpdateUser(ctx, username, ident); err != nil { + if firstErr == nil { + firstErr = err + } + continue + } + changed = true + } + // In-memory identities refresh via the filer metadata subscription + // (onIamConfigChange), which fans the update out to every S3 server. + return changed, firstErr +} + +// actionScopedToBucket reports whether a configured action string like +// "Read:bucket" or "Write:bucket/prefix" is scoped exclusively to the given +// bucket. Wildcard resources are never considered scoped to a single bucket. +func actionScopedToBucket(action, bucket string) bool { + idx := strings.Index(action, ":") + if idx < 0 { + return false + } + resource := action[idx+1:] + if strings.ContainsAny(resource, "*?") { + return false + } + return resource == bucket || strings.HasPrefix(resource, bucket+"/") +} + // LoadS3ApiConfigurationFromCredentialManager loads configuration using the credential manager func (iam *IdentityAccessManagement) LoadS3ApiConfigurationFromCredentialManager() error { glog.V(1).Infof("Loading S3 API configuration from credential manager") diff --git a/weed/s3api/s3api_bucket_handlers.go b/weed/s3api/s3api_bucket_handlers.go index eec6a11ed..ef24b40bf 100644 --- a/weed/s3api/s3api_bucket_handlers.go +++ b/weed/s3api/s3api_bucket_handlers.go @@ -405,6 +405,17 @@ func (s3a *S3ApiServer) DeleteBucketHandler(w http.ResponseWriter, r *http.Reque s3a.invalidateBucketConfigCache(bucket) stats_collect.DeleteBucketMetrics(bucket) + // Prune identity actions that were scoped to this bucket via s3.configure. + // Use a bounded background context so the cleanup survives client disconnect; + // the bucket is already gone and this is best-effort bookkeeping. + if s3a.iam != nil { + pruneCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + if _, err := s3a.iam.PruneBucketFromConfiguration(pruneCtx, bucket); err != nil { + glog.Errorf("DeleteBucketHandler: failed to prune IAM actions for bucket %s: %v", bucket, err) + } + cancel() + } + s3err.WriteEmptyResponse(w, r, http.StatusNoContent) }