On 7/6/06, Lieven Govaerts <lgo@mobsol.be> wrote:
> Hi Cong,
>
> thanks for sharing this patch, it's really interesting. I didn't review it
> thoroughly yet, I just have some high-level questions and remarks:
>
> - What new functionality does this patch provide? If I understand your example
> correctly the function is: 'If this account is part of atleast this LDAP group,
> then allow r/w access for a project/path'. Right?
> - When do you do the LDAP lookup, for each request? Apache already does an LDAP
> lookup for the authentication part (password validation), so this will have a
> negative performance impact.
> - Why don't you use the already defined AuthzLDAPURL and AuthzDAPBindDN
> commands?
> - I see you copy large parts of existing code from mod_auth_ldap.c. Why do you
> do that? If you need that functionality, use the existing functions are extract
> common functionality in a new function.
Another thing that comes to mind is, will this stuff compile if
apr-util is compiled without LDAP support? It's important that we not
add any extra requirements to mod_authz_svn.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Jul 6 16:28:16 2006