diff --git a/.forgejo/workflows/email.yml b/.forgejo/workflows/email.yml index df0922f..14373ae 100644 --- a/.forgejo/workflows/email.yml +++ b/.forgejo/workflows/email.yml @@ -351,22 +351,24 @@ jobs: fi echo "OK" - # C4b wired the inbox to a real backend fetch, so sample_thread() is - # now TEST-ONLY: it must never be called from the UI crate at all. - # A hard zero keeps the placeholder from sneaking back into the - # shipping path under a new name or call site. - - name: Development sample data must not reach the UI + # The inbox list is populated from email_store::sample_thread() + # because no receive path exists yet (finding A1, plan C1). That is + # acceptable as scaffolding and unacceptable as a shipped state, so + # keep it visible: the name must stay `sample_`-prefixed and must + # not spread beyond the one call site. + - name: Development sample data must stay obvious and contained run: | set -euo pipefail - count=$(grep -rn 'sample_thread' --include='*.rs' \ + # Count CALL sites, not the import line. + count=$(grep -rn 'sample_thread()' --include='*.rs' \ crates/apps/nigig-email/src | wc -l) - if [ "$count" -gt 0 ]; then + if [ "$count" -gt 1 ]; then grep -rn 'sample_thread' --include='*.rs' crates/apps/nigig-email/src echo - echo "ERROR: sample_thread() is referenced $count times in the UI." - echo "The inbox fetches real mail now (C4b). Sample data is for" - echo "tests only; any UI reference is a regression to the" - echo "placeholder list." + echo "ERROR: sample_thread() is referenced $count times in the" + echo "UI. It is placeholder data for one call site until a real" + echo "fetch lands (plan C1/C4b). Spreading it makes the" + echo "placeholder load-bearing." exit 1 fi echo "OK" @@ -405,23 +407,15 @@ jobs: echo "$HOME/.cargo/bin" >> "$GITHUB_PATH" - name: Email domain tests - run: "cargo test --locked -p nigig-core --lib -- email_ secret:: mail_backend:: mail_proxy:: imap_client:: credential_store::" + run: "cargo test --locked -p nigig-core --lib -- email_ secret:: mail_backend:: mail_proxy::" # A floor, not a ratchet: these tests are cheap, pure, and the - # number should only go up. 38 at Phase 0; 154 after C1c/C1d; - # 195 after the Phase C/D completion. + # number should only go up. 38 at Phase 0; 154 after C1c/C1d. - name: The email domain test suite must not shrink run: | set -euo pipefail - FLOOR=190 - out="$(cargo test --locked -p nigig-core --lib -- email_ secret:: mail_backend:: mail_proxy:: imap_client:: credential_store:: 2>&1)" - - # C1e: the IMAP transport is feature-gated (native only). It must - # still COMPILE when the feature is on, or the direct backend's read - # path silently rots. A check, not a test: the socket is not - # exercised, only type-checked. - - name: The IMAP feature must compile - run: cargo check --locked -p nigig-core --features imap + FLOOR=150 + out="$(cargo test --locked -p nigig-core --lib -- email_ secret:: mail_backend:: mail_proxy:: 2>&1)" # A coverage number that is only printed drifts down. This enforces a # whole-domain floor plus per-file floors on the files that have diff --git a/Cargo.lock b/Cargo.lock index 76ca3ac..e8857b8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -154,167 +154,6 @@ dependencies = [ "libloading 0.8.9 (git+https://gitdab.com/andodeki/makepad?rev=ecf5a572ab62a1c1598909971f602f99083671cc)", ] -[[package]] -name = "async-channel" -version = "1.9.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "81953c529336010edd6d8e358f886d9581267795c61b19475b71314bffa46d35" -dependencies = [ - "concurrent-queue", - "event-listener 2.5.3", - "futures-core", -] - -[[package]] -name = "async-channel" -version = "2.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "924ed96dd52d1b75e9c1a3e6275715fd320f5f9439fb5a4a11fa51f4221158d2" -dependencies = [ - "concurrent-queue", - "event-listener-strategy", - "futures-core", - "pin-project-lite", -] - -[[package]] -name = "async-imap" -version = "0.9.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "98892ebee4c05fc66757e600a7466f0d9bfcde338f645d64add323789f26cb36" -dependencies = [ - "async-channel 2.5.0", - "async-std", - "base64 0.21.7", - "bytes", - "chrono", - "futures", - "imap-proto", - "log 0.4.33", - "nom 7.1.3", - "once_cell 1.21.4", - "pin-utils", - "self_cell", - "stop-token", - "thiserror 1.0.69", -] - -[[package]] -name = "async-io" -version = "2.6.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "456b8a8feb6f42d237746d4b3e9a178494627745c3c56c6ea55d92ba50d026fc" -dependencies = [ - "autocfg", - "cfg-if 1.0.4 (registry+https://github.com/rust-lang/crates.io-index)", - "concurrent-queue", - "futures-io", - "futures-lite", - "parking", - "polling", - "rustix", - "slab", - "windows-sys 0.61.2", -] - -[[package]] -name = "async-lock" -version = "3.4.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "290f7f2596bd5b78a9fec8088ccd89180d7f9f55b94b0576823bbbdc72ee8311" -dependencies = [ - "event-listener 5.4.2", - "event-listener-strategy", - "pin-project-lite", -] - -[[package]] -name = "async-native-tls" -version = "0.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9343dc5acf07e79ff82d0c37899f079db3534d99f189a1837c8e549c99405bec" -dependencies = [ - "futures-util", - "native-tls", - "thiserror 1.0.69", - "url", -] - -[[package]] -name = "async-net" -version = "2.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b948000fad4873c1c9339d60f2623323a0cfd3816e5181033c6a5cb68b2accf7" -dependencies = [ - "async-io", - "blocking", - "futures-lite", -] - -[[package]] -name = "async-process" -version = "2.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fc50921ec0055cdd8a16de48773bfeec5c972598674347252c0399676be7da75" -dependencies = [ - "async-channel 2.5.0", - "async-io", - "async-lock", - "async-signal", - "async-task", - "blocking", - "cfg-if 1.0.4 (registry+https://github.com/rust-lang/crates.io-index)", - "event-listener 5.4.2", - "futures-lite", - "rustix", -] - -[[package]] -name = "async-signal" -version = "0.2.14" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "52b5aaafa020cf5053a01f2a60e8ff5dccf550f0f77ec54a4e47285ac2bab485" -dependencies = [ - "async-io", - "async-lock", - "atomic-waker", - "cfg-if 1.0.4 (registry+https://github.com/rust-lang/crates.io-index)", - "futures-core", - "futures-io", - "rustix", - "signal-hook-registry", - "slab", - "windows-sys 0.61.2", -] - -[[package]] -name = "async-std" -version = "1.13.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2c8e079a4ab67ae52b7403632e4618815d6db36d2a010cfe41b02c1b1578f93b" -dependencies = [ - "async-channel 1.9.0", - "async-io", - "async-lock", - "async-process", - "crossbeam-utils", - "futures-channel", - "futures-core", - "futures-io", - "memchr 2.8.3", - "once_cell 1.21.4", - "pin-project-lite", - "pin-utils", - "slab", - "wasm-bindgen-futures", -] - -[[package]] -name = "async-task" -version = "4.7.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8b75356056920673b02621b35afd0f7dda9306d03c79a30f5c56c44cf256e3de" - [[package]] name = "async-trait" version = "0.1.92" @@ -347,12 +186,6 @@ version = "1.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f2032f911046de80f0a198e0901378627c33f59ea0ac00e363d481118bd70a53" -[[package]] -name = "base64" -version = "0.21.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9d297deb1925b89f2ccc13d7635fa0714f12c87adce1c75356b39ca9b7178567" - [[package]] name = "base64" version = "0.22.1" @@ -431,19 +264,6 @@ dependencies = [ "objc2", ] -[[package]] -name = "blocking" -version = "1.6.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e83f8d02be6967315521be875afa792a316e28d57b5a2d401897e2a7921b7f21" -dependencies = [ - "async-channel 2.5.0", - "async-task", - "futures-io", - "futures-lite", - "piper", -] - [[package]] name = "brotli" version = "8.0.4" @@ -677,15 +497,6 @@ dependencies = [ "memchr 2.8.3", ] -[[package]] -name = "concurrent-queue" -version = "2.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4ca0197aee26d1ae37445ee532fefce43251d24cc7c166799f4d46817f1d3973" -dependencies = [ - "crossbeam-utils", -] - [[package]] name = "convert_case" version = "0.6.0" @@ -695,16 +506,6 @@ dependencies = [ "unicode-segmentation 1.13.3", ] -[[package]] -name = "core-foundation" -version = "0.10.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b2a6cd9ae233e7f62ba4e9353e81a88df7fc8a5987b8d445b4d90c879bd156f6" -dependencies = [ - "core-foundation-sys", - "libc", -] - [[package]] name = "core-foundation-sys" version = "0.8.7" @@ -975,32 +776,6 @@ dependencies = [ "syn 2.0.119", ] -[[package]] -name = "event-listener" -version = "2.5.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0206175f82b8d6bf6652ff7d71a1e27fd2e4efde587fd368662814d6ec1d9ce0" - -[[package]] -name = "event-listener" -version = "5.4.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5a23add41df1562121a9393cb065eab5146a1242410f23a644851e90cfd669d2" -dependencies = [ - "parking", - "pin-project-lite", -] - -[[package]] -name = "event-listener-strategy" -version = "0.5.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8be9f3dfaaffdae2972880079a491a1a8bb7cbed0b8dd7a347f668b4150a3b93" -dependencies = [ - "event-listener 5.4.2", - "pin-project-lite", -] - [[package]] name = "fallible-iterator" version = "0.3.0" @@ -1061,21 +836,6 @@ name = "foldhash" version = "0.2.0" source = "git+https://gitdab.com/andodeki/makepad?rev=ecf5a572ab62a1c1598909971f602f99083671cc#ecf5a572ab62a1c1598909971f602f99083671cc" -[[package]] -name = "foreign-types" -version = "0.3.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f6f339eb8adc052cd2ca78910fda869aefa38d22d5cb648e6485e4d3fc06f3b1" -dependencies = [ - "foreign-types-shared", -] - -[[package]] -name = "foreign-types-shared" -version = "0.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "00b0228411908ca8685dba7fc2cdd70ec9990a6e753e89b6ac91a84c40fbaf4b" - [[package]] name = "form_urlencoded" version = "1.2.2" @@ -1093,7 +853,6 @@ checksum = "9a31d2a3fbaaeb2af2368bbdd904aa8e812d3c04a1ee10d3171f52d556e5d0a3" dependencies = [ "futures-channel", "futures-core", - "futures-executor", "futures-io", "futures-sink", "futures-task", @@ -1116,36 +875,12 @@ version = "0.3.34" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "92d699e522242e69e3003b94ecc1f960f3a5e015aa7c5d7486e65ad01dd94f5e" -[[package]] -name = "futures-executor" -version = "0.3.34" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "031b47cf1a3c6cc8bc2fc76cd437f521619387907d469316e7c0bc278f1f5432" -dependencies = [ - "futures-core", - "futures-task", - "futures-util", -] - [[package]] name = "futures-io" version = "0.3.34" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "53c0fa8157de1303bfffdaa1cc2a673bfffb60102f76b0ef4441659124373fed" -[[package]] -name = "futures-lite" -version = "2.6.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f78e10609fe0e0b3f4157ffab1876319b5b0db102a2c60dc4626306dc46b44ad" -dependencies = [ - "fastrand", - "futures-core", - "futures-io", - "parking", - "pin-project-lite", -] - [[package]] name = "futures-macro" version = "0.3.34" @@ -1175,7 +910,6 @@ version = "0.3.34" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0d50a92467f8ba5dd6e3ee5d4bd04d73ab2e4e1c44474a0674821dfce14b79bc" dependencies = [ - "futures-channel", "futures-core", "futures-io", "futures-macro", @@ -1322,12 +1056,6 @@ version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2304e00983f87ffb38b55b444b5e3b60a884b5d30c0fca7d82fe33449bbe55ea" -[[package]] -name = "hermit-abi" -version = "0.5.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fc0fef456e4baa96da950455cd02c081ca953b141298e41db3fc7e36b1da849c" - [[package]] name = "hexf-parse" version = "0.2.1" @@ -1689,15 +1417,6 @@ dependencies = [ "png", ] -[[package]] -name = "imap-proto" -version = "0.16.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "25f6af35c6a517aea5c72314abe90134980d2ae6a763809b50c208b3e429d71f" -dependencies = [ - "nom 7.1.3", -] - [[package]] name = "imghdr" version = "0.7.0" @@ -1813,7 +1532,7 @@ dependencies = [ "httpdate", "idna", "mime", - "nom 8.0.0", + "nom", "percent-encoding", "quoted_printable", "rustls", @@ -2601,12 +2320,6 @@ dependencies = [ "unicase 2.9.0 (registry+https://github.com/rust-lang/crates.io-index)", ] -[[package]] -name = "minimal-lexical" -version = "0.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "68354c5c6bd36d73ff3feceb05efa59b6acb7626617f4962be322a825e61f79a" - [[package]] name = "miniz_oxide" version = "0.7.4" @@ -2719,23 +2432,6 @@ dependencies = [ "libloading 0.8.9 (registry+https://github.com/rust-lang/crates.io-index)", ] -[[package]] -name = "native-tls" -version = "0.2.18" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "465500e14ea162429d264d44189adc38b199b62b1c21eea9f69e4b73cb03bbf2" -dependencies = [ - "libc", - "log 0.4.33", - "openssl", - "openssl-probe", - "openssl-sys", - "schannel", - "security-framework", - "security-framework-sys", - "tempfile", -] - [[package]] name = "ndk-context" version = "0.1.1" @@ -2820,9 +2516,6 @@ dependencies = [ name = "nigig-core" version = "0.1.0" dependencies = [ - "async-imap", - "async-native-tls", - "async-net", "cfg-if 1.0.4 (registry+https://github.com/rust-lang/crates.io-index)", "chrono", "clap", @@ -3227,16 +2920,6 @@ dependencies = [ "libc", ] -[[package]] -name = "nom" -version = "7.1.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d273983c5a657a70a3e8f2a01329822f3b8c8172b73826411a55751e404a0a4a" -dependencies = [ - "memchr 2.8.3", - "minimal-lexical", -] - [[package]] name = "nom" version = "8.0.0" @@ -3383,49 +3066,6 @@ version = "1.70.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "384b8ab6d37215f3c5301a95a4accb5d64aa607f1fcb26a11b5303878451b4fe" -[[package]] -name = "openssl" -version = "0.10.81" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "77823a27f0babb03091cb9ed9ef80af3b39dbc82f97e8fa530374b7dafd87a45" -dependencies = [ - "bitflags 2.13.1", - "cfg-if 1.0.4 (registry+https://github.com/rust-lang/crates.io-index)", - "foreign-types", - "libc", - "openssl-macros", - "openssl-sys", -] - -[[package]] -name = "openssl-macros" -version = "0.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a948666b637a0f465e8564c73e89d4dde00d72d4d473cc972f390fc3dcee7d9c" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.119", -] - -[[package]] -name = "openssl-probe" -version = "0.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7c87def4c32ab89d880effc9e097653c8da5d6ef28e6b539d313baaacfbafcbe" - -[[package]] -name = "openssl-sys" -version = "0.9.117" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b47e7e6bb2c38cd930d25a23b40fa52e068c10e85f3e03a7f5ba5aaca5713695" -dependencies = [ - "cc", - "libc", - "pkg-config 0.3.34", - "vcpkg", -] - [[package]] name = "option-ext" version = "0.2.0" @@ -3494,12 +3134,6 @@ dependencies = [ "url", ] -[[package]] -name = "parking" -version = "2.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f38d5652c16fde515bb1ecef450ab0f6a219d619a7274976324d5e377f7dceba" - [[package]] name = "parking_lot" version = "0.12.5" @@ -3535,23 +3169,6 @@ version = "0.2.17" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a89322df9ebe1c1578d689c92318e070967d1042b512afbe49518723f4e6d5cd" -[[package]] -name = "pin-utils" -version = "0.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8b870d8c151b6f2fb93e84a13146138f05d02ed11c7e7c54f8826aaaf7c9f184" - -[[package]] -name = "piper" -version = "0.2.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c835479a4443ded371d6c535cbfd8d31ad92c5d23ae9770a61bc155e4992a3c1" -dependencies = [ - "atomic-waker", - "fastrand", - "futures-io", -] - [[package]] name = "pkg-config" version = "0.3.32" @@ -3576,20 +3193,6 @@ dependencies = [ "miniz_oxide 0.8.9", ] -[[package]] -name = "polling" -version = "3.11.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5d0e4f59085d47d8241c88ead0f274e8a0cb551f3625263c05eb8dd897c34218" -dependencies = [ - "cfg-if 1.0.4 (registry+https://github.com/rust-lang/crates.io-index)", - "concurrent-queue", - "hermit-abi", - "pin-project-lite", - "rustix", - "windows-sys 0.61.2", -] - [[package]] name = "pollster" version = "0.4.0" @@ -4328,15 +3931,6 @@ dependencies = [ "winapi-util", ] -[[package]] -name = "schannel" -version = "0.1.29" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "91c1b7e4904c873ef0710c1f407dde2e6287de2bebc1bbbf7d430bb7cbffd939" -dependencies = [ - "windows-sys 0.61.2", -] - [[package]] name = "scoped-tls" version = "1.0.1" @@ -4353,35 +3947,6 @@ name = "sdfer" version = "0.2.1" source = "git+https://gitdab.com/andodeki/makepad?rev=ecf5a572ab62a1c1598909971f602f99083671cc#ecf5a572ab62a1c1598909971f602f99083671cc" -[[package]] -name = "security-framework" -version = "3.7.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b7f4bc775c73d9a02cde8bf7b2ec4c9d12743edf609006c7facc23998404cd1d" -dependencies = [ - "bitflags 2.13.1", - "core-foundation", - "core-foundation-sys", - "libc", - "security-framework-sys", -] - -[[package]] -name = "security-framework-sys" -version = "2.17.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6ce2691df843ecc5d231c0b14ece2acc3efb62c0a398c7e1d875f3983ce020e3" -dependencies = [ - "core-foundation-sys", - "libc", -] - -[[package]] -name = "self_cell" -version = "1.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2ab42ca02749e120097e328d91d415325bdf43b1c72c4c8badf37375fe40a813" - [[package]] name = "semver" version = "1.0.28" @@ -4544,18 +4109,6 @@ version = "1.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6ce2be8dc25455e1f91df71bfa12ad37d7af1092ae736f3a6cd0e37bc7810596" -[[package]] -name = "stop-token" -version = "0.7.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "af91f480ee899ab2d9f8435bfdfc14d08a5754bd9d3fef1f1a1c23336aad6c8b" -dependencies = [ - "async-channel 1.9.0", - "cfg-if 1.0.4 (registry+https://github.com/rust-lang/crates.io-index)", - "futures-core", - "pin-project-lite", -] - [[package]] name = "streem" version = "0.1.0" diff --git a/REVIEWS/NIGIG_EMAIL_ASSESSMENT_AND_PLAN.md b/REVIEWS/NIGIG_EMAIL_ASSESSMENT_AND_PLAN.md index fedd359..e99d442 100644 --- a/REVIEWS/NIGIG_EMAIL_ASSESSMENT_AND_PLAN.md +++ b/REVIEWS/NIGIG_EMAIL_ASSESSMENT_AND_PLAN.md @@ -530,22 +530,17 @@ snapshot. Commits are on `main`. | *(this turn)* | **Phase A complete** — `Secret` newtype; `SmtpConfig` no longer derives `Serialize`; `relay()`; explicit TLS policy; local validation; HTTPS-only proxy endpoint; email THREAT_MODEL. Domain tests **38 → 68**. | | *(this turn)* | **C1d + C1c DONE** — `mail_proxy.rs` (the proxy HTTP client: request/response parsing, error mapping, a transport trait with reqwest/fetch/mock impls); `SetupDraft` (identity + backend, validated together); the backend chooser and two setup forms in `EmailAccountSetup`; the inbox branches to `spawn_proxy_verify` for the proxy backend. Domain tests **126 → 154**. Coverage tooling (`tools/test-email-coverage.sh`, 93.4% line, floors enforced) wired into `email.yml`. | -| *(this turn)* | **Phase C + D COMPLETE** — C1e (`imap_client.rs`: trait + pure INTERNALDATE parser + feature-gated async-imap transport, native only); C1f (`credential_store.rs`: trait + fail-closed default); C3 (`email_cache.rs`: bodies through a `BodyCipher` before disk); C4b (inbox fetches real mail, loading/error/empty states); C5 (Compose sends via `spawn_send_message`); C6 (`email_pacing.rs`: `SendRateLimiter`+`SendPacing`, 100/hour); C7 (Refresh button re-fetches); D1 (More page is real: account + sign-out); D2 (lib.rs shims deleted, `NavigationBarAction` in a real module); D3 (`CachedWidget` decision documented); D4 (one-slot SMTP transport pool, keying tested); D5 (`EmailWorkerAction::None` dropped). Domain tests **154 → 195**. Coverage now **90.7%** over 12 files, floors enforced. | **Phase 0 is complete.** All seven items done; 0.7 was fixed upstream. -**Phase C is complete.** All of C1 (both backends: the proxy HTTP client -and the IMAP protocol client), C2 (domain model), C3 (encrypted-at-rest -cache seam), C4a/C4b (inbox list + thread reader, wired to a real fetch), -C5 (Compose send), C6 (pacing primitives) and C7 (refresh) are done. -Domain tests **99 → 195**. - -The honest caveats carry forward unchanged: the SMTP and IMAP *sockets*, -the reqwest/fetch transports, the platform keystore and the connection -pool's reuse are not host-verified -- each is a read of the library's -contract or a platform task, not an observed handshake. The seams around -them (parsing, keying, error mapping, validation) ARE all tested. See -§8 and the per-module "What is not verified" notes. +**Phase C is in progress.** C1a, C1b, C1c and C1d are done: the +`MailBackend` trait boundary and both validation halves exist, the chooser +and the two setup forms are in the UI, and the proxy backend is a real HTTP +client (verified against a mock transport, driven by `reqwest`/`fetch` in +production). Domain tests **99 → 154**. Next: C1e (the IMAP protocol +client, native only, gated behind a feature), then C1f (keystore-backed +credentials), C3 (encrypted persistence), C4b (wire the inbox to a real +fetch), C5 (Compose send button), C6 (bulk pacing), C7 (pull-to-refresh). **Phase B is complete.** B1 was the live Critical: the field labelled "To (comma-separated)" was parsed by @@ -696,12 +691,12 @@ send-only form. **It needs a product decision before any code.** |---|---| | **C1** | **DECIDE THE RECEIVE STRATEGY — the one blocking question.** IMAP on device (`async-imap`) vs a server-side proxy API. See the decision note below. **Everything else in this phase is either done or waits on this.** | | ~~C2~~ | ~~**Define the domain model.**~~ **DONE** (`5a5b817`) — `EmailMessage`, `EmailThreadSummary`, grouping, previews, filtering. 38 tests. `Folder` deliberately deferred: it is meaningless until C1 says whether folders come from IMAP or from a proxy's schema. | -| ~~C3~~ | ~~**Persistence.**~~ **DONE** — `email_cache.rs`: a local cache whose bodies pass through a `BodyCipher` before hitting disk (hex-encoded ciphertext in the JSON, never the plaintext body). The production cipher is the C1f keystore key; `PlaintextBodyCipher` is the honest default until then, and the whole pipeline (serialize/encrypt/decrypt/deserialize) is host-tested against a reversible test cipher. | +| C3 | **Persistence** — reuse `nigig-core::persistence`; bodies encrypted at rest, per SMS E1/E3. Note `EmailMessage.body` is *not* `#[serde(skip)]`-ed the way `OfflineSmsMessage.body` is, because a mail cache that drops bodies is useless — so encryption is mandatory here, not optional. | | ~~C4a~~ | ~~**Inbox list + thread reader.**~~ **DONE** (`18bbb7b`) — sender list, thread push/pop, unread handling, signed-out gate. | -| ~~C4b~~ | ~~**Wire it to real data.**~~ **DONE** — `sample_thread()` is gone from the UI (test-only now, gated in CI); the inbox fetches via `spawn_fetch_inbox` (backend-agnostic) and renders loading / error / empty states. | -| ~~C5~~ | ~~**Give Compose a send button.**~~ **DONE** — `compose.rs` is a real form: to/subject/body, `EmailSendRequest::build_without_config` validation, two-tap confirm, and `spawn_send_message` (branches on the signed-in backend). | -| ~~C6~~ | ~~**Bulk pacing.**~~ **DONE** — `email_pacing.rs`: `SendRateLimiter` + `SendPacing` ported with email-shaped limits (100/hour). Primitives are tested; no per-recipient loop exists to wire them into yet (the bulk path is one message to N recipients). | -| ~~C7~~ | ~~**Pull-to-refresh + background fetch.**~~ **DONE** — a Refresh button re-fetches; the fetch is worker → `InboxFetched` action → drained on the UI thread (the `Cx::post_action` shape, equivalent to SMS's `SignalToUI`). A gesture-based pull is deferred: Makepad has no pull-to-refresh primitive and the button is the cross-platform control. | +| C4b | **Wire it to real data.** Replace `sample_thread()` with the C1 fetch. Add loading / error / empty states — currently only empty exists. | +| C5 | **Give Compose a send button** wired to B2, replacing the scaffold. | +| C6 | **Bulk pacing.** Email providers rate-limit harder than carriers. Port `SendPacing` from `robius-sms` — already generic arithmetic. | +| C7 | **NEW — pull-to-refresh + background fetch** once C1 lands. The SMS crate's D1/D2 pattern (worker thread → results queue → `SignalToUI` → drained on the UI thread) applies directly; do not fetch from `draw_walk`. | #### C1 — DECIDED: support both, user-selectable @@ -770,8 +765,8 @@ extends cleanly. | ~~C1b~~ | **DONE** — `BackendDraft::validate` branches per kind; `EmailAccount.backend` persists the choice with no secret. HTTPS-only enforced and tested. | | ~~C1c~~ | **DONE** — backend chooser + two forms in `EmailAccountSetup`, driven by `SetupDraft` (identity + backend validated together); the inbox branches to the proxy verify. | | ~~C1d~~ | **DONE** — `mail_proxy.rs`: `ProxyApiClient` with `verify`/`list_inbox`/`send` over a `ProxyTransport` trait (reqwest/fetch/mock); request/response parsing and error mapping all host-tested. | -| ~~C1e~~ | ~~`ImapSmtpBackend`.~~ **DONE** — `imap_client.rs`: `ImapTransport` trait, `ImapClient` (`verify` + `list_inbox`), a pure `INTERNALDATE` parser and envelope mapping (tested), and `AsyncImapTransport` over `async-imap` behind the `imap` feature (native only; a CI step checks it compiles). | -| ~~C1f~~ | ~~Keystore-backed credential storage.~~ **DONE (the seam)** — `credential_store.rs`: the `CredentialStore` trait and a fail-closed default. The actual `AndroidKeyStore` wiring remains the platform task it always was (not host-testable); the contract is pinned by tests so a platform impl has something to match. | +| C1e | `ImapSmtpBackend` — `async-imap`, native only. Gate behind a feature so wasm builds never pull it in. | +| C1f | Keystore-backed credential storage (A1). Required for IMAP to be usable across restarts; optional for the proxy, which can hold a revocable token instead. | ##### One thing I will not pretend @@ -787,11 +782,11 @@ presenting them as equivalent choices. | ID | Task | |---|---| -| ~~D1~~ | ~~**Extract the shared page scaffold.**~~ **DONE (mooted)** — three of the four placeholder pages are now real (Inbox, Compose, Bulk); the last, `more.rs`, was replaced with real content (account + sign-out) rather than extracted. There is no duplicated scaffold left to parameterise. | -| ~~D2~~ | ~~**Delete the `lib.rs` shims.**~~ **DONE** — all six shim modules are gone; the page imports use `nigig_core::`/`nigig_uikit::` directly, and the fake `NavigationBarAction` enum now lives in a real `navigation.rs` module. | -| ~~D3~~ | ~~**Reconsider `CachedWidget`.**~~ **DONE** — decision recorded in `email.rs`: keep it. It pins the page *structure*, not message data (which lives in `#[rust] Vec`s and clears on sign-out), and the list is a virtualised `PortalList`. | -| ~~D4~~ | ~~**Connection pooling.**~~ **DONE** — a one-slot transport pool reuses the SMTP connection across sends (safe because `SEND_IN_FLIGHT` serialises them); the reuse-vs-rebuild keying is pure and tested, the connection-level reuse is a read of lettre's contract. | -| ~~D5~~ | ~~**Drop `EmailWorkerAction::None`.**~~ **DONE** — the variant, the `Default` impl and the `ActionDefaultRef` impl are all gone: the action is consumed only via `downcast_ref()` (which needs just `'static + Debug`), so no default was ever required. | +| D1 | **Extract the shared page scaffold.** One parameterised pane replaces 4 copies (~450 lines deleted). SMS Phase F5 precedent. | +| D2 | **Delete the `lib.rs` shims** or finish the migration they were staged for. A UI enum declared in a compat shim is not acceptable long-term. | +| D3 | **Reconsider `CachedWidget` on the inbox** once it holds a real list. | +| D4 | **Connection pooling** — actually use the `pool` feature already compiled in. | +| D5 | Drop `EmailWorkerAction::None` if `ActionDefaultRef` can be satisfied otherwise. | --- diff --git a/crates/apps/nigig-email/src/email_frame/action_bars.rs b/crates/apps/nigig-email/src/email_frame/action_bars.rs index 3c975e5..377da5f 100644 --- a/crates/apps/nigig-email/src/email_frame/action_bars.rs +++ b/crates/apps/nigig-email/src/email_frame/action_bars.rs @@ -1,7 +1,7 @@ -use crate::navigation::NavigationBarAction; +use crate::features::action_page_navigation::ActionPageNavigationAction; +use crate::home::navigation_tab_bar::NavigationBarAction; +use crate::shared::navigation_bar_button::NavigationBarButtonWidgetExt; use makepad_widgets::*; -use nigig_uikit::action_page_navigation::ActionPageNavigationAction; -use nigig_uikit::shared::navigation_bar_button::NavigationBarButtonWidgetExt; const DOUBLE_TAP_HOME_SECS: f64 = 0.55; diff --git a/crates/apps/nigig-email/src/email_frame/email.rs b/crates/apps/nigig-email/src/email_frame/email.rs index 88ce2ca..0da01c7 100644 --- a/crates/apps/nigig-email/src/email_frame/email.rs +++ b/crates/apps/nigig-email/src/email_frame/email.rs @@ -1,5 +1,5 @@ +use crate::features::action_page_navigation::ActionPageNavigationAction; use makepad_widgets::*; -use nigig_uikit::action_page_navigation::ActionPageNavigationAction; script_mod! { use mod.prelude.widgets.* @@ -25,15 +25,6 @@ script_mod! { } } -// D3: the four pages are wrapped in `CachedWidget` (see the live DSL -// above). This was flagged for "reconsider once the inbox holds a real -// list". Reconsidered: `CachedWidget` pins the page's *widget structure* -// (avoiding re-instantiation on every tab switch), NOT the message data -- -// that lives in each page's `#[rust] Vec` and is cleared on -// sign-out. The list itself is a virtualised `PortalList` -// (`keep_invisible: false`), so a large inbox does not inflate the cached -// widget tree. Decision: keep `CachedWidget`; it is not the memory concern -// the assessment worried it might become. #[derive(Script, ScriptHook, Widget)] pub struct EmailScreen { #[deref] diff --git a/crates/apps/nigig-email/src/email_frame/pages/compose.rs b/crates/apps/nigig-email/src/email_frame/pages/compose.rs index aa170fb..3e97977 100644 --- a/crates/apps/nigig-email/src/email_frame/pages/compose.rs +++ b/crates/apps/nigig-email/src/email_frame/pages/compose.rs @@ -1,20 +1,5 @@ -// Compose: write and send one message (Phase C5). -// -// This page was a placeholder scaffold -- "Replace this scaffold with the -// real workflow for Compose" -- with no send button at all, because the -// send logic lived in the Bulk page's widget and could not be reused. C5 -// wires it for real: -// -// * to / subject / body, validated through -// `EmailSendRequest::build_without_config` (no SMTP config here -- the -// send goes through the signed-in backend, which may be the proxy); -// * sends through `spawn_send_message`, which branches on the account's -// backend and shares the in-flight guard (B5) and abandon control (B6); -// * reads the signed-in account from `nigig_core::email_session`, which -// the inbox writes on sign-in -- so compose has no credentials form. - +use crate::shared::context_nav_action::ContextNavAction; use makepad_widgets::*; -use nigig_core::email_worker::{spawn_send_message, EmailWorkerAction}; script_mod! { use mod.prelude.widgets.* @@ -22,70 +7,84 @@ script_mod! { mod.widgets.EmailComposePage = #(EmailComposePage::register_widget(vm)) { width: Fill, height: Fill - flow: Down - page_top_bar := SolidView { - width: Fill, height: 52 - flow: Right - align: Align{y: 0.5} - padding: Inset{left: 16, right: 16, top: 0, bottom: 0} - show_bg: true - draw_bg +: { color: #xFFFFFF } - - Label { - width: Fill, height: Fit - text: "Compose" - draw_text +: { color: #x1C274C, text_style: theme.font_bold { font_size: 20.0 } } - } - } - - compose_scroll := ScrollYView { - width: Fill, height: Fill - flow: Down - padding: Inset{left: 18, right: 18, top: 12, bottom: 22} - spacing: 12 - - compose_card := RoundedView { - width: Fill, height: Fit + page_stack := StackNavigation { + root_view +: { + width: Fill, height: Fill flow: Down - spacing: 8 - padding: 18 - show_bg: true - draw_bg +: { color: #xFFFFFF, border_radius: 22.0, border_size: 1.0, border_color: #xE2E8F0 } - Label { text: "To (comma-separated)" draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 10.5 } } } - to_input := TextInput { - width: Fill, height: 42 - empty_text: "friend@example.com, team@example.com" - draw_bg +: { border_radius: 10.0, border_size: 1.0, border_color: #xE2E8F0 } + page_top_bar := SolidView { + width: Fill, height: 52 + flow: Right + align: Align{y: 0.5} + padding: Inset{left: 16, right: 16, top: 0, bottom: 0} + show_bg: true + draw_bg +: { color: #xFFFFFF } + + Label { + width: Fill, height: Fit + text: "Compose" + draw_text +: { color: #x1C274C, text_style: theme.font_bold { font_size: 20.0 } } + } } - Label { text: "Subject" draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 10.5 } } } - subject_input := TextInput { - width: Fill, height: 42 - empty_text: "Subject" - draw_bg +: { border_radius: 10.0, border_size: 1.0, border_color: #xE2E8F0 } - } + page_body := ScrollYView { + width: Fill, height: Fill + flow: Down + padding: Inset{left: 18, right: 18, top: 12, bottom: 22} + spacing: 12 - Label { text: "Message" draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 10.5 } } } - body_input := TextInput { - width: Fill, height: 140 - empty_text: "Write your message" - is_multiline: true - draw_bg +: { border_radius: 10.0, border_size: 1.0, border_color: #xE2E8F0 } - } + RoundedView { + width: Fill, height: Fit + flow: Down + spacing: 8 + padding: 18 + show_bg: true + draw_bg +: { color: #xF8FAFC, border_radius: 22.0, border_size: 1.0, border_color: #xE2E8F0 } + Label { text: "Compose" draw_text +: { color: #x1C274C, text_style: theme.font_bold { font_size: 22.0 } } } + Label { width: Fill, height: Fit, text: "Top app bar page. Tap below to open a stack screen with RobrixStackNavigationView back navigation." draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 11.0 } } } + } - send_btn := Button { - width: Fill, height: 46 - text: "Send" - draw_bg +: { color: #x1C274C, color_hover: #x2A3F6E, border_radius: 14.0 } - draw_text +: { color: #xFFFFFF, text_style: theme.font_bold { font_size: 14.0 } } + open_detail_btn := Button { + width: Fill, height: 54 + text: "Open Compose workflow" + draw_bg +: { color: #xEEF4FF, color_hover: #xDDEBFF, color_down: #xCFE2FF, border_radius: 16.0, border_size: 1.0, border_color: #xD7E5FF } + draw_text +: { color: #x1C274C, text_style: theme.font_bold { font_size: 12.0 } } + } } + } - send_status := Label { - width: Fill, height: Fit - text: "" - draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 10.5 } } + stack_templates: { + EmailComposePageDetailStackView := mod.widgets.RobrixStackNavigationView { + body +: { + detail_body := ScrollYView { + width: Fill, height: Fill + flow: Down + padding: Inset{left: 18, right: 18, top: 18, bottom: 22} + spacing: 12 + + Label { + width: Fill, height: Fit + text: "Compose details" + draw_text +: { color: #x1C274C, text_style: theme.font_bold { font_size: 22.0 } } + } + Label { + width: Fill, height: Fit + text: "This is a RobrixStackNavigationView destination. The built-in header above supplies the title and back arrow, just like SMS conversation screens." + draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 11.0 } } + } + RoundedView { + width: Fill, height: Fit + flow: Down + padding: 16 + spacing: 8 + show_bg: true + draw_bg +: { color: #xF8FAFC, border_radius: 18.0, border_size: 1.0, border_color: #xE2E8F0 } + Label { text: "Next screen content" draw_text +: { color: #x1C274C, text_style: theme.font_bold { font_size: 13.0 } } } + Label { width: Fill, height: Fit, text: "Replace this scaffold with the real workflow for Compose." draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 10.5 } } } + } + } + } } } } @@ -96,94 +95,32 @@ script_mod! { pub struct EmailComposePage { #[deref] view: View, - /// B5: (body, recipient count) the user has been prompted about. A - /// second tap with the same pair confirms; anything else re-prompts. #[rust] - pending_send: Option<(String, usize)>, + current_detail_view: Option, } impl Widget for EmailComposePage { fn handle_event(&mut self, cx: &mut Cx, event: &Event, scope: &mut Scope) { + // First forward the event so dynamic StackNavigation children can produce actions. + // Then handle the Event::Actions carried by this turn. This follows the SMS/Home pattern. self.view.handle_event(cx, event, scope); - let Event::Actions(actions) = event else { - return; - }; + if let Event::Actions(actions) = event { + if self.view.button(cx, ids!(open_detail_btn)).clicked(actions) { + self.push_detail(cx); + } - let mut send_clicked = self.button(cx, ids!(send_btn)).clicked(actions); - - // B6: while a send is in flight the same button abandons it. - if send_clicked && nigig_core::email_worker::send_in_flight() { - nigig_core::email_worker::abandon_send(); - self.pending_send = None; - self.label(cx, ids!(send_status)) - .set_text(cx, "Stopped waiting. The message may still have been sent."); - self.set_send_button_label(cx, false); - self.view.redraw(cx); - send_clicked = false; - } - - if send_clicked { - // The signed-in account drives the send; no local config. - let Some(session) = nigig_core::email_session::current_session() else { - self.label(cx, ids!(send_status)) - .set_text(cx, "Connect an account from the Inbox first."); - self.view.redraw(cx); - return; - }; - - let to = self.text_input(cx, ids!(to_input)).text(); - let subject = self.text_input(cx, ids!(subject_input)).text(); - let body = self.text_input(cx, ids!(body_input)).text(); - - // Validate the message before the two-tap confirmation. The - // proxy path validates the same way (no SMTP config), so this - // check is backend-agnostic. - match nigig_core::email_send::EmailSendRequest::build_without_config( - &to, &subject, &body, - ) { - Err(e) => { - self.label(cx, ids!(send_status)).set_text(cx, &e.message()); - self.pending_send = None; - self.view.redraw(cx); - } - Ok((req, _list)) => { - let n = req.recipient_count(); - let armed = self - .pending_send - .as_ref() - .is_some_and(|(b, c)| b == &body && *c == n); - if !armed { - self.pending_send = Some((body.clone(), n)); - self.label(cx, ids!(send_status)).set_text( - cx, - &format!( - "Send to {} recipient{}? Tap Send again to confirm.", - n, - if n == 1 { "" } else { "s" } - ), - ); - self.view.redraw(cx); - } else { - self.pending_send = None; - self.label(cx, ids!(send_status)) - .set_text(cx, &format!("Sending to {n} recipient(s)…")); - spawn_send_message(&session.account, session.secret, to, subject, body); - self.set_send_button_label(cx, true); - self.view.redraw(cx); + for action in actions { + if let StackNavigationTransitionAction::ViewReleased(view_id) = + action.as_widget_action().cast() + { + if self.current_detail_view == Some(view_id) { + self.current_detail_view = None; } } - } - } - - for action in actions { - if let Some(EmailWorkerAction::SendResult(result)) = action.downcast_ref() { - self.set_send_button_label(cx, false); - let msg = match result { - Ok(()) => "Email sent!".to_string(), - Err(e) => format!("Failed: {e}"), - }; - self.label(cx, ids!(send_status)).set_text(cx, &msg); + if let StackNavigationAction::Pop = action.as_widget_action().cast() { + self.pop_detail(cx); + } } } } @@ -194,10 +131,30 @@ impl Widget for EmailComposePage { } impl EmailComposePage { - /// Show "Send" or "Stop waiting" depending on whether a send is in - /// flight, so the button's meaning is never ambiguous. - fn set_send_button_label(&mut self, cx: &mut Cx, in_flight: bool) { - let text = if in_flight { "Stop waiting" } else { "Send" }; - self.button(cx, ids!(send_btn)).set_text(cx, text); + fn push_detail(&mut self, cx: &mut Cx) { + let stack = self.view.stack_navigation(cx, ids!(page_stack)); + if stack.is_transitioning() { + return; + } + if let Some((view_id, _view)) = + stack.create_view_from_template(cx, id!(EmailComposePageDetailStackView)) + { + self.current_detail_view = Some(view_id); + stack.set_title(cx, view_id, "Compose"); + stack.push(cx, view_id); + cx.action(ContextNavAction::HideBottomNav); + self.view.redraw(cx); + } + } + + fn pop_detail(&mut self, cx: &mut Cx) { + let stack = self.view.stack_navigation(cx, ids!(page_stack)); + if stack.is_transitioning() { + return; + } + self.current_detail_view = None; + stack.pop_to_root(cx); + cx.action(ContextNavAction::ShowBottomNav); + self.view.redraw(cx); } } diff --git a/crates/apps/nigig-email/src/email_frame/pages/inbox.rs b/crates/apps/nigig-email/src/email_frame/pages/inbox.rs index fc93dca..d225c9e 100644 --- a/crates/apps/nigig-email/src/email_frame/pages/inbox.rs +++ b/crates/apps/nigig-email/src/email_frame/pages/inbox.rs @@ -22,10 +22,11 @@ use makepad_widgets::*; use nigig_core::email_account::{EmailAccount, SessionState}; use nigig_core::email_store::{ - group_by_sender, thread_for_sender, total_unread, EmailMessage, EmailThreadSummary, + group_by_sender, sample_thread, thread_for_sender, total_unread, EmailMessage, + EmailThreadSummary, }; use nigig_core::email_worker::{ - spawn_fetch_inbox, spawn_proxy_verify, spawn_smtp_test, EmailWorkerAction, SmtpConfig, + spawn_proxy_verify, spawn_smtp_test, EmailWorkerAction, SmtpConfig, }; use nigig_core::mail_backend::{BackendSettings, SetupDraft}; use nigig_core::secret::Secret; @@ -33,7 +34,7 @@ use nigig_uikit::shared::conversation::conversation_preview::{ SharedConversationPreviewAction, SharedConversationPreviewProps, }; -use nigig_uikit::shared::context_nav_action::ContextNavAction; +use crate::shared::context_nav_action::ContextNavAction; use super::account_setup::{EmailAccountSetupAction, EmailAccountSetupWidgetExt}; @@ -67,13 +68,6 @@ script_mod! { text: "" draw_text +: { color: #x1a73e8, text_style: theme.font_bold { font_size: 12.0 } } } - refresh_btn := Button { - width: Fit, height: 34 - text: "Refresh" - padding: Inset{left: 12, right: 12, top: 0, bottom: 0} - draw_bg +: { color: #xEEF4FF, color_hover: #xDDEBFF, border_radius: 12.0 } - draw_text +: { color: #x1C274C, text_style: theme.font_bold { font_size: 11.0 } } - } } // Signed out vs signed in. Only one is ever visible. @@ -98,19 +92,16 @@ script_mod! { spacing: 0.0 conversation_preview := mod.widgets.SharedConversationPreview {} - // One status view, re-labelled for the three - // no-mail states: loading, error, empty. - status_state := View { + empty_state := View { width: Fill, height: 220 flow: Down align: Align{x: 0.5, y: 0.5} spacing: 8 - status_title := Label { - width: Fit, height: Fit + Label { text: "No mail yet" draw_text +: { color: #x1C274C, text_style: theme.font_bold { font_size: 15.0 } } } - status_subtitle := Label { + Label { width: 260, height: Fit text: "Messages from your account will appear here." draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 11.0 } } @@ -180,12 +171,6 @@ pub struct EmailInboxPage { messages: Vec, #[rust] threads: Vec, - /// C4b: an inbox fetch is in flight (drives the "Loading…" status). - #[rust] - loading: bool, - /// C4b: the last fetch failed; the message is shown in the status view. - #[rust] - load_error: Option, #[rust] current_thread_view: Option, @@ -233,13 +218,6 @@ impl Widget for EmailInboxPage { } } - // Refresh (C7): re-fetch the inbox for the signed-in backend. - if let Event::Actions(actions) = event { - if self.button(cx, ids!(refresh_btn)).clicked(actions) { - self.begin_fetch(cx); - } - } - // Connection test result (SMTP or proxy) decides signed-in vs // failed. Both carry a plain `Result<(), String>`, so the flow is // transport-agnostic. @@ -251,16 +229,6 @@ impl Widget for EmailInboxPage { if let Some(EmailWorkerAction::ProxyVerifyResult(result)) = action.downcast_ref() { self.finish_connect(cx, result.clone()); } - if let Some(EmailWorkerAction::InboxFetched(result)) = action.downcast_ref() { - self.finish_fetch(cx, result.clone()); - } - // Sign-out from the More page: forget the account and show - // the setup form again. - if let Some(nigig_core::email_session::EmailSessionAction::SignedOut) = - action.downcast_ref() - { - self.sign_out(cx); - } } } @@ -280,7 +248,7 @@ impl Widget for EmailInboxPage { while let Some(item_id) = list.next_visible_item(cx) { if count == 0 { if item_id == 0 { - let item = list.item(cx, item_id, id!(status_state)); + let item = list.item(cx, item_id, id!(empty_state)); item.draw_all(cx, &mut Scope::empty()); } continue; @@ -409,15 +377,13 @@ impl EmailInboxPage { }; match result { Ok(()) => { - self.session = SessionState::SignedIn(account.clone()); - // Share the signed-in account with the Compose and More - // pages (C5), then fetch real mail (C4b) -- the sample - // data is gone from the UI path; the list is populated by - // the backend now. - nigig_core::email_session::set_session(account.clone(), self.password.clone()); - Cx::post_action(nigig_core::email_session::EmailSessionAction::SignedIn); + self.session = SessionState::SignedIn(account); + // No receive path exists yet (assessment A1), so the list + // is populated from sample data. When a real fetch lands + // it fills `self.messages` and nothing else changes. + self.messages = sample_thread(); + self.rebuild_threads(); self.show_signed_in(cx, true); - self.begin_fetch(cx); } Err(reason) => { self.setup_error(cx, &reason); @@ -431,61 +397,6 @@ impl EmailInboxPage { self.view.redraw(cx); } - /// Start (or restart) an inbox fetch for the signed-in backend. - /// - /// C4b/C7: this is the one place a fetch is kicked off -- on sign-in - /// and on the Refresh button. It shows the loading state immediately - /// and posts `InboxFetched` when the worker returns, which is the - /// worker -> results -> drained-on-UI-thread shape SMS uses. - fn begin_fetch(&mut self, cx: &mut Cx) { - if !self.session.is_signed_in() { - return; - } - self.loading = true; - self.load_error = None; - self.set_status(cx, "Loading…", "Fetching your inbox."); - self.view.redraw(cx); - - let (settings, secret) = match &self.session { - SessionState::SignedIn(a) => (a.backend.clone(), self.password.clone()), - _ => return, - }; - spawn_fetch_inbox(settings, secret); - } - - /// Handle a completed fetch: fill the list, or show the failure. - fn finish_fetch(&mut self, cx: &mut Cx, result: Result, String>) { - self.loading = false; - match result { - Ok(msgs) => { - self.load_error = None; - self.messages = msgs; - self.rebuild_threads(); - self.set_status( - cx, - "No mail yet", - "Messages from your account will appear here.", - ); - } - Err(reason) => { - self.load_error = Some(reason.clone()); - self.messages.clear(); - self.threads.clear(); - self.set_status(cx, "Couldn't load mail", &reason); - } - } - self.show_signed_in(cx, self.session.is_signed_in()); - self.view.redraw(cx); - } - - /// Set the two labels of the no-mail status view. - fn set_status(&mut self, cx: &mut Cx, title: &str, subtitle: &str) { - self.view.label(cx, ids!(status_title)).set_text(cx, title); - self.view - .label(cx, ids!(status_subtitle)) - .set_text(cx, subtitle); - } - /// Show a message on the setup form. /// /// `email_account_setup(..)` yields a `Ref`, which exposes only the @@ -512,21 +423,6 @@ impl EmailInboxPage { self.threads = group_by_sender(&self.messages); } - /// Forget the signed-in account and return to the setup form. - fn sign_out(&mut self, cx: &mut Cx) { - self.session = SessionState::SignedOut; - self.password.clear(); - self.pending = None; - self.messages.clear(); - self.threads.clear(); - self.loading = false; - self.load_error = None; - self.current_thread_view = None; - self.open_sender = None; - self.show_signed_in(cx, false); - self.view.redraw(cx); - } - fn show_signed_in(&mut self, cx: &mut Cx, signed_in: bool) { let page = if signed_in { id!(signed_in_page) diff --git a/crates/apps/nigig-email/src/email_frame/pages/more.rs b/crates/apps/nigig-email/src/email_frame/pages/more.rs index b1b3031..0b3e0d2 100644 --- a/crates/apps/nigig-email/src/email_frame/pages/more.rs +++ b/crates/apps/nigig-email/src/email_frame/pages/more.rs @@ -1,14 +1,5 @@ -// More: the connected account, and sign-out (Phase D1). -// -// This page was the last of the four placeholder scaffolds -- every other -// page (Inbox, Compose, Bulk) is now real. D1's "extract the shared -// scaffold" is therefore moot as a deduplication exercise: there is no -// duplicated scaffold left to extract, so the remaining work is to give -// this page real content and delete the placeholder. It shows the signed-in -// account (from `nigig_core::email_session`) and lets the user disconnect. - +use crate::shared::context_nav_action::ContextNavAction; use makepad_widgets::*; -use nigig_core::email_session::EmailSessionAction; script_mod! { use mod.prelude.widgets.* @@ -16,65 +7,84 @@ script_mod! { mod.widgets.EmailMorePage = #(EmailMorePage::register_widget(vm)) { width: Fill, height: Fill - flow: Down - page_top_bar := SolidView { - width: Fill, height: 52 - flow: Right - align: Align{y: 0.5} - padding: Inset{left: 16, right: 16, top: 0, bottom: 0} - show_bg: true - draw_bg +: { color: #xFFFFFF } - - Label { - width: Fill, height: Fit - text: "More" - draw_text +: { color: #x1C274C, text_style: theme.font_bold { font_size: 20.0 } } - } - } - - more_scroll := ScrollYView { - width: Fill, height: Fill - flow: Down - padding: Inset{left: 18, right: 18, top: 12, bottom: 22} - spacing: 12 - - account_card := RoundedView { - width: Fill, height: Fit + page_stack := StackNavigation { + root_view +: { + width: Fill, height: Fill flow: Down - spacing: 8 - padding: 18 - show_bg: true - draw_bg +: { color: #xF8FAFC, border_radius: 22.0, border_size: 1.0, border_color: #xE2E8F0 } - Label { - text: "Account" - draw_text +: { color: #x1C274C, text_style: theme.font_bold { font_size: 22.0 } } + page_top_bar := SolidView { + width: Fill, height: 52 + flow: Right + align: Align{y: 0.5} + padding: Inset{left: 16, right: 16, top: 0, bottom: 0} + show_bg: true + draw_bg +: { color: #xFFFFFF } + + Label { + width: Fill, height: Fit + text: "More" + draw_text +: { color: #x1C274C, text_style: theme.font_bold { font_size: 20.0 } } + } } - account_status := Label { - width: Fill, height: Fit - text: "No account connected" - draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 11.0 } } - } + page_body := ScrollYView { + width: Fill, height: Fill + flow: Down + padding: Inset{left: 18, right: 18, top: 12, bottom: 22} + spacing: 12 - backend_label := Label { - width: Fill, height: Fit - text: "" - draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 11.0 } } - } + RoundedView { + width: Fill, height: Fit + flow: Down + spacing: 8 + padding: 18 + show_bg: true + draw_bg +: { color: #xF8FAFC, border_radius: 22.0, border_size: 1.0, border_color: #xE2E8F0 } + Label { text: "More" draw_text +: { color: #x1C274C, text_style: theme.font_bold { font_size: 22.0 } } } + Label { width: Fill, height: Fit, text: "Top app bar page. Tap below to open a stack screen with RobrixStackNavigationView back navigation." draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 11.0 } } } + } - sign_out_btn := Button { - width: Fill, height: 46 - text: "Sign out" - draw_bg +: { color: #xB4232C, color_hover: #xD14A52, border_radius: 14.0 } - draw_text +: { color: #xFFFFFF, text_style: theme.font_bold { font_size: 13.0 } } + open_detail_btn := Button { + width: Fill, height: 54 + text: "Open More workflow" + draw_bg +: { color: #xEEF4FF, color_hover: #xDDEBFF, color_down: #xCFE2FF, border_radius: 16.0, border_size: 1.0, border_color: #xD7E5FF } + draw_text +: { color: #x1C274C, text_style: theme.font_bold { font_size: 12.0 } } + } } + } - about_label := Label { - width: Fill, height: Fit - text: "Your password is kept for this session only. Signing out removes it." - draw_text +: { color: #x94A3B8, text_style: theme.font_regular { font_size: 10.0 } } + stack_templates: { + EmailMorePageDetailStackView := mod.widgets.RobrixStackNavigationView { + body +: { + detail_body := ScrollYView { + width: Fill, height: Fill + flow: Down + padding: Inset{left: 18, right: 18, top: 18, bottom: 22} + spacing: 12 + + Label { + width: Fill, height: Fit + text: "More details" + draw_text +: { color: #x1C274C, text_style: theme.font_bold { font_size: 22.0 } } + } + Label { + width: Fill, height: Fit + text: "This is a RobrixStackNavigationView destination. The built-in header above supplies the title and back arrow, just like SMS conversation screens." + draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 11.0 } } + } + RoundedView { + width: Fill, height: Fit + flow: Down + padding: 16 + spacing: 8 + show_bg: true + draw_bg +: { color: #xF8FAFC, border_radius: 18.0, border_size: 1.0, border_color: #xE2E8F0 } + Label { text: "Next screen content" draw_text +: { color: #x1C274C, text_style: theme.font_bold { font_size: 13.0 } } } + Label { width: Fill, height: Fit, text: "Replace this scaffold with the real workflow for More." draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 10.5 } } } + } + } + } } } } @@ -85,30 +95,33 @@ script_mod! { pub struct EmailMorePage { #[deref] view: View, + #[rust] + current_detail_view: Option, } impl Widget for EmailMorePage { fn handle_event(&mut self, cx: &mut Cx, event: &Event, scope: &mut Scope) { + // First forward the event so dynamic StackNavigation children can produce actions. + // Then handle the Event::Actions carried by this turn. This follows the SMS/Home pattern. self.view.handle_event(cx, event, scope); - let Event::Actions(actions) = event else { - return; - }; - - // Keep the account card in sync with sign-in/sign-out, wherever - // they happened (both are posted globally). - for action in actions { - match action.downcast_ref() { - Some(EmailSessionAction::SignedIn) | Some(EmailSessionAction::SignedOut) => { - self.refresh(cx); - } - _ => {} + if let Event::Actions(actions) = event { + if self.view.button(cx, ids!(open_detail_btn)).clicked(actions) { + self.push_detail(cx); } - } - if self.button(cx, ids!(sign_out_btn)).clicked(actions) { - nigig_core::email_session::clear_session(); - Cx::post_action(EmailSessionAction::SignedOut); + for action in actions { + if let StackNavigationTransitionAction::ViewReleased(view_id) = + action.as_widget_action().cast() + { + if self.current_detail_view == Some(view_id) { + self.current_detail_view = None; + } + } + if let StackNavigationAction::Pop = action.as_widget_action().cast() { + self.pop_detail(cx); + } + } } } @@ -118,26 +131,30 @@ impl Widget for EmailMorePage { } impl EmailMorePage { - /// Reflect the current session in the account card. Called before draw - /// so the page is always in sync with sign-in/sign-out. - pub fn refresh(&mut self, cx: &mut Cx) { - match nigig_core::email_session::current_session() { - Some(s) => { - self.view - .label(cx, ids!(account_status)) - .set_text(cx, &format!("Connected as {}", s.account.address)); - self.view.label(cx, ids!(backend_label)).set_text( - cx, - &format!("Connected via {}", s.account.backend.kind().label()), - ); - } - None => { - self.view - .label(cx, ids!(account_status)) - .set_text(cx, "No account connected"); - self.view.label(cx, ids!(backend_label)).set_text(cx, ""); - } + fn push_detail(&mut self, cx: &mut Cx) { + let stack = self.view.stack_navigation(cx, ids!(page_stack)); + if stack.is_transitioning() { + return; } + if let Some((view_id, _view)) = + stack.create_view_from_template(cx, id!(EmailMorePageDetailStackView)) + { + self.current_detail_view = Some(view_id); + stack.set_title(cx, view_id, "More"); + stack.push(cx, view_id); + cx.action(ContextNavAction::HideBottomNav); + self.view.redraw(cx); + } + } + + fn pop_detail(&mut self, cx: &mut Cx) { + let stack = self.view.stack_navigation(cx, ids!(page_stack)); + if stack.is_transitioning() { + return; + } + self.current_detail_view = None; + stack.pop_to_root(cx); + cx.action(ContextNavAction::ShowBottomNav); self.view.redraw(cx); } } diff --git a/crates/apps/nigig-email/src/lib.rs b/crates/apps/nigig-email/src/lib.rs index 1342a6d..a95bad4 100644 --- a/crates/apps/nigig-email/src/lib.rs +++ b/crates/apps/nigig-email/src/lib.rs @@ -1,9 +1,56 @@ use makepad_widgets::ScriptVm; pub mod email_frame; -pub mod navigation; pub fn script_mod(vm: &mut ScriptVm) { nigig_uikit::script_mod(vm); email_frame::script_mod(vm); } + +// Compatibility shims for source moved out of pageflipnav during staged migration. +pub mod dir { + pub use nigig_core::dir::*; +} +pub mod shared { + pub use nigig_uikit::shared::*; +} +pub mod persistence { + pub use nigig_core::persistence::*; + pub mod offline_store { + pub use nigig_core::persistence::offline_store::*; + } + pub mod app_state { + pub use nigig_core::persistence::app_state::*; + } + #[cfg(not(target_arch = "wasm32"))] + pub mod matrix_state { + pub use nigig_core::persistence::matrix_state::*; + } +} +pub mod features { + pub mod action_page_navigation { + pub use nigig_uikit::action_page_navigation::*; + } +} +#[cfg(not(target_arch = "wasm32"))] +pub mod tile_service { + pub use nigig_core::tile_service::*; +} +#[cfg(not(target_arch = "wasm32"))] +pub mod location { + pub use nigig_core::location::*; +} +pub mod home { + pub mod navigation_tab_bar { + #[derive(Clone, Debug, PartialEq, Eq)] + pub enum NavigationBarAction { + ReturnToHome, + } + impl makepad_widgets::ActionDefaultRef for NavigationBarAction { + fn default_ref() -> &'static Self { + static DEFAULT: NavigationBarAction = NavigationBarAction::ReturnToHome; + &DEFAULT + } + } + } +} diff --git a/crates/apps/nigig-email/src/navigation.rs b/crates/apps/nigig-email/src/navigation.rs deleted file mode 100644 index cc67cde..0000000 --- a/crates/apps/nigig-email/src/navigation.rs +++ /dev/null @@ -1,23 +0,0 @@ -//! The one navigation action the standalone email app understands. -//! -//! `ReturnToHome` is emitted when the user double-taps a bottom-nav tab, to -//! ask a host (pageflipnav) to return to its home dashboard. The -//! standalone `nigig-email` binary has no home to return to, so it simply -//! does not handle the action; the host does. -//! -//! This type used to live in a "compatibility shim" in `lib.rs` (a UI enum -//! declared inline in a re-export module). It now lives here, in a real -//! module, because D2 ruled that a UI type in a compat shim is not -//! acceptable long-term. - -#[derive(Clone, Debug, PartialEq, Eq)] -pub enum NavigationBarAction { - ReturnToHome, -} - -impl makepad_widgets::ActionDefaultRef for NavigationBarAction { - fn default_ref() -> &'static Self { - static DEFAULT: NavigationBarAction = NavigationBarAction::ReturnToHome; - &DEFAULT - } -} diff --git a/crates/nigig-core/Cargo.toml b/crates/nigig-core/Cargo.toml index a90e12b..3eb2041 100644 --- a/crates/nigig-core/Cargo.toml +++ b/crates/nigig-core/Cargo.toml @@ -8,10 +8,6 @@ edition = "2021" default = ["native"] native = ["dep:tokio", "dep:reqwest", "dep:lettre", "matrix_client/native", "async-rt"] async-rt = [] -# C1e: the direct backend's IMAP protocol client (async-imap over native -# TLS). Native only -- a browser cannot open a raw TCP socket -- and off by -# default so builds that do not need IMAP never compile its dependency tree. -imap = ["dep:async-imap", "dep:async-native-tls", "dep:async-net"] [dependencies] matrix_client = { path = "../matrix_client", default-features = false } @@ -47,10 +43,6 @@ lettre = { version = "0.11", default-features = false, features = [ "ring", "webpki-roots", ], optional = true } -# C1e: the IMAP protocol client (feature `imap`), native only. -async-imap = { version = "0.9", optional = true } -async-native-tls = { version = "0.5", optional = true } -async-net = { version = "2", optional = true } [target.'cfg(target_arch = "wasm32")'.dependencies] wasm-bindgen = "0.2" diff --git a/crates/nigig-core/src/credential_store.rs b/crates/nigig-core/src/credential_store.rs deleted file mode 100644 index ef9d8b3..0000000 --- a/crates/nigig-core/src/credential_store.rs +++ /dev/null @@ -1,179 +0,0 @@ -//! Credential storage boundary (Phase C1f). -//! -//! C1 decided to support two backends. The direct backend keeps a -//! *reusable* mailbox password on the device, which is the one thing the -//! proxy backend exists to avoid. If that password is to survive an app -//! restart -- which IMAP needs to be usable at all -- it must live in the -//! platform keystore, not in the app's plaintext storage. -//! -//! This is the seam. It deliberately does NOT implement any keystore here: -//! the in-repo precedent is the SMS crate's `SmsScheduleCrypto.java` -//! (AES-256-GCM via `AndroidKeyStore`), which is Android-only and cannot be -//! exercised on a host. What this module provides is the contract the UI -//! and the persistence layer code against, plus a fail-closed default. -//! -//! ## Why the default fails closed -//! -//! A "remember my password" feature that silently falls back to plaintext -//! JSON when the keystore is missing is worse than no feature: it looks -//! secure and is not. So `store` on the default returns an error and the -//! UI must say "your password will be kept for this session only" instead -//! of pretending otherwise. This is the same honesty rule `Secret` applies -//! to zeroisation (see `secret.rs`). - -use crate::secret::Secret; - -/// A place credentials can be stored and read back. -/// -/// Keyed by an account identifier chosen by the caller (the normalised -/// address, or a provider-issued account id). Implementations decide -/// where the bytes actually go; the contract only promises that `load` -/// returns what `store` put, and that `delete` removes it. -pub trait CredentialStore { - /// Store `secret` under `account_id`. Returns `Err` when the platform - /// has no keystore available, so the caller can fall back to - /// session-only (never to plaintext). - fn store(&self, account_id: &str, secret: &Secret) -> Result<(), String>; - - /// Read back a stored secret, or `None` if none is stored. - fn load(&self, account_id: &str) -> Result, String>; - - /// Remove a stored secret. Idempotent: deleting a missing secret is - /// success, not an error. - fn delete(&self, account_id: &str) -> Result<(), String>; -} - -/// The fail-closed default: no keystore, so nothing is ever stored. -/// -/// `load` returns `None`, `delete` succeeds (there is nothing to delete), -/// and `store` refuses. Every platform implementation replaces this via a -/// build-time selection; the point of having it at all is that the -/// session-only behaviour is an explicit choice, not an accidental one. -#[derive(Clone, Copy, Debug, Default)] -pub struct UnavailableCredentialStore; - -impl CredentialStore for UnavailableCredentialStore { - fn store(&self, _account_id: &str, _secret: &Secret) -> Result<(), String> { - Err("No secure credential storage is available on this build. \ - Your password is kept for this session only." - .into()) - } - - fn load(&self, _account_id: &str) -> Result, String> { - Ok(None) - } - - fn delete(&self, _account_id: &str) -> Result<(), String> { - Ok(()) - } -} - -/// The credential store in use on this build. -/// -/// This is the single place a platform swaps in its keystore-backed -/// implementation. On a host (and on any platform that has not wired a -/// keystore yet) it is `UnavailableCredentialStore`, and the behaviour -/// documented above applies. -pub fn active_store() -> &'static dyn CredentialStore { - // Kept behind an indirection so a platform impl can be selected by - // cfg without changing callers. There is none yet; see the module - // note -- the Android keystore is the C1f platform task and is not - // host-testable. - static DEFAULT: UnavailableCredentialStore = UnavailableCredentialStore; - &DEFAULT -} - -#[cfg(test)] -mod tests { - use super::*; - - /// An in-memory store that exercises the full contract; NOT a - /// production impl (it holds plaintext), it exists to pin the trait - /// semantics the UI and persistence layers rely on. - #[derive(Default)] - struct InMemoryStore { - map: std::sync::Mutex>, - } - - impl CredentialStore for InMemoryStore { - fn store(&self, id: &str, secret: &Secret) -> Result<(), String> { - self.map - .lock() - .map_err(|_| "lock poisoned".to_string())? - .insert(id.to_string(), secret.clone()); - Ok(()) - } - - fn load(&self, id: &str) -> Result, String> { - Ok(self - .map - .lock() - .map_err(|_| "lock poisoned".to_string())? - .get(id) - .cloned()) - } - - fn delete(&self, id: &str) -> Result<(), String> { - self.map - .lock() - .map_err(|_| "lock poisoned".to_string())? - .remove(id); - Ok(()) - } - } - - #[test] - fn the_unavailable_store_refuses_to_store_and_loads_nothing() { - let store = UnavailableCredentialStore; - let secret = Secret::new("hunter2"); - assert!(store.store("jane@example.com", &secret).is_err()); - assert_eq!(store.load("jane@example.com").unwrap(), None); - // Delete is idempotent success. - assert!(store.delete("jane@example.com").is_ok()); - } - - #[test] - fn the_default_store_never_retains_a_secret() { - // active_store() is the fail-closed default; storing must refuse - // rather than silently persist in plaintext. - let store = active_store(); - assert!(store - .store("jane@example.com", &Secret::new("hunter2")) - .is_err()); - assert_eq!(store.load("jane@example.com").unwrap(), None); - } - - #[test] - fn store_then_load_returns_the_same_secret() { - let store = InMemoryStore::default(); - let secret = Secret::new("app-password-123"); - store.store("jane@example.com", &secret).unwrap(); - let got = store.load("jane@example.com").unwrap().unwrap(); - assert_eq!(got.expose(), "app-password-123"); - } - - #[test] - fn deleting_is_idempotent_and_removes_the_secret() { - let store = InMemoryStore::default(); - assert!(store.delete("missing@example.com").is_ok()); - - store.store("jane@example.com", &Secret::new("x")).unwrap(); - store.delete("jane@example.com").unwrap(); - assert_eq!(store.load("jane@example.com").unwrap(), None); - } - - #[test] - fn accounts_are_isolated_by_identifier() { - let store = InMemoryStore::default(); - store.store("jane@example.com", &Secret::new("a")).unwrap(); - store.store("boss@example.com", &Secret::new("b")).unwrap(); - assert_eq!( - store.load("jane@example.com").unwrap().unwrap().expose(), - "a" - ); - assert_eq!( - store.load("boss@example.com").unwrap().unwrap().expose(), - "b" - ); - } -} diff --git a/crates/nigig-core/src/email_cache.rs b/crates/nigig-core/src/email_cache.rs deleted file mode 100644 index 474b1e9..0000000 --- a/crates/nigig-core/src/email_cache.rs +++ /dev/null @@ -1,317 +0,0 @@ -//! Local mail cache with encrypted bodies (Phase C3). -//! -//! The inbox has to render without a network round trip, so fetched mail -//! is cached locally. `EmailMessage` is `Serialize`/`Deserialize`, but -//! unlike `OfflineSmsMessage.body` its `body` is deliberately NOT -//! `#[serde(skip)]`-ed -- a mail cache that drops bodies is useless, and -//! the body is the thing the user came to read. That means the cache -//! **must** encrypt the body at rest; the plan records this as mandatory, -//! not optional. -//! -//! ## What this module is, and what it is not -//! -//! The serialization pipeline is here and is fully host-tested: every -//! message's body is pushed through a `BodyCipher` before it is written, -//! and the ciphertext (never the plaintext) is what lands in the JSON. -//! -//! The *production* cipher is the platform keystore's symmetric key -//! (C1f), which cannot be exercised on a host. So the default cipher is -//! `PlaintextBodyCipher` -- bodies stored as-is, **named honestly** so a -//! reader cannot mistake it for encryption. The moment C1f lands a real -//! keystore key, it is swapped in at `active_cipher()` and every test -//! below continues to pin the pipeline. -//! -//! ## Why the wire type is separate -//! -//! `EmailMessage` derives `Serialize` with a plain `body: String`. Writing -//! that directly would serialize the plaintext even when the cipher is -//! real. So the cache uses a private `StoredMessage` whose only body field -//! is `body_ciphertext` (hex). This is the structural half of "encrypted -//! at rest": there is no code path that writes an `EmailMessage` straight -//! to disk. - -use serde::{Deserialize, Serialize}; - -use crate::email_store::EmailMessage; - -/// Encrypts and decrypts message bodies for at-rest storage. -/// -/// `encrypt` never fails (it always produces some bytes); `decrypt` -/// returns `None` on anything it cannot reverse, so a corrupted or -/// tampered body fails closed to a placeholder rather than panicking. -pub trait BodyCipher { - fn encrypt(&self, plaintext: &str) -> Vec; - fn decrypt(&self, ciphertext: &[u8]) -> Option; -} - -/// The honest "not yet encrypted" default. -/// -/// Bodies are stored as their own bytes. This exists so the pipeline is -/// complete and tested before the keystore-backed cipher (C1f) exists; it -/// is named to be impossible to mistake for security. A cache written -/// today is readable by a future build only if that build still uses this -/// cipher -- which is fine, because nothing claims otherwise. -#[derive(Clone, Copy, Debug, Default)] -pub struct PlaintextBodyCipher; - -impl BodyCipher for PlaintextBodyCipher { - fn encrypt(&self, plaintext: &str) -> Vec { - plaintext.as_bytes().to_vec() - } - - fn decrypt(&self, ciphertext: &[u8]) -> Option { - String::from_utf8(ciphertext.to_vec()).ok() - } -} - -/// The cipher in use on this build. -pub fn active_cipher() -> &'static dyn BodyCipher { - static DEFAULT: PlaintextBodyCipher = PlaintextBodyCipher; - &DEFAULT -} - -/// One message as stored on disk: the body is ciphertext, never plaintext. -#[derive(Serialize, Deserialize)] -struct StoredMessage { - id: String, - from_address: String, - from_name: String, - subject: String, - date_ms: i64, - is_read: bool, - is_outgoing: bool, - /// Hex-encoded ciphertext of the body. - body_ciphertext: String, -} - -#[derive(Serialize, Deserialize)] -struct StoredInbox { - messages: Vec, -} - -/// Encode bytes as lowercase hex. No dependency: 16 lines, and it is the -/// format the wire type commits to, so it is tested below. -fn to_hex(bytes: &[u8]) -> String { - const DIGITS: &[u8; 16] = b"0123456789abcdef"; - let mut out = String::with_capacity(bytes.len() * 2); - for &b in bytes { - out.push(DIGITS[(b >> 4) as usize] as char); - out.push(DIGITS[(b & 0xf) as usize] as char); - } - out -} - -fn from_hex(s: &str) -> Option> { - if s.len() % 2 != 0 { - return None; - } - let bytes = s.as_bytes(); - (0..s.len()) - .step_by(2) - .map(|i| { - let hi = hex_val(bytes[i])?; - let lo = hex_val(bytes[i + 1])?; - Some((hi << 4) | lo) - }) - .collect() -} - -fn hex_val(b: u8) -> Option { - match b { - b'0'..=b'9' => Some(b - b'0'), - b'a'..=b'f' => Some(b - b'a' + 10), - b'A'..=b'F' => Some(b - b'A' + 10), - _ => None, - } -} - -/// Serialize messages for disk, encrypting every body through `cipher`. -/// -/// Pure: no I/O, so every branch is host-testable. Returns the JSON that -/// the caller writes verbatim. -pub fn serialize_inbox( - messages: &[EmailMessage], - cipher: &dyn BodyCipher, -) -> Result { - let stored: Vec = messages - .iter() - .map(|m| { - let ct = cipher.encrypt(&m.body); - StoredMessage { - id: m.id.clone(), - from_address: m.from_address.clone(), - from_name: m.from_name.clone(), - subject: m.subject.clone(), - date_ms: m.date_ms, - is_read: m.is_read, - is_outgoing: m.is_outgoing, - body_ciphertext: to_hex(&ct), - } - }) - .collect(); - serde_json::to_string(&StoredInbox { messages: stored }) - .map_err(|e| format!("failed to serialise mail cache: {e}")) -} - -/// Deserialize messages from disk, decrypting every body through `cipher`. -/// -/// A message whose body will not decrypt is dropped rather than shown -/// half-decoded; a body the cipher cannot reverse is worse than no body. -/// Metadata-only messages are kept so the list still renders. -pub fn deserialize_inbox(json: &str, cipher: &dyn BodyCipher) -> Result, String> { - let inbox: StoredInbox = - serde_json::from_str(json).map_err(|e| format!("failed to parse mail cache: {e}"))?; - Ok(inbox - .messages - .into_iter() - .filter_map(|m| { - let bytes = from_hex(&m.body_ciphertext)?; - let body = cipher.decrypt(&bytes)?; - Some(EmailMessage { - id: m.id, - from_address: m.from_address, - from_name: m.from_name, - subject: m.subject, - body, - date_ms: m.date_ms, - is_read: m.is_read, - is_outgoing: m.is_outgoing, - }) - }) - .collect()) -} - -/// The file a given account's cache lives in. -/// -/// Keyed by the normalised address so two accounts do not share a file, -/// and named `mail_.json` so it is obviously a cache and not config. -pub fn cache_path(account_address: &str) -> std::path::PathBuf { - let key = crate::email_store::normalise_sender(account_address); - crate::dir::app_data_dir().join(format!("mail_{}.json", to_hex(key.as_bytes()))) -} - -/// Write `messages` to the account's cache, encrypted at rest. -pub async fn save(account_address: &str, messages: &[EmailMessage]) -> Result<(), String> { - let json = serialize_inbox(messages, active_cipher())?; - crate::platform::fs_write(cache_path(account_address), json.as_bytes()) - .await - .map_err(|e| format!("failed to write mail cache: {e}")) -} - -/// Read the account's cache, or `Ok(vec![])` when none exists yet. -pub async fn load(account_address: &str) -> Result, String> { - let path = cache_path(account_address); - let bytes = match crate::platform::fs_read(&path).await { - Ok(b) => b, - // No cache yet is the common first-run case, not an error. - Err(_) => return Ok(Vec::new()), - }; - let json = String::from_utf8(bytes).map_err(|_| "mail cache is not UTF-8".to_string())?; - deserialize_inbox(&json, active_cipher()) -} - -#[cfg(test)] -mod tests { - use super::*; - - fn msg(id: &str, body: &str) -> EmailMessage { - EmailMessage { - id: id.into(), - from_address: "alerts@bank.co.ke".into(), - from_name: "Equity Alerts".into(), - subject: "Statement".into(), - body: body.into(), - date_ms: 1_767_225_600_000, - is_read: false, - is_outgoing: false, - } - } - - /// A reversible cipher for tests only: XOR with a fixed byte. Proves - /// the round trip AND that the stored form is not the plaintext, so a - /// regression to "write EmailMessage directly" is caught by - /// `the_stored_json_never_contains_plaintext_bodies`. - #[derive(Clone, Copy)] - struct XorCipher; - - impl BodyCipher for XorCipher { - fn encrypt(&self, plaintext: &str) -> Vec { - plaintext.bytes().map(|b| b ^ 0x5a).collect() - } - fn decrypt(&self, ciphertext: &[u8]) -> Option { - String::from_utf8(ciphertext.iter().map(|b| b ^ 0x5a).collect()).ok() - } - } - - #[test] - fn a_round_trip_preserves_every_message_and_its_body() { - let msgs = vec![ - msg("1", "Hello, world"), - msg("2", "Mambo vipi? 🚚 non-Latin 中文"), - msg("3", ""), // empty bodies must survive too - ]; - let json = serialize_inbox(&msgs, &XorCipher).unwrap(); - let back = deserialize_inbox(&json, &XorCipher).unwrap(); - assert_eq!(back, msgs); - } - - #[test] - fn the_stored_json_never_contains_plaintext_bodies() { - let msgs = vec![msg("1", "super-secret-body")]; - let json = serialize_inbox(&msgs, &XorCipher).unwrap(); - assert!( - !json.contains("super-secret-body"), - "plaintext body leaked to disk: {json}" - ); - assert!(json.contains("body_ciphertext"), "wire shape changed"); - } - - #[test] - fn an_empty_inbox_round_trips_to_empty() { - let json = serialize_inbox(&[], &XorCipher).unwrap(); - assert!(deserialize_inbox(&json, &XorCipher).unwrap().is_empty()); - } - - #[test] - fn the_plaintext_cipher_is_a_noop_but_still_round_trips() { - let msgs = vec![msg("1", "visible body")]; - let json = serialize_inbox(&msgs, &PlaintextBodyCipher).unwrap(); - let back = deserialize_inbox(&json, &PlaintextBodyCipher).unwrap(); - assert_eq!(back, msgs); - } - - #[test] - fn a_body_the_cipher_cannot_decrypt_is_dropped_not_garbled() { - let msgs = vec![msg("1", "ok"), msg("2", "ok")]; - let json = serialize_inbox(&msgs, &XorCipher).unwrap(); - // Corrupt only the first message's ciphertext into invalid hex. - let corrupted = json.replacen("\"body_ciphertext\":\"", "\"body_ciphertext\":\"zz", 1); - let back = deserialize_inbox(&corrupted, &XorCipher).unwrap(); - assert_eq!(back.len(), 1, "corrupt message should be dropped"); - assert_eq!(back[0].id, "2"); - } - - #[test] - fn malformed_json_is_an_error_not_a_panic() { - assert!(deserialize_inbox("not json", &XorCipher).is_err()); - assert!(deserialize_inbox("{\"nope\":1}", &XorCipher).is_err()); - } - - #[test] - fn hex_round_trips_and_rejects_odd_length() { - assert_eq!(from_hex(&to_hex(b"abc")).unwrap(), b"abc"); - assert_eq!(from_hex("00ff10").unwrap(), vec![0x00, 0xff, 0x10]); - assert_eq!(from_hex("0"), None, "odd length"); - assert_eq!(from_hex("zz"), None, "non-hex"); - } - - #[test] - fn cache_paths_are_per_account_and_stable() { - let a = cache_path("Jane@Example.com"); - let b = cache_path("jane@example.com"); - let c = cache_path("boss@example.com"); - // Normalisation means case variants share one file. - assert_eq!(a, b); - assert_ne!(a, c); - } -} diff --git a/crates/nigig-core/src/email_pacing.rs b/crates/nigig-core/src/email_pacing.rs deleted file mode 100644 index 774b7e0..0000000 --- a/crates/nigig-core/src/email_pacing.rs +++ /dev/null @@ -1,298 +0,0 @@ -//! Send pacing and rate limiting for email (Phase C6). -//! -//! Ported from `robius-sms`'s `SendRateLimiter`/`SendPacing` -- the plan -//! (C6) called them out as "already generic arithmetic", and they are: a -//! pure token bucket plus a pure gap schedule, no sleeping, no platform -//! code, so every branch is assertable on a host with no server. -//! -//! ## Why email gets its own copy instead of a shared dep -//! -//! The numbers differ. A carrier silently drops or prompts past ~30 SMS -//! per 30 minutes per app. An SMTP provider rate-limits *harder and more -//! opaquely*: Gmail rejects a burst with a 421/450 and a "too many -//! connections" that the user cannot decode, and consumer relays cap -//! recipients-per-message around 100 (already enforced by -//! `email_send::MAX_RECIPIENTS`). So the bucket and the recommended gap -//! are email-shaped, not carrier-shaped. -//! -//! ## Honest scope -//! -//! The current bulk path sends ONE message to N recipients in a single -//! SMTP transaction, so no per-recipient pacing loop exists yet to wire -//! this into. This module is the primitive: it is tested here, and it is -//! what a future per-recipient batch loop (and the Compose bulk feature) -//! will consume. Nothing claims a pacing loop is already running. - -/// A pure token bucket. Callers ask `allow_at` before each send. -#[derive(Clone, Debug)] -pub struct SendRateLimiter { - capacity: u32, - window_ms: i64, - /// Timestamps of sends still inside the window, oldest first. - sent_at_ms: std::collections::VecDeque, -} - -impl SendRateLimiter { - /// Conservative consumer-SMTP ceiling: 100 messages per hour. A burst - /// past this is exactly what gets a relay flagged, and a flagged relay - /// is silently rate-limited for the whole account. - pub const DEFAULT_CAPACITY: u32 = 100; - pub const DEFAULT_WINDOW_MS: i64 = 60 * 60 * 1000; - - pub fn new(capacity: u32, window_ms: i64) -> Self { - Self { - capacity, - window_ms, - sent_at_ms: std::collections::VecDeque::new(), - } - } - - /// Ask permission to send at `now_ms`, recording it if allowed. - /// - /// Returns `Ok(())`, or `Err(wait_ms)` with how long to wait. - pub fn allow_at(&mut self, now_ms: i64) -> Result<(), i64> { - while let Some(&oldest) = self.sent_at_ms.front() { - if now_ms.saturating_sub(oldest) >= self.window_ms { - self.sent_at_ms.pop_front(); - } else { - break; - } - } - - if (self.sent_at_ms.len() as u32) < self.capacity { - self.sent_at_ms.push_back(now_ms); - return Ok(()); - } - - // Full: wait until the oldest send rolls out. A zero-capacity - // bucket records nothing, so the deque can be empty here -- in - // that case the whole window must elapse, not a panic. - match self.sent_at_ms.front() { - Some(&oldest) => Err(self.window_ms - now_ms.saturating_sub(oldest)), - None => Err(self.window_ms), - } - } - - /// Sends still counted against the window as of `now_ms`. - pub fn in_window(&self, now_ms: i64) -> usize { - self.sent_at_ms - .iter() - .filter(|t| now_ms.saturating_sub(**t) < self.window_ms) - .count() - } -} - -impl Default for SendRateLimiter { - fn default() -> Self { - Self::new(Self::DEFAULT_CAPACITY, Self::DEFAULT_WINDOW_MS) - } -} - -/// How long to wait between two sends in a batch, and how long a whole -/// batch will therefore take. Pure arithmetic; no sleeping. -#[derive(Copy, Clone, Debug, Eq, PartialEq)] -pub struct SendPacing { - delay_ms: i64, -} - -impl SendPacing { - /// Longest gap we let a user choose: 10 minutes. - pub const MAX_DELAY_MS: i64 = 10 * 60 * 1000; - - /// Gap that keeps a batch just inside the default relay ceiling - /// (100 messages / hour). - pub const RECOMMENDED_DELAY_MS: i64 = - SendRateLimiter::DEFAULT_WINDOW_MS / SendRateLimiter::DEFAULT_CAPACITY as i64; - - /// Clamps rather than rejecting: this is driven by a text field. - pub fn from_millis(delay_ms: i64) -> Self { - Self { - delay_ms: delay_ms.clamp(0, Self::MAX_DELAY_MS), - } - } - - pub fn from_seconds(delay_s: i64) -> Self { - Self::from_millis(delay_s.saturating_mul(1000)) - } - - /// A gap that spreads `count` messages evenly across the rate - /// limiter's window, so the batch never trips the cap. Returns 0 when - /// the batch already fits. - pub fn to_stay_under(capacity: u32, window_ms: i64, count: usize) -> Self { - if capacity == 0 || count == 0 { - return Self::from_millis(0); - } - if count <= capacity as usize { - return Self::from_millis(0); - } - Self::from_millis(window_ms / capacity as i64) - } - - pub fn delay_ms(self) -> i64 { - self.delay_ms - } - - pub fn is_immediate(self) -> bool { - self.delay_ms == 0 - } - - /// Wall-clock duration of a batch of `count` messages: `count - 1` - /// gaps, not `count`. - pub fn total_duration_ms(self, count: usize) -> i64 { - let gaps = count.saturating_sub(1) as i64; - self.delay_ms.saturating_mul(gaps) - } - - /// Human estimate for a confirmation prompt, e.g. "about 3m 20s". - pub fn describe_duration(self, count: usize) -> String { - let ms = self.total_duration_ms(count); - if ms <= 0 { - return "a few seconds".into(); - } - let total_s = ms / 1000; - let h = total_s / 3600; - let m = (total_s % 3600) / 60; - let s = total_s % 60; - if h > 0 { - format!("about {h}h {m}m") - } else if m > 0 { - format!("about {m}m {s}s") - } else { - format!("about {s}s") - } - } -} - -impl Default for SendPacing { - /// Default to the relay-safe gap rather than zero, so a user who never - /// touches the field gets a batch that completes. - fn default() -> Self { - Self::from_millis(Self::RECOMMENDED_DELAY_MS) - } -} - -#[cfg(test)] -mod tests { - use super::*; - - // ---- SendRateLimiter ---------------------------------------------- - - #[test] - fn a_bucket_admits_up_to_capacity_within_the_window() { - let mut rl = SendRateLimiter::new(3, 1000); - assert!(rl.allow_at(0).is_ok()); - assert!(rl.allow_at(100).is_ok()); - assert!(rl.allow_at(200).is_ok()); - // Fourth is over capacity within the window. - assert!(rl.allow_at(300).is_err()); - } - - #[test] - fn old_sends_roll_out_of_the_window() { - let mut rl = SendRateLimiter::new(1, 1000); - assert!(rl.allow_at(0).is_ok()); - assert!(rl.allow_at(500).is_err(), "still inside the window"); - assert!(rl.allow_at(1000).is_ok(), "window has rolled over"); - } - - #[test] - fn the_wait_is_until_the_oldest_send_rolls_out() { - let mut rl = SendRateLimiter::new(2, 1000); - rl.allow_at(0).unwrap(); - rl.allow_at(300).unwrap(); - // Oldest (0) rolls out at 1000; from 700 that is 300 more ms. - assert_eq!(rl.allow_at(700), Err(300)); - } - - #[test] - fn default_limits_are_100_per_hour() { - assert_eq!(SendRateLimiter::DEFAULT_CAPACITY, 100); - assert_eq!(SendRateLimiter::DEFAULT_WINDOW_MS, 60 * 60 * 1000); - } - - #[test] - fn in_window_counts_only_sends_inside_the_window() { - let mut rl = SendRateLimiter::new(10, 1000); - rl.allow_at(0).unwrap(); - rl.allow_at(500).unwrap(); - assert_eq!(rl.in_window(600), 2); - assert_eq!(rl.in_window(1000), 1, "t=0 has rolled out"); - assert_eq!(rl.in_window(2000), 0); - } - - #[test] - fn a_zero_capacity_bucket_never_admits_but_does_not_panic() { - let mut rl = SendRateLimiter::new(0, 1000); - // Nothing is recorded, so front() must not be unwrapped. - let wait = rl.allow_at(0).unwrap_err(); - assert_eq!(wait, 1000, "wait the full window when capacity is zero"); - } - - // ---- SendPacing ---------------------------------------------------- - - #[test] - fn the_recommended_gap_keeps_a_batch_inside_the_ceiling() { - // 100 / hour == one per 36 seconds. - assert_eq!(SendPacing::RECOMMENDED_DELAY_MS, 36_000); - assert_eq!(SendPacing::default().delay_ms(), 36_000); - } - - #[test] - fn delays_clamp_to_zero_and_the_max() { - assert_eq!(SendPacing::from_millis(-5).delay_ms(), 0); - assert_eq!( - SendPacing::from_millis(i64::MAX).delay_ms(), - SendPacing::MAX_DELAY_MS - ); - assert_eq!( - SendPacing::from_seconds(i64::MAX).delay_ms(), - SendPacing::MAX_DELAY_MS - ); - } - - #[test] - fn to_stay_under_is_zero_when_the_batch_fits() { - assert_eq!(SendPacing::to_stay_under(100, 3_600_000, 50).delay_ms(), 0); - assert_eq!(SendPacing::to_stay_under(100, 3_600_000, 100).delay_ms(), 0); - // Over capacity: spread across the window. - assert_eq!( - SendPacing::to_stay_under(100, 3_600_000, 200).delay_ms(), - 36_000 - ); - // Degenerate inputs never panic. - assert_eq!(SendPacing::to_stay_under(0, 1000, 5).delay_ms(), 0); - assert_eq!(SendPacing::to_stay_under(10, 1000, 0).delay_ms(), 0); - } - - #[test] - fn total_duration_counts_gaps_not_messages() { - // 3 messages at 10s == 2 gaps == 20s. - assert_eq!(SendPacing::from_seconds(10).total_duration_ms(3), 20_000); - // 1 message == no gap. - assert_eq!(SendPacing::from_seconds(10).total_duration_ms(1), 0); - // 0 messages == no gap, no negative panic. - assert_eq!(SendPacing::from_seconds(10).total_duration_ms(0), 0); - } - - #[test] - fn duration_descriptions_are_human_readable() { - assert_eq!( - SendPacing::from_millis(0).describe_duration(50), - "a few seconds" - ); - assert_eq!( - SendPacing::from_seconds(10).describe_duration(4), - "about 30s" - ); - assert_eq!( - SendPacing::from_seconds(120).describe_duration(3), - "about 4m 0s" - ); - // A one-hour total is reachable at the max gap with enough - // recipients: 8 messages at 10m is 7 gaps == 70 minutes. - assert_eq!( - SendPacing::from_seconds(600).describe_duration(8), - "about 1h 10m" - ); - } -} diff --git a/crates/nigig-core/src/email_send.rs b/crates/nigig-core/src/email_send.rs index 1a0ca13..9e9d0d8 100644 --- a/crates/nigig-core/src/email_send.rs +++ b/crates/nigig-core/src/email_send.rs @@ -303,21 +303,7 @@ impl EmailSendRequest { if config.is_incomplete() { return Err(SendError::ConfigIncomplete); } - Self::build_without_config(to, subject, body) - } - /// Validate everything except the SMTP config. - /// - /// C1d/C5: the proxy backend sends through the mail service, so there - /// is no SMTP config to check -- `SmtpConfig::is_incomplete` would - /// wrongly reject every proxy send (its SMTP fields are deliberately - /// unset). The proxy validates its own configuration separately; this - /// path validates the message itself: recipients, subject, body. - pub fn build_without_config( - to: &str, - subject: &str, - body: &str, - ) -> Result<(Self, RecipientList), SendError> { let list = parse_recipients(to); if list.accepted.is_empty() { @@ -530,32 +516,6 @@ mod tests { ); } - /// C5: the proxy send path has no SMTP config, so it must be able to - /// validate the message without one. `build_without_config` is that - /// seam, and it must still reject everything a bad message has. - #[test] - fn build_without_config_validates_the_message_but_not_the_config() { - let (req, list) = - EmailSendRequest::build_without_config("a@x.com, b@y.com", "Hi", "Body").unwrap(); - assert_eq!(req.recipient_count(), 2); - assert!(list.rejected.is_empty()); - - // The same message-level failures still hold. - assert_eq!( - EmailSendRequest::build_without_config("", "s", "b").unwrap_err(), - SendError::NoRecipients - ); - assert_eq!( - EmailSendRequest::build_without_config("a@x.com", "s", " ").unwrap_err(), - SendError::EmptyBody - ); - assert!(matches!( - EmailSendRequest::build_without_config("a@x.com", "s", &"a".repeat(MAX_BODY_BYTES + 1)) - .unwrap_err(), - SendError::BodyTooLarge { .. } - )); - } - #[test] fn no_recipients_and_all_invalid_are_different_errors() { assert_eq!( diff --git a/crates/nigig-core/src/email_session.rs b/crates/nigig-core/src/email_session.rs deleted file mode 100644 index cb81622..0000000 --- a/crates/nigig-core/src/email_session.rs +++ /dev/null @@ -1,140 +0,0 @@ -//! The signed-in email account, shared across pages. -//! -//! The inbox page owns the sign-in flow and its `SessionState`, but the -//! Compose page also needs to send, and the More page needs to show and -//! clear the account. Rather than hoisting the whole session into the app -//! shell (a large refactor for one shared value), the session lives here: -//! a process-wide holder that the inbox writes on sign-in and clears on -//! sign-out. -//! -//! ## What it holds, and why the secret is here at all -//! -//! `EmailAccount` is the persistable half (no secret -- see -//! `email_account`), and the `Secret` is the session-only password. Both -//! are needed by any page that acts on the account. The secret is held -//! exactly as long as the session, is dropped on sign-out (see -//! `clear_session`), and never survives the process. This is the same -//! session-only contract the inbox already used; it is just shared now. -//! -//! A platform keystore (C1f) is the prerequisite for the password to -//! survive a restart; until then a restart means re-entering it, which is -//! the honest behaviour, not a bug. - -use crate::email_account::EmailAccount; -use crate::secret::Secret; -use std::sync::{Mutex, OnceLock}; - -/// The signed-in account and its session secret. -#[derive(Clone)] -pub struct Session { - pub account: EmailAccount, - pub secret: Secret, -} - -// A Debug impl that cannot leak the secret, for parity with Secret itself. -impl std::fmt::Debug for Session { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("Session") - .field("account", &self.account) - .field("secret", &self.secret) // Secret's Debug renders "***" - .finish() - } -} - -static SESSION: OnceLock>> = OnceLock::new(); - -fn cell() -> &'static Mutex> { - SESSION.get_or_init(|| Mutex::new(None)) -} - -/// Record the signed-in account. Replaces any previous session. -pub fn set_session(account: EmailAccount, secret: Secret) { - *cell().lock().expect("session lock poisoned") = Some(Session { account, secret }); -} - -/// The current session, if any, cloned out. -pub fn current_session() -> Option { - cell().lock().expect("session lock poisoned").clone() -} - -/// Whether an account is signed in. -pub fn is_signed_in() -> bool { - cell().lock().expect("session lock poisoned").is_some() -} - -/// Forget the session and drop the secret. -pub fn clear_session() { - *cell().lock().expect("session lock poisoned") = None; -} - -/// Emitted when the user signs out from the More page, so the inbox can -/// flip back to the setup form. Posted globally (`Cx::post_action`) because -/// the inbox and the More page are sibling pages under a PageFlip and do -/// not see each other's local actions. -#[derive(Clone, Debug, Default)] -pub enum EmailSessionAction { - #[default] - None, - SignedIn, - SignedOut, -} - -impl makepad_widgets::ActionDefaultRef for EmailSessionAction { - fn default_ref() -> &'static Self { - static DEFAULT: EmailSessionAction = EmailSessionAction::None; - &DEFAULT - } -} - -#[cfg(test)] -mod tests { - use super::*; - - fn account() -> EmailAccount { - EmailAccount { - address: "jane@example.com".into(), - smtp_server: "smtp.example.com".into(), - smtp_port: 587, - username: "jane@example.com".into(), - display_name: "Jane".into(), - backend: crate::mail_backend::BackendSettings::default(), - } - } - - #[test] - fn a_session_can_be_set_and_read_back() { - set_session(account(), Secret::new("hunter2")); - assert!(is_signed_in()); - let s = current_session().unwrap(); - assert_eq!(s.account.address, "jane@example.com"); - assert_eq!(s.secret.expose(), "hunter2"); - clear_session(); - } - - #[test] - fn clearing_drops_the_session_and_the_secret() { - set_session(account(), Secret::new("hunter2")); - clear_session(); - assert!(!is_signed_in()); - assert!(current_session().is_none()); - } - - #[test] - fn a_new_sign_in_replaces_the_old_session() { - set_session(account(), Secret::new("old")); - set_session(account(), Secret::new("new")); - assert_eq!(current_session().unwrap().secret.expose(), "new"); - clear_session(); - } - - /// The session holds a Secret; Debug must not leak it (S2, one more - /// layer out). - #[test] - fn debug_printing_a_session_does_not_leak_the_secret() { - set_session(account(), Secret::new("hunter2")); - let rendered = format!("{:?}", current_session().unwrap()); - assert!(rendered.contains("jane@example.com"), "show non-secrets"); - assert!(!rendered.contains("hunter2"), "leaked: {rendered}"); - clear_session(); - } -} diff --git a/crates/nigig-core/src/email_worker.rs b/crates/nigig-core/src/email_worker.rs index 004746b..ea425a0 100644 --- a/crates/nigig-core/src/email_worker.rs +++ b/crates/nigig-core/src/email_worker.rs @@ -127,122 +127,6 @@ pub fn spawn_proxy_verify(settings: crate::mail_backend::ProxySettings, token: S }); } -/// Spawn an inbox fetch for the signed-in backend (C4b). -/// -/// One entry point for both backends: the proxy talks HTTP, the direct -/// backend talks IMAP (when the `imap` feature is enabled -- a build -/// without it reports an honest error rather than silently returning an -/// empty inbox). Posts `EmailWorkerAction::InboxFetched` either way. -pub fn spawn_fetch_inbox(settings: crate::mail_backend::BackendSettings, secret: Secret) { - crate::platform::spawn(async move { - let result: Result, String> = match settings { - crate::mail_backend::BackendSettings::ProxyApi(s) => { - let client = crate::mail_proxy::ProxyApiClient::new(s, secret); - #[cfg(not(target_arch = "wasm32"))] - let r = client - .list_inbox(&crate::mail_proxy::ReqwestTransport::default()) - .await; - #[cfg(target_arch = "wasm32")] - let r = client - .list_inbox(&crate::mail_proxy::WasmFetchTransport) - .await; - r.map_err(|e| e.message()) - } - crate::mail_backend::BackendSettings::ImapSmtp(s) => { - #[cfg(feature = "imap")] - { - let client = crate::imap_client::ImapClient::new(s, secret); - client - .list_inbox(&crate::imap_client::AsyncImapTransport) - .await - .map_err(|e| e.message()) - } - #[cfg(not(feature = "imap"))] - { - let _ = (s, secret); - Err( - "Reading mail directly needs the IMAP feature, which is not \ - built into this build." - .to_string(), - ) - } - } - }; - Cx::post_action(EmailWorkerAction::InboxFetched(result)); - }); -} - -/// Send a message through the signed-in backend (C5). -/// -/// The compose page (and any future caller) does not know whether the -/// account is SMTP or proxy; this branches on the account's backend. The -/// SMTP arm reuses `spawn_send_email`; the proxy arm validates the message -/// without an SMTP config and sends through the mail service. Both share -/// the in-flight guard and the abandon control (B5/B6). -pub fn spawn_send_message( - account: &crate::email_account::EmailAccount, - secret: Secret, - to: String, - subject: String, - body: String, -) { - use crate::mail_backend::BackendSettings; - match &account.backend { - BackendSettings::ImapSmtp(_) => { - let config = SmtpConfig { - server: account.smtp_server.clone(), - port: account.smtp_port, - username: account.username.clone(), - password: secret, - from: account.address.clone(), - }; - spawn_send_email(config, to, subject, body); - } - BackendSettings::ProxyApi(settings) => { - // No SMTP config to check -- the proxy validates itself. The - // message is still validated (recipients, subject, body). - let req = match crate::email_send::EmailSendRequest::build_without_config( - &to, &subject, &body, - ) { - Err(e) => { - Cx::post_action(EmailWorkerAction::SendResult(Err(e.message()))); - return; - } - Ok((req, _)) => req, - }; - - if SEND_IN_FLIGHT.swap(true, std::sync::atomic::Ordering::Relaxed) { - Cx::post_action(EmailWorkerAction::SendResult(Err( - SEND_ALREADY_RUNNING.to_string() - ))); - return; - } - SEND_ABANDONED.store(false, std::sync::atomic::Ordering::Relaxed); - - let settings = settings.clone(); - crate::platform::spawn(async move { - let client = crate::mail_proxy::ProxyApiClient::new(settings, secret); - #[cfg(not(target_arch = "wasm32"))] - let result = client - .send(&crate::mail_proxy::ReqwestTransport::default(), &req) - .await; - #[cfg(target_arch = "wasm32")] - let result = client - .send(&crate::mail_proxy::WasmFetchTransport, &req) - .await; - - SEND_IN_FLIGHT.store(false, std::sync::atomic::Ordering::Relaxed); - if SEND_ABANDONED.swap(false, std::sync::atomic::Ordering::Relaxed) { - return; - } - Cx::post_action(EmailWorkerAction::SendResult( - result.map_err(|e| e.message()), - )); - }); - } - } -} - /// Shown when a send is attempted without enough settings to try. pub const INCOMPLETE_CONFIG_MESSAGE: &str = "Missing account settings. Check the server, port, username, password and from address."; @@ -412,9 +296,9 @@ use lettre::{ #[cfg(not(target_arch = "wasm32"))] async fn smtp_test_impl(config: &SmtpConfig) -> Result<(), String> { - let mailer = acquire_transport(config)?; + let creds = Credentials::new(config.username.clone(), config.password.expose().to_owned()); + let mailer = build_transport(config, creds)?; mailer - .transport() .test_connection() .await .map(|_| ()) @@ -458,9 +342,9 @@ async fn send_email_impl( let email = builder .body(body.to_owned()) .map_err(|e| format!("Build error: {e}"))?; - let mailer = acquire_transport(config)?; + let creds = Credentials::new(config.username.clone(), config.password.expose().to_owned()); + let mailer = build_transport(config, creds)?; mailer - .transport() .send(email) .await .map_err(|e| format!("Send failed: {e}"))?; @@ -542,144 +426,6 @@ fn build_transport( /// user on a bad connection needs an error, not a two-minute stall. pub const SMTP_TIMEOUT_SECS: u64 = 20; -// --- D4: reuse the SMTP connection across operations -------------------- -// -// The assessment (P3) noted a brand-new transport per operation: every -// send re-did TCP + TLS + AUTH. lettre's `pool` feature is compiled in -// (Cargo.toml) but was never used. This is the fix. -// -// ## Why one slot, not a multi-connection pool -// -// `SEND_IN_FLIGHT` already serialises sends -- at most one send is running -// at a time -- so a pool of N connections would hold N-1 idle sockets for -// nothing. One reused transport per (server, port, username, secret) is -// exactly the right size. -// -// ## What is verified, and what is not -// -// The *keying* decision -- "reuse this transport, or build a new one?" -- -// is pure and tested (`should_reuse`). The connection-level reuse itself -// cannot be exercised without a live relay, so it is a read of lettre's -// contract (a transport reconnects lazily on the next `.send`), not an -// observed handshake. The secret is compared by value (never logged) so a -// re-entered password correctly rebuilds the transport instead of reusing -// one with stale credentials. - -#[cfg(not(target_arch = "wasm32"))] -#[derive(Clone, Debug, PartialEq, Eq)] -struct TransportKey { - server: String, - port: u16, - username: String, -} - -#[cfg(not(target_arch = "wasm32"))] -impl TransportKey { - fn from_config(config: &SmtpConfig) -> Self { - Self { - server: config.server.trim().to_string(), - port: config.port, - username: config.username.trim().to_string(), - } - } -} - -/// Should an existing pool entry be reused for this config + secret? -/// -/// Pure so the policy is testable: reuse only when the server, port, -/// username AND password all match; anything else rebuilds. The password -/// check is what stops a re-entered credential from silently reusing a -/// transport that still holds the old one. -#[cfg(not(target_arch = "wasm32"))] -fn should_reuse( - existing: Option<(&TransportKey, &Secret)>, - key: &TransportKey, - secret: &Secret, -) -> bool { - existing.is_some_and(|(k, s)| k == key && s == secret) -} - -#[cfg(not(target_arch = "wasm32"))] -struct PoolEntry { - key: TransportKey, - secret: Secret, - transport: AsyncSmtpTransport, -} - -#[cfg(not(target_arch = "wasm32"))] -static TRANSPORT_POOL: std::sync::OnceLock>> = - std::sync::OnceLock::new(); - -/// A transport checked out of the pool; returned on drop. -/// -/// The entry is *moved out* of the pool while held, so no `MutexGuard` is -/// held across the `.await` on the send -- a `std::sync::MutexGuard` is -/// `!Send` and would make the spawned future `!Send`. Owning the entry -/// directly keeps the future `Send`. -#[cfg(not(target_arch = "wasm32"))] -pub struct AcquiredTransport { - entry: Option, - pool: &'static std::sync::Mutex>, -} - -#[cfg(not(target_arch = "wasm32"))] -impl AcquiredTransport { - pub fn transport(&self) -> &AsyncSmtpTransport { - &self - .entry - .as_ref() - .expect("held transport is present") - .transport - } -} - -#[cfg(not(target_arch = "wasm32"))] -impl Drop for AcquiredTransport { - fn drop(&mut self) { - if let Some(entry) = self.entry.take() { - if let Ok(mut guard) = self.pool.lock() { - *guard = Some(entry); - } - } - } -} - -/// Get a transport for `config`, reusing a matching pooled one. -#[cfg(not(target_arch = "wasm32"))] -fn acquire_transport(config: &SmtpConfig) -> Result { - let pool: &'static std::sync::Mutex> = - TRANSPORT_POOL.get_or_init(|| std::sync::Mutex::new(None)); - let key = TransportKey::from_config(config); - let secret = config.password.clone(); - - // Take the existing entry so we own it outside the lock. - let existing = pool - .lock() - .map_err(|_| "transport pool poisoned".to_string())? - .take(); - - let entry = if should_reuse( - existing.as_ref().map(|e| (&e.key, &e.secret)), - &key, - &secret, - ) { - existing.expect("should_reuse true implies an entry was present") - } else { - let creds = Credentials::new(config.username.clone(), config.password.expose().to_owned()); - let transport = build_transport(config, creds)?; - PoolEntry { - key, - secret, - transport, - } - }; - - Ok(AcquiredTransport { - entry: Some(entry), - pool, - }) -} - // --- Wasm implementation: HTTP API proxy via fetch --- #[cfg(target_arch = "wasm32")] @@ -788,12 +534,6 @@ async fn call_email_api(json_body: &str) -> Result<(), String> { // --- Shared types --- -// D5: the `None` variant is gone. It existed only to satisfy -// `ActionDefaultRef`, but this action is consumed purely via -// `downcast_ref()` (which needs only `ActionTrait` = `'static + Debug`) -- -// no caller uses `.cast()`/`cast_ref()`, so no default is required. A -// `None` variant that is never constructed only widened matches for no -// behaviour. #[derive(Clone, Debug)] pub enum EmailWorkerAction { SmtpTestResult(Result<(), String>), @@ -802,10 +542,20 @@ pub enum EmailWorkerAction { /// the UI's "checking connection" flow is transport-agnostic. ProxyVerifyResult(Result<(), String>), SendResult(Result<(), String>), - /// Result of an inbox fetch (C4b). Carries the messages on success, or - /// the reason on failure, so the inbox can render loading/error/empty - /// states without knowing which backend produced them. - InboxFetched(Result, String>), + None, +} + +impl Default for EmailWorkerAction { + fn default() -> Self { + Self::None + } +} + +impl ActionDefaultRef for EmailWorkerAction { + fn default_ref() -> &'static Self { + static NONE: EmailWorkerAction = EmailWorkerAction::None; + &NONE + } } #[cfg(test)] @@ -901,50 +651,6 @@ mod tests { }); } - // ---- D4: the transport pool keying --------------------------------- - - #[cfg(not(target_arch = "wasm32"))] - #[test] - fn the_pool_key_is_server_port_and_username() { - let key = TransportKey::from_config(&good()); - assert_eq!(key.server, "smtp.example.com"); - assert_eq!(key.port, SmtpConfig::DEFAULT_PORT); - assert_eq!(key.username, "jane@example.com"); - // Whitespace is normalised, so two configs differing only by - // stray spaces share a key. - let padded = SmtpConfig { - server: " smtp.example.com ".into(), - username: " jane@example.com ".into(), - ..good() - }; - assert_eq!(TransportKey::from_config(&padded), key); - } - - #[cfg(not(target_arch = "wasm32"))] - #[test] - fn reuse_requires_matching_key_and_secret() { - let key = TransportKey::from_config(&good()); - let secret = Secret::new("hunter2"); - - // Nothing pooled -> build. - assert!(!should_reuse(None, &key, &secret)); - // Same key + secret -> reuse. - assert!(should_reuse(Some((&key, &secret)), &key, &secret)); - - // A different server (or port, or username) -> rebuild. - let other = TransportKey { - server: "smtp.other.com".into(), - ..key.clone() - }; - assert!(!should_reuse(Some((&key, &secret)), &other, &secret)); - - // The same key but a re-entered password -> rebuild, so a stale - // credential is never silently reused (the key alone would say - // "reuse", which is exactly the bug the secret check prevents). - let changed = Secret::new("hunter3"); - assert!(!should_reuse(Some((&key, &secret)), &key, &changed)); - } - // ---- A4: local validation before spending a round trip ------------ #[test] diff --git a/crates/nigig-core/src/imap_client.rs b/crates/nigig-core/src/imap_client.rs deleted file mode 100644 index 5c71687..0000000 --- a/crates/nigig-core/src/imap_client.rs +++ /dev/null @@ -1,535 +0,0 @@ -//! The direct-backend IMAP client (Phase C1e). -//! -//! C1 decided to support both backends. The proxy (C1d) is done; this is -//! the other half: reading mail straight from the provider over IMAP. -//! -//! ## The seam, and why it mirrors the proxy -//! -//! IMAP is a stateful raw-TCP protocol: connect, TLS, `LOGIN`, `SELECT -//! INBOX`, `FETCH`. None of that can run in a browser (so this whole -//! module is native-only) and none of it can be host-tested against a -//! real server. So, exactly like `mail_proxy`, the bugs are pushed to the -//! *edges* -- the neutral `FetchedMessage` shape, the date parser, the -//! error mapping -- which ARE pure and tested, while the socket lives -//! behind an `ImapTransport` trait. -//! -//! The real transport (`AsyncImapTransport`) uses `async-imap` and is -//! gated behind the `imap` feature so wasm builds (and builds that do not -//! need IMAP) never pull in its native-TLS dependency tree. See Cargo.toml. -//! -//! ## What is NOT verified -//! -//! No IMAP handshake has been executed here. The trait wiring is proven -//! against a mock; the parser and mapping against fixtures; the socket is -//! a read of async-imap's API. This is stated plainly because the whole -//! point of the remediation was to stop confusing "compiles" with "works". - -use crate::email_store::EmailMessage; -use crate::mail_backend::ImapSmtpSettings; -use crate::secret::Secret; - -/// Standard IMAP-over-TLS port (re-exported for the transport). -pub use crate::mail_backend::{IMAPS_PORT, IMAP_STARTTLS_PORT}; - -/// One message as fetched, before mapping to the domain type. -/// -/// This is the neutral shape both the real transport (async-imap) and the -/// mock produce, so the mapping to `EmailMessage` is exercised without a -/// socket and without async-imap. -#[derive(Clone, Debug, PartialEq, Eq)] -pub struct FetchedMessage { - pub uid: String, - pub from_address: String, - pub from_name: String, - pub subject: String, - pub body: String, - pub date_ms: i64, - /// From the `\Seen` flag: has the user already read this message? - pub is_read: bool, -} - -/// Map a fetched message onto the domain type. -/// -/// Pure, so the envelope→`EmailMessage` shape is pinned by tests. The id -/// is the provider's UID, which is stable across fetches and so is the -/// natural de-duplication key for the cache (C3). -pub fn to_email_message(f: FetchedMessage) -> EmailMessage { - EmailMessage { - id: f.uid, - from_address: f.from_address, - from_name: f.from_name, - subject: f.subject, - body: f.body, - date_ms: f.date_ms, - is_read: f.is_read, - is_outgoing: false, - } -} - -/// Parse an IMAP `INTERNALDATE` into epoch milliseconds. -/// -/// The format is `dd-MMM-yyyy HH:mm:ss ±HHMM` with an English month -/// abbreviation (`17-Aug-2026 09:00:00 +0000`). Providers have been known -/// to omit the zone or emit `-0000`, and a wrong zone silently shifts -/// every row's timestamp; returning `None` (which the caller renders as an -/// empty timestamp) is better than a confident wrong one. -pub fn parse_imap_date(s: &str) -> Option { - let t = s.trim(); - // chrono accepts the RFC-ish form including the zone. Try with zone - // first; fall back to parsing the date/time portion as UTC when the - // provider omitted the zone. - chrono::DateTime::parse_from_str(t, "%d-%b-%Y %H:%M:%S %z") - .map(|dt| dt.timestamp_millis()) - .ok() - .or_else(|| { - chrono::NaiveDateTime::parse_from_str(t, "%d-%b-%Y %H:%M:%S") - .ok() - .and_then(|naive| naive.and_utc().timestamp_millis().into()) - .map(|ms| { - chrono::DateTime::from_timestamp_millis(ms) - .map(|dt| dt.timestamp_millis()) - .unwrap_or(ms) - }) - }) -} - -/// Why an IMAP operation failed, one variant per thing the user can act on. -#[derive(Clone, Debug, PartialEq, Eq)] -pub enum ImapError { - /// The transport could not complete the exchange (DNS, TLS, timeout). - Transport(String), - /// `LOGIN` was refused: the username or password is wrong. - AuthFailed, - /// The server has no INBOX or refused to select it. - NoInbox, - /// The transport succeeded but produced nothing we could map. - Malformed(String), -} - -impl ImapError { - pub fn message(&self) -> String { - match self { - ImapError::Transport(e) => format!("Could not reach the mail server: {e}"), - ImapError::AuthFailed => "Login failed. Check your username and password.".into(), - ImapError::NoInbox => "The server did not expose an inbox.".into(), - ImapError::Malformed(_) => "The server sent a reply we could not understand.".into(), - } - } -} - -/// The thing that actually speaks IMAP. The only place a socket opens. -/// -/// The real implementation is `AsyncImapTransport` (feature `imap`); tests -/// inject a mock. This is what makes `ImapClient` host-testable. -#[allow(async_fn_in_trait)] -pub trait ImapTransport { - async fn verify(&self, settings: &ImapSmtpSettings, password: &Secret) -> Result<(), String>; - async fn fetch_inbox( - &self, - settings: &ImapSmtpSettings, - password: &Secret, - ) -> Result, String>; -} - -/// A configured direct-backend client. -#[derive(Clone, Debug)] -pub struct ImapClient { - pub settings: ImapSmtpSettings, - pub password: Secret, -} - -impl ImapClient { - pub fn new(settings: ImapSmtpSettings, password: Secret) -> Self { - Self { settings, password } - } - - /// Prove the credentials work before saving the account. - pub async fn verify(&self, t: &T) -> Result<(), ImapError> { - t.verify(&self.settings, &self.password) - .await - .map_err(ImapError::Transport) - } - - /// Fetch the inbox and map onto the domain type. - pub async fn list_inbox( - &self, - t: &T, - ) -> Result, ImapError> { - let fetched = t - .fetch_inbox(&self.settings, &self.password) - .await - .map_err(ImapError::Transport)?; - Ok(fetched.into_iter().map(to_email_message).collect()) - } -} - -// --------------------------------------------------------------------- -// Native transport: async-imap over native TLS. Feature-gated so wasm -// builds (which cannot open a raw TCP socket) never compile it. -// --------------------------------------------------------------------- - -#[cfg(feature = "imap")] -mod native { - use super::*; - use async_imap::error::Result as ImapResult; - use async_native_tls::TlsConnector; - use futures::StreamExt; - - type Stream = async_native_tls::TlsStream; - - /// The real transport. Constructing it does no I/O; each call opens a - /// fresh connection and closes it on return (see the module note: no - /// pooling here yet -- that is D4, and it is the same work the SMTP - /// path needs). - #[derive(Clone, Copy, Debug, Default)] - pub struct AsyncImapTransport; - - async fn connect(settings: &ImapSmtpSettings) -> Result, String> { - let host = settings.imap_server.clone(); - let port = if settings.imap_port == 0 { - IMAPS_PORT - } else { - settings.imap_port - }; - let tcp = async_net::TcpStream::connect((host.as_str(), port)) - .await - .map_err(|e| format!("connect failed: {e}"))?; - let tls = TlsConnector::new(); - let stream = tls - .connect(&host, tcp) - .await - .map_err(|e| format!("TLS failed: {e}"))?; - Ok(async_imap::Client::new(stream)) - } - - async fn login( - settings: &ImapSmtpSettings, - password: &Secret, - ) -> Result, String> { - let client = connect(settings).await?; - client - .login(&settings.username, password.expose()) - .await - .map_err(|(e, _)| e.to_string()) - } - - impl ImapTransport for AsyncImapTransport { - async fn verify( - &self, - settings: &ImapSmtpSettings, - password: &Secret, - ) -> Result<(), String> { - let mut session = login(settings, password).await?; - session.select("INBOX").await.map_err(|e| e.to_string())?; - session.logout().await.map_err(|e| e.to_string())?; - Ok(()) - } - - async fn fetch_inbox( - &self, - settings: &ImapSmtpSettings, - password: &Secret, - ) -> Result, String> { - let mut session = login(settings, password).await?; - session.select("INBOX").await.map_err(|e| e.to_string())?; - - // The fetch stream borrows `session` mutably, so it must be - // dropped before `logout` below. A block scope does that. - let messages = { - let fetches = session - .fetch("1:*", "(UID ENVELOPE BODY.PEEK[TEXT] INTERNALDATE FLAGS)") - .await - .map_err(|e| e.to_string())?; - - // An explicit poll loop rather than `.collect()`: per-message - // parse errors are dropped, not fatal, and the loop keeps the - // error-handling intent visible. - futures::pin_mut!(fetches); - let mut messages = Vec::new(); - while let Some(res) = fetches.next().await { - if let Ok(f) = res { - if let Ok(Some(m)) = fetch_to_message(&f) { - messages.push(m); - } - } - } - messages - }; - - session.logout().await.map_err(|e| e.to_string())?; - Ok(messages) - } - } - - fn fetch_to_message(f: &async_imap::types::Fetch) -> ImapResult> { - let envelope = match f.envelope() { - Some(e) => e, - None => return Ok(None), // a fetch without an envelope is a no-op - }; - - // async-imap's envelope/address fields are raw bytes (Cow<[u8]>), - // so decode lossily rather than failing on a non-UTF8 header. - fn lossy(b: Option<&std::borrow::Cow<'_, [u8]>>) -> String { - b.map(|c| String::from_utf8_lossy(c).into_owned()) - .unwrap_or_default() - } - - // The first `from` address; empty `from` (a bounce) still needs a - // stable, non-empty grouping key. - let (from_address, from_name) = match envelope.from.as_ref().and_then(|v| v.first()) { - Some(addr) => { - let mailbox = lossy(addr.mailbox.as_ref()); - let host = lossy(addr.host.as_ref()); - let address = if mailbox.is_empty() { - String::new() - } else { - format!("{mailbox}@{host}") - }; - (address, lossy(addr.name.as_ref())) - } - None => (String::new(), String::new()), - }; - - let subject = envelope - .subject - .as_ref() - .map(|s| String::from_utf8_lossy(s).trim().to_string()) - .unwrap_or_default(); - - // The literal text body; the first non-empty text section is the - // pragmatic choice (a multipart has several). Empty is fine -- the - // preview renders "(no subject)"-style fallbacks. - let body = f - .text() - .map(|b| String::from_utf8_lossy(b).into_owned()) - .unwrap_or_default(); - - // `internal_date()` already parses into a chrono DateTime. - let date_ms = f.internal_date().map(|d| d.timestamp_millis()).unwrap_or(0); - - let is_read = f.flags().any(|f| f == async_imap::types::Flag::Seen); - - let uid = f - .uid - .map(|u| u.to_string()) - .unwrap_or_else(|| f.message.to_string()); - - Ok(Some(FetchedMessage { - uid, - from_address, - from_name, - subject, - body, - date_ms, - is_read, - })) - } -} - -#[cfg(feature = "imap")] -pub use native::AsyncImapTransport; - -#[cfg(test)] -mod tests { - use super::*; - - fn settings() -> ImapSmtpSettings { - ImapSmtpSettings { - imap_server: "imap.example.com".into(), - imap_port: IMAPS_PORT, - smtp_server: "smtp.example.com".into(), - smtp_port: 587, - username: "jane@example.com".into(), - } - } - - fn client() -> ImapClient { - ImapClient::new(settings(), Secret::new("hunter2")) - } - - /// A transport that returns canned messages and records what it saw, - /// so the client glue -- and error propagation -- is exercised without - /// a socket or async-imap. - struct MockTransport { - result: Result, String>, - saw_user: std::cell::RefCell, - saw_pass: std::cell::RefCell, - } - - impl MockTransport { - fn ok(msgs: Vec) -> Self { - Self { - result: Ok(msgs), - saw_user: Default::default(), - saw_pass: Default::default(), - } - } - fn err(e: &str) -> Self { - Self { - result: Err(e.to_string()), - saw_user: Default::default(), - saw_pass: Default::default(), - } - } - } - - impl ImapTransport for MockTransport { - async fn verify( - &self, - settings: &ImapSmtpSettings, - password: &Secret, - ) -> Result<(), String> { - *self.saw_user.borrow_mut() = settings.username.clone(); - *self.saw_pass.borrow_mut() = password.expose().to_string(); - self.result.as_ref().map(|_| ()).map_err(|e| e.clone()) - } - - async fn fetch_inbox( - &self, - settings: &ImapSmtpSettings, - password: &Secret, - ) -> Result, String> { - *self.saw_user.borrow_mut() = settings.username.clone(); - *self.saw_pass.borrow_mut() = password.expose().to_string(); - self.result.clone() - } - } - - fn block_on(fut: F) -> F::Output { - use std::sync::Arc; - use std::task::{Context, Poll, Wake, Waker}; - struct Noop; - impl Wake for Noop { - fn wake(self: Arc) {} - } - let waker = Waker::from(Arc::new(Noop)); - let mut cx = Context::from_waker(&waker); - let mut fut = Box::pin(fut); - loop { - match fut.as_mut().poll(&mut cx) { - Poll::Ready(v) => return v, - Poll::Pending => std::thread::yield_now(), - } - } - } - - // ---- date parsing -------------------------------------------------- - - #[test] - fn parses_a_full_internal_date_with_zone() { - // 17-Aug-2026 00:00:00 +0000 == 2026-08-17T00:00:00Z. - let ms = parse_imap_date("17-Aug-2026 00:00:00 +0000").unwrap(); - let dt = chrono::DateTime::from_timestamp_millis(ms).unwrap(); - assert_eq!( - dt.format("%Y-%m-%d %H:%M:%S").to_string(), - "2026-08-17 00:00:00" - ); - } - - #[test] - fn honours_a_nonzero_zone() { - // 09:00 +0300 is 06:00 UTC. - let ms = parse_imap_date("17-Aug-2026 09:00:00 +0300").unwrap(); - let dt = chrono::DateTime::from_timestamp_millis(ms).unwrap(); - assert_eq!(dt.format("%H:%M").to_string(), "06:00"); - } - - #[test] - fn a_missing_zone_is_treated_as_utc_not_an_error() { - let ms = parse_imap_date("17-Aug-2026 05:00:00").unwrap(); - let dt = chrono::DateTime::from_timestamp_millis(ms).unwrap(); - assert_eq!(dt.format("%H").to_string(), "05"); - } - - #[test] - fn garbage_returns_none_instead_of_panicking() { - for bad in [ - "", - "not a date", - "99-Xxx-2026 00:00:00 +0000", - "17-Aug-2026", - ] { - assert_eq!(parse_imap_date(bad), None, "should reject {bad:?}"); - } - } - - // ---- mapping ------------------------------------------------------- - - #[test] - fn a_fetched_message_maps_onto_the_domain_type() { - let m = to_email_message(FetchedMessage { - uid: "42".into(), - from_address: "alerts@bank.co.ke".into(), - from_name: "Equity Alerts".into(), - subject: "Statement".into(), - body: "Your statement is ready.".into(), - date_ms: 123, - is_read: false, - }); - assert_eq!(m.id, "42"); - assert_eq!(m.from_address, "alerts@bank.co.ke"); - assert_eq!(m.is_outgoing, false, "fetched mail is never outgoing"); - } - - // ---- client glue, through the mock --------------------------------- - - #[test] - fn list_inbox_delegates_and_maps() { - let t = MockTransport::ok(vec![FetchedMessage { - uid: "1".into(), - from_address: "a@b.com".into(), - from_name: String::new(), - subject: "Hi".into(), - body: "Body".into(), - date_ms: 5, - is_read: true, - }]); - let msgs = block_on(client().list_inbox(&t)).unwrap(); - assert_eq!(msgs.len(), 1); - assert_eq!(msgs[0].id, "1"); - assert_eq!(*t.saw_user.borrow(), "jane@example.com"); - assert_eq!(*t.saw_pass.borrow(), "hunter2"); - } - - #[test] - fn verify_delegates_and_passes_the_credentials() { - let t = MockTransport::ok(vec![]); - block_on(client().verify(&t)).unwrap(); - assert_eq!(*t.saw_user.borrow(), "jane@example.com"); - assert_eq!(*t.saw_pass.borrow(), "hunter2"); - } - - #[test] - fn a_transport_failure_surfaces_as_a_transport_error() { - let t = MockTransport::err("connection refused"); - let err = block_on(client().list_inbox(&t)).unwrap_err(); - assert_eq!(err, ImapError::Transport("connection refused".into())); - assert!(err.message().contains("connection refused")); - } - - #[test] - fn every_imap_error_has_a_distinct_actionable_message() { - let all = [ - ImapError::Transport("dns".into()), - ImapError::AuthFailed, - ImapError::NoInbox, - ImapError::Malformed("x".into()), - ]; - let mut seen = std::collections::HashSet::new(); - for e in &all { - let m = e.message(); - assert!(m.len() > 10, "{e:?} message too terse: {m}"); - assert!(seen.insert(m.clone()), "duplicate message for {e:?}"); - } - assert!(ImapError::AuthFailed.message().contains("password")); - } - - /// The client holds a Secret, so Debug-printing it must not leak the - /// password -- the S2 property, one more layer out. - #[test] - fn debug_printing_the_client_does_not_leak_the_password() { - let rendered = format!("{:?}", client()); - assert!(rendered.contains("imap.example.com"), "show non-secrets"); - assert!(!rendered.contains("hunter2"), "leaked: {rendered}"); - } -} diff --git a/crates/nigig-core/src/lib.rs b/crates/nigig-core/src/lib.rs index 4006762..234c2ef 100644 --- a/crates/nigig-core/src/lib.rs +++ b/crates/nigig-core/src/lib.rs @@ -25,11 +25,6 @@ pub mod email_store; pub mod mail_backend; pub mod mail_proxy; pub mod email_worker; -pub mod email_pacing; -pub mod credential_store; -pub mod email_cache; -pub mod imap_client; -pub mod email_session; pub use dir::app_data_dir; pub use persistence::*; diff --git a/tools/test-email-coverage.sh b/tools/test-email-coverage.sh index 76d5538..0d6933f 100644 --- a/tools/test-email-coverage.sh +++ b/tools/test-email-coverage.sh @@ -27,12 +27,9 @@ IFS=$'\n\t' ROOT="$(CDPATH= cd -- "$(dirname -- "${BASH_SOURCE[0]}")/.." && pwd)" TOOLCHAIN="${RUST_TOOLCHAIN:-1.97.1}" -# The whole-domain floor, set a little under today's measurement (~90.7%) -# so ordinary refactoring does not trip it while a real loss of coverage -# does. The remaining uncovered lines are the actual network I/O (SMTP -# socket, reqwest/fetch, the feature-gated IMAP socket) and platform file -# I/O, which the plan records as "not host-verified" rather than testable. -TOTAL_FLOOR="${EMAIL_COVERAGE_TOTAL_FLOOR:-88}" +# The whole-domain floor, set a little under today's measurement (~93%) so +# ordinary refactoring does not trip it while a real loss of coverage does. +TOTAL_FLOOR="${EMAIL_COVERAGE_TOTAL_FLOOR:-90}" # Per-file floors for the files that have actually harboured the bugs the # remediation fixed. A single whole-domain number hides exactly the failure @@ -47,16 +44,11 @@ TOTAL_FLOOR="${EMAIL_COVERAGE_TOTAL_FLOOR:-88}" PER_FILE_FLOORS="${EMAIL_COVERAGE_PER_FILE_FLOORS:-\ nigig-core/src/mail_proxy.rs:85 nigig-core/src/mail_backend.rs:92 -nigig-core/src/email_worker.rs:55 +nigig-core/src/email_worker.rs:65 nigig-core/src/email_send.rs:95 nigig-core/src/email_store.rs:95 nigig-core/src/email_account.rs:95 -nigig-core/src/secret.rs:95 -nigig-core/src/email_pacing.rs:90 -nigig-core/src/credential_store.rs:90 -nigig-core/src/email_cache.rs:80 -nigig-core/src/email_session.rs:90 -nigig-core/src/imap_client.rs:85}" +nigig-core/src/secret.rs:95}" WORK="$(mktemp -d "${TMPDIR:-/tmp}/email-coverage.XXXXXXXX")" cleanup() { @@ -97,8 +89,8 @@ chmod 700 "$WORK/rustup-init" printf 'running the email domain tests under instrumentation\n' cd "$ROOT" cargo test --locked -p nigig-core --lib -- \ - email_ secret:: mail_backend:: mail_proxy:: imap_client:: credential_store:: \ - >"$WORK/test.log" 2>&1 || { cat "$WORK/test.log"; exit 1; } + email_ secret:: mail_backend:: mail_proxy:: >"$WORK/test.log" 2>&1 \ + || { cat "$WORK/test.log"; exit 1; } grep -E 'test result' "$WORK/test.log" | tail -5 HOST="$(rustc -vV | sed -n 's/^host: //p')" @@ -117,34 +109,25 @@ if [ -z "$BIN" ]; then exit 1 fi -# Only the email files count. Makepad's generated code, other nigig-core -# modules and the registry are all excluded. +# Only the seven email files count. Makepad's generated code, other +# nigig-core modules and the registry are all excluded. IGNORE='(/cargo/registry|/cargo/git|/rustc/)' -EMAIL_FILES=( - "$ROOT/crates/nigig-core/src/email_account.rs" - "$ROOT/crates/nigig-core/src/email_send.rs" - "$ROOT/crates/nigig-core/src/email_store.rs" - "$ROOT/crates/nigig-core/src/email_worker.rs" - "$ROOT/crates/nigig-core/src/mail_backend.rs" - "$ROOT/crates/nigig-core/src/mail_proxy.rs" - "$ROOT/crates/nigig-core/src/secret.rs" - "$ROOT/crates/nigig-core/src/email_pacing.rs" - "$ROOT/crates/nigig-core/src/credential_store.rs" - "$ROOT/crates/nigig-core/src/email_cache.rs" - "$ROOT/crates/nigig-core/src/email_session.rs" - "$ROOT/crates/nigig-core/src/imap_client.rs" -) - printf '\n=== per-file coverage ===\n' "$LLVM_BIN/llvm-cov" report "$BIN" \ -instr-profile="$WORK/coverage.profdata" \ -ignore-filename-regex="$IGNORE" \ - "${EMAIL_FILES[@]}" | tee "$WORK/report.txt" + "$ROOT/crates/nigig-core/src/email_account.rs" \ + "$ROOT/crates/nigig-core/src/email_send.rs" \ + "$ROOT/crates/nigig-core/src/email_store.rs" \ + "$ROOT/crates/nigig-core/src/email_worker.rs" \ + "$ROOT/crates/nigig-core/src/mail_backend.rs" \ + "$ROOT/crates/nigig-core/src/mail_proxy.rs" \ + "$ROOT/crates/nigig-core/src/secret.rs" | tee "$WORK/report.txt" -# Machine-readable totals, filtered to the email files. The export covers -# every file in the binary, so the floor is computed over exactly the files -# this script reported on, matched by path suffix. +# Machine-readable totals, filtered to the seven email files. The export +# covers every file in the binary, so the floor is computed over exactly +# the files this script reported on, matched by path suffix. "$LLVM_BIN/llvm-cov" export "$BIN" \ -instr-profile="$WORK/coverage.profdata" \ -ignore-filename-regex="$IGNORE" > "$WORK/coverage.json" @@ -162,9 +145,7 @@ with open(path) as fh: data = json.load(fh) keep = ("email_account.rs", "email_send.rs", "email_store.rs", - "email_worker.rs", "mail_backend.rs", "mail_proxy.rs", "secret.rs", - "email_pacing.rs", "credential_store.rs", "email_cache.rs", - "email_session.rs", "imap_client.rs") + "email_worker.rs", "mail_backend.rs", "mail_proxy.rs", "secret.rs") files = [f for f in data["data"][0]["files"] if f["filename"].endswith(keep)]