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

@@ -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.

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: <ERROR> %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.