From d4157b819c23e00bfa11c4d09757cf9bff2b3dab Mon Sep 17 00:00:00 2001 From: Taran Pelkey Date: Thu, 10 Oct 2024 19:40:37 -0400 Subject: [PATCH] Allow LDAP DNs with slashes to be loaded from object store (#20541) --- cmd/iam-object-store.go | 22 +++++++-- cmd/iam-object-store_test.go | 53 +++++++++++++++++++++ cmd/sts-handlers_test.go | 89 ++++++++++++++++++++++++++++++++++++ 3 files changed, 159 insertions(+), 5 deletions(-) create mode 100644 cmd/iam-object-store_test.go diff --git a/cmd/iam-object-store.go b/cmd/iam-object-store.go index a98b32ead..858abf69c 100644 --- a/cmd/iam-object-store.go +++ b/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 { - i = strings.LastIndex(s, "/") + if secondIndex { + 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) - listKey, trimmedItem := splitPath(item.Item, lastIndex) + secondIndex := strings.HasPrefix(item.Item, policyDBPrefix) + listKey, trimmedItem := splitPath(item.Item, secondIndex) if listKey == iamFormatFile { continue } diff --git a/cmd/iam-object-store_test.go b/cmd/iam-object-store_test.go new file mode 100644 index 000000000..dc0ce1326 --- /dev/null +++ b/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 . + +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) + } + } +} diff --git a/cmd/sts-handlers_test.go b/cmd/sts-handlers_test.go index 661af72ec..9e743ed50 100644 --- a/cmd/sts-handlers_test.go +++ b/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()