From 25b0f86bdacc5f035c61e80ae64e518117a6e8d4 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Wed, 28 Jan 2026 18:03:47 -0800 Subject: [PATCH] s3tables: Fix ownership consistency across handlers Address three related ownership consistency issues: 1. CreateNamespace now sets OwnerAccountID to bucketMetadata.OwnerAccountID instead of request principal. This prevents namespaces created by delegated callers (via bucket policy) from becoming unmanageable, since ListNamespaces filters by bucket owner. 2. CreateTable now: - Fetches bucket metadata to use correct owner for bucket policy evaluation - Uses namespaceMetadata.OwnerAccountID for namespace policy checks - Uses bucketMetadata.OwnerAccountID for bucket policy checks - Sets table OwnerAccountID to namespaceMetadata.OwnerAccountID (inherited) 3. GetTable now: - Fetches bucket metadata to use correct owner for bucket policy evaluation - Uses metadata.OwnerAccountID for table policy checks - Uses bucketMetadata.OwnerAccountID for bucket policy checks This ensures: - Bucket owner retains implicit "owner always allowed" behavior even when evaluating bucket policies - Ownership hierarchy is consistent (namespace owned by bucket, table owned by namespace) - Cross-principal delegation via policies doesn't break ownership chains --- weed/s3api/s3tables/handler_namespace.go | 5 ++-- weed/s3api/s3tables/handler_table.go | 32 +++++++++++++++++++++--- 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/weed/s3api/s3tables/handler_namespace.go b/weed/s3api/s3tables/handler_namespace.go index de7dca2f7..02c984273 100644 --- a/weed/s3api/s3tables/handler_namespace.go +++ b/weed/s3api/s3tables/handler_namespace.go @@ -98,12 +98,13 @@ func (h *S3TablesHandler) handleCreateNamespace(w http.ResponseWriter, r *http.R return err } - // Create the namespace + // Create the namespace with bucket owner to maintain consistency + // (authorization above ensures the caller has permission to create in this bucket) now := time.Now() metadata := &namespaceMetadata{ Namespace: req.Namespace, CreatedAt: now, - OwnerAccountID: h.getAccountID(r), + OwnerAccountID: bucketMetadata.OwnerAccountID, } metadataBytes, err := json.Marshal(metadata) diff --git a/weed/s3api/s3tables/handler_table.go b/weed/s3api/s3tables/handler_table.go index 9160714af..98f7ecbc7 100644 --- a/weed/s3api/s3tables/handler_table.go +++ b/weed/s3api/s3tables/handler_table.go @@ -90,8 +90,19 @@ func (h *S3TablesHandler) handleCreateTable(w http.ResponseWriter, r *http.Reque bucketPath := getTableBucketPath(bucketName) namespacePolicy := "" bucketPolicy := "" + var bucketMetadata tableBucketMetadata err = filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error { + // Fetch bucket metadata to use correct owner for bucket policy evaluation + data, err := h.getExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyMetadata) + if err == nil { + if err := json.Unmarshal(data, &bucketMetadata); err != nil { + return fmt.Errorf("failed to unmarshal bucket metadata: %w", err) + } + } else if !errors.Is(err, ErrAttributeNotFound) { + return fmt.Errorf("failed to fetch bucket metadata: %v", err) + } + // Fetch namespace policy if it exists policyData, err := h.getExtendedAttribute(r.Context(), client, namespacePath, ExtendedKeyPolicy) if err == nil { @@ -117,8 +128,10 @@ func (h *S3TablesHandler) handleCreateTable(w http.ResponseWriter, r *http.Reque } // Check authorization: namespace policy OR bucket policy OR ownership + // Use namespace owner for namespace policy (consistent with namespace authorization) nsAllowed := CanCreateTable(accountID, namespaceMetadata.OwnerAccountID, namespacePolicy) - bucketAllowed := CanCreateTable(accountID, namespaceMetadata.OwnerAccountID, bucketPolicy) + // Use bucket owner for bucket policy (bucket policy applies to bucket-level operations) + bucketAllowed := CanCreateTable(accountID, bucketMetadata.OwnerAccountID, bucketPolicy) if !nsAllowed && !bucketAllowed { h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to create table in this namespace") @@ -151,7 +164,7 @@ func (h *S3TablesHandler) handleCreateTable(w http.ResponseWriter, r *http.Reque Format: req.Format, CreatedAt: now, ModifiedAt: now, - OwnerAccountID: h.getAccountID(r), + OwnerAccountID: namespaceMetadata.OwnerAccountID, // Inherit namespace owner for consistency VersionToken: versionToken, Metadata: req.Metadata, } @@ -277,8 +290,19 @@ func (h *S3TablesHandler) handleGetTable(w http.ResponseWriter, r *http.Request, bucketPath := getTableBucketPath(bucketName) tablePolicy := "" bucketPolicy := "" + var bucketMetadata tableBucketMetadata err = filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error { + // Fetch bucket metadata to use correct owner for bucket policy evaluation + data, err := h.getExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyMetadata) + if err == nil { + if err := json.Unmarshal(data, &bucketMetadata); err != nil { + return fmt.Errorf("failed to unmarshal bucket metadata: %w", err) + } + } else if !errors.Is(err, ErrAttributeNotFound) { + return fmt.Errorf("failed to fetch bucket metadata: %v", err) + } + // Fetch table policy if it exists policyData, err := h.getExtendedAttribute(r.Context(), client, tablePath, ExtendedKeyPolicy) if err == nil { @@ -304,8 +328,10 @@ func (h *S3TablesHandler) handleGetTable(w http.ResponseWriter, r *http.Request, } // Check authorization: table policy OR bucket policy OR ownership + // Use table owner for table policy (table-level access control) tableAllowed := CanGetTable(accountID, metadata.OwnerAccountID, tablePolicy) - bucketAllowed := CanGetTable(accountID, metadata.OwnerAccountID, bucketPolicy) + // Use bucket owner for bucket policy (bucket-level access control) + bucketAllowed := CanGetTable(accountID, bucketMetadata.OwnerAccountID, bucketPolicy) if !tableAllowed && !bucketAllowed { h.writeError(w, http.StatusNotFound, ErrCodeNoSuchTable, fmt.Sprintf("table %s not found", tableName))