Conversation
…e password reset files
…ions into Kit module
…lt roles and permissions seeder
…s controllers and update users controller
…ariables retrieval in auth service, added a simple log for admin Id after seeds run
…ame for unnecessary use
There was a problem hiding this comment.
Pull request overview
This PR represents a major refactor transitioning from a basic authentication system to a comprehensive, production-ready authentication kit with enhanced OAuth support, RBAC, and admin user management.
Changes:
- Migrated from plain Mongoose models to NestJS @nestjs/mongoose decorators with proper service/repository architecture
- Implemented comprehensive JWT-based authentication with email verification, password reset, and automatic token invalidation
- Added OAuth support for Google, Microsoft, and Facebook with both web redirect and mobile token exchange flows
Reviewed changes
Copilot reviewed 48 out of 50 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/helper.ts | Added username generation utility function |
| src/services/*.service.ts | New service layer for auth, users, roles, permissions, OAuth, mail, and seeding |
| src/repositories/*.repository.ts | New repository layer abstracting database operations |
| src/models/*.model.ts | Migrated to NestJS @Schema decorators, removed legacy models (tenant, client) |
| src/middleware/*.guard.ts | Refactored authentication guards, added admin guard and role-based guards |
| src/controllers/*.controller.ts | Simplified controllers to use service layer, removed password-reset and admin controllers |
| src/config/passport.config.ts | Refactored OAuth strategies to use service layer |
| src/auth-kit.module.ts | Complete module restructure with proper DI and Mongoose integration |
| package.json | Updated dependencies and peer dependencies for NestJS 10/11 compatibility |
| README.md | Comprehensive documentation overhaul with API routes, examples, and setup guide |
| .env.example | New environment variable template |
Comments suppressed due to low confidence (1)
src/config/passport.config.ts:97
- There is an extra blank line at the end of the
microsoftCallbackmethod. This should be removed for consistency.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (await this.roles.findByName(dto.name)) throw new Error('Role already exists.'); | ||
| const permIds = (dto.permissions || []).map((p) => new Types.ObjectId(p)); | ||
| return this.roles.create({ name: dto.name, permissions: permIds }); | ||
|
|
There was a problem hiding this comment.
There is an extra blank line after the return statement in the create method. This should be removed to maintain consistency with other methods in the file.
|
|
||
|
|
There was a problem hiding this comment.
There are two unnecessary blank lines at the end of the setPermissions method. These should be removed for consistency.
| let user = await this.roles.findByName('user'); | ||
| if (!user) user = await this.roles.create({ name: 'user', permissions: [] }); | ||
|
|
||
|
|
There was a problem hiding this comment.
There is an extra blank line before the console.log statement. This should be removed for consistency.
| return v; | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
There is an extra blank line after the getEnv method. This should be removed to maintain consistent spacing.
|
|
||
|
|
There was a problem hiding this comment.
There is an extra blank line in the middle of the register method logic. This should be removed to maintain consistent spacing.
| return { accessToken, refreshToken }; | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
There is an extra blank line at the end of the issueTokensForUser method. This should be removed for consistency.
| permissions?: string[]; | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
There are two extra blank lines between class definitions. Only one blank line should be used to separate classes.
| if (await this.users.findByEmail(dto.email)) throw new Error('Email already in use.'); | ||
| if (await this.users.findByUsername(dto.username)) throw new Error('Username already in use.'); |
There was a problem hiding this comment.
The error messages 'Email already in use.' and 'Username already in use.' are not consistent with similar validation messages in auth.service.ts which don't include the trailing period. For consistency across the codebase, either add or remove the period from all similar error messages.
|
|
||
| async setBan(id: string, banned: boolean) { | ||
| const user = await this.users.updateById(id, { isBanned: banned }); | ||
| if (!user) throw new Error('User not found.'); |
There was a problem hiding this comment.
The error message 'User not found.' includes a trailing period while other similar messages in the same file do not. This should be made consistent.
| if (!user) throw new Error('User not found.'); | |
| if (!user) throw new Error('User not found'); |
…ame for unnecessary use
No description provided.