From 28f9c477a8b674b15a8c96a1924db05443cefd93 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Fri, 1 May 2020 08:05:14 -0700 Subject: [PATCH] fix: assume parentUser correctly for serviceAccounts (#9504) ListServiceAccounts/DeleteServiceAccount didn't work properly with STS credentials yet due to incorrect Parent user. --- cmd/admin-handlers-users.go | 18 +++++++++++++++--- cmd/iam.go | 9 ++++----- cmd/typed-errors.go | 3 --- 3 files changed, 19 insertions(+), 11 deletions(-) diff --git a/cmd/admin-handlers-users.go b/cmd/admin-handlers-users.go index 236eb068e..74af6321d 100644 --- a/cmd/admin-handlers-users.go +++ b/cmd/admin-handlers-users.go @@ -461,7 +461,12 @@ func (a adminAPIHandlers) ListServiceAccounts(w http.ResponseWriter, r *http.Req return } - serviceAccounts, err := globalIAMSys.ListServiceAccounts(ctx, cred.AccessKey) + parentUser := cred.AccessKey + if cred.ParentUser != "" { + parentUser = cred.ParentUser + } + + serviceAccounts, err := globalIAMSys.ListServiceAccounts(ctx, parentUser) if err != nil { writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL) return @@ -521,8 +526,15 @@ func (a adminAPIHandlers) DeleteServiceAccount(w http.ResponseWriter, r *http.Re return } - if cred.AccessKey != user || cred.ParentUser != user { - // The service account belongs to another user but return not found error to mitigate brute force attacks. + parentUser := cred.AccessKey + if cred.ParentUser != "" { + parentUser = cred.ParentUser + } + + if parentUser != user || user == "" { + // The service account belongs to another user but return not + // found error to mitigate brute force attacks. or the + // serviceAccount doesn't exist. writeErrorResponseJSON(ctx, w, errorCodes.ToAPIErr(ErrServiceAccountNotFound), r.URL) return } diff --git a/cmd/iam.go b/cmd/iam.go index 0a0e747c6..90450d1eb 100644 --- a/cmd/iam.go +++ b/cmd/iam.go @@ -889,11 +889,10 @@ func (sys *IAMSys) GetServiceAccountParent(ctx context.Context, accessKey string defer sys.store.runlock() sa, ok := sys.iamUsersMap[accessKey] - if !ok || !sa.IsServiceAccount() { - return "", errNoSuchServiceAccount + if ok && sa.IsServiceAccount() { + return sa.ParentUser, nil } - - return sa.ParentUser, nil + return "", nil } // DeleteServiceAccount - delete a service account @@ -908,7 +907,7 @@ func (sys *IAMSys) DeleteServiceAccount(ctx context.Context, accessKey string) e sa, ok := sys.iamUsersMap[accessKey] if !ok || !sa.IsServiceAccount() { - return errNoSuchServiceAccount + return nil } // It is ok to ignore deletion error on the mapped policy diff --git a/cmd/typed-errors.go b/cmd/typed-errors.go index 1d77c86be..254a4641b 100644 --- a/cmd/typed-errors.go +++ b/cmd/typed-errors.go @@ -77,9 +77,6 @@ var errInvalidDecompressedSize = errors.New("Invalid Decompressed Size") // error returned in IAM subsystem when user doesn't exist. var errNoSuchUser = errors.New("Specified user does not exist") -// error returned in IAM subsystem when the service account doesn't exist. -var errNoSuchServiceAccount = errors.New("Specified service account does not exist") - // error returned in IAM subsystem when groups doesn't exist. var errNoSuchGroup = errors.New("Specified group does not exist")