Skip to content

Commit 2fe4ea0

Browse files
committed
feat(1455): SMTP test-email button on the settings page
GitHub issue #1455 — operators had no way to verify SMTP credentials short of waiting for a real outbound mail (password reset, ban protest, etc.), so broken SMTP routinely shipped into production for days before anyone noticed. This change adds a "Send test email" affordance inside the SMTP card on Admin → Settings → Main that fires `system.test_email` (a new JSON handler) and surfaces an operator-actionable toast — success or one of four structured error envelopes (validation / smtp_not_configured / rate_limited / mail_failed). Key shape decisions: - Permission gate: `ADMIN_OWNER | ADMIN_WEB_SETTINGS` — matches every other settings-page-only handler (sel_theme / apply_theme / clear_cache / preview_intro_text). - Rate limit: 1 attempt / 10s per panel install, stamped BEFORE the SMTP I/O so a hung relay can't be hammered while the first call is mid-handshake. File-backed (`SB_CACHE/test-email-throttle`) with atomic tempfile + rename, mirroring `_api_system_release_save_cache`. Validation / smtp_not_configured short-circuit BEFORE the throttle file is stamped so a typo doesn't consume a slot. - Button disabled at first paint when smtp.host / smtp.user / config.mail.from_email are empty (server-rendered), AND live- re-evaluated as the operator edits the form inputs so a fresh- install operator who just typed valid creds doesn't have to save first to see the button enable. Server-side guard is the `smtp_not_configured` envelope. - Recipient defaults to the logged-in admin's email so the "send the test to me" path is one click; the operator can override. - Audit log: every send attempt lands a row in `:prefix_log` (success OR mail_failed) so test sends can't be used to silently probe SMTP credentials or enumerate valid relay endpoints. Row body interpolates the admin's name + recipient. Coverage: - 9 PHPUnit tests under `SystemTest::testTestEmail*` — anonymous reject / malformed recipient / oversized recipient (RFC 5321 cap via FILTER_VALIDATE_EMAIL) / smtp_not_configured / mail_failed / rate_limited / default recipient / "validation doesn't burn a slot" / "smtp_not_configured doesn't burn a slot". 4 paired snapshot files lock the wire format of every error envelope; the success-shape snapshot lives in the E2E suite (PHPUnit has no SMTP test seam). - 1 PermissionMatrixTest row pins the `ADMIN_OWNER | ADMIN_WEB_SETTINGS` gate. - 6 Playwright tests (3 specs × chromium + mobile-chromium) under `web/tests/e2e/specs/flows/smtp-test-email.spec.ts` — happy path drives the full chain through mailpit + asserts the email lands at the operator's address with the right subject, plus disabled-state + native-validation arms. Throttle-cache shim (`web/tests/e2e/scripts/clear-test-email-throttle-e2e.php`) clears the 10s lock between specs so parallel project profiles don't collide; the shim mirrors `reset-e2e-db.php`'s refuse-if-prod-DB guard. - FAQ doc entry under `docs/src/content/docs/faq/index.md` walks operators through the prerequisites + rate limit + audit-log surfacing.
1 parent 548375d commit 2fe4ea0

16 files changed

Lines changed: 1100 additions & 0 deletions

docs/src/content/docs/faq/index.md

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,29 @@ yourself to an admin group with the relevant notification flags
110110
(typically the group that gets emails for new ban submissions,
111111
protests, etc.).
112112

113+
### How do I verify SMTP without waiting for a real event?
114+
115+
Go to **Admin Panel → Settings → Main** and scroll to the SMTP card.
116+
Below the regular SMTP fields you'll see **Send a test email**
117+
it's pre-populated with your admin account's email but you can
118+
swap in any recipient. Click **Send test email** and the panel
119+
dispatches a one-shot verification message through the saved SMTP
120+
credentials.
121+
122+
A few things to keep in mind:
123+
124+
- The button is greyed out until **Host**, **Username**, and the
125+
**From address** are saved. Save the form first if you just
126+
changed any of them — the test reads the persisted values, not
127+
the unsaved contents of the inputs.
128+
- The action is rate-limited to one send every 10 seconds per
129+
install (prevents accidental misconfiguration from spamming
130+
your SMTP relay).
131+
- Both success and failure show up in **Admin Panel → System
132+
log**, so you can correlate a "Test email failed" toast with
133+
the underlying SMTP error (`Mail error`) entry from the same
134+
request.
135+
113136
### I locked myself out by enabling Steam-only login
114137

115138
If you flipped the Steam-only login switch and you no longer have

web/api/handlers/_register.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,3 +171,7 @@
171171
// settings page so we don't accidentally expose the renderer to
172172
// non-settings surfaces.
173173
Api::register('system.preview_intro_text', 'api_system_preview_intro_text', ADMIN_OWNER | ADMIN_WEB_SETTINGS);
174+
// #1455: SMTP test-email button on the settings page. Same permission
175+
// gate as the other settings-only actions — only operators who can
176+
// edit SMTP credentials have a reason to trigger a verification send.
177+
Api::register('system.test_email', 'api_system_test_email', ADMIN_OWNER | ADMIN_WEB_SETTINGS);

web/api/handlers/system.php

Lines changed: 225 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,10 @@
1111
work. If not, see <http://creativecommons.org/licenses/by-nc-sa/3.0/>.
1212
*************************************************************************/
1313

14+
use Sbpp\Auth\Host;
1415
use Sbpp\Mail\EmailType;
1516
use Sbpp\Mail\Mail;
17+
use Sbpp\Mail\Mailer;
1618
use Sbpp\Markup\IntroRenderer;
1719

1820
/**
@@ -376,6 +378,229 @@ function api_system_send_mail(array $params): array
376378
];
377379
}
378380

381+
/**
382+
* Issue #1455: send a SMTP test message so an operator can verify
383+
* the credentials they just typed into Admin → Settings actually work
384+
* end-to-end (Symfony Mailer → SMTP relay → recipient inbox) without
385+
* having to wait for a real password-reset / ban-protest event to fire
386+
* the next outbound mail.
387+
*
388+
* Permission gate: ADMIN_OWNER | ADMIN_WEB_SETTINGS. Matches the other
389+
* settings-page-only handlers (sel_theme / apply_theme / clear_cache /
390+
* preview_intro_text); only an operator who can edit SMTP settings has
391+
* a legitimate reason to trigger a test send.
392+
*
393+
* Error-envelope contract:
394+
* - validation missing or malformed recipient
395+
* - smtp_not_configured smtp.host / smtp.user / smtp.pass empty (we
396+
* short-circuit BEFORE Mail::send so the user
397+
* gets a pointed message instead of the
398+
* generic mail_failed branch — `Mail::send`'s
399+
* internal `Mailer::create() === null` arm
400+
* also returns false but only logs to
401+
* `:prefix_log`)
402+
* - rate_limited too many sends in the throttle window;
403+
* the message interpolates "try again in Ns"
404+
* so the client can echo it directly via a
405+
* toast — there's no separate machine-readable
406+
* `retry_after_seconds` field on the wire
407+
* (ApiError only carries code + message + field
408+
* + httpStatus; if a future client needs a
409+
* numeric value, parse it from the message
410+
* or extend ApiError with a `details` payload).
411+
* - mail_failed SMTP error reported by Symfony Mailer (full
412+
* cause already lives in `:prefix_log` by way
413+
* of `Mail::send`'s own catch block — we don't
414+
* echo it to the client to avoid leaking
415+
* provider error strings that may carry
416+
* credentials / hostnames)
417+
*
418+
* Throttle: at most one send attempt per 10 seconds per panel
419+
* install. The scope is per-install (not per-user) because the abuse
420+
* surface is the outbound SMTP relay, not a per-user resource — two
421+
* admins racing each other shouldn't double the relay's load. We
422+
* stamp the throttle file BEFORE the SMTP I/O so a slow / hung
423+
* connection can't be hammered while a first call is mid-handshake
424+
* (so a series of mail_failed outcomes ALSO consume rate-limit
425+
* slots — intentional). The counter is a single-int cache file
426+
* under SB_CACHE, mirroring the shape
427+
* `_api_system_release_save_cache` uses (atomic tempfile +
428+
* `rename()`). Stale-while-error: if the cache directory is
429+
* unwritable we let the send through rather than fail closed —
430+
* losing the rate limit is preferable to losing the diagnostic
431+
* affordance the operator opened this surface to use.
432+
*
433+
* Throttle interaction with api_system_clear_cache: that handler
434+
* removes every file in SB_CACHE, including this throttle file. An
435+
* operator who hits rate_limited and then clicks "Clear cache"
436+
* resets the throttle. Acceptable because (a) both surfaces are
437+
* gated by the same ADMIN_OWNER | ADMIN_WEB_SETTINGS mask, so the
438+
* "throttle bypass" doesn't escalate privileges; (b) the threat
439+
* model the throttle defends is a *script* hammering this endpoint,
440+
* not a settings-admin abusing their own cache button.
441+
*
442+
* Audit: every send (success OR mail_failed) lands an entry in
443+
* `:prefix_log` so test sends can't be used to silently probe SMTP
444+
* credentials or enumerate valid relay endpoints. The success entry
445+
* names the recipient (which is operator-controlled — by default the
446+
* operator's own email); the failure entry adds the SMTP error class
447+
* for diagnostics. The validation / smtp_not_configured / rate_limited
448+
* branches don't log (they never reach the SMTP wire — same shape
449+
* `api_system_send_mail` uses).
450+
*
451+
* @param array{to?: string} $params
452+
* @return array{to: string, sent_at: int}
453+
*/
454+
function api_system_test_email(array $params): array
455+
{
456+
global $userbank;
457+
458+
// Resolve the calling admin's display name via $userbank rather
459+
// than the legacy `global $username` shape — `$username` is only
460+
// set by the install-wizard pages (web/install/pages/page.[2-6].php)
461+
// and the PHPUnit ApiTestCase shim; the JSON-API lifecycle in
462+
// production never assigns it, so `global $username` would
463+
// resolve to an unset variable, emitting empty strings into the
464+
// audit log and the email body's {admin} placeholder. The same
465+
// bug-shape lives in api_system_send_mail / api_account_* and
466+
// is a separate cleanup — don't propagate it forward here. See
467+
// the broader sweep tracked alongside this PR.
468+
$adminName = (string) ($userbank->GetProperty('user') ?? '');
469+
470+
$to = trim((string) ($params['to'] ?? ''));
471+
if ($to === '') {
472+
// Default to the logged-in admin's email so an operator who
473+
// just wants to "send the test to me" doesn't have to retype
474+
// their own address (which they can read off Admin → My
475+
// account if it slipped their mind).
476+
$to = (string) ($userbank->GetProperty('email') ?? '');
477+
}
478+
if ($to === '') {
479+
throw new ApiError(
480+
'validation',
481+
'Enter a recipient email address (no email is configured on your admin account).',
482+
'to',
483+
);
484+
}
485+
// FILTER_VALIDATE_EMAIL enforces RFC 822 grammar. PHP's
486+
// implementation also rejects strings well over RFC 5321's
487+
// 254-octet forward-path cap in every build the panel
488+
// supports (the regex is internally bounded), but exact
489+
// byte-edge behavior varies subtly across PHP versions, so
490+
// don't rely on this as a hard size gate. If a future
491+
// bug-report surfaces a 255-byte address slipping through,
492+
// add an explicit `strlen($to) > 254` guard ahead of the
493+
// filter — for now the filter is sufficient in practice.
494+
if (filter_var($to, FILTER_VALIDATE_EMAIL) === false) {
495+
throw new ApiError('validation', 'Recipient email is not a valid address.', 'to');
496+
}
497+
498+
// Short-circuit on the smtp.host / smtp.user / smtp.pass-empty
499+
// arm so the user sees actionable copy ("configure SMTP first")
500+
// instead of the generic mail_failed branch. Mail::send itself
501+
// also returns false in this state but only logs to
502+
// `:prefix_log` — easy to miss. We deliberately don't validate
503+
// smtp.port: Mailer defaults to 25, which is the right shape
504+
// for a local relay (the most common test target).
505+
if (Mailer::create() === null) {
506+
throw new ApiError(
507+
'smtp_not_configured',
508+
'SMTP host, username, or password is empty. Configure SMTP first and save the form, then try again.',
509+
);
510+
}
511+
512+
// Rate limit: at most 1 send per 10s, scoped to the entire panel
513+
// install (not the calling admin). The throttle window is short
514+
// enough that a legitimate operator iterating on SMTP settings
515+
// isn't blocked, but long enough that a script can't flood the
516+
// outbound relay (a 10s window plus the audit-log row per send
517+
// makes burst abuse both bandwidth-bounded AND traceable).
518+
$throttleSeconds = 10;
519+
$now = time();
520+
$cachePath = SB_CACHE . 'test-email-throttle';
521+
$previous = @file_get_contents($cachePath);
522+
if ($previous !== false) {
523+
$previousTs = (int) trim((string) $previous);
524+
$elapsed = $now - $previousTs;
525+
if ($previousTs > 0 && $elapsed >= 0 && $elapsed < $throttleSeconds) {
526+
$retryAfter = $throttleSeconds - $elapsed;
527+
throw new ApiError(
528+
'rate_limited',
529+
sprintf('Test email throttled — try again in %d seconds.', $retryAfter),
530+
);
531+
}
532+
}
533+
534+
// Stamp the throttle file BEFORE the SMTP I/O so a slow / hung
535+
// SMTP connection (e.g. operator pointed `smtp.host` at an
536+
// unroutable address) can't be hammered while the first call is
537+
// still mid-handshake. We write atomically via tempfile+rename
538+
// (same shape `_api_system_release_save_cache` uses) so a
539+
// crashing PHP process leaves either the old timestamp or the
540+
// new one — never a half-written file.
541+
//
542+
// `tempnam()` falls back to `sys_get_temp_dir()` (typically
543+
// `/tmp`) when SB_CACHE is unwritable. If `/tmp` and SB_CACHE
544+
// are on different filesystems (common in containerized
545+
// deployments where `/tmp` is host tmpfs and the panel cache
546+
// is a bind mount), `rename()` fails with EXDEV and the tmp
547+
// file would leak if we didn't unlink it on the failure
548+
// branch. So: unlink whenever either the write OR the rename
549+
// misses, not just the write.
550+
if (!is_dir(SB_CACHE)) {
551+
@mkdir(SB_CACHE, 0o775, true);
552+
}
553+
$tmpPath = @tempnam(SB_CACHE, 'tem');
554+
if ($tmpPath !== false) {
555+
if (@file_put_contents($tmpPath, (string) $now) === false
556+
|| !@rename($tmpPath, $cachePath)
557+
) {
558+
@unlink($tmpPath);
559+
}
560+
}
561+
562+
// Both subject slots emit the same string: the email header
563+
// subject AND the body's `<strong>{subject}</strong>` slot in
564+
// `contact_custom.html`. They're rendered in different
565+
// surfaces (mail-client subject line vs HTML body), so a
566+
// recipient seeing one identifier in their inbox and a
567+
// different one in the body would be confusing. The
568+
// `[SourceBans++]` prefix is what their mail client surfaces
569+
// alongside other notifications from the panel.
570+
$subject = '[SourceBans++] SMTP test email';
571+
$message = "This is a test email from SourceBans++ confirming your SMTP credentials are working."
572+
. " It was triggered manually by admin '" . $adminName . "' from Admin → Settings → SMTP."
573+
. " You can safely ignore it — no action is required.";
574+
$sent = Mail::send($to, EmailType::Custom, [
575+
'{message}' => $message,
576+
'{subject}' => $subject,
577+
'{admin}' => $adminName,
578+
'{link}' => Host::complete(true),
579+
'{home}' => Host::complete(true),
580+
], $subject);
581+
582+
if (!$sent) {
583+
// The full cause already lives in `:prefix_log` (Mail::send
584+
// catches the Throwable from Symfony Mailer and emits a
585+
// `LogType::Error, 'Mail error', $e->getMessage()` row).
586+
// Mirror that with a paired audit entry so the success /
587+
// failure ratio is visible in the audit log without having
588+
// to cross-reference timestamps.
589+
Log::add(LogType::Warning, 'Test email failed', "Admin '$adminName' tried to send a test email to '$to'; see prior 'Mail error' entry for the SMTP-side cause.");
590+
throw new ApiError(
591+
'mail_failed',
592+
'SMTP send failed. Check the audit log under Admin → System Log for the cause.',
593+
);
594+
}
595+
596+
Log::add(LogType::Message, 'Test email sent', "Admin '$adminName' sent a SMTP test email to '$to'.");
597+
598+
return [
599+
'to' => $to,
600+
'sent_at' => $now,
601+
];
602+
}
603+
379604
/**
380605
* Render an admin-authored Markdown snippet (currently used by the
381606
* dashboard `dash.intro.text` setting; #1207 SET-1) through the same

web/includes/View/AdminSettingsView.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,14 @@ public function __construct(
7878
public readonly bool $banlist_nocountryfetch,
7979
public readonly bool $banlist_hideplayerips,
8080
public readonly bool $config_smtp_verify_peer,
81+
// #1455: pre-populated recipient for the SMTP test-email
82+
// button. Defaults to the logged-in admin's email so the
83+
// operator doesn't have to retype their own address to
84+
// verify SMTP works; can be edited inline before triggering
85+
// the send. May be empty when the admin row has no email
86+
// on file (the handler then requires the operator to type
87+
// a destination before firing).
88+
public readonly string $admin_email,
8189
) {
8290
}
8391
}

web/pages/admin.settings.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -503,6 +503,11 @@ function adminSettingsHasSteamApiKey(): bool
503503
config_smtp_verify_peer: Config::getBool('smtp.verify_peer'),
504504
config_mail_from_email: (string) Config::get('config.mail.from_email'),
505505
config_mail_from_name: (string) Config::get('config.mail.from_name'),
506+
// #1455: SMTP test-email button default recipient — current
507+
// admin's email if any. The page-tail JS keeps the input
508+
// editable so an operator can send the test to a different
509+
// mailbox (e.g. a shared on-call address).
510+
admin_email: (string) ($userbank->GetProperty('email') ?? ''),
506511
));
507512
}
508513

web/scripts/api-contract.js

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -548,6 +548,58 @@
548548
* @typedef {Object} ApiSystemSendMailRequest
549549
* @typedef {Object} ApiSystemSendMailResponse
550550
*/
551+
/**
552+
* Issue #1455: send a SMTP test message so an operator can verify the
553+
* credentials they just typed into Admin → Settings actually work end-to-end
554+
* (Symfony Mailer → SMTP relay → recipient inbox) without having to wait
555+
* for a real password-reset / ban-protest event to fire the next outbound
556+
* mail. Permission gate: ADMIN_OWNER | ADMIN_WEB_SETTINGS. Matches the other
557+
* settings-page-only handlers (sel_theme / apply_theme / clear_cache /
558+
* preview_intro_text); only an operator who can edit SMTP settings has a
559+
* legitimate reason to trigger a test send. Error-envelope contract: -
560+
* validation missing or malformed recipient - smtp_not_configured
561+
* smtp.host / smtp.user / smtp.pass empty (we short-circuit BEFORE Mail::send
562+
* so the user gets a pointed message instead of the generic mail_failed branch
563+
* — `Mail::send`'s internal `Mailer::create() === null` arm also returns
564+
* false but only logs to `:prefix_log`) - rate_limited too many sends in
565+
* the throttle window; the message interpolates "try again in Ns" so the
566+
* client can echo it directly via a toast — there's no separate
567+
* machine-readable `retry_after_seconds` field on the wire (ApiError only
568+
* carries code + message + field + httpStatus; if a future client needs a
569+
* numeric value, parse it from the message or extend ApiError with a `details`
570+
* payload). - mail_failed SMTP error reported by Symfony Mailer (full
571+
* cause already lives in `:prefix_log` by way of `Mail::send`'s own catch
572+
* block — we don't echo it to the client to avoid leaking provider error
573+
* strings that may carry credentials / hostnames) Throttle: at most one send
574+
* attempt per 10 seconds per panel install. The scope is per-install (not
575+
* per-user) because the abuse surface is the outbound SMTP relay, not a
576+
* per-user resource — two admins racing each other shouldn't double the
577+
* relay's load. We stamp the throttle file BEFORE the SMTP I/O so a slow /
578+
* hung connection can't be hammered while a first call is mid-handshake (so a
579+
* series of mail_failed outcomes ALSO consume rate-limit slots —
580+
* intentional). The counter is a single-int cache file under SB_CACHE,
581+
* mirroring the shape `_api_system_release_save_cache` uses (atomic tempfile +
582+
* `rename()`). Stale-while-error: if the cache directory is unwritable we let
583+
* the send through rather than fail closed — losing the rate limit is
584+
* preferable to losing the diagnostic affordance the operator opened this
585+
* surface to use. Throttle interaction with api_system_clear_cache: that
586+
* handler removes every file in SB_CACHE, including this throttle file. An
587+
* operator who hits rate_limited and then clicks "Clear cache" resets the
588+
* throttle. Acceptable because (a) both surfaces are gated by the same
589+
* ADMIN_OWNER | ADMIN_WEB_SETTINGS mask, so the "throttle bypass" doesn't
590+
* escalate privileges; (b) the threat model the throttle defends is a *script*
591+
* hammering this endpoint, not a settings-admin abusing their own cache
592+
* button. Audit: every send (success OR mail_failed) lands an entry in
593+
* `:prefix_log` so test sends can't be used to silently probe SMTP credentials
594+
* or enumerate valid relay endpoints. The success entry names the recipient
595+
* (which is operator-controlled — by default the operator's own email); the
596+
* failure entry adds the SMTP error class for diagnostics. The validation /
597+
* smtp_not_configured / rate_limited branches don't log (they never reach the
598+
* SMTP wire — same shape `api_system_send_mail` uses).
599+
*
600+
* @typedef {Object} ApiSystemTestEmailRequest
601+
* @typedef {{to: string, sent_at: number}} ApiSystemTestEmailResponse
602+
*/
551603

552604
/**
553605
* Action names accepted by sb.api.call(). Keys are PascalCase derived from
@@ -621,6 +673,7 @@ var Actions = Object.freeze({
621673
SystemRehashAdmins: 'system.rehash_admins',
622674
SystemSelTheme: 'system.sel_theme',
623675
SystemSendMail: 'system.send_mail',
676+
SystemTestEmail: 'system.test_email',
624677
});
625678

626679
/**

web/tests/api/PermissionMatrixTest.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,9 @@ public static function expectedMatrix(): array
177177
'system.apply_theme' => ['perm' => ADMIN_OWNER | ADMIN_WEB_SETTINGS, 'requireAdmin' => false, 'public' => false],
178178
'system.clear_cache' => ['perm' => ADMIN_OWNER | ADMIN_WEB_SETTINGS, 'requireAdmin' => false, 'public' => false],
179179
'system.preview_intro_text' => ['perm' => ADMIN_OWNER | ADMIN_WEB_SETTINGS, 'requireAdmin' => false, 'public' => false],
180+
// #1455: SMTP test-email button — only operators who can edit
181+
// SMTP credentials have a reason to trigger the verification send.
182+
'system.test_email' => ['perm' => ADMIN_OWNER | ADMIN_WEB_SETTINGS, 'requireAdmin' => false, 'public' => false],
180183
];
181184
}
182185

0 commit comments

Comments
 (0)