diff --git a/BUCKET_POLICY_ENGINE_INTEGRATION.md b/BUCKET_POLICY_ENGINE_INTEGRATION.md index c2b197094..5b9eefe6e 100644 --- a/BUCKET_POLICY_ENGINE_INTEGRATION.md +++ b/BUCKET_POLICY_ENGINE_INTEGRATION.md @@ -120,7 +120,7 @@ Created a wrapper around `policy_engine.PolicyEngine` to: } ``` - Anonymous users can read all objects -- Authenticated users also evaluated (still need IAM permissions) +- Authenticated users are also evaluated against this policy. If they don't match an explicit `Allow` for this action, they will fall back to their own IAM permissions ### 2. **Grant Access to Specific User** (Authenticated) ```json diff --git a/test/s3/iam/s3_iam_integration_test.go b/test/s3/iam/s3_iam_integration_test.go index f189cccff..d0f2f8acf 100644 --- a/test/s3/iam/s3_iam_integration_test.go +++ b/test/s3/iam/s3_iam_integration_test.go @@ -480,10 +480,15 @@ func TestS3IAMBucketPolicyIntegration(t *testing.T) { assert.Contains(t, *policyResult.Policy, "s3:DeleteObject") assert.Contains(t, *policyResult.Policy, "Deny") - // NOTE: Bucket policy enforcement is now fully implemented in the authorization flow. - // This test validates policy storage and retrieval. The actual enforcement of the - // deny policy (preventing delete operations) can be tested by attempting a delete - // operation and expecting AccessDenied. + // Test that the deny policy is correctly enforced - attempt to delete the object + _, err = adminClient.DeleteObject(&s3.DeleteObjectInput{ + Bucket: aws.String(bucketName), + Key: aws.String(testObjectKey), + }) + require.Error(t, err, "DeleteObject should be denied by the bucket policy") + awsErr, ok := err.(awserr.Error) + require.True(t, ok, "Error should be an awserr.Error") + assert.Equal(t, "AccessDenied", awsErr.Code(), "Expected AccessDenied error code") // Clean up bucket policy after this test _, err = adminClient.DeleteBucketPolicy(&s3.DeleteBucketPolicyInput{ diff --git a/weed/s3api/auth_credentials.go b/weed/s3api/auth_credentials.go index 39eac5013..7a6a706ff 100644 --- a/weed/s3api/auth_credentials.go +++ b/weed/s3api/auth_credentials.go @@ -617,6 +617,13 @@ func buildPrincipalARN(identity *Identity) string { return "*" // Anonymous } + // Check if this is the anonymous user identity (authenticated as anonymous) + // S3 policies expect Principal: "*" for anonymous access + if identity.Name == s3_constants.AccountAnonymousId || + (identity.Account != nil && identity.Account.Id == s3_constants.AccountAnonymousId) { + return "*" // Anonymous user + } + // Build an AWS-compatible principal ARN // Format: arn:aws:iam::account-id:user/user-name accountId := identity.Account.Id diff --git a/weed/s3api/s3api_bucket_policy_arn_test.go b/weed/s3api/s3api_bucket_policy_arn_test.go index b9daa9884..ef8946918 100644 --- a/weed/s3api/s3api_bucket_policy_arn_test.go +++ b/weed/s3api/s3api_bucket_policy_arn_test.go @@ -2,6 +2,8 @@ package s3api import ( "testing" + + "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" ) // TestBuildResourceARN verifies that resource ARNs use the AWS-compatible format @@ -60,6 +62,26 @@ func TestBuildPrincipalARN(t *testing.T) { identity: nil, expected: "*", }, + { + name: "anonymous user by name", + identity: &Identity{ + Name: s3_constants.AccountAnonymousId, + Account: &Account{ + Id: "123456789012", + }, + }, + expected: "*", + }, + { + name: "anonymous user by account ID", + identity: &Identity{ + Name: "test-user", + Account: &Account{ + Id: s3_constants.AccountAnonymousId, + }, + }, + expected: "*", + }, { name: "identity with account and name", identity: &Identity{