diff --git a/services/graph/pkg/identity/ldap.go b/services/graph/pkg/identity/ldap.go index 8d4c7674a..3673d3e7f 100644 --- a/services/graph/pkg/identity/ldap.go +++ b/services/graph/pkg/identity/ldap.go @@ -211,9 +211,6 @@ func (i *LDAP) UpdateUser(ctx context.Context, nameOrID string, user libregraph. return nil, errorcode.New(errorcode.NotAllowed, "changing the UserId is not allowed") } } - // TODO: In order to allow updating the user name we'd need to issue a ModRDN operation - // As we currently using uid as the naming Attribute for the user entries. (Do we even - // want to allow changing the user name?). For now just disallow it. if user.OnPremisesSamAccountName != nil && *user.OnPremisesSamAccountName != "" { if eu := e.GetEqualFoldAttributeValue(i.userAttributeMap.userName); eu != *user.OnPremisesSamAccountName { e, err = i.changeUserName(ctx, e.DN, eu, user.GetOnPremisesSamAccountName()) @@ -517,20 +514,19 @@ func (i *LDAP) changeUserName(ctx context.Context, dn, originalUserName, newUser func (i *LDAP) renameMemberInGroup(ctx context.Context, group *ldap.Entry, oldMember, newMember string) error { logger := i.logger.SubloggerWithRequestID(ctx) logger.Debug().Str("oldMember", oldMember).Str("newMember", newMember).Msg("replacing group member") - members := group.GetEqualFoldAttributeValues(i.groupAttributeMap.member) - match := -1 - for i, m := range members { - if m == oldMember { - match = i - } - } - if match != -1 { - members[match] = newMember - mr := ldap.NewModifyRequest(group.DN, nil) - mr.Replace(i.groupAttributeMap.member, members) - if err := i.conn.Modify(mr); err != nil { - return err + mr := ldap.NewModifyRequest(group.DN, nil) + mr.Delete(i.groupAttributeMap.member, []string{oldMember}) + mr.Add(i.groupAttributeMap.member, []string{newMember}) + if err := i.conn.Modify(mr); err != nil { + var lerr *ldap.Error + if errors.As(err, &lerr) { + if lerr.ResultCode == ldap.LDAPResultNoSuchObject { + groupID := group.GetEqualFoldAttributeValue(i.groupAttributeMap.id) + logger.Warn().Str("group", groupID).Msg("Group no longer exists") + return nil + } } + return err } return nil } diff --git a/services/graph/pkg/identity/ldap_group.go b/services/graph/pkg/identity/ldap_group.go index 6806f4984..786235cdc 100644 --- a/services/graph/pkg/identity/ldap_group.go +++ b/services/graph/pkg/identity/ldap_group.go @@ -423,7 +423,7 @@ func (i *LDAP) getGroupsForUser(dn string) ([]*ldap.Entry, error) { "(%s=%s)", i.groupAttributeMap.member, dn, ) - userGroups, err := i.getLDAPGroupsByFilter(groupFilter, true, false) + userGroups, err := i.getLDAPGroupsByFilter(groupFilter, false, false) if err != nil { return nil, err } diff --git a/services/graph/pkg/identity/ldap_test.go b/services/graph/pkg/identity/ldap_test.go index da0043260..f55702cb9 100644 --- a/services/graph/pkg/identity/ldap_test.go +++ b/services/graph/pkg/identity/ldap_test.go @@ -663,7 +663,7 @@ func TestUpdateUser(t *testing.T) { Scope: 2, DerefAliases: 0, SizeLimit: 0, TimeLimit: 0, TypesOnly: false, Filter: "(&(objectClass=groupOfNames)(member=uid=oldName))", - Attributes: []string{"cn", "entryUUID", "member"}, + Attributes: []string{"cn", "entryUUID"}, Controls: []ldap.Control(nil), }, }, @@ -681,10 +681,6 @@ func TestUpdateUser(t *testing.T) { Name: lconfig.GroupIDAttribute, Values: []string{"group1-id"}, }, - { - Name: "member", - Values: []string{"uid=oldName"}, - }, }, }, }, @@ -758,7 +754,14 @@ func TestUpdateUser(t *testing.T) { DN: "cn=group1", Changes: []ldap.Change{ { - Operation: 0x2, + Operation: 0x1, + Modification: ldap.PartialAttribute{ + Type: "member", + Vals: []string{"uid=oldName"}, + }, + }, + { + Operation: 0x0, Modification: ldap.PartialAttribute{ Type: "member", Vals: []string{"uid=newName,ou=people,dc=test"},