Allow LDAP DNs with slashes to be loaded from object store (#20541)

cmd/iam-object-store.go

@@ -481,12 +481,24 @@ var (
 	policyDBGroupsListKey   = "policydb/groups/"
 )
 
+func findSecondIndex(s string, substr string) int {
+	first := strings.Index(s, substr)
+	if first == -1 {
+		return -1
+	}
+	second := strings.Index(s[first+1:], substr)
+	if second == -1 {
+		return -1
+	}
+	return first + second + 1
+}
+
 // splitPath splits a path into a top-level directory and a child item. The
 // parent directory retains the trailing slash.
-func splitPath(s string, lastIndex bool) (string, string) {
+func splitPath(s string, secondIndex bool) (string, string) {
 	var i int
-	if lastIndex {
+	if secondIndex {
-		i = strings.LastIndex(s, "/")
+		i = findSecondIndex(s, "/")
 	} else {
 		i = strings.Index(s, "/")
 	}
@@ -506,8 +518,8 @@ func (iamOS *IAMObjectStore) listAllIAMConfigItems(ctx context.Context) (res map
 			return nil, item.Err
 		}
 
-		lastIndex := strings.HasPrefix(item.Item, policyDBPrefix)
+		secondIndex := strings.HasPrefix(item.Item, policyDBPrefix)
-		listKey, trimmedItem := splitPath(item.Item, lastIndex)
+		listKey, trimmedItem := splitPath(item.Item, secondIndex)
 		if listKey == iamFormatFile {
 			continue
 		}

cmd/iam-object-store_test.go

@@ -0,0 +1,53 @@
+// Copyright (c) 2015-2024 MinIO, Inc.
+//
+// This file is part of MinIO Object Storage stack
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package cmd
+
+import (
+	"testing"
+)
+
+func TestSplitPath(t *testing.T) {
+	cases := []struct {
+		path                          string
+		secondIndex                   bool
+		expectedListKey, expectedItem string
+	}{
+		{"format.json", false, "format.json", ""},
+		{"users/tester.json", false, "users/", "tester.json"},
+		{"groups/test/group.json", false, "groups/", "test/group.json"},
+		{"policydb/groups/testgroup.json", true, "policydb/groups/", "testgroup.json"},
+		{
+			"policydb/sts-users/uid=slash/user,ou=people,ou=swengg,dc=min,dc=io.json", true,
+			"policydb/sts-users/", "uid=slash/user,ou=people,ou=swengg,dc=min,dc=io.json",
+		},
+		{
+			"policydb/sts-users/uid=slash/user/twice,ou=people,ou=swengg,dc=min,dc=io.json", true,
+			"policydb/sts-users/", "uid=slash/user/twice,ou=people,ou=swengg,dc=min,dc=io.json",
+		},
+		{
+			"policydb/groups/cn=project/d,ou=groups,ou=swengg,dc=min,dc=io.json", true,
+			"policydb/groups/", "cn=project/d,ou=groups,ou=swengg,dc=min,dc=io.json",
+		},
+	}
+	for i, test := range cases {
+		listKey, item := splitPath(test.path, test.secondIndex)
+		if listKey != test.expectedListKey || item != test.expectedItem {
+			t.Errorf("unexpected result on test[%v]: expected[%s, %s] but got [%s, %s]", i, test.expectedListKey, test.expectedItem, listKey, item)
+		}
+	}
+}

cmd/sts-handlers_test.go

@@ -740,6 +740,7 @@ func TestIAMWithLDAPServerSuite(t *testing.T) {
 				suite.TestLDAPSTSServiceAccountsWithGroups(c)
 				suite.TestLDAPAttributesLookup(c)
 				suite.TestLDAPCyrillicUser(c)
+				suite.TestLDAPSlashDN(c)
 				suite.TearDownSuite(c)
 			},
 		)
@@ -770,6 +771,7 @@ func TestIAMWithLDAPNonNormalizedBaseDNConfigServerSuite(t *testing.T) {
 				suite.TestLDAPSTSServiceAccounts(c)
 				suite.TestLDAPSTSServiceAccountsWithUsername(c)
 				suite.TestLDAPSTSServiceAccountsWithGroups(c)
+				suite.TestLDAPSlashDN(c)
 				suite.TearDownSuite(c)
 			},
 		)
@@ -2008,6 +2010,93 @@ func (s *TestSuiteIAM) TestLDAPCyrillicUser(c *check) {
 	}
 }
 
+func (s *TestSuiteIAM) TestLDAPSlashDN(c *check) {
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	defer cancel()
+
+	policyReq := madmin.PolicyAssociationReq{
+		Policies: []string{"readwrite"},
+	}
+
+	cases := []struct {
+		username string
+		dn       string
+		group    string
+	}{
+		{
+			username: "slashuser",
+			dn:       "uid=slash/user,ou=people,ou=swengg,dc=min,dc=io",
+		},
+		{
+			username: "dillon",
+			dn:       "uid=dillon,ou=people,ou=swengg,dc=min,dc=io",
+			group:    "cn=project/d,ou=groups,ou=swengg,dc=min,dc=io",
+		},
+	}
+
+	conn, err := globalIAMSys.LDAPConfig.LDAP.Connect()
+	if err != nil {
+		c.Fatalf("LDAP connect failed: %v", err)
+	}
+	defer conn.Close()
+
+	for i, testCase := range cases {
+		if testCase.group != "" {
+			policyReq.Group = testCase.group
+			policyReq.User = ""
+		} else {
+			policyReq.User = testCase.dn
+			policyReq.Group = ""
+		}
+
+		if _, err := s.adm.AttachPolicyLDAP(ctx, policyReq); err != nil {
+			c.Fatalf("Unable to attach  policy: %v", err)
+		}
+
+		ldapID := cr.LDAPIdentity{
+			Client:       s.TestSuiteCommon.client,
+			STSEndpoint:  s.endPoint,
+			LDAPUsername: testCase.username,
+			LDAPPassword: testCase.username,
+		}
+
+		value, err := ldapID.Retrieve()
+		if err != nil {
+			c.Fatalf("Expected to generate STS creds, got err: %#v", err)
+		}
+
+		// Retrieve the STS account's credential object.
+		u, ok := globalIAMSys.GetUser(ctx, value.AccessKeyID)
+		if !ok {
+			c.Fatalf("Expected to find user %s", value.AccessKeyID)
+		}
+
+		if u.Credentials.AccessKey != value.AccessKeyID {
+			c.Fatalf("Expected access key %s, got %s", value.AccessKeyID, u.Credentials.AccessKey)
+		}
+
+		// Retrieve the credential's claims.
+		secret, err := getTokenSigningKey()
+		if err != nil {
+			c.Fatalf("Error getting token signing key: %v", err)
+		}
+		claims, err := getClaimsFromTokenWithSecret(value.SessionToken, secret)
+		if err != nil {
+			c.Fatalf("Error getting claims from token: %v", err)
+		}
+
+		// Validate claims.
+		dnClaim := claims.MapClaims[ldapActualUser].(string)
+		if dnClaim != testCase.dn {
+			c.Fatalf("Test %d: unexpected dn claim: %s", i+1, dnClaim)
+		}
+
+		if _, err = s.adm.DetachPolicyLDAP(ctx, policyReq); err != nil {
+			c.Fatalf("Unable to detach user policy: %v", err)
+		}
+	}
+}
+
 func (s *TestSuiteIAM) TestLDAPAttributesLookup(c *check) {
 	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
 	defer cancel()