Skip to content

solution#273

Open
aakash1sadhu wants to merge 2 commits intomate-academy:masterfrom
aakash1sadhu:develop
Open

solution#273
aakash1sadhu wants to merge 2 commits intomate-academy:masterfrom
aakash1sadhu:develop

Conversation

@aakash1sadhu
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

The implementation has multiple critical import path errors that will cause runtime failures and block core functionality:

Critical Blockers:

  1. Import path typo in src/middlewares/auth.middleware.js - imports from '../services/jwt.service.js' but the actual file is jwt.services.js (with 's'). This will crash all authenticated routes with MODULE_NOT_FOUND.

  2. Import path typo in src/index.js - imports cookieParser from 'cookieParser' instead of 'cookie-parser'. This will crash the server on startup.

  3. Import path typo in src/controller/auth.controller.js - imports from 'jwt.service.js' instead of 'jwt.services.js'.

  4. Function naming typo in src/controller/auth.controller.js line 144 - calls userServices.normilizeUser(user) but the actual function in user.services.js is named normalizeUser. This will cause a runtime error when registration or login succeeds.

  5. Missing /profile route - The requirements explicitly require a Profile page for authenticated users, but auth.router.js has no GET /profile route. Redirects in activation and login flows point to this non-existent endpoint.

  6. User routes lack authentication in src/index.js - The /user/* routes are mounted without authMiddleware, allowing unauthenticated access to user data endpoints.

These issues must be fixed before the application can run correctly.


✨ Please rate this review in the Platform's notification center1. 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 button2 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 tag3.
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.

Footnotes

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment thread src/middlewares/auth.mildware.js
Comment thread src/middlewares/auth.mildware.js Outdated
Comment thread src/middlewares/auth.mildware.js Outdated
Comment thread src/controller/auth.controller.js
Comment thread src/controller/auth.controller.js Outdated
Comment thread src/controller/auth.controller.js
Comment thread src/controller/auth.controller.js
Comment thread src/controller/auth.controller.js
Comment thread src/controller/user.controller.js
Comment thread src/controller/user.controller.js
Comment thread src/controller/user.controller.js
Comment thread src/controller/user.controller.js
Comment thread src/controller/user.controller.js
Comment thread src/index.js Outdated
Comment thread src/index.js
Comment thread src/index.js
Comment thread src/router/auth.router.js
Comment thread src/router/auth.router.js
Comment thread src/router/auth.router.js
Comment thread src/router/user.router.js
Comment thread src/models/User.model.js
Comment thread src/models/User.model.js
Comment thread src/models/User.model.js
Comment thread src/models/User.model.js
Comment thread src/models/Token.model.js
Comment thread src/models/Token.model.js
Comment thread src/services/user.services.js
Comment thread src/services/user.services.js
Comment thread src/services/user.services.js
Comment thread src/services/user.services.js
Comment thread src/services/jwt.services.js
Comment thread src/services/email.services.js
Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

Your implementation successfully covers all core requirements from the task: registration with validation and activation email, login with email activation check, logout with redirects, password reset flow, and profile page with name/password/email change capabilities. All authentication routes are properly protected with middleware, and 404 handling is in place. There are two functional improvements worth noting: 1. Profile response missing name: The normalizeUser function in src/services/user.services.js returns only { userId, email }, so the profile endpoint will display users without their name. Consider adding name: user.name to the returned object for complete profile data. 2. GET /user endpoint security: The user list endpoint returns full user objects including password hashes to any authenticated user. Consider removing this endpoint, restricting it to admin users, or returning only safe fields (excluding password). 3. Secure cookie flag: The refresh token cookie has secure: true, which may fail in development environments without HTTPS. Consider using secure: process.env.NODE_ENV === 'production' for local testing compatibility. These are improvements rather than blockers—your authentication system is functional and meets the task requirements. Great work resolving the import and routing issues from the previous review.


✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Footnotes

  1. Rate AI review example

Comment thread src/index.js
Comment on lines +5 to +8
import { authMiddleware } from './middlewares/auth.mildware.js';

const app = express();
const port = process.env.PORT || 3000;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The GET / endpoint returns ALL users without any context about which user is requesting. An authenticated user can see all other users' data. Consider removing this endpoint or restricting it to admin users only.

Comment on lines +9 to +18

const [, token] = authorization.split(' ');

if (!token) {
return res.sendStatus(401);
}

const userData = jwtService.verify(token);

if (!userData) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The validation functions return error messages but lack early returns after the first check. If both conditions fail, only the first error is returned correctly, but adding explicit returns after each condition improves clarity.

Comment on lines +29 to +34

function normalizeUser(user) {
return {
userId: user.userId,
email: user.email,
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The normalizeUser function excludes the name field, returning only { userId, email }. Since this is used to generate the JWT payload and the getProfile endpoint returns req.user, the profile response will be missing the user's name. Consider including name in the normalized user object so authenticated users can view their profile data with their name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants