Last Comment Bug 560793 - Multiple (master) password prompts on startup when using news authentication - Hook up NNTP to msgAsyncPrompt service
: Multiple (master) password prompts on startup when using news authentication ...
Status: RESOLVED FIXED
: regression
Product: MailNews Core
Classification: Components
Component: Networking: NNTP (show other bugs)
: Trunk
: All All
: -- normal with 5 votes (vote)
: Thunderbird 13.0
Assigned To: Joshua Cranmer [:jcranmer]
:
Mentors:
: 271587 419354 570935 571221 590441 593701 (view as bug list)
Depends on: 201750 783479
Blocks: 338549
  Show dependency treegraph
 
Reported: 2010-04-21 00:36 PDT by Mark Banner (:standard8) (afk until 26th July)
Modified: 2012-08-16 23:48 PDT (History)
17 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Introduce nsNNTPProtocol to the async prompter (12.29 KB, patch)
2011-12-16 15:41 PST, Joshua Cranmer [:jcranmer]
standard8: review-
Details | Diff | Splinter Review
Updated patch (17.52 KB, patch)
2012-01-26 15:55 PST, Joshua Cranmer [:jcranmer]
standard8: review+
Details | Diff | Splinter Review

Description Mark Banner (:standard8) (afk until 26th July) 2010-04-21 00:36:48 PDT
+++ This bug was initially created as a clone of Bug #560792 +++

Spinning out the separate work from bug 338549. We need to hook up NNTP to the msgAsyncPrompt service so that the prompts will occur in serial fashion.

I believe we'd hit this case very rarely, but we should still do it anyway at some stage.
Comment 1 AlexIhrig 2010-05-02 14:15:00 PDT
We hit this case, if in folderpane a newsgroup was selected, when Thunderbird was closed. (Re-)opening Thunderbird than leads to multiple masterpassword dialogs.
Comment 2 Mark Banner (:standard8) (afk until 26th July) 2010-06-09 04:10:05 PDT
*** Bug 570935 has been marked as a duplicate of this bug. ***
Comment 3 Mark Banner (:standard8) (afk until 26th July) 2010-06-09 04:12:50 PDT
*** Bug 419354 has been marked as a duplicate of this bug. ***
Comment 4 Henrik Skupin (:whimboo) 2010-06-09 08:12:59 PDT
As said on my now duped bug, this is a regression. Adding keyword.
Comment 5 Virgo Pärna 2010-06-10 23:10:15 PDT
*** Bug 571221 has been marked as a duplicate of this bug. ***
Comment 6 Herre de Jonge 2010-08-24 00:04:44 PDT
Every time I start Thunderbird I get the master password dialog twice. Once for mail and once for RSS feeds. (My news accounts don't load automatically at startup.) When I turn off "Check for new articles at startup" for the RSS feeds, I get the master password dialog only once. But I want to check these feeds only once a day, not every xx minutes, so I prefer to keep it the way I have it.

I found this bug through bug 570935, which more resembles my problem, but has been marked as a duplicate of this bug.

It worked better in TB 3.0 (as mentioned in comment 4).
Comment 7 Mark Banner (:standard8) (afk until 26th July) 2010-08-25 04:20:33 PDT
*** Bug 590441 has been marked as a duplicate of this bug. ***
Comment 8 Peter 2010-08-31 10:22:13 PDT
I have this problem too.  2 requests in a row for master password.

I have a master password set.  Additionally, I automatically download my gmail upon TB startup.  However, none of my newsgroup subscriptions are set to automatically download new messages upon startup (even though 1 of them requires a password each time I log in - I never get asked a 3rd time when I click on the name of that usenet service to download it's new messages).
Comment 9 David E. Ross 2010-10-23 17:28:30 PDT
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.14) Gecko/20100930 SeaMonkey/2.0.9

I don't know if it is this bug or bug #560792.  I get a prompt for my master password every time I launch SeaMonkey if the preference variable signon.startup.prompt has the default value of "true".  Thus, this is not a rare occurrence. 

I launch SeaMonkey only for the browser.  I don't use SeaMonkey's Mail-News capability and never open it.
Comment 10 Mark Banner (:standard8) (afk until 26th July) 2010-10-24 03:20:37 PDT
(In reply to comment #9)
> I launch SeaMonkey only for the browser.  I don't use SeaMonkey's Mail-News
> capability and never open it.

If you don't use mail-news and never set it up, then your issue is not likely to be a mailnews bug.
Comment 11 John David Galt 2010-10-24 07:11:40 PDT
I'm using Thunderbird 3.1.5 and this happens at every log in.  It's annoying because it happens right at startup, before it's possible to preempt the problem by using Tools -> Options -> Security -> Saved Passwords to enter the password manually first.
Comment 12 Olaf 2010-11-19 07:06:43 PST
Same problem here! TB 3.1.6, new profile, Email and news setup.
The only difference is that I have to enter the password three times, not only two.
Comment 13 Herre de Jonge 2010-11-22 00:34:17 PST
It seems it is not necessary to enter the password on all prompts. After entering the password at the first prompt, the other dialog(s) can just be closed by pressing <Enter>.
Comment 14 Eric Valette 2010-11-22 01:15:16 PST
If I do not enter all password "rapidly", the system hangs on Linux. This render this bug even more annoying.
Comment 15 Mark Banner (:standard8) (afk until 26th July) 2011-01-01 09:55:27 PST
*** Bug 593701 has been marked as a duplicate of this bug. ***
Comment 16 Eric Valette 2011-01-01 10:28:59 PST
7 months after, it is still not fixed and still freezes the application if password are not entered rapidly enough. 

If only kmail was available on windows I would have moved away!
Comment 17 Ruud H.G. van Tol 2011-01-01 17:03:19 PST
I found a workaround:

mail.server.default.login_at_startup -> true

mail.server.server3.login_at_startup -> false
mail.server.server4.login_at_startup -> false

mail.startup.enabledMailCheckOnce -> true

(am not sure that last one matters)
Comment 18 Angel L. Mateo 2011-01-10 23:51:55 PST
I have this problem too. The problem is not related with NNTP, it's just that TB opens as many prompts as passwords it needs at startup. I have a few mail accounts and lightning calendars and it opens as much as needed. It's like if TB tries to ask for the passwords concurrently instead of just a one and then lock waiting for the master password.

If you have an account that doesn't read mail automatically, when you read it, you don't have the problem, because when TB needs its password, you have previously (at startup) entered the master password.

I have the problem with TB 3.1.7 in linux (ubuntu package), but I'm getting since previous versions.
Comment 19 Eric Valette 2011-09-10 10:19:59 PDT
Nearly a year and a half and no progress. I still need to enter 3 passwd each time in start thunderbird. This is getting on my nerfs.
Comment 20 AlexIhrig 2011-09-10 13:05:40 PDT
The good "news" is, it isn't really necessary to enter the master password multiple times (same for Firefox). Enter the master password once and press OK for all other prompts without entering anything.
Comment 21 Joshua Cranmer [:jcranmer] 2011-12-16 15:41:08 PST
Created attachment 582417 [details] [diff] [review]
Introduce nsNNTPProtocol to the async prompter

This patch also has a bit of cleanup (s/NEWS_FINISHED/NNTP_SUSPENDED/ to correct what the state actually means), and a lot more documentation (particularly about NNTP_SUSPENDED)--if you don't feel comfortable reviewing this, switch it to someone else.
Comment 22 Joshua Cranmer [:jcranmer] 2012-01-05 08:45:11 PST
*** Bug 271587 has been marked as a duplicate of this bug. ***
Comment 23 Mark Banner (:standard8) (afk until 26th July) 2012-01-19 14:04:18 PST
Comment on attachment 582417 [details] [diff] [review]
Introduce nsNNTPProtocol to the async prompter

Review of attachment 582417 [details] [diff] [review]:
-----------------------------------------------------------------

::: mailnews/news/src/nsNNTPProtocol.cpp
@@ +2384,5 @@
>    nsCOMPtr<nsIMsgMailNewsUrl> mailnewsurl = do_QueryInterface(m_runningURL);
>    if (!m_msgWindow && mailnewsurl)
>      mailnewsurl->GetMsgWindow(getter_AddRefs(m_msgWindow));
>    bool validCredentials = false;
> +  rv = m_newsFolder->GetAuthenticationCredentials(m_msgWindow, false, false,

You can't do this here, it must be inside the prompter call. The reason being is that just accessing the login manager may cause the master password prompt to appear, and you need to be in async mode by that time.

I think with other protocols, we've generally checked the result value from GetPassword (stored in nsMsgIncomingServer iirc) to see if it is empty or not, if it is, then we've gone into the auth check. You could probably do similar here, but maybe with the username.

As this change may reflect on the other patch, I'll wait until you've taken a look before testing this out.
Comment 24 Joshua Cranmer [:jcranmer] 2012-01-21 14:33:26 PST
Reviewing the code manually seems to indicate that I was mentally overcompensating for bug 437930. If I make the async bit only a mayPrompt and not a mustPrompt, I don't need the mustNotPrompt here. Even though I could probably get by with only checking GetUsername, if I add support for having multiple Usenet accounts with the same server but different username, that would lead to a false positive. Patch pending while I think about bug 201750 some more.
Comment 25 Joshua Cranmer [:jcranmer] 2012-01-26 15:55:21 PST
Created attachment 591980 [details] [diff] [review]
Updated patch

This should remove the password prompter issue, although I suspect I need some more work later on to finally fix bug 437930...
Comment 26 Mark Banner (:standard8) (afk until 26th July) 2012-01-30 16:01:18 PST
Comment on attachment 591980 [details] [diff] [review]
Updated patch

Unfortunately, test_uriParser.js is always failing with:

###!!! ASSERTION: nsNSSComponent relies on profile manager to wait for synchronous shutdown of all network activity: 'mIsNetworkDown', file /Users/moztest/comm/main/src/mozilla/security/manager/ssl/src/nsNSSComponent.cpp, line 2590
###!!! ASSERTION: nsNSSComponent relies on profile manager to wait for synchronous shutdown of all network activity: 'mIsNetworkDown', file /Users/moztest/comm/main/src/mozilla/security/manager/ssl/src/nsNSSComponent.cpp, line 2590

(at least on my mac).
Comment 27 Joshua Cranmer [:jcranmer] 2012-01-30 16:23:00 PST
(In reply to Mark Banner (:standard8) from comment #26)
> ###!!! ASSERTION: nsNSSComponent relies on profile manager to wait for
> synchronous shutdown of all network activity: 'mIsNetworkDown', file
> /Users/moztest/comm/main/src/mozilla/security/manager/ssl/src/nsNSSComponent.
> cpp, line 2590
> ###!!! ASSERTION: nsNSSComponent relies on profile manager to wait for
> synchronous shutdown of all network activity: 'mIsNetworkDown', file
> /Users/moztest/comm/main/src/mozilla/security/manager/ssl/src/nsNSSComponent.
> cpp, line 2590


Hmm, that sounds like I need a while (gThreadManager.currentThread.hasPendingEvents()) before the .processNextEvent ?
Comment 28 Mark Banner (:standard8) (afk until 26th July) 2012-01-30 16:33:59 PST
Unfortunately that doesn't work.
Comment 29 Mark Banner (:standard8) (afk until 26th July) 2012-02-21 04:05:04 PST
Comment on attachment 591980 [details] [diff] [review]
Updated patch

Review of attachment 591980 [details] [diff] [review]:
-----------------------------------------------------------------

Weird, I just was retrying this to see if I could debug the failure, and with this additional patch it worked:

diff --git a/mailnews/news/test/unit/test_uriParser.js b/mailnews/news/test/unit/test_uriParser.js
--- a/mailnews/news/test/unit/test_uriParser.js
+++ b/mailnews/news/test/unit/test_uriParser.js
@@ -167,5 +167,7 @@ function run_test() {
   // The password migration is async, so trigger an event to prevent the logon
   // manager from trying to migrate after shutdown has started.
   var gThreadManager = Cc["@mozilla.org/thread-manager;1"].getService();
-  gThreadManager.currentThread.processNextEvent(true);
+  var thread = gThreadManager.currentThread;
+  while (thread.hasPendingEvents())
+    thread.processNextEvent(true);
 }

Not sure what I was doing wrong before.

::: mailnews/news/src/nsNewsFolder.cpp
@@ +1281,5 @@
>    }
>    NS_FREE_XPCOM_ISUPPORTS_POINTER_ARRAY(count, logins);
>    NS_ENSURE_SUCCESS(rv, rv);
>  
> +  // If there is nothing to migrate, then do othing

nit: othing -> nothing
Comment 30 Joshua Cranmer [:jcranmer] 2012-02-22 10:05:29 PST
Pushed: <http://hg.mozilla.org/comm-central/pushloghtml?changeset=841928c4a46c>.

Note You need to log in before you can comment on or make changes to this bug.