From d331bbf31cdccee35cf6cb46ca60790a4e806a72 Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Mon, 29 Jun 2026 16:30:33 +0100 Subject: [PATCH 1/3] Breaking: Add generic _access mechanism (fixes #98) Adds an extensible `_access` object that any API module can opt into via `enableAccessControl()`, replacing per-module hardcoded sharing. It extends the module schema with `_access.public` and registers the base `public` grant on both accessCheckHook (per-item, single-document reads) and accessQueryHook (query-level, keeps pagination accurate). Ships the `isPublicAccess` predicate and `addAccessClause` query helper (both re-exported) and the `access` schema extension. Other modules extend `_access` with their own keys and tap both hooks with additive grants. accessCheckHook observers are now additive access grants, OR-combined across observers (previously AND-combined). Returning false now abstains rather than denies; a restriction must veto by throwing. Existing in-repo observers are unaffected (roles vetoes by throw, adaptframework is a single observer), but external consumers relying on return-false-to-deny must switch to throwing. --- docs/writing-an-api.md | 30 ++++++++++--- index.js | 1 + lib/AbstractApiModule.js | 40 ++++++++++++----- lib/utils.js | 2 + lib/utils/addAccessClause.js | 29 +++++++++++++ lib/utils/isPublicAccess.js | 9 ++++ schema/access.schema.json | 22 ++++++++++ tests/utils-addAccessClause.spec.js | 66 +++++++++++++++++++++++++++++ tests/utils-isPublicAccess.spec.js | 20 +++++++++ 9 files changed, 202 insertions(+), 17 deletions(-) create mode 100644 lib/utils/addAccessClause.js create mode 100644 lib/utils/isPublicAccess.js create mode 100644 schema/access.schema.json create mode 100644 tests/utils-addAccessClause.spec.js create mode 100644 tests/utils-isPublicAccess.spec.js diff --git a/docs/writing-an-api.md b/docs/writing-an-api.md index fe1bfaa4..1101c612 100644 --- a/docs/writing-an-api.md +++ b/docs/writing-an-api.md @@ -341,7 +341,9 @@ See the table below for a list of hooks provided by the `AbstractApiModule` clas | `postUpdateHook` | After updating a document | No | | `preDeleteHook` | Before deleting a document | No | | `postDeleteHook` | After deleting a document | No | -| `accessCheckHook` | When checking access to a resource | No | +| `accessCheckHook` | Per-item access check (single-document reads) | No | +| `accessQueryHook` | Merges access-control clauses into a list query (skipped for super users) | No | +| `queryHook` | Merges user-driven filter clauses into a list query (runs for all users) | Yes | ### Using hooks @@ -374,17 +376,31 @@ class NotesModule extends AbstractApiModule { } ``` -### Access control with accessCheckHook +### Access control -Use `accessCheckHook` to implement custom access control: +Access is an **additive grant model**: observers widen access, they don't restrict it. Both access hooks are OR-combined across all observers, so any one observer granting access is sufficient. + +- `accessQueryHook` — the primary gate. Merge a clause into `req.apiData.query` so the database only returns documents the user can see. Filtering here (rather than after the query) keeps pagination counts and the `Link` header accurate. Skipped for super users. +- `accessCheckHook` — the per-item safety net for single-document reads. Return `true` to grant, a non-truthy value to abstain, or `throw` to hard-veto the item (a veto denies regardless of other grants — this is how a restriction is expressed). + +```javascript +this.accessCheckHook.tap((req, doc) => doc.createdBy === req.auth.user._id.toString()) +this.accessQueryHook.tap(req => addAccessClause(req.apiData.query, { createdBy: req.auth.user._id.toString() })) +``` + +#### The generic `_access` mechanism + +Call `enableAccessControl()` in your module's `init()` to opt into a shared, extensible `_access` object. This extends the module schema with `_access.public` and registers a `public` grant on both hooks: ```javascript -this.accessCheckHook.tap(async (req, doc) => { - // Return true to allow access, false or undefined to deny - return doc.createdBy === req.auth.user._id.toString() -}) +async init () { + await super.init() + await this.enableAccessControl() +} ``` +Other modules extend `_access` with their own keys (e.g. `_access.users`, `_access.groups`) and tap both hooks with additional grants, all OR-combined with the base `public` grant. Use `addAccessClause` (exported from `adapt-authoring-api`) for the query-level grant so observers compose safely with each other and with any user-driven `$or`. + ## Overriding methods You can override database methods to customise behaviour: diff --git a/index.js b/index.js index 071e0d08..442c5409 100644 --- a/index.js +++ b/index.js @@ -4,5 +4,6 @@ */ export { default as AbstractApiModule } from './lib/AbstractApiModule.js' export { default } from './lib/AbstractApiModule.js' +export { addAccessClause, isPublicAccess } from './lib/utils.js' /** @deprecated Use named import { stringifyValues } from 'adapt-authoring-core' instead */ export { stringifyValues } from 'adapt-authoring-core' diff --git a/lib/AbstractApiModule.js b/lib/AbstractApiModule.js index cd33a842..3cba72e6 100644 --- a/lib/AbstractApiModule.js +++ b/lib/AbstractApiModule.js @@ -1,6 +1,6 @@ import _ from 'lodash' import { AbstractModule, DataCache, Hook, stringifyValues } from 'adapt-authoring-core' -import { argsFromReq, generateApiMetadata, httpMethodToDBFunction } from './utils.js' +import { addAccessClause, argsFromReq, generateApiMetadata, httpMethodToDBFunction, isPublicAccess } from './utils.js' import { loadRouteConfig } from 'adapt-authoring-server' /** * Abstract module for creating APIs @@ -92,14 +92,15 @@ class AbstractApiModule extends AbstractModule { */ this.postDeleteHook = new Hook() /** - * Hook invoked to check access to individual data items. - * Observer contract — for each item, every observer must approve: - * - `return true` — approve (or abstain; AND-combined across observers) - * - `return false` — deny - * - `throw` — deny; for single-doc requests, propagates the thrown error - * `undefined` and any non-truthy return are treated as `false`. - * Single-doc requests that are denied by any observer respond `401 Unauthorised`. - * List requests silently filter denied items. + * Hook invoked to check access to individual data items. Observers are additive access grants — + * for each item, any one observer approving is sufficient (OR-combined): + * - `return true` — grant access + * - `return false` — abstain (no grant; another observer may still grant) + * - `throw` — hard veto; denies the item regardless of other grants, and for single-doc + * requests propagates the thrown error. Restrictions must veto by throwing. + * `undefined` and any non-truthy return are treated as an abstention. + * Single-doc requests granted by no observer respond `401 Unauthorised`. + * List requests silently filter ungranted items. * * Runs post-query, so it is best reserved as a safety net for checks that cannot be expressed * as a query. For filtering, prefer `accessQueryHook` — filtering at this stage produces short @@ -605,6 +606,25 @@ class AbstractApiModule extends AbstractModule { Object.assign(mongoOpts, { limit: pageSize, skip: mongoOpts.skip || (page - 1) * pageSize }) } + /** + * Opts this module into the generic `_access` access-control mechanism. Extends the module schema + * with `_access.public` and registers the base `public` grant on both access hooks: a per-item grant + * on `accessCheckHook` (single-document reads) and a query-level grant on `accessQueryHook` (list / + * paginated reads, kept query-level so pagination stays accurate). Other modules extend `_access` with + * their own keys and tap both hooks with additional additive grants. + * @return {Promise} + */ + async enableAccessControl () { + if (!this.schemaName) { + return this.log('warn', 'cannot enable access control, no schemaName defined') + } + const jsonschema = await this.app.waitForModule('jsonschema') + jsonschema.extendSchema(this.schemaName, 'access') + this.accessCheckHook.tap((req, resource) => isPublicAccess(resource)) + this.accessQueryHook.tap(req => addAccessClause(req.apiData.query, { '_access.public': true })) + this.log('debug', 'access control enabled') + } + /** * Invokes the access check hook to allow modules to determine whether the request user has sufficient access to the requested resource(s) * @param {external:ExpressRequest} req @@ -620,7 +640,7 @@ class AbstractApiModule extends AbstractModule { let error await Promise.allSettled((isArray ? data : [data]).map(async r => { try { - if (!this.accessCheckHook.hasObservers || (await this.accessCheckHook.invoke(req, r)).every(Boolean)) { + if (!this.accessCheckHook.hasObservers || (await this.accessCheckHook.invoke(req, r)).some(Boolean)) { filtered.push(r) } } catch (e) { diff --git a/lib/utils.js b/lib/utils.js index 70f4af46..f69902bd 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -1,4 +1,6 @@ +export { addAccessClause } from './utils/addAccessClause.js' export { argsFromReq } from './utils/argsFromReq.js' export { generateApiMetadata } from './utils/generateApiMetadata.js' export { httpMethodToAction } from './utils/httpMethodToAction.js' export { httpMethodToDBFunction } from './utils/httpMethodToDBFunction.js' +export { isPublicAccess } from './utils/isPublicAccess.js' diff --git a/lib/utils/addAccessClause.js b/lib/utils/addAccessClause.js new file mode 100644 index 00000000..357e611d --- /dev/null +++ b/lib/utils/addAccessClause.js @@ -0,0 +1,29 @@ +const accessGroups = new WeakMap() + +/** + * OR-merges an access-control clause into a mongo query, mutating it in place. Repeated calls on the + * same query accumulate clauses into a single shared `$or` group (additive grants), AND-combined with + * any pre-existing query — an existing user-driven `$or` (e.g. search) is lifted into `$and` so it + * isn't widened by the grants. + * @param {Object} query The mongo query to mutate + * @param {Object} clause The access clause to grant (e.g. `{ '_access.public': true }`) + * @return {Object} The mutated query + * @memberof api + */ +export function addAccessClause (query, clause) { + let group = accessGroups.get(query) + if (!group) { + group = [] + accessGroups.set(query, group) + if (query.$or) { + query.$and = [...(query.$and ?? []), { $or: query.$or }, { $or: group }] + delete query.$or + } else if (query.$and) { + query.$and.push({ $or: group }) + } else { + query.$or = group + } + } + group.push(clause) + return query +} diff --git a/lib/utils/isPublicAccess.js b/lib/utils/isPublicAccess.js new file mode 100644 index 00000000..b3d894c2 --- /dev/null +++ b/lib/utils/isPublicAccess.js @@ -0,0 +1,9 @@ +/** + * Predicate determining whether a resource has been granted public access + * @param {Object} resource The resource to check + * @return {Boolean} + * @memberof api + */ +export function isPublicAccess (resource) { + return resource?._access?.public === true +} diff --git a/schema/access.schema.json b/schema/access.schema.json new file mode 100644 index 00000000..63159bfe --- /dev/null +++ b/schema/access.schema.json @@ -0,0 +1,22 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$anchor": "access", + "description": "Adds a generic access-control object that modules extend with their own grant keys", + "$merge": { + "with": { + "properties": { + "_access": { + "type": "object", + "default": {}, + "properties": { + "public": { + "description": "Whether the resource is accessible to all users", + "type": "boolean", + "default": false + } + } + } + } + } + } +} diff --git a/tests/utils-addAccessClause.spec.js b/tests/utils-addAccessClause.spec.js new file mode 100644 index 00000000..b9c14cea --- /dev/null +++ b/tests/utils-addAccessClause.spec.js @@ -0,0 +1,66 @@ +import { describe, it } from 'node:test' +import assert from 'node:assert/strict' +import { addAccessClause } from '../lib/utils/addAccessClause.js' + +describe('addAccessClause()', () => { + it('should add the clause as a top-level $or on an empty query', () => { + const query = {} + addAccessClause(query, { '_access.public': true }) + assert.deepEqual(query, { $or: [{ '_access.public': true }] }) + }) + + it('should preserve non-$or top-level fields', () => { + const query = { _type: 'course' } + addAccessClause(query, { '_access.public': true }) + assert.deepEqual(query, { _type: 'course', $or: [{ '_access.public': true }] }) + }) + + it('should accumulate multiple grants into one shared $or group', () => { + const query = {} + addAccessClause(query, { '_access.public': true }) + addAccessClause(query, { createdBy: 'abc' }) + addAccessClause(query, { '_access.groups': { $in: ['g1'] } }) + assert.deepEqual(query, { + $or: [ + { '_access.public': true }, + { createdBy: 'abc' }, + { '_access.groups': { $in: ['g1'] } } + ] + }) + }) + + it('should lift a pre-existing user $or into $and so grants do not widen it', () => { + const query = { $or: [{ title: 'a' }, { title: 'b' }] } + addAccessClause(query, { '_access.public': true }) + addAccessClause(query, { createdBy: 'abc' }) + assert.deepEqual(query, { + $and: [ + { $or: [{ title: 'a' }, { title: 'b' }] }, + { $or: [{ '_access.public': true }, { createdBy: 'abc' }] } + ] + }) + assert.ok(!('$or' in query)) + }) + + it('should append the grant group to a pre-existing $and', () => { + const query = { $and: [{ x: 1 }] } + addAccessClause(query, { '_access.public': true }) + assert.deepEqual(query, { + $and: [{ x: 1 }, { $or: [{ '_access.public': true }] }] + }) + }) + + it('should return the mutated query', () => { + const query = {} + assert.equal(addAccessClause(query, { '_access.public': true }), query) + }) + + it('should track grant groups per query object', () => { + const a = {} + const b = {} + addAccessClause(a, { '_access.public': true }) + addAccessClause(b, { createdBy: 'x' }) + assert.deepEqual(a, { $or: [{ '_access.public': true }] }) + assert.deepEqual(b, { $or: [{ createdBy: 'x' }] }) + }) +}) diff --git a/tests/utils-isPublicAccess.spec.js b/tests/utils-isPublicAccess.spec.js new file mode 100644 index 00000000..3f35037c --- /dev/null +++ b/tests/utils-isPublicAccess.spec.js @@ -0,0 +1,20 @@ +import { describe, it } from 'node:test' +import assert from 'node:assert/strict' +import { isPublicAccess } from '../lib/utils/isPublicAccess.js' + +describe('isPublicAccess()', () => { + const cases = [ + { name: 'true when _access.public is true', resource: { _access: { public: true } }, expected: true }, + { name: 'false when _access.public is false', resource: { _access: { public: false } }, expected: false }, + { name: 'false when _access.public is missing', resource: { _access: {} }, expected: false }, + { name: 'false when _access is missing', resource: {}, expected: false }, + { name: 'false for a truthy non-boolean public value', resource: { _access: { public: 'true' } }, expected: false }, + { name: 'false when resource is undefined', resource: undefined, expected: false }, + { name: 'false when resource is null', resource: null, expected: false } + ] + cases.forEach(({ name, resource, expected }) => { + it(`should return ${expected}: ${name}`, () => { + assert.equal(isPublicAccess(resource), expected) + }) + }) +}) From 9dcf8a9cf4e891b0a9991f292a0d27e80e13fc6d Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Tue, 30 Jun 2026 15:56:07 +0100 Subject: [PATCH 2/3] Update: Default _access.public to true (refs #98) --- docs/writing-an-api.md | 4 ++++ schema/access.schema.json | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/docs/writing-an-api.md b/docs/writing-an-api.md index 1101c612..15ecbc0d 100644 --- a/docs/writing-an-api.md +++ b/docs/writing-an-api.md @@ -401,6 +401,10 @@ async init () { Other modules extend `_access` with their own keys (e.g. `_access.users`, `_access.groups`) and tap both hooks with additional grants, all OR-combined with the base `public` grant. Use `addAccessClause` (exported from `adapt-authoring-api`) for the query-level grant so observers compose safely with each other and with any user-driven `$or`. +`_access.public` defaults to `true` — resources are public unless a grant or the creating UI sets it otherwise. This preserves open access for resource types without a sharing UI; clients that scope access (e.g. the course wizard) set the value explicitly. + +Enforcement is request-scoped: both hooks read `req.auth`/`req.apiData.query`, so `_access` is only applied to REST API requests. Internal/programmatic data access (e.g. server-side `find()`, preview, publish, export) bypasses it by design — there is no request identity to check against outside a REST request. + ## Overriding methods You can override database methods to customise behaviour: diff --git a/schema/access.schema.json b/schema/access.schema.json index 63159bfe..f3c36886 100644 --- a/schema/access.schema.json +++ b/schema/access.schema.json @@ -7,12 +7,12 @@ "properties": { "_access": { "type": "object", - "default": {}, + "default": { "public": true }, "properties": { "public": { "description": "Whether the resource is accessible to all users", "type": "boolean", - "default": false + "default": true } } } From c2b11137961dc636239d79844af81cc5ade2e7ac Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Tue, 30 Jun 2026 16:30:18 +0100 Subject: [PATCH 3/3] Chore: Collapse _access.public default to one place (refs #98) --- schema/access.schema.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/schema/access.schema.json b/schema/access.schema.json index f3c36886..dde67788 100644 --- a/schema/access.schema.json +++ b/schema/access.schema.json @@ -7,7 +7,7 @@ "properties": { "_access": { "type": "object", - "default": { "public": true }, + "default": {}, "properties": { "public": { "description": "Whether the resource is accessible to all users",