Skip to content

Phase 1: Sending Foundation — Integrations-backed email credentials, per-integration delivery, V143 - #633

Merged
ddon merged 15 commits into
BeamLabEU:mainfrom
timujinne:feature/newsletters-sending-foundation
Jul 13, 2026
Merged

Phase 1: Sending Foundation — Integrations-backed email credentials, per-integration delivery, V143#633
ddon merged 15 commits into
BeamLabEU:mainfrom
timujinne:feature/newsletters-sending-foundation

Conversation

@timujinne

Copy link
Copy Markdown
Contributor

Phase 1 "Sending Foundation" — core half

Moves all email credentials into PhoenixKit.Integrations (encrypted, keys only) and adds a delivery path that can send through any configured integration. This is the core half of a three-repo change (see Merge order below).

What's in here

Integration providers (integrations/providers.ex)

  • aws_ses (:key_secret) — access key / secret / region.
  • smtp (:credentials) — one universal SMTP provider: any vendor is just a named connection ("SMTP 1", "Brevo SMTP", …). Multiple connections of the same provider coexist.
  • brevo_api (:api_key) — Brevo transactional API, with a real validation map (GET /v3/account) so Test Connection actually checks the key.

Credential gate (integrations/integrations.ex)

  • has_flat_credential_fields?/2:credentials providers store fields flat, so the old nested-"credentials"-map-only gate left them permanently "disconnected" and get_credentials/1 failed. Now data-driven off each provider's own required: true fields (numeric fields included; empty required-list guarded).

Encryption (integrations/encryption.ex)

  • ⚠️ Behavior change / security fix. encryption_key/0 read only the flat :phoenix_kit, :secret_key_base, which the installer never sets — so in real host apps enabled?() was false and every integration secret was stored in plaintext. Now falls back to the host app's Endpoint secret_key_base (via Config.get_parent_endpoint/0); the flat key keeps precedence, so existing installs derive the identical key. Pre-existing plaintext values still read back fine (only enc:v1:-prefixed values are decrypted) and are re-encrypted on next save.
  • "password" added to @sensitive_fields (SMTP passwords now encrypt).
  • Docs: the KDF is a single SHA-256, not PBKDF2 (was mis-documented); rotating secret_key_base makes existing enc:v1: blobs undecryptable — there is no re-wrap path.

Mailer (mailer.ex)

  • deliver_via_integration/3 + swoosh_config_for/1 — builds the Swoosh adapter/config from an integration's decrypted credentials (SES / SMTP / Brevo) and replicates the Email.Provider seam directly (intercept_before_send → deliver → handle_after_send). It deliberately does not go through deliver_email/2, whose runtime-config path is SES-only. It injects provider: into the tracking opts so integration sends aren't mis-attributed to the host's static mailer adapter.
  • SMTP transport: 465 → ssl: true (implicit TLS). gen_smtp picks the protocol solely from the ssl option — tls: :always on 465 opens a plaintext socket to an SMTPS port and hangs. Other ports → tls: :always when credentials are present (fail closed, no STARTTLS-strip downgrade), :if_available when there are none (local relays like MailHog:1025). An unparseable port is rejected instead of silently becoming gen_smtp's default 25.
  • ⚠️ Behavior change. Recipients blocklisted by the optional emails module (to/cc/bcc) are now refused before any tracking or delivery, on both delivery paths. This covers all app mail, so a user whose address was auto-blocklisted (hard bounce) can no longer receive e.g. a magic link — which is the intent. Soft dependency: no-op when the emails package isn't installed. Send-rate limits are deliberately not enforced here (they are un-gated and would cap bulk sends / throttle auth mail); pacing belongs to the newsletters send profiles.

Migration V143phoenix_kit_newsletters_send_profiles + broadcasts.send_profile_uuid. Idempotent, prefix-aware, TIMESTAMPTZ, partial unique index enforcing a single default profile. @current_version 142 → 143.

Verification

  • mix test test/phoenix_kit/{mailer_test.exs,integrations,migrations}158 tests, 0 failures; --warnings-as-errors, credo --strict, dialyzer clean (pre-commit hook ran on every commit).
  • Live on a real app (Hydroforce dev): SES-via-Integrations send after the legacy plaintext creds were blanked; Brevo-API send; universal-SMTP (Brevo relay) send; V143 applied 142 → 143; blocklisted recipient refused on both delivery paths and delivered again after removal.
  • Reviewed by two independent GLM-5.2 agents (architecture + code review); every finding is either fixed here or listed below.

Known follow-ups (not blockers)

  • aws_ses and smtp still have no validationTest Connection returns OK without checking them (SigV4 / a real SMTP handshake are needed). Only brevo_api validates for real.
  • Rotating secret_key_base silently breaks decryption of all integration secrets; no re-wrap tooling yet.

⚠️ Merge order (hard dependency)

This PR must merge and be published to hex FIRST. phoenix_kit_emails and phoenix_kit_newsletters both depend on code introduced here — newsletters calls Mailer.deliver_via_integration/3 and needs V143, and will not compile against any released phoenix_kit. Order: core (this) → hex release → emails → newsletters.

timujinne and others added 13 commits July 12, 2026 07:57
Add the smtp provider (auth_type :credentials; host/port/username/password
fields) and brevo_api provider (auth_type :api_key) to builtin_providers/0,
per the user decision that SMTP is one universal provider rather than
per-vendor variants. Named connections of the same smtp provider coexist
independently (e.g. "SMTP 1", "Brevo SMTP").

Also generalizes has_credentials?/1 and maybe_set_status/2 in
integrations.ex: the existing :credentials gate only recognized a nested
"credentials" map, which the smtp provider's flat host/port/username/password
fields never populate, so a headless save+validate could never reach status
"connected". has_flat_credential_fields?/2 checks that every required field
the provider itself declares is present instead, data-driven off the
provider's own setup_fields so it doesn't hardcode field names and doesn't
change behavior for any existing non-:credentials provider.
Add "password" to Encryption.@sensitive_fields so the universal smtp
provider's password is AES-256-GCM encrypted at rest like every other
credential field.

Also correct the moduledoc, which claimed PBKDF2 key derivation —
derive_key/1 is a single SHA-256 hash, not PBKDF2 (doc-only fix).
…edential encryption

encryption_key/0 previously read only the flat `config :phoenix_kit,
secret_key_base: ...` app env key. The installer never stamps that key,
so in real host apps Encryption.enabled?/0 was false and every
integration secret (AWS keys, SMTP passwords, API keys) was stored in
plaintext — verified live on Hydra Force during the B5 SES live-test.

Add a fallback: when the flat key is unset, derive from the host app's
own Endpoint secret_key_base via PhoenixKit.Config.get_parent_endpoint/0
(the same :parent_module-based discovery already used elsewhere in
config.ex, which the installer does set). The flat key keeps
precedence, so existing installs that set it explicitly are unaffected.
Rescue-safe: an unloaded/unstarted endpoint yields nil (plaintext
passthrough), not a crash.

Deviates from the plan's literal PhoenixKit.Config.get(:endpoint) —
nothing in the installer or codebase ever sets that key (verified by
grep), so it would never have engaged. get_parent_endpoint/0 is the
mechanism the installer's :parent_module config actually wires up.
…der seam preserved

Add swoosh_config_for/1, mapping a resolved Integrations credentials map
to a {adapter, config} pair per provider: aws_ses -> Swoosh.Adapters.AmazonSES
(region/access_key/secret), smtp -> Swoosh.Adapters.SMTP (relay/port/
username/password, tls: :always on port 465 else :if_available), brevo_api
-> Swoosh.Adapters.Brevo (api_key). Public (not defp) with @doc false so
the provider-to-adapter mapping can be unit-tested without triggering real
delivery.

Add deliver_via_integration(email, integration_uuid, opts) which resolves
credentials via Integrations.get_credentials/1, builds the adapter config,
and delivers directly through Swoosh.Mailer.deliver/2 -- deliberately NOT
routed through deliver_email/2, whose deliver_with_runtime_config/3 path
is hardcoded to AWS SES (Provider.current().get_aws_*) and would misroute
or ignore SMTP/Brevo credentials. The same Provider.current()
intercept_before_send/2 and handle_after_send/2 tracking hooks
deliver_email/2 uses are preserved around the Swoosh call.

Tested with the real Swoosh.Adapters.Brevo code path (a fake
Swoosh.ApiClient stub captures the outbound POST instead of a mocked
Test adapter substitution) to prove interceptor hooks fire and the
request/response shape is correct without any real network call; SES
and SMTP config-building are covered by direct swoosh_config_for/1 unit
tests since their adapters would otherwise attempt a real network call
in a plain unit test.
GLM-5.2 review of Stage C found (verified against gen_smtp_client.erl:854 —
protocol is chosen solely by the ssl option; tls: only drives a post-connect
STARTTLS upgrade). So port 465 with tls: :always opened a plaintext TCP socket
to an SMTPS port and hung. Fixes:
- 465 -> ssl: true (implicit TLS); other ports -> tls: :always (mandatory
  STARTTLS, fail-closed vs plaintext downgrade while sending relay creds).
- unparseable port -> {:error, {:invalid_smtp_port, _}} instead of silently
  becoming gen_smtp default 25.
- annotate swoosh_config_for/1 (decrypted secrets — never log/inspect).
- document that secret_key_base rotation bricks existing enc:v1: blobs.
C4 live-tested only 587/STARTTLS, which is why this slipped through.
…harden credential gate

GLM-5.2 component-architect review of Stage C found (both verified):
- deliver_via_integration/3 did not pass provider: into intercept_before_send,
  so the emails Interceptor.detect_provider fell back to the host app static
  mailer adapter -> SMTP/Brevo integration sends mis-attributed as aws_ses
  (+ a per-send "no provider data" warning). Now injects
  provider: creds["provider"] (put_new so explicit callers win). Directly
  benefits the Stage D DeliveryWorker.
- has_flat_credential_fields?/2: present?/1 is binary-only, so a :number field
  (SMTP port) arriving as an integer would fail the gate -> get_credentials
  breaks -> silent send failure. Added field_present?/1 (numbers ok) and
  guarded the Enum.all?([]) == true empty-required-list footgun.
…end_profile_uuid

Adds phoenix_kit_newsletters_send_profiles (named send configurations
referencing an Integrations connection by uuid, per-account send params,
at most one is_default via partial unique index) and a bare
send_profile_uuid column on phoenix_kit_newsletters_broadcasts, so a
broadcast can pin which send profile delivers it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011txEtBPj6dG8iB5ZqUg63f
RateLimiter.check_limits/1 (blocklist + send-rate limits) existed but
had zero production callers, so a blocklisted or hard-bounced address
could still receive mail. Both deliver_email/2 and
deliver_via_integration/3 now check every `to` recipient up front via a
soft-dependency call (Code.ensure_loaded?/1-guarded, since core has no
compile-time dependency on the optional emails package) and
short-circuit with {:error, {:blocked, reason}} without delivering or
tracking. Enforcement lives in the Mailer functions rather than
intercept_before_send/2, which has no abort channel. Non-blocked sends
and hosts without the emails module installed are unaffected.
…uite

Stage E2 originally wired RateLimiter.check_limits/1, which also enforces the
emails module per-recipient (100/h) and GLOBAL (10_000/h) send caps. Neither is
gated by an enable flag (only the blocklist is), so that silently:
  - throttled every outbound email app-wide, auth mail included, and
  - capped bulk newsletter broadcasts at 10k/hour.
The reported defect was the blocklist not being enforced. Send pacing/quotas
belong to the newsletters send-profile limits (roadmap Phase 5, per-profile
atomic caps) — one limiter, not two competing ones. Now calls check_blocklist/1.
…intext relays

Final GLM-5.2 review findings (both reviewers):
- check_recipient_allowed/1 only inspected , so a blocklisted address in
  cc/bcc bypassed suppression entirely. Now checks to ++ cc ++ bcc.
- tls: :always on every non-465 port broke credential-less relays (local
  MailHog:1025, internal plaintext smarthosts). Now: 465 -> ssl: true;
  credentials present -> tls: :always (fail closed, no plaintext downgrade);
  no credentials -> tls: :if_available (nothing on the wire to protect).
- docstrings still promised rate-limit enforcement (removed in 57b183a) and
  under-specified the error shapes.
Final GLM-5.2 review: none of the three new providers carried a validation map,
so do_validate/2 fell through to :ok and record_validation stamped the
connection connected without ever checking the credentials -- an operator who
pasted a wrong key saw a green check and a failing send. Brevo validates with a
bare api-key header, which the existing declarative validation mechanism already
supports (GET /v3/account). aws_ses (SigV4) and smtp (real SMTP handshake) need
bespoke validators and stay unvalidated for now -- documented as follow-up.
@timujinne

Copy link
Copy Markdown
Contributor Author

Code review record — GLM-5.2, 4 rounds × 2 independent agents

Every round was run as two independent GLM-5.2 agents (component-architect + reviewer, --effort max), reading the real diffs and the surrounding code. Every load-bearing claim was re-verified against the source before it was acted on. Full transcripts are committed in the newsletters repo under docs/superpowers/specs/reviews/ (spec → plan → Stage B+C implementation → final).

Findings against this repo, and how each was resolved

# Severity Finding Resolution
1 🔴 SMTP port 465 hung. tls: :always was used for implicit TLS, but gen_smtp picks the protocol solely from the ssl option (gen_smtp_client.erl:854) — tls: only drives a STARTTLS upgrade after a plaintext connect. So 465 opened a plaintext socket to an SMTPS port. Verified against the gen_smtp source. 5cda9ee6 — 465 → ssl: true; unparseable port → {:error, {:invalid_smtp_port, _}} instead of silently becoming gen_smtp's default 25
2 🟠 Tracking mis-attributed the provider. deliver_via_integration/3 didn't pass provider: into intercept_before_send, so the emails interceptor fell back to the host app's static mailer adapter — SMTP/Brevo sends were logged as aws_ses, plus a warning per send c5bdcd23 — injects provider: creds["provider"] (put_new, so an explicit caller still wins)
3 🟠 Credential gate. :credentials providers store fields flat, but the gate only recognised a nested "credentials" map → SMTP connections stayed "disconnected" and get_credentials/1 failed. Also present?/1 is binary-only, so a numeric port would fail the gate a04fd70d + c5bdcd23has_flat_credential_fields?/2, data-driven off each provider's own required fields; numbers accepted; empty-required-list footgun guarded
4 🟠 Blocklist bypassable via cc/bcc (only to was inspected) — a suppression list with a hole is a compliance problem efd2f6b7 — checks to ++ cc ++ bcc
5 🟠 tls: :always broke credential-less relays (local MailHog:1025, internal plaintext smarthosts) efd2f6b7 — creds present → :always (fail closed); no creds → :if_available (nothing on the wire to protect)
6 🟡 “Test Connection” validated nothing for the three new providers — do_validate/2 fell through to :ok and stamped the connection connected without checking anything fd034728 — real validation map for brevo_api (GET /v3/account). aws_ses (SigV4) and smtp (real handshake) need bespoke validators → open follow-up
7 🟡 Docs: the KDF is a single SHA-256, not PBKDF2; rotating secret_key_base makes existing enc:v1: blobs undecryptable with no re-wrap path; swoosh_config_for/1 returns decrypted secrets 5cda9ee6 / 7e4ef9ec — documented in-module

Positives the reviewers verified independently (not just claimed)

  • The encryption fallback is precedence-safe: the flat :secret_key_base still wins, so existing installs derive the identical key; pre-existing plaintext values read back unchanged (only enc:v1: values are decrypted) and re-encrypt on next save — no data-loss window.
  • Secrets are encrypted at rest (confirmed against the live DB) and never loggedmailer.ex contains no Logger call at all.
  • The tracking seam in deliver_via_integration/3 faithfully mirrors deliver_email/2 (intercept_before_send → deliver → handle_after_send), so X-PhoenixKit-Log-Id correlation is preserved.
  • The gate change has no blast radius: has_flat_credential_fields?/2 is guarded on auth_type: :credentials, and smtp is the only such provider in core.
  • V143 is idempotent, prefix-aware, TIMESTAMPTZ, and its partial unique index really does reject a second default.

Verdicts

Final round: Ship-with-changes (both agents) — spec compliance PASS, no architectural rework, no critical defects. All must-fix items above are fixed in this branch; only the aws_ses/smtp connection validators remain as a documented follow-up.

Pincer and others added 2 commits July 13, 2026 12:46
… -> V145

Third same-day PR to claim V143 (after new-login-alerts and the
manufacturing/warehouse consolidation, now V143 and V144 on main).
Renumbered this migration, its test, and the postgres.ex dispatcher
wiring to V145 throughout.

Also fixes a real bug surfaced independently of the renumbering: the
migration's `uuid UUID PRIMARY KEY DEFAULT uuid_generate_v7()` was
unqualified -- the exact bug class fixed in V26 during today's PR631
prefix-hardening review, and present in every sibling migration
(`#{p}uuid_generate_v7()`) but missed here. On a prefixed install whose
connecting role's search_path excludes the prefix schema, this would
fail outright or silently resolve to the wrong schema's function. Now
uses Helpers.uuid_v7_call/1, the current canonical helper for this.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Independent review pass: verified the encryption-key-fallback and
SMTP-TLS-selection security-fix claims against the actual code rather
than the PR description, confirmed the recipient-blocklist gating and
has_flat_credential_fields?/2's empty-required-list guard. One bug
found and fixed separately (unqualified uuid_generate_v7() in the
migration). Full findings in the doc.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@ddon

ddon commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Renumbered V143 → V145 (this is the third same-day PR to claim V143 — new-login-alerts and the manufacturing/warehouse consolidation took V143 and V144 respectively, both already merged to main). Rebased onto current main, renumbered the migration/test/dispatcher wiring throughout.

Also found and fixed a real bug independent of the renumbering: the migration's uuid UUID PRIMARY KEY DEFAULT uuid_generate_v7() was unqualified — the exact bug class fixed in V26 during today's PR #631 prefix-hardening review, and present in every sibling migration but missed here. On a prefixed install whose connecting role's search_path excludes the prefix schema, this fails outright or silently resolves to the wrong schema's function. Now uses Helpers.uuid_v7_call/1.

Independently verified the two security-fix claims (encryption key fallback to the host's Endpoint secret_key_base, and the SMTP TLS/port-465 handling) against the actual code rather than the PR description — both hold up. No further bugs found in encryption.ex/integrations.ex/providers.ex/mailer.ex. mix precommit clean.

Full write-up: dev_docs/pull_requests/2026/633-newsletters-sending-foundation/CLAUDE_REVIEW.md (pushed to this branch).

Ready to merge.

@ddon
ddon marked this pull request as ready for review July 13, 2026 12:48
@ddon
ddon merged commit 734c759 into BeamLabEU:main Jul 13, 2026
timujinne added a commit to timujinne/phoenix_kit_emails that referenced this pull request Jul 14, 2026
… the aws_ses provider)

migrate_legacy/0 calls Integrations.add_connection("aws_ses", ...), and the
aws_ses provider only exists from core v1.7.190 (PR BeamLabEU/phoenix_kit#633,
now merged and published). Against an older core this compiles fine and fails at
runtime -- exactly the class of break a version floor exists to prevent.

Verified against the real hex release (1.7.191): compiles with
--warnings-as-errors, 35 tests / 0 failures.
timujinne added a commit to timujinne/phoenix_kit_newsletters that referenced this pull request Jul 14, 2026
…>= 1.7.190

Two things, both surfaced by merging the latest upstream:

1. SendProfile was the only table-backed schema in this package without
   `use PhoenixKit.SchemaPrefix` -- caught by upstream's new
   schema_prefix_conformance_test. On a non-public Postgres schema install
   (multi-tenant), queries against the profiles table would have ignored the
   tenant prefix, even though the V143 migration itself is prefix-aware.

2. Version floor raised to >= 1.7.190. This package calls
   PhoenixKit.Mailer.deliver_via_integration/3 and needs migration V143, both of
   which first shipped in core v1.7.190 (BeamLabEU/phoenix_kit#633, now merged
   and published to hex). Upstream had bumped the floor to 1.7.189 -- one release
   short, which would compile and then fail at runtime.

The merge gate documented in this PR is now cleared: with core published, this
branch compiles against the REAL hex release (1.7.190) under
--warnings-as-errors, 80 tests / 0 failures. Previously it could only build in a
workspace where core was a path dep.

Also resolves the content.ex conflict in favour of upstream: both sides
independently migrated the retired earmark to MDEx, but upstream's version is
strictly better -- it passes @mdex_options, sanitizes the rendered HTML via
PhoenixKit.Utils.HtmlSanitizer, and escapes the source on a parse failure
instead of returning an empty string.
@timujinne
timujinne deleted the feature/newsletters-sending-foundation branch August 6, 2026 05:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants