fix: remove embedded-policy as requested by the user (#14847)

this PR introduces a few changes such as

- sessionPolicyName is not reused in an extracted manner
  to apply policies for incoming authenticated calls,
  instead uses a different key to designate this
  information for the callers.

- this differentiation is needed to ensure that service
  account updates do not accidentally store JSON representation
  instead of base64 equivalent on the disk.

- relax requirements for Deleting a service account, allow
  deleting a service account that might be unreadable, i.e
  a situation where the user might have removed session policy 
  which now carries a JSON representation, making it unparsable.

- introduce some constants to reuse instead of strings.

fixes #14784

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
 		}

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

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)

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
 	}