Bug 1528136 - move config object oauthSettings to incoming and outgoing where it belongs. r=aleca
authorMagnus Melin <mkmelin+mozilla@iki.fi>
Mon, 20 Apr 2020 20:26:49 +0300
changeset 38885 553b3e0e942a25cd48300f44dc6c72b6222bfe0a
parent 38884 3e7fa8d4bd21f2a1259e05134a84147737919199
child 38886 6e1d49280545bb8996902c188e4823e738e83b7c
push id401
push userclokep@gmail.com
push dateMon, 01 Jun 2020 20:41:59 +0000
reviewersaleca
bugs1528136
Bug 1528136 - move config object oauthSettings to incoming and outgoing where it belongs. r=aleca
mail/components/accountcreation/content/accountConfig.js
mail/components/accountcreation/content/createInBackend.js
mail/components/accountcreation/content/emailWizard.js
mail/components/accountcreation/content/exchangeAutoDiscover.js
mail/components/accountcreation/content/readFromXML.js
mail/components/accountcreation/content/verifyConfig.js
--- a/mail/components/accountcreation/content/accountConfig.js
+++ b/mail/components/accountcreation/content/accountConfig.js
@@ -44,18 +44,16 @@ AccountConfig.prototype = {
   /**
    * Other servers which can be used instead of |incoming|,
    * in order of decreasing preference.
    * (|incoming| itself should not be included here.)
    * { Array of incoming/createNewIncoming() }
    */
   incomingAlternatives: null,
   outgoingAlternatives: null,
-  // OAuth2 configuration, if needed.
-  oauthSettings: null,
   // just an internal string to refer to this. Do not show to user.
   id: null,
   // who created the config.
   // { one of kSource* }
   source: 0,
   displayName: null,
   // { Array of { varname (value without %), displayName, exampleValue } }
   inputFields: null,
@@ -110,16 +108,19 @@ AccountConfig.prototype = {
       daysToLeaveMessagesOnServer: 14,
       deleteByAgeFromServer: true,
       // When user hits delete, delete from local store and from server
       deleteOnServerWhenLocalDelete: true,
       downloadOnBiff: true,
       // Override `addThisServer` for a specific incoming server
       useGlobalPreferredServer: false,
 
+      // OAuth2 configuration, if needed.
+      oauthSettings: null,
+
       // for Microsoft Exchange servers. Optional.
       owaURL: null,
       ewsURL: null,
       easURL: null,
       // for when an addon overrides the account type. Optional.
       addonAccountType: null,
     };
   },
@@ -140,16 +141,19 @@ AccountConfig.prototype = {
       addThisServer: true, // if we already have an SMTP server, add this
       // if we already have an SMTP server, use it.
       useGlobalPreferredServer: false,
       // we should reuse an already configured SMTP server.
       // nsISmtpServer.key
       existingServerKey: null,
       // user display value for existingServerKey
       existingServerLabel: null,
+
+      // OAuth2 configuration, if needed.
+      oauthSettings: null,
     };
   },
 
   /**
    * The configuration needs an addon to handle the account type.
    * The addon needs to be installed before the account can be created
    * in the backend.
    * You can choose one, if there are several addons in the list.
--- a/mail/components/accountcreation/content/createInBackend.js
+++ b/mail/components/accountcreation/content/createInBackend.js
@@ -71,18 +71,21 @@ function createAccountInBackend(config) 
       .replace(/[{}]/g, "");
   }
 
   if (config.rememberPassword && config.incoming.password.length) {
     rememberPassword(inServer, config.incoming.password);
   }
 
   if (inServer.authMethod == Ci.nsMsgAuthMethod.OAuth2) {
-    inServer.setCharValue("oauth2.scope", config.oauthSettings.scope);
-    inServer.setCharValue("oauth2.issuer", config.oauthSettings.issuer);
+    inServer.setCharValue("oauth2.scope", config.incoming.oauthSettings.scope);
+    inServer.setCharValue(
+      "oauth2.issuer",
+      config.incoming.oauthSettings.issuer
+    );
   }
 
   // SSL
   if (config.incoming.socketType == 1) {
     // plain
     inServer.socketType = Ci.nsMsgSocketType.plain;
   } else if (config.incoming.socketType == 2) {
     // SSL / TLS
@@ -159,21 +162,21 @@ function createAccountInBackend(config) 
         rememberPassword(outServer, config.incoming.password);
       }
     }
 
     if (outServer.authMethod == Ci.nsMsgAuthMethod.OAuth2) {
       let prefBranch = "mail.smtpserver." + outServer.key + ".";
       Services.prefs.setCharPref(
         prefBranch + "oauth2.scope",
-        config.oauthSettings.scope
+        config.outgoing.oauthSettings.scope
       );
       Services.prefs.setCharPref(
         prefBranch + "oauth2.issuer",
-        config.oauthSettings.issuer
+        config.outgoing.oauthSettings.issuer
       );
     }
 
     if (config.outgoing.socketType == 1) {
       // no SSL
       outServer.socketType = Ci.nsMsgSocketType.plain;
     } else if (config.outgoing.socketType == 2) {
       // SSL / TLS
--- a/mail/components/accountcreation/content/emailWizard.js
+++ b/mail/components/accountcreation/content/emailWizard.js
@@ -1471,21 +1471,23 @@ EmailConfigWizard.prototype = {
     e("in-authMethod-oauth2").hidden = !iDetails;
     if (iDetails) {
       gEmailWizardLogger.info(
         "OAuth2 details for incoming server " +
           config.incoming.hostname +
           " is " +
           iDetails
       );
-      config.oauthSettings = {};
-      [config.oauthSettings.issuer, config.oauthSettings.scope] = iDetails;
-      // oauthsettings are not stored nor changeable in the user interface, so just
-      // store them in the base configuration.
-      this._currentConfig.oauthSettings = config.oauthSettings;
+      config.incoming.oauthSettings = {};
+      [
+        config.incoming.oauthSettings.issuer,
+        config.incoming.oauthSettings.scope,
+      ] = iDetails;
+      this._currentConfig.incoming.oauthSettings =
+        config.incoming.oauthSettings;
     }
 
     // outgoing server
     e("outgoing_hostname").value = config.outgoing.hostname;
     e("outgoing_username").value = config.outgoing.username;
     // While sameInOutUsernames is true we synchronize values of incoming
     // and outgoing username.
     this.sameInOutUsernames = true;
@@ -1510,21 +1512,23 @@ EmailConfigWizard.prototype = {
     e("out-authMethod-oauth2").hidden = !oDetails;
     if (oDetails) {
       gEmailWizardLogger.info(
         "OAuth2 details for outgoing server " +
           config.outgoing.hostname +
           " is " +
           oDetails
       );
-      config.oauthSettings = {};
-      [config.oauthSettings.issuer, config.oauthSettings.scope] = oDetails;
-      // oauthsettings are not stored nor changeable in the user interface, so just
-      // store them in the base configuration.
-      this._currentConfig.oauthSettings = config.oauthSettings;
+      config.outgoing.oauthSettings = {};
+      [
+        config.outgoing.oauthSettings.issuer,
+        config.outgoing.oauthSettings.scope,
+      ] = oDetails;
+      this._currentConfig.outgoing.oauthSettings =
+        config.outgoing.oauthSettings;
     }
 
     // populate fields even if existingServerKey, in case user changes back
     if (config.outgoing.existingServerKey) {
       let menulist = e("outgoing_hostname");
       // We can't use menulist.value = config.outgoing.existingServerKey
       // because would overwrite the text field, so have to do it manually:
       let menuitems = menulist.menupopup.children;
@@ -2070,20 +2074,24 @@ EmailConfigWizard.prototype = {
         self._currentConfig.outgoing.auth = successfulConfig.outgoing.auth;
         self._currentConfig.incoming.username =
           successfulConfig.incoming.username;
         self._currentConfig.outgoing.username =
           successfulConfig.outgoing.username;
 
         // We loaded dynamic client registration, fill this data back in to the
         // config set.
-        if (successfulConfig.oauthSettings) {
-          self._currentConfig.oauthSettings = successfulConfig.oauthSettings;
+        if (successfulConfig.incoming.oauthSettings) {
+          self._currentConfig.incoming.oauthSettings =
+            successfulConfig.incoming.oauthSettings;
         }
-
+        if (successfulConfig.outgoing.oauthSettings) {
+          self._currentConfig.outgoing.oauthSettings =
+            successfulConfig.outgoing.oauthSettings;
+        }
         self.finish(configFilledIn);
       },
       function(e) {
         // failed
         // Could be a wrong password, but there are 1000 other
         // reasons why this failed. Only the backend knows.
         // If we got no message, then something other than VerifyLogon failed.
         self.showErrorMsg(e.message || e.toString());
--- a/mail/components/accountcreation/content/exchangeAutoDiscover.js
+++ b/mail/components/accountcreation/content/exchangeAutoDiscover.js
@@ -230,17 +230,16 @@ function readAutoDiscoverXML(autoDiscove
 
   var config = new AccountConfig();
   config.source = AccountConfig.kSourceExchange;
   config.incoming.username = username || "%EMAILADDRESS%";
   config.incoming.socketType = 2; // only https supported
   config.incoming.port = 443;
   config.incoming.auth = Ci.nsMsgAuthMethod.passwordCleartext;
   config.incoming.authAlternatives = [Ci.nsMsgAuthMethod.OAuth2];
-  config.oauthSettings = {};
   config.outgoing.addThisServer = false;
   config.outgoing.useGlobalPreferredServer = true;
 
   for (let protocolX of array_or_undef(xml.$Protocol)) {
     try {
       let type = sanitize.enum(
         protocolX.Type,
         ["WEB", "EXHTTP", "EXCH", "EXPR", "POP3", "IMAP", "SMTP"],
@@ -349,17 +348,17 @@ function readAutoDiscoverXML(autoDiscove
       // else unknown or unsupported protocol
     } catch (e) {
       logException(e);
     }
   }
 
   // OAuth2 settings, so that createInBackend() doesn't bail out
   if (config.incoming.owaURL || config.incoming.ewsURL) {
-    config.oauthSettings = {
+    config.incoming.oauthSettings = {
       issuer: config.incoming.hostname,
       scope: config.incoming.owaURL || config.incoming.ewsURL,
     };
   }
 
   return config;
 }
 /* eslint-enable complexity */
@@ -542,18 +541,16 @@ function detectStandardProtocols(config,
   config2.incomingAlternatives = config.incomingAlternatives;
   config2.incomingAlternatives.push(config.incoming); // type=exchange
 
   config2.outgoingAlternatives = config.outgoingAlternatives;
   if (config.outgoing.hostname) {
     config2.outgoingAlternatives.push(config.outgoing);
   }
 
-  config2.oauthSettings = config.oauthSettings;
-
   guessConfig(
     domain,
     function(type, hostname, port, ssl, done, config) {
       gEmailWizardLogger.info(
         `Probing exchange server ${hostname} for ${type} protocol support.`
       );
     },
     function(probedConfig) {
--- a/mail/components/accountcreation/content/readFromXML.js
+++ b/mail/components/accountcreation/content/readFromXML.js
@@ -150,17 +150,17 @@ function readFromXML(clientConfigXML) {
         }
         try {
           if ("easURL" in iX) {
             iO.easURL = sanitize.url(iX.easURL);
           }
         } catch (e) {
           logException(e);
         }
-        d.oauthSettings = {
+        iO.oauthSettings = {
           issuer: iO.hostname,
           scope: iO.owaURL || iO.ewsURL || iO.easURL,
         };
       }
       // defaults are in accountConfig.js
       if (iO.type == "pop3" && "pop3" in iX) {
         try {
           if ("leaveMessagesOnServer" in iX.pop3) {
--- a/mail/components/accountcreation/content/verifyConfig.js
+++ b/mail/components/accountcreation/content/verifyConfig.js
@@ -92,51 +92,32 @@ function verifyConfig(
 
   gEmailWizardLogger.info(
     "Setting incoming server authMethod to " + config.incoming.auth
   );
   inServer.authMethod = config.incoming.auth;
 
   try {
     // Lookup issuer if needed.
-    if (
-      config.incoming.auth == Ci.nsMsgAuthMethod.OAuth2 ||
-      config.outgoing.auth == Ci.nsMsgAuthMethod.OAuth2
-    ) {
-      if (!config.oauthSettings) {
-        config.oauthSettings = {};
-      }
-      if (!config.oauthSettings.issuer || !config.oauthSettings.scope) {
-        // lookup issuer or scope from hostname
-        let hostname =
-          config.incoming.auth == Ci.nsMsgAuthMethod.OAuth2
-            ? config.incoming.hostname
-            : config.outgoing.hostname;
-        let hostDetails = OAuth2Providers.getHostnameDetails(hostname);
-        if (hostDetails) {
-          [
-            config.oauthSettings.issuer,
-            config.oauthSettings.scope,
-          ] = hostDetails;
-        }
-        if (!config.oauthSettings.issuer || !config.oauthSettings.scope) {
-          throw new Error("Could not get issuer for oauth2 authentication");
-        }
+    if (config.incoming.auth == Ci.nsMsgAuthMethod.OAuth2) {
+      let [issuer, scope] = OAuth2Providers.getHostnameDetails(
+        config.incoming.hostname,
+        config.incoming.port
+      );
+      if (!issuer || !scope) {
+        throw new Error(
+          `Could not get OAuth2 details for hostname=${config.incoming.hostname}:${config.incoming.port}`
+        );
       }
       gEmailWizardLogger.info(
-        "Saving oauth parameters for issuer " + config.oauthSettings.issuer
+        `Saving incoming server OAuth2 details for hostname=${config.incoming.hostname}: issuer=${issuer}, scope=${scope}`
       );
-      inServer.setCharValue("oauth2.scope", config.oauthSettings.scope);
-      inServer.setCharValue("oauth2.issuer", config.oauthSettings.issuer);
-      gEmailWizardLogger.info(
-        "OAuth2 issuer, scope is " +
-          config.oauthSettings.issuer +
-          ", " +
-          config.oauthSettings.scope
-      );
+      inServer.setCharValue("oauth2.scope", scope);
+      inServer.setCharValue("oauth2.issuer", issuer);
+      config.incoming.oauthSettings = { issuer, scope };
     }
     if (config.incoming.owaURL) {
       inServer.setUnicharValue("owa_url", config.incoming.owaURL);
     }
     if (config.incoming.ewsURL) {
       inServer.setUnicharValue("ews_url", config.incoming.ewsURL);
     }
     if (config.incoming.easURL) {