Update ACL structures to have flexible types

api/handler/acl.go

@@ -1,6 +1,7 @@
 package handler
 
 import (
+	"bytes"
 	"context"
 	"crypto/ecdsa"
 	"crypto/elliptic"
@@ -56,6 +57,8 @@ var (
 	errInvalidPublicKey = errors.New("invalid public key")
 )
 
+var rawWildcardJSON = []byte(`"*"`)
+
 const (
 	arnAwsPrefix     = "arn:aws:s3:::"
 	allUsersWildcard = "*"
@@ -102,18 +105,41 @@ type bucketPolicy struct {
 }
 
 type statement struct {
-	Sid       string    `json:"Sid"`
-	Effect    string    `json:"Effect"`
-	Principal principal `json:"Principal"`
-	Action    []string  `json:"Action"`
-	Resource  []string  `json:"Resource"`
+	Sid       string        `json:"Sid"`
+	Effect    string        `json:"Effect"`
+	Principal principal     `json:"Principal"`
+	Action    stringOrSlice `json:"Action"`
+	Resource  stringOrSlice `json:"Resource"`
 }
 
 type principal struct {
 	AWS           string `json:"AWS,omitempty"`
 	CanonicalUser string `json:"CanonicalUser,omitempty"`
 }
 
+// principalModel is a copy of principal to avoid infinity recursion in principal.UnmarshalJSON.
+type principalModel struct {
+	AWS           string `json:"AWS,omitempty"`
+	CanonicalUser string `json:"CanonicalUser,omitempty"`
+}
+
+func (s *principal) UnmarshalJSON(bts []byte) error {
+	if len(bts) == 3 && bytes.Equal(bts, rawWildcardJSON) {
+		s.AWS = allUsersWildcard
+		return nil
+	}
+
+	var pm principalModel
+	if err := json.Unmarshal(bts, &pm); err != nil {
+		return err
+	}
+
+	s.AWS = pm.AWS
+	s.CanonicalUser = pm.CanonicalUser
+
+	return nil
+}
+
 type orderedAstResource struct {
 	Index    int
 	Resource *astResource
@@ -1009,8 +1035,7 @@ func policyToAst(bktPolicy *bucketPolicy) (*ast, error) {
 		if state.Principal.AWS == allUsersWildcard {
 			groupGrantee = true
 		}
-
-		for _, resource := range state.Resource {
+		for _, resource := range state.Resource.values {
 			trimmedResource := strings.TrimPrefix(resource, arnAwsPrefix)
 			r, ok := rr[trimmedResource]
 			if !ok {
@@ -1022,7 +1047,7 @@ func policyToAst(bktPolicy *bucketPolicy) (*ast, error) {
 					resourceInfo: resourceInfoFromName(trimmedResource, bktPolicy.Bucket),
 				}
 			}
-			for _, action := range state.Action {
+			for _, action := range state.Action.values {
 				for _, op := range actionToOpMap[action] {
 					toAction := effectToAction(state.Effect)
 					r.Operations = addTo(r.Operations, state.Principal.CanonicalUser, op, groupGrantee, toAction)
@@ -1103,8 +1128,8 @@ func handleResourceOperations(bktPolicy *bucketPolicy, list []*astOperation, eac
 			state := statement{
 				Effect:    actionToEffect(eaclAction),
 				Principal: principal{CanonicalUser: user},
-				Action:    actions,
-				Resource:  []string{arnAwsPrefix + resourceName},
+				Action:    stringOrSlice{values: actions},
+				Resource:  stringOrSlice{values: []string{arnAwsPrefix + resourceName}},
 			}
 			if user == allUsersGroup {
 				state.Principal = principal{AWS: allUsersWildcard}
@@ -1247,8 +1272,8 @@ func getAllowStatement(resInfo *resourceInfo, id string, permission amazonS3Perm
 		Principal: principal{
 			CanonicalUser: id,
 		},
-		Action:   getActions(permission, resInfo.IsBucket()),
-		Resource: []string{arnAwsPrefix + resInfo.Name()},
+		Action:   stringOrSlice{values: getActions(permission, resInfo.IsBucket())},
+		Resource: stringOrSlice{values: []string{arnAwsPrefix + resInfo.Name()}},
 	}
 
 	if id == allUsersWildcard {
@@ -1264,8 +1289,8 @@ func getDenyStatement(resInfo *resourceInfo, id string, permission amazonS3Permi
 		Principal: principal{
 			CanonicalUser: id,
 		},
-		Action:   getActions(permission, resInfo.IsBucket()),
-		Resource: []string{arnAwsPrefix + resInfo.Name()},
+		Action:   stringOrSlice{values: getActions(permission, resInfo.IsBucket())},
+		Resource: stringOrSlice{values: []string{arnAwsPrefix + resInfo.Name()}},
 	}
 
 	if id == allUsersWildcard {

api/handler/acl_test.go

@@ -103,16 +103,16 @@ func TestPolicyToAst(t *testing.T) {
 			{
 				Effect:    "Allow",
 				Principal: principal{AWS: allUsersWildcard},
-				Action:    []string{"s3:PutObject"},
-				Resource:  []string{"arn:aws:s3:::bucketName"},
+				Action:    stringOrSlice{values: []string{"s3:PutObject"}},
+				Resource:  stringOrSlice{values: []string{"arn:aws:s3:::bucketName"}},
 			},
 			{
 				Effect: "Deny",
 				Principal: principal{
 					CanonicalUser: hex.EncodeToString(key.PublicKey().Bytes()),
 				},
-				Action:   []string{"s3:GetObject"},
-				Resource: []string{"arn:aws:s3:::bucketName/object"},
+				Action:   stringOrSlice{values: []string{"s3:GetObject"}},
+				Resource: stringOrSlice{values: []string{"arn:aws:s3:::bucketName/object"}},
 			}},
 	}
 	policy.Bucket = "bucketName"
@@ -746,22 +746,22 @@ func TestBucketAclToPolicy(t *testing.T) {
 				Principal: principal{
 					CanonicalUser: id,
 				},
-				Action:   []string{"s3:ListBucket", "s3:ListBucketVersions", "s3:ListBucketMultipartUploads", "s3:PutObject", "s3:DeleteObject"},
-				Resource: []string{arnAwsPrefix + resInfo.Name()},
+				Action:   stringOrSlice{values: []string{"s3:ListBucket", "s3:ListBucketVersions", "s3:ListBucketMultipartUploads", "s3:PutObject", "s3:DeleteObject"}},
+				Resource: stringOrSlice{values: []string{arnAwsPrefix + resInfo.Name()}},
 			},
 			{
 				Effect:    "Allow",
 				Principal: principal{AWS: allUsersWildcard},
-				Action:    []string{"s3:ListBucket", "s3:ListBucketVersions", "s3:ListBucketMultipartUploads"},
-				Resource:  []string{arnAwsPrefix + resInfo.Name()},
+				Action:    stringOrSlice{values: []string{"s3:ListBucket", "s3:ListBucketVersions", "s3:ListBucketMultipartUploads"}},
+				Resource:  stringOrSlice{values: []string{arnAwsPrefix + resInfo.Name()}},
 			},
 			{
 				Effect: "Allow",
 				Principal: principal{
 					CanonicalUser: id2,
 				},
-				Action:   []string{"s3:PutObject", "s3:DeleteObject"},
-				Resource: []string{arnAwsPrefix + resInfo.Name()},
+				Action:   stringOrSlice{values: []string{"s3:PutObject", "s3:DeleteObject"}},
+				Resource: stringOrSlice{values: []string{arnAwsPrefix + resInfo.Name()}},
 			},
 		},
 	}
@@ -819,22 +819,22 @@ func TestObjectAclToPolicy(t *testing.T) {
 				Principal: principal{
 					CanonicalUser: id,
 				},
-				Action:   []string{s3GetObject, s3GetObjectVersion, s3PutObject, s3DeleteObject},
-				Resource: []string{arnAwsPrefix + resInfo.Name()},
+				Action:   stringOrSlice{values: []string{s3GetObject, s3GetObjectVersion, s3PutObject, s3DeleteObject}},
+				Resource: stringOrSlice{values: []string{arnAwsPrefix + resInfo.Name()}},
 			},
 			{
 				Effect: "Allow",
 				Principal: principal{
 					CanonicalUser: id2,
 				},
-				Action:   []string{s3GetObject, s3GetObjectVersion, s3PutObject, s3DeleteObject},
-				Resource: []string{arnAwsPrefix + resInfo.Name()},
+				Action:   stringOrSlice{values: []string{s3GetObject, s3GetObjectVersion, s3PutObject, s3DeleteObject}},
+				Resource: stringOrSlice{values: []string{arnAwsPrefix + resInfo.Name()}},
 			},
 			{
 				Effect:    "Allow",
 				Principal: principal{AWS: allUsersWildcard},
-				Action:    []string{s3GetObject, s3GetObjectVersion},
-				Resource:  []string{arnAwsPrefix + resInfo.Name()},
+				Action:    stringOrSlice{values: []string{s3GetObject, s3GetObjectVersion}},
+				Resource:  stringOrSlice{values: []string{arnAwsPrefix + resInfo.Name()}},
 			},
 		},
 	}
@@ -1343,33 +1343,33 @@ func TestBucketPolicy(t *testing.T) {
 	for _, st := range bktPolicy.Statement {
 		if st.Effect == "Allow" {
 			require.Equal(t, hex.EncodeToString(key.PublicKey().Bytes()), st.Principal.CanonicalUser)
-			require.Equal(t, []string{arnAwsPrefix + bktName}, st.Resource)
+			require.Equal(t, []string{arnAwsPrefix + bktName}, st.Resource.values)
 		} else {
 			require.Equal(t, allUsersWildcard, st.Principal.AWS)
 			require.Equal(t, "Deny", st.Effect)
-			require.Equal(t, []string{arnAwsPrefix + bktName}, st.Resource)
+			require.Equal(t, []string{arnAwsPrefix + bktName}, st.Resource.values)
 		}
 	}
 
 	newPolicy := &bucketPolicy{
 		Statement: []statement{{
 			Effect:    "Allow",
 			Principal: principal{AWS: allUsersWildcard},
-			Action:    []string{s3GetObject},
-			Resource:  []string{arnAwsPrefix + "dummy"},
+			Action:    stringOrSlice{values: []string{s3GetObject}},
+			Resource:  stringOrSlice{values: []string{arnAwsPrefix + "dummy"}},
 		}},
 	}
 
 	putBucketPolicy(hc, bktName, newPolicy, box, http.StatusInternalServerError)
 
-	newPolicy.Statement[0].Resource[0] = arnAwsPrefix + bktName
+	newPolicy.Statement[0].Resource.values[0] = arnAwsPrefix + bktName
 	putBucketPolicy(hc, bktName, newPolicy, box, http.StatusOK)
 
 	bktPolicy = getBucketPolicy(hc, bktName)
 	for _, st := range bktPolicy.Statement {
 		if st.Effect == "Allow" && st.Principal.AWS == allUsersWildcard {
-			require.Equal(t, []string{arnAwsPrefix + bktName}, st.Resource)
-			require.ElementsMatch(t, []string{s3GetObject, s3ListBucket}, st.Action)
+			require.Equal(t, []string{arnAwsPrefix + bktName}, st.Resource.values)
+			require.ElementsMatch(t, []string{s3GetObject, s3ListBucket}, st.Action.values)
 		}
 	}
 }
@@ -1546,3 +1546,36 @@ func TestEACLEncode(t *testing.T) {
 		require.Contains(t, acp.AccessControlList, g)
 	}
 }
+
+func TestPrincipal(t *testing.T) {
+	type testCase struct {
+		Principal principal `json:"p"`
+	}
+
+	t.Run("wildcard", func(t *testing.T) {
+		payload := []byte(`{"p":"*"}`)
+
+		var testObj testCase
+		require.NoError(t, json.Unmarshal(payload, &testObj))
+		require.Equal(t, allUsersWildcard, testObj.Principal.AWS)
+
+		bts, err := json.Marshal(testObj)
+		require.NoError(t, err)
+
+		// we should be able to unmarshal string and structs, but marshals always a struct.
+		payload = []byte(`{"p":{"AWS":"*"}}`)
+		require.Equal(t, payload, bts)
+	})
+
+	t.Run("wildcard in struct", func(t *testing.T) {
+		payload := []byte(`{"p":{"AWS":"*"}}`)
+
+		var testObj testCase
+		require.NoError(t, json.Unmarshal(payload, &testObj))
+		require.Equal(t, allUsersWildcard, testObj.Principal.AWS)
+
+		bts, err := json.Marshal(testObj)
+		require.NoError(t, err)
+		require.Equal(t, payload, bts)
+	})
+}

api/handler/json_types.go

@@ -0,0 +1,39 @@
+package handler
+
+import (
+	"encoding/json"
+)
+
+type (
+	stringOrSlice struct {
+		values []string
+	}
+)
+
+func (s *stringOrSlice) UnmarshalJSON(bytes []byte) error {
+	if err := json.Unmarshal(bytes, &s.values); err == nil {
+		return nil
+	}
+
+	var str string
+	if err := json.Unmarshal(bytes, &str); err != nil {
+		return err
+	}
+
+	s.values = []string{str}
+	return nil
+}
+
+func (s stringOrSlice) MarshalJSON() ([]byte, error) {
+	values := s.values
+	if values == nil {
+		values = []string{}
+	}
+
+	if len(s.values) == 1 {
+		str := s.values[0]
+		return json.Marshal(&str)
+	}
+
+	return json.Marshal(values)
+}