feat(auth): Email OTP verification service#19
Merged
codebestia merged 4 commits intoJun 28, 2026
Conversation
Move OTP generation from client to backend with bcrypt-hashed codes, email delivery via Resend/SMTP, and protected verify-email and resend-otp endpoints. Closes ShadeProtocol#10 Co-authored-by: Cursor <cursoragent@cursor.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughServer-generated merchant email OTP support is added. The PR introduces email provider configuration, Merchant OTP persistence fields, OTP email dispatch, verification and resend flows, protected auth endpoints, and updated registration and integration tests. ChangesMerchant email OTP flow
Sequence Diagram(s)sequenceDiagram
participant MerchantClient
participant auth.routes
participant verifyEmailController
participant verifyEmailOtp
participant prisma
MerchantClient->>auth.routes: POST /verify-email { code }
auth.routes->>verifyEmailController: authenticateMerchant
verifyEmailController->>verifyEmailOtp: verifyEmailOtp(merchant.id, code)
verifyEmailOtp->>prisma: findUnique / update emailVerified, emailOtp, emailOtpExpiresAt
verifyEmailController-->>MerchantClient: 200 sanitized merchant
sequenceDiagram
participant MerchantClient
participant auth.routes
participant resendOtpController
participant resendEmailOtp
participant prisma
participant sendOtp
MerchantClient->>auth.routes: POST /resend-otp
auth.routes->>resendOtpController: authenticateMerchant
resendOtpController->>resendEmailOtp: resendEmailOtp(merchant.id)
resendEmailOtp->>prisma: check OTP state / update emailOtp, emailOtpExpiresAt
resendEmailOtp->>sendOtp: sendOtp(email, code, firstName)
resendOtpController-->>MerchantClient: 200 Verification code sent
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/services/merchant.services.ts (1)
114-131: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winEmail send happens after the registration commit; a delivery failure leaves the merchant in a confusing state.
prisma.merchant.updatecommitsregistered: trueplus the OTP hash, and only then issendOtpawaited. IfsendOtpthrows (Resend/SMTP outage),registerMerchantrejects and the client sees an error, but the record is already persisted as registered. A retry then hits the earliermerchant.registeredguard and returns409 'Profile already set up', so the user never receives a code from this path (recovery only viaresend-otp).Consider not failing registration on a transient mail error (log and let the user resend), or moving delivery into a flow where a failure is surfaced as "registered, but resend the code" rather than a hard 500.
♻️ One option: don't reject registration on mail failure
- await sendOtp(normalizedEmail, code, data.firstName.trim()); - - return sanitizeMerchant(updatedMerchant); + try { + await sendOtp(normalizedEmail, code, data.firstName.trim()); + } catch (err) { + // Registration is already committed; surface via resend-otp instead of failing the request. + console.error('Failed to send OTP email after registration', err); + } + + return sanitizeMerchant(updatedMerchant);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/merchant.services.ts` around lines 114 - 131, The registration flow in registerMerchant persists the merchant as registered before calling sendOtp, so a mail outage leaves the record committed but the request failing. Update the registerMerchant flow around prisma.merchant.update and sendOtp so a transient OTP delivery failure does not reject the whole registration; instead log the sendOtp error and return a response that tells the user the profile is set up and they should resend the code, or otherwise separate persistence from delivery so retries do not hit the merchant.registered guard.
🧹 Nitpick comments (2)
src/config/environment.ts (1)
24-35: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueProvider value is cast, not validated.
process.env.EMAIL_PROVIDERis force-cast toEmailProvider, so a misconfigured value (e.g.resnd) compiles fine and silently falls through to theconsolebranch inemail.service.ts, meaning real OTP emails are never sent while the app appears healthy. Consider validating against the allowed set at startup and failing fast (or logging a warning).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/config/environment.ts` around lines 24 - 35, The EMAIL_PROVIDER setting in environment configuration is being force-cast instead of validated, so invalid values can silently fall back to the console email path. Update the configuration logic in environment.ts for the email.provider field to validate process.env.EMAIL_PROVIDER against the allowed EmailProvider values before exposing it, and either fail fast at startup or emit a clear warning when the value is invalid; use the EmailProvider type and the email.service.ts provider switch as the key places to align behavior.src/services/email.service.ts (1)
5-16: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueUnescaped
firstNameinterpolated into email HTML.
firstNameis injected directly into the HTML body. Since it is the merchant's own value delivered to the merchant's own address, the risk is contained, but HTML-escaping it would prevent broken markup and harden against future reuse with attacker-influenced names.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/email.service.ts` around lines 5 - 16, The OTP email builder currently interpolates firstName directly into the HTML in buildOtpEmailContent, so update that path to HTML-escape the name before inserting it while keeping the text body unchanged. Use the existing buildOtpEmailContent helper as the fix point, and make sure the escaped value is used in the html template so future reuse of this function cannot break markup or allow injected HTML.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/services/otp.services.ts`:
- Around line 11-15: The OTP generator in generateOtp currently relies on
Math.random(), which is not secure for verification codes. Update generateOtp to
use a cryptographically secure source such as crypto.randomInt to produce the
numeric OTP, while preserving the existing OTP_LENGTH-based range and string
return value. Keep the change localized to generateOtp and any related OTP
constants/helpers used to compute the bounds.
---
Outside diff comments:
In `@src/services/merchant.services.ts`:
- Around line 114-131: The registration flow in registerMerchant persists the
merchant as registered before calling sendOtp, so a mail outage leaves the
record committed but the request failing. Update the registerMerchant flow
around prisma.merchant.update and sendOtp so a transient OTP delivery failure
does not reject the whole registration; instead log the sendOtp error and return
a response that tells the user the profile is set up and they should resend the
code, or otherwise separate persistence from delivery so retries do not hit the
merchant.registered guard.
---
Nitpick comments:
In `@src/config/environment.ts`:
- Around line 24-35: The EMAIL_PROVIDER setting in environment configuration is
being force-cast instead of validated, so invalid values can silently fall back
to the console email path. Update the configuration logic in environment.ts for
the email.provider field to validate process.env.EMAIL_PROVIDER against the
allowed EmailProvider values before exposing it, and either fail fast at startup
or emit a clear warning when the value is invalid; use the EmailProvider type
and the email.service.ts provider switch as the key places to align behavior.
In `@src/services/email.service.ts`:
- Around line 5-16: The OTP email builder currently interpolates firstName
directly into the HTML in buildOtpEmailContent, so update that path to
HTML-escape the name before inserting it while keeping the text body unchanged.
Use the existing buildOtpEmailContent helper as the fix point, and make sure the
escaped value is used in the html template so future reuse of this function
cannot break markup or allow injected HTML.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b6cb7543-bfc8-47be-b6ff-787a5b43195a
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (14)
.env.examplepackage.jsonprisma/migrations/20260623120000_add_merchant_email_otp/migration.sqlprisma/schema.prismasrc/config/environment.tssrc/controllers/auth.controllers.tssrc/routes/auth.routes.tssrc/services/email.service.tssrc/services/merchant.services.tssrc/services/otp.services.tstests/integration/auth.email-otp.test.tstests/integration/merchant.register.test.tstests/unit/merchant.register.test.tstests/unit/otp.services.test.ts
Use crypto.randomInt for OTP generation, handle email send failures gracefully after registration, validate EMAIL_PROVIDER, escape HTML in email template, and rename eslint.config to .cjs for ESM compatibility. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Author
Contributor
|
Hello @stiven-skyward |
Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Author
|
Hello @codebestia please check again. |
codebestia
approved these changes
Jun 28, 2026
codebestia
left a comment
codebestia
left a comment
Contributor
There was a problem hiding this comment.
LGTM!
Thank you for your contribution.
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
2 participants
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
email.service.tswith Resend/SMTP/console providers and email template including merchant first namePOST /auth/verify-emailandPOST /auth/resend-otpendpoints (resend rate-limited to 1/min)emailOtpandemailOtpExpiresAton Merchantcloses #10
Test plan
npm test— 115/115 tests passing locallyemailVerified: true; wrong/expired → 400Summary by CodeRabbit