diff --git a/auth/iam.go b/auth/iam.go index 0387ffb5..0a5b950f 100644 --- a/auth/iam.go +++ b/auth/iam.go @@ -28,6 +28,19 @@ const ( RoleUserPlus Role = "userplus" ) +func (r Role) IsValid() bool { + switch r { + case RoleAdmin: + return true + case RoleUser: + return true + case RoleUserPlus: + return true + default: + return false + } +} + // Account is a gateway IAM account type Account struct { Access string `json:"access"` @@ -37,6 +50,10 @@ type Account struct { GroupID int `json:"groupID"` } +type ListUserAccountsResult struct { + Accounts []Account +} + // Mutable props, which could be changed when updating an IAM account type MutableProps struct { Secret *string `json:"secret"` diff --git a/cmd/versitygw/admin.go b/cmd/versitygw/admin.go index 2f7be43b..dddc3aff 100644 --- a/cmd/versitygw/admin.go +++ b/cmd/versitygw/admin.go @@ -19,7 +19,7 @@ import ( "crypto/sha256" "crypto/tls" "encoding/hex" - "encoding/json" + "encoding/xml" "fmt" "io" "net/http" @@ -29,6 +29,7 @@ import ( "github.com/aws/aws-sdk-go-v2/aws" v4 "github.com/aws/aws-sdk-go-v2/aws/signer/v4" + "github.com/aws/smithy-go" "github.com/urfave/cli/v2" "github.com/versity/versitygw/auth" "github.com/versity/versitygw/s3response" @@ -224,19 +225,19 @@ func createUser(ctx *cli.Context) error { GroupID: groupID, } - accJson, err := json.Marshal(acc) + accxml, err := xml.Marshal(acc) if err != nil { return fmt.Errorf("failed to parse user data: %w", err) } - req, err := http.NewRequest(http.MethodPatch, fmt.Sprintf("%v/create-user", adminEndpoint), bytes.NewBuffer(accJson)) + req, err := http.NewRequest(http.MethodPatch, fmt.Sprintf("%v/create-user", adminEndpoint), bytes.NewBuffer(accxml)) if err != nil { return fmt.Errorf("failed to send the request: %w", err) } signer := v4.NewSigner() - hashedPayload := sha256.Sum256(accJson) + hashedPayload := sha256.Sum256(accxml) hexPayload := hex.EncodeToString(hashedPayload[:]) req.Header.Set("X-Amz-Content-Sha256", hexPayload) @@ -260,11 +261,9 @@ func createUser(ctx *cli.Context) error { } if resp.StatusCode >= 400 { - return fmt.Errorf("%s", body) + return parseApiError(body) } - fmt.Printf("%s\n", body) - return nil } @@ -305,11 +304,9 @@ func deleteUser(ctx *cli.Context) error { } if resp.StatusCode >= 400 { - return fmt.Errorf("%s", body) + return parseApiError(body) } - fmt.Printf("%s\n", body) - return nil } @@ -326,19 +323,19 @@ func updateUser(ctx *cli.Context) error { props.GroupID = &groupId } - propsJSON, err := json.Marshal(props) + propsxml, err := xml.Marshal(props) if err != nil { return fmt.Errorf("failed to parse user attributes: %w", err) } - req, err := http.NewRequest(http.MethodPatch, fmt.Sprintf("%v/update-user?access=%v", adminEndpoint, access), bytes.NewBuffer(propsJSON)) + req, err := http.NewRequest(http.MethodPatch, fmt.Sprintf("%v/update-user?access=%v", adminEndpoint, access), bytes.NewBuffer(propsxml)) if err != nil { return fmt.Errorf("failed to send the request: %w", err) } signer := v4.NewSigner() - hashedPayload := sha256.Sum256(propsJSON) + hashedPayload := sha256.Sum256(propsxml) hexPayload := hex.EncodeToString(hashedPayload[:]) req.Header.Set("X-Amz-Content-Sha256", hexPayload) @@ -362,11 +359,9 @@ func updateUser(ctx *cli.Context) error { } if resp.StatusCode >= 400 { - return fmt.Errorf("%s", body) + return parseApiError(body) } - fmt.Printf("%s\n", body) - return nil } @@ -402,15 +397,15 @@ func listUsers(ctx *cli.Context) error { } if resp.StatusCode >= 400 { - return fmt.Errorf("%s", body) + return parseApiError(body) } - var accs []auth.Account - if err := json.Unmarshal(body, &accs); err != nil { + var accs auth.ListUserAccountsResult + if err := xml.Unmarshal(body, &accs); err != nil { return err } - printAcctTable(accs) + printAcctTable(accs.Accounts) return nil } @@ -469,11 +464,9 @@ func changeBucketOwner(ctx *cli.Context) error { } if resp.StatusCode >= 400 { - return fmt.Errorf("%s", body) + return parseApiError(body) } - fmt.Println(string(body)) - return nil } @@ -521,15 +514,26 @@ func listBuckets(ctx *cli.Context) error { } if resp.StatusCode >= 400 { - return fmt.Errorf("%s", body) + return parseApiError(body) } - var buckets []s3response.Bucket - if err := json.Unmarshal(body, &buckets); err != nil { + var result s3response.ListBucketsResult + if err := xml.Unmarshal(body, &result); err != nil { return err } - printBuckets(buckets) + printBuckets(result.Buckets) return nil } + +func parseApiError(body []byte) error { + var apiErr smithy.GenericAPIError + err := xml.Unmarshal(body, &apiErr) + if err != nil { + apiErr.Code = "InternalServerError" + apiErr.Message = err.Error() + } + + return &apiErr +} diff --git a/metrics/actions.go b/metrics/actions.go index 36d5ad8b..4d46671a 100644 --- a/metrics/actions.go +++ b/metrics/actions.go @@ -72,6 +72,14 @@ var ( ActionPutBucketOwnershipControls = "s3_PutBucketOwnershipControls" ActionGetBucketOwnershipControls = "s3_GetBucketOwnershipControls" ActionDeleteBucketOwnershipControls = "s3_DeleteBucketOwnershipControls" + + // Admin actions + ActionAdminCreateUser = "admin_CreateUser" + ActionAdminUpdateUser = "admin_UpdateUser" + ActionAdminDeleteUser = "admin_DeleteUser" + ActionAdminChangeBucketOwner = "admin_ChangeBucketOwner" + ActionAdminListUsers = "admin_ListUsers" + ActionAdminListBuckets = "admin_ListBuckets" ) func init() { diff --git a/s3api/admin-server.go b/s3api/admin-server.go index 57290d35..ee132d74 100644 --- a/s3api/admin-server.go +++ b/s3api/admin-server.go @@ -53,6 +53,9 @@ func NewAdminServer(app *fiber.App, be backend.Backend, root middlewares.RootUse app.Use(middlewares.VerifyV4Signature(root, iam, l, nil, region, false)) app.Use(middlewares.VerifyMD5Body(l)) + // Admin role checker + app.Use(middlewares.IsAdmin(l)) + server.router.Init(app, be, iam, l) return server diff --git a/s3api/controllers/admin.go b/s3api/controllers/admin.go index 842fce5b..e56b7839 100644 --- a/s3api/controllers/admin.go +++ b/s3api/controllers/admin.go @@ -16,15 +16,19 @@ package controllers import ( "encoding/json" - "errors" + "encoding/xml" "fmt" + "net/http" "strings" "github.com/aws/aws-sdk-go-v2/service/s3/types" "github.com/gofiber/fiber/v2" "github.com/versity/versitygw/auth" "github.com/versity/versitygw/backend" + "github.com/versity/versitygw/metrics" + "github.com/versity/versitygw/s3err" "github.com/versity/versitygw/s3log" + "github.com/versity/versitygw/s3response" ) type AdminController struct { @@ -38,187 +42,124 @@ func NewAdminController(iam auth.IAMService, be backend.Backend, l s3log.AuditLo } func (c AdminController) CreateUser(ctx *fiber.Ctx) error { - acct := ctx.Locals("account").(auth.Account) - if acct.Role != "admin" { - return sendResponse(ctx, errors.New("access denied: only admin users have access to this resource"), nil, - &metaOptions{ - logger: c.l, - status: fiber.StatusForbidden, - action: "admin:CreateUser", - }) - } var usr auth.Account - err := json.Unmarshal(ctx.Body(), &usr) + err := xml.Unmarshal(ctx.Body(), &usr) if err != nil { - return sendResponse(ctx, fmt.Errorf("failed to parse request body: %w", err), nil, - &metaOptions{ - logger: c.l, - status: fiber.StatusBadRequest, - action: "admin:CreateUser", + return SendResponse(ctx, s3err.GetAPIError(s3err.ErrMalformedXML), + &MetaOpts{ + Logger: c.l, + Action: metrics.ActionAdminCreateUser, }) } - if usr.Role != auth.RoleAdmin && usr.Role != auth.RoleUser && usr.Role != auth.RoleUserPlus { - return sendResponse(ctx, errors.New("invalid parameters: user role have to be one of the following: 'user', 'admin', 'userplus'"), nil, - &metaOptions{ - logger: c.l, - status: fiber.StatusBadRequest, - action: "admin:CreateUser", + if !usr.Role.IsValid() { + return SendResponse(ctx, s3err.GetAPIError(s3err.ErrAdminInvalidUserRole), + &MetaOpts{ + Logger: c.l, + Action: metrics.ActionAdminCreateUser, }) } err = c.iam.CreateAccount(usr) if err != nil { - status := fiber.StatusInternalServerError - err = fmt.Errorf("failed to create user: %w", err) - if strings.Contains(err.Error(), "user already exists") { - status = fiber.StatusConflict + err = s3err.GetAPIError(s3err.ErrAdminUserExists) } - return sendResponse(ctx, err, nil, - &metaOptions{ - status: status, - logger: c.l, - action: "admin:CreateUser", + return SendResponse(ctx, err, + &MetaOpts{ + Logger: c.l, + Action: metrics.ActionAdminCreateUser, }) } - return sendResponse(ctx, nil, "The user has been created successfully", &metaOptions{ - status: fiber.StatusCreated, - logger: c.l, - action: "admin:CreateUser", - }) + return SendResponse(ctx, nil, + &MetaOpts{ + Logger: c.l, + Action: metrics.ActionAdminCreateUser, + Status: http.StatusCreated, + }) } func (c AdminController) UpdateUser(ctx *fiber.Ctx) error { - acct := ctx.Locals("account").(auth.Account) - if acct.Role != "admin" { - return sendResponse(ctx, errors.New("access denied: only admin users have access to this resource"), nil, - &metaOptions{ - logger: c.l, - status: fiber.StatusForbidden, - action: "admin:UpdateUser", - }) - } - access := ctx.Query("access") if access == "" { - return sendResponse(ctx, errors.New("missing user access parameter"), nil, - &metaOptions{ - status: fiber.StatusBadRequest, - logger: c.l, - action: "admin:UpdateUser", + return SendResponse(ctx, s3err.GetAPIError(s3err.ErrAdminMissingUserAcess), + &MetaOpts{ + Logger: c.l, + Action: metrics.ActionAdminUpdateUser, }) } var props auth.MutableProps - if err := json.Unmarshal(ctx.Body(), &props); err != nil { - return sendResponse(ctx, fmt.Errorf("invalid request body %w", err), nil, - &metaOptions{ - status: fiber.StatusBadRequest, - logger: c.l, - action: "admin:UpdateUser", + if err := xml.Unmarshal(ctx.Body(), &props); err != nil { + return SendResponse(ctx, s3err.GetAPIError(s3err.ErrMalformedXML), + &MetaOpts{ + Logger: c.l, + Action: metrics.ActionAdminUpdateUser, }) } err := c.iam.UpdateUserAccount(access, props) if err != nil { - status := fiber.StatusInternalServerError - err = fmt.Errorf("failed to update user account: %w", err) - if strings.Contains(err.Error(), "user not found") { - status = fiber.StatusNotFound + err = s3err.GetAPIError(s3err.ErrAdminUserNotFound) } - return sendResponse(ctx, err, nil, - &metaOptions{ - status: status, - logger: c.l, - action: "admin:UpdateUser", + return SendResponse(ctx, err, + &MetaOpts{ + Logger: c.l, + Action: metrics.ActionAdminUpdateUser, }) } - return sendResponse(ctx, nil, "the user has been updated successfully", - &metaOptions{ - logger: c.l, - action: "admin:UpdateUser", + return SendResponse(ctx, nil, + &MetaOpts{ + Logger: c.l, + Action: metrics.ActionAdminUpdateUser, }) } func (c AdminController) DeleteUser(ctx *fiber.Ctx) error { access := ctx.Query("access") - acct := ctx.Locals("account").(auth.Account) - if acct.Role != "admin" { - return sendResponse(ctx, errors.New("access denied: only admin users have access to this resource"), nil, - &metaOptions{ - logger: c.l, - status: fiber.StatusForbidden, - action: "admin:DeleteUser", - }) - } err := c.iam.DeleteUserAccount(access) - if err != nil { - return sendResponse(ctx, err, nil, - &metaOptions{ - logger: c.l, - action: "admin:DeleteUser", - }) - } - - return sendResponse(ctx, nil, "The user has been deleted successfully", - &metaOptions{ - logger: c.l, - action: "admin:DeleteUser", + return SendResponse(ctx, err, + &MetaOpts{ + Logger: c.l, + Action: metrics.ActionAdminDeleteUser, }) } func (c AdminController) ListUsers(ctx *fiber.Ctx) error { - acct := ctx.Locals("account").(auth.Account) - if acct.Role != "admin" { - return sendResponse(ctx, errors.New("access denied: only admin users have access to this resource"), nil, - &metaOptions{ - logger: c.l, - status: fiber.StatusForbidden, - action: "admin:ListUsers", - }) - } accs, err := c.iam.ListUserAccounts() - return sendResponse(ctx, err, accs, - &metaOptions{ - logger: c.l, - action: "admin:ListUsers", + return SendXMLResponse(ctx, + auth.ListUserAccountsResult{ + Accounts: accs, + }, err, + &MetaOpts{ + Logger: c.l, + Action: metrics.ActionAdminListUsers, }) } func (c AdminController) ChangeBucketOwner(ctx *fiber.Ctx) error { - acct := ctx.Locals("account").(auth.Account) - if acct.Role != "admin" { - return sendResponse(ctx, errors.New("access denied: only admin users have access to this resource"), nil, - &metaOptions{ - logger: c.l, - status: fiber.StatusForbidden, - action: "admin:ChangeBucketOwner", - }) - } owner := ctx.Query("owner") bucket := ctx.Query("bucket") accs, err := auth.CheckIfAccountsExist([]string{owner}, c.iam) if err != nil { - return sendResponse(ctx, err, nil, - &metaOptions{ - logger: c.l, - action: "admin:ChangeBucketOwner", + return SendResponse(ctx, err, + &MetaOpts{ + Logger: c.l, + Action: metrics.ActionAdminChangeBucketOwner, }) } if len(accs) > 0 { - return sendResponse(ctx, errors.New("user specified as the new bucket owner does not exist"), nil, - &metaOptions{ - logger: c.l, - action: "admin:ChangeBucketOwner", - status: fiber.StatusNotFound, + return SendResponse(ctx, s3err.GetAPIError(s3err.ErrAdminUserNotFound), + &MetaOpts{ + Logger: c.l, + Action: metrics.ActionAdminChangeBucketOwner, }) } @@ -235,91 +176,28 @@ func (c AdminController) ChangeBucketOwner(ctx *fiber.Ctx) error { aclParsed, err := json.Marshal(acl) if err != nil { - return sendResponse(ctx, fmt.Errorf("failed to marshal the bucket acl: %w", err), nil, - &metaOptions{ - logger: c.l, - action: "admin:ChangeBucketOwner", + return SendResponse(ctx, fmt.Errorf("failed to marshal the bucket acl: %w", err), + &MetaOpts{ + Logger: c.l, + Action: metrics.ActionAdminChangeBucketOwner, }) } err = c.be.ChangeBucketOwner(ctx.Context(), bucket, aclParsed) - return sendResponse(ctx, err, "Bucket owner has been updated successfully", - &metaOptions{ - logger: c.l, - action: "admin:ChangeBucketOwner", + return SendResponse(ctx, err, + &MetaOpts{ + Logger: c.l, + Action: metrics.ActionAdminChangeBucketOwner, }) } func (c AdminController) ListBuckets(ctx *fiber.Ctx) error { - acct := ctx.Locals("account").(auth.Account) - if acct.Role != "admin" { - return sendResponse(ctx, errors.New("access denied: only admin users have access to this resource"), nil, - &metaOptions{ - logger: c.l, - status: fiber.StatusForbidden, - action: "admin:ListBuckets", - }) - } - buckets, err := c.be.ListBucketsAndOwners(ctx.Context()) - return sendResponse(ctx, err, buckets, - &metaOptions{ - logger: c.l, - action: "admin:ListBuckets", - }) -} - -type metaOptions struct { - action string - status int - logger s3log.AuditLogger -} - -func sendResponse(ctx *fiber.Ctx, err error, data any, m *metaOptions) error { - status := m.status - if err != nil { - if status == 0 { - status = fiber.StatusInternalServerError - } - if m.logger != nil { - m.logger.Log(ctx, err, []byte(err.Error()), s3log.LogMeta{ - Action: m.action, - HttpStatus: status, - }) - } - - return ctx.Status(status).SendString(err.Error()) - } - - if status == 0 { - status = fiber.StatusOK - } - - msg, ok := data.(string) - if ok { - if m.logger != nil { - m.logger.Log(ctx, nil, []byte(msg), s3log.LogMeta{ - Action: m.action, - HttpStatus: status, - }) - } - - return ctx.Status(status).SendString(msg) - } - - dataJSON, err := json.Marshal(data) - if err != nil { - return err - } - - if m.logger != nil { - m.logger.Log(ctx, nil, dataJSON, s3log.LogMeta{ - HttpStatus: status, - Action: m.action, + return SendXMLResponse(ctx, + s3response.ListBucketsResult{ + Buckets: buckets, + }, err, &MetaOpts{ + Logger: c.l, + Action: metrics.ActionAdminListBuckets, }) - } - - ctx.Set(fiber.HeaderContentType, fiber.MIMEApplicationJSON) - - return ctx.Status(status).Send(dataJSON) } diff --git a/s3api/controllers/admin_test.go b/s3api/controllers/admin_test.go index 7ef1b78a..3382461a 100644 --- a/s3api/controllers/admin_test.go +++ b/s3api/controllers/admin_test.go @@ -15,12 +15,11 @@ package controllers import ( - "bytes" "context" - "encoding/json" "fmt" "net/http" "net/http/httptest" + "strings" "testing" "github.com/gofiber/fiber/v2" @@ -43,33 +42,26 @@ func TestAdminController_CreateUser(t *testing.T) { app := fiber.New() - app.Use(func(ctx *fiber.Ctx) error { - ctx.Locals("account", auth.Account{Access: "admin1", Secret: "secret", Role: "admin"}) - return ctx.Next() - }) - app.Patch("/create-user", adminController.CreateUser) - appErr := fiber.New() - - appErr.Use(func(ctx *fiber.Ctx) error { - ctx.Locals("account", auth.Account{Access: "user1", Secret: "secret", Role: "user"}) - return ctx.Next() - }) - - usr := auth.Account{ - Access: "access", - Secret: "secret", - Role: "invalid role", - } - - user, _ := json.Marshal(&usr) - - usr.Role = "admin" - - succUsr, _ := json.Marshal(&usr) - - appErr.Patch("/create-user", adminController.CreateUser) + succUser := ` + + access + secret + admin + 0 + 0 + + ` + invuser := ` + + access + secret + invalid_role + 0 + 0 + + ` tests := []struct { name string @@ -79,31 +71,31 @@ func TestAdminController_CreateUser(t *testing.T) { statusCode int }{ { - name: "Admin-create-user-success", + name: "Admin-create-user-malformed-body", app: app, args: args{ - req: httptest.NewRequest(http.MethodPatch, "/create-user", bytes.NewBuffer(succUsr)), + req: httptest.NewRequest(http.MethodPatch, "/create-user", nil), }, wantErr: false, - statusCode: 201, + statusCode: 400, }, { - name: "Admin-create-user-invalid-user-role", + name: "Admin-create-user-invalid-requester-role", app: app, args: args{ - req: httptest.NewRequest(http.MethodPatch, "/create-user", bytes.NewBuffer(user)), + req: httptest.NewRequest(http.MethodPatch, "/create-user", strings.NewReader(invuser)), }, wantErr: false, statusCode: 400, }, { - name: "Admin-create-user-invalid-requester-role", - app: appErr, + name: "Admin-create-user-success", + app: app, args: args{ - req: httptest.NewRequest(http.MethodPatch, "/create-user", nil), + req: httptest.NewRequest(http.MethodPatch, "/create-user", strings.NewReader(succUser)), }, wantErr: false, - statusCode: 403, + statusCode: 201, }, } for _, tt := range tests { @@ -134,24 +126,8 @@ func TestAdminController_UpdateUser(t *testing.T) { app := fiber.New() - app.Use(func(ctx *fiber.Ctx) error { - ctx.Locals("account", auth.Account{Access: "admin1", Secret: "secret", Role: "admin"}) - return ctx.Next() - }) - app.Patch("/update-user", adminController.UpdateUser) - appErr := fiber.New() - - appErr.Use(func(ctx *fiber.Ctx) error { - ctx.Locals("account", auth.Account{Access: "user1", Secret: "secret", Role: "user"}) - return ctx.Next() - }) - - appErr.Patch("/update-user", adminController.UpdateUser) - - successBody, _ := json.Marshal(auth.MutableProps{Secret: getPtr("hello")}) - adminControllerErr := AdminController{ iam: &IAMServiceMock{ UpdateUserAccountFunc: func(access string, props auth.MutableProps) error { @@ -162,13 +138,16 @@ func TestAdminController_UpdateUser(t *testing.T) { appNotFound := fiber.New() - appNotFound.Use(func(ctx *fiber.Ctx) error { - ctx.Locals("account", auth.Account{Access: "admin1", Secret: "secret", Role: "admin"}) - return ctx.Next() - }) - appNotFound.Patch("/update-user", adminControllerErr.UpdateUser) + succUser := ` + + secret + 0 + 0 + + ` + tests := []struct { name string app *fiber.App @@ -180,7 +159,7 @@ func TestAdminController_UpdateUser(t *testing.T) { name: "Admin-update-user-success", app: app, args: args{ - req: httptest.NewRequest(http.MethodPatch, "/update-user?access=access", bytes.NewBuffer(successBody)), + req: httptest.NewRequest(http.MethodPatch, "/update-user?access=access", strings.NewReader(succUser)), }, wantErr: false, statusCode: 200, @@ -189,10 +168,10 @@ func TestAdminController_UpdateUser(t *testing.T) { name: "Admin-update-user-missing-access", app: app, args: args{ - req: httptest.NewRequest(http.MethodPatch, "/update-user", bytes.NewBuffer(successBody)), + req: httptest.NewRequest(http.MethodPatch, "/update-user", strings.NewReader(succUser)), }, wantErr: false, - statusCode: 400, + statusCode: 404, }, { name: "Admin-update-user-invalid-request-body", @@ -203,20 +182,11 @@ func TestAdminController_UpdateUser(t *testing.T) { wantErr: false, statusCode: 400, }, - { - name: "Admin-update-user-invalid-requester-role", - app: appErr, - args: args{ - req: httptest.NewRequest(http.MethodPatch, "/update-user?access=access", nil), - }, - wantErr: false, - statusCode: 403, - }, { name: "Admin-update-user-not-found", app: appNotFound, args: args{ - req: httptest.NewRequest(http.MethodPatch, "/update-user?access=access", bytes.NewBuffer(successBody)), + req: httptest.NewRequest(http.MethodPatch, "/update-user?access=access", strings.NewReader(succUser)), }, wantErr: false, statusCode: 404, @@ -250,22 +220,8 @@ func TestAdminController_DeleteUser(t *testing.T) { app := fiber.New() - app.Use(func(ctx *fiber.Ctx) error { - ctx.Locals("account", auth.Account{Access: "admin1", Secret: "secret", Role: "admin"}) - return ctx.Next() - }) - app.Patch("/delete-user", adminController.DeleteUser) - appErr := fiber.New() - - appErr.Use(func(ctx *fiber.Ctx) error { - ctx.Locals("account", auth.Account{Access: "user1", Secret: "secret", Role: "user"}) - return ctx.Next() - }) - - appErr.Patch("/delete-user", adminController.DeleteUser) - tests := []struct { name string app *fiber.App @@ -282,15 +238,6 @@ func TestAdminController_DeleteUser(t *testing.T) { wantErr: false, statusCode: 200, }, - { - name: "Admin-delete-user-invalid-requester-role", - app: appErr, - args: args{ - req: httptest.NewRequest(http.MethodPatch, "/delete-user?access=test", nil), - }, - wantErr: false, - statusCode: 403, - }, } for _, tt := range tests { resp, err := tt.app.Test(tt.args.req) @@ -327,30 +274,9 @@ func TestAdminController_ListUsers(t *testing.T) { } appErr := fiber.New() - - appErr.Use(func(ctx *fiber.Ctx) error { - ctx.Locals("account", auth.Account{Access: "admin1", Secret: "secret", Role: "admin"}) - return ctx.Next() - }) - appErr.Patch("/list-users", adminControllerErr.ListUsers) - appRoleErr := fiber.New() - - appRoleErr.Use(func(ctx *fiber.Ctx) error { - ctx.Locals("account", auth.Account{Access: "user1", Secret: "secret", Role: "user"}) - return ctx.Next() - }) - - appRoleErr.Patch("/list-users", adminController.ListUsers) - appSucc := fiber.New() - - appSucc.Use(func(ctx *fiber.Ctx) error { - ctx.Locals("account", auth.Account{Access: "admin1", Secret: "secret", Role: "admin"}) - return ctx.Next() - }) - appSucc.Patch("/list-users", adminController.ListUsers) tests := []struct { @@ -360,15 +286,6 @@ func TestAdminController_ListUsers(t *testing.T) { wantErr bool statusCode int }{ - { - name: "Admin-list-users-access-denied", - app: appRoleErr, - args: args{ - req: httptest.NewRequest(http.MethodPatch, "/list-users", nil), - }, - wantErr: false, - statusCode: 403, - }, { name: "Admin-list-users-iam-error", app: appErr, @@ -435,39 +352,12 @@ func TestAdminController_ChangeBucketOwner(t *testing.T) { } app := fiber.New() - - app.Use(func(ctx *fiber.Ctx) error { - ctx.Locals("account", auth.Account{Access: "admin1", Secret: "secret", Role: "admin"}) - return ctx.Next() - }) - app.Patch("/change-bucket-owner", adminController.ChangeBucketOwner) - appRoleErr := fiber.New() - - appRoleErr.Use(func(ctx *fiber.Ctx) error { - ctx.Locals("account", auth.Account{Access: "user1", Secret: "secret", Role: "user"}) - return ctx.Next() - }) - - appRoleErr.Patch("/change-bucket-owner", adminController.ChangeBucketOwner) - appIamErr := fiber.New() - - appIamErr.Use(func(ctx *fiber.Ctx) error { - ctx.Locals("account", auth.Account{Access: "admin1", Secret: "secret", Role: "admin"}) - return ctx.Next() - }) - appIamErr.Patch("/change-bucket-owner", adminControllerIamErr.ChangeBucketOwner) appIamNoSuchUser := fiber.New() - - appIamNoSuchUser.Use(func(ctx *fiber.Ctx) error { - ctx.Locals("account", auth.Account{Access: "admin1", Secret: "secret", Role: "admin"}) - return ctx.Next() - }) - appIamNoSuchUser.Patch("/change-bucket-owner", adminControllerIamAccDoesNotExist.ChangeBucketOwner) tests := []struct { @@ -477,15 +367,6 @@ func TestAdminController_ChangeBucketOwner(t *testing.T) { 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: 403, - }, { name: "Change-bucket-owner-check-account-server-error", app: appIamErr, @@ -540,23 +421,8 @@ func TestAdminController_ListBuckets(t *testing.T) { } app := fiber.New() - - app.Use(func(ctx *fiber.Ctx) error { - ctx.Locals("account", auth.Account{Access: "admin1", Secret: "secret", Role: "admin"}) - return ctx.Next() - }) - app.Patch("/list-buckets", adminController.ListBuckets) - appRoleErr := fiber.New() - - appRoleErr.Use(func(ctx *fiber.Ctx) error { - ctx.Locals("account", auth.Account{Access: "user1", Secret: "secret", Role: "user"}) - return ctx.Next() - }) - - appRoleErr.Patch("/list-buckets", adminController.ListBuckets) - tests := []struct { name string app *fiber.App @@ -564,15 +430,6 @@ func TestAdminController_ListBuckets(t *testing.T) { wantErr bool statusCode int }{ - { - name: "List-buckets-incorrect-role", - app: appRoleErr, - args: args{ - req: httptest.NewRequest(http.MethodPatch, "/list-buckets", nil), - }, - wantErr: false, - statusCode: 403, - }, { name: "List-buckets-success", app: app, diff --git a/s3api/middlewares/admin.go b/s3api/middlewares/admin.go new file mode 100644 index 00000000..ad799a6f --- /dev/null +++ b/s3api/middlewares/admin.go @@ -0,0 +1,59 @@ +// Copyright 2023 Versity Software +// This file is licensed under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package middlewares + +import ( + "strings" + + "github.com/gofiber/fiber/v2" + "github.com/versity/versitygw/auth" + "github.com/versity/versitygw/metrics" + "github.com/versity/versitygw/s3api/controllers" + "github.com/versity/versitygw/s3err" + "github.com/versity/versitygw/s3log" +) + +func IsAdmin(logger s3log.AuditLogger) fiber.Handler { + return func(ctx *fiber.Ctx) error { + acct := ctx.Locals("account").(auth.Account) + if acct.Role != auth.RoleAdmin { + path := ctx.Path() + return controllers.SendResponse(ctx, s3err.GetAPIError(s3err.ErrAdminAccessDenied), + &controllers.MetaOpts{ + Logger: logger, + Action: detectAction(path), + }) + } + + return ctx.Next() + } +} + +func detectAction(path string) (action string) { + if strings.Contains(path, "create-user") { + action = metrics.ActionAdminCreateUser + } else if strings.Contains(path, "update-user") { + action = metrics.ActionAdminUpdateUser + } else if strings.Contains(path, "delete-user") { + action = metrics.ActionAdminDeleteUser + } else if strings.Contains(path, "list-user") { + action = metrics.ActionAdminListUsers + } else if strings.Contains(path, "list-buckets") { + action = metrics.ActionAdminListBuckets + } else if strings.Contains(path, "change-bucket-owner") { + action = metrics.ActionAdminChangeBucketOwner + } + return action +} diff --git a/s3api/router.go b/s3api/router.go index 23b43d16..448bf667 100644 --- a/s3api/router.go +++ b/s3api/router.go @@ -20,6 +20,7 @@ import ( "github.com/versity/versitygw/backend" "github.com/versity/versitygw/metrics" "github.com/versity/versitygw/s3api/controllers" + "github.com/versity/versitygw/s3api/middlewares" "github.com/versity/versitygw/s3event" "github.com/versity/versitygw/s3log" ) @@ -35,22 +36,22 @@ func (sa *S3ApiRouter) Init(app *fiber.App, be backend.Backend, iam auth.IAMServ adminController := controllers.NewAdminController(iam, be, aLogger) // CreateUser admin api - app.Patch("/create-user", adminController.CreateUser) + app.Patch("/create-user", middlewares.IsAdmin(logger), adminController.CreateUser) // DeleteUsers admin api - app.Patch("/delete-user", adminController.DeleteUser) + app.Patch("/delete-user", middlewares.IsAdmin(logger), adminController.DeleteUser) // UpdateUser admin api - app.Patch("update-user", adminController.UpdateUser) + app.Patch("update-user", middlewares.IsAdmin(logger), adminController.UpdateUser) // ListUsers admin api - app.Patch("/list-users", adminController.ListUsers) + app.Patch("/list-users", middlewares.IsAdmin(logger), adminController.ListUsers) // ChangeBucketOwner admin api - app.Patch("/change-bucket-owner", adminController.ChangeBucketOwner) + app.Patch("/change-bucket-owner", middlewares.IsAdmin(logger), adminController.ChangeBucketOwner) // ListBucketsAndOwners admin api - app.Patch("/list-buckets", adminController.ListBuckets) + app.Patch("/list-buckets", middlewares.IsAdmin(logger), adminController.ListBuckets) } // ListBuckets action diff --git a/s3err/s3err.go b/s3err/s3err.go index 8153a1af..8c62ea2e 100644 --- a/s3err/s3err.go +++ b/s3err/s3err.go @@ -143,6 +143,13 @@ const ( ErrDirectoryObjectContainsData ErrQuotaExceeded ErrVersioningNotConfigured + + // Admin api errors + ErrAdminAccessDenied + ErrAdminUserNotFound + ErrAdminUserExists + ErrAdminInvalidUserRole + ErrAdminMissingUserAcess ) var errorCodeResponse = map[ErrorCode]APIError{ @@ -578,6 +585,33 @@ var errorCodeResponse = map[ErrorCode]APIError{ Description: "Versioning has not been configured for the gateway.", HTTPStatusCode: http.StatusNotImplemented, }, + + // Admin api errors + ErrAdminAccessDenied: { + Code: "XAdminAccessDenied", + Description: "Only admin users have access to this resource.", + HTTPStatusCode: http.StatusForbidden, + }, + ErrAdminUserNotFound: { + Code: "XAdminUserNotFound", + Description: "No user exists with the provided access key ID.", + HTTPStatusCode: http.StatusNotFound, + }, + ErrAdminUserExists: { + Code: "XAdminUserExists", + Description: "A user with the provided access key ID already exists.", + HTTPStatusCode: http.StatusConflict, + }, + ErrAdminInvalidUserRole: { + Code: "XAdminInvalidArgument", + Description: "User role has to be one of the following: 'user', 'admin', 'userplus'.", + HTTPStatusCode: http.StatusBadRequest, + }, + ErrAdminMissingUserAcess: { + Code: "XAdminInvalidArgument", + Description: "User access key ID is missing.", + HTTPStatusCode: http.StatusNotFound, + }, } // GetAPIError provides API Error for input API error code. diff --git a/s3response/s3response.go b/s3response/s3response.go index e2e2163b..c84ddb1d 100644 --- a/s3response/s3response.go +++ b/s3response/s3response.go @@ -448,3 +448,8 @@ func (AmzDate) ISO8601Parse(date string) (t time.Time, err error) { return t, err } + +// Admin api response types +type ListBucketsResult struct { + Buckets []Bucket +} diff --git a/tests/integration/tests.go b/tests/integration/tests.go index 834753a2..448d933b 100644 --- a/tests/integration/tests.go +++ b/tests/integration/tests.go @@ -2744,7 +2744,6 @@ func PutObject_invalid_long_tags(s *S3Conf) error { Tagging: &tagging, }) cancel() - if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrInvalidTag)); err != nil { return err } @@ -10421,9 +10420,9 @@ func IAM_user_access_denied(s *S3Conf) error { failF("%v: expected cmd error", testName) return fmt.Errorf("%v: expected cmd error", testName) } - if !strings.Contains(string(out), adminAccessDeniedMsg) { - failF("%v: expected response error message to be %v, instead got %s", testName, adminAccessDeniedMsg, out) - return fmt.Errorf("%v: expected response error message to be %v, instead got %s", testName, adminAccessDeniedMsg, out) + if !strings.Contains(string(out), s3err.GetAPIError(s3err.ErrAdminAccessDenied).Code) { + failF("%v: expected response error message to be %v, instead got %s", testName, s3err.GetAPIError(s3err.ErrAdminAccessDenied).Error(), out) + return fmt.Errorf("%v: expected response error message to be %v, instead got %s", testName, s3err.GetAPIError(s3err.ErrAdminAccessDenied).Error(), out) } passF(testName) @@ -10452,9 +10451,9 @@ func IAM_userplus_access_denied(s *S3Conf) error { failF("%v: expected cmd error", testName) return fmt.Errorf("%v: expected cmd error", testName) } - if !strings.Contains(string(out), adminAccessDeniedMsg) { - failF("%v: expected response error message to be %v, instead got %s", testName, adminAccessDeniedMsg, out) - return fmt.Errorf("%v: expected response error message to be %v, instead got %s", testName, adminAccessDeniedMsg, out) + if !strings.Contains(string(out), s3err.GetAPIError(s3err.ErrAdminAccessDenied).Code) { + failF("%v: expected response error message to be %v, instead got %s", testName, s3err.GetAPIError(s3err.ErrAdminAccessDenied).Error(), out) + return fmt.Errorf("%v: expected response error message to be %v, instead got %s", testName, s3err.GetAPIError(s3err.ErrAdminAccessDenied).Error(), out) } passF(testName) diff --git a/tests/integration/utils.go b/tests/integration/utils.go index b9a33e84..24301105 100644 --- a/tests/integration/utils.go +++ b/tests/integration/utils.go @@ -43,11 +43,8 @@ import ( ) var ( - bcktCount = 0 - succUsrCrt = "The user has been created successfully" - failUsrCrt = "failed to create user: update iam data: account already exists" - adminAccessDeniedMsg = "access denied: only admin users have access to this resource" - succDeleteUserMsg = "The user has been deleted successfully" + bcktCount = 0 + adminErrorPrefix = "XAdmin" ) func getBucketName() string { @@ -765,8 +762,8 @@ func createUsers(s *S3Conf, users []user) error { if err != nil { return err } - if !strings.Contains(string(out), succUsrCrt) && !strings.Contains(string(out), failUsrCrt) { - return fmt.Errorf("failed to create user account") + if strings.Contains(string(out), adminErrorPrefix) { + return fmt.Errorf("failed to create user account: %s", out) } } return nil @@ -777,8 +774,8 @@ func deleteUser(s *S3Conf, access string) error { if err != nil { return err } - if !strings.Contains(string(out), succDeleteUserMsg) { - return fmt.Errorf("failed to delete the user account") + if strings.Contains(string(out), adminErrorPrefix) { + return fmt.Errorf("failed to delete the user account, %s", out) } return nil @@ -790,8 +787,8 @@ func changeBucketsOwner(s *S3Conf, buckets []string, owner string) error { if err != nil { return err } - if !strings.Contains(string(out), "Bucket owner has been updated successfully") { - return fmt.Errorf("%v", string(out)) + if strings.Contains(string(out), adminErrorPrefix) { + return fmt.Errorf("failed to change the bucket owner: %s", out) } }