feat(otp-worker): notify company@meridiona.com on sign-up or email change - #934
Merged
Conversation
…ange On a successful /otp/verify, the Worker now fires a fire-and-forget SES notification to NOTIFY_EMAIL telling the company an install signed up or changed its verified email (old -> new). The client sends its locally-known previous email as an optional, purely-informational previousEmail field; the Worker never treats it as a security input. Rides on ctx.waitUntil exactly like the existing rate-limit alert, so a failed notification send never affects the verify response itself.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The marketing site has sent this exact notification since June via Resend (Meridian Sign-ins <notify@meridiona.com> -> adithya@meridiona.com, subject "New sign-up: <email>"). Routing the desktop app's copy through the same provider keeps web and desktop sign-ups in one inbox with one sender identity instead of splitting them across two providers by accident of which codebase emitted them. This does not reverse the SES-over-Resend decision: that was specifically about OTP code delivery, where Resend's 100/day free tier cannot cover hundreds of user-facing sends. An internal notification to one address is a couple of dozen a day at most. OTP codes and the rate-limit alert stay on SES, and ses.ts goes back to being purely SES. Body and subject now mirror the website's format (plain text, no HTML part), so both sources thread together. The Rust client is unchanged - the previousEmail plumbing is transport-agnostic.
A test sign-up on staging is not a real one. Delivering "New sign-up: <test address>" to the team inbox makes the production notification less trustworthy every time someone exercises staging, so staging now notifies akarsh@ and production remains the only channel reaching adithya@.
Reverts the staging-only notify split and moves the rate-limit alert off akarsh@ as well, so both emails reach the same team inbox on both channels.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/otp/verifysucceeds, the Worker now fires a fire-and-forget SES notification toNOTIFY_EMAIL(company@meridiona.com) saying either "new sign-up:<email>" or "email changed:<old>-><new>".confirm_account_otp) sends its locally-known previous email as an optionalpreviousEmailfield in the verify request — purely informational, never a security input.Why this design
Reuses the existing
/otp/verifyflow rather than adding a new endpoint or putting SES credentials on the client. The Worker already holds the only SES credentials (server-side, least-privilege IAM), and already has this exact fire-and-forget pattern for the rate-limit alert (ctx.waitUntil, never awaited inline, so a failed notification never affects the verify response). Inherits the existing send/verify rate limits, so it can't be used to spam the notify inbox independently of those.resolveAccountEvent(pure, unit-tested inses.test.ts) decides the event kind:This app has no session/re-login concept — every
/otp/verifycall is either a genuine one-time capture or a deliberate "Change email" action, so there's no repeat-noise case to dedup against (unlikeALERT_EMAIL's once-per-day flag).Where to see the full picture
Per-event notifications land in an inbox — not a browsable directory. The existing PostHog integration (
tray/src-tauri/src/analytics/mod.rs,distinct_id= the signed-in email) remains the source of truth for the full list/count of captured emails; this is a complementary real-time heads-up, not a replacement.Changes
infra/otp-worker/src/ses.ts:AccountEventtype,resolveAccountEvent,buildAccountEventBody/Html,sendAccountEventEmail.infra/otp-worker/src/index.ts:handleVerifytakesctx: ExecutionContext, fires the notification on theverifiedoutcome only.infra/otp-worker/wrangler.jsonc/worker-configuration.d.ts: newNOTIFY_EMAILvar (both envs).tray/src-tauri/src/commands/otp.rs:confirm_account_otpreadscommands::account::read_account_email()and includes it aspreviousEmail; newbuild_verify_bodypure helper.infra/otp-worker/README.md: new "Account-event notification" section.Test plan
npx vitest runininfra/otp-worker/— 107/107 passing (8 new)npx tsc --noEmitcleancargo test --workspace— 446/446 passing (2 new)cargo clippy --workspace --all-targets -- -D warningscleanwrangler deploy) — code changes here have no effect until deployedcompany@meridiona.comreceives the sign-up notification; use "Change email" and confirm the changed-email notification