From 98e49d2d420ffe0dcc1a2ae234dfd5a3afaeb3d2 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Thu, 2 Jul 2026 21:27:58 -0700 Subject: [PATCH] s3: read bucket owner and crtime from the entry-free bucket config The owner index read the raw filer entry off BucketConfig, which no longer carries one. Cache Crtime alongside IdentityId, and pass the owner id rather than the entry through the ListBuckets visibility checks so the scan and granted-bucket paths share one implementation. --- weed/s3api/s3api_bucket_config.go | 5 ++++ weed/s3api/s3api_bucket_handlers.go | 38 ++++++++------------------ weed/s3api/s3api_bucket_owner_index.go | 4 +-- 3 files changed, 19 insertions(+), 28 deletions(-) diff --git a/weed/s3api/s3api_bucket_config.go b/weed/s3api/s3api_bucket_config.go index 8c561989f..3e8fe3706 100644 --- a/weed/s3api/s3api_bucket_config.go +++ b/weed/s3api/s3api_bucket_config.go @@ -38,6 +38,7 @@ type BucketConfig struct { ACL []byte Owner string IdentityId string // identity that created the bucket + Crtime int64 // bucket creation time, unix seconds IsPublicRead bool // Cached flag to avoid JSON parsing on every request CORS *cors.CORSConfiguration ObjectLockConfig *ObjectLockConfiguration // Cached parsed Object Lock configuration @@ -404,6 +405,10 @@ func (s3a *S3ApiServer) newBucketConfigFromEntry(bucket string, entry *filer_pb. return config } + if entry.Attributes != nil { + config.Crtime = entry.Attributes.Crtime + } + if entry.Extended != nil { if versioning, exists := entry.Extended[s3_constants.ExtVersioningKey]; exists { config.Versioning = string(versioning) diff --git a/weed/s3api/s3api_bucket_handlers.go b/weed/s3api/s3api_bucket_handlers.go index 08dae3d74..1181ff727 100644 --- a/weed/s3api/s3api_bucket_handlers.go +++ b/weed/s3api/s3api_bucket_handlers.go @@ -144,11 +144,11 @@ func decodeContinuationToken(token string) (string, error) { // bucketVisibleToIdentity reports whether ListBuckets should include the bucket: // the identity owns it or has explicit permission to list it. -func (s3a *S3ApiServer) bucketVisibleToIdentity(r *http.Request, entry *filer_pb.Entry, identity *Identity) bool { - if isBucketOwnedByIdentity(entry, identity) { +func (s3a *S3ApiServer) bucketVisibleToIdentity(r *http.Request, bucket, owner string, identity *Identity) bool { + if isBucketOwnedByIdentity(owner, identity) { return true } - return s3a.iam.VerifyActionPermission(r, identity, s3_constants.ACTION_LIST, entry.Name, "") == s3err.ErrNone + return s3a.iam.VerifyActionPermission(r, identity, s3_constants.ACTION_LIST, bucket, "") == s3err.ErrNone } // scanVisibleBuckets pages through /buckets and collects up to maxBuckets @@ -169,7 +169,7 @@ func (s3a *S3ApiServer) scanVisibleBuckets(r *http.Request, identity *Identity, if !entry.IsDirectory || strings.HasPrefix(entry.Name, ".") { continue } - if !s3a.bucketVisibleToIdentity(r, entry, identity) { + if !s3a.bucketVisibleToIdentity(r, entry.Name, bucketEntryOwner(entry), identity) { continue } buckets = append(buckets, ListAllMyBucketsEntry{ @@ -191,18 +191,14 @@ func (s3a *S3ApiServer) scanVisibleBuckets(r *http.Request, identity *Identity, return buckets, nextToken, nil } -// isBucketOwnedByIdentity checks if a bucket entry is owned by the given identity. -// Returns true if the identity owns the bucket, false otherwise. +// isBucketOwnedByIdentity checks if a bucket with the given owner id (its +// AmzIdentityId metadata) is owned by the given identity. // // Ownership rules: // - Admin users: considered owners of all buckets -// - Non-admin users: own buckets where AmzIdentityId matches identity.Name +// - Non-admin users: own buckets whose owner id matches identity.Name // - Buckets without owner metadata are not owned by anyone (except admins) -func isBucketOwnedByIdentity(entry *filer_pb.Entry, identity *Identity) bool { - if !entry.IsDirectory { - return false - } - +func isBucketOwnedByIdentity(owner string, identity *Identity) bool { if identity == nil { return false } @@ -214,17 +210,7 @@ func isBucketOwnedByIdentity(entry *filer_pb.Entry, identity *Identity) bool { // Non-admin users with no name cannot own buckets. // This prevents misconfigured identities from matching buckets with empty owner IDs. - if identity.Name == "" { - return false - } - - // Check ownership via AmzIdentityId metadata - id, ok := entry.Extended[s3_constants.AmzIdentityId] - if !ok || string(id) != identity.Name { - return false - } - - return true + return identity.Name != "" && identity.Name == owner } func (s3a *S3ApiServer) PutBucketHandler(w http.ResponseWriter, r *http.Request) { @@ -416,10 +402,10 @@ func (s3a *S3ApiServer) healBucketOwnerIndex(bucket, identityId string) { return } config, errCode := s3a.getBucketConfig(bucket) - if errCode != s3err.ErrNone || bucketEntryOwner(config.Entry) != identityId { + if errCode != s3err.ErrNone || config.IdentityId != identityId { return } - if err := s3a.addBucketToOwnerIndex(identityId, bucket, config.Entry.Attributes.Crtime); err != nil { + if err := s3a.addBucketToOwnerIndex(identityId, bucket, config.Crtime); err != nil { glog.V(1).Infof("owner index heal %s/%s: %v", identityId, bucket, err) } } @@ -483,7 +469,7 @@ func (s3a *S3ApiServer) DeleteBucketHandler(w http.ResponseWriter, r *http.Reque return } - if owner := bucketEntryOwner(bucketConfig.Entry); owner != "" { + if owner := bucketConfig.IdentityId; owner != "" { if err := s3a.removeBucketFromOwnerIndex(owner, bucket); err != nil { glog.Warningf("DeleteBucketHandler: owner index remove %s/%s: %v", owner, bucket, err) } diff --git a/weed/s3api/s3api_bucket_owner_index.go b/weed/s3api/s3api_bucket_owner_index.go index db8dbc0a8..977aa39f4 100644 --- a/weed/s3api/s3api_bucket_owner_index.go +++ b/weed/s3api/s3api_bucket_owner_index.go @@ -166,12 +166,12 @@ func (s3a *S3ApiServer) resolveGrantedBuckets(r *http.Request, identity *Identit } continue } - if !s3a.bucketVisibleToIdentity(r, config.Entry, identity) { + if !s3a.bucketVisibleToIdentity(r, name, config.IdentityId, identity) { continue } granted = append(granted, ListAllMyBucketsEntry{ Name: name, - CreationDate: time.Unix(config.Entry.Attributes.Crtime, 0).UTC(), + CreationDate: time.Unix(config.Crtime, 0).UTC(), }) } return granted