Add SMTP encryption, auth, certificate and timeout settings; add an optional queue hook

[timujinne]

Why

Two gaps in the email surface, found while wiring phoenix_kit_emails into a host app for the first time.

SMTP integrations could not be configured. The smtp provider exposed only host / port / username / password, and the TLS mode was inferred from the port: 465 meant implicit TLS, anything else meant STARTTLS — mandatory when credentials were present, opportunistic when not. So an operator could not run implicit TLS on a non-465 port, could not talk to an internal relay in the clear or with a private CA, and could not choose the AUTH mode or a timeout.

Two shared pieces of the integrations UI were quietly broken.setup_field/1 rendered every field as type="text", so the :select / :textarea / :number types the provider contract already allows were impossible — and the website-wide integration form carried a hand-rolled copy of that markup, so anything added to the shared component would not have appeared there anyway.

Nothing could queue outgoing mail. A provider could observe a message (intercept_before_send/2) but not take delivery over, so an optional package could only queue what its own API sent — never the host application's password resets and confirmations, which go through the statically configured mailer.

What changed

SMTP settings — five optional setup fields on the smtp provider:

field	values	default
security	auto / ssl / starttls / starttls_optional / none	auto = the old port-based rule
auth	if_available / always / never	if_available (gen_smtp's own default)
verify_cert	verify_peer / verify_none	verify_peer
ca_cert	PEM bundle	system store
timeout	seconds	gen_smtp default

Blank fields reproduce the previous behaviour, so existing connections send exactly as before. Unknown values are rejected rather than coerced to auto. Validators.smtp/1 keeps building the probe from the very same SmtpTransport.config/1, and now reads the operator's auth — with one deliberate asymmetry documented at the call site: never stays never, but if_available is upgraded to alwaysfor the probe, because gen_smtp tolerates a failed AUTH and a wrong password would otherwise pass the check.

The fail-closed :no_ca_store rule is unchanged and still unreachable from auto: it is bypassed only when the operator explicitly picks verify_none or security: none. A ca_cert is no longer parsed when the chosen transport never builds TLS options.

Setup fields — setup_field/1 renders by :type (with soft map access, so a provider contributed by an external module without a :type falls back to a text input instead of raising KeyError), and Live.Settings.IntegrationForm now uses the shared component instead of its own copy. Credential-shaped fields stay type="text" on purpose — the existing rationale about browsers' password-save heuristics still holds.

Queue hook — a new optionalmaybe_enqueue/2 callback on PhoenixKit.Email.Provider, called from both delivery paths right after interception through one shared helper. {:queued, ref} short-circuits the send and is returned to the caller as {:ok, %{id: ref, queued: true}}; :continue sends as before. Because the callback is optional and guarded by function_exported?/3, a package built against an older core keeps working; skip_queue: true lets a queue worker ask for the real send without being handed its own job back.

Sender warning — the Email Sending settings page now warns when the configured sender is not a full address. The optional emails package refuses to log such a message (its Log changeset requires name@domain.tld) and swallows the rejection, so with the built-in default noreply@localhost mail is delivered while the log stays empty and nothing in the UI explains it.

Verification

• mix test test/phoenix_kit/mailer/smtp_transport_test.exs — 28 tests, 0 failures (11 before). New cases cover explicit ssl on a non-465 port, starttls forced on 465, none → tls: :never, verify_none without a CA store, a PEM bundle (and garbage rejected), the auth modes, the timeout in milliseconds, and rejection of unknown values.
• mix format, mix credo --strict and mix compile --force --warnings-as-errors are clean on the changed files.
• Exercised in a host application: the integration form renders three selects, a textarea and two number inputs; an existing connection with no new fields set builds the same options as before; the sender warning appears for noreply@localhost and disappears for a real address.

[timujinne]

External code review — GLM-5.2 (effort: max, read-only)

Run against this branch on 2026-07-29; the reviewer had the full PR diff and the working tree, no shell. Verdict and findings below are unedited. Every finding is fixed in the follow-up commit 33cc6ae5, and the review is also committed at dev_docs/pull_requests/2026/668-smtp-tls-options-and-queue-hook/GLM_REVIEW.md.

What changed in response:

• Major (double interception):intercept_before_send/2 no longer runs on a queue worker's re-send. The worker passes already_intercepted: true; skip_queue keeps its narrower meaning ("do not offer this message to the queue"), so a caller that wants a synchronous send still gets tracking. Core no longer depends on a provider's interception being idempotent.
• Medium (untested queue path): added a queuing provider double and three tests — the offer short-circuits the send, skip_queue sends without offering, and already_intercepted skips interception while still running the after-send hook. These are :integration tests and were not executed here: this environment has no PostgreSQL for the core suite.
• Minor (untested translations): added tests for the new Validators.smtp/1 error arms. Run: validators 29 tests, transport 28 tests, 0 failures.
• Nit (silent fall-through): a return from maybe_enqueue/2 outside the callback contract still sends the message — a provider bug must not eat mail — but it is now logged instead of being invisible.
• Deliberately unchanged: the "sent" wording at the notification / test-email call sites (queued means accepted for delivery, and the return shape is documented), and min/step on the number inputs.

---

I've completed my verification. Here is my review.

---

VERDICT: APPROVE

The SMTP rewrite is faithful (verified option-by-option against the pre-PR logic), the fail-closed :no_ca_store rule is intact and reachable only through explicit operator choice, the callback version-skew is handled correctly, and no in-repo caller breaks on the new return shape. The issues below are one latent design hazard at the new queue chokepoint and test-coverage gaps — none affect correctness of what ships (no queue provider exists in core yet).

---

Major (latent) — queued messages are intercepted twice; skip_queue does not skip interception

lib/phoenix_kit/mailer.ex:375-386 — verified by reading:

defpintercept_and_offer_queue(email,opts)doprovider=Provider.current()tracked_email=provider.intercept_before_send(email,opts)# runs unconditionallyifKeyword.get(opts,:skip_queue,false)do{:continue,tracked_email}else...

intercept_before_send/2 runs on every call, including the queue worker's drain call (skip_queue: true). So a queued-then-drained message is intercepted twice: once at enqueue (on the original email) and once at drain (on the already-tracked email). handle_after_send/2 fires once, at drain.

Failing scenario: the optional emails package implements maybe_enqueue/2 (storing the tracked email it is handed) and a worker later calls Mailer.deliver_email(email, skip_queue: true). If its intercept_before_send inserts an email-log row — the documented purpose of the seam ("logs outgoing emails") and almost certainly not idempotent — every queued message produces two log rows. I could not verify the emails package's interceptor (it is a soft dependency, absent from this repo), so the impact is inferred; the double-invocation itself is verified.

Fix (pick one): when skip_queue: true, skip intercept_before_send too (the message was already intercepted at enqueue — skip_queue already means "I am the worker re-sending an already-handled message"); or move the intercept below the queue check; or state explicitly in the maybe_enqueue/2 contract that intercept_before_send must be idempotent. The first is the smallest change and matches the existing semantics of skip_queue.

Medium — the entire queue path is untested

Verified by grep: test/ contains no reference to maybe_enqueue, skip_queue, queued:, or intercept_and_offer. The three most behaviorally significant new facts are all uncovered:

• the new success shape {:ok, %{id: ref, queued: true}} (mailer.ex:383);
• that {:queued, _} short-circuits Swoosh.Mailer.deliver/2 (no send, no handle_after_send);
• that skip_queue: true bypasses the offer and still sends.

The infrastructure is already in place — PhoenixKit.MailerTest.TrackingProvider (test/phoenix_kit/mailer_test.exs:35-77) is a Provider double that messages the test process from both hooks, and extending it with a maybe_enqueue/2 returning {:queued, "ref"} would exercise all three. Note that double deliberately omits maybe_enqueue (legal, since it is @optional_callbacks), which is exactly why the default path still tests green — and why the queue path is invisible.

Fix: add a QueuingProvider double and assert (a) deliver_email/2 returns {:ok, %{id: ref, queued: true}}, (b) no {:handle_after_send_called, _} message arrives, (c) a second call with skip_queue: true does send and does call handle_after_send.

Minor — probe_auth/1 and the new Validators.smtp/1 error-translation clauses are untested

lib/phoenix_kit/integrations/validators.ex:127-132 (the never-stays-never asymmetry) and the new case arms at validators.ex:97-118 (translating :invalid_ca_cert, {:invalid_timeout, _}, and the {:invalid_security|:invalid_verify_cert|:invalid_auth, _} guard clause with its String.replace_prefix("invalid_", "") derivation) have no tests. The existing smtp/1 describe-block (validators_test.exs:254-299) covers only an unreachable relay, a bad port, the no-AUTH pass, and the tarpit deadline. SmtpTransport.config/1 is well covered; the translation layer above it is not.

Fix: unit-test the three new arms directly against Validators.smtp/1 with malformed security/verify_cert/auth/timeout/ca_cert values.

Minor — public deliver_email/2 success shape changed; user-facing callers now equate "queued" with "sent"

Verified at the call sites: notifications/channels/email.ex:60 ({:ok, _} -> :ok), users/auth/user_notifier.ex:47 (with {:ok, _metadata} <-), and email_sending.ex:137-144 (test-email "sent to %{recipient}" flash). All match broadly, so none break — confirmed. But once a provider queues, the test-email page will flash "sent" and the notification channel will return :ok (marking the notification delivered) for a message that has only been enqueued. This is inherent to adding the seam and is documented in the moduledoc, but the "Test email sent" wording will be literally false in the queued case.

Fix: either acceptable as-is (queue ≈ accepted-for-delivery) or have these callers check queued: and adjust copy. Flagging so the API evolution is a conscious choice.

Nit — non-{:queued, _} returns from maybe_enqueue/2 silently fall through to a live send

mailer.ex:382-385: case offer_to_queue(...) do {:queued, ref} -> ...; _ -> {:continue, tracked_email} end. A provider bug returning {:error, _} (outside the :continue | {:queued, term()} spec) causes the message to be sent anyway (fail-open to send). Defensible, but it will mask provider errors. Consider logging the unexpected return in the _ arm.

Nit — loggable_sender?/1 duplicates the emails module's regex; number inputs lack bounds

email_sending.ex:190-193 copies ~r/^[^\s]+@[^\s]+\.[^\s]+$/ from the optional module's Log changeset (acknowledged in the comment). Advisory-only, so drift is low-impact, but worth a pointer comment to the source of truth. Separately, the port field (providers.ex:762, pre-existing :number) and the new timeout field now render as <input type="number"> with no min/step (integrations_ui.ex:265-275); parse_timeout rejects 0, but the UI doesn't prevent typing it. Consider min="1".

---

Verified vs. inferred

Verified by reading code: default-blank options reproduce prior behavior (traced config/2 → resolve_cacerts → transport → tls_options for every security/verify/store combination); :no_ca_store is reachable only when security ∉ {:none} and verify = :verify_peer with no PEM and no system store (smtp_transport.ex:234-245), i.e. never via auto; verify_none/none/starttls_optional are gated behind explicit select choices; the transport/5 clause ordering (:auto,465 before :auto,port) is correct; the with short-circuit correctly propagates {:ok, %{id: ref, queued: true}} through both delivery paths; no other provider declares :select/:textarea (only SMTP's new fields + the pre-existing port:number), so the setup_field rewrite cannot regress other integrations; extract_setup_attrs (integration_form.ex:575-592) is generic over setup_fields, so the new control types submit under the same field.key and need no backend change; HEEx auto-escaping makes the textarea/PEM rendering safe; sender_loggable? is assigned in mount (no KeyError).

Inferred (could not verify): the actual double-logging impact of Finding 1 (emails package is out of scope); gen_smtp's default auth value being :if_available (taken from the PR's own claim, consistent with the explicit set).

Direction

This is a sound direction for the module. Concentrating every operator knob through one pure SmtpTransport.config/1 that both the probe and the send path read keeps the single-source-of-truth property intact, and the fail-closed-then-explicitly-escapable TLS posture is exactly the right trade-off for credentials on the wire. The optional maybe_enqueue/2 callback is the correct shape for an opt-in queue, and the function_exported?/3 guard handles version skew cleanly in the direction that matters. The work needed before this is relied upon rather than before it merges: pin down the intercept-on-drain contract (Finding 1) and add the queue-path tests (Finding 2) — both are cheap now and expensive once an external provider implements the queue against an ambiguous contract.