feat(devices): Add query param to allow client to filter idle devices

packages/db-migrations/databases/fxa/patches/patch-130-131.sql

@@ -0,0 +1,93 @@
+SET NAMES utf8mb4 COLLATE utf8mb4_bin;
+
+CALL assertPatchLevel('130');
+
+-- This migration adds a limit to the devices and sessions queries. In
+-- production we have some clients that have an unreasonable amount of these.
+-- This migration bounds the queries and should improve performance.
+
+CREATE PROCEDURE `accountDevices_17` (
+  IN `uidArg` BINARY(16),
+  IN `limitArg` INT
+)
+BEGIN
+  SELECT 
+    d.uid,
+    d.id,
+    s.tokenId AS sessionTokenId, -- Ensure we only return valid sessionToken ids
+    d.refreshTokenId,
+    d.nameUtf8 AS name,
+    d.type,
+    d.createdAt,
+    d.callbackURL,
+    d.callbackPublicKey,
+    d.callbackAuthKey,
+    d.callbackIsExpired,
+    s.uaBrowser,
+    s.uaBrowserVersion,
+    s.uaOS,
+    s.uaOSVersion,
+    s.uaDeviceType,
+    s.uaFormFactor,
+    s.lastAccessTime,
+    ci.commandName,
+    dc.commandData
+  FROM devices AS d
+  -- Left join, because it might not have a sessionToken.
+  LEFT JOIN sessionTokens AS s
+    ON d.sessionTokenId = s.tokenId
+  LEFT JOIN (
+    deviceCommands AS dc FORCE INDEX (PRIMARY)
+    INNER JOIN deviceCommandIdentifiers AS ci FORCE INDEX (PRIMARY)
+      ON ci.commandId = dc.commandId
+  ) ON (dc.uid = d.uid AND dc.deviceId = d.id)
+  WHERE d.uid = uidArg
+  -- We don't want to return 'zombie' device records where the sessionToken
+  -- no longer exists in the sessionTokens table.
+  AND (s.tokenId IS NOT NULL OR d.refreshTokenId IS NOT NULL)
+  -- For easy flattening, ensure rows are ordered by device id.
+  ORDER BY 1, 2
+  LIMIT limitArg;
+END;
+
+CREATE PROCEDURE `sessions_12` (
+  IN `uidArg` BINARY(16),
+  IN `limitArg` INT
+)
+BEGIN
+  SELECT
+    t.tokenId,
+    t.uid,
+    t.createdAt,
+    t.uaBrowser,
+    t.uaBrowserVersion,
+    t.uaOS,
+    t.uaOSVersion,
+    t.uaDeviceType,
+    t.uaFormFactor,
+    t.lastAccessTime,
+    COALESCE(t.authAt, t.createdAt) AS authAt,
+    d.id AS deviceId,
+    d.nameUtf8 AS deviceName,
+    d.type AS deviceType,
+    d.createdAt AS deviceCreatedAt,
+    d.callbackURL AS deviceCallbackURL,
+    d.callbackPublicKey AS deviceCallbackPublicKey,
+    d.callbackAuthKey AS deviceCallbackAuthKey,
+    d.callbackIsExpired AS deviceCallbackIsExpired,
+    ci.commandName AS deviceCommandName,
+    dc.commandData AS deviceCommandData
+  FROM sessionTokens AS t
+  LEFT JOIN devices AS d
+    ON (t.tokenId = d.sessionTokenId AND t.uid = d.uid)
+  LEFT JOIN (
+    deviceCommands AS dc FORCE INDEX (PRIMARY)
+    INNER JOIN deviceCommandIdentifiers AS ci FORCE INDEX (PRIMARY)
+      ON ci.commandId = dc.commandId
+  ) ON (dc.uid = d.uid AND dc.deviceId = d.id)
+  WHERE t.uid = uidArg
+  ORDER BY 1
+  LIMIT limitArg;
+END;
+
+UPDATE dbMetadata SET value = '131' WHERE name = 'schema-patch-level';

packages/db-migrations/databases/fxa/patches/patch-131-130.sql

@@ -0,0 +1,6 @@
+-- SET NAMES utf8mb4 COLLATE utf8mb4_bin;
+
+-- DROP PROCEDURE `accountDevices_17`;
+-- DROP PROCEDURE `sessions_12`;
+
+-- UPDATE dbMetadata SET value = '130' WHERE name = 'schema-patch-level';

packages/db-migrations/databases/fxa/target-patch.json

@@ -1,3 +1,3 @@
 {
-  "level": 130
+  "level": 131
 }

packages/fxa-auth-server/docs/swagger/devices-and-sessions-api.ts

@@ -26,6 +26,8 @@ const ACCOUNT_ATTACHED_CLIENTS_GET = {
       - \`deviceId\`: The id of the client's device record, if it has registered one.
 
       These identifiers can be passed to [**/account/attached_client/destroy**](https://github.com/mozilla/fxa/blob/main/packages/fxa-auth-server/docs/api.md#post-accountattached_clientdestroy) in order to disconnect the client.
+      
+      This endpoint returns a maximum 500 last used devices and sessions.
     `,
   ],
 };

packages/fxa-auth-server/docs/swagger/shared/descriptions.ts

@@ -40,6 +40,7 @@ const DESCRIPTIONS = {
   excluded:
     'Array of device ids to exclude from the notification. Ignored unless `to:"all"` is specified.',
   expiresIn: 'The number of seconds until the access token will expire.',
+  filterIdleDevicesTimestamp: 'Filter device list to only show devices active since UTC timestamp.',
   grantType: dedent`
     The type of grant flow being used. If not specified, it will default to fxa-credentials unless a code parameter is provided, in which case it will default to authorization_code. The value of this parameter determines which other parameters will be expected in the request body, as follows:
     - When \`grant_type=authorization_code\`:

packages/fxa-auth-server/lib/routes/attached-clients.js

@@ -15,6 +15,7 @@ const DEVICES_AND_SESSIONS_DOC =
   require('../../docs/swagger/devices-and-sessions-api').default;
 
 const { ConnectedServicesFactory } = require('fxa-shared/connected-services');
+const DESCRIPTIONS = require('../../docs/swagger/shared/descriptions').default;
 
 module.exports = (log, db, devices, clientUtils) => {
   return [
@@ -26,6 +27,11 @@ module.exports = (log, db, devices, clientUtils) => {
         auth: {
           strategy: 'sessionToken',
         },
+        validate: {
+          query: isA.object({
+            filterIdleDevicesTimestamp: isA.number().description(DESCRIPTIONS.filterIdleDevicesTimestamp).optional()
+          }),
+        },
         response: {
           schema: isA.array().items(
             isA.object({
@@ -80,7 +86,21 @@ module.exports = (log, db, devices, clientUtils) => {
             clientUtils.formatLocation(...args);
           },
           deviceList: async () => {
-            return await request.app.devices;
+            let devices = await request.app.devices;
+            
+            // To help reduce duplicate devices and help improve send tab 
+            // performance a client can request to filter device last access 
+            // time by a specified number of days. For reference, Sync currently
+            // considers devices that have been accessed in the last 21 days to 
+            // be active.
+            const idleDeviceTimestamp = request.query.filterIdleDevicesTimestamp;
+            if (idleDeviceTimestamp) {
+              devices = devices.filter((device) => {
+                return device.lastAccessTime > idleDeviceTimestamp;
+              });
+            }
+            
+            return devices;
           },
           oauthClients: async () => {
             return await authorizedClients.list(request.auth.credentials.uid);

packages/fxa-auth-server/test/local/routes/attached-clients.js

@@ -375,6 +375,52 @@ describe('/account/attached_clients', () => {
       os: null,
     });
   });
+
+  it('filters out idle devices', async () => {
+    const now = Date.now();
+    const FIVE_DAYS_AGO = now - (1000 * 60 * 60 * 24 * 5);
+    const ONE_DAY_AGO = now - (1000 * 60 * 60 * 24);
+    
+    request.query.filterIdleDevicesTimestamp = ONE_DAY_AGO; // Filter for devices active in the last day
+    const DEVICES = [
+      {
+        id: newId(),
+        sessionTokenId: newId(),
+        lastAccessTime: now,
+        createdAt: now,
+      },
+      {
+        id: newId(),
+        sessionTokenId: newId(),
+        lastAccessTime: FIVE_DAYS_AGO,
+        createdAt: FIVE_DAYS_AGO,
+      },
+    ];
+    const SESSIONS = [
+      {
+        id: DEVICES[0].sessionTokenId,
+        createdAt: now,
+        lastAccessTime: now,
+        location: { country: 'Germany' },
+      }
+    ];
+    const OAUTH_CLIENTS = [];
+
+    request.app.devices = (async () => {
+      return DEVICES;
+    })();
+    mockAuthorizedClients.list = sinon.spy(async () => {
+      return OAUTH_CLIENTS;
+    });
+    db.sessions = sinon.spy(async () => {
+      return SESSIONS;
+    });
+
+    request.auth.credentials.id = SESSIONS[0].id;
+    const result = await route(request);
+
+    assert.equal(result.length, 1);
+  });
 });
 
 describe('/account/attached_client/destroy', () => {

packages/fxa-shared/connected-services/accessors.ts

@@ -114,7 +114,7 @@ export interface IConnectedServicesDbStore {
 }
 
 /**
- * Wrapper around around database operations pertaining to connected services.
+ * Wrapper around database operations pertaining to connected services.
  */
 export class ConnectedServicesDb {
   constructor(

packages/fxa-shared/db/models/auth/base-auth.ts

@@ -9,7 +9,7 @@ import { Knex } from 'knex';
 export enum Proc {
   AccountRecord = 'accountRecord_9',
   AccountResetToken = 'accountResetToken_1',
-  AccountDevices = 'accountDevices_16',
+  AccountDevices = 'accountDevices_17',
   ConsumeRecoveryCode = 'consumeRecoveryCode_3',
   ConsumeSigninCode = 'consumeSigninCode_4',
   ConsumeUnblockCode = 'consumeUnblockCode_4',
@@ -53,7 +53,7 @@ export enum Proc {
   ResetAccount = 'resetAccount_16',
   ResetAccountTokens = 'resetAccountTokens_1',
   SessionWithDevice = 'sessionWithDevice_19',
-  Sessions = 'sessions_11',
+  Sessions = 'sessions_12',
   SetPrimaryEmail = 'setPrimaryEmail_6',
   TotpToken = 'totpToken_2',
   UpdateDevice = 'updateDevice_6',

packages/fxa-shared/db/models/auth/device.ts

@@ -209,10 +209,11 @@ export class Device extends BaseAuthModel {
     ).map((row) => this.fromDatabaseJson(row));
   }
 
-  static async findByUid(uid: string) {
+  static async findByUid(uid: string, limit = 500) {
     const { rows } = await this.callProcedure(
       Proc.AccountDevices,
-      uuidTransformer.to(uid)
+      uuidTransformer.to(uid),
+      limit
     );
     return this.fromRows(rows);
   }

packages/fxa-shared/db/models/auth/session-token.ts

@@ -261,10 +261,11 @@ export class SessionToken extends BaseToken {
     return notExpired(token) ? token : null;
   }
 
-  static async findByUid(uid: string): Promise<SessionToken[]> {
+  static async findByUid(uid: string, limit= 500): Promise<SessionToken[]> {
     const { rows } = await this.callProcedure(
       Proc.Sessions,
-      uuidTransformer.to(uid)
+      uuidTransformer.to(uid),
+      limit,
     );
     if (!rows.length) {
       return [];

packages/fxa-shared/test/db/models/auth/sp_accountDevices.sql

@@ -1,8 +1,9 @@
-CREATE PROCEDURE `accountDevices_16` (
+CREATE PROCEDURE `accountDevices_17` (
-  IN `uidArg` BINARY(16)
+  IN `uidArg` BINARY(16),
+  IN `limitArg` INT
 )
 BEGIN
-  SELECT
+  SELECT 
     d.uid,
     d.id,
     s.tokenId AS sessionTokenId, -- Ensure we only return valid sessionToken ids
@@ -37,5 +38,6 @@ BEGIN
   -- no longer exists in the sessionTokens table.
   AND (s.tokenId IS NOT NULL OR d.refreshTokenId IS NOT NULL)
   -- For easy flattening, ensure rows are ordered by device id.
-  ORDER BY 1, 2;
+  ORDER BY 1, 2
-END;
+  LIMIT limitArg;
+END;