From ce6193b191c6e21e222c5d0ffb01189a12b0078d Mon Sep 17 00:00:00 2001 From: niksis02 Date: Mon, 13 Oct 2025 21:39:04 +0400 Subject: [PATCH] feat: adds bucket policy version support Closes #1536 Adds bucket policy version support. Two versions are supported: **2008-10-17** and **2012-10-17**. If the `Version` field is omitted in the bucket policy document, it defaults to **2008-10-17**. However, if an empty string (`""`) is provided, it is considered invalid. --- auth/bucket_policy.go | 15 ++++++++- auth/bucket_policy_version.go | 32 ++++++++++++++++++ auth/bucket_policy_version_test.go | 54 ++++++++++++++++++++++++++++++ tests/integration/group-tests.go | 2 ++ tests/integration/tests.go | 54 ++++++++++++++++++++++++++++++ 5 files changed, 156 insertions(+), 1 deletion(-) create mode 100644 auth/bucket_policy_version.go create mode 100644 auth/bucket_policy_version_test.go diff --git a/auth/bucket_policy.go b/auth/bucket_policy.go index 94987c81..60ee8fbb 100644 --- a/auth/bucket_policy.go +++ b/auth/bucket_policy.go @@ -40,14 +40,17 @@ const ( policyErrInvalidFirstChar = policyErr("Policies must be valid JSON and the first byte must be '{'") policyErrEmptyStatement = policyErr("Could not parse the policy: Statement is empty!") policyErrMissingStatmentField = policyErr("Missing required field Statement") + policyErrInvalidVersion = policyErr("The policy must contain a valid version string") ) type BucketPolicy struct { + Version PolicyVersion `json:"Version"` Statement []BucketPolicyItem `json:"Statement"` } func (bp *BucketPolicy) UnmarshalJSON(data []byte) error { var tmp struct { + Version *PolicyVersion Statement *[]BucketPolicyItem `json:"Statement"` } @@ -60,12 +63,22 @@ func (bp *BucketPolicy) UnmarshalJSON(data []byte) error { return policyErrMissingStatmentField } - // Assign the parsed value to the actual struct + if tmp.Version == nil { + // bucket policy version should defualt to '2008-10-17' + bp.Version = PolicyVersion2008 + } else { + bp.Version = *tmp.Version + } + bp.Statement = *tmp.Statement return nil } func (bp *BucketPolicy) Validate(bucket string, iam IAMService) error { + if !bp.Version.isValid() { + return policyErrInvalidVersion + } + for _, statement := range bp.Statement { err := statement.Validate(bucket, iam) if err != nil { diff --git a/auth/bucket_policy_version.go b/auth/bucket_policy_version.go new file mode 100644 index 00000000..c1216a5b --- /dev/null +++ b/auth/bucket_policy_version.go @@ -0,0 +1,32 @@ +// 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 + +type PolicyVersion string + +const ( + PolicyVersion2008 PolicyVersion = "2008-10-17" + PolicyVersion2012 PolicyVersion = "2012-10-17" +) + +// isValid checks if the policy version is valid or not +func (pv PolicyVersion) isValid() bool { + switch pv { + case PolicyVersion2008, PolicyVersion2012: + return true + default: + return false + } +} diff --git a/auth/bucket_policy_version_test.go b/auth/bucket_policy_version_test.go new file mode 100644 index 00000000..4ebe9abd --- /dev/null +++ b/auth/bucket_policy_version_test.go @@ -0,0 +1,54 @@ +// 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 +// 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 TestPolicyVersion_isValid(t *testing.T) { + tests := []struct { + name string // description of this test case + value string + want bool + }{ + {"valid 2008", "2008-10-17", true}, + {"valid 2012", "2012-10-17", true}, + {"invalid empty", "", false}, + {"invalid 1", "invalid", false}, + {"invalid 2", "2010-10-17", false}, + {"invalid 3", "2006-00-12", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := PolicyVersion(tt.value).isValid() + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index 0a388872..2e81bc49 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -536,6 +536,7 @@ func TestPutBucketPolicy(ts *TestState) { ts.Run(PutBucketPolicy_explicit_deny) ts.Run(PutBucketPolicy_multi_wildcard_resource) ts.Run(PutBucketPolicy_any_char_match) + ts.Run(PutBucketPolicy_version) ts.Run(PutBucketPolicy_success) } @@ -1412,6 +1413,7 @@ func GetIntTests() IntTests { "PutBucketPolicy_explicit_deny": PutBucketPolicy_explicit_deny, "PutBucketPolicy_multi_wildcard_resource": PutBucketPolicy_multi_wildcard_resource, "PutBucketPolicy_any_char_match": PutBucketPolicy_any_char_match, + "PutBucketPolicy_version": PutBucketPolicy_version, "PutBucketPolicy_success": PutBucketPolicy_success, "GetBucketPolicy_non_existing_bucket": GetBucketPolicy_non_existing_bucket, "GetBucketPolicy_not_set": GetBucketPolicy_not_set, diff --git a/tests/integration/tests.go b/tests/integration/tests.go index 8d22dcce..a7c442ab 100644 --- a/tests/integration/tests.go +++ b/tests/integration/tests.go @@ -14830,6 +14830,60 @@ func PutBucketPolicy_any_char_match(s *S3Conf) error { }) } +func PutBucketPolicy_version(s *S3Conf) error { + testName := "PutBucketPolicy_version" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + invalidVersionErr := getMalformedPolicyError("The policy must contain a valid version string") + for i, test := range []struct { + version string + err error + }{ + {"2008-10-17", nil}, + {"2012-10-17", nil}, + {"", invalidVersionErr}, + {"invalid", invalidVersionErr}, + {"2000-10-17", invalidVersionErr}, + {"2012-10-16", invalidVersionErr}, + } { + policy := fmt.Sprintf( + `{ + "Version": "%s", + "Statement": [ + { + "Effect": "Deny", + "Principal": "%s", + "Action": "s3:GetObject", + "Resource": "arn:aws:s3:::%s/obj" + } + ] + } + `, test.version, s.awsID, bucket) + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{ + Bucket: &bucket, + Policy: &policy, + }) + cancel() + if test.err == nil && err != nil { + return fmt.Errorf("test %v failed: expected no error but got %v", i+1, err) + } + if test.err != nil { + apiErr, ok := test.err.(s3err.APIError) + if !ok { + return fmt.Errorf("test %v failed: expected s3err.APIError", i+1) + } + + if err := checkApiErr(err, apiErr); err != nil { + return fmt.Errorf("test %v failed: %v", i+1, err) + } + } + } + + return nil + }) +} + func PutBucketPolicy_success(s *S3Conf) error { testName := "PutBucketPolicy_success" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {