Update: Migrate course access check to query-time hook

lib/AdaptFrameworkModule.js

@@ -4,7 +4,7 @@ import AdaptFrameworkImport from './AdaptFrameworkImport.js'
 import fs from 'node:fs/promises'
 import { getHandler, postHandler, importHandler, postUpdateHandler, getUpdateHandler } from './handlers.js'
 import { loadRouteConfig, registerRoutes } from 'adapt-authoring-server'
-import { runCliCommand, readFrameworkPluginVersions, migrateExistingCourses, computePluginHash, prebuildCache } from './utils.js'
+import { applyContentAccessFilter, runCliCommand, readFrameworkPluginVersions, migrateExistingCourses, computePluginHash, prebuildCache } from './utils.js'
 import BuildCache from './BuildCache.js'
 import path from 'node:path'
 import semver from 'semver'
@@ -79,6 +79,7 @@ class AdaptFrameworkModule extends AbstractModule {
     this._targetFrameworkVersion = meta.framework?.targetVersion
 
     this.app.waitForModule('content').then(content => {
+      content.accessQueryHook.tap(this.onContentAccessQueryHook.bind(this))
       content.accessCheckHook.tap(this.checkContentAccess.bind(this))
     })
 
@@ -319,6 +320,18 @@ class AdaptFrameworkModule extends AbstractModule {
     schemas.forEach(s => jsonschema.registerSchema(s))
   }
 
+  /**
+   * Merges ownership/sharing access clauses into the content query when listing courses,
+   * so the database returns only courses the user can see. Non-course queries fall through
+   * to the per-item `checkContentAccess` safety net (the parent course's `_id` is not in
+   * scope at this stage).
+   * @param {external:ExpressRequest} req
+   */
+  async onContentAccessQueryHook (req) {
+    if (req.apiData.query._type !== 'course') return
+    applyContentAccessFilter(req.apiData.query, req.auth.user._id.toString())
+  }
+
   /**
    * Checks whether the request user should be given access to the content they're requesting
    * @param {external:ExpressRequest} req

lib/utils.js

@@ -1,3 +1,4 @@
+export { applyContentAccessFilter } from './utils/applyContentAccessFilter.js'
 export { inferBuildAction } from './utils/inferBuildAction.js'
 export { getPluginUpdateStatus } from './utils/getPluginUpdateStatus.js'
 export { getImportContentCounts } from './utils/getImportContentCounts.js'

lib/utils/applyContentAccessFilter.js

@@ -0,0 +1,26 @@
+/**
+ * Mutates a mongo content query so it only matches courses the given user can access:
+ * owned by them, marked public via `_isShared`, or in `_shareWithUsers`. Combines safely
+ * with an existing `$or` (e.g. from search) by lifting both into `$and`.
+ * @param {object} query The mongo query object to mutate
+ * @param {string} userId The user's `_id` (already coerced to string)
+ * @memberof adaptframework
+ */
+export function applyContentAccessFilter (query, userId) {
+  if (!userId) return
+  const clauses = [
+    { createdBy: userId },
+    { _isShared: true },
+    { _shareWithUsers: userId }
+  ]
+  if (query.$or) {
+    query.$and = [
+      ...(query.$and ?? []),
+      { $or: query.$or },
+      { $or: clauses }
+    ]
+    delete query.$or
+  } else {
+    query.$or = clauses
+  }
+}

tests/utils-applyContentAccessFilter.spec.js

@@ -0,0 +1,52 @@
+import { describe, it } from 'node:test'
+import assert from 'node:assert/strict'
+import { applyContentAccessFilter } from '../lib/utils/applyContentAccessFilter.js'
+
+describe('applyContentAccessFilter()', () => {
+  it('should be a no-op when userId is falsy', () => {
+    const query = { _type: 'course' }
+    applyContentAccessFilter(query, undefined)
+    assert.deepEqual(query, { _type: 'course' })
+  })
+
+  it('should add a $or clause covering creator, public, and per-user sharing', () => {
+    const query = { _type: 'course' }
+    applyContentAccessFilter(query, 'user1')
+    assert.deepEqual(query.$or, [
+      { createdBy: 'user1' },
+      { _isShared: true },
+      { _shareWithUsers: 'user1' }
+    ])
+  })
+
+  it('should preserve an existing $or by lifting both into $and', () => {
+    const query = { $or: [{ title: 'foo' }] }
+    applyContentAccessFilter(query, 'user1')
+    assert.equal(query.$or, undefined)
+    assert.deepEqual(query.$and, [
+      { $or: [{ title: 'foo' }] },
+      {
+        $or: [
+          { createdBy: 'user1' },
+          { _isShared: true },
+          { _shareWithUsers: 'user1' }
+        ]
+      }
+    ])
+  })
+
+  it('should append to an existing $and rather than clobber it', () => {
+    const query = { $or: [{ title: 'foo' }], $and: [{ flag: true }] }
+    applyContentAccessFilter(query, 'user1')
+    assert.equal(query.$or, undefined)
+    assert.equal(query.$and.length, 3)
+    assert.deepEqual(query.$and[0], { flag: true })
+  })
+
+  it('should leave other top-level keys intact', () => {
+    const query = { _type: 'course', title: { $regex: 'foo' } }
+    applyContentAccessFilter(query, 'user1')
+    assert.equal(query._type, 'course')
+    assert.deepEqual(query.title, { $regex: 'foo' })
+  })
+})