feat: block disposable email domains on signup and email change - #1969
feat: block disposable email domains on signup and email change#1969paustint wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a database-backed disposable-email domain blocklist and wires it into user flows where Jetstream will later send email to a user-chosen address (credentials signup + email change), with a syncable upstream list to protect Mailgun deliverability.
Changes:
- Add
BlockedEmailDomaintable +BlockedEmailDomainSourceenum (including seeded allowlist rows) and a cron-driven sync job to refresh LIST_SYNC entries from an upstream community list. - Enforce blocked-domain checks in credentials registration and authenticated email change; skip password-reset sends for blocked domains while keeping the endpoint response uniform.
- Add a shared AuthError type/message for blocked domains and targeted unit tests for the new domain-candidate logic and sync behavior.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| prisma/schema.prisma | Declares BlockedEmailDomain + enum and documents a non-unique User.email index to avoid drift. |
| prisma/migrations/20260817151003_add_blocked_email_domain/migration.sql | Creates the table/enum and seeds MANUAL allowlist rows. |
| package.json | Adds start scripts for the new blocked-email-domain sync cron task (incl. test mode). |
| libs/shared/constants/src/lib/shared-constants.ts | Adds user-facing copy for the new EmailDomainNotAllowed error type. |
| libs/auth/server/src/lib/email-change.db.service.ts | Rejects email changes to blocked domains. |
| libs/auth/server/src/lib/blocked-email-domain.db.service.ts | Implements domain candidate derivation + DB lookup (fail-open). |
| libs/auth/server/src/lib/auth.errors.ts | Introduces EmailDomainNotAllowed AuthError type. |
| libs/auth/server/src/lib/auth.db.service.ts | Applies blocked-domain rejection to credentials registration before the “already in use” branch. |
| libs/auth/server/src/lib/tests/email-change.db.service.spec.ts | Adds coverage for blocked-domain rejection on email change. |
| libs/auth/server/src/lib/tests/blocked-email-domain.db.service.spec.ts | Adds unit tests for candidate generation and fail-open lookup behavior. |
| libs/auth/server/src/index.ts | Exports the new blocked-domain DB service helpers. |
| apps/cron-tasks/src/utils/blocked-email-domain-sync.utils.ts | Implements upstream fetch/parse + batched add/remove sync semantics. |
| apps/cron-tasks/src/config/env-config.ts | Adds env override for the upstream list URL. |
| apps/cron-tasks/src/blocked-email-domain-sync.ts | Adds an executable entrypoint for the cron task. |
| apps/cron-tasks/src/tests/blocked-email-domain-sync.spec.ts | Adds unit tests for insert/delete semantics, allowlist preservation, and test mode. |
| apps/cron-tasks/project.json | Bundles the new cron entrypoint as an additional Nx build output. |
| apps/api/src/app/controllers/auth.controller.ts | Skips password-reset email sending for blocked domains while keeping uniform responses. |
Suppressed comments (1)
libs/auth/server/src/lib/auth.db.service.ts:1855
- The user-facing message for this AuthError is less actionable than the shared UI copy (AUTH_ERROR_MESSAGES.EmailDomainNotAllowed) and diverges from the email-change rejection copy. Since JSON callers receive err.message directly, this should include the guidance to use a permanent address and ideally match the shared constant to avoid inconsistent UX between JSON and redirect flows.
if (await isEmailDomainBlocked(email)) {
logger.warn({ email }, '[AUTH][REGISTER] Rejected registration from a blocked email domain');
throw new EmailDomainNotAllowed('Disposable email addresses are not supported');
}
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Went through Copilot's suppressed (low-confidence) comment — it was valid:
|
ea18725 to
23b84aa
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.
Suppressed comments (1)
apps/api/src/app/controllers/auth.controller.ts:992
- The new blocked-domain branch in requestPasswordReset is currently untested. Since this endpoint is intentionally uniform (anti-enumeration), it would be good to lock in that (1) blocked domains do not call sendPasswordReset/generatePasswordResetToken and (2) the handler still responds with the same JSON payload as other outcomes. There is already controller test infrastructure in apps/api/src/app/controllers/tests/auth.controller.spec.ts that mocks sendPasswordReset and @jetstream/auth/server, so this can be covered with a focused unit test.
try {
// Throwing here (rather than returning an error) keeps the response identical to every other
// outcome of this endpoint, which is the whole point of its design - a distinct
// "blocked domain" response would be a new differential signal on an endpoint that is
// deliberately uniform. The block is enforced at registration; this only stops us mailing the
// accounts that predate it, where the send would just bounce.
//
// Runs for every request, including unregistered addresses, so it adds no timing difference
// between the two paths.
if (await isEmailDomainBlocked(email)) {
throw new EmailDomainNotAllowed('Password reset skipped for a blocked email domain');
}
|
Went through Copilot's suppressed (low-confidence) comments — one was valid:
The earlier suppressed comment about the registration error message was already fixed — that throw now uses |
23b84aa to
bb48f0c
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 30 out of 30 changed files in this pull request and generated no new comments.
Suppressed comments (2)
libs/features/manage-permissions/src/usePermissionRecords.tsx:121
objectPermissionableSobjectsis built fromObjectPermissions.SobjectTypepicklist values, but it doesn’t filter out entries explicitly marked inactive (active: false). That can incorrectly treat an inactive/unsupported object as supporting object-level permissions, enabling edits that will later fail to save withINVALID_OR_NULL_FOR_RESTRICTED_PICKLIST.
Filter the picklist values the same way getPermissionAllowList() does (keep values where active !== false).
const objectPermissionableSobjects = new Set<string>(
objectPermissionMetadata.data.fields.find((field) => field.name === 'SobjectType')?.picklistValues?.map(({ value }) => value),
);
libs/features/manage-permissions/src/utils/permission-manager-utils.ts:67
- The PR title/description focus on blocking disposable email domains on auth flows, but this diff also includes a fairly substantial Permission Manager behavior change (new allow-list unioning of ObjectPermissions + FieldPermissions, new UI handling for field-only objects, and associated tests). This extra scope isn’t mentioned in the PR description, which makes it easy for reviewers to miss or for the PR to be harder to reason about.
Consider either (a) updating the PR description/title to explicitly call out the Permission Manager fix, or (b) splitting the permission-manager changes into a separate PR.
/**
* The objects the permission manager can manage, split by what each one actually supports.
*
* Salesforce models object CRUD and field-level security as two independent allow-lists. Neither is a
* subset of the other: `PricebookEntry`, `Task`, `Event`, `OpportunityLineItem`, `OrderItem` and `User`
* accept `FieldPermissions` but have no `ObjectPermissions` record, because their record-level access
* rolls up to a parent object.
*/
Attackers were registering with burner addresses to trigger password reset emails, which bounced and damaged our Mailgun sending reputation. This is a deliverability control rather than a security one - anyone determined can register a throwaway domain - so it is never the only protection on a path. The full blocklist lives in the new BlockedEmailDomain table and is refreshed from the community disposable-domains list by a new blocked-email-domain-sync cron task, so nothing is held in memory and adding a domain by hand does not require a deploy. The sync only ever deletes LIST_SYNC rows and never rewrites an existing domain, so a MANUAL decision in either direction survives the next run. Rows with blocked = false are allowlist entries, needed because public lists tend to include legitimate forwarding services like Apple Hide My Email and Firefox Relay. The rule is to block wherever a user freely chooses an address we then mail: credentials registration and email change both hard reject. OAuth/SSO paths are exempt because the address comes from a verified identity provider and a block there would be a dead end for the user. Registration checks the domain before the already-in-use branch, which deliberately hides whether an account exists - rejecting only unregistered blocked addresses would have turned that branch into an enumeration oracle. Password reset skips the send instead of rejecting it, keeping the response identical to every other outcome of that deliberately uniform endpoint. The check runs for every request, so it adds no timing difference between registered and unregistered addresses. Existing accounts on disposable domains are otherwise untouched, and the authenticated self-service reset is not gated. Lookups fail open: a database problem must not stop people signing up, and every caller needs the database to succeed anyway.
2014ce4 to
4fd74c0
Compare
Attackers were registering with burner addresses to trigger password reset emails, which bounced and damaged our Mailgun sending reputation. This is a deliverability control rather than a security one - anyone determined can register a throwaway domain - so it is never the only protection on a path.
The full blocklist lives in the new BlockedEmailDomain table and is refreshed from the community disposable-domains list by a new blocked-email-domain-sync cron task, so nothing is held in memory and adding a domain by hand does not require a deploy. The sync only ever deletes LIST_SYNC rows and never rewrites an existing domain, so a MANUAL decision in either direction survives the next run. Rows with blocked = false are allowlist entries, needed because public lists tend to include legitimate forwarding services like Apple Hide My Email and Firefox Relay.
The rule is to block wherever a user freely chooses an address we then mail: credentials registration and email change both hard reject. OAuth/SSO paths are exempt because the address comes from a verified identity provider and a block there would be a dead end for the user.
Registration checks the domain before the already-in-use branch, which deliberately hides whether an account exists - rejecting only unregistered blocked addresses would have turned that branch into an enumeration oracle.
Password reset skips the send instead of rejecting it, keeping the response identical to every other outcome of that deliberately uniform endpoint. The check runs for every request, so it adds no timing difference between registered and unregistered addresses. Existing accounts on disposable domains are otherwise untouched, and the authenticated self-service reset is not gated.
Lookups fail open: a database problem must not stop people signing up, and every caller needs the database to succeed anyway.