Skip to content

feat(admin): add domain blocklist sync from a list URL - #21098

Open
vbudhram wants to merge 1 commit into
mainfrom
fxa-13674
Open

feat(admin): add domain blocklist sync from a list URL#21098
vbudhram wants to merge 1 commit into
mainfrom
fxa-13674

Conversation

@vbudhram

Copy link
Copy Markdown
Contributor

Because

  • FXA-13596 shipped the domain blocklist panel and the auth-server enforcement, but not a seed path. Today an admin can only paste or upload domains by hand.
  • The public disposable-domain list holds about 40k entries. Nobody is going to paste that.

This pull request

  • Adds POST /api/domain-blocklist/sync. It fetches a list URL over https, parses it, and imports it through the existing DomainBlocklist.addMany().
  • Parses the list line by line. Blank lines and # comments are skipped, entries are normalized, invalid domains and repeats are dropped.
  • Imports in batches of 500 with a 50 ms pause between them. addMany ignores conflicts, so a domain that is already blocked does not error.
  • Adds an editable URL field to the panel, pre-filled with the disposable-email-domains list, plus a sync button and a result line.
  • Adds syncDomainBlocklist() to the panel API client and DomainBlocklistSyncResult to the admin-server types.

Issue that this pull request solves

Closes: https://mozilla-hub.atlassian.net/browse/FXA-13674

Checklist

Put an x in the boxes that apply

  • My commit is GPG signed.
  • If applicable, I have modified or added tests which pass locally.
  • I have added necessary documentation (if appropriate).
  • I have verified that my changes render correctly in RTL (if appropriate).
  • I have manually reviewed all AI generated code.

How to review (Optional)

  • Key files/areas to focus on: domain-blocklist.controller.ts, and inside it parseSyncUrl(), fetchList() and readCappedBody().
  • Suggested review order: the controller, then its spec, then the panel component and its test.
  • Risky or complex parts: the endpoint makes a server-side request to a URL the admin types in. Please read the open question below before you approve.

Screenshots (Optional)

None. The panel change is a URL field, a sync button and a one-line result under the existing add form. I could not take a screenshot because the local stack was not running for this change.

Other information (Optional)

  • Local runs: the admin-server domain-blocklist.controller.spec.ts is 27 passed, 0 failed, and the panel PageDomainBlocklist/index.test.tsx is 13 passed, 0 failed. nx lint passes for both packages.
  • The real 40k fetch is unverified. The sandbox has no outbound access to raw.githubusercontent.com, and a test that hits it would tie the suite to a third party. The import path is covered with a 1200 entry fixture that asserts the batch sizes. Please run one real import against a live instance.
  • Open question for the reviewer. The endpoint fetches an admin-supplied URL from inside our network, which is an SSRF surface. A Convict host allowlist would close it, but that conflicts with the reporter's requirement that the URL stays editable so a different list can be imported. For now the endpoint sits behind the admin auth header, the DomainBlocklist feature guard and the audit log, it accepts https only, and it caps the response body at 10MB.
  • The https check reads the final redirect hop only. fetch follows the whole chain before the check can run, so a middle hop over plaintext is still requested.
  • The periodic job is out of scope on purpose. The ticket asks whether another cloud task fits. That is still an open question and it is not in the acceptance criteria.

## Because

- FXA-13596 shipped the domain blocklist panel and the auth-server enforcement, but not a seed path. Today an admin can only paste or upload domains by hand.
- The public disposable-domain list holds about 40k entries. Nobody is going to paste that.

## This pull request

- Adds `POST /api/domain-blocklist/sync`. It fetches a list URL over https, parses it, and imports it through the existing `DomainBlocklist.addMany()`.
- Parses the list line by line. Blank lines and `#` comments are skipped, entries are normalized, invalid domains and repeats are dropped.
- Imports in batches of 500 with a 50 ms pause between them. `addMany` ignores conflicts, so a domain that is already blocked does not error.
- Adds an editable URL field to the panel, pre-filled with the disposable-email-domains list, plus a sync button and a result line.
- Adds `syncDomainBlocklist()` to the panel API client and `DomainBlocklistSyncResult` to the admin-server types.

## Issue that this pull request solves

Closes: https://mozilla-hub.atlassian.net/browse/FXA-13674
@vbudhram vbudhram added the auto label Aug 24, 2026
@vbudhram
vbudhram requested a review from a team as a code owner August 24, 2026 21:22
Copilot AI balanced review requested due to automatic review settings August 24, 2026 21:22
@vbudhram vbudhram added the auto label Aug 24, 2026
const abort = new AbortController();
const timer = setTimeout(() => abort.abort(), SYNC_FETCH_TIMEOUT_MS);
try {
const res = await fetch(source.toString(), { signal: abort.signal });

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds admin-driven domain blocklist synchronization from a remote URL.

Changes:

  • Adds HTTPS list fetching, parsing, validation, and batched imports.
  • Adds panel controls and API integration for synchronization.
  • Adds result types and server/UI tests.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
packages/fxa-admin-server/src/types.ts Defines the synchronization result.
packages/fxa-admin-server/src/rest/domain-blocklist/domain-blocklist.controller.ts Implements remote-list synchronization.
packages/fxa-admin-server/src/rest/domain-blocklist/domain-blocklist.controller.spec.ts Tests fetching, parsing, batching, and failures.
packages/fxa-admin-panel/src/lib/api.ts Adds the synchronization API client.
packages/fxa-admin-panel/src/components/PageDomainBlocklist/index.tsx Adds synchronization controls and results.
packages/fxa-admin-panel/src/components/PageDomainBlocklist/index.test.tsx Tests the new panel workflow.
Suppressed comments (1)

packages/fxa-admin-panel/src/components/PageDomainBlocklist/index.tsx:226

  • This asynchronously inserted result has no live-region semantics, so screen-reader users may not hear whether the sync succeeded or failed while focus remains on the button. Mark the message as a status (or use an alert for the error case).
      {syncResult && (
        <p data-testid="domain-blocklist-sync-result" className="mt-2">
          {syncResult}
        </p>

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

const abort = new AbortController();
const timer = setTimeout(() => abort.abort(), SYNC_FETCH_TIMEOUT_MS);
try {
const res = await fetch(source.toString(), { signal: abort.signal });
Comment on lines +12 to +13
const DEFAULT_SYNC_URL =
'https://raw.githubusercontent.com/disposable/disposable-email-domains/master/disposable_email_blocklist.conf';
@vbudhram

Copy link
Copy Markdown
Contributor Author

🤖 Blocked on a decision, not on a fix.

CodeQL flags a critical server-side request forgery at packages/fxa-admin-server/src/rest/domain-blocklist/domain-blocklist.controller.ts:172 — the fetch URL comes from a user-provided value.

This is inherent to the requirement. FXA-13674 asks for the list URL to be editable in the panel so a different list can be imported manually, and a server that fetches an admin-supplied URL is an SSRF. parseSyncUrl already enforces https, and fetchList adds an abort timeout, a capped body, and a final-hop protocol check, but there is no host allowlist, so an internal or link-local https target is still reachable.

Three defensible options, and picking one is a human call:

  • allowlist hosts in server config, with the panel choosing among allowed values
  • hardcode the single URL and drop configurability
  • accept the risk on the admin-panel auth boundary and dismiss the alert

The only other failure is a Pairing flow failed: Error while creating the pairing channel flake in the functional suite, unrelated to this PR. I did not rerun it, because any fix push re-runs everything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants