openerp-community team mailing list archive

["Stefan Rijnhart \(Therp\)" <stefan@xxxxxxxx>]

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: stefan@xxxxxxxx
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.