diff --git a/migrations/20260611120000-add-provider-internal-id-to-mail-provider-accounts.js b/migrations/20260611120000-add-provider-internal-id-to-mail-provider-accounts.js new file mode 100644 index 0000000..09c996d --- /dev/null +++ b/migrations/20260611120000-add-provider-internal-id-to-mail-provider-accounts.js @@ -0,0 +1,31 @@ +'use strict'; + +const INDEX_NAME = 'mail_provider_accounts_unique_provider_internal_id'; + +/** @type {import('sequelize-cli').Migration} */ +module.exports = { + async up(queryInterface, Sequelize) { + await queryInterface.addColumn( + 'mail_provider_accounts', + 'provider_internal_id', + { + type: Sequelize.STRING(255), + allowNull: false, + }, + ); + + await queryInterface.sequelize.query( + `CREATE UNIQUE INDEX ${INDEX_NAME} + ON mail_provider_accounts (provider, provider_internal_id) + WHERE deleted_at IS NULL`, + ); + }, + + async down(queryInterface) { + await queryInterface.sequelize.query(`DROP INDEX IF EXISTS ${INDEX_NAME}`); + await queryInterface.removeColumn( + 'mail_provider_accounts', + 'provider_internal_id', + ); + }, +}; diff --git a/src/modules/account/account-provider.port.ts b/src/modules/account/account-provider.port.ts index 1431179..f463bda 100644 --- a/src/modules/account/account-provider.port.ts +++ b/src/modules/account/account-provider.port.ts @@ -1,7 +1,13 @@ -import type { AccountInfo, CreateAccountParams } from './account.types.js'; +import type { + AccountInfo, + CreateAccountParams, + CreateAccountResult, +} from './account.types.js'; export abstract class AccountProvider { - abstract createAccount(params: CreateAccountParams): Promise; + abstract createAccount( + params: CreateAccountParams, + ): Promise; abstract deleteAccount(name: string): Promise; abstract getAccount(name: string): Promise; } diff --git a/src/modules/account/account.service.spec.ts b/src/modules/account/account.service.spec.ts index 0161e1f..388873f 100644 --- a/src/modules/account/account.service.spec.ts +++ b/src/modules/account/account.service.spec.ts @@ -371,6 +371,11 @@ describe('AccountService', () => { .mockResolvedValueOnce(provisionedAccount); accounts.create.mockResolvedValue(createdAccount); addresses.create.mockResolvedValue(createdAddressId); + provider.createAccount.mockResolvedValue({ + provider: 'stalwart', + externalId: params.address, + internalId: '42', + }); const result = await service.provisionAccount(params); @@ -388,6 +393,7 @@ describe('AccountService', () => { mailAddressId: createdAddressId, provider: 'stalwart', externalId: params.address, + providerInternalId: '42', }); expect(provider.createAccount).toHaveBeenCalledWith( expect.objectContaining({ @@ -461,6 +467,32 @@ describe('AccountService', () => { 'Stalwart down', ); expect(accounts.delete).toHaveBeenCalledWith(createdAccount.id); + expect(addresses.createProviderLink).not.toHaveBeenCalled(); + }); + + it('when provider link creation fails, then deletes the stalwart account and the 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'); + provider.createAccount.mockResolvedValue({ + provider: 'stalwart', + externalId: params.address, + internalId: '42', + }); + addresses.createProviderLink.mockRejectedValue(new Error('DB down')); + + await expect(service.provisionAccount(params)).rejects.toThrow('DB down'); + expect(provider.deleteAccount).toHaveBeenCalledWith(params.address); + expect(accounts.delete).toHaveBeenCalledWith(createdAccount.id); }); it('when concurrent provisioning race occurs, then returns the existing account', async () => { @@ -528,6 +560,11 @@ describe('AccountService', () => { domains.findByDomain.mockResolvedValue(domain); addresses.findByAddress.mockResolvedValue(null); addresses.create.mockResolvedValue(newAddressId); + provider.createAccount.mockResolvedValue({ + provider: 'stalwart', + externalId: newAddr, + internalId: '7', + }); await service.addAddress( accountAttrs.userId, @@ -553,6 +590,7 @@ describe('AccountService', () => { mailAddressId: newAddressId, provider: 'stalwart', externalId: newAddr, + providerInternalId: '7', }); }); @@ -621,6 +659,31 @@ describe('AccountService', () => { expect(addresses.delete).toHaveBeenCalledWith(newAddressId); expect(addresses.createProviderLink).not.toHaveBeenCalled(); }); + + it('when provider link creation fails, then deletes the stalwart account and the address (undo) and rethrows', 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); + provider.createAccount.mockResolvedValue({ + provider: 'stalwart', + externalId: newAddr, + internalId: '7', + }); + addresses.createProviderLink.mockRejectedValue(new Error('DB down')); + + await expect( + service.addAddress(account.userId, newAddr, domain.domain, 'pass'), + ).rejects.toThrow('DB down'); + + expect(provider.deleteAccount).toHaveBeenCalledWith(newAddr); + expect(addresses.delete).toHaveBeenCalledWith(newAddressId); + }); }); describe('removeAddress', () => { diff --git a/src/modules/account/account.service.ts b/src/modules/account/account.service.ts index 7e0cee5..021bf2a 100644 --- a/src/modules/account/account.service.ts +++ b/src/modules/account/account.service.ts @@ -10,6 +10,7 @@ import { ConfigService } from '@nestjs/config'; import dayjs from 'dayjs'; import { MailNotSetupException } from '../provisioning/mail-not-setup.exception.js'; import { AccountProvider } from './account-provider.port.js'; +import type { CreateAccountResult } from './account.types.js'; import { MailAccount, MailAccountState } from './domain/mail-account.domain.js'; import { MailDomain } from './domain/mail-domain.domain.js'; import { AccountRepository } from './repositories/account.repository.js'; @@ -189,12 +190,6 @@ export class AccountService { isDefault: true, }); - await this.addresses.createProviderLink({ - mailAddressId: addressId, - provider: 'stalwart', - externalId: params.address, - }); - await this.keys.create({ mailAddressId: addressId, ...params.keys, @@ -202,8 +197,9 @@ export class AccountService { const password = randomBytes(32).toString('base64url'); + let created: CreateAccountResult; try { - await this.provider.createAccount({ + created = await this.provider.createAccount({ accountId: account.id, primaryAddress: params.address, displayName: params.displayName, @@ -214,6 +210,19 @@ export class AccountService { throw error; } + try { + await this.addresses.createProviderLink({ + mailAddressId: addressId, + provider: created.provider, + externalId: created.externalId, + providerInternalId: created.internalId, + }); + } catch (error) { + await this.provider.deleteAccount(created.externalId); + await this.accounts.delete(account.id); + throw error; + } + return this.getAccountOrFail(params.userId); } @@ -261,8 +270,9 @@ export class AccountService { isDefault: false, }); + let created: CreateAccountResult; try { - await this.provider.createAccount({ + created = await this.provider.createAccount({ accountId: newAddressId, primaryAddress: address, displayName: displayName ?? '', @@ -273,11 +283,18 @@ export class AccountService { throw error; } - await this.addresses.createProviderLink({ - mailAddressId: newAddressId, - provider: 'stalwart', - externalId: address, - }); + try { + await this.addresses.createProviderLink({ + mailAddressId: newAddressId, + provider: created.provider, + externalId: created.externalId, + providerInternalId: created.internalId, + }); + } catch (error) { + await this.provider.deleteAccount(created.externalId); + await this.addresses.delete(newAddressId); + throw error; + } this.logger.log(`Added address '${address}' to account '${userId}'`); } diff --git a/src/modules/account/account.types.ts b/src/modules/account/account.types.ts index 4fb8f02..bdf7f2b 100644 --- a/src/modules/account/account.types.ts +++ b/src/modules/account/account.types.ts @@ -6,6 +6,12 @@ export interface CreateAccountParams { quota?: number; } +export interface CreateAccountResult { + provider: string; + externalId: string; + internalId: string; +} + export interface AccountInfo { name: string; displayName: string; diff --git a/src/modules/account/models/mail-provider-account.model.ts b/src/modules/account/models/mail-provider-account.model.ts index 9219e33..26a41ee 100644 --- a/src/modules/account/models/mail-provider-account.model.ts +++ b/src/modules/account/models/mail-provider-account.model.ts @@ -39,6 +39,10 @@ export class MailProviderAccountModel extends Model { @Column(DataType.STRING(255)) declare externalId: string; + @AllowNull(false) + @Column(DataType.STRING(255)) + declare providerInternalId: string; + @Column(DataType.DATE) declare deletedAt: Date | null; diff --git a/src/modules/account/repositories/address.repository.ts b/src/modules/account/repositories/address.repository.ts index 7556aae..34bf02e 100644 --- a/src/modules/account/repositories/address.repository.ts +++ b/src/modules/account/repositories/address.repository.ts @@ -138,6 +138,7 @@ export class AddressRepository { mailAddressId: string; provider: string; externalId: string; + providerInternalId: string; }): Promise { await this.providerAccountModel.create(params); } diff --git a/src/modules/infrastructure/stalwart/stalwart-account.provider.spec.ts b/src/modules/infrastructure/stalwart/stalwart-account.provider.spec.ts index 2831ad4..7056013 100644 --- a/src/modules/infrastructure/stalwart/stalwart-account.provider.spec.ts +++ b/src/modules/infrastructure/stalwart/stalwart-account.provider.spec.ts @@ -26,6 +26,7 @@ describe('StalwartAccountProvider', () => { primaryAddress: 'alice@example.com', }); stalwart.resolveDomainId.mockResolvedValue('dom1'); + stalwart.createAccount.mockResolvedValue('c'); await provider.createAccount(params); @@ -39,6 +40,22 @@ describe('StalwartAccountProvider', () => { }); }); + it('when the account is created, then returns the decoded numeric stalwart id', async () => { + const params = newCreateAccountParams({ + primaryAddress: 'alice@example.com', + }); + stalwart.resolveDomainId.mockResolvedValue('dom1'); + stalwart.createAccount.mockResolvedValue('ba'); + + const result = await provider.createAccount(params); + + expect(result).toEqual({ + provider: 'stalwart', + externalId: 'alice@example.com', + internalId: '32', + }); + }); + it('when domain is not configured, then throws and does not create', async () => { const params = newCreateAccountParams({ primaryAddress: 'alice@unknown.com', @@ -57,6 +74,7 @@ describe('StalwartAccountProvider', () => { quota: undefined, }); stalwart.resolveDomainId.mockResolvedValue('dom1'); + stalwart.createAccount.mockResolvedValue('c'); await provider.createAccount(params); diff --git a/src/modules/infrastructure/stalwart/stalwart-account.provider.ts b/src/modules/infrastructure/stalwart/stalwart-account.provider.ts index 5e2fe4c..1d7803d 100644 --- a/src/modules/infrastructure/stalwart/stalwart-account.provider.ts +++ b/src/modules/infrastructure/stalwart/stalwart-account.provider.ts @@ -3,7 +3,9 @@ import { AccountProvider } from '../../account/account-provider.port.js'; import type { AccountInfo, CreateAccountParams, + CreateAccountResult, } from '../../account/account.types.js'; +import { decodeStalwartId } from './stalwart-id.codec.js'; import { StalwartApiError, StalwartService, @@ -18,7 +20,9 @@ export class StalwartAccountProvider extends AccountProvider { super(); } - async createAccount(params: CreateAccountParams): Promise { + async createAccount( + params: CreateAccountParams, + ): Promise { const { local, domain } = splitEmail(params.primaryAddress); const domainId = await this.stalwart.resolveDomainId(domain); if (!domainId) { @@ -28,15 +32,23 @@ export class StalwartAccountProvider extends AccountProvider { ); } - await this.stalwart.createAccount({ + const id = await this.stalwart.createAccount({ name: local, domainId, description: params.displayName, password: params.password, quotaBytes: params.quota ?? 0, }); + const internalId = decodeStalwartId(id); - this.logger.log(`Created account '${params.primaryAddress}'`); + this.logger.log( + `Created account '${params.primaryAddress}' (stalwart id ${internalId})`, + ); + return { + provider: 'stalwart', + externalId: params.primaryAddress, + internalId: String(internalId), + }; } async deleteAccount(email: string): Promise { diff --git a/src/modules/infrastructure/stalwart/stalwart-id.codec.spec.ts b/src/modules/infrastructure/stalwart/stalwart-id.codec.spec.ts new file mode 100644 index 0000000..1d66eea --- /dev/null +++ b/src/modules/infrastructure/stalwart/stalwart-id.codec.spec.ts @@ -0,0 +1,39 @@ +import { describe, expect, it } from 'vitest'; +import { decodeStalwartId } from './stalwart-id.codec.js'; + +describe('decodeStalwartId', () => { + it('when given single-character ids, then decodes alphabet positions', () => { + expect(decodeStalwartId('a')).toBe(0); + expect(decodeStalwartId('b')).toBe(1); + expect(decodeStalwartId('z')).toBe(25); + expect(decodeStalwartId('7')).toBe(26); + expect(decodeStalwartId('9')).toBe(27); + expect(decodeStalwartId('2')).toBe(28); + expect(decodeStalwartId('0')).toBe(29); + expect(decodeStalwartId('1')).toBe(30); + expect(decodeStalwartId('3')).toBe(31); + }); + + it('when given multi-character ids, then decodes most significant digit first', () => { + expect(decodeStalwartId('ba')).toBe(32); + expect(decodeStalwartId('bb')).toBe(33); + expect(decodeStalwartId('baa')).toBe(1024); + expect(decodeStalwartId('d3')).toBe(3 * 32 + 31); + }); + + it('when given an empty string, then throws', () => { + expect(() => decodeStalwartId('')).toThrow('empty'); + }); + + it('when given a character outside the alphabet, then throws', () => { + expect(() => decodeStalwartId('A')).toThrow("Invalid character 'A'"); + expect(() => decodeStalwartId('b4')).toThrow("Invalid character '4'"); + }); + + it('when the value exceeds the safe integer range, then throws', () => { + // 13 base32 digits ≈ 2^65 — above Number.MAX_SAFE_INTEGER (2^53 - 1) + expect(() => decodeStalwartId('3333333333333')).toThrow( + 'exceeds safe integer range', + ); + }); +}); diff --git a/src/modules/infrastructure/stalwart/stalwart-id.codec.ts b/src/modules/infrastructure/stalwart/stalwart-id.codec.ts new file mode 100644 index 0000000..20875c3 --- /dev/null +++ b/src/modules/infrastructure/stalwart/stalwart-id.codec.ts @@ -0,0 +1,29 @@ +// Must match Stalwart's BASE32_ALPHABET (crates/utils/src/codec/base32_custom.rs): +// every JMAP id Stalwart returns (accounts, domains, emails) is this custom +// base32 encoding of an unsigned integer, most significant digit first. +const STALWART_BASE32_ALPHABET = 'abcdefghijklmnopqrstuvwxyz792013'; + +const CHAR_VALUES = new Map( + [...STALWART_BASE32_ALPHABET].map((char, index) => [char, BigInt(index)]), +); + +export function decodeStalwartId(id: string): number { + if (id.length === 0) { + throw new Error('Cannot decode empty Stalwart id'); + } + + let value = 0n; + for (const char of id) { + const digit = CHAR_VALUES.get(char); + if (digit === undefined) { + throw new Error(`Invalid character '${char}' in Stalwart id '${id}'`); + } + value = value * 32n + digit; + } + + if (value > BigInt(Number.MAX_SAFE_INTEGER)) { + throw new Error(`Stalwart id '${id}' exceeds safe integer range`); + } + + return Number(value); +}