feat: implement auth backend with EJS templates by Nazarii-Lesniak · Pull Request #256 · mate-academy/node_auth-app

Nazarii-Lesniak · 2026-03-27T12:42:55Z

No description provided.

…ress types

mateacademy-ai-mentor

Review Summary

Great work on this project! The code is well-structured and you've successfully implemented most of the authentication features. However, a couple of critical issues are preventing approval.

First, the email change confirmation feature is broken. In src/services/userService.ts, the activation function only looks up users by their primary email. When confirming a new email address, the lookup fails because the new address is stored in the newEmail field. You'll need to adjust the logic to also check for a user with a matching newEmail.

Second, the logout endpoint is not secure. The task requires this feature to be available only to authenticated users, but the /logout route in src/routes/authRoutes.ts is missing the authMiddleware. Please add the middleware to protect this route.

Once these issues are addressed, your solution will be in great shape!

✨ 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-27T12:47:43Z

src/routes/authRoutes.ts

+  authController.resetPasswordRequest,
+);
+
+authRouter.post('/logout', authController.logout);


According to the task description, the logout feature should be available to authenticated users only. This route should be protected by the authMiddleware to enforce this requirement.

mateacademy-ai-mentor · 2026-03-27T12:47:43Z

src/services/userService.ts

+  const user = await getByEmail(email);
+
+  if (!user) {
+    throw new Error('User not found');
+  }


The current logic for finding a user in the activation function will fail when a user is trying to confirm a new email address. The getByEmail function searches for a user by their primary email, but during an email change, the email parameter here is the new email address, which is stored in the newEmail field. You should adjust the logic to also check for a user with a matching newEmail if no user is found by their primary email address.

…ange, and strict typing

mateacademy-ai-mentor

Review Summary

Great job addressing the critical issues from the last review! Your project is very close to being complete. However, a couple of key requirements are not yet fully met, so I am requesting a few changes before approval.

404 Page Handling: The application needs to render a user-friendly HTML page for 404 errors. Currently, it sends a JSON response, which isn't suitable for a server-rendered application. Please update your errorMiddleware in src/middlewares/errorMiddleware.ts to check for a 404 status and render a dedicated 404 view.
Email Change Notification Timing: The notification to the user's old email address is sent as soon as the change is requested, not when it's confirmed. This is misleading. This notification logic should be moved from the updateProfile function to the activation function in src/services/userService.ts, so it's sent only after the new email is successfully verified.

Once these two issues are resolved, your project will be ready for approval. 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-03-27T13:28:03Z

src/controllers/userController.ts

+    await userService.updateProfile({ email, name });
+    response.redirect('/user/profile');
+  } catch (error) {
+    next(error);


The error handling here is inconsistent with updatePassword and updateEmail. In those functions, if an error occurs, you fetch the user's data and re-render the profile page with a specific error message. Here, you're passing the error to the next middleware, which will likely result in a generic error page or a JSON response. For a better and more consistent user experience, consider adopting the same pattern you used for the other update functions.

mateacademy-ai-mentor · 2026-03-27T13:28:04Z

src/controllers/authController.ts

+    await tokenService.removeToken(refreshToken);
+    response.clearCookie('refreshToken');
+
+    response.clearCookie('refreshToken');


You're calling response.clearCookie('refreshToken') twice. This line is redundant and can be removed.

mateacademy-ai-mentor · 2026-03-27T13:28:04Z

src/index.ts

+app.use((req, res, next) => {
+  next(ApiError.NotFound());
+});


The task requires a "404 for all the other pages". Since this is a server-side rendered application, sending a JSON error response is not the expected behavior. This should be handled by rendering a dedicated 404 page. You could add a check for error.status === 404 in your errorMiddleware to render a view.

mateacademy-ai-mentor · 2026-03-27T13:28:04Z

src/middlewares/errorMiddleware.ts

+    return response
+      .status(error.status)
+      .send({ message: error.message, errors: error.errors });


The task requires a "404 for all the other pages". Since this is a server-rendered application, sending a JSON response for a 404 error is not ideal for the user experience. You should add a check for when error.status is 404 and render a dedicated 404 page instead.

mateacademy-ai-mentor · 2026-03-27T13:28:04Z

src/services/userService.ts

+  if (user.newEmail === email) {
+    await prisma.user.update({
+      where: {
+        id: user.id,
+      },
+      data: {
+        email: user.newEmail,
+        newEmail: null,
+        activationToken: null,
+      },
+    });


When a user confirms their new email address, you update the user record here. This is the correct place to send the notification to the user's old email address, informing them that the change has been completed. You can capture the old email from the user.email property before this update query runs.

mateacademy-ai-mentor · 2026-03-27T13:28:04Z

src/services/userService.ts

+      throw new Error('Invalid credentials');
+    }
+
+    await emailService.sendMessageToChangeEmail(user.email);


This notification is being sent to the user's old email address as soon as they request a change, not when they confirm it. The requirement is to "notify the old email about the change," which implies the change has already been successfully confirmed. Please move this email sending logic to the activation function, where the new email address is actually confirmed and set.

mateacademy-ai-mentor · 2026-03-27T13:28:04Z

src/services/emailService.ts

+  const html = `
+    <h1>Email has been changed</h1>


The content of this email, 'Email has been changed,' is misleading. Based on the logic in userService.ts, this email is sent to the old address as soon as the change is requested, not after it's confirmed. A more accurate message would be something like, 'A request has been made to change the email address for your account.' This would prevent user confusion if they don't complete the confirmation process.

…tion logic

mateacademy-ai-mentor

Review Summary

Excellent work on this project! You've successfully implemented all the features and addressed the feedback from the previous review perfectly. The application's logic for authentication, user management, and error handling is solid.

I am approving your solution as an educational override so you can move forward, but there's one small configuration issue causing the build to fail. Your package.json is set to "type": "module", which tells Node.js to treat .js files as ES modules. However, your .eslintrc.js file uses CommonJS syntax (module.exports), causing a conflict.

To fix this, you just need to rename the file to explicitly mark it as a CommonJS module:
mv .eslintrc.js .eslintrc.cjs

This is a common configuration detail when mixing module types. Once you make that change, your project will be perfect. Great job overall!

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

2pasha

lgtm 🔥

Nazarii-Lesniak added 9 commits March 25, 2026 17:35

feat: initialize prisma schema and synchronize postgres database

f64659d

feat: implement core auth services and business logic

4064b66

refactor: rename jwtService to tokenService

d1eb9d9

feat: finalize auth and user controllers, resolve ESM imports and Exp…

e8a7dfa

…ress types

feat: implement isNormalizedUser type guard andadd middlewares

9571a7e

feat: setup express routing for auth and user controllers

b9fae21

feat: implement all EJS templates and connect them to controllers

72d86da

fix: install @prisma/adapter-pg and pg driver dependencies

d03ea0d

refactor: finalize auth system with security and SSR stability fixes

1b4da76

mateacademy-ai-mentor suggested changes Mar 27, 2026

View reviewed changes

refactor: finalize secure auth system with SSR support, safe email ch…

1226fb7

…ange, and strict typing

Nazarii-Lesniak requested a review from mateacademy-ai-mentor March 27, 2026 13:22

mateacademy-ai-mentor suggested changes Mar 27, 2026

View reviewed changes

fix: improve error handling, remove redundancy and fix email notifica…

1e959d4

…tion logic

Nazarii-Lesniak requested a review from mateacademy-ai-mentor March 27, 2026 16:23

mateacademy-ai-mentor suggested changes Mar 27, 2026

View reviewed changes

Nazarii-Lesniak force-pushed the develop branch from b143e43 to c8ff035 Compare March 27, 2026 17:27

chore: definitively remove old eslint config from PR

653d585

Nazarii-Lesniak force-pushed the develop branch from c8ff035 to 653d585 Compare March 28, 2026 07:54

Nazarii-Lesniak added 3 commits March 28, 2026 10:12

fix: switch to CommonJS and fix ESLint config conflict

8844a4a

fix: resolve eslint parsing errors by switching to module.exports

3efeaf7

fix: update projectType to typescript and fix ESLint overrides

9a978ec

Nazarii-Lesniak requested a review from mateacademy-ai-mentor March 28, 2026 08:34

2pasha approved these changes Mar 28, 2026

View reviewed changes

Conversation

Nazarii-Lesniak commented Mar 27, 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!

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!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

2pasha left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants