Updated walletAddressKey deletion migration, removed unnecessary test…

… for walletAddressKey service
interledger · Dec 5, 2024 · 2b2b34e · 2b2b34e
1 parent d166800
commit 2b2b34e
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 46 deletions.
diff --git a/packages/backend/migrations/20241119125038_add_unique_constraint_to_kid.js b/packages/backend/migrations/20241119125038_add_unique_constraint_to_kid.js
@@ -7,22 +7,35 @@ exports.up = async function (knex) {
     .raw(
       // Keep only one active walletAddressKey (the most recent one)
       `DELETE FROM "walletAddressKeys" w
-        WHERE revoked = false
-        AND "createdAt" <> (
-          SELECT MAX("createdAt")
+       WHERE revoked = false
+       AND w."createdAt" = (
+          SELECT "createdAt"
           FROM "walletAddressKeys"
           WHERE revoked = false
-          AND "walletAddressId" = w."walletAddressId"
-          GROUP BY "walletAddressId"
-);
+            AND kid = w.kid
+          ORDER BY "createdAt" DESC
+          LIMIT 1
+       )
+       /* 
+        if there are multiple keys that have the most recent "createdAt",
+        we keep just one of them and delete the rest
+       */
+       AND w.id <> (
+          SELECT id
+          FROM "walletAddressKeys"
+          WHERE revoked = false
+            AND kid = w.kid
+          ORDER BY "createdAt" DESC
+          LIMIT 1 OFFSET 1
+       );
       `
     )
     .then(() => {
       return knex.raw(`
-      CREATE UNIQUE INDEX "wallet_address_keys_revoked_false_idx"
-      ON "walletAddressKeys" ("walletAddressId", kid)
-      WHERE revoked = false;
-  `)
+        CREATE UNIQUE INDEX "wallet_address_keys_revoked_false_idx"
+        ON "walletAddressKeys" ("walletAddressId", kid)
+        WHERE revoked = false;
+      `)
     })
 }
 

diff --git a/packages/backend/src/open_payments/wallet_address/key/service.test.ts b/packages/backend/src/open_payments/wallet_address/key/service.test.ts
@@ -65,6 +65,27 @@ describe('Wallet Address Key Service', (): void => {
         WalletAddressKeyError.DuplicateKey
       )
     })
+
+    test('Creates a new key unrevoked', async (): Promise<void> => {
+      const keyOption = {
+        walletAddressId: walletAddress.id,
+        jwk: TEST_KEY
+      }
+
+      const key = await walletAddressKeyService.create(keyOption)
+      assert.ok(!isWalletAddressKeyError(key))
+      await walletAddressKeyService.revoke(key.id)
+
+      const unrevokedKey = await walletAddressKeyService.create(keyOption)
+
+      assert.ok(!isWalletAddressKeyError(key))
+
+      expect(unrevokedKey).toMatchObject({
+        jwk: key.jwk,
+        walletAddressId: key.walletAddressId,
+        revoked: false
+      })
+    })
   })
 
   describe('Fetch Wallet Address Keys', (): void => {
@@ -154,40 +175,4 @@ describe('Wallet Address Key Service', (): void => {
       )
     })
   })
-
-  describe('Unrevoke Wallet Address Keys', (): void => {
-    test('Creates a new key unrevoked', async (): Promise<void> => {
-      const keyOption = {
-        walletAddressId: walletAddress.id,
-        jwk: TEST_KEY
-      }
-
-      const key = await walletAddressKeyService.create(keyOption)
-      assert.ok(!isWalletAddressKeyError(key))
-      await walletAddressKeyService.revoke(key.id)
-
-      const unrevokedKey = await walletAddressKeyService.create(keyOption)
-
-      assert.ok(!isWalletAddressKeyError(key))
-
-      expect(unrevokedKey).toMatchObject({
-        jwk: key.jwk,
-        walletAddressId: key.walletAddressId,
-        revoked: false
-      })
-    })
-
-    test('Returns DuplicateKey when the key already exists and it is unrevoked', async (): Promise<void> => {
-      const keyOption = {
-        walletAddressId: walletAddress.id,
-        jwk: TEST_KEY
-      }
-
-      const key = await walletAddressKeyService.create(keyOption)
-      assert.ok(!isWalletAddressKeyError(key))
-
-      const unrevokedKey = await walletAddressKeyService.create(keyOption)
-      assert.ok(isWalletAddressKeyError(unrevokedKey))
-    })
-  })
 })