On 10-06-11 19:55, Olivier Dony (OpenERP) wrote: > 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): > Hi Olivier,
thank you for your considerate and educative response! After having read up on the RFCs, I reckon that there does not seem to be much of a reason to change the code. However, I do realize now that the label "Anonymous bind" is a bit of a misnoner. I will change the label. > - 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). LDAPv3 allows for unauthorized access when no bind is performed [1]. Although [2] describes how compatibility with other versions of the protocol can be improved by anonymous binding, the only candidate LDAP version 2 was retired in 2003. Following rfc4513, I leave the code to query the LDAP server in the implied unauthorized state without performing the bind. With regards to Unauthenticated Authentication, this is provided for trace purposes only and is considered something which client application developers should protect themselves against using, as it is too easy to mistake such a bind featuring the actual user name for a succesful authentication [3]. Illustratively, the option is disabled by default in the OpenLDAP server for this reason [4]. Of course, there is SASL as an honourable authentication mechanism but it is somewhat out of scope for this particular effort. I will therefore not display a selection of options, but leave the checkbox. > - Out of curiosity, what LDAP server are you using, and with what kind of > specific settings? Default settings? OpenLDAP 2.4, with no specific settings with regards to authentication mechanisms. I will change the label Cheers, Stefan. [1] http://tools.ietf.org/html/rfc4513#section-4, paragraph no. 4 [2] http://tools.ietf.org/html/rfc4511#section-4.2.1 [3] http://tools.ietf.org/html/rfc4513#section-5.1.2 [4] http://www.openldap.org/doc/admin24/security.html#Authentication%20Methods -- Therp - Maatwerk in open ontwikkeling Stefan Rijnhart - Ontwerp en implementatie mail: [email protected] tel: +31 (0) 614478606 web: http://therp.nl 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 : [email protected] Unsubscribe : https://launchpad.net/~openerp-community More help : https://help.launchpad.net/ListHelp
