Review: Needs Information
Hello Stefan,

Thanks for the nice merge proposal!
Your changes make me wonder about a few things, listed below (disclaimer: I 
haven't tested this, and haven't read the full LDAP RFCs):

- It was my understanding that LDAP defines a lot of authentication mechanism 
(anonymous, user/pass, and even "unauthenticated" authentication [1]), and as a 
result prior authentication is always required before any operation can be done 
in an LDAP session. Your patch seems to remove the initial bind() operation in 
case "anonymous" is enabled, while I would have expected you would need to 
really perform an anonymous bind operation to conform to the protocol? (e.g. 
bind(who='', cred='') as described in [2]).
- The python-ldap documentation also seems to imply that some variation of 
bind() needs to be called before any operation can be done[3]. Based on this, 
it looks like the search() performed to verify the user's existence would fail 
with some LDAP servers, only because no bind() has been done before.
- Since LDAP also has other authentication modes, wouldn't it be better to 
instead display a selection of options, at least with the simple mechanisms: 
"Name/Password", "Anonymous", "Unauthenticated", implement them accordingly 
(passing empty who/cred to bind()) and set the appropriate required fields for 
each (as you did for anonymous).
- Out of curiosity, what LDAP server are you using, and with what kind of 
specific settings? Default settings?

Thanks!

[1] http://tools.ietf.org/html/rfc4513#section-5.1
[2] http://tools.ietf.org/html/rfc4513#section-5.1.1
[3] http://www.python-ldap.org/doc/html/ldap.html#ldap.LDAPObject.simple_bind_s
-- 
https://code.launchpad.net/~openerp-community/openobject-addons/stefan-therp_lp794450/+merge/63831
Your team OpenERP Community is subscribed to branch 
lp:~openerp-community/openobject-addons/stefan-therp_lp794450.

_______________________________________________
Mailing list: https://launchpad.net/~openerp-community
Post to     : openerp-community@lists.launchpad.net
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp

Reply via email to