add solution by GabrielSpelchuk · Pull Request #264 · mate-academy/node_auth-app

GabrielSpelchuk · 2026-04-07T13:59:26Z

No description provided.

mateacademy-ai-mentor

Review Summary

This requires changes because several critical, blocking requirements are unimplemented or broken. First, password reset functionality is completely missing, including routes (/reset-password, /reset-password/:token), email service integration, and controller logic — yet it's explicitly required in the task. Second, login does not check if the user is activated, violating the requirement that inactive users must be prompted to activate their email. Third, auth middleware fails to attach req.user, breaking all protected profile routes. Fourth, profile updates lack essential validations: password changes don’t verify the old password or confirm new password matches, and email changes don’t require password verification or notify the old email address. Finally, there’s no 404 handler for undefined routes, and multiple await calls are missing (e.g., Token.destroy() in token.service.js), risking race conditions and runtime errors like res.statusStatus(204) in auth.controller.js. These issues prevent core functionality from working as specified and must be fixed before approval.

✨ 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-04-07T14:05:49Z

+app.use(authRoute);
+app.use(userRouter);
+
+app.get('/', (req, res) => {
+  res.send('Hello');
+});


The task requires a 404 page for all other pages. You have registered your application routes, but you are missing a catch-all middleware to handle requests that don't match any of them. You should add a middleware here, before the errorMiddleware, that sends a 404 response for any unhandled routes.

mateacademy-ai-mentor · 2026-04-07T14:05:49Z

+import { User } from '../models/user';
+import bcrypt from 'bcrypt';
+
+const updateProfile = async (req, res) => {


The requirements state changing password should require an old one, new password and confirmation. Currently accepting only password without validating old password or confirming new password matches

mateacademy-ai-mentor · 2026-04-07T14:05:49Z

+
+  const updateData = { name, email };
+
+  if (password) {


Missing validation that new password matches confirmation and verification of old password. The requirements specify these are needed for password changes

mateacademy-ai-mentor · 2026-04-07T14:05:49Z

+  if (password) {
+    updateData.password = await bcrypt.hash(password, 10);
+  }
+


Requirements state: to change an email you should type the password, confirm the new email and notify the old email about the change. None of this is implemented here

mateacademy-ai-mentor · 2026-04-07T14:05:49Z

+
+const login = async (req, res) => {
+  const { email, password } = req.body;
+
+  const user = await userService.findByEmail(email);
+
+  if (!user) {
+    throw ApiError.badRequest('No such user');
+  }
+
+  const isPasswordValid = await bcrypt.compare(password, user.password);
+
+  if (!isPasswordValid) {
+    throw ApiError.badRequest('Wrong password');
+  }
+
+  generateTokens(res, user);


Missing check for user activation status. According to requirements, if user is not active, they should be asked to activate their email.

mateacademy-ai-mentor · 2026-04-07T14:05:49Z

+
+  res.statusStatus(204);


Typo: res.statusStatus should be res.status. This will cause a runtime error.

mateacademy-ai-mentor · 2026-04-07T14:05:49Z

+import { User } from '../models/user.js';
+import { userService } from '../services/user.service.js';
+import { jwtService } from '../services/jwt.service.js';
+import { ApiError } from '../exeptions/api.error.js';
+import { tokenService } from '../services/token.service.js';
+
+const validateName = (value) => {
+  if (!value.trim()) {
+    return 'Name is required';
+  }
+
+  if (value.length < 4) {
+    return 'At least 4 characters';
+  }
+};
+
+const validateEmail = (value) => {
+  if (!value) {
+    return 'Email is required';
+  }


The authMiddleware extracts user data from JWT but doesn't attach it to req.user, so protected routes won't have access to user information.

mateacademy-ai-mentor · 2026-04-07T14:05:49Z

+import { userService } from '../services/user.service.js';
+import { jwtService } from '../services/jwt.service.js';
+import { ApiError } from '../exeptions/api.error.js';
+import { tokenService } from '../services/token.service.js';
+
+const validateName = (value) => {
+  if (!value.trim()) {
+    return 'Name is required';
+  }
+
+  if (value.length < 4) {
+    return 'At least 4 characters';
+  }


Profile update doesn't require old password verification. Requirements specify: 'to change an email you should type the password, confirm the new email and notify the old email about the change'.

mateacademy-ai-mentor · 2026-04-07T14:05:49Z

+const validateName = (value) => {
+  if (!value.trim()) {
+    return 'Name is required';


Profile update doesn't validate password confirmation. Requirements specify changing password requires an old one, new password and confirmation fields that must be equal.

mateacademy-ai-mentor · 2026-04-07T14:05:49Z

+  const authorization = req.headers['authorization'] || '';
+  const [, token] = authorization.split(' ');
+
+  if (!authorization || !token) {
+    res.status(401);
+
+    return;
+  }
+
+  const userData = jwtService.verify(token);
+
+  if (!userData) {
+    res.status(401);


Profile update doesn't validate old password when changing password. Per requirements: changing password requires an old one, 'new password' and 'confirmation'. Current implementation only accepts 'password' field.

mateacademy-ai-mentor · 2026-04-07T14:05:54Z

+  const authorization = req.headers['authorization'] || '';
+  const [, token] = authorization.split(' ');
+
+  if (!authorization || !token) {
+    res.status(401);
+
+    return;
+  }
+
+  const userData = jwtService.verify(token);
+
+  if (!userData) {
+    res.status(401);


Profile update doesn't handle email changes with proper validation. Per requirements: to change an email you should type the password, confirm the new email and notify the old email about the change.

mateacademy-ai-mentor · 2026-04-07T14:05:54Z

+  if (!authorization || !token) {
+    res.status(401);
+
+    return;
+  }
+
+  const userData = jwtService.verify(token);
+
+  if (!userData) {
+    res.status(401);
+


authMiddleware sets 401 status but doesn't send a response or return, which could cause the request to hang.

mateacademy-ai-mentor · 2026-04-07T14:05:54Z

+import { DataTypes } from 'sequelize';
+import { client } from '../utils/db.js';
+import { User } from './user.js';
+
+export const Token = client.define(
+  'Token',
+  {
+    refreshToken: {
+      type: DataTypes.STRING,
+      allowNull: false,
+    },
+  },
+  {
+    tableName: 'tokens',
+  },
+);


Routes are missing password reset endpoints and user profile routes required by the task (profile page to change name, password, email).

mateacademy-ai-mentor · 2026-04-07T14:05:54Z

+import express from 'express';
+import { authController } from '../controllers/auth.controller.js';
+import { catchError } from '../utils/catchError.js';
+
+export const authRoute = new express.Router();
+
+authRoute.post('/registration', catchError(authController.registration));
+authRoute.post('/login', catchError(authController.login));
+
+authRoute.get(
+  '/activation/:activationToken',
+  catchError(authController.activate),
+);
+
+authRoute.get('/refresh', catchError(authController.refresh));
+authRoute.post('/logout', catchError(authController.logout));


Auth routes are missing password reset endpoints. The task requires: password reset (ask for email → show email sent page → reset password confirmation page with password/confirmation fields → success page with link to login). No routes defined for /reset-password, /reset-password/:token, etc.

mateacademy-ai-mentor · 2026-04-07T14:05:54Z

+import express from 'express';
+import { authController } from '../controllers/auth.controller.js';
+import { catchError } from '../utils/catchError.js';
+
+export const authRoute = new express.Router();
+
+authRoute.post('/registration', catchError(authController.registration));
+authRoute.post('/login', catchError(authController.login));
+
+authRoute.get(
+  '/activation/:activationToken',
+  catchError(authController.activate),
+);
+
+authRoute.get('/refresh', catchError(authController.refresh));
+authRoute.post('/logout', catchError(authController.logout));


No profile routes defined for authenticated users to update their profile (change name, password, email). This is required by the task: 'Profile page: You can change a name', 'change a password', 'change an email'.

mateacademy-ai-mentor · 2026-04-07T14:05:54Z

+
+    activationToken: {
+      type: DataTypes.STRING,
+    },


User model is missing isActive field. The requirements state 'the user should be activated only after email confirmation' and 'If user is not active ask them to activate their email'. The current approach uses activationToken: null as a proxy for activation status, but a boolean isActive field would be more explicit and reliable.

mateacademy-ai-mentor · 2026-04-07T14:05:54Z

+import { DataTypes } from 'sequelize';
+import { client } from '../utils/db.js';
+
+export const User = client.define(
+  'User',
+  {
+    name: {
+      type: DataTypes.STRING,
+      allowNull: false,
+    },
+
+    email: {
+      type: DataTypes.STRING,
+      allowNull: false,
+      unique: true,
+    },


No routes defined for password reset flow. Requirements explicitly require: 'Ask for an email' → 'Show email sent page' → 'Reset Password confirmation page' → 'Show Success page with link to login'. None of these are implemented.

mateacademy-ai-mentor · 2026-04-07T14:05:54Z

+      type: DataTypes.STRING,
+      allowNull: false,
+      unique: true,
+    },


No 404 handling for undefined routes. Requirements state: '404 for all the other pages'. Need to add a catch-all route at the end that returns 404.

mateacademy-ai-mentor · 2026-04-07T14:05:54Z

+    throw ApiError.badRequest('User already exist', {
+      email: 'User already exist',
+    });


The token service's remove function is missing await before Token.destroy(). The function is async but doesn't wait for the database operation to complete, which could cause race conditions.

mateacademy-ai-mentor · 2026-04-07T14:05:54Z

+  return token;
+}


The Token.destroy() call is not awaited. Since it's an async database operation, it should be await Token.destroy(...) to ensure proper execution.

mateacademy-ai-mentor · 2026-04-07T14:05:59Z

@@ -0,0 +1,36 @@
+import jwt from 'jsonwebtoken';
+
+function sing(user) {


The access token expires in 30 seconds which is very short. This will cause users to re-authenticate frequently. Consider increasing to something like '1h' for better UX while maintaining security.

mateacademy-ai-mentor · 2026-04-07T14:05:59Z

+import jwt from 'jsonwebtoken';
+
+function sing(user) {
+  const token = jwt.sign(user, process.env.JWT_KEY, { expiresIn: '30s' });
+
+  return token;
+}
+
+function verify(token) {
+  try {
+    return jwt.verify(token, process.env.JWT_KEY);
+  } catch (error) {
+    return null;
+  }
+}
+
+function singRefresh(user) {
+  const token = jwt.sign(user, process.env.JWT_REFRESH_KEY);
+
+  return token;
+}
+
+function verifyRefresh(token) {
+  try {
+    return jwt.verify(token, process.env.JWT_REFRESH_KEY);
+  } catch (error) {
+    return null;
+  }
+}
+
+export const jwtService = {
+  sing,
+  verify,
+  singRefresh,
+  verifyRefresh,


User service is missing functions for: password reset (generating reset token, resetting password with validation), and profile update functions (change name, change password with old password verification, change email with notification). These are required by the task.

mateacademy-ai-mentor · 2026-04-07T14:05:59Z

+function sendActivationEmail(email, token) {
+  const href = `${process.env.CLIENT_HOST}/activate/${token}`;


The remove function calls Token.destroy which returns a promise, but it's not awaited. This could cause unhandled promise warnings or timing issues.

mateacademy-ai-mentor · 2026-04-07T14:05:59Z

+
+function remove(userId) {
+  Token.destroy({ where: { userId } });


The remove function is async but doesn't use await before Token.destroy. This should be await Token.destroy({ where: { userId } }); for proper async handling.

mateacademy-ai-mentor

Review Summary

Great work on the improvements from the last review, especially with the user activation flow! I am requesting a few changes because some core features are incomplete and have critical bugs that prevent the application from working as required.

Here are the main issues to address:

Password Reset is Incomplete: This feature is not functional. It's missing the routes in src/routes/auth.route.js, the required logic in src/services/user.service.js (updateResetToken, etc.), and a resetToken field in the User model.
Missing Profile Validations: When updating a profile in src/controllers/user.controller.js, you need to add a check for a matching confirmation field when changing a password and send a notification to the user's old email address when their email is changed.
Critical Bugs:
- In src/services/token.service.js, the remove function is missing await for Token.destroy(), which breaks the logout functionality.
- The authMiddleware.js hangs on invalid tokens because it doesn't send a response. You should use your error handler to send a proper 401 error.

Additionally, your build is failing because .eslintrc.js should be renamed to .eslintrc.cjs to work with your ES module setup. Addressing these points will get the project fully functional.

✨ 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-04-07T16:27:47Z

+import { ApiError } from '../exeptions/api.error';
+import { User } from '../models/user';
+import bcrypt from 'bcrypt';
+import { userService } from '../services/user.service';


These import statements are missing the .js file extension. While this might work in some environments, it's best practice in Node.js ESM to include the full file extension for clarity and to avoid potential module resolution issues. Other files in the project correctly include the extension.

mateacademy-ai-mentor · 2026-04-07T16:27:47Z

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


According to the task requirements, when changing a password, the user must provide a new password and a confirmation. The confirmation field is missing from the request body, and there's no logic here to verify that the new password and its confirmation match.

mateacademy-ai-mentor · 2026-04-07T16:27:48Z

+  if (email && email !== user.email) {
+    updateData.email = email;
+  }


The requirements state that when a user's email is changed, a notification must be sent to their old email address. This notification logic is missing from this block.

mateacademy-ai-mentor · 2026-04-07T16:27:48Z

+    throw ApiError.badRequest('User with this email does not exist');
+  }
+
+  const resetToken = crypto.randomBytes(32).toString('hex');


The crypto module is used here but it's not imported. You need to add import crypto from 'crypto'; at the top of the file for this to work.

mateacademy-ai-mentor · 2026-04-07T16:27:48Z

+
+  const resetToken = crypto.randomBytes(32).toString('hex');
+
+  await userService.updateResetToken(user.id, resetToken);


The updateResetToken function is called here, but it's not defined or exported in src/services/user.service.js. The same issue exists for findByResetToken and updatePassword used in the resetPassword function. The password reset feature needs to be fully implemented in the service layer, including adding the necessary fields to the User model.

mateacademy-ai-mentor · 2026-04-07T16:27:48Z

+    activationToken: {
+      type: DataTypes.STRING,
+    },


The task requires a password reset feature, which typically involves storing a temporary token for the user. This model is missing a field, such as resetToken, to hold that value. You might also want to add a resetTokenExpiry field to ensure the token expires after a certain period.

mateacademy-ai-mentor · 2026-04-07T16:27:48Z

@@ -0,0 +1,35 @@
+import { ApiError } from '../exeptions/api.error.js';
+import { User } from '../models/user';


This import statement is missing the .js file extension. While this might work in some environments, it's inconsistent with other imports in this file (like for ApiError and emailService) and the project as a whole. It's best practice in Node.js ESM to include the full file extension.

mateacademy-ai-mentor · 2026-04-07T16:27:48Z

+  await emailService.sendActivationEmail(email, activationToken);
+}
+
+export const userService = { normalize, findByEmail, register };


This service is missing the implementation for the password reset functionality. The auth.controller.js file calls several functions that should be defined here, such as:

updateResetToken

findByResetToken

updatePassword

These functions are essential for the password reset flow to work as required.

mateacademy-ai-mentor · 2026-04-07T16:27:48Z

@@ -0,0 +1,25 @@
+import { Token } from '../models/token';


This import is missing the .js extension. While it might work depending on the Node.js version and configuration, it's a best practice to include it for consistency with other files in your project (like user.service.js) and to prevent potential module resolution issues.

mateacademy-ai-mentor · 2026-04-07T16:27:48Z

+}
+
+function remove(userId) {
+  Token.destroy({ where: { userId } });


Token.destroy() is an asynchronous operation and must be awaited. Without await, the function will return before the token is actually deleted from the database. This means the logout process might not work as expected.

mateacademy-ai-mentor

Review Summary

I am approving this submission as an educational override to help you progress, but there are a few critical issues to address before the application will run correctly. You've done an excellent job fixing previous bugs and building out the core services! However, the password reset feature is currently non-functional. In src/controllers/auth.controller.js, you're using the crypto module without importing it, and you're calling a userService.updatePassword function that doesn't exist yet in the user service. Additionally, several files like src/services/user.service.js are missing the .js extension on local imports, which will prevent the server from starting. For the profile update, please also add the required emailConfirmation check when a user changes their email. You're very close to finishing—address these points, and you'll have a fully working application!

✨ 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-04-07T16:41:56Z

+    user.password = await bcrypt.hash(password, 10);
+  }
+
+  if (email && email !== user.email) {


According to the task description, when changing an email, you also need to "confirm the new email". This is similar to how you handle password changes. You should expect an emailConfirmation field in the request body and verify that it matches the new email before proceeding with the update.

mateacademy-ai-mentor · 2026-04-07T16:41:56Z

+    throw ApiError.badRequest('User with this email does not exist');
+  }
+
+  const resetToken = crypto.randomBytes(32).toString('hex');


The built-in crypto module is used here to generate a token, but it hasn't been imported at the top of the file. This will cause a ReferenceError at runtime.

mateacademy-ai-mentor · 2026-04-07T16:41:56Z

+const resetPassword = async (req, res) => {
+  const { token, newPassword } = req.body;
+  const user = await userService.findByResetToken(token);
+
+  if (!user) {
+    throw ApiError.badRequest('Invalid or expired reset token');
+  }
+
+  const hashedPassword = await bcrypt.hash(newPassword, 10);
+
+  await userService.updatePassword(user.id, hashedPassword);
+
+  res.send({ message: 'Password has been reset' });
+};


This password reset implementation is missing two key parts from the requirements:

Password Confirmation: You need to get a confirmation field from req.body and verify that it matches newPassword.

Update Logic: The userService.updatePassword function doesn't exist in the user service. You should implement the logic to update the user's password and set their resetToken to null directly in this function, then save the user.

mateacademy-ai-mentor · 2026-04-07T16:41:56Z

@@ -0,0 +1,53 @@
+import { ApiError } from '../exeptions/api.error.js';
+import { User } from '../models/user';


When using ES modules in Node.js, you need to include the file extension in your relative imports. This should be from '../models/user.js'; to ensure the module is found correctly.

mateacademy-ai-mentor · 2026-04-07T16:41:56Z

+export const userService = {
+  normalize,
+  findByEmail,
+  register,
+  updateResetToken,
+  findByResetToken,
+};


The resetPassword function in auth.controller.js calls userService.updatePassword, but that function is not implemented or exported in this service. You need to create this function to handle updating the user's password in the database.

mateacademy-ai-mentor · 2026-04-07T16:41:56Z

@@ -0,0 +1,25 @@
+import { Token } from '../models/token';


In an ES Modules environment like this project, you need to include the file extension in your relative imports. This should be import { Token } from '../models/token.js'; to match the pattern used in your other files.

brespect

Lgtm 🚀

add solution

7539bd6

mateacademy-ai-mentor suggested changes Apr 7, 2026

View reviewed changes

mateacademy-ai-mentor reviewed Apr 7, 2026

View reviewed changes

added reset and forget password

3b6a324

GabrielSpelchuk requested a review from mateacademy-ai-mentor April 7, 2026 16:24

mateacademy-ai-mentor suggested changes Apr 7, 2026

View reviewed changes

resolved bugs and created service forgor and reset password

920360c

GabrielSpelchuk requested a review from mateacademy-ai-mentor April 7, 2026 16:39

mateacademy-ai-mentor suggested changes Apr 7, 2026

View reviewed changes

GabrielSpelchuk added 2 commits April 7, 2026 20:15

changed all imports to require

5253ac2

renamed eslint

e9ad4cb

GabrielSpelchuk requested a review from mateacademy-ai-mentor April 7, 2026 17:17

brespect approved these changes Apr 8, 2026

View reviewed changes

		@@ -0,0 +1,36 @@
		import jwt from 'jsonwebtoken';

		function sing(user) {

		function sendActivationEmail(email, token) {
		const href = `${process.env.CLIENT_HOST}/activate/${token}`;


		function remove(userId) {
		Token.destroy({ where: { userId } });


		const resetToken = crypto.randomBytes(32).toString('hex');

		await userService.updateResetToken(user.id, resetToken);

		@@ -0,0 +1,35 @@
		import { ApiError } from '../exeptions/api.error.js';
		import { User } from '../models/user';

		@@ -0,0 +1,53 @@
		import { ApiError } from '../exeptions/api.error.js';
		import { User } from '../models/user';

Conversation

GabrielSpelchuk commented Apr 7, 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!

Choose a reason for hiding this comment