From 95a6b2c99167430926aa820c26b1ff3a413cfc0d Mon Sep 17 00:00:00 2001
From: Anis Elleuch <vadmeste@users.noreply.github.com>
Date: Thu, 19 May 2022 19:06:55 +0100
Subject: [PATCH] Merge LDAP STS policy evaluation with the generic STS code
 (#14944)

If LDAP is enabled, STS security token policy is evaluated using a
different code path and expects ldapUser claim to exist in the security
token. This means other STS temporary accounts generated by any Assume
Role function, such as AssumeRoleWithCertificate, won't be allowed to do any
operation as these accounts do not have LDAP user claim.

Since IsAllowedLDAPSTS() is similar to IsAllowedSTS(), this commit will
merge both.

Non harmful changes:
- IsAllowed for LDAP will start supporting RoleARN claim
- IsAllowed for LDAP will not check for parent claim anymore. This check doesn't
  seem to be useful since all STS login compare access/secret/security-token
  with the one saved in the disk.
- LDAP will support $username condition in policy documents.

Co-authored-by: Anis Elleuch <anis@min.io>
Co-authored-by: Aditya Manthramurthy <donatello@users.noreply.github.com>
---
 cmd/iam.go | 49 +++++--------------------------------------------
 1 file changed, 5 insertions(+), 44 deletions(-)

diff --git a/cmd/iam.go b/cmd/iam.go
index db054c96d..c3d9d8210 100644
--- a/cmd/iam.go
+++ b/cmd/iam.go
@@ -1543,54 +1543,10 @@ func (sys *IAMSys) IsAllowedServiceAccount(args iampolicy.Args, parentUser strin
 	return combinedPolicy.IsAllowed(parentArgs) && subPolicy.IsAllowed(parentArgs)
 }
 
-// IsAllowedLDAPSTS - checks for LDAP specific claims and values
-func (sys *IAMSys) IsAllowedLDAPSTS(args iampolicy.Args, parentUser string) bool {
-	// parentUser value must match the ldap user in the claim.
-	if parentInClaimIface, ok := args.Claims[ldapUser]; !ok {
-		// no ldapUser claim present reject it.
-		return false
-	} else if parentInClaim, ok := parentInClaimIface.(string); !ok {
-		// not the right type, reject it.
-		return false
-	} else if parentInClaim != parentUser {
-		// ldap claim has been modified maliciously reject it.
-		return false
-	}
-
-	// Check policy for this LDAP user.
-	ldapPolicies, err := sys.PolicyDBGet(parentUser, false, args.Groups...)
-	if err != nil {
-		return false
-	}
-
-	if len(ldapPolicies) == 0 {
-		return false
-	}
-
-	// Policies were found, evaluate all of them.
-	availablePoliciesStr, combinedPolicy := sys.store.FilterPolicies(strings.Join(ldapPolicies, ","), "")
-	if availablePoliciesStr == "" {
-		return false
-	}
-
-	hasSessionPolicy, isAllowedSP := isAllowedBySessionPolicy(args)
-	if hasSessionPolicy {
-		return isAllowedSP && combinedPolicy.IsAllowed(args)
-	}
-
-	return combinedPolicy.IsAllowed(args)
-}
-
 // IsAllowedSTS is meant for STS based temporary credentials,
 // which implements claims validation and verification other than
 // applying policies.
 func (sys *IAMSys) IsAllowedSTS(args iampolicy.Args, parentUser string) bool {
-	// If it is an LDAP request, check that user and group
-	// policies allow the request.
-	if sys.usersSysType == LDAPUsersSysType {
-		return sys.IsAllowedLDAPSTS(args, parentUser)
-	}
-
 	var policies []string
 	roleArn := args.GetRoleArn()
 	if roleArn != "" {
@@ -1625,6 +1581,11 @@ func (sys *IAMSys) IsAllowedSTS(args iampolicy.Args, parentUser string) bool {
 		}
 	}
 
+	// Defensive code: Do not allow any operation if no policy is found in the session token
+	if len(policies) == 0 {
+		return false
+	}
+
 	combinedPolicy, err := sys.store.GetPolicy(strings.Join(policies, ","))
 	if err == errNoSuchPolicy {
 		for _, pname := range policies {