bug(tests): Fix broken test tear downs

dschom · vbudhram · commit c06edd5293dd · 2025-02-06T11:43:34.000-05:00
diff --git a/packages/functional-tests/lib/testAccountTracker.ts b/packages/functional-tests/lib/testAccountTracker.ts
@@ -206,6 +206,18 @@ export class TestAccountTracker {
         account.email,
         account.password
       );
+
+      await this.target.authClient.sessionResendVerifyCode(
+        credentials.sessionToken
+      );
+      const code = await this.target.emailClient.getVerifyLoginCode(
+        account.email
+      );
+      await this.target.authClient.sessionVerifyCode(
+        credentials.sessionToken,
+        code
+      );
+
       await this.target.authClient.accountDestroy(
         account.email,
         account.password,
diff --git a/packages/functional-tests/tests/oauth/oauthPermissionsSignup.spec.ts b/packages/functional-tests/tests/oauth/oauthPermissionsSignup.spec.ts
@@ -7,7 +7,8 @@ import { expect, test } from '../../lib/fixtures/standard';
 test.describe('severity-1 #smoke', () => {
   test.describe('oauth permissions for trusted reliers - sign up', () => {
     test('signup without `prompt=consent`', async ({
-      pages: { page, signup, relier },
+      target,
+      pages: { page, signup, relier, confirmSignupCode },
       testAccountTracker,
     }) => {
       const { email, password } = testAccountTracker.generateAccountDetails();
@@ -19,12 +20,16 @@ test.describe('severity-1 #smoke', () => {
 
       //no permissions asked for, straight to confirm
       await expect(page).toHaveURL(/confirm_signup_code/);
+
+      // Provide the code so the account can be cleaned up.
+      const code = await target.emailClient.getVerifyShortCode(email);
+      await confirmSignupCode.fillOutCodeForm(code);
     });
 
     test('signup with `prompt=consent`', async ({
       target,
       page,
-      pages: { configPage, signup, relier },
+      pages: { configPage, signup, relier, confirmSignupCode },
       testAccountTracker,
     }) => {
       const config = await configPage.getConfig();
@@ -48,6 +53,10 @@ test.describe('severity-1 #smoke', () => {
 
       //Verify sign up code header
       await expect(page).toHaveURL(/confirm_signup_code/);
+
+      // Provide the code so the account can be cleaned up.
+      const code = await target.emailClient.getVerifyShortCode(email);
+      await confirmSignupCode.fillOutCodeForm(code);
     });
   });
 });
diff --git a/packages/functional-tests/tests/oauth/signin.spec.ts b/packages/functional-tests/tests/oauth/signin.spec.ts
@@ -138,8 +138,9 @@ test.describe('severity-1 #smoke', () => {
     });
 
     test('oauth endpoint chooses the right auth flows', async ({
-      pages: { page, signin, signup, relier },
+      pages: { page, signin, signup, relier, confirmSignupCode },
       testAccountTracker,
+      target,
     }) => {
       // Create unverified account
       const { email, password } = testAccountTracker.generateAccountDetails();
@@ -155,6 +156,11 @@ test.describe('severity-1 #smoke', () => {
       await relier.goto();
       await relier.clickChooseFlow();
       await expect(signin.cachedSigninHeading).toBeVisible();
+
+      await signin.signInButton.click();
+      await expect(page).toHaveURL(/confirm_signup_code/);
+      const code = await target.emailClient.getVerifyShortCode(email);
+      await confirmSignupCode.fillOutCodeForm(code);
     });
   });
 });
diff --git a/packages/functional-tests/tests/react-conversion/oauthSignin.spec.ts b/packages/functional-tests/tests/react-conversion/oauthSignin.spec.ts
@@ -182,7 +182,6 @@ test.describe('severity-1 #smoke', () => {
 
       // Verify email and ensure user is redirected to relier
       await expect(page).toHaveURL(/confirm_signup_code/);
-      await expect(page).toHaveURL(/confirm_signup_code/);
       const code = await target.emailClient.getVerifyShortCode(email);
       await confirmSignupCode.fillOutCodeForm(code);
 
@@ -227,6 +226,9 @@ test.describe('severity-1 #smoke', () => {
 
       await expect(signin.cachedSigninHeading).toBeVisible();
       await expect(page.getByText(email)).toBeVisible();
+
+      await signin.signInButton.click();
+      expect(await relier.isLoggedIn()).toBe(true);
     });
   });
 });
diff --git a/packages/fxa-auth-server/config/index.ts b/packages/fxa-auth-server/config/index.ts
@@ -1980,10 +1980,16 @@ const convictConf = convict({
   accountDestroy: {
     requireVerifiedAccount: {
       doc: 'Whether or not the account must be verified in order to destroy it.',
-      default: true,
+      default: false,
       format: Boolean,
       env: 'ACCOUNT_DESTROY__REQUIRE_VERIFIED_ACCOUNT',
     },
+    requireVerifiedSession: {
+      doc: 'Whether or not the account must have a verified session in order to destroy it.',
+      default: true,
+      format: Boolean,
+      env: 'ACCOUNT_DESTROY__REQUIRE_VERIFIED_SESSION',
+    },
   },
   passwordForgotOtp: {
     digits: {
diff --git a/packages/fxa-auth-server/lib/routes/account.ts b/packages/fxa-auth-server/lib/routes/account.ts
@@ -1873,7 +1873,10 @@ export class AccountHandler {
     // The UI will request an OTP code verification before destroying the account in the event
     // the session is currently unverified. The following check will ensure that this OTP code
     // was actually provided by the user.
-    if (!sessionToken.tokenVerified) {
+    if (
+      this.config.accountDestroy.requireVerifiedSession &&
+      !sessionToken.tokenVerified
+    ) {
       throw error.unverifiedSession();
     }
 
diff --git a/packages/fxa-auth-server/lib/routes/session.js b/packages/fxa-auth-server/lib/routes/session.js
@@ -421,6 +421,7 @@ module.exports = function (
       },
       handler: async function (request) {
         log.begin('Session.resend_code', request);
+        console.log('!!! resend code');
         const sessionToken = request.auth.credentials;
 
         request.emitMetricsEvent('session.resend_code');
@@ -471,12 +472,14 @@ module.exports = function (
         if (account.primaryEmail.isVerified) {
           // Unverified emails mean that the user is attempting to resend the code from signup page,
           // therefore they get sent a different email template with the code.
+          console.log('!!! sendVerifyLoginCodeEmail', account.email);
           await mailer.sendVerifyLoginCodeEmail(
             account.emails,
             account,
             options
           );
         } else {
+          console.log('!!! sendVerifyShortCodeEmail');
           await mailer.sendVerifyShortCodeEmail([], account, options);
         }
 
diff --git a/packages/fxa-auth-server/test/local/routes/account.js b/packages/fxa-auth-server/test/local/routes/account.js
@@ -3822,6 +3822,7 @@ describe('/account/keys', () => {
 
 describe('/account/destroy', () => {
   const email = 'foo@example.com';
+  const tokenVerified = true;
   const uid = uuid.v4({}, Buffer.alloc(16)).toString('hex');
 
   let mockDB, mockLog, mockRequest, mockPush, mockPushbox;
@@ -3832,6 +3833,7 @@ describe('/account/destroy', () => {
     };
     mockLog = mocks.mockLog();
     mockRequest = mocks.mockRequest({
+      credentials: { uid, email, tokenVerified },
       log: mockLog,
       payload: {
         email: email,
@@ -3856,6 +3858,9 @@ describe('/account/destroy', () => {
             enabled: true,
           },
         },
+        accountDestroy: {
+          requireVerifiedAccount: false,
+        },
         domain: 'wibble',
       },
       db: mockDB,
@@ -3912,6 +3917,7 @@ describe('/account/destroy', () => {
   it('should delete the passwordless account', () => {
     mockDB = { ...mocks.mockDB({ email, uid, verifierSetAt: 0 }) };
     mockRequest = mocks.mockRequest({
+      credentials: { uid, email, tokenVerified },
       log: mockLog,
       payload: {
         email: email,
diff --git a/packages/fxa-auth-server/test/remote/account_create_tests.js b/packages/fxa-auth-server/test/remote/account_create_tests.js
@@ -161,10 +161,12 @@ const {
         .then(() => {
           return server.mailbox.waitForEmail(email);
         })
-        .then((emailData) => {
+        .then(async (emailData) => {
           assert.include(emailData.text, 'Confirm account', 'en-US');
           // TODO: reinstate after translations catch up
           //assert.notInclude(emailData.text, 'Ativar agora', 'not pt-BR');
+          const code = emailData.headers['x-verify-code'];
+          await client.verifyEmail(code, {});
           return client.destroyAccount();
         })
         .then(() => {
@@ -179,10 +181,12 @@ const {
         .then(() => {
           return server.mailbox.waitForEmail(email);
         })
-        .then((emailData) => {
+        .then(async (emailData) => {
           assert.notInclude(emailData.text, 'Confirm email', 'not en-US');
           // TODO: reinstate after translations catch up
           //assert.include(emailData.text, 'Ativar agora', 'is pt-BR');
+          const code = emailData.headers['x-verify-code'];
+          await client.verifyEmail(code, {});
           return client.destroyAccount();
         });
     });
@@ -994,8 +998,6 @@ const {
       assert.equal(kB2, originalKb);
     });
 
-
-
     async function login(email, password, version = '') {
       return await Client.login(config.publicUrl, email, password, {
         ...testOptions,
diff --git a/packages/fxa-auth-server/test/remote/account_destroy_tests.js b/packages/fxa-auth-server/test/remote/account_destroy_tests.js
@@ -273,7 +273,7 @@ const config = require('../../config').default.getProperties();
         await client.destroyAccount();
         assert.fail('Should not be able allowed to destroy account.');
       } catch (err) {
-        assert.equal(err.message, 'Unconfirmed account');
+        assert.equal(err.message, 'Unconfirmed session');
       }
     });
   });
diff --git a/packages/fxa-auth-server/test/remote/email_validity_tests.js b/packages/fxa-auth-server/test/remote/email_validity_tests.js
@@ -10,77 +10,93 @@ const Client = require('../client')();
 
 const config = require('../../config').default.getProperties();
 
-[{version:""},{version:"V2"}].forEach((testOptions) => {
-
-describe(`#integration${testOptions.version} - remote email validity`, function () {
-  this.timeout(60000);
-  let server;
-  before(async () => {
-    server = await TestServer.start(config);
-  });
-
-  after(async () => {
-    await TestServer.stop(server);
-  });
-
-  it('/account/create with a variety of malformed email addresses', () => {
-    const pwd = '123456';
-
-    const emails = [
-      'notAnEmailAddress',
-      '\n@example.com',
-      'me@hello world.com',
-      'me@hello+world.com',
-      'me@.example',
-      'me@example',
-      'me@example.com-',
-      'me@example..com',
-      'me@example-.com',
-      'me@example.-com',
-      '\uD83D\uDCA9@unicodepooforyou.com',
-    ];
-    emails.forEach((email, i) => {
-      emails[i] = Client.create(config.publicUrl, email, pwd, testOptions).then(
-        assert.fail,
-        (err) => {
-          assert.equal(err.code, 400, 'http 400 : malformed email is rejected');
-        }
-      );
+[{ version: '' }, { version: 'V2' }].forEach((testOptions) => {
+  describe(`#integration${testOptions.version} - remote email validity`, function () {
+    this.timeout(60000);
+    let server;
+    const temp = {};
+
+    before(async () => {
+      temp.requireVerifiedAccount =
+        config.accountDestroy.requireVerifiedAccount;
+      temp.requireVerifiedSession =
+        config.accountDestroy.requireVerifiedSession;
+
+      // Temporarily disable this so we can destroy the unverified accounts in the test below.
+      config.accountDestroy.requireVerifiedAccount = false;
+      config.accountDestroy.requireVerifiedSession = false;
+      server = await TestServer.start(config);
     });
 
-    return Promise.all(emails);
-  });
-
-  it('/account/create with a variety of unusual but valid email addresses', () => {
-    const pwd = '123456';
+    after(async () => {
+      config.accountDestroy.requireVerifiedAccount =
+        temp.requireVerifiedAccount;
+      config.accountDestroy.requireVerifiedSession =
+        temp.requireVerifiedSession;
+      await TestServer.stop(server);
+    });
 
-    const emails = [
-      'tim@tim-example.net',
-      'a+b+c@example.com',
-      '#!?-@t-e-s-assert.c-o-m',
-      `${String.fromCharCode(1234)}@example.com`,
-      `test@${String.fromCharCode(5678)}.com`,
-    ];
+    it('/account/create with a variety of malformed email addresses', () => {
+      const pwd = '123456';
+
+      const emails = [
+        'notAnEmailAddress',
+        '\n@example.com',
+        'me@hello world.com',
+        'me@hello+world.com',
+        'me@.example',
+        'me@example',
+        'me@example.com-',
+        'me@example..com',
+        'me@example-.com',
+        'me@example.-com',
+        '\uD83D\uDCA9@unicodepooforyou.com',
+      ];
+      emails.forEach((email, i) => {
+        emails[i] = Client.create(
+          config.publicUrl,
+          email,
+          pwd,
+          testOptions
+        ).then(assert.fail, (err) => {
+          assert.equal(err.code, 400, 'http 400 : malformed email is rejected');
+        });
+      });
 
-    emails.forEach((email, i) => {
-      emails[i] = Client.create(config.publicUrl, email, pwd, testOptions).then(
-        (c) => {
-          return c.destroyAccount();
-        },
-        (_err) => {
-          assert(
-            false,
-            `Email address ${email} should have been allowed, but it wasn't`
-          );
-        }
-      );
+      return Promise.all(emails);
     });
 
-    return Promise.all(emails);
+    it('/account/create with a variety of unusual but valid email addresses', () => {
+      const pwd = '123456';
+
+      const emails = [
+        'tim@tim-example.net',
+        'a+b+c@example.com',
+        '#!?-@t-e-s-assert.c-o-m',
+        `${String.fromCharCode(1234)}@example.com`,
+        `test@${String.fromCharCode(5678)}.com`,
+      ];
+
+      emails.forEach((email, i) => {
+        emails[i] = Client.create(
+          config.publicUrl,
+          email,
+          pwd,
+          testOptions
+        ).then(
+          (c) => {
+            return c.destroyAccount();
+          },
+          (_err) => {
+            assert(
+              false,
+              `Email address ${email} should have been allowed, but it wasn't`
+            );
+          }
+        );
+      });
+
+      return Promise.all(emails);
+    });
   });
-
-
-});
-
-
 });

Original file line number	Diff line number	Diff line change
`@@ -273,7 +273,7 @@ const config = require('../../config').default.getProperties();`
`273`	`273`	`await client.destroyAccount();`
`274`	`274`	`assert.fail('Should not be able allowed to destroy account.');`
`275`	`275`	`} catch (err) {`
`276`		`- assert.equal(err.message, 'Unconfirmed account');`
	`276`	`+ assert.equal(err.message, 'Unconfirmed session');`
`277`	`277`	`}`
`278`	`278`	`});`
`279`	`279`	`});`