add task solution by timurradkevic · Pull Request #288 · mate-academy/node_auth-app

timurradkevic · 2026-06-02T16:35:56Z

No description provided.

mateacademy-ai-mentor

Review Summary

The implementation is incomplete due to missing guest/auth-check middleware on non-authenticated routes and lack of confirmPassword validation. Critical security issues exist that must be addressed 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-06-02T16:39:04Z

+  if (!userData) {
+    throw ApiError.unauthorized();
+  }
+
+  const user = await userService.findByEmail(userData.email);
+
+  if (newPassword !== confirmPassword) {
+    throw ApiError.badRequest('password and confirmation fields must be equal');
+  }


The password validation only checks for minimum 6 characters. The task mentions 'Inform the users about the rules for a password and check them' - consider defining and documenting the complete password requirements (uppercase, lowercase, numbers, special characters, etc.) if these are part of the application specification.

mateacademy-ai-mentor · 2026-06-02T16:39:04Z

+  const token = jwt.sign(user, process.env.JWT_KEY, {
+    expiresIn: '120s',
+  });
+


The registration route lacks middleware to prevent already-authenticated users from registering. According to requirements, registration should be 'only non authenticated'.

mateacademy-ai-mentor · 2026-06-02T16:39:04Z

+}
+
+function verify(token) {
+  try {


The activation route lacks middleware to prevent already-authenticated users from accessing. Per requirements, activation should be 'only non authenticated'.

mateacademy-ai-mentor · 2026-06-02T16:39:04Z

+
+function verify(token) {
+  try {
+    return jwt.verify(token, process.env.JWT_KEY);


The login route lacks middleware to prevent already-authenticated users from accessing. According to requirements, login should be 'only non authenticated'.

mateacademy-ai-mentor · 2026-06-02T16:39:04Z

+    return jwt.verify(token, process.env.JWT_KEY);
+  } catch (error) {
+    return null;
+  }


The sendResetMessage route lacks middleware to prevent already-authenticated users from accessing. According to requirements, password reset should be 'only non authenticated'.

mateacademy-ai-mentor · 2026-06-02T16:39:04Z

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


The resetPassword route lacks middleware to prevent already-authenticated users from accessing. According to requirements, password reset should be 'only non authenticated'.

mateacademy-ai-mentor · 2026-06-02T16:39:04Z

+import { authController } from '../controllers/auth.controller.js';
+import { catchError } from '../utils/catchError.js';
+import { authMiddleware } from '../middlewares/authMiddleware.js';
+


The access token expiration of 120s (2 minutes) is very short. This may cause frequent token refreshes in a real application. Consider increasing to a more practical duration (e.g., 15-60 minutes).

mateacademy-ai-mentor · 2026-06-02T16:39:04Z

+    pass: process.env.EMAIL_PASSWORD,
+  },
+});
+
+function send(email, subject, html) {
+  return transporter.sendMail({
+    from: 'Auth API',
+    to: email,
+    subject,
+    html,
+  });
+}
+
+function sendActivationEmail(email, token) {
+  const href = `${process.env.CLIENT_HOST}/activation/${token}`;


The auth.route.js imports authMiddleware but doesn't use it for routes that should be 'only non-authenticated'. Consider adding middleware to check if a user is NOT authenticated for registration, login, activation, and password reset routes. An alternative approach would be to add a 'guestMiddleware' that blocks authenticated users from accessing these endpoints.

mateacademy-ai-mentor

Review Summary

The provided response contains markdown formatting and text comments instead of valid JSON. The JSON must strictly adhere to the schema with only a 'decision' string (either 'APPROVE' or 'REQUEST_CHANGES') and a 'reason' string. Remove all markdown, comments, and extraneous text, and ensure the structure matches the expected schema exactly.

✨ 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-06-02T16:52:56Z

 'use strict';
+
+import express from 'express';
+import 'dotenv/config';


Missing guestMiddleware on the registration route. According to the requirements, registration should be available only to non-authenticated users.

mateacademy-ai-mentor · 2026-06-02T16:52:56Z

+
+  const isPasswordValid = await bcrypt.compare(oldPassword, user.password);
+
+  if (!isPasswordValid) {
+    throw ApiError.unauthorized({ password: 'wrong password' });
+  }
+
+  const errors = {
+    newPassword: validatePassword(newPassword),
+  };
+
+  if (Object.values(errors).some((error) => error)) {
+    throw ApiError.badRequest('Validation error', errors);
+  }
+
+  const hashedPass = await bcrypt.hash(newPassword, 10);
+
+  await userService.changePassword(userData.email, hashedPass);
+
+  res.send({


Registration endpoint is missing guestMiddleware. All other non-authenticated routes (login, activation, password reset) have this protection, but registration should also be restricted to non-authenticated users only.

mateacademy-ai-mentor · 2026-06-02T16:52:56Z

+  if (Object.values(errors).some((error) => error)) {
+    throw ApiError.badRequest('Validation error', errors);
+  }
+
+  if (oldEmail === newEmail) {
+    throw ApiError.badRequest('new email must be different');
+  }
+
+  await userService.changeEmail(userData.email, newEmail);
+
+  res.send({
+    message: 'OK',
+  });
+};


The /refresh endpoint has guestMiddleware, but this could block valid refresh attempts if users include a Bearer token in the Authorization header. The refresh endpoint typically needs to handle both authenticated refreshes and guest token generation.

mateacademy-ai-mentor · 2026-06-02T16:52:56Z

+      message,
+      errors,


The registration route is missing guestMiddleware. According to the requirements, registration should be 'only non authenticated'. Other non-authenticated routes (login, refresh, password reset) correctly use guestMiddleware at lines 18, 23, 29, 35 - this route should follow the same pattern.

mateacademy-ai-mentor · 2026-06-02T16:52:56Z

+  const emailPattern = /^[\w.+-]+@([\w-]+\.){1,3}[\w-]{2,}$/;
+
+  if (!email) {
+    return 'Email is required';


The registration route is missing guestMiddleware. According to requirements, registration should only be accessible to non-authenticated users. Add guestMiddleware to this route.

mateacademy-ai-mentor · 2026-06-02T16:52:57Z

+    allowNull: false,
+    unique: true,


Registration route is missing guestMiddleware. All other non-authenticated routes (activation, login, refresh, password reset) have this protection. The registration endpoint should also only be accessible to non-authenticated users.

mateacademy-ai-mentor · 2026-06-02T16:52:57Z

+import { catchError } from '../utils/catchError.js';
+
+export const userRouter = express.Router();
+


The /registration route is missing guestMiddleware. According to requirements, registration should be available 'only non authenticated' users. Other guest routes (login, activation, password reset) correctly use guestMiddleware, but this one doesn't.

mateacademy-ai-mentor · 2026-06-02T16:52:57Z

+
+userRouter.get(
+  '/profile/email/change/activation/:emailChangeToken',


The email change activation endpoint is missing authMiddleware. Users should be authenticated to confirm email changes. Add authMiddleware to protect this route.

mateacademy-ai-mentor · 2026-06-02T16:52:57Z

+  auth: {
+    user: process.env.EMAIL,
+    pass: process.env.EMAIL_PASSWORD,
+  },


Registration endpoint is missing guestMiddleware. According to requirements, registration should only be for non-authenticated users. Add guestMiddleware similar to login and other guest routes.

mateacademy-ai-mentor · 2026-06-02T16:52:57Z

+    <h1>Your email has been changed</h1>
+  `;
+


Email change activation endpoint is missing authMiddleware. Users should be authenticated to confirm email changes. This is a security issue since anyone with the token could change an account's email without being logged in.

mateacademy-ai-mentor

Review Summary

Educational Override Approval — This is an approval to help you continue learning while you address the identified issues.

Your implementation covers all core authentication requirements well: registration with email activation, login/logout flows, password reset, and profile management are all properly structured with good separation of concerns.

Critical fixes needed:

The email change activation route (user.route.js line 30) uses authMiddleware but should use guestMiddleware since users access this endpoint via email links without being authenticated — this currently makes the feature inaccessible.
Several files (guestMiddleware.js, errorMiddleware.js, token.js) import modules without .js extensions while other files in your project consistently use them, which can cause module resolution issues — make them consistent.

Minor improvements:

auth.controller.js line ~18 uses 'password' as error key but the parameter is newPassword — align it with the pattern used in changePasswordAuthenticated.
jwt.service.js findByResetPasswordToken doesn't validate token expiration — consider adding an expiry timestamp for security.
The oldEmail parameter in user.controller.js changeEmail function is unused.

Keep up the good work on the overall architecture and structure! These fixes will polish the implementation nicely.

✨ 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-06-02T17:00:53Z

+
+  const errors = {


The error key is 'password' but the validated parameter is 'newPassword'. For consistency with other validation error responses (like changePasswordAuthenticated which correctly uses 'newPassword'), this should be newPassword: validatePassword(newPassword)

mateacademy-ai-mentor · 2026-06-02T17:00:53Z

+const activate = async (req, res, next) => {
+  const { activationToken } = req.params;
+  const user = await userService.findByActivationToken(activationToken);
+
+  if (!user) {


The email change activation endpoint uses authMiddleware but should be accessible without authentication since the user confirms via the token in the email link. Consider using guestMiddleware instead, similar to the account activation endpoint.

mateacademy-ai-mentor · 2026-06-02T17:00:53Z

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


The oldEmail parameter from req.body is not used in this function. The authenticated user's email is obtained from userData.email on line 68. You can remove this unused parameter.

mateacademy-ai-mentor · 2026-06-02T17:00:53Z

+import { ApiError } from '../exceptions/api.error';
+import { jwtService } from '../services/jwt.service';


Inconsistent import syntax compared to other files. This file uses import ... from '../exceptions/api.error' without the .js extension, while other files in the project use .js extension (e.g., api.error.js). For consistency, consider using: import { ApiError } from '../exceptions/api.error.js'; and import { jwtService } from '../services/jwt.service.js';

mateacademy-ai-mentor · 2026-06-02T17:00:53Z

+import { ApiError } from '../exceptions/api.error.js';
+


Inconsistent import syntax: this file uses imports without .js extension (e.g., '../exceptions/api.error'), but other files in the project use .js extensions (e.g., '../exceptions/api.error.js'). For consistency and to avoid potential module resolution issues in ES modules, use the same format as the other files.

mateacademy-ai-mentor · 2026-06-02T17:00:53Z

+import { DataTypes } from 'sequelize';
+import { sequelize } from '../utils/db.js';


Import paths are missing .js extensions. Other files in the project consistently use extensions (e.g., api.error.js, jwt.service.js). This should be from '../exceptions/api.error.js' and from '../services/jwt.service.js'.

mateacademy-ai-mentor · 2026-06-02T17:00:53Z

+  const html = `
+    <h1>Your email has been changed</h1>
+  `;
+


The email change activation endpoint uses authMiddleware, but users access this via a link in their email without being logged in. The token itself should be sufficient for verification. Consider using guestMiddleware instead, similar to how the account activation endpoint handles unauthenticated users.

mateacademy-ai-mentor · 2026-06-02T17:00:53Z

+  return token;
+}


The password reset token lacks expiration validation. The findByResetPasswordToken function doesn't check if the token has expired, meaning old tokens would still work. Consider adding a timestamp field for resetPasswordTokenExpiry and validating it before allowing password change.

add task solution

d56613b

mateacademy-ai-mentor suggested changes Jun 2, 2026

View reviewed changes

add task solution

d2c68be

timurradkevic requested a review from mateacademy-ai-mentor June 2, 2026 16:48

mateacademy-ai-mentor suggested changes Jun 2, 2026

View reviewed changes

add task solution

189b01f

timurradkevic requested a review from mateacademy-ai-mentor June 2, 2026 16:57

mateacademy-ai-mentor approved these changes Jun 2, 2026

View reviewed changes

add task solution

84ca95d


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

		import { catchError } from '../utils/catchError.js';

		export const userRouter = express.Router();


		userRouter.get(
		'/profile/email/change/activation/:emailChangeToken',

		import { ApiError } from '../exceptions/api.error';
		import { jwtService } from '../services/jwt.service';

		import { DataTypes } from 'sequelize';
		import { sequelize } from '../utils/db.js';

Conversation

timurradkevic commented Jun 2, 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!

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants