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/4] 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/4] 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(), From 416c9a780dc5f15d609153889cfd1c1798466376 Mon Sep 17 00:00:00 2001 From: jzunigax2 <125698953+jzunigax2@users.noreply.github.com> Date: Thu, 4 Jun 2026 11:25:02 -0600 Subject: [PATCH 3/4] feat: implement MTA hooks for email processing - Added MtaHooksModule, MtaHooksController, and MtaHooksService to handle MTA hooks. - Introduced MtaHooksAuthGuard for authentication using basic credentials. - Implemented logic to manage recipient quota checks during the RCPT stage. - Updated configuration to include MTA hooks credentials. - Added unit tests for MtaHooksService and MtaHooksAuthGuard to ensure functionality. --- .env.template | 6 + src/app.module.ts | 2 + src/config/configuration.ts | 5 + src/modules/email/email.module.ts | 1 + .../mta-hooks/mta-hooks-auth.guard.spec.ts | 72 +++++++ src/modules/mta-hooks/mta-hooks-auth.guard.ts | 57 +++++ src/modules/mta-hooks/mta-hooks.controller.ts | 23 ++ src/modules/mta-hooks/mta-hooks.module.ts | 14 ++ .../mta-hooks/mta-hooks.service.spec.ts | 202 ++++++++++++++++++ src/modules/mta-hooks/mta-hooks.service.ts | 94 ++++++++ src/modules/mta-hooks/mta-hooks.types.ts | 51 +++++ 11 files changed, 527 insertions(+) create mode 100644 src/modules/mta-hooks/mta-hooks-auth.guard.spec.ts create mode 100644 src/modules/mta-hooks/mta-hooks-auth.guard.ts create mode 100644 src/modules/mta-hooks/mta-hooks.controller.ts create mode 100644 src/modules/mta-hooks/mta-hooks.module.ts create mode 100644 src/modules/mta-hooks/mta-hooks.service.spec.ts create mode 100644 src/modules/mta-hooks/mta-hooks.service.ts create mode 100644 src/modules/mta-hooks/mta-hooks.types.ts diff --git a/.env.template b/.env.template index 293c4c6..000718e 100644 --- a/.env.template +++ b/.env.template @@ -23,3 +23,9 @@ GATEWAY_PUBLIC_SECRET= # External APIs PAYMENTS_API_URL= +BRIDGE_API_URL= +BRIDGE_PRIVATE_GATEWAY_SECRET= + +# MTA hooks +MTA_HOOKS_USERNAME= +MTA_HOOKS_SECRET= diff --git a/src/app.module.ts b/src/app.module.ts index e2bd351..05913ff 100644 --- a/src/app.module.ts +++ b/src/app.module.ts @@ -12,6 +12,7 @@ import { EmailModule } from './modules/email/email.module'; import { AuthModule } from './modules/auth/auth.module'; import { AccountModule } from './modules/account/account.module'; import { GatewayModule } from './modules/gateway/gateway.module'; +import { MtaHooksModule } from './modules/mta-hooks/mta-hooks.module'; import { HttpGlobalExceptionFilter } from './common/filters/http-global-exception.filter'; import { AddressesModule } from './modules/addresses/addresses.module'; @@ -87,6 +88,7 @@ import { AddressesModule } from './modules/addresses/addresses.module'; AccountModule, AddressesModule, GatewayModule, + MtaHooksModule, ], controllers: [], providers: [ diff --git a/src/config/configuration.ts b/src/config/configuration.ts index 27fcdcf..1321ae7 100644 --- a/src/config/configuration.ts +++ b/src/config/configuration.ts @@ -28,6 +28,11 @@ export default () => ({ ), }, + mtaHooks: { + username: process.env.MTA_HOOKS_USERNAME ?? 'stalwart', + secret: process.env.MTA_HOOKS_SECRET ?? '', + }, + secrets: { jwt: process.env.JWT_SECRET, drivePublicGateway: process.env.GATEWAY_PUBLIC_SECRET, diff --git a/src/modules/email/email.module.ts b/src/modules/email/email.module.ts index 2cf5a3a..72ab264 100644 --- a/src/modules/email/email.module.ts +++ b/src/modules/email/email.module.ts @@ -9,5 +9,6 @@ import { Reflector } from '@nestjs/core'; imports: [JmapModule, ProvisioningModule], controllers: [EmailController], providers: [EmailService, Reflector], + exports: [EmailService], }) export class EmailModule {} diff --git a/src/modules/mta-hooks/mta-hooks-auth.guard.spec.ts b/src/modules/mta-hooks/mta-hooks-auth.guard.spec.ts new file mode 100644 index 0000000..0568821 --- /dev/null +++ b/src/modules/mta-hooks/mta-hooks-auth.guard.spec.ts @@ -0,0 +1,72 @@ +import { describe, it, expect, beforeEach } from 'vitest'; +import { createMock } from '@golevelup/ts-vitest'; +import { type ConfigService } from '@nestjs/config'; +import { UnauthorizedException, type ExecutionContext } from '@nestjs/common'; +import { MtaHooksAuthGuard } from './mta-hooks-auth.guard.js'; + +const username = 'stalwart'; +const secret = 'secret'; + +describe('MtaHooksAuthGuard', () => { + let guard: MtaHooksAuthGuard; + + const contextWithAuth = (header?: string): ExecutionContext => + createMock({ + switchToHttp: () => ({ + getRequest: () => ({ + headers: header ? { authorization: header } : {}, + }), + }), + }); + + const basic = (username: string, secret: string): string => + `Basic ${Buffer.from(`${username}:${secret}`).toString('base64')}`; + + beforeEach(() => { + const configService = createMock(); + configService.getOrThrow.mockImplementation((key: string) => { + if (key === 'mtaHooks.username') return username; + if (key === 'mtaHooks.secret') return secret; + throw new Error(`unknown key: ${key}`); + }); + guard = new MtaHooksAuthGuard(configService); + }); + + it('when credentials match, then allows the request', () => { + const context = contextWithAuth(basic(username, secret)); + + expect(guard.canActivate(context)).toBe(true); + }); + + it('when the secret is wrong, then throws Unauthorized', () => { + const context = contextWithAuth(basic(username, 'wrong')); + + expect(() => guard.canActivate(context)).toThrow(UnauthorizedException); + }); + + it('when the username is wrong, then throws Unauthorized', () => { + const context = contextWithAuth(basic('wrong', secret)); + + expect(() => guard.canActivate(context)).toThrow(UnauthorizedException); + }); + + it('when the Authorization header is missing, then throws Unauthorized', () => { + const context = contextWithAuth(undefined); + + expect(() => guard.canActivate(context)).toThrow(UnauthorizedException); + }); + + it('when the scheme is not Basic, then throws Unauthorized', () => { + const context = contextWithAuth('Bearer some-token'); + + expect(() => guard.canActivate(context)).toThrow(UnauthorizedException); + }); + + it('when the decoded credentials lack a colon separator, then throws Unauthorized', () => { + const context = contextWithAuth( + `Basic ${Buffer.from('nocolon').toString('base64')}`, + ); + + expect(() => guard.canActivate(context)).toThrow(UnauthorizedException); + }); +}); diff --git a/src/modules/mta-hooks/mta-hooks-auth.guard.ts b/src/modules/mta-hooks/mta-hooks-auth.guard.ts new file mode 100644 index 0000000..2cd27b8 --- /dev/null +++ b/src/modules/mta-hooks/mta-hooks-auth.guard.ts @@ -0,0 +1,57 @@ +import { timingSafeEqual } from 'node:crypto'; +import { + CanActivate, + type ExecutionContext, + Injectable, + UnauthorizedException, +} from '@nestjs/common'; +import { ConfigService } from '@nestjs/config'; +import type { Request } from 'express'; + +@Injectable() +export class MtaHooksAuthGuard implements CanActivate { + private readonly expectedUsername: string; + private readonly expectedSecret: string; + + constructor(configService: ConfigService) { + this.expectedUsername = + configService.getOrThrow('mtaHooks.username'); + this.expectedSecret = configService.getOrThrow('mtaHooks.secret'); + } + + canActivate(context: ExecutionContext): boolean { + const request = context.switchToHttp().getRequest(); + const header = request.headers.authorization ?? ''; + + const [scheme, encoded] = header.split(' '); + if (scheme !== 'Basic' || !encoded) { + throw new UnauthorizedException('Missing or malformed Basic credentials'); + } + + const decoded = Buffer.from(encoded, 'base64').toString('utf8'); + const separatorIndex = decoded.indexOf(':'); + if (separatorIndex === -1) { + throw new UnauthorizedException('Malformed Basic credentials'); + } + + const username = decoded.slice(0, separatorIndex); + const secret = decoded.slice(separatorIndex + 1); + + const usernameMatches = this.safeEqual(username, this.expectedUsername); + const secretMatches = this.safeEqual(secret, this.expectedSecret); + if (!usernameMatches || !secretMatches) { + throw new UnauthorizedException('Invalid MTA hook credentials'); + } + + return true; + } + + private safeEqual(a: string, b: string): boolean { + const bufferA = Buffer.from(a, 'utf8'); + const bufferB = Buffer.from(b, 'utf8'); + if (bufferA.length !== bufferB.length) { + return false; + } + return timingSafeEqual(bufferA, bufferB); + } +} diff --git a/src/modules/mta-hooks/mta-hooks.controller.ts b/src/modules/mta-hooks/mta-hooks.controller.ts new file mode 100644 index 0000000..b85e2cf --- /dev/null +++ b/src/modules/mta-hooks/mta-hooks.controller.ts @@ -0,0 +1,23 @@ +import { Body, Controller, Post, UseGuards } from '@nestjs/common'; +import { ApiBasicAuth, ApiOperation, ApiTags } from '@nestjs/swagger'; +import { Public } from '../auth/decorators/public.decorator.js'; +import { MtaHooksAuthGuard } from './mta-hooks-auth.guard.js'; +import { MtaHooksService } from './mta-hooks.service.js'; +import type { MtaHookRequest, MtaHookResponse } from './mta-hooks.types.js'; + +@ApiTags('MTA Hooks') +@ApiBasicAuth('mta-hooks') +@Public() +@UseGuards(MtaHooksAuthGuard) +@Controller('mta-hooks') +export class MtaHooksController { + constructor(private readonly mtaHooksService: MtaHooksService) {} + + @Post('rcpt') + @ApiOperation({ + summary: 'RCPT-stage hook', + }) + rcpt(@Body() request: MtaHookRequest): Promise { + return this.mtaHooksService.handleRcpt(request); + } +} diff --git a/src/modules/mta-hooks/mta-hooks.module.ts b/src/modules/mta-hooks/mta-hooks.module.ts new file mode 100644 index 0000000..479cc10 --- /dev/null +++ b/src/modules/mta-hooks/mta-hooks.module.ts @@ -0,0 +1,14 @@ +import { Module } from '@nestjs/common'; +import { AccountModule } from '../account/account.module.js'; +import { EmailModule } from '../email/email.module.js'; +import { BridgeModule } from '../infrastructure/bridge/bridge.module.js'; +import { MtaHooksAuthGuard } from './mta-hooks-auth.guard.js'; +import { MtaHooksController } from './mta-hooks.controller.js'; +import { MtaHooksService } from './mta-hooks.service.js'; + +@Module({ + imports: [AccountModule, EmailModule, BridgeModule], + controllers: [MtaHooksController], + providers: [MtaHooksService, MtaHooksAuthGuard], +}) +export class MtaHooksModule {} diff --git a/src/modules/mta-hooks/mta-hooks.service.spec.ts b/src/modules/mta-hooks/mta-hooks.service.spec.ts new file mode 100644 index 0000000..7293a48 --- /dev/null +++ b/src/modules/mta-hooks/mta-hooks.service.spec.ts @@ -0,0 +1,202 @@ +import { describe, it, expect, beforeEach } from 'vitest'; +import { Test, type TestingModule } from '@nestjs/testing'; +import { createMock, type DeepMocked } from '@golevelup/ts-vitest'; +import { AccountService } from '../account/account.service.js'; +import { EmailService } from '../email/email.service.js'; +import { BridgeClient } from '../infrastructure/bridge/bridge.service.js'; +import { MtaHooksService } from './mta-hooks.service.js'; +import type { MtaHookRequest } from './mta-hooks.types.js'; + +describe('MtaHooksService', () => { + let service: MtaHooksService; + let accountService: DeepMocked; + let emailService: DeepMocked; + let bridgeClient: DeepMocked; + + const buildRequest = ( + to: string[], + opts: { size?: number } = {}, + ): MtaHookRequest => ({ + context: { stage: 'rcpt' }, + envelope: { + from: { + address: 'sender@external.com', + parameters: + opts.size !== undefined ? { size: String(opts.size) } : undefined, + }, + to: to.map((address) => ({ address })), + }, + }); + + beforeEach(async () => { + const module: TestingModule = await Test.createTestingModule({ + providers: [MtaHooksService], + }) + .useMocker(() => createMock()) + .compile(); + + service = module.get(MtaHooksService); + accountService = module.get(AccountService); + emailService = module.get(EmailService); + bridgeClient = module.get(BridgeClient); + }); + + describe('handleRcpt', () => { + it('when the recipient stays within quota, then accepts', async () => { + accountService.findUserIdByAddress.mockResolvedValue('user-1'); + emailService.getQuota.mockResolvedValue({ used: 1000, limit: 5000 }); + bridgeClient.reportMailUsage.mockResolvedValue({ + driveUsed: 2000, + planQuota: 5000, + }); + + const result = await service.handleRcpt( + buildRequest(['jane@inxt.com'], { size: 500 }), + ); + + expect(result).toStrictEqual({ action: 'accept' }); + expect(accountService.findUserIdByAddress).toHaveBeenCalledWith( + 'jane@inxt.com', + ); + expect(bridgeClient.reportMailUsage).toHaveBeenCalledWith('user-1', 1000); + }); + + it('when the declared SIZE pushes the recipient over quota, then rejects with 452 4.2.2', async () => { + accountService.findUserIdByAddress.mockResolvedValue('user-1'); + emailService.getQuota.mockResolvedValue({ used: 1000, limit: 5000 }); + bridgeClient.reportMailUsage.mockResolvedValue({ + driveUsed: 3800, + planQuota: 5000, + }); + + const result = await service.handleRcpt( + buildRequest(['jane@inxt.com'], { size: 500 }), + ); + + expect(result).toStrictEqual({ + action: 'reject', + response: { + status: 452, + enhancedStatus: '4.2.2', + message: 'Recipient mailbox is over quota', + }, + }); + }); + + it('when projected usage exactly equals the quota, then accepts', async () => { + accountService.findUserIdByAddress.mockResolvedValue('user-1'); + emailService.getQuota.mockResolvedValue({ used: 1000, limit: 5000 }); + bridgeClient.reportMailUsage.mockResolvedValue({ + driveUsed: 3500, + planQuota: 5000, + }); + + const result = await service.handleRcpt( + buildRequest(['jane@inxt.com'], { size: 500 }), + ); + + expect(result).toStrictEqual({ action: 'accept' }); + }); + + it('when no SIZE is declared, then the incoming bytes are not counted (accepts if not already over)', async () => { + accountService.findUserIdByAddress.mockResolvedValue('user-1'); + emailService.getQuota.mockResolvedValue({ used: 1000, limit: 5000 }); + bridgeClient.reportMailUsage.mockResolvedValue({ + driveUsed: 3800, + planQuota: 5000, + }); + + // Without SIZE the projected usage is 3800 + 1000 = 4800 <= 5000. + const result = await service.handleRcpt(buildRequest(['jane@inxt.com'])); + + expect(result).toStrictEqual({ action: 'accept' }); + }); + + it('when no SIZE is declared but the mailbox is already over quota, then rejects', async () => { + accountService.findUserIdByAddress.mockResolvedValue('user-1'); + emailService.getQuota.mockResolvedValue({ used: 1000, limit: 5000 }); + bridgeClient.reportMailUsage.mockResolvedValue({ + driveUsed: 4500, + planQuota: 5000, + }); + + const result = await service.handleRcpt(buildRequest(['jane@inxt.com'])); + + expect(result.action).toBe('reject'); + }); + + it('when the address resolves to no internxt user, then skips it and accepts', async () => { + accountService.findUserIdByAddress.mockResolvedValue(null); + + const result = await service.handleRcpt( + buildRequest(['external@gmail.com'], { size: 500 }), + ); + + expect(result).toStrictEqual({ action: 'accept' }); + expect(emailService.getQuota).not.toHaveBeenCalled(); + expect(bridgeClient.reportMailUsage).not.toHaveBeenCalled(); + }); + + it('when the recipient address is upper-cased, then it is lowercased before resolution', async () => { + accountService.findUserIdByAddress.mockResolvedValue('user-1'); + emailService.getQuota.mockResolvedValue({ used: 0, limit: 5000 }); + bridgeClient.reportMailUsage.mockResolvedValue({ + driveUsed: 0, + planQuota: 5000, + }); + + await service.handleRcpt(buildRequest(['Jane@INXT.com'], { size: 10 })); + + expect(accountService.findUserIdByAddress).toHaveBeenCalledWith( + 'jane@inxt.com', + ); + expect(emailService.getQuota).toHaveBeenCalledWith('jane@inxt.com'); + }); + + it('when several recipients are present, then only the current (last) one is evaluated', async () => { + accountService.findUserIdByAddress.mockResolvedValue('user-1'); + emailService.getQuota.mockResolvedValue({ used: 0, limit: 5000 }); + bridgeClient.reportMailUsage.mockResolvedValue({ + driveUsed: 0, + planQuota: 5000, + }); + + await service.handleRcpt( + buildRequest(['first@inxt.com', 'current@inxt.com']), + ); + + expect(accountService.findUserIdByAddress).toHaveBeenCalledTimes(1); + expect(accountService.findUserIdByAddress).toHaveBeenCalledWith( + 'current@inxt.com', + ); + }); + + it('when quota lookup throws, then fails open and accepts', async () => { + accountService.findUserIdByAddress.mockResolvedValue('user-1'); + emailService.getQuota.mockRejectedValue(new Error('JMAP down')); + + const result = await service.handleRcpt(buildRequest(['jane@inxt.com'])); + + expect(result).toStrictEqual({ action: 'accept' }); + }); + + it('when Bridge reporting throws, then fails open and accepts', async () => { + accountService.findUserIdByAddress.mockResolvedValue('user-1'); + emailService.getQuota.mockResolvedValue({ used: 1000, limit: 5000 }); + bridgeClient.reportMailUsage.mockRejectedValue(new Error('bridge down')); + + const result = await service.handleRcpt(buildRequest(['jane@inxt.com'])); + + expect(result).toStrictEqual({ action: 'accept' }); + }); + + it('when the request carries no recipients, then accepts without lookups', async () => { + const result = await service.handleRcpt({ + context: { stage: 'rcpt' }, + }); + + expect(result).toStrictEqual({ action: 'accept' }); + expect(accountService.findUserIdByAddress).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/src/modules/mta-hooks/mta-hooks.service.ts b/src/modules/mta-hooks/mta-hooks.service.ts new file mode 100644 index 0000000..83969ef --- /dev/null +++ b/src/modules/mta-hooks/mta-hooks.service.ts @@ -0,0 +1,94 @@ +import { Injectable, Logger } from '@nestjs/common'; +import { AccountService } from '../account/account.service.js'; +import { EmailService } from '../email/email.service.js'; +import { BridgeClient } from '../infrastructure/bridge/bridge.service.js'; +import type { + MtaHookEnvelope, + MtaHookRequest, + MtaHookResponse, +} from './mta-hooks.types.js'; + +const ACCEPT: MtaHookResponse = { action: 'accept' }; + +const REJECT_OVER_QUOTA: MtaHookResponse = { + action: 'reject', + response: { + status: 452, + enhancedStatus: '4.2.2', + message: 'Recipient mailbox is over quota', + }, +}; + +@Injectable() +export class MtaHooksService { + private readonly logger = new Logger(MtaHooksService.name); + + constructor( + private readonly accountService: AccountService, + private readonly emailService: EmailService, + private readonly bridgeClient: BridgeClient, + ) {} + + async handleRcpt(request: MtaHookRequest): Promise { + const recipients = request.envelope?.to ?? []; + const recipient = recipients.at(-1); + if (!recipient) { + return ACCEPT; + } + + const declaredSize = this.parseDeclaredSize(request.envelope); + + try { + const overQuota = await this.isRecipientOverQuota( + recipient.address, + declaredSize, + ); + return overQuota ? REJECT_OVER_QUOTA : ACCEPT; + } catch (error) { + this.logger.error( + `MTA hook quota check failed, accepting recipient (fail-open): ${ + error instanceof Error ? error.message : String(error) + }`, + ); + return ACCEPT; + } + } + + private async isRecipientOverQuota( + rawAddress: string, + incomingSize: number, + ): Promise { + const address = rawAddress.toLowerCase(); + const userUuid = await this.accountService.findUserIdByAddress(address); + if (!userUuid) { + return false; + } + + const { used: mailUsed } = await this.emailService.getQuota(address); + + const { driveUsed, planQuota } = await this.bridgeClient.reportMailUsage( + userUuid, + mailUsed, + ); + + const projectedUsage = driveUsed + mailUsed + incomingSize; + if (projectedUsage > planQuota) { + this.logger.warn( + `Rejecting recipient '${address}' (user '${userUuid}'): ` + + `projected ${projectedUsage} > quota ${planQuota}`, + ); + return true; + } + + return false; + } + + private parseDeclaredSize(envelope?: MtaHookEnvelope): number { + const raw = envelope?.from.parameters?.size; + if (!raw) { + return 0; + } + const parsed = Number.parseInt(raw, 10); + return Number.isFinite(parsed) && parsed > 0 ? parsed : 0; + } +} diff --git a/src/modules/mta-hooks/mta-hooks.types.ts b/src/modules/mta-hooks/mta-hooks.types.ts new file mode 100644 index 0000000..d1fea81 --- /dev/null +++ b/src/modules/mta-hooks/mta-hooks.types.ts @@ -0,0 +1,51 @@ +/** + * Types for Stalwart's MTA Hooks protocol. + * + * Stalwart POSTs a JSON {@link MtaHookRequest} to the configured endpoint at a + * given SMTP stage and expects a JSON {@link MtaHookResponse} telling it whether + * to accept or reject the message. + * + * We only model the fields this service reads. Unmodelled fields (client, tls, + * sasl, server, queue, protocol, headers, ...) are still sent by Stalwart but + * are intentionally omitted here. + * + * @see https://stalw.art/docs/api/mta-hooks/overview + */ + +export type MtaHookStage = + | 'connect' + | 'ehlo' + | 'auth' + | 'mail' + | 'rcpt' + | 'data'; + +export interface MtaHookAddress { + address: string; + parameters?: Record | null; +} + +export interface MtaHookEnvelope { + from: MtaHookAddress; + to: MtaHookAddress[]; +} + +export interface MtaHookRequest { + context: { + stage: MtaHookStage; + }; + envelope?: MtaHookEnvelope; +} + +export type MtaHookAction = 'accept' | 'reject'; + +export interface MtaHookSmtpResponse { + status?: number; + enhancedStatus?: string; + message?: string; +} + +export interface MtaHookResponse { + action: MtaHookAction; + response?: MtaHookSmtpResponse; +} From 6e5d2023688506dec455cab683332d4c055c9763 Mon Sep 17 00:00:00 2001 From: jzunigax2 <125698953+jzunigax2@users.noreply.github.com> Date: Wed, 10 Jun 2026 14:33:47 -0600 Subject: [PATCH 4/4] feat: refactor email quota handling to use bucket-based storage - Updated MtaHooksService to utilize network bucket ID for quota checks during email processing. - Replaced reportMailUsage with reportBucketUsage in BridgeClient for accurate space tracking. - Enhanced unit tests to reflect changes in recipient context retrieval and bucket usage reporting. - Introduced new UserSpaceSnapshot type for managing bucket space data. --- src/modules/account/account.service.spec.ts | 69 ++++++++++ src/modules/account/account.service.ts | 35 +++++ .../repositories/address.repository.ts | 21 +++ .../bridge/bridge.service.spec.ts | 32 ++--- .../infrastructure/bridge/bridge.service.ts | 17 +-- .../infrastructure/bridge/bridge.types.ts | 6 +- .../infrastructure/jmap/jmap.service.spec.ts | 3 + .../infrastructure/jmap/jmap.service.ts | 11 +- .../mta-hooks/mta-hooks.service.spec.ts | 126 ++++++++++++------ src/modules/mta-hooks/mta-hooks.service.ts | 23 ++-- 10 files changed, 259 insertions(+), 84 deletions(-) diff --git a/src/modules/account/account.service.spec.ts b/src/modules/account/account.service.spec.ts index 78e4ea0..c86b122 100644 --- a/src/modules/account/account.service.spec.ts +++ b/src/modules/account/account.service.spec.ts @@ -141,6 +141,75 @@ describe('AccountService', () => { }); }); + describe('findRecipientContext', () => { + it('when the address has a bucket, then returns the user and bucket without provisioning', async () => { + addresses.findRecipientContextByAddress.mockResolvedValue({ + addressId: 'address-1', + userUuid: 'user-1', + networkBucketId: 'bucket-1', + }); + + const result = await service.findRecipientContext('alice@internxt.com'); + + expect(result).toEqual({ + userUuid: 'user-1', + networkBucketId: 'bucket-1', + }); + expect(bridge.createMailBucket).not.toHaveBeenCalled(); + expect(addresses.setNetworkBucketId).not.toHaveBeenCalled(); + }); + + it('when the address has no bucket, then lazily provisions one and persists it', async () => { + addresses.findRecipientContextByAddress.mockResolvedValue({ + addressId: 'address-1', + userUuid: 'user-1', + networkBucketId: null, + }); + bridge.createMailBucket.mockResolvedValue({ + id: 'bucket-new', + name: 'address-1', + }); + + const result = await service.findRecipientContext('alice@internxt.com'); + + expect(bridge.createMailBucket).toHaveBeenCalledWith( + 'user-1', + 'address-1', + ); + expect(addresses.setNetworkBucketId).toHaveBeenCalledWith( + 'address-1', + 'bucket-new', + ); + expect(result).toEqual({ + userUuid: 'user-1', + networkBucketId: 'bucket-new', + }); + }); + + it('when lazy provisioning fails, then the error propagates', async () => { + addresses.findRecipientContextByAddress.mockResolvedValue({ + addressId: 'address-1', + userUuid: 'user-1', + networkBucketId: null, + }); + bridge.createMailBucket.mockRejectedValue(new Error('bridge down')); + + await expect( + service.findRecipientContext('alice@internxt.com'), + ).rejects.toThrow('bridge down'); + expect(addresses.setNetworkBucketId).not.toHaveBeenCalled(); + }); + + it('when the address does not exist, then returns null', async () => { + addresses.findRecipientContextByAddress.mockResolvedValue(null); + + const result = await service.findRecipientContext('unknown@internxt.com'); + + expect(result).toBeNull(); + expect(bridge.createMailBucket).not.toHaveBeenCalled(); + }); + }); + describe('getAddressKeys', () => { it('when address belongs to user, then returns the key bundle', async () => { const addr = newMailAddressAttributes(); diff --git a/src/modules/account/account.service.ts b/src/modules/account/account.service.ts index 9e73114..1d51764 100644 --- a/src/modules/account/account.service.ts +++ b/src/modules/account/account.service.ts @@ -25,6 +25,11 @@ export interface MailAddressKeyBundle { recoveryPrivateKey: string; } +export interface RecipientContext { + userUuid: string; + networkBucketId: string; +} + export interface MailAccountStatus { id: string; defaultAddress: string | null; @@ -111,6 +116,36 @@ export class AccountService { return this.addresses.findUserIdByAddress(address); } + async findRecipientContext( + address: string, + ): Promise { + const context = await this.addresses.findRecipientContextByAddress(address); + if (!context) { + return null; + } + + if (context.networkBucketId) { + return { + userUuid: context.userUuid, + networkBucketId: context.networkBucketId, + }; + } + + this.logger.debug( + `Address '${address}' (id '${context.addressId}') has no network bucket, lazily provisioning one`, + ); + const bucket = await this.bridge.createMailBucket( + context.userUuid, + context.addressId, + ); + await this.addresses.setNetworkBucketId(context.addressId, bucket.id); + this.logger.log( + `Provisioned network bucket '${bucket.id}' for address '${address}' (user '${context.userUuid}')`, + ); + + return { userUuid: context.userUuid, networkBucketId: bucket.id }; + } + async getAddressKeys( userId: string, address: string, diff --git a/src/modules/account/repositories/address.repository.ts b/src/modules/account/repositories/address.repository.ts index 2d7f00b..6b9e8af 100644 --- a/src/modules/account/repositories/address.repository.ts +++ b/src/modules/account/repositories/address.repository.ts @@ -94,6 +94,27 @@ export class AddressRepository { return model?.account?.userId ?? null; } + async findRecipientContextByAddress(address: string): Promise<{ + addressId: string; + userUuid: string; + networkBucketId: string | null; + } | null> { + const model = await this.addressModel.findOne({ + where: { address }, + include: [{ model: MailAccountModel }], + }); + + if (!model?.account) { + return null; + } + + return { + addressId: model.id, + userUuid: model.account.userId, + networkBucketId: model.networkBucketId, + }; + } + async findDefaultForAccount( mailAccountId: string, ): Promise { diff --git a/src/modules/infrastructure/bridge/bridge.service.spec.ts b/src/modules/infrastructure/bridge/bridge.service.spec.ts index c9739b7..37e6c69 100644 --- a/src/modules/infrastructure/bridge/bridge.service.spec.ts +++ b/src/modules/infrastructure/bridge/bridge.service.spec.ts @@ -39,18 +39,18 @@ describe('BridgeClient', () => { }; }); - describe('reportMailUsage', () => { - it('when Bridge returns 200, then signs a gateway token, PUTs usage, and returns storage', async () => { - const storage = { driveUsed: 1024, planQuota: 5368709120 }; + describe('reportBucketUsage', () => { + it('when Bridge returns 200, then signs a gateway token, PUTs usage to the bucket, and returns the space snapshot', async () => { + const snapshot = { maxSpaceBytes: 5368709120, totalUsedSpaceBytes: 1024 }; jwtService.sign.mockReturnValue('signed-jwt'); httpRequest.mockResolvedValue({ statusCode: 200, - body: { text: () => Promise.resolve(JSON.stringify(storage)) }, + body: { text: () => Promise.resolve(JSON.stringify(snapshot)) }, }); - const result = await service.reportMailUsage('user-1', 512); + const result = await service.reportBucketUsage('user-1', 'bucket-1', 512); - expect(result).toStrictEqual(storage); + expect(result).toStrictEqual(snapshot); expect(jwtService.sign).toHaveBeenCalledWith( { payload: { uuid: 'user-1' } }, { @@ -63,8 +63,8 @@ describe('BridgeClient', () => { expect(httpRequest).toHaveBeenCalledWith( expect.objectContaining({ method: 'PUT', - path: '/v2/gateway/users/user-1/mail-usage', - body: JSON.stringify({ mailUsedBytes: 512 }), + path: '/v2/gateway/users/user-1/buckets/bucket-1/usage', + body: JSON.stringify({ usedSpaceBytes: 512 }), headers: expect.objectContaining({ authorization: 'Bearer signed-jwt', }) as unknown, @@ -75,29 +75,29 @@ describe('BridgeClient', () => { 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') }, + statusCode: 404, + body: { text: () => Promise.resolve('bucket not found') }, }); const error: unknown = await service - .reportMailUsage('user-1', 512) + .reportBucketUsage('user-1', 'bucket-1', 512) .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'); + expect(error.statusCode).toBe(404); + expect(error.details).toBe('bucket not found'); }); it('when the HTTP request throws, then the error propagates', async () => { jwtService.sign.mockReturnValue('signed-jwt'); httpRequest.mockRejectedValue(new Error('network failure')); - await expect(service.reportMailUsage('user-1', 512)).rejects.toThrow( - 'network failure', - ); + await expect( + service.reportBucketUsage('user-1', 'bucket-1', 512), + ).rejects.toThrow('network failure'); }); }); diff --git a/src/modules/infrastructure/bridge/bridge.service.ts b/src/modules/infrastructure/bridge/bridge.service.ts index 442391d..f36ea0c 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 { MailBucket, UserStorage } from './bridge.types.js'; +import type { MailBucket, UserSpaceSnapshot } from './bridge.types.js'; @Injectable() export class BridgeClient implements OnModuleInit, OnModuleDestroy { @@ -48,34 +48,35 @@ export class BridgeClient implements OnModuleInit, OnModuleDestroy { await this.httpClient.close(); } - async reportMailUsage( + async reportBucketUsage( userUuid: string, - mailUsedBytes: number, - ): Promise { + bucketId: string, + usedSpaceBytes: number, + ): Promise { const token = this.signGatewayToken(userUuid); const { statusCode, body } = await this.httpClient.request({ method: 'PUT', - path: `${this.basePath}/v2/gateway/users/${encodeURIComponent(userUuid)}/mail-usage`, + path: `${this.basePath}/v2/gateway/users/${encodeURIComponent(userUuid)}/buckets/${encodeURIComponent(bucketId)}/usage`, headers: { 'content-type': 'application/json', accept: 'application/json', authorization: `Bearer ${token}`, }, - body: JSON.stringify({ mailUsedBytes }), + body: JSON.stringify({ usedSpaceBytes }), }); const text = await body.text(); if (statusCode !== 200) { throw new BridgeApiError( - `Failed to report mail usage for user '${userUuid}': HTTP ${statusCode}`, + `Failed to report bucket usage for user '${userUuid}' bucket '${bucketId}': HTTP ${statusCode}`, statusCode, text, ); } - return JSON.parse(text) as UserStorage; + return JSON.parse(text) as UserSpaceSnapshot; } async createMailBucket(userUuid: string, name: string): Promise { diff --git a/src/modules/infrastructure/bridge/bridge.types.ts b/src/modules/infrastructure/bridge/bridge.types.ts index b1f8554..ebc8ed9 100644 --- a/src/modules/infrastructure/bridge/bridge.types.ts +++ b/src/modules/infrastructure/bridge/bridge.types.ts @@ -1,6 +1,6 @@ -export interface UserStorage { - driveUsed: number; - planQuota: number; +export interface UserSpaceSnapshot { + maxSpaceBytes: number; + totalUsedSpaceBytes: number; } export interface MailBucket { diff --git a/src/modules/infrastructure/jmap/jmap.service.spec.ts b/src/modules/infrastructure/jmap/jmap.service.spec.ts index eb2c51f..18dd047 100644 --- a/src/modules/infrastructure/jmap/jmap.service.spec.ts +++ b/src/modules/infrastructure/jmap/jmap.service.spec.ts @@ -7,6 +7,9 @@ vi.mock('undici', () => ({ Client: vi.fn().mockImplementation(function () { return { request: mockRequest, close: vi.fn() }; }), + Pool: vi.fn().mockImplementation(function () { + return { request: mockRequest, close: vi.fn() }; + }), })); function createConfigService(): ConfigService { diff --git a/src/modules/infrastructure/jmap/jmap.service.ts b/src/modules/infrastructure/jmap/jmap.service.ts index 242ed29..e2c19ed 100644 --- a/src/modules/infrastructure/jmap/jmap.service.ts +++ b/src/modules/infrastructure/jmap/jmap.service.ts @@ -5,7 +5,7 @@ import { type OnModuleInit, } from '@nestjs/common'; import { ConfigService } from '@nestjs/config'; -import { Client } from 'undici'; +import { Client, Pool } from 'undici'; import type { Readable } from 'node:stream'; import type { DownloadAttachmentPayload, @@ -50,7 +50,7 @@ export class JmapService implements OnModuleInit, OnModuleDestroy { private readonly masterUser: string; private readonly masterPassword: string; private readonly sessionCache = new Map(); // TODO: Implement cache ? - private httpClient!: Client; + private httpClient!: Pool; private blobClient!: Client; constructor(private readonly configService: ConfigService) { @@ -64,10 +64,13 @@ export class JmapService implements OnModuleInit, OnModuleDestroy { } onModuleInit() { - this.httpClient = new Client(this.stalwartUrl, { - allowH2: true, + this.httpClient = new Pool(this.stalwartUrl, { + connections: 16, + allowH2: false, keepAliveTimeout: 30_000, pipelining: 1, + headersTimeout: 10_000, + bodyTimeout: 10_000, }); this.blobClient = new Client(this.stalwartUrl, { allowH2: false, diff --git a/src/modules/mta-hooks/mta-hooks.service.spec.ts b/src/modules/mta-hooks/mta-hooks.service.spec.ts index 7293a48..3b55cf5 100644 --- a/src/modules/mta-hooks/mta-hooks.service.spec.ts +++ b/src/modules/mta-hooks/mta-hooks.service.spec.ts @@ -43,11 +43,14 @@ describe('MtaHooksService', () => { describe('handleRcpt', () => { it('when the recipient stays within quota, then accepts', async () => { - accountService.findUserIdByAddress.mockResolvedValue('user-1'); + accountService.findRecipientContext.mockResolvedValue({ + userUuid: 'user-1', + networkBucketId: 'bucket-1', + }); emailService.getQuota.mockResolvedValue({ used: 1000, limit: 5000 }); - bridgeClient.reportMailUsage.mockResolvedValue({ - driveUsed: 2000, - planQuota: 5000, + bridgeClient.reportBucketUsage.mockResolvedValue({ + maxSpaceBytes: 5000, + totalUsedSpaceBytes: 2000, }); const result = await service.handleRcpt( @@ -55,18 +58,25 @@ describe('MtaHooksService', () => { ); expect(result).toStrictEqual({ action: 'accept' }); - expect(accountService.findUserIdByAddress).toHaveBeenCalledWith( + expect(accountService.findRecipientContext).toHaveBeenCalledWith( 'jane@inxt.com', ); - expect(bridgeClient.reportMailUsage).toHaveBeenCalledWith('user-1', 1000); + expect(bridgeClient.reportBucketUsage).toHaveBeenCalledWith( + 'user-1', + 'bucket-1', + 1000, + ); }); it('when the declared SIZE pushes the recipient over quota, then rejects with 452 4.2.2', async () => { - accountService.findUserIdByAddress.mockResolvedValue('user-1'); + accountService.findRecipientContext.mockResolvedValue({ + userUuid: 'user-1', + networkBucketId: 'bucket-1', + }); emailService.getQuota.mockResolvedValue({ used: 1000, limit: 5000 }); - bridgeClient.reportMailUsage.mockResolvedValue({ - driveUsed: 3800, - planQuota: 5000, + bridgeClient.reportBucketUsage.mockResolvedValue({ + maxSpaceBytes: 5000, + totalUsedSpaceBytes: 4800, }); const result = await service.handleRcpt( @@ -84,11 +94,14 @@ describe('MtaHooksService', () => { }); it('when projected usage exactly equals the quota, then accepts', async () => { - accountService.findUserIdByAddress.mockResolvedValue('user-1'); + accountService.findRecipientContext.mockResolvedValue({ + userUuid: 'user-1', + networkBucketId: 'bucket-1', + }); emailService.getQuota.mockResolvedValue({ used: 1000, limit: 5000 }); - bridgeClient.reportMailUsage.mockResolvedValue({ - driveUsed: 3500, - planQuota: 5000, + bridgeClient.reportBucketUsage.mockResolvedValue({ + maxSpaceBytes: 5000, + totalUsedSpaceBytes: 4500, }); const result = await service.handleRcpt( @@ -98,26 +111,32 @@ describe('MtaHooksService', () => { expect(result).toStrictEqual({ action: 'accept' }); }); - it('when no SIZE is declared, then the incoming bytes are not counted (accepts if not already over)', async () => { - accountService.findUserIdByAddress.mockResolvedValue('user-1'); + it('when the snapshot already includes mail usage, then it is not double-counted', async () => { + accountService.findRecipientContext.mockResolvedValue({ + userUuid: 'user-1', + networkBucketId: 'bucket-1', + }); emailService.getQuota.mockResolvedValue({ used: 1000, limit: 5000 }); - bridgeClient.reportMailUsage.mockResolvedValue({ - driveUsed: 3800, - planQuota: 5000, + + bridgeClient.reportBucketUsage.mockResolvedValue({ + maxSpaceBytes: 5000, + totalUsedSpaceBytes: 4800, }); - // Without SIZE the projected usage is 3800 + 1000 = 4800 <= 5000. const result = await service.handleRcpt(buildRequest(['jane@inxt.com'])); expect(result).toStrictEqual({ action: 'accept' }); }); it('when no SIZE is declared but the mailbox is already over quota, then rejects', async () => { - accountService.findUserIdByAddress.mockResolvedValue('user-1'); + accountService.findRecipientContext.mockResolvedValue({ + userUuid: 'user-1', + networkBucketId: 'bucket-1', + }); emailService.getQuota.mockResolvedValue({ used: 1000, limit: 5000 }); - bridgeClient.reportMailUsage.mockResolvedValue({ - driveUsed: 4500, - planQuota: 5000, + bridgeClient.reportBucketUsage.mockResolvedValue({ + maxSpaceBytes: 5000, + totalUsedSpaceBytes: 5500, }); const result = await service.handleRcpt(buildRequest(['jane@inxt.com'])); @@ -126,7 +145,7 @@ describe('MtaHooksService', () => { }); it('when the address resolves to no internxt user, then skips it and accepts', async () => { - accountService.findUserIdByAddress.mockResolvedValue(null); + accountService.findRecipientContext.mockResolvedValue(null); const result = await service.handleRcpt( buildRequest(['external@gmail.com'], { size: 500 }), @@ -134,45 +153,54 @@ describe('MtaHooksService', () => { expect(result).toStrictEqual({ action: 'accept' }); expect(emailService.getQuota).not.toHaveBeenCalled(); - expect(bridgeClient.reportMailUsage).not.toHaveBeenCalled(); + expect(bridgeClient.reportBucketUsage).not.toHaveBeenCalled(); }); it('when the recipient address is upper-cased, then it is lowercased before resolution', async () => { - accountService.findUserIdByAddress.mockResolvedValue('user-1'); + accountService.findRecipientContext.mockResolvedValue({ + userUuid: 'user-1', + networkBucketId: 'bucket-1', + }); emailService.getQuota.mockResolvedValue({ used: 0, limit: 5000 }); - bridgeClient.reportMailUsage.mockResolvedValue({ - driveUsed: 0, - planQuota: 5000, + bridgeClient.reportBucketUsage.mockResolvedValue({ + maxSpaceBytes: 5000, + totalUsedSpaceBytes: 0, }); await service.handleRcpt(buildRequest(['Jane@INXT.com'], { size: 10 })); - expect(accountService.findUserIdByAddress).toHaveBeenCalledWith( + expect(accountService.findRecipientContext).toHaveBeenCalledWith( 'jane@inxt.com', ); expect(emailService.getQuota).toHaveBeenCalledWith('jane@inxt.com'); }); it('when several recipients are present, then only the current (last) one is evaluated', async () => { - accountService.findUserIdByAddress.mockResolvedValue('user-1'); + accountService.findRecipientContext.mockResolvedValue({ + userUuid: 'user-1', + networkBucketId: 'bucket-1', + }); emailService.getQuota.mockResolvedValue({ used: 0, limit: 5000 }); - bridgeClient.reportMailUsage.mockResolvedValue({ - driveUsed: 0, - planQuota: 5000, + bridgeClient.reportBucketUsage.mockResolvedValue({ + maxSpaceBytes: 5000, + totalUsedSpaceBytes: 0, }); await service.handleRcpt( buildRequest(['first@inxt.com', 'current@inxt.com']), ); - expect(accountService.findUserIdByAddress).toHaveBeenCalledTimes(1); - expect(accountService.findUserIdByAddress).toHaveBeenCalledWith( + expect(accountService.findRecipientContext).toHaveBeenCalledTimes(1); + expect(accountService.findRecipientContext).toHaveBeenCalledWith( 'current@inxt.com', ); }); it('when quota lookup throws, then fails open and accepts', async () => { - accountService.findUserIdByAddress.mockResolvedValue('user-1'); + accountService.findRecipientContext.mockResolvedValue({ + userUuid: 'user-1', + networkBucketId: 'bucket-1', + }); emailService.getQuota.mockRejectedValue(new Error('JMAP down')); const result = await service.handleRcpt(buildRequest(['jane@inxt.com'])); @@ -180,10 +208,26 @@ describe('MtaHooksService', () => { expect(result).toStrictEqual({ action: 'accept' }); }); + it('when recipient resolution throws (e.g. bucket provisioning fails), then fails open and accepts', async () => { + accountService.findRecipientContext.mockRejectedValue( + new Error('bucket provisioning failed'), + ); + + const result = await service.handleRcpt(buildRequest(['jane@inxt.com'])); + + expect(result).toStrictEqual({ action: 'accept' }); + expect(emailService.getQuota).not.toHaveBeenCalled(); + }); + it('when Bridge reporting throws, then fails open and accepts', async () => { - accountService.findUserIdByAddress.mockResolvedValue('user-1'); + accountService.findRecipientContext.mockResolvedValue({ + userUuid: 'user-1', + networkBucketId: 'bucket-1', + }); emailService.getQuota.mockResolvedValue({ used: 1000, limit: 5000 }); - bridgeClient.reportMailUsage.mockRejectedValue(new Error('bridge down')); + bridgeClient.reportBucketUsage.mockRejectedValue( + new Error('bridge down'), + ); const result = await service.handleRcpt(buildRequest(['jane@inxt.com'])); @@ -196,7 +240,7 @@ describe('MtaHooksService', () => { }); expect(result).toStrictEqual({ action: 'accept' }); - expect(accountService.findUserIdByAddress).not.toHaveBeenCalled(); + expect(accountService.findRecipientContext).not.toHaveBeenCalled(); }); }); }); diff --git a/src/modules/mta-hooks/mta-hooks.service.ts b/src/modules/mta-hooks/mta-hooks.service.ts index 83969ef..5486016 100644 --- a/src/modules/mta-hooks/mta-hooks.service.ts +++ b/src/modules/mta-hooks/mta-hooks.service.ts @@ -59,24 +59,23 @@ export class MtaHooksService { incomingSize: number, ): Promise { const address = rawAddress.toLowerCase(); - const userUuid = await this.accountService.findUserIdByAddress(address); - if (!userUuid) { + const context = await this.accountService.findRecipientContext(address); + if (!context) { return false; } + const { userUuid, networkBucketId } = context; const { used: mailUsed } = await this.emailService.getQuota(address); - const { driveUsed, planQuota } = await this.bridgeClient.reportMailUsage( - userUuid, - mailUsed, - ); - - const projectedUsage = driveUsed + mailUsed + incomingSize; - if (projectedUsage > planQuota) { - this.logger.warn( - `Rejecting recipient '${address}' (user '${userUuid}'): ` + - `projected ${projectedUsage} > quota ${planQuota}`, + const { maxSpaceBytes, totalUsedSpaceBytes } = + await this.bridgeClient.reportBucketUsage( + userUuid, + networkBucketId, + mailUsed, ); + + const projectedUsage = totalUsedSpaceBytes + incomingSize; + if (projectedUsage > maxSpaceBytes) { return true; }