diff --git a/app/src/components/channels/ChannelConfigPanel.test.tsx b/app/src/components/channels/ChannelConfigPanel.test.tsx new file mode 100644 index 000000000..d9071e2be --- /dev/null +++ b/app/src/components/channels/ChannelConfigPanel.test.tsx @@ -0,0 +1,34 @@ +import { screen } from '@testing-library/react'; +import { describe, expect, it, vi } from 'vitest'; + +import { FALLBACK_DEFINITIONS } from '../../lib/channels/definitions'; +import { renderWithProviders } from '../../test/test-utils'; +import ChannelConfigPanel from './ChannelConfigPanel'; + +// The credential form owns its own RPC/redux wiring (covered by its own suite); +// stub it so this test isolates the panel's channelโ†’component routing. +vi.mock('./CredentialChannelConfig', () => ({ + default: ({ definition }: { definition: { id: string } }) => ( +
{definition.id}
+ ), +})); + +describe('', () => { + it('routes the email channel to the credential form (#4280)', () => { + renderWithProviders( + + ); + const form = screen.getByTestId('credential-config'); + expect(form).toHaveTextContent('email'); + }); + + it('routes lark and dingtalk to the same credential form', () => { + for (const channel of ['lark', 'dingtalk'] as const) { + const { unmount } = renderWithProviders( + + ); + expect(screen.getByTestId('credential-config')).toHaveTextContent(channel); + unmount(); + } + }); +}); diff --git a/app/src/components/channels/ChannelConfigPanel.tsx b/app/src/components/channels/ChannelConfigPanel.tsx index 23072f812..77dbb3370 100644 --- a/app/src/components/channels/ChannelConfigPanel.tsx +++ b/app/src/components/channels/ChannelConfigPanel.tsx @@ -47,9 +47,9 @@ const ChannelConfigPanel = ({ selectedChannel, definitions }: ChannelConfigPanel {selectedChannel === 'telegram' && } {selectedChannel === 'discord' && } {selectedChannel === 'web' && } - {(selectedChannel === 'lark' || selectedChannel === 'dingtalk') && ( - - )} + {(selectedChannel === 'lark' || + selectedChannel === 'dingtalk' || + selectedChannel === 'email') && } diff --git a/app/src/components/channels/ChannelSetupModal.test.tsx b/app/src/components/channels/ChannelSetupModal.test.tsx index 606c35a37..4fa14d255 100644 --- a/app/src/components/channels/ChannelSetupModal.test.tsx +++ b/app/src/components/channels/ChannelSetupModal.test.tsx @@ -5,6 +5,7 @@ import { renderWithProviders } from '../../test/test-utils'; import ChannelSetupModal from './ChannelSetupModal'; const larkDefinition = FALLBACK_DEFINITIONS.find(def => def.id === 'lark')!; +const emailDefinition = FALLBACK_DEFINITIONS.find(def => def.id === 'email')!; describe(' header logo (issue #2854)', () => { it('renders the Lark / Feishu brand logo in the modal header', () => { @@ -12,3 +13,15 @@ describe(' header logo (issue #2854)', () => { expect(document.querySelector('img[src="/lark.png"]')).not.toBeNull(); }); }); + +describe(' credential-channel routing (#4280)', () => { + it('renders the credential form for email instead of "config not available"', () => { + const { getByPlaceholderText, queryByText } = renderWithProviders( + + ); + // The reused credential form renders the IMAP host fieldโ€ฆ + expect(getByPlaceholderText('imap.fastmail.com')).toBeInTheDocument(); + // โ€ฆand does not fall through to the not-available placeholder. + expect(queryByText(/config not available/i)).toBeNull(); + }); +}); diff --git a/app/src/components/channels/ChannelSetupModal.tsx b/app/src/components/channels/ChannelSetupModal.tsx index 4ae740d61..2e87ac8ab 100644 --- a/app/src/components/channels/ChannelSetupModal.tsx +++ b/app/src/components/channels/ChannelSetupModal.tsx @@ -11,6 +11,7 @@ import type { ChannelDefinition, ChannelType } from '../../types/channels'; import { CloseIcon } from '../ui'; import Button from '../ui/Button'; import { renderChannelIcon } from './channelIcon'; +import CredentialChannelConfig from './CredentialChannelConfig'; import DiscordConfig from './DiscordConfig'; import TelegramConfig from './TelegramConfig'; import YuanbaoConfig from './YuanbaoConfig'; @@ -30,6 +31,13 @@ function ChannelConfigContent({ definition }: { definition: ChannelDefinition }) return ; case 'yuanbao': return ; + // Credential-form channels (Lark/DingTalk/Email) render the same generic + // form here as on the Channels page โ€” otherwise clicking their Skills-grid + // tile fell through to "config not available" (#4280 review). + case 'lark': + case 'dingtalk': + case 'email': + return ; default: return (

diff --git a/app/src/components/channels/CredentialChannelConfig.test.tsx b/app/src/components/channels/CredentialChannelConfig.test.tsx index 2414c919a..6feebe7cb 100644 --- a/app/src/components/channels/CredentialChannelConfig.test.tsx +++ b/app/src/components/channels/CredentialChannelConfig.test.tsx @@ -15,6 +15,7 @@ vi.mock('../../utils/tauriCommands/core', () => ({ restartCoreProcess: vi.fn() } const larkDefinition = FALLBACK_DEFINITIONS.find(def => def.id === 'lark')!; const dingtalkDefinition = FALLBACK_DEFINITIONS.find(def => def.id === 'dingtalk')!; +const emailDefinition = FALLBACK_DEFINITIONS.find(def => def.id === 'email')!; const connectChannelMock = vi.mocked(channelConnectionsApi.connectChannel); const disconnectChannelMock = vi.mocked(channelConnectionsApi.disconnectChannel); @@ -47,9 +48,11 @@ describe('', () => { fireEvent.click(screen.getByText('Connect')); await waitFor(() => expect(connectChannelMock).toHaveBeenCalledTimes(1)); + // Booleans are always submitted (use_feishu defaults off) so a default-on + // field can be turned off from the form; strings only when filled. expect(connectChannelMock).toHaveBeenCalledWith('lark', { authMode: 'api_key', - credentials: { app_id: 'cli_abc123', app_secret: 'shh-secret' }, + credentials: { app_id: 'cli_abc123', app_secret: 'shh-secret', use_feishu: 'false' }, }); await waitFor(() => expect(restartCoreProcessMock).toHaveBeenCalledTimes(1)); }); @@ -127,4 +130,68 @@ describe('', () => { await waitFor(() => expect(disconnectChannelMock).toHaveBeenCalledTimes(1)); expect(disconnectChannelMock).toHaveBeenCalledWith('dingtalk', 'api_key'); }); + + it('renders and connects the native IMAP/SMTP email channel (#4280)', async () => { + renderWithProviders(); + + // The reused form renders the email server fields from the definition, + // including the TLS boolean as a checkbox. + expect(screen.getByPlaceholderText('imap.fastmail.com')).toBeInTheDocument(); + expect(screen.getByPlaceholderText('smtp.fastmail.com')).toBeInTheDocument(); + expect(screen.getByRole('checkbox')).toBeInTheDocument(); // smtp_tls + + fireEvent.change(screen.getByPlaceholderText('imap.fastmail.com'), { + target: { value: 'imap.fastmail.com' }, + }); + fireEvent.change(screen.getByPlaceholderText('you@example.com'), { + target: { value: 'me@fastmail.com' }, + }); + fireEvent.change(screen.getByPlaceholderText('App-specific password (recommended)'), { + target: { value: 'fmapp-pass' }, + }); + fireEvent.change(screen.getByPlaceholderText('smtp.fastmail.com'), { + target: { value: 'smtp.fastmail.com' }, + }); + fireEvent.click(screen.getByText('Connect')); + + await waitFor(() => expect(connectChannelMock).toHaveBeenCalledTimes(1)); + // smtp_tls is a default-on boolean: it is submitted as 'true' even when the + // (pre-checked) box is left untouched, so the persisted value matches the UI. + expect(connectChannelMock).toHaveBeenCalledWith('email', { + authMode: 'api_key', + credentials: { + imap_host: 'imap.fastmail.com', + username: 'me@fastmail.com', + password: 'fmapp-pass', + smtp_host: 'smtp.fastmail.com', + smtp_tls: 'true', + }, + }); + }); + + it('lets the user turn smtp_tls off from the pre-checked box (#4280 review)', async () => { + renderWithProviders(); + + // Default-on: the box renders checked before any interaction. + const tls = screen.getByRole('checkbox') as HTMLInputElement; + expect(tls.checked).toBe(true); + fireEvent.click(tls); // turn TLS off + + fireEvent.change(screen.getByPlaceholderText('imap.fastmail.com'), { + target: { value: 'mail.self.host' }, + }); + fireEvent.change(screen.getByPlaceholderText('you@example.com'), { + target: { value: 'me@self.host' }, + }); + fireEvent.change(screen.getByPlaceholderText('App-specific password (recommended)'), { + target: { value: 'pw' }, + }); + fireEvent.change(screen.getByPlaceholderText('smtp.fastmail.com'), { + target: { value: 'mail.self.host' }, + }); + fireEvent.click(screen.getByText('Connect')); + + await waitFor(() => expect(connectChannelMock).toHaveBeenCalledTimes(1)); + expect(connectChannelMock.mock.calls[0][1].credentials?.smtp_tls).toBe('false'); + }); }); diff --git a/app/src/components/channels/CredentialChannelConfig.tsx b/app/src/components/channels/CredentialChannelConfig.tsx index 6f1f0989b..5341c211e 100644 --- a/app/src/components/channels/CredentialChannelConfig.tsx +++ b/app/src/components/channels/CredentialChannelConfig.tsx @@ -60,11 +60,19 @@ const CredentialChannelConfig = ({ definition }: CredentialChannelConfigProps) = const credentials: Record = {}; for (const field of spec.fields) { - const raw = fieldValues[compositeKey]?.[field.key] ?? ''; - const val = field.field_type === 'boolean' ? raw : raw.trim(); - // Booleans are always semantically set (checkbox is on or off), so an - // untouched required boolean must not fail the empty-value check. - if (field.required && field.field_type !== 'boolean' && !val) { + // Booleans are always semantically set (checkbox is on or off). Submit + // them unconditionally, seeding an untouched box from its declared + // default, so a default-on field like smtp_tls can actually be turned + // off from the form instead of silently reverting to the default. + if (field.field_type === 'boolean') { + const raw = fieldValues[compositeKey]?.[field.key]; + const on = + raw === undefined || raw === '' ? (field.default_bool ?? false) : raw === 'true'; + credentials[field.key] = on ? 'true' : 'false'; + continue; + } + const val = (fieldValues[compositeKey]?.[field.key] ?? '').trim(); + if (field.required && !val) { const label = t(`channels.${channel}.fields.${field.key}.label`, field.label); dispatch( setChannelConnectionStatus({ diff --git a/app/src/components/channels/channelConfigPrimitives.tsx b/app/src/components/channels/channelConfigPrimitives.tsx index cece2872d..88d3095bd 100644 --- a/app/src/components/channels/channelConfigPrimitives.tsx +++ b/app/src/components/channels/channelConfigPrimitives.tsx @@ -101,7 +101,13 @@ export function ChannelAuthFields({ onChange(compositeKey, field.key, val)} disabled={disabled} /> diff --git a/app/src/components/channels/channelIcon.test.tsx b/app/src/components/channels/channelIcon.test.tsx index 7d1b7f71c..8fe7cc826 100644 --- a/app/src/components/channels/channelIcon.test.tsx +++ b/app/src/components/channels/channelIcon.test.tsx @@ -38,6 +38,7 @@ describe('renderChannelIcon', () => { ['discord', '๐ŸŽฎ'], ['web', '๐ŸŒ'], ['mcp', '๐Ÿ”Œ'], + ['email', 'โœ‰๏ธ'], ])('renders the %s channel as its emoji glyph', (icon, glyph) => { const { getByTestId } = renderIcon(icon); expect(getByTestId('icon-host')).toHaveTextContent(glyph); @@ -64,6 +65,7 @@ describe('renderChannelIcon', () => { 'imessage', 'lark', 'dingtalk', + 'email', 'yuanbao', ]; const supportedIcons = [ diff --git a/app/src/components/channels/channelIcon.tsx b/app/src/components/channels/channelIcon.tsx index bafabe013..6d62d8afa 100644 --- a/app/src/components/channels/channelIcon.tsx +++ b/app/src/components/channels/channelIcon.tsx @@ -21,7 +21,13 @@ const ICON_COMPONENTS: Record ReactEl * Emoji icons for channels without a dedicated brand mark, rendered as plain * text. Keyed by `ChannelDefinition.icon`. */ -const ICON_EMOJI: Record = { telegram: 'โœˆ๏ธ', discord: '๐ŸŽฎ', web: '๐ŸŒ', mcp: '๐Ÿ”Œ' }; +const ICON_EMOJI: Record = { + telegram: 'โœˆ๏ธ', + discord: '๐ŸŽฎ', + web: '๐ŸŒ', + mcp: '๐Ÿ”Œ', + email: 'โœ‰๏ธ', +}; /** * Render the brand icon for a channel, keyed by `ChannelDefinition.icon`. diff --git a/app/src/lib/channels/definitions.ts b/app/src/lib/channels/definitions.ts index 3de1c8a97..a69a68014 100644 --- a/app/src/lib/channels/definitions.ts +++ b/app/src/lib/channels/definitions.ts @@ -257,4 +257,94 @@ export const FALLBACK_DEFINITIONS: ChannelDefinition[] = [ ], capabilities: ['send_text', 'receive_text'], }, + // Native IMAP/SMTP email (#4280). Field keys map 1:1 to + // `config::schema::channels::EmailConfig` and `email_definition()` in + // `src/openhuman/channels/controllers/definitions.rs`; keep the two in sync. + { + id: 'email', + display_name: 'Email (IMAP/SMTP)', + description: 'Send and receive email via any standard IMAP/SMTP mailbox.', + icon: 'email', + auth_modes: [ + { + mode: 'api_key', + description: "Provide your mailbox's IMAP/SMTP server settings and an app password.", + fields: [ + { + key: 'imap_host', + label: 'IMAP Host', + field_type: 'string', + required: true, + placeholder: 'imap.fastmail.com', + }, + { + key: 'imap_port', + label: 'IMAP Port', + field_type: 'string', + required: false, + placeholder: '993 (TLS)', + }, + { + key: 'username', + label: 'Email Address', + field_type: 'string', + required: true, + placeholder: 'you@example.com', + }, + { + key: 'password', + label: 'Password / App Password', + field_type: 'secret', + required: true, + placeholder: 'App-specific password (recommended)', + }, + { + key: 'smtp_host', + label: 'SMTP Host', + field_type: 'string', + required: true, + placeholder: 'smtp.fastmail.com', + }, + { + key: 'smtp_port', + label: 'SMTP Port', + field_type: 'string', + required: false, + placeholder: '465 (TLS)', + }, + { + key: 'smtp_tls', + label: 'Use TLS for SMTP', + field_type: 'boolean', + required: false, + placeholder: 'On = TLS (recommended)', + default_bool: true, + }, + { + key: 'from_address', + label: 'From Address', + field_type: 'string', + required: false, + placeholder: 'Optional โ€” defaults to the email address above', + }, + { + key: 'imap_folder', + label: 'IMAP Folder', + field_type: 'string', + required: false, + placeholder: 'Optional โ€” defaults to INBOX', + }, + { + key: 'allowed_senders', + label: 'Allowed Senders', + field_type: 'string', + required: false, + placeholder: 'Comma-separated addresses or @domain; * to allow any', + }, + ], + auth_action: undefined, + }, + ], + capabilities: ['send_text', 'receive_text', 'file_attachments'], + }, ]; diff --git a/app/src/store/channelConnectionsSlice.ts b/app/src/store/channelConnectionsSlice.ts index b78589459..af0081a02 100644 --- a/app/src/store/channelConnectionsSlice.ts +++ b/app/src/store/channelConnectionsSlice.ts @@ -31,6 +31,8 @@ const initialState: ChannelConnectionsState = { // populates them when the user wires up credentials. lark: makeEmptyChannelModes(), dingtalk: makeEmptyChannelModes(), + // Native IMAP/SMTP email channel (#4280). + email: makeEmptyChannelModes(), // MCP Servers tab is a virtual channel โ€” no auth-mode connections, // but must be present to satisfy Record. mcp: makeEmptyChannelModes(), diff --git a/app/src/types/channels.ts b/app/src/types/channels.ts index d397b934f..8d87e66b0 100644 --- a/app/src/types/channels.ts +++ b/app/src/types/channels.ts @@ -1,4 +1,12 @@ -export type ChannelType = 'telegram' | 'discord' | 'web' | 'lark' | 'dingtalk' | 'mcp' | 'yuanbao'; +export type ChannelType = + | 'telegram' + | 'discord' + | 'web' + | 'lark' + | 'dingtalk' + | 'email' + | 'mcp' + | 'yuanbao'; /** Every valid {@link ChannelType}, for runtime validation of values that arrive * from the core (which is typed `string`). `satisfies` keeps this list in @@ -10,6 +18,7 @@ export const KNOWN_CHANNEL_TYPES = [ 'web', 'lark', 'dingtalk', + 'email', 'mcp', 'yuanbao', ] as const satisfies readonly ChannelType[]; @@ -62,6 +71,9 @@ export interface FieldRequirement { field_type: string; // "string" | "secret" | "boolean" required: boolean; placeholder: string; + /** Default state for boolean fields; seeds the checkbox so its visible state + * matches what persists when untouched (e.g. smtp_tls defaults on). */ + default_bool?: boolean; } export interface AuthModeSpec { diff --git a/docs/TEST-COVERAGE-MATRIX.md b/docs/TEST-COVERAGE-MATRIX.md index 2e47a5970..761bb5a73 100644 --- a/docs/TEST-COVERAGE-MATRIX.md +++ b/docs/TEST-COVERAGE-MATRIX.md @@ -400,6 +400,7 @@ End-to-end coverage of the agent harness via the web-chat RPC surface against an | 10.1.3 | Gmail Connection | WD | `gmail-flow.spec.ts` | โœ… | | | 10.1.4 | Slack Connection | WD | `app/test/e2e/specs/slack-flow.spec.ts` | โœ… | Was โŒ | | 10.1.5 | Yuanbao Connection | RU | `src/openhuman/channels/providers/yuanbao/`, `src/openhuman/channels/controllers/ops.rs::tests::connect_yuanbao_*`, `src/openhuman/channels/runtime/startup.rs::yuanbao_secret_tests` | ๐ŸŸก | New API-key channel for Tencent Yuanbao. RU covers sign-token preflight (valid/invalid creds, env-override cluster routing), credentials store hydration (incl. stale app_key guard), and WS reconnect/shutdown. No WDIO spec yet โ€” connect-flow UI is rendered via the generic `ChannelSetupModal` already exercised by other channel flow specs. | +| 10.1.6 | Email (IMAP/SMTP) Connection | RU+VU | `src/openhuman/channels/controllers/definitions_tests.rs::email_*`, `src/openhuman/channels/controllers/ops/connect.rs::email_config_tests`, `src/openhuman/channels/controllers/ops_tests.rs::{persist_email_config_*,disconnect_email_*,connect_email_rejects_invalid_port_*,test_channel_email_rejects_invalid_port_*}`, `app/src/components/channels/CredentialChannelConfig.test.tsx`, `app/src/components/channels/ChannelConfigPanel.test.tsx` | ๐ŸŸก | #4280 โ€” native IMAP/SMTP for non-Gmail/Outlook mailboxes surfacing the existing `EmailChannel`. RU covers credentialsโ†’`EmailConfig` mapping/defaults, port/sender parsing, definition/validation, config persist + disconnect, and pre-network invalid-port rejection. VU covers the connect form rendering/submit + panel routing. Live IMAP verify + WDIO connect-flow are follow-ups. | ### 10.2 Authentication & Authorization diff --git a/src/openhuman/channels/controllers/definitions.rs b/src/openhuman/channels/controllers/definitions.rs index 28892fd31..30a034f0d 100644 --- a/src/openhuman/channels/controllers/definitions.rs +++ b/src/openhuman/channels/controllers/definitions.rs @@ -57,6 +57,12 @@ pub struct FieldRequirement { pub required: bool, /// Placeholder / help text. pub placeholder: &'static str, + /// Default state for `field_type == "boolean"` fields. The UI seeds the + /// checkbox from this so its visible state matches what persists when the + /// user doesn't touch it (e.g. `smtp_tls` defaults on). `None` for + /// non-boolean fields and booleans that default off. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub default_bool: Option, } /// Describes one auth mode a channel supports. @@ -160,6 +166,7 @@ pub fn all_channel_definitions() -> Vec { imessage_definition(), lark_definition(), dingtalk_definition(), + email_definition(), yuanbao_definition(), ] } @@ -194,6 +201,7 @@ fn telegram_definition() -> ChannelDefinition { field_type: "secret", required: true, placeholder: "123456:ABC-DEF1234ghIkl-zyx57W2v1u123ew11", + default_bool: None, }, FieldRequirement { key: "chat_id", @@ -201,6 +209,7 @@ fn telegram_definition() -> ChannelDefinition { field_type: "string", required: false, placeholder: "Optional: default chat for outbound messages", + default_bool: None, }, FieldRequirement { key: "allowed_users", @@ -208,6 +217,7 @@ fn telegram_definition() -> ChannelDefinition { field_type: "string", required: false, placeholder: "Comma-separated Telegram usernames", + default_bool: None, }, ], auth_action: None, @@ -239,6 +249,7 @@ fn discord_definition() -> ChannelDefinition { field_type: "secret", required: true, placeholder: "Your Discord bot token", + default_bool: None, }, FieldRequirement { key: "guild_id", @@ -246,6 +257,7 @@ fn discord_definition() -> ChannelDefinition { field_type: "string", required: false, placeholder: "Optional: restrict to a specific server", + default_bool: None, }, FieldRequirement { key: "channel_id", @@ -253,6 +265,7 @@ fn discord_definition() -> ChannelDefinition { field_type: "string", required: false, placeholder: "Optional: default channel for outbound messages", + default_bool: None, }, FieldRequirement { key: "allowed_users", @@ -261,6 +274,7 @@ fn discord_definition() -> ChannelDefinition { required: false, placeholder: "Comma-separated Discord user IDs, or * for everyone (blank = everyone)", + default_bool: None, }, ], auth_action: None, @@ -322,6 +336,7 @@ fn imessage_definition() -> ChannelDefinition { field_type: "string", required: false, placeholder: "Comma-separated phone numbers or emails; * to allow any", + default_bool: None, }], auth_action: None, }], @@ -353,6 +368,7 @@ fn lark_definition() -> ChannelDefinition { field_type: "string", required: true, placeholder: "cli_xxxxxxxxxxxx", + default_bool: None, }, FieldRequirement { key: "app_secret", @@ -360,6 +376,7 @@ fn lark_definition() -> ChannelDefinition { field_type: "secret", required: true, placeholder: "Your Lark app secret", + default_bool: None, }, FieldRequirement { key: "encrypt_key", @@ -367,6 +384,7 @@ fn lark_definition() -> ChannelDefinition { field_type: "secret", required: false, placeholder: "Optional โ€” required only if you enabled message encryption", + default_bool: None, }, FieldRequirement { key: "verification_token", @@ -374,6 +392,7 @@ fn lark_definition() -> ChannelDefinition { field_type: "secret", required: false, placeholder: "Optional โ€” used for HTTP webhook verification", + default_bool: None, }, FieldRequirement { key: "use_feishu", @@ -381,6 +400,7 @@ fn lark_definition() -> ChannelDefinition { field_type: "boolean", required: false, placeholder: "On = open.feishu.cn (China); off = open.larksuite.com", + default_bool: None, }, FieldRequirement { key: "receive_mode", @@ -388,6 +408,7 @@ fn lark_definition() -> ChannelDefinition { field_type: "string", required: false, placeholder: "websocket (default) or webhook", + default_bool: None, }, FieldRequirement { key: "port", @@ -401,6 +422,7 @@ fn lark_definition() -> ChannelDefinition { required: false, placeholder: "Optional โ€” local HTTP port when receive_mode = webhook (e.g. 8080)", + default_bool: None, }, FieldRequirement { key: "allowed_users", @@ -408,6 +430,7 @@ fn lark_definition() -> ChannelDefinition { field_type: "string", required: false, placeholder: "Comma-separated open_id / union_id; leave empty to allow any", + default_bool: None, }, ], auth_action: None, @@ -438,6 +461,7 @@ fn dingtalk_definition() -> ChannelDefinition { field_type: "string", required: true, placeholder: "ding_xxxxxxxxxxxx", + default_bool: None, }, FieldRequirement { key: "client_secret", @@ -445,6 +469,7 @@ fn dingtalk_definition() -> ChannelDefinition { field_type: "secret", required: true, placeholder: "Your DingTalk app secret", + default_bool: None, }, FieldRequirement { key: "allowed_users", @@ -452,6 +477,7 @@ fn dingtalk_definition() -> ChannelDefinition { field_type: "string", required: false, placeholder: "Comma-separated DingTalk userIds; leave empty to allow any", + default_bool: None, }, ], auth_action: None, @@ -460,6 +486,119 @@ fn dingtalk_definition() -> ChannelDefinition { } } +/// Native IMAP/SMTP email channel for any standard mailbox (Fastmail, Proton +/// Bridge, iCloud, self-hosted, โ€ฆ) โ€” the option non-Gmail/non-Outlook users +/// lacked (#4280). The IMAP IDLE + SMTP wire-protocol already lives in +/// `src/openhuman/channels/providers/email_channel.rs`; this definition exposes +/// its config surface to the Connections UI so users no longer need to +/// hand-edit `config.toml`. +/// +/// Field keys map 1:1 to `config::schema::channels::EmailConfig` so the +/// frontend persists credentials through the same `channels_connect` RPC every +/// other channel uses. `imap_port` / `smtp_port` are typed as plain strings +/// (FieldRequirement only supports string/secret/boolean); `connect_channel` +/// parses them back to `u16`. +fn email_definition() -> ChannelDefinition { + ChannelDefinition { + id: "email", + display_name: "Email (IMAP/SMTP)", + description: "Send and receive email via any standard IMAP/SMTP mailbox.", + icon: "email", + auth_modes: vec![AuthModeSpec { + mode: ChannelAuthMode::ApiKey, + description: "Provide your mailbox's IMAP/SMTP server settings and an app password.", + fields: vec![ + FieldRequirement { + key: "imap_host", + label: "IMAP Host", + field_type: "string", + required: true, + placeholder: "imap.fastmail.com", + default_bool: None, + }, + FieldRequirement { + key: "imap_port", + label: "IMAP Port", + field_type: "string", + required: false, + placeholder: "993 (TLS)", + default_bool: None, + }, + FieldRequirement { + key: "username", + label: "Email Address", + field_type: "string", + required: true, + placeholder: "you@example.com", + default_bool: None, + }, + FieldRequirement { + key: "password", + label: "Password / App Password", + field_type: "secret", + required: true, + placeholder: "App-specific password (recommended)", + default_bool: None, + }, + FieldRequirement { + key: "smtp_host", + label: "SMTP Host", + field_type: "string", + required: true, + placeholder: "smtp.fastmail.com", + default_bool: None, + }, + FieldRequirement { + key: "smtp_port", + label: "SMTP Port", + field_type: "string", + required: false, + placeholder: "465 (TLS)", + default_bool: None, + }, + FieldRequirement { + key: "smtp_tls", + label: "Use TLS for SMTP", + field_type: "boolean", + required: false, + placeholder: "On = TLS (recommended)", + default_bool: Some(true), + }, + FieldRequirement { + key: "from_address", + label: "From Address", + field_type: "string", + required: false, + placeholder: "Optional โ€” defaults to the email address above", + default_bool: None, + }, + FieldRequirement { + key: "imap_folder", + label: "IMAP Folder", + field_type: "string", + required: false, + placeholder: "Optional โ€” defaults to INBOX", + default_bool: None, + }, + FieldRequirement { + key: "allowed_senders", + label: "Allowed Senders", + field_type: "string", + required: false, + placeholder: "Comma-separated addresses or @domain; * to allow any", + default_bool: None, + }, + ], + auth_action: None, + }], + capabilities: vec![ + ChannelCapability::SendText, + ChannelCapability::ReceiveText, + ChannelCapability::FileAttachments, + ], + } +} + fn yuanbao_definition() -> ChannelDefinition { // Endpoint URLs (api_domain / ws_domain) are not user-facing โ€” the // channel derives them from the `env` field of `YuanbaoConfig` @@ -479,6 +618,7 @@ fn yuanbao_definition() -> ChannelDefinition { field_type: "string", required: true, placeholder: "ๅ…ƒๅฎๅผ€ๆ”พๅนณๅฐ AppID", + default_bool: None, }, FieldRequirement { key: "app_secret", @@ -486,6 +626,7 @@ fn yuanbao_definition() -> ChannelDefinition { field_type: "secret", required: true, placeholder: "ๅ…ƒๅฎๅผ€ๆ”พๅนณๅฐ AppSecret", + default_bool: None, }, ], auth_action: None, diff --git a/src/openhuman/channels/controllers/definitions_tests.rs b/src/openhuman/channels/controllers/definitions_tests.rs index 321f550d5..abee9d0c0 100644 --- a/src/openhuman/channels/controllers/definitions_tests.rs +++ b/src/openhuman/channels/controllers/definitions_tests.rs @@ -366,3 +366,82 @@ fn auth_mode_serializes_to_expected_wire_values() { ChannelAuthMode::ManagedDm ); } + +#[test] +fn email_definition_is_registered() { + let def = find_channel_definition("email").expect("email channel not registered"); + assert_eq!(def.display_name, "Email (IMAP/SMTP)"); + assert!(def.capabilities.contains(&ChannelCapability::SendText)); + assert!(def.capabilities.contains(&ChannelCapability::ReceiveText)); +} + +#[test] +fn email_definition_field_shape_matches_email_config() { + let def = find_channel_definition("email").expect("email not found"); + let spec = def + .auth_mode_spec(ChannelAuthMode::ApiKey) + .expect("email must expose an api_key auth mode"); + + // Required fields โ€” these gate a usable IMAP/SMTP connection. + for key in ["imap_host", "username", "password", "smtp_host"] { + let field = spec + .fields + .iter() + .find(|f| f.key == key) + .unwrap_or_else(|| panic!("email spec missing required field: {key}")); + assert!(field.required, "email field {key} must be required"); + } + // Password must be secret-typed so the UI masks it. + let password = spec.fields.iter().find(|f| f.key == "password").unwrap(); + assert_eq!(password.field_type, "secret"); + + // Optional fields โ€” keys map 1:1 to `EmailConfig` in + // `src/openhuman/channels/providers/email_channel.rs`. A rename there must + // fail this assertion before the UI silently stops persisting the field. + for key in [ + "imap_port", + "smtp_port", + "smtp_tls", + "from_address", + "imap_folder", + "allowed_senders", + ] { + let field = spec + .fields + .iter() + .find(|f| f.key == key) + .unwrap_or_else(|| panic!("email spec missing optional field: {key}")); + assert!( + !field.required, + "email optional field {key} must not be required" + ); + } + let smtp_tls = spec.fields.iter().find(|f| f.key == "smtp_tls").unwrap(); + assert_eq!(smtp_tls.field_type, "boolean"); + // Default-on so the UI checkbox is pre-checked and a fresh connect keeps TLS + // rather than silently reverting the default when the box is left untouched. + assert_eq!(smtp_tls.default_bool, Some(true)); +} + +#[test] +fn email_validate_credentials_rejects_missing_password() { + let def = find_channel_definition("email").expect("email not found"); + let mut creds = serde_json::Map::new(); + creds.insert( + "imap_host".into(), + serde_json::Value::String("imap.x.com".into()), + ); + creds.insert( + "username".into(), + serde_json::Value::String("u@x.com".into()), + ); + creds.insert( + "smtp_host".into(), + serde_json::Value::String("smtp.x.com".into()), + ); + // password intentionally omitted. + let err = def + .validate_credentials(ChannelAuthMode::ApiKey, &creds) + .expect_err("must reject when password missing"); + assert!(err.contains("password"), "{err}"); +} diff --git a/src/openhuman/channels/controllers/ops/connect.rs b/src/openhuman/channels/controllers/ops/connect.rs index 0ba15f3c1..cf404e3b5 100644 --- a/src/openhuman/channels/controllers/ops/connect.rs +++ b/src/openhuman/channels/controllers/ops/connect.rs @@ -2,7 +2,9 @@ use serde_json::{json, Value}; +use crate::openhuman::channels::email_channel::{EmailChannel, EmailConfig}; use crate::openhuman::channels::providers::yuanbao::YuanbaoConfig; +use crate::openhuman::channels::traits::Channel; use crate::openhuman::config::{Config, DiscordConfig, IMessageConfig, TelegramConfig}; use crate::openhuman::credentials; use crate::openhuman::memory_store::chunks::store as memory_tree_store; @@ -134,6 +136,175 @@ pub(super) fn parse_optional_bool(value: Option<&Value>) -> Option { } } +/// Read a required non-empty string credential field. +fn require_cred_str(creds: &serde_json::Map, key: &str) -> Result { + creds + .get(key) + .and_then(|v| v.as_str()) + .map(str::trim) + .filter(|s| !s.is_empty()) + .map(str::to_string) + .ok_or_else(|| format!("missing required field: {key}")) +} + +/// Read an optional non-empty string credential field. +fn optional_cred_str(creds: &serde_json::Map, key: &str) -> Option { + creds + .get(key) + .and_then(|v| v.as_str()) + .map(str::trim) + .filter(|s| !s.is_empty()) + .map(str::to_string) +} + +/// Parse a `u16` port from a string/number credential field, falling back to +/// `default` when the field is absent or blank. Non-numeric values are a hard +/// error so a typo surfaces at connect time rather than silently reverting. +fn parse_port_field( + creds: &serde_json::Map, + key: &str, + default: u16, +) -> Result { + // Port 0 is the OS "any" sentinel โ€” never a valid mailbox port โ€” so reject it + // up front rather than letting it fail later with a generic connect error. + let invalid = || format!("invalid {key}: must be a port number 1-65535"); + match creds.get(key) { + Some(Value::Number(n)) => n + .as_u64() + .filter(|v| (1..=u64::from(u16::MAX)).contains(v)) + .map(|v| v as u16) + .ok_or_else(invalid), + Some(Value::String(s)) => { + let trimmed = s.trim(); + if trimmed.is_empty() { + Ok(default) + } else { + match trimmed.parse::() { + Ok(0) | Err(_) => Err(invalid()), + Ok(port) => Ok(port), + } + } + } + None | Some(Value::Null) => Ok(default), + _ => Err(invalid()), + } +} + +/// Parse the email `allowed_senders` allowlist from a comma/newline-separated +/// credential field. Unlike [`parse_allowed_users`], this preserves a leading +/// `@` (the domain-match syntax `@example.com` the email channel relies on) and +/// does not force lowercase beyond what the channel already does at match time. +/// An absent field defaults to `["*"]` (allow any) so a freshly-connected +/// mailbox actually receives โ€” the channel treats an *empty* list as deny-all. +fn parse_email_senders(value: Option<&Value>) -> Vec { + let raw = match value { + Some(Value::String(s)) => s.clone(), + Some(Value::Array(items)) => items + .iter() + .filter_map(|v| v.as_str()) + .collect::>() + .join(","), + _ => return vec!["*".to_string()], + }; + + let mut out: Vec = Vec::new(); + for part in raw.split([',', '\n', '\r']) { + let trimmed = part.trim(); + if trimmed.is_empty() { + continue; + } + if !out.iter().any(|e| e.eq_ignore_ascii_case(trimmed)) { + out.push(trimmed.to_string()); + } + } + if out.is_empty() { + out.push("*".to_string()); + } + out +} + +/// Build an [`EmailConfig`] from the connect form's credential map, filling +/// sensible defaults (ports 993/465, TLS on, folder INBOX, `from_address` = +/// username, allowlist = `*`). Field keys map 1:1 to the `email` channel +/// definition. Reuses `existing` only for the IDLE timeout so an advanced +/// hand-set value survives a UI reconnect. +fn build_email_config( + creds: &serde_json::Map, + existing: Option<&EmailConfig>, +) -> Result { + let username = require_cred_str(creds, "username")?; + let from_address = optional_cred_str(creds, "from_address").unwrap_or_else(|| username.clone()); + Ok(EmailConfig { + imap_host: require_cred_str(creds, "imap_host")?, + imap_port: parse_port_field(creds, "imap_port", 993)?, + imap_folder: optional_cred_str(creds, "imap_folder").unwrap_or_else(|| "INBOX".to_string()), + smtp_host: require_cred_str(creds, "smtp_host")?, + smtp_port: parse_port_field(creds, "smtp_port", 465)?, + smtp_tls: parse_optional_bool(creds.get("smtp_tls")).unwrap_or(true), + username, + password: require_cred_str(creds, "password")?, + from_address, + idle_timeout_secs: existing.map_or(1740, |c| c.idle_timeout_secs), + allowed_senders: parse_email_senders(creds.get("allowed_senders")), + }) +} + +/// Live-verify IMAP credentials by attempting a login. Runs before persistence +/// so a wrong host/password fails fast in the UI instead of silently wedging +/// the listener on the next core restart. +async fn verify_email_credentials(cfg: &EmailConfig) -> Result<(), String> { + // The probe dials IMAP + logs in over the network on the connect/test RPC + // path, so bound it: a blackholed host or stalled TLS handshake must not + // hang the UI. `health_check` has its own inner budget; this is a hard outer + // cap that also distinguishes a timeout from an auth failure for the user. + let probe = EmailChannel::new(cfg.clone()); + match tokio::time::timeout(std::time::Duration::from_secs(20), probe.health_check()).await { + Ok(true) => Ok(()), + Ok(false) => Err( + "IMAP connection failed โ€” check the host, port, email address, and app password" + .to_string(), + ), + Err(_) => Err(format!( + "IMAP connection to {} timed out โ€” check the host and port", + cfg.imap_host + )), + } +} + +/// Persist an already-built + verified [`EmailConfig`] into +/// `channels_config.email` so the supervised IMAP/SMTP listener picks it up on +/// the next restart. Kept separate from the verify step so persistence is unit +/// testable without a live mailbox. +/// +/// The `password` is deliberately **not** written to `config.toml` โ€” the secret +/// lives only in the encrypted credentials store (written on the generic connect +/// path under `channel:email:api_key`) and is re-hydrated at startup by +/// `resolve_email_password`. Mirrors the Yuanbao `app_secret` handling. +pub(super) async fn persist_email_config( + config: &Config, + mut email_cfg: EmailConfig, +) -> Result<(), String> { + let allowed_senders_count = email_cfg.allowed_senders.len(); + let smtp_tls = email_cfg.smtp_tls; + // Strip the secret before it ever touches disk. + email_cfg.password = String::new(); + + let mut persisted = config.clone(); + persisted.channels_config.email = Some(email_cfg); + persisted + .save() + .await + .map_err(|e| format!("failed to persist email config.toml: {e}"))?; + + tracing::info!( + target: "openhuman::channels", + allowed_senders_count, + smtp_tls, + "[email] connect_channel: wrote channels_config.email (password kept in credentials store); restart core for IMAP/SMTP listener" + ); + Ok(()) +} + fn clear_channel_memory(config: &Config, channel_id: &str) -> anyhow::Result { let exact = memory_tree_store::delete_chunks_by_source(config, SourceKind::Chat, channel_id)?; let prefixed = memory_tree_store::delete_chunks_by_source_prefix( @@ -211,6 +382,17 @@ pub async fn connect_channel( prebuilt_yuanbao_config = Some(effective); } + // Email (IMAP/SMTP): build the effective config and live-verify the IMAP + // login BEFORE storing anything, so bad server settings surface in the UI + // rather than persisting and wedging the listener on the next restart. + // Reused below for persistence so verify and runtime can never diverge. + let mut prebuilt_email_config: Option = None; + if channel_id == "email" && auth_mode == ChannelAuthMode::ApiKey { + let email_cfg = build_email_config(creds_map, config.channels_config.email.as_ref())?; + verify_email_credentials(&email_cfg).await?; + prebuilt_email_config = Some(email_cfg); + } + // iMessage is local-only (no credentials): persist channels_config + return connected. if channel_id == "imessage" && auth_mode == ChannelAuthMode::ManagedDm { let allowed_contacts = parse_allowed_users(creds_map.get("allowed_contacts")); @@ -424,6 +606,13 @@ pub async fn connect_channel( target: "openhuman::channels", "[yuanbao] connect_channel: wrote channels_config.yuanbao (secret stored in credentials); restart core for WS listener" ); + } else if channel_id == "email" && auth_mode == ChannelAuthMode::ApiKey { + // Reuse the config already built + IMAP-verified above so persistence + // and verification can never diverge. + let email_cfg = prebuilt_email_config.take().ok_or_else(|| { + "internal error: email config not built before persistence".to_string() + })?; + persist_email_config(config, email_cfg).await?; } Ok(RpcOutcome::single_log( @@ -507,6 +696,18 @@ pub async fn disconnect_channel( "[yuanbao] disconnect_channel: cleared channels_config.yuanbao" ); } + } else if channel_id == "email" && auth_mode == ChannelAuthMode::ApiKey { + let mut persisted = config.clone(); + if persisted.channels_config.email.take().is_some() { + persisted + .save() + .await + .map_err(|e| format!("failed to clear email config.toml: {e}"))?; + tracing::info!( + target: "openhuman::channels", + "[email] disconnect_channel: cleared channels_config.email" + ); + } } let memory_chunks_deleted = if clear_memory { @@ -739,8 +940,22 @@ pub async fn test_channel( // Validate fields first. def.validate_credentials(auth_mode, creds_map)?; - // For now, field validation is the test. A future version can instantiate - // the channel provider and call health_check(). + // Email supports a real connection test: build the effective config and + // attempt an IMAP login without persisting anything. + if channel_id == "email" && auth_mode == ChannelAuthMode::ApiKey { + let email_cfg = build_email_config(creds_map, None)?; + verify_email_credentials(&email_cfg).await?; + return Ok(RpcOutcome::new( + ChannelTestResult { + success: true, + message: "IMAP login succeeded.".to_string(), + }, + vec![], + )); + } + + // For other channels, field validation is the test. A future version can + // instantiate the channel provider and call health_check(). Ok(RpcOutcome::new( ChannelTestResult { success: true, @@ -752,3 +967,121 @@ pub async fn test_channel( vec![], )) } + +#[cfg(test)] +mod email_config_tests { + use super::*; + use serde_json::json; + + fn creds(v: Value) -> serde_json::Map { + v.as_object().cloned().unwrap() + } + + #[test] + fn build_email_config_applies_defaults() { + let c = creds(json!({ + "imap_host": "imap.fastmail.com", + "smtp_host": "smtp.fastmail.com", + "username": "alice@example.com", + "password": "app-pass", + })); + let cfg = build_email_config(&c, None).expect("should build"); + assert_eq!(cfg.imap_port, 993); + assert_eq!(cfg.smtp_port, 465); + assert!(cfg.smtp_tls); + assert_eq!(cfg.imap_folder, "INBOX"); + // from_address defaults to the username when omitted. + assert_eq!(cfg.from_address, "alice@example.com"); + // Absent allowlist defaults to allow-any so a fresh mailbox receives. + assert_eq!(cfg.allowed_senders, vec!["*".to_string()]); + assert_eq!(cfg.idle_timeout_secs, 1740); + } + + #[test] + fn build_email_config_honors_explicit_values() { + let c = creds(json!({ + "imap_host": "mail.self.host", + "imap_port": "1993", + "imap_folder": "Archive", + "smtp_host": "mail.self.host", + "smtp_port": "2465", + "smtp_tls": "false", + "username": "bob@self.host", + "password": "secret", + "from_address": "Bob ", + "allowed_senders": "@team.com, boss@corp.com , @team.com", + })); + let cfg = build_email_config(&c, None).expect("should build"); + assert_eq!(cfg.imap_port, 1993); + assert_eq!(cfg.smtp_port, 2465); + assert!(!cfg.smtp_tls); + assert_eq!(cfg.imap_folder, "Archive"); + assert_eq!(cfg.from_address, "Bob "); + // '@'-domain syntax preserved; duplicate collapsed case-insensitively. + assert_eq!( + cfg.allowed_senders, + vec!["@team.com".to_string(), "boss@corp.com".to_string()] + ); + } + + #[test] + fn build_email_config_rejects_missing_required() { + for missing in ["imap_host", "smtp_host", "username", "password"] { + let mut obj = json!({ + "imap_host": "h", + "smtp_host": "h", + "username": "u", + "password": "p", + }); + obj.as_object_mut().unwrap().remove(missing); + let err = build_email_config(&creds(obj), None) + .expect_err("must reject missing required field"); + assert!(err.contains(missing), "error should name {missing}: {err}"); + } + } + + #[test] + fn build_email_config_preserves_existing_idle_timeout() { + let existing = EmailConfig { + idle_timeout_secs: 600, + ..EmailConfig::default() + }; + let c = creds(json!({ + "imap_host": "h", "smtp_host": "h", "username": "u", "password": "p", + })); + let cfg = build_email_config(&c, Some(&existing)).expect("should build"); + assert_eq!(cfg.idle_timeout_secs, 600); + } + + #[test] + fn parse_port_field_variants() { + let c = creds(json!({ + "p_str": "8143", "p_blank": " ", "p_num": 143, "p_bad": "abc", + "p_zero_str": "0", "p_zero_num": 0 + })); + assert_eq!(parse_port_field(&c, "p_str", 993).unwrap(), 8143); + assert_eq!(parse_port_field(&c, "p_blank", 993).unwrap(), 993); + assert_eq!(parse_port_field(&c, "p_num", 993).unwrap(), 143); + assert_eq!(parse_port_field(&c, "absent", 465).unwrap(), 465); + assert!(parse_port_field(&c, "p_bad", 993).is_err()); + // Port 0 is the OS "any" sentinel, never valid for a mailbox. + assert!(parse_port_field(&c, "p_zero_str", 993).is_err()); + assert!(parse_port_field(&c, "p_zero_num", 993).is_err()); + } + + #[test] + fn parse_email_senders_defaults_and_dedup() { + // Absent โ†’ allow any. + assert_eq!(parse_email_senders(None), vec!["*".to_string()]); + // Blank string โ†’ allow any (never accidental deny-all). + assert_eq!( + parse_email_senders(Some(&json!(" "))), + vec!["*".to_string()] + ); + // Array form joins, preserves '@', dedups. + assert_eq!( + parse_email_senders(Some(&json!(["@x.com", "a@y.com", "@X.COM"]))), + vec!["@x.com".to_string(), "a@y.com".to_string()] + ); + } +} diff --git a/src/openhuman/channels/controllers/ops_tests.rs b/src/openhuman/channels/controllers/ops_tests.rs index 1a038dcd6..369d59937 100644 --- a/src/openhuman/channels/controllers/ops_tests.rs +++ b/src/openhuman/channels/controllers/ops_tests.rs @@ -1255,3 +1255,123 @@ async fn connect_yuanbao_persists_env_override() { Some("canary") ); } + +// โ”€โ”€ email (IMAP/SMTP) channel โ€” #4280 โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + +#[tokio::test] +async fn persist_email_config_writes_channels_config_email() { + let (_tmp, config) = isolated_test_config(); + let cfg = EmailConfig { + imap_host: "imap.fastmail.com".into(), + smtp_host: "smtp.fastmail.com".into(), + username: "me@fastmail.com".into(), + password: "app-pass".into(), + from_address: "me@fastmail.com".into(), + allowed_senders: vec!["*".into()], + ..EmailConfig::default() + }; + + super::connect::persist_email_config(&config, cfg) + .await + .expect("persist should succeed"); + + let raw = tokio::fs::read_to_string(&config.config_path) + .await + .expect("saved config should exist"); + let parsed: toml::Value = toml::from_str(&raw).expect("saved config should parse"); + let email = parsed + .get("channels_config") + .and_then(|v| v.get("email")) + .and_then(toml::Value::as_table) + .expect("channels_config.email persisted"); + assert_eq!( + email.get("imap_host").and_then(toml::Value::as_str), + Some("imap.fastmail.com") + ); + assert_eq!( + email.get("smtp_host").and_then(toml::Value::as_str), + Some("smtp.fastmail.com") + ); + // The secret must never hit disk โ€” it lives only in the credentials store. + assert_eq!( + email.get("password").and_then(toml::Value::as_str), + Some(""), + "password must not be persisted to config.toml" + ); +} + +#[tokio::test] +async fn disconnect_email_clears_channels_config() { + let (_tmp, mut config) = isolated_test_config(); + config.channels_config.email = Some(EmailConfig { + imap_host: "imap.x".into(), + smtp_host: "smtp.x".into(), + username: "u@x".into(), + password: "p".into(), + from_address: "u@x".into(), + allowed_senders: vec!["*".into()], + ..EmailConfig::default() + }); + config + .save() + .await + .expect("preloaded config should be persisted"); + + disconnect_channel(&config, "email", ChannelAuthMode::ApiKey, false) + .await + .expect("email disconnect should succeed"); + + let raw = tokio::fs::read_to_string(&config.config_path) + .await + .expect("saved config should exist"); + let parsed: toml::Value = toml::from_str(&raw).expect("saved config should parse"); + assert!( + parsed + .get("channels_config") + .and_then(|v| v.get("email")) + .is_none(), + "channels_config.email should be removed after disconnect" + ); +} + +#[tokio::test] +async fn connect_email_rejects_invalid_port_before_network() { + // All required fields present so validation passes; a non-numeric port makes + // build_email_config fail in the pre-verify step, before any IMAP dial. + let config = Config::default(); + let err = connect_channel( + &config, + "email", + ChannelAuthMode::ApiKey, + serde_json::json!({ + "imap_host": "imap.x.com", + "imap_port": "not-a-port", + "username": "u@x.com", + "password": "secret", + "smtp_host": "smtp.x.com", + }), + ) + .await + .expect_err("invalid port must be rejected"); + assert!(err.contains("imap_port"), "{err}"); +} + +#[tokio::test] +async fn test_channel_email_rejects_invalid_port_before_network() { + let config = Config::default(); + let err = test_channel( + &config, + "email", + ChannelAuthMode::ApiKey, + serde_json::json!({ + "imap_host": "imap.x.com", + "username": "u@x.com", + "password": "secret", + "smtp_host": "smtp.x.com", + "smtp_port": "nope", + }), + ) + .await + .expect_err("invalid smtp port must be rejected"); + assert!(err.contains("smtp_port"), "{err}"); +} diff --git a/src/openhuman/channels/runtime/startup.rs b/src/openhuman/channels/runtime/startup.rs index accd31783..e6db8f63c 100644 --- a/src/openhuman/channels/runtime/startup.rs +++ b/src/openhuman/channels/runtime/startup.rs @@ -589,7 +589,8 @@ pub async fn start_channels(mut config: Config) -> Result<()> { } if let Some(ref email_cfg) = config.channels_config.email { - channels.push(Arc::new(EmailChannel::new(email_cfg.clone()))); + let hydrated = resolve_email_password(email_cfg.clone(), &config); + channels.push(Arc::new(EmailChannel::new(hydrated))); } if let Some(ref irc) = config.channels_config.irc { @@ -894,6 +895,48 @@ fn resolve_yuanbao_app_secret( yb_cfg } +/// Best-effort fill of `email_cfg.password` from the encrypted credentials store +/// when TOML doesn't already carry one. +/// +/// The IMAP/SMTP `password` is intentionally not persisted in `config.toml` (see +/// `persist_email_config` in `controllers/ops/connect.rs`); it lives only in the +/// credentials store under `channel:email:api_key`. Existing TOML values still +/// win so manually-installed deployments keep working. The stored secret is only +/// copied when the stored profile's `username` matches, so editing `username` in +/// `config.toml` can't silently pair a fresh account with a stale password. +fn resolve_email_password( + mut email_cfg: crate::openhuman::channels::email_channel::EmailConfig, + config: &Config, +) -> crate::openhuman::channels::email_channel::EmailConfig { + if !email_cfg.password.is_empty() { + return email_cfg; + } + let auth = crate::openhuman::credentials::AuthService::from_config(config); + match auth.get_profile("channel:email:api_key", None) { + Ok(Some(profile)) => { + let stored_username = profile.metadata.get("username").map(String::as_str); + if stored_username != Some(email_cfg.username.as_str()) { + tracing::warn!( + "[channels] email stored credentials are for a different username (toml={:?}, store={:?}); reconnect the channel to refresh the password", + email_cfg.username, + stored_username, + ); + } else if let Some(password) = profile.metadata.get("password") { + email_cfg.password = password.clone(); + } + } + Ok(None) => { + tracing::warn!( + "[channels] email credentials missing โ€” connect the channel again from the UI" + ); + } + Err(e) => { + tracing::warn!("[channels] failed to load email credentials: {e}"); + } + } + email_cfg +} + #[cfg(any(test, debug_assertions))] pub mod test_support { use super::*; @@ -1005,3 +1048,75 @@ mod yuanbao_secret_tests { ); } } + +#[cfg(test)] +mod email_secret_tests { + use super::*; + use crate::openhuman::channels::email_channel::EmailConfig; + use crate::openhuman::credentials::AuthService; + use std::collections::HashMap; + use tempfile::tempdir; + + fn isolated_config() -> (tempfile::TempDir, Config) { + let tmp = tempdir().expect("tempdir"); + let mut config = Config::default(); + config.workspace_dir = tmp.path().join("workspace"); + config.config_path = tmp.path().join("config.toml"); + std::fs::create_dir_all(&config.workspace_dir).expect("workspace dir"); + (tmp, config) + } + + fn store_email_creds(config: &Config, username: &str, password: &str) { + let auth = AuthService::from_config(config); + let mut metadata = HashMap::new(); + metadata.insert("username".to_string(), username.to_string()); + metadata.insert("password".to_string(), password.to_string()); + auth.store_provider_token("channel:email:api_key", "default", "", metadata, true) + .expect("store credentials"); + } + + #[test] + fn loads_password_from_credentials_when_toml_empty() { + let (_tmp, config) = isolated_config(); + store_email_creds(&config, "me@example.com", "from-credentials"); + + let cfg = EmailConfig { + username: "me@example.com".into(), + password: String::new(), + ..EmailConfig::default() + }; + let resolved = resolve_email_password(cfg, &config); + assert_eq!(resolved.password, "from-credentials"); + } + + #[test] + fn preserves_existing_toml_password_without_consulting_store() { + let (_tmp, config) = isolated_config(); + let cfg = EmailConfig { + username: "me@example.com".into(), + password: "from-toml".into(), + ..EmailConfig::default() + }; + let resolved = resolve_email_password(cfg, &config); + assert_eq!(resolved.password, "from-toml"); + } + + #[test] + fn skips_hydration_when_stored_profile_has_different_username() { + // User changed `username` in config.toml; the stored profile is for the + // old account. The resolver must not graft the old password onto it. + let (_tmp, config) = isolated_config(); + store_email_creds(&config, "old@example.com", "old-password-do-not-use"); + + let cfg = EmailConfig { + username: "new@example.com".into(), + password: String::new(), + ..EmailConfig::default() + }; + let resolved = resolve_email_password(cfg, &config); + assert_eq!( + resolved.password, "", + "stale profile for old username must not hydrate the new account", + ); + } +}