Re: Adding ldap group support to subversion

On 12.07.2013 15:35, Stefan Sperling wrote:
> On Fri, Jul 12, 2013 at 03:21:30PM +0200, Branko Čibej wrote:
>> The fact that the dependency coupling and
>> layering violation in the original patch didn't raise a whole lot more
>> objections frankly scares me.
> Which layering violations? That we're using literal ldap group names
> in the authz file and ask ldap for them? I don't think that's an issue
> since the new functionality is only enabled if the admin configures an
> ldap connection.

Yes, that is a layering violation. The authz implementation shouldn't
care where groups names and group membership info comes from. I can
think of two ways to do this:

 1. The caller provides a callback that the authz resolver can use to
    determine if the current user is a member of some group.
 2. The caller sends the transitive closure of group memberships along
    with the username, and the authz resolver uses that to determine
    group membership

Both of these options require a libsvn_repos API change. They're
functionally equivalent, but the callback gives the caller a bit more
flexibility (and therefore can probably support more kinds of group
membership models).

The current approach where the authz resolver queries a (set of) known
database(s) is a non-starter, IMO.

-- Brane

-- 
Branko Čibej | Director of Subversion
WANdisco // Non-Stop Data
e. brane_at_wandisco.com