From a27f6527abba52f20e6ead24255a451b8bcf80ad Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Wed, 28 Jan 2026 16:24:07 -0800 Subject: [PATCH] s3tables: Extract resource owner and bucket extraction into helper method Create extractResourceOwnerAndBucket() helper to consolidate the repeated pattern of unmarshaling metadata and extracting bucket name from resource path. This pattern was duplicated in handleTagResource, handleListTagsForResource, and handleUntagResource. Update all three handlers to use the helper. Also update remaining uses of getPrincipalFromRequest() (in handler_bucket_create, handler_bucket_get_list_delete, handler_namespace) to use getAccountID() after consolidating the two identical methods. --- weed/s3api/s3tables/handler_bucket_create.go | 2 +- .../handler_bucket_get_list_delete.go | 6 +- weed/s3api/s3tables/handler_namespace.go | 8 +- weed/s3api/s3tables/handler_policy.go | 121 +++++++----------- 4 files changed, 53 insertions(+), 84 deletions(-) diff --git a/weed/s3api/s3tables/handler_bucket_create.go b/weed/s3api/s3tables/handler_bucket_create.go index 9e0a02fba..b71622b3b 100644 --- a/weed/s3api/s3tables/handler_bucket_create.go +++ b/weed/s3api/s3tables/handler_bucket_create.go @@ -15,7 +15,7 @@ import ( func (h *S3TablesHandler) handleCreateTableBucket(w http.ResponseWriter, r *http.Request, filerClient FilerClient) error { // Check permission accountID := h.getAccountID(r) - principal := h.getPrincipalFromRequest(r) + principal := h.getAccountID(r) if !CanCreateTableBucket(principal, accountID, "") { h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to create table buckets") return NewAuthError("CreateTableBucket", principal, "not authorized to create table buckets") diff --git a/weed/s3api/s3tables/handler_bucket_get_list_delete.go b/weed/s3api/s3tables/handler_bucket_get_list_delete.go index 7c3e8eefe..09ff62fa9 100644 --- a/weed/s3api/s3tables/handler_bucket_get_list_delete.go +++ b/weed/s3api/s3tables/handler_bucket_get_list_delete.go @@ -64,7 +64,7 @@ func (h *S3TablesHandler) handleGetTableBucket(w http.ResponseWriter, r *http.Re } // Check permission - principal := h.getPrincipalFromRequest(r) + principal := h.getAccountID(r) if !CanGetTableBucket(principal, metadata.OwnerAccountID, bucketPolicy) { h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to get table bucket details") return ErrAccessDenied @@ -90,7 +90,7 @@ func (h *S3TablesHandler) handleListTableBuckets(w http.ResponseWriter, r *http. } // Check permission - principal := h.getPrincipalFromRequest(r) + principal := h.getAccountID(r) accountID := h.getAccountID(r) if !CanListTableBuckets(principal, accountID, "") { h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to list table buckets") @@ -260,7 +260,7 @@ func (h *S3TablesHandler) handleDeleteTableBucket(w http.ResponseWriter, r *http } // 2. Check permission - principal := h.getPrincipalFromRequest(r) + principal := h.getAccountID(r) if !CanDeleteTableBucket(principal, metadata.OwnerAccountID, bucketPolicy) { return NewAuthError("DeleteTableBucket", principal, fmt.Sprintf("not authorized to delete bucket %s", bucketName)) } diff --git a/weed/s3api/s3tables/handler_namespace.go b/weed/s3api/s3tables/handler_namespace.go index 66efaeb3e..5e0e33787 100644 --- a/weed/s3api/s3tables/handler_namespace.go +++ b/weed/s3api/s3tables/handler_namespace.go @@ -74,7 +74,7 @@ func (h *S3TablesHandler) handleCreateNamespace(w http.ResponseWriter, r *http.R } // Check permission - principal := h.getPrincipalFromRequest(r) + principal := h.getAccountID(r) if !CanCreateNamespace(principal, bucketMetadata.OwnerAccountID, bucketPolicy) { h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to create namespace in this bucket") return ErrAccessDenied @@ -197,7 +197,7 @@ func (h *S3TablesHandler) handleGetNamespace(w http.ResponseWriter, r *http.Requ } // Check permission - principal := h.getPrincipalFromRequest(r) + principal := h.getAccountID(r) if !CanGetNamespace(principal, metadata.OwnerAccountID, bucketPolicy) { h.writeError(w, http.StatusNotFound, ErrCodeNoSuchNamespace, "namespace not found") return ErrAccessDenied @@ -269,7 +269,7 @@ func (h *S3TablesHandler) handleListNamespaces(w http.ResponseWriter, r *http.Re return err } - principal := h.getPrincipalFromRequest(r) + principal := h.getAccountID(r) if !CanListNamespaces(principal, bucketMetadata.OwnerAccountID, bucketPolicy) { h.writeError(w, http.StatusNotFound, ErrCodeNoSuchBucket, fmt.Sprintf("table bucket %s not found", bucketName)) return ErrAccessDenied @@ -440,7 +440,7 @@ func (h *S3TablesHandler) handleDeleteNamespace(w http.ResponseWriter, r *http.R } // Check permission - principal := h.getPrincipalFromRequest(r) + principal := h.getAccountID(r) if !CanDeleteNamespace(principal, metadata.OwnerAccountID, bucketPolicy) { h.writeError(w, http.StatusNotFound, ErrCodeNoSuchNamespace, "namespace not found") return ErrAccessDenied diff --git a/weed/s3api/s3tables/handler_policy.go b/weed/s3api/s3tables/handler_policy.go index bdf172dec..5d822a68b 100644 --- a/weed/s3api/s3tables/handler_policy.go +++ b/weed/s3api/s3tables/handler_policy.go @@ -10,6 +10,42 @@ import ( "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" ) +// extractResourceOwnerAndBucket extracts ownership info and bucket name from resource metadata. +// This helper consolidates the repeated pattern used in handleTagResource, handleListTagsForResource, +// and handleUntagResource. +func (h *S3TablesHandler) extractResourceOwnerAndBucket( + data []byte, + resourcePath string, + rType ResourceType, +) (ownerAccountID, bucketName string, err error) { + if rType == ResourceTypeTable { + var meta tableMetadataInternal + if err := json.Unmarshal(data, &meta); err != nil { + return "", "", err + } + ownerAccountID = meta.OwnerAccountID + // Extract bucket name from resource path for tables + // resourcePath format: /tables/{bucket}/{namespace}/{table} + parts := strings.Split(strings.Trim(resourcePath, "/"), "/") + if len(parts) >= 2 { + bucketName = parts[1] + } + } else { + var meta tableBucketMetadata + if err := json.Unmarshal(data, &meta); err != nil { + return "", "", err + } + ownerAccountID = meta.OwnerAccountID + // Extract bucket name from resource path for buckets + // resourcePath format: /tables/{bucket} + parts := strings.Split(strings.Trim(resourcePath, "/"), "/") + if len(parts) >= 2 { + bucketName = parts[1] + } + } + return ownerAccountID, bucketName, nil +} + // handlePutTableBucketPolicy puts a policy on a table bucket func (h *S3TablesHandler) handlePutTableBucketPolicy(w http.ResponseWriter, r *http.Request, filerClient FilerClient) error { @@ -515,32 +551,9 @@ func (h *S3TablesHandler) handleTagResource(w http.ResponseWriter, r *http.Reque return err } - var ownerAccountID string - var bucketName string - if rType == ResourceTypeTable { - var meta tableMetadataInternal - if err := json.Unmarshal(data, &meta); err != nil { - return err - } - ownerAccountID = meta.OwnerAccountID - // Extract bucket name from resource path for tables - // resourcePath format: /tables/{bucket}/{namespace}/{table} - parts := strings.Split(strings.Trim(resourcePath, "/"), "/") - if len(parts) >= 2 { - bucketName = parts[1] - } - } else { - var meta tableBucketMetadata - if err := json.Unmarshal(data, &meta); err != nil { - return err - } - ownerAccountID = meta.OwnerAccountID - // Extract bucket name from resource path for buckets - // resourcePath format: /tables/{bucket} - parts := strings.Split(strings.Trim(resourcePath, "/"), "/") - if len(parts) >= 2 { - bucketName = parts[1] - } + ownerAccountID, bucketName, err := h.extractResourceOwnerAndBucket(data, resourcePath, rType) + if err != nil { + return err } // Fetch bucket policy if we have a bucket name @@ -637,32 +650,9 @@ func (h *S3TablesHandler) handleListTagsForResource(w http.ResponseWriter, r *ht return err } - var ownerAccountID string - var bucketName string - if rType == ResourceTypeTable { - var meta tableMetadataInternal - if err := json.Unmarshal(data, &meta); err != nil { - return err - } - ownerAccountID = meta.OwnerAccountID - // Extract bucket name from resource path for tables - // resourcePath format: /tables/{bucket}/{namespace}/{table} - parts := strings.Split(strings.Trim(resourcePath, "/"), "/") - if len(parts) >= 2 { - bucketName = parts[1] - } - } else { - var meta tableBucketMetadata - if err := json.Unmarshal(data, &meta); err != nil { - return err - } - ownerAccountID = meta.OwnerAccountID - // Extract bucket name from resource path for buckets - // resourcePath format: /tables/{bucket} - parts := strings.Split(strings.Trim(resourcePath, "/"), "/") - if len(parts) >= 2 { - bucketName = parts[1] - } + ownerAccountID, bucketName, err := h.extractResourceOwnerAndBucket(data, resourcePath, rType) + if err != nil { + return err } // Fetch bucket policy if we have a bucket name @@ -747,30 +737,9 @@ func (h *S3TablesHandler) handleUntagResource(w http.ResponseWriter, r *http.Req return err } - var ownerAccountID string - var bucketName string - if rType == ResourceTypeTable { - var meta tableMetadataInternal - if err := json.Unmarshal(data, &meta); err != nil { - return err - } - ownerAccountID = meta.OwnerAccountID - // Extract bucket name from resource path for tables - parts := strings.Split(strings.Trim(resourcePath, "/"), "/") - if len(parts) >= 2 { - bucketName = parts[1] - } - } else { - var meta tableBucketMetadata - if err := json.Unmarshal(data, &meta); err != nil { - return err - } - ownerAccountID = meta.OwnerAccountID - // Extract bucket name from resource path for buckets - parts := strings.Split(strings.Trim(resourcePath, "/"), "/") - if len(parts) >= 2 { - bucketName = parts[1] - } + ownerAccountID, bucketName, err := h.extractResourceOwnerAndBucket(data, resourcePath, rType) + if err != nil { + return err } // Fetch bucket policy if we have a bucket name