diff --git a/docs/writing-an-api.md b/docs/writing-an-api.md index fe1bfaa4..15ecbc0d 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,35 @@ 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`. + +`_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/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..dde67788 --- /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": true + } + } + } + } + } + } +} 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) + }) + }) +})