mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
fix(notifications): the smtp test names its failure and the log carries it (#2196)
testSmtp logged nothing and the transport ran on nodemailer's two-minute defaults while the client gave up after eight seconds, so every failure was a generic toast over an empty log. The transport gets real timeouts, every outcome is logged without credentials, and a classified, scrubbed cause (auth, DNS, refused, filtered port, TLS mismatch, certificate) goes into the response the admin sees; the channel test calls wait long enough to receive it. The password field also rendered its mask as value, so replacing a stored password silently saved the bullets as part of the new one; it is a placeholder now, on desktop and the phone admin.
This commit is contained in:
@@ -864,4 +864,21 @@ describe('client > multipart uploads', () => {
|
||||
expect(post.mock.calls[1][0]).toBe('/backup/upload-restore')
|
||||
expect(((post.mock.calls[1][1] as FormData).get('backup') as File).name).toBe('backup.zip')
|
||||
})
|
||||
|
||||
it('FE-APISURF-053: every channel test outlives the 8s global timeout', async () => {
|
||||
const post = spyPost()
|
||||
|
||||
// The server budgets up to 20s for an SMTP dial and 10s for a webhook or
|
||||
// ntfy ping; aborting at 8s threw away the reason and left the admin with a
|
||||
// bare "failed" (#2196).
|
||||
await notificationsApi.testSmtp('a@b.c')
|
||||
await notificationsApi.testWebhook('https://hook')
|
||||
await notificationsApi.testNtfy({ topic: 't' })
|
||||
await notificationsApi.testChannel('plugin/ch')
|
||||
|
||||
expect(post.mock.calls).toHaveLength(4)
|
||||
for (const call of post.mock.calls) {
|
||||
expect(call[2]).toMatchObject({ timeout: 40000 })
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1232,15 +1232,21 @@ export const tripInviteApi = {
|
||||
accept: (token: string) => apiClient.post(`/trip-invites/${token}/accept`).then(r => r.data),
|
||||
}
|
||||
|
||||
// A channel test dials a third party the admin just typed in, and the server
|
||||
// budgets for it: up to 20s for a wrong SMTP port, 10s for a webhook or ntfy
|
||||
// endpoint. The 8s instance timeout aborted those before the reason came back,
|
||||
// so the toast could only ever say "failed" (#2196).
|
||||
const CHANNEL_TEST_TIMEOUT = 40000
|
||||
|
||||
export const notificationsApi = {
|
||||
getPreferences: () => apiClient.get('/notifications/preferences').then(r => r.data),
|
||||
updatePreferences: (prefs: Record<string, Record<string, boolean>>) => apiClient.put('/notifications/preferences', prefs).then(r => r.data),
|
||||
testSmtp: (email?: string) => apiClient.post('/notifications/test-smtp', { email }).then(r => checkInDev(channelTestResultSchema, r.data, 'notifications.testSmtp')),
|
||||
testWebhook: (url?: string) => apiClient.post('/notifications/test-webhook', { url }).then(r => checkInDev(channelTestResultSchema, r.data, 'notifications.testWebhook')),
|
||||
testNtfy: (payload: { topic?: string; server?: string | null; token?: string | null }) => apiClient.post('/notifications/test-ntfy', payload).then(r => checkInDev(channelTestResultSchema, r.data, 'notifications.testNtfy')),
|
||||
testSmtp: (email?: string) => apiClient.post('/notifications/test-smtp', { email }, { timeout: CHANNEL_TEST_TIMEOUT }).then(r => checkInDev(channelTestResultSchema, r.data, 'notifications.testSmtp')),
|
||||
testWebhook: (url?: string) => apiClient.post('/notifications/test-webhook', { url }, { timeout: CHANNEL_TEST_TIMEOUT }).then(r => checkInDev(channelTestResultSchema, r.data, 'notifications.testWebhook')),
|
||||
testNtfy: (payload: { topic?: string; server?: string | null; token?: string | null }) => apiClient.post('/notifications/test-ntfy', payload, { timeout: CHANNEL_TEST_TIMEOUT }).then(r => checkInDev(channelTestResultSchema, r.data, 'notifications.testNtfy')),
|
||||
// Generic channel test — this is how a PLUGIN channel's "Send test" button works.
|
||||
testChannel: (channelId: string) =>
|
||||
apiClient.post(`/notifications/test/${encodeURIComponent(channelId)}`)
|
||||
apiClient.post(`/notifications/test/${encodeURIComponent(channelId)}`, undefined, { timeout: CHANNEL_TEST_TIMEOUT })
|
||||
.then(r => checkInDev(channelTestResultSchema, r.data, 'notifications.testChannel')),
|
||||
}
|
||||
|
||||
|
||||
@@ -187,9 +187,12 @@ export default function MAdminNotificationsSection({ admin, t }: MAdminNotificat
|
||||
{smtpLoaded &&
|
||||
SMTP_FIELDS.map((field) => (
|
||||
<MAdminField key={field.key} label={field.label}>
|
||||
{/* A stored password comes back masked. Showing the mask as the VALUE meant
|
||||
typing a new one appended it to eight bullet characters and saved that,
|
||||
so the same treatment as the webhook URL below: mask as placeholder. */}
|
||||
<MAdminInput
|
||||
type={field.type || 'text'}
|
||||
value={smtpValues[field.key] || ''}
|
||||
value={smtpValues[field.key] === '••••••••' ? '' : smtpValues[field.key] || ''}
|
||||
onChange={(e) => setSmtpValues((prev) => ({ ...prev, [field.key]: e.target.value }))}
|
||||
placeholder={field.placeholder}
|
||||
/>
|
||||
|
||||
@@ -539,4 +539,24 @@ describe('AdminNotificationsTab', () => {
|
||||
|
||||
await waitFor(() => expect(admin.toast.error).toHaveBeenCalledWith('Error'));
|
||||
});
|
||||
|
||||
it('FE-ADMNOT-042: a stored SMTP password is a placeholder, so a new one replaces it', async () => {
|
||||
let body: Record<string, unknown> | null = null;
|
||||
server.use(
|
||||
http.put('/api/auth/app-settings', async ({ request }) => {
|
||||
body = (await request.json()) as Record<string, unknown>;
|
||||
return HttpResponse.json({});
|
||||
}),
|
||||
http.get('/api/auth/app-config', () => HttpResponse.json({}))
|
||||
);
|
||||
render(<StatefulHarness initial={{ ...EMAIL_ON, smtp_pass: '••••••••' }} />);
|
||||
|
||||
// Typing into the eight bullet characters used to save them along with the
|
||||
// password, which then failed to authenticate with nothing in the log.
|
||||
expect(screen.getByPlaceholderText('••••••••')).toHaveValue('');
|
||||
fireEvent.change(screen.getByPlaceholderText('••••••••'), { target: { value: 'hunter2' } });
|
||||
fireEvent.click(within(card('Email (SMTP)')).getByRole('button', { name: /^save$/i }));
|
||||
|
||||
await waitFor(() => expect(body).toEqual({ smtp_host: 'mail.example.com', smtp_pass: 'hunter2' }));
|
||||
});
|
||||
});
|
||||
|
||||
@@ -87,9 +87,12 @@ export default function AdminNotificationsTab({ admin, t }: AdminNotificationsTa
|
||||
].map(field => (
|
||||
<div key={field.key}>
|
||||
<label className="block text-xs font-medium text-slate-500 mb-1">{field.label}</label>
|
||||
{/* A stored password comes back masked. Showing the mask as the VALUE meant
|
||||
typing a new one appended it to eight bullet characters and saved that,
|
||||
so the same treatment as the webhook URL below: mask as placeholder. */}
|
||||
<input
|
||||
type={field.type || 'text'}
|
||||
value={smtpValues[field.key] || ''}
|
||||
value={smtpValues[field.key] === '••••••••' ? '' : smtpValues[field.key] || ''}
|
||||
onChange={e => setSmtpValues(prev => ({ ...prev, [field.key]: e.target.value }))}
|
||||
placeholder={field.placeholder}
|
||||
className="w-full px-3 py-2 border border-slate-300 rounded-lg text-sm focus:ring-2 focus:ring-slate-400 focus:border-transparent"
|
||||
|
||||
@@ -65,6 +65,10 @@ function ntfyTokenInput() {
|
||||
return Array.from(document.querySelectorAll<HTMLInputElement>('input[type="password"]'))[1];
|
||||
}
|
||||
|
||||
function smtpPasswordInput() {
|
||||
return Array.from(document.querySelectorAll<HTMLInputElement>('input[type="password"]'))[0];
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
resetAllStores();
|
||||
});
|
||||
@@ -537,4 +541,21 @@ describe('MAdminNotificationsSection', () => {
|
||||
expect(setTripRemindersEnabled).not.toHaveBeenCalled();
|
||||
expect(toast.error).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('FE-MOB-ANOTIF-032: a stored SMTP password is a placeholder, so a new one replaces it', async () => {
|
||||
const bodies = captureAppSettings();
|
||||
const user = userEvent.setup();
|
||||
render(<Harness initial={{ smtp_host: 'mail.test', smtp_pass: MASK }} />);
|
||||
|
||||
const password = smtpPasswordInput();
|
||||
expect(password).toHaveValue('');
|
||||
|
||||
await user.type(password, 'hunter2');
|
||||
await user.click(screen.getAllByRole('button', { name: 'Save' })[0]);
|
||||
|
||||
// Typing into the eight bullet characters used to save them along with the
|
||||
// password, which then failed to authenticate with nothing in the log.
|
||||
await waitFor(() => expect(bodies).toHaveLength(1));
|
||||
expect(bodies[0].smtp_pass).toBe('hunter2');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -6,6 +6,7 @@ import { logError, logInfo, logDebug, logWarn } from '../../audit/audit-log.logg
|
||||
import { decrypt_api_key } from '../../common/crypto/apiKeyCrypto';
|
||||
import { DatabaseService } from '../../database/database.service';
|
||||
import { buildEmailHtml, buildPasswordResetHtml } from './email-html';
|
||||
import { describeSmtpFailure, describeSmtpGap, parseSmtpPort, type SmtpTarget } from './smtp-diagnostics';
|
||||
|
||||
interface SmtpConfig {
|
||||
host: string;
|
||||
@@ -16,6 +17,20 @@ interface SmtpConfig {
|
||||
secure: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* Nodemailer's own defaults are 2 minutes to connect and 30 seconds for the
|
||||
* greeting, both of which outlive the browser's 8-second API timeout: an admin
|
||||
* pressing "Send test email" against a blocked port got a dead "Test email
|
||||
* failed" while the socket was still waiting, and the eventual error had nobody
|
||||
* left to tell. A relay that needs more than ten seconds to accept a connection
|
||||
* or to send its banner is broken either way, so those two are cut hard. The
|
||||
* inactivity timeout stays generous (nodemailer's default is ten minutes)
|
||||
* because a relay that scans the message can be slow to acknowledge DATA.
|
||||
*/
|
||||
const CONNECT_TIMEOUT_MS = 10_000;
|
||||
const GREETING_TIMEOUT_MS = 10_000;
|
||||
const SOCKET_TIMEOUT_MS = 30_000;
|
||||
|
||||
/**
|
||||
* Outgoing mail: SMTP config resolution, the password-reset delivery and the
|
||||
* generic notification email.
|
||||
@@ -36,21 +51,31 @@ export class MailerService {
|
||||
return this.db.get<{ value: string }>('SELECT value FROM app_settings WHERE key = ?', key)?.value || null;
|
||||
}
|
||||
|
||||
private getSmtpConfig(): SmtpConfig | null {
|
||||
/** Env wins over the admin panel, per field. Read fresh on every send. */
|
||||
private readSmtpSettings() {
|
||||
const smtpEnv = readEnv().smtp;
|
||||
const host = smtpEnv.host || this.getAppSetting('smtp_host');
|
||||
const port = smtpEnv.port || this.getAppSetting('smtp_port');
|
||||
const user = smtpEnv.user || this.getAppSetting('smtp_user');
|
||||
const pass = smtpEnv.pass || decrypt_api_key(this.getAppSetting('smtp_pass')) || '';
|
||||
const from = smtpEnv.from || this.getAppSetting('smtp_from');
|
||||
if (!host || !port || !from) return null;
|
||||
return {
|
||||
host: smtpEnv.host || this.getAppSetting('smtp_host'),
|
||||
port: smtpEnv.port || this.getAppSetting('smtp_port'),
|
||||
user: smtpEnv.user || this.getAppSetting('smtp_user'),
|
||||
pass: smtpEnv.pass || decrypt_api_key(this.getAppSetting('smtp_pass')) || '',
|
||||
from: smtpEnv.from || this.getAppSetting('smtp_from'),
|
||||
};
|
||||
}
|
||||
|
||||
private getSmtpConfig(): SmtpConfig | null {
|
||||
const { host, port, user, pass, from } = this.readSmtpSettings();
|
||||
// An unusable port counts as unconfigured rather than as a dial: net.connect
|
||||
// on NaN throws from inside nodemailer, where the reason gets lost.
|
||||
const portNumber = parseSmtpPort(port);
|
||||
if (!host || !portNumber || !from) return null;
|
||||
return {
|
||||
host,
|
||||
port: Number.parseInt(port, 10),
|
||||
port: portNumber,
|
||||
user: user || '',
|
||||
pass: pass || '',
|
||||
from,
|
||||
secure: Number.parseInt(port, 10) === 465,
|
||||
secure: portNumber === 465,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -69,6 +94,9 @@ export class MailerService {
|
||||
port: config.port,
|
||||
secure: config.secure,
|
||||
auth: config.user ? { user: config.user, pass: config.pass } : undefined,
|
||||
connectionTimeout: CONNECT_TIMEOUT_MS,
|
||||
greetingTimeout: GREETING_TIMEOUT_MS,
|
||||
socketTimeout: SOCKET_TIMEOUT_MS,
|
||||
// The operator's opt-out for a relay with a self-signed certificate, off by
|
||||
// default and reachable only through SMTP_SKIP_TLS_VERIFY or the matching
|
||||
// admin setting. It stays because self-hosted installs behind an internal
|
||||
@@ -155,7 +183,7 @@ export class MailerService {
|
||||
logInfo(`Password reset email sent to=${to}`);
|
||||
return { delivered: 'email' };
|
||||
} catch (err) {
|
||||
logError(`Password reset email failed to=${to}: ${err instanceof Error ? err.message : err}`);
|
||||
logError(`Password reset email failed to=${to} ${this.explain(err, smtpCfg)}`);
|
||||
return { delivered: 'failed' };
|
||||
}
|
||||
}
|
||||
@@ -184,14 +212,26 @@ export class MailerService {
|
||||
logDebug(`Email smtp=${config.host}:${config.port} from=${config.from} to=${to}`);
|
||||
return true;
|
||||
} catch (err) {
|
||||
logError(`Email send failed to=${to}: ${err instanceof Error ? err.message : err}`);
|
||||
logError(`Email send failed to=${to} ${this.explain(err, config)}`);
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The admin's "Send test email" button. Unlike the delivery paths this one has
|
||||
* somebody waiting for an answer, so the classified reason goes into the
|
||||
* response as well as the log. Issue #2196 was an admin staring at "Test email
|
||||
* failed" with an empty container log, unable to tell a wrong password from a
|
||||
* blocked port.
|
||||
*/
|
||||
async testSmtp(to: string): Promise<{ success: boolean; error?: string }> {
|
||||
const config = this.getSmtpConfig();
|
||||
if (!config) return { success: false, error: 'SMTP not configured' };
|
||||
if (!config) {
|
||||
const { host, port, from } = this.readSmtpSettings();
|
||||
const reason = describeSmtpGap({ host, port, from });
|
||||
logWarn(`SMTP test not attempted to=${to}: ${reason}`);
|
||||
return { success: false, error: reason };
|
||||
}
|
||||
try {
|
||||
await this.createTransport(config).sendMail({
|
||||
from: config.from,
|
||||
@@ -199,9 +239,27 @@ export class MailerService {
|
||||
subject: 'TREK — Test Notification',
|
||||
text: 'This is a test email from TREK. If you received this, your SMTP configuration is working correctly.',
|
||||
});
|
||||
logInfo(`SMTP test email sent to=${to} ${this.describeTarget(config)}`);
|
||||
return { success: true };
|
||||
} catch (err) {
|
||||
return { success: false, error: err instanceof Error ? err.message : 'Unknown error' };
|
||||
const failure = describeSmtpFailure(err, this.target(config), config.pass);
|
||||
logError(`SMTP test email failed to=${to} ${this.describeTarget(config)} code=${failure.code}: ${failure.reason}`);
|
||||
return { success: false, error: failure.reason };
|
||||
}
|
||||
}
|
||||
|
||||
private target(config: SmtpConfig): SmtpTarget {
|
||||
return { host: config.host, port: config.port, secure: config.secure };
|
||||
}
|
||||
|
||||
/** The tail every failed-send log line carries: where it went, and why it broke. */
|
||||
private explain(err: unknown, config: SmtpConfig): string {
|
||||
const failure = describeSmtpFailure(err, this.target(config), config.pass);
|
||||
return `${this.describeTarget(config)} code=${failure.code}: ${failure.reason}`;
|
||||
}
|
||||
|
||||
/** Never the user or the password: the relay and how it was dialled. */
|
||||
private describeTarget(config: SmtpConfig): string {
|
||||
return `smtp=${config.host}:${config.port} secure=${config.secure} auth=${config.user ? 'yes' : 'no'}`;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,111 @@
|
||||
/**
|
||||
* Turning an SMTP failure into something an admin can act on.
|
||||
*
|
||||
* Nodemailer throws an Error carrying `code`, `command` and, when the relay said
|
||||
* something back, `response` (already appended to the message). None of it used
|
||||
* to reach anywhere: the test-email button logged nothing at all, so a wrong
|
||||
* password and a blocked port looked identical from the outside. The classifier
|
||||
* below keeps the relay's own words, which are the most useful part, and puts
|
||||
* the diagnosis in front of them.
|
||||
*
|
||||
* Everything here is pure, so the wording can be pinned in unit tests without a
|
||||
* socket.
|
||||
*/
|
||||
|
||||
export interface SmtpTarget {
|
||||
host: string;
|
||||
port: number;
|
||||
secure: boolean;
|
||||
}
|
||||
|
||||
export interface SmtpFailure {
|
||||
/** Nodemailer's error code, or 'UNKNOWN' when the throw carried none. */
|
||||
code: string;
|
||||
/** Admin-facing and credential-free: safe for both the log and the response. */
|
||||
reason: string;
|
||||
}
|
||||
|
||||
const MAX_REASON_LENGTH = 400;
|
||||
const MAX_ECHOED_VALUE = 32;
|
||||
|
||||
/** A port TREK can actually dial, or null. Rejects '', 'abc', '0', '70000', '587abc'. */
|
||||
export function parseSmtpPort(raw: string | null | undefined): number | null {
|
||||
if (!raw) return null;
|
||||
const trimmed = String(raw).trim();
|
||||
if (!/^\d+$/.test(trimmed)) return null;
|
||||
const port = Number.parseInt(trimmed, 10);
|
||||
return port >= 1 && port <= 65535 ? port : null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Why getSmtpConfig() came back empty. Named per field: "SMTP not configured" on
|
||||
* its own sent admins hunting through a form where four of the five fields were
|
||||
* filled in.
|
||||
*/
|
||||
export function describeSmtpGap(raw: { host?: string | null; port?: string | null; from?: string | null }): string {
|
||||
if (raw.port && !parseSmtpPort(raw.port)) {
|
||||
return `SMTP not configured: the port "${clip(String(raw.port), MAX_ECHOED_VALUE)}" is not a number between 1 and 65535.`;
|
||||
}
|
||||
const missing: string[] = [];
|
||||
if (!raw.host) missing.push('host (SMTP_HOST)');
|
||||
if (!raw.port) missing.push('port (SMTP_PORT)');
|
||||
if (!raw.from) missing.push('from address (SMTP_FROM)');
|
||||
return `SMTP not configured: missing ${missing.join(', ')}. Set it under Admin > Notifications or as the matching environment variable.`;
|
||||
}
|
||||
|
||||
export function describeSmtpFailure(err: unknown, target: SmtpTarget, secret = ''): SmtpFailure {
|
||||
const thrown = (err ?? {}) as { code?: unknown; message?: unknown };
|
||||
const code = typeof thrown.code === 'string' && thrown.code ? thrown.code : 'UNKNOWN';
|
||||
const message = scrub(typeof thrown.message === 'string' && thrown.message ? thrown.message : String(err), secret);
|
||||
const where = `${target.host}:${target.port}`;
|
||||
|
||||
if (code === 'EAUTH') {
|
||||
return { code, reason: `${where} rejected the credentials. Check the SMTP user and password, and whether the account needs an app-specific password. Server said: ${message}` };
|
||||
}
|
||||
if (code === 'EDNS' || /ENOTFOUND|EAI_AGAIN/.test(message)) {
|
||||
return { code, reason: `The host ${target.host} could not be resolved: ${message}` };
|
||||
}
|
||||
if (/ECONNREFUSED/.test(message)) {
|
||||
return { code, reason: `${where} refused the connection: nothing is listening on that port, or a firewall closed it. ${portHint(target)}` };
|
||||
}
|
||||
if (/EHOSTUNREACH|ENETUNREACH|EACCES/.test(message)) {
|
||||
return { code, reason: `${where} is unreachable from the TREK container: ${message}` };
|
||||
}
|
||||
if (code === 'ETIMEDOUT' || /timed? ?out|Greeting never received/i.test(message)) {
|
||||
return { code, reason: `${where} did not answer in time: ${message}. Outbound mail ports are often blocked by the host or the hosting provider. ${portHint(target)}` };
|
||||
}
|
||||
if (/certificate|self.signed/i.test(message)) {
|
||||
return { code, reason: `The TLS certificate of ${where} was not accepted: ${message}. For an internal relay with its own certificate, turn on "Skip TLS certificate check".` };
|
||||
}
|
||||
if (/wrong version number|packet length too long|ssl|routines|tlsv1/i.test(message)) {
|
||||
return { code, reason: `The TLS handshake with ${where} failed: ${message}. ${portHint(target)}` };
|
||||
}
|
||||
if (code === 'EENVELOPE') {
|
||||
return { code, reason: `${where} rejected the envelope. The from address normally has to belong to the authenticated account. Server said: ${message}` };
|
||||
}
|
||||
if (code === 'EMESSAGE') {
|
||||
return { code, reason: `${where} accepted the connection but rejected the message: ${message}` };
|
||||
}
|
||||
return { code, reason: `Sending through ${where} failed (${code}): ${message}` };
|
||||
}
|
||||
|
||||
/**
|
||||
* The 465-versus-587 mix-up is the one that produces a silent hang rather than a
|
||||
* refusal, so every connection-level diagnosis carries it.
|
||||
*/
|
||||
function portHint(target: SmtpTarget): string {
|
||||
return target.secure
|
||||
? 'Port 465 is dialled with implicit TLS; a relay that expects STARTTLS listens on 587.'
|
||||
: `Port ${target.port} is dialled in plain mode with STARTTLS; a relay that expects TLS from the first byte listens on 465.`;
|
||||
}
|
||||
|
||||
/** The relay's words, minus anything that could carry the credential back out. */
|
||||
function scrub(text: string, secret: string): string {
|
||||
const collapsed = text.replace(/\s+/g, ' ').trim();
|
||||
const redacted = secret.length >= 4 ? collapsed.split(secret).join('***') : collapsed;
|
||||
return clip(redacted, MAX_REASON_LENGTH);
|
||||
}
|
||||
|
||||
function clip(text: string, max: number): string {
|
||||
return text.length > max ? `${text.slice(0, max)}...` : text;
|
||||
}
|
||||
@@ -4,6 +4,9 @@
|
||||
* Covers the SMTP transport options MailerService hands to nodemailer, and in
|
||||
* particular the skip-TLS opt-out: it must stay reachable for operators behind an
|
||||
* internal relay, and it must announce itself instead of downgrading quietly.
|
||||
* The second half covers what the admin sees when a send goes wrong (#2196):
|
||||
* bounded phases, a classified reason in the response, a line in the log, and
|
||||
* the password in neither.
|
||||
* Constructed directly (no TestingModule, repo convention).
|
||||
*/
|
||||
|
||||
@@ -46,7 +49,7 @@ import { runMigrations } from '../../../src/db/migrations';
|
||||
import { resetTestDb } from '../../helpers/test-db';
|
||||
import { MailerService } from '../../../src/nest/notifications/mailer/mailer.service';
|
||||
import { DatabaseService } from '../../../src/nest/database/database.service';
|
||||
import { logWarn } from '../../../src/nest/audit/audit-log.logger';
|
||||
import { logError, logInfo, logWarn } from '../../../src/nest/audit/audit-log.logger';
|
||||
|
||||
function setAppSetting(key: string, value: string): void {
|
||||
testDb.prepare('INSERT OR REPLACE INTO app_settings (key, value) VALUES (?, ?)').run(key, value);
|
||||
@@ -143,4 +146,107 @@ describe('MailerService TLS options', () => {
|
||||
expect(createTransport).not.toHaveBeenCalled();
|
||||
expect(sendMail).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('MAILER-007: every SMTP phase is bounded, so a dead relay fails instead of hanging', async () => {
|
||||
configureSmtp();
|
||||
|
||||
await newMailer().sendEmail('someone@example.com', 'Subject', 'Body');
|
||||
|
||||
// Nodemailer's own defaults (120s / 30s / 600s) all outlive the client's 8s
|
||||
// API timeout, which is how #2196 produced a failure nobody could read.
|
||||
expect(lastTransportOptions()).toMatchObject({
|
||||
connectionTimeout: 10_000,
|
||||
greetingTimeout: 10_000,
|
||||
socketTimeout: 30_000,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('MailerService test send', () => {
|
||||
/** The nodemailer shape: an Error carrying the SMTP code and the relay's reply. */
|
||||
function smtpError(message: string, code: string): Error {
|
||||
return Object.assign(new Error(message), { code });
|
||||
}
|
||||
|
||||
it('MAILER-008: an incomplete configuration names the fields that are missing', async () => {
|
||||
setAppSetting('smtp_host', 'mail.internal.example');
|
||||
setAppSetting('smtp_port', '587');
|
||||
|
||||
const result = await newMailer().testSmtp('admin@example.com');
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
expect(result.error).toContain('SMTP_FROM');
|
||||
expect(createTransport).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('MAILER-009: an unusable port is refused before anything reaches a socket', async () => {
|
||||
setAppSetting('smtp_host', 'mail.internal.example');
|
||||
setAppSetting('smtp_port', 'smtp.example.com');
|
||||
setAppSetting('smtp_from', 'trek@example.com');
|
||||
|
||||
const result = await newMailer().testSmtp('admin@example.com');
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
expect(result.error).toContain('smtp.example.com');
|
||||
expect(result.error).toContain('65535');
|
||||
expect(createTransport).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('MAILER-010: a rejected login comes back as a reason, not as a bare failure', async () => {
|
||||
configureSmtp();
|
||||
sendMail.mockRejectedValueOnce(smtpError('Invalid login: 535 5.7.8 Username and Password not accepted', 'EAUTH'));
|
||||
|
||||
const result = await newMailer().testSmtp('admin@example.com');
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
expect(result.error).toContain('rejected the credentials');
|
||||
expect(result.error).toContain('535 5.7.8');
|
||||
});
|
||||
|
||||
it('MAILER-011: a refused connection is classified and reaches the log with its code', async () => {
|
||||
configureSmtp();
|
||||
sendMail.mockRejectedValueOnce(smtpError('connect ECONNREFUSED 10.0.0.5:587', 'ESOCKET'));
|
||||
|
||||
const result = await newMailer().testSmtp('admin@example.com');
|
||||
|
||||
expect(result.error).toContain('refused the connection');
|
||||
expect(logError).toHaveBeenCalledTimes(1);
|
||||
const line = vi.mocked(logError).mock.calls[0][0];
|
||||
expect(line).toContain('SMTP test email failed to=admin@example.com');
|
||||
expect(line).toContain('smtp=mail.internal.example:587');
|
||||
expect(line).toContain('code=ESOCKET');
|
||||
});
|
||||
|
||||
it('MAILER-012: a stalled relay is named as a timeout and points at the 465/587 split', async () => {
|
||||
setAppSetting('smtp_host', 'mail.internal.example');
|
||||
setAppSetting('smtp_port', '465');
|
||||
setAppSetting('smtp_from', 'trek@example.com');
|
||||
sendMail.mockRejectedValueOnce(smtpError('Greeting never received', 'ETIMEDOUT'));
|
||||
|
||||
const result = await newMailer().testSmtp('admin@example.com');
|
||||
|
||||
expect(result.error).toContain('did not answer in time');
|
||||
expect(result.error).toContain('587');
|
||||
});
|
||||
|
||||
it('MAILER-013: the password appears in neither the response nor the log', async () => {
|
||||
configureSmtp();
|
||||
setAppSetting('smtp_user', 'trek@example.com');
|
||||
setAppSetting('smtp_pass', 'correct-horse-battery');
|
||||
sendMail.mockRejectedValueOnce(smtpError('Invalid login for correct-horse-battery', 'EAUTH'));
|
||||
|
||||
const result = await newMailer().testSmtp('admin@example.com');
|
||||
|
||||
expect(result.error).not.toContain('correct-horse-battery');
|
||||
expect(vi.mocked(logError).mock.calls[0][0]).not.toContain('correct-horse-battery');
|
||||
});
|
||||
|
||||
it('MAILER-014: a successful test leaves a line in the log naming the relay', async () => {
|
||||
configureSmtp();
|
||||
|
||||
expect(await newMailer().testSmtp('admin@example.com')).toEqual({ success: true });
|
||||
|
||||
const lines = vi.mocked(logInfo).mock.calls.map(call => call[0]);
|
||||
expect(lines.some(line => line.includes('SMTP test email sent to=admin@example.com smtp=mail.internal.example:587'))).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,204 @@
|
||||
/**
|
||||
* smtp-diagnostics.test.ts
|
||||
*
|
||||
* The wording an admin gets when a test email does not go out (#2196). Pure
|
||||
* functions, no socket and no DB: what is pinned here is that every common SMTP
|
||||
* failure comes back as a cause rather than as "failed", and that the credential
|
||||
* never rides along.
|
||||
*/
|
||||
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import {
|
||||
describeSmtpFailure,
|
||||
describeSmtpGap,
|
||||
parseSmtpPort,
|
||||
type SmtpTarget,
|
||||
} from '../../../src/nest/notifications/mailer/smtp-diagnostics';
|
||||
|
||||
const PLAIN: SmtpTarget = { host: 'mail.example.com', port: 587, secure: false };
|
||||
const IMPLICIT_TLS: SmtpTarget = { host: 'mail.example.com', port: 465, secure: true };
|
||||
|
||||
/** The nodemailer shape: an Error carrying the SMTP code and the relay's reply. */
|
||||
function smtpError(message: string, code?: string): Error {
|
||||
return code ? Object.assign(new Error(message), { code }) : new Error(message);
|
||||
}
|
||||
|
||||
describe('parseSmtpPort', () => {
|
||||
it('SMTPDIAG-001: accepts a port number, with or without surrounding space', () => {
|
||||
expect(parseSmtpPort('587')).toBe(587);
|
||||
expect(parseSmtpPort(' 465 ')).toBe(465);
|
||||
expect(parseSmtpPort('1')).toBe(1);
|
||||
expect(parseSmtpPort('65535')).toBe(65535);
|
||||
});
|
||||
|
||||
it('SMTPDIAG-002: refuses everything that would reach net.connect as NaN or out of range', () => {
|
||||
expect(parseSmtpPort(null)).toBeNull();
|
||||
expect(parseSmtpPort(undefined)).toBeNull();
|
||||
expect(parseSmtpPort('')).toBeNull();
|
||||
expect(parseSmtpPort('smtp.example.com')).toBeNull();
|
||||
expect(parseSmtpPort('587abc')).toBeNull();
|
||||
expect(parseSmtpPort('58.7')).toBeNull();
|
||||
expect(parseSmtpPort('-1')).toBeNull();
|
||||
expect(parseSmtpPort('0')).toBeNull();
|
||||
expect(parseSmtpPort('65536')).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('describeSmtpGap', () => {
|
||||
it('SMTPDIAG-003: lists every field that is still empty', () => {
|
||||
const gap = describeSmtpGap({ host: null, port: null, from: null });
|
||||
|
||||
expect(gap).toContain('SMTP_HOST');
|
||||
expect(gap).toContain('SMTP_PORT');
|
||||
expect(gap).toContain('SMTP_FROM');
|
||||
});
|
||||
|
||||
it('SMTPDIAG-004: names only the one that is missing', () => {
|
||||
const gap = describeSmtpGap({ host: 'mail.example.com', port: '587', from: null });
|
||||
|
||||
expect(gap).toContain('SMTP_FROM');
|
||||
expect(gap).not.toContain('SMTP_HOST');
|
||||
});
|
||||
|
||||
it('SMTPDIAG-005: a port that is not a port is reported as such, not as absent', () => {
|
||||
const gap = describeSmtpGap({ host: 'mail.example.com', port: 'ssl', from: null });
|
||||
|
||||
expect(gap).toContain('"ssl"');
|
||||
expect(gap).toContain('65535');
|
||||
expect(gap).not.toContain('missing');
|
||||
});
|
||||
|
||||
it('SMTPDIAG-006: an absurd port value is clipped before it is echoed back', () => {
|
||||
const gap = describeSmtpGap({ host: 'mail.example.com', port: 'x'.repeat(500), from: 'a@b.c' });
|
||||
|
||||
expect(gap.length).toBeLessThan(120);
|
||||
expect(gap).toContain('...');
|
||||
});
|
||||
});
|
||||
|
||||
describe('describeSmtpFailure', () => {
|
||||
it('SMTPDIAG-007: a rejected login points at the credentials and keeps the relay reply', () => {
|
||||
const failure = describeSmtpFailure(
|
||||
smtpError('Invalid login: 535 5.7.8 Username and Password not accepted', 'EAUTH'),
|
||||
PLAIN,
|
||||
);
|
||||
|
||||
expect(failure.code).toBe('EAUTH');
|
||||
expect(failure.reason).toContain('rejected the credentials');
|
||||
expect(failure.reason).toContain('535 5.7.8');
|
||||
});
|
||||
|
||||
it('SMTPDIAG-008: an unresolvable host is named as DNS', () => {
|
||||
const byCode = describeSmtpFailure(smtpError('getaddrinfo failed', 'EDNS'), PLAIN);
|
||||
const byMessage = describeSmtpFailure(smtpError('getaddrinfo ENOTFOUND mail.example.com', 'ESOCKET'), PLAIN);
|
||||
|
||||
expect(byCode.reason).toContain('could not be resolved');
|
||||
expect(byMessage.reason).toContain('could not be resolved');
|
||||
});
|
||||
|
||||
it('SMTPDIAG-009: a refused connection separates "closed port" from "wrong password"', () => {
|
||||
const failure = describeSmtpFailure(smtpError('connect ECONNREFUSED 10.0.0.5:587', 'ESOCKET'), PLAIN);
|
||||
|
||||
expect(failure.reason).toContain('refused the connection');
|
||||
expect(failure.reason).toContain('mail.example.com:587');
|
||||
});
|
||||
|
||||
it('SMTPDIAG-010: an unroutable relay is reported as unreachable', () => {
|
||||
const failure = describeSmtpFailure(smtpError('connect EHOSTUNREACH 10.0.0.5:587', 'ESOCKET'), PLAIN);
|
||||
|
||||
expect(failure.reason).toContain('unreachable');
|
||||
});
|
||||
|
||||
it('SMTPDIAG-011: a stalled greeting on 465 suggests the STARTTLS port', () => {
|
||||
const failure = describeSmtpFailure(smtpError('Greeting never received', 'ETIMEDOUT'), IMPLICIT_TLS);
|
||||
|
||||
expect(failure.reason).toContain('did not answer in time');
|
||||
expect(failure.reason).toContain('587');
|
||||
});
|
||||
|
||||
it('SMTPDIAG-012: a connection timeout on 587 suggests the implicit-TLS port', () => {
|
||||
const failure = describeSmtpFailure(smtpError('Connection timeout', 'ETIMEDOUT'), PLAIN);
|
||||
|
||||
expect(failure.reason).toContain('465');
|
||||
});
|
||||
|
||||
it('SMTPDIAG-013: an untrusted certificate points at the skip-verification switch', () => {
|
||||
const failure = describeSmtpFailure(
|
||||
smtpError('unable to verify the first certificate', 'ESOCKET'),
|
||||
IMPLICIT_TLS,
|
||||
);
|
||||
|
||||
expect(failure.reason).toContain('Skip TLS certificate check');
|
||||
});
|
||||
|
||||
it('SMTPDIAG-014: a handshake against a plain-text port is named as TLS', () => {
|
||||
const failure = describeSmtpFailure(
|
||||
smtpError('140A:error:0A00010B:SSL routines:ssl3_get_record:wrong version number', 'ESOCKET'),
|
||||
IMPLICIT_TLS,
|
||||
);
|
||||
|
||||
expect(failure.reason).toContain('TLS handshake');
|
||||
expect(failure.reason).toContain('587');
|
||||
});
|
||||
|
||||
it('SMTPDIAG-015: a rejected envelope points at the from address', () => {
|
||||
const failure = describeSmtpFailure(
|
||||
smtpError('Mail command failed: 550 5.7.1 Sender address rejected', 'EENVELOPE'),
|
||||
PLAIN,
|
||||
);
|
||||
|
||||
expect(failure.reason).toContain('from address');
|
||||
expect(failure.reason).toContain('550 5.7.1');
|
||||
});
|
||||
|
||||
it('SMTPDIAG-016: a rejected message says the connection itself was fine', () => {
|
||||
const failure = describeSmtpFailure(smtpError('Message failed: 552 too large', 'EMESSAGE'), PLAIN);
|
||||
|
||||
expect(failure.reason).toContain('rejected the message');
|
||||
});
|
||||
|
||||
it('SMTPDIAG-017: an unclassified failure still carries the code and the message', () => {
|
||||
const failure = describeSmtpFailure(smtpError('something odd happened', 'EPROTOCOL'), PLAIN);
|
||||
|
||||
expect(failure.code).toBe('EPROTOCOL');
|
||||
expect(failure.reason).toContain('EPROTOCOL');
|
||||
expect(failure.reason).toContain('something odd happened');
|
||||
});
|
||||
|
||||
it('SMTPDIAG-018: a throw that is not an Error is still described', () => {
|
||||
const failure = describeSmtpFailure('boom', PLAIN);
|
||||
|
||||
expect(failure.code).toBe('UNKNOWN');
|
||||
expect(failure.reason).toContain('boom');
|
||||
});
|
||||
|
||||
it('SMTPDIAG-019: the password is redacted wherever it turns up in the message', () => {
|
||||
const failure = describeSmtpFailure(
|
||||
smtpError('Invalid login for user with correct-horse-battery', 'EAUTH'),
|
||||
PLAIN,
|
||||
'correct-horse-battery',
|
||||
);
|
||||
|
||||
expect(failure.reason).not.toContain('correct-horse-battery');
|
||||
expect(failure.reason).toContain('***');
|
||||
});
|
||||
|
||||
it('SMTPDIAG-020: a secret too short to be one is left alone rather than blanking the text', () => {
|
||||
const failure = describeSmtpFailure(smtpError('bad login at a b c', 'EAUTH'), PLAIN, 'a b');
|
||||
|
||||
expect(failure.reason).toContain('bad login at a b c');
|
||||
});
|
||||
|
||||
it('SMTPDIAG-021: a runaway relay reply is collapsed and clipped', () => {
|
||||
const failure = describeSmtpFailure(smtpError(`550 ${'x'.repeat(2000)}`, 'EENVELOPE'), PLAIN);
|
||||
|
||||
expect(failure.reason.length).toBeLessThan(700);
|
||||
expect(failure.reason).toContain('...');
|
||||
});
|
||||
|
||||
it('SMTPDIAG-022: newlines in a multi-line reply become one readable line', () => {
|
||||
const failure = describeSmtpFailure(smtpError('550 first\n 550 second', 'EENVELOPE'), PLAIN);
|
||||
|
||||
expect(failure.reason).toContain('550 first 550 second');
|
||||
});
|
||||
});
|
||||
@@ -257,6 +257,30 @@ docker logs <container> 2>&1 | grep "OIDC"
|
||||
|
||||
---
|
||||
|
||||
## "Send test email" fails and says nothing else
|
||||
|
||||
**Cause:** older builds swallowed the SMTP error. Nothing was written to the container log, and the toast fell back to a bare *Test email failed*. A blocked port made it worse: nodemailer waited up to two minutes for the connection while the browser gave up after eight seconds, so the eventual error had nobody left to report to. Both are fixed. Every SMTP phase is now bounded, and the button names the cause.
|
||||
|
||||
**Fix:** press **Send test email** again and read the toast. The same diagnosis, plus the SMTP error code, is in the log:
|
||||
|
||||
```bash
|
||||
docker logs <container> 2>&1 | grep -E "SMTP test email (sent|failed)|SMTP test not attempted"
|
||||
```
|
||||
|
||||
| What the message says | What to change |
|
||||
|-----------------------|----------------|
|
||||
| `rejected the credentials` (`code=EAUTH`) | Wrong SMTP user or password. Mailboxes with 2FA normally need an app-specific password, not the account one. |
|
||||
| `refused the connection` | Nothing is listening on that port, or a firewall closed it. |
|
||||
| `did not answer in time` | The port is filtered, or 465 and 587 are swapped: TREK dials 465 with implicit TLS and every other port in plain mode with STARTTLS. |
|
||||
| `could not be resolved` | The container's DNS cannot resolve the host. Test with `docker exec <container> nc -zv <SMTP_HOST> <SMTP_PORT>`. |
|
||||
| `TLS certificate ... was not accepted` | Turn on **Skip TLS certificate check** (or `SMTP_SKIP_TLS_VERIFY=true`) for an internal relay carrying its own certificate. |
|
||||
| `rejected the envelope` | The from address usually has to belong to the authenticated mailbox. |
|
||||
| `SMTP not configured: ...` | The named field is empty, or the port is not a number. Host, port and from address are all required. |
|
||||
|
||||
> **Note:** a saved SMTP password is shown as a placeholder, never as a value. Leave the field alone to keep it, or type into it to replace it.
|
||||
|
||||
---
|
||||
|
||||
## Password reset emails are not delivered / SMTP is silent
|
||||
|
||||
**Cause:** SMTP failures are logged but do not surface as errors to the end user — the "reset email sent" message appears regardless. Common causes: wrong `SMTP_HOST` or `SMTP_PORT`, bad credentials, firewall blocking outbound on the SMTP port, or a self-signed certificate on the SMTP server.
|
||||
|
||||
Reference in New Issue
Block a user