feat(auth): deliver verification codes via email with security hardening - #39
Merged
Merged
Conversation
added 8 commits
September 17, 2026 23:51
Replace the plaintext dev log code delivery with real email delivery and add the
security hardening agreed in review. Existing EmailService methods keep their
swallow-on-failure contract; a new throwing sendVerificationCode is added.
- EmailService.sendVerificationCode(to, code, expiresMinutes): renders the new
Chinese verification-code template, logs recipients via maskEmail, and throws
on failure so the caller can abort the flow
- AuthService.sendVerificationCode(phoneNumber, type, email?): REGISTER uses the
requested email, LOGIN resolves the bound email from DB (never the request),
missing recipient email raises RecipientEmailMissingException
- Throw order: generate -> send email (abort on failure) -> save to Redis ->
set per-recipient cooldown; SMTP failures map to ExternalServiceException
- Redis keys are type scoped (verification_code:{type}:{phone}) with the stored
value kept as the raw 6-digit string for external e2e compatibility
- Wrong-code attempts are counted with a get/set read-modify-write, invalidating
the code at the limit; missing and mismatching codes return the same message
- Codes are now generated with crypto.randomInt; the plaintext send log and the
SMS TODO are removed, the dev log masks the phone number
- UsersService.findUserEmailByPhoneNumber returns the unmasked email
- SendVerificationCodeDto.email is required for REGISTER (ValidateIf + IsEmail)
- AuthModule imports EmailModule and ThrottlerModule.forRoot; only the send
endpoint is guarded (ThrottlerGuard + @Throttle 5/60s), no global APP_GUARD
- New RecipientEmailMissingException and VerificationCodeRateLimitException(429)
- auth.service.spec: email dispatch per scenario (register email / login bound email), missing-recipient errors, SMTP failure abort, cooldown rejection, type-scoped keys, attempt counter and attempt-limit invalidation, indistinguishable messages - email.service.spec: success (subject/context) and rejects-on-failure cases for sendVerificationCode (existing 3 describes unchanged) - users.service.spec: findUserEmailByPhoneNumber (found / user missing / no email) - auth.controller.spec: import ThrottlerModule so the guarded endpoint resolves - test/auth.e2e-spec: fixture gains an email, EmailService is stubbed, login send asserts delivery to the bound email plus new REGISTER cases (missing email 400 / requested email delivery) - test/users.e2e-spec: fixtures gain emails, EmailService stubbed, login helper seeds the type-scoped code key directly (IP throttle 5/60s and the 60s recipient cooldown make repeated send calls impossible in one spec file) - test/email.e2e-spec: MailHog assertion for the verification-code email
- api-contract.md / api-contract.ja.md: send-verification-code gains the email parameter (required for register, forbidden for login), documents the 5-minute email delivery, the 5 requests / 60 s endpoint throttle plus 60 s per-recipient cooldown (429) and the RECIPIENT_EMAIL_MISSING / EXTERNAL_SERVICE_ERROR errors - docs/redis-usage-and-schema.md: key inventory updated to 5 kinds / 6 patterns with the type-scoped verification_code keys (code / attempts / cooldown), the stored value kept as the raw 6-digit string, email wording replacing SMS, refreshed code line references and verification greps Note: the mirrored docs outside this repository (../docs/redis-usage-and-schema.md and ../docs/business-flow/auth/01,02,06) received the same corrections but are not tracked by this repository.
… issued address
Review follow-up for the email verification code flow (P1-1 / P1-2).
- saveVerificationCode now deletes the wrong-code attempt counter, so a
previously failing code can no longer eat into the fresh code's attempt budget
(unit test: 4 wrong attempts -> resend -> the new code is still accepted, and
the counter restarts at 1; verified by mutation check)
- sendVerificationCode records the (trimmed, lower-cased) recipient address as
verification_code_email:{type}:{phone} with the same 5-minute TTL
- register() compares registerDto.email against that recorded address BEFORE
validateVerificationCode, so a mismatch (or missing email / no issued address)
rejects with 400 验证码与邮箱不匹配，请重新获取 without consuming the code or
the attempt counter (one message for all causes, no extra information leak)
- SMTP failure logging no longer prints the raw error.message (may contain RCPT
addresses): fixed text + maskEmail recipient + error.code (<= 40 chars) only
- auth.e2e-spec: register describe added (success / mismatch with untouched code
and no attempt counter / case-normalized match); the send test now asserts the
recorded issued address
Review follow-up (P2-2 / P2-4) plus the matching contract documentation. - the subject becomes 【Booking System】邮箱验证码 so the code can no longer leak through inbox previews, notifications or subject-only logs; the code stays in the body context unchanged - email.service.spec asserts the subject does not contain the code and still checks the body context; the MailHog e2e asserts Subject lacks the code while Body contains it - docs/api-contract.md / .ja.md: register notes gain the email-must-match-issued -address rule (normalization, 400 response, no code/attempt consumption), the send endpoint notes now say login email is ignored (not "must be omitted"), mention the recorded issued address, the code-free subject and the attempt counter reset on resend - docs/redis-usage-and-schema.md: key inventory extended with verification_code_email (6 kinds / 7 patterns), refreshed line references and verification greps
Reject a REGISTER verification-code request when the submitted email already belongs to an existing account, before any code is generated, mailed or stored in Redis. Previously the duplicate was only detected at register time, so the UI advanced to code entry and failed with 409 only after submission. - auth.service: pre-check via usersService.findUserByEmail with the same normalization (trim + lowercase) used for the register email binding and rethrow EmailExistsException as-is (no DatabaseException wrapping) - auth.service.spec: duplicate/email-normalization/pre-check-passed cases plus no-send/no-cache-write assertions, LOGIN unaffected, and 429-before-409 cooldown precedence - auth.e2e-spec: MailHog-backed test proving 409 EMAIL_EXISTS adds no message to the inbox and writes nothing to Redis - docs: document 409 EMAIL_EXISTS (pre-send check) and the 429 precedence
The pre-send duplicate check normalized the address to lowercase but searched
with findUserByEmail, whose findUnique exact match misses a stored raw address
that differs only in case (DB emails are stored as entered and the PostgreSQL
unique constraint is case-sensitive), so a duplicate account could still be
created. Documented behaviour already promised a case-insensitive comparison.
- users.service: add findUserByEmailInsensitive using
findFirst({ where: { email: { equals, mode: 'insensitive' } } }); the exact
match findUserByEmail keeps its contract and is no longer used by the
register pre-check
- auth.service: register pre-check calls findUserByEmailInsensitive
- auth.service.spec: cover both case directions (mixed-case stored/small input
and lowercase stored/upper input) with no-send/no-cache-write assertions
- users.service.spec: pin the insensitive findFirst predicate
- auth.e2e-spec: create a mixed-case fixture user and assert a lowercase
register request is rejected with 409 before any mail is delivered
- docs: state that the pre-send check is case-insensitive while the DB unique
constraint is case-sensitive
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
1 participant
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
findFirst + mode:'insensitive') are rejected before any code is generated, emailed, or written to Redis (409 EMAIL_EXISTS, no side effects).verification_code:{type}:{phone}), attempt counter with resend reset, per-destination 60s cooldown, IP throttle 5/60s on the send endpoint (@nestjs/throttler scoped to AuthModule, app.module untouched),crypto.randomIntgeneration, plaintext code logs removed (both call sites), dev-only log masked, email subject contains no code, SMTP failure logs sanitized.UsersService.findUserEmailByPhoneNumber(sha256 phoneHash reuse); missing email returns explicit error. REGISTER binds the account email to the verified destination (verification_code_email:{type}:{phone}, checked before code validation, no consumption on mismatch).Test evidence
npm run build/typecheck/lint:check→ 0 errorsNotes