Closed Bug 231965 Opened 17 years ago Closed 15 years ago

LDAP Download User Interface and function broken

Categories

(MailNews Core :: Address Book, defect)

x86
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: hadmut, Assigned: standard8)

References

(Regression)

Details

(Keywords: crash, fixed1.8, regression)

Attachments

(1 file, 1 obsolete file)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.5) Gecko/20031107 Debian/1.5-3
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6) Gecko/20040122 Debian/1.6-1

Hi,

I just stepped to Mozilla 1.6 and still found that LDAP 
address book download for offline working does still not work. 
This might be related to bugs 140131, 144204, 230211, but 
it covers more and might not be the same.

I found several problems which indicate that the whole 
LDAP address book module does not properly fit into the
current version of Mozilla and needs some update:

- When trying to download the LDAP directory, an window
  is opened and asks for username and password. 
  In contrast to the bug reports mentioned above I found
  that Mozilla _is_ doing something after entering these, 
  but the user interface is misleading:

  In a first step, mozilla tries to find an object where
  the "mail" attribute is equal to the given username. 
  A normal user has no chance to know this and that such 
  a user has to exist. When found, Mozilla indeed binds as
  this particular user.

  I'd suggest to complete remove this username/password dialog,
  since the BINDDN and password used to query the LDAP server
  is already known and stored in the password repository. 
  Asking again for username/password does not make sense.

- Even if Mozilla does bind successfully, it ignores the 
  basedn given in the configuration. This needs to be fixed.
  The basedn is NULL which obviously does not give any results.

- I'd like to remember the Filter bug, which is mentioned in 
  bugs 135273, 138478, 180012, 215958, 227482, and also applies
  here. This bug is ages old, Mozilla still does not use the
  filter expression.

So the LDAP part in common needs some refurbishment.

If you need some debugging support, just ask me.

regards
Hadmut


Reproducible: Always

Steps to Reproduce:
1. Enter an LDAP addressbook
2. Try to download for offline work
3.




A pretty good way to debug LDAP queries is to use
ethereal.
I might add that with Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7b)
Gecko/20040310 it is crashing. Right after entering the username/pw to bind to,
moz crashes. Talkback isn't avail.
Keywords: crash
*** Bug 140131 has been marked as a duplicate of this bug. ***
I can confirm this behaviour with Thunderbird 0.7.3 (20040812).
Agreed - LDAP download seems very broken. It asks for a password and pops up a
progress dialog but doesn't appear to actually download anything. Other users
have reported the same problem with our corporate ldap server (win2k active
directory).
Product: Browser → Seamonkey
Running Mozilla 1.7.5 on Windows 2000.  

I can confirm this behaviour in this release.

We use anonymous LDAP for address lookup, so the username/password dialog that
comes up when you click "Download" doesn't work for me.

I'd like to see this fixed.  I agree with all of the suggestions and findings of
the original poster.
Assignee: sspitzer → mail
Please fix this. It is a needed frature.
Assignee: mail → nobody
Component: Address Book → MailNews: Address Book
Product: Mozilla Application Suite → Core
QA Contact: nbaca → addressbook
Assignee: nobody → bugzilla
Attached patch Change to Download All (obsolete) — Splinter Review
This patch changes the default replication function from Change Log to Download
All. Whilst this isn't the ideal fix, Dan & I want to try and get at least
something working wrt replication for Thunderbird 1.5/SeaMonkey 1.0.

I have raised bug 311632 to cover changing back to the "Change Log" protocol
once we get this fix in - I couldn't see an easy way to fix that protocol.

The patch also fixes a crash in nsAbLDAPReplicationData by passing a nsCOMPtr
wrapped card rather than just a pointer to one.
Attachment #198903 - Flags: superreview?(bienvenu)
Attachment #198903 - Flags: review?(dmose)
Blocks: 311632
Comment on attachment 198903 [details] [diff] [review]
Change to Download All

Sorry, I've just noticed a couple of problems with this patch - new one coming
up.
Attachment #198903 - Attachment is obsolete: true
Attachment #198903 - Flags: superreview?(bienvenu)
Attachment #198903 - Flags: review?(dmose)
Attached patch Patch v2Splinter Review
This is the revised patch. We now don't use nsAbLDAPCard as there isn't much
point as all we do with it is copy it to a mdb card. The
SetCardPropertiesFromLDAPMessage function takes a nsIAbCard so we are ok there
as well.
Attachment #198906 - Flags: superreview?(bienvenu)
Attachment #198906 - Flags: review?(dmose)
*** Bug 270156 has been marked as a duplicate of this bug. ***
Attachment #198906 - Flags: superreview?(bienvenu) → superreview+
Comment on attachment 198906 [details] [diff] [review]
Patch v2

> 
>Index: mailnews/addrbook/src/nsAbLDAPReplicationData.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/mailnews/addrbook/src/nsAbLDAPReplicationData.cpp,v
>retrieving revision 1.13
>diff -u -r1.13 nsAbLDAPReplicationData.cpp
>--- mailnews/addrbook/src/nsAbLDAPReplicationData.cpp	26 Jul 2005 15:27:17 -0000	1.13
>+++ mailnews/addrbook/src/nsAbLDAPReplicationData.cpp	8 Oct 2005 09:24:06 -0000
>@@ -44,8 +44,7 @@
> #include "nsIAddrBookSession.h"
> #include "nsAbBaseCID.h"
> #include "nsAbUtils.h"
>-#include "nsAbMDBCard.h"
>-#include "nsAbLDAPCard.h"
>+#include "nsIAbMDBCard.h"
> #include "nsAbLDAPReplicationQuery.h"
> #include "nsProxiedService.h"
> #include "nsCPasswordManager.h"
>@@ -334,9 +333,24 @@
>     if(!mReplicationDB || !mDBOpen) 
>         return NS_ERROR_FAILURE;
> 
>-    nsAbLDAPCard card;
>+  nsresult rv = NS_OK;
> 
>-    nsresult rv = mAttrMap->SetCardPropertiesFromLDAPMessage(aMessage, &card);
>+  // Although we would may naturally create an nsIAbLDAPCard here, we don't

Don't need the "may" here, i think.

r=dmose
Attachment #198906 - Flags: review?(dmose) → review+
Comment on attachment 198906 [details] [diff] [review]
Patch v2

This patch fixes ldap replication/download (in address book) for offline use.
It also fixes a crash in that code that was due to not handling allocation
correctly.

At the moment ldap download/replication doesn't work at all, repeatedly
prompting the user for a username/password and never getting anywhere.

The fix is partially masking the problem in that the address book will download
all data. We'll restore change log download capability for future releases.
Attachment #198906 - Flags: approval1.8rc1?
Comment on attachment 198906 [details] [diff] [review]
Patch v2

Patch checked into trunk. I'm leaving this bug open until we get a resolution
on approval request.
Attachment #198906 - Flags: approval1.8rc1? → approval1.8rc1+
Fix checked in to trunk and branch.
Status: NEW → RESOLVED
Closed: 15 years ago
Keywords: fixed1.8
Resolution: --- → FIXED
*** Bug 284602 has been marked as a duplicate of this bug. ***
(In reply to comment #14)
> Fix checked in to trunk and branch.
> 

Are you sure this is fixed?  I can't seem to get 1.5 RC1 or the nightly build from Friday to work properly.  More specifically, it doesn't use the data I entered to bind to the server.  I guess if I make my server give out data without authenticating, it will work, but that isn't going to happen.  Here's part of my log file, note the BIND dn part:

Nov  6 19:23:29 thread slapd[31559]: conn=24 fd=10 ACCEPT from IP=26.71.161.84:64977 (IP=0.0.0.0:389) 
Nov  6 19:23:29 thread slapd[31559]: conn=24 op=0 BIND dn="" method=128 
Nov  6 19:23:29 thread slapd[31559]: conn=24 op=0 RESULT tag=97 err=0 text= 
Nov  6 19:23:29 thread slapd[31559]: conn=24 op=1 SRCH base="ou=staff,dc=example,dc=com" scope=1 deref=0 filter="(objectClass=*)" 
Nov  6 19:23:29 thread slapd[31559]: conn=24 op=1 SEARCH RESULT tag=101 err=0 nentries=0 text= 
Nov  6 19:23:29 thread slapd[31559]: conn=24 op=2 UNBIND 
Nov  6 19:23:29 thread slapd[31559]: conn=24 fd=10 closed 

It is certainly better than before, since it at least skips that business about trying to look up a user account from a 'mail=' filter.  I'm not confident enough in my LDAP knowledge to reopen the bug, but if someone could look at before the 1.5 release this I'd appreciate it.
(In reply to comment #16)
> (In reply to comment #14)
> > Fix checked in to trunk and branch.
> > 
> 
> Are you sure this is fixed?  I can't seem to get 1.5 RC1 or the nightly build
> from Friday to work properly.  More specifically, it doesn't use the data I
> entered to bind to the server. 

This WFM on a current thunderbird trunk build. I've verified the fix has gone into the branch, but I haven't been able to test a branch build yet. Are your normal ldap lookups working correctly?
(In reply to comment #17)
> This WFM on a current thunderbird trunk build. I've verified the fix has gone
> into the branch, but I haven't been able to test a branch build yet. Are your
> normal ldap lookups working correctly?

I've run some new tests to be sure and current.

Using 1.0.7 on Windows, I can get address completion working correctly from the directory.  Of course, the download feature is pooched on 1.0.x, so that isn't worth discussing.

Using 1.6a1 (20051108) on Mac OS X, I can get address completion working correctly as well, with slapd log output like so:

Nov  8 12:57:38 thread slapd[28805]: conn=4 fd=10 ACCEPT from IP=10.187.200.86:49533 (IP=0.0.0.0:389) 
Nov  8 12:57:38 thread slapd[28805]: conn=4 op=0 BIND dn="cn=Fletcher,ou=staff,dc=example,dc=com" method=128 
Nov  8 12:57:38 thread slapd[28805]: conn=4 op=0 BIND dn="cn=Fletcher,ou=staff,dc=example,dc=com" mech=SIMPLE ssf=0 
Nov  8 12:57:38 thread slapd[28805]: conn=4 op=0 RESULT tag=97 err=0 text= 
Nov  8 12:57:38 thread slapd[28805]: conn=4 op=1 SRCH base="ou=staff,dc=example,dc=com" scope=1 deref=0 filter="(|(cn=joe*)(mail=joe*)(sn=joe*))" 
Nov  8 12:57:38 thread slapd[28805]: conn=4 op=1 SRCH attr=cn mail 
Nov  8 12:57:38 thread slapd[28805]: conn=4 op=1 SEARCH RESULT tag=101 err=0 nentries=1 text= 
Nov  8 12:57:58 thread slapd[28805]: conn=4 op=2 UNBIND 
Nov  8 12:57:58 thread slapd[28805]: conn=4 fd=10 closed 

Beautiful stuff.  I love it.  If I search for a contact in the Compose window's Contacts sidebar, I can get all the other contact info for one person as well through the Properties option.

In Address Book, I see two problems.  The lack of a BIND dn means I can only get decent results in the log file if I enable 'anonymous read' access for my contact list.  When I do this, I can get identical log file output with Thunderbird and ldapsearch.  Unfortunately, even though Thunderbird appears to connect properly, it doesn't actually populate the address book with data it should have downloaded.  Here's what a 'successful' Thunderbird download looks like:

Nov  8 13:18:27 thread slapd[28805]: conn=8 fd=10 ACCEPT from IP=10.187.200.86:49537 (IP=0.0.0.0:389) 
Nov  8 13:18:27 thread slapd[28805]: conn=8 op=0 BIND dn="" method=128 
Nov  8 13:18:27 thread slapd[28805]: conn=8 op=0 RESULT tag=97 err=0 text= 
Nov  8 13:18:27 thread slapd[28805]: conn=8 op=1 SRCH base="ou=staff,dc=example,dc=com" scope=1 deref=0 filter="(objectClass=*)" 
Nov  8 13:18:27 thread slapd[28805]: conn=8 op=1 SEARCH RESULT tag=101 err=0 nentries=23 text= 
Nov  8 13:18:29 thread slapd[28805]: conn=8 op=2 UNBIND 
Nov  8 13:18:29 thread slapd[28805]: conn=8 fd=10 closed 

Note the nentries=23, which nicely matches the number of people in the contact list.  As I said, the log file output from ldapsearch is identical, although ldapsearch actually shows the corresponding contact records.
I've now moved the autheticated connection case into bug 316170 as I've been able to confirm it myself.
Product: Core → MailNews Core
No longer blocks: 311632
Regressed by: 311632
Keywords: regression
You need to log in before you can comment on or make changes to this bug.