Skip to content

Commit

Permalink
Implements remove user API (#42)
Browse files Browse the repository at this point in the history
* Implementation of RemoveUser from madmin

* Added removeUser structure.

* Added removeUserResponse actions

* Added delete API to swagger

* Added tests to removeUser functions

* Removed extra space at EOF

* Changed context to be a parameter in admin_users functions

Co-authored-by: Benjamin Perez <benjamin@bexsoft.net>
  • Loading branch information
bexsoft and Benjamin Perez authored Apr 7, 2020
1 parent 2001ab6 commit 3dac86d
Show file tree
Hide file tree
Showing 10 changed files with 584 additions and 11 deletions.
50 changes: 44 additions & 6 deletions restapi/admin_users.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,21 @@ func registerUsersHandlers(api *operations.McsAPI) {
}
return admin_api.NewAddUserCreated().WithPayload(userResponse)
})
// Remove User
api.AdminAPIRemoveUserHandler = admin_api.RemoveUserHandlerFunc(func(params admin_api.RemoveUserParams, principal *models.Principal) middleware.Responder {
err := getRemoveUserResponse(params)
if err != nil {
return admin_api.NewRemoveUserDefault(500).WithPayload(&models.Error{Code: 500, Message: swag.String(err.Error())})
}
return admin_api.NewRemoveUserNoContent()
})
}

func listUsers(client MinioAdmin) ([]*models.User, error) {
func listUsers(ctx context.Context, client MinioAdmin) ([]*models.User, error) {

// Get list of all users in the MinIO
// This call requires explicit authentication, no anonymous requests are
// allowed for listing users.
ctx := context.Background()
userMap, err := client.listUsers(ctx)
if err != nil {
return []*models.User{}, err
Expand All @@ -75,6 +82,7 @@ func listUsers(client MinioAdmin) ([]*models.User, error) {

// getListUsersResponse performs listUsers() and serializes it to the handler's output
func getListUsersResponse() (*models.ListUsersResponse, error) {
ctx := context.Background()
mAdmin, err := newMAdminClient()
if err != nil {
log.Println("error creating Madmin Client:", err)
Expand All @@ -84,7 +92,7 @@ func getListUsersResponse() (*models.ListUsersResponse, error) {
// defining the client to be used
adminClient := adminClient{client: mAdmin}

users, err := listUsers(adminClient)
users, err := listUsers(ctx, adminClient)
if err != nil {
log.Println("error listing users:", err)
return nil, err
Expand All @@ -97,9 +105,8 @@ func getListUsersResponse() (*models.ListUsersResponse, error) {
}

// addUser invokes adding a users on `MinioAdmin` and builds the response `models.User`
func addUser(client MinioAdmin, accessKey, secretKey *string) (*models.User, error) {
func addUser(ctx context.Context, client MinioAdmin, accessKey, secretKey *string) (*models.User, error) {
// Calls into MinIO to add a new user if there's an error return it
ctx := context.Background()
err := client.addUser(ctx, *accessKey, *secretKey)
if err != nil {
return nil, err
Expand All @@ -113,6 +120,7 @@ func addUser(client MinioAdmin, accessKey, secretKey *string) (*models.User, err
}

func getUserAddResponse(params admin_api.AddUserParams) (*models.User, error) {
ctx := context.Background()
mAdmin, err := newMAdminClient()
if err != nil {
log.Println("error creating Madmin Client:", err)
Expand All @@ -122,10 +130,40 @@ func getUserAddResponse(params admin_api.AddUserParams) (*models.User, error) {
// defining the client to be used
adminClient := adminClient{client: mAdmin}

user, err := addUser(adminClient, params.Body.AccessKey, params.Body.SecretKey)
user, err := addUser(ctx, adminClient, params.Body.AccessKey, params.Body.SecretKey)
if err != nil {
log.Println("error adding user:", err)
return nil, err
}
return user, nil
}

//removeUser invokes removing an user on `MinioAdmin`, then we return the response from API
func removeUser(ctx context.Context, client MinioAdmin, accessKey string) error {
if err := client.removeUser(ctx, accessKey); err != nil {
return err
}
return nil
}

func getRemoveUserResponse(params admin_api.RemoveUserParams) error {
ctx := context.Background()

mAdmin, err := newMAdminClient()
if err != nil {
log.Println("error creating Madmin Client:", err)
return err
}

// create a minioClient interface implementation
// defining the client to be used
adminClient := adminClient{client: mAdmin}

if err := removeUser(ctx, adminClient, params.Name); err != nil {
log.Println("error removing user:", err)
return err
}

log.Println("User removed successfully:", params.Name)
return nil
}
45 changes: 40 additions & 5 deletions restapi/admin_users_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
// assigning mock at runtime instead of compile time
var minioListUsersMock func() (map[string]madmin.UserInfo, error)
var minioAddUserMock func(accessKey, secreyKey string) error
var minioRemoveUserMock func(accessKey string) error

// mock function of listUsers()
func (ac adminClientMock) listUsers(ctx context.Context) (map[string]madmin.UserInfo, error) {
Expand All @@ -42,9 +43,15 @@ func (ac adminClientMock) addUser(ctx context.Context, accessKey, secretKey stri
return minioAddUserMock(accessKey, secretKey)
}

// mock function of removeUser()
func (ac adminClientMock) removeUser(ctx context.Context, accessKey string) error {
return minioRemoveUserMock(accessKey)
}

func TestListUsers(t *testing.T) {
assert := asrt.New(t)
adminClient := adminClientMock{}
ctx := context.Background()
// Test-1 : listUsers() Get response from minio client with two users and return the same number on listUsers()
// mock minIO client
mockUserMap := map[string]madmin.UserInfo{
Expand All @@ -69,7 +76,7 @@ func TestListUsers(t *testing.T) {
// get list users response this response should have Name, CreationDate, Size and Access
// as part of of each user
function := "listUsers()"
userMap, err := listUsers(adminClient)
userMap, err := listUsers(ctx, adminClient)
if err != nil {
t.Errorf("Failed on %s:, error occurred: %s", function, err.Error())
}
Expand All @@ -87,7 +94,7 @@ func TestListUsers(t *testing.T) {
minioListUsersMock = func() (map[string]madmin.UserInfo, error) {
return nil, errors.New("error")
}
_, err = listUsers(adminClient)
_, err = listUsers(ctx, adminClient)
if assert.Error(err) {
assert.Equal("error", err.Error())
}
Expand All @@ -96,7 +103,7 @@ func TestListUsers(t *testing.T) {
func TestAddUser(t *testing.T) {
assert := asrt.New(t)
adminClient := adminClientMock{}

ctx := context.Background()
// Test-1: valid case of adding a user with a proper access key
accessKey := "ABCDEFGHI"
secretKey := "ABCDEFGHIABCDEFGHI"
Expand All @@ -107,7 +114,7 @@ func TestAddUser(t *testing.T) {
}
// adds a valid user to MinIO
function := "addUser()"
user, err := addUser(adminClient, &accessKey, &secretKey)
user, err := addUser(ctx, adminClient, &accessKey, &secretKey)
if err != nil {
t.Errorf("Failed on %s:, error occurred: %s", function, err.Error())
}
Expand All @@ -124,7 +131,7 @@ func TestAddUser(t *testing.T) {
return errors.New("error")
}

user, err = addUser(adminClient, &accessKey, &secretKey)
user, err = addUser(ctx, adminClient, &accessKey, &secretKey)

// no error should have been returned
assert.Nil(user, "User is not null")
Expand All @@ -134,3 +141,31 @@ func TestAddUser(t *testing.T) {
assert.Equal("error", err.Error())
}
}

func TestRemoveUser(t *testing.T) {
assert := asrt.New(t)
// mock minIO client
adminClient := adminClientMock{}
ctx := context.Background()
function := "removeUser()"

// Test-1: removeUser() delete a user
// mock function response from removeUser(accessKey)
minioRemoveUserMock = func(accessKey string) error {
return nil
}

if err := removeUser(ctx, adminClient, "ABCDEFGHI"); err != nil {
t.Errorf("Failed on %s:, error occurred: %s", function, err.Error())
}

// Test-2: removeUser() make sure errors are handled correctly when error on DeleteUser()
// mock function response from removeUser(accessKey)
minioRemoveUserMock = func(accessKey string) error {
return errors.New("error")
}

if err := removeUser(ctx, adminClient, "notexistentuser"); assert.Error(err) {
assert.Equal("error", err.Error())
}
}
6 changes: 6 additions & 0 deletions restapi/client-admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ var s3AdminNew = mcCmd.NewAdminFactory()
type MinioAdmin interface {
listUsers(ctx context.Context) (map[string]madmin.UserInfo, error)
addUser(ctx context.Context, acessKey, SecretKey string) error
removeUser(ctx context.Context, accessKey string) error
listGroups(ctx context.Context) ([]string, error)
updateGroupMembers(ctx context.Context, greq madmin.GroupAddRemove) error
getGroupDescription(ctx context.Context, group string) (*madmin.GroupDesc, error)
Expand Down Expand Up @@ -93,6 +94,11 @@ func (ac adminClient) addUser(ctx context.Context, acessKey, secretKey string) e
return ac.client.AddUser(ctx, acessKey, secretKey)
}

// implements madmin.RemoveUser()
func (ac adminClient) removeUser(ctx context.Context, accessKey string) error {
return ac.client.RemoveUser(ctx, accessKey)
}

// implements madmin.ListGroups()
func (ac adminClient) listGroups(ctx context.Context) ([]string, error) {
return ac.client.ListGroups(ctx)
Expand Down
56 changes: 56 additions & 0 deletions restapi/embedded_spec.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

90 changes: 90 additions & 0 deletions restapi/operations/admin_api/remove_user.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 3dac86d

Please sign in to comment.