fix: Fixes #204, Change ListBuckets action logic to return all the buckets for admin users and the buckets owned by a user for regular users. Added integration test cases for ListBuckets action

backend/backend.go

@@ -30,7 +30,7 @@ type Backend interface {
 	Shutdown()
 
 	// bucket operations
-	ListBuckets(_ context.Context, owner string, isRoot bool) (s3response.ListAllMyBucketsResult, error)
+	ListBuckets(_ context.Context, owner string, isAdmin bool) (s3response.ListAllMyBucketsResult, error)
 	HeadBucket(context.Context, *s3.HeadBucketInput) (*s3.HeadBucketOutput, error)
 	GetBucketAcl(context.Context, *s3.GetBucketAclInput) ([]byte, error)
 	CreateBucket(context.Context, *s3.CreateBucketInput) error

backend/posix/posix.go

@@ -98,7 +98,7 @@ func (p *Posix) String() string {
 	return "Posix Gateway"
 }
 
-func (p *Posix) ListBuckets(_ context.Context, owner string, isRoot bool) (s3response.ListAllMyBucketsResult, error) {
+func (p *Posix) ListBuckets(_ context.Context, owner string, isAdmin bool) (s3response.ListAllMyBucketsResult, error) {
 	entries, err := os.ReadDir(".")
 	if err != nil {
 		return s3response.ListAllMyBucketsResult{},
@@ -118,10 +118,32 @@ func (p *Posix) ListBuckets(_ context.Context, owner string, isRoot bool) (s3res
 			continue
 		}
 
-		buckets = append(buckets, s3response.ListAllMyBucketsEntry{
-			Name:         entry.Name(),
-			CreationDate: fi.ModTime(),
-		})
+		// return all the buckets for admin users
+		if isAdmin {
+			buckets = append(buckets, s3response.ListAllMyBucketsEntry{
+				Name:         entry.Name(),
+				CreationDate: fi.ModTime(),
+			})
+			continue
+		}
+
+		aclTag, err := xattr.Get(entry.Name(), aclkey)
+		if err != nil {
+			return s3response.ListAllMyBucketsResult{}, fmt.Errorf("get acl tag: %w", err)
+		}
+
+		var acl auth.ACL
+		err = json.Unmarshal(aclTag, &acl)
+		if err != nil {
+			return s3response.ListAllMyBucketsResult{}, fmt.Errorf("parse acl tag: %w", err)
+		}
+
+		if acl.Owner == owner {
+			buckets = append(buckets, s3response.ListAllMyBucketsEntry{
+				Name:         entry.Name(),
+				CreationDate: fi.ModTime(),
+			})
+		}
 	}
 
 	sort.Sort(backend.ByBucketName(buckets))
@@ -130,6 +152,9 @@ func (p *Posix) ListBuckets(_ context.Context, owner string, isRoot bool) (s3res
 		Buckets: s3response.ListAllMyBucketsList{
 			Bucket: buckets,
 		},
+		Owner: s3response.CanonicalUser{
+			ID: owner,
+		},
 	}, nil
 }
 

integration/action-tests.go

@@ -33,6 +33,12 @@ func TestHeadBucket(s *S3Conf) {
 	HeadBucket_success(s)
 }
 
+func TestListBuckets(s *S3Conf) {
+	ListBuckets_as_user(s)
+	ListBuckets_as_admin(s)
+	ListBuckets_success(s)
+}
+
 func TestDeleteBucket(s *S3Conf) {
 	DeleteBucket_non_existing_bucket(s)
 	DeleteBucket_non_empty_bucket(s)
@@ -176,6 +182,7 @@ func TestFullFlow(s *S3Conf) {
 	TestAuthentication(s)
 	TestCreateBucket(s)
 	TestHeadBucket(s)
+	TestListBuckets(s)
 	TestDeleteBucket(s)
 	TestPutObject(s)
 	TestHeadObject(s)

integration/tests.go

@@ -18,6 +18,7 @@ import (
 	"github.com/aws/aws-sdk-go-v2/service/s3/types"
 	"github.com/google/uuid"
 	"github.com/versity/versitygw/s3err"
+	"github.com/versity/versitygw/s3response"
 )
 
 var (
@@ -710,6 +711,189 @@ func HeadBucket_success(s *S3Conf) {
 	})
 }
 
+func ListBuckets_as_user(s *S3Conf) {
+	testName := "ListBuckets_as_user"
+	actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
+		buckets := []s3response.ListAllMyBucketsEntry{{Name: bucket}}
+		for i := 0; i < 6; i++ {
+			bckt := getBucketName()
+
+			err := setup(s, bckt)
+			if err != nil {
+				return err
+			}
+
+			buckets = append(buckets, s3response.ListAllMyBucketsEntry{
+				Name: bckt,
+			})
+		}
+		usr := user{
+			access: "grt1",
+			secret: "grt1secret",
+			role:   "user",
+		}
+
+		err := createUsers(s, []user{usr})
+		if err != nil {
+			return err
+		}
+
+		cfg := *s
+		cfg.awsID = usr.access
+		cfg.awsSecret = usr.secret
+
+		bckts := []string{}
+		for i := 0; i < 3; i++ {
+			bckts = append(bckts, buckets[i].Name)
+		}
+
+		err = changeBucketsOwner(s, bckts, usr.access)
+		if err != nil {
+			return err
+		}
+
+		userClient := s3.NewFromConfig(cfg.Config())
+
+		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
+		out, err := userClient.ListBuckets(ctx, &s3.ListBucketsInput{})
+		cancel()
+		if err != nil {
+			return err
+		}
+
+		if *out.Owner.ID != usr.access {
+			return fmt.Errorf("expected buckets owner to be %v, instead got %v", usr.access, *out.Owner.ID)
+		}
+		if ok := compareBuckets(out.Buckets, buckets[:3]); !ok {
+			return fmt.Errorf("expected list buckets result to be %v, instead got %v", buckets[:3], out.Buckets)
+		}
+
+		for _, elem := range buckets[1:] {
+			err = teardown(s, elem.Name)
+			if err != nil {
+				return err
+			}
+		}
+
+		return nil
+	})
+}
+
+func ListBuckets_as_admin(s *S3Conf) {
+	testName := "ListBuckets_as_admin"
+	actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
+		buckets := []s3response.ListAllMyBucketsEntry{{Name: bucket}}
+		for i := 0; i < 6; i++ {
+			bckt := getBucketName()
+
+			err := setup(s, bckt)
+			if err != nil {
+				return err
+			}
+
+			buckets = append(buckets, s3response.ListAllMyBucketsEntry{
+				Name: bckt,
+			})
+		}
+		usr := user{
+			access: "grt1",
+			secret: "grt1secret",
+			role:   "user",
+		}
+		admin := user{
+			access: "admin1",
+			secret: "admin1secret",
+			role:   "admin",
+		}
+
+		err := createUsers(s, []user{usr, admin})
+		if err != nil {
+			return err
+		}
+
+		cfg := *s
+		cfg.awsID = admin.access
+		cfg.awsSecret = admin.secret
+
+		bckts := []string{}
+		for i := 0; i < 3; i++ {
+			bckts = append(bckts, buckets[i].Name)
+		}
+
+		err = changeBucketsOwner(s, bckts, usr.access)
+		if err != nil {
+			return err
+		}
+
+		adminClient := s3.NewFromConfig(cfg.Config())
+
+		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
+		out, err := adminClient.ListBuckets(ctx, &s3.ListBucketsInput{})
+		cancel()
+		if err != nil {
+			return err
+		}
+
+		if *out.Owner.ID != admin.access {
+			return fmt.Errorf("expected buckets owner to be %v, instead got %v", admin.access, *out.Owner.ID)
+		}
+		if ok := compareBuckets(out.Buckets, buckets); !ok {
+			return fmt.Errorf("expected list buckets result to be %v, instead got %v", buckets, out.Buckets)
+		}
+
+		for _, elem := range buckets[1:] {
+			err = teardown(s, elem.Name)
+			if err != nil {
+				return err
+			}
+		}
+
+		return nil
+	})
+}
+
+func ListBuckets_success(s *S3Conf) {
+	testName := "ListBuckets_success"
+	actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
+		buckets := []s3response.ListAllMyBucketsEntry{{Name: bucket}}
+		for i := 0; i < 5; i++ {
+			bckt := getBucketName()
+
+			err := setup(s, bckt)
+			if err != nil {
+				return err
+			}
+
+			buckets = append(buckets, s3response.ListAllMyBucketsEntry{
+				Name: bckt,
+			})
+		}
+
+		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
+		out, err := s3client.ListBuckets(ctx, &s3.ListBucketsInput{})
+		cancel()
+		if err != nil {
+			return err
+		}
+
+		if *out.Owner.ID != s.awsID {
+			return fmt.Errorf("expected owner to be %v, instead got %v", s.awsID, *out.Owner.ID)
+		}
+		if ok := compareBuckets(out.Buckets, buckets); !ok {
+			return fmt.Errorf("expected list buckets result to be %v, instead got %v", buckets, out.Buckets)
+		}
+
+		for _, elem := range buckets[1:] {
+			err = teardown(s, elem.Name)
+			if err != nil {
+				return err
+			}
+		}
+
+		return nil
+	})
+}
+
 func CreateDeleteBucket_success(s *S3Conf) {
 	testName := "CreateBucket_success"
 	runF(testName)

integration/utils.go

@@ -22,6 +22,7 @@ import (
 	"github.com/aws/aws-sdk-go-v2/service/s3/types"
 	"github.com/aws/smithy-go"
 	"github.com/versity/versitygw/s3err"
+	"github.com/versity/versitygw/s3response"
 )
 
 var (
@@ -360,6 +361,26 @@ func areMapsSame(mp1, mp2 map[string]string) bool {
 	return true
 }
 
+func compareBuckets(list1 []types.Bucket, list2 []s3response.ListAllMyBucketsEntry) bool {
+	if len(list1) != len(list2) {
+		return false
+	}
+
+	elementMap := make(map[string]bool)
+
+	for _, elem := range list1 {
+		elementMap[*elem.Name] = true
+	}
+
+	for _, elem := range list2 {
+		if _, found := elementMap[elem.Name]; !found {
+			return false
+		}
+	}
+
+	return true
+}
+
 func compareObjects(list1 []string, list2 []types.Object) bool {
 	if len(list1) != len(list2) {
 		return false
@@ -471,3 +492,17 @@ func createUsers(s *S3Conf, users []user) error {
 	}
 	return nil
 }
+
+func changeBucketsOwner(s *S3Conf, buckets []string, owner string) error {
+	for _, bucket := range buckets {
+		out, err := execCommand("admin", "-a", s.awsID, "-s", s.awsSecret, "-er", s.endpoint, "change-bucket-owner", "-b", bucket, "-o", owner)
+		if err != nil {
+			return err
+		}
+		if !strings.Contains(string(out), "Bucket owner has been updated successfully") {
+			return fmt.Errorf(string(out))
+		}
+	}
+
+	return nil
+}

s3api/controllers/base.go

@@ -53,8 +53,8 @@ func New(be backend.Backend, iam auth.IAMService, logger s3log.AuditLogger, evs
 }
 
 func (c S3ApiController) ListBuckets(ctx *fiber.Ctx) error {
-	access, isRoot := ctx.Locals("access").(string), ctx.Locals("isRoot").(bool)
-	res, err := c.be.ListBuckets(ctx.Context(), access, isRoot)
+	access, role := ctx.Locals("access").(string), ctx.Locals("role").(string)
+	res, err := c.be.ListBuckets(ctx.Context(), access, role == "admin")
 	return SendXMLResponse(ctx, res, err, &MetaOpts{Logger: c.logger, Action: "ListBucket"})
 }
 

s3api/controllers/base_test.go

@@ -90,9 +90,6 @@ func TestS3ApiController_ListBuckets(t *testing.T) {
 	app := fiber.New()
 	s3ApiController := S3ApiController{
 		be: &BackendMock{
-			GetBucketAclFunc: func(context.Context, *s3.GetBucketAclInput) ([]byte, error) {
-				return acldata, nil
-			},
 			ListBucketsFunc: func(context.Context, string, bool) (s3response.ListAllMyBucketsResult, error) {
 				return s3response.ListAllMyBucketsResult{}, nil
 			},
@@ -101,7 +98,7 @@ func TestS3ApiController_ListBuckets(t *testing.T) {
 
 	app.Use(func(ctx *fiber.Ctx) error {
 		ctx.Locals("access", "valid access")
-		ctx.Locals("isRoot", true)
+		ctx.Locals("role", "admin")
 		ctx.Locals("isDebug", false)
 		return ctx.Next()
 	})
@@ -111,9 +108,6 @@ func TestS3ApiController_ListBuckets(t *testing.T) {
 	appErr := fiber.New()
 	s3ApiControllerErr := S3ApiController{
 		be: &BackendMock{
-			GetBucketAclFunc: func(context.Context, *s3.GetBucketAclInput) ([]byte, error) {
-				return acldata, nil
-			},
 			ListBucketsFunc: func(context.Context, string, bool) (s3response.ListAllMyBucketsResult, error) {
 				return s3response.ListAllMyBucketsResult{}, s3err.GetAPIError(s3err.ErrMethodNotAllowed)
 			},
@@ -122,7 +116,7 @@ func TestS3ApiController_ListBuckets(t *testing.T) {
 
 	appErr.Use(func(ctx *fiber.Ctx) error {
 		ctx.Locals("access", "valid access")
-		ctx.Locals("isRoot", true)
+		ctx.Locals("role", "admin")
 		ctx.Locals("isDebug", false)
 		return ctx.Next()
 	})