Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
'use strict';

const TABLE_NAME = 'mail_addresses';

/** @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');
},
};
2 changes: 2 additions & 0 deletions src/modules/account/account.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -29,6 +30,7 @@ import { MailAddressKeysRepository } from './repositories/mail-address-keys.repo
]),
StalwartModule,
PaymentsModule,
BridgeModule,
],
controllers: [UserController],
providers: [
Expand Down
120 changes: 119 additions & 1 deletion src/modules/account/account.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -33,6 +34,7 @@ describe('AccountService', () => {
let addresses: DeepMocked<AddressRepository>;
let domains: DeepMocked<DomainRepository>;
let keys: DeepMocked<MailAddressKeysRepository>;
let bridge: DeepMocked<BridgeClient>;
let config: DeepMocked<ConfigService>;

beforeEach(async () => {
Expand All @@ -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);
});

Expand Down Expand Up @@ -364,20 +367,30 @@ describe('AccountService', () => {
}),
);

const bucket = { id: 'bucket-1', name: createdAddressId };
domains.findByDomain.mockResolvedValue(domain);
addresses.findByAddress.mockResolvedValue(null);
accounts.findByUserId
.mockResolvedValueOnce(null)
.mockResolvedValueOnce(provisionedAccount);
accounts.create.mockResolvedValue(createdAccount);
addresses.create.mockResolvedValue(createdAddressId);
bridge.createMailBucket.mockResolvedValue(bucket);

const result = await service.provisionAccount(params);

expect(result.userId).toBe(params.userId);
expect(accounts.create).toHaveBeenCalledWith({
userId: params.userId,
});
expect(bridge.createMailBucket).toHaveBeenCalledWith(
params.userId,
createdAddressId,
);
expect(addresses.setNetworkBucketId).toHaveBeenCalledWith(
createdAddressId,
bucket.id,
);
expect(addresses.create).toHaveBeenCalledWith({
mailAccountId: createdAccount.id,
address: params.address,
Expand Down Expand Up @@ -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(addresses.setNetworkBucketId).not.toHaveBeenCalled();
});

it('when concurrent provisioning race occurs, then returns the existing account', async () => {
const existingAccount = MailAccount.build(
newMailAccountAttributes({ userId: params.userId }),
Expand Down Expand Up @@ -506,6 +542,47 @@ describe('AccountService', () => {
expect(accounts.delete).toHaveBeenCalledWith(account.id);
});

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({ addresses: [addr] }),
);
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 addresses have no network bucket, then does not call the bridge', async () => {
const addr = newMailAddressAttributes({ networkBucketId: null });
const account = MailAccount.build(
newMailAccountAttributes({ addresses: [addr] }),
);
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 addr = newMailAddressAttributes({ networkBucketId: 'bucket-1' });
const account = MailAccount.build(
newMailAccountAttributes({ addresses: [addr] }),
);
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);

Expand All @@ -528,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,
Expand All @@ -554,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 () => {
Expand Down Expand Up @@ -625,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: [
Expand All @@ -645,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 () => {
Expand Down
46 changes: 46 additions & 0 deletions src/modules/account/account.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@ 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';
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';
Expand Down Expand Up @@ -41,6 +43,7 @@ export class AccountService {
private readonly addresses: AddressRepository,
private readonly domains: DomainRepository,
private readonly keys: MailAddressKeysRepository,
private readonly bridge: BridgeClient,
private readonly config: ConfigService,
) {}

Expand Down Expand Up @@ -214,6 +217,15 @@ export class AccountService {
throw error;
}

try {
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);
await this.accounts.delete(account.id);
throw error;
}

return this.getAccountOrFail(params.userId);
}

Expand All @@ -224,6 +236,7 @@ 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);
}),
);

Expand Down Expand Up @@ -279,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}'`);
}

Expand All @@ -303,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}'`);
}
Expand Down Expand Up @@ -370,6 +393,29 @@ export class AccountService {
}
}

private async createNetworkBucket(
userUuid: string,
addressId: string,
): Promise<void> {
const bucket = await this.bridge.createMailBucket(userUuid, addressId);
await this.addresses.setNetworkBucketId(addressId, bucket.id);
}

private async deleteNetworkBucket(
userUuid: string,
address: MailAddress,
): Promise<void> {
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<MailAccount> {
const account = await this.accounts.findByUserId(userId);
if (!account) {
Expand Down
2 changes: 2 additions & 0 deletions src/modules/account/domain/mail-address.domain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ export interface MailAddressAttributes {
domainId: string;
isDefault: boolean;
providerExternalId: string;
networkBucketId: string | null;
createdAt: Date;
updatedAt: Date;
}
Expand All @@ -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;

Expand Down
4 changes: 4 additions & 0 deletions src/modules/account/models/mail-address.model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
11 changes: 11 additions & 0 deletions src/modules/account/repositories/address.repository.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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' } },
);
});
});
});
5 changes: 5 additions & 0 deletions src/modules/account/repositories/address.repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -127,6 +128,10 @@ export class AddressRepository {
await this.addressModel.destroy({ where: { id } });
}

async setNetworkBucketId(id: string, networkBucketId: string): Promise<void> {
await this.addressModel.update({ networkBucketId }, { where: { id } });
}

async setDefault(addressId: string, mailAccountId: string): Promise<void> {
await this.sequelize.query(
`UPDATE mail_addresses SET is_default = (id = :addressId) WHERE mail_account_id = :mailAccountId`,
Expand Down
Loading
Loading