From ef3c807b4aef9bc2197d2625b8893fd9d8335d89 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Mon, 19 Sep 2016 13:52:28 -0700 Subject: [PATCH] policies: Parser should handle Principals with various forms. (#2733) Handles cases for these three combinations - "Principal": "*", - "Principal": { "AWS" : "*" } - "Principal": { "AWS" : [ "*" ]} Fixes #2732 --- cmd/bucket-policy-parser.go | 55 ++++++++++++++++++++++++++------ cmd/bucket-policy-parser_test.go | 32 ++++++++++++++----- 2 files changed, 70 insertions(+), 17 deletions(-) diff --git a/cmd/bucket-policy-parser.go b/cmd/bucket-policy-parser.go index 7f7c10c4d..51aaf9e96 100644 --- a/cmd/bucket-policy-parser.go +++ b/cmd/bucket-policy-parser.go @@ -50,18 +50,12 @@ var supportedConditionsKey = set.CreateStringSet("s3:prefix", "s3:max-keys") // supportedEffectMap - supported effects. var supportedEffectMap = set.CreateStringSet("Allow", "Deny") -// policyUser - canonical users list. -type policyUser struct { - AWS set.StringSet `json:"AWS,omitempty"` - CanonicalUser set.StringSet `json:"CanonicalUser,omitempty"` -} - // Statement - minio policy statement type policyStatement struct { Actions set.StringSet `json:"Action"` Conditions map[string]map[string]set.StringSet `json:"Condition,omitempty"` Effect string - Principal policyUser `json:"Principal"` + Principal interface{} `json:"Principal"` Resources set.StringSet `json:"Resource"` Sid string } @@ -131,8 +125,51 @@ func isValidResources(resources set.StringSet) (err error) { return nil } +// Parse principals parses a incoming json. Handles cases for +// these three combinations. +// - "Principal": "*", +// - "Principal": { "AWS" : "*" } +// - "Principal": { "AWS" : [ "*" ]} +func parsePrincipals(principal interface{}) set.StringSet { + principals, ok := principal.(map[string]interface{}) + if !ok { + var principalStr string + principalStr, ok = principal.(string) + if ok { + return set.CreateStringSet(principalStr) + } + } // else { + var principalStrs []string + for _, p := range principals { + principalStr, isStr := p.(string) + if !isStr { + principalsAdd, isInterface := p.([]interface{}) + if !isInterface { + principalStrsAddr, isStrs := p.([]string) + if !isStrs { + continue + } + principalStrs = append(principalStrs, principalStrsAddr...) + } else { + for _, pa := range principalsAdd { + var pstr string + pstr, isStr = pa.(string) + if !isStr { + continue + } + principalStrs = append(principalStrs, pstr) + } + } + continue + } // else { + principalStrs = append(principalStrs, principalStr) + } + return set.CreateStringSet(principalStrs...) +} + // isValidPrincipals - are valid principals. -func isValidPrincipals(principals set.StringSet) (err error) { +func isValidPrincipals(principal interface{}) (err error) { + principals := parsePrincipals(principal) // Statement principal should have a value. if len(principals) == 0 { err = errors.New("Principal cannot be empty.") @@ -274,7 +311,7 @@ func parseBucketPolicy(bucketPolicyReader io.Reader, policy *bucketPolicy) (err return err } // Statement principal should be supported format. - if err := isValidPrincipals(statement.Principal.AWS); err != nil { + if err := isValidPrincipals(statement.Principal); err != nil { return err } // Statement actions should be valid. diff --git a/cmd/bucket-policy-parser_test.go b/cmd/bucket-policy-parser_test.go index 9427e970e..ce25e15e6 100644 --- a/cmd/bucket-policy-parser_test.go +++ b/cmd/bucket-policy-parser_test.go @@ -76,7 +76,9 @@ var ( func getReadWriteObjectStatement(bucketName, objectPrefix string) policyStatement { objectResourceStatement := policyStatement{} objectResourceStatement.Effect = "Allow" - objectResourceStatement.Principal.AWS = set.CreateStringSet([]string{"*"}...) + objectResourceStatement.Principal = map[string]interface{}{ + "AWS": "*", + } objectResourceStatement.Resources = set.CreateStringSet([]string{fmt.Sprintf("%s%s", AWSResourcePrefix, bucketName+"/"+objectPrefix+"*")}...) objectResourceStatement.Actions = set.CreateStringSet(readWriteObjectActions...) return objectResourceStatement @@ -86,7 +88,9 @@ func getReadWriteObjectStatement(bucketName, objectPrefix string) policyStatemen func getReadWriteBucketStatement(bucketName, objectPrefix string) policyStatement { bucketResourceStatement := policyStatement{} bucketResourceStatement.Effect = "Allow" - bucketResourceStatement.Principal.AWS = set.CreateStringSet([]string{"*"}...) + bucketResourceStatement.Principal = map[string]interface{}{ + "AWS": "*", + } bucketResourceStatement.Resources = set.CreateStringSet([]string{fmt.Sprintf("%s%s", AWSResourcePrefix, bucketName)}...) bucketResourceStatement.Actions = set.CreateStringSet(readWriteBucketActions...) return bucketResourceStatement @@ -104,7 +108,9 @@ func getReadWriteStatement(bucketName, objectPrefix string) []policyStatement { func getReadOnlyBucketStatement(bucketName, objectPrefix string) policyStatement { bucketResourceStatement := policyStatement{} bucketResourceStatement.Effect = "Allow" - bucketResourceStatement.Principal.AWS = set.CreateStringSet([]string{"*"}...) + bucketResourceStatement.Principal = map[string]interface{}{ + "AWS": "*", + } bucketResourceStatement.Resources = set.CreateStringSet([]string{fmt.Sprintf("%s%s", AWSResourcePrefix, bucketName)}...) bucketResourceStatement.Actions = set.CreateStringSet(readOnlyBucketActions...) return bucketResourceStatement @@ -114,7 +120,9 @@ func getReadOnlyBucketStatement(bucketName, objectPrefix string) policyStatement func getReadOnlyObjectStatement(bucketName, objectPrefix string) policyStatement { objectResourceStatement := policyStatement{} objectResourceStatement.Effect = "Allow" - objectResourceStatement.Principal.AWS = set.CreateStringSet([]string{"*"}...) + objectResourceStatement.Principal = map[string]interface{}{ + "AWS": "*", + } objectResourceStatement.Resources = set.CreateStringSet([]string{fmt.Sprintf("%s%s", AWSResourcePrefix, bucketName+"/"+objectPrefix+"*")}...) objectResourceStatement.Actions = set.CreateStringSet(readOnlyObjectActions...) return objectResourceStatement @@ -133,7 +141,9 @@ func getWriteOnlyBucketStatement(bucketName, objectPrefix string) policyStatemen bucketResourceStatement := policyStatement{} bucketResourceStatement.Effect = "Allow" - bucketResourceStatement.Principal.AWS = set.CreateStringSet([]string{"*"}...) + bucketResourceStatement.Principal = map[string]interface{}{ + "AWS": "*", + } bucketResourceStatement.Resources = set.CreateStringSet([]string{fmt.Sprintf("%s%s", AWSResourcePrefix, bucketName)}...) bucketResourceStatement.Actions = set.CreateStringSet(writeOnlyBucketActions...) return bucketResourceStatement @@ -143,7 +153,9 @@ func getWriteOnlyBucketStatement(bucketName, objectPrefix string) policyStatemen func getWriteOnlyObjectStatement(bucketName, objectPrefix string) policyStatement { objectResourceStatement := policyStatement{} objectResourceStatement.Effect = "Allow" - objectResourceStatement.Principal.AWS = set.CreateStringSet([]string{"*"}...) + objectResourceStatement.Principal = map[string]interface{}{ + "AWS": "*", + } objectResourceStatement.Resources = set.CreateStringSet([]string{fmt.Sprintf("%s%s", AWSResourcePrefix, bucketName+"/"+objectPrefix+"*")}...) objectResourceStatement.Actions = set.CreateStringSet(writeOnlyObjectActions...) return objectResourceStatement @@ -312,7 +324,9 @@ func TestIsValidPrincipals(t *testing.T) { {[]string{"*"}, nil, true}, } for i, testCase := range testCases { - err := isValidPrincipals(set.CreateStringSet(testCase.principals...)) + err := isValidPrincipals(map[string]interface{}{ + "AWS": testCase.principals, + }) if err != nil && testCase.shouldPass { t.Errorf("Test %d: Expected to pass, but failed with: %s", i+1, err.Error()) } @@ -587,7 +601,9 @@ func TestParseBucketPolicy(t *testing.T) { // set unsupported principals. setUnsupportedPrincipals := func(statements []policyStatement) []policyStatement { // "User1111"" is an Unsupported Principal. - statements[0].Principal.AWS = set.CreateStringSet([]string{"*", "User1111"}...) + statements[0].Principal = map[string]interface{}{ + "AWS": []string{"*", "User1111"}, + } return statements } // set unsupported Resources.