fix: remove LDAP groups claim and store them on server (#9637)

Groups information shall be now stored as part of the
credential data structure, this is a more idiomatic
way to support large LDAP groups.

Avoids the complication of setups where LDAP groups
can be in the range of 150+ which may lead to excess
HTTP header size > 8KiB, to reduce such an occurrence
we shall save the group information on the server as
part of the credential data structure.

Bonus change support multiple mapped policies, across
all types of users.

cmd/iam-etcd-store.go

@@ -197,7 +197,7 @@ func (ies *IAMEtcdStore) migrateUsersConfigToV1(ctx context.Context, isSTS bool)
 		// then the parsed auth.Credentials will have
 		// the zero value for the struct.
 		var zeroCred auth.Credentials
-		if cred == zeroCred {
+		if cred.Equal(zeroCred) {
 			// nothing to do
 			continue
 		}

cmd/iam-object-store.go

@@ -140,7 +140,7 @@ func (iamOS *IAMObjectStore) migrateUsersConfigToV1(ctx context.Context, isSTS b
 		// then the parsed auth.Credentials will have
 		// the zero value for the struct.
 		var zeroCred auth.Credentials
-		if cred == zeroCred {
+		if cred.Equal(zeroCred) {
 			// nothing to do
 			continue
 		}

cmd/iam.go

@@ -162,16 +162,37 @@ func newGroupInfo(members []string) GroupInfo {
 
 // MappedPolicy represents a policy name mapped to a user or group
 type MappedPolicy struct {
-	Version int    `json:"version"`
+	Version  int    `json:"version"`
-	Policy  string `json:"policy"`
+	Policies string `json:"policy"`
+}
+
+// converts a mapped policy into a slice of distinct policies
+func (mp MappedPolicy) toSlice() []string {
+	var policies []string
+	for _, policy := range strings.Split(mp.Policies, ",") {
+		policy = strings.TrimSpace(policy)
+		if policy == "" {
+			continue
+		}
+		policies = append(policies, policy)
+	}
+	return policies
 }
 
 func (mp MappedPolicy) policySet() set.StringSet {
-	return set.CreateStringSet(strings.Split(mp.Policy, ",")...)
+	var policies []string
+	for _, policy := range strings.Split(mp.Policies, ",") {
+		policy = strings.TrimSpace(policy)
+		if policy == "" {
+			continue
+		}
+		policies = append(policies, policy)
+	}
+	return set.CreateStringSet(policies...)
 }
 
 func newMappedPolicy(policy string) MappedPolicy {
-	return MappedPolicy{Version: 1, Policy: policy}
+	return MappedPolicy{Version: 1, Policies: policy}
 }
 
 // IAMSys - config system.
@@ -435,38 +456,32 @@ func (sys *IAMSys) DeletePolicy(policyName string) error {
 	delete(sys.iamPolicyDocsMap, policyName)
 
 	// Delete user-policy mappings that will no longer apply
-	var usersToDel []string
-	var usersType []IAMUserType
 	for u, mp := range sys.iamUserPolicyMap {
-		if mp.Policy == policyName {
+		pset := mp.policySet()
+		if pset.Contains(policyName) {
 			cr, ok := sys.iamUsersMap[u]
 			if !ok {
 				// This case cannot happen
 				return errNoSuchUser
 			}
+			pset.Remove(policyName)
 			// User is from STS if the cred are temporary
 			if cr.IsTemp() {
-				usersType = append(usersType, stsUser)
+				sys.policyDBSet(u, strings.Join(pset.ToSlice(), ","), stsUser, false)
 			} else {
-				usersType = append(usersType, regularUser)
+				sys.policyDBSet(u, strings.Join(pset.ToSlice(), ","), regularUser, false)
 			}
-			usersToDel = append(usersToDel, u)
 		}
 	}
-	for i, u := range usersToDel {
-		sys.policyDBSet(u, "", usersType[i], false)
-	}
 
 	// Delete group-policy mappings that will no longer apply
-	var groupsToDel []string
 	for g, mp := range sys.iamGroupPolicyMap {
-		if mp.Policy == policyName {
+		pset := mp.policySet()
-			groupsToDel = append(groupsToDel, g)
+		if pset.Contains(policyName) {
+			pset.Remove(policyName)
+			sys.policyDBSet(g, strings.Join(pset.ToSlice(), ","), regularUser, true)
 		}
 	}
-	for _, g := range groupsToDel {
-		sys.policyDBSet(g, "", regularUser, true)
-	}
 
 	return err
 }
@@ -596,14 +611,11 @@ func (sys *IAMSys) SetTempUser(accessKey string, cred auth.Credentials, policyNa
 	// policies for this server.
 	if globalPolicyOPA == nil && policyName != "" {
 		var availablePolicies []iampolicy.Policy
-		for _, pname := range strings.Split(policyName, ",") {
+		mp := newMappedPolicy(policyName)
-			pname = strings.TrimSpace(pname)
+		for _, policy := range mp.toSlice() {
-			if pname == "" {
+			p, found := sys.iamPolicyDocsMap[policy]
-				continue
-			}
-			p, found := sys.iamPolicyDocsMap[pname]
 			if !found {
-				return fmt.Errorf("%w: (%s)", errNoSuchPolicy, pname)
+				return fmt.Errorf("%w: (%s)", errNoSuchPolicy, policy)
 			}
 			availablePolicies = append(availablePolicies, p)
 		}
@@ -619,7 +631,6 @@ func (sys *IAMSys) SetTempUser(accessKey string, cred auth.Credentials, policyNa
 			return nil
 		}
 
-		mp := newMappedPolicy(policyName)
 		if err := sys.store.saveMappedPolicy(accessKey, stsUser, false, mp); err != nil {
 			return err
 		}
@@ -655,7 +666,7 @@ func (sys *IAMSys) ListUsers() (map[string]madmin.UserInfo, error) {
 	for k, v := range sys.iamUsersMap {
 		if !v.IsTemp() && !v.IsServiceAccount() {
 			users[k] = madmin.UserInfo{
-				PolicyName: sys.iamUserPolicyMap[k].Policy,
+				PolicyName: sys.iamUserPolicyMap[k].Policies,
 				Status: func() madmin.AccountStatus {
 					if v.IsValid() {
 						return madmin.AccountEnabled
@@ -728,7 +739,7 @@ func (sys *IAMSys) GetUserInfo(name string) (u madmin.UserInfo, err error) {
 			return u, errNoSuchUser
 		}
 		return madmin.UserInfo{
-			PolicyName: mappedPolicy.Policy,
+			PolicyName: mappedPolicy.Policies,
 			MemberOf:   memberships.ToSlice(),
 		}, nil
 	}
@@ -743,7 +754,7 @@ func (sys *IAMSys) GetUserInfo(name string) (u madmin.UserInfo, err error) {
 	}
 
 	u = madmin.UserInfo{
-		PolicyName: sys.iamUserPolicyMap[name].Policy,
+		PolicyName: sys.iamUserPolicyMap[name].Policies,
 		Status: func() madmin.AccountStatus {
 			if cred.IsValid() {
 				return madmin.AccountEnabled
@@ -1320,19 +1331,15 @@ func (sys *IAMSys) policyDBSet(name, policyName string, userType IAMUserType, is
 		return nil
 	}
 
-	for _, pname := range strings.Split(policyName, ",") {
+	mp := newMappedPolicy(policyName)
-		pname = strings.TrimSpace(pname)
+	for _, policy := range mp.toSlice() {
-		if pname == "" {
+		if _, found := sys.iamPolicyDocsMap[policy]; !found {
-			continue
+			logger.LogIf(GlobalContext, fmt.Errorf("%w: (%s)", errNoSuchPolicy, policy))
-		}
-		if _, found := sys.iamPolicyDocsMap[pname]; !found {
-			logger.LogIf(GlobalContext, fmt.Errorf("%w: (%s)", errNoSuchPolicy, pname))
 			return errNoSuchPolicy
 		}
 	}
 
 	// Handle policy mapping set/update
-	mp := newMappedPolicy(policyName)
 	if err := sys.store.saveMappedPolicy(name, userType, isGroup, mp); err != nil {
 		return err
 	}
@@ -1370,12 +1377,8 @@ func (sys *IAMSys) policyDBGet(name string, isGroup bool) ([]string, error) {
 			return nil, errNoSuchGroup
 		}
 
-		policy := sys.iamGroupPolicyMap[name]
+		mp := sys.iamGroupPolicyMap[name]
-		// returned policy could be empty
+		return mp.toSlice(), nil
-		if policy.Policy == "" {
-			return nil, nil
-		}
-		return []string{policy.Policy}, nil
 	}
 
 	// When looking for a user's policies, we also check if the
@@ -1388,12 +1391,12 @@ func (sys *IAMSys) policyDBGet(name string, isGroup bool) ([]string, error) {
 		return nil, nil
 	}
 
-	result := []string{}
+	var policies []string
-	policy := sys.iamUserPolicyMap[name]
+
+	mp := sys.iamUserPolicyMap[name]
 	// returned policy could be empty
-	if policy.Policy != "" {
+	policies = append(policies, mp.toSlice()...)
-		result = append(result, policy.Policy)
+
-	}
 	for _, group := range sys.iamUserGroupMemberships[name].ToSlice() {
 		// Skip missing or disabled groups
 		gi, ok := sys.iamGroupsMap[group]
@@ -1401,12 +1404,10 @@ func (sys *IAMSys) policyDBGet(name string, isGroup bool) ([]string, error) {
 			continue
 		}
 
-		p, ok := sys.iamGroupPolicyMap[group]
+		p := sys.iamGroupPolicyMap[group]
-		if ok && p.Policy != "" {
+		policies = append(policies, p.toSlice()...)
-			result = append(result, p.Policy)
-		}
 	}
-	return result, nil
+	return policies, nil
 }
 
 // IsAllowedServiceAccount - checks if the given service account is allowed to perform
@@ -1512,6 +1513,65 @@ func (sys *IAMSys) IsAllowedServiceAccount(args iampolicy.Args, parent string) b
 	return combinedPolicy.IsAllowed(parentArgs) && subPolicy.IsAllowed(parentArgs)
 }
 
+// IsAllowedLDAPSTS - checks for LDAP specific claims and values
+func (sys *IAMSys) IsAllowedLDAPSTS(args iampolicy.Args) bool {
+	userIface, ok := args.Claims[ldapUser]
+	if !ok {
+		return false
+	}
+	user, ok := userIface.(string)
+	if !ok {
+		return false
+	}
+
+	sys.store.rlock()
+	defer sys.store.runlock()
+
+	var groups []string
+	cred, ok := sys.iamUsersMap[args.AccountName]
+	if !ok {
+		return false
+	}
+	groups = cred.Groups
+
+	// We look up the policy mapping directly to bypass
+	// users exists, group exists validations that do not
+	// apply here.
+	var policies []iampolicy.Policy
+	if mp, ok := sys.iamUserPolicyMap[user]; ok {
+		for _, pname := range mp.toSlice() {
+			p, found := sys.iamPolicyDocsMap[pname]
+			if !found {
+				return false
+			}
+			policies = append(policies, p)
+		}
+	}
+	for _, group := range groups {
+		mp, ok := sys.iamGroupPolicyMap[group]
+		if !ok {
+			continue
+		}
+		for _, pname := range mp.toSlice() {
+			p, found := sys.iamPolicyDocsMap[pname]
+			if !found {
+				return false
+			}
+			policies = append(policies, p)
+		}
+	}
+	if len(policies) == 0 {
+		return false
+	}
+	combinedPolicy := policies[0]
+	for i := 1; i < len(policies); i++ {
+		combinedPolicy.Statements =
+			append(combinedPolicy.Statements,
+				policies[i].Statements...)
+	}
+	return combinedPolicy.IsAllowed(args)
+}
+
 // IsAllowedSTS is meant for STS based temporary credentials,
 // which implements claims validation and verification other than
 // applying policies.
@@ -1519,73 +1579,7 @@ func (sys *IAMSys) IsAllowedSTS(args iampolicy.Args) bool {
 	// If it is an LDAP request, check that user and group
 	// policies allow the request.
 	if sys.usersSysType == LDAPUsersSysType {
-		if userIface, ok := args.Claims[ldapUser]; ok {
+		return sys.IsAllowedLDAPSTS(args)
-			var user string
-			if u, ok := userIface.(string); ok {
-				user = u
-			} else {
-				return false
-			}
-
-			var groups []string
-			groupsVal := args.Claims[ldapGroups]
-			if g, ok := groupsVal.([]interface{}); ok {
-				for _, eachG := range g {
-					if eachGStr, ok := eachG.(string); ok {
-						groups = append(groups, eachGStr)
-					}
-				}
-			}
-
-			sys.store.rlock()
-			defer sys.store.runlock()
-
-			// We look up the policy mapping directly to bypass
-			// users exists, group exists validations that do not
-			// apply here.
-			var policies []iampolicy.Policy
-			if mp, ok := sys.iamUserPolicyMap[user]; ok {
-				for _, pname := range strings.Split(mp.Policy, ",") {
-					pname = strings.TrimSpace(pname)
-					if pname == "" {
-						continue
-					}
-					p, found := sys.iamPolicyDocsMap[pname]
-					if !found {
-						return false
-					}
-					policies = append(policies, p)
-				}
-			}
-			for _, group := range groups {
-				mp, ok := sys.iamGroupPolicyMap[group]
-				if !ok {
-					continue
-				}
-				for _, pname := range strings.Split(mp.Policy, ",") {
-					pname = strings.TrimSpace(pname)
-					if pname == "" {
-						continue
-					}
-					p, found := sys.iamPolicyDocsMap[pname]
-					if !found {
-						return false
-					}
-					policies = append(policies, p)
-				}
-			}
-			if len(policies) == 0 {
-				return false
-			}
-			combinedPolicy := policies[0]
-			for i := 1; i < len(policies); i++ {
-				combinedPolicy.Statements =
-					append(combinedPolicy.Statements,
-						policies[i].Statements...)
-			}
-			return combinedPolicy.IsAllowed(args)
-		}
-		return false
 	}
 
 	policies, ok := args.GetPolicies(iamPolicyClaimNameOpenID())

cmd/sts-handlers.go

@@ -61,8 +61,7 @@ const (
 	parentClaim = "parent"
 
 	// LDAP claim keys
-	ldapUser   = "ldapUser"
+	ldapUser = "ldapUser"
-	ldapGroups = "ldapGroups"
 )
 
 // stsAPIHandlers implements and provides http handlers for AWS STS API.
@@ -491,9 +490,8 @@ func (sts *stsAPIHandlers) AssumeRoleWithLDAPIdentity(w http.ResponseWriter, r *
 
 	expiryDur := globalLDAPConfig.GetExpiryDuration()
 	m := map[string]interface{}{
-		expClaim:   UTCNow().Add(expiryDur).Unix(),
+		expClaim: UTCNow().Add(expiryDur).Unix(),
-		ldapUser:   ldapUsername,
+		ldapUser: ldapUsername,
-		ldapGroups: groups,
 	}
 
 	if len(sessionPolicyStr) > 0 {
@@ -511,6 +509,10 @@ func (sts *stsAPIHandlers) AssumeRoleWithLDAPIdentity(w http.ResponseWriter, r *
 	// in obtaining service accounts by this cred.
 	cred.ParentUser = ldapUsername
 
+	// Set this value to LDAP groups, LDAP user can be part
+	// of large number of groups
+	cred.Groups = groups
+
 	// Set the newly generated credentials, policyName is empty on purpose
 	// LDAP policies are applied automatically using their ldapUser, ldapGroups
 	// mapping.

pkg/auth/credentials.go

@@ -91,6 +91,7 @@ type Credentials struct {
 	SessionToken string    `xml:"SessionToken" json:"sessionToken,omitempty"`
 	Status       string    `xml:"-" json:"status,omitempty"`
 	ParentUser   string    `xml:"-" json:"parentUser,omitempty"`
+	Groups       []string  `xml:"-" json:"groups,omitempty"`
 }
 
 func (cred Credentials) String() string {