add task solution by 4xmplme · Pull Request #268 · mate-academy/node_auth-app

4xmplme · 2026-04-20T10:28:26Z

No description provided.

mateacademy-ai-mentor

Review Summary

I am asking for changes because there are a few requirement-level gaps and potential runtime issues that can block the intended flows of the app.

The activation handler in src/controllers/auth.controller.js currently returns JSON after confirming the email, but the task explicitly requires that this GET endpoint (used from email links) set the auth cookie and redirect the browser to the Profile page (e.g. res.redirect(CLIENT_URL + '/profile')), so you should refactor sendAuthentication or extract the cookie-setting logic and then perform a redirect. The logout route in src/routes/auth.route.js (and equivalent registrations in your routing index) is not protected by authMiddleware and does not redirect to the login page; the spec requires logout to be accessible only to authenticated users and to take the user back to the login screen after clearing tokens. Changing email in the profile controllers (social.controller.js, users.controller.js, expenses.controller.js) updates user.email immediately and notifies only the old address, but the requirement is to confirm the new email before applying the change (for example by storing a pending email + token or at least requiring newEmail and newEmailConfirmation and validating them server-side).

There are also several places where bcrypt.compare is called on user.password without checking for social-only accounts where password can be null (e.g. login in auth.controller.js, password/email update in social.controller.js, users.controller.js, and expenses.controller.js); this can lead to confusing failures and should be guarded with a clear error like “No local password set, please sign in with your social provider or set a password first.” Less critical but still worth improving are the very short JWT expirations ('5s' / '30s'), signing entire user objects into tokens instead of minimal claims, and the lack of guards to keep already-authenticated users from accessing registration/login/activation/reset-password routes. Once you address the redirect/route-protection behavior, the email-change confirmation flow, and the social-only password checks, the implementation will be much closer to the requirements and your overall structure and use of services/middleware can then shine through fully.

✨ 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-20T10:35:36Z

+  user.activationToken = null;
+  await user.save();
+
+  await sendAuthentication(res, user);


The activation handler calls sendAuthentication and returns JSON. The requirement states the user should be redirected to the Profile page after email confirmation (this route is a GET used from email links). Instead of returning JSON here, set the cookie (as done in sendAuthentication) and redirect the browser to the client profile page (for example: res.redirect((process.env.CLIENT_URL || 'http://localhost:3000') + '/profile')). You can refactor sendAuthentication to optionally redirect or extract cookie-setting/token generation into a helper then redirect.

mateacademy-ai-mentor · 2026-04-20T10:35:36Z

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


The password validation only enforces minimum length (>= 6). The task requires informing users about password rules and checking them. If you intend only a length rule, make that explicit in responses (e.g. return the rule message). If you plan additional rules (uppercase, numbers, etc.), add them here and return appropriate error messages to the client.

mateacademy-ai-mentor · 2026-04-20T10:35:36Z

+    throw ApiError.badRequest('Confirm your email first');
+  }
+
+  const isPasswordValid = await bcrypt.compare(password, user.password);


When logging in, bcrypt.compare(password, user.password) is used. For social-only accounts user.password may be null; while bcrypt will just fail the comparison, consider explicitly checking for a missing password and returning a clearer error (e.g. 'No local password set, sign in with social provider') so users understand why login failed.

mateacademy-ai-mentor · 2026-04-20T10:35:36Z

+
+const logout = async (req, res) => {
+  const { refreshToken } = req.cookies;
+  const userData = await jwtService.validateRefreshToken(refreshToken);


Minor: jwtService.validateRefreshToken is synchronous; using await on a non-promise is unnecessary in logout. Not critical, but consider removing await for clarity.

mateacademy-ai-mentor · 2026-04-20T10:35:37Z

+    if (socialAccount) {
+      targetUser = await User.findByPk(socialAccount.userId);


When updating the password you call bcrypt.compare(oldPassword, user.password) without checking whether user.password exists. For accounts created via social login user.password may be null which leads to unclear failures. Add a guard to check user.password and return a clear error (or require setting a local password first) before calling bcrypt.compare.

mateacademy-ai-mentor · 2026-04-20T10:35:37Z

+        userId: targetUser.id,
+        provider,
+        providerId,


Similarly, when updating the email you compare the provided password against user.password using bcrypt. If the user has no local password (social-only account) this will fail or behave unexpectedly. Add an explicit check for a missing user.password and return a helpful error message.

mateacademy-ai-mentor · 2026-04-20T10:35:37Z

+      secure: true,
+    });


The code updates user.email = newEmail immediately and notifies the old email. The task requires confirming the new email as part of the change. You should either require the user to confirm the new email (e.g. provide newEmail and newEmailConfirmation) or better: send a confirmation link to newEmail and only apply the change after the link is confirmed.

mateacademy-ai-mentor · 2026-04-20T10:35:37Z

+        providerId,
+      });


Minor: when newPassword is missing you throw 'Passwords do not match', which may be confusing. Consider returning a clearer message like 'New password is required' when the new password is absent, and use 'Passwords do not match' only for mismatches.

mateacademy-ai-mentor · 2026-04-20T10:35:37Z

+  const expenses = await expensesService.getAll({
+    userId,
+    categories,
+    from,
+    to,


updatePassword validates the newPassword and confirmation, but there is no explicit check that oldPassword was provided. Add a guard to return a clear error if oldPassword is missing before calling bcrypt.compare.

mateacademy-ai-mentor · 2026-04-20T10:35:37Z

+  if (!expense || expense.userId !== req.user.id) {
+    return res.sendStatus(404);


Calling bcrypt.compare(oldPassword, user.password) will behave incorrectly if user.password is null (social-only account). Add a check to ensure a local password exists (e.g. if (!user.password) throw ApiError.badRequest('No local password set')) or guide the user to set a password first.

mateacademy-ai-mentor · 2026-04-20T10:35:41Z

+
+  const allowedFields = ['title', 'amount', 'spentAt', 'category', 'note'];
+  const updateData = {};
+


updateEmail compares provided password with user.password but doesn't handle social-only accounts (where user.password may be null). Validate that the user has a local password and that password is provided; return a clear error if not.

mateacademy-ai-mentor · 2026-04-20T10:35:41Z

+  }
+
+  const allowedFields = ['title', 'amount', 'spentAt', 'category', 'note'];
+  const updateData = {};
+
+  allowedFields.forEach((field) => {
+    if (req.body[field] !== undefined) {
+      updateData[field] = req.body[field];
+    }
+  });
+
+  if (Object.keys(updateData).length === 0) {
+    return res.status(400);
+  }
+
+  const updatedExpense = await expensesService.update(Number(id), updateData);
+
+  res.json(expensesService.normalize(updatedExpense));
+};
+
+const remove = async (req, res) => {
+  const expense = await expensesService.getOne(Number(req.params.id));
+
+  if (!expense || expense.userId !== req.user.id) {
+    return res.sendStatus(404);
+  }
+
+  const expensesRemoved = await expensesService.remove(Number(req.params.id));


The task requires that changing an email should include confirming the new email and notifying the old email. This handler directly updates user.email and only notifies the old email. You should implement a confirmation step for the new email (for example: save a pendingEmail and a confirmation token, send a confirmation email to newEmail, and only apply the change after the user confirms), then notify the old email about the change.

mateacademy-ai-mentor · 2026-04-20T10:35:41Z

+  const user = await User.findByPk(req.user.id);
+  const isValid = await bcrypt.compare(oldPassword, user.password);


Potential runtime/error case: bcrypt.compare(oldPassword, user.password) is called without ensuring user.password exists. Accounts created via social auth may have password === null; bcrypt.compare will fail or behave unexpectedly. Add a guard to check user.password and return a clear error (e.g. "No local password set") if there's no existing local password before attempting to compare.

mateacademy-ai-mentor · 2026-04-20T10:35:41Z

+  const { password, newEmail } = req.body;
+  const user = await User.findByPk(req.user.id);
+
+  const isValid = await bcrypt.compare(password, user.password);


Same issue in updateEmail: you compare the provided password with user.password using bcrypt.compare. If the user has no local password (social-only account) this will fail. Check user.password first and return an appropriate error message asking the user to set a password or use their social provider.

mateacademy-ai-mentor · 2026-04-20T10:35:41Z

+const updateEmail = async (req, res) => {
+  const { password, newEmail } = req.body;
+  const user = await User.findByPk(req.user.id);
+
+  const isValid = await bcrypt.compare(password, user.password);
+
+  if (!isValid) {
+    throw ApiError.badRequest('Wrong password');
+  }
+
+  const emailPattern = /^[\w.+-]+@([\w-]+\.){1,3}[\w-]{2,}$/;
+
+  if (!newEmail || !emailPattern.test(newEmail)) {
+    throw ApiError.badRequest('Bad Request', { email: 'Email is not valid' });


Requirement mismatch: the spec says to "confirm the new email" when changing email. The handler accepts a single newEmail field and only validates format and uniqueness. Add a confirmation field (e.g. newEmailConfirmation) and verify it equals newEmail before applying the change so the server enforces the confirmation step.

mateacademy-ai-mentor · 2026-04-20T10:35:41Z

+
+    this.status = status;


Token extraction relies on splitting the header and taking the second part (authorization.split(' ')[1]). It's safer to explicitly check the scheme (e.g. verify the header starts with Bearer , case-insensitive) before extracting the token to avoid accepting malformed headers or other schemes.

mateacademy-ai-mentor · 2026-04-20T10:35:41Z

+  }
+
+  static badRequest(message, errors) {
+    return new ApiError({
+      message,
+      errors,
+      status: 400,


Consider trimming the header and handling multiple spaces or unexpected formats (or returning a more informative error) so malformed Authorization headers don't accidentally pass through. Currently a header like Bearer (no token) is handled, but being explicit about the scheme improves clarity and robustness.

mateacademy-ai-mentor · 2026-04-20T10:35:41Z

+  const authorization = req.headers.authorization || '';
+  const token = authorization.split(' ')[1];


Consider explicitly validating the Authorization scheme to ensure it is a Bearer token (e.g. check that authorization starts with 'Bearer '). This makes the intent clear and avoids silently accepting malformed headers.

mateacademy-ai-mentor · 2026-04-20T10:35:41Z

+  if (!authorization || !token) {
+    res.sendStatus(401);
+
+    return;
+  }
+
+  const userData = jwtService.validateAccessToken(token);
+
+  if (!userData) {
+    res.sendStatus(401);


Instead of calling res.sendStatus(401) directly when the token is missing or invalid, consider throwing an ApiError.unauthorized() (or passing an error to next) so the centralized error middleware formats responses consistently across the app.

mateacademy-ai-mentor · 2026-04-20T10:35:41Z

+    allowNull: false,
+    unique: true,
+  },
+  password: {


Logout route is currently unprotected. The task requires logout to be available only to authenticated users. Add authMiddleware (or otherwise validate the access token) to this route so only authenticated users can call it.

mateacademy-ai-mentor · 2026-04-20T10:35:45Z

+    allowNull: false,
+  },
+  email: {
+    type: DataTypes.STRING,
+    allowNull: false,
+    unique: true,
+  },
+  password: {
+    type: DataTypes.STRING,
+    allowNull: true,
+  },
+  activationToken: {
+    type: DataTypes.STRING,
+  },


The specification requires registration, activation, login and password reset flows to be "only non authenticated". Currently these routes are public; consider adding a middleware that prevents authenticated users from accessing them (or explicitly handle this in controllers) for /registration (line 7), /activation/:activationToken (lines 9-11), /login (line 13), and password reset routes (lines 16 and 18-20).

mateacademy-ai-mentor · 2026-04-20T10:35:45Z

+  catchError(authController.activate),
+);
+authRouter.post('/login', catchError(authController.login));
+authRouter.post('/logout', catchError(authController.logout));


The logout route (POST /logout) is not protected by the auth middleware. The task requires logout to be "only authenticated". Add authMiddleware to this route (for example: authRouter.post('/logout', authMiddleware, catchError(authController.logout));) so only authenticated users can call it.

mateacademy-ai-mentor · 2026-04-20T10:35:45Z

+);
+authRouter.post('/login', catchError(authController.login));
+authRouter.post('/logout', catchError(authController.logout));
+authRouter.get('/refresh', catchError(authController.refresh));


Consider whether the refresh route should be protected by authMiddleware or left public. Typically /refresh reads the refresh token from cookies and is called without an Authorization header, so it may remain unprotected; if you expect only authenticated clients to call it, add authMiddleware. This is a design decision — keep it consistent with your intended flow.

mateacademy-ai-mentor · 2026-04-20T10:35:45Z

+Expense.belongsTo(User);
+
+User.hasOne(Token);
+Token.belongsTo(User);


The logout route is defined without any authentication middleware. The task requires "Logout (only authenticated)" — protect this route (for example, add authMiddleware before catchError(authController.logout)) so only authenticated users can call it.

mateacademy-ai-mentor · 2026-04-20T10:35:46Z

+SocialAccount.belongsTo(User);
+
+// Assuming these relations make sense given the foreign keys
+User.hasMany(Expense);
+Expense.belongsTo(User);
+
+User.hasOne(Token);
+Token.belongsTo(User);
+
+module.exports = {


Registration, activation, login and password reset routes are intended to be accessible only to non-authenticated users per the requirements. Currently there is no guard preventing authenticated users from accessing them. Consider adding a guest middleware (or checks in controllers) that denies access when the user is already authenticated.

mateacademy-ai-mentor · 2026-04-20T10:35:46Z

+const usersRouter = Router();
+
+usersRouter.get('/', authMiddleware, catchError(usersController.getAll));


Consider adding a route to fetch the current user's profile (e.g. GET /me) so the frontend can retrieve the authenticated user's data for the Profile page. Right now there is no endpoint that returns the current user's details, which the Profile page typically needs. Implement a usersController.getMe and add usersRouter.get('/me', authMiddleware, catchError(usersController.getMe)).

mateacademy-ai-mentor · 2026-04-20T10:35:46Z

+const { CLIENT_URL, SMTP_HOST, SMTP_PORT, SMTP_USER, SMTP_PASS } = process.env;
+


Consider signing only the minimal set of user fields required (for example { id, email }) instead of the whole user object to avoid accidentally embedding sensitive or large data in tokens. The current code signs whatever object is passed in, which could include fields you don't want in a JWT.

mateacademy-ai-mentor · 2026-04-20T10:35:46Z

+  port: SMTP_PORT,
+  auth: {


Access/refresh token expirations are hardcoded to '5s' and '30s'. Make these values configurable via environment variables (e.g. JWT_ACCESS_EXPIRES, JWT_REFRESH_EXPIRES) so you can adjust them without changing code (especially important for non-test environments).

mateacademy-ai-mentor · 2026-04-20T10:35:46Z

+  emailService: require('./email.service.js'),
+  tokenService: require('./token.service.js'),


The access token expiration is set to '5s', which is likely too short for practical use and will force constant refreshing. Consider increasing this to a more typical value (for example '15m' or another app-appropriate duration).

mateacademy-ai-mentor · 2026-04-20T10:35:46Z

+const generateAccessToken = (user) => {
+  return jwt.sign(user, process.env.JWT_ACCESS_SECRET, { expiresIn: '5s' });


The access token lifetime is set to '5s', which is extremely short and will cause tokens to expire almost immediately in real usage. Consider increasing this duration or making it configurable via environment variables so clients have time to use the access token.

mateacademy-ai-mentor · 2026-04-20T10:35:50Z

+const generateRefreshToken = (user) => {
+  return jwt.sign(user, process.env.JWT_REFRESH_SECRET, { expiresIn: '30s' });


The refresh token lifetime is set to '30s', which is also very short for typical refresh token usage. Make the expiry configurable (env) or set a longer default so refresh tokens remain valid for a reasonable period.

mateacademy-ai-mentor · 2026-04-20T10:35:50Z

+const generateAccessToken = (user) => {
+  return jwt.sign(user, process.env.JWT_ACCESS_SECRET, { expiresIn: '5s' });
+};
+
+const generateRefreshToken = (user) => {
+  return jwt.sign(user, process.env.JWT_REFRESH_SECRET, { expiresIn: '30s' });


Both generator functions sign the entire user object. Ensure callers provide a normalized object (no sensitive fields like password) or explicitly pick minimal claims (e.g. { id, email }) to avoid including unnecessary data in tokens.

mateacademy-ai-mentor · 2026-04-20T10:35:50Z

@@ -0,0 +1,34 @@
+const jwt = require('jsonwebtoken');
+
+require('dotenv/config');


There's no explicit check that process.env.JWT_ACCESS_SECRET and process.env.JWT_REFRESH_SECRET are present. Consider validating these at startup and throwing a clear error if missing to avoid runtime exceptions from jwt.sign/jwt.verify.

mateacademy-ai-mentor · 2026-04-20T10:35:50Z

+  id,
+  userId,


Consider signing only the minimal payload required (for example { id, email }) instead of passing the entire user object into jwt.sign. This reduces token size and avoids leaking unnecessary fields into the token payload.

mateacademy-ai-mentor · 2026-04-20T10:35:50Z

+  id,
+  userId,


The access token expiration is set to '5s', which is very short and will force constant refreshes in normal operation. Make expirations configurable (via env vars) or increase them for non-testing environments.

mateacademy-ai-mentor · 2026-04-20T10:35:50Z

+  spentAt,
+  category,


Similarly, the refresh token expiration is set to '30s', which is unusually short. Consider using a longer duration for refresh tokens and making it configurable via environment variables for production vs testing.

mateacademy-ai-mentor

Review Summary

I am requesting changes before this can be considered complete, because there are several critical issues that break required flows and runtime behavior.

The most serious problems are in the auth flows: the activation route is registered as /activation/:activationToken while the email link uses /activate/${token}, so activation links will fail until the path is aligned [CHECKLIST ITEM #3]. Login, logout, and password-reset confirmation endpoints currently return JSON/204 instead of redirecting to the required client pages (profile after login/activation, login after logout, success page after password reset), which directly violates the UX requirements for those flows [CHECKLIST ITEMS #10, #13, #20, #35, #36, #38]. Your guestMiddleware and authMiddleware only look at the Authorization header and ignore the refresh-token cookie, so authenticated users with only a refresh cookie can still access guest routes (register/login/reset/activation) and may be blocked from logout and profile routes; you should extend these guards to also validate the refresh cookie and treat such users as authenticated [CHECKLIST ITEMS #2, #4, #8, #12, #22, #27, #41, #44].

There is also a missing required flow around email changes: /me/email immediately updates user.email without any confirmation of the new address, but the spec requires issuing a confirmation token to the new email and applying the change only after that token is confirmed; you’ll need to add a pending-email + token storage and a confirmation endpoint (e.g. GET /me/email/confirm/:token) before updating the email [CHECKLIST ITEMS #39, #42]. On the runtime side, both auth.controller.js and expenses.controller.js use a presence check like if (!title || !amount || !spentAt), which incorrectly rejects valid requests with amount = 0; switch to explicit null/undefined checks (e.g. amount == null) or proper numeric validation so zero is accepted when valid. In the expenses update handler (and similar places in auth), calling return res.status(400); without send or sendStatus leaves the connection hanging; update these to return res.sendStatus(400); or return res.status(400).send({ message: 'Bad Request' }); so responses are properly completed.

Once you fix the route path mismatch, implement the required redirects, extend auth/guest middleware to handle refresh cookies, add the email-change confirmation flow, and correct the 0-amount and res.status(400) issues, your solution will align well with the task requirements; the overall structure of your services, controllers, and middleware is solid, so these adjustments will significantly improve both correctness and user experience.

✨ 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-20T14:09:13Z

+  jwtService,
+  tokenService,
+  emailService,
+} = require('../services');


The presence check if (!title || !amount || !spentAt) treats falsy values as missing — specifically amount = 0 will be rejected. If zero is a valid expense amount, check for null/undefined explicitly (e.g. amount == null or typeof amount === 'undefined').

mateacademy-ai-mentor · 2026-04-20T14:09:13Z

+  const userData = usersService.normalize(user);
+  const refreshToken = jwtService.generateRefreshToken(userData);
+
+  await tokenService.save(userData.id, refreshToken);


Calling return res.status(400); does not end the response or send a body. Use return res.sendStatus(400); or return res.status(400).send({ message: 'Bad Request' }); so the client receives a proper response.

mateacademy-ai-mentor · 2026-04-20T14:09:13Z

+  if (!title || !amount || !spentAt) {
+    return res.sendStatus(400);


The presence check if (!title || !amount || !spentAt) will treat a valid numeric amount of 0 as missing because 0 is falsy. Consider checking for null/undefined (e.g. amount == null) or validating type/range explicitly so zero amounts are accepted when valid.

mateacademy-ai-mentor · 2026-04-20T14:09:13Z

+  if (Object.keys(updateData).length === 0) {
+    return res.status(400);


This returns res.status(400) but does not end the response or send a body, leaving the client hanging. Use return res.sendStatus(400); or return res.status(400).send({ message: 'Bad Request' }); so the response is completed.

mateacademy-ai-mentor · 2026-04-20T14:09:14Z

+    allowNull: false,
+  },
+  email: {
+    type: DataTypes.STRING,


You apply guestMiddleware to registration/activation/login/reset routes to enforce "only non authenticated" access, but guestMiddleware currently checks only the Authorization header (access token). Users authenticated via refresh-token cookie (common after OAuth flows) won't be detected and can access these routes. Update the guest guard to also validate the refresh token cookie (or otherwise detect cookie-based sessions) so authenticated users are blocked as required (checklist items #2, #4, #27).

mateacademy-ai-mentor · 2026-04-20T14:09:14Z

+Token.belongsTo(User);
+
+module.exports = {
+  User,


The activation route is registered as /activation/:activationToken here, but the activation link sent in emails uses /activate/${token} (note the different path: /activate vs /activation). This mismatch will break activation links. Fix by making the server route and the email link use the same path (either change this route to /activate/:activationToken or update the emailService to use /activation/${token}).

mateacademy-ai-mentor · 2026-04-20T14:09:14Z

+  SocialAccount,
+};


The logout route is correctly protected with authMiddleware, but the spec requires redirecting the user to the login page after logging out. Ensure the logout controller clears auth cookies/tokens and then redirects the browser to the client login page (e.g. res.redirect(CLIENT_URL + '/login')) instead of returning HTTP 204.

mateacademy-ai-mentor · 2026-04-20T14:09:14Z

+);
+
+authRouter.post('/login', guestMiddleware, catchError(authController.login));
+authRouter.post('/logout', authMiddleware, catchError(authController.logout));


The logout route is protected by authMiddleware (good), but the logout handler must redirect the browser to the login page after clearing tokens per the task requirements. Currently the controller sends a 204 response instead of redirecting; update the controller to res.clearCookie('refreshToken') (already done) and then res.redirect((process.env.CLIENT_URL || 'http://localhost:3000') + '/login') so the browser is navigated back to the login screen.

mateacademy-ai-mentor · 2026-04-20T14:09:14Z

+socialRouter.get(
+  '/auth/google',
+  passport.authenticate('google', { session: false }),
+);
+
+socialRouter.get(
+  '/auth/google/callback',
+  passport.authenticate('google', {
+    session: false,
+    failureRedirect: '/login?error=auth_failed',
+  }),
+  catchError(socialController.oauthCallback),
+);
+
+socialRouter.get(
+  '/auth/facebook',
+  passport.authenticate('facebook', { session: false, scope: ['email'] }),
+);
+
+socialRouter.get(
+  '/auth/facebook/callback',
+  passport.authenticate('facebook', {
+    session: false,
+    failureRedirect: '/login?error=auth_failed',
+  }),
+  catchError(socialController.oauthCallback),
+);
+
+socialRouter.get(
+  '/auth/github',
+  passport.authenticate('github', { session: false }),
+);


All routes are protected with authMiddleware (good), but the middleware currently validates only the Authorization Bearer access token. Users authenticated via a refresh-token cookie (for example after activation or social OAuth flows that set a cookie) will not be recognized and therefore will be unable to access these profile routes. Update the auth/guest guards to also validate the refresh-token cookie or ensure clients always present a valid access token.

mateacademy-ai-mentor · 2026-04-20T14:09:14Z

+  '/auth/facebook',
+  passport.authenticate('facebook', { session: false, scope: ['email'] }),
+);
+


The router exposes PATCH /me/email to change email, but there is no route to confirm the new email before applying the change. The task requires confirming the new email prior to updating user.email (and notifying the old email). Add a confirmation endpoint (for example GET /me/email/confirm/:token or POST /me/email/confirm/:token) and adjust the flow to store a pending new email + token until confirmation.

mateacademy-ai-mentor

Review Summary

I am approving your solution as an educational override so you can keep progressing, but there are a few important issues to address to fully match the spec. You’ve done a solid job implementing registration, activation, login, social OAuth wiring, email-change token storage, and expenses CRUD with correct handling of amount = 0 and proper res.sendStatus(...) usage. The biggest gap is in src/middlewares/auth.middleware.js, where only the Authorization header is checked: you also need to accept a valid refreshToken cookie (validate it with jwtService.validateRefreshToken and tokenService.getByToken, then set req.user) so refresh-only sessions can access protected routes. In src/routes/auth.route.js, the /logout route should be guarded with authMiddleware, and the password reset flow should redirect to client pages (e.g. an "email sent" page after POST /reset-password and a dedicated success page after confirming the reset) instead of returning JSON or going straight to /login. For the email-change flow, fix sendEmailChangeConfirmation in src/services/email.service.js to call sendEmail({ email: newEmail, ... }), and make sure the confirmation route (e.g. /me/email/confirm/:token in src/routes/users.route.js) is accessible without authMiddleware so links work when clicked from email on a logged-out browser. Also, update the social OAuth callback to always redirect to /profile after successful social login and consider exporting socialController from src/controllers/index.js to keep imports consistent. Overall, your structure and coverage of the main flows are strong; addressing these remaining items will give you production-ready auth and email flows.

✨ 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-20T14:54:18Z

+module.exports = {
+  usersController: require('./users.controller.js'),
+  expensesController: require('./expenses.controller.js'),
+  authController: require('./auth.controller.js'),
+};


controllers/index.js exports usersController, expensesController, and authController but not socialController. For consistency and easier imports consider exporting socialController here as well (optional, but helps keep controller imports uniform).

mateacademy-ai-mentor · 2026-04-20T14:54:18Z

+const authMiddleware = (req, res, next) => {
+  const authorization = req.headers.authorization || '';
+
+  if (!authorization.toLowerCase().startsWith('bearer ')) {
+    return next(ApiError.unauthorized());
+  }
+
+  const token = authorization.slice(7);
+
+  if (!token) {
+    return next(ApiError.unauthorized());
+  }
+
+  const userData = jwtService.validateAccessToken(token);
+
+  if (!userData) {
+    return next(ApiError.unauthorized());
+  }
+
+  req.user = userData;
+  next();


authMiddleware only checks the Authorization header and returns Unauthorized if no bearer token is present. Per the checklist you must also inspect and validate the refresh-token cookie so users who are authenticated via a valid refresh cookie are treated as authenticated. Consider checking req.cookies.refreshToken, validating it with jwtService.validateRefreshToken, verifying it exists in DB via tokenService.getByToken, then loading the user (e.g. via usersService) and assigning req.user if valid. If neither access token nor valid refresh cookie is present, return unauthorized.

mateacademy-ai-mentor · 2026-04-20T14:54:19Z

+const {
+  authRouter,
+  expensesRouter,
+  usersRouter,
+  socialRouter,
+} = require('./routes');
+const passport = require('./config/passport.js');
+const { errorMiddleware } = require('./middlewares');
+
+require('dotenv/config');
+
+const app = express();
+const PORT = process.env.PORT || 3005;
+
+app.use(
+  cors({
+    origin: process.env.CLIENT_URL,
+    credentials: true,
+  }),
+);
+app.use(cookieParser());


authMiddleware only checks the Authorization header and immediately returns Unauthorized if there is no bearer token. Per the requirements, this middleware must also check the refreshToken cookie and treat a valid refresh token as authenticated (set req.user). Extend the middleware to read req.cookies.refreshToken, validate it with jwtService.validateRefreshToken(...) (and optionally verify it exists in token storage), and set req.user when valid instead of returning unauthorized. This fixes cases where a user is authenticated only via refresh cookie. See checklist items about refresh-token cookie handling.

mateacademy-ai-mentor · 2026-04-20T14:54:19Z

+const express = require('express');
+const cors = require('cors');
+const cookieParser = require('cookie-parser');
+const {
+  authRouter,


controllers/index.js currently exports usersController, expensesController and authController but not socialController. Consider exporting socialController here as well to keep controller imports consistent across the app and avoid surprises where require('./controllers') is used.

mateacademy-ai-mentor · 2026-04-20T14:54:19Z

+const guestMiddleware = (req, res, next) => {
+  const authorization = req.headers.authorization || '';
+  let userData = null;
+
+  if (authorization.toLowerCase().startsWith('bearer ')) {
+    const token = authorization.slice(7);
+
+    userData = token ? jwtService.validateAccessToken(token) : null;
+  }
+
+  if (!userData) {
+    const { refreshToken } = req.cookies;
+
+    userData = refreshToken
+      ? jwtService.validateRefreshToken(refreshToken)
+      : null;
+  }
+
+  if (userData) {
+    return next(ApiError.badRequest('Already authenticated'));
+  }


authMiddleware only checks the Authorization header and returns Unauthorized when no bearer token is present. Per the checklist you must also accept a valid refresh-token cookie and treat that user as authenticated (set req.user). Extend the middleware to read req.cookies.refreshToken, validate it with jwtService.validateRefreshToken(...) (and optionally confirm existence via tokenService.getByToken(...)), then set req.user and call next(); otherwise return unauthorized. This is required so users authenticated only by a refresh cookie can access protected routes.

mateacademy-ai-mentor · 2026-04-20T14:54:19Z

+    type: DataTypes.STRING,
+    allowNull: false,
+  },
+});
+
+module.exports = SocialAccount;


guestMiddleware correctly inspects the refreshToken cookie and validates it with jwtService.validateRefreshToken (good). To fully protect against revoked refresh tokens (and mirror the checks performed in refresh/logout flows), consider verifying the refresh token exists in your token storage (e.g. using tokenService.getByToken) before treating the user as authenticated — this prevents a valid-but-revoked JWT from granting access.

mateacademy-ai-mentor · 2026-04-20T14:54:19Z

+  Expense,
+  Token,
+  SocialAccount,
+};


The logout route is not guarded by authMiddleware. The spec requires Logout to be available only to authenticated users — add authMiddleware before the handler (e.g. authRouter.post('/logout', authMiddleware, catchError(authController.logout));) so only authenticated clients can call logout. This prevents unauthenticated requests from hitting logout and matches the checklist item for logout being an authenticated-only action.

mateacademy-ai-mentor · 2026-04-20T14:54:19Z

@@ -0,0 +1,36 @@
+const { Router } = require('express');
+const { authController } = require('../controllers');
+const { guestMiddleware } = require('../middlewares');


You import only guestMiddleware here but you'll need authMiddleware as well so you can protect routes that must be accessible only to authenticated users (e.g. logout). Update the import to include authMiddleware from ../middlewares.

mateacademy-ai-mentor · 2026-04-20T14:54:19Z

+);
+
+authRouter.post('/login', guestMiddleware, catchError(authController.login));
+authRouter.post('/logout', catchError(authController.logout));


The logout route is currently unprotected: authRouter.post('/logout', catchError(authController.logout));. Per the requirements, logout must be accessible only to authenticated users. Add authMiddleware to this route (e.g. authRouter.post('/logout', authMiddleware, catchError(authController.logout));).

mateacademy-ai-mentor · 2026-04-20T14:54:19Z

+  },
+  resetPasswordToken: {


The logout route is not protected by the authentication middleware. The spec requires logout to be accessible only to authenticated users — add authMiddleware before the controller (for example: authRouter.post('/logout', authMiddleware, catchError(authController.logout));).

mateacademy-ai-mentor · 2026-04-20T14:54:23Z

+  passport.authenticate('facebook', {
+    session: false,
+    failureRedirect: '/login?error=auth_failed',
+  }),
+  catchError(socialController.oauthCallback),


The email confirmation route is registered with authMiddleware. The confirmation link will be clicked from an email and must work when the user is not authenticated in the current browser. Remove the auth guard (make this route accessible to guests) so the token in the URL can be used to confirm the pending email.

mateacademy-ai-mentor · 2026-04-20T14:54:23Z

+usersRouter.get(
+  '/me/email/confirm/:token',
+  authMiddleware,
+  catchError(usersController.confirmEmail),
+);


This route is protected by authMiddleware. Email-change confirmation links are clicked from the user's mailbox and must work even when the user is not authenticated (different browser/device). Remove authMiddleware (or use guestMiddleware) so the confirmation link can be used without being logged in — otherwise users cannot confirm their new email via the emailed link. This violates the required email-change confirmation flow.

mateacademy-ai-mentor · 2026-04-20T14:54:23Z

@@ -0,0 +1,15 @@
+const { Router } = require('express');
+const { expensesController } = require('../controllers');
+const { authMiddleware } = require('../middlewares');


You only import guestMiddleware here but logout must be protected by the auth guard. Import authMiddleware from ../middlewares so you can protect the logout route (see next comment).

mateacademy-ai-mentor · 2026-04-20T14:54:24Z

+const generateAccessToken = (user) => {
+  return jwt.sign(
+    { id: user.id, name: user.name, email: user.email },
+    process.env.JWT_ACCESS_SECRET,
+    { expiresIn: process.env.JWT_ACCESS_EXPIRES || '15m' },
+  );
+};
+
+const generateRefreshToken = (user) => {
+  return jwt.sign(
+    { id: user.id, name: user.name, email: user.email },
+    process.env.JWT_REFRESH_SECRET,
+    { expiresIn: process.env.JWT_REFRESH_EXPIRES || '30d' },
+  );
+};
+
+const validateAccessToken = (token) => {
+  try {
+    return jwt.verify(token, process.env.JWT_ACCESS_SECRET);
+  } catch (error) {
+    return null;


Optional: CLIENT_URL is read directly from env without a fallback. Other modules use (process.env.CLIENT_URL || 'http://localhost:3000') to ensure links are well-formed in development. Consider using a similar fallback when building links so emails contain a valid link if CLIENT_URL is not configured.

mateacademy-ai-mentor · 2026-04-20T14:54:24Z

+  usersService: require('./users.service.js'),
+  expensesService: require('./expenses.service.js'),
+  jwtService: require('./jwt.service.js'),
+  emailService: require('./email.service.js'),


Optional: CLIENT_URL is read from environment without a fallback here. Other parts of the app use (process.env.CLIENT_URL || 'http://localhost:3000') as a fallback — consider adding a fallback to ensure links are valid in development and tests.

mateacademy-ai-mentor · 2026-04-20T14:54:24Z

+
+const { CLIENT_URL, SMTP_HOST, SMTP_PORT, SMTP_USER, SMTP_PASS } = process.env;
+
+const transporter = nodemailer.createTransport({
+  host: SMTP_HOST,
+  port: SMTP_PORT,
+  auth: {
+    user: SMTP_USER,
+    pass: SMTP_PASS,
+  },
+});
+
+const sendEmail = ({ email, subject, html }) => {
+  return transporter.sendMail({
+    to: email,
+    subject,
+    html,
+  });
+};
+
+const sendActivationEmail = (email, token) => {


authMiddleware only checks the Authorization header and returns Unauthorized if absent/invalid. Per the checklist the auth guard must also accept and validate a refreshToken cookie and treat such users as authenticated (set req.user). Extend this middleware to check req.cookies.refreshToken, validate it with jwtService.validateRefreshToken(...) (and optionally verify existence via tokenService.getByToken(...)), then set req.user and call next(). If neither access token nor valid refresh cookie is present, return unauthorized. This is required so users authenticated only by a refresh cookie can access protected routes.

mateacademy-ai-mentor · 2026-04-20T14:54:24Z

@@ -0,0 +1,79 @@
+const nodemailer = require('nodemailer');
+
+require('dotenv/config');


You only import guestMiddleware here. Logout must be accessible only to authenticated users — import authMiddleware so you can protect the logout route.

mateacademy-ai-mentor · 2026-04-20T14:54:24Z

+    html,
+  });


The logout route is currently unprotected: authRouter.post('/logout', catchError(authController.logout));. The spec requires Logout to be available only to authenticated users. Protect this route with authMiddleware (e.g. authRouter.post('/logout', authMiddleware, catchError(authController.logout));).

mateacademy-ai-mentor · 2026-04-20T14:54:24Z

+    subject: 'Account activation',
+    html: `
+      <h1>Activate account</h1>
+      <a href="${link}">${link}</a>
+    `,


The confirmation route for email change is protected by authMiddleware ('/me/email/confirm/:token'). Confirmation links are clicked from email and must work when the user is not authenticated (different browser/device). Remove the auth guard or expose a guest-accessible confirmation endpoint so the token can be confirmed without being logged in.

mateacademy-ai-mentor · 2026-04-20T14:54:24Z

+const sendEmailChangeConfirmation = (newEmail, token) => {
+  const link = `${CLIENT_URL}/me/email/confirm/${token}`;
+
+  return sendEmail({
+    newEmail,


You call emailService.sendEmailChangeConfirmation(newEmail, emailChangeToken) but the mail helper expects the recipient under the email key. In sendEmailChangeConfirmation you pass { newEmail, ... } to sendEmail — change that to { email: newEmail, subject, html } so the confirmation email is sent to the intended address.

mateacademy-ai-mentor · 2026-04-20T14:54:28Z

+const nodemailer = require('nodemailer');
+
+require('dotenv/config');
+
+const { CLIENT_URL, SMTP_HOST, SMTP_PORT, SMTP_USER, SMTP_PASS } = process.env;


controllers/index.js exports usersController, expensesController, and authController but not socialController. For consistency and to avoid surprises when requiring controllers via a single import, consider exporting socialController here as well.

mateacademy-ai-mentor · 2026-04-20T14:54:28Z

@@ -0,0 +1,4 @@
+module.exports = {
+  sequelize: require('./db.js'),
+  catchError: require('./catchError.js'),


You only import guestMiddleware here but logout must be available only to authenticated users. Import authMiddleware from ../middlewares so you can protect the logout route.

add task solution

2b1633f

mateacademy-ai-mentor suggested changes Apr 20, 2026

View reviewed changes

mateacademy-ai-mentor reviewed Apr 20, 2026

View reviewed changes

improve task solution according to ai-mentor suggestions

3a4417f

4xmplme requested a review from mateacademy-ai-mentor April 20, 2026 14:03

mateacademy-ai-mentor suggested changes Apr 20, 2026

View reviewed changes

improve task solution according to ai-mentor suggestions mate-academy#2

68e04b0

4xmplme requested a review from mateacademy-ai-mentor April 20, 2026 14:45

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

View reviewed changes

mateacademy-ai-mentor reviewed Apr 20, 2026

View reviewed changes

improve task solution according to ai-mentor suggestions mate-academy#3

5ebf114

4xmplme requested a review from mateacademy-ai-mentor April 21, 2026 10:20

		if (socialAccount) {
		targetUser = await User.findByPk(socialAccount.userId);

		if (!expense \|\| expense.userId !== req.user.id) {
		return res.sendStatus(404);


		const allowedFields = ['title', 'amount', 'spentAt', 'category', 'note'];
		const updateData = {};

		const user = await User.findByPk(req.user.id);
		const isValid = await bcrypt.compare(oldPassword, user.password);

		const authorization = req.headers.authorization \|\| '';
		const token = authorization.split(' ')[1];

		const usersRouter = Router();

		usersRouter.get('/', authMiddleware, catchError(usersController.getAll));

		const { CLIENT_URL, SMTP_HOST, SMTP_PORT, SMTP_USER, SMTP_PASS } = process.env;

		emailService: require('./email.service.js'),
		tokenService: require('./token.service.js'),

		const generateAccessToken = (user) => {
		return jwt.sign(user, process.env.JWT_ACCESS_SECRET, { expiresIn: '5s' });

		const generateRefreshToken = (user) => {
		return jwt.sign(user, process.env.JWT_REFRESH_SECRET, { expiresIn: '30s' });

		@@ -0,0 +1,34 @@
		const jwt = require('jsonwebtoken');

		require('dotenv/config');

		if (!title \|\| !amount \|\| !spentAt) {
		return res.sendStatus(400);

		if (Object.keys(updateData).length === 0) {
		return res.status(400);

		@@ -0,0 +1,79 @@
		const nodemailer = require('nodemailer');

		require('dotenv/config');

Conversation

4xmplme commented Apr 20, 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!

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!

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!

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!