Move custom email verification to app-level (1/3)#2679
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds DB persistence and model for sender verification codes, server endpoints (send/verify) with rate limiting and feature-flag gating, integrates verification into magic-code send logic, and provides a client hook and UI to send/resend and verify 6‑digit OTPs. ChangesCustom Sender Email Verification with OTP
Sequence Diagram(s)sequenceDiagram
participant Client
participant SendHandler as sender-verification-send-magic-code
participant RateLimit as rate_limit.try-custom-sender
participant CodeModel as app_email_verification_code.put!
participant DB as app_email_verification_codes
participant VerifyHandler as sender-verification-verify-magic-code
Client->>SendHandler: POST /dash/apps/:app_id/sender-verification/send (email)
SendHandler->>RateLimit: try-custom-sender(email)
SendHandler->>CodeModel: put!(code, app_id, verification_id)
CodeModel->>DB: INSERT code row
SendHandler->>Client: send formatted email (format-email)
Client->>VerifyHandler: POST /dash/apps/:app_id/sender-verification/verify (code)
VerifyHandler->>CodeModel: consume!(code, verification_id, expiry)
CodeModel->>DB: DELETE returned row if within expiry
VerifyHandler->>DB: mark verification verified (transaction)
VerifyHandler->>Client: {:verified true} or validation error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
253b606 to
19eff43
Compare
19eff43 to
68c7afb
Compare
|
View Vercel preview at instant-www-js-drewh-validate-senders-app-level-jsv.vercel.app. |
68c7afb to
8e8d726
Compare
f48d7f6 to
115db8a
Compare
stopachka
left a comment
There was a problem hiding this comment.
Overall LGTM!
One higher level issue I had was with the naming.
Frontend
- The world
verificationis stacked with the wordverify.verifySenderVerificationCode
- The other names are long and slightly different, which makes it hard to understand the data model
- For example, consider
getSenderVerificationandsendSenderVerificationCode- I guess
SenderVerificationis the object, whileSenderVerificationCodeis just the code linked to the object. But this can be hard to parse out
- I guess
Suggestion
Maybe we cross out the word verification from the codes:
getSenderVerificationstays- The rest are
sendSenderMagicCodeandverifySenderMagicCode
HTTP
- The HTTP endpoint has the same issue of overloading
verifywithverification.
Suggestion
Maybe we follow the same idea:
GET /dash/apps/:app_id/sender-verification
POST /dash/apps/:app_id/sender-verification/send-magic-code
POST /dash/apps/:app_id/sender-verification/verify-magic-code
This drops some of the pretense on REST, which imo is fine for the sake of clarity.
You can then change the handler names too, to fns like sender-verification-send-magic-code etc
The response structure is also confusing:
sender-verificationis a boolean, when the expectation would be an object
Suggestion
Maybe we return a response like:
:sender { email }
:postmark { id, ... }
:instant { 'verified?' }
8607959 to
ad0aafd
Compare
ad0aafd to
a98e807
Compare
| verification | ||
| .sendCode() | ||
| .then(() => { | ||
| successToast(`Verification code sent to ${address}`); |
There was a problem hiding this comment.
I never see this success toast when I click the (re)send code button.
dwwoelfel
left a comment
There was a problem hiding this comment.
LGTM, one comment about the success toast not showing up for me.
112b5c1 to
ac7068e
Compare
ac7068e to
d4332ea
Compare
This PR migrates the current user-based system for custom oauth email senders to use app ids instead. This adds another on top of the postmark email to verify sender. We will also send a magic code to the user so that they can link the sender's identity with the app.
This is gated behind the boolean feature flag:
use-app-email-verification?Added Routes
POST /dash/apps/:id/sender-verification/send-magic-code
POST /dash/apps/:id/sender-verification/verify-magic-code
I also updated the frontend to include the ui needed for seeing the status of both verification steps and submitting the magic code / resending. It is backwards compatible with the backend feature flag.
Important! Includes database migrations which must be run before turning on the feature flag.
Deploy Plan
use-app-email-verification?feature flag to trueHow to test:

Turn on the feature flag.
Try adding / changing an oauth email template with a custom sender address on the dashboard.