From 4c7584c99fc263047035ae91cbdef7e39c60e404 Mon Sep 17 00:00:00 2001 From: jonaustin09 Date: Wed, 6 Sep 2023 17:41:47 -0400 Subject: [PATCH] feat: Closes #206, Added an admin api endpoint and a CLI action to change buckets owner --- auth/acl.go | 4 +- backend/backend.go | 7 ++ backend/posix/posix.go | 35 +++++++ cmd/versitygw/admin.go | 56 +++++++++++ s3api/controllers/admin.go | 38 +++++++- s3api/controllers/admin_test.go | 135 +++++++++++++++++++++++++- s3api/controllers/backend_moq_test.go | 56 +++++++++++ s3api/router.go | 7 +- 8 files changed, 326 insertions(+), 12 deletions(-) diff --git a/auth/acl.go b/auth/acl.go index b2e2802..3fa6995 100644 --- a/auth/acl.go +++ b/auth/acl.go @@ -148,7 +148,7 @@ func UpdateACL(input *s3.PutBucketAclInput, acl ACL, iam IAMService) ([]byte, er } // Check if the specified accounts exist - accList, err := checkIfAccountsExist(accs, iam) + accList, err := CheckIfAccountsExist(accs, iam) if err != nil { return nil, err } @@ -168,7 +168,7 @@ func UpdateACL(input *s3.PutBucketAclInput, acl ACL, iam IAMService) ([]byte, er return result, nil } -func checkIfAccountsExist(accs []string, iam IAMService) ([]string, error) { +func CheckIfAccountsExist(accs []string, iam IAMService) ([]string, error) { result := []string{} for _, acc := range accs { diff --git a/backend/backend.go b/backend/backend.go index d334f8e..09cc486 100644 --- a/backend/backend.go +++ b/backend/backend.go @@ -67,6 +67,9 @@ type Backend interface { GetTags(_ context.Context, bucket, object string) (map[string]string, error) SetTags(_ context.Context, bucket, object string, tags map[string]string) error RemoveTags(_ context.Context, bucket, object string) error + + // non AWS actions + ChangeBucketOwner(_ context.Context, bucket, newOwner string) error } type BackendUnsupported struct{} @@ -171,3 +174,7 @@ func (BackendUnsupported) SetTags(_ context.Context, bucket, object string, tags func (BackendUnsupported) RemoveTags(_ context.Context, bucket, object string) error { return s3err.GetAPIError(s3err.ErrNotImplemented) } + +func (BackendUnsupported) ChangeBucketOwner(_ context.Context, bucket, newOwner string) error { + return s3err.GetAPIError(s3err.ErrNotImplemented) +} diff --git a/backend/posix/posix.go b/backend/posix/posix.go index 1c45ab9..44f9d83 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -1506,6 +1506,41 @@ func (p *Posix) RemoveTags(ctx context.Context, bucket, object string) error { return p.SetTags(ctx, bucket, object, nil) } +func (p *Posix) ChangeBucketOwner(ctx context.Context, bucket, newOwner string) error { + _, err := os.Stat(bucket) + if errors.Is(err, fs.ErrNotExist) { + return s3err.GetAPIError(s3err.ErrNoSuchBucket) + } + if err != nil { + return fmt.Errorf("stat bucket: %w", err) + } + + aclTag, err := xattr.Get(bucket, aclkey) + if err != nil { + return fmt.Errorf("get acl: %w", err) + } + + var acl auth.ACL + err = json.Unmarshal(aclTag, &acl) + if err != nil { + return fmt.Errorf("unmarshal acl: %w", err) + } + + acl.Owner = newOwner + + newAcl, err := json.Marshal(acl) + if err != nil { + return fmt.Errorf("marshal acl: %w", err) + } + + err = xattr.Set(bucket, aclkey, newAcl) + if err != nil { + return fmt.Errorf("set acl: %w", err) + } + + return nil +} + const ( iamMode = 0600 ) diff --git a/cmd/versitygw/admin.go b/cmd/versitygw/admin.go index 71b4fbf..b01dca4 100644 --- a/cmd/versitygw/admin.go +++ b/cmd/versitygw/admin.go @@ -84,6 +84,25 @@ func adminCommand() *cli.Command { Usage: "List all the gateway users", Action: listUsers, }, + { + Name: "change-bucket-owner", + Usage: "Changes the bucket owner", + Flags: []cli.Flag{ + &cli.StringFlag{ + Name: "bucket", + Usage: "the bucket name to change the owner", + Required: true, + Aliases: []string{"b"}, + }, + &cli.StringFlag{ + Name: "owner", + Usage: "the user access key id, who should be the bucket owner", + Required: true, + Aliases: []string{"o"}, + }, + }, + Action: changeBucketOwner, + }, }, Flags: []cli.Flag{ // TODO: create a configuration file for this @@ -244,3 +263,40 @@ func listUsers(ctx *cli.Context) error { return nil } + +func changeBucketOwner(ctx *cli.Context) error { + bucket, owner := ctx.String("bucket"), ctx.String("owner") + req, err := http.NewRequest(http.MethodPatch, fmt.Sprintf("%v/change-bucket-owner/?bucket=%v&owner=%v", adminEndpoint, bucket, owner), nil) + if err != nil { + return fmt.Errorf("failed to send the request: %w", err) + } + + signer := v4.NewSigner() + + hashedPayload := sha256.Sum256([]byte{}) + hexPayload := hex.EncodeToString(hashedPayload[:]) + + req.Header.Set("X-Amz-Content-Sha256", hexPayload) + + signErr := signer.SignHTTP(req.Context(), aws.Credentials{AccessKeyID: adminAccess, SecretAccessKey: adminSecret}, req, hexPayload, "s3", region, time.Now()) + if signErr != nil { + return fmt.Errorf("failed to sign the request: %w", err) + } + + client := http.Client{} + + resp, err := client.Do(req) + if err != nil { + return fmt.Errorf("failed to send the request: %w", err) + } + + body, err := io.ReadAll(resp.Body) + if err != nil { + return err + } + defer resp.Body.Close() + + fmt.Println(string(body)) + + return nil +} diff --git a/s3api/controllers/admin.go b/s3api/controllers/admin.go index 2606b5e..63c3654 100644 --- a/s3api/controllers/admin.go +++ b/s3api/controllers/admin.go @@ -19,10 +19,16 @@ import ( "github.com/gofiber/fiber/v2" "github.com/versity/versitygw/auth" + "github.com/versity/versitygw/backend" ) type AdminController struct { - IAMService auth.IAMService + iam auth.IAMService + be backend.Backend +} + +func NewAdminController(iam auth.IAMService, be backend.Backend) AdminController { + return AdminController{iam: iam, be: be} } func (c AdminController) CreateUser(ctx *fiber.Ctx) error { @@ -38,7 +44,7 @@ func (c AdminController) CreateUser(ctx *fiber.Ctx) error { user := auth.Account{Secret: secret, Role: role} - err := c.IAMService.CreateAccount(access, user) + err := c.iam.CreateAccount(access, user) if err != nil { return fmt.Errorf("failed to create a user: %w", err) } @@ -53,7 +59,7 @@ func (c AdminController) DeleteUser(ctx *fiber.Ctx) error { return fmt.Errorf("access denied: only admin users have access to this resource") } - err := c.IAMService.DeleteUserAccount(access) + err := c.iam.DeleteUserAccount(access) if err != nil { return err } @@ -66,10 +72,34 @@ func (c AdminController) ListUsers(ctx *fiber.Ctx) error { if role != "admin" { return fmt.Errorf("access denied: only admin users have access to this resource") } - accs, err := c.IAMService.ListUserAccounts() + accs, err := c.iam.ListUserAccounts() if err != nil { return err } return ctx.JSON(accs) } + +func (c AdminController) ChangeBucketOwner(ctx *fiber.Ctx) error { + role := ctx.Locals("role").(string) + if role != "admin" { + return fmt.Errorf("access denied: only admin users have access to this resource") + } + owner := ctx.Query("owner") + bucket := ctx.Query("bucket") + + accs, err := auth.CheckIfAccountsExist([]string{owner}, c.iam) + if err != nil { + return err + } + if len(accs) > 0 { + return fmt.Errorf("user specified as the new bucket owner does not exist") + } + + err = c.be.ChangeBucketOwner(ctx.Context(), bucket, owner) + if err != nil { + return err + } + + return ctx.Status(201).SendString("Bucket owner has been updated successfully") +} diff --git a/s3api/controllers/admin_test.go b/s3api/controllers/admin_test.go index 6996368..7384d04 100644 --- a/s3api/controllers/admin_test.go +++ b/s3api/controllers/admin_test.go @@ -15,6 +15,7 @@ package controllers import ( + "context" "fmt" "net/http" "net/http/httptest" @@ -30,7 +31,7 @@ func TestAdminController_CreateUser(t *testing.T) { } adminController := AdminController{ - IAMService: &IAMServiceMock{ + iam: &IAMServiceMock{ CreateAccountFunc: func(access string, account auth.Account) error { return nil }, @@ -109,7 +110,7 @@ func TestAdminController_DeleteUser(t *testing.T) { } adminController := AdminController{ - IAMService: &IAMServiceMock{ + iam: &IAMServiceMock{ DeleteUserAccountFunc: func(access string) error { return nil }, @@ -179,7 +180,7 @@ func TestAdminController_ListUsers(t *testing.T) { } adminController := AdminController{ - IAMService: &IAMServiceMock{ + iam: &IAMServiceMock{ ListUserAccountsFunc: func() ([]auth.UserAcc, error) { return []auth.UserAcc{}, nil }, @@ -187,7 +188,7 @@ func TestAdminController_ListUsers(t *testing.T) { } adminControllerErr := AdminController{ - IAMService: &IAMServiceMock{ + iam: &IAMServiceMock{ ListUserAccountsFunc: func() ([]auth.UserAcc, error) { return []auth.UserAcc{}, fmt.Errorf("server error") }, @@ -268,3 +269,129 @@ func TestAdminController_ListUsers(t *testing.T) { } } } + +func TestAdminController_ChangeBucketOwner(t *testing.T) { + type args struct { + req *http.Request + } + adminController := AdminController{ + be: &BackendMock{ + ChangeBucketOwnerFunc: func(contextMoqParam context.Context, bucket, newOwner string) error { + return nil + }, + }, + iam: &IAMServiceMock{ + GetUserAccountFunc: func(access string) (auth.Account, error) { + return auth.Account{}, nil + }, + }, + } + + adminControllerIamErr := AdminController{ + iam: &IAMServiceMock{ + GetUserAccountFunc: func(access string) (auth.Account, error) { + return auth.Account{}, fmt.Errorf("unknown server error") + }, + }, + } + + adminControllerIamAccDoesNotExist := AdminController{ + iam: &IAMServiceMock{ + GetUserAccountFunc: func(access string) (auth.Account, error) { + return auth.Account{}, auth.ErrNoSuchUser + }, + }, + } + + app := fiber.New() + + app.Use(func(ctx *fiber.Ctx) error { + ctx.Locals("role", "admin") + return ctx.Next() + }) + + app.Patch("/change-bucket-owner", adminController.ChangeBucketOwner) + + appRoleErr := fiber.New() + + appRoleErr.Use(func(ctx *fiber.Ctx) error { + ctx.Locals("role", "user") + return ctx.Next() + }) + + appRoleErr.Patch("/change-bucket-owner", adminController.ChangeBucketOwner) + + appIamErr := fiber.New() + + appIamErr.Use(func(ctx *fiber.Ctx) error { + ctx.Locals("role", "admin") + return ctx.Next() + }) + + appIamErr.Patch("/change-bucket-owner", adminControllerIamErr.ChangeBucketOwner) + + appIamNoSuchUser := fiber.New() + + appIamNoSuchUser.Use(func(ctx *fiber.Ctx) error { + ctx.Locals("role", "admin") + return ctx.Next() + }) + + appIamNoSuchUser.Patch("/change-bucket-owner", adminControllerIamAccDoesNotExist.ChangeBucketOwner) + + tests := []struct { + name string + app *fiber.App + args args + wantErr bool + statusCode int + }{ + { + name: "Change-bucket-owner-access-denied", + app: appRoleErr, + args: args{ + req: httptest.NewRequest(http.MethodPatch, "/change-bucket-owner", nil), + }, + wantErr: false, + statusCode: 500, + }, + { + name: "Change-bucket-owner-check-account-server-error", + app: appIamErr, + args: args{ + req: httptest.NewRequest(http.MethodPatch, "/change-bucket-owner", nil), + }, + wantErr: false, + statusCode: 500, + }, + { + name: "Change-bucket-owner-acc-does-not-exist", + app: appIamNoSuchUser, + args: args{ + req: httptest.NewRequest(http.MethodPatch, "/change-bucket-owner", nil), + }, + wantErr: false, + statusCode: 500, + }, + { + name: "Change-bucket-owner-success", + app: app, + args: args{ + req: httptest.NewRequest(http.MethodPatch, "/change-bucket-owner?bucket=bucket&owner=owner", nil), + }, + wantErr: false, + statusCode: 201, + }, + } + for _, tt := range tests { + resp, err := tt.app.Test(tt.args.req) + + if (err != nil) != tt.wantErr { + t.Errorf("AdminController.ChangeBucketOwner() error = %v, wantErr %v", err, tt.wantErr) + } + + if resp.StatusCode != tt.statusCode { + t.Errorf("AdminController.ChangeBucketOwner() statusCode = %v, wantStatusCode = %v", resp.StatusCode, tt.statusCode) + } + } +} diff --git a/s3api/controllers/backend_moq_test.go b/s3api/controllers/backend_moq_test.go index 4ace61a..f2ff431 100644 --- a/s3api/controllers/backend_moq_test.go +++ b/s3api/controllers/backend_moq_test.go @@ -25,6 +25,9 @@ var _ backend.Backend = &BackendMock{} // AbortMultipartUploadFunc: func(contextMoqParam context.Context, abortMultipartUploadInput *s3.AbortMultipartUploadInput) error { // panic("mock out the AbortMultipartUpload method") // }, +// ChangeBucketOwnerFunc: func(contextMoqParam context.Context, bucket string, newOwner string) error { +// panic("mock out the ChangeBucketOwner method") +// }, // CompleteMultipartUploadFunc: func(contextMoqParam context.Context, completeMultipartUploadInput *s3.CompleteMultipartUploadInput) (*s3.CompleteMultipartUploadOutput, error) { // panic("mock out the CompleteMultipartUpload method") // }, @@ -125,6 +128,9 @@ type BackendMock struct { // AbortMultipartUploadFunc mocks the AbortMultipartUpload method. AbortMultipartUploadFunc func(contextMoqParam context.Context, abortMultipartUploadInput *s3.AbortMultipartUploadInput) error + // ChangeBucketOwnerFunc mocks the ChangeBucketOwner method. + ChangeBucketOwnerFunc func(contextMoqParam context.Context, bucket string, newOwner string) error + // CompleteMultipartUploadFunc mocks the CompleteMultipartUpload method. CompleteMultipartUploadFunc func(contextMoqParam context.Context, completeMultipartUploadInput *s3.CompleteMultipartUploadInput) (*s3.CompleteMultipartUploadOutput, error) @@ -224,6 +230,15 @@ type BackendMock struct { // AbortMultipartUploadInput is the abortMultipartUploadInput argument value. AbortMultipartUploadInput *s3.AbortMultipartUploadInput } + // ChangeBucketOwner holds details about calls to the ChangeBucketOwner method. + ChangeBucketOwner []struct { + // ContextMoqParam is the contextMoqParam argument value. + ContextMoqParam context.Context + // Bucket is the bucket argument value. + Bucket string + // NewOwner is the newOwner argument value. + NewOwner string + } // CompleteMultipartUpload holds details about calls to the CompleteMultipartUpload method. CompleteMultipartUpload []struct { // ContextMoqParam is the contextMoqParam argument value. @@ -442,6 +457,7 @@ type BackendMock struct { } } lockAbortMultipartUpload sync.RWMutex + lockChangeBucketOwner sync.RWMutex lockCompleteMultipartUpload sync.RWMutex lockCopyObject sync.RWMutex lockCreateBucket sync.RWMutex @@ -510,6 +526,46 @@ func (mock *BackendMock) AbortMultipartUploadCalls() []struct { return calls } +// ChangeBucketOwner calls ChangeBucketOwnerFunc. +func (mock *BackendMock) ChangeBucketOwner(contextMoqParam context.Context, bucket string, newOwner string) error { + if mock.ChangeBucketOwnerFunc == nil { + panic("BackendMock.ChangeBucketOwnerFunc: method is nil but Backend.ChangeBucketOwner was just called") + } + callInfo := struct { + ContextMoqParam context.Context + Bucket string + NewOwner string + }{ + ContextMoqParam: contextMoqParam, + Bucket: bucket, + NewOwner: newOwner, + } + mock.lockChangeBucketOwner.Lock() + mock.calls.ChangeBucketOwner = append(mock.calls.ChangeBucketOwner, callInfo) + mock.lockChangeBucketOwner.Unlock() + return mock.ChangeBucketOwnerFunc(contextMoqParam, bucket, newOwner) +} + +// ChangeBucketOwnerCalls gets all the calls that were made to ChangeBucketOwner. +// Check the length with: +// +// len(mockedBackend.ChangeBucketOwnerCalls()) +func (mock *BackendMock) ChangeBucketOwnerCalls() []struct { + ContextMoqParam context.Context + Bucket string + NewOwner string +} { + var calls []struct { + ContextMoqParam context.Context + Bucket string + NewOwner string + } + mock.lockChangeBucketOwner.RLock() + calls = mock.calls.ChangeBucketOwner + mock.lockChangeBucketOwner.RUnlock() + return calls +} + // CompleteMultipartUpload calls CompleteMultipartUploadFunc. func (mock *BackendMock) CompleteMultipartUpload(contextMoqParam context.Context, completeMultipartUploadInput *s3.CompleteMultipartUploadInput) (*s3.CompleteMultipartUploadOutput, error) { if mock.CompleteMultipartUploadFunc == nil { diff --git a/s3api/router.go b/s3api/router.go index c0e52a0..d51d545 100644 --- a/s3api/router.go +++ b/s3api/router.go @@ -27,9 +27,9 @@ type S3ApiRouter struct{} func (sa *S3ApiRouter) Init(app *fiber.App, be backend.Backend, iam auth.IAMService, logger s3log.AuditLogger, evs s3event.S3EventSender) { s3ApiController := controllers.New(be, iam, logger, evs) - adminController := controllers.AdminController{IAMService: iam} + adminController := controllers.NewAdminController(iam, be) - //CreateUser admin api + // CreateUser admin api app.Patch("/create-user", adminController.CreateUser) // DeleteUsers admin api @@ -38,6 +38,9 @@ func (sa *S3ApiRouter) Init(app *fiber.App, be backend.Backend, iam auth.IAMServ // ListUsers admin api app.Patch("/list-users", adminController.ListUsers) + // ChangeBucketOwner + app.Patch("/change-bucket-owner", adminController.ChangeBucketOwner) + // ListBuckets action app.Get("/", s3ApiController.ListBuckets)