Skip to content

fix: distinguish first-time sign-up from returning login in OTP emails - #5

Merged
aspiers merged 3 commits into
mainfrom
signup-vs-login-otp-email
Feb 26, 2026
Merged

fix: distinguish first-time sign-up from returning login in OTP emails#5
aspiers merged 3 commits into
mainfrom
signup-vs-login-otp-email

Conversation

@aspiers

@aspiers aspiers commented Feb 24, 2026

Copy link
Copy Markdown
Contributor

Summary

  • The sendVerificationOTP callback used type === 'sign-in' to determine isNewUser, but all callers pass type: 'sign-in', so the welcome template was always used — even for returning users
  • Now checks PDS account existence via /_internal/account-by-email before sending the OTP email
  • New users (no PDS account yet) get the welcome email; returning users get the simpler sign-in code email

How it works

In better-auth.ts, the sendVerificationOTP callback now calls the PDS internal API to check if an account exists for the email:

  • Account foundisNewUser = false → "Your sign-in code for ePDS"
  • No accountisNewUser = true → "Welcome to ePDS"
  • Check fails → defaults to welcome email (safe fallback)

This matches the same pattern used in complete.ts for consent gating.

Fixes: atproto-n1n

Depends on: #3 (merged)

Summary by CodeRabbit

  • New Features

    • Enhanced user identification in authentication flow using PDS DID lookup for more reliable new user detection.
  • Documentation

    • Updated Docker build guidance with selective per-service rebuild instructions and service-to-image mapping documentation.
  • Refactor

    • Consolidated DID lookup logic into a shared utility module for improved code reusability.
    • Simplified OTP verification flow to use standardized DID lookup instead of type-based inference.
  • Tests

    • Added comprehensive test coverage for DID lookup functionality.
    • Updated authentication tests to validate new user identification method.

@coderabbitai

coderabbitai Bot commented Feb 24, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dc4db47 and c718f4e.

📒 Files selected for processing (7)
  • AGENTS.md
  • packages/auth-service/src/__tests__/better-auth-otp.test.ts
  • packages/auth-service/src/__tests__/get-did-by-email.test.ts
  • packages/auth-service/src/better-auth.ts
  • packages/auth-service/src/lib/get-did-by-email.ts
  • packages/auth-service/src/routes/account-settings.ts
  • packages/auth-service/src/routes/complete.ts

📝 Walkthrough

Walkthrough

The PR refactors the auth service to extract a reusable utility function getDidByEmail that queries an internal PDS endpoint to determine user account existence. This replaces inline implementations and removes OTP type-based user state detection. The Docker build documentation in AGENTS.md is updated to clarify per-service rebuild commands and service-to-image mappings.

Changes

Cohort / File(s) Summary
New DID lookup utility
packages/auth-service/src/lib/get-did-by-email.ts, packages/auth-service/src/__tests__/get-did-by-email.test.ts
Introduces new getDidByEmail function to query internal PDS endpoint for DID by email with timeout and error handling; includes comprehensive test suite covering success, null, error, and encoding scenarios.
Auth service refactoring
packages/auth-service/src/better-auth.ts, packages/auth-service/src/routes/complete.ts, packages/auth-service/src/routes/account-settings.ts
Refactors auth service to import and use new getDidByEmail utility; removes type-based user state detection and replaces with DID presence check; eliminates duplicate inline PDS queries; updates sendVerificationOTP signature to remove type parameter.
Test updates for OTP flow
packages/auth-service/src/__tests__/better-auth-otp.test.ts
Updates OTP tests to determine user state via mocked getDidByEmail fetch instead of hardcoded type check; adds fetch spy for API response simulation.
Documentation
AGENTS.md
Adds per-service Docker build guidance with service-to-image mapping; documents which package changes trigger rebuilds for auth, core, demo, and caddy services.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

Poem

🐰 A DID by email, the rabbit now knows,
No more type-checking where the OTP flows!
From PDS endpoints, the truth shall spring—
Refactored and clean, a tidy thing! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: implementing account existence checks via PDS to distinguish first-time sign-ups from returning logins in OTP emails, which directly addresses the core issue where the previous logic always treated users as new.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch signup-vs-login-otp-email

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@aspiers
aspiers force-pushed the signup-vs-login-otp-email branch from 188dc31 to 40f4772 Compare February 26, 2026 18:45
@vercel

vercel Bot commented Feb 26, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
epds-demo Ready Ready Preview, Comment Feb 26, 2026 7:01pm

Request Review

The sendVerificationOTP callback was using `type === 'sign-in'` to
determine isNewUser, but all callers pass type='sign-in', so the
welcome template was always used regardless of whether the user had
an existing account.

Now checks PDS account existence via /_internal/account-by-email
before sending the OTP — new users (no PDS account yet) get the
welcome email, returning users get the simpler sign-in code email.

Fixes: atproto-n1n
Deduplicate the account-by-email fetch logic that was repeated in
better-auth.ts, complete.ts, and account-settings.ts into a single
lib/get-did-by-email.ts function. Add 11 tests covering all branches.
Update better-auth-otp tests to use the real function instead of
duplicating the isNewUser logic.
@aspiers
aspiers force-pushed the signup-vs-login-otp-email branch from 40f4772 to c718f4e Compare February 26, 2026 19:00
@aspiers
aspiers merged commit bf52747 into main Feb 26, 2026
7 checks passed
@aspiers
aspiers deleted the signup-vs-login-otp-email branch February 26, 2026 19:05
@coderabbitai coderabbitai Bot mentioned this pull request Apr 20, 2026
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant