571264 - Autoconfiguration of new account ignores ISP's provided values if the password is wrong

Status: VERIFIED FIXED

(Thunderbird :: Account Manager, defect)

Description

[Max Spicer]

If the user provides the wrong password for an email account that Thunderbird is able to configure automatically by obtaining acount details from the ISP, Thunderbird ignores the ISP's details and reverts to trying common server names.

Steps to reproduce:
1) Set up your domain web server with a /.well-known/autoconfig/mail/config-v1.1.xml with appropriate settings
2) Start creating a new account in Thunderbird entering an email address in your domain but with the wrong password
3) Click Continue
- Thunderbird finds the correct settings from 'Email provider'
4) Click Create Account
- Thunderbird checks the password and reports 'Configuration could not be verified - is the username or password wrong?'
5) Enter correct password and click Re-test Configuration
- Thunderbird bins the correct settings and immediately starts trying common server names (which fails in our case)

Thunderbird should instead try to use the previously obtained settings with the new password.

If the correct password is provided from the start, the account is set up correctly.

This is with TB 3.1 RC2 - Mozilla/5.0 (X11; U; Linux i686; en-GB; rv:1.9.2.4) Gecko/20100608 Thunderbird/3.1

Comment 1

[Mark Banner (:standard8)]

Blake, can you investigate?

Comment 2

[Blake Winton (:bwinton) (:☕️)]

(In reply to comment #1)
> Blake, can you investigate?

Sure.  I can see where that's happening, and I think I have a fix, but I want to write a MozMill test that replicates the behaviour first.

Later,
Blake.

Comment 3

[Blake Winton (:bwinton) (:☕️)]

Attachment: The patch, and a test for it.

Here's the change, along with the test that fails without it, and succeeds with it.

Thanks,
Blake.

Comment 4

[Ben Bucksch (:BenB)]

I'd rather we didn't go into Manual Edit mode at all when the password failed. We should we presume the config wrong when the password is wrong?
Sure, the config may be wrong as well, but we don't know that. The user can still click on "Edit" button himself.

In other words, I suggest to remove the |me.editConfigDetails();| line from the failure case of verifyConfig() in line 841.

Alternatively, you could do that only when the config was guessed. (Otherwise we either got the config from XML, which is presumably correct, or the user is in Manual Edit mode anyways.)

Comment 5

[David :Bienvenu]

Sorry for the delay - the mozmill test works fine, but I can't get "retest configuration" to work with my gmail account at all. The stop button enables and create account disables, but nothing seems to be happening.  I'll see if verify logon is even getting called.

Comment 6

[David :Bienvenu]

verify logon isn't getting called in this scenario (the second time, that is)

Comment 7

[Blake Winton (:bwinton) (:☕️)]

(In reply to comment #4)
> I'd rather we didn't go into Manual Edit mode at all when the password failed.
> We should we presume the config wrong when the password is wrong?
> Sure, the config may be wrong as well, but we don't know that. The user can
> still click on "Edit" button himself.

Sure, but that wouldn't actually fix this, because we would still re-probe the settings if they hit "Edit" then "Re-test".

I'll see what I can do to replicate (and then fix) the error you're seeing, David.

Thanks,
Blake.

Comment 8

[Ben Bucksch (:BenB)]

> I can't get "retest configuration" to work with my gmail account at all.

The bug is about "wrong password", no? So, you don't need valid account.

> we would still re-probe the settings if they hit "Edit" then "Re-test".

Yes, and that's fine and expected for Edit mode (at least in the old dialog, would be better in bug 549045).

The bug here is that we do that when we have a config and the user just entered a wrong password. In this case, we should not go in edit mode at all (unless the user goes there), and therefore would never "Retest".

Comment 9

[David :Bienvenu]

(In reply to comment #8)
> 
> The bug is about "wrong password", no? So, you don't need valid account.

I would say that the bug is that if I enter the wrong password the first time, I can't create an account correctly after that. I'm undecided about whether we go into edit mode, requiring a retest, or whether we just enable create account after the user enters a new password. I'm not really sure what the better UI would be. Re-enabling "create account" once the user starts entering a different password is definitely a possibility.

Comment 10

[Max Spicer]

I think you do need to retest once the user has entered another password.  I don't think it's very helpful to create an account that's not been tested.  When you've retrieved details from autoconfig (not including guessing), they're likely to be correct, so shouldn't the focus be on getting the user to check the details that they've provided (email and password)?  Getting them to check and or edit the server details should be an advanced, last-resort step, shouldn't it?

Apologies to go off topic, but how does TB respond to a 500 error from a domain-hosted auto-config server (autoconfig.example.com or example.com/.well-known/.../?email=[e-mail])?  I'm wondering if a server that is creating the xml dynamically is able to flag up an invalid email address at the stage that it is queried.

Comment 11

[David :Bienvenu]

(In reply to comment #10)
> I think you do need to retest once the user has entered another password.
I agree - I don't think anyone's arguing that - but when you click create account, we verify the password at that point, which is an implicit retest.

Comment 12

[Ben Bucksch (:BenB)]

There's a confusion here. "Re-Test" refers to a button in Manual Edit mode which guesses server names and settings. It does not check the password. The password check is called Verification and always happens before we create the account (unless you go into "Advanced Config").

Comment 13

[Blake Winton (:bwinton) (:☕️)]

Attachment: A patch that just enables the username and password fields on password failure.

So, I tried going down the other route for a while, but it got more and more complicated, so I figured it might be simpler to leave the "Create Account" button enabled, and enable the username and password fields for the user to change.

Comment 14

[David :Bienvenu]

Comment on attachment 456100 [details] [diff] [review]
A patch that just enables the username and password fields on password failure.

great, this works much better. I can't run the mozmill tests on my mac, but as long as you've verified the mozmill test works, I'm satisfied.

Comment 15

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 456100 [details] [diff] [review]
A patch that just enables the username and password fields on password failure.

looks good!

Comment 16

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 456100 [details] [diff] [review]
A patch that just enables the username and password fields on password failure.

Seems odd to request this for a blocking bug, but I just wanted to double-check.

The code changes are fairly minor, and are in a place where we don't really do the right thing currently, and the patch adds tests, so hopefully we can get this into 3.1.2.

Thanks,
Blake.

Comment 17

[Mark Banner (:standard8)]

Comment on attachment 456100 [details] [diff] [review]
A patch that just enables the username and password fields on password failure.

3.1.2 is a quick low-risk release focussed at fixing a couple of issues for enabling major updates. I don't think this counts under that, so I'm pushing this out to 3.1.3.

However, please land on comm-1.9.2 default so that it hopefully gets some testing in the nightlies.

Comment 18

[Blake Winton (:bwinton) (:☕️)]

Landed on trunk as <http://hg.mozilla.org/comm-central/rev/9f4d6e54923d> and 1.9.2 default as <http://hg.mozilla.org/releases/comm-1.9.2/rev/9a4aff1b73bc>.

Comment 19

[Ludovic Hirlimann [:Usul]]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.9) Gecko/20100825 Thunderbird/3.1.3