From 3e8d2a0a71d8be9cf8b79c488756dceddecda7ba Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Wed, 28 Jan 2026 16:37:31 -0800 Subject: [PATCH] s3tables: Use policy_engine wildcard matcher for complete IAM compatibility Replace the custom suffix-only wildcard implementation in matchesActionPattern and matchesPrincipal with the policy_engine.MatchesWildcard function from PR #8052. This enables full wildcard support including: - Middle wildcards: s3tables:Get*Table matches GetTable - Question mark wildcards: Get? matches any single character - Combined patterns: s3tables:*Table* matches any action containing 'Table' Benefits: - Code reuse: eliminates duplicate wildcard logic - Complete IAM compatibility: supports all AWS wildcard patterns - Performance: uses efficient O(n) backtracking algorithm - Consistency: same wildcard behavior across S3 API and S3 Tables Add comprehensive unit tests covering exact matches, suffix wildcards, middle wildcards, question marks, and combined patterns for both action and principal matching. --- weed/s3api/s3tables/permissions.go | 25 ++++--- weed/s3api/s3tables/permissions_test.go | 90 +++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 9 deletions(-) create mode 100644 weed/s3api/s3tables/permissions_test.go diff --git a/weed/s3api/s3tables/permissions.go b/weed/s3api/s3tables/permissions.go index a85591c46..3bd21b403 100644 --- a/weed/s3api/s3tables/permissions.go +++ b/weed/s3api/s3tables/permissions.go @@ -2,7 +2,8 @@ package s3tables import ( "encoding/json" - "strings" + + "github.com/seaweedfs/seaweedfs/weed/s3api/policy_engine" ) // Permission represents a specific action permission @@ -80,7 +81,11 @@ func matchesPrincipal(principalSpec interface{}, principal string) bool { switch p := principalSpec.(type) { case string: // Direct string match or wildcard - return p == "*" || p == principal + if p == "*" || p == principal { + return true + } + // Support wildcard matching for principals (e.g., "arn:aws:iam::*:user/admin") + return policy_engine.MatchesWildcard(p, principal) case []interface{}: // Array of principals for _, item := range p { @@ -88,6 +93,10 @@ func matchesPrincipal(principalSpec interface{}, principal string) bool { if str == "*" || str == principal { return true } + // Support wildcard matching + if policy_engine.MatchesWildcard(str, principal) { + return true + } } } case map[string]interface{}: @@ -126,6 +135,8 @@ func matchesAction(actionSpec interface{}, action string) bool { } // matchesActionPattern checks if an action matches a pattern (supports wildcards) +// This uses the policy_engine.MatchesWildcard function for full wildcard support, +// including middle wildcards (e.g., "s3tables:Get*Table") for complete IAM compatibility. func matchesActionPattern(pattern, action string) bool { if pattern == "*" { return true @@ -136,13 +147,9 @@ func matchesActionPattern(pattern, action string) bool { return true } - // Wildcard match (e.g., "s3tables:*" matches "s3tables:GetTable") - if strings.HasSuffix(pattern, "*") { - prefix := strings.TrimSuffix(pattern, "*") - return strings.HasPrefix(action, prefix) - } - - return false + // Wildcard match using policy engine's wildcard matcher + // Supports both * (any sequence) and ? (single character) anywhere in the pattern + return policy_engine.MatchesWildcard(pattern, action) } // Helper functions for specific permissions diff --git a/weed/s3api/s3tables/permissions_test.go b/weed/s3api/s3tables/permissions_test.go new file mode 100644 index 000000000..e9fe443ee --- /dev/null +++ b/weed/s3api/s3tables/permissions_test.go @@ -0,0 +1,90 @@ +package s3tables + +import "testing" + +func TestMatchesActionPattern(t *testing.T) { + tests := []struct { + name string + pattern string + action string + expected bool + }{ + // Exact matches + {"exact match", "GetTable", "GetTable", true}, + {"no match", "GetTable", "DeleteTable", false}, + + // Universal wildcard + {"universal wildcard", "*", "anything", true}, + + // Suffix wildcards + {"suffix wildcard match", "s3tables:*", "s3tables:GetTable", true}, + {"suffix wildcard no match", "s3tables:*", "iam:GetUser", false}, + + // Middle wildcards (new capability from policy_engine) + {"middle wildcard Get*Table", "s3tables:Get*Table", "s3tables:GetTable", true}, + {"middle wildcard Get*Table no match GetTableBucket", "s3tables:Get*Table", "s3tables:GetTableBucket", false}, + {"middle wildcard Get*Table no match DeleteTable", "s3tables:Get*Table", "s3tables:DeleteTable", false}, + {"middle wildcard *Table*", "s3tables:*Table*", "s3tables:GetTableBucket", true}, + {"middle wildcard *Table* match CreateTable", "s3tables:*Table*", "s3tables:CreateTable", true}, + + // Question mark wildcards + {"question mark single char", "GetTable?", "GetTableX", true}, + {"question mark no match", "GetTable?", "GetTableXY", false}, + + // Combined wildcards + {"combined * and ?", "s3tables:Get?able*", "s3tables:GetTable", true}, + {"combined * and ?", "s3tables:Get?able*", "s3tables:GetTables", true}, + {"combined no match - ? needs 1 char", "s3tables:Get?able*", "s3tables:Getable", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := matchesActionPattern(tt.pattern, tt.action) + if result != tt.expected { + t.Errorf("matchesActionPattern(%q, %q) = %v, want %v", tt.pattern, tt.action, result, tt.expected) + } + }) + } +} + +func TestMatchesPrincipal(t *testing.T) { + tests := []struct { + name string + principalSpec interface{} + principal string + expected bool + }{ + // String principals + {"exact match", "user123", "user123", true}, + {"no match", "user123", "user456", false}, + {"universal wildcard", "*", "anyone", true}, + + // Wildcard principals + {"prefix wildcard", "arn:aws:iam::123456789012:user/*", "arn:aws:iam::123456789012:user/admin", true}, + {"prefix wildcard no match", "arn:aws:iam::123456789012:user/*", "arn:aws:iam::987654321098:user/admin", false}, + {"middle wildcard", "arn:aws:iam::*:user/admin", "arn:aws:iam::123456789012:user/admin", true}, + + // Array of principals + {"array match first", []interface{}{"user1", "user2"}, "user1", true}, + {"array match second", []interface{}{"user1", "user2"}, "user2", true}, + {"array no match", []interface{}{"user1", "user2"}, "user3", false}, + {"array wildcard", []interface{}{"user1", "arn:aws:iam::*:user/admin"}, "arn:aws:iam::123:user/admin", true}, + + // Map-style AWS principals + {"AWS map exact", map[string]interface{}{"AWS": "user123"}, "user123", true}, + {"AWS map wildcard", map[string]interface{}{"AWS": "arn:aws:iam::*:user/admin"}, "arn:aws:iam::123:user/admin", true}, + {"AWS map array", map[string]interface{}{"AWS": []interface{}{"user1", "user2"}}, "user1", true}, + + // Nil/empty cases + {"nil principal", nil, "user123", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := matchesPrincipal(tt.principalSpec, tt.principal) + if result != tt.expected { + t.Errorf("matchesPrincipal(%v, %q) = %v, want %v", tt.principalSpec, tt.principal, result, tt.expected) + } + }) + } +}