feat: added support for the SMTP email provider. #1535
Conversation
WalkthroughAdds explicit SMTP support and wiring: new SMTP helper using Nodemailer, email service updated to call SMTP and stop defaulting to SendGrid, Resend client instantiation made conditional on provider, constants updated to explicit provider names, and runtime provider validation/logging added to service bootstrap. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant EmailSvc as Email Service
participant SMTP as SMTP Helper
participant Nodemailer as Nodemailer Transporter
participant Resend as Resend Helper
participant ResendClient as Resend Client
participant SendGrid as SendGrid Helper
App->>EmailSvc: sendEmail(emailDto)
EmailSvc->>EmailSvc: resolve EMAIL_PROVIDER (CommonConstants)
alt provider is SMTP
EmailSvc->>SMTP: sendWithSMTP(emailDto)
SMTP->>Nodemailer: transporter.sendMail(...)
Nodemailer-->>SMTP: success / error
SMTP-->>EmailSvc: boolean result
else provider is Resend
EmailSvc->>Resend: sendWithResend(emailDto)
Resend->>ResendClient: API send
ResendClient-->>Resend: response
Resend-->>EmailSvc: boolean result
else provider is SendGrid
EmailSvc->>SendGrid: sendWithSendGrid(emailDto)
SendGrid-->>EmailSvc: response
else unknown provider
EmailSvc-->>App: log warning, return false
end
EmailSvc-->>App: delivery status (boolean)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/common/src/resend-helper-file.ts (1)
21-36: Add null check for resend client before use.The
sendWithResendfunction callsresend.emails.send()without verifying thatresendis initialized. Sinceresendis now conditionally initialized (only whenEMAIL_PROVIDERis 'resend'), calling this function when the provider is misconfigured or when called directly outside the normal flow will result in a TypeError.🔎 Proposed fix to add defensive null check
export const sendWithResend = async (emailDto: EmailDto): Promise<boolean> => { + if (!resend) { + Logger.error('Resend email provider is not initialized'); + return false; + } + try { const response = await resend.emails.send({
🧹 Nitpick comments (1)
libs/common/src/smtp-helper-file.ts (1)
38-46: Unescaped interpolation of user-controlled data in email HTML templates.The HTML templates interpolate user-controlled values like
orgNameandfirstNamedirectly into HTML without escaping HTML entities. While email clients have limited script execution capabilities, apply HTML entity encoding (e.g., using a utility to escape<,>,",&) to these values before template insertion to follow HTML safety best practices.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
libs/common/src/email.service.tslibs/common/src/resend-helper-file.tslibs/common/src/smtp-helper-file.ts
🧰 Additional context used
🧬 Code graph analysis (2)
libs/common/src/smtp-helper-file.ts (2)
libs/common/src/dtos/email.dto.ts (1)
EmailDto(1-8)libs/logger/src/logger.interface.ts (1)
Logger(6-20)
libs/common/src/email.service.ts (1)
libs/common/src/smtp-helper-file.ts (1)
sendWithSMTP(32-53)
🪛 GitHub Check: CodeQL
libs/common/src/smtp-helper-file.ts
[failure] 44-44: Client-side cross-site scripting
HTML injection vulnerability due to user-provided value.
🔇 Additional comments (2)
libs/common/src/email.service.ts (1)
6-6: LGTM! Clean integration of SMTP provider.The SMTP provider integration follows the established pattern and is consistent with existing provider implementations.
Also applies to: 27-29
libs/common/src/smtp-helper-file.ts (1)
1-53: Well-structured SMTP implementation.The overall implementation follows best practices:
- Environment-based configuration
- Proper error handling and logging
- Conditional initialization matching the pattern used in other provider helpers
- Standard nodemailer configuration with appropriate TLS settings
Signed-off-by: pranalidhanavade <pranali.dhanavade@ayanworks.com>
Signed-off-by: pranalidhanavade <pranali.dhanavade@ayanworks.com>
pranalidhanavade
force-pushed
the
feat/support-smtp-provider
branch
from
59f1ef5 to
fad64b1
Compare
pranalidhanavade
changed the title
Signed-off-by: pranalidhanavade <pranali.dhanavade@ayanworks.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/common/src/resend-helper-file.ts (1)
21-36: Add null check to prevent runtime error.The
resendclient is conditionally initialized (Line 12-19) and can benullifEMAIL_PROVIDERis not'resend'. However,sendWithResendattempts to callresend.emails.send()at Line 23 without checking ifresendis null, which will cause a TypeError at runtime.🔎 Proposed fix
export const sendWithResend = async (emailDto: EmailDto): Promise<boolean> => { + if (!resend) { + Logger.error('Resend email provider is not initialized'); + return false; + } + try { const response = await resend.emails.send({ from: emailDto.emailFrom,
🧹 Nitpick comments (1)
apps/user/src/main.ts (1)
39-39: Use consistent camelCase variable naming.The variable
Emailprovidershould follow camelCase convention asemailProvider.🔎 Proposed fix
- const Emailprovider = provider as EmailProvider; - logger.log(`Email provider configured: ${Emailprovider}`); + const emailProvider = provider as EmailProvider; + logger.log(`Email provider configured: ${emailProvider}`);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/user/src/main.tslibs/common/src/common.constant.tslibs/common/src/email.service.tslibs/common/src/resend-helper-file.tslibs/common/src/smtp-helper-file.ts
🧰 Additional context used
🧬 Code graph analysis (3)
apps/user/src/main.ts (1)
libs/logger/src/logger.interface.ts (1)
Logger(6-20)
libs/common/src/email.service.ts (4)
libs/common/src/dtos/email.dto.ts (1)
EmailDto(1-8)libs/common/src/send-grid-helper-file.ts (1)
sendWithSendGrid(10-27)libs/common/src/resend-helper-file.ts (1)
sendWithResend(21-37)libs/common/src/smtp-helper-file.ts (1)
sendWithSMTP(32-53)
libs/common/src/smtp-helper-file.ts (2)
libs/common/src/dtos/email.dto.ts (1)
EmailDto(1-8)libs/logger/src/logger.interface.ts (1)
Logger(6-20)
🪛 Biome (2.1.2)
libs/common/src/email.service.ts
[error] 24-24: Duplicate case label.
The first similar label is here:
(lint/suspicious/noDuplicateCase)
🪛 GitHub Check: CodeQL
libs/common/src/smtp-helper-file.ts
[failure] 44-44: Client-side cross-site scripting
HTML injection vulnerability due to user-provided value.
🔇 Additional comments (3)
libs/common/src/common.constant.ts (1)
308-310: LGTM!The explicit email provider constants are well-defined and replace the previous default approach, enabling clear provider selection.
libs/common/src/smtp-helper-file.ts (2)
13-30: Verify the SMTP port validation concern from previous review.A previous review comment (still unresolved) flagged that Line 22 converts
SMTP_PORTto a number without validation. If the environment variable contains a non-numeric value,Number(SMTP_PORT)will returnNaN, which could lead to unexpected transporter configuration.Please address the port validation suggestion from the previous review to ensure robust configuration.
Based on the previous review comment.
32-53: LGTM!The
sendWithSMTPfunction correctly handles the null transporter case and properly maps theEmailDtofields to the nodemailersendMailoptions. Error handling with logging is appropriate.
Signed-off-by: pranalidhanavade <pranali.dhanavade@ayanworks.com>
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
libs/common/src/smtp-helper-file.ts (2)
26-35: Use the validatedportvariable consistently.The
portvariable is computed and validated on line 20, but thenNumber(SMTP_PORT)is called again on lines 28, 29, and 34. This is inefficient and inconsistent. Use the already-validatedportvariable instead.🔎 Proposed fix to use validated port variable
transporter = nodemailer.createTransport({ host: SMTP_HOST, - port: Number(SMTP_PORT), - secure: 465 === Number(SMTP_PORT), + port, + secure: 465 === port, auth: { user: SMTP_USER, pass: SMTP_PASS }, - requireTLS: 587 === Number(SMTP_PORT) + requireTLS: 587 === port });
20-24: Add upper bound validation for SMTP_PORT.The current validation checks that the port is an integer greater than 0, but doesn't verify it's within the valid port range (1-65535). This could allow invalid port numbers that would fail at connection time.
🔎 Proposed fix to validate port range
const port = Number(SMTP_PORT); - if (!Number.isInteger(port) || 0 >= port) { + if (!Number.isInteger(port) || port <= 0 || port > 65535) { throw new Error(`Invalid SMTP_PORT value: "${SMTP_PORT}". Must be a valid number.`); }Or with a more descriptive error message:
const port = Number(SMTP_PORT); - if (!Number.isInteger(port) || 0 >= port) { - throw new Error(`Invalid SMTP_PORT value: "${SMTP_PORT}". Must be a valid number.`); + if (!Number.isInteger(port) || port <= 0 || port > 65535) { + throw new Error(`Invalid SMTP_PORT value: "${SMTP_PORT}". Must be a valid port number (1-65535).`); }
🧹 Nitpick comments (1)
libs/common/src/common.utils.ts (1)
149-156: HTML escaping implementation is correct.The function properly escapes HTML special characters in the correct order (ampersand first to prevent double-escaping). This effectively mitigates HTML injection risks when user-provided values are embedded in email templates.
Optional: Add defensive null/undefined handling
For additional robustness, consider adding a guard clause:
export const escapeHtml = (value: string): string => + !value ? '' : value .replace(/&/g, '&') .replace(/</g, '<') .replace(/>/g, '>') .replace(/"/g, '"') .replace(/'/g, ''') .replace(/\//g, '/');This prevents runtime errors if the function is called with null or undefined values.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/organization/templates/organization-invitation.template.tslibs/common/src/common.utils.tslibs/common/src/smtp-helper-file.ts
🧰 Additional context used
🧬 Code graph analysis (2)
apps/organization/templates/organization-invitation.template.ts (1)
libs/common/src/common.utils.ts (1)
escapeHtml(149-156)
libs/common/src/smtp-helper-file.ts (2)
libs/common/src/dtos/email.dto.ts (1)
EmailDto(1-8)libs/logger/src/logger.interface.ts (1)
Logger(6-20)
🔇 Additional comments (3)
apps/organization/templates/organization-invitation.template.ts (2)
1-1: LGTM!The import is correctly placed and the
escapeHtmlutility will be used to sanitize user-provided values in the template.
22-24: Excellent: HTML injection vulnerability properly mitigated.All user-provided values (
orgName,firstName) are correctly sanitized usingescapeHtmlbefore being interpolated into the HTML template. This effectively prevents HTML/XSS injection attacks through organization invitation emails.The sanitization covers all insertion points:
- Line 43:
safeEmailin the greeting- Line 46:
safeFirstNameandsafeOrgNamein the invitation message- Line 49:
safeOrgNamein the organization details listAlso applies to: 43-43, 46-46, 49-49
libs/common/src/smtp-helper-file.ts (1)
38-59: SMTP email sending implementation looks solid.The function properly:
- Validates transporter initialization before attempting to send
- Maps EmailDto fields to nodemailer's sendMail options
- Handles errors gracefully with logging
- Returns boolean to indicate success/failure
The implementation integrates well with the broader email provider switching logic.
What
Summary by CodeRabbit
New Features
Bug Fixes
Behavior
✏️ Tip: You can customize this high-level summary in your review settings.