[#681] acl: Prepare owner ID before processing

Signed-off-by: Denis Kirillov <denis@nspcc.ru>

api/handler/acl.go

@@ -65,13 +65,10 @@ const (
 	aclRead        AWSACL = "READ"
 )
 
-// GranteeType is aws grantee permission type constants.
-type GranteeType string
-
 const (
-	acpCanonicalUser         GranteeType = "CanonicalUser"
+	acpCanonicalUser         = "CanonicalUser"
-	acpAmazonCustomerByEmail GranteeType = "AmazonCustomerByEmail"
+	acpAmazonCustomerByEmail = "AmazonCustomerByEmail"
-	acpGroup                 GranteeType = "Group"
+	acpGroup                 = "Group"
 )
 
 type bucketPolicy struct {
@@ -220,6 +217,9 @@ func (h *handler) PutBucketACLHandler(w http.ResponseWriter, r *http.Request) {
 	} else if err = xml.NewDecoder(r.Body).Decode(list); err != nil {
 		h.logAndSendError(w, "could not parse bucket acl", reqInfo, errors.GetAPIError(errors.ErrMalformedXML))
 		return
+	} else {
+		// workaround to decode owner ID
+		list.Owner.ID = hex.EncodeToString(key.Bytes())
 	}
 
 	resInfo := &resourceInfo{Bucket: reqInfo.BucketName}
@@ -355,6 +355,9 @@ func (h *handler) PutObjectACLHandler(w http.ResponseWriter, r *http.Request) {
 	} else if err = xml.NewDecoder(r.Body).Decode(list); err != nil {
 		h.logAndSendError(w, "could not parse bucket acl", reqInfo, errors.GetAPIError(errors.ErrMalformedXML))
 		return
+	} else {
+		// workaround to decode owner ID
+		list.Owner.ID = hex.EncodeToString(key.Bytes())
 	}
 
 	resInfo := &resourceInfo{
@@ -469,7 +472,7 @@ func parseACLHeaders(header http.Header, key *keys.PublicKey) (*AccessControlPol
 		Grantee: &Grantee{
 			ID:          hex.EncodeToString(key.Bytes()),
 			DisplayName: key.Address(),
-			Type:        acpCanonicalUser,
+			Type:        formGranteeType(acpCanonicalUser),
 		},
 		Permission: aclFullControl,
 	}}
@@ -509,7 +512,7 @@ func addGrantees(list []*Grant, headers http.Header, hdr string) ([]*Grant, erro
 	}
 
 	for _, grantee := range grantees {
-		if grantee.Type == acpAmazonCustomerByEmail || (grantee.Type == acpGroup && grantee.URI != allUsersGroup) {
+		if grantee.matchType(acpAmazonCustomerByEmail) || (grantee.matchType(acpGroup) && grantee.URI != allUsersGroup) {
 			return nil, stderrors.New("unsupported grantee type")
 		}
 
@@ -559,17 +562,17 @@ func formGrantee(granteeType, value string) (*Grantee, error) {
 	case "id":
 		return &Grantee{
 			ID:   value,
-			Type: acpCanonicalUser,
+			Type: formGranteeType(acpCanonicalUser),
 		}, nil
 	case "uri":
 		return &Grantee{
 			URI:  value,
-			Type: acpGroup,
+			Type: formGranteeType(acpGroup),
 		}, nil
 	case "emailAddress":
 		return &Grantee{
 			EmailAddress: value,
-			Type:         acpAmazonCustomerByEmail,
+			Type:         formGranteeType(acpAmazonCustomerByEmail),
 		}, nil
 	}
 	// do not return grantee type to avoid sensitive data logging (#489)
@@ -583,7 +586,7 @@ func addPredefinedACP(acp *AccessControlPolicy, cannedACL string) (*AccessContro
 		acp.AccessControlList = append(acp.AccessControlList, &Grant{
 			Grantee: &Grantee{
 				URI:  allUsersGroup,
-				Type: acpGroup,
+				Type: formGranteeType(acpGroup),
 			},
 			Permission: aclFullControl,
 		})
@@ -593,7 +596,7 @@ func addPredefinedACP(acp *AccessControlPolicy, cannedACL string) (*AccessContro
 		acp.AccessControlList = append(acp.AccessControlList, &Grant{
 			Grantee: &Grantee{
 				URI:  allUsersGroup,
-				Type: acpGroup,
+				Type: formGranteeType(acpGroup),
 			},
 			Permission: aclRead,
 		})
@@ -1173,12 +1176,12 @@ func aclToAst(acl *AccessControlPolicy, resInfo *resourceInfo) (*ast, error) {
 	}
 
 	for _, grant := range acl.AccessControlList {
-		if grant.Grantee.Type == acpAmazonCustomerByEmail || (grant.Grantee.Type == acpGroup && grant.Grantee.URI != allUsersGroup) {
+		if grant.Grantee.matchType(acpAmazonCustomerByEmail) || (grant.Grantee.matchType(acpGroup) && grant.Grantee.URI != allUsersGroup) {
 			return nil, stderrors.New("unsupported grantee type")
 		}
 
 		var groupGrantee bool
-		if grant.Grantee.Type == acpGroup {
+		if grant.Grantee.matchType(acpGroup) {
 			groupGrantee = true
 		} else if grant.Grantee.ID == acl.Owner.ID {
 			continue
@@ -1212,12 +1215,12 @@ func aclToPolicy(acl *AccessControlPolicy, resInfo *resourceInfo) (*bucketPolicy
 	}
 
 	for _, grant := range acl.AccessControlList {
-		if grant.Grantee.Type == acpAmazonCustomerByEmail || (grant.Grantee.Type == acpGroup && grant.Grantee.URI != allUsersGroup) {
+		if grant.Grantee.matchType(acpAmazonCustomerByEmail) || (grant.Grantee.matchType(acpGroup) && grant.Grantee.URI != allUsersGroup) {
 			return nil, stderrors.New("unsupported grantee type")
 		}
 
 		user := grant.Grantee.ID
-		if grant.Grantee.Type == acpGroup {
+		if grant.Grantee.matchType(acpGroup) {
 			user = allUsersWildcard
 		} else if user == acl.Owner.ID {
 			continue
@@ -1377,10 +1380,10 @@ func (h *handler) encodeObjectACL(bucketACL *layer.BucketACL, bucketName, object
 
 		var grantee *Grantee
 		if key == allUsersGroup {
-			grantee = NewGrantee(acpGroup)
+			grantee = NewGrantee(formGranteeType(acpGroup))
 			grantee.URI = allUsersGroup
 		} else {
-			grantee = NewGrantee(acpCanonicalUser)
+			grantee = NewGrantee(formGranteeType(acpCanonicalUser))
 			grantee.ID = key
 		}
 
@@ -1453,7 +1456,7 @@ func bucketACLToTable(acp *AccessControlPolicy, resInfo *resourceInfo) (*eacl.Ta
 }
 
 func getRecordFunction(grantee *Grantee) (getRecordFunc, error) {
-	switch grantee.Type {
+	switch grantee.TypeString() {
 	case acpAmazonCustomerByEmail:
 	case acpCanonicalUser:
 		pk, err := keys.NewPublicKeyFromString(grantee.ID)
@@ -1473,7 +1476,7 @@ func getRecordFunction(grantee *Grantee) (getRecordFunc, error) {
 
 func isValidGrant(grant *Grant) bool {
 	return (grant.Permission == aclFullControl || grant.Permission == aclRead || grant.Permission == aclWrite) &&
-		(grant.Grantee.Type == acpCanonicalUser || (grant.Grantee.Type == acpGroup && grant.Grantee.URI == allUsersGroup))
+		(grant.Grantee.matchType(acpCanonicalUser) || (grant.Grantee.matchType(acpGroup) && grant.Grantee.URI == allUsersGroup))
 }
 
 func getAllowRecord(op eacl.Operation, pk *keys.PublicKey) *eacl.Record {

api/handler/acl_test.go

@@ -8,6 +8,7 @@ import (
 	"crypto/sha256"
 	"encoding/hex"
 	"encoding/json"
+	"encoding/xml"
 	"fmt"
 	"io"
 	"net/http"
@@ -724,13 +725,13 @@ func TestBucketAclToPolicy(t *testing.T) {
 		AccessControlList: []*Grant{{
 			Grantee: &Grantee{
 				URI:  allUsersGroup,
-				Type: acpGroup,
+				Type: formGranteeType(acpGroup),
 			},
 			Permission: aclRead,
 		}, {
 			Grantee: &Grantee{
 				ID:   id2,
-				Type: acpCanonicalUser,
+				Type: formGranteeType(acpCanonicalUser),
 			},
 			Permission: aclWrite,
 		}},
@@ -790,19 +791,19 @@ func TestObjectAclToPolicy(t *testing.T) {
 		AccessControlList: []*Grant{{
 			Grantee: &Grantee{
 				ID:   id,
-				Type: acpCanonicalUser,
+				Type: formGranteeType(acpCanonicalUser),
 			},
 			Permission: aclFullControl,
 		}, {
 			Grantee: &Grantee{
 				ID:   id2,
-				Type: acpCanonicalUser,
+				Type: formGranteeType(acpCanonicalUser),
 			},
 			Permission: aclFullControl,
 		}, {
 			Grantee: &Grantee{
 				URI:  allUsersGroup,
-				Type: acpGroup,
+				Type: formGranteeType(acpGroup),
 			},
 			Permission: aclRead,
 		}},
@@ -859,7 +860,7 @@ func TestObjectWithVersionAclToTable(t *testing.T) {
 		AccessControlList: []*Grant{{
 			Grantee: &Grantee{
 				ID:   id,
-				Type: acpCanonicalUser,
+				Type: formGranteeType(acpCanonicalUser),
 			},
 			Permission: aclFullControl,
 		}},
@@ -980,13 +981,13 @@ func TestParseCannedACLHeaders(t *testing.T) {
 			Grantee: &Grantee{
 				ID:          id,
 				DisplayName: address,
-				Type:        acpCanonicalUser,
+				Type:        formGranteeType(acpCanonicalUser),
 			},
 			Permission: aclFullControl,
 		}, {
 			Grantee: &Grantee{
 				URI:  allUsersGroup,
-				Type: acpGroup,
+				Type: formGranteeType(acpGroup),
 			},
 			Permission: aclRead,
 		}},
@@ -1021,37 +1022,37 @@ func TestParseACLHeaders(t *testing.T) {
 			Grantee: &Grantee{
 				ID:          id,
 				DisplayName: address,
-				Type:        acpCanonicalUser,
+				Type:        formGranteeType(acpCanonicalUser),
 			},
 			Permission: aclFullControl,
 		}, {
 			Grantee: &Grantee{
 				ID:   "user1",
-				Type: acpCanonicalUser,
+				Type: formGranteeType(acpCanonicalUser),
 			},
 			Permission: aclFullControl,
 		}, {
 			Grantee: &Grantee{
 				URI:  allUsersGroup,
-				Type: acpGroup,
+				Type: formGranteeType(acpGroup),
 			},
 			Permission: aclRead,
 		}, {
 			Grantee: &Grantee{
 				ID:   "user2",
-				Type: acpCanonicalUser,
+				Type: formGranteeType(acpCanonicalUser),
 			},
 			Permission: aclRead,
 		}, {
 			Grantee: &Grantee{
 				ID:   "user2",
-				Type: acpCanonicalUser,
+				Type: formGranteeType(acpCanonicalUser),
 			},
 			Permission: aclWrite,
 		}, {
 			Grantee: &Grantee{
 				ID:   "user3",
-				Type: acpCanonicalUser,
+				Type: formGranteeType(acpCanonicalUser),
 			},
 			Permission: aclWrite,
 		}},
@@ -1112,13 +1113,13 @@ func TestBucketAclToTable(t *testing.T) {
 		AccessControlList: []*Grant{{
 			Grantee: &Grantee{
 				URI:  allUsersGroup,
-				Type: acpGroup,
+				Type: formGranteeType(acpGroup),
 			},
 			Permission: aclRead,
 		}, {
 			Grantee: &Grantee{
 				ID:   id2,
-				Type: acpCanonicalUser,
+				Type: formGranteeType(acpCanonicalUser),
 			},
 			Permission: aclWrite,
 		}},
@@ -1169,13 +1170,13 @@ func TestObjectAclToAst(t *testing.T) {
 		AccessControlList: []*Grant{{
 			Grantee: &Grantee{
 				ID:   id,
-				Type: acpCanonicalUser,
+				Type: formGranteeType(acpCanonicalUser),
 			},
 			Permission: aclFullControl,
 		}, {
 			Grantee: &Grantee{
 				ID:   id2,
-				Type: acpCanonicalUser,
+				Type: formGranteeType(acpCanonicalUser),
 			},
 			Permission: aclRead,
 		},
@@ -1238,13 +1239,13 @@ func TestBucketAclToAst(t *testing.T) {
 			{
 				Grantee: &Grantee{
 					ID:   id2,
-					Type: acpCanonicalUser,
+					Type: formGranteeType(acpCanonicalUser),
 				},
 				Permission: aclWrite,
 			}, {
 				Grantee: &Grantee{
 					URI:  allUsersGroup,
-					Type: acpGroup,
+					Type: formGranteeType(acpGroup),
 				},
 				Permission: aclRead,
 			},
@@ -1436,3 +1437,47 @@ func putBucketACL(t *testing.T, tc *handlerContext, bktName string, box *accessb
 	tc.Handler().PutBucketACLHandler(w, r)
 	assertStatus(t, w, http.StatusOK)
 }
+
+func TestACLGranteeParse(t *testing.T) {
+	body := []byte(`
+<AccessControlPolicy xmlns="http://s3.amazonaws.com/doc/2006-03-01/">
+	<Owner>
+		<DisplayName>NbUgTSFvPmsRxmGeWpuuGeJUoRoi6PErcM</DisplayName>
+		<ID>NbUgTSFvPmsRxmGeWpuuGeJUoRoi6PErcM</ID>
+	</Owner>
+	<AccessControlList>
+		<Grant>
+			<Grantee xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="CanonicalUser">
+				<ID>031a6c6fbbdf02ca351745fa86b9ba5a9452d785ac4f7fc2b7548ca2a46c4fcf4a</ID>
+			</Grantee>
+			<Permission>FULL_CONTROL</Permission>
+		</Grant>
+		<Grant>
+			<Grantee xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="Group">
+				<URI>http://acs.amazonaws.com/groups/global/AllUsers</URI>
+			</Grantee>
+			<Permission>FULL_CONTROL</Permission>
+		</Grant>
+	</AccessControlList>
+</AccessControlPolicy>
+`)
+
+	acl := &AccessControlPolicy{}
+	err := xml.Unmarshal(body, acl)
+	require.NoError(t, err)
+	require.True(t, acl.AccessControlList[0].Grantee.matchType(acpCanonicalUser))
+	require.True(t, acl.AccessControlList[1].Grantee.matchType(acpGroup))
+
+	grantee := NewGrantee(formGranteeType(acpGroup))
+	grantee.URI = allUsersGroup
+
+	raw, err := xml.MarshalIndent(grantee, "", "  ")
+	require.NoError(t, err)
+
+	grantee2 := &Grantee{}
+	err = xml.Unmarshal(raw, grantee2)
+	require.NoError(t, err)
+
+	grantee2.XMLNS.Value = granteeXMLNS
+	require.Equal(t, grantee, grantee2)
+}

api/handler/response.go

@@ -70,21 +70,50 @@ type Grant struct {
 
 // Grantee is info about access rights of some actor.
 type Grantee struct {
-	XMLName      xml.Name    `xml:"Grantee"`
+	XMLName      xml.Name `xml:"Grantee"`
-	XMLNS        string      `xml:"xmlns:xsi,attr"`
+	XMLNS        xml.Attr `xml:"xsi,attr"`
-	ID           string      `xml:"ID,omitempty"`
+	ID           string   `xml:"ID,omitempty"`
-	DisplayName  string      `xml:"DisplayName,omitempty"`
+	DisplayName  string   `xml:"DisplayName,omitempty"`
-	EmailAddress string      `xml:"EmailAddress,omitempty"`
+	EmailAddress string   `xml:"EmailAddress,omitempty"`
-	URI          string      `xml:"URI,omitempty"`
+	URI          string   `xml:"URI,omitempty"`
-	Type         GranteeType `xml:"xsi:type,attr"`
+	Type         xml.Attr `xml:"type,attr"`
 }
 
-// NewGrantee creates new grantee using workaround
+func (g Grantee) matchType(t string) bool {
-// https://github.com/golang/go/issues/9519#issuecomment-252196382
+	return g.Type.Value == t
-func NewGrantee(t GranteeType) *Grantee {
+}
+
+func (g Grantee) TypeString() string {
+	return g.Type.Value
+}
+
+func formGranteeType(str string) xml.Attr {
+	return xml.Attr{
+		Name: xml.Name{
+			Space: "xsi",
+			Local: "type",
+		},
+		Value: str,
+	}
+}
+
+const granteeXMLNS = "http://www.w3.org/2001/XMLSchema-instance"
+
+// NewGrantee creates new grantee.
+func NewGrantee(t xml.Attr) *Grantee {
 	return &Grantee{
-		XMLNS: "http://www.w3.org/2001/XMLSchema-instance",
+		XMLName: xml.Name{
-		Type:  t,
+			Local: "Grantee",
+		},
+
+		XMLNS: xml.Attr{
+			Name: xml.Name{
+				Space: "xmlns",
+				Local: "xsi",
+			},
+			Value: granteeXMLNS,
+		},
+		Type: t,
 	}
 }