From 7eedb1e57a0c5b5b346b7fb4ee6bc6fc8e1fca5f Mon Sep 17 00:00:00 2001
From: Max Kurapov <max@interledger.org>
Date: Tue, 26 Sep 2023 18:56:39 +0200
Subject: [PATCH] fix: grant related fixes (#1960)

* fix: fix grant continuation fetch

* fix: move finalizationReason addition to separate migration

* chore: lint

* feat: check for column existence before migrating

* feat: update migration to use enum update
---
 ...25183132_add_finalization_reason_grants.js | 54 +++++++++++++++++++
 packages/auth/src/grant/service.test.ts       | 48 +++++++++++++++++
 packages/auth/src/grant/service.ts            |  7 +--
 3 files changed, 106 insertions(+), 3 deletions(-)
 create mode 100644 packages/auth/migrations/20230925183132_add_finalization_reason_grants.js

diff --git a/packages/auth/migrations/20230925183132_add_finalization_reason_grants.js b/packages/auth/migrations/20230925183132_add_finalization_reason_grants.js
new file mode 100644
index 0000000000..ffab8303ce
--- /dev/null
+++ b/packages/auth/migrations/20230925183132_add_finalization_reason_grants.js
@@ -0,0 +1,54 @@
+const grantsTable = 'grants'
+const finalizationReasonColumn = 'finalizationReason'
+const finalzationReasonType = 'finalization_reason_type'
+const finalizationReasonList = ['ISSUED', 'REVOKED', 'REJECTED']
+
+const toPsqlList = (list) => `(${list.map((el) => `'${el}'`).join(', ')})`
+
+/**
+ * @param { import("knex").Knex } knex
+ * @returns { Promise<void> }
+ */
+exports.up = function (knex) {
+  return knex.schema
+    .hasColumn(grantsTable, finalizationReasonColumn)
+    .then((exists) => {
+      if (!exists) {
+        return knex.schema.alterTable(grantsTable, (table) => {
+          table.enum(finalizationReasonColumn, finalizationReasonList, {
+            useNative: true,
+            enumName: finalzationReasonType
+          })
+        })
+      }
+
+      return knex.schema.raw(
+        `CREATE TYPE ${finalzationReasonType} AS ENUM ${toPsqlList(
+          finalizationReasonList
+        )};
+        ALTER TABLE "${grantsTable}" ALTER COLUMN "${finalizationReasonColumn}" TYPE ${finalzationReasonType} USING "${finalizationReasonColumn}"::${finalzationReasonType}`
+      )
+    })
+}
+
+/**
+ * @param { import("knex").Knex } knex
+ * @returns { Promise<void> }
+ */
+exports.down = function (knex) {
+  return knex.schema
+    .hasColumn(grantsTable, finalizationReasonColumn)
+    .then((exists) => {
+      if (exists) {
+        return knex.schema
+          .alterTable(grantsTable, (table) => {
+            table.dropColumn(finalizationReasonColumn)
+          })
+          .then(() => {
+            return knex.schema.raw(
+              `DROP TYPE IF EXISTS ${finalzationReasonType}`
+            )
+          })
+      }
+    })
+}
diff --git a/packages/auth/src/grant/service.test.ts b/packages/auth/src/grant/service.test.ts
index 388d825f6d..3361f7b632 100644
--- a/packages/auth/src/grant/service.test.ts
+++ b/packages/auth/src/grant/service.test.ts
@@ -229,6 +229,54 @@ describe('Grant Service', (): void => {
       expect(fetchedGrant?.continueId).toEqual(continueId)
       expect(fetchedGrant?.continueToken).toEqual(continueToken)
     })
+
+    test('properly fetches grant by continuation information with multiple existing grants', async (): Promise<void> => {
+      const grantRequest: GrantRequest = {
+        ...BASE_GRANT_REQUEST,
+        access_token: {
+          access: [
+            {
+              ...BASE_GRANT_ACCESS,
+              type: AccessType.IncomingPayment
+            }
+          ]
+        },
+        interact: undefined
+      }
+
+      const grant1 = await grantService.create(grantRequest)
+      await grant1
+        .$query()
+        .patch({ finalizationReason: GrantFinalization.Issued })
+
+      const grant2 = await grantService.create(grantRequest)
+      const grant3 = await grantService.create(grantRequest)
+      await grant3
+        .$query()
+        .patch({ finalizationReason: GrantFinalization.Revoked })
+
+      const fetchedGrant1 = await grantService.getByContinue(
+        grant1.continueId,
+        grant1.continueToken
+      )
+
+      expect(fetchedGrant1?.id).toEqual(grant1.id)
+      expect(fetchedGrant1?.continueId).toEqual(grant1.continueId)
+      expect(fetchedGrant1?.continueToken).toEqual(grant1.continueToken)
+
+      const fetchedGrant2 = await grantService.getByContinue(
+        grant2.continueId,
+        grant2.continueToken
+      )
+
+      expect(fetchedGrant2?.id).toEqual(grant2.id)
+      expect(fetchedGrant2?.continueId).toEqual(grant2.continueId)
+      expect(fetchedGrant2?.continueToken).toEqual(grant2.continueToken)
+
+      await expect(
+        grantService.getByContinue(grant3.continueId, grant3.continueToken)
+      ).resolves.toBeUndefined()
+    })
   })
 
   describe('getByIdWithAccess', (): void => {
diff --git a/packages/auth/src/grant/service.ts b/packages/auth/src/grant/service.ts
index 77fffb2f51..24f525c42a 100644
--- a/packages/auth/src/grant/service.ts
+++ b/packages/auth/src/grant/service.ts
@@ -258,9 +258,10 @@ async function getByContinue(
     .withGraphFetched('interaction')
 
   if (!includeRevoked) {
-    queryBuilder
-      .whereNull('finalizationReason')
-      .orWhereNot('finalizationReason', GrantFinalization.Revoked)
+    queryBuilder.andWhere((queryBuilder) => {
+      queryBuilder.whereNull('finalizationReason')
+      queryBuilder.orWhereNot('finalizationReason', GrantFinalization.Revoked)
+    })
   }
 
   const grant = await queryBuilder