Solution #225

oberezhnay · 2026-01-24T16:30:24Z

Auth application

mateacademy-ai-mentor

Review Summary

Hello! You've done a great job structuring the project with separate controllers, services, and models. However, I'm requesting changes to address a few critical issues that are currently preventing the application from working as required.

Here are the main points to focus on:

Module System Incompatibility: Several files (like jwt.service.js, mailer.js, and auth.middleware.js) use ES Module export, but they are being imported with CommonJS require(). This will cause the application to crash. Please ensure all files consistently use the CommonJS module system (module.exports).
Critical Runtime Errors:
- In auth.controller.js, the login function sends a JSON response and then tries to redirect, which will cause a ERR_HTTP_HEADERS_SENT error. A request can only have one response.
- The findUserById function in user.service.js is missing an await, causing it to return a promise instead of the user data.
Incorrect Logic:
- The account activation logic in auth.controller.js incorrectly tries to nullify a non-existent accessToken field instead of the activationToken.
- The /logout route needs to be protected by the authMiddleware to meet the requirement that only authenticated users can log out.

Once these key issues are fixed, your project will be in a much better state. You're on the right track!

✨ 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-01-24T16:33:13Z

src/middlewares/auth.middleware.js

@@ -0,0 +1,22 @@
+// const { Request, Response, NextFunction } = require('express');
+const { jwt } = require('./../service/jwt.service.js');


This require statement is attempting to import from jwt.service.js, which uses ES Module export syntax. This will not work as expected in a CommonJS environment. To fix this, jwt.service.js should also use module.exports.

mateacademy-ai-mentor · 2026-01-24T16:33:13Z

src/middlewares/auth.middleware.js

+// const { Request, Response, NextFunction } = require('express');
+const { jwt } = require('./../service/jwt.service.js');
+
+export function authMiddleware(req, res, next) {


You're using export, which is ES Module syntax. However, this file is being imported in src/router/user.router.js using require, which is CommonJS. This will cause an error. To align with the rest of the project, you should use module.exports instead.

mateacademy-ai-mentor · 2026-01-24T16:33:13Z

src/controller/user.controller.js

@@ -0,0 +1,143 @@
+'use strict';
+
+const serviceUser = require('./../service/users.service');


The path to the user service module appears to be incorrect. Based on the file structure, it should likely be './../service/user.service.js' instead of './../service/users.service'. This will cause a MODULE_NOT_FOUND error at runtime.

mateacademy-ai-mentor · 2026-01-24T16:33:13Z

src/controller/user.controller.js

+    if (passwordError) {
+      return res.status(400).json({
+        errors: { password: passwordError },
+        maeesage: 'Validation error',


There's a typo in the response key. maeesage should be message to maintain consistency and ensure client applications can parse the error response correctly.

mateacademy-ai-mentor · 2026-01-24T16:33:13Z

src/controller/user.controller.js

+    const isValid = await bcrypt.compare(oldPassword, user.password);
+
+    if (!isValid) {
+      return res.status(401).send('Confirmation do not match');


The error message "Confirmation do not match" is misleading here. This check validates the oldPassword. A more accurate message would be something like "Incorrect old password" to better inform the user.

mateacademy-ai-mentor · 2026-01-24T16:33:13Z

src/controller/user.controller.js

+  }
+};
+
+const updateUserEmail = async (req, res) => {


This asynchronous function is missing a try...catch block. Any error during the database operations or sending the email notification will crash the server. It's important to wrap the logic in a try...catch block to handle potential errors gracefully, similar to the other functions in this file.

mateacademy-ai-mentor · 2026-01-24T16:33:13Z

src/controller/user.controller.js

+  const isValidPassword = await bcrypt.compare(password, user.password);
+
+  if (!isValidPassword) {
+    return res.status(401).send('Wrong passsword');


There is a typo in the error message. It should be Wrong password.

mateacademy-ai-mentor · 2026-01-24T16:33:13Z

src/controller/auth.controller.js

+    return res.status(404).json({ message: 'User not found' });
+  }
+
+  user.accessToken = null;


The User model does not have an accessToken property. To activate the user, you should nullify the activationToken. The current implementation will not correctly activate the user account.

mateacademy-ai-mentor · 2026-01-24T16:33:13Z

src/controller/auth.controller.js

+  }
+
+  sendAuthentification(res, user);
+  res.redirect('/profile');


According to the requirements, the user should be redirected to the profile page after a successful login. However, the sendAuthentification function on the previous line already sends a JSON response. You cannot send a response and then perform a redirect in the same request, as this will cause a ERR_HTTP_HEADERS_SENT error. You should choose one method: either send data back and let the client handle the redirect, or perform the redirect from the server.

mateacademy-ai-mentor · 2026-01-24T16:33:13Z

src/controller/auth.controller.js

+};
+
+const refresh = (req, res) => {
+  const { refreshToken } = req.cookies.refreshToken;


The refreshToken is being destructured from req.cookies.refreshToken, which is the token string itself, not an object. This will result in refreshToken being undefined. It should probably be const { refreshToken } = req.cookies; or const refreshToken = req.cookies.refreshToken;.

mateacademy-ai-mentor · 2026-01-24T16:33:17Z

src/controller/auth.controller.js

+  user.password = bcrypt.hashSync(password, 10);
+  user.resetToken = null;
+
+  await userService.updateUserPassword(user.id, user.password);


The User model defines the primary key as userId, not id. Using user.id will pass undefined to the updateUserPassword service, and the user's password will not be updated.

mateacademy-ai-mentor · 2026-01-24T16:33:17Z

src/models/Auth.model.js

+  'Token',
+  {
+    refreshToken: {
+      type: DataTypes.INTEGER,


The refreshToken is a JWT string, but its type is defined as DataTypes.INTEGER. This will cause an error when trying to save the token to the database. The type should be DataTypes.STRING or DataTypes.TEXT to accommodate the token string.

mateacademy-ai-mentor · 2026-01-24T16:33:17Z

src/models/Auth.model.js

+  },
+);
+
+Token.belongsTo(this.User, { foreignKey: 'user_id', onDelete: 'CASCADE' });


The use of this.User is incorrect here. Since User was imported at the top of the file, you should use the User variable directly to define the association, like so: Token.belongsTo(User, ...). Using this in this context does not refer to the module exports and will cause a runtime error.

mateacademy-ai-mentor · 2026-01-24T16:33:17Z

src/models/Auth.model.js

+module.exports = {
+  User,
+};


This module defines the Token model, but it only exports the User model which was imported. To make the Token model available to other parts of your application, you should export it. For example: module.exports = { Token };.

mateacademy-ai-mentor · 2026-01-24T16:33:17Z

src/router/auth.router.js

+
+const router = express.Router();
+
+router.post('/auth', authController.register);


The route path /auth for user registration is a bit ambiguous. It's a common convention to use a more descriptive path like /register to make the API's intent clearer.

mateacademy-ai-mentor · 2026-01-24T16:33:17Z

src/router/auth.router.js

+router.get('/activate/:activationToken', authController.activate);
+router.post('/login', authController.loginUser);
+router.get('/refresh', authController.refresh);
+router.get('/logout', authController.logout);


The task requirements state that logout should only be for authenticated users. This route is currently public. You should add the authMiddleware here to ensure only logged-in users can access it.

mateacademy-ai-mentor · 2026-01-24T16:33:17Z

src/router/user.router.js

@@ -0,0 +1,12 @@
+const express = require('express');
+const userController = require('./../controller/user.controller.js');
+const authMiddleware = require('./../middlewares/auth.middleware.js');


You are using require to import the authMiddleware, but the auth.middleware.js file uses ES Module export syntax. This will not work correctly, and authMiddleware will not be the expected middleware function, causing a runtime error. To ensure compatibility, the middleware should be exported using module.exports.

mateacademy-ai-mentor · 2026-01-24T16:33:17Z

src/router/user.router.js

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


The requirements for the 'Profile page' specify that an authenticated user can manage their own data. This route, however, fetches all users, which is typically an admin-level function. It would be more appropriate to have an endpoint like /profile or /me that fetches only the authenticated user's data.

mateacademy-ai-mentor · 2026-01-24T16:33:17Z

src/service/jwt.service.js

+export const jwt = {
+  generateAccessToken,
+  generateRefreshToken,
+  validateAccessToken,
+  validateRefreshToken,
+};


This file uses the ES Module export syntax. However, other files in the project, such as auth.controller.js and auth.middleware.js, use the CommonJS require() function to import it. This will lead to an ERR_REQUIRE_ESM error at runtime. To fix this, you should use the CommonJS export syntax, for example: module.exports = { jwt };.

mateacademy-ai-mentor · 2026-01-24T16:33:17Z

src/service/user.service.js

+};
+
+const findUserById = async (userId) => {
+  const user = User.findOne({


The User.findOne method is asynchronous and returns a promise. You are missing the await keyword here. As a result, this function will return a pending promise instead of the user object, which will cause issues in any controller that calls this method.

mateacademy-ai-mentor · 2026-01-24T16:33:20Z

src/util/mailer.js

+export const mailer = {
+  send,
+  sendActivationLink,
+  sendResetPasswordNotification,
+  sendChangeEmailNotification,
+};


This file uses ES Module export syntax, but it is being imported into the controllers using CommonJS require. This will cause a runtime error because the two module systems are not compatible in this setup. To fix this, you should convert this file to use the CommonJS module system by replacing the export statements with module.exports.

mateacademy-ai-mentor

Review Summary

Excellent progress on the project! You've successfully fixed several important issues from the previous review. I am requesting a few more changes, as there are still some critical bugs that cause the application to crash at runtime or fail key requirements.

Please address the following blockers:

Module System Inconsistency: In src/util/mailer.js, you're still using ES Module export statements. Since the project uses CommonJS, this will cause a SyntaxError. Please remove the export keywords.
Multiple Responses on Login: The loginUser function in src/controller/auth.controller.js sends two responses, which will crash the server. Please remove the res.redirect() call and only send the JSON response.
Incorrect Middleware Import: In src/router/auth.router.js, the middleware is imported incorrectly. You need to destructure the import to get the function: const { authMiddleware } = require(...).
Service Import Typo: In src/controller/user.controller.js, the path to the user service is misspelled as users.service.js. It should be user.service.js.
Missing Activation Redirect: The requirement to redirect after account activation is not met because the relevant line in src/controller/auth.controller.js is commented out.

Once these issues are resolved, the application should be fully functional. 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-01-24T19:41:46Z

src/controller/user.controller.js

@@ -0,0 +1,147 @@
+'use strict';
+
+const serviceUser = require('./../service/users.service.js');


The filename for the user service is user.service.js, but it's being imported here as users.service.js (plural). This typo will cause a MODULE_NOT_FOUND error when the application tries to run this code.

mateacademy-ai-mentor · 2026-01-24T19:41:46Z

src/controller/auth.controller.js

+      });
+    }
+
+    const activationToken = bcrypt.genSaltSync(10);


Using bcrypt.genSaltSync to generate an activation token is not its intended purpose. This function is designed to create salts for password hashing. For generating unique tokens, a library like uuid (which you're already using for password resets) or Node's built-in crypto module would be a more appropriate and secure choice.

mateacademy-ai-mentor · 2026-01-24T19:41:46Z

src/controller/auth.controller.js

+  user.activationToken = null;
+  await user.save();
+
+  // res.redirect('/profile');


The requirement is to redirect the user to their profile page after successful activation. This line is currently commented out. For the redirect to work as expected, the user should also be authenticated. Consider calling the sendAuthentification function here before redirecting.

mateacademy-ai-mentor · 2026-01-24T19:41:47Z

src/controller/auth.controller.js

+  sendAuthentification(res, user);
+  res.redirect('/profile');


This function still attempts to send two responses to a single request: one from sendAuthentification (which calls res.send) and another from res.redirect. This will cause an ERR_HTTP_HEADERS_SENT error and crash the application, as pointed out in the previous review.

For an API serving a front-end application, you should typically send the JSON response with tokens and let the client handle the redirection. In that case, res.redirect('/profile') should be removed.

mateacademy-ai-mentor · 2026-01-24T19:41:47Z

src/controller/auth.controller.js

+  user.password = bcrypt.hashSync(password, 10);
+  user.resetToken = null;
+
+  await userService.updateUserPassword(user.userId, user.password);


After modifying the user instance on the lines above, you can simply call await user.save() to persist the changes. Calling userService.updateUserPassword here is redundant as it performs a separate database query to update a field that has already been changed in memory.

mateacademy-ai-mentor · 2026-01-24T19:41:47Z

src/router/auth.router.js

@@ -0,0 +1,15 @@
+const express = require('express');
+const authController = require('./../controller/auth.controller.js');
+const authMiddleware = require('./../middlewares/auth.middleware.js');


The auth.middleware.js file exports an object: module.exports = { authMiddleware }. This import assigns that entire object to the authMiddleware constant. When used in router.get on line 11, it passes an object instead of a function, which will cause an error.

You should use destructuring to extract the function correctly, like this:

const { authMiddleware } = require('./../middlewares/auth.middleware.js');

mateacademy-ai-mentor · 2026-01-24T19:41:47Z

src/util/mailer.js

+  },
+});
+
+export function send(email, subject, html) {


The export keyword is part of the ES Module system. Since the rest of your project uses CommonJS (require/module.exports), this will cause a syntax error. Please remove export from this function declaration.

mateacademy-ai-mentor · 2026-01-24T19:41:47Z

src/util/mailer.js

+  });
+}
+
+export function sendActivationLink(email, activationToken) {


Similar to the send function above, using export here is incorrect for a CommonJS module. This function is already correctly included in the module.exports object at the end of the file, so the export keyword should be removed.

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 in the authentication flow that need your attention. You've done an excellent job structuring your services, models, and middleware! The main problem lies in src/controller/auth.controller.js, where the loginUser, activate, and refresh functions don't send the accessToken back to the client, which is essential for accessing protected routes. The activate function also needs to redirect to the profile page as required, not the login page. Additionally, there's a small but critical import error in src/router/user.router.js; you need to destructure the middleware import: const { authMiddleware } = require(...). You are very close to a complete solution, 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-01-24T20:03:14Z

src/controller/auth.controller.js

+  user.activationToken = null;
+  await user.save();
+
+  res.redirect('/login?activated=true');


The task description requires redirecting to the Profile page after activation. Currently, the user is redirected to the login page. To meet the requirement, you could consider logging the user in automatically upon successful activation and then performing the redirect to their profile.

mateacademy-ai-mentor · 2026-01-24T20:03:14Z

src/controller/auth.controller.js

+  sendAuthentification(res, user);
+  res.redirect('/profile');


This implementation doesn't send the accessToken to the client. The sendAuthentification function returns it, but it's ignored. Without the access token, the client cannot make authenticated API calls. As mentioned in the previous review, you should remove the res.redirect() and instead send a JSON response containing the accessToken. The client-side application will then be responsible for redirecting the user to the profile page.

mateacademy-ai-mentor · 2026-01-24T20:03:14Z

src/controller/auth.controller.js

+const refresh = (req, res) => {
+  const refreshToken = req.cookies.refreshToken;
+
+  if (!jwt.validateRefreshToken(refreshToken)) {
+    return res.status(401);
+  }
+
+  sendAuthentification(res, jwt.validateRefreshToken(refreshToken));
+};


This refresh endpoint is not fully implemented and will cause client requests to hang.

On line 111, res.status(401) only sets the HTTP status code but does not send a response or terminate the request. You should use res.sendStatus(401) or res.status(401).send().

The call to sendAuthentification on line 114 returns a new accessToken, but you are not sending it back to the client. This function needs to send a JSON response with the new token to be useful.

mateacademy-ai-mentor · 2026-01-24T20:03:14Z

src/router/user.router.js

@@ -0,0 +1,12 @@
+const express = require('express');
+const userController = require('./../controller/user.controller.js');
+const authMiddleware = require('./../middlewares/auth.middleware.js');


The authMiddleware is being imported incorrectly. The middleware file exports an object { authMiddleware }, so you are currently passing an object to your routes instead of the middleware function. You need to destructure the import to get the function itself, like this: const { authMiddleware } = require('./../middlewares/auth.middleware.js');.

Solution

93f4939

mateacademy-ai-mentor suggested changes Jan 24, 2026

View reviewed changes

mateacademy-ai-mentor reviewed Jan 24, 2026

View reviewed changes

add fixes after review

bfdce69

oberezhnay requested a review from mateacademy-ai-mentor January 24, 2026 19:39

mateacademy-ai-mentor suggested changes Jan 24, 2026

View reviewed changes

add fixes after second review

5239667

oberezhnay requested a review from mateacademy-ai-mentor January 24, 2026 20:01

mateacademy-ai-mentor approved these changes Jan 24, 2026

View reviewed changes

		@@ -0,0 +1,22 @@
		// const { Request, Response, NextFunction } = require('express');
		const { jwt } = require('./../service/jwt.service.js');

		@@ -0,0 +1,143 @@
		'use strict';

		const serviceUser = require('./../service/users.service');


		const router = express.Router();

		router.post('/auth', authController.register);


		const router = express.Router();

		router.get('/', authMiddleware, userController.getAllUsers);

		@@ -0,0 +1,147 @@
		'use strict';

		const serviceUser = require('./../service/users.service.js');

Solution #225

Are you sure you want to change the base?

Solution #225

Uh oh!

Conversation

oberezhnay commented Jan 24, 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!

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!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!