diff --git a/lib/AdaptFrameworkModule.js b/lib/AdaptFrameworkModule.js index 3e73e89..a042381 100644 --- a/lib/AdaptFrameworkModule.js +++ b/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 diff --git a/lib/utils.js b/lib/utils.js index 0013b47..6448bfc 100644 --- a/lib/utils.js +++ b/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' diff --git a/lib/utils/applyContentAccessFilter.js b/lib/utils/applyContentAccessFilter.js new file mode 100644 index 0000000..5ca9470 --- /dev/null +++ b/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 + } +} diff --git a/tests/utils-applyContentAccessFilter.spec.js b/tests/utils-applyContentAccessFilter.spec.js new file mode 100644 index 0000000..8ccc3f2 --- /dev/null +++ b/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' }) + }) +})