mirrors/versitygw
1
0
Fork 0
mirror of https://github.com/versity/versitygw.git synced 2026-01-04 19:13:57 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: Changed bucket policy validation error messages

Browse Source
This commit is contained in:
jonaustin09
2024-06-24 13:14:28 -04:00
parent a2330959ea
commit 329fae5203
7 changed files with 98 additions and 61 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
auth/bucket_policy.go
View File

@@ -16,12 +16,22 @@ package auth
import (
"encoding/json"
"fmt"
"errors"
"net/http"
"github.com/versity/versitygw/s3err"
)
var (
errResourceMismatch = errors.New("Action does not apply to any resource(s) in statement")
//lint:ignore ST1005 Reason: This error message is intended for end-user clarity and follows their expectations
errInvalidResource = errors.New("Policy has invalid resource")
//lint:ignore ST1005 Reason: This error message is intended for end-user clarity and follows their expectations
errInvalidPrincipal = errors.New("Invalid principal in policy")
//lint:ignore ST1005 Reason: This error message is intended for end-user clarity and follows their expectations
errInvalidAction = errors.New("Policy has invalid action")
)
type BucketPolicy struct {
Statement []BucketPolicyItem `json:"Statement"`
}
@@ -75,11 +85,14 @@ func (bpi *BucketPolicyItem) Validate(bucket string, iam IAMService) error {
for action := range bpi.Actions {
isObjectAction := action.IsObjectAction()
if isObjectAction && !containsObjectAction {
return fmt.Errorf("unsupported object action '%v' on the specified resources", action)
if isObjectAction == nil {
break
}
if !isObjectAction && !containsBucketAction {
return fmt.Errorf("unsupported bucket action '%v' on the specified resources", action)
if *isObjectAction && !containsObjectAction {
return errResourceMismatch
}
if !*isObjectAction && !containsBucketAction {
return errResourceMismatch
}
}
@@ -108,6 +121,11 @@ func ValidatePolicyDocument(policyBin []byte, bucket string, iam IAMService) err
return getMalformedPolicyError(err)
}
if len(policy.Statement) == 0 {
//lint:ignore ST1005 Reason: This error message is intended for end-user clarity and follows their expectations
return getMalformedPolicyError(errors.New("Could not parse the policy: Statement is empty!"))
}
if err := policy.Validate(bucket, iam); err != nil {
return getMalformedPolicyError(err)
}

27
auth/bucket_policy_actions.go
View File

@@ -16,7 +16,6 @@ package auth
import (
"encoding/json"
"fmt"
"strings"
)
@@ -119,7 +118,7 @@ var supportedObjectActionList = map[Action]struct{}{
// Validates Action: it should either wildcard match with supported actions list or be in it
func (a Action) IsValid() error {
if !strings.HasPrefix(string(a), "s3:") {
return fmt.Errorf("invalid action: %v", a)
return errInvalidAction
}
if a == AllActions {
@@ -134,31 +133,39 @@ func (a Action) IsValid() error {
}
}
return fmt.Errorf("invalid wildcard usage: %v prefix is not in the supported actions list", pattern)
return errInvalidAction
}
_, found := supportedActionList[a]
if !found {
return fmt.Errorf("unsupported action: %v", a)
return errInvalidAction
}
return nil
}
func getBoolPtr(bl bool) *bool {
return &bl
}
// Checks if the action is object action
func (a Action) IsObjectAction() bool {
// 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 true
return getBoolPtr(true)
}
}
return false
return getBoolPtr(false)
}
_, found := supportedObjectActionList[a]
return found
return &found
}
func (a Action) WildCardMatch(act Action) bool {
@@ -177,7 +184,7 @@ func (a *Actions) UnmarshalJSON(data []byte) error {
var err error
if err = json.Unmarshal(data, &ss); err == nil {
if len(ss) == 0 {
return fmt.Errorf("actions can't be empty")
return errInvalidAction
}
*a = make(Actions)
for _, s := range ss {
@@ -190,7 +197,7 @@ func (a *Actions) UnmarshalJSON(data []byte) error {
var s string
if err = json.Unmarshal(data, &s); err == nil {
if s == "" {
return fmt.Errorf("actions can't be empty")
return errInvalidAction
}
*a = make(Actions)
err = a.Add(s)

3
auth/bucket_policy_effect.go
View File

@@ -30,5 +30,6 @@ func (bpat BucketPolicyAccessType) Validate() error {
return nil
}
return fmt.Errorf("invalid effect: %v", bpat)
//lint:ignore ST1005 Reason: This error message is intended for end-user clarity and follows their expectations
return fmt.Errorf("Invalid effect: %v", bpat)
}

13
auth/bucket_policy_principals.go
View File

@@ -16,7 +16,6 @@ package auth
import (
"encoding/json"
"fmt"
)
type Principals map[string]struct{}
@@ -37,7 +36,7 @@ func (p *Principals) UnmarshalJSON(data []byte) error {
if err = json.Unmarshal(data, &ss); err == nil {
if len(ss) == 0 {
return fmt.Errorf("principals can't be empty")
return errInvalidPrincipal
}
*p = make(Principals)
for _, s := range ss {
@@ -46,7 +45,7 @@ func (p *Principals) UnmarshalJSON(data []byte) error {
return nil
} else if err = json.Unmarshal(data, &s); err == nil {
if s == "" {
return fmt.Errorf("principals can't be empty")
return errInvalidPrincipal
}
*p = make(Principals)
p.Add(s)
@@ -54,7 +53,7 @@ func (p *Principals) UnmarshalJSON(data []byte) error {
return nil
} else if err = json.Unmarshal(data, &k); err == nil {
if k.AWS == "" {
return fmt.Errorf("principals can't be empty")
return errInvalidPrincipal
}
*p = make(Principals)
p.Add(k.AWS)
@@ -66,7 +65,7 @@ func (p *Principals) UnmarshalJSON(data []byte) error {
}
if err = json.Unmarshal(data, &sk); err == nil {
if len(sk.AWS) == 0 {
return fmt.Errorf("principals can't be empty")
return errInvalidPrincipal
}
*p = make(Principals)
for _, s := range sk.AWS {
@@ -98,7 +97,7 @@ func (p Principals) Validate(iam IAMService) error {
if len(p) == 1 {
return nil
}
return fmt.Errorf("principals should either contain * or user access keys")
return errInvalidPrincipal
}
accs, err := CheckIfAccountsExist(p.ToSlice(), iam)
@@ -106,7 +105,7 @@ func (p Principals) Validate(iam IAMService) error {
return err
}
if len(accs) > 0 {
return fmt.Errorf("user accounts don't exist: %v", accs)
return errInvalidPrincipal
}
return nil

14
auth/bucket_policy_resources.go
View File

@@ -16,7 +16,6 @@ package auth
import (
"encoding/json"
"fmt"
"strings"
)
@@ -30,7 +29,7 @@ func (r *Resources) UnmarshalJSON(data []byte) error {
var err error
if err = json.Unmarshal(data, &ss); err == nil {
if len(ss) == 0 {
return fmt.Errorf("resources can't be empty")
return errInvalidResource
}
*r = make(Resources)
for _, s := range ss {
@@ -43,7 +42,7 @@ func (r *Resources) UnmarshalJSON(data []byte) error {
var s string
if err = json.Unmarshal(data, &s); err == nil {
if s == "" {
return fmt.Errorf("resources can't be empty")
return errInvalidResource
}
*r = make(Resources)
err = r.Add(s)
@@ -60,12 +59,7 @@ func (r *Resources) UnmarshalJSON(data []byte) error {
func (r Resources) Add(rc string) error {
ok, pattern := isValidResource(rc)
if !ok {
return fmt.Errorf("invalid resource: %v", rc)
}
_, found := r[pattern]
if found {
return fmt.Errorf("duplicate resource: %v", rc)
return errInvalidResource
}
r[pattern] = struct{}{}
@@ -99,7 +93,7 @@ func (r Resources) ContainsBucketPattern() bool {
func (r Resources) Validate(bucket string) error {
for resource := range r {
if !strings.HasPrefix(resource, bucket) {
return fmt.Errorf("incorrect bucket name in %v", resource)
return errInvalidResource
}
}

2
tests/integration/group-tests.go
View File

@@ -286,6 +286,7 @@ func TestGetBucketAcl(s *S3Conf) {
func TestPutBucketPolicy(s *S3Conf) {
PutBucketPolicy_non_existing_bucket(s)
PutBucketPolicy_empty_statement(s)
PutBucketPolicy_invalid_effect(s)
PutBucketPolicy_empty_actions_string(s)
PutBucketPolicy_empty_actions_array(s)
@@ -638,6 +639,7 @@ func GetIntTests() IntTests {
"GetBucketAcl_access_denied": GetBucketAcl_access_denied,
"GetBucketAcl_success": GetBucketAcl_success,
"PutBucketPolicy_non_existing_bucket": PutBucketPolicy_non_existing_bucket,
"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,

72
tests/integration/tests.go
View File

@@ -6320,6 +6320,28 @@ func PutBucketPolicy_non_existing_bucket(s *S3Conf) error {
})
}
func PutBucketPolicy_empty_statement(s *S3Conf) error {
testName := "PutBucketPolicy_empty_statement"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
doc := `
{
"Statement": []
}
`
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err := s3client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{
Bucket: &bucket,
Policy: &doc,
})
cancel()
if err := checkApiErr(err, getMalformedPolicyError("Could not parse the policy: Statement is empty!")); err != nil {
return err
}
return nil
})
}
func PutBucketPolicy_invalid_effect(s *S3Conf) error {
testName := "PutBucketPolicy_invalid_effect"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
@@ -6332,7 +6354,7 @@ func PutBucketPolicy_invalid_effect(s *S3Conf) error {
})
cancel()
if err := checkApiErr(err, getMalformedPolicyError("invalid effect: invalid_effect")); err != nil {
if err := checkApiErr(err, getMalformedPolicyError("Invalid effect: invalid_effect")); err != nil {
return err
}
return nil
@@ -6351,7 +6373,7 @@ func PutBucketPolicy_empty_actions_string(s *S3Conf) error {
})
cancel()
if err := checkApiErr(err, getMalformedPolicyError("actions can't be empty")); err != nil {
if err := checkApiErr(err, getMalformedPolicyError("Policy has invalid action")); err != nil {
return err
}
return nil
@@ -6370,7 +6392,7 @@ func PutBucketPolicy_empty_actions_array(s *S3Conf) error {
})
cancel()
if err := checkApiErr(err, getMalformedPolicyError("actions can't be empty")); err != nil {
if err := checkApiErr(err, getMalformedPolicyError("Policy has invalid action")); err != nil {
return err
}
return nil
@@ -6389,7 +6411,7 @@ func PutBucketPolicy_invalid_action(s *S3Conf) error {
})
cancel()
if err := checkApiErr(err, getMalformedPolicyError("invalid action: ListObjects")); err != nil {
if err := checkApiErr(err, getMalformedPolicyError("Policy has invalid action")); err != nil {
return err
}
return nil
@@ -6408,7 +6430,7 @@ func PutBucketPolicy_unsupported_action(s *S3Conf) error {
})
cancel()
if err := checkApiErr(err, getMalformedPolicyError("unsupported action: s3:PutLifecycleConfiguration")); err != nil {
if err := checkApiErr(err, getMalformedPolicyError("Policy has invalid action")); err != nil {
return err
}
return nil
@@ -6427,7 +6449,7 @@ func PutBucketPolicy_incorrect_action_wildcard_usage(s *S3Conf) error {
})
cancel()
if err := checkApiErr(err, getMalformedPolicyError("invalid wildcard usage: s3:hello prefix is not in the supported actions list")); err != nil {
if err := checkApiErr(err, getMalformedPolicyError("Policy has invalid action")); err != nil {
return err
}
return nil
@@ -6446,7 +6468,7 @@ func PutBucketPolicy_empty_principals_string(s *S3Conf) error {
})
cancel()
if err := checkApiErr(err, getMalformedPolicyError("principals can't be empty")); err != nil {
if err := checkApiErr(err, getMalformedPolicyError("Invalid principal in policy")); err != nil {
return err
}
return nil
@@ -6465,7 +6487,7 @@ func PutBucketPolicy_empty_principals_array(s *S3Conf) error {
})
cancel()
if err := checkApiErr(err, getMalformedPolicyError("principals can't be empty")); err != nil {
if err := checkApiErr(err, getMalformedPolicyError("Invalid principal in policy")); err != nil {
return err
}
return nil
@@ -6484,7 +6506,7 @@ func PutBucketPolicy_principals_aws_struct_empty_string(s *S3Conf) error {
})
cancel()
if err := checkApiErr(err, getMalformedPolicyError("principals can't be empty")); err != nil {
if err := checkApiErr(err, getMalformedPolicyError("Invalid principal in policy")); err != nil {
return err
}
return nil
@@ -6503,7 +6525,7 @@ func PutBucketPolicy_principals_aws_struct_empty_string_slice(s *S3Conf) error {
})
cancel()
if err := checkApiErr(err, getMalformedPolicyError("principals can't be empty")); err != nil {
if err := checkApiErr(err, getMalformedPolicyError("Invalid principal in policy")); err != nil {
return err
}
return nil
@@ -6522,7 +6544,7 @@ func PutBucketPolicy_principals_incorrect_wildcard_usage(s *S3Conf) error {
})
cancel()
if err := checkApiErr(err, getMalformedPolicyError("principals should either contain * or user access keys")); err != nil {
if err := checkApiErr(err, getMalformedPolicyError("Invalid principal in policy")); err != nil {
return err
}
return nil
@@ -6541,14 +6563,8 @@ func PutBucketPolicy_non_existing_principals(s *S3Conf) error {
})
cancel()
apiErr1 := getMalformedPolicyError(fmt.Sprintf("user accounts don't exist: %v", []string{"a_rarely_existing_user_account_1", "a_rarely_existing_user_account_2"}))
apiErr2 := getMalformedPolicyError(fmt.Sprintf("user accounts don't exist: %v", []string{"a_rarely_existing_user_account_2", "a_rarely_existing_user_account_1"}))
err1 := checkApiErr(err, apiErr1)
err2 := checkApiErr(err, apiErr2)
if err1 != nil && err2 != nil {
return err1
if err := checkApiErr(err, getMalformedPolicyError("Invalid principal in policy")); err != nil {
return err
}
return nil
@@ -6567,7 +6583,7 @@ func PutBucketPolicy_empty_resources_string(s *S3Conf) error {
})
cancel()
if err := checkApiErr(err, getMalformedPolicyError("resources can't be empty")); err != nil {
if err := checkApiErr(err, getMalformedPolicyError("Policy has invalid resource")); err != nil {
return err
}
return nil
@@ -6586,7 +6602,7 @@ func PutBucketPolicy_empty_resources_array(s *S3Conf) error {
})
cancel()
if err := checkApiErr(err, getMalformedPolicyError("resources can't be empty")); err != nil {
if err := checkApiErr(err, getMalformedPolicyError("Policy has invalid resource")); err != nil {
return err
}
return nil
@@ -6606,7 +6622,7 @@ func PutBucketPolicy_invalid_resource_prefix(s *S3Conf) error {
})
cancel()
if err := checkApiErr(err, getMalformedPolicyError(fmt.Sprintf("invalid resource: %v", resource[1:len(resource)-1]))); err != nil {
if err := checkApiErr(err, getMalformedPolicyError("Policy has invalid resource")); err != nil {
return err
}
return nil
@@ -6626,7 +6642,7 @@ func PutBucketPolicy_invalid_resource_with_starting_slash(s *S3Conf) error {
})
cancel()
if err := checkApiErr(err, getMalformedPolicyError(fmt.Sprintf("invalid resource: %v", resource[1:len(resource)-1]))); err != nil {
if err := checkApiErr(err, getMalformedPolicyError("Policy has invalid resource")); err != nil {
return err
}
return nil
@@ -6645,10 +6661,10 @@ func PutBucketPolicy_duplicate_resource(s *S3Conf) error {
Policy: &doc,
})
cancel()
if err := checkApiErr(err, getMalformedPolicyError(fmt.Sprintf("duplicate resource: %v", resource[1:len(resource)-1]))); err != nil {
if err != nil {
return err
}
return nil
})
}
@@ -6666,7 +6682,7 @@ func PutBucketPolicy_incorrect_bucket_name(s *S3Conf) error {
})
cancel()
if err := checkApiErr(err, getMalformedPolicyError(fmt.Sprintf("incorrect bucket name in prefix-%v", bucket))); err != nil {
if err := checkApiErr(err, getMalformedPolicyError("Policy has invalid resource")); err != nil {
return err
}
return nil
@@ -6686,7 +6702,7 @@ func PutBucketPolicy_object_action_on_bucket_resource(s *S3Conf) error {
})
cancel()
if err := checkApiErr(err, getMalformedPolicyError("unsupported object action 's3:PutObjectTagging' on the specified resources")); err != nil {
if err := checkApiErr(err, getMalformedPolicyError("Action does not apply to any resource(s) in statement")); err != nil {
return err
}
return nil
@@ -6706,7 +6722,7 @@ func PutBucketPolicy_bucket_action_on_object_resource(s *S3Conf) error {
})
cancel()
if err := checkApiErr(err, getMalformedPolicyError("unsupported bucket action 's3:DeleteBucket' on the specified resources")); err != nil {
if err := checkApiErr(err, getMalformedPolicyError("Action does not apply to any resource(s) in statement")); err != nil {
return err
}
return nil