feat(system): build the Email SMTP settings screen + a live send test - #187
Merged
Conversation
The System → Email SMTP tab was a placeholder, so mail could only be configured
by hand-editing `local.ini` or the `config` table, and there was no way at all to
tell whether it worked. That second half is the real problem: a dead MTA is
INVISIBLE. The password-reset flow deliberately reveals nothing (no account
enumeration), so "sent" and "silently discarded" look identical to an admin.
Adds the full form — transport, SMTP host/port/encryption/auth/credentials, and
the From identity — writing to the `config` DB tier via a new
`Tiger_Mail::saveSettings()`, mirroring `Tiger_Recaptcha::saveSettings()`. The
connection block hides for the sendmail transport. Port is range-checked and the
protocol is restricted to STARTTLS/SSL/none, so an unusable value is normalized
server-side rather than handed to the transport.
The password is encrypted at rest (`mail.smtp.password_enc`) via Tiger_Crypto,
and `settings()` never returns it — only `has_password` — so the UI cannot read
the secret back out. A blank password on save KEEPS the stored one, so editing a
host can't silently wipe credentials. Critically, `_smtpPassword()` still falls
back to a legacy plaintext `mail.smtp.password`: installs configured before this
screen existed have one, and breaking their outgoing mail on upgrade would be
the worst possible regression precisely because it fails silently.
"Send test" (`mailTest`) follows the existing `locationTest` precedent and sends
using the values CURRENTLY in the form, so a setup can be verified WITHOUT first
saving bad config over a working one; a blank password falls back to the stored
secret. The transport's own error text is returned verbatim — on a connection
test the message ("Could not open socket", an auth rejection, a TLS failure) is
the entire diagnostic, and the action is admin-only.
i18n: 34 screen keys + 4 client keys authored in all six locales (en/es/pt/hi/de/
fr), key parity held at 362. The two dead "under construction" placeholders are
removed, including from tlh.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WXgMENcwa4Q8yCJaHpjHpf
Adds a provider dropdown covering SES, SendGrid, Mailgun, Postmark, Resend,
Brevo, Mailjet, Google Workspace and Microsoft 365 — each listed as an explicit
SMTP and/or API entry ("Amazon SES (SMTP)" vs "Amazon SES (API)") so the choice
is visible rather than buried in a transport setting.
WHY BOTH. Nearly every service speaks SMTP, so SMTP needs no driver at all —
just the right host/port/encryption, which is exactly what an operator otherwise
has to go hunting for. Picking a provider now prefills those. The API drivers
exist for what SMTP CAN'T do: hosts that firewall outbound 587/465 (common on
shared/cPanel, a target platform), bulk throughput, and provider features SMTP
can't express.
Architecture. `Tiger_Mail_Provider` is one declarative table driving three
things — the dropdown, which credential fields render, and how a transport is
built; adding a provider is a row plus (for API) one class. Drivers extend
`Zend_Mail_Transport_Abstract` via `Tiger_Mail_Transport_Api`, so they are just
transports: `send()`, the per-call override and the test-capture default all
work unchanged. The base reads the STRUCTURED message off `$this->_mail` rather
than parsing rendered MIME, because these APIs take JSON fields.
SES rides the vendored AWS SDK (tiger-sdk-aws), capability-detected — so the
option only appears when the SDK is installed, and core never hard-depends on
it. The payoff is the SDK's default credential chain: leave key/secret blank and
it uses the instance IAM role, so an install on AWS sends mail with NO STORED
SECRET AT ALL — strictly better than SES SMTP, which needs a static username and
password in the config table. It posts raw MIME via Content.Raw so the message
Zend already built goes out byte-for-byte.
Google Workspace and Microsoft 365 are deliberately SMTP-only: both send APIs
require an OAuth2 flow, not a pasteable key, so they can't be configured on a
settings screen. That belongs behind the TigerConnect broker; SMTP is the honest
working option until then. Their help text names the real gotchas (App Password;
Microsoft disabling SMTP AUTH by default).
Secrets are encrypted at rest per provider, a blank secret keeps the stored one,
and `settings()` returns has_* flags only. An unresolvable API provider (its SDK
deactivated) degrades to sendmail rather than fataling every request that sends.
i18n: 11 core provider keys + 3 screen keys in all six locales; parity held.
Tests (+14): every declared API provider has a loadable driver extending the
base; capability detection for the SDK; placeholder host interpolation, and an
UNFILLED placeholder yielding no host rather than a literal "{region}" DNS
failure; per-provider payload shapes; credential headers; blank-secret-keeps;
degradation to sendmail. The load-bearing one asserts the body is NEVER
quoted-printable encoded — getBodyHtml(true)/getContent() return wire-encoded
content, which would deliver visible =3D soup, and that is invisible until
someone reads a real email.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WXgMENcwa4Q8yCJaHpjHpf
…real defects CI caught two legitimate blocks on the provider work, plus the tests written to clear them found two genuine bugs. - CAPABILITIES.md was STALE: the nine new @api classes weren't in the generated index. Regenerated (194 classes). - Coverage fell to 71.4%, under the ratcheting 72% floor. The floor only ever goes up, so the fix is coverage, not a lower bar. Two causes: the concrete drivers reported 0% despite being exercised (the known #[CoversClass] gotcha — pcov attributes nothing to a class the test doesn't DECLARE), and System_Form_Settings was never instantiated at all (0/129). Now 72.7%. Defects the new tests surfaced: - `curl_close()` is deprecated in PHP 8.5 (a no-op since 8.0), so every API send would have emitted a deprecation in production. Removed. - `mail_from_email` used the default EmailAddress validator, which requires a DNS-resolvable TLD — meaning core's OWN shipped default, `no-reply@localhost`, could not be saved through the screen, and no intranet install could set a local sender. Now ALLOW_DNS | ALLOW_LOCAL. Adds MailApiSendTest (the real send() entry point through probe subclasses that capture instead of transmit; the cURL-failure branch via a closed local port; SES explaining the missing SDK rather than fataling) and SettingsFormTest (the form is one big elements() array whose typos only surface at render time on a live admin screen — so instantiating it IS the test, plus port-range and address validation, and that nothing in the SMTP block is required). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WXgMENcwa4Q8yCJaHpjHpf
…base CI's integration job fataled at class-composition time: Zend_Mail_Transport_Abstract and ProbesTheRequest define the same property ($body) ... the definition differs and is considered incompatible The base declares `public $body = ''` and `public $header = ''` for the rendered message; the probe trait redeclared `$body` with no default, which PHP treats as an incompatible composition. Renamed to $probeUrl/$probeBody/$probeHeaders, with a comment naming the trap so it isn't reintroduced. This also explains the paired coverage failure — the coverage job runs the full suite, which died on the same fatal. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WXgMENcwa4Q8yCJaHpjHpf
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The System → Email SMTP tab was an "Under construction" placeholder. Mail could only be configured by hand-editing
local.inior theconfigtable — and there was no way to tell whether it worked.That second half is the actual problem: a dead MTA is invisible. The password-reset flow deliberately reveals nothing (no account enumeration), so "delivered" and "silently discarded" look identical from the admin's side. Hit exactly this on dev-com —
mail.transportdefaulting to PHPmail(),sendmail_pathpointing at/usr/sbin/sendmail, and that binary not existing.What
The form — transport, SMTP host / port / encryption / auth / credentials, and the From identity. The connection block hides when the transport is sendmail. Port is range-checked (1–65535, default 587) and encryption is restricted to STARTTLS / SSL / none, so an unusable value is normalized server-side rather than handed to the transport.
Tiger_Mail::saveSettings()writes to theconfigDB tier — the live-override pattern, effective next request — mirroringTiger_Recaptcha::saveSettings().Secret handling, three rules, each tested:
mail.smtp.password_encviaTiger_Crypto.settings()never returns the password — onlyhas_password— so the UI can't read it back out.Backward compatibility is load-bearing:
_smtpPassword()still falls back to a legacy plaintextmail.smtp.password. Installs configured before this screen existed have one, and breaking their outgoing mail on upgrade would be the worst possible regression — precisely because it fails silently. Saving from the screen upgrades them to the encrypted key."Send test" (
mailTest) follows the existinglocationTestprecedent: it sends using the values currently in the form, so a setup can be verified without first saving bad config over a working one. A blank password falls back to the stored secret. The transport's own error text is returned verbatim — on a connection test the message (Could not open socket, an auth rejection, a TLS failure) is the diagnostic, and the action is admin-only.i18n
34 screen keys + 4 client keys authored in all six locales (en/es/pt/hi/de/fr) — parity held at 362 keys each. The two dead "under construction" placeholders are removed, including from
tlh.Tests
7 new integration tests on the settings surface: sendmail fallback (including
smtpwith no host degrading instead of fatalling), a legacy plaintext password still building an SMTP transport, the encrypted key winning over a stale plaintext one,settings()never exposing the secret, port/protocol normalization, blank-password-keeps-stored, andtransportFor()building from unsaved values.Full suite: 2055 tests, 0 failures, 15 deprecations — the existing baseline (2048 + 7).