From 513b6db967229a1d32447b09695262ea8d1a9e9a Mon Sep 17 00:00:00 2001 From: chrislu Date: Tue, 26 Aug 2025 19:40:42 -0700 Subject: [PATCH] address comments --- test/s3/iam/docker-compose.yml | 2 +- test/s3/iam/s3_iam_framework.go | 38 ++++++++++-------- test/s3/iam/s3_iam_integration_test.go | 53 +++++++++++++++++--------- 3 files changed, 57 insertions(+), 36 deletions(-) diff --git a/test/s3/iam/docker-compose.yml b/test/s3/iam/docker-compose.yml index c7df89a97..e106a26fb 100644 --- a/test/s3/iam/docker-compose.yml +++ b/test/s3/iam/docker-compose.yml @@ -159,4 +159,4 @@ volumes: networks: seaweedfs-iam: - driver: bridge \ No newline at end of file + driver: bridge diff --git a/test/s3/iam/s3_iam_framework.go b/test/s3/iam/s3_iam_framework.go index d23aef9e5..51e52d23b 100644 --- a/test/s3/iam/s3_iam_framework.go +++ b/test/s3/iam/s3_iam_framework.go @@ -302,27 +302,33 @@ func (t *BearerTokenTransport) RoundTrip(req *http.Request) (*http.Response, err // Add Bearer token authorization header newReq.Header.Set("Authorization", "Bearer "+t.Token) - // Debug: log the token being sent (first 50 chars) and subject + // Token preview for logging (first 50 chars for security) tokenPreview := t.Token if len(tokenPreview) > 50 { tokenPreview = tokenPreview[:50] + "..." } - // Debug logging can be enabled if needed - // Extract subject from token for debugging - // parts := strings.Split(t.Token, ".") - // subject := "unknown" - // if len(parts) >= 2 { - // if decoded, err := base64.RawURLEncoding.DecodeString(parts[1]); err == nil { - // var claims map[string]interface{} - // if json.Unmarshal(decoded, &claims) == nil { - // if sub, ok := claims["sub"].(string); ok { - // subject = sub[:8] + "..." // First 8 chars of subject - // } - // } - // } - // } - // fmt.Printf("DEBUG: Sending Bearer token (subject: %s): %s\n", subject, tokenPreview) + // Conditional debug logging - enable via TEST_DEBUG=1 environment variable + if os.Getenv("TEST_DEBUG") == "1" { + // Extract subject from JWT token for enhanced debugging + parts := strings.Split(t.Token, ".") + subject := "unknown" + if len(parts) >= 2 { + if decoded, err := base64.RawURLEncoding.DecodeString(parts[1]); err == nil { + var claims map[string]interface{} + if json.Unmarshal(decoded, &claims) == nil { + if sub, ok := claims["sub"].(string); ok { + if len(sub) > 8 { + subject = sub[:8] + "..." // First 8 chars of subject for security + } else { + subject = sub + } + } + } + } + } + fmt.Printf("DEBUG: Sending Bearer token (subject: %s): %s\n", subject, tokenPreview) + } // Use underlying transport transport := t.Transport diff --git a/test/s3/iam/s3_iam_integration_test.go b/test/s3/iam/s3_iam_integration_test.go index ccd7d1a0b..5c89bda6f 100644 --- a/test/s3/iam/s3_iam_integration_test.go +++ b/test/s3/iam/s3_iam_integration_test.go @@ -467,9 +467,7 @@ func TestS3IAMBucketPolicyIntegration(t *testing.T) { }) require.NoError(t, err) - // Note: Bucket policy enforcement is not fully implemented yet - // For now, just verify that the bucket policy was stored successfully - // by retrieving it + // Verify that the bucket policy was stored successfully by retrieving it policyResult, err := adminClient.GetBucketPolicy(&s3.GetBucketPolicyInput{ Bucket: aws.String(testBucket), }) @@ -477,8 +475,10 @@ func TestS3IAMBucketPolicyIntegration(t *testing.T) { assert.Contains(t, *policyResult.Policy, "s3:DeleteObject") assert.Contains(t, *policyResult.Policy, "Deny") - // TODO: Implement bucket policy enforcement in authorization flow - // Once implemented, this should test that delete operations are denied + // IMPLEMENTATION NOTE: Bucket policy enforcement in authorization flow + // is planned for a future phase. Currently, this test validates policy + // storage and retrieval. When enforcement is implemented, this test + // should be extended to verify that delete operations are actually denied. }) // Cleanup - delete bucket policy first, then objects and bucket @@ -509,16 +509,21 @@ func TestS3IAMContextualPolicyEnforcement(t *testing.T) { // For now, we'll focus on the basic structure t.Run("ip_based_policy_enforcement", func(t *testing.T) { - // TODO: Implement IP-based policy testing - // This would require configuring policies with IP restrictions - // and testing from different source IPs - t.Skip("IP-based policy testing requires network configuration") + // IMPLEMENTATION NOTE: IP-based policy testing framework planned for future release + // Requirements: + // - Configure IAM policies with IpAddress/NotIpAddress conditions + // - Multi-container test setup with controlled source IP addresses + // - Test policy enforcement from allowed vs denied IP ranges + t.Skip("IP-based policy testing requires advanced network configuration and multi-container setup") }) t.Run("time_based_policy_enforcement", func(t *testing.T) { - // TODO: Implement time-based policy testing - // This would require configuring policies with time restrictions - t.Skip("Time-based policy testing requires time manipulation") + // IMPLEMENTATION NOTE: Time-based policy testing framework planned for future release + // Requirements: + // - Configure IAM policies with DateGreaterThan/DateLessThan conditions + // - Time manipulation capabilities for testing different time windows + // - Test policy enforcement during allowed vs restricted time periods + t.Skip("Time-based policy testing requires time manipulation capabilities") }) } @@ -553,18 +558,28 @@ func TestS3IAMPresignedURLIntegration(t *testing.T) { require.NoError(t, err) t.Run("presigned_url_generation_and_usage", func(t *testing.T) { - // Note: AWS SDK's presigned URL generation is not compatible with JWT Bearer token authentication - // The AWS SDK generates signature-based presigned URLs, but SeaweedFS with JWT uses Bearer tokens - // For JWT authentication, direct API calls with Bearer tokens should be used instead - - // Test direct object access with JWT token (which is what JWT authentication supports) + // ARCHITECTURAL NOTE: AWS SDK presigned URLs are incompatible with JWT Bearer authentication + // + // AWS SDK presigned URLs use AWS Signature Version 4 (SigV4) which requires: + // - Access Key ID and Secret Access Key for signing + // - Query parameter-based authentication in the URL + // + // SeaweedFS JWT authentication uses: + // - Bearer tokens in the Authorization header + // - Stateless JWT validation without AWS-style signing + // + // RECOMMENDATION: For JWT-authenticated applications, use direct API calls + // with Bearer tokens rather than presigned URLs. + + // Test direct object access with JWT Bearer token (recommended approach) _, err := adminClient.GetObject(&s3.GetObjectInput{ Bucket: aws.String(testBucketPrefix), Key: aws.String(testObjectKey), }) - require.NoError(t, err, "Direct object access with JWT should work") + require.NoError(t, err, "Direct object access with JWT Bearer token works correctly") - t.Log("JWT-based object access successful - presigned URLs not applicable for JWT Bearer token authentication") + t.Log("✅ JWT Bearer token authentication confirmed working for direct S3 API calls") + t.Log("ℹ️ Note: Presigned URLs are not supported with JWT Bearer authentication by design") }) // Cleanup