diff --git a/cmd/admin-handlers-users.go b/cmd/admin-handlers-users.go index 5a0d7da11..43de2bdb8 100644 --- a/cmd/admin-handlers-users.go +++ b/cmd/admin-handlers-users.go @@ -1014,11 +1014,9 @@ func (a adminAPIHandlers) DeleteServiceAccount(w http.ResponseWriter, r *http.Re return } - svcAccount, _, err := globalIAMSys.GetServiceAccount(ctx, serviceAccount) - if err != nil { - writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL) - return - } + // We do not care if service account is readable or not at this point, + // since this is a delete call we shall allow it to be deleted if possible. + svcAccount, _, _ := globalIAMSys.GetServiceAccount(ctx, serviceAccount) adminPrivilege := globalIAMSys.IsAllowed(iampolicy.Args{ AccountName: cred.AccessKey, @@ -1033,7 +1031,7 @@ func (a adminAPIHandlers) DeleteServiceAccount(w http.ResponseWriter, r *http.Re if cred.ParentUser != "" { parentUser = cred.ParentUser } - if parentUser != svcAccount.ParentUser { + if svcAccount.ParentUser != "" && parentUser != svcAccount.ParentUser { // The service account belongs to another user but return not // found error to mitigate brute force attacks. or the // serviceAccount doesn't exist. @@ -1042,23 +1040,21 @@ func (a adminAPIHandlers) DeleteServiceAccount(w http.ResponseWriter, r *http.Re } } - err = globalIAMSys.DeleteServiceAccount(ctx, serviceAccount, true) - if err != nil { + if err := globalIAMSys.DeleteServiceAccount(ctx, serviceAccount, true); err != nil { writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL) return } // Call site replication hook - non-root user accounts are replicated. - if svcAccount.ParentUser != globalActiveCred.AccessKey { - err = globalSiteReplicationSys.IAMChangeHook(ctx, madmin.SRIAMItem{ + if svcAccount.ParentUser != "" && svcAccount.ParentUser != globalActiveCred.AccessKey { + if err := globalSiteReplicationSys.IAMChangeHook(ctx, madmin.SRIAMItem{ Type: madmin.SRIAMItemSvcAcc, SvcAccChange: &madmin.SRSvcAccChange{ Delete: &madmin.SRSvcAccDelete{ AccessKey: serviceAccount, }, }, - }) - if err != nil { + }); err != nil { writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL) return } diff --git a/cmd/auth-handler.go b/cmd/auth-handler.go index 5bfb332b3..5fa945a31 100644 --- a/cmd/auth-handler.go +++ b/cmd/auth-handler.go @@ -235,7 +235,7 @@ func getClaimsFromTokenWithSecret(token, secret string) (map[string]interface{}, logger.LogIf(GlobalContext, err, logger.Application) return nil, errAuthentication } - claims.MapClaims[iampolicy.SessionPolicyName] = string(spBytes) + claims.MapClaims[sessionPolicyNameExtracted] = string(spBytes) } return claims.Map(), nil diff --git a/cmd/iam-store.go b/cmd/iam-store.go index afde1a5b4..4deeba2c8 100644 --- a/cmd/iam-store.go +++ b/cmd/iam-store.go @@ -1684,6 +1684,17 @@ func (store *IAMStoreSys) UpdateServiceAccount(ctx context.Context, accessKey st return fmt.Errorf("unable to get svc acc claims: %v", err) } + // Extracted session policy name string can be removed as its not useful + // at this point. + delete(m, sessionPolicyNameExtracted) + + // sessionPolicy is nil and there is embedded policy attached we remove + // rembedded policy at that point. + if _, ok := m[iampolicy.SessionPolicyName]; ok && opts.sessionPolicy == nil { + delete(m, iampolicy.SessionPolicyName) + m[iamPolicyClaimNameSA()] = inheritedPolicyType + } + if opts.sessionPolicy != nil { if err := opts.sessionPolicy.Validate(); err != nil { return err @@ -1700,7 +1711,7 @@ func (store *IAMStoreSys) UpdateServiceAccount(ctx context.Context, accessKey st // Overwrite session policy claims. m[iampolicy.SessionPolicyName] = base64.StdEncoding.EncodeToString(policyBuf) - m[iamPolicyClaimNameSA()] = "embedded-policy" + m[iamPolicyClaimNameSA()] = embeddedPolicyType } cr.SessionToken, err = auth.JWTSignWithAccessKey(accessKey, m, cr.SecretKey) diff --git a/cmd/iam.go b/cmd/iam.go index 11233f83b..791380aad 100644 --- a/cmd/iam.go +++ b/cmd/iam.go @@ -63,6 +63,11 @@ const ( statusDisabled = "disabled" ) +const ( + embeddedPolicyType = "embedded-policy" + inheritedPolicyType = "inherited-policy" +) + // IAMSys - config system. type IAMSys struct { // Need to keep them here to keep alignment - ref: https://golang.org/pkg/sync/atomic/#pkg-note-BUG @@ -866,9 +871,9 @@ func (sys *IAMSys) NewServiceAccount(ctx context.Context, parentUser string, gro if len(policyBuf) > 0 { m[iampolicy.SessionPolicyName] = base64.StdEncoding.EncodeToString(policyBuf) - m[iamPolicyClaimNameSA()] = "embedded-policy" + m[iamPolicyClaimNameSA()] = embeddedPolicyType } else { - m[iamPolicyClaimNameSA()] = "inherited-policy" + m[iamPolicyClaimNameSA()] = inheritedPolicyType } // Add all the necessary claims for the service accounts. @@ -989,7 +994,7 @@ func (sys *IAMSys) getServiceAccount(ctx context.Context, accessKey string) (aut } pt, ptok := jwtClaims.Lookup(iamPolicyClaimNameSA()) sp, spok := jwtClaims.Lookup(iampolicy.SessionPolicyName) - if ptok && spok && pt == "embedded-policy" { + if ptok && spok && pt == embeddedPolicyType { policyBytes, err := base64.StdEncoding.DecodeString(sp) if err != nil { return auth.Credentials{}, nil, err @@ -1417,6 +1422,8 @@ func (sys *IAMSys) PolicyDBGet(name string, isGroup bool, groups ...string) ([]s return sys.store.PolicyDBGet(name, isGroup, groups...) } +const sessionPolicyNameExtracted = iampolicy.SessionPolicyName + "-extracted" + // IsAllowedServiceAccount - checks if the given service account is allowed to perform // actions. The permission of the parent user is checked first func (sys *IAMSys) IsAllowedServiceAccount(args iampolicy.Args, parentUser string) bool { @@ -1493,12 +1500,12 @@ func (sys *IAMSys) IsAllowedServiceAccount(args iampolicy.Args, parentUser strin return false } - if saPolicyClaimStr == "inherited-policy" { + if saPolicyClaimStr == inheritedPolicyType { return combinedPolicy.IsAllowed(parentArgs) } // Now check if we have a sessionPolicy. - spolicy, ok := args.Claims[iampolicy.SessionPolicyName] + spolicy, ok := args.Claims[sessionPolicyNameExtracted] if !ok { return false } @@ -1646,7 +1653,7 @@ func isAllowedBySessionPolicy(args iampolicy.Args) (hasSessionPolicy bool, isAll isAllowed = false // Now check if we have a sessionPolicy. - spolicy, ok := args.Claims[iampolicy.SessionPolicyName] + spolicy, ok := args.Claims[sessionPolicyNameExtracted] if !ok { return }