Forms UI and Profile Update #21

naveensanjula975 · 2026-02-01T14:23:54Z

No description provided.

…ice.

…eature/password-reset

Copilot

Pull request overview

This PR implements a password reset functionality using OTP (One-Time Password) instead of JWT tokens, adds email notification capabilities using Nodemailer, and makes minor adjustments to file upload validation. The changes also include database schema updates to store OTP data and updated documentation.

Changes:

Replaced JWT-based password reset with OTP-based system (6-digit codes with 15-minute expiry)
Added email service integration using Nodemailer for sending password reset codes
Added database fields (resetOTP, resetOTPExpiry) to User model with corresponding migrations
Removed GIF support from image upload middleware (now supports JPEG, PNG, WebP only)
Added debug logging to validation and upload middleware

Reviewed changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
src/services/emailService.js	New email service for sending password reset OTPs via SMTP
src/routes/auth.js	Updated forgot-password and reset-password endpoints to use OTP instead of JWT
src/models/user.js	Added resetOTP and resetOTPExpiry fields to User model
src/database/migrations/20251220165748-add-reset-otp-fields.js	Empty migration file (duplicate/incomplete)
src/database/migrations/20251220000000-add-reset-otp-fields.cjs	Migration to add OTP fields to Users table
src/middleware/validation.js	Added debug logging for validation
src/middleware/upload.js	Removed GIF support and added debug logging
package.json	Added nodemailer dependency (v7.0.11)
package-lock.json	Lock file update for nodemailer
README.md	Added database setup and migration instructions
.env.example	Added SMTP configuration variables

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-01T14:30:19Z

src/services/emailService.js

+const createTransporter = () => {
+    // Check if SMTP settings are present
+    if (!process.env.SMTP_HOST || !process.env.SMTP_USER || !process.env.SMTP_PASS) {
+        console.warn('⚠️ SMTP settings not missing. Email sending functionality might fail.');


The warning message contains a logical error. It says "SMTP settings not missing" when it should say "SMTP settings missing" or "SMTP settings not found". The current message states the opposite of what the code is checking.

Suggested change

console.warn('⚠️ SMTP settings not missing. Email sending functionality might fail.');

console.warn('⚠️ SMTP settings missing. Email sending functionality might fail.');

Copilot · 2026-02-01T14:30:19Z

src/services/emailService.js

+        tls: {
+            // Do not fail on invalid certs (useful for dev with self-signed certs)
+            // In production, you might want to remove this
+            rejectUnauthorized: false


Setting rejectUnauthorized to false disables TLS certificate validation, which creates a security vulnerability by allowing man-in-the-middle attacks. While the comment mentions this is useful for development, it should be controlled by an environment variable to ensure it's never accidentally deployed to production. Consider using process.env.NODE_ENV === 'development' ? false : true or a dedicated SMTP_REJECT_UNAUTHORIZED environment variable.

Copilot · 2026-02-01T14:30:19Z

src/routes/auth.js

+        // Verify OTP logic
+        // 1. Check if OTP matches
+        if (!user.resetOTP || user.resetOTP !== otp) {
+            return res.status(400).json({
+                success: false,
+                message: 'Invalid OTP code'
+            });
+        }
+
+        // 2. Check expiry
+        if (!user.resetOTPExpiry || new Date() > new Date(user.resetOTPExpiry)) {
+            return res.status(400).json({
+                success: false,
+                message: 'OTP code has expired'
+            });
+        }


The password reset OTP verification lacks brute force protection. An attacker could attempt unlimited OTP guesses within the 15-minute window. With only 6 digits (1,000,000 possible combinations), this creates a security vulnerability. Consider implementing one or more protections: 1) Add a failed attempt counter with account lockout after a few failures, 2) Implement rate limiting on this endpoint, or 3) Add exponential backoff delays after failed attempts.

Copilot · 2026-02-01T14:30:20Z

src/services/emailService.js

+import nodemailer from 'nodemailer';
+
+/**
+ * Email Service for sending emails using Nodemailer
+ */
+
+// Create reusable transporter
+const createTransporter = () => {
+    // Check if SMTP settings are present
+    if (!process.env.SMTP_HOST || !process.env.SMTP_USER || !process.env.SMTP_PASS) {
+        console.warn('⚠️ SMTP settings not missing. Email sending functionality might fail.');
+    }
+
+    return nodemailer.createTransport({
+        host: process.env.SMTP_HOST,
+        port: parseInt(process.env.SMTP_PORT || '587'),
+        secure: process.env.SMTP_SECURE === 'true', // true for 465, false for other ports
+        auth: {
+            user: process.env.SMTP_USER,
+            pass: process.env.SMTP_PASS,
+        },
+        tls: {
+            // Do not fail on invalid certs (useful for dev with self-signed certs)
+            // In production, you might want to remove this
+            rejectUnauthorized: false
+        }
+    });
+};
+
+/**
+ * Send password reset OTP email
+ * @param {string} to - Recipient email address
+ * @param {string} otp - 6-digit OTP
+ * @param {string} userName - User's name
+ * @returns {Promise<void>}
+ */
+export const sendPasswordResetEmail = async (to, otp, userName = 'User') => {
+    try {
+        const transporter = createTransporter();
+
+        // Email HTML template
+        const htmlContent = `
+      <!DOCTYPE html>
+      <html>
+      <head>
+        <meta charset="utf-8">
+        <meta name="viewport" content="width=device-width, initial-scale=1.0">
+        <title>Password Reset</title>
+        <style>
+          body { font-family: Arial, sans-serif; line-height: 1.6; color: #333; max-width: 600px; margin: 0 auto; padding: 20px; }
+          .container { background-color: #f9f9f9; border-radius: 10px; padding: 30px; border: 1px solid #e0e0e0; }
+          .header { text-align: center; margin-bottom: 30px; }
+          .logo { font-size: 24px; font-weight: bold; color: #4FC3F7; }
+          .content { background-color: white; padding: 25px; border-radius: 8px; }
+          .otp-box { background-color: #f5f5f5; font-size: 32px; font-weight: bold; letter-spacing: 5px; text-align: center; padding: 15px; border-radius: 8px; margin: 20px 0; border: 2px dashed #4FC3F7; color: #333; }
+          .footer { margin-top: 30px; text-align: center; font-size: 12px; color: #666; }
+          .warning { background-color: #fff3cd; border-left: 4px solid #ffc107; padding: 12px; margin: 20px 0; font-size: 14px; }
+        </style>
+      </head>
+      <body>
+        <div class="container">
+          <div class="header">
+            <div class="logo">🔧 FixPoint</div>
+            <h2>Password Reset Code</h2>
+          </div>
+
+          <div class="content">
+            <p>Hello <strong>${userName}</strong>,</p>
+
+            <p>We received a request to reset your password. Use the code below to complete the process:</p>
+
+            <div class="otp-box">
+              ${otp}
+            </div>
+
+            <div class="warning">
+              <strong>⏰ Expires in 15 minutes.</strong><br>
+              If you didn't request this code, you can safely ignore this email.
+            </div>
+          </div>
+
+          <div class="footer">
+            <p>This is an automated email from FixPoint Maintenance Management System.</p>
+            <p>&copy; 2025 FixPoint. All rights reserved.</p>
+          </div>
+        </div>
+      </body>
+      </html>
+    `;
+
+        const textContent = `
+Password Reset Code
+
+Hello ${userName},
+
+Your password reset code is: ${otp}
+
+This code expires in 15 minutes.
+If you didn't request this, please ignore this email.
+
+FixPoint Maintenance Management System
+    `;
+
+        // Send email
+        const info = await transporter.sendMail({
+            from: `"${process.env.EMAIL_FROM_NAME || 'FixPoint Support'}" <${process.env.EMAIL_FROM || process.env.SMTP_USER}>`,
+            to: to,
+            subject: 'Your Password Reset Code - FixPoint',
+            text: textContent,
+            html: htmlContent,
+        });
+
+        console.log('✅ Password reset email sent successfully:', info.messageId);
+        return info;
+
+    } catch (error) {
+        console.error('❌ Error sending password reset email:', error);
+        throw new Error('Failed to send password reset email');
+    }
+};
+
+export default {
+    sendPasswordResetEmail
+};


The new password reset OTP functionality lacks test coverage. Given that the codebase has comprehensive testing (as evidenced by test files for other services and controllers), this critical security feature should have tests covering: OTP generation and storage, email sending, OTP validation (both valid and invalid cases), expiry checking, and password update flow. This is especially important for security-critical authentication flows.

Copilot · 2026-02-01T14:30:20Z

src/routes/auth.js


+        // Verify OTP logic
+        // 1. Check if OTP matches
+        if (!user.resetOTP || user.resetOTP !== otp) {


The OTP comparison uses a simple string equality check (user.resetOTP !== otp) which is vulnerable to timing attacks. An attacker could potentially use timing differences to determine correct OTP digits one at a time. Use crypto.timingSafeEqual() for constant-time string comparison to prevent timing attacks. Convert both strings to buffers before comparison.

Copilot · 2026-02-01T14:30:20Z

src/routes/auth.js

+        // Save OTP to database
+        await userService.updateUser(user.id, {
+            resetOTP: otp,
+            resetOTPExpiry: otpExpiry
+        });
+
+        // Send Email
+        try {
+            await emailService.sendPasswordResetEmail(user.email, otp, user.name);
+        } catch (emailError) {
+            console.error('Failed to send email:', emailError);
+            // In a real scenario, you might want to rollback the DB change or return an error,
+            // but for security "blind" responses, we might still return success or a generic error.
+            // Here we return 500 because if email fails, user can't reset.
+            return res.status(500).json({
+                success: false,
+                message: 'Failed to send verification email. Please try again.'
+            });
+        }


When email sending fails, the OTP is already saved in the database but the error response prevents the user from knowing about it. This means the OTP is stored but unusable. Consider either: 1) Wrapping both database update and email sending in a try-catch to rollback the OTP on email failure, or 2) Clearing the OTP from the database when email sending fails. Currently, the database contains an OTP that was never delivered to the user.

Copilot · 2026-02-01T14:30:20Z

README.md

+   npm run db:migrate
+   ```
+
+2. **Seed Data**: Populate the database with demo users and branches.
+   ```bash
+   npm run db:seed
+   ```
+
+**To reset the entire database (Undo all migrations, remigrate, and re-seed):**
+```bash
+npm run db:reset


The README.md references npm scripts (db:migrate, db:seed, db:reset) that don't exist in package.json. These scripts need to be added to package.json, or the README documentation should be updated to use the correct commands (e.g., npx sequelize-cli db:migrate).

Suggested change

npm run db:migrate

```

2. **Seed Data**: Populate the database with demo users and branches.

```bash

npm run db:seed

```

**To reset the entire database (Undo all migrations, remigrate, and re-seed):**

```bash

npm run db:reset

npx sequelize-cli db:migrate

Seed Data: Populate the database with demo users and branches.
npx sequelize-cli db:seed

To reset the entire database (Undo all migrations, remigrate, and re-seed):

npx sequelize-cli db:migrate:undo:all && npx sequelize-cli db:migrate && npx sequelize-cli db:seed

Copilot · 2026-02-01T14:30:21Z

src/routes/auth.js

-            { expiresIn: '1h' }
-        );
+        // Generate 6-digit OTP
+        const otp = Math.floor(100000 + Math.random() * 900000).toString();


Using Math.random() for OTP generation is cryptographically insecure as Math.random() is a pseudo-random number generator not suitable for security-sensitive operations. An attacker could potentially predict OTP values. Use crypto.randomInt() instead for cryptographically secure random number generation. For example: crypto.randomInt(100000, 1000000).toString()

Copilot · 2026-02-01T14:30:21Z

src/database/migrations/20251220165748-add-reset-otp-fields.cjs

+/** @type {import('sequelize-cli').Migration} */
+module.exports = {
+  async up (queryInterface, Sequelize) {
+    /**
+     * Add altering commands here.
+     *
+     * Example:
+     * await queryInterface.createTable('users', { id: Sequelize.INTEGER });
+     */
+  },
+
+  async down (queryInterface, Sequelize) {
+    /**
+     * Add reverting commands here.
+     *
+     * Example:
+     * await queryInterface.dropTable('users');
+     */
+  }
+};


This migration file is empty and contains only template comments with no actual migration logic. The up and down methods are empty stubs. This appears to be a duplicate of the .cjs migration file (20251220000000-add-reset-otp-fields.cjs) which contains the actual implementation. This empty migration file should be removed to avoid confusion and potential issues during migration execution.

Suggested change

/** @type {import('sequelize-cli').Migration} */

module.exports = {

async up (queryInterface, Sequelize) {

/**

* Add altering commands here.

*

* Example:

* await queryInterface.createTable('users', { id: Sequelize.INTEGER });

*/

},

async down (queryInterface, Sequelize) {

/**

* Add reverting commands here.

*

* Example:

* await queryInterface.dropTable('users');

*/

}

};

/**

* This file is intentionally left empty.

*

* The actual "add reset OTP fields" migration is implemented in:

* 20251220000000-add-reset-otp-fields.cjs

*

* This .js stub exists only to prevent confusion; it is not used as a

* Sequelize migration and exports no migration logic.

*/

Copilot · 2026-02-01T14:30:21Z

src/database/migrations/20251220000000-add-reset-otp-fields.cjs

+        await queryInterface.addColumn('Users', 'resetOTP', {
+            type: Sequelize.STRING(10),
+            allowNull: true,
+            comment: '6-digit OTP for password reset'


The resetOTP column is defined to store the raw 6-digit password reset code in plaintext, which means anyone who can read the database (including via a potential SQL injection elsewhere) can see valid reset codes and hijack accounts before they expire. Instead of persisting the raw OTP value, store only a hashed representation (similar to password hashing) and compare hashes during verification so that a database compromise does not expose usable reset codes.

naveensanjula975 added 4 commits December 21, 2025 11:56

feat: implement OTP-based password reset fields with a new email serv…

b927b3f

…ice.

Merge branch 'dev' of https://github.com/fixpoint-tech/backend into f…

7446e85

…eature/password-reset

feat: profile photo update

19b7b35

Merge branch 'dev' of https://github.com/fixpoint-tech/backend into f…

b911731

…eature/password-reset

naveensanjula975 requested review from Copilot and pabasara20 February 1, 2026 14:23

Copilot started reviewing on behalf of naveensanjula975 February 1, 2026 14:24 View session

Copilot AI reviewed Feb 1, 2026

View reviewed changes

naveensanjula975 added 2 commits February 1, 2026 20:03

feat: update migration file

47ebac0

feat: update package dependencies

df77c0b

pabasara20 approved these changes Feb 2, 2026

View reviewed changes

pabasara20 merged commit 6bd555d into dev Feb 2, 2026
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Forms UI and Profile Update #21

Forms UI and Profile Update #21

Uh oh!

naveensanjula975 commented Feb 1, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 1, 2026

Uh oh!

Copilot AI Feb 1, 2026

Uh oh!

Copilot AI Feb 1, 2026

Uh oh!

Copilot AI Feb 1, 2026

Uh oh!

Copilot AI Feb 1, 2026

Uh oh!

Copilot AI Feb 1, 2026

Uh oh!

Copilot AI Feb 1, 2026

Uh oh!

Copilot AI Feb 1, 2026

Uh oh!

Copilot AI Feb 1, 2026

Uh oh!

Copilot AI Feb 1, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	console.warn('⚠️ SMTP settings not missing. Email sending functionality might fail.');
	console.warn('⚠️ SMTP settings missing. Email sending functionality might fail.');

Forms UI and Profile Update #21

Forms UI and Profile Update #21

Uh oh!

Conversation

naveensanjula975 commented Feb 1, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants