Solution by in7264 · Pull Request #253 · mate-academy/node_auth-app

in7264 · 2026-03-17T07:45:57Z

No description provided.

mateacademy-ai-mentor

Review Summary

Great job on setting up the project structure and implementing the main authentication flows! Your use of middleware for route protection is well done. However, there are a few critical issues that need to be addressed before this can be approved.

Here are the main points to focus on:

Crashing on Errors: In auth.controller.js and user.controller.js, several functions send an error response but don't stop execution. You need to add a return statement after sending a response (e.g., res.status(404).send(...)) to prevent the code from continuing and trying to access properties on null objects, which causes the server to crash.
Missing 404 Page: The task requires a 404 handler for any undefined routes. Please add a catch-all middleware in src/index.js after all your other routes are defined.
Password Confirmation Logic: In the changePassword function in user.controller.js, you're checking if the confirm field exists, but the requirement is to verify that newPassword is identical to confirm.
Incorrect Email Change Flow: When a user changes their email, the activation link for the new address is incorrectly sent to the old address. The requirement is to send a simple notification to the old address and the activation link to the new one.

Please fix these issues, and I'll be happy to review your work again!

✨ 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-17T07:48:36Z

src/controllers/user.controller.js

+  const { password, newPassword, confirm } = req.body;
+
+  if (!confirm) {


The task requires a confirmation field for the new password. The logic should verify that newPassword and confirm are identical. Currently, you are only checking if confirm is truthy, which doesn't meet the requirement.

mateacademy-ai-mentor · 2026-03-17T07:48:36Z

src/controllers/user.controller.js

+  const isValid = await bcrypt.compare(password, user.password);
+
+  if (!isValid) {
+    res.status(401).send('Invalid credentials');


After sending a response for an invalid old password, you must add a return statement. Without it, the function will continue to execute and will change the password even if the old one was incorrect.

mateacademy-ai-mentor · 2026-03-17T07:48:36Z

src/controllers/user.controller.js

+  user.password = await bcrypt.hash(password, 10);
+  user.resetToken = null;
+  user.save();
+  res.send('password changed');


The task requires showing a success page with a link to login. Simply sending the text 'password changed' is not sufficient. Consider redirecting to a success page or returning a more structured response that the client-side can use to render the correct view.

mateacademy-ai-mentor · 2026-03-17T07:48:36Z

src/controllers/user.controller.js

+    res.send('passwords not equil');
+  }
+
+  const user = await User.findOne({ where: { resetToken } });


You should check if a user was found with the provided resetToken. If user is null (because the token is invalid), the subsequent code will throw an error when trying to access user.password. Add a check for user and return an error response if it's not found.

mateacademy-ai-mentor · 2026-03-17T07:48:36Z

src/controllers/user.controller.js

+  const isValid = await bcrypt.compare(password, user.password);
+
+  if (!isValid) {
+    res.status(401).send('Invalid credentials');


You're missing return statements after sending error responses on lines 38, 46, 88, and 108. Without them, the function continues execution even after an error has been sent, leading to unexpected behavior. For example, here, an invalid password would still allow the email change process to continue.

mateacademy-ai-mentor · 2026-03-17T07:48:37Z

src/controllers/auth.controller.js

+  if (!existUser) {
+    res.send('User not found');


If the user does not exist, you send a response but do not stop the function's execution. The code will then attempt to access existUser.password on line 74, which will cause a TypeError because existUser is null. You need to add a return statement here.

mateacademy-ai-mentor · 2026-03-17T07:48:37Z

src/controllers/auth.controller.js

+  if (!isValid) {
+    res.send('Invalid credentials');


Similarly, if the password is not valid, you should return after sending the response. While it might not crash immediately in this case, continuing execution after sending a response is incorrect and can lead to unexpected behavior, such as trying to set headers after they have already been sent.

mateacademy-ai-mentor · 2026-03-17T07:48:37Z

src/index.js

+    app.use(cookieParser());
+    app.use(express.json()); // allow json in body
+    app.use(authRouter);
+    app.use(userRouter);


The task requires you to return a 404 error for any pages that are not defined by your routers. You should add a catch-all middleware here, after all other routes, to handle requests to non-existent pages.

mateacademy-ai-mentor · 2026-03-17T07:48:37Z

src/routes/auth.routes.js

+/* eslint-disable no-unused-vars */
+const express = require('express');
+const router = express.Router();
+const User = require('../models/user.model');


The User model is imported but never used in this file. It's good practice to remove any unused imports to keep the code clean.

mateacademy-ai-mentor · 2026-03-17T07:48:37Z

src/routes/auth.routes.js

+
+const { authController } = require('../controllers/auth.controller');
+const {
+  authMiddleware,


The authMiddleware is imported here but it's not used on any of the routes in this file. This import can be safely removed.

mateacademy-ai-mentor

Review Summary

Excellent progress on this PR! You've fixed all the critical server-crashing bugs from the last review, which is a huge step forward.

I'm requesting a few changes, primarily to align the password reset flow with the task requirements before this can be approved.

Required Changes:

Password Reset Flow: In src/controllers/user.controller.js, the resetPasswordConfirm function needs two adjustments to meet the requirements:
1. It should show a success page with a link to the login page, but it currently redirects the user directly to /login.
2. The code expects a repeatPassword field, but the requirement specifies it should be named confirmation.

Minor Suggestions:

HTTP Status Codes: For better API design, consider sending specific error status codes. For example, in src/controllers/auth.controller.js, use res.status(404).send('User not found') instead of sending a plain message with the default 200 OK status.
Database Sync: A quick note on sequelize.sync({ force: true }) in src/index.js — be very careful with this, as it drops all tables on every server restart. It's fine for early development but should be removed for production.

You're very close to finishing this. Just address the password reset flow, and it will be ready to go!

✨ 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-17T08:19:31Z

src/controllers/auth.controller.js

+    'Activate email',
+    `go to link http://localhost:3000/activation/${uuid}`,
+  );
+  res.send(201);


While res.send(201) works, it's more idiomatic in Express to be explicit with your status codes. Consider using res.sendStatus(201) or res.status(201).send('User created'). This makes the code's intent clearer.

mateacademy-ai-mentor · 2026-03-17T08:19:31Z

src/controllers/auth.controller.js

+  const user = await User.findOne({ where: { activationToken } });
+
+  if (!user) {
+    return res.status(404).send('invalind activation token');


There's a small typo here. It should be "invalid".

mateacademy-ai-mentor · 2026-03-17T08:19:31Z

src/controllers/auth.controller.js

+  const existUser = await User.findOne({ where: { email } });
+
+  if (!existUser) {
+    return res.send('User not found');


Sending a response without a status code defaults to 200 OK. For an error case like a user not being found, it's better to send an appropriate HTTP status code, such as 404 Not Found. This helps client-side applications correctly interpret the response.

Example: return res.status(404).send('User not found');

mateacademy-ai-mentor · 2026-03-17T08:19:31Z

src/controllers/auth.controller.js

+  const isValid = await bcrypt.compare(password, existUser.password);
+
+  if (!isValid) {
+    return res.send('Invalid credentials');


Similar to the point above, for invalid credentials, the standard HTTP status code is 401 Unauthorized. Using the correct status code improves API consistency and makes it easier for clients to handle different types of errors.

Example: return res.status(401).send('Invalid credentials');

mateacademy-ai-mentor · 2026-03-17T08:19:31Z

src/controllers/user.controller.js

+  user.resetToken = null;
+  await user.save();
+
+  return res.redirect('/login');


The implementation for the password reset confirmation is solid. However, the requirement is to "Show Success page with a link to login". Currently, the user is redirected directly to the login page. Consider sending a success message instead, which the frontend can then display to the user along with a link to the login page.

mateacademy-ai-mentor · 2026-03-17T08:19:31Z

src/index.js

+    await sequelize.authenticate(); // connect to database
+    console.log('database connected');
+
+    await sequelize.sync({ force: true }); // create tables


Using { force: true } is useful for development as it resets the database on every server start. However, be very careful with this in a production environment, as it will drop all your tables and delete all data. For production apps, it's safer to use database migration tools to manage schema changes.

mateacademy-ai-mentor · 2026-03-17T08:19:31Z

src/middlewares/auth.middleware.js

+  const token = req.cookies.token;
+
+  if (!token) {
+    return res.status(401).send('unautorised');


There's a small typo here. It should be "unauthorized".

mateacademy-ai-mentor · 2026-03-17T08:19:31Z

src/routes/user.routes.js

+/* eslint-disable no-unused-vars */
+const express = require('express');
+const router = express.Router();
+const User = require('../models/user.model');


This User model is imported but never used within this file. It's a good practice to remove unused imports to keep the code clean.

mateacademy-ai-mentor

Review Summary

I am approving your solution as an educational override so you can keep making progress. You've done a fantastic job on the overall structure and successfully fixed the password reset flow from the last review!

There are just a couple of things to address:

In src/controllers/user.controller.js, the task specifies that the password confirmation field should be named confirmation. Your code currently uses confirm for changing the password and repeatPassword for resetting it. A quick update to these variable names will align it with the requirements.
In src/controllers/auth.controller.js, the user isn't logged in after activating their account. To create a smoother experience, you should issue a JWT and set the cookie right after activation, just like you do in the login function, before redirecting to the profile page.

These are minor but important fixes. Keep up the excellent 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-17T08:29:22Z

src/controllers/user.controller.js

+
+const changePassword = async (req, res) => {
+  const userId = req.user.id;
+  const { password, newPassword, confirm } = req.body;


According to the task requirements, this function should expect fields named newPassword and confirmation for changing the password. The current code is using confirm.

mateacademy-ai-mentor · 2026-03-17T08:29:22Z

src/controllers/user.controller.js

+
+const resetPasswordConfirm = async (req, res) => {
+  const { resetToken } = req.params;
+  const { password, repeatPassword } = req.body;


The task requirements specify that the password reset confirmation should expect password and confirmation fields. This implementation is using repeatPassword instead.

mateacademy-ai-mentor · 2026-03-17T08:29:22Z

src/controllers/auth.controller.js

+  user.isActivated = true;
+  user.activationToken = null;
+  await user.save();
+  res.redirect('http://localhost:3000/profile');


The task requires redirecting to the profile page after activation, which you've done. However, the profile page is protected and requires the user to be authenticated. Currently, after activation, the user isn't logged in, so they'll be blocked from accessing the profile.

To fix this, you should log the user in by creating a JWT and setting it as a cookie, just like you do in the login function, before redirecting them to their profile.

Solution

1cc457c

mateacademy-ai-mentor suggested changes Mar 17, 2026

View reviewed changes

Solution

dbc81da

in7264 requested a review from mateacademy-ai-mentor March 17, 2026 08:17

mateacademy-ai-mentor suggested changes Mar 17, 2026

View reviewed changes

Solution

da255fa

in7264 requested a review from mateacademy-ai-mentor March 17, 2026 08:27

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

View reviewed changes

		const { password, newPassword, confirm } = req.body;

		if (!confirm) {

Conversation

in7264 commented Mar 17, 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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants