diff options
| -rw-r--r-- | weed/s3api/auth_credentials.go | 64 | ||||
| -rw-r--r-- | weed/s3api/auth_credentials_test.go | 183 | ||||
| -rw-r--r-- | weed/s3api/s3api_bucket_handlers.go | 4 |
3 files changed, 194 insertions, 57 deletions
diff --git a/weed/s3api/auth_credentials.go b/weed/s3api/auth_credentials.go index 5115e21af..266a6144a 100644 --- a/weed/s3api/auth_credentials.go +++ b/weed/s3api/auth_credentials.go @@ -455,68 +455,20 @@ func (iam *IdentityAccessManagement) authRequest(r *http.Request, action Action) object = prefix } - if !identity.canDo(action, bucket, object) { - return identity, s3err.ErrAccessDenied - } - - r.Header.Set(s3_constants.AmzAccountId, identity.Account.Id) - - return identity, s3err.ErrNone - -} - -func (iam *IdentityAccessManagement) authUser(r *http.Request) (*Identity, s3err.ErrorCode) { - var identity *Identity - var s3Err s3err.ErrorCode - var found bool - var authType string - switch getRequestAuthType(r) { - case authTypeStreamingSigned: - glog.V(3).Infof("signed streaming upload") - return identity, s3err.ErrNone - case authTypeStreamingUnsigned: - glog.V(3).Infof("unsigned streaming upload") - return identity, s3err.ErrNone - case authTypeUnknown: - glog.V(3).Infof("unknown auth type") - r.Header.Set(s3_constants.AmzAuthType, "Unknown") - return identity, s3err.ErrAccessDenied - case authTypePresignedV2, authTypeSignedV2: - glog.V(3).Infof("v2 auth type") - identity, s3Err = iam.isReqAuthenticatedV2(r) - authType = "SigV2" - case authTypeSigned, authTypePresigned: - glog.V(3).Infof("v4 auth type") - identity, s3Err = iam.reqSignatureV4Verify(r) - authType = "SigV4" - case authTypePostPolicy: - glog.V(3).Infof("post policy auth type") - r.Header.Set(s3_constants.AmzAuthType, "PostPolicy") - return identity, s3err.ErrNone - case authTypeJWT: - glog.V(3).Infof("jwt auth type") - r.Header.Set(s3_constants.AmzAuthType, "Jwt") - return identity, s3err.ErrNotImplemented - case authTypeAnonymous: - authType = "Anonymous" - identity, found = iam.lookupAnonymous() - if !found { - r.Header.Set(s3_constants.AmzAuthType, authType) + // For ListBuckets, authorization is performed in the handler by iterating + // through buckets and checking permissions for each. Skip the global check here. + if action == s3_constants.ACTION_LIST && bucket == "" { + // ListBuckets operation - authorization handled per-bucket in the handler + } else { + if !identity.canDo(action, bucket, object) { return identity, s3err.ErrAccessDenied } - default: - return identity, s3err.ErrNotImplemented } - if len(authType) > 0 { - r.Header.Set(s3_constants.AmzAuthType, authType) - } + r.Header.Set(s3_constants.AmzAccountId, identity.Account.Id) - glog.V(3).Infof("auth error: %v", s3Err) - if s3Err != s3err.ErrNone { - return identity, s3Err - } return identity, s3err.ErrNone + } func (identity *Identity) canDo(action Action, bucket string, objectKey string) bool { diff --git a/weed/s3api/auth_credentials_test.go b/weed/s3api/auth_credentials_test.go index b751eb8bc..ae89285a2 100644 --- a/weed/s3api/auth_credentials_test.go +++ b/weed/s3api/auth_credentials_test.go @@ -357,3 +357,186 @@ func TestNewIdentityAccessManagementWithStoreEnvVars(t *testing.T) { }) } } + +// TestBucketLevelListPermissions tests that bucket-level List permissions work correctly +// This test validates the fix for issue #7066 +func TestBucketLevelListPermissions(t *testing.T) { + // Test the functionality that was broken in issue #7066 + + t.Run("Bucket Wildcard Permissions", func(t *testing.T) { + // Create identity with bucket-level List permission using wildcards + identity := &Identity{ + Name: "bucket-user", + Actions: []Action{ + "List:mybucket*", + "Read:mybucket*", + "ReadAcp:mybucket*", + "Write:mybucket*", + "WriteAcp:mybucket*", + "Tagging:mybucket*", + }, + } + + // Test cases for bucket-level wildcard permissions + testCases := []struct { + name string + action Action + bucket string + object string + shouldAllow bool + description string + }{ + { + name: "exact bucket match", + action: "List", + bucket: "mybucket", + object: "", + shouldAllow: true, + description: "Should allow access to exact bucket name", + }, + { + name: "bucket with suffix", + action: "List", + bucket: "mybucket-prod", + object: "", + shouldAllow: true, + description: "Should allow access to bucket with matching prefix", + }, + { + name: "bucket with numbers", + action: "List", + bucket: "mybucket123", + object: "", + shouldAllow: true, + description: "Should allow access to bucket with numbers", + }, + { + name: "different bucket", + action: "List", + bucket: "otherbucket", + object: "", + shouldAllow: false, + description: "Should deny access to bucket with different prefix", + }, + { + name: "partial match", + action: "List", + bucket: "notmybucket", + object: "", + shouldAllow: false, + description: "Should deny access to bucket that contains but doesn't start with the prefix", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + result := identity.canDo(tc.action, tc.bucket, tc.object) + assert.Equal(t, tc.shouldAllow, result, tc.description) + }) + } + }) + + t.Run("Global List Permission", func(t *testing.T) { + // Create identity with global List permission + identity := &Identity{ + Name: "global-user", + Actions: []Action{ + "List", + }, + } + + // Should allow access to any bucket + testCases := []string{"anybucket", "mybucket", "test-bucket", "prod-data"} + + for _, bucket := range testCases { + result := identity.canDo("List", bucket, "") + assert.True(t, result, "Global List permission should allow access to bucket %s", bucket) + } + }) + + t.Run("No Wildcard Exact Match", func(t *testing.T) { + // Create identity with exact bucket permission (no wildcard) + identity := &Identity{ + Name: "exact-user", + Actions: []Action{ + "List:specificbucket", + }, + } + + // Should only allow access to the exact bucket + assert.True(t, identity.canDo("List", "specificbucket", ""), "Should allow access to exact bucket") + assert.False(t, identity.canDo("List", "specificbucket-test", ""), "Should deny access to bucket with suffix") + assert.False(t, identity.canDo("List", "otherbucket", ""), "Should deny access to different bucket") + }) + + t.Log("This test validates the fix for issue #7066") + t.Log("Bucket-level List permissions like 'List:bucket*' work correctly") + t.Log("ListBucketsHandler now uses consistent authentication flow") +} + +// TestListBucketsAuthRequest tests that authRequest works correctly for ListBuckets operations +// This test validates that the fix for the regression identified in PR #7067 works correctly +func TestListBucketsAuthRequest(t *testing.T) { + t.Run("ListBuckets special case handling", func(t *testing.T) { + // Create identity with bucket-specific permissions (no global List permission) + identity := &Identity{ + Name: "bucket-user", + Account: &AccountAdmin, + Actions: []Action{ + Action("List:mybucket*"), + Action("Read:mybucket*"), + }, + } + + // Test 1: ListBuckets operation should succeed (bucket = "") + // This would have failed before the fix because canDo("List", "", "") would return false + // After the fix, it bypasses the canDo check for ListBuckets operations + + // Simulate what happens in authRequest for ListBuckets: + // action = ACTION_LIST, bucket = "", object = "" + + // Before fix: identity.canDo(ACTION_LIST, "", "") would fail + // After fix: the canDo check should be bypassed + + // Test the individual canDo method to show it would fail without the special case + result := identity.canDo(Action(ACTION_LIST), "", "") + assert.False(t, result, "canDo should return false for empty bucket with bucket-specific permissions") + + // Test with a specific bucket that matches the permission + result2 := identity.canDo(Action(ACTION_LIST), "mybucket", "") + assert.True(t, result2, "canDo should return true for matching bucket") + + // Test with a specific bucket that doesn't match + result3 := identity.canDo(Action(ACTION_LIST), "otherbucket", "") + assert.False(t, result3, "canDo should return false for non-matching bucket") + }) + + t.Run("Object listing maintains permission enforcement", func(t *testing.T) { + // Create identity with bucket-specific permissions + identity := &Identity{ + Name: "bucket-user", + Account: &AccountAdmin, + Actions: []Action{ + Action("List:mybucket*"), + }, + } + + // For object listing operations, the normal permission checks should still apply + // These operations have a specific bucket in the URL + + // Should succeed for allowed bucket + result1 := identity.canDo(Action(ACTION_LIST), "mybucket", "prefix/") + assert.True(t, result1, "Should allow listing objects in permitted bucket") + + result2 := identity.canDo(Action(ACTION_LIST), "mybucket-prod", "") + assert.True(t, result2, "Should allow listing objects in wildcard-matched bucket") + + // Should fail for disallowed bucket + result3 := identity.canDo(Action(ACTION_LIST), "otherbucket", "") + assert.False(t, result3, "Should deny listing objects in non-permitted bucket") + }) + + t.Log("This test validates the fix for the regression identified in PR #7067") + t.Log("ListBuckets operation bypasses global permission check when bucket is empty") + t.Log("Object listing still properly enforces bucket-level permissions") +} diff --git a/weed/s3api/s3api_bucket_handlers.go b/weed/s3api/s3api_bucket_handlers.go index bf37d1ca1..6a7052208 100644 --- a/weed/s3api/s3api_bucket_handlers.go +++ b/weed/s3api/s3api_bucket_handlers.go @@ -37,7 +37,9 @@ func (s3a *S3ApiServer) ListBucketsHandler(w http.ResponseWriter, r *http.Reques var identity *Identity var s3Err s3err.ErrorCode if s3a.iam.isEnabled() { - identity, s3Err = s3a.iam.authUser(r) + // Use authRequest instead of authUser for consistency with other endpoints + // This ensures the same authentication flow and any fixes (like prefix handling) are applied + identity, s3Err = s3a.iam.authRequest(r, s3_constants.ACTION_LIST) if s3Err != s3err.ErrNone { s3err.WriteErrorResponse(w, r, s3Err) return |