Develop by maximtsyrulnyk · Pull Request #260 · mate-academy/node_auth-app

maximtsyrulnyk · 2026-04-01T10:41:47Z

make some changes in "Auth App" project

mateacademy-ai-mentor

Review Summary

This is a great effort, and the structure of your services, controllers, and middleware is very well done. However, a couple of critical configuration issues are preventing the application from building and running correctly, so I am requesting changes.

First, the build is failing because src/controllers/auth.controller.js contains TypeScript syntax (e.g., request: ExpressRequest) but has a .js file extension. Please rename this file to src/controllers/auth.controller.ts to resolve the syntax error.

Second, the main server entry point, server.js, is importing the application from src/app.js. This app.js file is an incomplete version that is missing the user profile routes, a 404 handler, and correct error middleware. The fully functional application setup is in src/index.ts, which should be imported and used in server.js instead.

Once you fix these two issues, your application should be fully functional. Keep up the excellent 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.

mateacademy-ai-mentor · 2026-04-01T10:45:43Z

src/app.js

@@ -0,0 +1,16 @@
+import express from 'express';
+import cors from 'cors';
+import authRoutes from './routes/auth.routes.js';


This import is incorrect for two reasons. First, the router in src/routes/authRoutes.ts is a named export (authRouter), so a named import like import { authRouter } from './routes/authRoutes.js' should be used. Second, the filename auth.routes.js seems incorrect.

mateacademy-ai-mentor · 2026-04-01T10:45:43Z

src/app.js

+app.use(cors());
+app.use(express.json());
+
+app.use('/api/auth', authRoutes);


The user profile routes are missing. According to the task description, the application must allow users to manage their profiles, but the necessary routes (from userRoutes) have not been registered with the app instance.

mateacademy-ai-mentor · 2026-04-01T10:45:43Z

src/app.js

+
+export default app;
+
+import errorMiddleware from './middleware/error.middleware.js';


The import path for the error middleware is incorrect. Based on the project structure, the middleware is located in the middlewares directory, not middleware.

mateacademy-ai-mentor · 2026-04-01T10:45:43Z

src/app.js

+
+import errorMiddleware from './middleware/error.middleware.js';
+
+app.use(errorMiddleware);


This middleware is being applied after the app instance was exported on line 12. This means the exported module that server.js imports and uses will not have this error handler attached, and it will never be called.

maximtsyrulnyk · 2026-04-01T19:59:27Z

make changes according Reccommendations

mateacademy-ai-mentor

Review Summary

Excellent work on refactoring the application to use JavaScript and Sequelize! You've built a solid foundation, but there are several critical issues that prevent the application from running correctly and meeting all the requirements. Changes are needed before this can be approved.

Here are the main blockers:

Application Fails to Start: There's a circular dependency in src/db.js because it imports src/setup.js. This prevents the server from launching. Please remove this import, as setup.js is already correctly initialized in src/index.js.
Crashes on Core Features: The application crashes during user registration and password reset because the crypto module is used without being imported in src/services/userService.js and src/controllers/userController.js.
Security Vulnerability: In src/services/jwtService.js, the access and refresh tokens are created without an expiration date. Please add an expiresIn option to jwt.sign() to make them temporary.
Missing Requirements: The task requires a 404 handler for unknown routes in src/createServer.js, and password change/reset functionality in src/controllers/userController.js must validate a confirmation field. These features are currently missing.
Incomplete Authentication: The logout route in src/routes/userRoutes.js is not protected by authMiddleware, and the middleware itself in src/middlewares/authMiddleware.js doesn't attach the decoded user to the request object, which is crucial for protected routes.

Once these issues are addressed, your application will 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.

mateacademy-ai-mentor · 2026-04-01T20:03:34Z

src/createServer.js

+  // Use routes for the respective paths
+
+  app.use('/users', userRoutes);
+


According to the task description, the application must include a "404 for all the other pages". You should add a middleware here, after all other routes, to catch any requests that haven't been handled and respond with a 404 status.

mateacademy-ai-mentor · 2026-04-01T20:03:34Z

src/config/prisma.js

+import { PrismaClient } from '@prisma/client';
+
+const prisma = new PrismaClient();
+
+export default prisma;


This Prisma client configuration seems to be unused in the project. The application consistently uses Sequelize for all database operations (as seen in src/db.js, models, and services). It's recommended to remove this file to avoid confusion and keep the project's dependencies clean.

mateacademy-ai-mentor · 2026-04-01T20:03:35Z

src/controllers/userController.js

+import { userService } from '../services/userService.js';
+import { User } from '../models/userModel.js';
+import bcrypt from 'bcrypt';
+// import crypto from 'crypto';


The crypto module is used for password reset functionality on lines 328 and 359, but the import statement for it is commented out. This will lead to a ReferenceError when the password reset endpoints are called. Please uncomment this line to make the module available.

mateacademy-ai-mentor · 2026-04-01T20:03:35Z

src/controllers/userController.js

+};
+
+export const resetUserPassword = async (req, res) => {
+  const { newPassword } = req.body;


The task requirements state that the password reset confirmation page must have password and confirmation fields that are equal. This function only accepts a newPassword from the request body. You need to add a confirmationPassword to the body and validate that it matches newPassword before proceeding with the password update.

mateacademy-ai-mentor · 2026-04-01T20:03:35Z

src/controllers/userController.js

+
+export const changeUserPassword = async (req, res) => {
+  const { id } = req.params;
+  const { oldPassword, newPassword } = req.body;


According to the requirements, changing a user's password requires an old one, a new password, and a confirmation. This function is missing the confirmation field and the necessary validation to ensure it matches the newPassword.

mateacademy-ai-mentor · 2026-04-01T20:03:35Z

src/services/jwtService.js

+  return token;
+}
+
+function verifyRefresh(user) {


The parameter here is named user, but the function expects a refresh token string. It should be renamed to token for better code readability.

mateacademy-ai-mentor · 2026-04-01T20:03:35Z

src/services/jwtService.js

+function verifyToken(token) {
+  try {
+    return jwt.verify(token, ACCESS_SECRET);
+  } catch {
+    return null;
+  }
+}


This function, verifyToken, is identical to the verify function on line 43. Having duplicate logic can be confusing and hard to maintain. Please remove this redundant function and use the verify function instead for verifying access tokens.

mateacademy-ai-mentor · 2026-04-01T20:03:35Z

src/services/tokenService.js

+export const removeByUserId = (userId) => {
+  return Token.destroy({
+    where: { UserId: userId },
+  });
+};


This function removeByUserId has the exact same implementation as the remove function on line 21. Having duplicate logic can lead to confusion and maintenance issues. Since the logout controller already uses remove, this function is redundant and can be removed to keep the service concise.

mateacademy-ai-mentor · 2026-04-01T20:03:35Z

src/services/tokenService.js

+export const tokenService = {
+  save,
+  getByToken,
+  removeByUserId,


To complete the cleanup, this redundant function removeByUserId should also be removed from the exported service object.

mateacademy-ai-mentor · 2026-04-01T20:03:35Z

src/services/userService.js

+};
+
+const create = async ({ name, email, password, activated }) => {
+  const activationToken = crypto.randomBytes(32).toString('hex');


The native crypto module is used here to generate a random token, but it has not been imported in this file. This will cause a ReferenceError and crash the application when a new user is created. Please import crypto at the top of the file.

…ts to .env. Improved configuration handling

maximtsyrulnyk · 2026-04-02T10:39:55Z

Added expiresIn for access & refresh tokens
Moved BASE_URL and secrets to .env
Improved configuration handling

mateacademy-ai-mentor

Review Summary

I am approving your submission as an educational override to help you move forward, as there are a few critical issues preventing the app from running. You've done an excellent job fixing the authentication middleware, securing the JWTs, and implementing the 404 handler!

However, the application currently crashes on startup and during registration. To get it working, please address these two points:

In src/index.js, you need to import the setup file at the very top to load your environment variables: import './setup.js';.
In src/services/userService.js, the crypto module is used but never imported, which causes a crash. Adding import crypto from 'crypto'; will fix this.

Once those are resolved, you can focus on implementing the remaining features like returning tokens on login and building out the profile management flows. You're building a solid foundation, 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-04-02T10:42:51Z

src/controllers/userController.js

+export const register = async (req, res) => {
+  const { name, email, password } = req.body;
+
+  const user = await userService.create({ name, email, password });


The task requires password validation rules to be checked during registration. This check should be performed here before calling userService.create, or the service should handle it and return an error if the password is invalid.

mateacademy-ai-mentor · 2026-04-02T10:42:51Z

src/controllers/userController.js

+    return res.status(400).json({ message: 'Wrong password' });
+  }
+
+  res.json({ message: 'Login success' });


A successful login should provide the client with authentication tokens (access and refresh) to enable access to protected routes. Instead of just returning a success message, you need to generate these tokens using the jwtService and include them in the response.

mateacademy-ai-mentor · 2026-04-02T10:42:51Z

src/controllers/userController.js

+  }
+
+  res.json({ message: 'Login success' });
+};


The task requires a password reset flow and a profile page where users can update their details. This controller is missing the handler functions for these features, such as requestPasswordReset, resetPassword, getProfile, and updateProfile.

mateacademy-ai-mentor · 2026-04-02T10:42:51Z

src/index.js

@@ -1 +1,14 @@
-'use strict';
+import { createServer } from './createServer.js';


To ensure environment variables from your .env file are available throughout the application, you need to import the setup file here, at the very beginning of your application's entry point. Add import './setup.js'; before the other imports.

mateacademy-ai-mentor · 2026-04-02T10:42:51Z

src/routes/userRoutes.js

+userRoutes.post('/logout', authMiddleware, (req, res) => {
+  res.json({ message: 'Logged out' });
+});


It's great that you've protected this route with authMiddleware. To make the logout functionality complete, the user's refresh token should be invalidated on the server side. Consider moving this logic into a logout function in your userController. This controller should then use the tokenService to remove the refresh token associated with the user's session.

mateacademy-ai-mentor · 2026-04-02T10:42:51Z

src/routes/userRoutes.js

+
+userRoutes.post('/logout', authMiddleware, (req, res) => {
+  res.json({ message: 'Logged out' });
+});


This file is missing several required routes. Based on the task description, you need to add endpoints for the password reset flow (requesting a reset and confirming the new password) and for the profile page (changing name, password, and email).

mateacademy-ai-mentor · 2026-04-02T10:42:51Z

src/services/userService.js

+import bcrypt from 'bcrypt';
+
+const create = async ({ name, email, password }) => {
+  const activationToken = crypto.randomBytes(32).toString('hex');


The application will crash here because the crypto module is used without being imported. This was a critical issue from the previous review. Please add import crypto from 'crypto'; at the top of the file to fix this.

mateacademy-ai-mentor · 2026-04-02T10:42:51Z

src/services/userService.js

+  return User.create({
+    name,
+    email,
+    password: hashedPassword,
+    activationToken,
+  });


According to the task requirements, an activation email must be sent upon registration. This is the perfect place to trigger that action. After the user is created successfully, you should call your emailService to send the email with the activationToken.

mateacademy-ai-mentor · 2026-04-02T10:42:51Z

src/services/userService.js

+export const userService = {
+  create,
+  getByEmail,
+  getByActivationToken,
+  activate,
+};


This service is missing several key functions required by the task description. You'll need to add methods to handle the password reset flow (generating a token, finding a user by it, and updating the password) and profile management features (updating name and password).

maximtsyrulnyk added 3 commits March 31, 2026 21:45

doing project, but have some problems

bfcd299

make some changes but have troubles

5dee2dc

make a lot of changes

03e30be

mateacademy-ai-mentor suggested changes Apr 1, 2026

View reviewed changes

make changes

4b4be31

maximtsyrulnyk requested a review from mateacademy-ai-mentor April 1, 2026 19:59

mateacademy-ai-mentor suggested changes Apr 1, 2026

View reviewed changes

maximtsyrulnyk added 3 commits April 2, 2026 12:45

make updates but have troubles with code

c5794c8

Added expiresIn for access & refresh tokens. Moved BASE_URL and secre…

bb542c3

…ts to .env. Improved configuration handling

Added expiresIn for access & refresh tokens. Moved BASE_URL and secre…

77d4764

…ts to .env. Improved configuration handling

maximtsyrulnyk requested a review from mateacademy-ai-mentor April 2, 2026 10:40

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

View reviewed changes


		export default app;

		import errorMiddleware from './middleware/error.middleware.js';


		import errorMiddleware from './middleware/error.middleware.js';

		app.use(errorMiddleware);

		// Use routes for the respective paths

		app.use('/users', userRoutes);

		@@ -1 +1,14 @@
		'use strict';
		import { createServer } from './createServer.js';

Conversation

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

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

maximtsyrulnyk commented Apr 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!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants