343721 - Password prompt required on start for each remote webdav calendar

Status: VERIFIED FIXED

(Calendar :: Internal Components, defect)

Description

[Matthew Hall]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.1) Gecko/20060203 Fedora/1.5.0.1-1.1.fc4.nr Firefox/1.5.0.1
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.1) Gecko/20060203 Fedora/1.5.0.1-1.1.fc4.nr Firefox/1.5.0.1

Each time I start sunbird (CVS trunk as of 2006/07/05) I get prompted for my password for every remote webdav calendar I have subscribed to (which is around a dozen). The behaviour was slightly different in SB 0.3a2 - I was only asked for my password *once* when starting SB (though this is still wrong).

SB *does* pickup the saved password details from key3.db (?) and already has my username and password entered into the fields for me - so all I have to do is 'OK' several dialogs, but I think, as it already has the saved details, send the auth info to the server first, and only ask for the username and password if authentication fails.

Reproducible: Always

Steps to Reproduce:
1. Subscribe to some remote webdav calendars
2. Start SB
3. Watch how you get a lot of password prompts, click ok many times. Restart SB, rinse, repeat.

Actual Results:  
I was prompted for my password details for each calendar subscribed to.

Expected Results:  
That SB transparently authenticates me to the remote server using my stored credentials and only ask for my username/password if authentication fails.

It doesn't make any difference if I tell SB to not reload calendars on startup. It still reloads the calendars anyway (different bug) and prompts me everytime.

Comment 1

[Reed Loden [:reed]]

The bugspam monkeys have struck again. They are currently chewing on default assignees for Calendar. Be afraid for your sanity!

Comment 2

[Matthew Hall]

Adding +blocking0.3  - I'm still seeing this in nightly latest (20060807)

Comment 3

[Matthew (lilmatt) Willis]

(In reply to comment #2)
> Adding +blocking0.3  - I'm still seeing this in nightly latest (20060807)

Unless you're a calendar 0.3 driver, please nominate bugs for blocking by setting them to "?".  Please see http://wiki.mozilla.org/Calendar:For_Everyone:Blocking_Flags for details.

-> blocking0.3? for Matthew Hall

Comment 4

[Matthew Hall]

Apologies. I didn't notice that. My bad :)

I'd be happy to start coding if anyone could point me in the right direction. I'm a stubborn C coder with little spare time, so would like some displacement activities. The whole mozilla/calendar code base is completely alien to me.

Comment 5

[Joey Minta]

(In reply to comment #4)
> Apologies. I didn't notice that. My bad :)
> 
> I'd be happy to start coding if anyone could point me in the right direction.
> I'm a stubborn C coder with little spare time, so would like some displacement
> activities. The whole mozilla/calendar code base is completely alien to me.
> 
http://wiki.mozilla.org/Calendar:Dev_Guide is intended to be a high-level view of the mozilla/calendar code-base.  In your case, you probably want to poke at providers/ics/calICSCalendar.js.  That's the component that handles loading/parsing webdav (ics) calendars.

Comment 6

[Thorsten 'nightrat' Fritz]

Confirm this bug with Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20060812 Calendar/0.3a2+

This behaviour confuses users as Sunbird should only ask if the authentication failed.

Comment 7

[Sascha Nemecek]

Just wanted to confirm, that this problem also exists on the windows builds: 

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060817 Calendar/0.3a2+

But there is also one difference: 
 - only the first password promt is displayed properly 
 - the other password promts are minimizend and cannot be maximized. you have to close them 

Comment 8

[Celina]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060821 Calendar/0.3a2+

Bug is still occurring in 20068021 with the following changes:
Upon first subscribing to remote calendar, 3 password prompts (all fully visible, not just titlebar)
Upon Calendar close/relaunch, 5 password prompts (all fully visible).

Number of tasks/events does not appear to effect the number of prompts.  One thing noticed is after the 2nd prompt on first subscribing, and 4th prompt on re-launch, tasks/events are loaded and focus is taken from the password prompt.

Comment 9

[Celina]

(In reply to comment #8)

Apologies... this occurs using a CalDAV server/calendar, not WebDAV.  Copied comment to Bug# 335462.

> Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060821
> Calendar/0.3a2+
> 
> Bug is still occurring in 20068021 with the following changes:
> Upon first subscribing to remote calendar, 3 password prompts (all fully
> visible, not just titlebar)
> Upon Calendar close/relaunch, 5 password prompts (all fully visible).
> 
> Number of tasks/events does not appear to effect the number of prompts.  One
> thing noticed is after the 2nd prompt on first subscribing, and 4th prompt on
> re-launch, tasks/events are loaded and focus is taken from the password prompt.
> 

Comment 10

[Michiel van Leeuwen (email: mvl+moz@)]

To fix this, the providers need to talk to nsIPasswordManagerInternal. Then they can get auth info, and hopefully, pass that info to necko.
It might be best to put that code in a general, shared wrapper code
otoh, do we really want to write our own wrappers around necko? Maybe this should be fixed inside necko itself.

Comment 11

[Dan Mosedale (:dmosedale, :dmose)]

is nsIAuthPromptWrapper relevant here?

Comment 12

[Michiel van Leeuwen (email: mvl+moz@)]

I wonder how thunderbird has fixed this.

Comment 13

[Bruno Browning]

It's not clear from the description whether the webdav calendars mentioned in this bug are on the same webdav server, but if so I suspect that bug 335462, and particularly comment #5, are apropos here. If so, loading remote .ics calendars sequentially instead of all-at-once could go a long way towards fixing this bug.

Comment 14

[Michiel van Leeuwen (email: mvl+moz@)]

Attachment: auto login v1

This patch implements our own prompter, that tries to get the password out of the passwordmanager first, before actually prompting. That makes us auto-login when possible.

Comment 15

[Matthew (lilmatt) Willis]

Comment on attachment 236992 [details] [diff] [review]
auto login v1

>Index: providers/ics/calICSCalendar.js
>===================================================================
>     getInterface: function(iid, instance) {
>         if (iid.equals(Components.interfaces.nsIAuthPrompt)) {
>-            // use the window watcher service to get a nsIAuthPrompt impl
>+            return new calAuthPrompt();
>             return Components.classes["@mozilla.org/embedcomp/window-watcher;1"]
>                              .getService(Components.interfaces.nsIWindowWatcher)
>                              .getNewAuthPrompter(null);
Remove the extra/old return

>+calAuthPrompt.prototype = {
>+    prompt: function(aDialogTitle, aText, aPasswordRealm, aSavePassword, 
Nit: trailing space ^^^
>+                     aDefaultText, aResult) {
>+        return this.mPrompter.prompt(aDialogTitle, aText, aPasswordRealm,
>+                                     aSavePassword, aDefaultText, aResult);
>+    },
>+    
Nit: trailing spaces ^^^

>+        } else {
>+            return this.mPrompter.promptUsernameAndPassword(aDialogTitle, aText, 
Nit: trailing space ^^^

>+    promptPassword: function(aDialogTitle, aText, aPasswordRealm, 
Nit: trailing space ^^^

>+                             aSavePassword, aPwd) { 
Nit: trailing space ^^^


r1=lilmatt with those fixed

Comment 16

[Matthew Hall]

In response to the query regarding whether these webdav calendars are on the same server, yes they were - so I suspect sequential loading may solve the problem to an extent (being prompted once for each .ics on a particular server would be fine with me).

I'll try the patch out asap and get back with my results.

Comment 17

[Michiel van Leeuwen (email: mvl+moz@)]

For 0.3, We can live with having to click 'ok' a few times. (If the patch gets reviewed, it might still get checked in)

Comment 18

[Matthew Hall]

I disagree Michiel - I don't believe users (including me) will like living with having to press 'Ok' for every remote calendar subscribed to (I would think this is the main 'use case'?).

This is also a regression against 0.3a2 - we* should not ship a newer version with such an obvious regression.

If the patch supplied solves the problem (as small as it is) - it should be reviewed and committed before 0.3 goes out.


(*) - When I say 'we', I haven't committed any code, but do consider myself an active floss supporter/contributor.

Comment 19

[Sascha Nemecek]

I have to agree with Matthew. It's a big annoyance if you have to click 'OK' several times. That is, if the 'OK' button is displayed. I still have the problem that only the first login dialogue is properly displayed. The others - four in my case - are minimized and cannot be maximized. 

I'm using: 
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060911 Calendar/0.3a2+

Comment 20

[Mike Coppins]

Using the Win32 version of 0.3 RC1, I have to re-enter my auth details once when loading SB, then I have to click OK on the rest of the password prompts (If I don't enter my auth details once and just click on OK, it continually asks me way beyond the 7 calendars I have set up).

Otherwise, One thing I notice is that when I am clicking OK quickly, I see my authentication details (well, password in asterisks) occasionally flash in the text boxes then disappear.

Comment 21

[Matthew Hall]

Apologies for the slow feedback (holidays). 

I've applied the supplied patch to the 0.3rc2 codebase and can confirm it solves the auth prompt problem - and so far without any visible regressions.

I'll test a little more with different configurations just to get a feel for stability.

It'd be really nice to see this being applied before 0.3 gets out of the door :)

Comment 22

[David Berlioz]

Hi,

I've build a WebDav/Apache2 Server and tried the webdav features of Sunbird (0.3 fr).

I ve played with ldap auth...

I ve created 3 iCalendars (ics) on my webdav server...

it works fine tu update etc...

but at startup sunbird prompts login/password for every online calendars... so it popups 3 login/password dialog in a row.

maybe it should wait the user fill in the first dialog to promt the others... and if it the same URL, just not prompt again ;)

cheers

Comment 23

[Mike Coppins]

I think the OS field should be changed to all, since it also affects win32.

Comment 24

[Matthew Hall]

Can we set this patch inclusion as a target for 0.5? 

It affects win32 and linux in the same manner, and the supplied patch resolves the problem in both cases.

If someone would educate me in what the blocking factor is, i'd happily follow that up.

Comment 25

[Matthew (lilmatt) Willis]

(In reply to comment #24)
> Can we set this patch inclusion as a target for 0.5? 
Sure. Done.

> If someone would educate me in what the blocking factor is...
The blocking problem is this:
We have three peers who can give r2: dmose, mvl and jminta.

dmose is stuck on jury duty, so his review queue is filling up.
mvl wrote the patch, so he can't review it.
jminta's away at law school, so he generally isn't around to review.

Hopefully dmose can get to this soon.

Comment 26

[Matthew (lilmatt) Willis]

Comment on attachment 236992 [details] [diff] [review]
auto login v1

Moving r2 to jminta per dmose

Comment 27

[Joey Minta]

Comment on attachment 236992 [details] [diff] [review]
auto login v1

-            // use the window watcher service to get a nsIAuthPrompt impl
+            return new calAuthPrompt();
             return Components.classes["@mozilla.org/embedcomp/window-watcher;1"]
lilmatt already mentioned about removing this line.

+function calAuthPrompt() {
+    // use the window watcher service to get a nsIAuthPrompt impl
+    this.mPrompter = Components.classes["@mozilla.org/embedcomp/window-watcher;1"]
+                               .getService(Components.interfaces.nsIWindowWatcher)
+                               .getNewAuthPrompter(null);
+    this.mTriedStoredPassword = false;
Shouldn't you have mPrompter and mTriedStoredPassword defined in the prototype initially?

+    prompt: function(aDialogTitle, aText, aPasswordRealm, aSavePassword, 
+                     aDefaultText, aResult) {
Anonymous functions are bad. :-(

Does this want to live in calUtils.js, so it can be shared with, for instance, caldav?

Lastly, can you explain why we want a new calAuthPrompt each time, rather than a single one to always use?

Comment 28

[Michiel van Leeuwen (email: mvl+moz@)]

(In reply to comment #27)
> Does this want to live in calUtils.js, so it can be shared with, for instance,
> caldav?

I'm not going to put something in calutils.js, just because something might use it in the future. When caldav needs it, the code can be moved.

> Lastly, can you explain why we want a new calAuthPrompt each time, rather than
> a single one to always use?

Because calAuthPrompt has state.

Comment 29

[The Anarcat]

This is a major annoyance for me, and a regression from the Firefox extension I was using before. I would very much have liked to see 0.3 with that fix in... Is there a nightly I can use that has it?

Comment 30

[Joey Minta]

Comment on attachment 236992 [details] [diff] [review]
auto login v1

+        var found = false;
+        var pw;
+        if (!this.mTriedStoredPassword) {
+            pw = this.getPasswordInfo(aPasswordRealm);
+            found = pw.found;
+        }
+
+        if (found) {

I would write each of these blocks as
var pw;
if (!this.mTriedStoredPassword) {
    pw = this.getPasswordInfo(aPasswordRealm);
}

if (pw && pw.found)
saves a variable and makes a bit more sense to my eyes.
r2=jminta with that and the above comments.

Comment 31

[Michiel van Leeuwen (email: mvl+moz@)]

patch checked in

Comment 32

[Mike Coppins]

This may be a common and irritating question to ask, but if I want to test a build with this patch, what build should I go for?

Comment 33

[The Anarcat]

I tested the nightly builds at http://ftp.mozilla.org/pub/mozilla.org/calendar/sunbird/nightly/latest-trunk/. I tested three days ago and today. 3 days ago, it was not working. Today, it's still not working, and this time I took care of flushing my sunbird configuration (rm -rf .mozilla/sunbird) before starting it.

I define "not working" as: "Sunbird is asking me N times my password for the N calendars on the same servers." The upside is that if I make it remember my password, it doesn't prompt me at all anymore, which is nice, but I don't want to store my password in Sunbird.

I think this bug should be reopened.

So I guess this also answers comment #32. 

Comment 34

[Michiel van Leeuwen (email: mvl+moz@)]

No, this bug is fixed. Read comment 0 carefully:
> That SB transparently authenticates me to the remote server using my stored
> credentials and only ask for my username/password if authentication fails.
That's how the current behaviour is. Your problem is a different bug.

Comment 35

[The Anarcat]

(In reply to comment #34)
> No, this bug is fixed. Read comment 0 carefully:
> > That SB transparently authenticates me to the remote server using my stored
> > credentials and only ask for my username/password if authentication fails.
> That's how the current behaviour is. Your problem is a different bug.
> 

Fair enough, I've filed #365036. Thanks!

Comment 36

[Matthew (lilmatt) Willis]

(In reply to comment #15)
> >     getInterface: function(iid, instance) {
> >         if (iid.equals(Components.interfaces.nsIAuthPrompt)) {
> >-            // use the window watcher service to get a nsIAuthPrompt impl
> >+            return new calAuthPrompt();
> >             return Components.classes["@mozilla.org/embedcomp/window-watcher;1"]
> >                              .getService(Components.interfaces.nsIWindowWatcher)
> >                              .getNewAuthPrompter(null);
> Remove the extra/old return


It looks as if that review comment didn't make it into what got checked in.
We still have two return statements.

http://lxr.mozilla.org/mozilla1.8/source/calendar/providers/ics/calICSCalendar.js#562

REOPENING to get this fixed.

Comment 37

[Michiel van Leeuwen (email: mvl+moz@)]

fixed

Comment 38

[Damian Szczepanik]

verified with
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a2pre) Gecko/20070112 Calendar/0.4a1