feat(deletion): revoke fxa tokens when deleting accounts (#897)

* feat(deletion): revoke fxa tokens when deleting accounts This data was deleted before on the Pocket side, but now it will remove Pocket from integrations on the Mozilla account page. [POCKET-9990] * fix(test): add reference to user_firefox_account for test seeds * chore: fix a lost import and nock
Pocket · Jan 29, 2025 · c943522 · c943522
1 parent 8980227
commit c943522
Show file tree

Hide file tree

Showing 15 changed files with 282 additions and 36 deletions.
diff --git a/infrastructure/account-data-deleter/src/dataDeleterApp.ts b/infrastructure/account-data-deleter/src/dataDeleterApp.ts
@@ -217,6 +217,18 @@ export class DataDeleterApp extends Construct {
               name: 'EXPORT_SIGNEDURL_USER_SECRET_KEY',
               valueFrom: `arn:aws:secretsmanager:${region.name}:${caller.accountId}:secret:${config.name}/${config.environment}/EXPORT_USER_CREDS:secretAccessKey::`,
             },
+            {
+              name: 'FXA_CLIENT_ID',
+              valueFrom: `arn:aws:ssm:${region.name}:${caller.accountId}:parameter/Web/${config.environment}/FIREFOX_WEB_AUTH_CLIENT_ID`,
+            },
+            {
+              name: 'FXA_CLIENT_SECRET',
+              valueFrom: `arn:aws:ssm:${region.name}:${caller.accountId}:parameter/Web/${config.environment}/FIREFOX_WEB_AUTH_CLIENT_SECRET`,
+            },
+            {
+              name: 'FXA_OAUTH_URL',
+              valueFrom: `arn:aws:ssm:${region.name}:${caller.accountId}:parameter/Web/${config.environment}/FIREFOX_AUTH_OAUTH_URL`,
+            },
           ],
         },
       ],

diff --git a/lambdas/account-data-deleter-events/src/config.ts b/lambdas/account-data-deleter-events/src/config.ts
@@ -8,6 +8,7 @@ export const config = {
     'https://account-data-deleter-api.getpocket.dev',
   queueDeletePath: '/queueDelete',
   stripeDeletePath: '/stripeDelete',
+  fxaRevokePath: '/revokeFxa',
   sentry: {
     // these values are inserted into the environment in
     // .aws/src/.ts

diff --git a/lambdas/account-data-deleter-events/src/handlers/accountDelete/index.spec.ts b/lambdas/account-data-deleter-events/src/handlers/accountDelete/index.spec.ts
@@ -17,6 +17,10 @@ describe('accountDelete handler', () => {
     }),
   };
 
+  beforeEach(() => {
+    nock(config.endpoint).post(config.fxaRevokePath).reply(200);
+  });
+
   afterEach(() => {
     nock.cleanAll();
     jest.restoreAllMocks();

diff --git a/lambdas/account-data-deleter-events/src/handlers/accountDelete/index.ts b/lambdas/account-data-deleter-events/src/handlers/accountDelete/index.ts
@@ -2,6 +2,7 @@ import { AccountDeleteEvent } from '../../schemas/accountDeleteEvent.ts';
 import {
   callQueueDeleteEndpoint,
   callStripeDeleteEndpoint,
+  callFxARevokeEndpoint,
 } from './postRequest.ts';
 import { SQSRecord } from 'aws-lambda';
 import { AggregateError } from '../../errors/AggregateError.ts';
@@ -47,6 +48,7 @@ export function validatePostBody(
  *       rows from Pocket's internal database
  *   * stripe API - deletes stripe customer data (and internal
  *       stripe-related data in database)
+ *   * FxA Auth API - revokes access tokens from FxA (Mozilla Accounts)
  * @param record the event forwarded from event bridge via SQS
  * @throws Error if the record body does not conform to expected schema
  * @throws AggregateError if any errors encountered making
@@ -59,10 +61,12 @@ export async function accountDeleteHandler(record: SQSRecord): Promise<void> {
   const postBody = validatePostBody(message);
   const queueRes = await callQueueDeleteEndpoint(postBody);
   const stripeRes = await callStripeDeleteEndpoint(postBody);
+  const fxaRes = await callFxARevokeEndpoint(postBody);
   const errors: Error[] = [];
   for await (const { endpoint, res } of [
     { endpoint: 'queueDelete', res: queueRes },
     { endpoint: 'stripeDelete', res: stripeRes },
+    { endpoint: 'fxaDelete', res: fxaRes },
   ]) {
     if (!res.ok) {
       const data = await res.json();

diff --git a/lambdas/account-data-deleter-events/src/handlers/accountDelete/postRequest.ts b/lambdas/account-data-deleter-events/src/handlers/accountDelete/postRequest.ts
@@ -51,3 +51,12 @@ export async function callQueueDeleteEndpoint(body: any): Promise<any> {
 export async function callStripeDeleteEndpoint(body: any): Promise<any> {
   return postRequest(body, config.stripeDeletePath, config.endpoint);
 }
+
+/**
+ * Revoke FxA Access Token when a user deletes their account
+ * @param body
+ * @returns
+ */
+export async function callFxARevokeEndpoint(body: any): Promise<any> {
+  return postRequest(body, config.fxaRevokePath, config.endpoint);
+}
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
diff --git a/servers/account-data-deleter/package.json b/servers/account-data-deleter/package.json
@@ -40,6 +40,7 @@
     "csv-stringify": "^6.5.1",
     "express": "4.21.2",
     "express-validator": "^7.1.0",
+    "fetch-retry": "^5.0.6",
     "knex": "3.1.0",
     "lodash": "4.17.21",
     "mysql2": "3.12.0",

diff --git a/servers/account-data-deleter/src/config/index.ts b/servers/account-data-deleter/src/config/index.ts
@@ -79,6 +79,12 @@ export const config = {
     apiVersion: '2024-06-20' as const,
     productId: 7,
   },
+  fxa: {
+    clientId: process.env.FXA_CLIENT_ID || 'somefakeclientid',
+    secret: process.env.FXA_CLIENT_SECRET || 'somefakesecret',
+    oauthEndpoint: process.env.FXA_OAUTH_URL || 'https://localhost/',
+    version: 'v1',
+  },
   database: {
     // contains tables for user, list, tags, annotations, etc.
     read: {

diff --git a/servers/account-data-deleter/src/dataService/FxaRevoker.spec.ts b/servers/account-data-deleter/src/dataService/FxaRevoker.spec.ts
@@ -0,0 +1,52 @@
+import { FxaRevoker } from './FxaRevoker';
+
+describe('FxARevoker', () => {
+  afterEach(() => {
+    jest.restoreAllMocks();
+  });
+  describe('with an access token', () => {
+    beforeEach(() => {
+      jest
+        .spyOn(FxaRevoker.prototype, 'fetchAccessToken')
+        .mockResolvedValue('accessme123');
+    });
+    it('returns false if revoking throws an error', async () => {
+      jest
+        .spyOn(FxaRevoker.prototype, 'requestRevokeToken')
+        .mockRejectedValueOnce(new Error('error fetching'));
+      const res = await new FxaRevoker('abc123').revokeToken();
+      expect(res).toBeFalse();
+    });
+    it('returns false if revoking response is not ok', async () => {
+      jest
+        .spyOn(FxaRevoker.prototype, 'requestRevokeToken')
+        .mockResolvedValueOnce(new Response(null, { status: 500 }));
+      const res = await new FxaRevoker('abc123').revokeToken();
+      expect(res).toBeFalse();
+    });
+    it('returns false if db delete method throws error', async () => {
+      jest
+        .spyOn(FxaRevoker.prototype, 'deleteAuthRecord')
+        .mockRejectedValueOnce(new Error('error deleting'));
+      const res = await new FxaRevoker('abc123').revokeToken();
+      expect(res).toBeFalse();
+    });
+    it('returns true if process does not error', async () => {
+      jest
+        .spyOn(FxaRevoker.prototype, 'requestRevokeToken')
+        .mockResolvedValueOnce(new Response(null, { status: 200 }));
+      jest
+        .spyOn(FxaRevoker.prototype, 'deleteAuthRecord')
+        .mockResolvedValueOnce(1);
+      const res = await new FxaRevoker('abc123').revokeToken();
+      expect(res).toBeTrue();
+    });
+  });
+  it('returns true if no FxA tokens exist', async () => {
+    jest
+      .spyOn(FxaRevoker.prototype, 'fetchAccessToken')
+      .mockResolvedValueOnce(undefined);
+    const res = await new FxaRevoker('abc123').revokeToken();
+    expect(res).toBeTrue();
+  });
+});