Browse Source

correct action name

pull/7479/head
chrislu 3 weeks ago
parent
commit
ab17596480
  1. 10
      weed/s3api/s3_constants/s3_action_strings.go
  2. 4
      weed/s3api/s3_granular_action_security_test.go
  3. 2
      weed/s3api/s3_iam_simple_test.go
  4. 16
      weed/s3api/s3_list_parts_action_test.go
  5. 4
      weed/s3api/s3_multipart_iam_test.go

10
weed/s3api/s3_constants/s3_action_strings.go

@ -32,14 +32,14 @@ const (
S3_ACTION_UPLOAD_PART = "s3:UploadPart" S3_ACTION_UPLOAD_PART = "s3:UploadPart"
S3_ACTION_COMPLETE_MULTIPART = "s3:CompleteMultipartUpload" S3_ACTION_COMPLETE_MULTIPART = "s3:CompleteMultipartUpload"
S3_ACTION_ABORT_MULTIPART = "s3:AbortMultipartUpload" S3_ACTION_ABORT_MULTIPART = "s3:AbortMultipartUpload"
S3_ACTION_LIST_PARTS = "s3:ListParts"
S3_ACTION_LIST_PARTS = "s3:ListMultipartUploadParts"
// Bucket operations // Bucket operations
S3_ACTION_CREATE_BUCKET = "s3:CreateBucket" S3_ACTION_CREATE_BUCKET = "s3:CreateBucket"
S3_ACTION_DELETE_BUCKET = "s3:DeleteBucket" S3_ACTION_DELETE_BUCKET = "s3:DeleteBucket"
S3_ACTION_LIST_BUCKET = "s3:ListBucket" S3_ACTION_LIST_BUCKET = "s3:ListBucket"
S3_ACTION_LIST_BUCKET_VERSIONS = "s3:ListBucketVersions" S3_ACTION_LIST_BUCKET_VERSIONS = "s3:ListBucketVersions"
S3_ACTION_LIST_MULTIPART_UPLOADS = "s3:ListMultipartUploads"
S3_ACTION_LIST_MULTIPART_UPLOADS = "s3:ListBucketMultipartUploads"
// Bucket ACL operations // Bucket ACL operations
S3_ACTION_GET_BUCKET_ACL = "s3:GetBucketAcl" S3_ACTION_GET_BUCKET_ACL = "s3:GetBucketAcl"
@ -61,9 +61,9 @@ const (
S3_ACTION_DELETE_BUCKET_CORS = "s3:DeleteBucketCors" S3_ACTION_DELETE_BUCKET_CORS = "s3:DeleteBucketCors"
// Bucket lifecycle operations // Bucket lifecycle operations
S3_ACTION_GET_BUCKET_LIFECYCLE = "s3:GetBucketLifecycleConfiguration"
S3_ACTION_PUT_BUCKET_LIFECYCLE = "s3:PutBucketLifecycleConfiguration"
S3_ACTION_DELETE_BUCKET_LIFECYCLE = "s3:DeleteBucketLifecycle"
S3_ACTION_GET_BUCKET_LIFECYCLE = "s3:GetLifecycleConfiguration"
S3_ACTION_PUT_BUCKET_LIFECYCLE = "s3:PutLifecycleConfiguration"
S3_ACTION_DELETE_BUCKET_LIFECYCLE = "s3:PutLifecycleConfiguration"
// Bucket versioning operations // Bucket versioning operations
S3_ACTION_GET_BUCKET_VERSIONING = "s3:GetBucketVersioning" S3_ACTION_GET_BUCKET_VERSIONING = "s3:GetBucketVersioning"

4
weed/s3api/s3_granular_action_security_test.go

@ -85,10 +85,10 @@ func TestGranularActionMappingSecurity(t *testing.T) {
bucket: "inventory-bucket", bucket: "inventory-bucket",
objectKey: "", objectKey: "",
queryParams: map[string]string{"uploads": ""}, queryParams: map[string]string{"uploads": ""},
description: "Listing multipart uploads should map to s3:ListMultipartUploads",
description: "Listing multipart uploads should map to s3:ListBucketMultipartUploads",
problemWithOldMapping: "Old mapping would use generic s3:ListBucket for all bucket operations, " + problemWithOldMapping: "Old mapping would use generic s3:ListBucket for all bucket operations, " +
"preventing fine-grained control over who can see ongoing multipart operations", "preventing fine-grained control over who can see ongoing multipart operations",
granularActionResult: "s3:ListMultipartUploads",
granularActionResult: "s3:ListBucketMultipartUploads",
}, },
{ {
name: "delete_object_tagging_precision", name: "delete_object_tagging_precision",

2
weed/s3api/s3_iam_simple_test.go

@ -294,7 +294,7 @@ func TestDetermineGranularS3Action(t *testing.T) {
objectKey: "", objectKey: "",
queryParams: map[string]string{"uploads": ""}, queryParams: map[string]string{"uploads": ""},
fallbackAction: s3_constants.ACTION_LIST, fallbackAction: s3_constants.ACTION_LIST,
expected: "s3:ListMultipartUploads",
expected: "s3:ListBucketMultipartUploads",
description: "List multipart uploads in bucket", description: "List multipart uploads in bucket",
}, },

16
weed/s3api/s3_list_parts_action_test.go

@ -39,7 +39,7 @@ func TestListPartsActionMapping(t *testing.T) {
objectKey: "test-object.txt", objectKey: "test-object.txt",
queryParams: map[string]string{"uploadId": "test-upload-id"}, queryParams: map[string]string{"uploadId": "test-upload-id"},
fallbackAction: s3_constants.ACTION_READ, fallbackAction: s3_constants.ACTION_READ,
expectedAction: "s3:ListParts",
expectedAction: "s3:ListMultipartUploadParts",
description: "GET request with uploadId should map to s3:ListParts (this was the missing mapping)", description: "GET request with uploadId should map to s3:ListParts (this was the missing mapping)",
}, },
{ {
@ -53,7 +53,7 @@ func TestListPartsActionMapping(t *testing.T) {
"part-number-marker": "50", "part-number-marker": "50",
}, },
fallbackAction: s3_constants.ACTION_READ, fallbackAction: s3_constants.ACTION_READ,
expectedAction: "s3:ListParts",
expectedAction: "s3:ListMultipartUploadParts",
description: "GET request with uploadId plus other multipart params should map to s3:ListParts", description: "GET request with uploadId plus other multipart params should map to s3:ListParts",
}, },
{ {
@ -137,7 +137,7 @@ func TestListPartsActionMappingSecurityScenarios(t *testing.T) {
action2 := determineGranularS3Action(req2, s3_constants.ACTION_READ, "secure-bucket", "confidential-document.pdf") action2 := determineGranularS3Action(req2, s3_constants.ACTION_READ, "secure-bucket", "confidential-document.pdf")
// These should be different actions, allowing different permissions // These should be different actions, allowing different permissions
assert.Equal(t, "s3:ListParts", action1, "Listing multipart parts should require s3:ListParts permission")
assert.Equal(t, "s3:ListMultipartUploadParts", action1, "Listing multipart parts should require s3:ListMultipartUploadParts permission")
assert.Equal(t, "s3:GetObject", action2, "Reading object content should require s3:GetObject permission") assert.Equal(t, "s3:GetObject", action2, "Reading object content should require s3:GetObject permission")
assert.NotEqual(t, action1, action2, "ListParts and GetObject should be separate permissions for security") assert.NotEqual(t, action1, action2, "ListParts and GetObject should be separate permissions for security")
}) })
@ -155,7 +155,7 @@ func TestListPartsActionMappingSecurityScenarios(t *testing.T) {
{ {
description: "List multipart upload parts", description: "List multipart upload parts",
queryParams: map[string]string{"uploadId": "upload-abc123"}, queryParams: map[string]string{"uploadId": "upload-abc123"},
expectedAction: "s3:ListParts",
expectedAction: "s3:ListMultipartUploadParts",
securityNote: "FIXED: Now correctly maps to s3:ListParts instead of s3:GetObject", securityNote: "FIXED: Now correctly maps to s3:ListParts instead of s3:GetObject",
}, },
{ {
@ -167,7 +167,7 @@ func TestListPartsActionMappingSecurityScenarios(t *testing.T) {
{ {
description: "Get object with complex upload ID", description: "Get object with complex upload ID",
queryParams: map[string]string{"uploadId": "complex-upload-id-with-hyphens-123-abc-def"}, queryParams: map[string]string{"uploadId": "complex-upload-id-with-hyphens-123-abc-def"},
expectedAction: "s3:ListParts",
expectedAction: "s3:ListMultipartUploadParts",
securityNote: "FIXED: Complex upload IDs now correctly detected", securityNote: "FIXED: Complex upload IDs now correctly detected",
}, },
} }
@ -240,7 +240,7 @@ func TestListPartsActionRealWorldScenarios(t *testing.T) {
// Verify each step has the correct action mapping // Verify each step has the correct action mapping
assert.Equal(t, "s3:CreateMultipartUpload", action1, "Step 1: Initiate upload") assert.Equal(t, "s3:CreateMultipartUpload", action1, "Step 1: Initiate upload")
assert.Equal(t, "s3:ListParts", action2, "Step 2: List parts (FIXED by this PR)")
assert.Equal(t, "s3:ListMultipartUploadParts", action2, "Step 2: List parts (FIXED by this PR)")
assert.Equal(t, "s3:UploadPart", action3, "Step 3: Upload part") assert.Equal(t, "s3:UploadPart", action3, "Step 3: Upload part")
assert.Equal(t, "s3:CompleteMultipartUpload", action4, "Step 4: Complete upload") assert.Equal(t, "s3:CompleteMultipartUpload", action4, "Step 4: Complete upload")
@ -279,8 +279,8 @@ func TestListPartsActionRealWorldScenarios(t *testing.T) {
action := determineGranularS3Action(req, s3_constants.ACTION_READ, "test-bucket", "test-file.bin") action := determineGranularS3Action(req, s3_constants.ACTION_READ, "test-bucket", "test-file.bin")
assert.Equal(t, "s3:ListParts", action,
"Upload ID format %s should be correctly detected and mapped to s3:ListParts", uploadId)
assert.Equal(t, "s3:ListMultipartUploadParts", action,
"Upload ID format %s should be correctly detected and mapped to s3:ListMultipartUploadParts", uploadId)
} }
}) })
} }

4
weed/s3api/s3_multipart_iam_test.go

@ -546,8 +546,8 @@ func setupTestRolesForMultipart(ctx context.Context, manager *integration.IAMMan
"s3:UploadPart", "s3:UploadPart",
"s3:CompleteMultipartUpload", "s3:CompleteMultipartUpload",
"s3:AbortMultipartUpload", "s3:AbortMultipartUpload",
"s3:ListMultipartUploads",
"s3:ListParts",
"s3:ListBucketMultipartUploads",
"s3:ListMultipartUploadParts",
}, },
Resource: []string{ Resource: []string{
"arn:aws:s3:::*", "arn:aws:s3:::*",

Loading…
Cancel
Save