[dev] [cvs] commit: framework/Group/Group ldap.php

[Ben Chavet]

On 11/24/06, Ben Klang <ben at alkaloid.net> wrote:
>
>
> >   Revert some of the changes that made ldap groups act more like
> > datatree
> >   groups.  I can't think of any reason the ldap group driver should
> > have to
> >   handle colon-separated parent/child group names.  The ldap driver
> > uses
> >   dn's to handle that relationship.
> Can you explain this to me?  I'm not sure what you mean.  I didn't
> think that colon-separated names were DataTree-specific, but rather
> the appropriate way to represent nested groups.  What are you
> proposing as the alternative to specify a nested group when
> communicating with the Groups driver (LDAP or otherwise) if not colon-
> delimited group names?


The colon-separated names are how the datatree backend keeps track of nested
groups, this is true.  But, LDAP uses dn's to keep track of them.
(cn=mygroup,cn=parentgroup,dc=example,dc=com is a child of
cn=parentgroup,dc=example,dc=com)

>   In general, it shouldn't matter to an application what the group
> > names
> >   or IDs are, or where they come from.  What should matter is that
> > the API
> >   is consistent, and the backend should handle the parent/child
> > relationsip
> >   in whatever manner is natural to it.  If there is code somewhere
> > that
> >   explicitly generates colon-separated group names, it should be
> > fixed to
> >   use the group API, not the other way around.
> The colon-separated names are required by apps which manipulate or
> read from nested groups.  How is this breaking the API?  I agree with
> you that the DN can represent a parent/child group, but at the
> application level we don't want to be parsing a DN.  Using colons to
> separate names is a backend agnostic way to represent nested groups.
> In fact the UI in many places shows the group name with colons to the
> user when selecting nested groups.


That's exactly the problem, then.  Apps shouldn't be  manipulating  nested
groups in a driver-specific manner.  They should be using getparent() or
getchildren() (or whatever the equivalent is, I don't have the code in front
of me) API calls, and the backend driver should handle the parent/child
relationships.

Incidentally this patch breaks things for me.  Specifically where you
> modify getGroupById() (there may be others, I haven't fully tested
> yet).  You added a note that says if you have more than one group
> with the same name, results can become unpredictable.  Before you
> reverted the patch, there was no chance for ambiguity.


Oddly enough, I changed this back because LDAP groups were broken for me
when setting up a fresh LDAP-based test environment, and this is what it
took to make it work :)

The only ambiguity potential is where we try to get a group ID from a group
name.  This potential exists because a group name can be duplicated
somewhere else in the directory.  For example, cn=test,dc=example,dc=com,
and cn=test,cn=parent,dc=example,dc=com both have the same group name (test)
and which ID is returned depends on which one is first in the list of
results.

Was there something that was broken in the pre-reverted changes or
> are you just trying to make the group API consistent?  I am totally
> on board with making the API match across backends, but I was not
> aware it was broken for LDAP.


The group API should be completely backend-independent.  In other words, a
group has a unique ID, a name, a list of members, and optionally a parent
and/or children groups.  The unique ID should always be used when
referencing the group, and the API calls should be used to handle
parent/child relationships.  The longname vs. shortname is a side effect of
the datatree backend, and we should shy away from distinguishing the two.
We should also move away from using getGroupId(name) because of the
ambiguity potential.

--Ben