diff --git a/changelog/unreleased/fix-ldap-del-usermembership.md b/changelog/unreleased/fix-ldap-del-usermembership.md new file mode 100644 index 00000000000..930ad5bec41 --- /dev/null +++ b/changelog/unreleased/fix-ldap-del-usermembership.md @@ -0,0 +1,6 @@ +Bugfix: Remove group memberships when deleting a user + +The LDAP backend in the graph API now takes care of removing a user's group +membership when deleting the user. + +https://github.com/owncloud/ocis/issues/3027 diff --git a/graph/pkg/identity/ldap.go b/graph/pkg/identity/ldap.go index 71f402eae3c..5a9601d039a 100644 --- a/graph/pkg/identity/ldap.go +++ b/graph/pkg/identity/ldap.go @@ -182,6 +182,23 @@ func (i *LDAP) DeleteUser(ctx context.Context, nameOrID string) error { if err = i.conn.Del(&dr); err != nil { return err } + + // Find all the groups that this user was a member of and remove it from there + groupEntries, err := i.getLDAPGroupsByFilter(fmt.Sprintf("(%s=%s)", i.groupAttributeMap.member, e.DN), true, false) + if err != nil { + return err + } + for _, group := range groupEntries { + i.logger.Debug().Str("group", group.DN).Str("user", e.DN).Msg("Cleaning up group membership") + + if mr, err := i.removeMemberFromGroupEntry(group, e.DN); err == nil && mr != nil { + if err = i.conn.Modify(mr); err != nil { + // Errors when deleting the memberships are only logged as warnings but not returned + // to the user as we already successfully deleted the users itself + i.logger.Warn().Str("group", group.DN).Str("user", e.DN).Err(err).Msg("failed to remove member") + } + } + } return nil } @@ -397,10 +414,22 @@ func (i *LDAP) getLDAPGroupByNameOrID(nameOrID string, requestMembers bool) (*ld return i.getLDAPGroupByFilter(filter, requestMembers) } +func (i *LDAP) getLDAPGroupByFilter(filter string, requestMembers bool) (*ldap.Entry, error) { + e, err := i.getLDAPGroupsByFilter(filter, requestMembers, true) + if err != nil { + return nil, err + } + if len(e) == 0 { + return nil, errorcode.New(errorcode.ItemNotFound, "not found") + } + + return e[0], nil +} + // Search for LDAP Groups matching the specified filter, if requestMembers is true the groupMemberShip // attribute will be part of the result attributes. The LDAP filter is combined with the configured groupFilter // resulting in a filter like "(&(LDAP.groupFilter)())" -func (i *LDAP) getLDAPGroupByFilter(filter string, requestMembers bool) (*ldap.Entry, error) { +func (i *LDAP) getLDAPGroupsByFilter(filter string, requestMembers, single bool) ([]*ldap.Entry, error) { attrs := []string{ i.groupAttributeMap.name, i.groupAttributeMap.id, @@ -410,8 +439,12 @@ func (i *LDAP) getLDAPGroupByFilter(filter string, requestMembers bool) (*ldap.E attrs = append(attrs, i.groupAttributeMap.member) } + sizelimit := 0 + if single { + sizelimit = 1 + } searchRequest := ldap.NewSearchRequest( - i.groupBaseDN, i.groupScope, ldap.NeverDerefAliases, 1, 0, false, + i.groupBaseDN, i.groupScope, ldap.NeverDerefAliases, sizelimit, 0, false, fmt.Sprintf("(&%s%s)", i.groupFilter, filter), attrs, nil, @@ -429,11 +462,46 @@ func (i *LDAP) getLDAPGroupByFilter(filter string, requestMembers bool) (*ldap.E } return nil, errorcode.New(errorcode.ItemNotFound, errmsg) } - if len(res.Entries) == 0 { - return nil, errNotFound + return res.Entries, nil +} + +// removeMemberFromGroupEntry creates an LDAP Modify request (not sending it) +// that would update the supplied entry to remove the specified member from the +// group +func (i *LDAP) removeMemberFromGroupEntry(group *ldap.Entry, memberDN string) (*ldap.ModifyRequest, error) { + nOldMemberDN, err := ldapdn.ParseNormalize(memberDN) + if err != nil { + return nil, err + } + members := group.GetEqualFoldAttributeValues(i.groupAttributeMap.member) + found := false + for _, member := range members { + if member == "" { + continue + } + if nMember, err := ldapdn.ParseNormalize(member); err != nil { + // We couldn't parse the member value as a DN. Let's keep it + // as it is but log a warning + i.logger.Warn().Str("memberDN", member).Err(err).Msg("Couldn't parse DN") + continue + } else { + if nMember == nOldMemberDN { + found = true + } + } + } + if !found { + i.logger.Debug().Str("backend", "ldap").Str("groupdn", group.DN).Str("member", memberDN). + Msg("The target is not a member of the group") + return nil, nil } - return res.Entries[0], nil + mr := ldap.ModifyRequest{DN: group.DN} + if len(members) == 1 { + mr.Add(i.groupAttributeMap.member, []string{""}) + } + mr.Delete(i.groupAttributeMap.member, []string{memberDN}) + return &mr, nil } func (i *LDAP) GetGroups(ctx context.Context, queryParam url.Values) ([]*libregraph.Group, error) { @@ -642,42 +710,8 @@ func (i *LDAP) RemoveMemberFromGroup(ctx context.Context, groupID string, member } i.logger.Debug().Str("backend", "ldap").Str("groupdn", ge.DN).Str("member", me.DN).Msg("remove member") - nOldMemberDN, err := ldapdn.ParseNormalize(me.DN) - if err != nil { - i.logger.Error().Str("old member", me.DN).Err(err).Msg("Couldn't parse DN") - return err - } - members := ge.GetEqualFoldAttributeValues(i.groupAttributeMap.member) - found := false - for _, member := range members { - if member == "" { - continue - } - if nMember, err := ldapdn.ParseNormalize(member); err != nil { - // We couldn't parse the member value as a DN. Let's keep it - // as it is but log a warning - i.logger.Warn().Str("memberDN", member).Err(err).Msg("Couldn't parse DN") - continue - } else { - if nMember == nOldMemberDN { - found = true - } - } - } - if !found { - i.logger.Debug().Str("backend", "ldap").Str("groupdn", ge.DN).Str("member", me.DN). - Msg("The target is not a member of the group") - return nil - } - - mr := ldap.ModifyRequest{DN: ge.DN} - if len(members) == 1 { - mr.Add(i.groupAttributeMap.member, []string{""}) - } - mr.Delete(i.groupAttributeMap.member, []string{me.DN}) - - if err := i.conn.Modify(&mr); err != nil { - return err + if mr, err := i.removeMemberFromGroupEntry(ge, me.DN); err == nil && mr != nil { + return i.conn.Modify(mr) } return nil }