Solution by alyonashunevych · Pull Request #250 · mate-academy/node_auth-app

alyonashunevych · 2026-03-13T17:21:46Z

No description provided.

mateacademy-ai-mentor

Review Summary

This is a very solid and well-structured project, and you've successfully implemented most of the features. However, I'm requesting changes because there are a few critical issues that prevent the application from running and meeting key security requirements.

Here are the main points to address:

The server will not start. Your nodemon.json and src/server/app.js are configured to run TypeScript files with the standard node runtime, which will cause a crash. You need to use a TypeScript runner like tsx or ts-node in your exec command.
Insecure Profile Updates: The backend endpoint in src/server/controllers/users.controller.ts does not validate the user's current password when changing their password or email. This is a critical security vulnerability and a direct requirement of the task.
Client Imports Server Code: The client-side component src/client/components/FormTemplate.tsx directly imports code from the src/server directory. Client and server code must be completely separate; this needs to be refactored.
Missing Email Confirmation: The form for changing an email in src/client/components/ChangeEmailForm.tsx is missing the required confirmation field to prevent typos.

Please address these issues to make your application functional and secure.

✨ 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-13T17:27:50Z

nodemon.json

+  "watch": ["src"],
+  "ext": "ts,js,json",
+  "ignore": ["node_modules"],
+  "exec": "node src/server/app.js"


The exec command node src/server/app.js will cause an error because app.js imports .ts files directly. The standard node runtime cannot handle TypeScript modules. Consider using a tool like tsx or ts-node to run your server in development, which can execute TypeScript files on the fly. For example: "exec": "tsx src/server/app.js".

mateacademy-ai-mentor · 2026-03-13T17:27:50Z

src/client/App.tsx

+          <Routes>
+            <Route path="/" element={<HomePage />} />
+            <Route path="sign-up" element={<SignUpPage />} />
+            <Route
+              path="auth/activation/:email/:activationToken"
+              element={<AccountActivationPage />}
+            />
+            <Route
+              path="reset-password/:resetToken"
+              element={<ResetPassPage />}
+            />
+            <Route path="reset-password" element={<ResetPassPage />} />
+            <Route path="login" element={<LoginPage />} />
+
+            <Route path="/" element={<RequireAuth />}>
+              <Route path="profile" element={<ProfilePage />} />
+              <Route
+                path="profile/change/:paramToChange"
+                element={<ChangePage />}
+              />
+            </Route>


While the individual page components handle the redirection for authenticated users, a more consistent approach would be to use a layout route with the RequireNonAuth component to protect all non-authenticated routes. This would centralize your authorization logic and align with how you're using RequireAuth for protected routes.

Consider wrapping SignUpPage, AccountActivationPage, ResetPassPage, and LoginPage in a parent route that uses RequireNonAuth as its element.

mateacademy-ai-mentor · 2026-03-13T17:27:50Z

src/client/components/ChangeEmailForm.tsx

+const fields = {
+  newEmail: { type: 'email', label: 'New email' },
+} as Fields;


According to the task requirements, changing an email should include a confirmation step for the new email address. This form currently only has a single newEmail field. You should add a second field (e.g., confirmNewEmail) and add validation to ensure both email fields match before allowing the form to be submitted.

mateacademy-ai-mentor · 2026-03-13T17:27:50Z

src/client/components/FormTemplate.tsx

+import React from 'react';
+import cn from 'classnames';
+import { Link, useNavigate } from 'react-router-dom';
+import { userService } from '../../server/services/user.service.ts';


Client-side components should not import code directly from the server directory. This is a significant architectural issue that can lead to security vulnerabilities and incorrect application bundling. Validation logic, if needed on both client and server, should be placed in a shared directory that both can access, or the validation rules should be duplicated on the client side.

mateacademy-ai-mentor · 2026-03-13T17:27:50Z

src/client/http/httpClient.ts

+
+// add `Authorization` header to all requests
+httpClient.interceptors.request.use((request) => {
+  const accessToken = localStorage.getItem('accessToken');


You have an accessTokenService that abstracts away the details of token storage. For better consistency and maintainability, it would be ideal to use accessTokenService.get() here instead of accessing localStorage directly.

mateacademy-ai-mentor · 2026-03-13T17:27:50Z

src/client/pages/ProfilePage.tsx

+          <Link to={`/profile/change/${item}`} key={item}>
+            <li className="button menu-item-box">
+              <p style={{ margin: 0 }}>Change {item}</p>
+              <i className="fa-sharp-duotone fa-solid fa-chevron-right"></i>
+            </li>
+          </Link>


For semantically correct HTML, the <Link> component should be nested inside the <li> element, not the other way around. A <ul> element's direct children should only be <li> elements. The correct structure would be <li><Link to={...}>...</Link></li>.

mateacademy-ai-mentor · 2026-03-13T17:27:50Z

src/server/controllers/users.controller.ts

+    throw ApiError.badRequest('Email is already taken');
+  }
+
+  const activationToken = bcrypt.genSaltSync(1);


Using bcrypt.genSaltSync(1) is not a secure or appropriate method for generating an activation token. This function is designed to create salts for password hashing, not for generating unique, unpredictable tokens. You should use a cryptographically secure random string generator, like crypto.randomBytes(32).toString('hex'), which is already used correctly elsewhere in the application for registration.

mateacademy-ai-mentor · 2026-03-13T17:27:50Z

src/server/controllers/users.controller.ts

+  switch (type) {
+    case 'name':
+      dataToUpdate = await checkNewName(req, user);
+      break;
+
+    case 'password':
+      dataToUpdate = await checkNewPassword(req);
+      break;
+
+    case 'email':
+      await checkNewEmail(req, user);
+      res.sendStatus(204);
+
+      return;
+
+    default:
+      throw ApiError.badRequest('Invalid update type');
+  }


The task requires users to provide their current password when changing their password or email. While the client-side handles this in a separate step, this API endpoint does not enforce it. For security, the logic within the 'password' and 'email' cases should also require the user's current password in the request body and validate it before making any changes.

mateacademy-ai-mentor · 2026-03-13T17:27:50Z

src/server/controllers/auth.controller.ts

+const activate: RequestHandler = async (req, res) => {
+  const { token } = req.params;
+  const tokenString = Array.isArray(token) ? token[0] : token;
+
+  const user = await usersRepository.getByActivationToken(tokenString);
+
+  if (!user) {
+    throw ApiError.notFound();
+  }
+
+  const updatedUser = await usersRepository.update(user.id, {
+    email: user.pendingEmail ?? user.email,
+    pendingEmail: null,
+    activationToken: null,
+  });
+
+  await sendAuthentication(res, updatedUser);
+
+  if (user.pendingEmail) {
+    await mailer.sendEmailChangeNotification(user.email, user.pendingEmail);
+  }
+};


The activate function should verify that the activation token belongs to the user with the email address provided in the URL. Currently, it only uses the token to find the user, ignoring the email parameter from the request. You should retrieve both email and token from req.params and ensure the user found by the token also has a matching email or pendingEmail.

mateacademy-ai-mentor · 2026-03-13T17:27:51Z

src/server/app.js

+import { authRouter } from './routes/auth.router.ts';
+import { errorMiddleware } from './middlewares/errorMiddleware.ts';
+import { userRouter } from './routes/user.router.ts';
+import { authMiddleware } from './middlewares/authMiddleware.ts';


This server file is a JavaScript file (.js), but it's importing TypeScript files (.ts) directly. The standard node runtime cannot execute TypeScript code. This will cause a runtime error.

To fix this, you should either:

Rename this file to app.ts and use a TypeScript runner like tsx or ts-node to start your server.

Compile your TypeScript files to JavaScript and import the compiled .js files instead.

mateacademy-ai-mentor

Review Summary

Excellent progress on the project! The application is fully functional and very well-structured. However, I am requesting changes for two critical issues before I can approve it.

First, there is a security vulnerability on the backend. The API endpoint for changing a user's email does not validate their current password. While the UI has a check, the server must always be secure independently. In src/server/controllers/users.controller.ts, please add password validation inside the case 'email' block, just as you have for the case 'password' block.

Second, the build is failing due to a linting configuration error. Your project's package.json is configured with "type": "module", but your .eslintrc.cjs file uses CommonJS syntax. To fix this, you should rename it to .eslintrc.js and update its content to use ES Module syntax (export default).

Once these two blockers are resolved, your work will be ready for approval. You are very close to finishing!

✨ 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-15T21:39:56Z

src/client/components/AuthContext.tsx

+import React, { useMemo, useState } from 'react';
+import { accessTokenService } from '../services/accessTokenService';
+import { authService } from '../services/authService';
+import { type User } from '@prisma/client';


While using a type import prevents this package from being bundled with your client-side code, it's not ideal to directly reference server-specific packages like @prisma/client in the frontend. This creates a tight coupling between your client and your database schema.

A better practice is to define a dedicated User type within your client's types directory that describes the shape of the user data your API returns. This keeps your client and server codebases decoupled.

mateacademy-ai-mentor · 2026-03-15T21:39:56Z

src/client/pages/App.tsx

+            <Route path="/" element={<HomePage />} />
+            <Route path="sign-up" element={<SignUpPage />} />
+            <Route
+              path="auth/activation/:email/:activationToken"
+              element={<AccountActivationPage />}
+            />
+            <Route
+              path="reset-password/:resetToken"
+              element={<ResetPassPage />}
+            />
+            <Route path="reset-password" element={<ResetPassPage />} />
+            <Route path="login" element={<LoginPage />} />


The task requires that pages like the homepage, sign-up, and login should be accessible only to non-authenticated users. This routing setup allows logged-in users to access these pages. You should wrap these routes in a route guard component (like RequireNonAuth) to redirect authenticated users away from them.

mateacademy-ai-mentor · 2026-03-15T21:39:56Z

src/client/pages/App.tsx

+            <Route
+              path="reset-password/:resetToken"
+              element={<ResetPassPage />}
+            />
+            <Route path="reset-password" element={<ResetPassPage />} />


Defining two separate routes for the password reset page is redundant. A cleaner approach with react-router-dom v6 is to use nested routes to handle the optional :resetToken parameter, which makes the routing configuration more concise.

mateacademy-ai-mentor · 2026-03-15T21:39:56Z

src/client/services/authService.ts

@@ -0,0 +1,45 @@
+import { authClient as client } from '../http/authClient';
+import type { User } from '@prisma/client';


Importing types directly from @prisma/client in your frontend service creates a tight coupling between the client and the server's database layer. Your client should only be aware of the API contract (the shape of the JSON it receives), not the backend's implementation details like the ORM it uses.

To fix this, you should define a User type within your client's src/client/types directory. This type will describe the user object as your API returns it, decoupling your frontend from the backend's dependencies.

mateacademy-ai-mentor · 2026-03-15T21:39:56Z

src/server/controllers/users.controller.ts

+    case 'email':
+      await checkNewEmail(req, user);
+      res.sendStatus(204);
+
+      return;


While the password change flow now correctly validates the user's current password, this check is missing for the email change flow. The task requirement is: "To change an email you should type the password...".

To meet this requirement and secure the endpoint, you must add a password validation step here, just like you did for the 'password' case. The API endpoint that performs the data change must always be secure on its own, regardless of the client-side flow.

mateacademy-ai-mentor · 2026-03-15T21:39:56Z

src/server/types/vite-env.d.ts

+interface ImportMetaEnv {
+  readonly VITE_API_URL: string;
+  [key: string]: string;
+}
+
+// eslint-disable-next-line no-shadow
+interface ImportMeta {
+  readonly env: ImportMetaEnv;
+}


This type declaration file is for Vite environment variables, which are used on the client side. It should be located in the src/client directory, not in src/server, to maintain a proper separation of concerns between your frontend and backend code.

mateacademy-ai-mentor

Review Summary

Excellent work on this project, especially for fixing the critical backend security vulnerability! I am approving your solution as an educational override to help you move forward, but there are a few important issues to address to get your build passing and fully meet the requirements.

The main issue is a build failure caused by your ESLint configuration. To fix this, you need to update .eslintrc.js to use ES Module syntax (export default instead of module.exports) and add parserOptions to correctly handle TypeScript and JSX.

// .eslintrc.js
export default {
  // ... existing configuration
  parser: '@typescript-eslint/parser',
  parserOptions: {
    ecmaVersion: 'latest',
    sourceType: 'module',
    ecmaFeatures: {
      jsx: true,
    },
  },
  // ... rest of the configuration
};

Additionally, the password reset page is currently accessible to logged-in users, but it should be for non-authenticated users only. Please wrap the route in src/client/pages/App.tsx with your RequireNonAuth component to enforce this. You've built a very solid application, and these final adjustments will make it perfect!

✨ 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-15T22:08:59Z

.eslintrc.js

 module.exports = {
-  extends: '@mate-academy/eslint-config',
+  extends: [
+    '@mate-academy/eslint-config',
+    'plugin:@typescript-eslint/recommended',
+  ],
+  parser: '@typescript-eslint/parser',
  env: {
-    jest: true
+    jest: true,
  },
+  plugins: ['jest', '@typescript-eslint'],
  rules: {
-    'no-proto': 0
+    'no-proto': 0,
  },
-  plugins: ['jest']
 };


Good job renaming the file! However, since your project is configured to use ES Modules ("type": "module" in package.json), this configuration file should also use ES Module syntax. Please change module.exports to export default.

mateacademy-ai-mentor · 2026-03-15T22:08:59Z

src/client/pages/App.tsx

+            <Route path="/" element={<HomePage />} />
+            <Route path="sign-up" element={<SignUpPage />} />
+            <Route
+              path="auth/activation/:email/:activationToken"
+              element={<AccountActivationPage />}
+            />
+            <Route
+              path="reset-password/:resetToken"
+              element={<ResetPassPage />}
+            />
+            <Route path="reset-password" element={<ResetPassPage />} />
+            <Route path="login" element={<LoginPage />} />


These routes are accessible to authenticated users, which goes against the requirements. Pages like sign-up, activation, login, and password reset should only be for non-authenticated users. Consider wrapping these routes in a component like RequireNonAuth to redirect logged-in users.

mateacademy-ai-mentor · 2026-03-15T22:08:59Z

src/client/pages/ResetPassPage.tsx

+  const [done, setDone] = useState(false);
+  const { resetToken } = useParams();
+  const { currentUser, checkAuth } = useAuth();
+  const wasAuthenticated = Boolean(currentUser);


The task requirements specify that the password reset functionality should be available only to non-authenticated users. This implementation handles both authenticated and unauthenticated users, which is not required.

Please update the routing in App.tsx to ensure this page is only accessible when the user is not logged in. After that, you can simplify the logic in this component by removing all checks and effects related to currentUser and wasAuthenticated.

alyonashunevych added 2 commits March 13, 2026 19:20

Solution

04ec630

Solution

e11bc08

mateacademy-ai-mentor suggested changes Mar 13, 2026

View reviewed changes

Solution

35a691e

alyonashunevych requested a review from mateacademy-ai-mentor March 15, 2026 21:35

mateacademy-ai-mentor suggested changes Mar 15, 2026

View reviewed changes

Solution

4e6dd0d

alyonashunevych requested a review from mateacademy-ai-mentor March 15, 2026 22:04

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

View reviewed changes

		@@ -0,0 +1,45 @@
		import { authClient as client } from '../http/authClient';
		import type { User } from '@prisma/client';

Conversation

alyonashunevych commented Mar 13, 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!

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