Skip to content

Commit

Permalink
fix: Various fixes related to peers, CSRs and backend startup (#2455)
Browse files Browse the repository at this point in the history
Fixes for the following issues:
- Peers can be deleted if CSRs don't sync
- Backend starting before the frontend is ready, resulting in missed events
- Adding duplicate CSRs
  • Loading branch information
Lucas Leblow authored Apr 29, 2024
1 parent e2f0204 commit 2af0531
Show file tree
Hide file tree
Showing 36 changed files with 256 additions and 448 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ export class ConnectionsManagerService extends EventEmitter implements OnModuleI
const network = await this.localDbService.getNetworkInfo()

if (community && network) {
const sortedPeers = await this.localDbService.getSortedPeers(community.peerList)
const sortedPeers = await this.localDbService.getSortedPeers(community.peerList ?? [])
this.logger('launchCommunityFromStorage - sorted peers', sortedPeers)
if (sortedPeers.length > 0) {
community.peerList = sortedPeers
Expand Down Expand Up @@ -517,6 +517,7 @@ export class ConnectionsManagerService extends EventEmitter implements OnModuleI
agent: this.socksProxyAgent,
localAddress: this.libp2pService.createLibp2pAddress(onionAddress, peerId.toString()),
targetPort: this.ports.libp2pHiddenService,
// Ignore local address
peers: peers ? peers.slice(1) : [],
psk: Libp2pService.generateLibp2pPSK(community.psk).fullKey,
}
Expand Down
18 changes: 3 additions & 15 deletions packages/backend/src/nest/local-db/local-db.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,21 +95,9 @@ export class LocalDbService {
}
}

public async getSortedPeers(
peers?: string[] | undefined,
includeLocalPeerAddress: boolean = true
): Promise<string[]> {
if (!peers) {
const currentCommunity = await this.getCurrentCommunity()
if (!currentCommunity) {
throw new Error('No peers were provided and no community was found to extract peers from')
}
peers = currentCommunity.peerList
if (!peers) {
throw new Error('No peers provided and no peers found on current stored community')
}
}

// I think we can move this into StorageService (keep this service
// focused on CRUD).
public async getSortedPeers(peers: string[], includeLocalPeerAddress: boolean = true): Promise<string[]> {
const peersStats = (await this.get(LocalDBKeys.PEERS)) || {}
const stats: NetworkStats[] = Object.values(peersStats)
const network = await this.getNetworkInfo()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ export const extractPendingCsrs = async (payload: { csrs: string[]; certificates
pendingCsrs.push(csr)
}
}
logger('DuplicatedCertBug', { parsedUniqueCsrs, pendingNames, certNames })
return pendingCsrs
}

Expand Down
13 changes: 7 additions & 6 deletions packages/backend/src/nest/registration/registration.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,14 @@ export class RegistrationService extends EventEmitter implements OnModuleInit {
}

public async tryIssueCertificates() {
this.logger('Trying to issue certificates', this.registrationEventInProgress, this.registrationEvents)
this.logger('Trying to process registration event')
// Process only a single registration event at a time so that we
// do not register two certificates with the same name.
if (!this.registrationEventInProgress) {
// Get the next event.
const event = this.registrationEvents.shift()
if (event) {
this.logger('Issuing certificates', event)
this.logger('Processing registration event', event)
// Event processing in progress
this.registrationEventInProgress = true

Expand All @@ -62,7 +62,7 @@ export class RegistrationService extends EventEmitter implements OnModuleInit {
certificates: (await this.storageService?.loadAllCertificates()) as string[],
})

this.logger('Finished issuing certificates')
this.logger('Finished processing registration event')
// Event processing finished
this.registrationEventInProgress = false

Expand All @@ -71,6 +71,8 @@ export class RegistrationService extends EventEmitter implements OnModuleInit {
setTimeout(this.tryIssueCertificates.bind(this), 0)
}
}
} else {
this.logger('Registration event processing already in progress')
}
}

Expand All @@ -90,14 +92,14 @@ export class RegistrationService extends EventEmitter implements OnModuleInit {
return
}

this.logger('DuplicatedCertBug', { payload })
const pendingCsrs = await extractPendingCsrs(payload)
this.logger('DuplicatedCertBug', { pendingCsrs })
this.logger(`Issuing certificates`)
await Promise.all(
pendingCsrs.map(async csr => {
await this.registerUserCertificate(csr)
})
)
this.logger('Total certificates issued:', pendingCsrs.length)
}

// TODO: This doesn't save the owner's certificate in OrbitDB, so perhaps we
Expand All @@ -121,7 +123,6 @@ export class RegistrationService extends EventEmitter implements OnModuleInit {

public async registerUserCertificate(csr: string): Promise<void> {
const result = await issueCertificate(csr, this.permsData)
this.logger('DuplicatedCertBug', { result })
if (result?.cert) {
// Save certificate (awaited) so that we are sure that the certs
// are saved before processing the next round of CSRs.
Expand Down
14 changes: 9 additions & 5 deletions packages/backend/src/nest/socket/socket.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,36 +47,40 @@ export class SocketService extends EventEmitter implements OnModuleInit {
}

async onModuleInit() {
this.logger('init:started')
this.logger('init: Started')

this.attachListeners()
await this.init()

this.logger('init:finished')
this.logger('init: Finished')
}

public async init() {
const connection = new Promise<void>(resolve => {
this.serverIoProvider.io.on(SocketActionTypes.CONNECTION, socket => {
this.logger('init: connection')
resolve()
socket.on(SocketActionTypes.START, async () => {
resolve()
})
})
})

await this.listen()

this.logger('init: Waiting for frontend to connect')
await connection
this.logger('init: Frontend connected')
}

private readonly attachListeners = (): void => {
// Attach listeners here
this.serverIoProvider.io.on(SocketActionTypes.CONNECTION, socket => {
this.logger('socket connection')
this.logger('Socket connection')

// On websocket connection, update presentation service with network data
this.emit(SocketActionTypes.CONNECTION)

socket.on(SocketActionTypes.CLOSE, async () => {
this.logger('Socket connection closed')
this.emit(SocketActionTypes.CLOSE)
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ export class CertificatesRequestsStore extends EventEmitter {
write: ['*'],
},
})
await this.store.load()

this.store.events.on('write', async (_address, entry) => {
this.logger('Added CSR to database')
Expand All @@ -41,8 +40,8 @@ export class CertificatesRequestsStore extends EventEmitter {
this.loadedCertificateRequests()
})

// TODO: Load CSRs in case the owner closes the app before issuing
// certificates
// @ts-ignore
await this.store.load({ fetchEntryTimeout: 15000 })
this.logger('Initialized')
}

Expand Down Expand Up @@ -77,7 +76,7 @@ export class CertificatesRequestsStore extends EventEmitter {
await parsedCsr.verify()
await this.validateCsrFormat(csr)
} catch (err) {
console.error('Failed to validate user csr:', csr, err?.message)
console.error('Failed to validate user CSR:', csr, err?.message)
return false
}
return true
Expand All @@ -100,15 +99,13 @@ export class CertificatesRequestsStore extends EventEmitter {
.map(e => {
return e.payload.value
})
this.logger('Total CSRs:', allEntries.length)

this.logger('DuplicatedCertBug', { allEntries })
const allCsrsUnique = [...new Set(allEntries)]
this.logger('DuplicatedCertBug', { allCsrsUnique })
await Promise.all(
allCsrsUnique
.filter(async csr => {
const validation = await CertificatesRequestsStore.validateUserCsr(csr)
this.logger('DuplicatedCertBug', { validation, csr })
if (validation) return true
return false
})
Expand All @@ -122,8 +119,9 @@ export class CertificatesRequestsStore extends EventEmitter {
filteredCsrsMap.set(pubKey, csr)
})
)
this.logger('DuplicatedCertBug', '[...filteredCsrsMap.values()]', [...filteredCsrsMap.values()])
return [...filteredCsrsMap.values()]
const validCsrs = [...filteredCsrsMap.values()]
this.logger('Valid CSRs:', validCsrs.length)
return validCsrs
}

public clean() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,6 @@ describe('CertificatesStore', () => {

await certificatesStore.addCertificate(certificate)

// @ts-expect-error - getCertificates is protected
const certificates = await certificatesStore.getCertificates()

expect(certificates).toContain(certificate)
Expand All @@ -149,7 +148,6 @@ describe('CertificatesStore', () => {

await certificatesStore.addCertificate(certificate)

// @ts-expect-error - getCertificates is protected
const certificates = await certificatesStore.getCertificates()

expect(certificates).not.toContain(certificate)
Expand All @@ -161,7 +159,6 @@ describe('CertificatesStore', () => {

certificatesStore.updateMetadata(communityMetadata)

// @ts-expect-error - getCertificates is protected
jest.spyOn(certificatesStore, 'getCertificates').mockResolvedValue([certificate1, certificate2])

const certificates = await certificatesStore.loadAllCertificates()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ export class CertificatesStore extends EventEmitter {
* as specified in the comment section of
* https://github.com/TryQuiet/quiet/issues/1899
*/
protected async getCertificates() {
public async getCertificates(): Promise<string[]> {
if (!this.store) {
return []
}
Expand All @@ -166,7 +166,6 @@ export class CertificatesStore extends EventEmitter {
}

const validation = await this.validateCertificate(certificate)
this.logger('DuplicatedCertBug', { validation, certificate })
if (validation) {
const parsedCertificate = parseCertificate(certificate)
const pubkey = keyFromCertificate(parsedCertificate)
Expand All @@ -190,7 +189,8 @@ export class CertificatesStore extends EventEmitter {

const validCerts = validCertificates.filter(i => i != undefined)
this.logger(`Valid certificates: ${validCerts.length}`)
return validCerts
// TODO: Why doesn't TS infer this properly?
return validCerts as string[]
}

public async getCertificateUsername(pubkey: string) {
Expand Down
49 changes: 42 additions & 7 deletions packages/backend/src/nest/storage/storage.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -511,17 +511,52 @@ describe('StorageService', () => {

describe('Users', () => {
it('gets all users from db', async () => {
await storageService.init(peerId)
const mockGetCsrs = jest.fn()
// @ts-ignore - Property 'getAllEventLogEntries' is protected
storageService.getAllEventLogEntries = mockGetCsrs
mockGetCsrs.mockReturnValue([
const certs = [
// b
'MIICITCCAcegAwIBAgIGAY8GkBEVMAoGCCqGSM49BAMCMAwxCjAIBgNVBAMTAWEwHhcNMjQwNDIyMTYwNzM1WhcNMzAwMjAxMDcwMDAwWjBJMUcwRQYDVQQDEz56Z2hpZGV4czdxdDI0aXZ1M2pvYmpxZHR6end0eWF1NGxwcG51bng1cGtpZjc2cGtweXA3cWNpZC5vbmlvbjBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABDG8SNnoS1BYoV72jcyQFVlsrwvd2Bb9/9L13Tc4SHJwitTUB3F+y/7pk0tAPrZi2qasU2PO9lTwUxXYcAfpCRSjgdcwgdQwCQYDVR0TBAIwADALBgNVHQ8EBAMCAIAwHQYDVR0lBBYwFAYIKwYBBQUHAwIGCCsGAQUFBwMBMBEGCisGAQQBg4wbAgEEAxMBYjA9BgkrBgECAQ8DAQEEMBMuUW1lUGJCMjVoMWZYN1dBRk42ckZSNGFWRFdVRlFNU3RSSEdERFM0UlFaUTRZcTBJBgNVHREEQjBAgj56Z2hpZGV4czdxdDI0aXZ1M2pvYmpxZHR6end0eWF1NGxwcG51bng1cGtpZjc2cGtweXA3cWNpZC5vbmlvbjAKBggqhkjOPQQDAgNIADBFAiBkTZo6/D0YgNMPcDpuf7n+rDEQls6cMVxEVw/H8vxbhwIhAM+e6we9YP4JeNgOGgd0iZNEpq8N7dla4XO+YVWrh0YG',

// c
'MIICITCCAcegAwIBAgIGAY8Glf+pMAoGCCqGSM49BAMCMAwxCjAIBgNVBAMTAWEwHhcNMjQwNDIyMTYxNDA0WhcNMzAwMjAxMDcwMDAwWjBJMUcwRQYDVQQDEz5uaGxpdWpuNjZlMzQ2ZXZ2dnhlYWY0cW1hN3Bxc2hjZ2J1NnQ3d3Nlb2FubmMyZnk0Y25zY3J5ZC5vbmlvbjBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABP1WBKQdMz5yMpv5hWj6j+auIsnfiJE8dtuxeeM4N03K1An61F0o47CWg04DydwmoPn5gwefEv8t9Cz9nv/VUGejgdcwgdQwCQYDVR0TBAIwADALBgNVHQ8EBAMCAIAwHQYDVR0lBBYwFAYIKwYBBQUHAwIGCCsGAQUFBwMBMBEGCisGAQQBg4wbAgEEAxMBYzA9BgkrBgECAQ8DAQEEMBMuUW1WY1hRTXVmRWNZS0R0d3NFSlRIUGJzc3BCeU02U0hUYlJHR2VEdkVFdU1RQTBJBgNVHREEQjBAgj5uaGxpdWpuNjZlMzQ2ZXZ2dnhlYWY0cW1hN3Bxc2hjZ2J1NnQ3d3Nlb2FubmMyZnk0Y25zY3J5ZC5vbmlvbjAKBggqhkjOPQQDAgNIADBFAiEAgMCBxF3oK4ituEWcAK6uawMCludZu4YujIpBIR+v2LICIBhMHXrBy1KWc70t6idB+5XkInsRZz5nw1vwgRJ4mw98',
]

const csrs = [
// c
'MIIB4TCCAYgCAQAwSTFHMEUGA1UEAxM+emdoaWRleHM3cXQyNGl2dTNqb2JqcWR0enp3dHlhdTRscHBudW54NXBraWY3NnBrcHlwN3FjaWQub25pb24wWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAAQxvEjZ6EtQWKFe9o3MkBVZbK8L3dgW/f/S9d03OEhycIrU1Adxfsv+6ZNLQD62YtqmrFNjzvZU8FMV2HAH6QkUoIHcMC4GCSqGSIb3DQEJDjEhMB8wHQYDVR0OBBYEFG1W6vJTK/uPuRK2LPaVZyebVVc+MA8GCSqGSIb3DQEJDDECBAAwEQYKKwYBBAGDjBsCATEDEwFiMD0GCSsGAQIBDwMBATEwEy5RbWVQYkIyNWgxZlg3V0FGTjZyRlI0YVZEV1VGUU1TdFJIR0REUzRSUVpRNFlxMEcGA1UdETFAEz56Z2hpZGV4czdxdDI0aXZ1M2pvYmpxZHR6end0eWF1NGxwcG51bng1cGtpZjc2cGtweXA3cWNpZC5vbmlvbjAKBggqhkjOPQQDAgNHADBEAiAjxneoJZtCzkd75HTT+pcj+objG3S04omjeMMw1N+B/wIgAaJRgifnWEnWFYm614UmPw9un2Uwk1gVhN2tSwJ65sM=',

// o
'MIIDHjCCAsMCAQAwSTFHMEUGA1UEAxM+NnZ1MmJ4a2k3NzdpdDNjcGF5djZmcTZ2cGw0a2Uza3pqN2d4aWNmeWdtNTVkaGh0cGh5ZmR2eWQub25pb24wWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAATMpfp2hSfWFL26OZlZKZEWG9fyAM1ndlEzO0kLxT0pA/7/fs+a5X/s4TkzqCVVQSzhas/84q0WE99ScAcM1LQJoIICFjAuBgkqhkiG9w0BCQ4xITAfMB0GA1UdDgQWBBR6VRzktP1pzZxsGUaJivNUrtgSrzCCAUcGCSqGSIb3DQEJDDGCATgEggE0KZq9s6HEViRfplVgYkulg6XV411ZRe4U1UjfXTf1pRaygfcenGbT6RRagPtZzjuq5hHdYhqDjRzZhnbn8ZASYTgBM7qcseUq5UpS1pE08DI2jePKqatp3Pzm6a/MGSziESnREx784JlKfwKMjJl33UA8lQm9nhSeAIHyBx3c4Lf8IXdW2n3rnhbVfjpBMAxwh6lt+e5agtGXy+q/xAESUeLPfUgRYWctlLgt8Op+WTpLyBkZsVFoBvJrMt2XdM0RI32YzTRr56GXFa4VyQmY5xXwlQSPgidAP7jPkVygNcoeXvAz2ZCk3IR1Cn3mX8nMko53MlDNaMYldUQA0ug28/S7BlSlaq2CDD4Ol3swTq7C4KGTxKrI36ruYUZx7NEaQDF5V7VvqPCZ0fZoTIJuSYTQ67gwEQYKKwYBBAGDjBsCATEDEwFvMD0GCSsGAQIBDwMBATEwEy5RbVhSWTRyaEF4OE11cThkTUdrcjlxa25KZEU2VUhaRGRHYURSVFFFYndGTjViMEcGA1UdETFAEz42dnUyYnhraTc3N2l0M2NwYXl2NmZxNnZwbDRrZTNremo3Z3hpY2Z5Z201NWRoaHRwaHlmZHZ5ZC5vbmlvbjAKBggqhkjOPQQDAgNJADBGAiEAt+f1u/bchg5AZHv6NTGNoXeejTRWUhX3ioGwW6TGg84CIQCHqKNzDh2JjS/hUHx5PApAmfNnQTSf19X6LnNHQweU1g==',

// o
'MIIDHTCCAsMCAQAwSTFHMEUGA1UEAxM+eTd5Y3ptdWdsMnRla2FtaTdzYmR6NXBmYWVtdng3YmFod3RocmR2Y2J6dzV2ZXgyY3JzcjI2cWQub25pb24wWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAATMq0l4bCmjdb0grtzpwtDVLM9E1IQpL9vrB4+lD9OBZzlrx2365jV7shVu9utas8w8fxtKoBZSnT5+32ZMFTB4oIICFjAuBgkqhkiG9w0BCQ4xITAfMB0GA1UdDgQWBBSoDQpTZdEvi1/Rr/muVXT1clyKRDCCAUcGCSqGSIb3DQEJDDGCATgEggE0BQvyvkiiXEf/PLKnsR1Ba9AhYsVO8o56bnftUnoVzBlRZgUzLJvOSroPk/EmbVz+okhMrcYNgCWHvxrAqHVVq0JRP6bi98BtCUotx6OPFHp5K5QCL60hod1uAnhKocyJG9tsoM9aS+krn/k+g4RCBjiPZ25cC7QG/UNr6wyIQ8elBho4MKm8iOp7EShSsZOV1f6xrnXYCC/zyUc85GEuycLzVImgAQvPATbdMzY4zSGnNLHxkvSUNxaR9LnEWf+i1jeqcOiXOvmdyU5Be3ZqhGKvvBg/5vyLQiCIfeapjZemnLqFHQBitglDm2xnKL6HzMyfZoAHPV7YcWYR4spU9Ju8Q8aqSeAryx7sx55eSR4GO5UQTo5DrQn6xtkwOZ/ytsOknFthF8jcA9uTAMDKA2TylCUwEQYKKwYBBAGDjBsCATEDEwFvMD0GCSsGAQIBDwMBATEwEy5RbVQxOFV2blVCa3NlTWMzU3FuZlB4cEh3TjhuekxySmVOU0xadGM4ckFGWGh6MEcGA1UdETFAEz55N3ljem11Z2wydGVrYW1pN3NiZHo1cGZhZW12eDdiYWh3dGhyZHZjYnp3NXZleDJjcnNyMjZxZC5vbmlvbjAKBggqhkjOPQQDAgNIADBFAiEAoFrAglxmk7ciD6AHQOB1qEoLu0NARcxgwmIry8oeTHwCICyXp5NJQ9Z8vReIAQNng2H2+/XjHifZEWzhoN0VkcBx',
])
const allUsers = storageService.getAllUsers()
]

await storageService.init(peerId)
// @ts-ignore
storageService.certificatesRequestsStore = {
getCsrs: jest.fn(() => {
return csrs
}),
}
// @ts-ignore
storageService.certificatesStore = {
getCertificates: jest.fn(() => {
return certs
}),
}

const allUsers = await storageService.getAllUsers()

expect(allUsers).toStrictEqual([
{
onionAddress: 'zghidexs7qt24ivu3jobjqdtzzwtyau4lppnunx5pkif76pkpyp7qcid.onion',
peerId: 'QmePbB25h1fX7WAFN6rFR4aVDWUFQMStRHGDDS4RQZQ4Yq',
username: 'b',
},
{
onionAddress: 'nhliujn66e346evvvxeaf4qma7pqshcgbu6t7wseoannc2fy4cnscryd.onion',
peerId: 'QmVcXQMufEcYKDtwsEJTHPbsspByM6SHTbRGGeDvEEuMQA',
username: 'c',
},
{
onionAddress: '6vu2bxki777it3cpayv6fq6vpl4ke3kzj7gxicfygm55dhhtphyfdvyd.onion',
peerId: 'QmXRY4rhAx8Muq8dMGkr9qknJdE6UHZDdGaDRTQEbwFN5b',
Expand Down
Loading

0 comments on commit 2af0531

Please sign in to comment.