Skip to content

Conversation

@hib4
Copy link
Member

@hib4 hib4 commented May 17, 2025

PR Type

Enhancement


Description

  • Implement password reset request endpoint

  • Configure nodemailer email transporter

  • Exempt reset routes from authentication check

  • Update CORS, lint, and dependencies


Changes walkthrough 📝

Relevant files
Enhancement
2 files
auth_controller.ts
Add password reset email logic                                                     
+215/-0 
auth.ts
Add request-reset API endpoint                                                     
+16/-4   
Configuration changes
3 files
auth_middleware.ts
Exempt reset routes from auth middleware                                 
+19/-11 
csrf_middleware.ts
Exempt reset routes from CSRF protection                                 
+2/-1     
server.ts
Allow additional CORS origin                                                         
+1/-0     
Formatting
1 files
.eslintrc.js
Disable quotes linting rule                                                           
+1/-1     
Dependencies
1 files
package.json
Add nodemailer dependency                                                               
+2/-0     

Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @hib4 hib4 self-assigned this May 17, 2025
    @github-actions
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Env Vars

    Ensure MAILTRAP_USER and MAILTRAP_PASS environment variables are set and validated before attempting to send emails, otherwise the transporter may be misconfigured or fail silently.

    // Configure Nodemailer to use Mailtrap's SMTP
    const transporter = nodemailer.createTransport({
      host: "live.smtp.mailtrap.io",
      port: 587,
      auth: {
        user: process.env.MAILTRAP_USER,
        pass: process.env.MAILTRAP_PASS,
      },
    Route Consistency

    The middleware exempts /auth/reset-password but no such route is registered in auth.ts. Remove the exemption or add the corresponding endpoint implementation to keep routing and middleware in sync.

      "/auth/session-login",
      "/auth/request-reset",
      "/auth/reset-password",
    ];

    @github-actions
    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Parameterize SMTP configuration

    Parameterize the SMTP host and port using environment variables so you can switch
    providers without changing code. This also allows you to secure credentials and
    adapt for production. Provide sensible defaults for development.

    functions/src/controllers/auth_controller.ts [400-407]

     const transporter = nodemailer.createTransport({
    -  host: "live.smtp.mailtrap.io",
    -  port: 587,
    +  host: process.env.SMTP_HOST || "live.smtp.mailtrap.io",
    +  port: Number(process.env.SMTP_PORT) || 587,
       auth: {
         user: process.env.MAILTRAP_USER,
         pass: process.env.MAILTRAP_PASS,
       },
     });
    Suggestion importance[1-10]: 6

    __

    Why: Parameterizing host and port via env vars improves configurability across environments without changing code.

    Low
    Normalize frontend URL

    Normalize the frontend URL by trimming any trailing slash to avoid double slashes in
    your reset link. This ensures consistent link construction regardless of how the env
    var is set.

    functions/src/controllers/auth_controller.ts [422-427]

    -const getActionCodeSettings = (): ActionCodeSettings => ({
    -  url: process.env.FRONTEND_URL
    -    ? `${process.env.FRONTEND_URL}/reset-password`
    -    : "https://portal.garudahacks.com/reset-password",
    -  handleCodeInApp: true,
    -});
    +const getActionCodeSettings = (): ActionCodeSettings => {
    +  const baseUrl = (process.env.FRONTEND_URL || "https://portal.garudahacks.com")
    +    .replace(/\/+$/, "");
    +  return {
    +    url: `${baseUrl}/reset-password`,
    +    handleCodeInApp: true,
    +  };
    +};
    Suggestion importance[1-10]: 6

    __

    Why: Trimming trailing slashes on process.env.FRONTEND_URL prevents double slashes in reset links and ensures consistency.

    Low
    Add email send error handling

    Wrap the sendMail call in its own try/catch to log transporter-specific errors
    separately and avoid unhandled rejections. This makes debugging email failures
    easier.

    functions/src/controllers/auth_controller.ts [491-498]

     const sendPasswordResetEmail = async (
       email: string,
       link: string
     ): Promise<void> => {
       const mailOptions = createMailOptions(email, link);
    -  await transporter.sendMail(mailOptions);
    -  functions.logger.info("Password reset email sent successfully to:", email);
    +  try {
    +    await transporter.sendMail(mailOptions);
    +    functions.logger.info(`Password reset email sent to ${email}`);
    +  } catch (err) {
    +    functions.logger.error("Failed to send password reset email:", err);
    +    throw err;
    +  }
     };
    Suggestion importance[1-10]: 6

    __

    Why: Wrapping transporter.sendMail in try/catch captures SMTP errors explicitly and improves debugging of email failures.

    Low

    @hib4 hib4 merged commit 27f9993 into dev May 17, 2025
    1 check passed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    2 participants