Merge pull request #18922 from mozilla/310_12-backport

bug(session): Ensure that the session matches account on destroy

Author: vbudhram

packages/fxa-auth-server/lib/routes/account.ts

@@ -1815,12 +1815,22 @@ export class AccountHandler {
     this.log.begin('Account.destroy', request);
 
     const { authPW, email: emailAddress } = request.payload as any;
+    const sessionToken = request.auth && request.auth.credentials as unknown as {
+      uid: string;
+      email: string;
+      tokenVerified: boolean;
+      tokenVerificationId?: string;
+      authenticatorAssuranceLevel?: number;
+    }
 
     await this.customs.check(request, emailAddress, 'accountDestroy');
 
     let accountRecord: Account;
     try {
       accountRecord = await this.db.accountRecord(emailAddress);
+      if (accountRecord.uid !== sessionToken.uid) {
+        throw error.unknownAccount(emailAddress);
+      }
     } catch (err) {
       if (err.errno === error.ERRNO.ACCOUNT_UNKNOWN) {
         await this.customs.flag(request.app.clientAddress, {
@@ -1832,7 +1842,6 @@ export class AccountHandler {
       throw err;
     }
 
-    const sessionToken = request.auth && request.auth.credentials;
     const hasTotpToken = await this.otpUtils.hasTotpToken(accountRecord);
 
     // Someone tried to delete an account with TOTP but did not specify a session.

packages/fxa-auth-server/test/local/routes/account.js

@@ -3847,13 +3847,14 @@ describe('/account/destroy', () => {
   const tokenVerified = true;
   const uid = uuid.v4({}, Buffer.alloc(16)).toString('hex');
 
-  let mockDB, mockLog, mockRequest, mockPush, mockPushbox;
+  let mockDB, mockLog, mockRequest, mockPush, mockPushbox, mockCustoms;
 
   beforeEach(async () => {
     mockDB = {
       ...mocks.mockDB({ email: email, uid: uid }),
     };
     mockLog = mocks.mockLog();
+    mockCustoms = mocks.mockCustoms();
     mockRequest = mocks.mockRequest({
       credentials: { uid, email, tokenVerified },
       log: mockLog,
@@ -3889,6 +3890,7 @@ describe('/account/destroy', () => {
       log: mockLog,
       push: mockPush,
       pushbox: mockPushbox,
+      customs: mockCustoms
     });
     return getRoute(accountRoutes, '/account/destroy');
   }
@@ -3969,6 +3971,26 @@ describe('/account/destroy', () => {
       });
     });
   });
+
+  it('should fail for mismatch session and account ui', async () => {
+    mockDB = { ...mocks.mockDB({ email, uid }) };
+    mockRequest = mocks.mockRequest({
+      credentials: { uid: 'anotherone', email: `[email protected]`, tokenVerified },
+      log: mockLog,
+      payload: {
+        email,
+      },
+    });
+    const route = buildRoute();
+
+    try {
+      await runTest(route, mockRequest);
+      sinon.assert.fail('should have errored');
+    } catch (error) {
+      sinon.assert.calledOnceWithExactly(mockCustoms.flag, "63.245.221.32", { email, errno: 102 });
+      assert.equal(error.errno, 102, 'unknown account');
+    }
+  });
 });
 
 describe('/account', () => {