Skip to content

Commit

Permalink
Merge pull request #634 from versity/fix/iam-api-resp-status
Browse files Browse the repository at this point in the history 
Admin api error response statuses
  • Loading branch information
@benmcclelland
benmcclelland authored Jun 20, 2024
2 parents be098d2 + 9853302 commit df375b7
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 20 deletions.
30 changes: 19 additions & 11 deletions s3api/controllers/admin.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package controllers
import (
"encoding/json"
"fmt"
"strings"

"github.com/gofiber/fiber/v2"
"github.com/versity/versitygw/auth"
Expand All @@ -35,31 +36,38 @@ func NewAdminController(iam auth.IAMService, be backend.Backend) AdminController
func (c AdminController) CreateUser(ctx *fiber.Ctx) error {
acct := ctx.Locals("account").(auth.Account)
if acct.Role != "admin" {
return fmt.Errorf("access denied: only admin users have access to this resource")
return ctx.Status(fiber.StatusForbidden).SendString("access denied: only admin users have access to this resource")
}
var usr auth.Account
err := json.Unmarshal(ctx.Body(), &usr)
if err != nil {
return fmt.Errorf("failed to parse request body: %w", err)
return ctx.Status(fiber.StatusBadRequest).SendString(fmt.Errorf("failed to parse request body: %w", err).Error())
}

if usr.Role != auth.RoleAdmin && usr.Role != auth.RoleUser && usr.Role != auth.RoleUserPlus {
return fmt.Errorf("invalid parameters: user role have to be one of the following: 'user', 'admin', 'userplus'")
return ctx.Status(fiber.StatusBadRequest).SendString("invalid parameters: user role have to be one of the following: 'user', 'admin', 'userplus'")
}

err = c.iam.CreateAccount(usr)
if err != nil {
return fmt.Errorf("failed to create user: %w", err)
status := fiber.StatusInternalServerError
msg := fmt.Errorf("failed to create user: %w", err).Error()

if strings.Contains(msg, "user already exists") {
status = fiber.StatusConflict
}

return ctx.Status(status).SendString(msg)
}

return ctx.SendString("The user has been created successfully")
return ctx.Status(fiber.StatusCreated).SendString("The user has been created successfully")
}

func (c AdminController) DeleteUser(ctx *fiber.Ctx) error {
access := ctx.Query("access")
acct := ctx.Locals("account").(auth.Account)
if acct.Role != "admin" {
return fmt.Errorf("access denied: only admin users have access to this resource")
return ctx.Status(fiber.StatusForbidden).SendString("access denied: only admin users have access to this resource")
}

err := c.iam.DeleteUserAccount(access)
Expand All @@ -73,7 +81,7 @@ func (c AdminController) DeleteUser(ctx *fiber.Ctx) error {
func (c AdminController) ListUsers(ctx *fiber.Ctx) error {
acct := ctx.Locals("account").(auth.Account)
if acct.Role != "admin" {
return fmt.Errorf("access denied: only admin users have access to this resource")
return ctx.Status(fiber.StatusForbidden).SendString("access denied: only admin users have access to this resource")
}
accs, err := c.iam.ListUserAccounts()
if err != nil {
Expand All @@ -86,7 +94,7 @@ func (c AdminController) ListUsers(ctx *fiber.Ctx) error {
func (c AdminController) ChangeBucketOwner(ctx *fiber.Ctx) error {
acct := ctx.Locals("account").(auth.Account)
if acct.Role != "admin" {
return fmt.Errorf("access denied: only admin users have access to this resource")
return ctx.Status(fiber.StatusForbidden).SendString("access denied: only admin users have access to this resource")
}
owner := ctx.Query("owner")
bucket := ctx.Query("bucket")
Expand All @@ -96,21 +104,21 @@ func (c AdminController) ChangeBucketOwner(ctx *fiber.Ctx) error {
return err
}
if len(accs) > 0 {
return fmt.Errorf("user specified as the new bucket owner does not exist")
return ctx.Status(fiber.StatusNotFound).SendString("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")
return ctx.SendString("Bucket owner has been updated successfully")
}

func (c AdminController) ListBuckets(ctx *fiber.Ctx) error {
acct := ctx.Locals("account").(auth.Account)
if acct.Role != "admin" {
return fmt.Errorf("access denied: only admin users have access to this resource")
return ctx.Status(fiber.StatusForbidden).SendString("access denied: only admin users have access to this resource")
}

buckets, err := c.be.ListBucketsAndOwners(ctx.Context())
Expand Down
18 changes: 9 additions & 9 deletions s3api/controllers/admin_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func TestAdminController_CreateUser(t *testing.T) {
req: httptest.NewRequest(http.MethodPatch, "/create-user", bytes.NewBuffer(succUsr)),
},
wantErr: false,
statusCode: 200,
statusCode: 201,
},
{
name: "Admin-create-user-invalid-user-role",
Expand All @@ -94,7 +94,7 @@ func TestAdminController_CreateUser(t *testing.T) {
req: httptest.NewRequest(http.MethodPatch, "/create-user", bytes.NewBuffer(user)),
},
wantErr: false,
statusCode: 500,
statusCode: 400,
},
{
name: "Admin-create-user-invalid-requester-role",
Expand All @@ -103,7 +103,7 @@ func TestAdminController_CreateUser(t *testing.T) {
req: httptest.NewRequest(http.MethodPatch, "/create-user", nil),
},
wantErr: false,
statusCode: 500,
statusCode: 403,
},
}
for _, tt := range tests {
Expand Down Expand Up @@ -173,7 +173,7 @@ func TestAdminController_DeleteUser(t *testing.T) {
req: httptest.NewRequest(http.MethodPatch, "/delete-user?access=test", nil),
},
wantErr: false,
statusCode: 500,
statusCode: 403,
},
}
for _, tt := range tests {
Expand Down Expand Up @@ -251,7 +251,7 @@ func TestAdminController_ListUsers(t *testing.T) {
req: httptest.NewRequest(http.MethodPatch, "/list-users", nil),
},
wantErr: false,
statusCode: 500,
statusCode: 403,
},
{
name: "Admin-list-users-iam-error",
Expand Down Expand Up @@ -368,7 +368,7 @@ func TestAdminController_ChangeBucketOwner(t *testing.T) {
req: httptest.NewRequest(http.MethodPatch, "/change-bucket-owner", nil),
},
wantErr: false,
statusCode: 500,
statusCode: 403,
},
{
name: "Change-bucket-owner-check-account-server-error",
Expand All @@ -386,7 +386,7 @@ func TestAdminController_ChangeBucketOwner(t *testing.T) {
req: httptest.NewRequest(http.MethodPatch, "/change-bucket-owner", nil),
},
wantErr: false,
statusCode: 500,
statusCode: 404,
},
{
name: "Change-bucket-owner-success",
Expand All @@ -395,7 +395,7 @@ func TestAdminController_ChangeBucketOwner(t *testing.T) {
req: httptest.NewRequest(http.MethodPatch, "/change-bucket-owner?bucket=bucket&owner=owner", nil),
},
wantErr: false,
statusCode: 201,
statusCode: 200,
},
}
for _, tt := range tests {
Expand Down Expand Up @@ -455,7 +455,7 @@ func TestAdminController_ListBuckets(t *testing.T) {
req: httptest.NewRequest(http.MethodPatch, "/list-buckets", nil),
},
wantErr: false,
statusCode: 500,
statusCode: 403,
},
{
name: "List-buckets-success",
Expand Down

0 comments on commit df375b7

Please sign in to comment.