diff --git a/src/datatype/get-if-exists.js b/src/datatype/get-if-exists.js index b8cd7130..4f5f4f98 100644 --- a/src/datatype/get-if-exists.js +++ b/src/datatype/get-if-exists.js @@ -3,6 +3,20 @@ import { NotFoundError } from '../errors.js' /** @import { MapeoDocMap, MapeoValueMap } from '../types.js' */ /** @import { DataType, MapeoDocTables } from './index.js' */ +/** + * @template T + * @param {() => PromiseLike} fn + * @returns {Promise} + */ +async function nullIfNotFound(fn) { + try { + return await fn() + } catch (err) { + if (err instanceof NotFoundError) return null + throw err + } +} + /** * @template {MapeoDocTables} TTable * @template {TTable['_']['name']} TSchemaName @@ -12,11 +26,17 @@ import { NotFoundError } from '../errors.js' * @param {string} docId * @returns {Promise} */ -export async function getByDocIdIfExists(dataType, docId) { - try { - return await dataType.getByDocId(docId) - } catch (err) { - if (err instanceof NotFoundError) return null - throw err - } -} +export const getByDocIdIfExists = (dataType, docId) => + nullIfNotFound(() => dataType.getByDocId(docId)) + +/** + * @template {MapeoDocTables} TTable + * @template {TTable['_']['name']} TSchemaName + * @template {MapeoDocMap[TSchemaName]} TDoc + * @template {MapeoValueMap[TSchemaName]} TValue + * @param {DataType} dataType + * @param {string} versionId + * @returns {Promise} + */ +export const getByVersionIdIfExists = (dataType, versionId) => + nullIfNotFound(() => dataType.getByVersionId(versionId)) 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 261eaf5d..cdc396b7 100644 --- a/src/roles.js +++ b/src/roles.js @@ -1,9 +1,14 @@ -import { currentSchemaVersions } from '@comapeo/schema' +import { currentSchemaVersions, parseVersionId } from '@comapeo/schema' import mapObject from 'map-obj' import { kCreateWithDocId, kDataStore } from './datatype/index.js' import { assert, setHas } from './utils.js' -import { getByDocIdIfExists } from './datatype/get-if-exists.js' +import { + getByDocIdIfExists, + getByVersionIdIfExists, +} from './datatype/get-if-exists.js' import { TypedEmitter } from 'tiny-typed-emitter' +/** @import { Role as RoleRecord } from '@comapeo/schema' */ +/** @import { ReadonlyDeep } from 'type-fest' */ /** @import { Namespace } from './types.js' */ // Randomly generated 8-byte encoded as hex @@ -14,6 +19,8 @@ export const BLOCKED_ROLE_ID = '9e6d29263cba36c9' export const LEFT_ROLE_ID = '8ced989b1904606b' export const NO_ROLE_ID = '08e4251e36f6e7ed' +const CREATOR_ROLE_RECORD = Symbol('creator role assignment') + /** * @typedef {T extends Iterable ? U : never} ElementOf * @template T @@ -270,24 +277,143 @@ export class Roles extends TypedEmitter { * @returns {Promise} */ async getRole(deviceId) { - const roleRecord = await getByDocIdIfExists(this.#dataType, deviceId) - if (!roleRecord) { - // 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 roleRecord = await this.#getRoleRecord(deviceId) + + switch (roleRecord) { + case null: return NO_ROLE + case CREATOR_ROLE_RECORD: + return CREATOR_ROLE + default: { + const { roleId } = roleRecord + if (isRoleId(roleId) && (await this.#isRoleChainValid(roleRecord))) { + return ROLES[roleId] + } else { + return BLOCKED_ROLE + } } } + } + + /** + * @param {string} deviceId + * @returns {Promise} + */ + async #getRoleRecord(deviceId) { + const result = await getByDocIdIfExists(this.#dataType, deviceId) + if (result) return result - const { roleId } = roleRecord - if (!isRoleId(roleId)) { - return BLOCKED_ROLE + // The project creator will have the creator role + const authCoreId = await this.#coreOwnership.getCoreId(deviceId, 'auth') + if (authCoreId === this.#projectCreatorAuthCoreId) { + return CREATOR_ROLE_RECORD } - return ROLES[roleId] + + // When no role assignment exists, e.g. a newly added device which has + // not yet synced role records. + return null + } + + /** + * @param {ReadonlyDeep} roleRecord + * @returns {Promise} + */ + async #isRoleChainValid(roleRecord) { + if (roleRecord.roleId === LEFT_ROLE_ID) return true + + /** @type {null | ReadonlyDeep} */ + let currentRoleRecord = roleRecord + + while (currentRoleRecord) { + if (this.#isAssignedByProjectCreator(currentRoleRecord)) { + return true + } + + const parentRoleRecord = await this.#getParentRoleRecord( + currentRoleRecord + ) + switch (parentRoleRecord) { + case null: + break + case CREATOR_ROLE_RECORD: + return true + default: + if ( + !canAssign({ + assigner: parentRoleRecord, + assignee: currentRoleRecord, + }) + ) { + return false + } + break + } + + currentRoleRecord = parentRoleRecord + } + + return false + } + + /** + * @param {ReadonlyDeep>} roleRecord + * @returns {boolean} + */ + #isAssignedByProjectCreator({ versionId }) { + const { coreDiscoveryKey } = parseVersionId(versionId) + const coreDiscoveryKeyString = coreDiscoveryKey.toString('hex') + return coreDiscoveryKeyString === this.#projectCreatorAuthCoreId + } + + /** + * @param {ReadonlyDeep} roleRecord + * @returns {Promise} + */ + async #getParentRoleRecord(roleRecord) { + const { + coreDiscoveryKey: assignerCoreDiscoveryKey, + index: assignerIndexAtAssignmentTime, + } = parseVersionId(roleRecord.versionId) + + 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(() => null) + if (!assignerDeviceId) return null + + const latestRoleRecord = await this.#getRoleRecord(assignerDeviceId) + + let roleRecordToCheck = latestRoleRecord + /** @type {RoleRecord[]} */ const roleRecordsToCheck = [] + while (roleRecordToCheck) { + if ( + roleRecordToCheck === CREATOR_ROLE_RECORD || + (roleRecordToCheck.fromIndex <= assignerIndexAtAssignmentTime && + roleRecordToCheck.versionId !== roleRecord.versionId) + ) { + return roleRecordToCheck + } + + const linkedRoleRecords = await Promise.all( + roleRecordToCheck.links.map((linkedVersionId) => + getByVersionIdIfExists(this.#dataType, linkedVersionId) + ) + ) + for (const linkedRoleRecord of linkedRoleRecords) { + if (linkedRoleRecord) { + roleRecordsToCheck.push(linkedRoleRecord) + } + } + + roleRecordToCheck = roleRecordsToCheck.shift() || null + } + + return null } /** @@ -344,8 +470,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}` @@ -368,7 +500,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" ) @@ -380,7 +516,10 @@ 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) } } @@ -412,3 +551,24 @@ 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) && + roleRecordToRole(assigner).roleAssignment.includes(assignee.roleId) + ) +} + +/** + * @param {ReadonlyDeep>} roleRecord + * @returns {Role} + */ +function roleRecordToRole({ roleId }) { + return isRoleId(roleId) ? ROLES[roleId] : BLOCKED_ROLE +} 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/get-if-exists.js b/test/data-type/get-if-exists.js index f34021e0..2a70a751 100644 --- a/test/data-type/get-if-exists.js +++ b/test/data-type/get-if-exists.js @@ -1,8 +1,11 @@ import { testenv } from './test-helpers.js' import test, { describe } from 'node:test' import assert from 'node:assert/strict' -import { getByDocIdIfExists } from '../../src/datatype/get-if-exists.js' -import { valueOf } from '@comapeo/schema' +import { + getByDocIdIfExists, + getByVersionIdIfExists, +} from '../../src/datatype/get-if-exists.js' +import { getVersionId, parseVersionId, valueOf } from '@comapeo/schema' import { generate } from '@mapeo/mock-data' describe('getByDocIdIfExists', () => { @@ -18,3 +21,25 @@ describe('getByDocIdIfExists', () => { assert(await getByDocIdIfExists(dataType, observation.docId)) }) }) + +describe('getByVersionIdIfExists', () => { + test('resolves with null if no document exists with that ID', async () => { + const { dataType } = await testenv() + + const fixture = valueOf(generate('observation')[0]) + const observation = await dataType.create(fixture) + const bogusVersionId = getVersionId({ + ...parseVersionId(observation.versionId), + index: 9999999, + }) + + assert.equal(await getByVersionIdIfExists(dataType, bogusVersionId), null) + }) + + test('resolves with the document if it exists', async () => { + const { dataType } = await testenv() + const fixture = valueOf(generate('observation')[0]) + const observation = await dataType.create(fixture) + assert(await getByVersionIdIfExists(dataType, observation.versionId)) + }) +})