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. br...@wandisco.com