feat: remove user from all shared vaults upon account deletion (#843)

jest.config.js

@@ -3,7 +3,7 @@ module.exports = {
   testEnvironment: 'node',
   testRegex: '(/__tests__/.*|(\\.|/)(test|spec))\\.ts$',
   testTimeout: 20000,
-  coverageReporters: ['text-summary'],
+  coverageReporters: ['text'],
   reporters: ['summary'],
   coverageThreshold: {
     global: {

packages/auth/src/Bootstrap/Container.ts

@@ -1008,7 +1008,16 @@ export class ContainerConfigLoader {
     container.bind<UserRegisteredEventHandler>(TYPES.Auth_UserRegisteredEventHandler).to(UserRegisteredEventHandler)
     container
       .bind<AccountDeletionRequestedEventHandler>(TYPES.Auth_AccountDeletionRequestedEventHandler)
-      .to(AccountDeletionRequestedEventHandler)
+      .toConstantValue(
+        new AccountDeletionRequestedEventHandler(
+          container.get<UserRepositoryInterface>(TYPES.Auth_UserRepository),
+          container.get<SessionRepositoryInterface>(TYPES.Auth_SessionRepository),
+          container.get<EphemeralSessionRepositoryInterface>(TYPES.Auth_EphemeralSessionRepository),
+          container.get<RevokedSessionRepositoryInterface>(TYPES.Auth_RevokedSessionRepository),
+          container.get<RemoveSharedVaultUser>(TYPES.Auth_RemoveSharedVaultUser),
+          container.get<winston.Logger>(TYPES.Auth_Logger),
+        ),
+      )
     container
       .bind<SubscriptionPurchasedEventHandler>(TYPES.Auth_SubscriptionPurchasedEventHandler)
       .to(SubscriptionPurchasedEventHandler)

packages/auth/src/Domain/Handler/AccountDeletionRequestedEventHandler.spec.ts

@@ -1,114 +0,0 @@
-import 'reflect-metadata'
-
-import { AccountDeletionRequestedEvent } from '@standardnotes/domain-events'
-import { Logger } from 'winston'
-import { EphemeralSession } from '../Session/EphemeralSession'
-import { EphemeralSessionRepositoryInterface } from '../Session/EphemeralSessionRepositoryInterface'
-import { RevokedSession } from '../Session/RevokedSession'
-import { RevokedSessionRepositoryInterface } from '../Session/RevokedSessionRepositoryInterface'
-import { Session } from '../Session/Session'
-import { SessionRepositoryInterface } from '../Session/SessionRepositoryInterface'
-import { User } from '../User/User'
-import { UserRepositoryInterface } from '../User/UserRepositoryInterface'
-import { AccountDeletionRequestedEventHandler } from './AccountDeletionRequestedEventHandler'
-
-describe('AccountDeletionRequestedEventHandler', () => {
-  let userRepository: UserRepositoryInterface
-  let sessionRepository: SessionRepositoryInterface
-  let ephemeralSessionRepository: EphemeralSessionRepositoryInterface
-  let revokedSessionRepository: RevokedSessionRepositoryInterface
-  let logger: Logger
-  let session: Session
-  let ephemeralSession: EphemeralSession
-  let revokedSession: RevokedSession
-  let user: User
-  let event: AccountDeletionRequestedEvent
-
-  const createHandler = () =>
-    new AccountDeletionRequestedEventHandler(
-      userRepository,
-      sessionRepository,
-      ephemeralSessionRepository,
-      revokedSessionRepository,
-      logger,
-    )
-
-  beforeEach(() => {
-    user = {} as jest.Mocked<User>
-
-    userRepository = {} as jest.Mocked<UserRepositoryInterface>
-    userRepository.findOneByUuid = jest.fn().mockReturnValue(user)
-    userRepository.remove = jest.fn()
-
-    session = {
-      uuid: '1-2-3',
-    } as jest.Mocked<Session>
-
-    sessionRepository = {} as jest.Mocked<SessionRepositoryInterface>
-    sessionRepository.findAllByUserUuid = jest.fn().mockReturnValue([session])
-    sessionRepository.remove = jest.fn()
-
-    ephemeralSession = {
-      uuid: '2-3-4',
-      userUuid: '00000000-0000-0000-0000-000000000000',
-    } as jest.Mocked<EphemeralSession>
-
-    ephemeralSessionRepository = {} as jest.Mocked<EphemeralSessionRepositoryInterface>
-    ephemeralSessionRepository.findAllByUserUuid = jest.fn().mockReturnValue([ephemeralSession])
-    ephemeralSessionRepository.deleteOne = jest.fn()
-
-    revokedSession = {
-      uuid: '3-4-5',
-    } as jest.Mocked<RevokedSession>
-
-    revokedSessionRepository = {} as jest.Mocked<RevokedSessionRepositoryInterface>
-    revokedSessionRepository.findAllByUserUuid = jest.fn().mockReturnValue([revokedSession])
-    revokedSessionRepository.remove = jest.fn()
-
-    event = {} as jest.Mocked<AccountDeletionRequestedEvent>
-    event.createdAt = new Date(1)
-    event.payload = {
-      userUuid: '00000000-0000-0000-0000-000000000000',
-      userCreatedAtTimestamp: 1,
-      regularSubscriptionUuid: '2-3-4',
-      roleNames: ['CORE_USER'],
-    }
-
-    logger = {} as jest.Mocked<Logger>
-    logger.info = jest.fn()
-    logger.warn = jest.fn()
-  })
-
-  it('should remove a user', async () => {
-    await createHandler().handle(event)
-
-    expect(userRepository.remove).toHaveBeenCalledWith(user)
-  })
-
-  it('should not remove a user with invalid uuid', async () => {
-    event.payload.userUuid = 'invalid'
-
-    await createHandler().handle(event)
-
-    expect(userRepository.remove).not.toHaveBeenCalled()
-  })
-
-  it('should not remove a user if one does not exist', async () => {
-    userRepository.findOneByUuid = jest.fn().mockReturnValue(null)
-
-    await createHandler().handle(event)
-
-    expect(userRepository.remove).not.toHaveBeenCalled()
-    expect(sessionRepository.remove).not.toHaveBeenCalled()
-    expect(revokedSessionRepository.remove).not.toHaveBeenCalled()
-    expect(ephemeralSessionRepository.deleteOne).not.toHaveBeenCalled()
-  })
-
-  it('should remove all user sessions', async () => {
-    await createHandler().handle(event)
-
-    expect(sessionRepository.remove).toHaveBeenCalledWith(session)
-    expect(revokedSessionRepository.remove).toHaveBeenCalledWith(revokedSession)
-    expect(ephemeralSessionRepository.deleteOne).toHaveBeenCalledWith('2-3-4', '00000000-0000-0000-0000-000000000000')
-  })
-})

packages/auth/src/Domain/Handler/AccountDeletionRequestedEventHandler.ts

@@ -1,22 +1,21 @@
 import { AccountDeletionRequestedEvent, DomainEventHandlerInterface } from '@standardnotes/domain-events'
-import { inject, injectable } from 'inversify'
+import { Uuid } from '@standardnotes/domain-core'
 import { Logger } from 'winston'
-import TYPES from '../../Bootstrap/Types'
+
 import { EphemeralSessionRepositoryInterface } from '../Session/EphemeralSessionRepositoryInterface'
 import { RevokedSessionRepositoryInterface } from '../Session/RevokedSessionRepositoryInterface'
 import { SessionRepositoryInterface } from '../Session/SessionRepositoryInterface'
 import { UserRepositoryInterface } from '../User/UserRepositoryInterface'
-import { Uuid } from '@standardnotes/domain-core'
+import { RemoveSharedVaultUser } from '../UseCase/RemoveSharedVaultUser/RemoveSharedVaultUser'
 
-@injectable()
 export class AccountDeletionRequestedEventHandler implements DomainEventHandlerInterface {
   constructor(
-    @inject(TYPES.Auth_UserRepository) private userRepository: UserRepositoryInterface,
-    @inject(TYPES.Auth_SessionRepository) private sessionRepository: SessionRepositoryInterface,
-    @inject(TYPES.Auth_EphemeralSessionRepository)
+    private userRepository: UserRepositoryInterface,
+    private sessionRepository: SessionRepositoryInterface,
     private ephemeralSessionRepository: EphemeralSessionRepositoryInterface,
-    @inject(TYPES.Auth_RevokedSessionRepository) private revokedSessionRepository: RevokedSessionRepositoryInterface,
-    @inject(TYPES.Auth_Logger) private logger: Logger,
+    private revokedSessionRepository: RevokedSessionRepositoryInterface,
+    private removeSharedVaultUser: RemoveSharedVaultUser,
+    private logger: Logger,
   ) {}
 
   async handle(event: AccountDeletionRequestedEvent): Promise<void> {
@@ -38,6 +37,13 @@ export class AccountDeletionRequestedEventHandler implements DomainEventHandlerI
 
     await this.removeSessions(userUuid.value)
 
+    const result = await this.removeSharedVaultUser.execute({
+      userUuid: userUuid.value,
+    })
+    if (result.isFailed()) {
+      this.logger.error(`Could not remove shared vault user: ${result.getError()}`)
+    }
+
     await this.userRepository.remove(user)
 
     this.logger.info(`Finished account cleanup for user: ${userUuid.value}`)

packages/auth/src/Domain/Handler/UserRemovedFromSharedVaultEventHandler.ts

@@ -10,6 +10,12 @@ export class UserRemovedFromSharedVaultEventHandler implements DomainEventHandle
   ) {}
 
   async handle(event: UserRemovedFromSharedVaultEvent): Promise<void> {
+    if (!event.payload.sharedVaultUuid) {
+      this.logger.error(`Shared vault uuid is missing from event: ${JSON.stringify(event)}`)
+
+      return
+    }
+
     const result = await this.removeSharedVaultUser.execute({
       userUuid: event.payload.userUuid,
       sharedVaultUuid: event.payload.sharedVaultUuid,

packages/auth/src/Domain/UseCase/RemoveSharedVaultUser/RemoveSharedVaultUser.spec.ts

@@ -13,6 +13,7 @@ describe('RemoveSharedVaultUser', () => {
     sharedVaultUserRepository.findByUserUuidAndSharedVaultUuid = jest
       .fn()
       .mockReturnValue({} as jest.Mocked<SharedVaultUser>)
+    sharedVaultUserRepository.findByUserUuid = jest.fn().mockReturnValue([{} as jest.Mocked<SharedVaultUser>])
     sharedVaultUserRepository.remove = jest.fn()
   })
 
@@ -28,6 +29,17 @@ describe('RemoveSharedVaultUser', () => {
     expect(sharedVaultUserRepository.remove).toHaveBeenCalled()
   })
 
+  it('should remove all shared vault users', async () => {
+    const useCase = createUseCase()
+
+    const result = await useCase.execute({
+      userUuid: '00000000-0000-0000-0000-000000000000',
+    })
+
+    expect(result.isFailed()).toBeFalsy()
+    expect(sharedVaultUserRepository.remove).toHaveBeenCalled()
+  })
+
   it('should fail when user uuid is invalid', async () => {
     const useCase = createUseCase()
 

packages/auth/src/Domain/UseCase/RemoveSharedVaultUser/RemoveSharedVaultUser.ts

@@ -13,21 +13,31 @@ export class RemoveSharedVaultUser implements UseCaseInterface<void> {
     }
     const userUuid = userUuidOrError.getValue()
 
-    const sharedVaultUuidOrError = Uuid.create(dto.sharedVaultUuid)
-    if (sharedVaultUuidOrError.isFailed()) {
-      return Result.fail(sharedVaultUuidOrError.getError())
-    }
-    const sharedVaultUuid = sharedVaultUuidOrError.getValue()
-
-    const sharedVaultUser = await this.sharedVaultUserRepository.findByUserUuidAndSharedVaultUuid({
-      userUuid,
-      sharedVaultUuid,
-    })
-    if (!sharedVaultUser) {
-      return Result.fail('Shared vault user not found')
+    let sharedVaultUuid: Uuid | undefined
+    if (dto.sharedVaultUuid !== undefined) {
+      const sharedVaultUuidOrError = Uuid.create(dto.sharedVaultUuid)
+      if (sharedVaultUuidOrError.isFailed()) {
+        return Result.fail(sharedVaultUuidOrError.getError())
+      }
+      sharedVaultUuid = sharedVaultUuidOrError.getValue()
     }
 
-    await this.sharedVaultUserRepository.remove(sharedVaultUser)
+    if (sharedVaultUuid) {
+      const sharedVaultUser = await this.sharedVaultUserRepository.findByUserUuidAndSharedVaultUuid({
+        userUuid,
+        sharedVaultUuid,
+      })
+      if (!sharedVaultUser) {
+        return Result.fail('Shared vault user not found')
+      }
+
+      await this.sharedVaultUserRepository.remove(sharedVaultUser)
+    } else {
+      const sharedVaultUsers = await this.sharedVaultUserRepository.findByUserUuid(userUuid)
+      for (const sharedVaultUser of sharedVaultUsers) {
+        await this.sharedVaultUserRepository.remove(sharedVaultUser)
+      }
+    }
 
     return Result.ok()
   }

packages/auth/src/Domain/UseCase/RemoveSharedVaultUser/RemoveSharedVaultUserDTO.ts

@@ -1,4 +1,4 @@
 export interface RemoveSharedVaultUserDTO {
-  sharedVaultUuid: string
+  sharedVaultUuid?: string
   userUuid: string
 }

packages/syncing-server/src/Bootstrap/Container.ts

@@ -169,6 +169,7 @@ import { DeleteSharedVaults } from '../Domain/UseCase/SharedVaults/DeleteSharedV
 import { RemoveItemsFromSharedVault } from '../Domain/UseCase/SharedVaults/RemoveItemsFromSharedVault/RemoveItemsFromSharedVault'
 import { SharedVaultRemovedEventHandler } from '../Domain/Handler/SharedVaultRemovedEventHandler'
 import { DesignateSurvivor } from '../Domain/UseCase/SharedVaults/DesignateSurvivor/DesignateSurvivor'
+import { RemoveUserFromSharedVaults } from '../Domain/UseCase/SharedVaults/RemoveUserFromSharedVaults/RemoveUserFromSharedVaults'
 
 export class ContainerConfigLoader {
   private readonly DEFAULT_CONTENT_SIZE_TRANSFER_LIMIT = 10_000_000
@@ -876,6 +877,15 @@ export class ContainerConfigLoader {
           container.get<DomainEventPublisherInterface>(TYPES.Sync_DomainEventPublisher),
         ),
       )
+    container
+      .bind<RemoveUserFromSharedVaults>(TYPES.Sync_RemoveUserFromSharedVaults)
+      .toConstantValue(
+        new RemoveUserFromSharedVaults(
+          container.get<SharedVaultUserRepositoryInterface>(TYPES.Sync_SharedVaultUserRepository),
+          container.get<RemoveUserFromSharedVault>(TYPES.Sync_RemoveSharedVaultUser),
+          container.get<Logger>(TYPES.Sync_Logger),
+        ),
+      )
 
     // Services
     container
@@ -938,6 +948,7 @@ export class ContainerConfigLoader {
         new AccountDeletionRequestedEventHandler(
           container.get<ItemRepositoryResolverInterface>(TYPES.Sync_ItemRepositoryResolver),
           container.get<DeleteSharedVaults>(TYPES.Sync_DeleteSharedVaults),
+          container.get<RemoveUserFromSharedVaults>(TYPES.Sync_RemoveUserFromSharedVaults),
           container.get<Logger>(TYPES.Sync_Logger),
         ),
       )

packages/syncing-server/src/Bootstrap/Types.ts

@@ -88,6 +88,7 @@ const TYPES = {
   Sync_SendEventToClient: Symbol.for('Sync_SendEventToClient'),
   Sync_RemoveItemsFromSharedVault: Symbol.for('Sync_RemoveItemsFromSharedVault'),
   Sync_DesignateSurvivor: Symbol.for('Sync_DesignateSurvivor'),
+  Sync_RemoveUserFromSharedVaults: Symbol.for('Sync_RemoveUserFromSharedVaults'),
   // Handlers
   Sync_AccountDeletionRequestedEventHandler: Symbol.for('Sync_AccountDeletionRequestedEventHandler'),
   Sync_DuplicateItemSyncedEventHandler: Symbol.for('Sync_DuplicateItemSyncedEventHandler'),

packages/syncing-server/src/Domain/Handler/AccountDeletionRequestedEventHandler.ts

@@ -4,11 +4,13 @@ import { Logger } from 'winston'
 
 import { ItemRepositoryResolverInterface } from '../Item/ItemRepositoryResolverInterface'
 import { DeleteSharedVaults } from '../UseCase/SharedVaults/DeleteSharedVaults/DeleteSharedVaults'
+import { RemoveUserFromSharedVaults } from '../UseCase/SharedVaults/RemoveUserFromSharedVaults/RemoveUserFromSharedVaults'
 
 export class AccountDeletionRequestedEventHandler implements DomainEventHandlerInterface {
   constructor(
     private itemRepositoryResolver: ItemRepositoryResolverInterface,
     private deleteSharedVaults: DeleteSharedVaults,
+    private removeUserFromSharedVaults: RemoveUserFromSharedVaults,
     private logger: Logger,
   ) {}
 
@@ -23,13 +25,24 @@ export class AccountDeletionRequestedEventHandler implements DomainEventHandlerI
 
     await itemRepository.deleteByUserUuid(event.payload.userUuid)
 
-    const result = await this.deleteSharedVaults.execute({
+    const deletingVaultsResult = await this.deleteSharedVaults.execute({
       ownerUuid: event.payload.userUuid,
     })
-    if (result.isFailed()) {
-      this.logger.error(`Failed to delete shared vaults for user: ${event.payload.userUuid}: ${result.getError()}`)
+    if (deletingVaultsResult.isFailed()) {
+      this.logger.error(
+        `Failed to delete shared vaults for user: ${event.payload.userUuid}: ${deletingVaultsResult.getError()}`,
+      )
+    }
 
-      return
+    const deletingUserFromOtherVaultsResult = await this.removeUserFromSharedVaults.execute({
+      userUuid: event.payload.userUuid,
+    })
+    if (deletingUserFromOtherVaultsResult.isFailed()) {
+      this.logger.error(
+        `Failed to remove user: ${
+          event.payload.userUuid
+        } from shared vaults: ${deletingUserFromOtherVaultsResult.getError()}`,
+      )
     }
 
     this.logger.info(`Finished account cleanup for user: ${event.payload.userUuid}`)

packages/syncing-server/src/Domain/UseCase/SharedVaults/RemoveUserFromSharedVaults/RemoveUserFromSharedVaults.spec.ts

@@ -0,0 +1,79 @@
+import { Result, SharedVaultUser, SharedVaultUserPermission, Timestamps, Uuid } from '@standardnotes/domain-core'
+import { Logger } from 'winston'
+
+import { SharedVaultUserRepositoryInterface } from '../../../SharedVault/User/SharedVaultUserRepositoryInterface'
+import { RemoveUserFromSharedVault } from '../RemoveUserFromSharedVault/RemoveUserFromSharedVault'
+import { RemoveUserFromSharedVaults } from './RemoveUserFromSharedVaults'
+
+describe('RemoveUserFromSharedVaults', () => {
+  let sharedVaultUserRepository: SharedVaultUserRepositoryInterface
+  let sharedVaultUser: SharedVaultUser
+  let removeUserFromSharedVault: RemoveUserFromSharedVault
+  let logger: Logger
+
+  const createUseCase = () =>
+    new RemoveUserFromSharedVaults(sharedVaultUserRepository, removeUserFromSharedVault, logger)
+
+  beforeEach(() => {
+    sharedVaultUser = SharedVaultUser.create({
+      permission: SharedVaultUserPermission.create(SharedVaultUserPermission.PERMISSIONS.Write).getValue(),
+      sharedVaultUuid: Uuid.create('00000000-0000-0000-0000-000000000000').getValue(),
+      userUuid: Uuid.create('00000000-0000-0000-0000-000000000000').getValue(),
+      timestamps: Timestamps.create(123, 123).getValue(),
+      isDesignatedSurvivor: false,
+    }).getValue()
+
+    sharedVaultUserRepository = {} as jest.Mocked<SharedVaultUserRepositoryInterface>
+    sharedVaultUserRepository.findByUserUuid = jest.fn().mockResolvedValue([sharedVaultUser])
+
+    removeUserFromSharedVault = {} as jest.Mocked<RemoveUserFromSharedVault>
+    removeUserFromSharedVault.execute = jest.fn().mockResolvedValue(Result.ok())
+
+    logger = {} as jest.Mocked<Logger>
+    logger.error = jest.fn()
+  })
+
+  it('should remove user from shared vaults', async () => {
+    const useCase = createUseCase()
+
+    const result = await useCase.execute({
+      userUuid: '00000000-0000-0000-0000-000000000000',
+    })
+
+    expect(result.isFailed()).toBeFalsy()
+    expect(removeUserFromSharedVault.execute).toHaveBeenCalledTimes(1)
+    expect(removeUserFromSharedVault.execute).toHaveBeenCalledWith({
+      sharedVaultUuid: '00000000-0000-0000-0000-000000000000',
+      originatorUuid: '00000000-0000-0000-0000-000000000000',
+      userUuid: '00000000-0000-0000-0000-000000000000',
+      forceRemoveOwner: true,
+    })
+  })
+
+  it('should log error if removing user from shared vault fails', async () => {
+    removeUserFromSharedVault.execute = jest.fn().mockResolvedValue(Result.fail('error'))
+
+    const useCase = createUseCase()
+
+    const result = await useCase.execute({
+      userUuid: '00000000-0000-0000-0000-000000000000',
+    })
+
+    expect(result.isFailed()).toBeFalsy()
+    expect(logger.error).toHaveBeenCalledTimes(1)
+    expect(logger.error).toHaveBeenCalledWith(
+      'Failed to remove user: 00000000-0000-0000-0000-000000000000 from shared vault: 00000000-0000-0000-0000-000000000000: error',
+    )
+  })
+
+  it('should fail if the user uuid is invalid', async () => {
+    const useCase = createUseCase()
+
+    const result = await useCase.execute({
+      userUuid: 'invalid',
+    })
+
+    expect(result.isFailed()).toBeTruthy()
+    expect(removeUserFromSharedVault.execute).not.toHaveBeenCalled()
+  })
+})

packages/syncing-server/src/Domain/UseCase/SharedVaults/RemoveUserFromSharedVaults/RemoveUserFromSharedVaults.ts

@@ -0,0 +1,41 @@
+import { Result, UseCaseInterface, Uuid } from '@standardnotes/domain-core'
+import { SharedVaultUserRepositoryInterface } from '../../../SharedVault/User/SharedVaultUserRepositoryInterface'
+import { RemoveUserFromSharedVault } from '../RemoveUserFromSharedVault/RemoveUserFromSharedVault'
+import { Logger } from 'winston'
+import { RemoveUserFromSharedVaultsDTO } from './RemoveUserFromSharedVaultsDTO'
+
+export class RemoveUserFromSharedVaults implements UseCaseInterface<void> {
+  constructor(
+    private sharedVaultUserRepository: SharedVaultUserRepositoryInterface,
+    private removeUserFromSharedVault: RemoveUserFromSharedVault,
+    private logger: Logger,
+  ) {}
+
+  async execute(dto: RemoveUserFromSharedVaultsDTO): Promise<Result<void>> {
+    const userUuidOrError = Uuid.create(dto.userUuid)
+    if (userUuidOrError.isFailed()) {
+      return Result.fail(userUuidOrError.getError())
+    }
+    const userUuid = userUuidOrError.getValue()
+
+    const sharedVaultUsers = await this.sharedVaultUserRepository.findByUserUuid(userUuid)
+    for (const sharedVaultUser of sharedVaultUsers) {
+      const result = await this.removeUserFromSharedVault.execute({
+        sharedVaultUuid: sharedVaultUser.props.sharedVaultUuid.value,
+        originatorUuid: userUuid.value,
+        userUuid: userUuid.value,
+        forceRemoveOwner: true,
+      })
+
+      if (result.isFailed()) {
+        this.logger.error(
+          `Failed to remove user: ${userUuid.value} from shared vault: ${
+            sharedVaultUser.props.sharedVaultUuid.value
+          }: ${result.getError()}`,
+        )
+      }
+    }
+
+    return Result.ok()
+  }
+}

packages/syncing-server/src/Domain/UseCase/SharedVaults/RemoveUserFromSharedVaults/RemoveUserFromSharedVaultsDTO.ts

@@ -0,0 +1,3 @@
+export interface RemoveUserFromSharedVaultsDTO {
+  userUuid: string
+}