Last Comment Bug 201750 - news authentication: username and password are separate dialogs
: news authentication: username and password are separate dialogs
Status: RESOLVED FIXED
:
Product: MailNews Core
Classification: Components
Component: Networking: NNTP (show other bugs)
: Trunk
: x86 All
: -- minor with 2 votes (vote)
: Thunderbird 13.0
Assigned To: Joshua Cranmer [:jcranmer]
:
:
Mentors:
: 697397 (view as bug list)
Depends on:
Blocks: 218998 560793
  Show dependency treegraph
 
Reported: 2003-04-11 20:37 PDT by Louis Kruger
Modified: 2012-05-30 11:35 PDT (History)
18 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Unify the dialog and add more information (42.65 KB, patch)
2011-12-16 15:34 PST, Joshua Cranmer [:jcranmer]
bwinton: ui‑review+
Details | Diff | Splinter Review
Version 2 (42.65 KB, patch)
2012-01-26 15:49 PST, Joshua Cranmer [:jcranmer]
standard8: review+
mozilla: superreview+
Pidgeot18: ui‑review+
Details | Diff | Splinter Review

Description Louis Kruger 2003-04-11 20:37:22 PDT
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4b) Gecko/20030405
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4b) Gecko/20030405

I find it to be an annoying usability issue that I have to enter the username in
a dialog box, click "okay", then wait for another dialog to enter my password,
then click "okay" again.  It especially seems inconsistent in light of the fact
that the web browser component does it with one dialog.

Reproducible: Always

Steps to Reproduce:
N/A
Actual Results:  
have to muck through 2 seperate dialogs to enter my username and password

Expected Results:  
should only have to see 1 dialog, like most other programs.
Comment 1 Christian Eyrich 2003-04-23 09:57:13 PDT
When do you get asked for username and password?
When configuring an account? When receiving mail? When sending mail?

Please describe it more detailed.
Comment 2 Louis Kruger 2003-04-24 18:15:17 PDT
Christian: 
 
I notice it mostly when reading news on an NNTP server that requires 
authentication.  If I can, I'll edit the bug description to make that more explicit.  I 
haven't verified whether or not it also occurs for mail. 
 
Also, as a side note, bug 201749 makes this annoyance even worse. 
Comment 3 Louis Kruger 2003-04-24 18:24:52 PDT
On further investigation, it looks like this is an newsgroup specific problem.  The 
difference with mail is that it already knows my username because it is set in the 
account settings dialog.  Whereas with newsgroups, it isn't part of the account 
settings so it has to ask each time. 
Comment 4 Wolfi 2003-05-27 18:02:03 PDT
To comment #3:

It wouldn't be quite as bad, if the stupid password manager would be able to
actually memorize username and PW and autofill it in, but the darn thing doesn't.
You have to type the whole shitbang every time, often also when moving between
different NGs on the same server.
The PW manager shows empty <> for the usernames belonging to each subscribed NG.

The current state is a serious flaw and unfortunately I have to see that it
hasen't improved a single bit towards 1.4b.

Mozilla/5.0 (OS/2; U; Warp 4.5; de-AT; rv:1.3) Gecko/20030313  (we don't yet
have a 1.3.1)
Comment 5 Robert Kaiser 2009-06-14 17:04:22 PDT
MASS-CHANGE:
This bug report is registered in the SeaMonkey product, but has been without a comment since the inception of the SeaMonkey project. This means that it was logged against the old Mozilla suite and we cannot determine that it's still valid for the current SeaMonkey suite. Because of this, we are setting it to an UNCONFIRMED state.

If you can confirm that this report still applies to current SeaMonkey 2.x nightly builds, please set it back to the NEW state along with a comment on how you reproduced it on what Build ID, or if it's an enhancement request, why it's still worth implementing and in what way.
If you can confirm that the report doesn't apply to current SeaMonkey 2.x nightly builds, please set it to the appropriate RESOLVED state (WORKSFORME, INVALID, WONTFIX, or similar).
If no action happens within the next few months, we move this bug report to an EXPIRED state.

Query tag for this change: mass-UNCONFIRM-20090614
Comment 6 Steve Wendt 2009-06-29 22:52:43 PDT
This is no different in Gecko/20090628 SeaMonkey/2.0b1pre; username and password are stored as separate password entries (with no username).
Comment 7 Robert Kaiser 2009-07-11 13:34:04 PDT
Mark, weren't we about to change that with the password manager change?
Comment 8 Mark Banner (:standard8, limited time in Dec) 2009-07-11 14:29:03 PDT
(In reply to comment #7)
> Mark, weren't we about to change that with the password manager change?

Nope. The thought had crossed our minds I think, however the news server protocols mean that there is no easy way around this. The protocols specify that the username is supplied first, and if required then the password should be supplied. However, there is no way of knowing if a password is required or not before entering the username.
Comment 9 Robert Kaiser 2009-07-11 14:33:22 PDT
So, Mark, you're basically saying this is a valid bug but we can't fix it, i.e. we need to mark this as WONTFIX?
Comment 10 Joshua Cranmer [:jcranmer] 2009-07-11 15:13:37 PDT
(In reply to comment #8)
> Nope. The thought had crossed our minds I think, however the news server
> protocols mean that there is no easy way around this. The protocols specify
> that the username is supplied first, and if required then the password should
> be supplied. However, there is no way of knowing if a password is required or
> not before entering the username.

From practical experience, I don't think there exists any news servers that support username. The one source code I have access to (INN) doesn't support using only AUTHINFO USER without AUTHINFO PASS. I can't think of a use case for usernames without passwords: if you don't have the password, it becomes too easy to spoof. Furthermore, if we every implement the other AUTHINFO variants, I believe all of those require username and password together.

I've currently asked the NNTP newsgroup how important it is to support username-without-password from a client level, so I should be able to tell you when I get home tomorrow.

A final point is that, since the much more common case is with password, it might be advisable to combine the dialog and have a hidden or not-so-hidden server preference to use username without password. On the other hand, asking for a password in the same dialog wouldn't be detrimental to username-without-password, since we would simply ignore the password. And if the dialog accepts blank passwords, then the user doesn't have to enter anything in.
Comment 11 Karsten Düsterloh 2009-07-13 01:09:42 PDT
(In reply to comment #10)
> I've currently asked the NNTP newsgroup 

Which one?
Comment 12 Joshua Cranmer [:jcranmer] 2009-07-13 18:28:58 PDT
(In reply to comment #11)
> (In reply to comment #10)
> > I've currently asked the NNTP newsgroup 
> 
> Which one?

The one where you would expect to find discussion of NNTP, of course: news.software.nntp.

The answer seems to be that some major software implementations don't even support this functionality (there seems to be confusion as to whether or not it's permitted under the now-outdated RFC 2980, although it seems to me that it is quite clearly stated to be permitted). INN and Leafnode definitely don't permit it, and the only person who came up with one had a very internal server setup.
Comment 13 Karsten Düsterloh 2011-08-12 13:00:06 PDT
I think we should just ask for both username and password once we know we can't get away without login credentials. A comment like "You can leave the password field blank if your server doesn't require one." should suffice.
Comment 14 Joshua Cranmer [:jcranmer] 2011-08-12 20:16:02 PDT
I think a comment like that is too verbose to stick in a password dialog, especially for a very rare case. As long as we can ensure that the dialog allows you to hit enter for a blank password, it should be fine (we'd ignore the password in that case anyways).
Comment 15 WADA 2011-11-15 00:10:10 PST
No one looks to have tried news server definition in Tb with ServerName=<username>@news.server.name format. Does this format work as expected? Is username prompted by Tb even with this format?
Comment 16 WADA 2011-11-15 15:34:46 PST
<username>@news.server.name format was not usable. Sorry for my confusion and wrong comment.
Following is copy of bug 702038 comment #17.

Changing the server name to username@server.name only causes Thunderbird to sit there endlessly with "Looking up username@server.name..." on the message bar.  It never prompts for a username or password because it simply *doesn't work.*
Comment 17 Joshua Cranmer [:jcranmer] 2011-12-16 15:34:52 PST
Created attachment 582412 [details] [diff] [review]
Unify the dialog and add more information

Apologies for the length of the patch, but my attempt to break it up didn't really do much to make the patches shorter.

As a side note, this patch would also fix bug 218998.
Comment 18 Jim Porter (:squib) 2011-12-16 22:50:36 PST
As a quick drive-by, maybe enterUserPassTitle should just be "Username/Password Required"? It looks like it's being used for cases where a group has a password, as well as when the server has a password.
Comment 19 Mark Banner (:standard8, limited time in Dec) 2012-01-19 06:11:16 PST
Comment on attachment 582412 [details] [diff] [review]
Unify the dialog and add more information

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

Generally this looks good. There's a couple of questions inline though, and a little bit of bitrot. I've also still got to test it.

::: mail/locales/en-US/chrome/messenger/news.properties
@@ +45,5 @@
>  removeExpiredArtLinkText=Click here to remove all expired articles
>  cancelDisallowed=This message does not appear to be from you.  You may only cancel your own posts, not those made by others.
>  cancelConfirm=Are you sure you want to cancel this message?
>  messageCancelled=Message cancelled.
> +enterUserPassTitle=News Server Username/Password Required

I think Blake should take a quick look at the string changes.

::: mailnews/news/public/nsIMsgNewsFolder.idl
@@ +129,5 @@
> +  /// The username that should be used for this group
> +  attribute ACString groupUsername;
> +
> +  /// The password that should be used for this group
> +  attribute ACString groupPassword;

Can news do utf-8 based passwords/usernames? If so, we should probably change this to AUTF8String (although not used from js at the moment, it'll break in js if someone does want to use them).

::: mailnews/news/src/nsNewsFolder.cpp
@@ +219,5 @@
>    // cache this for when we open the db
>    rv = newsFolder->SetReadSetFromStr(setStr);
>  
> +  // I don't have a good time to do this, but this is as good as any...
> +  newsFolder->MigrateLegacyCredentials();

Have you tested this with a master password? Touching the login manager here could cause a master password prompt, so it depends when this gets hit in the startup cycle if its a good place or not.

I'd also be concerned it would make bug 560793 worse without really being an easy fix.

@@ +1283,5 @@
> +    {
> +      nsString uniUsername, uniPassword;
> +      logins[0]->GetUsername(uniUsername);
> +      logins[0]->GetPassword(uniPassword);
> +      mGroupUsername = NS_LossyConvertUTF16toASCII(uniUsername);

Hmm, maybe this answers the question about ascii usernames. Would be good just to check the spec though.
Comment 20 Joshua Cranmer [:jcranmer] 2012-01-19 07:19:35 PST
(In reply to Mark Banner (:standard8) from comment #19)
> Can news do utf-8 based passwords/usernames? If so, we should probably
> change this to AUTF8String (although not used from js at the moment, it'll
> break in js if someone does want to use them).

The original implementation used ASCII-only authentication (IIRC) and I don't recall seeing any bugs filed asking for NNTP i18n passwords. In theory, NNTP uses UTF-8 for all connection data; in practice, <http://lists.eyrie.org/pipermail/ietf-nntp/2005-July/005777.html> suggests that the charset of passwords is not liable to be reliable in practice (probably less reliable than the server itself). RFC 4643 defers to RFC 3454's SASL string preparation for authentication, but I suspect that is only reliable if using AUTHINFO SASL.

Quote from RFC 4643:
>  Also note that historically the username is not canonicalized in any
>  way.  Servers MAY use the [SASLprep] profile of the [StringPrep]
>  algorithm to prepare usernames for comparison, but doing so may cause
>  interoperability problems with legacy implementations.  If
>  canonicalization is desired, the SASL PLAIN [PLAIN] mechanism is
>  recommended as an alternative.

If/when AUTHINFO SASL gets supported, this may be worth revisiting.
Comment 21 Blake Winton (:bwinton) (:☕️) 2012-01-24 13:49:04 PST
Comment on attachment 582412 [details] [diff] [review]
Unify the dialog and add more information

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

A couple of small things, but I basically like it.  ui-r=me.

::: mail/locales/en-US/chrome/messenger/news.properties
@@ -46,4 +46,4 @@
> >  cancelDisallowed=This message does not appear to be from you.  You may only cancel your own posts, not those made by others.
> >  cancelConfirm=Are you sure you want to cancel this message?
> >  messageCancelled=Message cancelled.
> > -enterUsername=Please enter a username for news server access:
> > +enterUserPassTitle=News Server Username/Password Required

I would rather see this as "News Server Username and Password Required".

@@ -49,5 +49,5 @@
> > -enterUsername=Please enter a username for news server access:
> > +enterUserPassTitle=News Server Username/Password Required
> > -enterUsernameTitle=News Server Username Required
> > +# LOCALIZATION NOTE (enterUserPassServer): %S is the server being accessed
> > -saveUsername=Use Password Manager to remember this value.
> > +enterUserPassServer=Please enter a username and password for %S:
> > -enterPassword=Please enter a password for news server access:
> > +# LOCALIZATION NOTE (enterUserPassGroup): %1$S is a specific newsgroup to set
> > -enterPasswordTitle=News Server Password Required
> > +# the password for; %2$S is the server from which the newsgroup is accessed

I thought Localization Notes had to be on one like for some reason…

::: suite/locales/en-US/chrome/mailnews/news.properties
@@ -46,4 +46,4 @@
> >  cancelDisallowed=This message does not appear to be from you.  You may only cancel your own posts, not those made by others.
> >  cancelConfirm=Are you sure you want to cancel this message?
> >  messageCancelled=Message cancelled.
> > -enterUsername=Please enter a username for news server access:
> > +enterUserPassTitle=News Server Username/Password Required

(Thunderbird doesn't use these, right?  So I don't need to, and indeed probably shouldn't, comment on them?)
Comment 22 Joshua Cranmer [:jcranmer] 2012-01-24 14:03:46 PST
(In reply to Blake Winton (:bwinton - Thunderbird UX) from comment #21)
> I thought Localization Notes had to be on one like for some reason…

I'm not sure, but I'll ask about this.

> ::: suite/locales/en-US/chrome/mailnews/news.properties
> @@ -46,4 +46,4 @@
> > >  cancelDisallowed=This message does not appear to be from you.  You may only cancel your own posts, not those made by others.
> > >  cancelConfirm=Are you sure you want to cancel this message?
> > >  messageCancelled=Message cancelled.
> > > -enterUsername=Please enter a username for news server access:
> > > +enterUserPassTitle=News Server Username/Password Required
> 
> (Thunderbird doesn't use these, right?  So I don't need to, and indeed
> probably shouldn't, comment on them?)

This is effectively a clone of the relevant file for SeaMonkey. We ought to have a central localization place for shared code in mailnews/, considering that several places in the backend can spawn their own dialogs...
Comment 23 Joshua Cranmer [:jcranmer] 2012-01-24 14:04:32 PST
(In reply to Joshua Cranmer [:jcranmer] from comment #22)
> (In reply to Blake Winton (:bwinton - Thunderbird UX) from comment #21)
> > I thought Localization Notes had to be on one like for some reason…
> 
> I'm not sure, but I'll ask about this.

gandalf in #l10n said "multilines are okay."
Comment 24 Robert Kaiser 2012-01-25 04:17:54 PST
(In reply to Joshua Cranmer [:jcranmer] from comment #22)
> We ought to
> have a central localization place for shared code in mailnews/, considering
> that several places in the backend can spawn their own dialogs...

I would cheer for someone introducing mailnews/locales/ for those - I never came around to doing that myself.
Comment 25 Joshua Cranmer [:jcranmer] 2012-01-25 10:37:40 PST
*** Bug 697397 has been marked as a duplicate of this bug. ***
Comment 26 Joshua Cranmer [:jcranmer] 2012-01-26 15:49:55 PST
Created attachment 591975 [details] [diff] [review]
Version 2
Comment 27 Joshua Cranmer [:jcranmer] 2012-01-26 15:58:41 PST
(In reply to Mark Banner (:standard8) from comment #19)
> Have you tested this with a master password? Touching the login manager here
> could cause a master password prompt, so it depends when this gets hit in
> the startup cycle if its a good place or not.
> 
> I'd also be concerned it would make bug 560793 worse without really being an
> easy fix.

Testing indeed does cause a master password prompt, but I made the code to call it asynchronously on bug 560793 (it isn't all that complicated).

(In reply to Robert Kaiser (:kairo@mozilla.com) from comment #24)
> (In reply to Joshua Cranmer [:jcranmer] from comment #22)
> > We ought to
> > have a central localization place for shared code in mailnews/, considering
> > that several places in the backend can spawn their own dialogs...
> 
> I would cheer for someone introducing mailnews/locales/ for those - I never
> came around to doing that myself.

Bug 721657 if anyone is interested.
Comment 28 Jens Hatlak (:InvisibleSmiley) 2012-01-26 16:01:44 PST
(In reply to Joshua Cranmer [:jcranmer] from comment #27)
> Bug 721657 if anyone is interested.

ITYM Bug 721567. ;-)
Comment 29 David :Bienvenu 2012-02-03 09:35:51 PST
Comment on attachment 591975 [details] [diff] [review]
Version 2

thx for this, Joshua. Looks good.

nsIMsgIncomingServer due not correctly work.

should be

nsIMsgIncomingServer *do* not work correctly.
Comment 30 Joshua Cranmer [:jcranmer] 2012-02-14 14:08:09 PST
Pushed: <http://hg.mozilla.org/comm-central/pushloghtml?changeset=9d43b85434a1>.

Note that I made a slight change from this patch when pushing: I updated the nsIStringBundleService to be initialized from mozilla/Services.h in light of Callek's patch landing first.

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