From 50a52713f8c0a4a3748a51b9d5686209b22a3b02 Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Tue, 29 Oct 2024 00:01:30 +0000 Subject: [PATCH] Send project name to server when adding it Closes [#912]. [#912]: https://github.com/digidem/comapeo-core/issues/912 --- src/mapeo-project.js | 8 +++ src/member-api.js | 16 ++++++ src/server/routes.js | 4 +- src/server/test/add-project-endpoint.js | 60 ++++++++++++++++------- src/server/test/list-projects-endpoint.js | 26 ++++++---- src/server/test/observations-endpoint.js | 8 +-- src/server/test/sync-endpoint.js | 8 +-- src/server/test/test-helpers.js | 17 ++++--- test-e2e/server.js | 31 +++++++++--- 9 files changed, 126 insertions(+), 52 deletions(-) diff --git a/src/mapeo-project.js b/src/mapeo-project.js index c58cb3ea..5afc70cb 100644 --- a/src/mapeo-project.js +++ b/src/mapeo-project.js @@ -321,6 +321,7 @@ export class MapeoProject extends TypedEmitter { roles: this.#roles, coreOwnership: this.#coreOwnership, encryptionKeys, + getProjectName: this.#getProjectName.bind(this), projectKey, rpc: localPeers, getReplicationStream, @@ -609,6 +610,13 @@ export class MapeoProject extends TypedEmitter { } } + /** + * @returns {Promise} + */ + async #getProjectName() { + return (await this.$getProjectSettings()).name + } + async $getOwnRole() { return this.#roles.getRole(this.#deviceId) } diff --git a/src/member-api.js b/src/member-api.js index 668bb2a1..a4454b41 100644 --- a/src/member-api.js +++ b/src/member-api.js @@ -27,6 +27,7 @@ import { wsCoreReplicator } from './server/ws-core-replicator.js' * ProjectSettingsValue * } from '@comapeo/schema' */ +/** @import { Promisable } from 'type-fest' */ /** @import { Invite, InviteResponse } from './generated/rpc.js' */ /** @import { DataType } from './datatype/index.js' */ /** @import { DataStore } from './datastore/index.js' */ @@ -52,6 +53,7 @@ export class MemberApi extends TypedEmitter { #roles #coreOwnership #encryptionKeys + #getProjectName #projectKey #rpc #getReplicationStream @@ -67,6 +69,7 @@ export class MemberApi extends TypedEmitter { * @param {import('./roles.js').Roles} opts.roles * @param {import('./core-ownership.js').CoreOwnership} opts.coreOwnership * @param {import('./generated/keys.js').EncryptionKeys} opts.encryptionKeys + * @param {() => Promisable} opts.getProjectName * @param {Buffer} opts.projectKey * @param {import('./local-peers.js').LocalPeers} opts.rpc * @param {() => ReplicationStream} opts.getReplicationStream @@ -80,6 +83,7 @@ export class MemberApi extends TypedEmitter { roles, coreOwnership, encryptionKeys, + getProjectName, projectKey, rpc, getReplicationStream, @@ -91,6 +95,7 @@ export class MemberApi extends TypedEmitter { this.#roles = roles this.#coreOwnership = coreOwnership this.#encryptionKeys = encryptionKeys + this.#getProjectName = getProjectName this.#projectKey = projectKey this.#rpc = rpc this.#getReplicationStream = getReplicationStream @@ -268,6 +273,8 @@ export class MemberApi extends TypedEmitter { * Can reject with any of the following error codes (accessed via `err.code`): * * - `INVALID_URL`: the base URL is invalid, likely due to user error. + * - `MISSING_DATA`: some required data is missing in order to add the server + * peer. For example, the project must have a name. * - `NETWORK_ERROR`: there was an issue connecting to the server. Is the * device online? Is the server online? * - `INVALID_SERVER_RESPONSE`: we connected to the server but it returned @@ -308,8 +315,17 @@ export class MemberApi extends TypedEmitter { * @returns {Promise<{ serverDeviceId: string }>} */ async #addServerToProject(baseUrl) { + const projectName = await this.#getProjectName() + if (!projectName) { + throw new ErrorWithCode( + 'MISSING_DATA', + 'Project must have name to add server peer' + ) + } + const requestUrl = new URL('projects', baseUrl) const requestBody = { + projectName, projectKey: encodeBufferForServer(this.#projectKey), encryptionKeys: { auth: encodeBufferForServer(this.#encryptionKeys.auth), diff --git a/src/server/routes.js b/src/server/routes.js index e9fb24e3..66285217 100644 --- a/src/server/routes.js +++ b/src/server/routes.js @@ -133,6 +133,7 @@ export default async function routes( { schema: { body: Type.Object({ + projectName: Type.String(), projectKey: HEX_STRING_32_BYTES, encryptionKeys: Type.Object({ auth: HEX_STRING_32_BYTES, @@ -153,6 +154,7 @@ export default async function routes( }, }, async function (req, reply) { + const { projectName } = req.body const projectKey = Buffer.from(req.body.projectKey, 'hex') const projectPublicId = projectKeyToPublicId(projectKey) @@ -207,7 +209,7 @@ export default async function routes( const projectId = await this.comapeo.addProject( { projectKey, - projectName: 'TODO: Figure out if this should be named', + projectName, encryptionKeys: { auth: Buffer.from(req.body.encryptionKeys.auth, 'hex'), config: Buffer.from(req.body.encryptionKeys.config, 'hex'), diff --git a/src/server/test/add-project-endpoint.js b/src/server/test/add-project-endpoint.js index 46e2c299..2d06793b 100644 --- a/src/server/test/add-project-endpoint.js +++ b/src/server/test/add-project-endpoint.js @@ -2,7 +2,31 @@ import assert from 'node:assert/strict' import test from 'node:test' import { omit } from '../../lib/omit.js' import { projectKeyToPublicId } from '../../utils.js' -import { createTestServer, randomProjectKeys } from './test-helpers.js' +import { createTestServer, randomAddProjectBody } from './test-helpers.js' + +test('request missing project name', async (t) => { + const server = createTestServer(t) + + const response = await server.inject({ + method: 'PUT', + url: '/projects', + body: omit(randomAddProjectBody(), ['projectName']), + }) + + assert.equal(response.statusCode, 400) +}) + +test('request with empty project name', async (t) => { + const server = createTestServer(t) + + const response = await server.inject({ + method: 'PUT', + url: '/projects', + body: { ...randomAddProjectBody(), projectName: '' }, + }) + + assert.equal(response.statusCode, 400) +}) test('request missing project key', async (t) => { const server = createTestServer(t) @@ -10,7 +34,7 @@ test('request missing project key', async (t) => { const response = await server.inject({ method: 'PUT', url: '/projects', - body: omit(randomProjectKeys(), ['projectKey']), + body: omit(randomAddProjectBody(), ['projectKey']), }) assert.equal(response.statusCode, 400) @@ -22,7 +46,7 @@ test('request missing any encryption keys', async (t) => { const response = await server.inject({ method: 'PUT', url: '/projects', - body: omit(randomProjectKeys(), ['encryptionKeys']), + body: omit(randomAddProjectBody(), ['encryptionKeys']), }) assert.equal(response.statusCode, 400) @@ -30,14 +54,14 @@ test('request missing any encryption keys', async (t) => { test('request missing an encryption key', async (t) => { const server = createTestServer(t) - const projectKeys = randomProjectKeys() + const body = randomAddProjectBody() const response = await server.inject({ method: 'PUT', url: '/projects', body: { - ...projectKeys, - encryptionKeys: omit(projectKeys.encryptionKeys, ['config']), + ...body, + encryptionKeys: omit(body.encryptionKeys, ['config']), }, }) @@ -50,7 +74,7 @@ test('adding a project', async (t) => { const response = await server.inject({ method: 'PUT', url: '/projects', - body: randomProjectKeys(), + body: randomAddProjectBody(), }) assert.equal(response.statusCode, 200) @@ -65,14 +89,14 @@ test('adding a second project fails by default', async (t) => { const firstAddResponse = await server.inject({ method: 'PUT', url: '/projects', - body: randomProjectKeys(), + body: randomAddProjectBody(), }) assert.equal(firstAddResponse.statusCode, 200) const response = await server.inject({ method: 'PUT', url: '/projects', - body: randomProjectKeys(), + body: randomAddProjectBody(), }) assert.equal(response.statusCode, 403) assert.match(response.json().message, /maximum number of projects/) @@ -86,7 +110,7 @@ test('allowing a maximum number of projects', async (t) => { const response = await server.inject({ method: 'PUT', url: '/projects', - body: randomProjectKeys(), + body: randomAddProjectBody(), }) assert.equal(response.statusCode, 200) } @@ -96,7 +120,7 @@ test('allowing a maximum number of projects', async (t) => { const response = await server.inject({ method: 'PUT', url: '/projects', - body: randomProjectKeys(), + body: randomAddProjectBody(), }) assert.equal(response.statusCode, 403) assert.match(response.json().message, /maximum number of projects/) @@ -107,9 +131,9 @@ test( 'allowing a specific list of projects', { concurrency: true }, async (t) => { - const projectKeys = randomProjectKeys() + const body = randomAddProjectBody() const projectPublicId = projectKeyToPublicId( - Buffer.from(projectKeys.projectKey, 'hex') + Buffer.from(body.projectKey, 'hex') ) const server = createTestServer(t, { allowedProjects: [projectPublicId], @@ -119,7 +143,7 @@ test( const response = await server.inject({ method: 'PUT', url: '/projects', - body: projectKeys, + body, }) assert.equal(response.statusCode, 200) }) @@ -128,7 +152,7 @@ test( const response = await server.inject({ method: 'PUT', url: '/projects', - body: randomProjectKeys(), + body: randomAddProjectBody(), }) assert.equal(response.statusCode, 403) }) @@ -137,19 +161,19 @@ test( test('adding the same project twice is idempotent', async (t) => { const server = createTestServer(t, { allowedProjects: 1 }) - const projectKeys = randomProjectKeys() + const body = randomAddProjectBody() const firstResponse = await server.inject({ method: 'PUT', url: '/projects', - body: projectKeys, + body, }) assert.equal(firstResponse.statusCode, 200) const secondResponse = await server.inject({ method: 'PUT', url: '/projects', - body: projectKeys, + body, }) assert.equal(secondResponse.statusCode, 200) }) diff --git a/src/server/test/list-projects-endpoint.js b/src/server/test/list-projects-endpoint.js index f1b8760b..40e28377 100644 --- a/src/server/test/list-projects-endpoint.js +++ b/src/server/test/list-projects-endpoint.js @@ -3,7 +3,7 @@ import test from 'node:test' import { BEARER_TOKEN, createTestServer, - randomProjectKeys, + randomAddProjectBody, } from './test-helpers.js' import { projectKeyToPublicId } from '../../utils.js' @@ -30,15 +30,15 @@ test('listing projects', async (t) => { }) await t.test('with projects', async () => { - const projectKeys1 = randomProjectKeys() - const projectKeys2 = randomProjectKeys() + const body1 = randomAddProjectBody() + const body2 = randomAddProjectBody() await Promise.all( - [projectKeys1, projectKeys2].map(async (projectKeys) => { + [body1, body2].map(async (body) => { const response = await server.inject({ method: 'PUT', url: '/projects', - body: projectKeys, + body, }) assert.equal(response.statusCode, 200) }) @@ -54,13 +54,19 @@ test('listing projects', async (t) => { const { data } = response.json() assert(Array.isArray(data)) assert.equal(data.length, 2, 'expected 2 projects') - for (const projectKeys of [projectKeys1, projectKeys2]) { + for (const body of [body1, body2]) { const projectPublicId = projectKeyToPublicId( - Buffer.from(projectKeys.projectKey, 'hex') + Buffer.from(body.projectKey, 'hex') ) - assert( - data.some((project) => project.projectId === projectPublicId), - `expected ${projectPublicId} to be found` + /** @type {Record} */ + const project = data.find( + (project) => project.projectId === projectPublicId + ) + assert(project, `expected ${projectPublicId} to be found`) + assert.equal( + project.name, + body.projectName, + 'expected project name to match' ) } }) diff --git a/src/server/test/observations-endpoint.js b/src/server/test/observations-endpoint.js index 4a7587c6..328ad7f5 100644 --- a/src/server/test/observations-endpoint.js +++ b/src/server/test/observations-endpoint.js @@ -10,7 +10,7 @@ import { createManager } from '../../../test-e2e/utils.js' import { BEARER_TOKEN, createTestServer, - randomProjectKeys, + randomAddProjectBody, } from './test-helpers.js' /** @import { ObservationValue } from '@comapeo/schema'*/ /** @import { FastifyInstance } from 'fastify' */ @@ -44,7 +44,7 @@ test('returns a 403 if incorrect auth is provided', async (t) => { test('returning no observations', async (t) => { const server = createTestServer(t) - const projectKeys = randomProjectKeys() + const projectKeys = randomAddProjectBody() const projectPublicId = projectKeyToPublicId( Buffer.from(projectKeys.projectKey, 'hex') ) @@ -72,7 +72,7 @@ test('returning observations with fetchable attachments', async (t) => { const serverUrl = new URL(serverAddress) const manager = createManager('client', t) - const projectId = await manager.createProject() + const projectId = await manager.createProject({ name: 'CoMapeo project' }) const project = await manager.getProject(projectId) await project.$member.addServerPeer(serverAddress, { @@ -161,7 +161,7 @@ test('returning observations with fetchable attachments', async (t) => { function randomProjectPublicId() { return projectKeyToPublicId( - Buffer.from(randomProjectKeys().projectKey, 'hex') + Buffer.from(randomAddProjectBody().projectKey, 'hex') ) } diff --git a/src/server/test/sync-endpoint.js b/src/server/test/sync-endpoint.js index af34109d..1ef34f04 100644 --- a/src/server/test/sync-endpoint.js +++ b/src/server/test/sync-endpoint.js @@ -1,13 +1,13 @@ import assert from 'node:assert/strict' import test from 'node:test' import { projectKeyToPublicId } from '../../utils.js' -import { createTestServer, randomProjectKeys } from './test-helpers.js' +import { createTestServer, randomAddProjectBody } from './test-helpers.js' test('sync endpoint is available after adding a project', async (t) => { const server = createTestServer(t) - const projectKeys = randomProjectKeys() + const addProjectBody = randomAddProjectBody() const projectPublicId = projectKeyToPublicId( - Buffer.from(projectKeys.projectKey, 'hex') + Buffer.from(addProjectBody.projectKey, 'hex') ) await t.test('sync endpoint not available yet', async () => { @@ -26,7 +26,7 @@ test('sync endpoint is available after adding a project', async (t) => { await server.inject({ method: 'PUT', url: '/projects', - body: projectKeys, + body: addProjectBody, }) await t.test('sync endpoint available', async (t) => { diff --git a/src/server/test/test-helpers.js b/src/server/test/test-helpers.js index be46b777..f3875e18 100644 --- a/src/server/test/test-helpers.js +++ b/src/server/test/test-helpers.js @@ -39,16 +39,17 @@ export function createTestServer(t, serverOptions) { return server } -const randomHexKey = (length = 32) => +const randomHex = (length = 32) => Buffer.from(randomBytes(length)).toString('hex') -export const randomProjectKeys = () => ({ - projectKey: randomHexKey(), +export const randomAddProjectBody = () => ({ + projectName: randomHex(16), + projectKey: randomHex(), encryptionKeys: { - auth: randomHexKey(), - config: randomHexKey(), - data: randomHexKey(), - blobIndex: randomHexKey(), - blob: randomHexKey(), + auth: randomHex(), + config: randomHex(), + data: randomHex(), + blobIndex: randomHex(), + blob: randomHex(), }, }) diff --git a/test-e2e/server.js b/test-e2e/server.js index 9f0dcfb4..5ded90fc 100644 --- a/test-e2e/server.js +++ b/test-e2e/server.js @@ -72,11 +72,28 @@ test('invalid base URLs', async (t) => { assert(!(await findServerPeer(project)), 'no server peers should be added') }) -test("fails if we can't connect to the server", async (t) => { +test('project with no name', async (t) => { const manager = createManager('device0', t) const projectId = await manager.createProject() const project = await manager.getProject(projectId) + await assert.rejects( + () => + project.$member.addServerPeer('http://localhost:9999', { + dangerouslyAllowInsecureConnections: true, + }), + { + code: 'MISSING_DATA', + message: /name/, + } + ) +}) + +test("fails if we can't connect to the server", async (t) => { + const manager = createManager('device0', t) + const projectId = await manager.createProject({ name: 'foo' }) + const project = await manager.getProject(projectId) + const serverBaseUrl = 'http://localhost:9999' await assert.rejects( () => @@ -95,7 +112,7 @@ test( { concurrency: true }, async (t) => { const manager = createManager('device0', t) - const projectId = await manager.createProject() + const projectId = await manager.createProject({ name: 'foo' }) const project = await manager.getProject(projectId) await Promise.all( @@ -129,7 +146,7 @@ test( { concurrency: true }, async (t) => { const manager = createManager('device0', t) - const projectId = await manager.createProject() + const projectId = await manager.createProject({ name: 'foo' }) const project = await manager.getProject(projectId) await Promise.all( @@ -167,7 +184,7 @@ test( test("fails if first request succeeds but sync doesn't", async (t) => { const manager = createManager('device0', t) - const projectId = await manager.createProject() + const projectId = await manager.createProject({ name: 'foo' }) const project = await manager.getProject(projectId) const fastify = createFastify() @@ -199,7 +216,7 @@ test("fails if first request succeeds but sync doesn't", async (t) => { test('adding a server peer', async (t) => { const manager = createManager('device0', t) - const projectId = await manager.createProject() + const projectId = await manager.createProject({ name: 'foo' }) const project = await manager.getProject(projectId) const { serverBaseUrl } = await createTestServer(t) @@ -287,8 +304,8 @@ test.skip('removing a server peer', async (t) => { test("can't add a server to two different projects", async (t) => { const [managerA, managerB] = await createManagers(2, t, 'mobile') - const projectIdA = await managerA.createProject() - const projectIdB = await managerB.createProject() + const projectIdA = await managerA.createProject({ name: 'project A' }) + const projectIdB = await managerB.createProject({ name: 'project B' }) const projectA = await managerA.getProject(projectIdA) const projectB = await managerB.getProject(projectIdB)