Task Solution by Vovanchu · Pull Request #221 · mate-academy/node_auth-app

Vovanchu · 2026-01-03T10:43:18Z

No description provided.

mateacademy-ai-mentor

Review Summary

Good work on the overall structure of the application, especially the database models and JWT service. However, there are several critical issues that block core functionality and prevent the project from meeting its requirements, so I am requesting changes.

First, the application will crash on startup due to a typo in src/index.js: cokieParser() should be cookieParser(). A major security flaw is that all user profile routes in src/router/user.router.js are missing authentication middleware, leaving them unprotected. Additionally, profile updates are broken because the database queries in src/services/user.services.js use the wrong field (where: { id } instead of where: { userId: id }), and the JWT payload is missing the userId needed to identify the user making requests.

Please address these blocking issues to ensure the application is secure and functional. I'm looking forward to reviewing your next submission!

✨ 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.

src/controller/auth.controller.js

mateacademy-ai-mentor · 2026-01-03T10:45:56Z

src/router/auth.router.js

+router.get('/refresh', authController.refresh);
+router.get('/logout', authController.logout);
+router.post('/forgot', authController.forgot);
+router.post('/password-reset', authController.resetPassword);


This route is mapped to the resetPassword controller, but the controller function expects a resetToken from the URL parameters (req.params). Since this route definition doesn't include a token parameter, the controller will fail to find the user.

mateacademy-ai-mentor · 2026-01-03T10:45:56Z

src/router/auth.router.js

+router.get('/logout', authController.logout);
+router.post('/forgot', authController.forgot);
+router.post('/password-reset', authController.resetPassword);
+router.post('/password-reset/:resetToken', authController.updatePassword);


This route handler points to authController.updatePassword, which is incorrect. The updatePassword function is part of the userController and is used by authenticated users. The correct handler for resetting a password with a token is authController.resetPassword. It appears the handlers for this route and the one above have been confused.

src/services/user.services.js

src/services/email.services.js

mateacademy-ai-mentor

Review Summary

Great job addressing the server crash and fixing the database queries from the last review. The core services are looking much more solid. However, several critical issues are preventing key features from working, so changes are needed before this can be approved.

Here are the main blockers:

Missing Authentication: The user profile routes in src/router/user.router.js are still unprotected. They must have an authentication middleware to secure the endpoints and make req.user available to the controllers.
Multiple Responses: In src/controller/auth.controller.js, the activateUser and loginUser functions attempt to send multiple responses for a single request (e.g., res.send() and then res.redirect()), which will crash the server. Please ensure each handler sends only one response.
Incorrect User ID: The controllers in src/controller/user.controller.js are using req.user.id, but the JWT payload provides req.user.userId. This needs to be corrected to make profile updates work.
Broken Route: The password reset route in src/router/auth.router.js is missing the :resetToken parameter, which breaks the feature. It should be defined as /password-reset/:resetToken.
Invalid Import Path: The User model import path in src/controller/user.controller.js is incorrect and will cause a startup crash. It should be ../models/User.model.js.

Once these issues are resolved, the application should be fully functional. Keep up the great work!

✨ 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.

src/controller/user.controller.js

src/controller/auth.controller.js

src/router/auth.router.js

src/router/user.router.js

src/services/email.services.js

mateacademy-ai-mentor

Review Summary

Great progress on this project! I'm approving this submission as an educational override to help you move forward, but there are a few critical issues you'll need to address for the application to work correctly.

You've done an excellent job securing the user routes and fixing the password reset logic. However, the application currently crashes on login because the loginUser function in src/controller/auth.controller.js attempts to send two responses (generateTokens and res.redirect()). Remember, an API endpoint should only send one response, and the client should handle redirection.

Additionally, a typo in an import path within src/middlewares/auth.mildware.js (jwt.service.js instead of jwt.services.js) will prevent the server from starting. Also, note that the updateName function sends back the wrong data because User.update returns the count of updated rows, not the user object itself.

Fixing these will make your application fully functional. Keep up the fantastic 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-03T11:21:00Z

src/controller/auth.controller.js

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


This function still attempts to send two responses for a single request, which was a critical issue from the previous review. The generateTokens function on the line above already sends a response containing the user data and access token. Calling res.redirect() here will cause a server crash because headers have already been sent. The client should be responsible for redirecting the user to the profile page after a successful login.

mateacademy-ai-mentor · 2026-01-03T11:21:00Z

src/controller/user.controller.js

+    const user = await userServices.updateNameService(userId, name);
+
+    res.send(user);


The userServices.updateNameService method uses User.update, which returns an array containing the number of affected rows (e.g., [1]), not the updated user object. As a result, res.send(user) sends this array to the client.

To fix this and maintain consistency with updatePassword and updateEmail, you should first find the user, update their name, and then save the changes. This will allow you to return either the updated user object or a success message.

mateacademy-ai-mentor · 2026-01-03T11:21:00Z

src/middlewares/auth.mildware.js

@@ -0,0 +1,24 @@
+import { jwtService } from '../services/jwt.service.js';


This import path contains a typo. The service file is named jwt.services.js (plural), but here it is imported from jwt.service.js (singular). This will cause a module not found error and crash the application on startup.

mateacademy-ai-mentor · 2026-01-03T11:21:00Z

src/services/user.services.js

+const updateNameService = (id, name) =>
+  User.update({ name }, { where: { userId: id } });


The User.update method returns an array containing the number of affected rows (e.g., [1]), not the updated user object. Your user.controller.js is currently sending this array as the response when updating a name.

To provide a consistent and useful API response, consider changing the implementation to match your password and email update logic: first, find the user record, then update its properties, and finally, save the changes.

Task Solution

57338a3

mateacademy-ai-mentor suggested changes Jan 3, 2026

View reviewed changes

Fix all mistake

3eaf253

Vovanchu requested a review from mateacademy-ai-mentor January 3, 2026 11:02

mateacademy-ai-mentor suggested changes Jan 3, 2026

View reviewed changes

Fix mistake again

c03e011

Vovanchu requested a review from mateacademy-ai-mentor January 3, 2026 11:19

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

View reviewed changes

		const user = await userServices.updateNameService(userId, name);

		res.send(user);

		@@ -0,0 +1,24 @@
		import { jwtService } from '../services/jwt.service.js';

		const updateNameService = (id, name) =>
		User.update({ name }, { where: { userId: id } });

Conversation

Vovanchu commented Jan 3, 2026

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mateacademy-ai-mentor Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

mateacademy-ai-mentor Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants