Allow multipart upload operations when s3:PutObject is authorized (#8445)

* Allow multipart upload operations when s3:PutObject is authorized Multipart upload is an implementation detail of putting objects, not a separate permission. When a policy grants s3:PutObject, it should implicitly allow: - s3:CreateMultipartUpload - s3:UploadPart - s3:CompleteMultipartUpload - s3:AbortMultipartUpload - s3:ListParts This fixes a compatibility issue where clients like PyArrow that use multipart uploads by default would fail even though the role had s3:PutObject permission. The session policy intersection still applies - both the identity-based policy AND session policy must allow s3:PutObject for multipart operations to work. Implementation: - Added constants for S3 multipart action strings - Added multipartActionSet to efficiently check if action is multipart-related - Updated MatchesAction method to implicitly grant multipart when PutObject allowed * Update weed/s3api/policy_engine/types.go Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * Add s3:ListMultipartUploads to multipart action set Include s3:ListMultipartUploads in the multipartActionSet so that listing multipart uploads is implicitly granted when s3:PutObject is authorized. ListMultipartUploads is a critical part of the multipart upload workflow, allowing clients to query in-progress uploads before completing them. Changes: - Added s3ListMultipartUploads constant definition - Included s3ListMultipartUploads in multipartActionSet initialization - Existing references to multipartActionSet automatically now cover ListMultipartUploads All policy engine tests pass (0.351s execution time) * Refactor: reuse multipart action constants from s3_constants package Remove duplicate constant definitions from policy_engine/types.go and import the canonical definitions from s3api/s3_constants/s3_actions.go instead. This eliminates duplication and ensures a single source of truth for multipart action strings: - ACTION_CREATE_MULTIPART_UPLOAD - ACTION_UPLOAD_PART - ACTION_COMPLETE_MULTIPART - ACTION_ABORT_MULTIPART - ACTION_LIST_PARTS - ACTION_LIST_MULTIPART_UPLOADS All policy engine tests pass (0.350s execution time) * Fix S3_ACTION_LIST_MULTIPART_UPLOADS constant value Move S3_ACTION_LIST_MULTIPART_UPLOADS from bucket operations to multipart operations section and change value from 's3:ListBucketMultipartUploads' to 's3:ListMultipartUploads' to match the action strings used in policy_engine and s3_actions.go. This ensures consistent action naming across all S3 constant definitions. * refactor names * Fix S3 action constant mismatches and MatchesAction early return bug Fix two critical issues in policy engine: 1. S3Actions map had incorrect multipart action mappings: - 'ListMultipartUploads' was 's3:ListMultipartUploads' (should be 's3:ListBucketMultipartUploads') - 'ListParts' was 's3:ListParts' (should be 's3:ListMultipartUploadParts') These mismatches caused authorization checks to fail for list operations 2. CompiledStatement.MatchesAction() had early return bug: - Previously returned true immediately upon first direct action match - This prevented scanning remaining matchers for s3:PutObject permission - Now scans ALL matchers before returning, tracking both direct match and PutObject grant - Ensures multipart operations inherit s3:PutObject authorization even when explicitly requested action doesn't match (e.g., s3:ListMultipartUploadParts) Changes: - Track matchedAction flag to defer Fix two critical issues in policy engine: 1. S3Actions map had incorrect multipart action mappings: - 'ListMultipartUploads' was 's3:ListMultipartUplPer 1. S3Actions map had incorrect multiparAll - 'ListMultipartUploads(0.334s execution time) * Refactor S3Actions map to use s3_constants Replace hardcoded action strings in the S3Actions map with references to canonical S3_ACTION_* constants from s3_constants/s3_action_strings.go. Benefits: - Single source of truth for S3 action values - Eliminates string duplication across codebase - Ensures consistency between policy engine and middleware - Reduces maintenance burden when action strings need updates All policy engine tests pass (0.334s execution time) * Remove unused S3Actions map The S3Actions map in types.go was never referenced anywhere in the codebase. All action mappings are handled by GetActionMappings() in integration.go instead. This removes 42 lines of dead code. * Fix test: reload configuration function must also reload IAM state TestEmbeddedIamAttachUserPolicyRefreshesIAM was failing because the test's reloadConfigurationFunc only updated mockConfig but didn't reload the actual IAM state. When AttachUserPolicy calls refreshIAMConfiguration(), it would use the test's incomplete reload function instead of the real LoadS3ApiConfigurationFromCredentialManager(). Fixed by making the test's reloadConfigurationFunc also call e.iam.LoadS3ApiConfigurationFromCredentialManager() so lookupByIdentityName() sees the updated policy attachments. --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
2 days ago · b9fa05153a
8 changed files with 72 additions and 86 deletions
--- a/weed/s3api/policy_engine/types.go
+++ b/weed/s3api/policy_engine/types.go
@ -9,6 +9,7 @@ import (
 	"time"

 	"github.com/seaweedfs/seaweedfs/weed/glog"
+	s3const "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
 )

 // Policy Engine Types
@ -35,6 +36,17 @@ const (
 var (
 	// PolicyVariableRegex detects AWS IAM policy variables like ${aws:username}
 	PolicyVariableRegex = regexp.MustCompile(`\$\{([^}]+)\}`)
+
+	// multipartActionSet contains all S3 multipart upload actions
+	// These are treated as equivalent to s3:PutObject for authorization purposes
+	multipartActionSet = map[string]bool{
+		s3const.S3_ACTION_CREATE_MULTIPART:       true,
+		s3const.S3_ACTION_UPLOAD_PART:            true,
+		s3const.S3_ACTION_COMPLETE_MULTIPART:     true,
+		s3const.S3_ACTION_ABORT_MULTIPART:        true,
+		s3const.S3_ACTION_LIST_PARTS:             true,
+		s3const.S3_ACTION_LIST_MULTIPART_UPLOADS: true,
+	}
 )

 // StringOrStringSlice represents a value that can be either a string or []string
@ -455,55 +467,33 @@ func IsObjectResource(resource string) bool {
 	return strings.Contains(resource, "/")
 }

-// S3Actions contains common S3 actions
-var S3Actions = map[string]string{
-	"GetObject":                        "s3:GetObject",
-	"PutObject":                        "s3:PutObject",
-	"DeleteObject":                     "s3:DeleteObject",
-	"GetObjectVersion":                 "s3:GetObjectVersion",
-	"DeleteObjectVersion":              "s3:DeleteObjectVersion",
-	"ListBucket":                       "s3:ListBucket",
-	"ListBucketVersions":               "s3:ListBucketVersions",
-	"GetBucketLocation":                "s3:GetBucketLocation",
-	"GetBucketVersioning":              "s3:GetBucketVersioning",
-	"PutBucketVersioning":              "s3:PutBucketVersioning",
-	"GetBucketAcl":                     "s3:GetBucketAcl",
-	"PutBucketAcl":                     "s3:PutBucketAcl",
-	"GetObjectAcl":                     "s3:GetObjectAcl",
-	"PutObjectAcl":                     "s3:PutObjectAcl",
-	"GetBucketPolicy":                  "s3:GetBucketPolicy",
-	"PutBucketPolicy":                  "s3:PutBucketPolicy",
-	"DeleteBucketPolicy":               "s3:DeleteBucketPolicy",
-	"GetBucketCors":                    "s3:GetBucketCors",
-	"PutBucketCors":                    "s3:PutBucketCors",
-	"DeleteBucketCors":                 "s3:DeleteBucketCors",
-	"GetBucketNotification":            "s3:GetBucketNotification",
-	"PutBucketNotification":            "s3:PutBucketNotification",
-	"GetBucketTagging":                 "s3:GetBucketTagging",
-	"PutBucketTagging":                 "s3:PutBucketTagging",
-	"DeleteBucketTagging":              "s3:DeleteBucketTagging",
-	"GetObjectTagging":                 "s3:GetObjectTagging",
-	"PutObjectTagging":                 "s3:PutObjectTagging",
-	"DeleteObjectTagging":              "s3:DeleteObjectTagging",
-	"ListMultipartUploads":             "s3:ListMultipartUploads",
-	"AbortMultipartUpload":             "s3:AbortMultipartUpload",
-	"ListParts":                        "s3:ListParts",
-	"GetObjectRetention":               "s3:GetObjectRetention",
-	"PutObjectRetention":               "s3:PutObjectRetention",
-	"GetObjectLegalHold":               "s3:GetObjectLegalHold",
-	"PutObjectLegalHold":               "s3:PutObjectLegalHold",
-	"GetBucketObjectLockConfiguration": "s3:GetBucketObjectLockConfiguration",
-	"PutBucketObjectLockConfiguration": "s3:PutBucketObjectLockConfiguration",
-	"BypassGovernanceRetention":        "s3:BypassGovernanceRetention",
-}
-
-// MatchesAction checks if an action matches any of the compiled action matchers
+// MatchesAction checks if an action matches any of the compiled action matchers.
+// It also implicitly grants multipart upload actions if s3:PutObject is allowed,
+// since multipart upload is an implementation detail of putting objects.
 func (cs *CompiledStatement) MatchesAction(action string) bool {
+	var matchedAction, hasPutObjectPermission bool
+
+	// Scan all matchers to check for both direct action match and s3:PutObject grant
 	for _, matcher := range cs.ActionMatchers {
 		if matcher.Match(action) {
-			return true
+			matchedAction = true
+		}
+		if !hasPutObjectPermission && matcher.Match("s3:PutObject") {
+			hasPutObjectPermission = true
 		}
 	}
+
+	// Return true if action matched directly or if multipart action with PutObject permission
+	if matchedAction {
+		return true
+	}
+
+	// Multipart upload operations are part of s3:PutObject permission
+	// If s3:PutObject is allowed, implicitly allow multipart operations
+	if hasPutObjectPermission && multipartActionSet[action] {
+		return true
+	}
+
 	return false
 }

--- a/weed/s3api/s3_constants/s3_action_strings.go
+++ b/weed/s3api/s3_constants/s3_action_strings.go
@ -27,18 +27,18 @@ const (
 	S3_ACTION_BYPASS_GOVERNANCE     = "s3:BypassGovernanceRetention"

 	// Multipart upload operations
-	S3_ACTION_CREATE_MULTIPART   = "s3:CreateMultipartUpload"
-	S3_ACTION_UPLOAD_PART        = "s3:UploadPart"
-	S3_ACTION_COMPLETE_MULTIPART = "s3:CompleteMultipartUpload"
-	S3_ACTION_ABORT_MULTIPART    = "s3:AbortMultipartUpload"
-	S3_ACTION_LIST_PARTS         = "s3:ListMultipartUploadParts"
+	S3_ACTION_CREATE_MULTIPART       = "s3:CreateMultipartUpload"
+	S3_ACTION_UPLOAD_PART            = "s3:UploadPart"
+	S3_ACTION_COMPLETE_MULTIPART     = "s3:CompleteMultipartUpload"
+	S3_ACTION_ABORT_MULTIPART        = "s3:AbortMultipartUpload"
+	S3_ACTION_LIST_PARTS             = "s3:ListMultipartUploadParts"
+	S3_ACTION_LIST_MULTIPART_UPLOADS = "s3:ListBucketMultipartUploads"

 	// Bucket operations
-	S3_ACTION_CREATE_BUCKET          = "s3:CreateBucket"
-	S3_ACTION_DELETE_BUCKET          = "s3:DeleteBucket"
-	S3_ACTION_LIST_BUCKET            = "s3:ListBucket"
-	S3_ACTION_LIST_BUCKET_VERSIONS   = "s3:ListBucketVersions"
-	S3_ACTION_LIST_MULTIPART_UPLOADS = "s3:ListBucketMultipartUploads"
+	S3_ACTION_CREATE_BUCKET        = "s3:CreateBucket"
+	S3_ACTION_DELETE_BUCKET        = "s3:DeleteBucket"
+	S3_ACTION_LIST_BUCKET          = "s3:ListBucket"
+	S3_ACTION_LIST_BUCKET_VERSIONS = "s3:ListBucketVersions"

 	// Bucket ACL operations
 	S3_ACTION_GET_BUCKET_ACL = "s3:GetBucketAcl"
--- a/weed/s3api/s3_constants/s3_actions.go
+++ b/weed/s3api/s3_constants/s3_actions.go
@ -17,14 +17,6 @@ const (
 	ACTION_GET_BUCKET_OBJECT_LOCK_CONFIG = "GetBucketObjectLockConfiguration"
 	ACTION_PUT_BUCKET_OBJECT_LOCK_CONFIG = "PutBucketObjectLockConfiguration"

-	// Granular multipart upload actions for fine-grained IAM policies
-	ACTION_CREATE_MULTIPART_UPLOAD = "s3:CreateMultipartUpload"
-	ACTION_UPLOAD_PART             = "s3:UploadPart"
-	ACTION_COMPLETE_MULTIPART      = "s3:CompleteMultipartUpload"
-	ACTION_ABORT_MULTIPART         = "s3:AbortMultipartUpload"
-	ACTION_LIST_MULTIPART_UPLOADS  = "s3:ListMultipartUploads"
-	ACTION_LIST_PARTS              = "s3:ListParts"
-
 	SeaweedStorageDestinationHeader = "x-seaweedfs-destination"
 	MultipartUploadsFolder          = ".uploads"
 	VersionsFolder                  = ".versions"
--- a/weed/s3api/s3_iam_middleware.go
+++ b/weed/s3api/s3_iam_middleware.go
@ -479,17 +479,17 @@ func mapLegacyActionToIAM(legacyAction Action) string {
 		return "s3:*" // Fallback for unmapped admin operations

 	// Handle granular multipart actions (already correctly mapped)
-	case s3_constants.ACTION_CREATE_MULTIPART_UPLOAD:
-		return "s3:CreateMultipartUpload"
-	case s3_constants.ACTION_UPLOAD_PART:
-		return "s3:UploadPart"
-	case s3_constants.ACTION_COMPLETE_MULTIPART:
-		return "s3:CompleteMultipartUpload"
-	case s3_constants.ACTION_ABORT_MULTIPART:
-		return "s3:AbortMultipartUpload"
-	case s3_constants.ACTION_LIST_MULTIPART_UPLOADS:
+	case s3_constants.S3_ACTION_CREATE_MULTIPART:
+		return s3_constants.S3_ACTION_CREATE_MULTIPART
+	case s3_constants.S3_ACTION_UPLOAD_PART:
+		return s3_constants.S3_ACTION_UPLOAD_PART
+	case s3_constants.S3_ACTION_COMPLETE_MULTIPART:
+		return s3_constants.S3_ACTION_COMPLETE_MULTIPART
+	case s3_constants.S3_ACTION_ABORT_MULTIPART:
+		return s3_constants.S3_ACTION_ABORT_MULTIPART
+	case s3_constants.S3_ACTION_LIST_MULTIPART_UPLOADS:
 		return s3_constants.S3_ACTION_LIST_MULTIPART_UPLOADS
-	case s3_constants.ACTION_LIST_PARTS:
+	case s3_constants.S3_ACTION_LIST_PARTS:
 		return s3_constants.S3_ACTION_LIST_PARTS

 	default:
--- a/weed/s3api/s3_iam_simple_test.go
+++ b/weed/s3api/s3_iam_simple_test.go
@ -370,8 +370,8 @@ func TestMapLegacyActionToIAM(t *testing.T) {
 		},
 		{
 			name:         "granular_multipart_action",
-			legacyAction: s3_constants.ACTION_CREATE_MULTIPART_UPLOAD,
-			expected:     "s3:CreateMultipartUpload",
+			legacyAction: s3_constants.S3_ACTION_CREATE_MULTIPART,
+			expected:     s3_constants.S3_ACTION_CREATE_MULTIPART,
 		},
 		{
 			name:         "unknown_action_with_s3_prefix",
--- a/weed/s3api/s3_multipart_iam.go
+++ b/weed/s3api/s3_multipart_iam.go
@ -284,17 +284,17 @@ func (s3a *S3ApiServer) UploadPartWithIAM(w http.ResponseWriter, r *http.Request
 func determineMultipartS3Action(operation MultipartOperation) Action {
 	switch operation {
 	case MultipartOpInitiate:
-		return s3_constants.ACTION_CREATE_MULTIPART_UPLOAD
+		return s3_constants.S3_ACTION_CREATE_MULTIPART
 	case MultipartOpUploadPart:
-		return s3_constants.ACTION_UPLOAD_PART
+		return s3_constants.S3_ACTION_UPLOAD_PART
 	case MultipartOpComplete:
-		return s3_constants.ACTION_COMPLETE_MULTIPART
+		return s3_constants.S3_ACTION_COMPLETE_MULTIPART
 	case MultipartOpAbort:
-		return s3_constants.ACTION_ABORT_MULTIPART
+		return s3_constants.S3_ACTION_ABORT_MULTIPART
 	case MultipartOpList:
-		return s3_constants.ACTION_LIST_MULTIPART_UPLOADS
+		return s3_constants.S3_ACTION_LIST_MULTIPART_UPLOADS
 	case MultipartOpListParts:
-		return s3_constants.ACTION_LIST_PARTS
+		return s3_constants.S3_ACTION_LIST_PARTS
 	default:
 		// Fail closed for unmapped operations to prevent unintended access
 		glog.Errorf("unmapped multipart operation: %s", operation)
--- a/weed/s3api/s3_multipart_iam_test.go
+++ b/weed/s3api/s3_multipart_iam_test.go
@ -281,12 +281,12 @@ func TestMultipartS3ActionMapping(t *testing.T) {
 		operation      MultipartOperation
 		expectedAction Action
 	}{
-		{MultipartOpInitiate, s3_constants.ACTION_CREATE_MULTIPART_UPLOAD},
-		{MultipartOpUploadPart, s3_constants.ACTION_UPLOAD_PART},
-		{MultipartOpComplete, s3_constants.ACTION_COMPLETE_MULTIPART},
-		{MultipartOpAbort, s3_constants.ACTION_ABORT_MULTIPART},
-		{MultipartOpList, s3_constants.ACTION_LIST_MULTIPART_UPLOADS},
-		{MultipartOpListParts, s3_constants.ACTION_LIST_PARTS},
+		{MultipartOpInitiate, s3_constants.S3_ACTION_CREATE_MULTIPART},
+		{MultipartOpUploadPart, s3_constants.S3_ACTION_UPLOAD_PART},
+		{MultipartOpComplete, s3_constants.S3_ACTION_COMPLETE_MULTIPART},
+		{MultipartOpAbort, s3_constants.S3_ACTION_ABORT_MULTIPART},
+		{MultipartOpList, s3_constants.S3_ACTION_LIST_MULTIPART_UPLOADS},
+		{MultipartOpListParts, s3_constants.S3_ACTION_LIST_PARTS},
 		{MultipartOperation("unknown"), "s3:InternalErrorUnknownMultipartAction"}, // Fail-closed for security
 	}

--- a/weed/s3api/s3api_embedded_iam_test.go
+++ b/weed/s3api/s3api_embedded_iam_test.go
@ -83,6 +83,10 @@ func NewEmbeddedIamApiForTest() *EmbeddedIamApiForTest {
 			return err
 		}
 		e.mockConfig = config
+		// Also refresh the IAM state so lookup functions see the updated configuration
+		if err := e.iam.LoadS3ApiConfigurationFromCredentialManager(); err != nil {
+			return err
+		}
 		return nil
 	}
 	return e