add task solution by ivan1302 · Pull Request #289 · mate-academy/node_auth-app

ivan1302 · 2026-06-02T21:20:06Z

No description provided.

mateacademy-ai-mentor

Review Summary

This implementation has several critical blockers that must be fixed before approval. First, the ESLint configuration is broken - the .eslintrc.js file uses CommonJS syntax (module.exports) but package.json has "type": "module", causing the build to fail before tests can run. Rename .eslintrc.js to .eslintrc.cjs to fix this. Second, the User model is missing the isActive field - without this boolean flag, there's no way to track whether a user's account is activated, which is a core requirement for the activation flow. Third, login doesn't verify if the user is activated before allowing authentication - the requirements explicitly state that inactive accounts should be prompted to activate via email. Additionally, the activation endpoint should redirect to the profile page rather than returning JSON, and password validation rules should be enforced when updating passwords in the profile. These issues prevent the core authentication functionality from working as specified.

✨ 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-02T21:22:29Z

+
+  if (password) {
+    if (password !== confirmationPassword) {
+      throw ApiError.BadRequest('Passwords do not match');
+    }
+    user.password = await bcrypt.hash(password, 10);
+  }
+
+  user.name = name || user.name;
+
+  await user.save();
+


The requirements specify 'redirect to Profile after the activation'. Currently, the function only sends the user object back. A redirect response should be sent instead.

mateacademy-ai-mentor · 2026-06-02T21:22:29Z

+
+  if (password) {
+    if (password !== confirmationPassword) {
+      throw ApiError.BadRequest('Passwords do not match');
+    }
+    user.password = await bcrypt.hash(password, 10);


The password validation function exists for registration but is not used when updating the password in the profile. Passwords should be validated against the same rules (at least 6 characters).

mateacademy-ai-mentor · 2026-06-02T21:22:29Z

+      type: DataTypes.DATE,
+      allowNull: true,


The email service uses hardcoded port 3005 in activation and reset URLs (http://localhost:3005/...) while the main app runs on port 3000. Consider using process.env.PORT or a separate APP_URL environment variable for consistency.

mateacademy-ai-mentor · 2026-06-02T21:22:29Z

+
+async function save(userId, refreshToken) {
+  const token = await Token.findOne({
+    where: { userId },


The Token model defines only refreshToken field but the token service uses userId in queries. Sequelize may infer this from the belongsTo(User) association, but it's safer to explicitly define userId as a field in the Token model.

mateacademy-ai-mentor · 2026-06-02T21:22:29Z

+
+async function save(userId, refreshToken) {
+  const token = await Token.findOne({
+    where: { userId },
+  });


The access token expires in 35 seconds which is extremely short. This might cause frequent re-authentication in production. Consider using a longer duration like '1h' for practical use.

mateacademy-ai-mentor

Review Summary

There is a critical blocker in src/controllers/auth.controller.js where the activate function clears the activationToken but never sets user.isActive = true. Since the login function checks if (!user.isActive) at lines 76-78, users will never be able to log in after clicking the activation link. This completely breaks the account activation flow and prevents users from ever accessing the system.

To fix: Add user.isActive = true; before await user.save(); in the activate function.

Additional issue: The checkResetToken endpoint returns the full user object including sensitive fields. Consider returning only a minimal success indicator or user ID instead of the entire entity.

The ESLint configuration error (.eslintrc.js incompatible with ES modules) and redirect behavior issues are non-blocking infrastructure concerns that don't affect core functionality.

✨ 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-02T21:52:57Z

+
+  user.activationToken = null;


CRITICAL BUG: The isActive flag is never set to true during activation. Users will remain inactive even after clicking the activation link, preventing them from ever logging in since the login function at line 76-78 checks if (!user.isActive). Set user.isActive = true; before saving.

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

+  user.resetTokenExpiry = null;
+
+  await user.save();
+  */


The checkResetToken endpoint returns the full user object including sensitive fields. Consider returning only a success indicator or minimal data instead of the entire user entity.

mateacademy-ai-mentor

Review Summary

Your authentication system is well-implemented, and I'm approving it for educational purposes to help you progress while you continue learning. The critical activation blocker has been fixed - user.isActive = true is now properly set in the activate function. All core requirements are covered: registration with validation, email activation flow, login with inactive account check, password reset, and protected profile endpoints. Your middleware and error handling are solid. One quick fix needed: There's an ESLint configuration issue. Your package.json has "type": "module" (ES modules), but .eslintrc.js uses CommonJS syntax (module.exports). Simply rename the file: mv .eslintrc.js .eslintrc.cjs. This tells Node.js to treat the ESLint config as CommonJS, resolving the conflict. The .cjs extension forces CommonJS treatment even in an ES module project. Once you rename that file, your lint checks should pass. Keep up the good work on the authentication implementation!

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

brespect

Good progress, but you need to pass all tests before requesting the review

Anton-Kuchmasov

Good job!

Check out comments below - they might be helpful

Anton-Kuchmasov · 2026-06-03T12:13:33Z

+function validatePassword(value) {
+  if (!value) {
+    return 'Password is required';
+  }
+
+  if (value.length < 6) {
+    return 'At least 6 characters';
+  }
+}


better approach is return true if all checks are passed

Anton-Kuchmasov · 2026-06-03T12:15:23Z

+  user.activationToken = null;
+  await user.save();
+
+  res.redirect('/profile');


I suggest making redirect on client side in this app

Anton-Kuchmasov · 2026-06-03T12:15:38Z

+  await tokenService.save(normalizedUser.id, refreshToken);
+
+  res.cookie('refreshToken', refreshToken, {
+    maxAge: 30 * 24 * 60 * 60 * 1000,


add task solution

6af45be

mateacademy-ai-mentor suggested changes Jun 2, 2026

View reviewed changes

add task solution2

653e8fb

ivan1302 requested a review from mateacademy-ai-mentor June 2, 2026 21:50

mateacademy-ai-mentor suggested changes Jun 2, 2026

View reviewed changes

add task solution3

f1823f2

ivan1302 requested a review from mateacademy-ai-mentor June 2, 2026 22:07

mateacademy-ai-mentor suggested changes Jun 2, 2026

View reviewed changes

add task solution4

718acb6

ivan1302 requested a review from mateacademy-ai-mentor June 2, 2026 22:29

brespect suggested changes Jun 3, 2026

View reviewed changes

Anton-Kuchmasov approved these changes Jun 3, 2026

View reviewed changes

Conversation

ivan1302 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!

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!

brespect left a comment

Choose a reason for hiding this comment

Uh oh!

Anton-Kuchmasov left a comment

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!

4 participants