fix(email): test saved SMTP config and add TLS modes (#3388)

fluxer_api/src/api/admin/controllers/InstanceConfigAdminController.ts

@@ -25,6 +25,7 @@ import {RateLimitConfigs} from '@app/api/RateLimitConfig';
 import type {HonoApp, HonoEnv} from '@app/api/types/HonoEnv';
 import {Validator} from '@app/api/Validator';
 import {AdminACLs} from '@fluxer/constants/src/AdminACLs';
+import {resolveSmtpTlsMode, type SmtpTlsMode, smtpTlsModeFromSecure} from '@fluxer/constants/src/SmtpConstants';
 import {InputValidationError} from '@fluxer/errors/src/domains/core/InputValidationError';
 import {InstancePolicyTransitionNotAllowedError} from '@fluxer/errors/src/domains/core/InstancePolicyTransitionNotAllowedError';
 import {
@@ -71,6 +72,16 @@ function readOptionalField<T extends object, K extends keyof T>(value: T, key: K
 	return Object.hasOwn(value, key) ? value[key] : undefined;
 }
 
+function readSmtpTlsModeUpdate(smtp: {
+	tls_mode?: SmtpTlsMode | null;
+	secure?: boolean | null;
+}): SmtpTlsMode | null | undefined {
+	const tlsMode = readOptionalField(smtp, 'tls_mode');
+	if (tlsMode !== undefined) return tlsMode;
+	const secure = readOptionalField(smtp, 'secure');
+	return secure === undefined ? undefined : smtpTlsModeFromSecure(secure);
+}
+
 function mergeOptionalField<T>(currentValue: T, nextValue: T | undefined): T {
 	return nextValue === undefined ? currentValue : nextValue;
 }
@@ -554,7 +565,7 @@ export function InstanceConfigAdminController(app: HonoApp) {
 											port: readOptionalField(data.integrations.email.smtp, 'port'),
 											username: readOptionalField(data.integrations.email.smtp, 'username'),
 											password: readOptionalField(data.integrations.email.smtp, 'password'),
-											secure: readOptionalField(data.integrations.email.smtp, 'secure'),
+											tls_mode: readSmtpTlsModeUpdate(data.integrations.email.smtp),
 										})
 									: undefined,
 							}
@@ -689,22 +700,19 @@ export function InstanceConfigAdminController(app: HonoApp) {
 			operationId: 'create_admin_instance_smtp_test',
 			summary: 'Run an SMTP configuration test',
 			description:
-				'Validates that an SMTP configuration can authenticate and accept a connection. Requires INSTANCE_CONFIG_UPDATE permission.',
+				'Validates that an SMTP configuration can authenticate and accept a connection. Fields left out of the body come from the saved configuration, and the saved password is reused only for the saved host and username. An empty body tests what outgoing email uses and fails when email is off or incomplete. Requires INSTANCE_CONFIG_UPDATE permission.',
 			responseSchema: InstanceEmailSmtpTestResponse,
 			statusCode: 200,
 			security: 'adminApiKey',
 			tags: 'Admin',
 		}),
 		async (ctx) => {
-			const data = ctx.req.valid('json');
+			const {smtp, problem} = await instanceConfigRepository.getSmtpTestConfig(ctx.req.valid('json'));
 			let result: InstanceEmailSmtpTestResponse;
 			try {
+				if (problem) throw new Error(problem);
 				const provider = new SmtpEmailProvider({
-					host: data.host,
-					port: data.port,
-					username: data.username,
-					password: data.password,
-					secure: data.secure,
+					...smtp,
 					connectionTimeoutMs: 10000,
 					greetingTimeoutMs: 10000,
 					socketTimeoutMs: 10000,
@@ -718,7 +726,7 @@ export function InstanceConfigAdminController(app: HonoApp) {
 				targetType: 'instance_config',
 				targetId: 0n,
 				action: 'test_smtp_connection',
-				metadata: {port: data.port, secure: data.secure, ok: result.ok},
+				metadata: {port: smtp.port, tls_mode: resolveSmtpTlsMode(smtp.tlsMode, smtp.port), ok: result.ok},
 			});
 			return ctx.json(result);
 		},

fluxer_api/src/api/admin/tests/InstanceConfigSmtpTest.test.ts

@@ -0,0 +1,200 @@
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+import type {TestAccount} from '@app/api/auth/tests/AuthTestUtils';
+import {createTestAccount, setUserACLs} from '@app/api/auth/tests/AuthTestUtils';
+import {getConfig} from '@app/api/Config';
+import {type ApiTestHarness, createApiTestHarness} from '@app/api/test/ApiTestHarness';
+import {createBuilder} from '@app/api/test/TestRequestBuilder';
+import {AdminACLs} from '@fluxer/constants/src/AdminACLs';
+import type {
+	InstanceConfigResponse,
+	InstanceEmailSmtpTestResponse,
+} from '@fluxer/schema/src/domains/admin/AdminSchemas';
+import nodemailer, {type Transporter} from 'nodemailer';
+import {afterAll, beforeAll, beforeEach, describe, expect, it, vi} from 'vitest';
+
+describe('instance SMTP settings', () => {
+	let harness: ApiTestHarness;
+	let admin: TestAccount;
+	const verify = vi.fn();
+	const createTransport = vi.spyOn(nodemailer, 'createTransport');
+
+	beforeAll(async () => {
+		harness = await createApiTestHarness();
+	});
+
+	beforeEach(async () => {
+		await harness.reset();
+		createTransport.mockReset();
+		createTransport.mockReturnValue({verify} as unknown as Transporter);
+		verify.mockReset();
+		verify.mockResolvedValue(true);
+		admin = await setUserACLs(harness, await createTestAccount(harness), [
+			AdminACLs.AUTHENTICATE,
+			AdminACLs.INSTANCE_CONFIG_VIEW,
+			AdminACLs.INSTANCE_CONFIG_UPDATE,
+		]);
+	});
+
+	afterAll(async () => {
+		await harness.shutdown();
+		createTransport.mockRestore();
+	});
+
+	async function patchSmtp(smtp: Record<string, unknown>) {
+		const config = await createBuilder<InstanceConfigResponse>(harness, admin.token)
+			.patch('/admin/instance/config')
+			.body({integrations: {email: {smtp}}})
+			.execute();
+		return config.integrations.email.smtp;
+	}
+
+	async function runSmtpTest(body: Record<string, unknown>) {
+		return await createBuilder<InstanceEmailSmtpTestResponse>(harness, admin.token)
+			.post('/admin/instance/config/smtp-tests')
+			.body(body)
+			.execute();
+	}
+
+	it('tests the saved settings when the request names none', async () => {
+		await patchSmtp({
+			host: 'smtp.example.com',
+			port: 587,
+			username: 'mailer',
+			password: 'saved-password',
+			tls_mode: 'implicit',
+		});
+		await expect(runSmtpTest({})).resolves.toEqual({ok: true, error: null});
+		expect(createTransport).toHaveBeenCalledWith(
+			expect.objectContaining({
+				host: 'smtp.example.com',
+				port: 587,
+				secure: true,
+				requireTLS: false,
+				ignoreTLS: false,
+				auth: {user: 'mailer', pass: 'saved-password'},
+			}),
+		);
+	});
+
+	it('reports the failure a send with the saved settings would hit', async () => {
+		await patchSmtp({host: 'smtp.example.com', port: 587, username: 'mailer', password: 'saved-password'});
+		verify.mockRejectedValue(new Error('wrong version number'));
+		await expect(runSmtpTest({})).resolves.toEqual({ok: false, error: 'wrong version number'});
+	});
+
+	it('fills a draft for the saved host with the saved password', async () => {
+		await patchSmtp({
+			host: 'smtp.example.com',
+			port: 465,
+			username: 'mailer',
+			password: 'saved-password',
+			tls_mode: 'implicit',
+		});
+		await expect(runSmtpTest({host: 'SMTP.example.com', port: 2525, tls_mode: null})).resolves.toEqual({
+			ok: true,
+			error: null,
+		});
+		expect(createTransport).toHaveBeenCalledWith(
+			expect.objectContaining({
+				host: 'SMTP.example.com',
+				port: 2525,
+				secure: false,
+				ignoreTLS: true,
+				auth: {user: 'mailer', pass: 'saved-password'},
+			}),
+		);
+	});
+
+	it.each([{host: 'draft.example.com'}, {username: 'someone-else'}, {host: 'draft.example.com', password: null}])(
+		'keeps the saved password away from the draft %j',
+		async (draft) => {
+			await patchSmtp({host: 'smtp.example.com', port: 465, username: 'mailer', password: 'saved-password'});
+			await expect(runSmtpTest(draft)).resolves.toEqual({
+				ok: false,
+				error: 'Enter the SMTP password to test a different host or username',
+			});
+			expect(createTransport).not.toHaveBeenCalled();
+		},
+	);
+
+	it('tests a draft for another host with the password it brings', async () => {
+		await patchSmtp({host: 'smtp.example.com', port: 465, username: 'mailer', password: 'saved-password'});
+		await runSmtpTest({host: 'draft.example.com', username: 'drafter', password: 'draft-password'});
+		expect(createTransport).toHaveBeenCalledWith(
+			expect.objectContaining({host: 'draft.example.com', auth: {user: 'drafter', pass: 'draft-password'}}),
+		);
+	});
+
+	it.each([
+		[{enabled: false}, 'Email is turned off'],
+		[{provider: 'none'}, 'The email provider is not SMTP'],
+	])('fails the saved test when %j stops outgoing email', async (email, error) => {
+		await patchSmtp({host: 'smtp.example.com', port: 587, username: 'mailer', password: 'saved-password'});
+		await createBuilder<InstanceConfigResponse>(harness, admin.token)
+			.patch('/admin/instance/config')
+			.body({integrations: {email}})
+			.execute();
+		await expect(runSmtpTest({})).resolves.toEqual({ok: false, error});
+		expect(createTransport).not.toHaveBeenCalled();
+	});
+
+	it.each([
+		[{fromEmail: ''}, 'No from address is set'],
+		[{smtp: {host: '', port: 587, username: 'mailer', password: 'pass', tlsMode: null}}, 'No SMTP host is configured'],
+		[
+			{smtp: {host: 'relay.internal', port: 25, username: '', password: '', tlsMode: null}},
+			'The SMTP username and password are both required',
+		],
+	])('fails the saved test when the environment leaves %j', async (environment, error) => {
+		const original = {...getConfig().email};
+		Object.assign(getConfig().email, environment);
+		try {
+			await expect(runSmtpTest({})).resolves.toEqual({ok: false, error});
+			expect(createTransport).not.toHaveBeenCalled();
+		} finally {
+			Object.assign(getConfig().email, original);
+		}
+	});
+
+	it('picks the TLS mode from the port until one is saved', async () => {
+		const original = getConfig().email.smtp;
+		getConfig().email.smtp = original ? {...original, tlsMode: null} : original;
+		try {
+			expect(await patchSmtp({host: 'smtp.example.com', port: 587})).toMatchObject({
+				tls_mode: null,
+				effective_tls_mode: 'starttls',
+			});
+			expect(await patchSmtp({port: 465})).toMatchObject({tls_mode: null, effective_tls_mode: 'implicit'});
+			expect(await patchSmtp({tls_mode: 'none'})).toMatchObject({tls_mode: 'none', effective_tls_mode: 'none'});
+		} finally {
+			getConfig().email.smtp = original;
+		}
+	});
+
+	it('reads the legacy secure flag and lets the mode replace it', async () => {
+		expect(await patchSmtp({host: 'smtp.example.com', port: 587, secure: true})).toMatchObject({
+			tls_mode: 'implicit',
+		});
+		expect(await patchSmtp({secure: false})).toMatchObject({tls_mode: 'opportunistic'});
+		expect(await patchSmtp({tls_mode: 'starttls'})).toMatchObject({tls_mode: 'starttls'});
+	});
+
+	it('clears every saved field so the environment applies again', async () => {
+		await patchSmtp({
+			host: 'smtp.example.com',
+			port: 465,
+			username: 'mailer',
+			password: 'saved-password',
+			tls_mode: 'implicit',
+		});
+		const environment = getConfig().email.smtp;
+		expect(await patchSmtp({host: null, port: null, username: null, password: null, tls_mode: null})).toMatchObject({
+			host: environment?.host,
+			port: environment?.port,
+			username: environment?.username,
+			tls_mode: null,
+			effective_tls_mode: 'none',
+		});
+	});
+});

fluxer_api/src/api/admin/tests/audit_coverage/InstanceConfigAdminAuditCases.ts

@@ -102,13 +102,13 @@ export const InstanceConfigAdminAuditCases: ReadonlyArray<AdminAuditCoverageCase
 			return {
 				request: {
 					path: '/admin/instance/config/smtp-tests',
-					body: {host: '127.0.0.1', port: 1, username: 'coverage', password: 'coverage-password', secure: false},
+					body: {host: '127.0.0.1', port: 1, username: 'coverage', password: 'coverage-password', tls_mode: 'none'},
 				},
 				expected: {
 					action: 'test_smtp_connection',
 					targetType: 'instance_config',
 					targetId: '0',
-					metadata: {port: '1', secure: 'false', ok: 'false'},
+					metadata: {port: '1', tls_mode: 'none', ok: 'false'},
 				},
 			};
 		},