Merge pull request #1107 from versity/fix/bucket-policy-multi-wildcard-support

fix: Implements a generic wildcard match for bucket policy resources

auth/bucket_policy_resources.go

@@ -102,21 +102,45 @@ func (r Resources) Validate(bucket string) error {
 
 func (r Resources) FindMatch(resource string) bool {
 	for res := range r {
-		if strings.HasSuffix(res, "*") {
-			pattern := strings.TrimSuffix(res, "*")
-			if strings.HasPrefix(resource, pattern) {
-				return true
-			}
-		} else {
-			if res == resource {
-				return true
-			}
+		if r.Match(res, resource) {
+			return true
 		}
 	}
 
 	return false
 }
 
+// Match checks if the input string matches the given pattern with wildcards (`*`, `?`).
+// - `?` matches exactly one occurrence of any character.
+// - `*` matches arbitrary many (including zero) occurrences of any character.
+func (r Resources) Match(pattern, input string) bool {
+	pIdx, sIdx := 0, 0
+	starIdx, matchIdx := -1, 0
+
+	for sIdx < len(input) {
+		if pIdx < len(pattern) && (pattern[pIdx] == '?' || pattern[pIdx] == input[sIdx]) {
+			sIdx++
+			pIdx++
+		} else if pIdx < len(pattern) && pattern[pIdx] == '*' {
+			starIdx = pIdx
+			matchIdx = sIdx
+			pIdx++
+		} else if starIdx != -1 {
+			pIdx = starIdx + 1
+			matchIdx++
+			sIdx = matchIdx
+		} else {
+			return false
+		}
+	}
+
+	for pIdx < len(pattern) && pattern[pIdx] == '*' {
+		pIdx++
+	}
+
+	return pIdx == len(pattern)
+}
+
 // Checks the resource to have arn prefix and not starting with /
 func isValidResource(rc string) (isValid bool, pattern string) {
 	if !strings.HasPrefix(rc, ResourceArnPrefix) {

auth/bucket_policy_resources_test.go

@@ -0,0 +1,182 @@
+// Copyright 2023 Versity Software
+// This file is licensed under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package auth
+
+import (
+	"encoding/json"
+	"testing"
+)
+
+func TestUnmarshalJSON(t *testing.T) {
+	var r Resources
+
+	cases := []struct {
+		input    string
+		expected int
+		wantErr  bool
+	}{
+		{`"arn:aws:s3:::my-bucket/*"`, 1, false},
+		{`["arn:aws:s3:::my-bucket/*", "arn:aws:s3:::other-bucket"]`, 2, false},
+		{`""`, 0, true},
+		{`[]`, 0, true},
+		{`["invalid-bucket"]`, 0, true},
+	}
+
+	for _, tc := range cases {
+		r = Resources{}
+		err := json.Unmarshal([]byte(tc.input), &r)
+		if (err != nil) != tc.wantErr {
+			t.Errorf("Unexpected error status for input %s: %v", tc.input, err)
+		}
+		if len(r) != tc.expected {
+			t.Errorf("Expected %d resources, got %d", tc.expected, len(r))
+		}
+	}
+}
+
+func TestAdd(t *testing.T) {
+	r := Resources{}
+
+	cases := []struct {
+		input   string
+		wantErr bool
+	}{
+		{"arn:aws:s3:::valid-bucket/*", false},
+		{"arn:aws:s3:::valid-bucket/object", false},
+		{"invalid-bucket/*", true},
+		{"/invalid-start", true},
+	}
+
+	for _, tc := range cases {
+		err := r.Add(tc.input)
+		if (err != nil) != tc.wantErr {
+			t.Errorf("Unexpected error status for input %s: %v", tc.input, err)
+		}
+	}
+}
+
+func TestContainsObjectPattern(t *testing.T) {
+	cases := []struct {
+		resources []string
+		expected  bool
+	}{
+		{[]string{"arn:aws:s3:::my-bucket/my-object"}, true},
+		{[]string{"arn:aws:s3:::my-bucket/*"}, true},
+		{[]string{"arn:aws:s3:::my-bucket"}, false},
+	}
+
+	for _, tc := range cases {
+		r := Resources{}
+		for _, res := range tc.resources {
+			r.Add(res)
+		}
+		if r.ContainsObjectPattern() != tc.expected {
+			t.Errorf("Expected object pattern to be %v for %v", tc.expected, tc.resources)
+		}
+	}
+}
+
+func TestContainsBucketPattern(t *testing.T) {
+	cases := []struct {
+		resources []string
+		expected  bool
+	}{
+		{[]string{"arn:aws:s3:::my-bucket"}, true},
+		{[]string{"arn:aws:s3:::my-bucket/*"}, false},
+		{[]string{"arn:aws:s3:::my-bucket/object"}, false},
+	}
+
+	for _, tc := range cases {
+		r := Resources{}
+		for _, res := range tc.resources {
+			r.Add(res)
+		}
+		if r.ContainsBucketPattern() != tc.expected {
+			t.Errorf("Expected bucket pattern to be %v for %v", tc.expected, tc.resources)
+		}
+	}
+}
+
+func TestValidate(t *testing.T) {
+	cases := []struct {
+		resources []string
+		bucket    string
+		expected  bool
+	}{
+		{[]string{"arn:aws:s3:::valid-bucket/*"}, "valid-bucket", true},
+		{[]string{"arn:aws:s3:::wrong-bucket/*"}, "valid-bucket", false},
+		{[]string{"arn:aws:s3:::valid-bucket/*", "arn:aws:s3:::valid-bucket/object/*"}, "valid-bucket", true},
+	}
+
+	for _, tc := range cases {
+		r := Resources{}
+		for _, res := range tc.resources {
+			r.Add(res)
+		}
+		if (r.Validate(tc.bucket) == nil) != tc.expected {
+			t.Errorf("Expected validation to be %v for bucket %s", tc.expected, tc.bucket)
+		}
+	}
+}
+
+func TestFindMatch(t *testing.T) {
+	cases := []struct {
+		resources []string
+		input     string
+		expected  bool
+	}{
+		{[]string{"arn:aws:s3:::my-bucket/*"}, "my-bucket/my-object", true},
+		{[]string{"arn:aws:s3:::my-bucket/object"}, "other-bucket/my-object", false},
+		{[]string{"arn:aws:s3:::my-bucket/object"}, "my-bucket/object", true},
+		{[]string{"arn:aws:s3:::my-bucket/*", "arn:aws:s3:::other-bucket/*"}, "other-bucket/something", true},
+	}
+
+	for _, tc := range cases {
+		r := Resources{}
+		for _, res := range tc.resources {
+			r.Add(res)
+		}
+		if r.FindMatch(tc.input) != tc.expected {
+			t.Errorf("Expected FindMatch to be %v for input %s", tc.expected, tc.input)
+		}
+	}
+}
+
+func TestMatch(t *testing.T) {
+	r := Resources{}
+	cases := []struct {
+		pattern  string
+		input    string
+		expected bool
+	}{
+		{"my-bucket/*", "my-bucket/object", true},
+		{"my-bucket/?bject", "my-bucket/object", true},
+		{"my-bucket/*", "other-bucket/object", false},
+		{"*", "any-bucket/object", true},
+		{"my-bucket/*", "my-bucket/subdir/object", true},
+		{"my-bucket/*", "other-bucket", false},
+		{"my-bucket/*/*", "my-bucket/hello", false},
+		{"my-bucket/*/*", "my-bucket/hello/world", true},
+		{"foo/???/bar", "foo/qux/bar", true},
+		{"foo/???/bar", "foo/quxx/bar", false},
+		{"foo/???/bar/*/?", "foo/qux/bar/hello/g", true},
+		{"foo/???/bar/*/?", "foo/qux/bar/hello/smth", false},
+	}
+	for _, tc := range cases {
+		if r.Match(tc.pattern, tc.input) != tc.expected {
+			t.Errorf("Match(%s, %s) failed, expected %v", tc.pattern, tc.input, tc.expected)
+		}
+	}
+}

tests/integration/group-tests.go

@@ -472,6 +472,8 @@ func TestPutBucketPolicy(s *S3Conf) {
 	PutBucketPolicy_object_action_on_bucket_resource(s)
 	PutBucketPolicy_bucket_action_on_object_resource(s)
 	PutBucketPolicy_explicit_deny(s)
+	PutBucketPolicy_multi_wildcard_resource(s)
+	PutBucketPolicy_any_char_match(s)
 	PutBucketPolicy_success(s)
 }
 
@@ -1045,6 +1047,8 @@ func GetIntTests() IntTests {
 		"PutBucketPolicy_object_action_on_bucket_resource":                        PutBucketPolicy_object_action_on_bucket_resource,
 		"PutBucketPolicy_explicit_deny":                                           PutBucketPolicy_explicit_deny,
 		"PutBucketPolicy_bucket_action_on_object_resource":                        PutBucketPolicy_bucket_action_on_object_resource,
+		"PutBucketPolicy_multi_wildcard_resource":                                 PutBucketPolicy_multi_wildcard_resource,
+		"PutBucketPolicy_any_char_match":                                          PutBucketPolicy_any_char_match,
 		"PutBucketPolicy_success":                                                 PutBucketPolicy_success,
 		"GetBucketPolicy_non_existing_bucket":                                     GetBucketPolicy_non_existing_bucket,
 		"GetBucketPolicy_not_set":                                                 GetBucketPolicy_not_set,

tests/integration/tests.go

@@ -11195,7 +11195,6 @@ func PutBucketPolicy_bucket_action_on_object_resource(s *S3Conf) error {
 		return nil
 	})
 }
-
 func PutBucketPolicy_explicit_deny(s *S3Conf) error {
 	testName := "PutBucketPolicy_object_action_on_bucket_resource"
 	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
@@ -11281,6 +11280,80 @@ func PutBucketPolicy_explicit_deny(s *S3Conf) error {
 	})
 }
 
+func PutBucketPolicy_multi_wildcard_resource(s *S3Conf) error {
+	testName := "PutBucketPolicy_multi_wildcard_resource"
+	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
+		usr := user{"grt1", "grt1secret", "user"}
+		if err := createUsers(s, []user{usr}); err != nil {
+			return err
+		}
+
+		resource := fmt.Sprintf(`["arn:aws:s3:::%v/*/*", "arn:aws:s3:::%v"]`, bucket, bucket)
+		principal := fmt.Sprintf("\"%v\"", usr.access)
+		doc := genPolicyDoc("Allow", principal, `"s3:*"`, resource)
+
+		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
+		_, err := s3client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{
+			Bucket: &bucket,
+			Policy: &doc,
+		})
+		cancel()
+		if err != nil {
+			return err
+		}
+
+		userClient := getUserS3Client(usr, s)
+		_, err = putObjects(userClient, []string{"foo"}, bucket)
+		if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrAccessDenied)); err != nil {
+			return err
+		}
+
+		_, err = putObjects(userClient, []string{"bar/quxx", "foo/bar/baz", "foo/bar/xyz/quxx"}, bucket)
+		if err != nil {
+			return err
+		}
+
+		return nil
+	})
+}
+
+func PutBucketPolicy_any_char_match(s *S3Conf) error {
+	testName := "PutBucketPolicy_any_char_match"
+	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
+		usr := user{"grt1", "grt1secret", "user"}
+		if err := createUsers(s, []user{usr}); err != nil {
+			return err
+		}
+
+		resource := fmt.Sprintf(`["arn:aws:s3:::%v/m?-obj/*"]`, bucket)
+		principal := fmt.Sprintf("\"%v\"", usr.access)
+		doc := genPolicyDoc("Allow", principal, `"s3:*"`, resource)
+
+		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
+		_, err := s3client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{
+			Bucket: &bucket,
+			Policy: &doc,
+		})
+		cancel()
+		if err != nil {
+			return err
+		}
+
+		userClient := getUserS3Client(usr, s)
+		_, err = putObjects(userClient, []string{"myy-obj/hello", "rand/foo", "my-objj/bar"}, bucket)
+		if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrAccessDenied)); err != nil {
+			return err
+		}
+
+		_, err = putObjects(userClient, []string{"my-obj/hello", "mk-obj/foo", "m--obj/bar"}, bucket)
+		if err != nil {
+			return err
+		}
+
+		return nil
+	})
+}
+
 func PutBucketPolicy_success(s *S3Conf) error {
 	testName := "PutBucketPolicy_success"
 	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {