diff --git a/src/core-ownership.js b/src/core-ownership.js index 47bd65487..6eee2960a 100644 --- a/src/core-ownership.js +++ b/src/core-ownership.js @@ -16,6 +16,7 @@ import pDefer from 'p-defer' import { NAMESPACES } from './constants.js' import { TypedEmitter } from 'tiny-typed-emitter' import { omit } from './lib/omit.js' +import { NotFoundError } from './errors.js' /** * @import { * CoreOwnershipWithSignatures, @@ -86,13 +87,10 @@ export class CoreOwnership extends TypedEmitter { for (const namespace of NAMESPACES) { expressions.push(eq(table[`${namespace}CoreId`], coreId)) } - // prettier-ignore const result = (await this.#dataType[kSelect]()) .where(or.apply(null, expressions)) .get() - if (!result) { - throw new Error('NotFound') - } + if (!result) throw new NotFoundError() return result.docId } diff --git a/src/datastore/index.js b/src/datastore/index.js index 19b9ddd86..dab7458f2 100644 --- a/src/datastore/index.js +++ b/src/datastore/index.js @@ -5,6 +5,7 @@ import pDefer from 'p-defer' import { discoveryKey } from 'hypercore-crypto' import { NAMESPACE_SCHEMAS } from '../constants.js' import { createMap } from '../utils.js' +import { NotFoundError } from '../errors.js' /** @import { MapeoDoc } from '@comapeo/schema' */ /** @@ -182,7 +183,7 @@ export class DataStore extends TypedEmitter { const coreRecord = this.#coreManager.getCoreByDiscoveryKey(coreDiscoveryKey) if (!coreRecord) throw new Error('Invalid versionId') const block = await coreRecord.core.get(index, { wait: false }) - if (!block) throw new Error('Not Found') + if (!block) throw new NotFoundError('Not Found') return decode(block, { coreDiscoveryKey, index }) } @@ -202,9 +203,9 @@ export class DataStore extends TypedEmitter { async readRaw(versionId) { const { coreDiscoveryKey, index } = parseVersionId(versionId) const coreRecord = this.#coreManager.getCoreByDiscoveryKey(coreDiscoveryKey) - if (!coreRecord) throw new Error('core not found') + if (!coreRecord) throw new NotFoundError('core not found') const block = await coreRecord.core.get(index, { wait: false }) - if (!block) throw new Error('Not Found') + if (!block) throw new NotFoundError() return block } diff --git a/src/datatype/index.d.ts b/src/datatype/index.d.ts index 2a6f7d367..39922b265 100644 --- a/src/datatype/index.d.ts +++ b/src/datatype/index.d.ts @@ -87,8 +87,12 @@ export class DataType< getByDocId( docId: string, - opts?: { lang?: string } + opts?: { mustBeFound?: true; lang?: string } ): Promise + getByDocId( + docId: string, + opts?: { mustBeFound?: boolean; lang?: string } + ): Promise getByVersionId(versionId: string, opts?: { lang?: string }): Promise diff --git a/src/datatype/index.js b/src/datatype/index.js index 82459ca2c..1784987cd 100644 --- a/src/datatype/index.js +++ b/src/datatype/index.js @@ -164,16 +164,30 @@ export class DataType extends TypedEmitter { } /** + * @overload * @param {string} docId - * @param {{ lang?: string }} [opts] + * @param {object} [options] + * @param {true} [options.mustBeFound] + * @param {string} [options.lang] + * @returns {Promise} + */ + /** + * @param {string} docId + * @param {object} [options] + * @param {boolean} [options.mustBeFound] + * @param {string} [options.lang] + * @returns {Promise} */ - async getByDocId(docId, { lang } = {}) { + async getByDocId(docId, { mustBeFound = true, lang } = {}) { await this.#dataStore.indexer.idle() - const result = /** @type {undefined | MapeoDoc} */ ( - this.#sql.getByDocId.get({ docId }) - ) - if (!result) throw new NotFoundError() - return this.#translate(deNullify(result), { lang }) + const result = this.#sql.getByDocId.get({ docId }) + if (result) { + return this.#translate(deNullify(result), { lang }) + } else if (mustBeFound) { + throw new NotFoundError() + } else { + return null + } } /** @@ -186,7 +200,7 @@ export class DataType extends TypedEmitter { } /** - * @param {MapeoDoc} doc + * @param {any} doc * @param {{ lang?: string }} [opts] */ async #translate(doc, { lang } = {}) { @@ -278,7 +292,6 @@ export class DataType extends TypedEmitter { const doc = { ...existingDoc, updatedAt: new Date().toISOString(), - // @ts-expect-error - TS just doesn't work in this class links: [existingDoc.versionId, ...existingDoc.forks], deleted: true, } diff --git a/src/errors.js b/src/errors.js index 223461383..65bd6c968 100644 --- a/src/errors.js +++ b/src/errors.js @@ -1,5 +1,5 @@ export class NotFoundError extends Error { - constructor() { - super('Not found') + constructor(message = 'Not found') { + super(message) } } diff --git a/src/mapeo-manager.js b/src/mapeo-manager.js index 71fb26843..5df3d83d9 100644 --- a/src/mapeo-manager.js +++ b/src/mapeo-manager.js @@ -51,6 +51,7 @@ import { kRequestFullStop, kRescindFullStopRequest, } from './sync/sync-api.js' +import { NotFoundError } from './errors.js' /** @import { ProjectSettingsValue as ProjectValue } from '@comapeo/schema' */ /** @import NoiseSecretStream from '@hyperswarm/secret-stream' */ /** @import { SetNonNullable } from 'type-fest' */ @@ -456,7 +457,7 @@ export class MapeoManager extends TypedEmitter { .get() if (!projectKeysTableResult) { - throw new Error(`NotFound: project ID ${projectPublicId} not found`) + throw new NotFoundError(`Project ID ${projectPublicId} not found`) } const { projectId } = projectKeysTableResult @@ -896,7 +897,7 @@ export class MapeoManager extends TypedEmitter { .get() if (!row) { - throw new Error(`NotFound: project ID ${projectPublicId} not found`) + throw new NotFoundError(`Project ID ${projectPublicId} not found`) } const { keysCipher, projectId, projectInfo } = row diff --git a/src/mapeo-project.js b/src/mapeo-project.js index 2ece9dc55..331d64a62 100644 --- a/src/mapeo-project.js +++ b/src/mapeo-project.js @@ -56,6 +56,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 { ProjectSettingsValue } from '@comapeo/schema' */ /** @import { CoreStorage, KeyPair, Namespace, ReplicationStream } from './types.js' */ @@ -616,14 +617,9 @@ export class MapeoProject extends TypedEmitter { async $setProjectSettings(settings) { const { projectSettings } = this.#dataTypes - // We only want to catch the error to the getByDocId call - // Using try/catch for this is a little verbose when dealing with TS types - const existing = await projectSettings - .getByDocId(this.#projectId) - .catch(() => { - // project does not exist so return null - return null - }) + const existing = await projectSettings.getByDocId(this.#projectId, { + mustBeFound: false, + }) if (existing) { return extractEditableProjectSettings( @@ -676,7 +672,7 @@ export class MapeoProject extends TypedEmitter { const coreId = this.#coreManager .getCoreByDiscoveryKey(coreDiscoveryKey) ?.key.toString('hex') - if (!coreId) throw new Error('NotFound') + if (!coreId) throw new NotFoundError() return this.#coreOwnership.getOwner(coreId) } @@ -731,14 +727,14 @@ export class MapeoProject extends TypedEmitter { schemaName: /** @type {const} */ ('deviceInfo'), } - let existingDoc - try { - existingDoc = await deviceInfo.getByDocId(configCoreId) - } catch (err) { + const existingDoc = await deviceInfo.getByDocId(configCoreId, { + mustBeFound: false, + }) + if (existingDoc) { + return await deviceInfo.update(existingDoc.versionId, doc) + } else { return await deviceInfo[kCreateWithDocId](configCoreId, doc) } - - return deviceInfo.update(existingDoc.versionId, doc) } /** @param {boolean} isArchiveDevice */ @@ -900,7 +896,7 @@ export class MapeoProject extends TypedEmitter { const fieldRefs = fieldNames.map((fieldName) => { const fieldRef = fieldNameToRef.get(fieldName) if (!fieldRef) { - throw new Error( + throw new NotFoundError( `field ${fieldName} not found (referenced by preset ${value.name})})` ) } @@ -912,7 +908,7 @@ export class MapeoProject extends TypedEmitter { } const iconRef = iconNameToRef.get(iconName) if (!iconRef) { - throw new Error( + throw new NotFoundError( `icon ${iconName} not found (referenced by preset ${value.name})` ) } @@ -959,7 +955,7 @@ export class MapeoProject extends TypedEmitter { }) ) } else { - throw new Error( + throw new NotFoundError( `docRef for ${value.docRefType} with name ${name} not found` ) } diff --git a/src/roles.js b/src/roles.js index f3aa6566e..a7fc8dfb3 100644 --- a/src/roles.js +++ b/src/roles.js @@ -98,6 +98,33 @@ export const CREATOR_ROLE = { }, } +/** + * @type {Role} + */ +const BLOCKED_ROLE = { + roleId: BLOCKED_ROLE_ID, + name: 'Blocked', + docs: mapObject(currentSchemaVersions, (key) => { + return [ + key, + { + readOwn: false, + writeOwn: false, + readOthers: false, + writeOthers: false, + }, + ] + }), + roleAssignment: [], + sync: { + auth: 'blocked', + config: 'blocked', + data: 'blocked', + blobIndex: 'blocked', + blob: 'blocked', + }, +} + /** * This is the role assumed for a device when no role record can be found. This * can happen when an invited device did not manage to sync with the device that @@ -166,29 +193,7 @@ export const ROLES = { blob: 'allowed', }, }, - [BLOCKED_ROLE_ID]: { - roleId: BLOCKED_ROLE_ID, - name: 'Blocked', - docs: mapObject(currentSchemaVersions, (key) => { - return [ - key, - { - readOwn: false, - writeOwn: false, - readOthers: false, - writeOthers: false, - }, - ] - }), - roleAssignment: [], - sync: { - auth: 'blocked', - config: 'blocked', - data: 'blocked', - blobIndex: 'blocked', - blob: 'blocked', - }, - }, + [BLOCKED_ROLE_ID]: BLOCKED_ROLE, [LEFT_ROLE_ID]: { roleId: LEFT_ROLE_ID, name: 'Left', @@ -264,12 +269,10 @@ export class Roles extends TypedEmitter { * @returns {Promise} */ async getRole(deviceId) { - /** @type {string} */ - let roleId - try { - const roleAssignment = await this.#dataType.getByDocId(deviceId) - roleId = roleAssignment.roleId - } catch (e) { + 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) { @@ -280,8 +283,10 @@ export class Roles extends TypedEmitter { return NO_ROLE } } + + const { roleId } = roleAssignment if (!isRoleId(roleId)) { - return ROLES[BLOCKED_ROLE_ID] + return BLOCKED_ROLE } return ROLES[roleId] } @@ -381,9 +386,9 @@ export class Roles extends TypedEmitter { } } - const existingRoleDoc = await this.#dataType - .getByDocId(deviceId) - .catch(() => null) + const existingRoleDoc = await this.#dataType.getByDocId(deviceId, { + mustBeFound: false, + }) if (existingRoleDoc) { await this.#dataType.update( @@ -403,7 +408,7 @@ export class Roles extends TypedEmitter { } } - async #isProjectCreator() { + #isProjectCreator() { const ownAuthCoreId = this.#coreManager .getWriterCore('auth') .key.toString('hex') diff --git a/src/translation-api.js b/src/translation-api.js index 05d0a92eb..636a8a20f 100644 --- a/src/translation-api.js +++ b/src/translation-api.js @@ -1,7 +1,6 @@ import { and, sql } from 'drizzle-orm' import { kCreateWithDocId, kSelect } from './datatype/index.js' import { hashObject } from './utils.js' -import { NotFoundError } from './errors.js' import { omit } from './lib/omit.js' /** @import { Translation, TranslationValue } from '@comapeo/schema' */ /** @import { SetOptional } from 'type-fest' */ @@ -50,15 +49,11 @@ export default class TranslationApi { async put(value) { const identifiers = omit(value, ['message']) const docId = hashObject(identifiers) - try { - const doc = await this.#dataType.getByDocId(docId) + const doc = await this.#dataType.getByDocId(docId, { mustBeFound: false }) + if (doc) { return await this.#dataType.update(doc.versionId, value) - } catch (e) { - if (e instanceof NotFoundError) { - return await this.#dataType[kCreateWithDocId](docId, value) - } else { - throw new Error(`Error on translation ${e}`) - } + } else { + return await this.#dataType[kCreateWithDocId](docId, value) } } diff --git a/test-e2e/members.js b/test-e2e/members.js index b47f61f51..bdc0c9b81 100644 --- a/test-e2e/members.js +++ b/test-e2e/members.js @@ -6,6 +6,7 @@ import { once } from 'node:events' import { COORDINATOR_ROLE_ID, CREATOR_ROLE, + CREATOR_ROLE_ID, ROLES, MEMBER_ROLE_ID, NO_ROLE, @@ -18,6 +19,8 @@ import { waitForSync, } from './utils.js' import { kDataTypes } from '../src/mapeo-project.js' +/** @import { MapeoProject } from '../src/mapeo-project.js' */ +/** @import { RoleId } from '../src/roles.js' */ test('getting yourself after creating project', async (t) => { const [manager] = await createManagers(1, t, 'tablet') @@ -355,8 +358,8 @@ test('roles - getMany() on newly invited device before sync', async (t) => { }) test('roles - assignRole()', async (t) => { - const managers = await createManagers(2, t) - const [invitor, invitee] = managers + const managers = await createManagers(3, t) + const [invitor, invitee, invitee2] = managers const disconnectPeers = connectPeers(managers) t.after(disconnectPeers) @@ -365,7 +368,7 @@ test('roles - assignRole()', async (t) => { await invite({ invitor, projectId, - invitees: [invitee], + invitees: [invitee, invitee2], roleId: MEMBER_ROLE_ID, }) @@ -373,13 +376,40 @@ test('roles - assignRole()', async (t) => { managers.map((m) => m.getProject(projectId)) ) - const [invitorProject, inviteeProject] = projects + const [invitorProject, inviteeProject, invitee2Project] = projects + + /** + * @param {MapeoProject} project + * @param {string} otherDeviceId + * @param {RoleId} expectedRoleId + * @param {string} message + * @returns {Promise} + */ + const assertRole = async ( + project, + otherDeviceId, + expectedRoleId, + message + ) => { + assert.equal( + (await project.$member.getById(otherDeviceId)).role.roleId, + expectedRoleId, + message + ) + } - assert.deepEqual( - (await invitorProject.$member.getById(invitee.deviceId)).role, - ROLES[MEMBER_ROLE_ID], + await assertRole( + invitorProject, + invitee.deviceId, + MEMBER_ROLE_ID, 'invitee has member role from invitor perspective' ) + await assertRole( + invitorProject, + invitee2.deviceId, + MEMBER_ROLE_ID, + 'invitee 2 has member role from invitor perspective' + ) assert.deepEqual( await inviteeProject.$getOwnRole(), @@ -410,9 +440,10 @@ test('roles - assignRole()', async (t) => { await waitForSync(projects, 'initial') - assert.deepEqual( - (await invitorProject.$member.getById(invitee.deviceId)).role, - ROLES[COORDINATOR_ROLE_ID], + await assertRole( + invitorProject, + invitee.deviceId, + COORDINATOR_ROLE_ID, 'invitee now has coordinator role from invitor perspective' ) @@ -447,9 +478,10 @@ test('roles - assignRole()', async (t) => { await waitForSync(projects, 'initial') - assert.deepEqual( - (await invitorProject.$member.getById(invitee.deviceId)).role, - ROLES[MEMBER_ROLE_ID], + await assertRole( + invitorProject, + invitee.deviceId, + MEMBER_ROLE_ID, 'invitee now has member role from invitor perspective' ) @@ -459,6 +491,84 @@ test('roles - assignRole()', async (t) => { 'invitee now has member role from invitee perspective' ) }) + + await t.test( + 'regular members cannot assign roles to coordinator', + async () => { + await Promise.all( + [invitorProject, inviteeProject, invitee2Project].flatMap((project) => [ + assertRole( + project, + invitee.deviceId, + MEMBER_ROLE_ID, + 'test setup: everyone believes invitee 1 is a regular member' + ), + assertRole( + project, + invitee2.deviceId, + MEMBER_ROLE_ID, + 'test setup: everyone believes invitee 2 is a regular member' + ), + ]) + ) + + await assert.rejects(() => + inviteeProject.$member.assignRole(invitee.deviceId, COORDINATOR_ROLE_ID) + ) + await assert.rejects(() => + inviteeProject.$member.assignRole( + invitee2.deviceId, + COORDINATOR_ROLE_ID + ) + ) + + await waitForSync(projects, 'initial') + + await Promise.all( + [invitorProject, inviteeProject, invitee2Project].flatMap((project) => [ + assertRole( + project, + invitee.deviceId, + MEMBER_ROLE_ID, + 'everyone believes invitee 1 is a regular member, even after attempting to assign higher role' + ), + assertRole( + project, + invitee2.deviceId, + MEMBER_ROLE_ID, + 'everyone believes invitee 2 is a regular member, even after attempting to assign higher role' + ), + ]) + ) + } + ) + + await t.test( + 'non-creator members cannot change roles of creator', + async () => { + await invitorProject.$member.assignRole( + invitee.deviceId, + COORDINATOR_ROLE_ID + ) + await waitForSync(projects, 'initial') + + await assert.rejects(() => + inviteeProject.$member.assignRole(invitor.deviceId, COORDINATOR_ROLE_ID) + ) + + await waitForSync(projects, 'initial') + await Promise.all( + [invitorProject, inviteeProject, invitee2Project].map((project) => + assertRole( + project, + invitor.deviceId, + CREATOR_ROLE_ID, + 'everyone still believes creator to be a creator' + ) + ) + ) + } + ) }) test('roles - assignRole() with forked role', async (t) => { diff --git a/test-e2e/sync.js b/test-e2e/sync.js index 627f99f70..81c6cbb60 100644 --- a/test-e2e/sync.js +++ b/test-e2e/sync.js @@ -955,6 +955,12 @@ test('Correct sync state prior to data sync', async function (t) { managers.map((m) => m.getProject(projectId)) ) + for (const project of projects) { + const { remoteDeviceSyncState } = project.$sync.getState() + const otherDeviceCount = Object.keys(remoteDeviceSyncState).length + assert.equal(otherDeviceCount, COUNT - 1) + } + const generated = await seedDatabases(projects, { schemas: ['observation'] }) await waitForSync(projects, 'initial') diff --git a/test/data-type.js b/test/data-type.js index 297e3c4c0..1559f7028 100644 --- a/test/data-type.js +++ b/test/data-type.js @@ -114,6 +114,15 @@ test('private createWithDocId() method throws when doc exists', async () => { ) }) +test('getByVersionId fetches docs by their version ID', async () => { + const { dataType } = await testenv() + + const created = await dataType.create(obsFixture) + const fetched = await dataType.getByVersionId(created.versionId) + + assert.equal(created.docId, fetched.docId) +}) + test('`originalVersionId` field', async () => { const { dataType, dataStore } = await testenv() @@ -200,6 +209,14 @@ 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 new file mode 100644 index 000000000..2e68dee80 --- /dev/null +++ b/test/errors.js @@ -0,0 +1,17 @@ +import test, { describe } from 'node:test' +import assert from 'node:assert/strict' +import { NotFoundError } from '../src/errors.js' + +describe('NotFoundError', () => { + test('subclasses Error', () => { + assert(new NotFoundError() instanceof Error) + }) + + test('with no error message', () => { + assert.equal(new NotFoundError().message, 'Not found') + }) + + test('with custom error message', () => { + assert.equal(new NotFoundError('foo').message, 'foo') + }) +})