Bug 948580 - [B2G][Email] Send "MAIL FROM" in SMTP probe to validate …

…e-mail address.

data/lib/mailapi/composite/configurator.js

@@ -50,6 +50,7 @@ exports.configurator = {
         incomingInfo.preferredAuthMethod = null;
       }
       smtpConnInfo = {
+        emailAddress: userDetails.emailAddress, // used for probing
         hostname: domainInfo.outgoing.hostname,
         port: domainInfo.outgoing.port,
         crypto: (typeof domainInfo.outgoing.socketType === 'string' ?

data/lib/mailapi/mailapi.js

@@ -57,6 +57,7 @@ function MailAccount(api, wireRep, acctsSlice) {
   /**
    * @listof[@oneof[
    *   @case['bad-user-or-pass']
+   *   @case['bad-address']
    *   @case['needs-app-pass']
    *   @case['imap-disabled']
    *   @case['pop-server-not-great']{
@@ -2642,6 +2643,9 @@ MailAPI.prototype = {
    *     The username and password didn't check out.  We don't know which one
    *     is wrong, just that one of them is wrong.
    *   }
+   *   @case['bad-address']{
+   *     The e-mail address provided was rejected by the SMTP probe.
+   *   }
    *   @case['pop-server-not-great']{
    *     The POP3 server doesn't support IDLE and TOP, so we can't use it.
    *   }

data/lib/mailapi/mailbridge.js

@@ -219,6 +219,7 @@ MailBridge.prototype = {
       // we're offline.
       if (!err || (
           err !== 'bad-user-or-pass' &&
+          err !== 'bad-address' &&
           err !== 'needs-app-pass' &&
           err !== 'imap-disabled'
         )) {

data/lib/mailapi/mailuniverse.js

@@ -841,6 +841,7 @@ MailUniverse.prototype = {
 
     switch (problem) {
       case 'bad-user-or-pass':
+      case 'bad-address':
       case 'imap-disabled':
       case 'needs-app-pass':
         this.__notifyBadLogin(account, problem);

data/lib/mailapi/smtp/probe.js

@@ -35,9 +35,20 @@ exports.CONNECT_TIMEOUT_MS = 30000;
  * Validate that we find an SMTP server using the connection info and that it
  * seems to like our credentials.
  *
- * Because the SMTP client has no connection timeout support, use our own timer
- * to decide when to give up on the SMTP connection.  We use the timer for the
- * whole process, including even after the connection is established.
+ * Because the SMTP client has no connection timeout support, use our
+ * own timer to decide when to give up on the SMTP connection. We use
+ * the timer for the whole process, including even after the
+ * connection is established and we probe for a valid address.
+ *
+ * The process here is in two steps: First, connect to the server and
+ * make sure that we can authenticate properly. Then, if that
+ * succeeds, we send a "MAIL FROM:<our address>" line to see if the
+ * server will reject the e-mail address, followed by "RCPT TO" for
+ * the same purpose. This could fail if the user uses manual setup and
+ * gets everything right except for their e-mail address. We want to
+ * catch this error before they complete account setup; if we don't,
+ * they'll be left with an account that can't send e-mail, and we
+ * currently don't allow them to change their address after setup.
  */
 function SmtpProber(credentials, connInfo) {
   console.log("PROBE:SMTP attempting to connect to", connInfo.hostname);
@@ -48,56 +59,123 @@ function SmtpProber(credentials, connInfo) {
       auth: { user: credentials.username, pass: credentials.password },
       debug: exports.TEST_USE_DEBUG_MODE,
     });
-  // onIdle happens after successful login, and so is what our probing uses.
-  this._conn.on('idle', this.onResult.bind(this, null));
-  this._conn.on('error', this.onResult.bind(this));
-  this._conn.on('end', this.onResult.bind(this, 'unknown'));
 
-  this.timeoutId = setTimeoutFunc(
-                     this.onResult.bind(this, 'unresponsive-server'),
-                     exports.CONNECT_TIMEOUT_MS);
+  // For the first step (connection/authentication), handle callbacks
+  // in this.onConnectionResult.
+  this.setConnectionListenerCallback(this.onConnectionResult);
+
+  this.timeoutId = setTimeoutFunc(function() {
+    // Emit a fake error from the connection so that we can send the
+    // error to the proper callback handler depending on what state
+    // the connection is in.
+    this._conn.emit('error', 'unresponsive-server');
+  }.bind(this), exports.CONNECT_TIMEOUT_MS);
 
+  this.emailAddress = connInfo.emailAddress;
   this.onresult = null;
   this.error = null;
   this.errorDetails = { server: connInfo.hostname };
 }
 exports.SmtpProber = SmtpProber;
 SmtpProber.prototype = {
-  onResult: function(err) {
+
+  /**
+   * Unsubscribe any existing listeners, and resubscribe to all
+   * relevant events for the given fn handler.
+   */
+  setConnectionListenerCallback: function(fn) {
+    this._conn.removeAllListeners();
+    // onIdle happens after successful login, and so is what our probing uses.
+    this._conn.on('idle', fn.bind(this, null));
+    this._conn.on('error', fn.bind(this));
+    this._conn.on('end', fn.bind(this, 'unknown'));
+  },
+
+  /**
+   * Callback for initial connection, before we check for address
+   * validity. Connection and security errors will happen here.
+   */
+  onConnectionResult: function(err) {
     if (!this.onresult)
-      return;
+      return; // We already handled the result.
+
+    // XXX just map all security errors as indicated by name
     if (err && typeof(err) === 'object') {
-      // XXX just map all security errors as indicated by name
       if (err.name && /^Security/.test(err.name)) {
         err = 'bad-security';
-      }
-      else {
+      } else {
         switch (err.name) {
-          case 'AuthError':
-            err = 'bad-user-or-pass';
-            break;
-          case 'UnknownAuthError':
-          default:
-            err = 'server-problem';
-            break;
+        case 'AuthError':
+          err = 'bad-user-or-pass';
+          break;
+        case 'UnknownAuthError':
+        default:
+          err = 'server-problem';
+          break;
         }
       }
     }
 
-    this.error = err;
-    if (err)
+    if (err) {
+      this.cleanup(err);
+    } else {
+      console.log('PROBE:SMTP connected, checking address validity');
+      // For clarity, send callbacks to onAddressValidityResult.
+      this.setConnectionListenerCallback(this.onAddressValidityResult);
+      this._conn.useEnvelope({
+        from: this.emailAddress,
+        to: [this.emailAddress]
+      });
+      this._conn.on('message', function() {
+        // Success! Our recipient was valid.
+        this.onAddressValidityResult(null);
+      }.bind(this));
+    }
+  },
+
+  /**
+   * The server will respond to a "MAIL FROM" probe, indicating
+   * whether or not the e-mail address is invalid. We try to succeed
+   * unless we're positive that the server actually rejected the
+   * address (in other words, any error other than "SenderError" is
+   * ignored).
+   */
+  onAddressValidityResult: function(err) {
+    if (!this.onresult)
+      return; // We already handled the result.
+
+    if (err && (err.name === 'SenderError' ||
+                err.name === 'RecipientError')) {
+      err = 'bad-address';
+    } else if (err && err.name) {
+      // This error wasn't normalized (so it's not
+      // "unresponsive-server"); we don't expect any auth or
+      // connection failures here, so treat it as an unknown error.
+      err = 'server-problem';
+    }
+    this.cleanup(err);
+  },
+
+  /**
+   * Send the final probe result (with error or not) and close the
+   * SMTP connection.
+   */
+  cleanup: function(err) {
+    clearTimeoutFunc(this.timeoutId);
+
+    if (err) {
       console.warn('PROBE:SMTP sad. error: | ' + (err && err.name || err) +
                    ' | '  + (err && err.message || '') + ' |');
-    else
+    } else {
       console.log('PROBE:SMTP happy');
+    }
 
-    clearTimeoutFunc(this.timeoutId);
-
+    this.error = err;
     this.onresult(this.error, this.errorDetails);
     this.onresult = null;
 
     this._conn.close();
-  },
+  }
 };
 
 }); // end define

test/unit/test_smtp_prober.js

@@ -201,32 +201,35 @@ function cannedLoginTest(T, RT, opts) {
     eCheck.expect_namedValue('smtp:setTimeout', $smtpprobe.CONNECT_TIMEOUT_MS);
     eCheck.expect_event('smtp:clearTimeout');
     eCheck.expect_namedValue('probe result', opts.expectResult);
+    var precommands = [
+      {
+        match: true,
+        actions: [
+          {
+            cmd: 'fake-receive',
+            data: opts.ehloResponse || SMTP_EHLO_RESPONSE
+          },
+        ],
+      },
+      {
+        match: true,
+        actions: [
+          {
+            cmd: 'fake-receive',
+            data: opts.loginErrorString
+          }
+        ],
+      },
+    ];
+    if (opts.precommands) {
+      precommands = precommands.concat(opts.precommands);
+    }
     FawltySocketFactory.precommand(
       HOST, PORT,
       {
         cmd: 'fake',
         data: SMTP_GREETING,
-      },
-      [
-        {
-          match: true,
-          actions: [
-            {
-              cmd: 'fake-receive',
-              data: opts.ehloResponse || SMTP_EHLO_RESPONSE
-            },
-          ],
-        },
-        {
-          match: true,
-          actions: [
-            {
-              cmd: 'fake-receive',
-              data: opts.loginErrorString
-            }
-          ],
-        },
-      ]);
+      }, precommands);
     prober = new $smtpprobe.SmtpProber(cci.credentials, cci.connInfo);
     prober.onresult = function(err) {
       eCheck.namedValue('probe result', err);
@@ -250,4 +253,68 @@ TD.commonCase('angry server', function(T, RT) {
   });
 });
 
+TD.commonCase('bad address', function(T, RT) {
+  cannedLoginTest(T, RT, {
+    loginErrorString: '200 Keep up the good work\r\n',
+    precommands: [
+      {
+        match: /MAIL FROM/ig,
+        actions: [
+          {
+            cmd: 'fake-receive',
+            data: '200 Continue\r\n',
+          }
+        ]
+      },
+      {
+        match: /RCPT TO/ig,
+        actions: [
+          {
+            cmd: 'fake-receive',
+            data: '554 Sender Address Rejected\r\n',
+          }
+        ]
+      }
+    ],
+    expectResult: 'bad-address',
+  });
+});
+
+TD.commonCase('good address', function(T, RT) {
+  cannedLoginTest(T, RT, {
+    loginErrorString: '200 Keep up the good work\r\n',
+    precommands: [
+      {
+        match: /MAIL FROM/ig,
+        actions: [
+          {
+            cmd: 'fake-receive',
+            data: '200 Continue\r\n',
+          }
+        ]
+      },
+      {
+        match: /RCPT TO/ig,
+        actions: [
+          {
+            cmd: 'fake-receive',
+            data: '200 rock on little buddy\r\n',
+          }
+        ]
+      },
+      {
+        match: /DATA/ig,
+        actions: [
+          {
+            cmd: 'fake-receive',
+            data: '200 go ahead\r\n',
+          }
+        ]
+      },
+    ],
+    expectResult: null,
+  });
+});
+
+
 }); // end define