You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Status: expand step in review — PR #108 (open, unmerged), tagged Breaking (major bump). The breaking element is the checkAccess semantics flip — see Breaking change below. The dependent chain is blocked on #108 landing; resolved design decisions and the linked issue map are in the comments below.
Context
Sharing is currently hardcoded in the adaptframework module for courses only (_isShared, _shareWithUsers, createdBy, via applyContentAccessFilter + checkContentAccess). We want a single extensible _access object that any module can opt into and extend with its own keys.
This has already started to drift: group sharing was bolted straight onto the hardcoded adaptframework path (a userGroups clause) and, separately, onto a restrictively-scoped asset filter in the usergroups module — so group access is currently implemented twice with opposite semantics (content = additive OR, assets = restrictive AND). Unifying onto _access removes that duplication, and it gets more expensive to unwind the longer it is deferred.
Ownership of _access keys
The API module owns the base _access schema and the public check. Other modules extend _access with their own keys and hook observers:
api — _access.public
authored — createdBy ownership check
users — _access.users (per-user sharing)
usergroups — _access.groups (group sharing)
Note: _access.groups is owned by the standalone adapt-authoring-usergroups module, notusers. Group functionality was split into its own module; putting _access.groups in users would recreate the dependency-direction problem that motivated the split.
Design
Modules opt in by calling this.enableAccessControl() in their init(). This:
Extends the module's schema with _access.public via the access schema extension
Registers the base public grant on both access hooks:
accessCheckHook — per-item grant when _access.public === true (single-document reads)
accessQueryHook — OR-merges { '_access.public': true } into the query (list / paginated reads)
Each module that adds _access keys taps both hooks with its own observer. Access is additive — any one observer granting is sufficient. This requires checkAccess to OR-combine observer results (some(Boolean)); the current code AND-combines them (every(Boolean)), so this issue flips that. See Breaking change below.
Both hooks are required: the project gates at the query level (accessQueryHook) for pagination accuracy, with the per-item accessCheckHook as the safety net for non-list reads. An accessCheckHook-only design would reintroduce the short-page / wrong-count problem that query-level filtering exists to avoid.
Implementation
New files
schema/access.schema.json — extension schema adding _access.public (matches the authored.schema.json$merge pattern)
lib/utils/isPublicAccess.js — pure predicate: resource._access?.public === true
lib/utils/addAccessClause.js — OR-merges an access clause into a mongo query (mirrors adaptframework's applyContentAccessFilter$or/$and handling), so query-level observers compose safely
lib/AbstractApiModule.js — add enableAccessControl(); flip checkAccess from .every(Boolean) to .some(Boolean) and rewrite the accessCheckHook JSDoc for the additive-grant contract
docs/writing-an-api.md — document the access hooks and the _access mechanism
enableAccessControl() method
asyncenableAccessControl(){if(!this.schemaName){returnthis.log('warn','cannot enable access control, no schemaName defined')}constjsonschema=awaitthis.app.waitForModule('jsonschema')jsonschema.extendSchema(this.schemaName,'access')// per-item grant (single-document reads)this.accessCheckHook.tap((req,resource)=>isPublicAccess(resource))// query-level grant (list / paginated reads) — keeps pagination accuratethis.accessQueryHook.tap(req=>addAccessClause(req.apiData.query,{'_access.public': true}))this.log('debug','access control enabled')}
The additive-grant model requires checkAccess to OR-combine accessCheckHook observer results, but the current code AND-combines them:
- if (!this.accessCheckHook.hasObservers || (await this.accessCheckHook.invoke(req, r)).every(Boolean)) {+ if (!this.accessCheckHook.hasObservers || (await this.accessCheckHook.invoke(req, r)).some(Boolean)) {
This is a breaking change to the published accessCheckHook contract:
Observers are now additive grants — any one returning true grants access (was: all must approve).
Returning false now abstains rather than denies. A restriction must veto by throw (a throw still denies the item regardless of other grants).
In-repo observers are unaffected (roles vetoes by throw; adaptframework is a single observer doing its own internal OR). External consumers relying on return-false-to-deny must switch to throwing. Released under the Breaking tag.
Dependent work in other modules
Once this lands, the following modules need updates (tracked in separate issues):
authored — tap both hooks with the createdBy ownership grant
users — extend _access with the users key, tap both hooks
usergroups — extend _access with the groups key, tap both hooks (replaces both the adaptframework userGroups clause and the bespoke asset filter)
adaptframework — remove checkContentAccessandapplyContentAccessFilter; rely on the generic mechanism instead
content — call enableAccessControl() to opt in
assets — call enableAccessControl() to opt in
contentplugin — call enableAccessControl() to opt in
UI (React + legacy) — patch both UIs to read/write _access.* instead of the legacy fields. The React UI uses _isShared/_shareWithUsers at CreateCourseWizard.jsx and Projects.jsx; the legacy adapt-security/adapt-authoring-ui needs an equivalent patch PR. These ship in the same release as the enforcement switch — see migration step 3.
Migration (expand → contract)
Existing data is migrated to _access.* via adapt-authoring-migrationsDataMigration scripts (.where({ collection, ...filter }).mutate(fn)), rather than a permanent legacy/new dual-read. Sequence it expand→contract, not as a flag-day — mutate rewrites docs one at a time and enforcement reads whatever field is live, so dropping legacy fields before the new reads deploy (or vice versa) leaves a window where shared content is inaccessible:
Backfill — migration writes _access.* from the legacy fields while keeping the legacy fields in place.
Switch — deploy enforcement (enableAccessControl() observers) reading _access.*together with the UI patches (React + legacy) in one release, so no UI is writing a field that nothing reads.
Contract — a later, separate migration drops the legacy fields once the new reads are verified and all UIs write _access.*.
Clean cutover, not a dual-write bridge
The expand→contract data migration only handles existing docs. The open risk is ongoing writes: once enforcement reads _access.*, a UI still writing the legacy fields would have its sharing toggles silently no-op. Rather than maintain a temporary dual-write (mirroring legacy-field writes into _access.* and back), we do a clean cutover — patch the UIs to write _access.* and switch enforcement in the same release (step 3). This is viable because the deployment is single-client-per-instance (see usergroups#13), so the backend and the legacy-UI deploy are coordinated. Keep the legacy fields readable until that release lands (don't contract early), which closes the access-gap window without juggling two field systems.
createdBy is not migrated — it stays an authored ownership check, not folded into _access
Caveats
Per-module ownership → per-module migrations. The _isShared/_shareWithUsers mapping belongs with adaptframework/content; the userGroups → _access.groups mapping belongs with usergroups. The access schema (api) must land first.
The assets change is a semantic flip, not a rename. Assets currently gate restrictively (AND: public OR in-group); _access.groups is an additive grant (OR). Copying the field across silently changes asset visibility behaviour, so this migration needs a conscious decision about intended semantics.
Idempotency. Scope each .where(...) to docs that still carry the legacy field (or lack _access) so re-runs are no-ops. mutate writes raw via replaceOne (no schema validation), so the migration itself must produce a schema-valid _access shape.
Scope
This issue unifies the access grant model — it makes access consistent, but not exhaustively enforced. Out of scope (separate follow-ups):
Enforcement holes: preview / publish / export and server-side content.find() calls bypass both hooks today (gated only by the coarse build scopes). These must be closed before access can be treated as a security boundary.
Sharing vs security semantics:_access observers are additive grants (sharing), which matches current requirements (see usergroups#13). If a key ever needs to restrict rather than widen access, that switch then lives in a single observer rather than scattered across modules.
Relationship to usergroups#13 and prioritisation
usergroups#13 concluded that groups are a sharing mechanism (not security) for the current single-client-per-instance deployment, and that group access should be built on this _access foundation as _access.groups. This work should therefore be prioritised ahead of further usergroups access enhancements (non-access usergroups work — UI, member management, naming — can proceed in parallel).
Context
Sharing is currently hardcoded in the adaptframework module for courses only (
_isShared,_shareWithUsers,createdBy, viaapplyContentAccessFilter+checkContentAccess). We want a single extensible_accessobject that any module can opt into and extend with its own keys.This has already started to drift: group sharing was bolted straight onto the hardcoded adaptframework path (a
userGroupsclause) and, separately, onto a restrictively-scoped asset filter in the usergroups module — so group access is currently implemented twice with opposite semantics (content = additiveOR, assets = restrictiveAND). Unifying onto_accessremoves that duplication, and it gets more expensive to unwind the longer it is deferred.Ownership of
_accesskeysThe API module owns the base
_accessschema and thepubliccheck. Other modules extend_accesswith their own keys and hook observers:_access.publiccreatedByownership check_access.users(per-user sharing)_access.groups(group sharing)Design
Modules opt in by calling
this.enableAccessControl()in theirinit(). This:_access.publicvia theaccessschema extensionpublicgrant on both access hooks:accessCheckHook— per-item grant when_access.public === true(single-document reads)accessQueryHook— OR-merges{ '_access.public': true }into the query (list / paginated reads)Each module that adds
_accesskeys taps both hooks with its own observer. Access is additive — any one observer granting is sufficient. This requirescheckAccessto OR-combine observer results (some(Boolean)); the current code AND-combines them (every(Boolean)), so this issue flips that. See Breaking change below.Both hooks are required: the project gates at the query level (
accessQueryHook) for pagination accuracy, with the per-itemaccessCheckHookas the safety net for non-list reads. AnaccessCheckHook-only design would reintroduce the short-page / wrong-count problem that query-level filtering exists to avoid.Implementation
New files
schema/access.schema.json— extension schema adding_access.public(matches theauthored.schema.json$mergepattern)lib/utils/isPublicAccess.js— pure predicate:resource._access?.public === truelib/utils/addAccessClause.js— OR-merges an access clause into a mongo query (mirrors adaptframework'sapplyContentAccessFilter$or/$andhandling), so query-level observers compose safelytests/utils-isPublicAccess.spec.js,tests/utils-addAccessClause.spec.js— table-driven testsModified files
lib/utils.js— re-exportisPublicAccess,addAccessClauseindex.js— export both for consuming moduleslib/AbstractApiModule.js— addenableAccessControl(); flipcheckAccessfrom.every(Boolean)to.some(Boolean)and rewrite theaccessCheckHookJSDoc for the additive-grant contractdocs/writing-an-api.md— document the access hooks and the_accessmechanismenableAccessControl()methodaccess.schema.json{ "$anchor": "access", "$merge": { "with": { "properties": { "_access": { "type": "object", "default": {}, "properties": { "public": { "type": "boolean", "default": false } } } } } } }Breaking change:
checkAccesssemanticsThe additive-grant model requires
checkAccessto OR-combineaccessCheckHookobserver results, but the current code AND-combines them:This is a breaking change to the published
accessCheckHookcontract:truegrants access (was: all must approve).falsenow abstains rather than denies. A restriction must veto bythrow(a throw still denies the item regardless of other grants).In-repo observers are unaffected (roles vetoes by
throw; adaptframework is a single observer doing its own internal OR). External consumers relying on return-false-to-deny must switch to throwing. Released under theBreakingtag.Dependent work in other modules
Once this lands, the following modules need updates (tracked in separate issues):
createdByownership grant_accesswith theuserskey, tap both hooks_accesswith thegroupskey, tap both hooks (replaces both the adaptframeworkuserGroupsclause and the bespoke asset filter)checkContentAccessandapplyContentAccessFilter; rely on the generic mechanism insteadenableAccessControl()to opt inenableAccessControl()to opt inenableAccessControl()to opt in_access.*instead of the legacy fields. The React UI uses_isShared/_shareWithUsersatCreateCourseWizard.jsxandProjects.jsx; the legacyadapt-security/adapt-authoring-uineeds an equivalent patch PR. These ship in the same release as the enforcement switch — see migration step 3.Migration (expand → contract)
Existing data is migrated to
_access.*viaadapt-authoring-migrationsDataMigrationscripts (.where({ collection, ...filter }).mutate(fn)), rather than a permanent legacy/new dual-read. Sequence it expand→contract, not as a flag-day —mutaterewrites docs one at a time and enforcement reads whatever field is live, so dropping legacy fields before the new reads deploy (or vice versa) leaves a window where shared content is inaccessible:accessschema extension (this issue / PR Breaking: Add generic _access mechanism (fixes #98) #108)._access.*from the legacy fields while keeping the legacy fields in place.enableAccessControl()observers) reading_access.*together with the UI patches (React + legacy) in one release, so no UI is writing a field that nothing reads._access.*.Clean cutover, not a dual-write bridge
The expand→contract data migration only handles existing docs. The open risk is ongoing writes: once enforcement reads
_access.*, a UI still writing the legacy fields would have its sharing toggles silently no-op. Rather than maintain a temporary dual-write (mirroring legacy-field writes into_access.*and back), we do a clean cutover — patch the UIs to write_access.*and switch enforcement in the same release (step 3). This is viable because the deployment is single-client-per-instance (see usergroups#13), so the backend and the legacy-UI deploy are coordinated. Keep the legacy fields readable until that release lands (don't contract early), which closes the access-gap window without juggling two field systems.Data mapping
_isShared: true→_access.public: true;_shareWithUsers→_access.users; courseuserGroups→_access.groups_access.groupscreatedByis not migrated — it stays an authored ownership check, not folded into_accessCaveats
_isShared/_shareWithUsersmapping belongs with adaptframework/content; theuserGroups→_access.groupsmapping belongs with usergroups. Theaccessschema (api) must land first._access.groupsis an additive grant (OR). Copying the field across silently changes asset visibility behaviour, so this migration needs a conscious decision about intended semantics..where(...)to docs that still carry the legacy field (or lack_access) so re-runs are no-ops.mutatewrites raw viareplaceOne(no schema validation), so the migration itself must produce a schema-valid_accessshape.Scope
This issue unifies the access grant model — it makes access consistent, but not exhaustively enforced. Out of scope (separate follow-ups):
content.find()calls bypass both hooks today (gated only by the coarse build scopes). These must be closed before access can be treated as a security boundary._accessobservers are additive grants (sharing), which matches current requirements (see usergroups#13). If a key ever needs to restrict rather than widen access, that switch then lives in a single observer rather than scattered across modules.Relationship to usergroups#13 and prioritisation
usergroups#13 concluded that groups are a sharing mechanism (not security) for the current single-client-per-instance deployment, and that group access should be built on this
_accessfoundation as_access.groups. This work should therefore be prioritised ahead of further usergroups access enhancements (non-access usergroups work — UI, member management, naming — can proceed in parallel).