fix(database): paginating/sorting on findMany twice, causing data issues

ConduitPlatform · Jun 29, 2023 · a5bf9da · a5bf9da
1 parent 1b728f6
commit a5bf9da
Show file tree

Hide file tree

Showing 3 changed files with 90 additions and 50 deletions.
diff --git a/modules/database/src/adapters/SchemaAdapter.ts b/modules/database/src/adapters/SchemaAdapter.ts
@@ -135,42 +135,82 @@ export abstract class SchemaAdapter<T> {
     many: boolean = false,
     userId?: string,
     scope?: string,
-    skip?: number,
-    limit?: number,
   ) {
-    if (!this.originalSchema.modelOptions.conduit?.authorization?.enabled)
+    if (
+      !this.originalSchema.modelOptions.conduit?.authorization?.enabled ||
+      (isNil(userId) && isNil(scope))
+    )
       return parsedQuery;
-    if (!isNil(userId) || !isNil(scope)) {
-      const view = await this.permissionCheck(operation, userId, scope);
-      if (!view) return parsedQuery;
-      if (many) {
-        const docs = await view.findMany(parsedQuery, {
-          select: '_id',
-          skip,
-          limit,
-          userId: undefined,
-          scope: undefined,
-        });
-        if (isNil(docs)) {
-          return null;
-        }
-        if (this.adapter.getDatabaseType() === 'MongoDB') {
-          return { _id: { $in: docs.map((doc: any) => doc._id) } };
-        } else {
-          return { _id: { [Op.in]: docs.map((doc: any) => doc._id) } };
-        }
+    const view = await this.permissionCheck(operation, userId, scope);
+    if (!view) return parsedQuery;
+    if (many) {
+      const docs = await view.findMany(parsedQuery, {
+        select: '_id',
+        userId: undefined,
+        scope: undefined,
+      });
+      if (isNil(docs)) {
+        return null;
+      }
+      if (this.adapter.getDatabaseType() === 'MongoDB') {
+        return { _id: { $in: docs.map((doc: any) => doc._id) } };
       } else {
-        const doc = await view.findOne(parsedQuery, {
-          userId: undefined,
-          scope: undefined,
-        });
-        if (isNil(doc)) {
-          throw new GrpcError(status.PERMISSION_DENIED, 'Access denied');
-        }
-        return { _id: doc._id };
+        return { _id: { [Op.in]: docs.map((doc: any) => doc._id) } };
       }
+    } else {
+      const doc = await view.findOne(parsedQuery, {
+        userId: undefined,
+        scope: undefined,
+      });
+      if (isNil(doc)) {
+        throw new GrpcError(status.PERMISSION_DENIED, 'Access denied');
+      }
+      return { _id: doc._id };
+    }
+  }
+
+  async getPaginatedAuthorizedQuery(
+    operation: string,
+    parsedQuery: Indexable,
+    userId?: string,
+    scope?: string,
+    skip?: number,
+    limit?: number,
+    sort?: Indexable,
+  ) {
+    if (
+      !this.originalSchema.modelOptions.conduit?.authorization?.enabled ||
+      (isNil(userId) && isNil(scope))
+    )
+      return { parsedQuery, modified: false };
+    const view = await this.permissionCheck(operation, userId, scope);
+    if (!view) return { parsedQuery, modified: false };
+    const docs = await view.findMany(parsedQuery, {
+      select: '_id',
+      skip,
+      limit,
+      sort,
+      userId: undefined,
+      scope: undefined,
+    });
+    if (isNil(docs)) {
+      return { parsedQuery: null, modified: false };
+    }
+    if (this.adapter.getDatabaseType() === 'MongoDB') {
+      return {
+        parsedQuery: {
+          _id: {
+            $in: docs.map((doc: any) => doc._id),
+          },
+        },
+        modified: true,
+      };
+    } else {
+      return {
+        parsedQuery: { _id: { [Op.in]: docs.map((doc: any) => doc._id) } },
+        modified: true,
+      };
     }
-    return parsedQuery;
   }
 
   async addPermissionToData(

diff --git a/modules/database/src/adapters/mongoose-adapter/MongooseSchema.ts b/modules/database/src/adapters/mongoose-adapter/MongooseSchema.ts
@@ -260,30 +260,29 @@ export class MongooseSchema extends SchemaAdapter<Model<any>> {
       scope?: string;
     },
   ) {
-    let parsedQuery: Indexable | null = parseQuery(this.parseStringToQuery(query));
-    parsedQuery = await this.getAuthorizedQuery(
+    let { parsedQuery, modified } = await this.getPaginatedAuthorizedQuery(
       'read',
-      parsedQuery,
-      true,
+      parseQuery(this.parseStringToQuery(query)),
       options?.userId,
       options?.scope,
       options?.skip,
       options?.limit,
+      options?.sort,
     );
     if (isNil(parsedQuery)) {
       return [];
     }
     let finalQuery = this.model.find(parsedQuery, options?.select);
-    if (!isNil(options?.skip)) {
+    if (!isNil(options?.skip) && !modified) {
       finalQuery = finalQuery.skip(options?.skip!);
     }
-    if (!isNil(options?.limit)) {
+    if (!isNil(options?.limit) && !modified) {
       finalQuery = finalQuery.limit(options?.limit!);
     }
     if (!isNil(options?.populate)) {
       finalQuery = this.populate(finalQuery, options?.populate ?? []);
     }
-    if (!isNil(options?.sort)) {
+    if (!isNil(options?.sort) && !modified) {
       finalQuery = finalQuery.sort(this.parseSort(options?.sort));
     }
     return finalQuery.lean().exec();

diff --git a/modules/database/src/adapters/sequelize-adapter/SequelizeSchema.ts b/modules/database/src/adapters/sequelize-adapter/SequelizeSchema.ts
@@ -377,15 +377,16 @@ export class SequelizeSchema extends SchemaAdapter<ModelStatic<any>> {
         select: options?.select,
       },
     );
-    const parsedFilter = await this.getAuthorizedQuery(
-      'read',
-      filter,
-      true,
-      options?.userId,
-      options?.scope,
-      options?.skip,
-      options?.limit,
-    );
+    const { parsedQuery: parsedFilter, modified } =
+      await this.getPaginatedAuthorizedQuery(
+        'read',
+        filter,
+        options?.userId,
+        options?.scope,
+        options?.skip,
+        options?.limit,
+        options?.sort,
+      );
     if (isNil(parsedFilter)) {
       return [];
     }
@@ -399,13 +400,13 @@ export class SequelizeSchema extends SchemaAdapter<ModelStatic<any>> {
         options?.populate || [],
       ),
     };
-    if (!isNil(options?.skip)) {
+    if (!isNil(options?.skip) && !modified) {
       findOptions.offset = options?.skip;
     }
-    if (!isNil(options?.limit)) {
+    if (!isNil(options?.limit) && !modified) {
       findOptions.limit = options?.limit;
     }
-    if (!isNil(options?.sort)) {
+    if (!isNil(options?.sort) && !modified) {
       findOptions.order = this.parseSort(options?.sort);
     }