Merge pull request #20267 from mozilla/fxa-12710

fix(auth): reject non-ASCII email local parts at validation

Author: vbudhram

packages/fxa-auth-server/lib/routes/validators.js

@@ -193,7 +193,14 @@ module.exports.isValidEmailAddress = function (value) {
     return false;
   }
 
-  if (!EMAIL_USER.test(punycode.toASCII(parts[0]))) {
+  // Reject non-ASCII characters in the local part.
+  // AWS SES does not support SMTPUTF8 (RFC 6531), so we cannot
+  // deliver email to addresses with non-ASCII local parts.
+  // See: https://docs.aws.amazon.com/ses/latest/APIReference/API_Destination.html
+  // Note: punycode.toASCII() is intentionally NOT used here because
+  // it would encode non-ASCII chars (e.g. 'andrée' → 'xn--andre-9ua'),
+  // masking the problem.
+  if (!EMAIL_USER.test(parts[0])) {
     return false;
   }
 

packages/fxa-auth-server/lib/routes/validators.spec.ts

@@ -17,8 +17,9 @@ describe('lib/routes/validators:', () => {
     expect(
       validators.isValidEmailAddress('.+#$!%&|*/+-=?^_{}~`@example.com')
     ).toBe(true);
-    expect(validators.isValidEmailAddress('Δ٢@example.com')).toBe(true);
-    expect(validators.isValidEmailAddress('🦀🧙@example.com')).toBe(true);
+    // Non-ASCII local parts are rejected (SES doesn't support SMTPUTF8)
+    expect(validators.isValidEmailAddress('Δ٢@example.com')).toBe(false);
+    expect(validators.isValidEmailAddress('🦀🧙@example.com')).toBe(false);
     expect(
       validators.isValidEmailAddress(
         `${new Array(64).fill('a').join('')}@example.com`
@@ -1052,4 +1053,14 @@ describe('lib/routes/validators:', () => {
       ).toBeTruthy();
     });
   });
+
+  it('isValidEmailAddress rejects non-ASCII local parts', () => {
+    // Accented letters
+    expect(validators.isValidEmailAddress('café@restmail.net')).toBe(false);
+    expect(validators.isValidEmailAddress('naï[email protected]')).toBe(false);
+    expect(validators.isValidEmailAddress('ü[email protected]')).toBe(false);
+    // Invisible unicode (zero-width space, RTL mark)
+    expect(validators.isValidEmailAddress('foo\[email protected]')).toBe(false);
+    expect(validators.isValidEmailAddress('foo\[email protected]')).toBe(false);
+  });
 });

packages/fxa-auth-server/lib/senders/email.js

@@ -506,6 +506,16 @@ module.exports = function (log, config, bounces, statsd) {
 
     const to = message.email;
 
+    // Skip non-ASCII local parts. SES doesn't support SMTPUTF8, so these
+    // always fail at RCPT TO. Silently return (like bounce check below)
+    // to avoid Sentry noise from pre-existing accounts.
+    const [localPart] = to.split('@');
+    if (/[^\x20-\x7E]/.test(localPart)) {
+      statsd.increment('email.nonAsciiLocalPart', { template });
+      log.warn('mailer.send.nonAsciiLocalPart', { to, template });
+      return;
+    }
+
     try {
       await bounces.check(to, template);
     } catch (err) {

packages/fxa-auth-server/lib/subscription-account-reminders.in.spec.ts

@@ -13,6 +13,9 @@ const EXPECTED_CREATE_DELETE_RESULT = REMINDERS.reduce(
 
 const config = require('../config').default.getProperties();
 const mocks = require('../test/mocks');
+const TEST_PREFIX = `test-lib-subscription-account-reminders:${
+  process.env.JEST_WORKER_ID || '1'
+}:`;
 
 describe('#integration - lib/subscription-account-reminders', () => {
   let log: any, mockConfig: any, redis: any, subscriptionAccountReminders: any;
@@ -30,7 +33,7 @@ describe('#integration - lib/subscription-account-reminders', () => {
         redis: {
           maxConnections: 1,
           minConnections: 1,
-          prefix: 'test-subscription-account-reminders-lib:',
+          prefix: TEST_PREFIX,
         },
       },
     };

packages/fxa-auth-server/test/remote/account_create.in.spec.ts

@@ -627,19 +627,15 @@ describe.each(testVersions)(
       expect(emailData.headers['x-service-id']).toBe('bar');
     });
 
-    it('account creation works with unicode email address', async () => {
+    it('account creation rejects unicode email address', async () => {
       const email = server.uniqueUnicodeEmail();
 
-      const client = await Client.create(
-        server.publicUrl,
-        email,
-        'foo',
-        testOptions
-      );
-      expect(client).toBeTruthy();
-
-      const emailData = await server.mailbox.waitForEmail(email);
-      expect(emailData).toBeTruthy();
+      try {
+        await Client.create(server.publicUrl, email, 'foo', testOptions);
+        throw new Error('should have failed');
+      } catch (err: any) {
+        expect(err.errno).toBe(107);
+      }
     });
 
     it('account creation fails with invalid metricsContext flowId', async () => {

packages/fxa-auth-server/test/remote/account_login.in.spec.ts

@@ -125,24 +125,15 @@ describe.each(testVersions)(
       expect(c.keyFetchToken).toBeNull();
     });
 
-    it('login works with unicode email address', async () => {
+    it('login rejects unicode email address', async () => {
       const email = server.uniqueUnicodeEmail();
-      const password = 'wibble';
-      await Client.createAndVerify(
-        server.publicUrl,
-        email,
-        password,
-        server.mailbox,
-        testOptions
-      );
 
-      const client = await Client.login(
-        server.publicUrl,
-        email,
-        password,
-        testOptions
-      );
-      expect(client).toBeTruthy();
+      try {
+        await Client.login(server.publicUrl, email, 'wibble', testOptions);
+        throw new Error('should have failed');
+      } catch (err: any) {
+        expect(err.errno).toBe(107);
+      }
     });
 
     it('account login works with minimal metricsContext metadata', async () => {

packages/fxa-auth-server/test/remote/account_profile.in.spec.ts

@@ -180,16 +180,20 @@ describe.each(testVersions)(
 
     describe('when the profile data is not default', () => {
       describe('when the email address is unicode', () => {
-        it('returns the email address correctly with the profile data', async () => {
+        it('rejects unicode email address', async () => {
           const email = server.uniqueUnicodeEmail();
-          const client = await Client.create(
-            server.publicUrl,
-            email,
-            'password',
-            testOptions
-          );
-          const response = await client.accountProfile();
-          expect(response.email).toBe(email);
+
+          try {
+            await Client.create(
+              server.publicUrl,
+              email,
+              'password',
+              testOptions
+            );
+            throw new Error('should have failed');
+          } catch (err: any) {
+            expect(err.errno).toBe(107);
+          }
         });
       });
 

packages/fxa-auth-server/test/remote/account_status.in.spec.ts

@@ -130,16 +130,15 @@ describe.each(testVersions)(
       }
     });
 
-    it('account status by email works with unicode email address', async () => {
+    it('account status rejects unicode email address', async () => {
       const email = server.uniqueUnicodeEmail();
-      const c = await Client.create(
-        server.publicUrl,
-        email,
-        'password',
-        testOptions
-      );
-      const response = await c.api.accountStatusByEmail(email);
-      expect(response.exists).toBeTruthy();
+
+      try {
+        await Client.create(server.publicUrl, email, 'password', testOptions);
+        throw new Error('should have failed');
+      } catch (err: any) {
+        expect(err.errno).toBe(107);
+      }
     });
   }
 );

packages/fxa-auth-server/test/remote/email_validity.in.spec.ts

@@ -73,7 +73,6 @@ describe.each(testVersions)(
         '[email protected]',
         '[email protected]',
         '#[email protected]',
-        `${String.fromCharCode(1234)}@example.com`,
         `test@${String.fromCharCode(5678)}.com`,
       ];
 
@@ -90,5 +89,22 @@ describe.each(testVersions)(
         )
       );
     });
+
+    it('/account/create rejects non-ASCII local parts but accepts unicode domains', async () => {
+      const pwd = '123456';
+
+      // Unicode local part should be rejected
+      try {
+        await Client.create(
+          server.publicUrl,
+          `${String.fromCharCode(1234)}@example.com`,
+          pwd,
+          testOptions
+        );
+        throw new Error('should have rejected unicode local part');
+      } catch (err: any) {
+        expect(err.errno).toBe(107);
+      }
+    });
   }
 );

packages/fxa-auth-server/test/remote/oauth_db.in.spec.ts

@@ -243,6 +243,11 @@ describe('db', () => {
 
       beforeEach(async () => {
         tokenFirstUsage = {};
+        const mysql = await db.mysql;
+        const mysqlToken = await mysql._getRefreshToken(tokenId);
+        expect(hex(mysqlToken.tokenId)).toBe(hex(tokenId));
+        tokenFirstUsage.mysqlLastUsedAt = new Date(mysqlToken.lastUsedAt);
+
         const t = await db.getRefreshToken(tokenId);
         expect(hex(t.tokenId)).toBe(hex(tokenId));
         tokenFirstUsage.createdAt = new Date(t.createdAt);
@@ -284,7 +289,7 @@ describe('db', () => {
         expect(hex(t.tokenId)).toBe(hex(tokenId));
         expect(
           Math.abs(
-            t.lastUsedAt.getTime() - tokenFirstUsage.lastUsedAt.getTime()
+            t.lastUsedAt.getTime() - tokenFirstUsage.mysqlLastUsedAt.getTime()
           )
         ).toBeLessThanOrEqual(2000);
       });