temporary solution by eg598 · Pull Request #249 · mate-academy/node_auth-app

eg598 · 2026-03-11T20:20:47Z

No description provided.

mateacademy-ai-mentor

Review Summary

You've built a great foundation for the application with a well-organized structure and solid error handling. However, several core features required by the task are missing, so I'll need you to make some updates before this can be approved.

Here are the main issues that need to be addressed:

Missing Core Functionality: The entire password reset flow and the user profile page (including the ability to change name, password, and email) are unimplemented.
Incomplete User Flows: The registration process must accept a name, which is currently missing from the user model and registration logic. The login process also needs to check if a user's account is active.
Incorrect Redirects: Several endpoints, such as login, logout, and activation, are required to redirect the user to another page (e.g., /profile or /login) instead of sending a JSON response.
Missing Routes: The router for authentication is missing an endpoint for logout, and there is no catch-all route to handle 404 Not Found errors for undefined pages.

Please review the task requirements carefully to implement these missing pieces. You've got a strong start, and I'm confident you can get this completed!

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button² in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag³.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

mateacademy-ai-mentor · 2026-03-11T20:23:47Z

src/controllers/auth.controller.js

+function validateEmail(value) {}
+
+function validatePassword(value) {}


These validation functions are currently empty. The task requires you to check password rules during registration and provide feedback to the user if the rules are not met.

mateacademy-ai-mentor · 2026-03-11T20:23:47Z

src/controllers/auth.controller.js

+function validatePassword(value) {}
+
+const register = async (req, res) => {
+  const { email, password } = req.body;


The requirements state that registration should accept a name, email, and password. The name field is missing from the request body destructuring.

mateacademy-ai-mentor · 2026-03-11T20:23:47Z

src/controllers/auth.controller.js

+  user.activationToken = null;
+  await user.save();
+
+  res.send(user);


The requirement is to "redirect to Profile after the activation". Instead of sending the user object as a JSON response, you should perform a redirect, for example, using res.redirect('/profile'). The exact URL would depend on your client-side routing.

mateacademy-ai-mentor · 2026-03-11T20:23:47Z

src/controllers/auth.controller.js

+  res.send(user);
+};
+
+const login = async (req, res) => {


The login logic is missing a required check. The requirements state that if a user is not active, you should ask them to activate their email. You need to check if the user.activationToken is still present and handle that case before generating tokens.

mateacademy-ai-mentor · 2026-03-11T20:23:47Z

src/controllers/auth.controller.js

+    throw ApiError.badRequest('Wrong password');
+  }
+
+  return generateTokens(res, user);


The requirement is to "Redirect to profile after login". The current implementation sends a JSON response with the tokens. Based on the consistent 'redirect' requirements in the task description, you should perform a server-side redirect here.

mateacademy-ai-mentor · 2026-03-11T20:23:47Z

src/controllers/auth.controller.js

+
+  await tokenService.remove(userData.id);
+
+  res.sendStatus(204);


The requirements state that the user should be redirected to the login page after logging out. Instead of res.sendStatus(204), you should use a redirect like res.redirect('/login').

mateacademy-ai-mentor · 2026-03-11T20:23:47Z

setup.js

+
+const { client } = require('./src/utils/db');
+
+client.sync({ force: true })


Using force: true drops all existing tables and recreates them, which leads to complete data loss. While this can be useful for development to quickly reset the database, it's extremely dangerous in a production environment. A safer approach for managing database schema changes in production is to use migration tools.

mateacademy-ai-mentor · 2026-03-11T20:23:47Z

src/controllers/user.controller.js

+const userController = {
+  getAllActivated,
+};


According to the task description, the application should have a Profile page for authenticated users. This page should allow users to change their name, password, and email. This controller is the right place for that logic, but the required handlers for these actions are missing.

mateacademy-ai-mentor · 2026-03-11T20:23:47Z

src/index.js

+  res.send('hello');
+});
+
+app.use(errorMiddleware);


The task requires a '404 for all the other pages'. Currently, Express will handle this by default, but it's better to implement an explicit catch-all middleware to send a consistent response. You should add a middleware here, before the errorMiddleware, to handle any requests that haven't been matched by a route handler.

mateacademy-ai-mentor · 2026-03-11T20:23:47Z

src/middlewares/errorMiddleware.js

+    return;
+  }
+
+  next();


This next() call is unreachable. An Express error-handling middleware is only invoked when an error is passed to it from a previous middleware or route handler (e.g., via next(error)). Therefore, the error parameter will always be defined inside this function. One of the if (error ...) blocks above will always execute and its return statement will terminate the function, preventing this line from ever being reached.

mateacademy-ai-mentor · 2026-03-11T20:23:52Z

src/middlewares/authMiddleware.js

+    return;
+  }
+
+  next();


After successfully verifying the token, the decoded userData should be attached to the request object so that downstream route handlers can access it. A common convention is to add req.user = userData; before calling next(). This will be essential for implementing profile page features where you need to know which user's data to modify.

mateacademy-ai-mentor · 2026-03-11T20:23:52Z

src/models/user.model.js

+const User = client.define('user', {
+  email: {
+    type: DataTypes.STRING,
+    allowNull: false,
+    unique: true,
+  },
+  password: {
+    type: DataTypes.STRING,
+    allowNull: false,
+  },
+  activationToken: {
+    type: DataTypes.STRING,
+  },
+});


The task description requires users to register with a name, email, and password. The name field is missing from this model definition. You'll also need it for the profile page functionality where a user can change their name.

mateacademy-ai-mentor · 2026-03-11T20:23:52Z

src/routes/user.route.js

+
+const router = express.Router();
+
+router.get('/', authMiddleware, catchError(userController.getAllActivated));


This route provides a list of all activated users, which is not a requirement for the user profile page. The task requires endpoints that allow an authenticated user to manage their own profile, such as updating their name, password, or email. The necessary routes for these actions are missing.

mateacademy-ai-mentor · 2026-03-11T20:23:52Z

src/routes/auth.route.js

+router.post('/registration', catchError(authController.register));
+router.get('/activation/:activationToken', catchError(authController.activate));
+router.post('/login', catchError(authController.login));
+router.get('/refresh', catchError(authController.refresh));


The authentication router is missing endpoints for required features. According to the task description, you need to implement:

A route for user logout (e.g., POST /logout). The controller function for this already exists.

Routes for the password reset flow. This typically involves at least two endpoints: one to request a password reset email and another to handle the password update itself.

mateacademy-ai-mentor · 2026-03-11T20:23:52Z

src/services/email.service.js

@@ -0,0 +1,38 @@
+const nodemailer = require('nodemailer');
+
+require('dotenv').config();


The require('dotenv').config() call is only necessary in the main entry point of your application (src/index.js). Since it's already called there, it's not needed in other files. Environment variables loaded at the start are available throughout the Node.js process.

mateacademy-ai-mentor · 2026-03-11T20:23:52Z

src/services/email.service.js

+    <a href="${href}">${href}</a>
+  `;
+
+  send({ email, html, subject: 'Activate' });


This function calls send() but doesn't await its result or return the promise it creates. This means the await emailService.sendActivationEmail(...) call in user.service.js will resolve immediately, regardless of whether the email was sent successfully. To handle potential errors and ensure the email is sent before the registration process is considered complete, you should return the promise from the send function call.

mateacademy-ai-mentor · 2026-03-11T20:23:52Z

src/services/email.service.js

+  send,
+};
+
+module.exports = { emailService, transporter, send };


You are exporting send twice: once as a standalone function and again as part of the emailService object. Similarly, transporter is an internal detail of this service that probably doesn't need to be exported. It's generally better to export just the service object (emailService) to provide a clean and consistent API to the rest of your application.

mateacademy-ai-mentor · 2026-03-11T20:23:52Z

src/services/user.service.js

+  });
+}
+
+function normalize({ id, email }) {


The task description requires users to have a name. This normalization function should also include the name in the returned object to ensure user data is consistent across the application. You'll need to update the function parameters to destructure name as well.

mateacademy-ai-mentor · 2026-03-11T20:23:52Z

src/services/user.service.js

+  return User.findOne({ where: { email } });
+}
+
+async function register(email, password) {


According to the requirements, registration should use the user's name, email, and password. The name parameter is missing from this function's signature.

mateacademy-ai-mentor · 2026-03-11T20:23:52Z

src/services/user.service.js

+    });
+  }
+
+  await User.create({ email, password, activationToken });


When creating the new user, you should also include the name field, which is a required part of the registration process.

mateacademy-ai-mentor · 2026-03-11T20:23:56Z

src/services/user.service.js

+const userService = {
+  getAllActivated,
+  normalize,
+  findByEmail,
+  register,
+};


This service is missing implementations for several key features required by the task description. You need to add functions to handle:

The entire password reset flow.

Profile page actions, such as changing the user's name, password, and email.

mateacademy-ai-mentor · 2026-03-11T20:23:56Z

src/services/jwt.service.js

@@ -0,0 +1,42 @@
+const jwt = require('jsonwebtoken');
+
+require('dotenv').config();


The require('dotenv').config() call is only necessary in the main entry point of your application (src/index.js). Since it's already called there, it's not needed in other files. Environment variables loaded at the start are available throughout the Node.js process.

mateacademy-ai-mentor · 2026-03-11T20:23:56Z

src/services/jwt.service.js

+}
+
+function signRefresh(user) {
+  const token = jwt.sign(user, process.env.JWT_REFRESH_KEY);


Refresh tokens should have an expiration date to limit the time window an attacker has if the token is compromised. Although they should live much longer than access tokens, they shouldn't be valid forever. Consider adding an expiresIn option here, similar to the access token, but with a longer duration (e.g., '7d' or '30d').

mateacademy-ai-mentor · 2026-03-11T20:23:56Z

src/utils/db.js

@@ -0,0 +1,13 @@
+const { Sequelize } = require('sequelize');
+
+require('dotenv').config();


The require('dotenv').config() call is only necessary in the main entry point of your application (src/index.js). Since it's already called there, it's not needed in other files. Environment variables loaded at the start are available throughout the Node.js process.

mateacademy-ai-mentor

Review Summary

You've built a solid foundation for the application, and the overall structure is great. However, I'm requesting a few changes because some core features are currently broken or incomplete.

Here are the main issues to address:

Login Crash: In src/controllers/auth.controller.js, the login function sends two responses (JSON and a redirect), which will cause a server crash. Please refactor this so only the redirect is sent.
Broken Password Reset: The password reset flow is not functional. The /change-password/:resetToken route in src/routes/auth.route.js is pointing to the wrong controller function. It should use resetPassword.
Incomplete Profile Management: The password change logic in src/controllers/user.controller.js is missing the required confirmation field. Additionally, the email change feature needs to verify the user's password and send a notification to the old email address as per the requirements.
Missing Validation: The validation functions for registration in src/controllers/auth.controller.js are empty. Please implement the required input validation, especially for the password.

Fixing these critical issues will make your authentication and profile management flows fully functional and secure. You're very close!

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button² in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag³.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

mateacademy-ai-mentor · 2026-03-11T23:49:04Z

src/controllers/user.controller.js

+};
+
+const changePassword = async (req, res) => {
+  const { email, oldPassword, newPassword } = req.body;


According to the task description, changing a password requires oldPassword, newPassword, and a confirmation field. You are missing the confirmation field from the request body and the logic to verify that newPassword and confirmation are the same.

mateacademy-ai-mentor · 2026-03-11T23:49:04Z

src/controllers/user.controller.js

+  if (!(await bcrypt.compare(oldPassword, user.password))) {
+    throw ApiError.badRequest('New password is same as current');


This check correctly verifies if the oldPassword is incorrect, but the error message New password is same as current is misleading. The message should state that the old password provided is incorrect.

mateacademy-ai-mentor · 2026-03-11T23:49:04Z

src/controllers/user.controller.js

+};
+
+const changeEmail = async (req, res) => {
+  const { email, newEmail } = req.body;


The requirements state that to change an email, the user must provide their password and confirm the new email. These fields are missing from the request body.

mateacademy-ai-mentor · 2026-03-11T23:49:04Z

src/controllers/user.controller.js

+    throw ApiError.badRequest('New email is same as current');
+  }
+
+  await user.update({ email: newEmail });


The task requires notifying the user's old email address about this change. This step is missing. You should call an email service function here to send the notification.

mateacademy-ai-mentor · 2026-03-11T23:49:04Z

src/controllers/auth.controller.js

+function validateEmail(value) {}
+
+function validatePassword(value) {}
+
+function validateName(value) {}


These validation functions are currently empty. According to the task requirements, you need to implement validation for registration fields, especially to check if the password meets the specified rules.

mateacademy-ai-mentor · 2026-03-11T23:49:04Z

src/controllers/auth.controller.js

+  await generateTokens(res, user);
+  res.redirect(`${process.env.CLIENT_HOST}/profile`);


The generateTokens function sends a JSON response, and then you're calling res.redirect() immediately after. This will cause a "Cannot set headers after they are sent to the client" error. The login flow should only redirect as per the requirements. You might need to refactor generateTokens so it doesn't send a response, allowing the login function to handle the redirect.

mateacademy-ai-mentor · 2026-03-11T23:49:04Z

src/routes/auth.route.js

+router.post(
+  '/change-password/:resetToken',
+  catchError(authController.requestChangePassword),


This route is meant for submitting the new password after the user clicks the reset link. However, it's incorrectly calling authController.requestChangePassword, which is used to initiate the password reset process. This should be pointing to authController.resetPassword to handle the password update.

mateacademy-ai-mentor · 2026-03-11T23:49:04Z

src/services/user.service.js

+function normalize({ id, email }) {
+  return { id, email };


The user object you're returning is missing the name field. According to the requirements, the profile page needs to display the user's name. You should include the name in the normalized object so the client has access to it.

mateacademy-ai-mentor · 2026-03-11T23:49:04Z

src/services/user.service.js

+  user.resetToken = resetToken;
+  await user.save();
+
+  await emailService.sendActivationEmail(email, resetToken);


You're calling sendActivationEmail here, but this function is for resetting a password. This will send an email with the wrong content and subject line. You should be calling emailService.sendResetPasswordEmail instead.

mateacademy-ai-mentor

Review Summary

I am approving this submission as an educational override to help you move forward, but there are a few important issues to address. You've done an excellent job fixing the critical bugs from the last review—the application is much more stable now!

However, several key requirements are still missing:

Registration Validation: The validation functions in src/controllers/auth.controller.js are empty. You need to implement the checks for user input, especially for the password rules.
Incomplete Profile Features: The changeEmail function in src/controllers/user.controller.js must verify the user's current password and notify the old email address. The feature for a logged-in user to change their password is also missing entirely.
Incorrect Password Reset Email: In src/services/user.service.js, the resetPassword function is calling sendActivationEmail instead of sendResetPasswordEmail, which breaks the flow.

Focus on these areas to complete the functionality. Keep up the great work!

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Rate AI review example ↩

mateacademy-ai-mentor · 2026-03-12T00:00:37Z

src/controllers/user.controller.js

+const changeEmail = async (req, res) => {
+  const { email, newEmail } = req.body;
+
+  const user = await userService.findByEmail(email);
+
+  if (!user) {
+    throw ApiError.badRequest('Incorrect user');
+  }
+
+  if (!newEmail) {
+    throw ApiError.badRequest('No email entered');
+  }
+
+  if (email === newEmail) {
+    throw ApiError.badRequest('New email is same as current');
+  }
+
+  await user.update({ email: newEmail });
+
+  res.redirect(`${process.env.CLIENT_HOST}/profile`);
+};


This function is missing two key requirements for changing a user's email:

It needs to verify the user's current password before allowing the change.

A notification must be sent to the old email address after the email has been updated.

mateacademy-ai-mentor · 2026-03-12T00:00:37Z

src/controllers/user.controller.js

+const userController = {
+  getAllActivated,
+  changeEmail,
+  changeName,
+  requestChangePassword,
+  resetPassword,
+};


The functionality for an authenticated user to change their password from the profile page is missing. The requirements state that this should require the user's old password, a new password, and a confirmation field.

mateacademy-ai-mentor · 2026-03-12T00:00:37Z

src/controllers/auth.controller.js

+function validateEmail(value) {}
+
+function validatePassword(value) {}
+
+function validateName(value) {}


These validation functions are still empty. According to the task requirements, you need to implement checks for the user's input during registration, especially for the password which has specific rules that need to be enforced.

mateacademy-ai-mentor · 2026-03-12T00:00:37Z

src/routes/user.route.js

+router.get('/', authMiddleware, catchError(userController.getAllActivated));
+
+router.post(
+  '/change-password',
+  catchError(userController.requestChangePassword),
+);
+
+router.post(
+  '/change-password/:resetToken',
+  catchError(userController.resetPassword),
+);
+
+router.patch('/email', authMiddleware, catchError(userController.changeEmail));
+
+router.patch('/name', authMiddleware, catchError(userController.changeName));


These routes cover most of the user management features, but the route for an authenticated user to change their password is missing. The task requires a feature where a logged-in user can change their password by providing their old password, a new password, and a confirmation.

mateacademy-ai-mentor · 2026-03-12T00:00:37Z

src/services/user.service.js

+  user.resetToken = resetToken;
+  await user.save();
+
+  await emailService.sendActivationEmail(email, resetToken);


This function is sending the wrong email. For a password reset request, you should be calling emailService.sendResetPasswordEmail, not sendActivationEmail. This will ensure the user receives the correct link to reset their password.

temporary solution

5154b57

mateacademy-ai-mentor suggested changes Mar 11, 2026

View reviewed changes

mateacademy-ai-mentor reviewed Mar 11, 2026

View reviewed changes

solution

f8a0c59

eg598 requested a review from mateacademy-ai-mentor March 11, 2026 23:46

mateacademy-ai-mentor suggested changes Mar 11, 2026

View reviewed changes

fix

191bdb5

eg598 requested a review from mateacademy-ai-mentor March 11, 2026 23:58

mateacademy-ai-mentor approved these changes Mar 12, 2026

View reviewed changes

		function validateEmail(value) {}

		function validatePassword(value) {}


		const { client } = require('./src/utils/db');

		client.sync({ force: true })

+                  return;
+                }
+                next();


		const router = express.Router();

		router.get('/', authMiddleware, catchError(userController.getAllActivated));

		@@ -0,0 +1,38 @@
		const nodemailer = require('nodemailer');

		require('dotenv').config();

		@@ -0,0 +1,42 @@
		const jwt = require('jsonwebtoken');

		require('dotenv').config();

		@@ -0,0 +1,13 @@
		const { Sequelize } = require('sequelize');

		require('dotenv').config();

		if (!(await bcrypt.compare(oldPassword, user.password))) {
		throw ApiError.badRequest('New password is same as current');

		await generateTokens(res, user);
		res.redirect(`${process.env.CLIENT_HOST}/profile`);

Conversation

eg598 commented Mar 11, 2026

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment