157925 - Change LDAP properties to be compatible / compliant with 'Official Mozilla LDAP schema'

Status: RESOLVED FIXED

(MailNews Core :: LDAP Integration, defect)

Description

[Roland Felnhofer]

Change the properties Mozilla uses when querying a LDAP server so they are
compliant and compatible with 'Official Mozilla LDAP schema'

Comment 1

[Srilatha Moturi]

over to dmose

Comment 2

[Roland Felnhofer]

Attachment: Draft - nsAbLDAPProperties patch

I know it's not perfect - it's just meant as a starting point

Comment 3

[Kristof Petr]

Damned. Roland, you are pretty quick.

I have had plan to make the exactly same patch this day lately :-)
Really.

Comment 4

[Roland Felnhofer]

Hi Petr,
I already used this patch quit a while for myself. ;-)

Comment 5

[Roland Felnhofer]

Hi Petr,
If you are so keen to write patches - I don't have a patch for the LDIF
export/import (bug 157926) ;-)
I think a lot of people (including myself) would highly appreciate such a patch.

Comment 6

[Roland Felnhofer]

Attachment: nsAbLDAPProperties patch V0.2

I added support for 3 attributes: 
	"postaladdress"
	"mozillapostaladdress2"
	"mozillahomepostaladdress2"
Now all used AB fields should be displayed if LDAP is used as address source
(using Mozilla schema V0.2).
I was not able to test this patch so far. If you discover any problems please
inform me.

Comment 7

[Roland Felnhofer]

I checked the new patch (nsAbLDAPProperties patch V0.2) and it seems to work!

Comment 8

[Dan Mosedale (:dmosedale, :dmose)]

I'd like the activity in 116692 to stabilize a bit before I take this in run
with it.  Looks like an excellent start, however.  Thanks!

Comment 9

[Roland Felnhofer]

Attachment: nsAbLDAPProperties patch V0.3

'mozillaWorkUrl' <-> 'mozillaHomeUrl'

Comment 10

[Kristof Petr]

Attachment: nsAbLDAPProperties patch V0.4

Used capital letters for easy reading on attribute names.

Comment 11

[John Marmion]

I have a query as a result of doing some investigation for bug 126749. It
transpires that the ldap attributes "cn,commonname" , "sn,surname",
"fax,facsimiletelephonenumber","l,locality" (and there may be others in the
mozilla ldap mapping) are in fact the one instance of aliases pointing to the
same OID in the ldap server schema. Thus when we create the ldap query: select
sn,surname, etc., we should simply be selecting sn which will return the correct
value. Is that everyone's else understanding of this and if so should we not
remove these redundant duplicates from nsAbLDAPProperties.cpp  

Comment 12

[Roland Felnhofer]

John - that's as well as my understanding.

Comment 13

[Roland Felnhofer]

Attachment: nsAbLDAPProperties patch V0.5

Keep up-to-date with tree:

+    // ?
+    {MozillaProperty_String, "_AimScreenName",        "nscpAimScreenName"},

Comment 14

[Roland Felnhofer]

Attachment: nsAbLDAPProperties patch V0.6

I removed the Uppercase letters again! It lead to some problems.

Comment 15

[Roland Felnhofer]

Attachment: nsAbLDAPProperties patch V0.7

Changed "nscpaimscreenname" to "nsaimid" so it complies with schema proposal

Comment 16

[Chan Min Wai]

Attachment: nsAbLDAPProperties patch V0.7a

This is the modification of 0.7 by restore the remove line so that it will be
compatible with the old schema until migration patch is avaliable.

Comment 17

[Chan Min Wai]

Bug 116692 need this patch to become usable.
request for review and checkin into 1.3 Beta.

Comment 18

[Dan Mosedale (:dmosedale, :dmose)]

I suck for not getting back to this much sooner; sorry everyone.  I'm on
vacation for the next week, but this is too late for 1.3 anyway, I think.  I'll
try and look at it soon.

Comment 19

[Eric Nichols]

I've tested this patch with OpenLDAP and a custom schema and it works
wonderfully both on Linux & Win32.  Please consider this for the production code...

Comment 20

[Chan Min Wai]

Attachment: nsAbLDAPProperties patch V0.8

This is a newly modified version that can work with patch (the 0.7a) should not
work.

Thank You

Comment 21

[Roland Felnhofer]

Hi Dan,

can we check it in, finally? In Comment #18 you said you would look at it soon -
that's quit long ago (2003-02-14)!

Comment 22

[Scott MacGregor]

dmose needs to approve this before I'll sr it. 

Comment 23

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 142156 [details] [diff] [review]
nsAbLDAPProperties patch V0.8

I'm sorry, but I'm not a qualified reviewer for this patch.  I know nothing
about ldap or any of the other issues involved.

Comment 24

[Dan Mosedale (:dmosedale, :dmose)]

Sorry, I'll try and get to this soon (ie the next several weeks).  Apologies for
the long delay.

Comment 25

[John Woodell]

I have a couple test server setup. The NS server uses an older schema,
so things like 'custom1' show up, but there are other issues, like the
fact that 'street' NEVER shows up when AB is pointed to an openldap server.

Netscape 4x Directory:
ldapsearch -x -LLL -h ns4xldap.netpress.com -b dc=netpress,dc=com sn=smith

OpenLDAP 2x Directory:
ldapsearch -x -LLL -h openldap.netpress.com -b dc=netpress,dc=com sn=smith

Comment 26

[Roland Felnhofer]

(In reply to comment #24)
> Sorry, I'll try and get to this soon (ie the next several weeks).  Apologies for
> the long delay.
> 

Hi Dan,

can we get this patch checked in finally??
In addition we should also approve and check-in the official Mozilla LDAP schema
extension (bug 116692).

When do you think it can be done??

Comment 27

[Simon Paquet [:sipaq]]

Comment on attachment 142156 [details] [diff] [review]
nsAbLDAPProperties patch V0.8

Dan, you promised to look at this a few times already. Could you please try to
look at this, when you have the time. Thank you.

Comment 28

[Simon Paquet [:sipaq]]

Comment on attachment 112305 [details]
nsAbLDAPProperties patch V0.7a

patch is obsolete, removing review request.

Comment 29

[Rene Rask]

Dan, please review this bug and bug #116692 which is the ldap schema. These bugs
seem to be stalled again.
Thanks
Rene

Comment 30

[John Woodell]

Some LDAP servers have issues with attributes that start with underscore,
but we should be able to use the altername attribute name.

   {MozillaProperty_String, "_AimScreenName",        "nsaimid"},

Comment 31

[John Woodell]

I'd like to know if it is possible to lowercase the attribute names
when they come from the ldap server, then compare to a lowercase list? 
I have found that netscape schema filess have attributes in lowercase, 
while openldap schela files are mixed. This may make the list of attributes 
in the pasth smaller, and make it more compatible with OpenLDAP!

Here is what I have found in other clients:

ldapsearch (command-line)... displays same case as in schema files
perl-ldap API............... attributes can be extracted in by providing
                             attribute names in any case (this works great)
PHP built-in LDAP........... attribute names (array keys) are lowercased
                             so you always need to extract them in lowercase
PHP/Pear NET_LDAP API....... must use exact-case when extracting attributes

Comment 32

[John Woodell]

The OpenLDAP core schema has no 'zip' attribute, but I could add it here...

attributetype ( 2.5.4.17 NAME 'postalCode'
        EQUALITY caseIgnoreMatch
        SUBSTR caseIgnoreSubstringsMatch
        SYNTAX 1.3.6.1.4.1.1466.115.121.1.15{40} )

It would be best to fix this and other attributes in the patch, like this...

@@ -132,6 +155,14 @@
     {MozillaProperty_String, "WorkZipCode",        "zip"},
+    // ?
+    {MozillaProperty_String, "WorkZipCode",        "postalCode"},

To make sure this patch is comprehensive, I will generate a 
complete list of attributes based on the following...
- The official mozilla schema
- What is currently exported from AB
- The netscape and OpenLDAP schema files

I try to do this tonight!

Comment 33

[Roland Felnhofer]

(In reply to comment #30)
> Some LDAP servers have issues with attributes that start with underscore,
> but we should be able to use the altername attribute name.
> 
>    {MozillaProperty_String, "_AimScreenName",        "nsaimid"},

Hi John, as far as I know is the first string (in this case "_AimScreenName")
not an LDAP attribute or used as such. It's just the Mozilla internal name. In
case I'm right the underscore is not a problem.

Comment 34

[John Woodell]

Attachment: netscape attributes

This is a tidy list of attributes from a Netscape Directory server

Comment 35

[John Woodell]

Attachment: openldap attributes

This is a tidy list of attributes from an OpenLDAP server

Comment 36

[John Woodell]

Attachment: nsAbLDAPProperties patch V0.8w

I have added a some missing attributes, made some comments.
I found the the ou/department/departmentNumber was a problem
so I reordered it to give users the ability to use ou and 
departmentNumber if they want. Here is the comment...

// Use 'department' before 'departmentnumber' & 'ou' because some people
// feel compelled to set 'ou' to 'People' (or whatever the branch is),
// even thought this in not necessary.	We want to be able to assign
// a number to 'departmentnumber', but display the name in the AB,
// so we will use the legacy (undefined) attribute 'department' for this.
// We can add a new attribute 'mozillaDepartment' to 'mozillaOrgPerson'
// so that this attribute won't be so mysterious to assign.

Comment 37

[John Woodell]

I will create a chart that indicated the order in which attribute names will
be used (tossing the previous attribute) and why, so we can have something
to look at when we ask, "why are we doing this?".

Currently, there a few 'made up' attribute names, that will replace other 
defined attribute names. Example: ou -> deartment -> departmentNumber.
I would like to use departmentNumber which is in inetOrgPerson, but it will
show up in th AB, instead of ou. I created a 'department' attribute to do 
what I need, but I'd like to have a 'design' behind the assignment order.

Without the chart, this is difficult to discuss here, so I'll get to it.

Comment 38

[John Woodell]

Attachment: nsAbLDAPProperties patch V0.8w2

Here is an updated patch with aq couple more attributes from activedirectory.
I put the c/co order back to frindly name wins, I dumped mozillaDepartment
because the legacy name department has been defined, and I added a homeAddress
atribute picking up on the momentun of 'street' replacing 'postalAddress'.

Comment 39

[John Woodell]

Attachment: nsAbLDAPProperties patch V0.8w3

same, but better format...

Comment 40

[John Woodell]

Attachment: nsAbLDAPProperties patch V0.8w4

Change mozillaHomePostalAddress to mozillaHomeStreet per bug 116692 coment #173

Comment 41

[John Woodell]

Attachment: nsAbLDAPProperties patch V0.8w5

Put undefined/legacy attributes first (// ?), then alternate namess.
Perferred attribute name for export is not always last, as with displayName.
Cleaned up the home and work address names.,,

  {MozillaProperty_String, "HomeAddress",	 "mozillahomestreet"},
  {MozillaProperty_String, "HomeAddress2",	 "mozillahomestreet2"},
  {MozillaProperty_String, "WorkAddress",	 "street"},
  {MozillaProperty_String, "WorkAddress2",	 "mozillaworkstreet2"},

Comment 42

[John Woodell]

Attachment: MozillaProperty_String additions

I wanted to generate this diff of the old and new MozillaProperty_String
entries,
so that we can clearly see what was added without the comments and ordering
adding complexity. The first thing I noticed, is that the current file has a
duplicate entry for  "WorkCountry","countryname".

Comment 43

[Dan Mosedale (:dmosedale, :dmose)]

*** Bug 118585 has been marked as a duplicate of this bug. ***

Comment 44

[John Woodell]

The current version of mozilla cannot browse WorkAddress2 or HomeAddress2.
I suggest that we NOT add this now, and we especially NOT define attributes
for this in the schema file (a second address line seems very rare). 

Instead 'we' should add this to a future release when we have time to modify
nsAbQueryLDAPMessageListener::OnLDAPMessageSearchEntry so that once val[0] in
nsAbDirectoryQueryPropertyValue() has been extracted, we look for vals[1],
and also look for an internal property name with '+2' at the end of the one
we just set... and set it.

We are able to use common attributes for everything in the work address,
and it seems foolish to define a 'street2' when 'street' is multivalued.

Comment 45

[John Woodell]

In my testing, the 'Get Map' button does not use the address2 data
when you put something in there.

------------------------------------

On another note, I don't like how browsing LDAP entries in the AB look disabled
when you double click to the the Edit Card dialog.

Comment 46

[John Woodell]

Attachment: nsAbLDAPProperties patch V0.8w6

Given the order-used is out-of-control, we cannot have less preferable
attributes replacing ones in the schema. The homePostalAddress and
postalAddress
attributes are examples of this. Also , we don't want 'United Stetes of
America' in the co pushing c out of the way and busting the 'Get Map' button.

I ditched the 'street2' attributes because there is a better way to do this
in a future release that won't require schema changes.

Lastly, I attempted to fudge the order so that the changes don't look so bad.
The only non-comment that should be removed is the duplicate entry for...
{MozillaProperty_String, "WorkCountry", "countryname"}

Comment 47

[John Woodell]

Attachment: nsAbLDAPProperties patch V0.8w7

The homefriendlycountryname attribute needs to go as well. Also, there is no
place to enter birthyear, and there is no schema defined for it.

Comment 48

[John Woodell]

Attachment: nsAbLDAPProperties patch V0.8w8

This one contains homeZipCode (my bad)

Comment 49

[John Woodell]

Attachment: attributes names...

These are the attribute names that I think we should ask for.
Note that some properties have more than one LDAP attribute.

Comment 50

[John Woodell]

Attachment: nsAbLDAPProperties patch V0.8w9

We can simply use use 'url' instaed of defining a new attribute,
and still support the legacy 'homeurl' as well.

Comment 51

[John Woodell]

Attachment: nsAbLDAPProperties patch V0.8w10

small shuffle to make the diff less complex

Comment 52

[John Woodell]

Attachment: nsAbLDAPProperties patch V0.8w11

simplify the diff a bit more

Comment 53

[John Woodell]

Attachment: nsAbLDAPProperties patch V0.8w12

I had a capitalizaton issue in the last patch

Comment 54

[Dan Mosedale (:dmosedale, :dmose)]

(In reply to comment #44)
> The current version of mozilla cannot browse WorkAddress2 or HomeAddress2.
> I suggest that we NOT add this now, and we especially NOT define attributes
> for this in the schema file (a second address line seems very rare). 
> 
> Instead 'we' should add this to a future release when we have time to modify
> nsAbQueryLDAPMessageListener::OnLDAPMessageSearchEntry so that once val[0] in
> nsAbDirectoryQueryPropertyValue() has been extracted, we look for vals[1],
> and also look for an internal property name with '+2' at the end of the one
> we just set... and set it.
> 
> We are able to use common attributes for everything in the work address,
> and it seems foolish to define a 'street2' when 'street' is multivalued.

I've already voiced my thoughts about the problems with using multivalued
attributes for this sort of thing because of ordering issues.  My specific
concern with mozilla is that if we do something that relies on attribute
ordered, sooner or later, we're going to run across an LDAP server that follows
that particular part of the RFC to the letter, and we won't work with it, and
we'll be in a world of hurt.

Perhaps we can investigate better ways of dealing with multi-attribute ordering
down the line in another bug, but I'd like to get a first cut of the schema
landed that supports all addressbook attributes now.

Comment 55

[Dan Mosedale (:dmosedale, :dmose)]

(In reply to comment #45)
> In my testing, the 'Get Map' button does not use the address2 data
> when you put something in there.
> 
> ------------------------------------
> 
> On another note, I don't like how browsing LDAP entries in the AB look disabled
> when you double click to the the Edit Card dialog.

These sounds like they should both be filed as separate bugs.

Comment 56

[John Woodell]

(In reply to comment #54)
> if we do something that relies on attribute ordered, sooner or later, 
> we're going to run across an LDAP server that follows that particular 
> part of the RFC to the letter...

If multi-valued attributes truely are unordered, then yes this is a disater,
but I've started thinking that this is a myth. I'll keep looking for that RFC.
Given that multi-valued is this default for an attribute, and 'street' is 
already multi-valued, I would not consider it uncharted territory.

A bigger issue for the current AB LDAP support is the attribute assign-order,
so I opted towards removing attributes that may contain something bad, 
and replace the correct attribute.

Comment 57

[John Woodell]

OK, I have what I was looking for. Multi-valued attributes are unordered.

> The RFC defines no order, and you should never assume they will be ordered. 
> However, they do seem to come back in the order they were written. 
> That behavior could change without warning.

Comment 58

[John Woodell]

Attachment: nsAbLDAPProperties patch V0.8w12

put back street2 attributes.

Comment 59

[Dan Mosedale (:dmosedale, :dmose)]

(In reply to comment #57)
> OK, I have what I was looking for. Multi-valued attributes are unordered.
> 
> > The RFC defines no order, and you should never assume they will be ordered. 
> > However, they do seem to come back in the order they were written. 
> > That behavior could change without warning.
> 

For reference, the clause in question is the last sentence in section 4.1.8 of
RFC 2251, which says, "The order of attribute values within the vals set is
undefined and implementation-dependent, and MUST NOT be relied upon."

Comment 60

[John Woodell]

Attachment: nsAbLDAPProperties patch V0.8w-order

Added more comments, legacy attributes are labeled as such.
Has been reordered assuming that first attribute wins.
Alternate attributes do not need to be requested.

Assumes adschema attributes are valid for url and department.

Comment 61

[John Woodell]

Attachment: published attribute names

The attribute names to the right would be requested from LDAP.
Alternate and legacy attributes would not (and are not included)

This could also replace the table on this page:
http://www.mozilla.org/projects/thunderbird/specs/ldap.html

Comment 62

[John Woodell]

Attachment: published attribute names

now with preferredou

Comment 63

[John Woodell]

Attachment: nsAbLDAPProperties patch V0.9

This contains preferredou and has first-one-wins order.

Comment 64

[John Woodell]

Attachment: nsAbLDAPProperties patch V0.9.1

The 'fax' attribute is altername

Comment 65

[John Woodell]

Attachment: publish attributes

replaced 'fax' with 'facsimiletelephonenumber'

Comment 66

[John Woodell]

Attachment: nsAbLDAPProperties patch V0.9.2

Push url down so that NO adschema attributes are required 
in the objectclass. Add mozillaWorkUrl to take its place.

Comment 67

[Anze Zagar]

Has there been made any effort to replace the entire nsAbLDAPProperties.cpp code
with a more flexible alternative as requested in bug #119291 ?

Once that bug is fixed, changes made to mozilla ldap schema from #116692 will no
longer require entire rebuild of thunderbird (or wait for the next release), but
more likely only a replace of 'mailnews.js' file.

That would also give users the ability to override these properties in
'prefs.js' file to have them comply with their own ldap directory based on e.g.
outlook schema.

Comment 68

[John Woodell]

(In reply to comment #66)

The IBM schema browser seems to list 'url' as an official attribute:
http://www-1.ibm.com/servers/eserver/iseries/ldap/schema/html/Attribute/url.html

Comment 69

[James Lick]

Comments based on nsAbLDAPProperties patch V0.9.2:

For the most part I've checked and agree with the current state of the ordering
and attribute naming in this version.  My remaining concerns are probably fairly
minor, and not all of them have a clear solution.

For webpage1, there is also the labeledURI attribute defined in RFC2079.  That
is an X.500 standard, but several LDAP servers have it as part of their standard
schema and as an allowed attribute for one of the person objectclasses.  However
it is not universal.  For example, openldap has it commented out in the standard
schema for the current version.  Nevertheless, I think it's worth putting in the
list for token WebPage1, since it is 'more standard' than some of the other
options there.  As for what ordering to use, it probably doesn't matter for this
one.

Also for token LastModifiedDate, there appears not to be a standard for this,
but the attribute lastModifiedTime seems to be a common variant, probably enough
to be added.  Again, ordering doesn't seem to matter here.

Also as a general comment, it seems that this bug has been stalled for quite a
while.  Currently the WorkAddress and WorkCountry tokens are broken in current
Mozila products because they look for the alternative attribute names
streetAddress and countryName, not the primary attribute name used in standard
schemas and returned by compliant LDAP servers.  Can we at least get fixes out
for that problem soon?

Comment 70

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 142156 [details] [diff] [review]
nsAbLDAPProperties patch V0.8

This version is obsolete.  There's still a bit more discussion that I need to
drive in 116692, which I'll do soon, before we're ready to go here.

Comment 71

[Dan Mosedale (:dmosedale, :dmose)]

This is fixed, thanks to changes from bug 119291 and bug 116692.