update auth to better-auth v1 #71
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe pull request introduces significant changes to the Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@wtfdivyansh is attempting to deploy a commit to the BlueFinZ Team on Vercel. A member of the Team first needs to authorize it. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (6)
packages/database/prisma/schema.prisma (2)
30-32: Consider consolidating session identifiersThe model now has both an
idprimary key and a uniquetokenfield. Since we're transitioning to token-based sessions, consider using thetokenas the primary identifier to avoid confusion and maintain a single source of truth.model Session { - id String @id token String @id expiresAt DateTime createdAt DateTime updatedAt DateTime ipAddress String? userAgent String? userId String user User @relation(fields: [userId], references: [id], onDelete: Cascade) - @@unique([token]) @@map("session") }Also applies to: 38-38
43-56: Add security and performance optimizations
Security Considerations:
- The
passwordfield should be properly hashed before storage- Access tokens and refresh tokens should be encrypted at rest
Performance Optimization:
- Consider adding indexes for
userIdandaccountIdwhich are likely to be frequently queriedmodel Account { // ... existing fields ... + @@index([userId]) + @@index([accountId]) }packages/database/prisma/migrations/20241126154111_final_auth/migration.sql (1)
22-24: Review token storage security implicationsThe session token is stored as plain text. Consider whether this meets security requirements:
- Ensure the token is cryptographically secure
- Consider whether additional encryption at rest is needed
- Verify token rotation/invalidation mechanisms are in place
apps/www/components/custom/account-switcher.tsx (3)
12-15: Remove debugging console.log statementsProduction code should not contain console.log statements.
- console.log(token);
Line range hint
12-19: Consider improving the user experience after session switchInstead of reloading the entire page, consider using a more seamless approach:
- Update the client-side state
- Show a loading state during the transition
- Refresh only the necessary data
const onSelect = async (token: string) => { - console.log(token); const active = await authClient.multiSession.setActive({ sessionToken: token, }); - console.log(active); - window.location.reload(); + // Show loading state + setIsLoading(true); + // Refresh necessary data + await refreshUserData(); + // Update UI + setIsLoading(false); };
Line range hint
47-74: Improve accessibility and UI consistencySeveral accessibility and UI consistency issues need attention:
- Use semantic HTML buttons instead of divs for clickable elements
- Add proper ARIA attributes
- Consistent text casing for actions
<select onChange={(e) => onSelect(e.target.value)} value={activeSession.session.token} - className="border-2 border-gray-300[0.3] rounded-md p-2" + className="border-2 border-gray-300[0.3] rounded-md p-2" + aria-label="Switch active account" > - <div - className="flex border border-neutral-900[0.2] bg-neutral-900/60 p-2" + <button + className="flex border border-neutral-900[0.2] bg-neutral-900/60 p-2" onClick={handleSignOut} + aria-label="Sign out from all accounts" > - Logout from all - </div> + Log Out From All + </button> - <div - className="flex border border-neutral-900[0.2] bg-neutral-900/60 p-2" + <button + className="flex border border-neutral-900[0.2] bg-neutral-900/60 p-2" onClick={handleCurrentSignOut} + aria-label="Sign out from current account" > - Logout - </div> + Log Out + </button> <Link href={"/auth"}> - <div className="flex border border-neutral-900[0.2] bg-neutral-900/60 p-2"> + <button className="flex border border-neutral-900[0.2] bg-neutral-900/60 p-2"> Add Account - </div> + </button> </Link>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
apps/www/components/custom/account-switcher.tsx(3 hunks)apps/www/package.json(1 hunks)packages/auth/package.json(1 hunks)packages/auth/tsconfig.json(1 hunks)packages/database/prisma/migrations/20241126154111_final_auth/migration.sql(1 hunks)packages/database/prisma/schema.prisma(2 hunks)packages/typescript-config/base.json(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/auth/tsconfig.json
- packages/typescript-config/base.json
🔇 Additional comments (11)
packages/auth/package.json (3)
15-16: Verify compatibility between better-auth and better-call
There might be compatibility issues between the stable better-auth v1.0.5 and the beta version of better-call (0.2.14-beta.3).
✅ Verification successful
No compatibility issues found between better-auth and better-call
Based on the analysis of the dependencies:
- better-auth v1.0.5 includes better-call v0.3.3-beta.2 as a dependency
- The package.json specifies better-call v0.2.14-beta.3 directly
- Both versions of better-call have identical dependencies (@better-fetch/fetch, rou3, set-cookie-parser, uncrypto)
- The codebase shows no direct imports from better-call, suggesting it's an internal dependency of better-auth
Since better-call is primarily used internally by better-auth and there are no direct usages in the codebase, the version mismatch won't cause compatibility issues. The stable version of better-auth (1.0.5) has been tested with its bundled better-call version.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for version compatibility issues
# Search for direct dependencies between packages
rg -A 5 "better-(auth|call)"
# Look for potential integration points
ast-grep --pattern 'import { $$$ } from "better-auth"'
ast-grep --pattern 'import { $$$ } from "better-call"'
Length of output: 5487
15-15: Verify breaking changes in better-auth v1.0.5
The upgrade from better-auth 0.8.6-beta.4 to 1.0.5 represents a major version change. This could introduce breaking changes that need careful migration.
15-15: Verify security implications of the auth update
As this is an authentication library update, we need to ensure there are no known vulnerabilities in the new version.
Consider implementing a thorough testing plan for the authentication flow changes, especially around session token handling.
packages/database/prisma/schema.prisma (2)
21-21: LGTM: Accounts relation properly restored
The reintroduction of the accounts relation is correct and necessary for maintaining the relationship with the Account model.
Line range hint 1-85: Verify schema migration impact
Please ensure that:
- Migration scripts handle existing data appropriately
- Token-based session changes are backward compatible
- Account model changes don't affect existing authentication flows
✅ Verification successful
Schema changes are properly handled with migrations
The migration files show a clear progression of schema changes with appropriate handling:
- Initial auth setup (20241110144439_auth) creates the base tables with proper relations
- Social update (20241117140010_social) adds verification timestamps and Trigger model
- Final auth changes (20241126154111_final_auth) properly handles:
- Session token uniqueness with
session_token_keyindex - Account model refinements with separate expiry fields
- All timestamp columns (createdAt/updatedAt) with proper migrations
- Session token uniqueness with
The migrations include necessary warnings about required fields and handle potential data issues through proper ALTER TABLE statements. The schema is consistent with all migrations, and the changes maintain backward compatibility through careful column additions rather than destructive changes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for related migration files and their content
fd -e sql -e prisma . | while read -r file; do
echo "=== $file ==="
cat "$file"
done
# Look for potential references to old session handling
rg -i "sessionId|session_id" --type ts --type js
Length of output: 6249
packages/database/prisma/migrations/20241126154111_final_auth/migration.sql (2)
27-28: Verify impact of making createdAt nullable in verification table
Making createdAt nullable in the verification table is unusual and could impact data integrity. This change might affect:
- Audit trails
- Verification expiration logic
- Data consistency checks
✅ Verification successful
Let me gather more information about the verification table and its usage.
Based on the gathered information, I can now provide the final response:
Making createdAt nullable in verification table is acceptable
The verification table is used for temporary verification records (like email verification codes) as indicated by the presence of expiresAt field. Making createdAt nullable while adding updatedAt suggests a design choice where the focus is on tracking when records are modified rather than created, which is reasonable for temporary verification data that gets cleaned up after expiration.
- The schema shows
Verificationis a standalone entity without relations to other tables - No business logic was found that depends on
createdAtfor verification expiration (it uses dedicatedexpiresAtfield) - The change doesn't impact data integrity as the verification records are temporary by nature
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for dependencies on createdAt in verification logic
rg -A 5 "verification.*createdAt"
ast-grep --pattern 'verification {
$$$
createdAt
$$$
}'
Length of output: 699
Script:
#!/bin/bash
# Check schema definition and usage of verification table
fd schema.prisma --exec cat {}
rg -A 10 "verification" --type ts --type js
Length of output: 3056
30-31: Ensure proper index for session token lookups
The unique index on session tokens is good for preventing duplicates and optimizing lookups. However, verify:
- Token length constraints to prevent index size issues
- Impact on write performance
apps/www/package.json (2)
31-31: Verify breaking changes from better-auth beta to stable
This is a significant upgrade from a beta version (0.8.1-beta.1) to a stable version (1.0.5). Given the session management changes mentioned in the PR summary, ensure all breaking changes are properly handled.
✅ Verification successful
Let me gather more information about the auth implementation to better understand the impact.
Let me check the auth client implementation and the session handling code to understand if there are any breaking changes that need attention.
Based on the code analysis, I can now provide the final response:
Better-auth upgrade appears safe with current implementation
The codebase is already using the modern better-auth APIs and patterns that are compatible with v1.0.5:
- Using the recommended
multiSessionplugin and its client counterpart - Proper session token handling in account switcher
- Standard auth client setup with React hooks
- Correct API implementation for session management
The implementation aligns well with better-auth's stable API patterns, and no breaking changes are evident in the current usage. The upgrade from beta to stable version should be safe.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for breaking changes and migration guides
# Search for migration guides or breaking changes in the codebase
rg -i "migration|upgrade|breaking.?changes" --type md
# Look for better-auth initialization or configuration files
fd -e ts -e js -e json "auth.?config|better.?auth.?(config)"
# Search for current usage patterns that might need updates
ast-grep --pattern 'import { $$$ } from "better-auth"'
Length of output: 264
Script:
#!/bin/bash
# Check the auth package implementation
cat packages/auth/src/auth.ts
# Look for better-auth usage patterns across the codebase
rg -l "better-auth" --type=ts --type=js
# Search for session-related code that might need updates
rg -l "session" --type=ts --type=js
# Look for any auth-related tests
fd -e test.ts -e test.js -e spec.ts -e spec.js auth
Length of output: 1783
Script:
#!/bin/bash
# Check the auth client implementation
cat apps/www/lib/auth-client.ts
# Check session-related implementations
cat apps/api/app/api/[[...route]]/session.ts
cat apps/www/components/custom/account-switcher.tsx
# Look for any migration guides in node_modules
cat node_modules/better-auth/CHANGELOG.md 2>/dev/null || echo "Changelog not found"
cat node_modules/better-auth/README.md 2>/dev/null || echo "README not found"
Length of output: 3341
31-31: Verify better-auth compatibility with React RC
The project uses React 19.0.0-rc. Verify that better-auth v1.0.5 is compatible with this React version, especially for session management features.
apps/www/components/custom/account-switcher.tsx (2)
4-4: LGTM!
The Link import is correctly added to support the new account addition feature.
12-15: Verify better-auth version compatibility
Ensure the token-based session management is compatible with the installed version of better-auth.
✅ Verification successful
Let me search for better-auth related files and dependencies to verify this further.
Let me check the auth client implementation and usage to verify the compatibility.
Token-based session management is correctly implemented and compatible
The implementation is using better-auth v1.0.5 with the proper multiSession plugin configuration. The token-based approach in account-switcher.tsx aligns with the better-auth setup in packages/auth/src/auth.ts and the client configuration using multiSessionClient().
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check the installed version of better-auth
cat package.json | jq '.dependencies["better-auth"]'
# Look for any breaking changes in the migration guide
fd -t f "MIGRATION" -X cat {}
Length of output: 91
Script:
#!/bin/bash
# Search for better-auth in all package.json files
fd package.json -x cat {} | grep -i "better-auth"
# Search for better-auth imports
rg "from ['\"]better-auth['\"]" -A 2
# Look for auth client initialization
ast-grep --pattern 'authClient = new $_'
Length of output: 463
Script:
#!/bin/bash
# Check auth client implementation
rg "multiSession.setActive" -A 3
# Check the auth client initialization and configuration
cat packages/auth/src/auth.ts
# Look for any version-specific implementations
ast-grep --pattern 'const authClient = $_'
Length of output: 1982
| createdAt DateTime? | ||
| updatedAt DateTime? |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enforce timestamp consistency
Making createdAt and updatedAt optional deviates from standard practices and could lead to data consistency issues. Consider making these fields required with default values.
model Verification {
id String @id
identifier String
value String
expiresAt DateTime
- createdAt DateTime?
- updatedAt DateTime?
+ createdAt DateTime @default(now())
+ updatedAt DateTime @updatedAt
@@map("verification")
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| createdAt DateTime? | |
| updatedAt DateTime? | |
| createdAt DateTime @default(now()) | |
| updatedAt DateTime @updatedAt |
| - You are about to drop the column `expiresAt` on the `account` table. All the data in the column will be lost. | ||
| - A unique constraint covering the columns `[token]` on the table `session` will be added. If there are existing duplicate values, this will fail. | ||
| - Added the required column `createdAt` to the `account` table without a default value. This is not possible if the table is not empty. | ||
| - Added the required column `updatedAt` to the `account` table without a default value. This is not possible if the table is not empty. | ||
| - Added the required column `createdAt` to the `session` table without a default value. This is not possible if the table is not empty. | ||
| - Added the required column `token` to the `session` table without a default value. This is not possible if the table is not empty. | ||
| - Added the required column `updatedAt` to the `session` table without a default value. This is not possible if the table is not empty. |
There was a problem hiding this comment.
The migration adds multiple required columns (createdAt, updatedAt) without default values to both account and session tables. This will fail if these tables contain existing data.
Consider modifying the migration to handle existing data:
- ADD COLUMN "createdAt" TIMESTAMP(3) NOT NULL,
- ADD COLUMN "updatedAt" TIMESTAMP(3) NOT NULL
+ ADD COLUMN "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP,
+ ADD COLUMN "updatedAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMPCommittable suggestion skipped: line range outside the PR's diff.
| ALTER TABLE "account" DROP COLUMN "expiresAt", | ||
| ADD COLUMN "accessTokenExpiresAt" TIMESTAMP(3), | ||
| ADD COLUMN "createdAt" TIMESTAMP(3) NOT NULL, | ||
| ADD COLUMN "refreshTokenExpiresAt" TIMESTAMP(3), | ||
| ADD COLUMN "scope" TEXT, | ||
| ADD COLUMN "updatedAt" TIMESTAMP(3) NOT NULL; |
There was a problem hiding this comment.
💡 Codebase verification
Data loss warning: expiresAt column dropped without migration
The migration script shows that the expiresAt column is being dropped with the warning "All the data in the column will be lost" and there's no data migration logic to transfer existing expiration data to the new accessTokenExpiresAt column. This is concerning because:
- The
accounttable was created in the previous migration (20241110144439_auth) - The current migration (20241126154111_final_auth) drops
expiresAtwithout preserving its data - The new columns
accessTokenExpiresAtandrefreshTokenExpiresAtare nullable, but any existing expiration data will be lost
Suggested actions:
- Add a data migration step to copy
expiresAtvalues toaccessTokenExpiresAtbefore dropping the column - Or confirm that data loss is acceptable if this is a development environment with no production data
🔗 Analysis chain
Verify data migration for removed expiresAt column
The expiresAt column is being dropped and replaced with more specific expiration columns. Ensure any existing expiration data is migrated appropriately.
Let me check the migration history to understand if this is a new table or if there's existing data that needs migration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if there's existing data in the expiresAt column that needs migration
echo "Checking for existing expiration data..."
ast-grep --pattern 'expiresAt' # Look for usages in the codebase
rg -l "expiresAt" # Find files referencing this column
Length of output: 324
Script:
#!/bin/bash
# Check the previous migration that introduced the expiresAt column
cat packages/database/prisma/migrations/20241110144439_auth/migration.sql
# Check if there are any data migrations in the current migration
cat packages/database/prisma/migrations/20241126154111_final_auth/migration.sql
Length of output: 3275
| const handleCurrentSignOut = async () => { | ||
| await authClient.multiSession.revoke({ | ||
| sessionToken: activeSession?.session.token | ||
| }); | ||
| window.location.reload(); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and improve user experience
The current implementation lacks error handling and forces a page reload.
const handleCurrentSignOut = async () => {
+ try {
+ setIsLoading(true);
await authClient.multiSession.revoke({
sessionToken: activeSession?.session.token
});
- window.location.reload();
+ router.push('/auth');
+ } catch (error) {
+ console.error('Failed to sign out:', error);
+ // Show error notification to user
+ } finally {
+ setIsLoading(false);
+ }
};Committable suggestion skipped: line range outside the PR's diff.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Dependencies
better-authpackage to a stable version (1.0.5) across multiple components.Database Changes