mirror of
https://github.com/HeyPuter/puter.git
synced 2026-10-11 22:32:26 +00:00
fix(email): one canonicalization and blocklist for every address path
Share invites, share notifications and app feedback checked addresses through EmailClient.clean/validate, a second copy of the rules in util/email.ts that stripped dots on iCloud, stripped `+tags` on every domain, and matched the blocklist case-sensitively. They now use util/email.ts: isBlockedEmail for the blocklist, abuseKey for the per-address notification budget, and cleanEmail for the invite digest key. EmailClient keeps transport and templates only. isBlockedEmail also lowercases the configured suffixes, so a mixed-case blocklist entry still matches.
This commit is contained in:
1 parent
d4589caefe
commit
41385cc327
10 files changed
+169
-207
No files matched your search
@@ -425,86 +425,3 @@ describe('EmailClient — share notification templates', () => {
|
||||
expect(html).toContain('href="https://puter.test"');
|
||||
});
|
||||
});
|
||||
|
||||
describe('EmailClient.clean', () => {
|
||||
const clean = (email: string) => startClient().clean(email);
|
||||
|
||||
it('strips subaddressing on ordinary domains', () => {
|
||||
expect(clean('person+tag@example.test')).toBe('person@example.test');
|
||||
});
|
||||
|
||||
it('ignores dots and subaddressing for gmail', () => {
|
||||
expect(clean('first.last+news@gmail.com')).toBe('firstlast@gmail.com');
|
||||
});
|
||||
|
||||
it('canonicalizes googlemail onto gmail', () => {
|
||||
expect(clean('first.last@googlemail.com')).toBe('firstlast@gmail.com');
|
||||
});
|
||||
|
||||
it('keeps yahoo subaddressing, which yahoo treats as distinct', () => {
|
||||
expect(clean('person-tag+x@yahoo.com')).toBe('person-tag+x@yahoo.com');
|
||||
});
|
||||
|
||||
it('ignores dots for icloud aliases', () => {
|
||||
expect(clean('first.last+tag@me.com')).toBe('firstlast@me.com');
|
||||
});
|
||||
|
||||
it('returns anything that is not an address unchanged', () => {
|
||||
expect(clean('not-an-email')).toBe('not-an-email');
|
||||
expect(clean('@nolocal.test')).toBe('@nolocal.test');
|
||||
});
|
||||
});
|
||||
|
||||
describe('EmailClient.validate', () => {
|
||||
it('waves everything through in a dev environment', async () => {
|
||||
const client = startClient({ env: 'dev' } as Partial<IConfig>);
|
||||
await expect(client.validate('anything@blocked.test')).resolves.toBe(
|
||||
true,
|
||||
);
|
||||
});
|
||||
|
||||
it('rejects a blocked domain suffix', async () => {
|
||||
const client = startClient({
|
||||
blockedEmailDomains: ['blocked.test'],
|
||||
} as unknown as Partial<IConfig>);
|
||||
|
||||
await expect(client.validate('person@blocked.test')).resolves.toBe(
|
||||
false,
|
||||
);
|
||||
await expect(client.validate('person@allowed.test')).resolves.toBe(
|
||||
true,
|
||||
);
|
||||
});
|
||||
|
||||
it('matches the blocklist against the cleaned address', async () => {
|
||||
const client = startClient({
|
||||
blockedEmailDomains: ['@gmail.com'],
|
||||
} as unknown as Partial<IConfig>);
|
||||
await expect(
|
||||
client.validate('first.last+tag@googlemail.com'),
|
||||
).resolves.toBe(false);
|
||||
});
|
||||
|
||||
it('lets a registered validator veto an address', async () => {
|
||||
const client = startClient();
|
||||
const seen: string[] = [];
|
||||
client.addValidator((email) => {
|
||||
seen.push(email);
|
||||
return !email.startsWith('spam');
|
||||
});
|
||||
|
||||
await expect(client.validate('spam+x@example.test')).resolves.toBe(
|
||||
false,
|
||||
);
|
||||
await expect(client.validate('fine@example.test')).resolves.toBe(true);
|
||||
// Validators always see the canonical form.
|
||||
expect(seen).toEqual(['spam@example.test', 'fine@example.test']);
|
||||
});
|
||||
|
||||
it('supports asynchronous validators', async () => {
|
||||
const client = startClient();
|
||||
client.addValidator(async (email) => email !== 'no@example.test');
|
||||
await expect(client.validate('no@example.test')).resolves.toBe(false);
|
||||
await expect(client.validate('yes@example.test')).resolves.toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -71,54 +71,12 @@ export interface SendMailOptions {
|
||||
headers?: Record<string, string>;
|
||||
}
|
||||
|
||||
export type EmailValidator = (email: string) => Promise<boolean> | boolean;
|
||||
|
||||
interface CompiledTemplate {
|
||||
subject: ReturnType<typeof template>;
|
||||
html: ReturnType<typeof template>;
|
||||
text?: ReturnType<typeof template>;
|
||||
}
|
||||
|
||||
// -- Clean-email rules ------------------------------------------------
|
||||
|
||||
type CleanRule = (parts: { local: string; domain: string }) => {
|
||||
local: string;
|
||||
domain: string;
|
||||
};
|
||||
|
||||
const CLEAN_RULES: Record<string, CleanRule> = {
|
||||
dots_dont_matter: ({ local, domain }) => ({
|
||||
local: local.replace(/\./g, ''),
|
||||
domain,
|
||||
}),
|
||||
remove_subaddressing: ({ local, domain }) => ({
|
||||
local: local.split('+')[0],
|
||||
domain,
|
||||
}),
|
||||
};
|
||||
|
||||
const PROVIDER_RULES: Record<string, { apply: string[]; skip: string[] }> = {
|
||||
gmail: { apply: ['dots_dont_matter'], skip: [] },
|
||||
icloud: { apply: ['dots_dont_matter'], skip: [] },
|
||||
yahoo: { apply: [], skip: ['remove_subaddressing'] },
|
||||
};
|
||||
|
||||
const DOMAIN_TO_PROVIDER: Record<string, string> = {
|
||||
'gmail.com': 'gmail',
|
||||
'googlemail.com': 'gmail',
|
||||
'yahoo.com': 'yahoo',
|
||||
'yahoo.co.uk': 'yahoo',
|
||||
'yahoo.ca': 'yahoo',
|
||||
'yahoo.com.au': 'yahoo',
|
||||
'icloud.com': 'icloud',
|
||||
'me.com': 'icloud',
|
||||
'mac.com': 'icloud',
|
||||
};
|
||||
|
||||
const DOMAIN_ALIASES: Record<string, string> = {
|
||||
'googlemail.com': 'gmail.com',
|
||||
};
|
||||
|
||||
// -- EmailClient ------------------------------------------------------
|
||||
|
||||
/**
|
||||
@@ -126,8 +84,8 @@ const DOMAIN_ALIASES: Record<string, string> = {
|
||||
*
|
||||
* - Template-based outbound mail (via `send`)
|
||||
* - Raw nodemailer passthrough (via `sendRaw`)
|
||||
* - Canonical-form normalization for dedup (via `clean`)
|
||||
* - Policy + extensible validation (via `validate`)
|
||||
*
|
||||
* Address normalization and the domain blocklist live in `util/email.ts`.
|
||||
*/
|
||||
export class EmailClient extends PuterClient {
|
||||
private transport: ReturnType<typeof nodemailer.createTransport> | null =
|
||||
@@ -135,7 +93,6 @@ export class EmailClient extends PuterClient {
|
||||
private compiledTemplates: Partial<
|
||||
Record<EmailTemplateName, CompiledTemplate>
|
||||
> = {};
|
||||
private validators: EmailValidator[] = [];
|
||||
|
||||
constructor(config: IConfig) {
|
||||
super(config);
|
||||
@@ -218,72 +175,6 @@ export class EmailClient extends PuterClient {
|
||||
});
|
||||
}
|
||||
|
||||
// -- Public API: clean / validate ---------------------------------
|
||||
|
||||
/**
|
||||
* Normalize an email to its canonical form for dedup comparisons. Applies
|
||||
* provider-specific rules (e.g. Gmail ignores dots in the local part) plus
|
||||
* generic subaddressing removal.
|
||||
*/
|
||||
clean(email: string): string {
|
||||
let [local, domain] = email.split('@');
|
||||
if (!local || !domain) return email;
|
||||
|
||||
if (DOMAIN_ALIASES[domain]) {
|
||||
domain = DOMAIN_ALIASES[domain];
|
||||
}
|
||||
|
||||
// Default: strip subaddressing on everything unless provider skips it
|
||||
const ruleNames = new Set<string>(['remove_subaddressing']);
|
||||
const provider = DOMAIN_TO_PROVIDER[domain];
|
||||
const rules = provider ? PROVIDER_RULES[provider] : undefined;
|
||||
|
||||
if (rules) {
|
||||
rules.apply.forEach((r) => ruleNames.add(r));
|
||||
rules.skip.forEach((r) => ruleNames.delete(r));
|
||||
}
|
||||
|
||||
let parts = { local, domain };
|
||||
for (const name of ruleNames) {
|
||||
parts = CLEAN_RULES[name](parts);
|
||||
}
|
||||
|
||||
return `${parts.local}@${parts.domain}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Check whether an email is allowed to be used. Checks domain blocklist
|
||||
* plus any registered validators (services can call `addValidator()` to
|
||||
* register custom policy hooks).
|
||||
*/
|
||||
async validate(email: string): Promise<boolean> {
|
||||
if (this.config.env === 'dev') return true;
|
||||
|
||||
const cleaned = this.clean(email);
|
||||
|
||||
const blocked = this.config.blockedEmailDomains;
|
||||
if (Array.isArray(blocked)) {
|
||||
for (const suffix of blocked) {
|
||||
if (cleaned.endsWith(suffix)) return false;
|
||||
}
|
||||
}
|
||||
|
||||
for (const validator of this.validators) {
|
||||
const ok = await validator(cleaned);
|
||||
if (!ok) return false;
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Register a custom validation hook. Services can call this during their
|
||||
* startup to veto specific emails (e.g. a disposable-email service).
|
||||
*/
|
||||
addValidator(fn: EmailValidator): void {
|
||||
this.validators.push(fn);
|
||||
}
|
||||
|
||||
// -- Internals ----------------------------------------------------
|
||||
|
||||
private defaultFrom(): string {
|
||||
|
||||
@@ -632,6 +632,29 @@ describe('AppFeedbackService owner email', () => {
|
||||
}
|
||||
});
|
||||
|
||||
it('stores without emailing an owner on a blocked domain, whatever its case', async () => {
|
||||
const send = mockEmailReady();
|
||||
const ownerId = await makeDeliverableOwner();
|
||||
await server.stores.user.update(ownerId, {
|
||||
email: `Owner-${ownerId}@Blocked.Example`,
|
||||
});
|
||||
await server.stores.user.invalidateById(ownerId);
|
||||
const app = await makeApp(ownerId, { feedbackEnabled: true });
|
||||
const userId = await makeUser();
|
||||
const prev = liveConfig().blockedEmailDomains;
|
||||
liveConfig().blockedEmailDomains = ['blocked.example'];
|
||||
try {
|
||||
await service.submit({ userId, app: app.name, message: 'hi' });
|
||||
} finally {
|
||||
liveConfig().blockedEmailDomains = prev;
|
||||
}
|
||||
|
||||
expect(send).not.toHaveBeenCalled();
|
||||
const rows = await feedbackRows(userId);
|
||||
expect(rows).toHaveLength(1);
|
||||
expect(Boolean(rows[0].email_sent)).toBe(false);
|
||||
});
|
||||
|
||||
it('stores but does not email past the per-app daily email cap', async () => {
|
||||
const send = mockEmailReady();
|
||||
const ownerId = await makeDeliverableOwner();
|
||||
|
||||
@@ -18,6 +18,7 @@
|
||||
*/
|
||||
|
||||
import { HttpError } from '../../core/http/HttpError.js';
|
||||
import { isBlockedEmail } from '../../util/email.js';
|
||||
import { PuterService } from '../types.js';
|
||||
|
||||
/**
|
||||
@@ -299,7 +300,8 @@ export class AppFeedbackService extends PuterService {
|
||||
) {
|
||||
return;
|
||||
}
|
||||
if (!(await this.clients.email.validate(owner.email))) return;
|
||||
if (isBlockedEmail(owner.email, this.config.blockedEmailDomains))
|
||||
return;
|
||||
|
||||
// Claim an email-cap slot *before* sending: flip email_sent, recount
|
||||
// with the claim included, and release the slot if a concurrent
|
||||
|
||||
@@ -41,6 +41,7 @@ import {
|
||||
sharedViewLink,
|
||||
} from './shareDeepLink';
|
||||
import { NOTIFY_FANOUT_CAP } from '../../stores/team/TeamStore';
|
||||
import { abuseKey, cleanEmail, isBlockedEmail } from '../../util/email.js';
|
||||
import { blocksAllShares, type ResolvedShare } from './ShareService';
|
||||
|
||||
/**
|
||||
@@ -603,9 +604,8 @@ export class ShareNotificationService extends PuterService {
|
||||
}
|
||||
|
||||
#addressKey(email: string): string {
|
||||
const canonical = this.clients.email.clean(email.trim().toLowerCase());
|
||||
return createHash('sha256')
|
||||
.update(canonical)
|
||||
.update(abuseKey(email.trim()))
|
||||
.digest('hex')
|
||||
.slice(0, 32);
|
||||
}
|
||||
@@ -670,8 +670,8 @@ export class ShareNotificationService extends PuterService {
|
||||
skipped('recipient has unsubscribed', { holderId });
|
||||
return;
|
||||
}
|
||||
if (!(await this.clients.email.validate(to))) {
|
||||
skipped('address refused by validate', { holderId });
|
||||
if (isBlockedEmail(to, this.config.blockedEmailDomains)) {
|
||||
skipped('address is on a blocked domain', { holderId });
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -1077,13 +1077,15 @@ export class ShareNotificationService extends PuterService {
|
||||
// Each address fails alone — one refused send must not cost the
|
||||
// next invitee their only channel.
|
||||
try {
|
||||
if (!(await this.clients.email.validate(to))) {
|
||||
skipped('invite address refused by validate', { to });
|
||||
if (isBlockedEmail(to, this.config.blockedEmailDomains)) {
|
||||
skipped('invite address is on a blocked domain', { to });
|
||||
continue;
|
||||
}
|
||||
const mayOpen = await this.#claimInviteEmail(issuerId, to);
|
||||
// Canonical, so a later share typed in another case or alias
|
||||
// joins this digest instead of losing to the spent budget.
|
||||
await this.#queueDigest(
|
||||
`invite:${to}`,
|
||||
`invite:${cleanEmail(to)}`,
|
||||
{ kind: 'invite', to },
|
||||
issuer,
|
||||
count,
|
||||
|
||||
@@ -5563,6 +5563,42 @@ describe('ShareService', () => {
|
||||
),
|
||||
).toBe(before);
|
||||
});
|
||||
|
||||
it('refuses an invite to a blocked domain whatever its case', async () => {
|
||||
const owner = await makeUser();
|
||||
const file = await makeFile(owner.user);
|
||||
const cfg = server.services.share.config as {
|
||||
blockedEmailDomains?: string[];
|
||||
};
|
||||
const prev = cfg.blockedEmailDomains;
|
||||
cfg.blockedEmailDomains = ['mailinator.com'];
|
||||
try {
|
||||
for (const email of [
|
||||
'temp@mailinator.com',
|
||||
'Temp@Mailinator.com',
|
||||
'temp@SUB.MAILINATOR.COM',
|
||||
]) {
|
||||
await expect(
|
||||
share(owner.actor, {
|
||||
uid: file.uuid,
|
||||
recipient: { email },
|
||||
mode: 'read',
|
||||
}),
|
||||
email,
|
||||
).rejects.toMatchObject({
|
||||
statusCode: 400,
|
||||
legacyCode: 'email_not_allowed',
|
||||
});
|
||||
expect(
|
||||
await server.stores.share.listPendingByEmail(
|
||||
email.toLowerCase(),
|
||||
),
|
||||
).toEqual([]);
|
||||
}
|
||||
} finally {
|
||||
cfg.blockedEmailDomains = prev;
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('claiming is safe against races and duplicates', () => {
|
||||
|
||||
@@ -32,6 +32,7 @@ import { isUniqueViolation } from '../../util/dbError.js';
|
||||
import {
|
||||
abuseKey,
|
||||
cleanEmail,
|
||||
isBlockedEmail,
|
||||
isProviderCanonicalized,
|
||||
isStorableEmail,
|
||||
} from '../../util/email.js';
|
||||
@@ -3151,7 +3152,7 @@ export class ShareService extends PuterService {
|
||||
// happens to the email.
|
||||
if (
|
||||
!isStorableEmail(email) ||
|
||||
!(await this.clients.email.validate(email))
|
||||
isBlockedEmail(email, this.config.blockedEmailDomains)
|
||||
) {
|
||||
throw new HttpError(400, 'invalid recipient email address', {
|
||||
legacyCode: 'email_not_allowed',
|
||||
|
||||
@@ -650,6 +650,81 @@ describe('share email', () => {
|
||||
expect(mail.html).not.toContain('?shared=');
|
||||
});
|
||||
|
||||
const mailToInbox = (address: string) =>
|
||||
sent.filter((mail) => mail.to.toLowerCase() === address.toLowerCase());
|
||||
|
||||
it("keys an invitee's digest and budget case-insensitively", async () => {
|
||||
const owner = env.users.user;
|
||||
const invitee = uninvitedAddress();
|
||||
const shouted = invitee.toUpperCase();
|
||||
const [first, second, third] = [
|
||||
await makeFile(owner, 'case-a'),
|
||||
await makeFile(owner, 'case-b'),
|
||||
await makeFile(owner, 'case-c'),
|
||||
];
|
||||
|
||||
await withDigestWindow(env, DIGEST_WINDOW_SECONDS, async () => {
|
||||
await shareWith(owner, invitee, [{ uid: first.uid }]);
|
||||
await sleep(SETTLE_MS);
|
||||
await shareWith(owner, shouted, [{ uid: second.uid }]);
|
||||
});
|
||||
|
||||
// One inbox, one digest, both files.
|
||||
const mail = await waitForMail({ to: invitee });
|
||||
await sleep(SETTLE_MS);
|
||||
expect(mailToInbox(invitee)).toHaveLength(1);
|
||||
expect(mail.html).toContain(first.name);
|
||||
expect(mail.html).toContain(second.name);
|
||||
|
||||
// The pair's quiet window holds whatever case the next share uses.
|
||||
const capitalized = `${invitee[0].toUpperCase()}${invitee.slice(1)}`;
|
||||
await shareWith(owner, capitalized, [{ uid: third.uid }]);
|
||||
await sleep(SETTLE_MS);
|
||||
expect(mailToInbox(invitee)).toHaveLength(1);
|
||||
}, 15_000);
|
||||
|
||||
it('folds gmail spellings of one invitee into one digest', async () => {
|
||||
const owner = env.users.admin;
|
||||
const local = `gm${uniqueSuffix()}`;
|
||||
// Gmail ignores dots and `+tag`: both are `${local}@gmail.com`.
|
||||
const tagged = `${local.slice(0, 4)}.${local.slice(4)}+news@gmail.com`;
|
||||
const plain = `${local}@gmail.com`;
|
||||
const first = await makeFile(owner, 'gmail-a');
|
||||
const second = await makeFile(owner, 'gmail-b');
|
||||
|
||||
await withDigestWindow(env, DIGEST_WINDOW_SECONDS, async () => {
|
||||
await shareWith(owner, tagged, [{ uid: first.uid }]);
|
||||
await sleep(SETTLE_MS);
|
||||
await shareWith(owner, plain, [{ uid: second.uid }]);
|
||||
});
|
||||
|
||||
const mail = await waitForMail({ to: tagged });
|
||||
await sleep(SETTLE_MS);
|
||||
expect(mailToInbox(plain)).toHaveLength(0);
|
||||
expect(mailToInbox(tagged)).toHaveLength(1);
|
||||
expect(mail.html).toContain(first.name);
|
||||
expect(mail.html).toContain(second.name);
|
||||
}, 15_000);
|
||||
|
||||
it('treats dotted and undotted iCloud addresses as two invitees', async () => {
|
||||
const owner = env.users.admin;
|
||||
const local = `ic${uniqueSuffix()}`;
|
||||
// Apple allocates the exact string: these are two mailboxes.
|
||||
const dotted = `${local.slice(0, 4)}.${local.slice(4)}@icloud.com`;
|
||||
const undotted = `${local}@icloud.com`;
|
||||
const first = await makeFile(owner, 'icloud-a');
|
||||
const second = await makeFile(owner, 'icloud-b');
|
||||
|
||||
await shareWith(owner, dotted, [{ uid: first.uid }]);
|
||||
expect((await waitForMail({ to: dotted })).html).toContain(first.name);
|
||||
|
||||
// Its own budget: the dotted address's window doesn't silence it.
|
||||
await shareWith(owner, undotted, [{ uid: second.uid }]);
|
||||
const mail = await waitForMail({ to: undotted });
|
||||
expect(mail.html).toContain(second.name);
|
||||
expect(mail.html).not.toContain(first.name);
|
||||
});
|
||||
|
||||
it('honors an account-wide unsubscribe, and offers the link to those who have not', async () => {
|
||||
const owner = env.users.user;
|
||||
const recipient = await signUpAndConfirm(uninvitedAddress());
|
||||
|
||||
@@ -120,6 +120,18 @@ describe('isBlockedEmail', () => {
|
||||
expect(
|
||||
isBlockedEmail('A.B+tag@MAILINATOR.com', ['mailinator.com']),
|
||||
).toBe(true);
|
||||
expect(
|
||||
isBlockedEmail('first.last+tag@googlemail.com', ['@gmail.com']),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it('ignores case in the configured suffixes too', () => {
|
||||
expect(isBlockedEmail('a@mailinator.com', ['Mailinator.COM'])).toBe(
|
||||
true,
|
||||
);
|
||||
expect(isBlockedEmail('a@Mailinator.com', ['Mailinator.com'])).toBe(
|
||||
true,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -18,8 +18,8 @@
|
||||
*/
|
||||
|
||||
/**
|
||||
* Email normalization + block-list check. Stateless — used by AuthController
|
||||
* (signup / change-email / save-account).
|
||||
* Email normalization + block-list check. Stateless; every path that matches,
|
||||
* keys or blocks on an address uses these rules.
|
||||
*
|
||||
* CleanEmail('foo.bar+tag@gmail.com') === 'foobar@gmail.com'
|
||||
* isBlockedEmail('temp@mailinator.com', ['mailinator.com']) === true
|
||||
@@ -170,7 +170,8 @@ export function isProviderCanonicalized(email: string): boolean {
|
||||
|
||||
/**
|
||||
* Returns true when the (cleaned) email matches any of the blocked domain
|
||||
* suffixes. Suffix-match so `mailinator.com` blocks `foo@bar.mailinator.com`.
|
||||
* suffixes. Suffix-match so `mailinator.com` blocks `foo@bar.mailinator.com`;
|
||||
* case-insensitive on both sides.
|
||||
*/
|
||||
export function isBlockedEmail(
|
||||
email: string,
|
||||
@@ -178,5 +179,7 @@ export function isBlockedEmail(
|
||||
): boolean {
|
||||
if (!blockedDomains || blockedDomains.length === 0) return false;
|
||||
const clean = cleanEmail(email);
|
||||
return blockedDomains.some((suffix) => clean.endsWith(suffix));
|
||||
return blockedDomains.some((suffix) =>
|
||||
clean.endsWith(suffix.toLowerCase()),
|
||||
);
|
||||
}
|
||||
Reference in new issue
Block a user