Skip to content

Conversation

@hib4
Copy link
Member

@hib4 hib4 commented May 23, 2025

PR Type

Enhancement


Description

  • Configure email transporter and MailOptions interfaces

  • Add password reset and verification email builders

  • Send verification link on register and update response

  • Expose emailVerified and add verifyAccount endpoint


Changes walkthrough 📝

Relevant files
Enhancement
auth_controller.ts
Enhance auth controller email workflows                                   

functions/src/controllers/auth_controller.ts

  • Configure Nodemailer transporter and MailOptions interface
  • Implement password reset & verification email creators
  • Send verification link during registration, update message
  • Add verifyAccount controller and include emailVerified
  • +205/-154
    auth.ts
    Add verifyAccount route                                                                   

    functions/src/routes/auth.ts

    • Register new /verify-account POST route
    +4/-0     
    Configuration changes
    auth_middleware.ts
    Update auth middleware exemptions                                               

    functions/src/middlewares/auth_middleware.ts

    • Remove /auth/request-reset from auth-exempt routes
    +0/-1     
    csrf_middleware.ts
    Update CSRF middleware exemptions                                               

    functions/src/middlewares/csrf_middleware.ts

  • Remove /auth/request-reset from CSRF exemptions
  • Add /auth/verify-account to CSRF exemptions
  • +1/-1     

    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 23, 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

    Missing Link Configuration

    Removing ActionCodeSettings may generate reset links pointing to default Firebase endpoints instead of the frontend URL, breaking the password reset flow

    const link = await auth.generatePasswordResetLink(email);
    Unhandled Email Errors

    Calls to transporter.sendMail are not wrapped in try/catch inside controller flows, so email failures could crash requests without proper error responses

    const sendPasswordResetEmail = async (
      email: string,
      link: string
    ): Promise<void> => {
      const mailOptions = createPasswordResetMailOptions(email, link);
      await transporter.sendMail(mailOptions);
      functions.logger.info("Password reset email sent successfully to:", email);
    };

    @github-actions
    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Add redirect URL options

    Include ActionCodeSettings when generating the reset link so it correctly redirects
    back to your front-end reset page and handles in-app flows. This ensures users land
    on the right page after clicking the link.

    functions/src/controllers/auth_controller.ts [581]

    -const link = await auth.generatePasswordResetLink(email);
    +const actionCodeSettings = {
    +  url: `${process.env.FRONTEND_URL}/reset-password`,
    +  handleCodeInApp: true,
    +};
    +const link = await auth.generatePasswordResetLink(email, actionCodeSettings);
    Suggestion importance[1-10]: 7

    __

    Why: Including ActionCodeSettings ensures the password reset link correctly redirects back to the front-end, preventing users from being sent to the default Firebase flow.

    Medium
    General
    Handle sendMail errors

    Wrap the call to sendMail in a try/catch to handle potential SMTP failures and log
    errors. This prevents unhandled promise rejections and surfaces email delivery
    issues.

    functions/src/controllers/auth_controller.ts [118]

    -await transporter.sendMail(mailOptions);
    +try {
    +  await transporter.sendMail(mailOptions);
    +} catch (error) {
    +  functions.logger.error("Error sending email:", error);
    +  throw error;
    +}
    Suggestion importance[1-10]: 6

    __

    Why: Wrapping transporter.sendMail in a try/catch prevents unhandled rejections and logs SMTP failures, improving reliability.

    Low
    Prevent duplicate verification

    Before sending a verification link, fetch the user record and check if emailVerified
    is already true. Return a clear error if the account is already verified to avoid
    redundant emails.

    functions/src/controllers/auth_controller.ts [635]

    +const userRecord = await auth.getUser(decodedSessionCookie.sub);
    +if (userRecord.emailVerified) {
    +  return res.status(400).json({ status: 400, error: "Email already verified" });
    +}
     const link = await auth.generateEmailVerificationLink(email);
    Suggestion importance[1-10]: 5

    __

    Why: Checking emailVerified on the fetched user record avoids sending redundant verification emails and improves UX.

    Low
    Validate SMTP credentials

    Validate that MAILTRAP_USER and MAILTRAP_PASS are set before creating the
    transporter to catch missing credentials early during initialization.

    functions/src/controllers/auth_controller.ts [41-48]

    +if (!process.env.MAILTRAP_USER || !process.env.MAILTRAP_PASS) {
    +  throw new Error("Mailtrap credentials are not set");
    +}
     const transporter = nodemailer.createTransport({
       host: "live.smtp.mailtrap.io",
       port: 587,
       auth: {
         user: process.env.MAILTRAP_USER,
         pass: process.env.MAILTRAP_PASS,
       },
     });
    Suggestion importance[1-10]: 5

    __

    Why: Verifying MAILTRAP_USER and MAILTRAP_PASS upfront catches missing credentials early and prevents transporter initialization errors.

    Low

    @hib4 hib4 merged commit 8605de5 into main May 23, 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