From 811591f49260f17524093c78aa98c608554d540e Mon Sep 17 00:00:00 2001 From: sethvincent Date: Wed, 23 Aug 2023 15:49:52 -0700 Subject: [PATCH 01/27] initial invite api --- src/invite-api.js | 76 ++++++++++++++++++++++++++++++++++++++++++++ tests/helpers/rpc.js | 34 ++++++++++++++++++++ tests/invite-api.js | 49 ++++++++++++++++++++++++++++ tests/rpc.js | 35 +------------------- 4 files changed, 160 insertions(+), 34 deletions(-) create mode 100644 src/invite-api.js create mode 100644 tests/helpers/rpc.js create mode 100644 tests/invite-api.js diff --git a/src/invite-api.js b/src/invite-api.js new file mode 100644 index 000000000..681a8d147 --- /dev/null +++ b/src/invite-api.js @@ -0,0 +1,76 @@ +import { TypedEmitter } from 'tiny-typed-emitter' +import { InviteResponse_Decision } from './generated/rpc.js' +import { idToKey, keyToId } from './utils.js' + +/** @typedef {import('./rpc/index.js').MapeoRPC} MapeoRPC */ + +export class InviteApi extends TypedEmitter { + // TODO: are invites persisted beyond this api? + #invites = new Map() + + /** + * @param {Object} options + * @param {MapeoRPC} options.rpc + */ + constructor({ rpc }) { + super() + this.rpc = rpc + + // TODO: I'm not seeing encryption keys used in the inviteResponse in the rpc api + // what is the purpose of the encryption keys at this stage of the process or afterward? + this.rpc.on('invite', (peerId, invite) => { + const projectId = keyToId(invite.projectKey) + const peerIds = this.#invites.get(projectId) || new Set() + peerIds.add(peerId) + this.#invites.set(projectId, peerIds) + + if (peerIds.size === 1) { + this.emit('invite-received', { + projectId, + peerId, + }) + } + }) + } + + /** + * @param {string} projectId + */ + accept(projectId) { + this.#respond({ projectId, decision: InviteResponse_Decision.ACCEPT }) + } + + /** + * @param {string} projectId + */ + reject(projectId) { + this.#respond({ projectId, decision: InviteResponse_Decision.REJECT }) + } + + /** + * @param {Object} options + * @param {string} options.projectId + * @param {InviteResponse_Decision} options.decision + */ + #respond({ projectId, decision }) { + const peerIds = this.#getPeerIds(projectId) + const projectKey = idToKey(projectId) + + // TODO: should this reply to one peer with `ACCEPT` and the rest with `ALREADY`? + // How is the `ALREADY` decision determined? It looks like that isn't used yet. + // Does anything bad happen if we respond to multiple peers with `ACCEPT`? + for (const peerId of peerIds.values()) { + this.rpc.inviteResponse(peerId, { projectKey, decision }) + } + + this.#invites.delete(projectId) + } + + /** + * @param {string} projectId + * @returns {Set} peerIds + */ + #getPeerIds(projectId) { + return this.#invites.get(projectId) || new Set() + } +} diff --git a/tests/helpers/rpc.js b/tests/helpers/rpc.js new file mode 100644 index 000000000..3070eeba8 --- /dev/null +++ b/tests/helpers/rpc.js @@ -0,0 +1,34 @@ +import NoiseSecretStream from '@hyperswarm/secret-stream' + +export function replicate(rpc1, rpc2) { + const n1 = new NoiseSecretStream(true, undefined, { + // Keep keypairs deterministic for tests, since we use peer.publicKey as an identifier. + keyPair: NoiseSecretStream.keyPair(Buffer.allocUnsafe(32).fill(0)), + }) + const n2 = new NoiseSecretStream(false, undefined, { + keyPair: NoiseSecretStream.keyPair(Buffer.allocUnsafe(32).fill(1)), + }) + n1.rawStream.pipe(n2.rawStream).pipe(n1.rawStream) + + rpc1.connect(n1) + rpc2.connect(n2) + + return async function destroy() { + return Promise.all([ + /** @type {Promise} */ + ( + new Promise((res) => { + n1.on('close', res) + n1.destroy() + }) + ), + /** @type {Promise} */ + ( + new Promise((res) => { + n2.on('close', res) + n2.destroy() + }) + ), + ]) + } +} diff --git a/tests/invite-api.js b/tests/invite-api.js new file mode 100644 index 000000000..9621c4f5c --- /dev/null +++ b/tests/invite-api.js @@ -0,0 +1,49 @@ +// @ts-check +import test from 'brittle' +import { MapeoRPC } from '../src/rpc/index.js' +import { InviteApi } from '../src/invite-api.js' +import { replicate } from './helpers/rpc.js' + +test('Accept invite', async (t) => { + t.plan(3) + const r1 = new MapeoRPC() + const r2 = new MapeoRPC() + const inviteApi = new InviteApi({ rpc: r2 }) + + const projectKey = Buffer.allocUnsafe(32).fill(0) + + r1.on('peers', async (peers) => { + t.is(peers.length, 1) + const response = await r1.invite(peers[0].id, { projectKey }) + t.is(response, MapeoRPC.InviteResponse.ACCEPT) + }) + + inviteApi.on('invite-received', ({ projectId }) => { + t.is(projectId, projectKey.toString('hex')) + inviteApi.accept(projectId) + }) + + replicate(r1, r2) +}) + +test('Reject invite', async (t) => { + t.plan(3) + const r1 = new MapeoRPC() + const r2 = new MapeoRPC() + const inviteApi = new InviteApi({ rpc: r2 }) + + const projectKey = Buffer.allocUnsafe(32).fill(0) + + r1.on('peers', async (peers) => { + t.is(peers.length, 1) + const response = await r1.invite(peers[0].id, { projectKey }) + t.is(response, MapeoRPC.InviteResponse.REJECT) + }) + + inviteApi.on('invite-received', ({ projectId }) => { + t.is(projectId, projectKey.toString('hex')) + inviteApi.reject(projectId) + }) + + replicate(r1, r2) +}) diff --git a/tests/rpc.js b/tests/rpc.js index 1c187c71e..0e3e258f6 100644 --- a/tests/rpc.js +++ b/tests/rpc.js @@ -1,6 +1,5 @@ // @ts-check import test from 'brittle' -import NoiseSecretStream from '@hyperswarm/secret-stream' import { MapeoRPC, PeerDisconnectedError, @@ -10,6 +9,7 @@ import { import FakeTimers from '@sinonjs/fake-timers' import { once } from 'events' import { Duplex } from 'streamx' +import { replicate } from './helpers/rpc.js' test('Send invite and accept', async (t) => { t.plan(3) @@ -365,36 +365,3 @@ test('invalid stream', (t) => { const regularStream = new Duplex() t.exception(() => r1.connect(regularStream), 'Invalid stream') }) - -function replicate(rpc1, rpc2) { - const n1 = new NoiseSecretStream(true, undefined, { - // Keep keypairs deterministic for tests, since we use peer.publicKey as an identifier. - keyPair: NoiseSecretStream.keyPair(Buffer.allocUnsafe(32).fill(0)), - }) - const n2 = new NoiseSecretStream(false, undefined, { - keyPair: NoiseSecretStream.keyPair(Buffer.allocUnsafe(32).fill(1)), - }) - n1.rawStream.pipe(n2.rawStream).pipe(n1.rawStream) - - rpc1.connect(n1) - rpc2.connect(n2) - - return async function destroy() { - return Promise.all([ - /** @type {Promise} */ - ( - new Promise((res) => { - n1.on('close', res) - n1.destroy() - }) - ), - /** @type {Promise} */ - ( - new Promise((res) => { - n2.on('close', res) - n2.destroy() - }) - ), - ]) - } -} From 88d06c9801b2166057c78bc542c85319988d6550 Mon Sep 17 00:00:00 2001 From: sethvincent Date: Wed, 23 Aug 2023 17:03:49 -0700 Subject: [PATCH 02/27] small type fixes --- src/generated/rpc.ts | 2 +- src/utils.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/generated/rpc.ts b/src/generated/rpc.ts index 08e1f410f..913721291 100644 --- a/src/generated/rpc.ts +++ b/src/generated/rpc.ts @@ -14,7 +14,7 @@ export interface Invite_ProjectInfo { } export interface InviteResponse { - projectKey: Buffer; + projectKey: Buffer | Uint8Array; decision: InviteResponse_Decision; } diff --git a/src/utils.js b/src/utils.js index cae516650..04837e322 100644 --- a/src/utils.js +++ b/src/utils.js @@ -14,7 +14,7 @@ export function idToKey(id) { /** * - * @param {Buffer|String} key + * @param {Buffer|Uint8Array|String} key * @returns {String} */ export function keyToId(key) { From 9011e1a0ed864fd690ba091e9bb60f009870e8f7 Mon Sep 17 00:00:00 2001 From: sethvincent Date: Thu, 31 Aug 2023 15:29:27 -0700 Subject: [PATCH 03/27] speculative isMember and addProject function usage --- src/invite-api.js | 95 +++++++++++++++++++++++++++++++++++++---------- 1 file changed, 75 insertions(+), 20 deletions(-) diff --git a/src/invite-api.js b/src/invite-api.js index 681a8d147..c58803dfc 100644 --- a/src/invite-api.js +++ b/src/invite-api.js @@ -5,30 +5,42 @@ import { idToKey, keyToId } from './utils.js' /** @typedef {import('./rpc/index.js').MapeoRPC} MapeoRPC */ export class InviteApi extends TypedEmitter { - // TODO: are invites persisted beyond this api? #invites = new Map() + #keys = new Map() + + #isMember + #addProject /** * @param {Object} options * @param {MapeoRPC} options.rpc + * @param {object} options.queries + * @param {(projectId: string) => Promise} options.queries.isMember + * @param {(projectId: string, encryptionKeys: import('./types.js').KeyPair) => Promise} options.queries.addProject */ - constructor({ rpc }) { + constructor({ rpc, queries }) { super() this.rpc = rpc + this.#isMember = queries.isMember + this.#addProject = queries.addProject - // TODO: I'm not seeing encryption keys used in the inviteResponse in the rpc api - // what is the purpose of the encryption keys at this stage of the process or afterward? - this.rpc.on('invite', (peerId, invite) => { + this.rpc.on('invite', async (peerId, invite) => { const projectId = keyToId(invite.projectKey) - const peerIds = this.#invites.get(projectId) || new Set() - peerIds.add(peerId) - this.#invites.set(projectId, peerIds) - - if (peerIds.size === 1) { - this.emit('invite-received', { - projectId, - peerId, - }) + + if (await this.#isMember(projectId)) { + this.alreadyJoined(projectId) + } else { + const peerIds = this.#invites.get(projectId) || new Set() + peerIds.add(peerId) + this.#invites.set(projectId, peerIds) + this.#keys.set(projectId, invite.encryptionKeys) + + if (peerIds.size === 1) { + this.emit('invite-received', { + projectId, + peerId, + }) + } } }) } @@ -47,23 +59,66 @@ export class InviteApi extends TypedEmitter { this.#respond({ projectId, decision: InviteResponse_Decision.REJECT }) } + /** + * @param {string} projectId + */ + alreadyJoined(projectId) { + this.#respond({ projectId, decision: InviteResponse_Decision.ALREADY }) + } + /** * @param {Object} options * @param {string} options.projectId * @param {InviteResponse_Decision} options.decision */ #respond({ projectId, decision }) { - const peerIds = this.#getPeerIds(projectId) + const peerIds = Array.from(this.#getPeerIds(projectId)) + const encryptionKeys = this.#keys.get(projectId) const projectKey = idToKey(projectId) - // TODO: should this reply to one peer with `ACCEPT` and the rest with `ALREADY`? - // How is the `ALREADY` decision determined? It looks like that isn't used yet. - // Does anything bad happen if we respond to multiple peers with `ACCEPT`? - for (const peerId of peerIds.values()) { - this.rpc.inviteResponse(peerId, { projectKey, decision }) + let connectedPeerId + let remainingPeerIds = [] + + for (const peerId of peerIds) { + if (!connectedPeerId && this.#isPeerConnected(peerId)) { + connectedPeerId = peerId + } else if (this.#isPeerConnected(peerId)) { + remainingPeerIds.push(peerId) + } + } + + if (!connectedPeerId) { + throw new Error('No connected peer to respond to') + } + + this.rpc.inviteResponse(connectedPeerId, { projectKey, decision }) + + if (decision === InviteResponse_Decision.ACCEPT) { + this.#addProject(projectId, encryptionKeys) + + for (const peerId of remainingPeerIds) { + this.alreadyJoined(projectId) + } + } else if (decision === InviteResponse_Decision.REJECT) { + for (const peerId of remainingPeerIds) { + this.reject(projectId) + } + } else if (decision === InviteResponse_Decision.ALREADY) { + for (const peerId of remainingPeerIds) { + this.alreadyJoined(projectId) + } } this.#invites.delete(projectId) + this.#keys.delete(projectId) + } + + /** + * @param {string} peerId + * @returns {boolean} + */ + #isPeerConnected(peerId) { + return this.rpc.peers.some((peer) => peer.id === peerId) } /** From 057d8d668adeb7abb0fae40069edd7f067693a63 Mon Sep 17 00:00:00 2001 From: Andrew Chou Date: Tue, 19 Sep 2023 13:33:34 -0400 Subject: [PATCH 04/27] fix type errors in tests --- tests/invite-api.js | 51 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 44 insertions(+), 7 deletions(-) diff --git a/tests/invite-api.js b/tests/invite-api.js index 9621c4f5c..8c88e973e 100644 --- a/tests/invite-api.js +++ b/tests/invite-api.js @@ -1,5 +1,6 @@ -// @ts-check import test from 'brittle' +import { randomBytes } from 'crypto' +import { KeyManager } from '@mapeo/crypto' import { MapeoRPC } from '../src/rpc/index.js' import { InviteApi } from '../src/invite-api.js' import { replicate } from './helpers/rpc.js' @@ -8,13 +9,31 @@ test('Accept invite', async (t) => { t.plan(3) const r1 = new MapeoRPC() const r2 = new MapeoRPC() - const inviteApi = new InviteApi({ rpc: r2 }) - const projectKey = Buffer.allocUnsafe(32).fill(0) + const members = new Set() + const projects = new Map() + + const inviteApi = new InviteApi({ + rpc: r2, + queries: { + isMember: async (deviceId) => { + return members.has(deviceId) + }, + addProject: async (projectId, encryptionKeys) => { + projects.set(projectId, encryptionKeys) + }, + }, + }) + + const projectKey = KeyManager.generateProjectKeypair().publicKey + const encryptionKeys = { auth: randomBytes(32) } r1.on('peers', async (peers) => { t.is(peers.length, 1) - const response = await r1.invite(peers[0].id, { projectKey }) + const response = await r1.invite(peers[0].id, { + projectKey, + encryptionKeys, + }) t.is(response, MapeoRPC.InviteResponse.ACCEPT) }) @@ -30,13 +49,31 @@ test('Reject invite', async (t) => { t.plan(3) const r1 = new MapeoRPC() const r2 = new MapeoRPC() - const inviteApi = new InviteApi({ rpc: r2 }) - const projectKey = Buffer.allocUnsafe(32).fill(0) + const members = new Set() + const projects = new Map() + + const inviteApi = new InviteApi({ + rpc: r2, + queries: { + isMember: async (deviceId) => { + return members.has(deviceId) + }, + addProject: async (projectId, encryptionKeys) => { + projects.set(projectId, encryptionKeys) + }, + }, + }) + + const projectKey = KeyManager.generateProjectKeypair().publicKey + const encryptionKeys = { auth: randomBytes(32) } r1.on('peers', async (peers) => { t.is(peers.length, 1) - const response = await r1.invite(peers[0].id, { projectKey }) + const response = await r1.invite(peers[0].id, { + projectKey, + encryptionKeys, + }) t.is(response, MapeoRPC.InviteResponse.REJECT) }) From 754ea9c3605d02a1da5fe819b00f209d8549509c Mon Sep 17 00:00:00 2001 From: Andrew Chou Date: Tue, 19 Sep 2023 13:41:36 -0400 Subject: [PATCH 05/27] revert manual change to generated rpc type --- src/generated/rpc.ts | 2 +- src/invite-api.js | 11 +++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/generated/rpc.ts b/src/generated/rpc.ts index eb5b5d358..a075be9ba 100644 --- a/src/generated/rpc.ts +++ b/src/generated/rpc.ts @@ -14,7 +14,7 @@ export interface Invite_ProjectInfo { } export interface InviteResponse { - projectKey: Buffer | Uint8Array; + projectKey: Buffer; decision: InviteResponse_Decision; } diff --git a/src/invite-api.js b/src/invite-api.js index 9bd17b646..a0f18a822 100644 --- a/src/invite-api.js +++ b/src/invite-api.js @@ -1,7 +1,7 @@ import { TypedEmitter } from 'tiny-typed-emitter' import { InviteResponse_Decision } from './generated/rpc.js' import { EncryptionKeys } from './generated/keys.js' -import { idToKey, keyToId } from './utils.js' +import { projectKeyToId } from './utils.js' /** @typedef {import('./rpc/index.js').MapeoRPC} MapeoRPC */ @@ -26,7 +26,7 @@ export class InviteApi extends TypedEmitter { this.#addProject = queries.addProject this.rpc.on('invite', async (peerId, invite) => { - const projectId = keyToId(invite.projectKey) + const projectId = projectKeyToId(invite.projectKey) if (await this.#isMember(projectId)) { this.alreadyJoined(projectId) @@ -75,7 +75,7 @@ export class InviteApi extends TypedEmitter { #respond({ projectId, decision }) { const peerIds = Array.from(this.#getPeerIds(projectId)) const encryptionKeys = this.#keys.get(projectId) - const projectKey = idToKey(projectId) + const projectKey = Buffer.from(projectId, 'hex') let connectedPeerId let remainingPeerIds = [] @@ -92,7 +92,10 @@ export class InviteApi extends TypedEmitter { throw new Error('No connected peer to respond to') } - this.rpc.inviteResponse(connectedPeerId, { projectKey, decision }) + this.rpc.inviteResponse(connectedPeerId, { + projectKey, + decision, + }) if (decision === InviteResponse_Decision.ACCEPT) { this.#addProject(projectId, encryptionKeys) From 09ec579a1165d98db5ace3adccd41480bc2383fc Mon Sep 17 00:00:00 2001 From: Andrew Chou Date: Tue, 19 Sep 2023 13:57:46 -0400 Subject: [PATCH 06/27] types improvements --- src/invite-api.js | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/invite-api.js b/src/invite-api.js index a0f18a822..131e068cb 100644 --- a/src/invite-api.js +++ b/src/invite-api.js @@ -1,12 +1,13 @@ +// @ts-check import { TypedEmitter } from 'tiny-typed-emitter' import { InviteResponse_Decision } from './generated/rpc.js' import { EncryptionKeys } from './generated/keys.js' import { projectKeyToId } from './utils.js' -/** @typedef {import('./rpc/index.js').MapeoRPC} MapeoRPC */ - export class InviteApi extends TypedEmitter { + /** @type {Map>} */ #invites = new Map() + /** @type {Map} */ #keys = new Map() #isMember @@ -14,7 +15,7 @@ export class InviteApi extends TypedEmitter { /** * @param {Object} options - * @param {MapeoRPC} options.rpc + * @param {import('./rpc/index.js').MapeoRPC} options.rpc * @param {object} options.queries * @param {(projectId: string) => Promise} options.queries.isMember * @param {(projectId: string, encryptionKeys: EncryptionKeys) => Promise} options.queries.addProject @@ -98,6 +99,12 @@ export class InviteApi extends TypedEmitter { }) if (decision === InviteResponse_Decision.ACCEPT) { + if (!encryptionKeys) { + throw new Error( + `Missing encryption keys for project with ID ${projectId}` + ) + } + this.#addProject(projectId, encryptionKeys) for (const peerId of remainingPeerIds) { From 1d30037afa3395486f6bcea228cef96f519121e4 Mon Sep 17 00:00:00 2001 From: Andrew Chou Date: Wed, 20 Sep 2023 11:22:22 -0400 Subject: [PATCH 07/27] variable names and types updates --- src/invite-api.js | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/src/invite-api.js b/src/invite-api.js index 131e068cb..22ab6e7b5 100644 --- a/src/invite-api.js +++ b/src/invite-api.js @@ -1,14 +1,16 @@ // @ts-check import { TypedEmitter } from 'tiny-typed-emitter' import { InviteResponse_Decision } from './generated/rpc.js' -import { EncryptionKeys } from './generated/keys.js' -import { projectKeyToId } from './utils.js' +import { projectKeyToId, projectKeyToPublicId } from './utils.js' export class InviteApi extends TypedEmitter { + // Maps project id -> set of device ids /** @type {Map>} */ + #peersToRespondTo = new Map() + + // Maps project id -> invite + /** @type {Map} */ #invites = new Map() - /** @type {Map} */ - #keys = new Map() #isMember #addProject @@ -18,7 +20,7 @@ export class InviteApi extends TypedEmitter { * @param {import('./rpc/index.js').MapeoRPC} options.rpc * @param {object} options.queries * @param {(projectId: string) => Promise} options.queries.isMember - * @param {(projectId: string, encryptionKeys: EncryptionKeys) => Promise} options.queries.addProject + * @param {(invite: import('./generated/rpc.js').Invite) => Promise} options.queries.addProject */ constructor({ rpc, queries }) { super() @@ -32,10 +34,10 @@ export class InviteApi extends TypedEmitter { if (await this.#isMember(projectId)) { this.alreadyJoined(projectId) } else { - const peerIds = this.#invites.get(projectId) || new Set() + const peerIds = this.#peersToRespondTo.get(projectId) || new Set() peerIds.add(peerId) - this.#invites.set(projectId, peerIds) - this.#keys.set(projectId, invite.encryptionKeys) + this.#peersToRespondTo.set(projectId, peerIds) + this.#invites.set(projectId, invite) if (peerIds.size === 1) { this.emit('invite-received', { @@ -75,7 +77,7 @@ export class InviteApi extends TypedEmitter { */ #respond({ projectId, decision }) { const peerIds = Array.from(this.#getPeerIds(projectId)) - const encryptionKeys = this.#keys.get(projectId) + const invite = this.#invites.get(projectId) const projectKey = Buffer.from(projectId, 'hex') let connectedPeerId @@ -99,13 +101,13 @@ export class InviteApi extends TypedEmitter { }) if (decision === InviteResponse_Decision.ACCEPT) { - if (!encryptionKeys) { + if (!invite) { throw new Error( - `Missing encryption keys for project with ID ${projectId}` + `Cannot find invite for project with ID ${projectKeyToPublicId}` ) } - this.#addProject(projectId, encryptionKeys) + this.#addProject(invite) for (const peerId of remainingPeerIds) { this.alreadyJoined(projectId) @@ -120,8 +122,8 @@ export class InviteApi extends TypedEmitter { } } + this.#peersToRespondTo.delete(projectId) this.#invites.delete(projectId) - this.#keys.delete(projectId) } /** @@ -137,6 +139,6 @@ export class InviteApi extends TypedEmitter { * @returns {Set} peerIds */ #getPeerIds(projectId) { - return this.#invites.get(projectId) || new Set() + return this.#peersToRespondTo.get(projectId) || new Set() } } From 284cffe8723f8ab436cf32c00dcc7dd465fc9c4f Mon Sep 17 00:00:00 2001 From: Andrew Chou Date: Wed, 20 Sep 2023 11:25:20 -0400 Subject: [PATCH 08/27] revert unused type change in keyToId util --- src/utils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils.js b/src/utils.js index 19abb4020..0938fed17 100644 --- a/src/utils.js +++ b/src/utils.js @@ -15,7 +15,7 @@ export function idToKey(id) { /** * - * @param {Buffer|Uint8Array|String} key + * @param {Buffer|String} key * @returns {String} */ export function keyToId(key) { From fafbef51f344a55622be0f61b36cbd1083ded89d Mon Sep 17 00:00:00 2001 From: Andrew Chou Date: Wed, 20 Sep 2023 11:27:17 -0400 Subject: [PATCH 09/27] revert change in tests/rpc.js to clean up PR diff --- tests/rpc.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/rpc.js b/tests/rpc.js index b1ec22c92..1119c42ca 100644 --- a/tests/rpc.js +++ b/tests/rpc.js @@ -1,6 +1,5 @@ // @ts-check import test from 'brittle' -import { randomBytes } from 'node:crypto' import { MapeoRPC, PeerDisconnectedError, @@ -11,7 +10,7 @@ import FakeTimers from '@sinonjs/fake-timers' import { once } from 'events' import { Duplex } from 'streamx' import { replicate } from './helpers/rpc.js' - +import { randomBytes } from 'node:crypto' test('Send invite and accept', async (t) => { t.plan(3) From a7f6e854ca830ddcbf500a9932ec2cd194263ce7 Mon Sep 17 00:00:00 2001 From: Andrew Chou Date: Wed, 20 Sep 2023 11:32:26 -0400 Subject: [PATCH 10/27] fix lint errors --- src/invite-api.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/invite-api.js b/src/invite-api.js index 22ab6e7b5..afd702b33 100644 --- a/src/invite-api.js +++ b/src/invite-api.js @@ -109,15 +109,18 @@ export class InviteApi extends TypedEmitter { this.#addProject(invite) - for (const peerId of remainingPeerIds) { + // eslint-disable-next-line no-unused-vars + for (const _peerId of remainingPeerIds) { this.alreadyJoined(projectId) } } else if (decision === InviteResponse_Decision.REJECT) { - for (const peerId of remainingPeerIds) { + // eslint-disable-next-line no-unused-vars + for (const _peerId of remainingPeerIds) { this.reject(projectId) } } else if (decision === InviteResponse_Decision.ALREADY) { - for (const peerId of remainingPeerIds) { + // eslint-disable-next-line no-unused-vars + for (const _peerId of remainingPeerIds) { this.alreadyJoined(projectId) } } From fc3ef15a86ebcb1269faa21e2dd85de1913582bf Mon Sep 17 00:00:00 2001 From: Andrew Chou Date: Wed, 20 Sep 2023 11:56:03 -0400 Subject: [PATCH 11/27] fix usage of projectKeyToPublicId --- src/invite-api.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/invite-api.js b/src/invite-api.js index afd702b33..3011bf0c1 100644 --- a/src/invite-api.js +++ b/src/invite-api.js @@ -103,7 +103,9 @@ export class InviteApi extends TypedEmitter { if (decision === InviteResponse_Decision.ACCEPT) { if (!invite) { throw new Error( - `Cannot find invite for project with ID ${projectKeyToPublicId}` + `Cannot find invite for project with ID ${projectKeyToPublicId( + projectKey + )}` ) } From 7f849ee968a817963ef13db52c58321e7f5c37bd Mon Sep 17 00:00:00 2001 From: Andrew Chou Date: Wed, 20 Sep 2023 11:57:57 -0400 Subject: [PATCH 12/27] move retrieval of invite in respond method --- src/invite-api.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/invite-api.js b/src/invite-api.js index 3011bf0c1..ef964ab43 100644 --- a/src/invite-api.js +++ b/src/invite-api.js @@ -77,7 +77,6 @@ export class InviteApi extends TypedEmitter { */ #respond({ projectId, decision }) { const peerIds = Array.from(this.#getPeerIds(projectId)) - const invite = this.#invites.get(projectId) const projectKey = Buffer.from(projectId, 'hex') let connectedPeerId @@ -101,6 +100,8 @@ export class InviteApi extends TypedEmitter { }) if (decision === InviteResponse_Decision.ACCEPT) { + const invite = this.#invites.get(projectId) + if (!invite) { throw new Error( `Cannot find invite for project with ID ${projectKeyToPublicId( From d9b147ec55fd060fb1c69ee1724bd8f4c86ac755 Mon Sep 17 00:00:00 2001 From: Andrew Chou Date: Wed, 20 Sep 2023 12:00:50 -0400 Subject: [PATCH 13/27] fix isMember mock in tests --- tests/invite-api.js | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tests/invite-api.js b/tests/invite-api.js index 8c88e973e..880c271f3 100644 --- a/tests/invite-api.js +++ b/tests/invite-api.js @@ -10,14 +10,13 @@ test('Accept invite', async (t) => { const r1 = new MapeoRPC() const r2 = new MapeoRPC() - const members = new Set() const projects = new Map() const inviteApi = new InviteApi({ rpc: r2, queries: { - isMember: async (deviceId) => { - return members.has(deviceId) + isMember: async (projectId) => { + return !!projects.has(projectId) }, addProject: async (projectId, encryptionKeys) => { projects.set(projectId, encryptionKeys) @@ -50,14 +49,13 @@ test('Reject invite', async (t) => { const r1 = new MapeoRPC() const r2 = new MapeoRPC() - const members = new Set() const projects = new Map() const inviteApi = new InviteApi({ rpc: r2, queries: { - isMember: async (deviceId) => { - return members.has(deviceId) + isMember: async (projectId) => { + return !!projects.has(projectId) }, addProject: async (projectId, encryptionKeys) => { projects.set(projectId, encryptionKeys) From 69a560a09657247b144c089fb8367e7f9bbb35db Mon Sep 17 00:00:00 2001 From: Andrew Chou Date: Wed, 20 Sep 2023 12:17:41 -0400 Subject: [PATCH 14/27] small optimizations to respond method --- src/invite-api.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/invite-api.js b/src/invite-api.js index ef964ab43..18a18c51d 100644 --- a/src/invite-api.js +++ b/src/invite-api.js @@ -76,17 +76,17 @@ export class InviteApi extends TypedEmitter { * @param {InviteResponse_Decision} options.decision */ #respond({ projectId, decision }) { - const peerIds = Array.from(this.#getPeerIds(projectId)) + const peerIds = this.#getPeerIds(projectId) const projectKey = Buffer.from(projectId, 'hex') let connectedPeerId - let remainingPeerIds = [] + let remainingPeerIds = new Set() for (const peerId of peerIds) { if (!connectedPeerId && this.#isPeerConnected(peerId)) { connectedPeerId = peerId } else if (this.#isPeerConnected(peerId)) { - remainingPeerIds.push(peerId) + remainingPeerIds.add(peerId) } } From 08a5eaf94783c6018c69468fb15f3ffcd7e1c9de Mon Sep 17 00:00:00 2001 From: Andrew Chou Date: Wed, 20 Sep 2023 12:27:22 -0400 Subject: [PATCH 15/27] add test for autoresponding with already response --- src/invite-api.js | 7 ++++--- tests/invite-api.js | 50 +++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/src/invite-api.js b/src/invite-api.js index 18a18c51d..251b6a3b8 100644 --- a/src/invite-api.js +++ b/src/invite-api.js @@ -31,12 +31,13 @@ export class InviteApi extends TypedEmitter { this.rpc.on('invite', async (peerId, invite) => { const projectId = projectKeyToId(invite.projectKey) + const peerIds = this.#peersToRespondTo.get(projectId) || new Set() + peerIds.add(peerId) + this.#peersToRespondTo.set(projectId, peerIds) + if (await this.#isMember(projectId)) { this.alreadyJoined(projectId) } else { - const peerIds = this.#peersToRespondTo.get(projectId) || new Set() - peerIds.add(peerId) - this.#peersToRespondTo.set(projectId, peerIds) this.#invites.set(projectId, invite) if (peerIds.size === 1) { diff --git a/tests/invite-api.js b/tests/invite-api.js index 880c271f3..477bbe0fd 100644 --- a/tests/invite-api.js +++ b/tests/invite-api.js @@ -16,7 +16,7 @@ test('Accept invite', async (t) => { rpc: r2, queries: { isMember: async (projectId) => { - return !!projects.has(projectId) + return projects.has(projectId) }, addProject: async (projectId, encryptionKeys) => { projects.set(projectId, encryptionKeys) @@ -55,7 +55,7 @@ test('Reject invite', async (t) => { rpc: r2, queries: { isMember: async (projectId) => { - return !!projects.has(projectId) + return projects.has(projectId) }, addProject: async (projectId, encryptionKeys) => { projects.set(projectId, encryptionKeys) @@ -82,3 +82,49 @@ test('Reject invite', async (t) => { replicate(r1, r2) }) + +test('Receiving invite for project that peer already belongs to', async (t) => { + t.plan(2) + + const r1 = new MapeoRPC() + const r2 = new MapeoRPC() + + const projectKey = KeyManager.generateProjectKeypair().publicKey + const encryptionKeys = { auth: randomBytes(32) } + + // Start off being already part of the project + const projects = new Map([[projectKey.toString('hex'), encryptionKeys]]) + + const inviteApi = new InviteApi({ + rpc: r2, + queries: { + isMember: async (projectId) => { + return projects.has(projectId) + }, + addProject: async (projectId, encryptionKeys) => { + projects.set(projectId, encryptionKeys) + }, + }, + }) + + inviteApi.on('invite-received', () => { + t.fail('invite-received event should not have been emitted') + }) + + r1.on('peers', async (peers) => { + t.is(peers.length, 1) + + const response = await r1.invite(peers[0].id, { + projectKey, + encryptionKeys, + }) + + t.is( + response, + MapeoRPC.InviteResponse.ALREADY, + 'invited peer automatically responds with "ALREADY"' + ) + }) + + replicate(r1, r2) +}) From 800325216eb4ced197b42a1715853ec5552e25ce Mon Sep 17 00:00:00 2001 From: Andrew Chou Date: Wed, 20 Sep 2023 12:35:27 -0400 Subject: [PATCH 16/27] minor tests refactor --- tests/invite-api.js | 52 ++++++++++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 19 deletions(-) diff --git a/tests/invite-api.js b/tests/invite-api.js index 477bbe0fd..c360cae3c 100644 --- a/tests/invite-api.js +++ b/tests/invite-api.js @@ -7,11 +7,13 @@ import { replicate } from './helpers/rpc.js' test('Accept invite', async (t) => { t.plan(3) - const r1 = new MapeoRPC() - const r2 = new MapeoRPC() + + const { rpc: r1, projectKey, encryptionKeys } = setup() const projects = new Map() + const r2 = new MapeoRPC() + const inviteApi = new InviteApi({ rpc: r2, queries: { @@ -24,20 +26,20 @@ test('Accept invite', async (t) => { }, }) - const projectKey = KeyManager.generateProjectKeypair().publicKey - const encryptionKeys = { auth: randomBytes(32) } - r1.on('peers', async (peers) => { t.is(peers.length, 1) + const response = await r1.invite(peers[0].id, { projectKey, encryptionKeys, }) + t.is(response, MapeoRPC.InviteResponse.ACCEPT) }) inviteApi.on('invite-received', ({ projectId }) => { t.is(projectId, projectKey.toString('hex')) + inviteApi.accept(projectId) }) @@ -46,11 +48,13 @@ test('Accept invite', async (t) => { test('Reject invite', async (t) => { t.plan(3) - const r1 = new MapeoRPC() - const r2 = new MapeoRPC() + + const { rpc: r1, projectKey, encryptionKeys } = setup() const projects = new Map() + const r2 = new MapeoRPC() + const inviteApi = new InviteApi({ rpc: r2, queries: { @@ -63,20 +67,20 @@ test('Reject invite', async (t) => { }, }) - const projectKey = KeyManager.generateProjectKeypair().publicKey - const encryptionKeys = { auth: randomBytes(32) } - r1.on('peers', async (peers) => { t.is(peers.length, 1) + const response = await r1.invite(peers[0].id, { projectKey, encryptionKeys, }) + t.is(response, MapeoRPC.InviteResponse.REJECT) }) inviteApi.on('invite-received', ({ projectId }) => { t.is(projectId, projectKey.toString('hex')) + inviteApi.reject(projectId) }) @@ -86,15 +90,13 @@ test('Reject invite', async (t) => { test('Receiving invite for project that peer already belongs to', async (t) => { t.plan(2) - const r1 = new MapeoRPC() - const r2 = new MapeoRPC() - - const projectKey = KeyManager.generateProjectKeypair().publicKey - const encryptionKeys = { auth: randomBytes(32) } + const { rpc: r1, projectKey, encryptionKeys } = setup() // Start off being already part of the project const projects = new Map([[projectKey.toString('hex'), encryptionKeys]]) + const r2 = new MapeoRPC() + const inviteApi = new InviteApi({ rpc: r2, queries: { @@ -107,10 +109,6 @@ test('Receiving invite for project that peer already belongs to', async (t) => { }, }) - inviteApi.on('invite-received', () => { - t.fail('invite-received event should not have been emitted') - }) - r1.on('peers', async (peers) => { t.is(peers.length, 1) @@ -126,5 +124,21 @@ test('Receiving invite for project that peer already belongs to', async (t) => { ) }) + inviteApi.on('invite-received', () => { + t.fail('invite-received event should not have been emitted') + }) + replicate(r1, r2) }) + +function setup() { + const encryptionKeys = { auth: randomBytes(32) } + const projectKey = KeyManager.generateProjectKeypair().publicKey + const rpc = new MapeoRPC() + + return { + rpc, + projectKey, + encryptionKeys, + } +} From efe181a6751c2ca804fc5edb198d4df84550352e Mon Sep 17 00:00:00 2001 From: Andrew Chou Date: Wed, 20 Sep 2023 13:21:46 -0400 Subject: [PATCH 17/27] add event typing for InviteApi and update emitted value --- src/invite-api.js | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/invite-api.js b/src/invite-api.js index 251b6a3b8..891a8ee64 100644 --- a/src/invite-api.js +++ b/src/invite-api.js @@ -3,6 +3,15 @@ import { TypedEmitter } from 'tiny-typed-emitter' import { InviteResponse_Decision } from './generated/rpc.js' import { projectKeyToId, projectKeyToPublicId } from './utils.js' +/** + * @typedef {Object} InviteApiEvents + * + * @property {(info: { projectId: string, projectName?: string }) => void} invite-received + */ + +/** + * @extends {TypedEmitter} + */ export class InviteApi extends TypedEmitter { // Maps project id -> set of device ids /** @type {Map>} */ @@ -42,8 +51,11 @@ export class InviteApi extends TypedEmitter { if (peerIds.size === 1) { this.emit('invite-received', { + // TODO: Should this be the project public ID since it can be exposed to the client? + // Probably would require changing the public methods to accept the public ID + // and using the public ID for #invites and #peersToRespondTo keys instead projectId, - peerId, + projectName: invite.projectInfo?.name, }) } } From b2ff8d492b12cc8828406bf9c6a044b969acc3a1 Mon Sep 17 00:00:00 2001 From: Andrew Chou Date: Wed, 20 Sep 2023 15:02:27 -0400 Subject: [PATCH 18/27] fix addProject mock in tests --- tests/invite-api.js | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/tests/invite-api.js b/tests/invite-api.js index c360cae3c..b286a0c26 100644 --- a/tests/invite-api.js +++ b/tests/invite-api.js @@ -20,8 +20,8 @@ test('Accept invite', async (t) => { isMember: async (projectId) => { return projects.has(projectId) }, - addProject: async (projectId, encryptionKeys) => { - projects.set(projectId, encryptionKeys) + addProject: async (invite) => { + projects.set(invite.projectKey.toString('hex'), invite) }, }, }) @@ -61,8 +61,8 @@ test('Reject invite', async (t) => { isMember: async (projectId) => { return projects.has(projectId) }, - addProject: async (projectId, encryptionKeys) => { - projects.set(projectId, encryptionKeys) + addProject: async (invite) => { + projects.set(invite.projectKey.toString('hex'), invite) }, }, }) @@ -93,7 +93,9 @@ test('Receiving invite for project that peer already belongs to', async (t) => { const { rpc: r1, projectKey, encryptionKeys } = setup() // Start off being already part of the project - const projects = new Map([[projectKey.toString('hex'), encryptionKeys]]) + const projects = new Map([ + [projectKey.toString('hex'), { projectKey, encryptionKeys }], + ]) const r2 = new MapeoRPC() @@ -103,8 +105,8 @@ test('Receiving invite for project that peer already belongs to', async (t) => { isMember: async (projectId) => { return projects.has(projectId) }, - addProject: async (projectId, encryptionKeys) => { - projects.set(projectId, encryptionKeys) + addProject: async (invite) => { + projects.set(invite.projectKey.toString('hex'), invite) }, }, }) From 5c3f473a5a06fdf004c41bd7f34f7ae3901bd28a Mon Sep 17 00:00:00 2001 From: Andrew Chou Date: Wed, 20 Sep 2023 15:31:28 -0400 Subject: [PATCH 19/27] refactor respond method to not be recursive --- src/invite-api.js | 156 +++++++++++++++++++++++++++++--------------- tests/invite-api.js | 131 ++++++++++++++++++++++++++----------- 2 files changed, 197 insertions(+), 90 deletions(-) diff --git a/src/invite-api.js b/src/invite-api.js index 891a8ee64..3f4ec783e 100644 --- a/src/invite-api.js +++ b/src/invite-api.js @@ -44,8 +44,11 @@ export class InviteApi extends TypedEmitter { peerIds.add(peerId) this.#peersToRespondTo.set(projectId, peerIds) - if (await this.#isMember(projectId)) { - this.alreadyJoined(projectId) + const isAlreadyMember = await this.#isMember(projectId) + + if (isAlreadyMember) { + // TODO: Catch the error and no-op here? + this.#respond({ projectId, decision: InviteResponse_Decision.ALREADY }) } else { this.#invites.set(projectId, invite) @@ -64,23 +67,35 @@ export class InviteApi extends TypedEmitter { /** * @param {string} projectId + * @returns {Promise} */ - accept(projectId) { - this.#respond({ projectId, decision: InviteResponse_Decision.ACCEPT }) - } + async accept(projectId) { + const isAlreadyMember = await this.#isMember(projectId) + + // TODO: Is this check necessary? + // If so, do we want to respond here or throw an error? + if (isAlreadyMember) { + return this.#respond({ + projectId, + decision: InviteResponse_Decision.ALREADY, + }) + } - /** - * @param {string} projectId - */ - reject(projectId) { - this.#respond({ projectId, decision: InviteResponse_Decision.REJECT }) + return this.#respond({ + projectId, + decision: InviteResponse_Decision.ACCEPT, + }) } /** * @param {string} projectId + * @returns {Promise} */ - alreadyJoined(projectId) { - this.#respond({ projectId, decision: InviteResponse_Decision.ALREADY }) + async reject(projectId) { + return this.#respond({ + projectId, + decision: InviteResponse_Decision.REJECT, + }) } /** @@ -88,56 +103,75 @@ export class InviteApi extends TypedEmitter { * @param {string} options.projectId * @param {InviteResponse_Decision} options.decision */ - #respond({ projectId, decision }) { - const peerIds = this.#getPeerIds(projectId) + async #respond({ projectId, decision }) { const projectKey = Buffer.from(projectId, 'hex') - let connectedPeerId - let remainingPeerIds = new Set() + switch (decision) { + case InviteResponse_Decision.ACCEPT: { + const invite = this.#invites.get(projectId) - for (const peerId of peerIds) { - if (!connectedPeerId && this.#isPeerConnected(peerId)) { - connectedPeerId = peerId - } else if (this.#isPeerConnected(peerId)) { - remainingPeerIds.add(peerId) - } - } + if (!invite) { + throw new Error( + `Cannot find invite for project with ID ${projectKeyToPublicId( + projectKey + )}` + ) + } - if (!connectedPeerId) { - throw new Error('No connected peer to respond to') - } + // TODO: What should we do if this throws? + await this.#addProject(invite) - this.rpc.inviteResponse(connectedPeerId, { - projectKey, - decision, - }) + const connectedPeers = this.#getConnectedProjectPeers(projectId) - if (decision === InviteResponse_Decision.ACCEPT) { - const invite = this.#invites.get(projectId) + // TODO: Is this what we want to do here? + if (connectedPeers.size === 0) { + throw new Error('No connected peers to respond to') + } - if (!invite) { - throw new Error( - `Cannot find invite for project with ID ${projectKeyToPublicId( - projectKey - )}` - ) - } + const [firstPeerId, ...remainingPeerIds] = Array.from(connectedPeers) - this.#addProject(invite) + this.rpc.inviteResponse(firstPeerId, { + projectKey, + decision: InviteResponse_Decision.ACCEPT, + }) - // eslint-disable-next-line no-unused-vars - for (const _peerId of remainingPeerIds) { - this.alreadyJoined(projectId) + // Respond to the remaining peers with ALREADY + for (const peerId of remainingPeerIds) { + this.rpc.inviteResponse(peerId, { + projectKey, + decision: InviteResponse_Decision.ALREADY, + }) + } } - } else if (decision === InviteResponse_Decision.REJECT) { - // eslint-disable-next-line no-unused-vars - for (const _peerId of remainingPeerIds) { - this.reject(projectId) + case InviteResponse_Decision.REJECT: { + const connectedPeers = this.#getConnectedProjectPeers(projectId) + + // TODO: Is this what we want to do here? + if (connectedPeers.size === 0) { + throw new Error('No connected peers to respond to') + } + + for (const peerId of connectedPeers) { + this.rpc.inviteResponse(peerId, { + projectKey, + decision: InviteResponse_Decision.REJECT, + }) + } } - } else if (decision === InviteResponse_Decision.ALREADY) { - // eslint-disable-next-line no-unused-vars - for (const _peerId of remainingPeerIds) { - this.alreadyJoined(projectId) + case InviteResponse_Decision.ALREADY: { + const connectedPeers = this.#getConnectedProjectPeers(projectId) + + // TODO: Is this what we want to do here? + if (connectedPeers.size === 0) { + throw new Error('No connected peers to respond to') + } + + for (const peerId of connectedPeers) { + this.rpc.inviteResponse(peerId, { + projectKey, + decision: InviteResponse_Decision.ALREADY, + }) + } } } @@ -157,7 +191,25 @@ export class InviteApi extends TypedEmitter { * @param {string} projectId * @returns {Set} peerIds */ - #getPeerIds(projectId) { + #getProjectPeerIds(projectId) { return this.#peersToRespondTo.get(projectId) || new Set() } + + /** + * + * @param {string} projectId + * @returns {Set} + */ + #getConnectedProjectPeers(projectId) { + const connected = new Set() + const projectPeerIds = this.#getProjectPeerIds(projectId) + + for (const id of projectPeerIds) { + if (this.#isPeerConnected(id)) { + connected.add(id) + } + } + + return connected + } } diff --git a/tests/invite-api.js b/tests/invite-api.js index b286a0c26..208c87ab4 100644 --- a/tests/invite-api.js +++ b/tests/invite-api.js @@ -6,7 +6,7 @@ import { InviteApi } from '../src/invite-api.js' import { replicate } from './helpers/rpc.js' test('Accept invite', async (t) => { - t.plan(3) + t.plan(4) const { rpc: r1, projectKey, encryptionKeys } = setup() @@ -37,17 +37,19 @@ test('Accept invite', async (t) => { t.is(response, MapeoRPC.InviteResponse.ACCEPT) }) - inviteApi.on('invite-received', ({ projectId }) => { + inviteApi.on('invite-received', async ({ projectId }) => { t.is(projectId, projectKey.toString('hex')) - inviteApi.accept(projectId) + await inviteApi.accept(projectId) + + t.ok(projects.has(projectId), 'project successfully added') }) replicate(r1, r2) }) test('Reject invite', async (t) => { - t.plan(3) + t.plan(4) const { rpc: r1, projectKey, encryptionKeys } = setup() @@ -78,59 +80,112 @@ test('Reject invite', async (t) => { t.is(response, MapeoRPC.InviteResponse.REJECT) }) - inviteApi.on('invite-received', ({ projectId }) => { + inviteApi.on('invite-received', async ({ projectId }) => { t.is(projectId, projectKey.toString('hex')) - inviteApi.reject(projectId) + await inviteApi.reject(projectId) + + t.is(projects.has(projectId), false, 'project not added') }) replicate(r1, r2) }) test('Receiving invite for project that peer already belongs to', async (t) => { - t.plan(2) + t.test('was member prior to connection', async (t) => { + t.plan(2) + + const { rpc: r1, projectKey, encryptionKeys } = setup() + + // Start off being already part of the project + const projects = new Map([ + [projectKey.toString('hex'), { projectKey, encryptionKeys }], + ]) + + const r2 = new MapeoRPC() + + const inviteApi = new InviteApi({ + rpc: r2, + queries: { + isMember: async (projectId) => { + return projects.has(projectId) + }, + addProject: async (invite) => { + projects.set(invite.projectKey.toString('hex'), invite) + }, + }, + }) - const { rpc: r1, projectKey, encryptionKeys } = setup() + r1.on('peers', async (peers) => { + t.is(peers.length, 1) - // Start off being already part of the project - const projects = new Map([ - [projectKey.toString('hex'), { projectKey, encryptionKeys }], - ]) + const response = await r1.invite(peers[0].id, { + projectKey, + encryptionKeys, + }) - const r2 = new MapeoRPC() + t.is( + response, + MapeoRPC.InviteResponse.ALREADY, + 'invited peer automatically responds with "ALREADY"' + ) + }) - const inviteApi = new InviteApi({ - rpc: r2, - queries: { - isMember: async (projectId) => { - return projects.has(projectId) - }, - addProject: async (invite) => { - projects.set(invite.projectKey.toString('hex'), invite) - }, - }, + inviteApi.on('invite-received', () => { + t.fail('invite-received event should not have been emitted') + }) + + replicate(r1, r2) }) - r1.on('peers', async (peers) => { - t.is(peers.length, 1) + t.test('became member from accepting prior invite', async (t) => { + t.plan(3) - const response = await r1.invite(peers[0].id, { - projectKey, - encryptionKeys, + const { rpc: r1, projectKey, encryptionKeys } = setup() + + const projects = new Map() + + const r2 = new MapeoRPC() + + const inviteApi = new InviteApi({ + rpc: r2, + queries: { + isMember: async (projectId) => { + return projects.has(projectId) + }, + addProject: async (invite) => { + projects.set(invite.projectKey.toString('hex'), invite) + }, + }, }) - t.is( - response, - MapeoRPC.InviteResponse.ALREADY, - 'invited peer automatically responds with "ALREADY"' - ) - }) + r1.on('peers', async (peers) => { + const response1 = await r1.invite(peers[0].id, { + projectKey, + encryptionKeys, + }) - inviteApi.on('invite-received', () => { - t.fail('invite-received event should not have been emitted') - }) + t.is(response1, MapeoRPC.InviteResponse.ACCEPT) - replicate(r1, r2) + const response2 = await r1.invite(peers[0].id, { + projectKey, + encryptionKeys, + }) + + t.is(response2, MapeoRPC.InviteResponse.ALREADY) + }) + + let inviteReceivedEventCount = 0 + + inviteApi.on('invite-received', ({ projectId }) => { + inviteReceivedEventCount += 1 + t.is(inviteReceivedEventCount, 1) + + inviteApi.accept(projectId) + }) + + replicate(r1, r2) + }) }) function setup() { From 09738cafe16bda0ec6f076e6a4a85adbfd2225e2 Mon Sep 17 00:00:00 2001 From: Andrew Chou Date: Wed, 20 Sep 2023 15:34:45 -0400 Subject: [PATCH 20/27] fix lint error --- src/invite-api.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/invite-api.js b/src/invite-api.js index 3f4ec783e..1a50b03da 100644 --- a/src/invite-api.js +++ b/src/invite-api.js @@ -142,6 +142,8 @@ export class InviteApi extends TypedEmitter { decision: InviteResponse_Decision.ALREADY, }) } + + break } case InviteResponse_Decision.REJECT: { const connectedPeers = this.#getConnectedProjectPeers(projectId) @@ -157,6 +159,8 @@ export class InviteApi extends TypedEmitter { decision: InviteResponse_Decision.REJECT, }) } + + break } case InviteResponse_Decision.ALREADY: { const connectedPeers = this.#getConnectedProjectPeers(projectId) @@ -172,6 +176,8 @@ export class InviteApi extends TypedEmitter { decision: InviteResponse_Decision.ALREADY, }) } + + break } } From 8168ebbc5803248afc91b7146373e6649db0ba91 Mon Sep 17 00:00:00 2001 From: Gregor MacLennan Date: Thu, 21 Sep 2023 12:02:09 +0100 Subject: [PATCH 21/27] Cleanup code and add more tests --- src/invite-api.js | 272 ++++++++++++++++++++----------------------- src/rpc/index.js | 1 + tests/helpers/rpc.js | 15 ++- tests/invite-api.js | 262 +++++++++++++++++++++++++++++++++++++++-- 4 files changed, 390 insertions(+), 160 deletions(-) diff --git a/src/invite-api.js b/src/invite-api.js index 1a50b03da..bea8c63e2 100644 --- a/src/invite-api.js +++ b/src/invite-api.js @@ -1,25 +1,41 @@ // @ts-check import { TypedEmitter } from 'tiny-typed-emitter' import { InviteResponse_Decision } from './generated/rpc.js' -import { projectKeyToId, projectKeyToPublicId } from './utils.js' +import { projectKeyToId } from './utils.js' /** * @typedef {Object} InviteApiEvents * - * @property {(info: { projectId: string, projectName?: string }) => void} invite-received + * @property {(info: { projectId: string, projectName?: string, peerId: string }) => void} invite-received */ +/** + * @template K + * @template V + * @extends {Map>} + */ +class MapOfSets extends Map { + /** @param {K} key */ + get(key) { + const existing = super.get(key) + if (existing) return existing + const set = new Set() + super.set(key, set) + return set + } +} + /** * @extends {TypedEmitter} */ export class InviteApi extends TypedEmitter { // Maps project id -> set of device ids - /** @type {Map>} */ - #peersToRespondTo = new Map() + /** @type {MapOfSets} */ + #peersToRespondTo = new MapOfSets() - // Maps project id -> invite - /** @type {Map} */ - #invites = new Map() + // Maps project id -> { invite, fromPeerId } + /** @type {Map} */ + #pendingInvites = new Map() #isMember #addProject @@ -37,53 +53,40 @@ export class InviteApi extends TypedEmitter { this.#isMember = queries.isMember this.#addProject = queries.addProject - this.rpc.on('invite', async (peerId, invite) => { - const projectId = projectKeyToId(invite.projectKey) - - const peerIds = this.#peersToRespondTo.get(projectId) || new Set() - peerIds.add(peerId) - this.#peersToRespondTo.set(projectId, peerIds) - - const isAlreadyMember = await this.#isMember(projectId) - - if (isAlreadyMember) { - // TODO: Catch the error and no-op here? - this.#respond({ projectId, decision: InviteResponse_Decision.ALREADY }) - } else { - this.#invites.set(projectId, invite) - - if (peerIds.size === 1) { - this.emit('invite-received', { - // TODO: Should this be the project public ID since it can be exposed to the client? - // Probably would require changing the public methods to accept the public ID - // and using the public ID for #invites and #peersToRespondTo keys instead - projectId, - projectName: invite.projectInfo?.name, - }) - } - } + this.rpc.on('invite', (peerId, invite) => { + this.#handleInvite(peerId, invite).catch(() => { + /* c8 ignore next */ + // TODO: Log errors, but otherwise ignore them, but can't think of a reason there would be an error here + }) }) } /** - * @param {string} projectId - * @returns {Promise} + * @param {string} peerId + * @param {import('./generated/rpc.js').Invite} invite */ - async accept(projectId) { + async #handleInvite(peerId, invite) { + const projectId = projectKeyToId(invite.projectKey) const isAlreadyMember = await this.#isMember(projectId) - // TODO: Is this check necessary? - // If so, do we want to respond here or throw an error? if (isAlreadyMember) { - return this.#respond({ - projectId, - decision: InviteResponse_Decision.ALREADY, - }) + this.#sendAlreadyResponse({ peerId, projectId }) + return } - return this.#respond({ + const peerIds = this.#peersToRespondTo.get(projectId) + peerIds.add(peerId) + + if (this.#pendingInvites.has(projectId)) return + + this.#pendingInvites.set(projectId, { fromPeerId: peerId, invite }) + this.emit('invite-received', { + // TODO: Should this be the project public ID since it can be exposed to the client? + // Probably would require changing the public methods to accept the public ID + // and using the public ID for #invites and #peersToRespondTo keys instead + peerId, projectId, - decision: InviteResponse_Decision.ACCEPT, + projectName: invite.projectInfo?.name, }) } @@ -91,131 +94,106 @@ export class InviteApi extends TypedEmitter { * @param {string} projectId * @returns {Promise} */ - async reject(projectId) { - return this.#respond({ - projectId, - decision: InviteResponse_Decision.REJECT, - }) - } - - /** - * @param {Object} options - * @param {string} options.projectId - * @param {InviteResponse_Decision} options.decision - */ - async #respond({ projectId, decision }) { - const projectKey = Buffer.from(projectId, 'hex') - - switch (decision) { - case InviteResponse_Decision.ACCEPT: { - const invite = this.#invites.get(projectId) - - if (!invite) { - throw new Error( - `Cannot find invite for project with ID ${projectKeyToPublicId( - projectKey - )}` - ) - } - - // TODO: What should we do if this throws? - await this.#addProject(invite) - - const connectedPeers = this.#getConnectedProjectPeers(projectId) + async accept(projectId) { + const isAlreadyMember = await this.#isMember(projectId) + const peersToRespondTo = this.#peersToRespondTo.get(projectId) - // TODO: Is this what we want to do here? - if (connectedPeers.size === 0) { - throw new Error('No connected peers to respond to') - } + if (isAlreadyMember) { + for (const peerId of peersToRespondTo) { + this.#sendAlreadyResponse({ peerId, projectId }) + } + return + } - const [firstPeerId, ...remainingPeerIds] = Array.from(connectedPeers) + const pendingInvite = this.#pendingInvites.get(projectId) - this.rpc.inviteResponse(firstPeerId, { - projectKey, - decision: InviteResponse_Decision.ACCEPT, - }) + if (!pendingInvite) { + throw new Error(`Cannot find invite for project with ID ${projectId}`) + } - // Respond to the remaining peers with ALREADY - for (const peerId of remainingPeerIds) { - this.rpc.inviteResponse(peerId, { - projectKey, - decision: InviteResponse_Decision.ALREADY, - }) - } + try { + this.#sendAcceptResponse({ peerId: pendingInvite.fromPeerId, projectId }) + // TODO: Add another RPC message to confirm invitee is written into project after accepting + } catch (e) { + // TODO: If can't accept invite because peer it was sent from has + // disconnected, and another still-connected peer has sent an invite, then + // emit another invite-received event + throw new Error('Could not accept invite: Peer disconnected') + } - break - } - case InviteResponse_Decision.REJECT: { - const connectedPeers = this.#getConnectedProjectPeers(projectId) - - // TODO: Is this what we want to do here? - if (connectedPeers.size === 0) { - throw new Error('No connected peers to respond to') - } - - for (const peerId of connectedPeers) { - this.rpc.inviteResponse(peerId, { - projectKey, - decision: InviteResponse_Decision.REJECT, - }) - } - - break - } - case InviteResponse_Decision.ALREADY: { - const connectedPeers = this.#getConnectedProjectPeers(projectId) - - // TODO: Is this what we want to do here? - if (connectedPeers.size === 0) { - throw new Error('No connected peers to respond to') - } - - for (const peerId of connectedPeers) { - this.rpc.inviteResponse(peerId, { - projectKey, - decision: InviteResponse_Decision.ALREADY, - }) - } - - break - } + try { + await this.#addProject(pendingInvite.invite) + } catch (e) { + // TODO: Add a reason for the user + throw new Error('Failed to join project') } - this.#peersToRespondTo.delete(projectId) - this.#invites.delete(projectId) + // Respond to the remaining peers with ALREADY + for (const peerId of peersToRespondTo) { + if (peerId === pendingInvite.fromPeerId) continue + this.#sendAlreadyResponse({ peerId, projectId }) + } } /** - * @param {string} peerId - * @returns {boolean} + * @param {string} projectId + * @returns {Promise} */ - #isPeerConnected(peerId) { - return this.rpc.peers.some((peer) => peer.id === peerId) + async reject(projectId) { + if (!this.#pendingInvites.has(projectId)) { + throw new Error(`Cannot find invite for project with ID ${projectId}`) + } + for (const peerId of this.#peersToRespondTo.get(projectId)) { + this.#sendRejectResponse({ peerId, projectId }) + } } /** - * @param {string} projectId - * @returns {Set} peerIds + * Will throw if the response fails to be sent + * + * @param {{ peerId: string, projectId: string }} opts */ - #getProjectPeerIds(projectId) { - return this.#peersToRespondTo.get(projectId) || new Set() + #sendAcceptResponse({ peerId, projectId }) { + const projectKey = Buffer.from(projectId, 'hex') + this.rpc.inviteResponse(peerId, { + projectKey, + decision: InviteResponse_Decision.ACCEPT, + }) } /** + * Will not throw, will silently fail if the response fails to send. * - * @param {string} projectId - * @returns {Set} + * @param {{ peerId: string, projectId: string }} opts */ - #getConnectedProjectPeers(projectId) { - const connected = new Set() - const projectPeerIds = this.#getProjectPeerIds(projectId) - - for (const id of projectPeerIds) { - if (this.#isPeerConnected(id)) { - connected.add(id) - } + #sendAlreadyResponse({ peerId, projectId }) { + const projectKey = Buffer.from(projectId, 'hex') + try { + this.rpc.inviteResponse(peerId, { + projectKey, + decision: InviteResponse_Decision.ALREADY, + }) + } catch (e) { + // Ignore errors trying to send an already response because the invitor + // will consider the invite failed anyway } + } - return connected + /** + * Will not throw, will silently fail if the response fails to send. + * + * @param {{ peerId: string, projectId: string }} opts + */ + #sendRejectResponse({ peerId, projectId }) { + const projectKey = Buffer.from(projectId, 'hex') + try { + this.rpc.inviteResponse(peerId, { + projectKey, + decision: InviteResponse_Decision.REJECT, + }) + } catch (e) { + // Ignore errors trying to send an reject response because the invitor + // will consider the invite failed anyway + } } } diff --git a/src/rpc/index.js b/src/rpc/index.js index a6e72d3b6..7f4e03af6 100644 --- a/src/rpc/index.js +++ b/src/rpc/index.js @@ -138,6 +138,7 @@ export class MapeoRPC extends TypedEmitter { */ async invite(peerId, { timeout, ...invite }) { const peer = this.#peers.get(peerId) + if (!peer) console.log([...this.#peers.keys()]) if (!peer) throw new UnknownPeerError('Unknown peer ' + peerId) /** @type {Promise} */ return new Promise((origResolve, origReject) => { diff --git a/tests/helpers/rpc.js b/tests/helpers/rpc.js index 3070eeba8..544f6365a 100644 --- a/tests/helpers/rpc.js +++ b/tests/helpers/rpc.js @@ -1,12 +1,19 @@ import NoiseSecretStream from '@hyperswarm/secret-stream' -export function replicate(rpc1, rpc2) { - const n1 = new NoiseSecretStream(true, undefined, { +export function replicate( + rpc1, + rpc2, + { // Keep keypairs deterministic for tests, since we use peer.publicKey as an identifier. - keyPair: NoiseSecretStream.keyPair(Buffer.allocUnsafe(32).fill(0)), + kp1 = NoiseSecretStream.keyPair(Buffer.allocUnsafe(32).fill(0)), + kp2 = NoiseSecretStream.keyPair(Buffer.allocUnsafe(32).fill(1)), + } = {} +) { + const n1 = new NoiseSecretStream(true, undefined, { + keyPair: kp1, }) const n2 = new NoiseSecretStream(false, undefined, { - keyPair: NoiseSecretStream.keyPair(Buffer.allocUnsafe(32).fill(1)), + keyPair: kp2, }) n1.rawStream.pipe(n2.rawStream).pipe(n1.rawStream) diff --git a/tests/invite-api.js b/tests/invite-api.js index 208c87ab4..432f665be 100644 --- a/tests/invite-api.js +++ b/tests/invite-api.js @@ -4,6 +4,8 @@ import { KeyManager } from '@mapeo/crypto' import { MapeoRPC } from '../src/rpc/index.js' import { InviteApi } from '../src/invite-api.js' import { replicate } from './helpers/rpc.js' +import NoiseSecretStream from '@hyperswarm/secret-stream' +import pDefer from 'p-defer' test('Accept invite', async (t) => { t.plan(4) @@ -97,21 +99,16 @@ test('Receiving invite for project that peer already belongs to', async (t) => { const { rpc: r1, projectKey, encryptionKeys } = setup() - // Start off being already part of the project - const projects = new Map([ - [projectKey.toString('hex'), { projectKey, encryptionKeys }], - ]) - const r2 = new MapeoRPC() const inviteApi = new InviteApi({ rpc: r2, queries: { - isMember: async (projectId) => { - return projects.has(projectId) + isMember: async () => { + return true }, - addProject: async (invite) => { - projects.set(invite.projectKey.toString('hex'), invite) + addProject: async () => { + t.fail('should not add project') }, }, }) @@ -138,6 +135,53 @@ test('Receiving invite for project that peer already belongs to', async (t) => { replicate(r1, r2) }) + t.test( + 'became member (somehow!) between receiving invite and accepting', + async (t) => { + t.plan(3) + + const { rpc: r1, projectKey, encryptionKeys } = setup() + + const r2 = new MapeoRPC() + let isMember = false + + const inviteApi = new InviteApi({ + rpc: r2, + queries: { + isMember: async () => { + return isMember + }, + addProject: async () => { + t.fail('should not add project') + }, + }, + }) + + r1.on('peers', async (peers) => { + t.is(peers.length, 1) + + const response = await r1.invite(peers[0].id, { + projectKey, + encryptionKeys, + }) + + t.is( + response, + MapeoRPC.InviteResponse.ALREADY, + 'invited peer automatically responds with "ALREADY"' + ) + }) + + inviteApi.on('invite-received', async ({ projectId }) => { + isMember = true + await inviteApi.accept(projectId) + t.pass('sending accept does not throw') + }) + + replicate(r1, r2) + } + ) + t.test('became member from accepting prior invite', async (t) => { t.plan(3) @@ -188,6 +232,206 @@ test('Receiving invite for project that peer already belongs to', async (t) => { }) }) +test('trying to accept or reject non-existent invite throws', async (t) => { + const rpc = new MapeoRPC() + const inviteApi = new InviteApi({ + rpc, + queries: { + isMember: async () => {}, + addProject: async () => {}, + }, + }) + await t.exception(() => { + return inviteApi.accept(randomBytes(32)) + }) + await t.exception(() => { + return inviteApi.reject(randomBytes(32)) + }) +}) + +test('invitor disconnecting results in accept throwing', async (t) => { + t.plan(3) + + const { rpc: r1, projectKey, encryptionKeys } = setup() + + const r2 = new MapeoRPC() + + const inviteApi = new InviteApi({ + rpc: r2, + queries: { + isMember: async () => false, + addProject: async () => { + t.fail('should not try to add project if could not accept') + }, + }, + }) + + r1.on('peers', async (peers) => { + if (peers.length !== 1 || peers[0].status === 'disconnected') return + + await t.exception(() => { + return r1.invite(peers[0].id, { + projectKey, + encryptionKeys, + }) + }, 'Invite fails') + }) + + inviteApi.on('invite-received', async ({ projectId }) => { + t.is(projectId, projectKey.toString('hex'), 'received invite') + await disconnect() + await t.exception(() => { + return inviteApi.accept(projectId) + }) + }) + + const disconnect = replicate(r1, r2) +}) + +test('Invite from multiple peers', async (t) => { + const invitorCount = 10 + t.plan(5 + invitorCount) + + const { projectKey, encryptionKeys } = setup() + const invitee = new MapeoRPC() + const inviteeKeyPair = NoiseSecretStream.keyPair() + + const projects = new Map() + + const inviteApi = new InviteApi({ + rpc: invitee, + queries: { + isMember: async (projectId) => { + return projects.has(projectId) + }, + addProject: async (invite) => { + const projectId = invite.projectKey.toString('hex') + t.absent(projects.has(projectId), 'add project called only once') + projects.set(projectId, invite) + }, + }, + }) + + let first + let connected = 0 + const deferred = pDefer() + + inviteApi.on('invite-received', async ({ projectId, peerId }) => { + t.is(projectId, projectKey.toString('hex'), 'expected project id') + t.absent(first, 'should only receive invite once') + first = peerId + + // Wait for all the invites to be sent before we accept + await deferred.promise + await inviteApi.accept(projectId) + + t.ok(projects.has(projectId), 'project successfully added') + }) + + for (let i = 0; i < invitorCount; i++) { + const invitor = new MapeoRPC() + const keyPair = NoiseSecretStream.keyPair() + invitor.on('peers', async (peers) => { + if (++connected === invitorCount) deferred.resolve() + const response = await invitor.invite(peers[0].id, { + projectKey, + encryptionKeys, + }) + if (first === keyPair.publicKey.toString('hex')) { + t.pass('One invitor did receive accept response') + t.is(response, MapeoRPC.InviteResponse.ACCEPT, 'accept response') + } else { + t.is(response, MapeoRPC.InviteResponse.ALREADY, 'already response') + } + }) + replicate(invitee, invitor, { kp1: inviteeKeyPair, kp2: keyPair }) + } +}) + +// TODO: for now this is not handled +test.skip('Invite from multiple peers, first disconnects before accepted, receives invite from next in queue', async (t) => { + const invitorCount = 10 + t.plan(8 + invitorCount) + + const { projectKey, encryptionKeys } = setup() + const invitee = new MapeoRPC() + const inviteeKeyPair = NoiseSecretStream.keyPair() + + const projects = new Map() + + const inviteApi = new InviteApi({ + rpc: invitee, + queries: { + isMember: async (projectId) => { + return projects.has(projectId) + }, + addProject: async (invite) => { + const projectId = invite.projectKey.toString('hex') + t.absent(projects.has(projectId), 'add project called only once') + projects.set(projectId, invite) + }, + }, + }) + + let invitesReceived = [] + let connected = 0 + const disconnects = new Map() + const deferred = pDefer() + + inviteApi.on('invite-received', async ({ projectId, peerId }) => { + t.is(projectId, projectKey.toString('hex'), 'expected project id') + t.ok(invitesReceived.length < 2, 'should only receive two invites') + invitesReceived.push(peerId) + const isFirst = (invitesReceived.length = 1) + + // Wait for all the invites to be sent before we accept + await deferred.promise + if (isFirst) { + await disconnects.get(peerId)() + + await t.exception(() => { + return inviteApi.accept(projectId) + }, 'accept throws') + } else { + await inviteApi.accept(projectId) + t.ok(projects.has(projectId), 'project successfully added') + } + }) + + for (let i = 0; i < invitorCount; i++) { + const invitor = new MapeoRPC() + const keyPair = NoiseSecretStream.keyPair() + const invitorId = keyPair.publicKey.toString('hex') + invitor.on('peers', async (peers) => { + if (peers[0].status !== 'connected') return + if (++connected === invitorCount) deferred.resolve() + try { + const response = await invitor.invite(peers[0].id, { + projectKey, + encryptionKeys, + }) + if (invitorId === invitesReceived[1]) { + t.pass('One invitor did receive accept response') + t.is(response, MapeoRPC.InviteResponse.ACCEPT, 'accept response') + } else { + t.is(response, MapeoRPC.InviteResponse.ALREADY, 'already response') + } + } catch (e) { + t.is( + invitorId, + invitesReceived[0], + 'first invitor invite throws because disconnected' + ) + } + }) + const disconnect = replicate(invitee, invitor, { + kp1: inviteeKeyPair, + kp2: keyPair, + }) + disconnects.set(invitorId, disconnect) + } +}) + function setup() { const encryptionKeys = { auth: randomBytes(32) } const projectKey = KeyManager.generateProjectKeypair().publicKey From 54b958537de2130e1242b676ad887c091609b948 Mon Sep 17 00:00:00 2001 From: Andrew Chou Date: Thu, 21 Sep 2023 10:50:36 -0400 Subject: [PATCH 22/27] add test for when addProject throws --- tests/invite-api.js | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/invite-api.js b/tests/invite-api.js index 432f665be..cc3c5da04 100644 --- a/tests/invite-api.js +++ b/tests/invite-api.js @@ -288,6 +288,39 @@ test('invitor disconnecting results in accept throwing', async (t) => { const disconnect = replicate(r1, r2) }) +test('addProject throwing results in invite accept throwing', async (t) => { + t.plan(1) + + const { rpc: r1, projectKey, encryptionKeys } = setup() + + const r2 = new MapeoRPC() + + const inviteApi = new InviteApi({ + rpc: r2, + queries: { + isMember: async () => {}, + addProject: async () => { + throw new Error('Failed to add project') + }, + }, + }) + + r1.on('peers', (peers) => { + r1.invite(peers[0].id, { + projectKey, + encryptionKeys, + }) + }) + + inviteApi.on('invite-received', async ({ projectId }) => { + t.exception(async () => { + return inviteApi.accept(projectId) + }) + }) + + replicate(r1, r2) +}) + test('Invite from multiple peers', async (t) => { const invitorCount = 10 t.plan(5 + invitorCount) From cb1a2e70b15b39ee865c447130cd96c20616efff Mon Sep 17 00:00:00 2001 From: Andrew Chou Date: Thu, 21 Sep 2023 11:37:30 -0400 Subject: [PATCH 23/27] add test checking payload of invite-received event --- tests/invite-api.js | 52 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/tests/invite-api.js b/tests/invite-api.js index cc3c5da04..19148f38b 100644 --- a/tests/invite-api.js +++ b/tests/invite-api.js @@ -7,6 +7,58 @@ import { replicate } from './helpers/rpc.js' import NoiseSecretStream from '@hyperswarm/secret-stream' import pDefer from 'p-defer' +test('invite-received event has expected payload', async (t) => { + t.plan(5) + + const { rpc: r1, projectKey, encryptionKeys } = setup() + + const projects = new Map() + + const r2 = new MapeoRPC() + + const inviteApi = new InviteApi({ + rpc: r2, + queries: { + isMember: async (projectId) => { + return projects.has(projectId) + }, + addProject: async (invite) => { + projects.set(invite.projectKey.toString('hex'), invite) + }, + }, + }) + + let expectedInvitorPeerId + + r2.on('peers', (peers) => { + t.is(peers.length, 1) + expectedInvitorPeerId = peers[0].id + }) + + r1.on('peers', (peers) => { + t.is(peers.length, 1) + + r1.invite(peers[0].id, { + projectKey, + encryptionKeys, + projectInfo: { + name: 'Mapeo', + }, + }) + }) + + inviteApi.on( + 'invite-received', + async ({ peerId, projectId, projectName }) => { + t.is(peerId, expectedInvitorPeerId) + t.is(projectName, 'Mapeo') + t.is(projectId, projectKey.toString('hex')) + } + ) + + replicate(r1, r2) +}) + test('Accept invite', async (t) => { t.plan(4) From 2c64ed2a20c5daff4646797db21cb77f4c8f9986 Mon Sep 17 00:00:00 2001 From: Andrew Chou Date: Thu, 21 Sep 2023 11:38:56 -0400 Subject: [PATCH 24/27] add test for sending reject response after invitor disconnects --- tests/invite-api.js | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/invite-api.js b/tests/invite-api.js index 19148f38b..d84768fc8 100644 --- a/tests/invite-api.js +++ b/tests/invite-api.js @@ -340,6 +340,42 @@ test('invitor disconnecting results in accept throwing', async (t) => { const disconnect = replicate(r1, r2) }) +test('invitor disconnecting results in invite reject response not throwing', async (t) => { + t.plan(3) + + const { rpc: r1, projectKey, encryptionKeys } = setup() + + const r2 = new MapeoRPC() + + const inviteApi = new InviteApi({ + rpc: r2, + queries: { + isMember: async () => {}, + addProject: async () => {}, + }, + }) + + r1.on('peers', async (peers) => { + if (peers.length !== 1 || peers[0].status === 'disconnected') return + + await t.exception(() => { + return r1.invite(peers[0].id, { + projectKey, + encryptionKeys, + }) + }, 'invite fails') + }) + + inviteApi.on('invite-received', async ({ projectId }) => { + t.is(projectId, projectKey.toString('hex'), 'received invite') + await disconnect() + await inviteApi.reject(projectId) + t.pass() + }) + + const disconnect = replicate(r1, r2) +}) + test('addProject throwing results in invite accept throwing', async (t) => { t.plan(1) From bccebdb50489a5e8c5716588c79fb28f31468741 Mon Sep 17 00:00:00 2001 From: Andrew Chou Date: Thu, 21 Sep 2023 11:39:03 -0400 Subject: [PATCH 25/27] add test for sending already response after invitor disconnects --- tests/invite-api.js | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/tests/invite-api.js b/tests/invite-api.js index d84768fc8..32245920f 100644 --- a/tests/invite-api.js +++ b/tests/invite-api.js @@ -376,6 +376,47 @@ test('invitor disconnecting results in invite reject response not throwing', asy const disconnect = replicate(r1, r2) }) +test('invitor disconnecting results in invite already response not throwing', async (t) => { + t.plan(3) + + const { rpc: r1, projectKey, encryptionKeys } = setup() + + const r2 = new MapeoRPC() + + let isMember = false + + const inviteApi = new InviteApi({ + rpc: r2, + queries: { + isMember: async () => { + return isMember + }, + addProject: async () => {}, + }, + }) + + r1.on('peers', async (peers) => { + if (peers.length !== 1 || peers[0].status === 'disconnected') return + + await t.exception(() => { + return r1.invite(peers[0].id, { + projectKey, + encryptionKeys, + }) + }, 'invite fails') + }) + + inviteApi.on('invite-received', async ({ projectId }) => { + t.is(projectId, projectKey.toString('hex'), 'received invite') + await disconnect() + isMember = true + await inviteApi.accept(projectId) + t.pass() + }) + + const disconnect = replicate(r1, r2) +}) + test('addProject throwing results in invite accept throwing', async (t) => { t.plan(1) From 693af73d18ecbf189abc0ec098ca7320bf8feb04 Mon Sep 17 00:00:00 2001 From: Andrew Chou Date: Mon, 25 Sep 2023 12:02:00 -0400 Subject: [PATCH 26/27] use public id instead of project id for public interface --- src/invite-api.js | 58 +++++++++++++++++++++++++++++++++++---------- tests/invite-api.js | 40 +++++++++++++++++-------------- 2 files changed, 68 insertions(+), 30 deletions(-) diff --git a/src/invite-api.js b/src/invite-api.js index bea8c63e2..ba66f5798 100644 --- a/src/invite-api.js +++ b/src/invite-api.js @@ -1,7 +1,7 @@ // @ts-check import { TypedEmitter } from 'tiny-typed-emitter' import { InviteResponse_Decision } from './generated/rpc.js' -import { projectKeyToId } from './utils.js' +import { projectKeyToId, projectKeyToPublicId } from './utils.js' /** * @typedef {Object} InviteApiEvents @@ -19,6 +19,7 @@ class MapOfSets extends Map { get(key) { const existing = super.get(key) if (existing) return existing + /** @type {Set} */ const set = new Set() super.set(key, set) return set @@ -29,6 +30,10 @@ class MapOfSets extends Map { * @extends {TypedEmitter} */ export class InviteApi extends TypedEmitter { + // Maps project public id -> project id + /** @type {Map} */ + #projectIdsMapping = new Map() + // Maps project id -> set of device ids /** @type {MapOfSets} */ #peersToRespondTo = new MapOfSets() @@ -74,27 +79,31 @@ export class InviteApi extends TypedEmitter { return } + const projectPublicId = projectKeyToPublicId(invite.projectKey) + + this.#projectIdsMapping.set(projectPublicId, projectId) + const peerIds = this.#peersToRespondTo.get(projectId) peerIds.add(peerId) if (this.#pendingInvites.has(projectId)) return this.#pendingInvites.set(projectId, { fromPeerId: peerId, invite }) + this.emit('invite-received', { - // TODO: Should this be the project public ID since it can be exposed to the client? - // Probably would require changing the public methods to accept the public ID - // and using the public ID for #invites and #peersToRespondTo keys instead peerId, - projectId, + projectId: projectPublicId, projectName: invite.projectInfo?.name, }) } /** - * @param {string} projectId + * @param {string} projectPublicId * @returns {Promise} */ - async accept(projectId) { + async accept(projectPublicId) { + const projectId = this.#getProjectIdFromPublicId(projectPublicId) + const isAlreadyMember = await this.#isMember(projectId) const peersToRespondTo = this.#peersToRespondTo.get(projectId) @@ -108,11 +117,16 @@ export class InviteApi extends TypedEmitter { const pendingInvite = this.#pendingInvites.get(projectId) if (!pendingInvite) { - throw new Error(`Cannot find invite for project with ID ${projectId}`) + throw new Error( + `Cannot find invite for project with ID ${projectPublicId}` + ) } try { - this.#sendAcceptResponse({ peerId: pendingInvite.fromPeerId, projectId }) + this.#sendAcceptResponse({ + peerId: pendingInvite.fromPeerId, + projectId, + }) // TODO: Add another RPC message to confirm invitee is written into project after accepting } catch (e) { // TODO: If can't accept invite because peer it was sent from has @@ -136,12 +150,16 @@ export class InviteApi extends TypedEmitter { } /** - * @param {string} projectId + * @param {string} projectPublicId * @returns {Promise} */ - async reject(projectId) { + async reject(projectPublicId) { + const projectId = this.#getProjectIdFromPublicId(projectPublicId) + if (!this.#pendingInvites.has(projectId)) { - throw new Error(`Cannot find invite for project with ID ${projectId}`) + throw new Error( + `Cannot find invite for project with ID ${projectPublicId}` + ) } for (const peerId of this.#peersToRespondTo.get(projectId)) { this.#sendRejectResponse({ peerId, projectId }) @@ -196,4 +214,20 @@ export class InviteApi extends TypedEmitter { // will consider the invite failed anyway } } + + /** + * @param {string} projectPublicId + * @returns {string} + */ + #getProjectIdFromPublicId(projectPublicId) { + const projectId = this.#projectIdsMapping.get(projectPublicId) + + if (!projectId) { + throw new Error( + `Cannot find project internal ID for public ID ${projectPublicId}` + ) + } + + return projectId + } } diff --git a/tests/invite-api.js b/tests/invite-api.js index 32245920f..85f19f152 100644 --- a/tests/invite-api.js +++ b/tests/invite-api.js @@ -3,6 +3,7 @@ import { randomBytes } from 'crypto' import { KeyManager } from '@mapeo/crypto' import { MapeoRPC } from '../src/rpc/index.js' import { InviteApi } from '../src/invite-api.js' +import { projectKeyToPublicId } from '../src/utils.js' import { replicate } from './helpers/rpc.js' import NoiseSecretStream from '@hyperswarm/secret-stream' import pDefer from 'p-defer' @@ -41,9 +42,7 @@ test('invite-received event has expected payload', async (t) => { r1.invite(peers[0].id, { projectKey, encryptionKeys, - projectInfo: { - name: 'Mapeo', - }, + projectInfo: { name: 'Mapeo' }, }) }) @@ -52,7 +51,7 @@ test('invite-received event has expected payload', async (t) => { async ({ peerId, projectId, projectName }) => { t.is(peerId, expectedInvitorPeerId) t.is(projectName, 'Mapeo') - t.is(projectId, projectKey.toString('hex')) + t.is(projectId, projectKeyToPublicId(projectKey)) } ) @@ -72,10 +71,11 @@ test('Accept invite', async (t) => { rpc: r2, queries: { isMember: async (projectId) => { - return projects.has(projectId) + const projectKey = Buffer.from(projectId, 'hex') + return projects.has(projectKeyToPublicId(projectKey)) }, addProject: async (invite) => { - projects.set(invite.projectKey.toString('hex'), invite) + projects.set(projectKeyToPublicId(invite.projectKey), invite) }, }, }) @@ -92,11 +92,13 @@ test('Accept invite', async (t) => { }) inviteApi.on('invite-received', async ({ projectId }) => { - t.is(projectId, projectKey.toString('hex')) + t.is(projectId, projectKeyToPublicId(projectKey)) await inviteApi.accept(projectId) t.ok(projects.has(projectId), 'project successfully added') + + // t.exception(async () => inviteApi.accept(projectId)) }) replicate(r1, r2) @@ -115,10 +117,11 @@ test('Reject invite', async (t) => { rpc: r2, queries: { isMember: async (projectId) => { - return projects.has(projectId) + const projectKey = Buffer.from(projectId, 'hex') + return projects.has(projectKeyToPublicId(projectKey)) }, addProject: async (invite) => { - projects.set(invite.projectKey.toString('hex'), invite) + projects.set(projectKeyToPublicId(invite.projectKey), invite) }, }, }) @@ -135,7 +138,7 @@ test('Reject invite', async (t) => { }) inviteApi.on('invite-received', async ({ projectId }) => { - t.is(projectId, projectKey.toString('hex')) + t.is(projectId, projectKeyToPublicId(projectKey)) await inviteApi.reject(projectId) @@ -330,7 +333,7 @@ test('invitor disconnecting results in accept throwing', async (t) => { }) inviteApi.on('invite-received', async ({ projectId }) => { - t.is(projectId, projectKey.toString('hex'), 'received invite') + t.is(projectId, projectKeyToPublicId(projectKey), 'received invite') await disconnect() await t.exception(() => { return inviteApi.accept(projectId) @@ -367,7 +370,7 @@ test('invitor disconnecting results in invite reject response not throwing', asy }) inviteApi.on('invite-received', async ({ projectId }) => { - t.is(projectId, projectKey.toString('hex'), 'received invite') + t.is(projectId, projectKeyToPublicId(projectKey), 'received invite') await disconnect() await inviteApi.reject(projectId) t.pass() @@ -407,7 +410,7 @@ test('invitor disconnecting results in invite already response not throwing', as }) inviteApi.on('invite-received', async ({ projectId }) => { - t.is(projectId, projectKey.toString('hex'), 'received invite') + t.is(projectId, projectKeyToPublicId(projectKey), 'received invite') await disconnect() isMember = true await inviteApi.accept(projectId) @@ -464,12 +467,13 @@ test('Invite from multiple peers', async (t) => { rpc: invitee, queries: { isMember: async (projectId) => { - return projects.has(projectId) + const projectKey = Buffer.from(projectId, 'hex') + return projects.has(projectKeyToPublicId(projectKey)) }, addProject: async (invite) => { - const projectId = invite.projectKey.toString('hex') - t.absent(projects.has(projectId), 'add project called only once') - projects.set(projectId, invite) + const projectPublicId = projectKeyToPublicId(invite.projectKey) + t.absent(projects.has(projectPublicId), 'add project called only once') + projects.set(projectPublicId, invite) }, }, }) @@ -479,7 +483,7 @@ test('Invite from multiple peers', async (t) => { const deferred = pDefer() inviteApi.on('invite-received', async ({ projectId, peerId }) => { - t.is(projectId, projectKey.toString('hex'), 'expected project id') + t.is(projectId, projectKeyToPublicId(projectKey), 'expected project id') t.absent(first, 'should only receive invite once') first = peerId From 370b4be031a6078e09ac65f234d269c631aa2486 Mon Sep 17 00:00:00 2001 From: Andrew Chou Date: Mon, 25 Sep 2023 12:21:25 -0400 Subject: [PATCH 27/27] remove commented-out line --- tests/invite-api.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/invite-api.js b/tests/invite-api.js index 85f19f152..850419669 100644 --- a/tests/invite-api.js +++ b/tests/invite-api.js @@ -97,8 +97,6 @@ test('Accept invite', async (t) => { await inviteApi.accept(projectId) t.ok(projects.has(projectId), 'project successfully added') - - // t.exception(async () => inviteApi.accept(projectId)) }) replicate(r1, r2)