Fixing user registration concurrency issues

packages/backend/src/nest/connections-manager/connections-manager.service.ts

@@ -318,11 +318,25 @@ export class ConnectionsManagerService extends EventEmitter implements OnModuleI
     await this.launchCommunity(payload)
     this.logger(`Created and launched community ${payload.id}`)
     this.serverIoProvider.io.emit(SocketActionTypes.NEW_COMMUNITY, { id: payload.id })
+
+    if (!payload.ownerCsr || !payload.permsData) {
+      throw new Error("Owner CSR and PermsData required to create community!")
+    }
+    // In launchCommunity, we initialize the StorageService and also
+    // the RegistrationService for the first time. And then now, once
+    // those are ready, we register the owner's certificate.
+    const cert = await this.registrationService.registerOwnerCertificate({ communityId: payload.id, userCsr: payload.ownerCsr, permsData: payload.permsData })
+    if (payload.certs) {
+      // Hacking, perhaps make certs.certificate optional
+      payload.certs.certificate = cert || ''
+    }
+    await this.localDbService.put(LocalDBKeys.COMMUNITY, payload)
   }
 
   public async launchCommunity(payload: InitCommunityPayload) {
     this.logger('Launching community: peers:', payload.peers)
     this.communityState = ServiceState.LAUNCHING
+
     // Perhaps we should call this data something else, since we already have a Community type.
     // It seems like InitCommunityPayload is a mix of various connection metadata.
     const communityData: InitCommunityPayload = await this.localDbService.get(LocalDBKeys.COMMUNITY)
@@ -424,6 +438,7 @@ export class ConnectionsManagerService extends EventEmitter implements OnModuleI
       this.serverIoProvider.io.emit(SocketActionTypes.PEER_DISCONNECTED, payload)
     })
     await this.storageService.init(_peerId)
+    await this.registrationService.init(this.storageService)
     this.logger('storage initialized')
   }
 
@@ -444,10 +459,6 @@ export class ConnectionsManagerService extends EventEmitter implements OnModuleI
     this.registrationService.on(RegistrationEvents.ERROR, payload => {
       emitError(this.serverIoProvider.io, payload)
     })
-    this.registrationService.on(RegistrationEvents.NEW_USER, async payload => {
-      await this.storageService?.saveCertificate(payload)
-    })
-
     this.registrationService.on(RegistrationEvents.FINISHED_ISSUING_CERTIFICATES_FOR_ID, payload => {
       if (payload.id) {
         this.storageService.resolveCsrReplicatedPromise(payload.id)

packages/backend/src/nest/registration/registration.service.ts

@@ -3,13 +3,15 @@ import { EventEmitter } from 'events'
 import { extractPendingCsrs, issueCertificate } from './registration.functions'
 import { ErrorCodes, ErrorMessages, PermsData, RegisterOwnerCertificatePayload, SocketActionTypes } from '@quiet/types'
 import { RegistrationEvents } from './registration.types'
+import { StorageService } from '../storage/storage.service'
 import Logger from '../common/logger'
 
 @Injectable()
 export class RegistrationService extends EventEmitter implements OnModuleInit {
   private readonly logger = Logger(RegistrationService.name)
   public certificates: string[] = []
   private _permsData: PermsData
+  private storageService: StorageService
 
   constructor() {
     super()
@@ -19,31 +21,13 @@ export class RegistrationService extends EventEmitter implements OnModuleInit {
     this.on(
       RegistrationEvents.REGISTER_USER_CERTIFICATE,
       async (payload: { csrs: string[]; certificates: string[]; id: string }) => {
-        await this.issueCertificates(payload)
+        await this.registerUserCertificates({ csrs: payload.csrs, id: payload.id })
       }
     )
   }
 
-  private async issueCertificates(payload: { csrs: string[]; certificates: string[]; id?: string }) {
-    // Lack of permsData means that we are not the owner of the
-    // community in the official model of the app, however anyone can
-    // modify the source code, put malicious permsData here, issue
-    // false certificates and try to trick other users. To prevent
-    // that, peers verify that anything that is written to the
-    // certificate store is signed by the owner.
-    if (!this._permsData) {
-      if (payload.id) this.emit(RegistrationEvents.FINISHED_ISSUING_CERTIFICATES_FOR_ID, { id: payload.id })
-      return
-    }
-    const pendingCsrs = await extractPendingCsrs(payload)
-
-    await Promise.all(
-      pendingCsrs.map(async csr => {
-        await this.registerUserCertificate(csr)
-      })
-    )
-
-    if (payload.id) this.emit(RegistrationEvents.FINISHED_ISSUING_CERTIFICATES_FOR_ID, { id: payload.id })
+  public init(storageService: StorageService) {
+    this.storageService = storageService
   }
 
   public set permsData(perms: PermsData) {
@@ -53,15 +37,26 @@ export class RegistrationService extends EventEmitter implements OnModuleInit {
     }
   }
 
-  public async registerOwnerCertificate(payload: RegisterOwnerCertificatePayload): Promise<void> {
+  public async registerOwnerCertificate(payload: RegisterOwnerCertificatePayload): Promise<string | undefined> {
+    if (!this.storageService) {
+      throw new Error("Storage Service must be initialized before the Registration Service")
+    }
+
     // FIXME: We should resolve problems with events order and we should set permsData only on LAUNCH_REGISTRART socket event in connectionsManager.
     this._permsData = payload.permsData
     const result = await issueCertificate(payload.userCsr.userCsr, this._permsData)
     if (result?.cert) {
+      await this.storageService.certificatesStore.addCertificate(result.cert)
+      // Not sure if this is necessary
+      const certs = await this.storageService.certificatesStore.loadAllCertificates()
+      if (!certs.includes(result.cert)) {
+        throw new Error("Cert wasn't added to CertificateStore correctly")
+      }
       this.emit(SocketActionTypes.SAVED_OWNER_CERTIFICATE, {
         communityId: payload.communityId,
         network: { certificate: result.cert },
       })
+      return result?.cert
     } else {
       this.emit(SocketActionTypes.ERROR, {
         type: SocketActionTypes.REGISTRAR,
@@ -72,10 +67,53 @@ export class RegistrationService extends EventEmitter implements OnModuleInit {
     }
   }
 
-  public async registerUserCertificate(csr: string): Promise<void> {
-    const result = await issueCertificate(csr, this._permsData)
-    if (result?.cert) {
-      this.emit(RegistrationEvents.NEW_USER, { certificate: result.cert })
+  // Apparently, JS will run each function to completion. So assuming
+  // we do not have multiple threads, this function should run to
+  // completion before it is called again. Because we take the CSRs
+  // and then get the latest view of certificates, filter CSRs and
+  // then update certificates in a single function, we should never
+  // issue a CSR that is contained in the certificates list, at least
+  // in this function... as long as there is no other code that
+  // updates the certificates list. Something else to consider is that
+  // because this function is called asynchronously, there may be
+  // several invocations with different CSRs and they may run in an
+  // unexpected order. We could address that if it's an issue, but I
+  // think that might only affect the order of CSR registration.
+  public async registerUserCertificates(payload: { csrs: string[]; id?: string }) {
+    if (!this.storageService) {
+      throw new Error("Storage Service must be initialized before the Registration Service")
     }
+
+    // Lack of permsData means that we are not the owner of the
+    // community in the official model of the app, however anyone can
+    // modify the source code, put malicious permsData here, issue
+    // false certificates and try to trick other users. To prevent
+    // that, peers verify that anything that is written to the
+    // certificate store is signed by the owner.
+    if (!this._permsData) {
+      if (payload.id) this.emit(RegistrationEvents.FINISHED_ISSUING_CERTIFICATES_FOR_ID, { id: payload.id })
+      return
+    }
+
+    const certificates = await this.storageService.certificatesStore.loadAllCertificates()
+
+    const pendingCsrs = await extractPendingCsrs({ csrs: payload.csrs, certificates: certificates as string[] })
+
+    await Promise.all(
+      pendingCsrs.map(async csr => {
+        const result = await issueCertificate(csr, this._permsData)
+        if (result?.cert) {
+          await this.storageService.certificatesStore.addCertificate(result.cert)
+          // Not sure if this is necessary
+          const certs = await this.storageService.certificatesStore.loadAllCertificates()
+          if (!certs.includes(result.cert)) {
+            throw new Error("Cert wasn't added to CertificateStore correctly")
+          }
+          this.emit(RegistrationEvents.NEW_USER, { certificate: result.cert })
+        }
+      })
+    )
+
+    if (payload.id) this.emit(RegistrationEvents.FINISHED_ISSUING_CERTIFICATES_FOR_ID, { id: payload.id })
   }
 }

packages/state-manager/src/sagas/identity/registerCertificate/registerCertificate.saga.ts

@@ -2,11 +2,13 @@ import { applyEmitParams, type Socket } from '../../../types'
 import { type PayloadAction } from '@reduxjs/toolkit'
 import { apply, select, put } from 'typed-redux-saga'
 import { communitiesSelectors } from '../../communities/communities.selectors'
+import { identitySelectors } from '../../identity/identity.selectors'
 import { identityActions } from '../identity.slice'
 import {
   type RegisterOwnerCertificatePayload,
   type RegisterUserCertificatePayload,
   SocketActionTypes,
+  type InitCommunityPayload,
 } from '@quiet/types'
 import { communitiesActions } from '../../communities/communities.slice'
 
@@ -22,16 +24,30 @@ export function* registerCertificateSaga(
   }
 
   if (currentCommunity.CA?.rootCertString) {
-    const payload: RegisterOwnerCertificatePayload = {
-      communityId: action.payload.communityId,
-      userCsr: action.payload.userCsr,
+    const identity = yield* select(identitySelectors.selectById(currentCommunity.id))
+    if (!identity?.userCsr || !currentCommunity?.rootCa) {
+      console.error("User CSR or root cert missing", identity?.userCsr, currentCommunity?.rootCa)
+      return
+    }
+
+    const payload: InitCommunityPayload = {
+      id: currentCommunity.id,
+      peerId: identity.peerId,
+      hiddenService: identity.hiddenService,
+      certs: {
+        // Hacking, perhaps make certs.certificate optional
+        certificate: identity.userCertificate || '',
+        key: identity.userCsr.userKey,
+        CA: [currentCommunity.rootCa],
+      },
+      ownerCsr: identity.userCsr,
       permsData: {
         certificate: currentCommunity.CA.rootCertString,
         privKey: currentCommunity.CA.rootKeyString,
       },
     }
 
-    yield* apply(socket, socket.emit, applyEmitParams(SocketActionTypes.REGISTER_OWNER_CERTIFICATE, payload))
+    yield* apply(socket, socket.emit, applyEmitParams(SocketActionTypes.CREATE_COMMUNITY, payload))
   } else {
     if (!isUsernameTaken) {
       yield* put(communitiesActions.launchCommunity(action.payload.communityId))

packages/state-manager/src/sagas/identity/savedOwnerCertificate/savedOwnerCertificate.saga.ts

@@ -10,26 +10,5 @@ export function* savedOwnerCertificateSaga(
   socket: Socket,
   action: PayloadAction<ReturnType<typeof identityActions.savedOwnerCertificate>['payload']>
 ): Generator {
-  let communityId: string = action.payload
-
-  if (!communityId) {
-    communityId = yield* select(communitiesSelectors.currentCommunityId)
-  }
-
-  const community = yield* select(communitiesSelectors.selectById(communityId))
-  const identity = yield* select(identitySelectors.selectById(communityId))
-  if (!identity?.userCertificate || !identity?.userCsr || !community?.rootCa) return
-
-  const payload: InitCommunityPayload = {
-    id: communityId,
-    peerId: identity.peerId,
-    hiddenService: identity.hiddenService,
-    certs: {
-      certificate: identity.userCertificate,
-      key: identity.userCsr.userKey,
-      CA: [community.rootCa],
-    },
-  }
-
-  yield* apply(socket, socket.emit, applyEmitParams(SocketActionTypes.CREATE_COMMUNITY, payload))
+  // TODO: Remove?
 }

packages/types/src/community.ts

@@ -1,4 +1,4 @@
-import { type HiddenService, type PeerId, type Identity } from './identity'
+import { type HiddenService, type PeerId, type PermsData, type Identity, type UserCsr } from './identity'
 import { InvitationPair } from './network'
 
 export interface Community {
@@ -59,6 +59,8 @@ export interface InitCommunityPayload {
   hiddenService: HiddenService
   certs?: Certificates
   peers?: string[]
+  ownerCsr?: UserCsr
+  permsData?: PermsData
 }
 
 export interface LaunchRegistrarPayload {