From a5c4ea231496eaac8b8ba5e591814a1567d7c64e Mon Sep 17 00:00:00 2001 From: jzunigax2 <125698953+jzunigax2@users.noreply.github.com> Date: Fri, 5 Jun 2026 15:48:42 -0600 Subject: [PATCH 1/2] feat: add network bucket creation to mail account provisioning - Introduced a new column `network_bucket_id` in the `mail_accounts` table to associate accounts with network buckets. - Updated `AccountService` to create and delete mail buckets via the BridgeClient. - Enhanced unit tests to cover new functionality related to network bucket management. - Modified relevant models and repositories to handle the new `networkBucketId` attribute. --- ...-add-network-bucket-id-to-mail-accounts.js | 18 +++ src/modules/account/account.module.ts | 2 + src/modules/account/account.service.spec.ts | 74 +++++++++++++ src/modules/account/account.service.ts | 28 +++++ .../account/domain/mail-account.domain.ts | 2 + .../account/models/mail-account.model.ts | 4 + .../repositories/account.repository.spec.ts | 12 ++ .../repositories/account.repository.ts | 5 + .../bridge/bridge.service.spec.ts | 103 ++++++++++++++++++ .../infrastructure/bridge/bridge.service.ts | 52 ++++++++- .../infrastructure/bridge/bridge.types.ts | 5 + test/fixtures.ts | 1 + 12 files changed, 305 insertions(+), 1 deletion(-) create mode 100644 migrations/20260605214402-add-network-bucket-id-to-mail-accounts.js diff --git a/migrations/20260605214402-add-network-bucket-id-to-mail-accounts.js b/migrations/20260605214402-add-network-bucket-id-to-mail-accounts.js new file mode 100644 index 0000000..26056fe --- /dev/null +++ b/migrations/20260605214402-add-network-bucket-id-to-mail-accounts.js @@ -0,0 +1,18 @@ +'use strict'; + +const TABLE_NAME = 'mail_accounts'; + +/** @type {import('sequelize-cli').Migration} */ +module.exports = { + async up(queryInterface, Sequelize) { + await queryInterface.addColumn(TABLE_NAME, 'network_bucket_id', { + type: Sequelize.STRING(24), + allowNull: true, + defaultValue: null, + }); + }, + + async down(queryInterface) { + await queryInterface.removeColumn(TABLE_NAME, 'network_bucket_id'); + }, +}; diff --git a/src/modules/account/account.module.ts b/src/modules/account/account.module.ts index a086d74..77152db 100644 --- a/src/modules/account/account.module.ts +++ b/src/modules/account/account.module.ts @@ -3,6 +3,7 @@ import { SequelizeModule } from '@nestjs/sequelize'; import { Reflector } from '@nestjs/core'; import { StalwartModule } from '../infrastructure/stalwart/stalwart.module.js'; import { PaymentsModule } from '../infrastructure/payments/payments.module.js'; +import { BridgeModule } from '../infrastructure/bridge/bridge.module.js'; import { AccountService } from './account.service.js'; import { UserController } from './user.controller.js'; import { MailAccountGuard } from '../provisioning/provisioning.guard.js'; @@ -29,6 +30,7 @@ import { MailAddressKeysRepository } from './repositories/mail-address-keys.repo ]), StalwartModule, PaymentsModule, + BridgeModule, ], controllers: [UserController], providers: [ diff --git a/src/modules/account/account.service.spec.ts b/src/modules/account/account.service.spec.ts index 0161e1f..b7df12e 100644 --- a/src/modules/account/account.service.spec.ts +++ b/src/modules/account/account.service.spec.ts @@ -16,6 +16,7 @@ import { AccountRepository } from './repositories/account.repository.js'; import { AddressRepository } from './repositories/address.repository.js'; import { DomainRepository } from './repositories/domain.repository.js'; import { MailAddressKeysRepository } from './repositories/mail-address-keys.repository.js'; +import { BridgeClient } from '../infrastructure/bridge/bridge.service.js'; import { newMailAccountAttributes, newMailAddressKeyBundle, @@ -33,6 +34,7 @@ describe('AccountService', () => { let addresses: DeepMocked; let domains: DeepMocked; let keys: DeepMocked; + let bridge: DeepMocked; let config: DeepMocked; beforeEach(async () => { @@ -48,6 +50,7 @@ describe('AccountService', () => { addresses = module.get(AddressRepository); domains = module.get(DomainRepository); keys = module.get(MailAddressKeysRepository); + bridge = module.get(BridgeClient); config = module.get(ConfigService); }); @@ -364,6 +367,7 @@ describe('AccountService', () => { }), ); + const bucket = { id: 'bucket-1', name: createdAccount.id }; domains.findByDomain.mockResolvedValue(domain); addresses.findByAddress.mockResolvedValue(null); accounts.findByUserId @@ -371,6 +375,7 @@ describe('AccountService', () => { .mockResolvedValueOnce(provisionedAccount); accounts.create.mockResolvedValue(createdAccount); addresses.create.mockResolvedValue(createdAddressId); + bridge.createMailBucket.mockResolvedValue(bucket); const result = await service.provisionAccount(params); @@ -378,6 +383,14 @@ describe('AccountService', () => { expect(accounts.create).toHaveBeenCalledWith({ userId: params.userId, }); + expect(bridge.createMailBucket).toHaveBeenCalledWith( + params.userId, + createdAccount.id, + ); + expect(accounts.setNetworkBucketId).toHaveBeenCalledWith( + createdAccount.id, + bucket.id, + ); expect(addresses.create).toHaveBeenCalledWith({ mailAccountId: createdAccount.id, address: params.address, @@ -463,6 +476,29 @@ describe('AccountService', () => { expect(accounts.delete).toHaveBeenCalledWith(createdAccount.id); }); + it('when bucket creation fails, then deletes the principal and account (undo) and rethrows', async () => { + const createdAccount = MailAccount.build( + newMailAccountAttributes({ + userId: params.userId, + addresses: [], + }), + ); + + domains.findByDomain.mockResolvedValue(domain); + addresses.findByAddress.mockResolvedValue(null); + accounts.findByUserId.mockResolvedValue(null); + accounts.create.mockResolvedValue(createdAccount); + addresses.create.mockResolvedValue('addr-id'); + bridge.createMailBucket.mockRejectedValue(new Error('Bridge down')); + + await expect(service.provisionAccount(params)).rejects.toThrow( + 'Bridge down', + ); + expect(provider.deleteAccount).toHaveBeenCalledWith(params.address); + expect(accounts.delete).toHaveBeenCalledWith(createdAccount.id); + expect(accounts.setNetworkBucketId).not.toHaveBeenCalled(); + }); + it('when concurrent provisioning race occurs, then returns the existing account', async () => { const existingAccount = MailAccount.build( newMailAccountAttributes({ userId: params.userId }), @@ -506,6 +542,44 @@ describe('AccountService', () => { expect(accounts.delete).toHaveBeenCalledWith(account.id); }); + it('when account has a network bucket, then deletes it via the bridge', async () => { + const account = MailAccount.build( + newMailAccountAttributes({ networkBucketId: 'bucket-1' }), + ); + accounts.findByUserId.mockResolvedValue(account); + + await service.deleteAccount(account.userId); + + expect(bridge.deleteMailBucket).toHaveBeenCalledWith( + account.userId, + 'bucket-1', + ); + expect(accounts.delete).toHaveBeenCalledWith(account.id); + }); + + it('when account has no network bucket, then does not call the bridge', async () => { + const account = MailAccount.build( + newMailAccountAttributes({ networkBucketId: null }), + ); + accounts.findByUserId.mockResolvedValue(account); + + await service.deleteAccount(account.userId); + + expect(bridge.deleteMailBucket).not.toHaveBeenCalled(); + }); + + it('when bridge bucket deletion fails, then logs a warning and still deletes the account', async () => { + const account = MailAccount.build( + newMailAccountAttributes({ networkBucketId: 'bucket-1' }), + ); + accounts.findByUserId.mockResolvedValue(account); + bridge.deleteMailBucket.mockRejectedValue(new Error('Bridge down')); + + await service.deleteAccount(account.userId); + + expect(accounts.delete).toHaveBeenCalledWith(account.id); + }); + it('when account does not exist, then throws NotFoundException', async () => { accounts.findByUserId.mockResolvedValue(null); diff --git a/src/modules/account/account.service.ts b/src/modules/account/account.service.ts index 7e0cee5..706393f 100644 --- a/src/modules/account/account.service.ts +++ b/src/modules/account/account.service.ts @@ -8,6 +8,7 @@ import { } from '@nestjs/common'; import { ConfigService } from '@nestjs/config'; import dayjs from 'dayjs'; +import { BridgeClient } from '../infrastructure/bridge/bridge.service.js'; import { MailNotSetupException } from '../provisioning/mail-not-setup.exception.js'; import { AccountProvider } from './account-provider.port.js'; import { MailAccount, MailAccountState } from './domain/mail-account.domain.js'; @@ -41,6 +42,7 @@ export class AccountService { private readonly addresses: AddressRepository, private readonly domains: DomainRepository, private readonly keys: MailAddressKeysRepository, + private readonly bridge: BridgeClient, private readonly config: ConfigService, ) {} @@ -214,6 +216,19 @@ export class AccountService { throw error; } + try { + const bucket = await this.bridge.createMailBucket( + params.userId, + account.id, + ); + await this.accounts.setNetworkBucketId(account.id, bucket.id); + } catch (error) { + // The principal already exists at this point, so roll it back too. + await this.provider.deleteAccount(params.address); + await this.accounts.delete(account.id); + throw error; + } + return this.getAccountOrFail(params.userId); } @@ -227,6 +242,19 @@ export class AccountService { }), ); + if (account.networkBucketId) { + try { + await this.bridge.deleteMailBucket( + driveUserUuid, + account.networkBucketId, + ); + } catch (error) { + this.logger.warn( + `Failed to delete network bucket '${account.networkBucketId}' for '${driveUserUuid}': ${(error as Error).message}`, + ); + } + } + await this.accounts.delete(account.id); this.logger.log(`Deleted account for user '${driveUserUuid}'`); } diff --git a/src/modules/account/domain/mail-account.domain.ts b/src/modules/account/domain/mail-account.domain.ts index 7bc3512..38071d1 100644 --- a/src/modules/account/domain/mail-account.domain.ts +++ b/src/modules/account/domain/mail-account.domain.ts @@ -13,6 +13,7 @@ export interface MailAccountAttributes { userId: string; status: MailAccountState; suspendedAt: Date | null; + networkBucketId: string | null; addresses: MailAddressAttributes[]; createdAt: Date; updatedAt: Date; @@ -23,6 +24,7 @@ export class MailAccount { readonly userId!: string; readonly status!: MailAccountState; readonly suspendedAt!: Date | null; + readonly networkBucketId!: string | null; readonly addresses!: MailAddress[]; readonly createdAt!: Date; readonly updatedAt!: Date; diff --git a/src/modules/account/models/mail-account.model.ts b/src/modules/account/models/mail-account.model.ts index a15030f..4f08414 100644 --- a/src/modules/account/models/mail-account.model.ts +++ b/src/modules/account/models/mail-account.model.ts @@ -36,6 +36,10 @@ export class MailAccountModel extends Model { @Column(DataType.DATE) declare suspendedAt: Date | null; + @AllowNull(true) + @Column({ field: 'network_bucket_id', type: DataType.UUID }) + declare networkBucketId: string | null; + @Column(DataType.DATE) declare deletedAt: Date | null; diff --git a/src/modules/account/repositories/account.repository.spec.ts b/src/modules/account/repositories/account.repository.spec.ts index 4572034..af4830f 100644 --- a/src/modules/account/repositories/account.repository.spec.ts +++ b/src/modules/account/repositories/account.repository.spec.ts @@ -34,6 +34,7 @@ describe('AccountRepository', () => { userId: 'user-1', status: 'active', suspendedAt: null, + networkBucketId: null, createdAt: new Date('2026-01-01T00:00:00.000Z'), updatedAt: new Date('2026-01-02T00:00:00.000Z'), addresses: [], @@ -129,4 +130,15 @@ describe('AccountRepository', () => { }); }); }); + + describe('setNetworkBucketId', () => { + it('when given an id and bucket id, then updates the account row', async () => { + await repository.setNetworkBucketId('acc-1', 'bucket-1'); + + expect(accountModel.update).toHaveBeenCalledWith( + { networkBucketId: 'bucket-1' }, + { where: { id: 'acc-1' } }, + ); + }); + }); }); diff --git a/src/modules/account/repositories/account.repository.ts b/src/modules/account/repositories/account.repository.ts index 5b71339..1c759ba 100644 --- a/src/modules/account/repositories/account.repository.ts +++ b/src/modules/account/repositories/account.repository.ts @@ -46,12 +46,17 @@ export class AccountRepository { await this.accountModel.destroy({ where: { id } }); } + async setNetworkBucketId(id: string, networkBucketId: string): Promise { + await this.accountModel.update({ networkBucketId }, { where: { id } }); + } + private toDomain(model: MailAccountModel): MailAccount { return MailAccount.build({ id: model.id, userId: model.userId, status: model.status as MailAccountState, suspendedAt: model.suspendedAt, + networkBucketId: model.networkBucketId, createdAt: model.createdAt as Date, updatedAt: model.updatedAt as Date, addresses: (model.addresses ?? []).map(toAddressAttributes), diff --git a/src/modules/infrastructure/bridge/bridge.service.spec.ts b/src/modules/infrastructure/bridge/bridge.service.spec.ts index 956dcad..c9739b7 100644 --- a/src/modules/infrastructure/bridge/bridge.service.spec.ts +++ b/src/modules/infrastructure/bridge/bridge.service.spec.ts @@ -100,4 +100,107 @@ describe('BridgeClient', () => { ); }); }); + + describe('createMailBucket', () => { + it('when Bridge returns 200, then signs a gateway token, POSTs the name, and returns the bucket', async () => { + const bucket = { id: 'bucket-1', name: 'account-1' }; + jwtService.sign.mockReturnValue('signed-jwt'); + httpRequest.mockResolvedValue({ + statusCode: 200, + body: { text: () => Promise.resolve(JSON.stringify(bucket)) }, + }); + + const result = await service.createMailBucket('user-1', 'account-1'); + + expect(result).toStrictEqual(bucket); + expect(jwtService.sign).toHaveBeenCalledWith( + { payload: { uuid: 'user-1' } }, + { + secret: 'test-key', + algorithm: 'RS256', + expiresIn: '1m', + allowInsecureKeySizes: true, + }, + ); + expect(httpRequest).toHaveBeenCalledWith( + expect.objectContaining({ + method: 'POST', + path: '/v2/gateway/users/user-1/buckets', + body: JSON.stringify({ name: 'account-1' }), + headers: expect.objectContaining({ + authorization: 'Bearer signed-jwt', + }) as unknown, + }), + ); + }); + + it('when Bridge returns a non-200 status, then throws BridgeApiError with statusCode and details', async () => { + jwtService.sign.mockReturnValue('signed-jwt'); + httpRequest.mockResolvedValue({ + statusCode: 500, + body: { text: () => Promise.resolve('internal error') }, + }); + + const error: unknown = await service + .createMailBucket('user-1', 'account-1') + .catch((e: unknown) => e); + + expect(error).toBeInstanceOf(BridgeApiError); + if (!(error instanceof BridgeApiError)) { + throw new Error('expected BridgeApiError'); + } + expect(error.statusCode).toBe(500); + expect(error.details).toBe('internal error'); + }); + }); + + describe('deleteMailBucket', () => { + it('when Bridge returns 204, then signs a gateway token and DELETEs the bucket', async () => { + jwtService.sign.mockReturnValue('signed-jwt'); + httpRequest.mockResolvedValue({ + statusCode: 204, + body: { text: () => Promise.resolve('') }, + }); + + await service.deleteMailBucket('user-1', 'bucket-1'); + + expect(jwtService.sign).toHaveBeenCalledWith( + { payload: { uuid: 'user-1' } }, + { + secret: 'test-key', + algorithm: 'RS256', + expiresIn: '1m', + allowInsecureKeySizes: true, + }, + ); + expect(httpRequest).toHaveBeenCalledWith( + expect.objectContaining({ + method: 'DELETE', + path: '/v2/gateway/users/user-1/buckets/bucket-1', + headers: expect.objectContaining({ + authorization: 'Bearer signed-jwt', + }) as unknown, + }), + ); + }); + + it('when Bridge returns a non-204 status, then throws BridgeApiError with statusCode and details', async () => { + jwtService.sign.mockReturnValue('signed-jwt'); + httpRequest.mockResolvedValue({ + statusCode: 404, + body: { text: () => Promise.resolve('not found') }, + }); + + const error: unknown = await service + .deleteMailBucket('user-1', 'bucket-1') + .catch((e: unknown) => e); + + expect(error).toBeInstanceOf(BridgeApiError); + if (!(error instanceof BridgeApiError)) { + throw new Error('expected BridgeApiError'); + } + expect(error.statusCode).toBe(404); + expect(error.details).toBe('not found'); + }); + }); }); diff --git a/src/modules/infrastructure/bridge/bridge.service.ts b/src/modules/infrastructure/bridge/bridge.service.ts index aad8d4f..442391d 100644 --- a/src/modules/infrastructure/bridge/bridge.service.ts +++ b/src/modules/infrastructure/bridge/bridge.service.ts @@ -7,7 +7,7 @@ import { import { ConfigService } from '@nestjs/config'; import { JwtService } from '@nestjs/jwt'; import { Client } from 'undici'; -import type { UserStorage } from './bridge.types.js'; +import type { MailBucket, UserStorage } from './bridge.types.js'; @Injectable() export class BridgeClient implements OnModuleInit, OnModuleDestroy { @@ -78,6 +78,56 @@ export class BridgeClient implements OnModuleInit, OnModuleDestroy { return JSON.parse(text) as UserStorage; } + async createMailBucket(userUuid: string, name: string): Promise { + const token = this.signGatewayToken(userUuid); + + const { statusCode, body } = await this.httpClient.request({ + method: 'POST', + path: `${this.basePath}/v2/gateway/users/${encodeURIComponent(userUuid)}/buckets`, + headers: { + 'content-type': 'application/json', + accept: 'application/json', + authorization: `Bearer ${token}`, + }, + body: JSON.stringify({ name }), + }); + + const text = await body.text(); + + if (statusCode !== 200) { + throw new BridgeApiError( + `Failed to create mail bucket for user '${userUuid}': HTTP ${statusCode}`, + statusCode, + text, + ); + } + + return JSON.parse(text) as MailBucket; + } + + async deleteMailBucket(userUuid: string, bucketId: string): Promise { + const token = this.signGatewayToken(userUuid); + + const { statusCode, body } = await this.httpClient.request({ + method: 'DELETE', + path: `${this.basePath}/v2/gateway/users/${encodeURIComponent(userUuid)}/buckets/${encodeURIComponent(bucketId)}`, + headers: { + accept: 'application/json', + authorization: `Bearer ${token}`, + }, + }); + + const text = await body.text(); + + if (statusCode !== 204) { + throw new BridgeApiError( + `Failed to delete mail bucket '${bucketId}' for user '${userUuid}': HTTP ${statusCode}`, + statusCode, + text, + ); + } + } + private signGatewayToken(userUuid: string): string { return this.jwtService.sign( { payload: { uuid: userUuid } }, diff --git a/src/modules/infrastructure/bridge/bridge.types.ts b/src/modules/infrastructure/bridge/bridge.types.ts index 5468bef..b1f8554 100644 --- a/src/modules/infrastructure/bridge/bridge.types.ts +++ b/src/modules/infrastructure/bridge/bridge.types.ts @@ -2,3 +2,8 @@ export interface UserStorage { driveUsed: number; planQuota: number; } + +export interface MailBucket { + id: string; + name: string; +} diff --git a/test/fixtures.ts b/test/fixtures.ts index 5cd950e..c416cef 100644 --- a/test/fixtures.ts +++ b/test/fixtures.ts @@ -254,6 +254,7 @@ export function newMailAccountAttributes( userId: randomUuid(), status: MailAccountState.Active, suspendedAt: null, + networkBucketId: null, addresses: [address], createdAt: new Date(), updatedAt: new Date(), From c7143b6252f0ba4af5e131ca4216c17bd97947e1 Mon Sep 17 00:00:00 2001 From: jzunigax2 <125698953+jzunigax2@users.noreply.github.com> Date: Mon, 8 Jun 2026 15:21:38 -0600 Subject: [PATCH 2/2] feat: add network bucket ID to mail addresses and update related services - Introduced a new column `network_bucket_id` in the `mail_addresses` table to associate addresses with network buckets. - Updated `AccountService` to manage network bucket creation and deletion for mail addresses. - Refactored related models and repositories to handle the new `networkBucketId` attribute. - Enhanced unit tests to cover the new functionality and ensure proper handling of network buckets. --- ...dd-network-bucket-id-to-mail-addresses.js} | 2 +- src/modules/account/account.service.spec.ts | 66 +++++++++++++++---- src/modules/account/account.service.ts | 54 ++++++++++----- .../account/domain/mail-account.domain.ts | 2 - .../account/domain/mail-address.domain.ts | 2 + .../account/models/mail-account.model.ts | 4 -- .../account/models/mail-address.model.ts | 4 ++ .../repositories/account.repository.spec.ts | 12 ---- .../repositories/account.repository.ts | 5 -- .../repositories/address.repository.spec.ts | 11 ++++ .../repositories/address.repository.ts | 5 ++ test/fixtures.ts | 2 +- 12 files changed, 115 insertions(+), 54 deletions(-) rename migrations/{20260605214402-add-network-bucket-id-to-mail-accounts.js => 20260605214402-add-network-bucket-id-to-mail-addresses.js} (91%) diff --git a/migrations/20260605214402-add-network-bucket-id-to-mail-accounts.js b/migrations/20260605214402-add-network-bucket-id-to-mail-addresses.js similarity index 91% rename from migrations/20260605214402-add-network-bucket-id-to-mail-accounts.js rename to migrations/20260605214402-add-network-bucket-id-to-mail-addresses.js index 26056fe..0c7fae6 100644 --- a/migrations/20260605214402-add-network-bucket-id-to-mail-accounts.js +++ b/migrations/20260605214402-add-network-bucket-id-to-mail-addresses.js @@ -1,6 +1,6 @@ 'use strict'; -const TABLE_NAME = 'mail_accounts'; +const TABLE_NAME = 'mail_addresses'; /** @type {import('sequelize-cli').Migration} */ module.exports = { diff --git a/src/modules/account/account.service.spec.ts b/src/modules/account/account.service.spec.ts index b7df12e..78e4ea0 100644 --- a/src/modules/account/account.service.spec.ts +++ b/src/modules/account/account.service.spec.ts @@ -367,7 +367,7 @@ describe('AccountService', () => { }), ); - const bucket = { id: 'bucket-1', name: createdAccount.id }; + const bucket = { id: 'bucket-1', name: createdAddressId }; domains.findByDomain.mockResolvedValue(domain); addresses.findByAddress.mockResolvedValue(null); accounts.findByUserId @@ -385,10 +385,10 @@ describe('AccountService', () => { }); expect(bridge.createMailBucket).toHaveBeenCalledWith( params.userId, - createdAccount.id, + createdAddressId, ); - expect(accounts.setNetworkBucketId).toHaveBeenCalledWith( - createdAccount.id, + expect(addresses.setNetworkBucketId).toHaveBeenCalledWith( + createdAddressId, bucket.id, ); expect(addresses.create).toHaveBeenCalledWith({ @@ -496,7 +496,7 @@ describe('AccountService', () => { ); expect(provider.deleteAccount).toHaveBeenCalledWith(params.address); expect(accounts.delete).toHaveBeenCalledWith(createdAccount.id); - expect(accounts.setNetworkBucketId).not.toHaveBeenCalled(); + expect(addresses.setNetworkBucketId).not.toHaveBeenCalled(); }); it('when concurrent provisioning race occurs, then returns the existing account', async () => { @@ -542,9 +542,10 @@ describe('AccountService', () => { expect(accounts.delete).toHaveBeenCalledWith(account.id); }); - it('when account has a network bucket, then deletes it via the bridge', async () => { + it('when an address has a network bucket, then deletes it via the bridge', async () => { + const addr = newMailAddressAttributes({ networkBucketId: 'bucket-1' }); const account = MailAccount.build( - newMailAccountAttributes({ networkBucketId: 'bucket-1' }), + newMailAccountAttributes({ addresses: [addr] }), ); accounts.findByUserId.mockResolvedValue(account); @@ -557,9 +558,10 @@ describe('AccountService', () => { expect(accounts.delete).toHaveBeenCalledWith(account.id); }); - it('when account has no network bucket, then does not call the bridge', async () => { + it('when addresses have no network bucket, then does not call the bridge', async () => { + const addr = newMailAddressAttributes({ networkBucketId: null }); const account = MailAccount.build( - newMailAccountAttributes({ networkBucketId: null }), + newMailAccountAttributes({ addresses: [addr] }), ); accounts.findByUserId.mockResolvedValue(account); @@ -569,8 +571,9 @@ describe('AccountService', () => { }); it('when bridge bucket deletion fails, then logs a warning and still deletes the account', async () => { + const addr = newMailAddressAttributes({ networkBucketId: 'bucket-1' }); const account = MailAccount.build( - newMailAccountAttributes({ networkBucketId: 'bucket-1' }), + newMailAccountAttributes({ addresses: [addr] }), ); accounts.findByUserId.mockResolvedValue(account); bridge.deleteMailBucket.mockRejectedValue(new Error('Bridge down')); @@ -602,6 +605,10 @@ describe('AccountService', () => { domains.findByDomain.mockResolvedValue(domain); addresses.findByAddress.mockResolvedValue(null); addresses.create.mockResolvedValue(newAddressId); + bridge.createMailBucket.mockResolvedValue({ + id: 'bucket-1', + name: newAddressId, + }); await service.addAddress( accountAttrs.userId, @@ -628,6 +635,36 @@ describe('AccountService', () => { provider: 'stalwart', externalId: newAddr, }); + expect(bridge.createMailBucket).toHaveBeenCalledWith( + accountAttrs.userId, + newAddressId, + ); + expect(addresses.setNetworkBucketId).toHaveBeenCalledWith( + newAddressId, + 'bucket-1', + ); + }); + + it('when bucket creation fails, then rolls back principal, link, and address', async () => { + const account = MailAccount.build(newMailAccountAttributes()); + const domain = MailDomain.build(newMailDomainAttributes()); + const newAddr = 'new@example.com'; + const newAddressId = 'new-address-id'; + + accounts.findByUserId.mockResolvedValue(account); + domains.findByDomain.mockResolvedValue(domain); + addresses.findByAddress.mockResolvedValue(null); + addresses.create.mockResolvedValue(newAddressId); + bridge.createMailBucket.mockRejectedValue(new Error('Bridge down')); + + await expect( + service.addAddress(account.userId, newAddr, domain.domain, 'pass'), + ).rejects.toThrow('Bridge down'); + + expect(provider.deleteAccount).toHaveBeenCalledWith(newAddr); + expect(addresses.deleteProviderLink).toHaveBeenCalledWith(newAddressId); + expect(addresses.delete).toHaveBeenCalledWith(newAddressId); + expect(addresses.setNetworkBucketId).not.toHaveBeenCalled(); }); it('when account not found, then throws NotFoundException', async () => { @@ -699,7 +736,10 @@ describe('AccountService', () => { describe('removeAddress', () => { it('when address exists and is not default, then deletes principal and address', async () => { - const nonDefaultAddr = newMailAddressAttributes({ isDefault: false }); + const nonDefaultAddr = newMailAddressAttributes({ + isDefault: false, + networkBucketId: 'bucket-1', + }); const account = MailAccount.build( newMailAccountAttributes({ addresses: [ @@ -719,6 +759,10 @@ describe('AccountService', () => { nonDefaultAddr.id, ); expect(addresses.delete).toHaveBeenCalledWith(nonDefaultAddr.id); + expect(bridge.deleteMailBucket).toHaveBeenCalledWith( + account.userId, + 'bucket-1', + ); }); it('when address is default, then throws UnprocessableEntityException', async () => { diff --git a/src/modules/account/account.service.ts b/src/modules/account/account.service.ts index 706393f..9e73114 100644 --- a/src/modules/account/account.service.ts +++ b/src/modules/account/account.service.ts @@ -12,6 +12,7 @@ import { BridgeClient } from '../infrastructure/bridge/bridge.service.js'; import { MailNotSetupException } from '../provisioning/mail-not-setup.exception.js'; import { AccountProvider } from './account-provider.port.js'; import { MailAccount, MailAccountState } from './domain/mail-account.domain.js'; +import { MailAddress } from './domain/mail-address.domain.js'; import { MailDomain } from './domain/mail-domain.domain.js'; import { AccountRepository } from './repositories/account.repository.js'; import { AddressRepository } from './repositories/address.repository.js'; @@ -217,11 +218,7 @@ export class AccountService { } try { - const bucket = await this.bridge.createMailBucket( - params.userId, - account.id, - ); - await this.accounts.setNetworkBucketId(account.id, bucket.id); + await this.createNetworkBucket(params.userId, addressId); } catch (error) { // The principal already exists at this point, so roll it back too. await this.provider.deleteAccount(params.address); @@ -239,22 +236,10 @@ export class AccountService { account.addresses.map(async (a) => { await this.provider.deleteAccount(a.providerExternalId); await this.addresses.deleteProviderLink(a.id); + await this.deleteNetworkBucket(driveUserUuid, a); }), ); - if (account.networkBucketId) { - try { - await this.bridge.deleteMailBucket( - driveUserUuid, - account.networkBucketId, - ); - } catch (error) { - this.logger.warn( - `Failed to delete network bucket '${account.networkBucketId}' for '${driveUserUuid}': ${(error as Error).message}`, - ); - } - } - await this.accounts.delete(account.id); this.logger.log(`Deleted account for user '${driveUserUuid}'`); } @@ -307,6 +292,15 @@ export class AccountService { externalId: address, }); + try { + await this.createNetworkBucket(userId, newAddressId); + } catch (error) { + await this.provider.deleteAccount(address); + await this.addresses.deleteProviderLink(newAddressId); + await this.addresses.delete(newAddressId); + throw error; + } + this.logger.log(`Added address '${address}' to account '${userId}'`); } @@ -331,6 +325,7 @@ export class AccountService { this.addresses.deleteProviderLink(addressRecord.id), this.addresses.delete(addressRecord.id), ]); + await this.deleteNetworkBucket(userId, addressRecord); this.logger.log(`Removed address '${address}' from account '${userId}'`); } @@ -398,6 +393,29 @@ export class AccountService { } } + private async createNetworkBucket( + userUuid: string, + addressId: string, + ): Promise { + const bucket = await this.bridge.createMailBucket(userUuid, addressId); + await this.addresses.setNetworkBucketId(addressId, bucket.id); + } + + private async deleteNetworkBucket( + userUuid: string, + address: MailAddress, + ): Promise { + if (!address.networkBucketId) return; + + try { + await this.bridge.deleteMailBucket(userUuid, address.networkBucketId); + } catch (error) { + this.logger.warn( + `Failed to delete network bucket '${address.networkBucketId}' for '${userUuid}': ${(error as Error).message}`, + ); + } + } + private async getAccountOrFail(userId: string): Promise { const account = await this.accounts.findByUserId(userId); if (!account) { diff --git a/src/modules/account/domain/mail-account.domain.ts b/src/modules/account/domain/mail-account.domain.ts index 38071d1..7bc3512 100644 --- a/src/modules/account/domain/mail-account.domain.ts +++ b/src/modules/account/domain/mail-account.domain.ts @@ -13,7 +13,6 @@ export interface MailAccountAttributes { userId: string; status: MailAccountState; suspendedAt: Date | null; - networkBucketId: string | null; addresses: MailAddressAttributes[]; createdAt: Date; updatedAt: Date; @@ -24,7 +23,6 @@ export class MailAccount { readonly userId!: string; readonly status!: MailAccountState; readonly suspendedAt!: Date | null; - readonly networkBucketId!: string | null; readonly addresses!: MailAddress[]; readonly createdAt!: Date; readonly updatedAt!: Date; diff --git a/src/modules/account/domain/mail-address.domain.ts b/src/modules/account/domain/mail-address.domain.ts index 02a37af..a3a7967 100644 --- a/src/modules/account/domain/mail-address.domain.ts +++ b/src/modules/account/domain/mail-address.domain.ts @@ -5,6 +5,7 @@ export interface MailAddressAttributes { domainId: string; isDefault: boolean; providerExternalId: string; + networkBucketId: string | null; createdAt: Date; updatedAt: Date; } @@ -16,6 +17,7 @@ export class MailAddress { readonly domainId!: string; readonly isDefault!: boolean; readonly providerExternalId!: string; + readonly networkBucketId!: string | null; readonly createdAt!: Date; readonly updatedAt!: Date; diff --git a/src/modules/account/models/mail-account.model.ts b/src/modules/account/models/mail-account.model.ts index 4f08414..a15030f 100644 --- a/src/modules/account/models/mail-account.model.ts +++ b/src/modules/account/models/mail-account.model.ts @@ -36,10 +36,6 @@ export class MailAccountModel extends Model { @Column(DataType.DATE) declare suspendedAt: Date | null; - @AllowNull(true) - @Column({ field: 'network_bucket_id', type: DataType.UUID }) - declare networkBucketId: string | null; - @Column(DataType.DATE) declare deletedAt: Date | null; diff --git a/src/modules/account/models/mail-address.model.ts b/src/modules/account/models/mail-address.model.ts index 8b90abf..429a606 100644 --- a/src/modules/account/models/mail-address.model.ts +++ b/src/modules/account/models/mail-address.model.ts @@ -50,6 +50,10 @@ export class MailAddressModel extends Model { @Column(DataType.BOOLEAN) declare isDefault: boolean; + @AllowNull(true) + @Column({ field: 'network_bucket_id', type: DataType.STRING(24) }) + declare networkBucketId: string | null; + @Column(DataType.DATE) declare deletedAt: Date | null; diff --git a/src/modules/account/repositories/account.repository.spec.ts b/src/modules/account/repositories/account.repository.spec.ts index af4830f..4572034 100644 --- a/src/modules/account/repositories/account.repository.spec.ts +++ b/src/modules/account/repositories/account.repository.spec.ts @@ -34,7 +34,6 @@ describe('AccountRepository', () => { userId: 'user-1', status: 'active', suspendedAt: null, - networkBucketId: null, createdAt: new Date('2026-01-01T00:00:00.000Z'), updatedAt: new Date('2026-01-02T00:00:00.000Z'), addresses: [], @@ -130,15 +129,4 @@ describe('AccountRepository', () => { }); }); }); - - describe('setNetworkBucketId', () => { - it('when given an id and bucket id, then updates the account row', async () => { - await repository.setNetworkBucketId('acc-1', 'bucket-1'); - - expect(accountModel.update).toHaveBeenCalledWith( - { networkBucketId: 'bucket-1' }, - { where: { id: 'acc-1' } }, - ); - }); - }); }); diff --git a/src/modules/account/repositories/account.repository.ts b/src/modules/account/repositories/account.repository.ts index 1c759ba..5b71339 100644 --- a/src/modules/account/repositories/account.repository.ts +++ b/src/modules/account/repositories/account.repository.ts @@ -46,17 +46,12 @@ export class AccountRepository { await this.accountModel.destroy({ where: { id } }); } - async setNetworkBucketId(id: string, networkBucketId: string): Promise { - await this.accountModel.update({ networkBucketId }, { where: { id } }); - } - private toDomain(model: MailAccountModel): MailAccount { return MailAccount.build({ id: model.id, userId: model.userId, status: model.status as MailAccountState, suspendedAt: model.suspendedAt, - networkBucketId: model.networkBucketId, createdAt: model.createdAt as Date, updatedAt: model.updatedAt as Date, addresses: (model.addresses ?? []).map(toAddressAttributes), diff --git a/src/modules/account/repositories/address.repository.spec.ts b/src/modules/account/repositories/address.repository.spec.ts index 1a1ed47..dc8a1cb 100644 --- a/src/modules/account/repositories/address.repository.spec.ts +++ b/src/modules/account/repositories/address.repository.spec.ts @@ -123,4 +123,15 @@ describe('AddressRepository', () => { ); }); }); + + describe('setNetworkBucketId', () => { + it('when given an id and bucket id, then updates the address row', async () => { + await repository.setNetworkBucketId('addr-1', 'bucket-1'); + + expect(addressModel.update).toHaveBeenCalledWith( + { networkBucketId: 'bucket-1' }, + { where: { id: 'addr-1' } }, + ); + }); + }); }); diff --git a/src/modules/account/repositories/address.repository.ts b/src/modules/account/repositories/address.repository.ts index 7556aae..2d7f00b 100644 --- a/src/modules/account/repositories/address.repository.ts +++ b/src/modules/account/repositories/address.repository.ts @@ -27,6 +27,7 @@ export function toAddressAttributes( domainId: model.domainId, isDefault: model.isDefault, providerExternalId, + networkBucketId: model.networkBucketId, createdAt: model.createdAt as Date, updatedAt: model.updatedAt as Date, }; @@ -127,6 +128,10 @@ export class AddressRepository { await this.addressModel.destroy({ where: { id } }); } + async setNetworkBucketId(id: string, networkBucketId: string): Promise { + await this.addressModel.update({ networkBucketId }, { where: { id } }); + } + async setDefault(addressId: string, mailAccountId: string): Promise { await this.sequelize.query( `UPDATE mail_addresses SET is_default = (id = :addressId) WHERE mail_account_id = :mailAccountId`, diff --git a/test/fixtures.ts b/test/fixtures.ts index c416cef..3d9b8ac 100644 --- a/test/fixtures.ts +++ b/test/fixtures.ts @@ -210,6 +210,7 @@ export function newMailAddressAttributes( domainId: randomUuid(), isDefault: true, providerExternalId: random.email(), + networkBucketId: null, createdAt: new Date(), updatedAt: new Date(), ...attrs, @@ -254,7 +255,6 @@ export function newMailAccountAttributes( userId: randomUuid(), status: MailAccountState.Active, suspendedAt: null, - networkBucketId: null, addresses: [address], createdAt: new Date(), updatedAt: new Date(),