Bug 1693883 - account provisioner doesn't handle utf-8 account configuration data. r=aleca a=wsmwk
authorMagnus Melin <mkmelin+mozilla@iki.fi>
Tue, 23 Feb 2021 12:57:42 +0200
changeset 41369 ad4d383ac29bee4fcddbed07ebea9b1460424bf0
parent 41368 6613590cabcb58f24b7ad42853c580034eefff37
child 41370 98ca31631af56a0e3a28290f5c6376ce417b9dd0
push id2989
push userthunderbird@calypsoblue.org
push dateMon, 01 Mar 2021 19:24:13 +0000
treeherdercomm-beta@c96075bebc30 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaleca, wsmwk
bugs1693883
Bug 1693883 - account provisioner doesn't handle utf-8 account configuration data. r=aleca a=wsmwk
mail/components/accountcreation/content/createInBackend.js
mail/components/newmailaccount/content/uriListener.js
mail/test/browser/account/browser_mailAccountSetupWizard.js
mail/test/browser/account/xml/example.com
mail/test/browser/newmailaccount/browser_newmailaccount.js
mail/test/browser/newmailaccount/html/config.xml
--- a/mail/components/accountcreation/content/createInBackend.js
+++ b/mail/components/accountcreation/content/createInBackend.js
@@ -66,17 +66,17 @@ function createAccountInBackend(config) 
   } else {
     // If the username/hostname are different then generate a new CLIENTID.
     inServer.clientid = uuidGen
       .generateUUID()
       .toString()
       .replace(/[{}]/g, "");
   }
 
-  if (config.rememberPassword && config.incoming.password.length) {
+  if (config.rememberPassword && config.incoming.password) {
     rememberPassword(inServer, config.incoming.password);
   }
 
   if (inServer.authMethod == Ci.nsMsgAuthMethod.OAuth2) {
     inServer.setCharValue("oauth2.scope", config.incoming.oauthSettings.scope);
     inServer.setCharValue(
       "oauth2.issuer",
       config.incoming.oauthSettings.issuer
@@ -161,19 +161,19 @@ function createAccountInBackend(config) 
     outServer.port = config.outgoing.port;
     outServer.authMethod = config.outgoing.auth;
     // Populate the clientid if it is enabled for this outgoing server.
     if (outServer.clientidEnabled) {
       outServer.clientid = newOutgoingClientid;
     }
     if (config.outgoing.auth != Ci.nsMsgAuthMethod.none) {
       outServer.username = username;
-      outServer.password = config.incoming.password;
-      if (config.rememberPassword && config.incoming.password.length) {
-        rememberPassword(outServer, config.incoming.password);
+      outServer.password = config.outgoing.password;
+      if (config.rememberPassword && config.outgoing.password) {
+        rememberPassword(outServer, config.outgoing.password);
       }
     }
 
     if (outServer.authMethod == Ci.nsMsgAuthMethod.OAuth2) {
       let prefBranch = "mail.smtpserver." + outServer.key + ".";
       Services.prefs.setCharPref(
         prefBranch + "oauth2.scope",
         config.outgoing.oauthSettings.scope
@@ -190,21 +190,17 @@ function createAccountInBackend(config) 
     } else if (config.outgoing.socketType == 2) {
       // SSL / TLS
       outServer.socketType = Ci.nsMsgSocketType.SSL;
     } else if (config.outgoing.socketType == 3) {
       // STARTTLS
       outServer.socketType = Ci.nsMsgSocketType.alwaysSTARTTLS;
     }
 
-    // API problem: <http://mxr.mozilla.org/seamonkey/source/mailnews/compose/public/nsISmtpServer.idl#93>
     outServer.description = config.displayName;
-    if (config.password) {
-      outServer.password = config.outgoing.password;
-    }
 
     // If this is the first SMTP server, set it as default
     if (
       !MailServices.smtp.defaultServer ||
       !MailServices.smtp.defaultServer.hostname
     ) {
       MailServices.smtp.defaultServer = outServer;
     }
--- a/mail/components/newmailaccount/content/uriListener.js
+++ b/mail/components/newmailaccount/content/uriListener.js
@@ -193,23 +193,28 @@ TracingListener.prototype = {
       accountCreationFuncs
     );
 
     let tabmail = document.getElementById("tabmail");
     let success = false;
     let account;
 
     try {
-      // Attempt to construct the downloaded data into XML
-      let data = this.chunks.join("");
+      // Construct the downloaded data (we'll assume UTF-8 bytes) into XML.
+      let xml = this.chunks.join("");
+      let bytes = new Uint8Array(xml.length);
+      for (let i = 0; i < xml.length; i++) {
+        bytes[i] = xml.charCodeAt(i);
+      }
+      xml = new TextDecoder().decode(bytes);
 
       // Attempt to derive email account information
       let domParser = new DOMParser();
       let accountConfig = accountCreationFuncs.readFromXML(
-        JXON.build(domParser.parseFromString(data, "text/xml"))
+        JXON.build(domParser.parseFromString(xml, "text/xml"))
       );
       accountCreationFuncs.replaceVariables(
         accountConfig,
         this.params.realName,
         this.params.email
       );
       account = accountCreationFuncs.createAccountInBackend(accountConfig);
       success = true;
--- a/mail/test/browser/account/browser_mailAccountSetupWizard.js
+++ b/mail/test/browser/account/browser_mailAccountSetupWizard.js
@@ -30,41 +30,38 @@ let { TelemetryTestUtils } = ChromeUtils
 
 var user = {
   name: "Yamato Nadeshiko",
   email: "yamato.nadeshiko@example.com",
   password: "abc12345",
   incomingHost: "testin.example.com",
   outgoingHost: "testout.example.com",
 };
+var outgoingShortName = "Example Två";
 
 const PREF_NAME = "mailnews.auto_config_url";
 const PREF_VALUE = Services.prefs.getCharPref(PREF_NAME);
 
 // Remove an account in the Account Manager, but not via the UI.
 function remove_account_internal(tab, account, outgoing) {
   let win = tab.browser.contentWindow;
 
-  try {
-    // Remove the account and incoming server
-    let serverId = account.incomingServer.serverURI;
-    MailServices.accounts.removeAccount(account);
-    account = null;
-    if (serverId in win.accountArray) {
-      delete win.accountArray[serverId];
-    }
-    win.selectServer(null, null);
+  // Remove the account and incoming server
+  let serverId = account.incomingServer.serverURI;
+  MailServices.accounts.removeAccount(account);
+  account = null;
+  if (serverId in win.accountArray) {
+    delete win.accountArray[serverId];
+  }
+  win.selectServer(null, null);
 
-    // Remove the outgoing server
-    let smtpKey = outgoing.key;
-    MailServices.smtp.deleteServer(outgoing);
-    win.replaceWithDefaultSmtpServer(smtpKey);
-  } catch (ex) {
-    throw new Error("failure to remove account: " + ex + "\n");
-  }
+  // Remove the outgoing server
+  let smtpKey = outgoing.key;
+  MailServices.smtp.deleteServer(outgoing);
+  win.replaceWithDefaultSmtpServer(smtpKey);
 }
 
 add_task(function test_mail_account_setup() {
   // Set the pref to load a local autoconfig file.
   let url =
     "http://mochi.test:8888/browser/comm/mail/test/browser/account/xml/";
   Services.prefs.setCharPref(PREF_NAME, url);
 
@@ -141,16 +138,20 @@ function subtest_verify_account(tab) {
     },
     "outgoing server hostname": {
       // And this is lowercase
       actual: outgoing.hostname,
       expected: user.outgoingHost,
     },
     "user real name": { actual: identity.fullName, expected: user.name },
     "user email address": { actual: identity.email, expected: user.email },
+    "outgoing description": {
+      actual: outgoing.description,
+      expected: outgoingShortName,
+    },
   };
 
   try {
     for (let i in config) {
       Assert.equal(
         config[i].actual,
         config[i].expected,
         "Configured " +
--- a/mail/test/browser/account/xml/example.com
+++ b/mail/test/browser/account/xml/example.com
@@ -1,24 +1,24 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <clientConfig>
   <emailProvider id="example.com">
     <domain>example.com</domain>
-    <displayName>Example</displayName>
-    <displayShortName>Example</displayShortName>
+    <displayName>Example Två</displayName>
     <incomingServer type="pop3">
       <hostname>testin.%EMAILDOMAIN%</hostname>
       <port>995</port>
       <socketType>SSL</socketType>
       <username>%EMAILLOCALPART%</username>
       <authentication>plain</authentication>
     </incomingServer>
     <outgoingServer type="smtp">
       <hostname>testout.%EMAILDOMAIN%</hostname>
       <port>587</port>
       <socketType>STARTTLS</socketType>
       <username>%EMAILADDRESS%</username>
+      <password>Blä4</password>
       <authentication>plain</authentication>
       <addThisServer>true</addThisServer>
       <useGlobalPreferredServer>false</useGlobalPreferredServer>
     </outgoingServer>
   </emailProvider>
 </clientConfig>
--- a/mail/test/browser/newmailaccount/browser_newmailaccount.js
+++ b/mail/test/browser/newmailaccount/browser_newmailaccount.js
@@ -1106,16 +1106,88 @@ add_task(function test_window_open_link_
 
   // Close the new tab.
   let newTab = mc.tabmail.currentTabInfo;
   mc.tabmail.closeTab(newTab);
   mc.tabmail.closeTab(tab);
 });
 
 /**
+ * Test that if the final provider sends ok account settings back in config.xml
+ * then the account is created and we're done.
+ */
+add_task(function test_provisioner_ok_account_setup() {
+  get_to_order_form("green@example.com");
+
+  let accounts0 = MailServices.accounts.accounts;
+
+  let tab = mc.tabmail.currentTabInfo;
+
+  plan_for_modal_dialog("AccountCreation", function(aController) {
+    aController.window.close();
+  });
+
+  // Click the OK button to order the account.
+  BrowserTestUtils.synthesizeMouseAtCenter(
+    "input[value=Send]",
+    {},
+    tab.browser
+  );
+
+  // Submitting will generate a response with a config.xml configuration for
+  // the account. That will be intercepted and the account set up automatically.
+
+  // The "Congratulations" dialog will be shown.
+  wait_for_modal_dialog("AccountCreation");
+
+  let accounts2 = MailServices.accounts.accounts;
+  Assert.equal(
+    accounts2.length,
+    accounts0.length + 1,
+    "Should have added an account"
+  );
+
+  let account = accounts2[accounts2.length - 1];
+  Assert.equal(account.incomingServer.type, "imap");
+  Assert.equal(account.incomingServer.hostName, "imap-provisioned.example.com");
+
+  let inURL = "imap://imap-provisioned.example.com";
+  let imapLogins = Services.logins.findLogins(inURL, null, inURL);
+  Assert.equal(imapLogins.length, 1, "should have incoming login");
+  Assert.equal(
+    imapLogins[0].username,
+    "green@example.com",
+    "imap username should be correct"
+  );
+  Assert.equal(
+    imapLogins[0].password,
+    "Håhå",
+    "imap password should be correct"
+  );
+
+  let outURL = "smtp://smtp-provisioned.example.com";
+  let smtpLogins = Services.logins.findLogins(outURL, null, outURL);
+  Assert.equal(smtpLogins.length, 1, "should have outgoing login");
+  Assert.equal(
+    smtpLogins[0].username,
+    "green@example.com",
+    "smtp username should be correct"
+  );
+  Assert.equal(
+    smtpLogins[0].password,
+    "Östad3",
+    "smtp password should be correct"
+  );
+
+  Services.logins.removeAllLogins();
+
+  // TODO: check that the folder pane is now visible
+});
+
+/**
  * Test that if the provider returns XML that we can't turn into an account,
  * then we error out and go back to the Account Provisioner dialog.
  */
 add_task(function test_return_to_provisioner_on_error_XML() {
   const kOriginalTabNum = mc.tabmail.tabContainer.allTabs.length;
 
   get_to_order_form("error@error.invalid");
 
--- a/mail/test/browser/newmailaccount/html/config.xml
+++ b/mail/test/browser/newmailaccount/html/config.xml
@@ -1,34 +1,33 @@
 <clientConfig version="1.1">
   <emailProvider id="%DOMAIN%">
     <domain>%EMAILDOMAIN%</domain>
     <displayName>Provisioned Account</displayName>
-    <displayShortName>Provisioned Account</displayShortName>
     <incomingServer type="imap">
-      <hostname>imap.%EMAILDOMAIN%</hostname>
+      <hostname>imap-provisioned.%EMAILDOMAIN%</hostname>
       <port>993</port>
       <socketType>SSL</socketType>
       <username>%EMAILADDRESS%</username>
       <authentication>password-cleartext</authentication>
-      <password>Testing</password>
+      <password>Håhå</password>
     </incomingServer>
     <incomingServer type="pop3">
-      <hostname>pop.%EMAILDOMAIN%</hostname>
+      <hostname>pop-provisioned.%EMAILDOMAIN%</hostname>
       <port>995</port>
       <socketType>SSL</socketType>
       <username>%EMAILLOCALPART%</username>
       <authentication>password-cleartext</authentication>
       <password>Testing</password>
       <pop3>
         <leaveMessagesOnServer>true</leaveMessagesOnServer>
       </pop3>
     </incomingServer>
     <outgoingServer type="smtp">
-      <hostname>smtp.%EMAILDOMAIN%</hostname>
+      <hostname>smtp-provisioned.%EMAILDOMAIN%</hostname>
       <port>465</port>
       <socketType>SSL</socketType>
       <username>%EMAILADDRESS%</username>
       <authentication>password-cleartext</authentication>
-      <password>Testing</password>
+      <password>Östad3</password>
     </outgoingServer>
   </emailProvider>
 </clientConfig>