s3api: prune bucket-scoped IAM actions on DeleteBucket (#9054)

* 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.
This commit is contained in:
Chris Lu
2026-04-13 12:13:38 -07:00
committed by GitHub
parent 8049fcc516
commit 7aaa431bb4
2 changed files with 88 additions and 0 deletions

View File

@@ -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")

View File

@@ -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)
}