diff --git a/package-lock.json b/package-lock.json index 7ec004e9..8ac8d645 100644 --- a/package-lock.json +++ b/package-lock.json @@ -42,6 +42,7 @@ "multi-core-indexer": "^1.0.0", "p-defer": "^4.0.0", "p-event": "^6.0.1", + "p-reduce": "^3.0.0", "p-timeout": "^6.1.2", "protobufjs": "^7.2.3", "protomux": "^3.4.1", @@ -7337,6 +7338,18 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/p-reduce": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/p-reduce/-/p-reduce-3.0.0.tgz", + "integrity": "sha512-xsrIUgI0Kn6iyDYm9StOpOeK29XM1aboGji26+QEortiFST1hGZaUQOLhtEbqHErPpGW/aSz6allwK2qcptp0Q==", + "license": "MIT", + "engines": { + "node": ">=12" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, "node_modules/p-timeout": { "version": "6.1.2", "resolved": "https://registry.npmjs.org/p-timeout/-/p-timeout-6.1.2.tgz", diff --git a/package.json b/package.json index dced69c0..ec0ff909 100644 --- a/package.json +++ b/package.json @@ -190,6 +190,7 @@ "multi-core-indexer": "^1.0.0", "p-defer": "^4.0.0", "p-event": "^6.0.1", + "p-reduce": "^3.0.0", "p-timeout": "^6.1.2", "protobufjs": "^7.2.3", "protomux": "^3.4.1", diff --git a/src/errors.js b/src/errors.js index 65bd6c96..38bc8764 100644 --- a/src/errors.js +++ b/src/errors.js @@ -3,3 +3,12 @@ export class NotFoundError extends Error { super(message) } } + +/** + * @param {unknown} err + * @returns {null} + */ +export function nullIfNotFound(err) { + if (err instanceof NotFoundError) return null + throw err +} diff --git a/src/lib/ponyfills.js b/src/lib/ponyfills.js index daf88557..471e3bf1 100644 --- a/src/lib/ponyfills.js +++ b/src/lib/ponyfills.js @@ -23,3 +23,19 @@ export function abortSignalAny(iterable) { return controller.signal } + +/** + * Ponyfill of `Set.prototype.isSubsetOf()`. + * + * [1]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set/isSubsetOf + * + * @param {ReadonlySet} me + * @param {ReadonlySet} other + * @returns {boolean} + */ +export function setIsSubsetOf(me, other) { + for (const value of me) { + if (!other.has(value)) return false + } + return true +} diff --git a/src/mapeo-project.js b/src/mapeo-project.js index c8d70331..f81e386f 100644 --- a/src/mapeo-project.js +++ b/src/mapeo-project.js @@ -59,7 +59,7 @@ import { Logger } from './logger.js' import { IconApi } from './icon-api.js' import { readConfig } from './config-import.js' import TranslationApi from './translation-api.js' -import { NotFoundError } from './errors.js' +import { NotFoundError, nullIfNotFound } from './errors.js' import { getErrorCode, getErrorMessage } from './lib/error.js' /** @import { ProjectSettingsValue } from '@comapeo/schema' */ /** @import { CoreStorage, BlobFilter, BlobStoreEntriesStream, KeyPair, Namespace, ReplicationStream } from './types.js' */ @@ -663,9 +663,9 @@ export class MapeoProject extends TypedEmitter { async $setProjectSettings(settings) { const { projectSettings } = this.#dataTypes - const existing = await projectSettings.getByDocId(this.#projectId, { - mustBeFound: false, - }) + const existing = await projectSettings + .getByDocId(this.#projectId) + .catch(nullIfNotFound) if (existing) { return extractEditableProjectSettings( @@ -773,9 +773,9 @@ export class MapeoProject extends TypedEmitter { schemaName: /** @type {const} */ ('deviceInfo'), } - const existingDoc = await deviceInfo.getByDocId(configCoreId, { - mustBeFound: false, - }) + const existingDoc = await deviceInfo + .getByDocId(configCoreId) + .catch(nullIfNotFound) if (existingDoc) { return await deviceInfo.update(existingDoc.versionId, doc) } else { diff --git a/src/member-api.js b/src/member-api.js index 08b64a38..3bcd3da1 100644 --- a/src/member-api.js +++ b/src/member-api.js @@ -496,10 +496,12 @@ export class MemberApi extends TypedEmitter { /** * @param {string} deviceId * @param {import('./roles.js').RoleIdAssignableToOthers} roleId + * @param {object} [options] + * @param {boolean} [options.__testOnlyAllowAnyRoleToBeAssigned] * @returns {Promise} */ - async assignRole(deviceId, roleId) { - return this.#roles.assignRole(deviceId, roleId) + async assignRole(deviceId, roleId, options) { + return this.#roles.assignRole(deviceId, roleId, options) } } diff --git a/src/roles.js b/src/roles.js index a7fc8dfb..ebab3325 100644 --- a/src/roles.js +++ b/src/roles.js @@ -1,8 +1,13 @@ -import { currentSchemaVersions } from '@comapeo/schema' +import { currentSchemaVersions, parseVersionId } from '@comapeo/schema' import mapObject from 'map-obj' +import pReduce from 'p-reduce' import { kCreateWithDocId, kDataStore } from './datatype/index.js' import { assert, setHas } from './utils.js' +import { nullIfNotFound } from './errors.js' import { TypedEmitter } from 'tiny-typed-emitter' +import { setIsSubsetOf } from './lib/ponyfills.js' +/** @import { Role as MembershipRecord } from '@comapeo/schema' */ +/** @import { ReadonlyDeep } from 'type-fest' */ /** @import { Namespace } from './types.js' */ // Randomly generated 8-byte encoded as hex @@ -13,6 +18,9 @@ export const BLOCKED_ROLE_ID = '9e6d29263cba36c9' export const LEFT_ROLE_ID = '8ced989b1904606b' export const NO_ROLE_ID = '08e4251e36f6e7ed' +const CREATOR_MEMBERSHIP_RECORD = Symbol('creator role assignment') +const ROLE_CHAIN_ITERATION_LIMIT = 1000 + /** * @typedef {T extends Iterable ? U : never} ElementOf * @template T @@ -126,11 +134,11 @@ const BLOCKED_ROLE = { } /** - * This is the role assumed for a device when no role record can be found. This + * This is the role assumed for a device when no membership record can be found. This * can happen when an invited device did not manage to sync with the device that * invited them, and they then try to sync with someone else. We want them to be * able to sync the auth and config store, because that way they may be able to - * receive their role record, and they can get the project config so that they + * receive their membership record, and they can get the project config so that they * can start collecting data. * * @type {Role} @@ -222,7 +230,7 @@ export const ROLES = { /** * @typedef {object} RolesEvents - * @property {(docIds: Set) => void} update Emitted when new role records are indexed + * @property {(docIds: Set) => void} update Emitted when new membership records are indexed */ /** @@ -269,31 +277,164 @@ export class Roles extends TypedEmitter { * @returns {Promise} */ async getRole(deviceId) { - const roleAssignment = await this.#dataType.getByDocId(deviceId, { - mustBeFound: false, - }) - if (!roleAssignment) { - // The project creator will have the creator role - const authCoreId = await this.#coreOwnership.getCoreId(deviceId, 'auth') - if (authCoreId === this.#projectCreatorAuthCoreId) { - return CREATOR_ROLE - } else { - // When no role assignment exists, e.g. a newly added device which has - // not yet synced role records. + const membershipRecord = await this.#getMembershipRecord(deviceId) + + switch (membershipRecord) { + case null: return NO_ROLE + case CREATOR_MEMBERSHIP_RECORD: + return CREATOR_ROLE + default: { + const { roleId } = membershipRecord + if ( + isRoleId(roleId) && + (await this.#isRoleChainValid(membershipRecord)) + ) { + return ROLES[roleId] + } else { + return BLOCKED_ROLE + } } } + } - const { roleId } = roleAssignment - if (!isRoleId(roleId)) { - return BLOCKED_ROLE + /** + * @param {string} deviceId + * @returns {Promise} + */ + async #getMembershipRecord(deviceId) { + const result = await this.#dataType + .getByDocId(deviceId) + .catch(nullIfNotFound) + if (result) return result + + // The project creator will have the creator role + const authCoreId = await this.#coreOwnership.getCoreId(deviceId, 'auth') + if (authCoreId === this.#projectCreatorAuthCoreId) { + return CREATOR_MEMBERSHIP_RECORD } - return ROLES[roleId] + + // When no role assignment exists, e.g. a newly added device which has + // not yet synced membership records. + return null + } + + /** + * @param {ReadonlyDeep} membershipRecord + * @returns {Promise} + */ + async #isRoleChainValid(membershipRecord) { + if (membershipRecord.roleId === LEFT_ROLE_ID) return true + + /** @type {null | ReadonlyDeep} */ + let currentMembershipRecord = membershipRecord + + for ( + let i = 0; + currentMembershipRecord && i < ROLE_CHAIN_ITERATION_LIMIT; + i++ + ) { + const parentMembershipRecord = await this.#getParentMembershipRecord( + currentMembershipRecord + ) + switch (parentMembershipRecord) { + case null: + break + case CREATOR_MEMBERSHIP_RECORD: + return true + default: + if ( + !canAssign({ + assigner: parentMembershipRecord, + assignee: currentMembershipRecord, + }) + ) { + return false + } + break + } + + currentMembershipRecord = parentMembershipRecord + } + + return false + } + + /** + * @param {ReadonlyDeep} membershipRecord + * @returns {Promise} + */ + async #getParentMembershipRecord(membershipRecord) { + const { + coreDiscoveryKey: assignerCoreDiscoveryKey, + index: assignerIndexAtAssignmentTime, + } = parseVersionId(membershipRecord.versionId) + + const isAssignedByProjectCreator = + assignerCoreDiscoveryKey.toString('hex') === + this.#projectCreatorAuthCoreId + if (isAssignedByProjectCreator) return CREATOR_MEMBERSHIP_RECORD + + const assignerCore = this.#coreManager.getCoreByDiscoveryKey( + assignerCoreDiscoveryKey + ) + if (assignerCore?.namespace !== 'auth') return null + + const assignerCoreId = assignerCore.key.toString('hex') + const assignerDeviceId = await this.#coreOwnership + .getOwner(assignerCoreId) + .catch(nullIfNotFound) + if (!assignerDeviceId) return null + + const latestMembershipRecord = await this.#getMembershipRecord( + assignerDeviceId + ) + + /** @type {null | typeof CREATOR_MEMBERSHIP_RECORD | MembershipRecord} */ + let membershipRecordToCheck = latestMembershipRecord + for ( + let i = 0; + membershipRecordToCheck && i < ROLE_CHAIN_ITERATION_LIMIT; + i++ + ) { + if ( + membershipRecordToCheck === CREATOR_MEMBERSHIP_RECORD || + (membershipRecordToCheck.fromIndex <= assignerIndexAtAssignmentTime && + membershipRecordToCheck.versionId !== membershipRecord.versionId) + ) { + return membershipRecordToCheck + } + + membershipRecordToCheck = await pReduce( + membershipRecord.links, + /** + * @param {null | Readonly} result + * @param {string} linkedVersionId + * @returns {Promise} + */ + async (result, linkedVersionId) => { + const linkedMembershipRecord = await this.#dataType + .getByVersionId(linkedVersionId) + .catch(nullIfNotFound) + if (linkedMembershipRecord && result) { + return chooseLeastPermissiveMembershipRecord( + result, + linkedMembershipRecord + ) + } else { + return linkedMembershipRecord || result + } + }, + null + ) + } + + return null } /** * Get roles of all devices in the project. For your own device, if you have - * not yet synced your own role record, the "no role" capabilties is + * not yet synced your own membership record, the "no role" capabilties is * returned. The project creator will have the creator role unless a * different one has been assigned. * @@ -345,8 +486,14 @@ export class Roles extends TypedEmitter { * * @param {string} deviceId * @param {RoleIdAssignableToAnyone} roleId + * @param {object} [options] + * @param {boolean} [options.__testOnlyAllowAnyRoleToBeAssigned] */ - async assignRole(deviceId, roleId) { + async assignRole( + deviceId, + roleId, + { __testOnlyAllowAnyRoleToBeAssigned = false } = {} + ) { assert( isRoleIdAssignableToAnyone(roleId), `Role ID should be assignable to anyone but got ${roleId}` @@ -369,7 +516,11 @@ export class Roles extends TypedEmitter { } const isAssigningProjectCreatorRole = authCoreId === this.#projectCreatorAuthCoreId - if (isAssigningProjectCreatorRole && !this.#isProjectCreator()) { + if ( + isAssigningProjectCreatorRole && + !this.#isProjectCreator() && + !__testOnlyAllowAnyRoleToBeAssigned + ) { throw new Error( "Only the project creator can assign the project creator's role" ) @@ -381,14 +532,17 @@ export class Roles extends TypedEmitter { } } else { const ownRole = await this.getRole(this.#ownDeviceId) - if (!ownRole.roleAssignment.includes(roleId)) { + if ( + !ownRole.roleAssignment.includes(roleId) && + !__testOnlyAllowAnyRoleToBeAssigned + ) { throw new Error('Lacks permission to assign role ' + roleId) } } - const existingRoleDoc = await this.#dataType.getByDocId(deviceId, { - mustBeFound: false, - }) + const existingRoleDoc = await this.#dataType + .getByDocId(deviceId) + .catch(nullIfNotFound) if (existingRoleDoc) { await this.#dataType.update( @@ -415,3 +569,50 @@ export class Roles extends TypedEmitter { return ownAuthCoreId === this.#projectCreatorAuthCoreId } } + +/** + * @param {object} options + * @param {ReadonlyDeep>} options.assigner + * @param {ReadonlyDeep>} options.assignee + * @returns {boolean} + */ +function canAssign({ assigner, assignee }) { + return ( + isRoleIdAssignableToOthers(assignee.roleId) && + membershipRecordToRole(assigner).roleAssignment.includes(assignee.roleId) + ) +} + +/** + * @param {ReadonlyDeep>} membershipRecord + * @returns {Role} + */ +function membershipRecordToRole({ roleId }) { + return isRoleId(roleId) ? ROLES[roleId] : BLOCKED_ROLE +} + +/** + * @param {MembershipRecord} a + * @param {MembershipRecord} b + * @returns {MembershipRecord} + */ +function chooseLeastPermissiveMembershipRecord(a, b) { + const aRoleAssignments = new Set(membershipRecordToRole(a).roleAssignment) + const bRoleAssignments = new Set(membershipRecordToRole(b).roleAssignment) + if (setIsSmallerSubsetOf(aRoleAssignments, bRoleAssignments)) return a + if (setIsSmallerSubsetOf(bRoleAssignments, aRoleAssignments)) return b + + if (a.updatedAt > b.updatedAt) return a + if (b.updatedAt > a.updatedAt) return b + + return a.versionId > b.versionId ? a : b +} + +/** + * @param {ReadonlySet} me + * @param {ReadonlySet} other + * @returns {boolean} + */ +function setIsSmallerSubsetOf(me, other) { + return setIsSubsetOf(me, other) && me.size < other.size +} diff --git a/src/translation-api.js b/src/translation-api.js index 636a8a20..e32e6fe0 100644 --- a/src/translation-api.js +++ b/src/translation-api.js @@ -1,6 +1,7 @@ import { and, sql } from 'drizzle-orm' import { kCreateWithDocId, kSelect } from './datatype/index.js' import { hashObject } from './utils.js' +import { nullIfNotFound } from './errors.js' import { omit } from './lib/omit.js' /** @import { Translation, TranslationValue } from '@comapeo/schema' */ /** @import { SetOptional } from 'type-fest' */ @@ -49,7 +50,7 @@ export default class TranslationApi { async put(value) { const identifiers = omit(value, ['message']) const docId = hashObject(identifiers) - const doc = await this.#dataType.getByDocId(docId, { mustBeFound: false }) + const doc = await this.#dataType.getByDocId(docId).catch(nullIfNotFound) if (doc) { return await this.#dataType.update(doc.versionId, value) } else { diff --git a/test-e2e/members.js b/test-e2e/members.js index bdc0c9b8..9618e4b7 100644 --- a/test-e2e/members.js +++ b/test-e2e/members.js @@ -4,6 +4,7 @@ import { randomBytes } from 'crypto' import { once } from 'node:events' import { + BLOCKED_ROLE_ID, COORDINATOR_ROLE_ID, CREATOR_ROLE, CREATOR_ROLE_ID, @@ -261,6 +262,59 @@ test('roles - creator role and role assignment', async (t) => { ) }) +test('role validation', async (t) => { + const managers = await createManagers(2, t) + const [creator, member] = managers + + const disconnectPeers = connectPeers(managers) + t.after(disconnectPeers) + + const projectId = await creator.createProject({ name: 'role test' }) + await invite({ + projectId, + invitor: creator, + invitees: [member], + roleId: MEMBER_ROLE_ID, + }) + + const projects = await Promise.all( + managers.map((manager) => manager.getProject(projectId)) + ) + const [creatorProject, memberProject] = projects + await waitForSync(projects, 'initial') + + assert.equal( + (await creatorProject.$member.getById(member.deviceId)).role.roleId, + MEMBER_ROLE_ID, + 'test setup: creator sees correct role for member' + ) + + await memberProject.$member.assignRole(member.deviceId, COORDINATOR_ROLE_ID, { + __testOnlyAllowAnyRoleToBeAssigned: true, + }) + await waitForSync(projects, 'initial') + + assert.equal( + (await creatorProject.$member.getById(member.deviceId)).role.roleId, + BLOCKED_ROLE_ID, + "creator sees member's bogus role assignment, and blocks them" + ) + + await creatorProject.$member.assignRole(member.deviceId, COORDINATOR_ROLE_ID) + assert.equal( + (await creatorProject.$member.getById(member.deviceId)).role.roleId, + COORDINATOR_ROLE_ID, + "creator can update the member's role" + ) + + await creatorProject.$member.assignRole(member.deviceId, MEMBER_ROLE_ID) + assert.equal( + (await creatorProject.$member.getById(member.deviceId)).role.roleId, + MEMBER_ROLE_ID, + "creator can update the member's role again" + ) +}) + test('roles - new device without role', async (t) => { const [manager] = await createManagers(1, t) diff --git a/test/data-type.js b/test/data-type.js index 1559f702..cc709a2f 100644 --- a/test/data-type.js +++ b/test/data-type.js @@ -209,14 +209,6 @@ test('getByDocId() throws if no document exists with that ID', async () => { await assert.rejects(() => dataType.getByDocId('foo bar'), NotFoundError) }) -test('getByDocId() can return null if no document exists with that ID', async () => { - const { dataType } = await testenv({ projectKey: randomBytes(32) }) - assert.equal( - await dataType.getByDocId('foo bar', { mustBeFound: false }), - null - ) -}) - test('delete()', async () => { const projectKey = randomBytes(32) const { dataType } = await testenv({ projectKey }) diff --git a/test/errors.js b/test/errors.js index 2e68dee8..5bfbd003 100644 --- a/test/errors.js +++ b/test/errors.js @@ -1,6 +1,6 @@ import test, { describe } from 'node:test' import assert from 'node:assert/strict' -import { NotFoundError } from '../src/errors.js' +import { NotFoundError, nullIfNotFound } from '../src/errors.js' describe('NotFoundError', () => { test('subclasses Error', () => { @@ -15,3 +15,13 @@ describe('NotFoundError', () => { assert.equal(new NotFoundError('foo').message, 'foo') }) }) + +describe('nullIfNotFound', () => { + test('returns null if passed a NotFoundError', () => { + assert.equal(nullIfNotFound(new NotFoundError()), null) + }) + + test('throws if passed something other than a NotFoundError', () => { + assert.throws(() => nullIfNotFound(new Error('foo')), { message: 'foo' }) + }) +}) diff --git a/test/lib/ponyfills.js b/test/lib/ponyfills.js index 34dd2ca9..93741275 100644 --- a/test/lib/ponyfills.js +++ b/test/lib/ponyfills.js @@ -1,48 +1,80 @@ -import test from 'node:test' +import test, { describe } from 'node:test' import assert from 'node:assert/strict' -import { abortSignalAny } from '../../src/lib/ponyfills.js' +import { abortSignalAny, setIsSubsetOf } from '../../src/lib/ponyfills.js' -test('abortSignalAny() handles empty iterables', () => { - assert.notEqual(abortSignalAny([]).aborted, 'not immediately aborted') - assert.notEqual(abortSignalAny(new Set()).aborted, 'not immediately aborted') -}) +describe('abortSignalAny', () => { + test('handles empty iterables', () => { + assert.notEqual(abortSignalAny([]).aborted, 'not immediately aborted') + assert.notEqual( + abortSignalAny(new Set()).aborted, + 'not immediately aborted' + ) + }) -test('abortSignalAny() aborts immediately if one of the arguments was aborted', () => { - const result = abortSignalAny([ - new AbortController().signal, - AbortSignal.abort('foo'), - AbortSignal.abort('ignored'), - new AbortController().signal, - ]) + test('aborts immediately if one of the arguments was aborted', () => { + const result = abortSignalAny([ + new AbortController().signal, + AbortSignal.abort('foo'), + AbortSignal.abort('ignored'), + new AbortController().signal, + ]) - assert(result.aborted, 'immediately aborted') - assert.equal(result.reason, 'foo', 'gets first abort reason') -}) + assert(result.aborted, 'immediately aborted') + assert.equal(result.reason, 'foo', 'gets first abort reason') + }) + + test('aborts as soon as one of its arguments aborts', () => { + const a = new AbortController() + const b = new AbortController() + const c = new AbortController() + + const result = abortSignalAny([a.signal, b.signal, c.signal]) + + assert.notEqual(result.aborted, 'not immediately aborted') + + b.abort('foo') + c.abort('ignored') -test('abortSignalAny() aborts as soon as one of its arguments aborts', () => { - const a = new AbortController() - const b = new AbortController() - const c = new AbortController() + assert(result.aborted, 'aborted') + assert.equal(result.reason, 'foo', 'gets first abort reason') + }) - const result = abortSignalAny([a.signal, b.signal, c.signal]) + test('handles non-array iterables', () => { + const a = new AbortController() + const b = new AbortController() + const c = new AbortController() - assert.notEqual(result.aborted, 'not immediately aborted') + const result = abortSignalAny(new Set([a.signal, b.signal, c.signal])) - b.abort('foo') - c.abort('ignored') + b.abort('foo') - assert(result.aborted, 'aborted') - assert.equal(result.reason, 'foo', 'gets first abort reason') + assert(result.aborted, 'aborted') + }) }) -test('abortSignalAny() handles non-array iterables', () => { - const a = new AbortController() - const b = new AbortController() - const c = new AbortController() +describe('setIsSubsetOf', () => { + const empty = new Set() + const justTwo = new Set([2]) + const evens = new Set([2, 4, 6]) + const odds = new Set([1, 3, 5]) + const firstSix = new Set([1, 2, 3, 4, 5, 6]) - const result = abortSignalAny(new Set([a.signal, b.signal, c.signal])) + test('sets are subsets of themselves', () => { + for (const set of [empty, justTwo, evens, odds, firstSix]) { + assert(setIsSubsetOf(set, set)) + } + }) - b.abort('foo') + test('returns true for subsets', () => { + assert(setIsSubsetOf(empty, justTwo)) + assert(setIsSubsetOf(justTwo, evens)) + assert(setIsSubsetOf(evens, firstSix)) + assert(setIsSubsetOf(odds, firstSix)) + }) - assert(result.aborted, 'aborted') + test('returns false for non-subsets', () => { + assert(!setIsSubsetOf(justTwo, empty)) + assert(!setIsSubsetOf(firstSix, evens)) + assert(!setIsSubsetOf(evens, odds)) + }) })