-
Notifications
You must be signed in to change notification settings - Fork 0
Drizzle/BetterAuth/Storage rework #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…Drizzle ORM, introduce new database transaction handling, and clean up related files. Remove unused Prisma service and generated files, and update package dependencies in pnpm-lock.yaml. Adjust API routes and components for improved integration.
… Valkey, implement S3 service for file handling, and enhance application bootstrap logic for storage setup. Remove deprecated services and update storage configuration. Adjust package dependencies in pnpm-lock.yaml and package.json to reflect changes.
…ename environment variables for clarity, add new configuration options for storage and caching, and introduce a new script for dependency checking in package.json. Update app configuration to include application name.
…edis and @scalar/nestjs-api-reference packages, convert better-auth.service to better-auth.provider, and clean up OpenAPI definitions by removing unused routes. Enhance application bootstrap logic for Swagger setup and adjust package.json and pnpm-lock.yaml accordingly.
… DrizzleTransactionClient across services and modules, and introduce a new drizzle provider for improved database handling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| @@ -0,0 +1 @@ | |||
| $(AppService).getHello() No newline at end of file | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
REPL history file accidentally committed to repository
The .nestjs_repl_history file is a local development artifact containing REPL command history. While it was added to .gitignore, the file itself was also committed to the repository. This file should be removed from tracking since it's user-specific development data and not intended to be shared.
| @IsString({ each: true }) | ||
| @IsNotEmpty() | ||
| trustedOrigins = ['http://localhost:3000']; | ||
| trustedOrigins = [process.env['APP_WEB_URL']!]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trusted origins may contain undefined at runtime
The trustedOrigins property uses process.env['APP_WEB_URL']! with a non-null assertion. This is evaluated at class definition time, so if APP_WEB_URL is not set in the environment, trustedOrigins will be [undefined] at runtime despite the TypeScript assertion claiming otherwise. This could cause authentication failures or security issues if the BetterAuth library doesn't handle undefined in the trusted origins array properly.
| }, | ||
| }); | ||
| } | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Email verification OTPs silently not sent during signup
The emailOTP plugin is configured with overrideDefaultEmailVerification: true, which means OTP-based verification replaces URL-based verification for email verification flows. However, the sendVerificationOTP callback only handles type === 'sign-in' and silently returns undefined for other types like 'email-verification' or 'forget-password'. This means when users sign up with sendOnSignUp: true, no verification OTP email will be sent because the type is 'email-verification', not 'sign-in'. Users won't receive verification codes and cannot complete email verification.
| imports: [DatabasesModule], | ||
| adapter: new TransactionalAdapterPrisma({ | ||
| prismaInjectionToken: PrismaService, | ||
| sqlFlavor: 'postgresql', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ClsModule missing global flag breaks cross-module access
The ClsModule.forRoot() configuration removed global: true that was previously present. This causes the CLS (Continuation Local Storage) module to not be globally available, which could break TransactionHost injection in modules like FilesService, UsersService, and BetterAuthProvider that depend on it for database transaction handling.
…or compatibility adjustments.
…pnpm-lock.yaml for improved performance and compatibility.
…sdk/client-s3 to 3.958.0, @itgorillaz/configify to 4.0.2, and various other packages for improved compatibility and performance. Adjust versions for @nestjs packages, react, and others to ensure alignment with latest updates.
…on, account, verification, and file tables. Update file table schema to enforce unique storage keys and adjust utility functions for better error handling.
|
|
||
| export const BetterAuthProvider = { | ||
| provide: BETTER_AUTH, | ||
| inject: [TransactionHost, MailService, Cache, AppConfig, AuthConfig], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Invalid Cache injection token in provider
The inject array uses Cache (a TypeScript interface) as an injection token, but TypeScript interfaces are erased at compile time. The correct injection token is CACHE_MANAGER, not Cache. At runtime, Cache would be undefined, causing NestJS dependency injection to fail with an error like "Cannot resolve dependencies of BetterAuthProvider". The code should import and use CACHE_MANAGER from @nestjs/cache-manager as the token while keeping Cache as the type annotation.
…update file handling in FilesService, and enhance error handling in transform-data interceptor. Introduce drizzle.relations and drizzle.utils for improved database interactions.
… project structure and best practices. Update package.json and pnpm-lock.yaml to include new dependencies like @tanstack/react-query and openapi-fetch, and remove unused prisma configuration.
…gin and OTP components for user authentication, integrate better-auth for session management, and update routing with authentication middleware. Refactor drizzle configuration and update dependencies in package.json and pnpm-lock.yaml for improved compatibility.
…rity and user experience.
| @@ -0,0 +1 @@ | |||
| ../../.. No newline at end of file | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| "db:migrate": "drizzle-kit migrate", | ||
| "db:reset": "drizzle-kit reset", | ||
| "db:studio": "drizzle-kit studio", | ||
| "seed:users": "tsx src/cli.ts seed-users", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…treamline code and improve clarity.
This pull request introduces a major refactor to the authentication and database layers, migrating from Prisma to Drizzle ORM and improving the integration of the BetterAuth authentication library. It also updates configuration files and environment variables to reflect these changes, and cleans up related code and dependencies.
Authentication & Database Refactor:
BetterAuthProvider), enabling dependency injection and simplifying authentication logic. All authentication guards and controllers now use this provider instead of the previous service [1] [2] [3] [4] [5] [6] [7].Configuration & Environment Updates:
.env.examplefile to standardize and rename environment variables, and to reflect the new database and cache configuration for Drizzle ORM.AppConfig, such asAPP_NAME, to support the refactored authentication and app setup.Dependency & Script Changes:
package.json, and added Drizzle ORM, Drizzle Kit, and related scripts for database migrations and management. Also added new utility scripts and updated dependencies for authentication and testing [1] [2] [3] [4].Miscellaneous Improvements:
AppService..gitignoreand REPL history files for developer convenience [1] [2].References: [1] [2] [3] [4] [5]
Note
Major auth and database refactor with ancillary config and tooling updates.
drizzle.provider, table schemas, relations, migration files; remove Prisma config/generated files; transactional adapter switched to DrizzleBetterAuthProvider) with DI helpers; guards/controllers updated; addsALL /auth/client/*proxy viatoNodeHandlerAPP_NAME/APP_URL/APP_WEB_URL,DATABASE_URL,CACHE_URL, storage creds; updateAppConfig,AuthConfig,StorageConfig; adddrizzle.config.tsS3ServiceandAppServicebootstrap to ensure bucket exists and apply public policy; newStorageModuleandFilesServiceseed:users,repl; remove openapi generated types; update.gitignoreAGENTS.md; small interceptor/util additions (Serialize, safer transform)Written by Cursor Bugbot for commit 413fec4. This will update automatically on new commits. Configure here.