Skip to content

LdapGroupService: attrisdn not ported, and group-ID convention mismatch (DN vs cn) with legacy driver #224

Description

@jcdelepine

Summary

While wiring LDAP support into the modern GroupService (see
accompanying PR, feat/groupservice-ldap-driver), testing against a
real directory surfaced two gaps in LdapGroupService that go beyond
that PR's scope. Filing here rather than folding them in, since both
need a design decision, not just a bugfix.

1. attrisdn (members stored as full DNs) isn't ported

Legacy Horde_Group_Ldap supports LDAP schemas where the member
attribute holds full user DNs rather than plain uids
($conf['group']['params']['attrisdn'] = true), resolving via
Horde_Ldap::findUserDN() on write and a simplified first-RDN
extraction on read. LdapGroupService (the modern port) has no
equivalent: it always treats member values as plain uids.

Concretely, a real group with attrisdn-style membership currently
comes back from the modern GroupService as raw DNs instead of
uids:

=== Modern GroupService ===
members:
  - uid=alice,ou=people,dc=example,dc=com
  - uid=bob,ou=people,dc=example,dc=com
  - uid=charlie,ou=people,dc=example,dc=com

=== Legacy Horde_Group (same group, same directory) ===
members:
  - alice
  - bob
  - charlie

A prior attempt at porting this (constructor attrisdn param,
findUserDN() on writes, DN normalization on reads, with test
coverage) exists in a dropped commit — happy to share it as a
starting point if useful, but it isn't upstream-ready and isn't
backing any real deployment yet, so it seemed better to check here
first rather than propose something that might duplicate or conflict
with work already in progress on the LDAP side.

2. Group ID convention mismatch: DN vs. cn

Legacy Horde_Group_Ldap uses the group's full DN as its ID
throughout its public API (listUsers($gid), getData($gid),
exists($gid) all take/return DNs — see e.g. _search() keying
$entries[$entry['dn']]). The modern LdapGroupService uses a bare
cn value instead, reconstructing the DN internally via
buildDN($groupId).

The two are not interchangeable. A DN passed to the modern service,
or a bare cn passed to the legacy one, produces an "invalid DN"
LDAP search error rather than a clear "wrong ID format" message:

op=1 do_search: invalid dn: "developers"
op=1 SEARCH RESULT tag=101 err=34 ... text=invalid DN

This matters if anything (permissions, ACLs) stores an LDAP group ID
created via the legacy admin UI and later resolves it through the
modern GroupService, or vice versa.

Not urgent

Neither of these blocks the accompanying PR: #225 : the login flow that
motivated it (OIDC/OAuth2 → TopbarBuilderPermissionService
GroupService) works correctly without either fix. Filing for
visibility and so a design decision can be made deliberately rather
than by whoever next needs one of these.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions