Closed Bug 1497900 Opened 7 years ago Closed 7 years ago

Patch for saving Passwords when pushed by Autoconfig

Categories

(Thunderbird :: Account Manager, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: edv, Assigned: aceman)

Details

Attachments

(1 file, 1 obsolete file)

1.46 KB, patch
BenB
: review-
Details | Diff | Splinter Review
Attached file emailWizard.js (obsolete) —
User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:62.0) Gecko/20100101 Firefox/62.0 Steps to reproduce: I created an account through Autoconfig mecanism and left the password field blank. The Config is retrieved from (my) ISP Autoconfig XML file. In this file I set up an (initial) password for the user account. It looks like this: <incomingServer type="imap"> <hostname>mail.example.com</hostname> <port>993</port> <socketType>SSL</socketType> <username>foo</username> <password>bar</password> <authentication>password-cleartext</authentication> </incomingServer> Actual results: I can create the account, all is set up right even if the password field is not visibile the password works - I can read mails an so on. But when I restart Thunderbird I need to enter the password again. I left the checkbox "Remember Password" checked of course. Expected results: When a password is pushed through this mecanism it should be stored. I went through the code and the interesting file is here: chrome\messenger\content\messenger\accountcreation\emailWizard.js I found out that the password is aviable in the function validateAndFinish (around line 1600 and the function succeeds) but when self.finish() is called the password information is lost. This is because it is only set when entering it into the password field in the variable this._password and not pushed via XML. I suggest to add this lines: if (!e("password").value && !!successfulConfig.incoming.password) { self._password = successfulConfig.incoming.password; } to the successful function so that the config saves the password it already has. I attached the patched file in this "bug" report. I got lost at Step 4 here https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Introduction, to find out who to ask for code review. I decided to bring it in here so hopefully someone pushes it in. Thanks in advance.
Comment on attachment 9015909 [details] emailWizard.js Thanks for this. Usually you need to provide a HG changeset patch, but we can handle that for you. I'm requesting review from out account manager specialist.
Attachment #9015909 - Flags: review?(acelists)
Attached patch 1497900.patchSplinter Review
The idea sound good if this is how admins may want predefine the password for the user. But I think Ben can judge this better. I have converted the change into a proper patch.
Assignee: nobody → acelists
Attachment #9016951 - Flags: review?(ben.bucksch)
1. I don't see the use case / need here. We expect the end user to enter the password in the dialog. If he does so, we store it. If the user did not enter a password, why would the autoconfig server return a valid password? 2. The code in this function should not access the UI.
Attachment #9015909 - Flags: review?(acelists)
Attachment #9016951 - Flags: review?(ben.bucksch) → review-
Attachment #9015909 - Attachment is obsolete: true
It seems the use case is that the admin predefines the password for the user so that he does not need to enter it. It is also claimed that the password is used in the first session, but then forgotten after TB restart. If the password from autoconfig is not to be stored, then why is it accepted initially? So what would be a better place in the code to fix it?
> It seems the use case is that the admin predefines the password for the user so that he does not need to enter it. Right, but I don't think that's a good policy to have. To let anybody just come without a password and to tell them the password for that account, over the network, automatically. Even if it's applicable for some extremely rare corner cases, I don't think that's a good policy for us to allow that over the Internet. I've been thinking about auto deployment mechanisms, how admins could roll out and automatically configure Thunderbird for their users. I could think about the problem in this context. The configuration would then come from the disk together with the Thunderbird installation files, not from the network, which is potentially the Internet.
I was on vacation and just catched up. What I now have summarized for me, is the option A to implement the patch or B to kill the code which initially accepts the password "one-time". I think Ben is right, it is not a generaly a good idea to send passwords for the users over the Internet but, when it is defined in the XML it is send wheter it is good or not. The XML Reader can't stop to read the data which is send via the webserver. The task for security is on the admin site with https, and whatever he does. So the question should be more do we encourage this or not? My application is: User Foo calls me he needs companyaccount Bar. So I can tell him use the autoconfig enter the correct data for Bar and leave the password empty. The state now is, it works till he reastarts, then the password is lost. The patch would help me greatly. It may get offtopic but how to automatically configure the user password over disk? Can you give a hint? As I don't now which user needs what companyaccounts on my domain I need to push an all accounts file in a secure way, which is "decypherable" for Thunderbird. Do we need a vote or something how we proceed here?
It seems the real solution here is to support automatic installation of accounts via on-disk files. We don't have such a solution now, but we should build one some day. Right now, your users will have to enter the password manually. I don't think that's too much to ask from them, given that it's only once during setup. The alternative proposed here sets a really bad precedent, and I don't want to support handing out passwords to unauthenticated users. That's a really bad practice, and we're not going to make that possible. WONTFIX
Status: UNCONFIRMED → RESOLVED
Closed: 7 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: