docs: expand Better Auth migration epic with detailed technical plan#130
docs: expand Better Auth migration epic with detailed technical plan#130
Conversation
Address loopholes and inconsistencies found during codebase review: - Fix code size estimate (~600 → ~1,000 lines) - Fix session expiry mismatch (30 days → 7 days to match current config) - Replace dual JWT+session token architecture with unified Better Auth sessions - Add bcrypt→scrypt dual-algorithm password verification strategy - Add database schema strategy: rename auth→auth_legacy, use Better Auth migrations with custom search_path - Add missing scope: shared package types, frontend API client, onboarding flow, preferences endpoint, layout/page files - Add external ID mapping strategy (use existing UUIDs as Better Auth primary keys) - Add custom user fields strategy (additionalFields for selfProfileId, preferences) - Replace unrealistic feature flag rollback with code+schema revert strategy - Add spike/PoC as first migration step - Expand risk table with new identified risks - Update file paths to be monorepo-aware - Add references to PostgreSQL custom schema docs and migration guides https://claude.ai/code/session_015gXWpvq2YYBXJHNVfZaKGL
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| // (omit hash to use default) | ||
| verify: async ({ hash, password }) => { | ||
| // Try scrypt first (new passwords) | ||
| const scryptResult = await defaultVerify({ hash, password }); |
There was a problem hiding this comment.
defaultVerify is undefined — and missing error handling for bcrypt hashes
Two related issues in this code snippet that would cause login failures for all migrated users:
1. defaultVerify is never imported. Better Auth doesn't export a function by that name. At runtime this would throw ReferenceError: defaultVerify is not defined. You likely want to call Better Auth's built-in verifier via the context object (see Better Auth password docs) or import a specific utility.
2. No try-catch around the scrypt call. When a bcrypt-formatted hash ($2b$...) is passed to a scrypt verifier, most implementations throw rather than return false (the hash prefix is unrecognised). Without a guard, the error propagates up and every legacy user fails to log in—defeating the zero-disruption goal.
Suggested fix:
import bcrypt from "bcrypt";
import { verifyPassword } from "better-auth"; // check actual export name
verify: async ({ hash, password }) => {
// Try scrypt first (new passwords)
try {
if (await verifyPassword({ hash, password })) return true;
} catch {
// Not a scrypt hash, fall through to bcrypt
}
// Fall back to bcrypt (migrated passwords)
try {
return await bcrypt.compare(password, hash);
} catch {
return false;
}
},Worth validating the exact Better Auth API for the default verifier during the spike (Step 1).
| 2. **Prepare database:** | ||
| - Rename `auth` schema to `auth_legacy` | ||
| - Create new `auth` schema | ||
| - Run Better Auth CLI migration: `npx auth@latest migrate` |
There was a problem hiding this comment.
💡 **Suggestion: Use pnpm dlx instead of npx\n\nThe project uses pnpm throughout (all AGENTS.md commands, package.json scripts). For consistency:\n\nbash\npnpm dlx auth@latest migrate\n
🔍 Automated Code Review
Findings
What Changed Since Last ReviewPrevious review (commit
New FindingOne new suggestion on this commit (see inline comment at line 115): AGENTS.md Compliance
SummaryThis is a documentation-only PR expanding the Better Auth migration epic into a thorough implementation guide. The architectural decisions are sound: external ID preservation strategy, dual-algorithm password verification, PostgreSQL schema isolation, and the spike-first approach for validating risky assumptions are all well-considered. No critical or important issues found. 🤖 Automated review by Claude Code • View CI Run |
- Fix password verify snippet: replace undefined `defaultVerify` with bcrypt prefix detection ($2b$/$2a$) and proper fallback logic with try-catch around both algorithms. Add note that exact Better Auth crypto import path must be validated during spike. - Replace `npx` with `pnpm dlx` for consistency with project toolchain. https://claude.ai/code/session_015gXWpvq2YYBXJHNVfZaKGL
|
|
||
| **Dual-algorithm strategy during parallel operation:** | ||
| ```typescript | ||
| import { scryptAsync, timingSafeEqual } from "@noble/hashes/scrypt"; |
There was a problem hiding this comment.
💡 Suggestion: Remove unused imports from password verification snippet
The imports scryptAsync and timingSafeEqual from @noble/hashes/scrypt are never used — scrypt verification is delegated to better-auth/crypto's verify function. Leaving them in would add an unnecessary dependency and would be flagged by Biome.
Suggested fix:
// Remove this line entirely — @noble/hashes is not needed
// import { scryptAsync, timingSafeEqual } from "@noble/hashes/scrypt";
import bcrypt from "bcrypt";
|
🎉 This PR is included in version 2.67.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
2 participants
Summary
This PR significantly expands the Better Auth migration epic documentation with comprehensive technical details, implementation strategies, and risk mitigation approaches. The changes transform the epic from a high-level overview into a detailed implementation guide.
Key Changes
auth_legacyschemaselfProfileIdandpreferencesusing Better Auth'sadditionalFieldsfeatureNotable Implementation Details
SESSION_EXPIRY_DAYS(7 days)verifyfunction for password hashing handles transparent bcrypt→scrypt migrationsearch_pathparameter to keep Better Auth tables separateselfProfileIdas additional fieldauth_tokencookie usagehttps://claude.ai/code/session_015gXWpvq2YYBXJHNVfZaKGL