64777 - if SMTP server needs authentication to send, not asking for authentication

Status: VERIFIED FIXED

(MailNews Core :: Networking: SMTP, defect)

Description

[(not reading, please use [e-mail] instead)]

from news://news.mozilla.org/3A5A67B9.7030608%40yahoo.com,

"I have a small problem. I am trying to set the SMTP server.
My SMTP server needs authentication to send the mail too.
Older browser's used to pop-up a message asking for username-password.

But Netscape6 is not asking for any password. It just complains that the SMTP
server is asking for authentication and stops."

on a related note,  if I check the "always use username and password" checkbox,
I will never get the password dialog, and I will silently fail to send mail.

Comment 1

[John G. Myers]

If SMTP auth is enabled but the username pref is blank, then the send will fail 
without explanation to the user.

Possibilities:

1) Issue an alert stating that the server requires authentication and user needs  
to configure a userid in the mail/news account settings

2) Use an alternate password dialog box which has an editable field for the 
username

   a) If authentication successful, store username in prefs
   b) After successful authentication, discard the username.

Comment 2

[(not reading, please use [e-mail] instead)]

adding john

Comment 3

[John G. Myers]

Attachment: Proposed fix

Comment 4

[John G. Myers]

mscott: please review.  Assigning to self.

Comment 5

[Scott MacGregor]

I've been out on vacation earlier this week. Hope you weren't waiting for the
review for too long. still digging through email.

Couple comments
1) Was there a reason why we needed to change the interface id for nsISmtpServer? 
actually I guess that's just one comment. the patch looks good to me.

sr=mscott

Comment 6

[John G. Myers]

The nsISmtpServer interface changed, thus the IID change.

Comment 7

[Scott MacGregor]

since we haven't actually released "final" versions of these APIs, such a change
isn't necessary. Eventually after all the APIs finish going through an API
review we'll reach a state where future changes to frozen apis would require IID
changes. But we aren't there yet. So you don't have to worry about doing that.

Comment 8

[timeless]

+    if (!passwordPromptString)
+    {
+        rv = NS_ERROR_NULL_POINTER;
+        goto done;
+    }
+    
+    rv = smtpServer->GetUsernamePasswordWithUI(passwordPromptString, 
passwordTitle,
+                                       netPrompt, aUsername, aPassword);
+
+done:
I don't think a goto here is justifyable.

rv = passwordPromptString ?
  smtpServer->GetUsernamePasswordWithUI(
    passwordPromptString, passwordTitle, netPrompt, aUsername, aPassword)
 : NS_ERROR_NULL_POINTER;
would work. Although you might not want to use ?: you can certainly use if else

-       if (aBadPassword && smtpServer)
+  if (aBadPassword && smtpServer)
     smtpServer->ForgetPassword();
^if you're changing indentation, you should probably fix this line

<q class='yuck[tabs="bad"]'>
+        NS_ENSURE_ARG_POINTER(aDialog);
+        
+               // prompt the user for the password
+               if (NS_SUCCEEDED(rv))
+               {
+            nsXPIDLString uniUsername;
</q>

+                       nsCString aCStr; 
+
+            aCStr.AssignWithConversion(uniUsername); 
+                       rv = SetUsername((const char *) aCStr);
^ this looks like bad string foo.

please comment; cc /be, scc, jag.

Comment 9

[(not reading, please use [e-mail] instead)]

sorry for the delay.  r=sspitzer

Comment 10

[John G. Myers]

Re comments From [e-mail] 2001-03-01 23:44:

1) Will simplify error handling in a followup patch.
2) This indentation fix is left over from a change that got undone before the 
patch was submitted.
3) This string logic was copied from the GetPasswordWithUI method.

Comment 11

[John G. Myers]

Attachment: Patch to enable SMTP auth by default

Comment 12

[racham]

r=racham for patch to enable SMTP auth by default (patch #26292).

Comment 13

[(not reading, please use [e-mail] instead)]

in mailnews.js, we have:

pref("mail.smtpserver.default.auth_method", 2); // auth login
pref("mail.smtpserver.default.try_ssl", 1);

currently, those values are ignored.

I think it would be better to have the default values controllable by
mailnews.js, instead of hard coded in nsSmtpServer.cpp

see nsMsgIncomingServer.cpp for how alecf implemented the getters and setters
with default prefs.

also, is the new default the same as we had in 4.x?  why the change?

Comment 14

[John G. Myers]

The new default is the same as in 4.x.  Users should not need to take 
extraordinary action in order to take advantage of SMTP authentication, thus the 
change.  Note the original problem report.

There is actually little reason to have the pref at all.

Comment 15

[(not reading, please use [e-mail] instead)]

having it pref controlled (and not hard coded) would allow an IS department or
an ISP to alter the mailnews.js for their users (directly or using MCD or
something) to have the appropriate defaults for their setups.

Comment 16

[John G. Myers]

Changing the default to be controlled by prefs instead of being hardcoded is a 
separate bug.  This bug does not change that--the default is hardcoded both 
before and after the patch.

My previous comment was that there was little need for the auth_method pref at 
all.  The only case where it is useful is when the server is misconfigured to 
ask for an authentication it doesn't need.

Comment 17

[(not reading, please use [e-mail] instead)]

you are right, changing from hard coded values to prefs would be another bug. 
(I'll go log it.)

sr=sspitzer

Comment 18

[Ben Bucksch (:BenB)]

Seth, please cite the number.

IIRC, the SMTP spec doesn't include auth. Are we possibly strictly conflicting
with the spec here?

Comment 19

[John G. Myers]

SMTP auth is specified in RFC 2554.

Comment 20

[Senguttuvan Elango]

OK people. I believe this is what we are doing right now. I am just thinking 
aloud here.

On the account settings, for Outgoing (SMTP) server, we have a check box to 
specify if the server needs authentication. If it needs, then we can type in 
the user name in the username box.

I feel that we need to have a checkbox to specify if the password is to be 
remembered. If that checkbox is checked, then we need to enable a password box 
to enter the password.

All these data are going into the js file.

While sending the mail

1. Get the SMTP server info from the js file.
2. Get the AUTH preference.
3. If (! AUTH) send the mail to the server.
4. Else If (AUTH)
    a. If (username == <empty>) // username in js file
        i. Get both username and password through a login dialog box
             //[Both username field and password field are editable]
    b. Else If (username != <empty>) // username in js file
        i. Get username from the the js file.
        ii. If (! remember password)
            1. Get the password through a login dialog box
                // [Only the password field is editable]
        iii. Else If (remember password)
            1. Get the password from the js file.

    Now that we know both username and password, try authenticating to the
    server. If it fails, ask the user for username and password through a
    login dialog box [Both username field and password field are editable].
    Repeat this till the user provides a valid username password combination
    or cancels the operation.

I don’t agree with overwriting the existing username and password with the new 
valid username password combination since the existing account may be disabled 
temporarily.

Comment 21

[(not reading, please use [e-mail] instead)]

the hard coding is covered in #71243

Comment 22

[John G. Myers]

The checkbox does not "specify if the server needs authentication".  If the 
checkbox is checked and the server doesn't require authentication, then 
everything works just fine--the user is not prompted for a username or password.

The only reason to ever uncheck the checkbox is when the SMTP server is 
misconfigured to ask for authentication when it is not actually required.  In 
that case, leaving it checked will cause the user to be asked for a password.  
Why we have a preference for working around a server misconfiguration is beyond 
me.

In general, the Account Manager should not have checkboxes for "remember 
password".  This should be handled through the wallet ui, including the password 
prompts themselves.

Comment 23

[Senguttuvan Elango]

That's true. We don't need a checkbox to see if the server needs authentication.
The server itself will ask for it. The checkbox is to identify if the user has
entered username/password information. But if we can incorporate the feature
with wallet, that's great.

If the server doesn't require authentication, we are not even going to check if
the check box is checked or not. 

I don't understand the part when you said "uncheck the checkbox". I don't agree
with changing the state that's been set by the user. If the user has made some
error, that is his problem and he needs to solve it. Not us.

I agree with you on the misconfigured server. If the server is misconfigured,
then it is possible that the server is not working properly. It is the sysadmin
of that server's responsibility to solve the problem. We can not assume and do
something for that.

> leaving it checked will cause the user to be asked for a password.

This happens if we check the status of the check box first. But in our case we
are giving precedence to the SMTP server's configuration. If the server doesn't
need authentication, we don't bother to check the status of the checkbox.

Comment 24

[John G. Myers]

When I said "uncheck the checkbox", I was explaining the only reason a user 
would rationally want to change the setting of the preference.  The "leaving it 
checked" comment is in the context of a misconfigured server.

Comment 25

[John G. Myers]

Attachment: Cleanup patch

Comment 26

[Scott MacGregor]

sr=mscott

Comment 27

[(not reading, please use [e-mail] instead)]

r=sspitzer

john, a useful macro is NS_ENSURE_SUCCESS();

doing 

NS_ENSURE_SUCCESS(rv,rv);

acts like

if (NS_FAILED(rv)) return rv;

except, that it asserts in debug builds when NS_FAILED(rv) is true.

Comment 28

[John G. Myers]

Marking FIXED.

Comment 29

[Sheela Ravindran]

Following are the steps that was done in order to verify this bug:
win98-buildid-2001-03-23-06
linux-buildid-2001-03-23-05
mac-buildid-2001-03-23-10
I used our smtp secure poisonivy server. 
I created a new profile with a test account with the above as my smtp server.
launch browser with the above profile
From the browser I did file/new message and click send button to send
resulted in: password dialog to send the message

The above was true when the outgoing SMTP server settings in accounts/settings 
is 1.set to never( uncheck the usr name and password and the  SSL selection 
stays at never)
2. set with usr name and password text filled and the  SSL selection to 'when 
available'.

But on Mac I do not get the password dlg to send the message. It is still not 
checking for the authentication. 

John,
Should I reopen this bug just for MAC or should I log a new bug? 

Comment 30

[Sheela Ravindran]

sorry,
The buildid that was used to verify this bug on win98 was 2001-03-21-06 

Comment 31

[John G. Myers]

If you fill in the smtp userid in account manager on mac, does it throw up the 
dialog box?  If not, you should file a new bug for Mac.

Comment 32

[Sheela Ravindran]

On Mac because of bug 65703 and when you have the userid filled in account 
manager the application freezes.  So I cannot test to see if having userif 
filled if it prompts me with a password.  But even without having the username 
and password fields filled and the account manager settings set to never it does 
not prompt me with a passoword dialog at all when sending a mail from 
browser. I would like to verify this bug and open a new bug for Mac and put bug 
65703 as a blocker in verifying.  Because this bug has gotten too lengthy now. 
Is that okay?

Comment 33

[John G. Myers]

Opening new bug is OK.

Comment 34

[Sheela Ravindran]

verifying this bug for linux and windows.  Reopened a new bug for Mac only 
bug 73693

Comment 35

[Senguttuvan Elango]

After all these discussions, It appears to be...

I still experience the problem.

Now When I try to send the mail, it askes for the password for my smtp server.
I enter the password and hit enter, it again pops up the dialog box asking for
the password. This happens till I close the login dialog by pressing cancel button.

I don't know what is happening. A little help is appreciated. I am using outlook
to send mails and Mozilla to receive them. :(

Comment 36

[Senguttuvan Elango]

I forgot to tell the build no.

Build ID: 2001040904

Thanks

      - Sengu

Comment 37

[John G. Myers]

That would be a different problem.  It should go into a separate bug.

In reporting that problem, it would help to have an SMTP log.  This can be done 
by setting

setenv NSPR_LOG_MODULES SMTP:5

Note that the log will include the base64 encoded form of your SMTP password 
that the client sends to the server.  You may want to censor that in any report.