Skip to content
Open
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
40 changes: 40 additions & 0 deletions migrations/20260821202931-create-mail-deleted-addresses.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
'use strict';

const TABLE_NAME = 'mail_deleted_addresses';

/** @type {import('sequelize-cli').Migration} */
module.exports = {
async up(queryInterface, Sequelize) {
await queryInterface.createTable(TABLE_NAME, {
id: {
type: Sequelize.UUID,
defaultValue: Sequelize.UUIDV4,
primaryKey: true,
allowNull: false,
},
address: {
type: Sequelize.STRING(255),
allowNull: false,
unique: true,
},
user_id: {
type: Sequelize.UUID,
allowNull: false,
},
created_at: {
type: Sequelize.DATE,
allowNull: false,
defaultValue: Sequelize.fn('now'),
},
updated_at: {
type: Sequelize.DATE,
allowNull: false,
defaultValue: Sequelize.fn('now'),
},
});
},

async down(queryInterface) {
await queryInterface.dropTable(TABLE_NAME);
},
};
4 changes: 4 additions & 0 deletions src/modules/account/account.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,13 @@ import {
MailAddressKeysModel,
MailAccountModel,
MailAddressModel,
MailDeletedAddressModel,
MailDomainModel,
MailProviderAccountModel,
} from './models/index.js';
import { AccountRepository } from './repositories/account.repository.js';
import { AddressRepository } from './repositories/address.repository.js';
import { DeletedAddressRepository } from './repositories/deleted-address.repository.js';
import { DomainRepository } from './repositories/domain.repository.js';
import { MailAddressKeysRepository } from './repositories/mail-address-keys.repository.js';

Expand All @@ -26,6 +28,7 @@ import { MailAddressKeysRepository } from './repositories/mail-address-keys.repo
MailAccountModel,
MailAddressKeysModel,
MailAddressModel,
MailDeletedAddressModel,
MailDomainModel,
MailProviderAccountModel,
]),
Expand All @@ -37,6 +40,7 @@ import { MailAddressKeysRepository } from './repositories/mail-address-keys.repo
providers: [
AccountRepository,
AddressRepository,
DeletedAddressRepository,
DomainRepository,
MailAddressKeysRepository,
AccountService,
Expand Down
121 changes: 117 additions & 4 deletions src/modules/account/account.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { MailDomain } from './domain/mail-domain.domain.js';
import { MailAddress } from './domain/mail-address.domain.js';
import { AccountRepository } from './repositories/account.repository.js';
import { AddressRepository } from './repositories/address.repository.js';
import { DeletedAddressRepository } from './repositories/deleted-address.repository.js';
import { DomainRepository } from './repositories/domain.repository.js';
import { MailAddressKeysRepository } from './repositories/mail-address-keys.repository.js';
import {
Expand Down Expand Up @@ -50,6 +51,7 @@ describe('AccountService', () => {
let provider: DeepMocked<AccountProvider>;
let accounts: DeepMocked<AccountRepository>;
let addresses: DeepMocked<AddressRepository>;
let deletedAddresses: DeepMocked<DeletedAddressRepository>;
let domains: DeepMocked<DomainRepository>;
let keys: DeepMocked<MailAddressKeysRepository>;
let bridge: DeepMocked<BridgeClient>;
Expand All @@ -67,6 +69,7 @@ describe('AccountService', () => {
provider = module.get(AccountProvider);
accounts = module.get(AccountRepository);
addresses = module.get(AddressRepository);
deletedAddresses = module.get(DeletedAddressRepository);
domains = module.get(DomainRepository);
keys = module.get(MailAddressKeysRepository);
bridge = module.get(BridgeClient);
Expand All @@ -77,6 +80,7 @@ describe('AccountService', () => {
maxSpaceBytes: 1000,
totalUsedSpaceBytes: 0,
});
deletedAddresses.findClaimedByOthers.mockResolvedValue(new Set());
});

describe('getAccount', () => {
Expand Down Expand Up @@ -661,6 +665,20 @@ describe('AccountService', () => {
expect(provider.createAccount).not.toHaveBeenCalled();
});

it('when the address was given up by another user, then throws a conflict', async () => {
domains.findByDomain.mockResolvedValue(domain);
addresses.findByAddress.mockResolvedValue(null);
accounts.findByUserId.mockResolvedValue(null);
deletedAddresses.findClaimedByOthers.mockResolvedValue(
new Set([params.address]),
);

await expect(service.provisionAccount(params)).rejects.toThrow(
ConflictException,
);
expect(accounts.create).not.toHaveBeenCalled();
});

it('when unique collision occurs but no account is visible, then throws a conflict', async () => {
const uniqueError = new Error('Unique constraint violated');
uniqueError.name = 'SequelizeUniqueConstraintError';
Expand Down Expand Up @@ -816,6 +834,33 @@ describe('AccountService', () => {
expect(accounts.delete).not.toHaveBeenCalled();
});

it('when the account is destroyed, then every address is tombstoned first', async () => {
const addr1 = newMailAddressAttributes({ isDefault: true });
const addr2 = newMailAddressAttributes({ isDefault: false });
const account = MailAccount.build(
newMailAccountAttributes({ addresses: [addr1, addr2] }),
);
accounts.findByUserId.mockResolvedValue(account);

await service.deleteAccount(account.userId);

expect(deletedAddresses.record).toHaveBeenCalledWith([
{ address: addr1.address, userId: account.userId },
{ address: addr2.address, userId: account.userId },
]);
});

it('when tombstoning fails, then the rows are kept so the address stays taken', async () => {
const account = MailAccount.build(newMailAccountAttributes());
accounts.findByUserId.mockResolvedValue(account);
deletedAddresses.record.mockRejectedValue(new Error('DB down'));

await expect(service.deleteAccount(account.userId)).rejects.toThrow(
'DB down',
);
expect(accounts.delete).not.toHaveBeenCalled();
});

it('when account does not exist, then throws NotFoundException', async () => {
accounts.findByUserId.mockResolvedValue(null);

Expand Down Expand Up @@ -1191,6 +1236,25 @@ describe('AccountService', () => {
);
});

it('when an address is removed, then it is tombstoned so nobody else gets it', async () => {
const nonDefaultAddr = newMailAddressAttributes({ isDefault: false });
const account = MailAccount.build(
newMailAccountAttributes({
addresses: [
newMailAddressAttributes({ isDefault: true }),
nonDefaultAddr,
],
}),
);
accounts.findByUserId.mockResolvedValue(account);

await service.removeAddress(account.userId, nonDefaultAddr.address);

expect(deletedAddresses.record).toHaveBeenCalledWith([
{ address: nonDefaultAddr.address, userId: account.userId },
]);
});

it('when address is default, then throws UnprocessableEntityException', async () => {
const defaultAddr = newMailAddressAttributes({ isDefault: true });
const account = MailAccount.build(
Expand Down Expand Up @@ -1239,7 +1303,11 @@ describe('AccountService', () => {
});

it('when domain is available and address is not taken, return is available', async () => {
const res = await service.checkAddressAvailability('username', 'domain');
const res = await service.checkAddressAvailability(
'username',
'domain',
'user-1',
);

expect(res).toStrictEqual({ available: true, suggestion: null });

Expand All @@ -1252,7 +1320,11 @@ describe('AccountService', () => {
newMailDomainAttributes({ domain: 'domain2' }),
]);

const res = await service.checkAddressAvailability('username', 'domain');
const res = await service.checkAddressAvailability(
'username',
'domain',
'user-1',
);

expect(res).toStrictEqual({
available: false,
Expand All @@ -1267,7 +1339,11 @@ describe('AccountService', () => {
it('when address is taken, return is not available and suggestion', async () => {
addresses.findByAddresses.mockResolvedValue(new Set(['username@domain']));

const res = await service.checkAddressAvailability('username', 'domain');
const res = await service.checkAddressAvailability(
'username',
'domain',
'user-1',
);

expect(res).toStrictEqual({
available: false,
Expand All @@ -1276,10 +1352,47 @@ describe('AccountService', () => {
expect(addresses.findByAddresses).toHaveBeenCalledExactlyOnceWith(taken);
});

it('when an address was given up by another user, then it is not offered', async () => {
deletedAddresses.findClaimedByOthers.mockResolvedValue(
new Set(['username@domain']),
);

const res = await service.checkAddressAvailability(
'username',
'domain',
'user-1',
);

expect(deletedAddresses.findClaimedByOthers).toHaveBeenCalledWith(
taken,
'user-1',
);
expect(res).toStrictEqual({
available: false,
suggestion: 'username@domain1',
});
});

it('when the caller gave the address up themselves, then they may take it back', async () => {
deletedAddresses.findClaimedByOthers.mockResolvedValue(new Set());

const res = await service.checkAddressAvailability(
'username',
'domain',
'user-1',
);

expect(res).toStrictEqual({ available: true, suggestion: null });
});

it('when all suggestions are taken, return is not available and no suggestion', async () => {
addresses.findByAddresses.mockResolvedValue(new Set(taken));

const res = await service.checkAddressAvailability('username', 'domain');
const res = await service.checkAddressAvailability(
'username',
'domain',
'user-1',
);

expect(res).toStrictEqual({ available: false, suggestion: null });
expect(addresses.findByAddresses).toHaveBeenCalledExactlyOnceWith(taken);
Expand Down
54 changes: 41 additions & 13 deletions src/modules/account/account.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
AddressRepository,
type ProviderAccountBucketContext,
} from './repositories/address.repository.js';
import { DeletedAddressRepository } from './repositories/deleted-address.repository.js';
import { DomainRepository } from './repositories/domain.repository.js';
import { MailAddressKeysRepository } from './repositories/mail-address-keys.repository.js';

Expand All @@ -50,6 +51,7 @@ export class AccountService {
private readonly provider: AccountProvider,
private readonly accounts: AccountRepository,
private readonly addresses: AddressRepository,
private readonly deletedAddresses: DeletedAddressRepository,
private readonly domains: DomainRepository,
private readonly keys: MailAddressKeysRepository,
private readonly bridge: BridgeClient,
Expand Down Expand Up @@ -173,14 +175,24 @@ export class AccountService {
displayName: string;
keys: MailAddressKeyBundle;
}): Promise<MailAccount> {
const [tier, usage, domainRecord, existingAddress, existingAccount] =
await Promise.all([
this.payments.getUserTier(params.userId),
this.bridge.getUserUsage(params.userId),
this.domains.findByDomain(params.domain),
this.addresses.findByAddress(params.address),
this.accounts.findByUserId(params.userId),
]);
const [
tier,
usage,
domainRecord,
existingAddress,
existingAccount,
givenUp,
] = await Promise.all([
this.payments.getUserTier(params.userId),
this.bridge.getUserUsage(params.userId),
this.domains.findByDomain(params.domain),
this.addresses.findByAddress(params.address),
this.accounts.findByUserId(params.userId),
this.deletedAddresses.findClaimedByOthers(
[params.address],
params.userId,
),
]);

if (!tier.featuresPerService.mail?.enabled) {
throw new ForbiddenException(
Expand All @@ -193,7 +205,7 @@ export class AccountService {
if (existingAccount) {
throw new ConflictException('User already has a mail account');
}
if (existingAddress) {
if (existingAddress || givenUp.has(params.address)) {
throw new ConflictException(
`Address '${params.address}' is already in use`,
);
Expand Down Expand Up @@ -288,6 +300,14 @@ export class AccountService {
this.releaseNetworkBucket(driveUserUuid, a.networkBucketId),
),
);

await this.deletedAddresses.record(
account.addresses.map((a) => ({
address: a.address,
userId: driveUserUuid,
})),
);

await this.accounts.delete(account.id, { force: true });

this.logger.log(`Deleted account for user '${driveUserUuid}'`);
Expand All @@ -300,11 +320,12 @@ export class AccountService {
password: string,
displayName?: string,
): Promise<void> {
const [usage, account, domain, existing] = await Promise.all([
const [usage, account, domain, existing, givenUp] = await Promise.all([
this.bridge.getUserUsage(userId),
this.accounts.findByUserId(userId),
this.domains.findByDomain(domainName),
this.addresses.findByAddress(address),
this.deletedAddresses.findClaimedByOthers([address], userId),
]);

if (!account) {
Expand All @@ -313,7 +334,7 @@ export class AccountService {
if (!domain) {
throw new NotFoundException(`Domain '${domainName}' not found`);
}
if (existing) {
if (existing || givenUp.has(address)) {
throw new ConflictException(`Address '${address}' already exists`);
}

Expand Down Expand Up @@ -378,6 +399,7 @@ export class AccountService {
}

await this.provider.deleteAccount(addressRecord.providerExternalId);
await this.deletedAddresses.record([{ address, userId }]);
await Promise.all([
this.addresses.deleteProviderLink(addressRecord.id),
this.addresses.delete(addressRecord.id),
Expand Down Expand Up @@ -411,6 +433,7 @@ export class AccountService {
async checkAddressAvailability(
username: string,
domain: string,
userId: string,
): Promise<{ available: boolean; suggestion: string | null }> {
const activeMailDomains = await this.domains.findAllActive();
const activeDomains = activeMailDomains.map((m) => m.domain);
Expand Down Expand Up @@ -438,8 +461,13 @@ export class AccountService {
);
}

const taken = await this.addresses.findByAddresses(possibleAddresses);
const suggestion = possibleAddresses.find((a) => !taken.has(a));
const [taken, givenUp] = await Promise.all([
this.addresses.findByAddresses(possibleAddresses),
this.deletedAddresses.findClaimedByOthers(possibleAddresses, userId),
]);
const suggestion = possibleAddresses.find(
(a) => !taken.has(a) && !givenUp.has(a),
);

if (suggestion === requestedAddress) {
return { available: true, suggestion: null };
Expand Down
Loading
Loading