-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat: implement SMTP email service, adding env also adding email verification #153
Conversation
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThis pull request integrates comprehensive email functionality into the application. Key updates include new dependencies for mailing, the introduction of a dedicated mail module and service with templating support, and modifications to the authentication flow. New GraphQL types and mutations enable email confirmation and resend operations. Additionally, the user model is enhanced with fields to track email confirmation status and email send time, while environment configuration is extended for SMTP settings. Frontend components are updated to handle email verification and resend operations. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant CP as ConfirmEmailPage (Frontend)
participant AR as AuthResolver
participant AS as AuthService
U->>CP: Click email confirmation link with token
CP->>AR: Execute confirmEmail(token) mutation
AR->>AS: Call confirmEmail(token)
AS-->>AR: Return EmailConfirmationResponse
AR-->>CP: Deliver confirmation response
CP->>U: Display success or error message
sequenceDiagram
participant U as User
participant SS as SignUpModal (Frontend)
participant UR as UserResolver
participant AS as AuthService
participant MS as MailService
participant SMTP as Mailer Service (SMTP)
U->>SS: Register or click "Resend Confirmation Email"
SS->>UR: Execute resendConfirmationEmail mutation
UR->>AS: Call resendVerificationEmail(email)
AS->>MS: Trigger sendConfirmationEmail(email, token)
MS->>SMTP: Send confirmation email
SMTP-->>MS: Email sent response
MS-->>AS: Return confirmation result
AS-->>UR: Deliver EmailConfirmationResponse
UR-->>SS: Update UI with result and cooldown
Poem
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. 🪧 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 (
|
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.
Actionable comments posted: 4
🧹 Nitpick comments (12)
backend/src/user/dto/resend-email.input.ts (1)
1-1
: Fix incorrect file path comment.The file header comment references an incorrect path
src/auth/dto/resend-email.input.ts
while the actual file is in the user module atsrc/user/dto/resend-email.input.ts
.-// src/auth/dto/resend-email.input.ts +// src/user/dto/resend-email.input.tsbackend/src/user/user.model.ts (1)
40-42
: Consider adding documentation to explain the purpose of lastEmailSendTime.While the field name is descriptive, adding a brief comment would help clarify its purpose in preventing email spam/abuse by tracking when the last email was sent.
@Field() @UpdateDateColumn({ type: 'datetime' }) + // Tracks the timestamp of the last email sent to prevent abuse/spam lastEmailSendTime: Date;
backend/src/mail/mail.module.ts (3)
15-41
: Consider improving security with secure SMTP connection.The current configuration has
secure: false
which might not be suitable for production environments. For enhanced security, consider enabling TLS/SSL when in production mode.transport: { host: config.get('MAIL_HOST'), port: config.get<number>('MAIL_PORT'), - secure: false, + secure: config.get('NODE_ENV') === 'production', auth: { user: config.get('MAIL_USER'), pass: config.get('MAIL_PASSWORD'), }, },
12-43
: Remove commented code and check for environment variable existence.The commented line and imports suggestion is unnecessary. Also, it's a good practice to verify that required environment variables exist during initialization to fail fast if configuration is missing.
imports: [ ConfigModule, TypeOrmModule.forFeature([User]), MailerModule.forRootAsync({ - // imports: [ConfigModule], // import module if not enabled globally useFactory: async (config: ConfigService) => { + // Verify required env variables exist + const requiredVars = ['MAIL_HOST', 'MAIL_PORT', 'MAIL_USER', 'MAIL_PASSWORD', 'MAIL_FROM', 'MAIL_DOMAIN']; + for (const v of requiredVars) { + if (!config.get(v)) { + throw new Error(`Required environment variable ${v} is missing`); + } + } + return { - // transport: config.get("MAIL_TRANSPORT"), - // or transport: {
30-30
: Improve email sender display name.The current "Your App" sender name is generic. Consider using a configurable app name from the environment variables for better branding.
defaults: { - from: `"Your App" <${config.get<string>('MAIL_FROM')}>`, + from: `"${config.get('APP_NAME', 'CodeFox')}" <${config.get<string>('MAIL_FROM')}>`, },backend/src/mail/mail.service.ts (3)
17-26
: Email confirmation URL uses hardcoded protocol.The confirmation URL uses a hardcoded HTTPS protocol. Consider making the protocol configurable for development environments where HTTP might be used.
async sendConfirmationEmail(email: string, token: string) { - const confirmUrl = `https://${this.configService.get('MAIL_DOMAIN')}/auth/confirm?token=${token}`; + const protocol = this.configService.get('NODE_ENV') === 'production' ? 'https' : 'http'; + const confirmUrl = `${protocol}://${this.configService.get('MAIL_DOMAIN')}/auth/confirm?token=${token}`; await this.mailerService.sendMail({
28-42
: Password reset email has redundant user data.The context includes both
name
andfirstName
with the same value (user.username). If they're intended to be the same, one should be removed for clarity.context: { name: user.username, - firstName: user.username, url, },
44-58
: Remove commented out code.There's a significant block of commented code that appears to be an alternative implementation of the confirmation email method. This should be removed to maintain clean code.
- // async sendConfirmationEmail(user: User, token: string) { - // const frontendUrl = this.configService.get('FRONTEND_URL'); - // const url = `${frontendUrl}/confirm-email?token=${token}`; - - // await this.mailerService.sendMail({ - // to: user.email, - // subject: 'Welcome! Confirm Your Email', - // template: './confirmation', // This will use the confirmation.hbs template - // context: { // Data to be sent to the template - // name: user.username, - // firstName: user.firstName || user.username, - // url, - // }, - // }); - // }backend/src/user/user.resolver.ts (1)
47-59
: Uncommented and updated resendConfirmationEmail mutation.The mutation has been properly updated to use the new ResendEmailInput type. However, the commented-out version should be removed to keep the code clean.
- // @Mutation(() => EmailConfirmationResponse) - // async resendConfirmationEmail( - // @Args('input') resendInput: ResendConfirmationInput, - // ): Promise<EmailConfirmationResponse> { - // return this.authService.resendVerificationEmail(resendInput.email); - // } @Mutation(() => EmailConfirmationResponse) async resendConfirmationEmail( @Args('input') input: ResendEmailInput, ): Promise<EmailConfirmationResponse> { return this.authService.resendVerificationEmail(input.email); }backend/src/auth/auth.service.ts (3)
46-83
: Error variable defined but not used in the catch block.The implementation of the email confirmation flow is logically sound and provides appropriate responses for different scenarios. However, there's a minor issue with the error handling.
In the catch block at line 77, you define an
error
variable but don't use it. Consider either logging the error or removing the variable name from the catch clause:- } catch (error) { + } catch { return { message: 'Invalid or expired token', success: false, }; }Alternatively, if you want to keep the error for debugging:
- } catch (error) { + } catch (error) { + Logger.error(`Token verification failed: ${error.message}`); return { message: 'Invalid or expired token', success: false, }; }🧰 Tools
🪛 GitHub Check: autofix
[warning] 77-77:
'error' is defined but never used
85-103
: Consider adding error handling for email sending.The method for sending verification emails looks good overall, but lacks error handling for the email sending operation.
Add a try-catch block to handle potential errors when sending emails:
async sendVerificationEmail(user: User): Promise<EmailConfirmationResponse> { // Generate confirmation token const verifyToken = this.jwtService.sign( { email: user.email }, { expiresIn: '30m' }, ); + try { // Send confirmation email await this.mailService.sendConfirmationEmail(user.email, verifyToken); // update user last time send email time user.lastEmailSendTime = new Date(); await this.userRepository.save(user); return { message: 'Verification email sent successfully!', success: true, }; + } catch (error) { + Logger.error(`Failed to send verification email: ${error.message}`); + return { + message: 'Failed to send verification email. Please try again later.', + success: false, + }; + } }
105-136
: Good implementation of a cooldown mechanism for email resending.The resendVerificationEmail method effectively implements a cooldown period to prevent email flooding, with clear error messages and proper user validation.
Consider moving the cooldown period to a configurable constant or environment variable instead of hardcoding the value. This would make it easier to adjust without code changes:
+private readonly EMAIL_COOLDOWN_PERIOD = 60 * 1000; // 1 minute in milliseconds async resendVerificationEmail(email: string) { // existing code... // Check if a cooldown period has passed - const cooldownPeriod = 1 * 60 * 1000; // 1 minute in milliseconds if ( user.lastEmailSendTime && - new Date().getTime() - user.lastEmailSendTime.getTime() < cooldownPeriod + new Date().getTime() - user.lastEmailSendTime.getTime() < this.EMAIL_COOLDOWN_PERIOD ) { const timeLeft = Math.ceil( - (cooldownPeriod - + (this.EMAIL_COOLDOWN_PERIOD - (new Date().getTime() - user.lastEmailSendTime.getTime())) / 1000, ); // rest of the code... } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
backend/.env
(1 hunks)backend/package.json
(2 hunks)backend/src/app.module.ts
(2 hunks)backend/src/auth/auth.module.ts
(2 hunks)backend/src/auth/auth.resolver.ts
(2 hunks)backend/src/auth/auth.service.ts
(6 hunks)backend/src/mail/mail.module.ts
(1 hunks)backend/src/mail/mail.service.ts
(1 hunks)backend/src/mail/templates/confirmation.hbs
(1 hunks)backend/src/mail/templates/passwordReset.hbs
(1 hunks)backend/src/user/dto/resend-email.input.ts
(1 hunks)backend/src/user/user.model.ts
(2 hunks)backend/src/user/user.module.ts
(1 hunks)backend/src/user/user.resolver.ts
(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- backend/src/mail/templates/passwordReset.hbs
🧰 Additional context used
🪛 GitHub Check: autofix
backend/src/auth/auth.service.ts
[warning] 77-77:
'error' is defined but never used
🔇 Additional comments (24)
backend/src/user/user.module.ts (2)
9-9
: Appropriate import of MailModule.The import of the MailModule is correctly added, which aligns with the PR objective of implementing email service functionality.
12-17
: Well-structured module imports with MailModule integration.The MailModule is properly added to the imports array, enabling the UserModule to utilize email functionality for user-related operations like verification emails. This integration supports the PR's email service implementation.
backend/src/app.module.ts (2)
18-18
: Appropriate import of MailModule at application level.The import statement is correctly added, following the project's import style.
51-51
: Clean integration of MailModule in the application imports.The MailModule is properly added to the AppModule imports, making email functionality available throughout the application. This follows NestJS best practices for module composition.
backend/src/user/dto/resend-email.input.ts (1)
5-11
: Well-structured input DTO with proper validation.The ResendEmailInput class is correctly defined with appropriate decorators for GraphQL and validation. The email field has proper validation using IsEmail and IsNotEmpty which ensures that only valid email addresses will be accepted.
backend/src/auth/auth.module.ts (3)
12-12
: Appropriate import of MailModule in AuthModule.The import statement is correctly added, following the project's import style.
27-27
: Proper integration of MailModule in AuthModule.The MailModule is correctly added to the imports array, enabling the AuthModule to utilize email functionality for authentication-related operations like sending verification emails, which aligns with the PR objectives.
12-27
:✅ Verification successful
Verify EmailService integration with AuthService.
While the MailModule is correctly imported, it would be beneficial to verify that the AuthService properly utilizes the email functionality for verification emails as mentioned in the PR objectives.
🏁 Script executed:
#!/bin/bash # Check if AuthService correctly imports and uses MailService for email verification echo "Checking AuthService for MailService usage..." rg -A 3 "MailService" "backend/src/auth/auth.service.ts" echo "Checking for email verification methods in AuthService..." rg -A 5 "verification|confirm|email" "backend/src/auth/auth.service.ts"Length of output: 3842
Action: Email verification integration confirmed.
The verification confirms that:
- AuthService imports and utilizes MailService as expected.
- The email confirmation and resend flows correctly invoke the email functionality (e.g., using
sendConfirmationEmail
).No further modifications are needed on this aspect of the AuthService integration.
backend/package.json (2)
33-33
: Good choice for email functionality with @nestjs-modules/mailerAdding the NestJS mailer module is appropriate for implementing the email service required for verification emails.
60-60
:✅ Verification successful
Consider updating nodemailer to a more recent version
The current version
^6.10.0
is somewhat dated. Consider updating to the latest version for bug fixes and security improvements.
🌐 Web query:
What is the latest version of nodemailer?
💡 Result:
The latest version of Nodemailer is 6.10.0, released on January 23, 2025[4][6]. This release includes features like adding the Seznam email service configuration and bug fixes related to proxy error handling[6].
For context:
- Nodemailer v6.8.0 was released in September 2022 with DNS timeout improvements[3].
- The package has maintained regular updates, with version 6.10.0 being the most recent stable release as of March 2025[4][6].
Citations:
- 1: https://github.com/nodemailer/nodemailer/blob/master/CHANGELOG.md
- 2: https://community.nodemailer.com
- 3: https://nodejs.libhunt.com/nodemailer-latest-version
- 4: https://www.npmjs.com/package/nodemailer
- 5: https://www.nodemailer.com
- 6: https://github.com/nodemailer/nodemailer/releases
- 7: https://community.nodemailer.com/category/releases/
- 8: https://nodemailer.com/app/
Nodemailer version is up-to-date.
After verifying the latest version from the web, it turns out that Nodemailer 6.10.0 (released on January 23, 2025) is indeed the current stable release. The dependency inbackend/package.json
correctly uses"nodemailer": "^6.10.0"
. No update is necessary.backend/src/mail/templates/confirmation.hbs (1)
1-26
:❓ Verification inconclusive
Email template looks good but needs consistency checks
The email template is well-structured with proper HTML formatting and responsive styling.
A few points to verify:
- Ensure the 24-hour expiration mentioned in line 19 matches the actual token expiration logic in your backend.
- Confirm that the support email address
support@codefox.net
at line 22 is correct and actively monitored.- The placeholder
{{confirmUrl}}
will need proper URL construction in the mail service.
🏁 Script executed:
#!/bin/bash # Check if the token expiration logic matches the 24-hour period mentioned in the email template # Search for token expiration configuration rg -A 3 "expiresIn|expiration|expires" --glob "*.ts" src/Length of output: 104
Action Required: Verify Configuration Details in Email Template
The email template is well-structured. However, please manually verify the following points:
- Token Expiration Logic: The 24-hour expiration mentioned in the template must match the backend’s configuration. Our initial search in the
src/
folder didn’t return results (note: “src/” was not found). Please confirm that the token expiration setting (likely defined inbackend/src/
or an appropriate configuration file) is set to 24 hours.- Support Email Address: Ensure that the support email address (
support@codefox.net
) is correct and actively monitored.- URL Construction: Confirm that the
{{confirmUrl}}
placeholder is being properly replaced with a correctly constructed URL within the mail service.backend/src/auth/auth.resolver.ts (2)
21-28
: Good implementation of EmailConfirmationResponse object typeThe response structure with a required message and optional success flag is appropriate for email confirmation responses.
46-51
:❓ Verification inconclusive
Ensure proper error handling for confirmEmail mutation
The implementation looks good, but make sure that:
- The
authService.confirmEmail()
method includes proper error handling for invalid or expired tokens- The method returns appropriate error messages when confirmation fails
- Consider adding rate limiting to prevent abuse of this endpoint
🏁 Script executed:
#!/bin/bash # Check the implementation of the confirmEmail method in AuthService # Look for the confirmEmail method implementation rg -A 15 "confirmEmail.*token" --glob "*.ts" src/auth/Length of output: 106
Action Required: Verify Error Handling and Rate Limiting in confirmEmail Mutation
The resolver code correctly delegates to the auth service, but please ensure that:
- The
authService.confirmEmail()
method (likely inbackend/src/auth/auth.service.ts
) implements robust error handling for invalid or expired tokens.- It returns clear, actionable error messages when email confirmation fails.
- Appropriate rate limiting is considered at the service or endpoint level to mitigate abuse.
Since the initial search command used an incorrect directory path and returned no output, please manually verify (or update the verification scripts) that these concerns are fully addressed in the auth service implementation.
backend/.env (1)
13-13
: Verify SMTP port configurationPort 587 is commonly used for TLS-enabled SMTP connections, which is appropriate for secure email sending. However, some providers might require different ports (465 for SSL, 25 for unencrypted). Confirm this matches your email provider's requirements.
backend/src/user/user.model.ts (2)
14-14
: Added TypeORM decorator import for email tracking feature.The
UpdateDateColumn
import will be used for the newlastEmailSendTime
property to automatically track when emails are sent to users.
36-38
: Email confirmation tracking looks good.The boolean field with a default value of
false
is appropriate for tracking whether a user's email has been confirmed. This is a standard approach for email verification implementations.backend/src/mail/mail.module.ts (1)
1-10
: Good module imports for mail functionality.The necessary dependencies for implementing email functionality are properly imported, including the Mailer module, Handlebars for templating, and the JWT module for token handling.
backend/src/user/user.resolver.ts (2)
19-20
: Imports for email functionality.Good addition of imports for the email confirmation response type and the resend email input type to support the new email verification workflow.
31-38
: New RegisterResponse type is well structured.The response includes both the user object and a message field, which is a good practice for providing context or instructions after registration.
backend/src/auth/auth.service.ts (5)
22-27
: Good addition of necessary imports.The imports added are appropriate for the new email verification functionality. The addition of
MailService
and response types aligns well with the PR objectives.
37-37
: Correct injection of the MailService.The MailService is properly injected into the constructor, enabling email sending capabilities for the authentication service.
138-164
: Updated registration flow to include email verification.The register method has been appropriately modified to:
- Set the initial email confirmation status to false
- Send a verification email after user creation
- Return a clear message about the confirmation email
This is a proper implementation of the PR's objective to add email verification.
177-179
: Good email confirmation enforcement for login.The login method now correctly checks if the user's email is confirmed before allowing login, which is essential for enforcing email verification requirements. The error message is clear and actionable for the user.
157-157
: Appropriate field initialization for email confirmation status.Setting
isEmailConfirmed
to false during user creation ensures new users start in the unverified state, which is consistent with the email verification flow being implemented.
backend/.env
Outdated
MAIL_HOST=smtp.example.com | ||
MAIL_USER=user@example.com | ||
MAIL_PASSWORD=topsecret | ||
MAIL_FROM=noreply@example.com | ||
MAIL_PORT=587 | ||
MAIL_DOMAIN=your_net |
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.
💡 Verification agent
❓ Verification inconclusive
Replace placeholder email credentials with actual values
The current mail configuration contains example values that must be replaced with actual credentials before deployment.
Issues to address:
- Replace
smtp.example.com
,user@example.com
, etc. with actual mail server details - Ensure
MAIL_PASSWORD
is properly secured and not committed to version control - Update
MAIL_DOMAIN
to match your actual domain (appears to be "codefox" based on the email template)
This is critical for the email service to function correctly.
Action Required: Update Email Credentials Securely
The configuration in backend/.env
(lines 8–14) still uses placeholder values, which must be replaced with the actual mail server settings before deployment. Please address the following:
- MAIL_HOST, MAIL_USER, MAIL_FROM, MAIL_PORT: Replace
smtp.example.com
,user@example.com
,noreply@example.com
, and587
with your production mail server details. - MAIL_PASSWORD: Ensure this password is secured (e.g., via proper secrets management) and not stored in version control.
- MAIL_DOMAIN: Update from
your_net
to reflect your actual domain name (e.g., based on the expected value like "codefox" from the email template).
This update is critical to ensure the email service functions correctly and securely in the deployed environment.
constructor( | ||
@InjectRepository(User) | ||
private userRepository: Repository<User>, | ||
private readonly mailerService: MailerService, | ||
private configService: ConfigService, | ||
) {} |
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.
🛠️ Refactor suggestion
Unused user repository dependency.
The userRepository
is injected but never used in the service. Either remove it or document why it's needed for future use.
constructor(
- @InjectRepository(User)
- private userRepository: Repository<User>,
private readonly mailerService: MailerService,
private configService: ConfigService,
) {}
📝 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.
constructor( | |
@InjectRepository(User) | |
private userRepository: Repository<User>, | |
private readonly mailerService: MailerService, | |
private configService: ConfigService, | |
) {} | |
constructor( | |
private readonly mailerService: MailerService, | |
private configService: ConfigService, | |
) {} |
import { Injectable } from '@nestjs/common'; | ||
import { MailerService } from '@nestjs-modules/mailer'; | ||
import { InjectRepository } from '@nestjs/typeorm'; | ||
import { User } from 'src/user/user.model'; | ||
import { Repository } from 'typeorm'; | ||
import { ConfigService } from '@nestjs/config'; | ||
|
||
@Injectable() | ||
export class MailService { | ||
constructor( | ||
@InjectRepository(User) | ||
private userRepository: Repository<User>, | ||
private readonly mailerService: MailerService, | ||
private configService: ConfigService, | ||
) {} | ||
|
||
async sendConfirmationEmail(email: string, token: string) { | ||
const confirmUrl = `https://${this.configService.get('MAIL_DOMAIN')}/auth/confirm?token=${token}`; | ||
|
||
await this.mailerService.sendMail({ | ||
to: email, | ||
subject: 'Confirm Your Email', | ||
template: './confirmation', | ||
context: { confirmUrl }, // Data for template | ||
}); | ||
} | ||
|
||
async sendPasswordResetEmail(user: User, token: string) { | ||
const frontendUrl = this.configService.get('FRONTEND_URL'); | ||
const url = `${frontendUrl}/reset-password?token=${token}`; | ||
|
||
await this.mailerService.sendMail({ | ||
to: user.email, | ||
subject: 'Password Reset Request', | ||
template: './passwordReset', | ||
context: { | ||
name: user.username, | ||
firstName: user.username, | ||
url, | ||
}, | ||
}); | ||
} | ||
|
||
// async sendConfirmationEmail(user: User, token: string) { | ||
// const frontendUrl = this.configService.get('FRONTEND_URL'); | ||
// const url = `${frontendUrl}/confirm-email?token=${token}`; | ||
|
||
// await this.mailerService.sendMail({ | ||
// to: user.email, | ||
// subject: 'Welcome! Confirm Your Email', | ||
// template: './confirmation', // This will use the confirmation.hbs template | ||
// context: { // Data to be sent to the template | ||
// name: user.username, | ||
// firstName: user.firstName || user.username, | ||
// 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.
🛠️ Refactor suggestion
Add error handling for email sending.
The methods don't have error handling for cases where email sending fails. Add try/catch blocks to handle potential errors from the mailer service.
async sendConfirmationEmail(email: string, token: string) {
const confirmUrl = `https://${this.configService.get('MAIL_DOMAIN')}/auth/confirm?token=${token}`;
+ try {
await this.mailerService.sendMail({
to: email,
subject: 'Confirm Your Email',
template: './confirmation',
context: { confirmUrl }, // Data for template
});
+ return true;
+ } catch (error) {
+ console.error(`Failed to send confirmation email to ${email}:`, error);
+ throw new Error('Failed to send confirmation email. Please try again later.');
+ }
}
📝 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.
import { Injectable } from '@nestjs/common'; | |
import { MailerService } from '@nestjs-modules/mailer'; | |
import { InjectRepository } from '@nestjs/typeorm'; | |
import { User } from 'src/user/user.model'; | |
import { Repository } from 'typeorm'; | |
import { ConfigService } from '@nestjs/config'; | |
@Injectable() | |
export class MailService { | |
constructor( | |
@InjectRepository(User) | |
private userRepository: Repository<User>, | |
private readonly mailerService: MailerService, | |
private configService: ConfigService, | |
) {} | |
async sendConfirmationEmail(email: string, token: string) { | |
const confirmUrl = `https://${this.configService.get('MAIL_DOMAIN')}/auth/confirm?token=${token}`; | |
await this.mailerService.sendMail({ | |
to: email, | |
subject: 'Confirm Your Email', | |
template: './confirmation', | |
context: { confirmUrl }, // Data for template | |
}); | |
} | |
async sendPasswordResetEmail(user: User, token: string) { | |
const frontendUrl = this.configService.get('FRONTEND_URL'); | |
const url = `${frontendUrl}/reset-password?token=${token}`; | |
await this.mailerService.sendMail({ | |
to: user.email, | |
subject: 'Password Reset Request', | |
template: './passwordReset', | |
context: { | |
name: user.username, | |
firstName: user.username, | |
url, | |
}, | |
}); | |
} | |
// async sendConfirmationEmail(user: User, token: string) { | |
// const frontendUrl = this.configService.get('FRONTEND_URL'); | |
// const url = `${frontendUrl}/confirm-email?token=${token}`; | |
// await this.mailerService.sendMail({ | |
// to: user.email, | |
// subject: 'Welcome! Confirm Your Email', | |
// template: './confirmation', // This will use the confirmation.hbs template | |
// context: { // Data to be sent to the template | |
// name: user.username, | |
// firstName: user.firstName || user.username, | |
// url, | |
// }, | |
// }); | |
// } | |
} | |
import { Injectable } from '@nestjs/common'; | |
import { MailerService } from '@nestjs-modules/mailer'; | |
import { InjectRepository } from '@nestjs/typeorm'; | |
import { User } from 'src/user/user.model'; | |
import { Repository } from 'typeorm'; | |
import { ConfigService } from '@nestjs/config'; | |
@Injectable() | |
export class MailService { | |
constructor( | |
@InjectRepository(User) | |
private userRepository: Repository<User>, | |
private readonly mailerService: MailerService, | |
private configService: ConfigService, | |
) {} | |
async sendConfirmationEmail(email: string, token: string) { | |
const confirmUrl = `https://${this.configService.get('MAIL_DOMAIN')}/auth/confirm?token=${token}`; | |
try { | |
await this.mailerService.sendMail({ | |
to: email, | |
subject: 'Confirm Your Email', | |
template: './confirmation', | |
context: { confirmUrl }, // Data for template | |
}); | |
return true; | |
} catch (error) { | |
console.error(`Failed to send confirmation email to ${email}:`, error); | |
throw new Error('Failed to send confirmation email. Please try again later.'); | |
} | |
} | |
async sendPasswordResetEmail(user: User, token: string) { | |
const frontendUrl = this.configService.get('FRONTEND_URL'); | |
const url = `${frontendUrl}/reset-password?token=${token}`; | |
await this.mailerService.sendMail({ | |
to: user.email, | |
subject: 'Password Reset Request', | |
template: './passwordReset', | |
context: { | |
name: user.username, | |
firstName: user.username, | |
url, | |
}, | |
}); | |
} | |
// async sendConfirmationEmail(user: User, token: string) { | |
// const frontendUrl = this.configService.get('FRONTEND_URL'); | |
// const url = `${frontendUrl}/confirm-email?token=${token}`; | |
// | |
// await this.mailerService.sendMail({ | |
// to: user.email, | |
// subject: 'Welcome! Confirm Your Email', | |
// template: './confirmation', // This will use the confirmation.hbs template | |
// context: { // Data to be sent to the template | |
// name: user.username, | |
// firstName: user.firstName || user.username, | |
// url, | |
// }, | |
// }); | |
// } | |
} |
backend/src/user/user.resolver.ts
Outdated
@Mutation(() => RegisterResponse) | ||
async registerUser( | ||
@Args('input') registerUserInput: RegisterUserInput, | ||
): Promise<User> { | ||
): Promise<RegisterResponse> { | ||
return this.authService.register(registerUserInput); | ||
} |
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.
💡 Verification agent
🧩 Analysis chain
Updated registerUser return type.
The method signature now correctly returns Promise to match the new response type. The implementation now needs to return both a user and a message.
🏁 Script executed:
#!/bin/bash
# Check if the AuthService.register method has been updated to return RegisterResponse
# Look for the register method in auth.service.ts
echo "Checking AuthService.register implementation..."
rg -A 10 "register.*RegisterUserInput" --type ts
Length of output: 5624
Updated registerUser response structure requires further completion.
The mutation definition in backend/src/user/user.resolver.ts
now correctly returns a Promise<RegisterResponse>
, which aligns with the new response type. However, the underlying implementation in AuthService.register
(defined in backend/src/auth/auth.service.ts
) currently does not return an object containing both the user and a message, as required by the updated RegisterResponse
specification. Please update the implementation accordingly so that it returns both a user and a descriptive message.
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.
Actionable comments posted: 2
🧹 Nitpick comments (4)
backend/src/user/user.resolver.ts (1)
38-44
: Remove the commented-out code if it's no longer needed.
Keeping commented-out code can lead to confusion. If the oldResendConfirmationInput
logic is not needed for reference anymore, it would be cleaner to remove it.- // @Mutation(() => EmailConfirmationResponse) - // ... - // }backend/src/auth/auth.service.ts (3)
45-82
: Refine error handling and avoid unused variable.
- The catch block’s
error
variable is never used, triggering a static analysis warning. Consider removing the parameter from thecatch
or logging it.- For the message
"Email already confirmed or user not found."
, consider returning separate messages for these distinct cases to improve clarity.- } catch (error) { + } catch { return { message: 'Invalid or expired token', success: false, }; }🧰 Tools
🪛 GitHub Check: autofix
[warning] 76-76:
'error' is defined but never used
157-159
: Handle potential errors when sending the verification email.
Currently, ifsendVerificationEmail
fails, the user is created but never gets a confirmation email. Consider wrapping it in a try-catch or returning partial success to inform the user.Also applies to: 160-160
174-176
: Prefer NestJS exceptions over generic Errors.
Throwing anError
here deviates from the usual NestJS exception handling. UseUnauthorizedException
or a more relevant NestJS exception to keep error handling consistent.-throw new Error('Email not confirmed. Please check your inbox.'); +throw new UnauthorizedException('Email not confirmed. Please check your inbox.');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/src/auth/auth.service.ts
(6 hunks)backend/src/user/user.resolver.ts
(2 hunks)
🧰 Additional context used
🪛 GitHub Check: autofix
backend/src/auth/auth.service.ts
[warning] 76-76:
'error' is defined but never used
🔇 Additional comments (3)
backend/src/user/user.resolver.ts (1)
19-20
: Looks good!
The new imports forEmailConfirmationResponse
andResendEmailInput
are consistent with the enhanced email confirmation structure.backend/src/auth/auth.service.ts (2)
22-26
: New imports and dependencies are fitting.
The added references toEmailConfirmationResponse
,RefreshTokenResponse
, andMailService
align with the new features.
36-36
: Injection of MailService is correct.
This injection follows NestJS best practices for consuming the mail service.
@Mutation(() => EmailConfirmationResponse) | ||
async resendConfirmationEmail( | ||
@Args('input') input: ResendEmailInput, | ||
): Promise<EmailConfirmationResponse> { | ||
return this.authService.resendVerificationEmail(input.email); | ||
} |
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.
Ensure consistent return shape from auth service.
resendVerificationEmail
may return objects without a success
property (for instance, the “Email already confirmed!” response). This might break the GraphQL response scheme of EmailConfirmationResponse
. Consider adjusting that method to always return { message: string; success: boolean; }
or throw a suitable exception.
async resendVerificationEmail(email: string) { | ||
const user = await this.userRepository.findOne({ | ||
where: { email }, | ||
}); | ||
|
||
if (!user) { | ||
throw new Error('User not found'); | ||
} | ||
|
||
if (user.isEmailConfirmed) { | ||
return { message: 'Email already confirmed!' }; | ||
} | ||
|
||
// Check if a cooldown period has passed (e.g., 1 minute) | ||
const cooldownPeriod = 1 * 60 * 1000; // 1 minute in milliseconds | ||
if ( | ||
user.lastEmailSendTime && | ||
new Date().getTime() - user.lastEmailSendTime.getTime() < cooldownPeriod | ||
) { | ||
const timeLeft = Math.ceil( | ||
(cooldownPeriod - | ||
(new Date().getTime() - user.lastEmailSendTime.getTime())) / | ||
1000, | ||
); | ||
return { | ||
message: `Please wait ${timeLeft} seconds before requesting another email`, | ||
success: false, | ||
}; | ||
} | ||
|
||
return this.sendVerificationEmail(user); | ||
} |
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.
Harmonize return values with the expected GraphQL type.
In resendVerificationEmail
, the block that returns { message: 'Email already confirmed!' }
lacks a success
field while other branches return the success
property. This can break the EmailConfirmationResponse
shape. Also, consider throwing a NestJS exception instead of a general Error for "User not found."
if (user.isEmailConfirmed) {
- return { message: 'Email already confirmed!' };
+ return { message: 'Email already confirmed!', success: true };
}
if (!user) {
- throw new Error('User not found');
+ throw new NotFoundException('User not found');
}
Committable suggestion skipped: line range outside the PR's diff.
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.
Actionable comments posted: 0
🧹 Nitpick comments (5)
frontend/src/app/auth/confirm/page.tsx (2)
24-44
: Robust Mutation Handling for ConfirmationUsing Apollo’s
onCompleted
andonError
callbacks here provides a clear success/error flow. Consider distinguishing more specific errors (e.g., expired token vs. already-verified email) if the backend surfaces them, so that users receive more actionable feedback.
63-102
: Clear UI States for Verification FlowThe conditional rendering of the loader, success icon, and error state is intuitive for end users. The timed redirect is helpful, but consider a user-driven approach (e.g., a “Go to Home Now” button) in case they want to stay on the page longer to read messages.
frontend/src/components/sign-up-modal.tsx (3)
42-44
: Good Use of Registration State TrackersStoring
registrationSuccess
,resendCooldown
, andresendMessage
in local state is a simple approach. Keep in mind that if additional flows grow more complex, you might consider a global store or a dedicated hook.
83-103
: Concise Resend Confirmation Email FlowThe mutation’s success and error handlers are straightforward, setting helpful messages in state. Ensure server-side triggers are in place to detect repeated resend attempts within a short period (brute force/spam control).
105-119
: Cooldown Timer LogicThis timer-based approach efficiently handles the resend cooldown. To avoid potential runtime errors, consider using optional chaining when checking
resendMessage.includes(...)
:- if (resendMessage && resendMessage.includes('has been resent')) { + if (resendMessage?.includes('has been resent')) { setResendMessage(null); }🧰 Tools
🪛 Biome (1.9.4)
[error] 113-113: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
frontend/src/app/auth/confirm/page.tsx
(1 hunks)frontend/src/components/sign-up-modal.tsx
(5 hunks)frontend/src/graphql/mutations/auth.ts
(2 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
frontend/src/components/sign-up-modal.tsx
[error] 113-113: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (5)
frontend/src/graphql/mutations/auth.ts (2)
31-38
: Nicely Structured Confirm Email MutationDefining a dedicated mutation for confirming email tokens is a clean approach. Ensure the backend throws clear, secure errors for invalid or expired tokens so that front-end error handling remains straightforward.
40-47
: Well-Defined Resend Confirmation Email MutationThis mutation is straightforward and aligns well with the confirm email flow. As a next step, verify that rate limiting or similar spam-prevention measures are in place on the server side to mitigate excessive requests.
frontend/src/app/auth/confirm/page.tsx (1)
46-61
: Token Check for ConfirmationGracefully handling the absence of a token is a smart UX approach. If your backend or route structure might change (e.g., handling token in the path instead of search params), ensure this logic remains consistent across both front-end and server.
frontend/src/components/sign-up-modal.tsx (2)
21-24
: Relevant Mutation ImportsImporting both
REGISTER_USER
andRESEND_CONFIRMATION_EMAIL_MUTATION
in a single file is tidy. Just be sure to maintain consistent naming if more mutations expand over time.
145-282
: Well-Structured Verification Success UIPresenting a clear prompt to check one's email, along with a dedicated resend button, significantly enhances user experience. The feedback message blocks and icon usage (mail, checkmark, clock) provide clarity. This flow is an all-around solid addition.
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.
Actionable comments posted: 0
♻️ Duplicate comments (2)
backend/src/auth/auth.service.ts (1)
111-122
:⚠️ Potential issueFix inconsistent return type structure.
In the
resendVerificationEmail
method, the return object when the email is already confirmed is missing thesuccess
field, which is inconsistent with other return values and will break type safety with theEmailConfirmationResponse
type. Also, using a customError
instead of NestJS exception is inconsistent with the codebase style.if (user.isEmailConfirmed) { - return { message: 'Email already confirmed!' }; + return { message: 'Email already confirmed!', success: true }; } if (!user) { - throw new Error('User not found'); + throw new NotFoundException('User not found'); }backend/.env (1)
12-17
:⚠️ Potential issueReplace placeholder email credentials with actual values.
The current mail configuration contains example values that must be replaced with actual credentials before deployment.
Issues to address:
- Replace
smtp.example.com
,user@example.com
, etc. with actual mail server details- Ensure
MAIL_PASSWORD
is properly secured and not committed to version control- Update
MAIL_DOMAIN
to match your actual domain (appears to be "codefox" based on the email template)This is critical for the email service to function correctly.
🧹 Nitpick comments (3)
backend/src/auth/auth.service.ts (2)
32-50
: Consider using built-in configuration validation.The constructor is well-organized with the addition of the
mailService
and initialization of theisMailEnabled
flag. However, you could leverage NestJS's configuration validation for type safety.Consider using NestJS's built-in configuration validation:
- // Read the MAIL_ENABLED environment variable, default to 'true' - this.isMailEnabled = - this.configService.get<string>('MAIL_ENABLED', 'true').toLowerCase() === - 'true'; + this.isMailEnabled = this.configService.get<boolean>('MAIL_ENABLED', true);This would require configuring your config validation schema in your app configuration, but provides better type safety.
195-197
: Good security measure requiring email confirmation.Adding a check to prevent login for users who haven't confirmed their email is a good security practice. However, consider using a proper NestJS exception with an appropriate status code.
if (!user.isEmailConfirmed) { - throw new Error('Email not confirmed. Please check your inbox.'); + throw new UnauthorizedException('Email not confirmed. Please check your inbox.'); }backend/.env (1)
8-17
: Consider environment-specific configuration files.For better security and configuration management, consider using separate environment files (.env.development, .env.production, etc.) or a secrets management solution rather than hardcoding sensitive values in the .env file.
You could use:
- Different .env files per environment (.env.development, .env.production)
- Environment-specific variables in deployment pipelines
- A secrets management solution (AWS Secrets Manager, HashiCorp Vault, etc.)
This would help prevent accidental exposure of production credentials.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/.env
(1 hunks)backend/src/auth/auth.service.ts
(4 hunks)
🔇 Additional comments (8)
backend/src/auth/auth.service.ts (7)
22-26
: Good import organization.The imports are well organized with the new types added for email confirmation functionality.
30-31
: LGTM: Class property addition for email flag.Adding a property to track mail status is a good approach.
52-89
: Good implementation of token verification and email confirmation.The
confirmEmail
method is well-structured with proper error handling and JWT verification. The flow for checking and updating the user's email confirmation status is clear and well-organized.
91-109
: Appropriate email verification process with token generation.The implementation for generating and sending verification emails is sound. Good practice to update the user's
lastEmailSendTime
for rate limiting purposes.
124-142
: Well implemented rate limiting for email resending.The cooldown period implementation is a good security practice to prevent abuse. The time calculation for the cooldown message is also a nice user experience touch.
158-173
: Good conditional user creation based on email configuration.The implementation properly handles creating users with the appropriate
isEmailConfirmed
status based on whether email verification is enabled.
175-181
:❓ Verification inconclusive
Verify email flow in the frontend.
Good implementation of sending verification email after user creation. Make sure the frontend is updated to handle the email verification flow by notifying users to check their email after registration.
🏁 Script executed:
#!/bin/bash # Find frontend code handling registration to check if it's ready for email verification flow # Look for registration components that might need updating echo "Checking frontend registration components:" rg -A 5 "register|signup|registration" --type ts --type tsx | grep -v 'test\|spec' | head -20 echo "Looking for email verification handling in frontend:" rg -A 5 "email.*(verif|confirm)" --type ts --type tsx | grep -v 'test\|spec' | head -20 echo "Checking for components that might need to display email verification messages:" rg -A 3 "alert|notification|message" --type ts --type tsx | head -15Length of output: 719
Action Required: Verify Frontend Email Verification Flow & Update rg File Type Handling
The backend implementation is solid for sending verification emails. However, our automated search script couldn’t check the frontend because of an issue with the file-type parameter for TSX files. Specifically, the
rg
command returned “unrecognized file type: tsx.” Please ensure:
- The frontend registration flow is updated to prompt users to check their email after sign-up.
- If using automated searches, adjust the command (e.g., define TSX as a custom file type with
--type-add 'tsx:*.tsx'
or remove the TSX filter) so the email verification handling code can be accurately identified.backend/.env (1)
8-10
: Good mail configuration toggle.Adding the
MAIL_ENABLED
flag is a great addition that allows for disabling the email functionality in development or testing environments.
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
backend/.env.example (1)
28-34
: SMTP Email Configuration VariablesThe SMTP-related environment variables (
MAIL_HOST
,MAIL_USER
,MAIL_PASSWORD
,MAIL_FROM
,MAIL_PORT
, andMAIL_DOMAIN
) have been added appropriately. A few suggestions:
- Consider updating
MAIL_DOMAIN
from the placeholder valueyour_net
to a value likeexample.com
or providing a brief inline comment to clarify its expected format.- Ensure that the example credentials are clearly marked as placeholders so developers know to replace them with production values when necessary.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
backend/.env.example
(1 hunks)backend/package.json
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/package.json
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Install and Build Backend
- GitHub Check: autofix
- GitHub Check: Install and Build Frontend
🔇 Additional comments (1)
backend/.env.example (1)
24-27
: Email Configuration Flag AddedThe new mail configuration flag (
MAIL_ENABLED
) is clearly defined and well-commented. This flag will allow users to easily enable or disable email functionality depending on the environment.
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.
LGTM
…fication (#153) This creates backend email service support. 1, send verification email 2, confirm token for User email 3, resend email 4, template for verification email Frontend: 1, register to tell user check email 2, after user click confirm email link. Frontend should send request to backend to do check Vedio: https://jam.dev/c/1e63139a-e24f-4baf-968c-a9d2e771fef0 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Introduced a robust email verification workflow that requires users to confirm their email addresses upon registration. - Added the ability to resend confirmation emails and a dedicated confirmation page that provides real-time success or error feedback. - **Chores** - Expanded configuration options to support customizable email settings via environment variables. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
This creates backend email service support.
1, send verification email
2, confirm token for User email
3, resend email
4, template for verification email
Frontend:
1, register to tell user check email
2, after user click confirm email link. Frontend should send request to backend to do check
Vedio:
https://jam.dev/c/1e63139a-e24f-4baf-968c-a9d2e771fef0
Summary by CodeRabbit