1278281 - autoconfig should have the secure ldap support

Status: RESOLVED FIXED

(Core :: AutoConfig (Mission Control Desktop), defect, P2)

Description

[karel-de-macil]

Attachment: bug-146566-fix.patch

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:46.0) Gecko/20100101 Firefox/46.0
Build ID: 20160425115534

Steps to reproduce:

trying to use a secure ldap server listening on port 636 with the function  getLDAPAttributes


Actual results:

it failed cause the function is not designed to be use with a secure ldap server. The function should allow more configuration as telling if you want to use a secure ldap and the port where it listen.


Expected results:

the function getLDAPAttributes should work with a secure ldap server and have the argument to make it work. I join a patch who add 2 optionnal arguement to the getLDAPAttributes function. The patch make it possible to use a secure ldap server.

Comment 1

[Mike Kaply [:mkaply]]

What is the port and protocol for a secure ldap server?

Comment 2

[Mike Kaply [:mkaply]]

Thanks for your patch. I would rather see us default to the old behavior and only make changes for the  new behavior.

How often is the port/protocol for secure ldap different?

Could we add a boolean that just specifies whether or not the connection is secure?

Comment 3

[karel-de-macil]

this is what happend with my function as the arguemnts i add are optionnal and set by default at the old behavior value . The only difference is that if you use more arguement you can change the default value and then make secure ldap work ...
old call to the getLDAPAttributes was done like that :

getLDAPAttributes( ldapHost, ldapBase, ldapFilter, ldapAttrs.join(","))
and generate this string : "ldap://ldapHost/...."

my function still work with the old syntax the only difference is that the string generated is now :
"ldap://ldapHost:389/..." which is pretty much the same as the default ldap port is 389

and my function can also be use with 2 optional argument as in the following :

getLDAPAttributes( ldapHost, ldapBase, ldapFilter, ldapAttrs.join(","),"ldaps","636")

in this case, the string generated is :

"ldaps://ldapHost:636/..." which make the secure ldap connection possible (all the secure connection thing are allready present in the thunderbird core api it's juste the autoconfig js script who create the limitation.

Comment 4

[Mike Kaply [:mkaply]]

If secure ldap is always on port 636 and secure ldap is ldaps, there's really not a need to add the additional parameters.

I'd rather see a boolean that turns this on and does something like this:

var urlSpec = "ldap" + (isSecure ? "s" : "") + "://" + host + (isSecure ? "636" : "") + "/" + base + "?" + attribs + "?sub?" + filter;

This means the original behavior is unchanged and we only add stuff for the new behavior.

Comment 5

[karel-de-macil]

ok , if you think it's better this way no trouble i can submit a patch who will follow your wish. evantually and if you think it's usefull i can try to add authentication too.

i will submit the "boolean" patch tommorrow . Thanks for your attention

Comment 6

[karel-de-macil]

Attachment: bug-1278281-fix.patch

Comment 7

[Mike Kaply [:mkaply]]

Nit:

Could you add a space between attribs, isSecure?

Otherwise this looks great. I'll get it checked in this week.

Comment 8

[karel-de-macil]

Attachment: bug-1278281-fix.patch

add a space between attribs and  isSecure

Comment 9

[Mike Kaply [:mkaply]]

Sorry, I see one last thing. IT looks like quotes are missing around the s that's being added to ldap.

Comment 10

[karel-de-macil]

Attachment: bug-1278281-fix.patch

add the space

Comment 11

[karel-de-macil]

Attachment: bug-1278281-fix.patch

really remove the space

Comment 12

[karel-de-macil]

not the space, juste adds the quote. Time to sleep for me. should be ok now.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/47e94b41aba1
add SSL functionnality to ldap query in autoconf. r=mkaply

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/47e94b41aba1