diff --git a/weed/s3api/s3api_object_handlers_put.go b/weed/s3api/s3api_object_handlers_put.go index 9e325801d..8c0561a89 100644 --- a/weed/s3api/s3api_object_handlers_put.go +++ b/weed/s3api/s3api_object_handlers_put.go @@ -141,7 +141,7 @@ func (s3a *S3ApiServer) PutObjectHandler(w http.ResponseWriter, r *http.Request) entry.Attributes.Mime = objectContentType // Set object owner for directory objects (same as regular objects) - s3a.setObjectOwnerFromRequest(r, entry) + s3a.setObjectOwnerFromRequest(r, bucket, entry) }); err != nil { s3err.WriteErrorResponse(w, r, s3err.ErrInternalError) return @@ -748,15 +748,44 @@ func filerErrorToS3Error(err error) s3err.ErrorCode { } } -// setObjectOwnerFromRequest sets the object owner metadata based on the authenticated user -func (s3a *S3ApiServer) setObjectOwnerFromRequest(r *http.Request, entry *filer_pb.Entry) { - amzAccountId := r.Header.Get(s3_constants.AmzAccountId) - if amzAccountId != "" { +// setObjectOwnerFromRequest sets the object owner metadata based on the bucket ownership policy. +// When BucketOwnerEnforced (the modern AWS default), the bucket owner owns all objects. +// Otherwise, the uploader's account ID is used (ObjectWriter mode). +func (s3a *S3ApiServer) setObjectOwnerFromRequest(r *http.Request, bucket string, entry *filer_pb.Entry) { + var ownerId string + + // Check if bucketRegistry is available + if s3a.bucketRegistry == nil { + // Fallback to uploader if registry unavailable + ownerId = r.Header.Get(s3_constants.AmzAccountId) + glog.V(2).Infof("setObjectOwnerFromRequest: bucketRegistry unavailable, fallback to uploader %s", ownerId) + } else { + // Check bucket ownership policy + bucketMetadata, errCode := s3a.bucketRegistry.GetBucketMetadata(bucket) + useBucketOwner := errCode == s3err.ErrNone && bucketMetadata != nil && + bucketMetadata.ObjectOwnership == s3_constants.OwnershipBucketOwnerEnforced && + bucketMetadata.Owner != nil && bucketMetadata.Owner.ID != nil + + if useBucketOwner { + ownerId = *bucketMetadata.Owner.ID + glog.V(2).Infof("setObjectOwnerFromRequest: using bucket owner %s (BucketOwnerEnforced)", ownerId) + } else { + ownerId = r.Header.Get(s3_constants.AmzAccountId) + if errCode != s3err.ErrNone || bucketMetadata == nil { + glog.V(2).Infof("setObjectOwnerFromRequest: fallback to uploader %s", ownerId) + } else if bucketMetadata.ObjectOwnership == s3_constants.OwnershipBucketOwnerEnforced { + glog.V(2).Infof("setObjectOwnerFromRequest: BucketOwnerEnforced but no owner found, fallback to uploader %s", ownerId) + } else { + glog.V(2).Infof("setObjectOwnerFromRequest: using uploader %s (ObjectWriter mode)", ownerId) + } + } + } + + if ownerId != "" { if entry.Extended == nil { entry.Extended = make(map[string][]byte) } - entry.Extended[s3_constants.ExtAmzOwnerKey] = []byte(amzAccountId) - glog.V(2).Infof("setObjectOwnerFromRequest: set object owner to %s", amzAccountId) + entry.Extended[s3_constants.ExtAmzOwnerKey] = []byte(ownerId) } } @@ -1035,7 +1064,7 @@ func (s3a *S3ApiServer) putVersionedObject(r *http.Request, bucket, object strin versionEntry.Extended[s3_constants.ExtETagKey] = []byte(etag) // Set object owner for versioned objects - s3a.setObjectOwnerFromRequest(r, versionEntry) + s3a.setObjectOwnerFromRequest(r, bucket, versionEntry) // Extract and store object lock metadata from request headers if err := s3a.extractObjectLockMetadataFromRequest(r, versionEntry); err != nil { diff --git a/weed/s3api/s3api_object_ownership_test.go b/weed/s3api/s3api_object_ownership_test.go new file mode 100644 index 000000000..849f442c7 --- /dev/null +++ b/weed/s3api/s3api_object_ownership_test.go @@ -0,0 +1,196 @@ +package s3api + +import ( + "net/http" + "testing" + + "github.com/aws/aws-sdk-go/service/s3" + "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" + "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" + "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" +) + +func TestSetObjectOwnerFromRequest(t *testing.T) { + tests := []struct { + name string + bucketRegistryNil bool + bucketMetadata *BucketMetaData + bucketMetadataError s3err.ErrorCode + uploaderAccountId string + expectedOwnerId string + description string + }{ + { + name: "BucketOwnerEnforced - use bucket owner", + bucketRegistryNil: false, + bucketMetadata: &BucketMetaData{ + Name: "test-bucket", + ObjectOwnership: s3_constants.OwnershipBucketOwnerEnforced, + Owner: &s3.Owner{ + ID: stringPtr("bucket-owner-123"), + DisplayName: stringPtr("Bucket Owner"), + }, + }, + bucketMetadataError: s3err.ErrNone, + uploaderAccountId: "uploader-456", + expectedOwnerId: "bucket-owner-123", + description: "Should use bucket owner when BucketOwnerEnforced", + }, + { + name: "ObjectWriter - use uploader", + bucketRegistryNil: false, + bucketMetadata: &BucketMetaData{ + Name: "test-bucket", + ObjectOwnership: s3_constants.OwnershipObjectWriter, + Owner: &s3.Owner{ + ID: stringPtr("bucket-owner-123"), + DisplayName: stringPtr("Bucket Owner"), + }, + }, + bucketMetadataError: s3err.ErrNone, + uploaderAccountId: "uploader-456", + expectedOwnerId: "uploader-456", + description: "Should use uploader when ObjectWriter mode", + }, + { + name: "BucketOwnerPreferred - use uploader", + bucketRegistryNil: false, + bucketMetadata: &BucketMetaData{ + Name: "test-bucket", + ObjectOwnership: s3_constants.OwnershipBucketOwnerPreferred, + Owner: &s3.Owner{ + ID: stringPtr("bucket-owner-123"), + DisplayName: stringPtr("Bucket Owner"), + }, + }, + bucketMetadataError: s3err.ErrNone, + uploaderAccountId: "uploader-456", + expectedOwnerId: "uploader-456", + description: "Should use uploader when BucketOwnerPreferred mode", + }, + { + name: "BucketOwnerEnforced but owner is nil - fallback to uploader", + bucketRegistryNil: false, + bucketMetadata: &BucketMetaData{ + Name: "test-bucket", + ObjectOwnership: s3_constants.OwnershipBucketOwnerEnforced, + Owner: nil, + }, + bucketMetadataError: s3err.ErrNone, + uploaderAccountId: "uploader-456", + expectedOwnerId: "uploader-456", + description: "Should fallback to uploader when bucket owner is nil", + }, + { + name: "BucketOwnerEnforced but owner ID is nil - fallback to uploader", + bucketRegistryNil: false, + bucketMetadata: &BucketMetaData{ + Name: "test-bucket", + ObjectOwnership: s3_constants.OwnershipBucketOwnerEnforced, + Owner: &s3.Owner{ + ID: nil, + DisplayName: stringPtr("Bucket Owner"), + }, + }, + bucketMetadataError: s3err.ErrNone, + uploaderAccountId: "uploader-456", + expectedOwnerId: "uploader-456", + description: "Should fallback to uploader when bucket owner ID is nil", + }, + { + name: "Bucket metadata error - fallback to uploader", + bucketRegistryNil: false, + bucketMetadata: nil, + bucketMetadataError: s3err.ErrNoSuchBucket, + uploaderAccountId: "uploader-456", + expectedOwnerId: "uploader-456", + description: "Should fallback to uploader when bucket metadata unavailable", + }, + { + name: "Bucket registry is nil - fallback to uploader", + bucketRegistryNil: true, + uploaderAccountId: "uploader-456", + expectedOwnerId: "uploader-456", + description: "Should fallback to uploader when bucketRegistry is nil", + }, + { + name: "Empty uploader account ID - no owner set", + bucketRegistryNil: false, + bucketMetadata: &BucketMetaData{ + Name: "test-bucket", + ObjectOwnership: s3_constants.OwnershipObjectWriter, + Owner: &s3.Owner{ + ID: stringPtr("bucket-owner-123"), + DisplayName: stringPtr("Bucket Owner"), + }, + }, + bucketMetadataError: s3err.ErrNone, + uploaderAccountId: "", + expectedOwnerId: "", + description: "Should not set owner when uploader account ID is empty", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create mock S3ApiServer + s3a := &S3ApiServer{} + + // Setup bucket registry with mock behavior + if !tt.bucketRegistryNil { + // Create a minimal BucketRegistry with overridden GetBucketMetadata + s3a.bucketRegistry = &BucketRegistry{ + metadataCache: map[string]*BucketMetaData{}, + notFound: map[string]struct{}{}, + } + + // Pre-populate the cache with test metadata + if tt.bucketMetadata != nil && tt.bucketMetadataError == s3err.ErrNone { + s3a.bucketRegistry.metadataCache["test-bucket"] = tt.bucketMetadata + } else if tt.bucketMetadataError == s3err.ErrNoSuchBucket { + s3a.bucketRegistry.notFound["test-bucket"] = struct{}{} + } + } + + // Create mock request with uploader account ID + req, _ := http.NewRequest("PUT", "/test-bucket/test-object", nil) + req.Header.Set(s3_constants.AmzAccountId, tt.uploaderAccountId) + + // Create entry + entry := &filer_pb.Entry{ + Name: "test-object", + } + + // Call the function + s3a.setObjectOwnerFromRequest(req, "test-bucket", entry) + + // Verify the owner ID + if tt.expectedOwnerId == "" { + if entry.Extended != nil { + if _, exists := entry.Extended[s3_constants.ExtAmzOwnerKey]; exists { + t.Errorf("%s: Expected no owner to be set, but owner was set", tt.description) + } + } + } else { + if entry.Extended == nil { + t.Errorf("%s: Expected owner to be set, but Extended is nil", tt.description) + return + } + ownerBytes, exists := entry.Extended[s3_constants.ExtAmzOwnerKey] + if !exists { + t.Errorf("%s: Expected owner to be set, but ExtAmzOwnerKey not found", tt.description) + return + } + actualOwnerId := string(ownerBytes) + if actualOwnerId != tt.expectedOwnerId { + t.Errorf("%s: Expected owner ID %s, got %s", tt.description, tt.expectedOwnerId, actualOwnerId) + } + } + }) + } +} + +// Helper function to create string pointers +func stringPtr(s string) *string { + return &s +}