Merge pull request #1495 from versity/sis/bucket-policy-wildcard-action

fix: adds full wildcard and any character match for bucket policy actions

auth/bucket_policy.go

@@ -257,3 +257,34 @@ func VerifyPublicBucketPolicy(policy []byte, bucket, object string, action Actio
 
 	return nil
 }
+
+// matchPattern checks if the input string matches the given pattern with wildcard(`*`) and any character(`?`).
+// - `?` matches exactly one occurrence of any character.
+// - `*` matches arbitrary many (including zero) occurrences of any character.
+func matchPattern(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)
+}

auth/bucket_policy_actions.go

@@ -192,55 +192,54 @@ func (a Action) IsValid() error {
 		return nil
 	}
 
-	if a[len(a)-1] == '*' {
-		pattern := strings.TrimSuffix(string(a), "*")
-		for act := range supportedActionList {
-			if strings.HasPrefix(string(act), pattern) {
-				return nil
-			}
+	// first check for an exact match
+	if _, ok := supportedActionList[a]; ok {
+		return nil
+	}
+
+	// walk through the supported actions and try wildcard match
+	for action := range supportedActionList {
+		if action.Match(a) {
+			return nil
 		}
-
-		return policyErrInvalidAction
 	}
 
-	_, found := supportedActionList[a]
-	if !found {
-		return policyErrInvalidAction
-	}
-	return nil
+	return policyErrInvalidAction
 }
 
 func getBoolPtr(bl bool) *bool {
 	return &bl
 }
 
+// String converts the action to string
+func (a Action) String() string {
+	return string(a)
+}
+
+// Match wildcard matches the given pattern to the action
+func (a Action) Match(pattern Action) bool {
+	return matchPattern(pattern.String(), a.String())
+}
+
 // Checks if the action is object action
 // nil points to 's3:*'
 func (a Action) IsObjectAction() *bool {
 	if a == AllActions {
 		return nil
 	}
-	if a[len(a)-1] == '*' {
-		pattern := strings.TrimSuffix(string(a), "*")
-		for act := range supportedObjectActionList {
-			if strings.HasPrefix(string(act), pattern) {
-				return getBoolPtr(true)
-			}
+
+	// first find an exact match
+	if _, ok := supportedObjectActionList[a]; ok {
+		return &ok
+	}
+
+	for action := range supportedObjectActionList {
+		if action.Match(a) {
+			return getBoolPtr(true)
 		}
-
-		return getBoolPtr(false)
 	}
 
-	_, found := supportedObjectActionList[a]
-	return &found
-}
-
-func (a Action) WildCardMatch(act Action) bool {
-	if strings.HasSuffix(string(a), "*") {
-		pattern := strings.TrimSuffix(string(a), "*")
-		return strings.HasPrefix(string(act), pattern)
-	}
-	return false
+	return getBoolPtr(false)
 }
 
 type Actions map[Action]struct{}
@@ -289,6 +288,7 @@ func (a Actions) Add(str string) error {
 	return nil
 }
 
+// FindMatch tries to match the given action to the actions list
 func (a Actions) FindMatch(action Action) bool {
 	_, ok := a[AllActions]
 	if ok {
@@ -300,8 +300,9 @@ func (a Actions) FindMatch(action Action) bool {
 		return true
 	}
 
+	// search for a wildcard match
 	for act := range a {
-		if strings.HasSuffix(string(act), "*") && act.WildCardMatch(action) {
+		if action.Match(act) {
 			return true
 		}
 	}

auth/bucket_policy_actions_test.go

@@ -0,0 +1,175 @@
+// 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"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestAction_IsValid(t *testing.T) {
+	tests := []struct {
+		name    string
+		action  Action
+		wantErr bool
+	}{
+		{"valid exact action", GetObjectAction, false},
+		{"valid all actions", AllActions, false},
+		{"invalid prefix", "invalid:Action", true},
+		{"unsupported action 1", "s3:Unsupported", true},
+		{"unsupported action 2", "s3:HeadObject", true},
+		{"valid wildcard match 1", "s3:Get*", false},
+		{"valid wildcard match 2", "s3:*Object*", false},
+		{"valid wildcard match 3", "s3:*Multipart*", false},
+		{"any char match 1", "s3:Get?bject", false},
+		{"any char match 2", "s3:Get??bject", true},
+		{"any char match 3", "s3:???", true},
+		{"mixed match 1", "s3:Get?*", false},
+		{"mixed match 2", "s3:*Object?????", true},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := tt.action.IsValid()
+			if tt.wantErr {
+				assert.EqualValues(t, policyErrInvalidAction, err)
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+}
+
+func TestAction_String(t *testing.T) {
+	a := Action("s3:TestAction")
+	assert.Equal(t, "s3:TestAction", a.String())
+}
+
+func TestAction_Match(t *testing.T) {
+	tests := []struct {
+		name    string
+		action  Action
+		pattern Action
+		want    bool
+	}{
+		{"exact match", "s3:GetObject", "s3:GetObject", true},
+		{"wildcard match", "s3:GetObject", "s3:Get*", true},
+		{"wildcard mismatch", "s3:PutObject", "s3:Get*", false},
+		{"any character match", "s3:Get1", "s3:Get?", true},
+		{"any character mismatch", "s3:Get12", "s3:Get?", false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := tt.action.Match(tt.pattern)
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}
+
+func TestAction_IsObjectAction(t *testing.T) {
+	tests := []struct {
+		name   string
+		action Action
+		want   *bool
+	}{
+		{"all actions", AllActions, nil},
+		{"object action exact", GetObjectAction, getBoolPtr(true)},
+		{"object action wildcard", "s3:Get*", getBoolPtr(true)},
+		{"non object action", GetBucketAclAction, getBoolPtr(false)},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := tt.action.IsObjectAction()
+			if tt.want == nil {
+				assert.Nil(t, got)
+			} else {
+				assert.NotNil(t, got)
+				assert.Equal(t, *tt.want, *got)
+			}
+		})
+	}
+}
+
+func TestActions_UnmarshalJSON(t *testing.T) {
+	tests := []struct {
+		name    string
+		input   string
+		wantErr bool
+	}{
+		{"valid slice", `["s3:GetObject","s3:PutObject"]`, false},
+		{"empty slice", `[]`, true},
+		{"invalid action in slice", `["s3:Invalid"]`, true},
+		{"valid string", `"s3:GetObject"`, false},
+		{"empty string", `""`, true},
+		{"invalid string", `"s3:Invalid"`, true},
+		{"invalid json", `{}`, true},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var a Actions
+			err := json.Unmarshal([]byte(tt.input), &a)
+			if tt.wantErr {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+}
+
+func TestActions_Add(t *testing.T) {
+	tests := []struct {
+		name    string
+		action  string
+		wantErr bool
+	}{
+		{"valid add", "s3:GetObject", false},
+		{"invalid add", "s3:InvalidAction", true},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			a := make(Actions)
+			err := a.Add(tt.action)
+			if tt.wantErr {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+				_, ok := a[Action(tt.action)]
+				assert.True(t, ok)
+			}
+		})
+	}
+}
+
+func TestActions_FindMatch(t *testing.T) {
+	tests := []struct {
+		name    string
+		actions Actions
+		check   Action
+		want    bool
+	}{
+		{"all actions present", Actions{AllActions: {}}, GetObjectAction, true},
+		{"exact match", Actions{GetObjectAction: {}}, GetObjectAction, true},
+		{"wildcard match", Actions{"s3:Get*": {}}, GetObjectAction, true},
+		{"no match", Actions{"s3:Put*": {}}, GetObjectAction, false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := tt.actions.FindMatch(tt.check)
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}

auth/bucket_policy_effect_test.go

@@ -0,0 +1,57 @@
+// 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 (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestBucketPolicyAccessType_Validate(t *testing.T) {
+	tests := []struct {
+		name    string
+		input   BucketPolicyAccessType
+		wantErr bool
+		errMsg  string
+	}{
+		{
+			name:    "valid allow",
+			input:   BucketPolicyAccessTypeAllow,
+			wantErr: false,
+		},
+		{
+			name:    "valid deny",
+			input:   BucketPolicyAccessTypeDeny,
+			wantErr: false,
+		},
+		{
+			name:    "invalid type",
+			input:   BucketPolicyAccessType("InvalidValue"),
+			wantErr: true,
+			errMsg:  "Invalid effect: InvalidValue",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := tt.input.Validate()
+			if tt.wantErr {
+				assert.EqualError(t, err, tt.errMsg)
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+}

auth/bucket_policy_principals_test.go

@@ -0,0 +1,106 @@
+// 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"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestPrincipals_Add(t *testing.T) {
+	p := make(Principals)
+	p.Add("user1")
+	_, ok := p["user1"]
+	assert.True(t, ok)
+}
+
+func TestPrincipals_UnmarshalJSON(t *testing.T) {
+	tests := []struct {
+		name    string
+		input   string
+		want    Principals
+		wantErr bool
+	}{
+		{"valid slice", `["user1","user2"]`, Principals{"user1": {}, "user2": {}}, false},
+		{"empty slice", `[]`, nil, true},
+		{"valid string", `"user1"`, Principals{"user1": {}}, false},
+		{"empty string", `""`, nil, true},
+		{"valid AWS object", `{"AWS":"user1"}`, Principals{"user1": {}}, false},
+		{"empty AWS object", `{"AWS":""}`, nil, true},
+		{"valid AWS array", `{"AWS":["user1","user2"]}`, Principals{"user1": {}, "user2": {}}, false},
+		{"empty AWS array", `{"AWS":[]}`, nil, true},
+		{"invalid json", `{invalid}`, nil, true},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var p Principals
+			err := json.Unmarshal([]byte(tt.input), &p)
+			if tt.wantErr {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+				assert.Equal(t, tt.want, p)
+			}
+		})
+	}
+}
+
+func TestPrincipals_ToSlice(t *testing.T) {
+	p := Principals{"user1": {}, "user2": {}, "*": {}}
+	got := p.ToSlice()
+	assert.Contains(t, got, "user1")
+	assert.Contains(t, got, "user2")
+	assert.NotContains(t, got, "*")
+}
+
+func TestPrincipals_Validate(t *testing.T) {
+	iamSingle := NewIAMServiceSingle(Account{
+		Access: "user1",
+	})
+	tests := []struct {
+		name       string
+		principals Principals
+		mockIAM    IAMService
+		err        error
+	}{
+		{"only wildcard", Principals{"*": {}}, iamSingle, nil},
+		{"wildcard and user", Principals{"*": {}, "user1": {}}, iamSingle, policyErrInvalidPrincipal},
+		{"accounts exist returns err", Principals{"user2": {}, "user3": {}}, iamSingle, policyErrInvalidPrincipal},
+		{"accounts exist non-empty", Principals{"user1": {}}, iamSingle, nil},
+		{"accounts valid", Principals{"user1": {}}, iamSingle, nil},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := tt.principals.Validate(tt.mockIAM)
+			assert.EqualValues(t, tt.err, err)
+		})
+	}
+}
+
+func TestPrincipals_Contains(t *testing.T) {
+	p := Principals{"user1": {}}
+	assert.True(t, p.Contains("user1"))
+	assert.False(t, p.Contains("user2"))
+
+	p = Principals{"*": {}}
+	assert.True(t, p.Contains("anyuser"))
+}
+
+func TestPrincipals_isPublic(t *testing.T) {
+	assert.True(t, Principals{"*": {}}.isPublic())
+	assert.False(t, Principals{"user1": {}}.isPublic())
+}

auth/bucket_policy_resources.go

@@ -110,35 +110,9 @@ func (r Resources) FindMatch(resource string) bool {
 	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.
+// Match matches the given input resource with the pattern
 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)
+	return matchPattern(pattern, input)
 }
 
 // Checks the resource to have arn prefix and not starting with /

tests/integration/group-tests.go

@@ -494,10 +494,7 @@ func TestPutBucketPolicy(s *S3Conf) {
 	PutBucketPolicy_statement_not_provided(s)
 	PutBucketPolicy_empty_statement(s)
 	PutBucketPolicy_invalid_effect(s)
-	PutBucketPolicy_empty_actions_string(s)
-	PutBucketPolicy_empty_actions_array(s)
 	PutBucketPolicy_invalid_action(s)
-	PutBucketPolicy_incorrect_action_wildcard_usage(s)
 	PutBucketPolicy_empty_principals_string(s)
 	PutBucketPolicy_empty_principals_array(s)
 	PutBucketPolicy_principals_aws_struct_empty_string(s)
@@ -510,8 +507,7 @@ func TestPutBucketPolicy(s *S3Conf) {
 	PutBucketPolicy_invalid_resource_with_starting_slash(s)
 	PutBucketPolicy_duplicate_resource(s)
 	PutBucketPolicy_incorrect_bucket_name(s)
-	PutBucketPolicy_object_action_on_bucket_resource(s)
-	PutBucketPolicy_bucket_action_on_object_resource(s)
+	PutBucketPolicy_action_resource_mismatch(s)
 	PutBucketPolicy_explicit_deny(s)
 	PutBucketPolicy_multi_wildcard_resource(s)
 	PutBucketPolicy_any_char_match(s)
@@ -1328,10 +1324,7 @@ func GetIntTests() IntTests {
 		"PutBucketPolicy_statement_not_provided":                                  PutBucketPolicy_statement_not_provided,
 		"PutBucketPolicy_empty_statement":                                         PutBucketPolicy_empty_statement,
 		"PutBucketPolicy_invalid_effect":                                          PutBucketPolicy_invalid_effect,
-		"PutBucketPolicy_empty_actions_string":                                    PutBucketPolicy_empty_actions_string,
-		"PutBucketPolicy_empty_actions_array":                                     PutBucketPolicy_empty_actions_array,
 		"PutBucketPolicy_invalid_action":                                          PutBucketPolicy_invalid_action,
-		"PutBucketPolicy_incorrect_action_wildcard_usage":                         PutBucketPolicy_incorrect_action_wildcard_usage,
 		"PutBucketPolicy_empty_principals_string":                                 PutBucketPolicy_empty_principals_string,
 		"PutBucketPolicy_empty_principals_array":                                  PutBucketPolicy_empty_principals_array,
 		"PutBucketPolicy_principals_aws_struct_empty_string":                      PutBucketPolicy_principals_aws_struct_empty_string,
@@ -1344,9 +1337,8 @@ func GetIntTests() IntTests {
 		"PutBucketPolicy_invalid_resource_with_starting_slash":                    PutBucketPolicy_invalid_resource_with_starting_slash,
 		"PutBucketPolicy_duplicate_resource":                                      PutBucketPolicy_duplicate_resource,
 		"PutBucketPolicy_incorrect_bucket_name":                                   PutBucketPolicy_incorrect_bucket_name,
-		"PutBucketPolicy_object_action_on_bucket_resource":                        PutBucketPolicy_object_action_on_bucket_resource,
+		"PutBucketPolicy_action_resource_mismatch":                                PutBucketPolicy_action_resource_mismatch,
 		"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,

tests/integration/tests.go

@@ -12938,78 +12938,42 @@ func PutBucketPolicy_invalid_effect(s *S3Conf) error {
 	})
 }
 
-func PutBucketPolicy_empty_actions_string(s *S3Conf) error {
-	testName := "PutBucketPolicy_empty_actions_string"
-	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
-		doc := genPolicyDoc("Allow", `"*"`, `""`, `"arn:aws:s3:::*"`)
-
-		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
-		_, err := s3client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{
-			Bucket: &bucket,
-			Policy: &doc,
-		})
-		cancel()
-
-		if err := checkApiErr(err, getMalformedPolicyError("Policy has invalid action")); err != nil {
-			return err
-		}
-		return nil
-	})
-}
-
-func PutBucketPolicy_empty_actions_array(s *S3Conf) error {
-	testName := "PutBucketPolicy_empty_actions_array"
-	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
-		doc := genPolicyDoc("Allow", `"*"`, `[]`, `"arn:aws:s3:::*"`)
-
-		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
-		_, err := s3client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{
-			Bucket: &bucket,
-			Policy: &doc,
-		})
-		cancel()
-
-		if err := checkApiErr(err, getMalformedPolicyError("Policy has invalid action")); err != nil {
-			return err
-		}
-		return nil
-	})
-}
-
 func PutBucketPolicy_invalid_action(s *S3Conf) error {
 	testName := "PutBucketPolicy_invalid_action"
 	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
-		doc := genPolicyDoc("Allow", `"*"`, `"ListObjects"`, `"arn:aws:s3:::*"`)
-
-		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
-		_, err := s3client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{
-			Bucket: &bucket,
-			Policy: &doc,
-		})
-		cancel()
-
-		if err := checkApiErr(err, getMalformedPolicyError("Policy has invalid action")); err != nil {
+		err := createUsers(s, []user{testuser1})
+		if err != nil {
 			return err
 		}
-		return nil
-	})
-}
 
-func PutBucketPolicy_incorrect_action_wildcard_usage(s *S3Conf) error {
-	testName := "PutBucketPolicy_incorrect_action_wildcard_usage"
-	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
-		doc := genPolicyDoc("Allow", `"*"`, `"s3:hello*"`, `"arn:aws:s3:::*"`)
+		for _, action := range []string{
+			// empty actions
+			`""`, "[]",
+			// completely invalid action
+			`"invalid_action"`, `["invalid_action"]`,
+			// only prefix
+			`"s3"`, `"s3:"`,
+			// malformed prefix
+			`"s4:GetObject"`, `"ss3:ListBucket"`, `"s3x:PutBucketAcl"`, `":GetObject"`, `"s3GetObject"`,
+			// bad separator
+			`"s3::GetObject"`, `"s3:Put-Object"`, `"s3:GetObject:"`, `"s3:Put(Object)"`,
+			// wildcard abuse
+			`"s3:*Obj??ect*"`, `"s3:????"`, `"s3:*:"`, `"*GetObject"`, `"???PutObject"`, `"s3:Abort?"`, `"s3:??Abort*"`,
+		} {
+			doc := genPolicyDoc("Allow", fmt.Sprintf(`"%s"`, testuser1.access), action, fmt.Sprintf(`"arn:aws:s3:::%s"`, bucket))
 
-		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
-		_, err := s3client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{
-			Bucket: &bucket,
-			Policy: &doc,
-		})
-		cancel()
+			ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
+			_, err := s3client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{
+				Bucket: &bucket,
+				Policy: &doc,
+			})
+			cancel()
 
-		if err := checkApiErr(err, getMalformedPolicyError("Policy has invalid action")); err != nil {
-			return err
+			if err := checkApiErr(err, getMalformedPolicyError("Policy has invalid action")); err != nil {
+				return err
+			}
 		}
+
 		return nil
 	})
 }
@@ -13247,45 +13211,46 @@ func PutBucketPolicy_incorrect_bucket_name(s *S3Conf) error {
 	})
 }
 
-func PutBucketPolicy_object_action_on_bucket_resource(s *S3Conf) error {
-	testName := "PutBucketPolicy_object_action_on_bucket_resource"
+func PutBucketPolicy_action_resource_mismatch(s *S3Conf) error {
+	testName := "PutBucketPolicy_action_resource_mismatch"
 	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
-		resource := fmt.Sprintf(`"arn:aws:s3:::%v"`, bucket)
-		doc := genPolicyDoc("Allow", `["*"]`, `"s3:PutObjectTagging"`, resource)
+		bucketResource := fmt.Sprintf(`"arn:aws:s3:::%s"`, bucket)
+		objectResource := fmt.Sprintf(`"arn:aws:s3:::%v/*"`, bucket)
 
-		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
-		_, err := s3client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{
-			Bucket: &bucket,
-			Policy: &doc,
-		})
-		cancel()
-		if err := checkApiErr(err, getMalformedPolicyError("Action does not apply to any resource(s) in statement")); err != nil {
-			return err
+		for _, test := range []struct {
+			resource string
+			action   string
+		}{
+			// bucket resources
+			{bucketResource, `"s3:GetObject"`},
+			{bucketResource, `"s3:PutObjectTagging"`},
+			{bucketResource, `"s3:GetObjec?"`},
+			{bucketResource, `"s3:Abort*"`},
+			{bucketResource, `"s3:*Multipart*"`},
+			{bucketResource, `"s3:???Object"`},
+			// object resources
+			{objectResource, `"s3:ListBucket"`},
+			{objectResource, `"s3:GetBucketTagging"`},
+			{objectResource, `"s3:???BucketVersioning"`},
+			{objectResource, `"s3:*Bucket*"`},
+			{objectResource, `"s3:GetBucket*"`},
+		} {
+			doc := genPolicyDoc("Allow", `["*"]`, test.action, test.resource)
+			ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
+			_, err := s3client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{
+				Bucket: &bucket,
+				Policy: &doc,
+			})
+			cancel()
+			if err := checkApiErr(err, getMalformedPolicyError("Action does not apply to any resource(s) in statement")); err != nil {
+				return err
+			}
 		}
 
 		return nil
 	})
 }
 
-func PutBucketPolicy_bucket_action_on_object_resource(s *S3Conf) error {
-	testName := "PutBucketPolicy_bucket_action_on_object_resource"
-	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
-		resource := fmt.Sprintf(`"arn:aws:s3:::%v/*"`, bucket)
-		doc := genPolicyDoc("Allow", `["*"]`, `"s3:DeleteBucket"`, resource)
-
-		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
-		_, err := s3client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{
-			Bucket: &bucket,
-			Policy: &doc,
-		})
-		cancel()
-
-		if err := checkApiErr(err, getMalformedPolicyError("Action does not apply to any resource(s) in statement")); err != nil {
-			return err
-		}
-		return nil
-	})
-}
 func PutBucketPolicy_explicit_deny(s *S3Conf) error {
 	testName := "PutBucketPolicy_explicit_deny"
 	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {