From 828eb93beed670a7ac523d148d87b266c85b03a2 Mon Sep 17 00:00:00 2001 From: jonaustin09 Date: Tue, 16 Jul 2024 18:17:16 -0400 Subject: [PATCH] fix: Added 'Type' property support in bucket ACL Grantee schema --- auth/acl.go | 113 ++++++++++++++++++++++++++++++------- s3api/controllers/admin.go | 1 + s3api/controllers/base.go | 49 +++++++++------- s3api/server.go | 4 +- tests/integration/tests.go | 21 ++++--- tests/integration/utils.go | 3 + 6 files changed, 144 insertions(+), 47 deletions(-) diff --git a/auth/acl.go b/auth/acl.go index 8bcb00cc..fa449933 100644 --- a/auth/acl.go +++ b/auth/acl.go @@ -35,6 +35,7 @@ type ACL struct { type Grantee struct { Permission types.Permission Access string + Type types.Type } type GetBucketAclOutput struct { @@ -42,14 +43,38 @@ type GetBucketAclOutput struct { AccessControlList AccessControlList } -type AccessControlList struct { - Grants []types.Grant `xml:"Grant"` +type PutBucketAclInput struct { + Bucket *string + ACL types.BucketCannedACL + AccessControlPolicy *AccessControlPolicy + GrantFullControl *string + GrantRead *string + GrantReadACP *string + GrantWrite *string + GrantWriteACP *string } + type AccessControlPolicy struct { AccessControlList AccessControlList `xml:"AccessControlList"` Owner types.Owner } +type AccessControlList struct { + Grants []Grant `xml:"Grant"` +} + +type Grant struct { + Grantee *Grt + Permission types.Permission +} + +type Grt struct { + XMLNS string `xml:"xmlns:xsi,attr"` + XMLXSI types.Type `xml:"xsi:type,attr"` + Type types.Type `xml:"Type"` + ID string `xml:"ID"` +} + func ParseACL(data []byte) (ACL, error) { if len(data) == 0 { return ACL{}, nil @@ -68,11 +93,19 @@ func ParseACLOutput(data []byte) (GetBucketAclOutput, error) { return GetBucketAclOutput{}, fmt.Errorf("parse acl: %w", err) } - grants := []types.Grant{} + grants := []Grant{} for _, elem := range acl.Grantees { acs := elem.Access - grants = append(grants, types.Grant{Grantee: &types.Grantee{ID: &acs}, Permission: elem.Permission}) + grants = append(grants, Grant{ + Grantee: &Grt{ + XMLNS: "http://www.w3.org/2001/XMLSchema-instance", + XMLXSI: elem.Type, + ID: acs, + Type: elem.Type, + }, + Permission: elem.Permission, + }) } return GetBucketAclOutput{ @@ -85,7 +118,7 @@ func ParseACLOutput(data []byte) (GetBucketAclOutput, error) { }, nil } -func UpdateACL(input *s3.PutBucketAclInput, acl ACL, iam IAMService) ([]byte, error) { +func UpdateACL(input *PutBucketAclInput, acl ACL, iam IAMService) ([]byte, error) { if input == nil { return nil, s3err.GetAPIError(s3err.ErrInvalidRequest) } @@ -97,6 +130,7 @@ func UpdateACL(input *s3.PutBucketAclInput, acl ACL, iam IAMService) ([]byte, er { Permission: types.PermissionFullControl, Access: acl.Owner, + Type: types.TypeCanonicalUser, }, } @@ -107,16 +141,19 @@ func UpdateACL(input *s3.PutBucketAclInput, acl ACL, iam IAMService) ([]byte, er defaultGrantees = append(defaultGrantees, Grantee{ Permission: types.PermissionRead, Access: "all-users", + Type: types.TypeGroup, }) case types.BucketCannedACLPublicReadWrite: defaultGrantees = append(defaultGrantees, []Grantee{ { Permission: types.PermissionRead, Access: "all-users", + Type: types.TypeGroup, }, { Permission: types.PermissionWrite, Access: "all-users", + Type: types.TypeGroup, }, }...) } @@ -129,45 +166,71 @@ func UpdateACL(input *s3.PutBucketAclInput, acl ACL, iam IAMService) ([]byte, er if input.GrantFullControl != nil && *input.GrantFullControl != "" { fullControlList = splitUnique(*input.GrantFullControl, ",") for _, str := range fullControlList { - defaultGrantees = append(defaultGrantees, Grantee{Access: str, Permission: "FULL_CONTROL"}) + defaultGrantees = append(defaultGrantees, Grantee{ + Access: str, + Permission: types.PermissionFullControl, + Type: types.TypeCanonicalUser, + }) } } if input.GrantRead != nil && *input.GrantRead != "" { readList = splitUnique(*input.GrantRead, ",") for _, str := range readList { - defaultGrantees = append(defaultGrantees, Grantee{Access: str, Permission: "READ"}) + defaultGrantees = append(defaultGrantees, Grantee{ + Access: str, + Permission: types.PermissionRead, + Type: types.TypeCanonicalUser, + }) } } if input.GrantReadACP != nil && *input.GrantReadACP != "" { readACPList = splitUnique(*input.GrantReadACP, ",") for _, str := range readACPList { - defaultGrantees = append(defaultGrantees, Grantee{Access: str, Permission: "READ_ACP"}) + defaultGrantees = append(defaultGrantees, Grantee{ + Access: str, + Permission: types.PermissionReadAcp, + Type: types.TypeCanonicalUser, + }) } } if input.GrantWrite != nil && *input.GrantWrite != "" { writeList = splitUnique(*input.GrantWrite, ",") for _, str := range writeList { - defaultGrantees = append(defaultGrantees, Grantee{Access: str, Permission: "WRITE"}) + defaultGrantees = append(defaultGrantees, Grantee{ + Access: str, + Permission: types.PermissionWrite, + Type: types.TypeCanonicalUser, + }) } } if input.GrantWriteACP != nil && *input.GrantWriteACP != "" { writeACPList = splitUnique(*input.GrantWriteACP, ",") for _, str := range writeACPList { - defaultGrantees = append(defaultGrantees, Grantee{Access: str, Permission: "WRITE_ACP"}) + defaultGrantees = append(defaultGrantees, Grantee{ + Access: str, + Permission: types.PermissionWriteAcp, + Type: types.TypeCanonicalUser, + }) } } accs = append(append(append(append(fullControlList, readList...), writeACPList...), readACPList...), writeList...) } else { cache := make(map[string]bool) - for _, grt := range input.AccessControlPolicy.Grants { - if grt.Grantee == nil || grt.Grantee.ID == nil || grt.Permission == "" { + for _, grt := range input.AccessControlPolicy.AccessControlList.Grants { + if grt.Grantee == nil || grt.Grantee.ID == "" || grt.Permission == "" { return nil, s3err.GetAPIError(s3err.ErrInvalidRequest) } - defaultGrantees = append(defaultGrantees, Grantee{Access: *grt.Grantee.ID, Permission: grt.Permission}) - if _, ok := cache[*grt.Grantee.ID]; !ok { - cache[*grt.Grantee.ID] = true - accs = append(accs, *grt.Grantee.ID) + + access := grt.Grantee.ID + defaultGrantees = append(defaultGrantees, Grantee{ + Access: access, + Permission: grt.Permission, + Type: types.TypeCanonicalUser, + }) + if _, ok := cache[access]; !ok { + cache[access] = true + accs = append(accs, access) } } } @@ -227,9 +290,21 @@ func splitUnique(s, divider string) []string { } func verifyACL(acl ACL, access string, permission types.Permission) error { - grantee := Grantee{Access: access, Permission: permission} - granteeFullCtrl := Grantee{Access: access, Permission: "FULL_CONTROL"} - granteeAllUsers := Grantee{Access: "all-users", Permission: permission} + grantee := Grantee{ + Access: access, + Permission: permission, + Type: types.TypeCanonicalUser, + } + granteeFullCtrl := Grantee{ + Access: access, + Permission: types.PermissionFullControl, + Type: types.TypeCanonicalUser, + } + granteeAllUsers := Grantee{ + Access: "all-users", + Permission: permission, + Type: types.TypeGroup, + } isFound := false diff --git a/s3api/controllers/admin.go b/s3api/controllers/admin.go index 1deaa039..9562d824 100644 --- a/s3api/controllers/admin.go +++ b/s3api/controllers/admin.go @@ -145,6 +145,7 @@ func (c AdminController) ChangeBucketOwner(ctx *fiber.Ctx) error { { Permission: types.PermissionFullControl, Access: owner, + Type: types.TypeCanonicalUser, }, }, } diff --git a/s3api/controllers/base.go b/s3api/controllers/base.go index edfd6fe6..fa6647c9 100644 --- a/s3api/controllers/base.go +++ b/s3api/controllers/base.go @@ -1224,7 +1224,7 @@ func (c S3ApiController) PutBucketActions(ctx *fiber.Ctx) error { if ctx.Request().URI().QueryArgs().Has("acl") { parsedAcl := ctx.Locals("parsedAcl").(auth.ACL) - var input *s3.PutBucketAclInput + var input *auth.PutBucketAclInput ownership, err := c.be.GetBucketOwnershipControls(ctx.Context(), bucket) if err != nil && !errors.Is(err, s3err.GetAPIError(s3err.ErrOwnershipControlsNotFound)) { @@ -1299,13 +1299,10 @@ func (c S3ApiController) PutBucketActions(ctx *fiber.Ctx) error { }) } - input = &s3.PutBucketAclInput{ - Bucket: &bucket, - ACL: "", - AccessControlPolicy: &types.AccessControlPolicy{ - Owner: &accessControlPolicy.Owner, - Grants: accessControlPolicy.AccessControlList.Grants, - }, + input = &auth.PutBucketAclInput{ + Bucket: &bucket, + ACL: "", + AccessControlPolicy: &accessControlPolicy, } } if acl != "" { @@ -1337,26 +1334,26 @@ func (c S3ApiController) PutBucketActions(ctx *fiber.Ctx) error { }) } - input = &s3.PutBucketAclInput{ + input = &auth.PutBucketAclInput{ Bucket: &bucket, ACL: types.BucketCannedACL(acl), - AccessControlPolicy: &types.AccessControlPolicy{ - Owner: &types.Owner{ + AccessControlPolicy: &auth.AccessControlPolicy{ + Owner: types.Owner{ ID: &acct.Access, }, }, } } if grants != "" { - input = &s3.PutBucketAclInput{ + input = &auth.PutBucketAclInput{ Bucket: &bucket, GrantFullControl: &grantFullControl, GrantRead: &grantRead, GrantReadACP: &grantReadACP, GrantWrite: &granWrite, GrantWriteACP: &grantWriteACP, - AccessControlPolicy: &types.AccessControlPolicy{ - Owner: &types.Owner{ + AccessControlPolicy: &auth.AccessControlPolicy{ + Owner: types.Owner{ ID: &acct.Access, }, }, @@ -1440,15 +1437,16 @@ func (c S3ApiController) PutBucketActions(ctx *fiber.Ctx) error { Owner: acct.Access, } - updAcl, err := auth.UpdateACL(&s3.PutBucketAclInput{ + updAcl, err := auth.UpdateACL(&auth.PutBucketAclInput{ GrantFullControl: &grantFullControl, GrantRead: &grantRead, GrantReadACP: &grantReadACP, GrantWrite: &granWrite, GrantWriteACP: &grantWriteACP, - AccessControlPolicy: &types.AccessControlPolicy{Owner: &types.Owner{ - ID: &acct.Access, - }}, + AccessControlPolicy: &auth.AccessControlPolicy{ + Owner: types.Owner{ + ID: &acct.Access, + }}, ACL: types.BucketCannedACL(acl), }, defACL, c.iam) if err != nil { @@ -1867,13 +1865,26 @@ func (c S3ApiController) PutActions(ctx *fiber.Ctx) error { }) } + //TODO: This part will be changed when object acls are implemented + + grants := []types.Grant{} + for _, grt := range accessControlPolicy.AccessControlList.Grants { + grants = append(grants, types.Grant{ + Grantee: &types.Grantee{ + ID: &grt.Grantee.ID, + Type: grt.Grantee.Type, + }, + Permission: grt.Permission, + }) + } + input = &s3.PutObjectAclInput{ Bucket: &bucket, Key: &keyStart, ACL: "", AccessControlPolicy: &types.AccessControlPolicy{ Owner: &accessControlPolicy.Owner, - Grants: accessControlPolicy.AccessControlList.Grants, + Grants: grants, }, } } diff --git a/s3api/server.go b/s3api/server.go index 1b1ef5e5..942c1221 100644 --- a/s3api/server.go +++ b/s3api/server.go @@ -64,7 +64,9 @@ func New( // Logging middlewares if !server.quiet { - app.Use(logger.New()) + app.Use(logger.New(logger.Config{ + Format: "${time} | ${status} | ${latency} | ${ip} | ${method} | ${path} | ${error} | ${queryParams}\n", + })) } // Set up health endpoint if specified if server.health != "" { diff --git a/tests/integration/tests.go b/tests/integration/tests.go index b7b22217..1454ffb1 100644 --- a/tests/integration/tests.go +++ b/tests/integration/tests.go @@ -1807,25 +1807,29 @@ func CreateBucket_non_default_acl(s *S3Conf) error { grants := []types.Grant{ { Grantee: &types.Grantee{ - ID: &s.awsID, + ID: &s.awsID, + Type: types.TypeCanonicalUser, }, Permission: types.PermissionFullControl, }, { Grantee: &types.Grantee{ - ID: getPtr("grt1"), + ID: getPtr("grt1"), + Type: types.TypeCanonicalUser, }, Permission: types.PermissionFullControl, }, { Grantee: &types.Grantee{ - ID: getPtr("grt2"), + ID: getPtr("grt2"), + Type: types.TypeCanonicalUser, }, Permission: types.PermissionReadAcp, }, { Grantee: &types.Grantee{ - ID: getPtr("grt3"), + ID: getPtr("grt3"), + Type: types.TypeCanonicalUser, }, Permission: types.PermissionWrite, }, @@ -6491,7 +6495,7 @@ func GetBucketAcl_translation_canned_public_read(s *S3Conf) error { { Grantee: &types.Grantee{ ID: getPtr("all-users"), - Type: types.TypeCanonicalUser, + Type: types.TypeGroup, }, Permission: types.PermissionRead, }, @@ -6541,14 +6545,14 @@ func GetBucketAcl_translation_canned_public_read_write(s *S3Conf) error { { Grantee: &types.Grantee{ ID: getPtr("all-users"), - Type: types.TypeCanonicalUser, + Type: types.TypeGroup, }, Permission: types.PermissionRead, }, { Grantee: &types.Grantee{ ID: getPtr("all-users"), - Type: types.TypeCanonicalUser, + Type: types.TypeGroup, }, Permission: types.PermissionWrite, }, @@ -6716,7 +6720,8 @@ func GetBucketAcl_success(s *S3Conf) error { grants = append([]types.Grant{ { Grantee: &types.Grantee{ - ID: &s.awsID, + ID: &s.awsID, + Type: types.TypeCanonicalUser, }, Permission: types.PermissionFullControl, }, diff --git a/tests/integration/utils.go b/tests/integration/utils.go index 84de0ea0..785ee88b 100644 --- a/tests/integration/utils.go +++ b/tests/integration/utils.go @@ -398,6 +398,9 @@ func compareGrants(grts1, grts2 []types.Grant) bool { if *grt.Grantee.ID != *grts2[i].Grantee.ID { return false } + if grt.Grantee.Type != grts2[i].Grantee.Type { + return false + } } return true }