Repository: minio/minio (language: go)

Issue:
Cannot delete Service Account after removing its policy
After creating a Service Account with a policy, then deleting said policy, the Service Account cannot be deleted anymore. 

I know the WebUI says "You cannot modify the Service Account optional policy after saving." I can still edit it though.

Error in UI:
```
We encountered an internal error, please try again. (illegal base64 data at input byte 0)
```

Error in Console:
```
minio_1  | API: DeleteServiceAccount()
minio_1  | Time: 14:22:02 UTC 04/20/2022
minio_1  | DeploymentID: dfd72018-48e7-460a-abb6-9bc42927c805
minio_1  | RequestID: 16E7A0BD1BA1B1CF
minio_1  | RemoteHost: 172.24.0.2
minio_1  | Host: 172.24.0.2:9000
minio_1  | UserAgent: MinIO (linux; amd64) madmin-go/0.0.1
minio_1  | Error: illegal base64 data at input byte 0 (base64.CorruptInputError)
minio_1  |        5: cmd/api-errors.go:2112:cmd.toAPIErrorCode()
minio_1  |        4: cmd/admin-handler-utils.go:217:cmd.toAdminAPIErrCode()
minio_1  |        3: cmd/admin-handler-utils.go:204:cmd.toAdminAPIErr()
minio_1  |        2: cmd/admin-handlers-users.go:998:cmd.adminAPIHandlers.DeleteServiceAccount()
minio_1  |        1: net/http/server.go:2047:http.HandlerFunc.ServeHTTP()
```

## Expected Behavior
Either I woudn't be able to edit the Service Account's policy, like the UI mentions. and-or I should be able to delete the Service Account.

## Current Behavior
After creating a Service Account with a policy, then deleting said policy, the Service Account cannot be deleted anymore. 

## Possible Solution
Maybe the Service Account's policy shouldn't be modifiable in the UI?

## Steps to Reproduce (for bugs)

1. Start fresh minio tenant
2. Create a new user 
3. Give the new user a policy (i.e. readwrite)
4. Login with the new user
5. Create a Service Account with a Policy
6. Remove the Service Account's policy afterwards
7. Delete the Service Account
8. Encounter Error `(illegal base64 data at input byte 0)`

## Context
Just trying Service Accounts and their policies while testing the product.

## Regression
No? Don't know

## Your Environment
* Version used (`minio --version`): RELEASE.2022-04-16T04-26-02Z
* Server setup and configuration: Simple docker-compose with `docker.io/minio/minio:latest`



Discussion / hints:
Curious if you found a way to delete it afterward as I found myself in the same situation - was testing policies and removed the policy in testing, not realizing that I couldn't.  I tried using `mc` but that did not work; same error as the UI.
Yeah `mc` runs into the same error, which is expected since the backend fails at the same function `DeleteServiceAccount()`

```
mc admin user svcacct remove minio KuoiCo6SvzVw
mc: <ERROR> Unable to remove the specified service account. 
We encountered an internal error, please try again. (illegal base64 data at input byte 0).
```

It's all functions that try to read the policy, more specifically the function `globalIAMSys.GetServiceAccount` I think.

```
Error: illegal base64 data at input byte 0 (base64.CorruptInputError)
       5: cmd/api-errors.go:2112:cmd.toAPIErrorCode()
       4: cmd/admin-handler-utils.go:217:cmd.toAdminAPIErrCode()
       3: cmd/admin-handler-utils.go:204:cmd.toAdminAPIErr()
       2: cmd/admin-handlers-users.go:731:cmd.adminAPIHandlers.UpdateServiceAccount()
       1: net/http/server.go:2047:http.HandlerFunc.ServeHTTP()

Error: illegal base64 data at input byte 0 (base64.CorruptInputError)
       5: cmd/api-errors.go:2112:cmd.toAPIErrorCode()
       4: cmd/admin-handler-utils.go:217:cmd.toAdminAPIErrCode()
       3: cmd/admin-handler-utils.go:204:cmd.toAdminAPIErr()
       2: cmd/admin-handlers-users.go:834:cmd.adminAPIHandlers.InfoServiceAccount()
       1: net/http/server.go:2047:http.HandlerFunc.ServeHTTP()
```

A workaround is to delete the user who created the service account, which is a bit unpleasant.

I think a solution would be to actually enforce what is mentioned in the UI "You cannot modify the Service Account optional policy after saving.". The input box could be read-only when modifying the service account.

Fix patch:
diff --git a/cmd/admin-handlers-users.go b/cmd/admin-handlers-users.go
index c21203ab3d1bb..a05c739d96089 100644
--- a/cmd/admin-handlers-users.go
+++ b/cmd/admin-handlers-users.go
@@ -993,11 +993,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,
@@ -1012,7 +1010,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.
@@ -1021,23 +1019,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 5bfb332b32bcf..5fa945a31944c 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 b308c643d3ba9..035109cc53c31 100644
--- a/cmd/iam-store.go
+++ b/cmd/iam-store.go
@@ -1685,6 +1685,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
@@ -1701,7 +1712,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 11233f83b1239..791380aad3cf1 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
 	}


Test patch: