impl node_auth-app

[DimasSavchenko]

No description provided.

[mateacademy-ai-mentor]

Review Summary

Great job building out the core authentication features! The code is well-structured with a clear separation of concerns. However, a few key requirements are missing, so I'm requesting some changes before this can be approved.

Here are the main points to address:

• Missing "Change Name" Feature: The profile page functionality is incomplete. Please implement the ability for a user to change their name, which will require adding a new route, controller function, and service logic.
• Insecure Email Change: The task requires users to "confirm the new email". Your current implementation in src/controllers/auth.controller.js updates the email immediately. You'll need to add a verification step, such as sending a confirmation link to the new email address before finalizing the change.
• Missing 404 Handler: The application needs a catch-all middleware in src/index.js to handle requests to undefined API routes and return a standardized JSON 404 error, as specified in the requirements.
• Database Migrations in .gitignore: The prisma/migrations directory is currently ignored. These files are essential for database schema consistency and must be tracked in version control. Please remove the corresponding line from your .gitignore file.

Once these items are addressed, your submission will be in great shape. Keep up the excellent 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.

Footnotes

1. Rate AI review example ↩

2. Re-request AI review example ↩

3. Add AI review comments example ↩

[mateacademy-ai-mentor]

According to the task description, when changing an email, the user should "confirm the new email". This implementation updates the email address immediately without a verification step. A common and more secure approach is to send a confirmation link to the new email address to verify ownership before the change is finalized.

[mateacademy-ai-mentor]

Database migration files are essential for maintaining a consistent database schema across different environments and among team members. These files should be tracked by version control. Please remove this line to include the migrations directory in your git repository.

[mateacademy-ai-mentor]

The task requirements specify that the profile page should allow a user to change their name. This controller is missing the necessary function to handle that update request.

[mateacademy-ai-mentor]

The task requires returning a 404 error for all pages not explicitly defined. Currently, the application relies on Express's default behavior for unhandled routes, which might not be a JSON response. It's better to add a catch-all middleware right before this line to handle any requests that didn't match a route and forward a proper ApiError.notFound() to your errorMiddleware for a consistent API error response.

[mateacademy-ai-mentor]

The task requirements state that a user should be able to change their name on the profile page. This router is missing a dedicated route (e.g., PATCH /change-name) to handle this functionality.

[mateacademy-ai-mentor]

The profile page requirements include the ability for a user to change their name. The service is missing a function to handle this functionality.

[mateacademy-ai-mentor]

The upsert method already handles the logic for creating a record if it doesn't exist. The initial findUnique check and the if block are redundant. You can simplify this entire function to just the db.token.upsert call, which will be more efficient as it reduces the number of database queries.

[mateacademy-ai-mentor]

Prisma's delete operation will throw an error if no record is found matching the where clause. This could cause an unhandled exception if, for example, a logout is attempted for an already-logged-out session. Using deleteMany here would be more robust, as it will not throw an error if no matching record is found.

[mateacademy-ai-mentor]

The task requirements state that when a user changes their email, they must "confirm the new email". This service is missing a function to send a verification link to the new email address, similar to the sendActivationEmail function. Currently, you only notify the old address of the change.

[mateacademy-ai-mentor]

The task requirements state that a user should be able to change their name on the profile page. This service is missing a function to handle that update. You should add a function here, for example, updateName(id, newName), to manage this logic.

[mateacademy-ai-mentor]

Review Summary

I am requesting changes because there are still a few critical blockers that affect core functionality and explicit task requirements. First, the Express app is missing a catch‑all 404 handler in src/index.js, so undefined routes do not return the required standardized JSON 404 response, which directly violates the task spec. Second, the global error middleware in src/middlewares/error.middleware.js calls next() after sending a 500 response, which can cause “headers already sent” runtime errors; you should stop the chain after sending the response and just log the error. Third, several sensitive profile operations in src/services/auth.service.js (updateName, updatePassword, updateEmailNotification) only check that a refreshToken string is present but never verify it (e.g. with jwtService.verifyRefresh and tokenService.getByToken), which means an attacker with a valid req.userId and any non-empty cookie value could modify another user’s data. Finally, one migration (prisma/migrations/20260211144328_add_pending_email_token/migration.sql) attempts to drop a resetToken index/column that may not exist and adds a pendingEmailToken without ensuring a corresponding pendingEmail column, which can break migration runs and leaves the email-change flow incomplete at the DB level. Once you add the 404 middleware, fix the error handler, properly validate refresh tokens for profile changes, and make the migration safe and consistent (including storing the pending email), your implementation will satisfy the core requirements; the remaining issues are mostly improvements and can be refined afterward.

---

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

Footnotes

1. Rate AI review example ↩

2. Re-request AI review example ↩

3. Add AI review comments example ↩

[mateacademy-ai-mentor]

The warning here is a bit misleading: because you're adding a new nullable column (no default), there cannot be pre-existing duplicate values in it. Consider clarifying or removing this line so it doesn't confuse reviewers or operators running the migration.

[mateacademy-ai-mentor]

Consider adding a timestamp column such as resetTokenExpiresAt (or similar) to store token expiry. Without an expiry you can't enforce short-lived reset tokens in the DB layer which is important for security and for cleaning up old tokens.

[mateacademy-ai-mentor]

Prefer creating a partial unique index that enforces uniqueness only for non-null tokens, e.g. CREATE UNIQUE INDEX ... ON "users_auth"("resetToken") WHERE "resetToken" IS NOT NULL; — this makes the intent explicit and avoids ambiguity across DB engines/environments.

[mateacademy-ai-mentor]

The import path is incorrect — defineConfig should be imported from the prisma package, not prisma/config. This will cause a module resolution/runtime error. Change to: import { defineConfig } from "prisma" or the correct package export for your Prisma version.

[mateacademy-ai-mentor]

You set migrations.path to prisma/migrations which is correct. Make sure the prisma/migrations directory is tracked in version control (remove it from .gitignore) so migrations are preserved for other developers and deployments, per the project requirements.

[mateacademy-ai-mentor]

updatedAt is declared NOT NULL but has no default value. If the application does not explicitly set updatedAt on INSERT you will get errors. Consider adding a default such as DEFAULT CURRENT_TIMESTAMP or a trigger to set/refresh updatedAt on updates, or allow NULL if your app will set it.

[mateacademy-ai-mentor]

activationToken exists which can be used for activation, but there is no explicit activation flag/timestamp (for example isActive BOOLEAN DEFAULT FALSE or activatedAt TIMESTAMPTZ). The requirements state the user should be activated only after email confirmation — adding an isActive or activatedAt column makes the activation state explicit and simpler to query.

[mateacademy-ai-mentor]

tokens_auth contains only refreshToken and no column to distinguish token purposes. The app needs password-reset tokens and also an email-change confirmation token (or another mechanism). Either add a type column (e.g. 'refresh' | 'resetPassword' | 'emailChange') and a token column, or create a separate table for one-time action tokens. Without this, implementing password reset and email-change confirmation per the task will be awkward or error-prone.

[mateacademy-ai-mentor]

There is a UNIQUE index on tokens_auth.userId, which enforces at most one token row per user. That prevents multiple refresh tokens (multi-device sessions). If you want to support multiple active refresh tokens, remove the unique constraint on userId and use a non-unique index instead. If one-token-per-user is intentional, keep it but be aware of the limitation.

[mateacademy-ai-mentor]

tokens_auth.updatedAt is also NOT NULL with no default (same issue as the users table). Ensure updatedAt has an appropriate default or that the application always supplies it on INSERT, otherwise inserts will fail.

[mateacademy-ai-mentor]

This ALTER TABLE adds the pendingEmail column which is necessary to support the email-change verification flow. Consider making the intent explicit by marking the column nullable (e.g. VARCHAR(255) NULL) in the SQL so it's clear to readers and schema tools that existing rows are unaffected.

[mateacademy-ai-mentor]

Creating a plain UNIQUE INDEX can fail if there are duplicate non-null pendingEmail values in existing data (the file warning mentions this). To reduce risk consider creating a partial unique index (applying uniqueness only for non-null values) or ensure you run a pre-migration cleanup. Example: CREATE UNIQUE INDEX CONCURRENTLY name ON users_auth(pendingEmail) WHERE pendingEmail IS NOT NULL (also reduces locking).

[mateacademy-ai-mentor]

The top-of-file warning correctly notes potential failure due to duplicates; you should codify a migration-safe approach: either (a) add a pre-migration step that removes or resolves duplicate pendingEmail values, or (b) create the unique index as a partial index and/or use CONCURRENTLY to avoid locks and make the migration safer.

[mateacademy-ai-mentor]

These handlers require a refreshToken cookie (extracted here). However authMiddleware validates the access token from Authorization header and sets req.userId. If a client has a valid access token but no refresh-token cookie, userService.getProfile / authService calls will throw unauthorized. Consider either relying on the access token (req.userId) only for profile reads/changes or documenting/ensuring the refresh cookie is always sent by the client.

[mateacademy-ai-mentor]

Missing the required catch-all 404 handler. Per the task and the previous review, you must add a middleware before the error handler that returns a standardized JSON 404 for undefined routes. For example:

app.use((req, res) => {
  res.status(404).json({ status: 404, message: 'Not Found' });
});

Add this before app.use(errorMiddleware); so the error middleware remains last.

[mateacademy-ai-mentor]

The register handler simply forwards name, email, password to authService without exposing password rules or validating them here. The task requires informing users about password rules and checking them. Either validate and return human-friendly rule errors here, or ensure authService.register enforces the rules and returns clear messages that you surface to the client.

[mateacademy-ai-mentor]

After activation you call authService.activate and return the normalized user object. The requirements state: “Redirect to Profile after the activation.” Returning the user may be fine for an API, but the spec expects a redirect or a clear flow that leads the client to the Profile. Please ensure the activation endpoint triggers the expected redirect flow (or document why redirect is handled on the client and provide a clear response/token to allow the client to navigate to profile).

[mateacademy-ai-mentor]

confirmPasswordReset accepts password and confirmation and forwards them to the service. The task requires these fields must be equal. Make sure authService.confirmPasswordReset validates equality and returns a helpful error if they differ; if you prefer to validate in the controller, add the check before calling the service.

[mateacademy-ai-mentor]

There is a confirmEmailChange endpoint that accepts only a confirmation token, but I don't see an endpoint here to initiate the email change (where the user supplies their current password and the new email). The requirements specify: To change an email the user should type the password, confirm the new email and notify the old email about the change. Add a controller action (e.g. requestEmailChange) that accepts password and newEmail, verifies the password, creates a pending token sent to the newEmail, and sends a notification to the old email.

[mateacademy-ai-mentor]

The controller does not expose any handler to change the user's name on the Profile page. The previous review requested adding a route/controller/service to allow users to change their name. Please add an authenticated endpoint (e.g. updateName) that accepts the new name and delegates to the service layer to persist it.

[mateacademy-ai-mentor]

Dropping the index users_auth_resetToken_key will fail if that index/column wasn't created by earlier migrations. Use DROP INDEX IF EXISTS or verify earlier migrations create resetToken before trying to drop it.

[mateacademy-ai-mentor]

This ALTER tries to DROP COLUMN "resetToken". If that column does not exist in the current schema this will fail. Either use DROP COLUMN IF EXISTS or ensure previous migration includes that column. Also note the in-file warning about data loss — migrate or back up values if needed.

[mateacademy-ai-mentor]

You add pendingEmailToken but do not add a pendingEmail column to store the new email address being confirmed. The task requires the user to "confirm the new email" and notifying the old email; without storing the pending email you cannot complete that flow. Add a pendingEmail VARCHAR(255) (and consider a pendingEmailRequestedAt) to support confirmation and notifications.

[mateacademy-ai-mentor]

Creating a UNIQUE index on passwordResetToken can fail if duplicate non-null values already exist (the file warning notes this). Ensure you clean duplicates before applying or consider a non-unique index or a separate tokens table to avoid migration failures.

[mateacademy-ai-mentor]

Similarly, a UNIQUE index on pendingEmailToken restricts tokens to be unique and will fail on duplicates. Also consider adding token metadata (createdAt/expiry) or using a dedicated tokens table so you can manage token expiry, revocation and multiple concurrent tokens per user.

[mateacademy-ai-mentor]

After sending the 500 response the middleware calls next() — do not call next() after sending a response. This can cause "headers already sent" errors or duplicate responses. Remove next() or return after res.status(...).send(...) and optionally log the error instead.

[mateacademy-ai-mentor]

Consider logging unexpected errors here (e.g. console.error(error) or using your logger) and include stack trace information only in non-production environments to help debugging without exposing internals in production.

[mateacademy-ai-mentor]

This middleware reads the Authorization header but elsewhere (profile endpoints) the app expects a refreshToken cookie. That creates inconsistent auth behavior between routes — either require the cookie consistently or rely only on the access token validated here. See the profile controller and route wiring for context.

[mateacademy-ai-mentor]

Throwing ApiError.unauthorized() here inside an async middleware will reject the promise; Express 4 does not handle async promise rejections automatically. Wrap the middleware body in try/catch and call next(error) instead (or return errors via next).

[mateacademy-ai-mentor]

Same problem as above: throwing inside the async function. Use try/catch and next(error) so the central error middleware receives the error reliably.

[mateacademy-ai-mentor]

If the token's user cannot be found you return ApiError.notFound(). For token verification failures it's better to return unauthorized to avoid user-enumeration and to keep behavior consistent for invalid tokens.

[mateacademy-ai-mentor]

The middleware only checks for a refreshToken cookie. A user might be authenticated with a valid access token in the Authorization header but no cookie — in that case the middleware would treat them as unauthenticated. Consider also checking Authorization header (or verifying the refresh token) to reliably detect logged-in users.

[mateacademy-ai-mentor]

You log userId: req.userId but req.userId is typically set by authMiddleware (which isn't run for non-authenticated routes). That means the log will usually contain undefined. Either remove/adjust this log or extract/verify the token before logging an id.

[mateacademy-ai-mentor]

port is taken directly from process.env.SMTP_PORT (string). Nodemailer expects a numeric port and you should also set secure depending on the SMTP port. Use port: Number(process.env.SMTP_PORT) and set secure (true for 465).

[mateacademy-ai-mentor]

transporter.sendMail is called without a from address. If no default from is configured in the transporter, sends will fail. Add a from either in transporter defaults or include it in the send payload (e.g. from: process.env.SMTP_FROM).

[mateacademy-ai-mentor]

Consider adding basic error handling/logging around transporter.sendMail (or let callers handle rejections) so delivery failures are logged and observable in production.

[mateacademy-ai-mentor]

This router does not apply authMiddleware itself — it relies on being mounted under /profile with auth in src/index.js. That works now, but consider protecting the router at its level to avoid accidental public mounting elsewhere.

[mateacademy-ai-mentor]

All routes are wrapped with the catchError helper (good). However, there is no per-route request validation middleware (e.g. to validate name, password shapes). Validation currently happens deeper in services; adding small validation middleware would provide earlier, clearer errors.

[mateacademy-ai-mentor]

The /change-email endpoint uses POST, which is acceptable, but semantically it represents a resource update and could be PATCH. This is optional — keep POST if you prefer an action-style endpoint for initiating email-change confirmation.

[mateacademy-ai-mentor]

checkNotLoggedIn is applied here to protect registration for non-authenticated users, but the middleware only checks the refreshToken cookie. If a client authenticates using an Authorization Bearer access token (validated by authMiddleware), checkNotLoggedIn will not detect it and will allow access. Consider updating checkNotLoggedIn to also inspect the Authorization header (or unify authentication checks) so the route truly blocks authenticated users. See check-login.middleware.js and auth.middleware.js for the current behavior.

[mateacademy-ai-mentor]

POST /logout requires authMiddleware (access token), but the logout handler in the controller reads the refreshToken cookie and removes the refresh token. This mismatch can cause issues if the client only sends the cookie or only the header. Decide whether logout should require the access token, the refresh cookie, or both, and make route and handler consistent.

[mateacademy-ai-mentor]

GET /activate-new-email/:pendingEmailToken is not using checkNotLoggedIn. For consistency with other activation flows you may want to restrict this to non-authenticated users (if desired) or document why it should remain accessible while authenticated. Review the intended UX for confirming a new email and align the route protection with it.

[mateacademy-ai-mentor]

updateName currently only checks presence of refreshToken but does not validate it. An attacker could pass any non-empty value and the update will proceed as long as req.userId is valid. Verify the refresh token (e.g. jwtService.verifyRefresh) and ensure the token record from tokenService.getByToken(refreshToken) belongs to id before allowing the name change.

[mateacademy-ai-mentor]

updateEmailNotification also only checks for refreshToken presence. You must validate the refresh token and ensure it maps to userId (use jwtService.verifyRefresh and tokenService.getByToken) before proceeding with sensitive email-change actions.

[mateacademy-ai-mentor]

updatePassword similarly gates on refreshToken existence but never verifies it. Verify the refresh token and that it belongs to userId before allowing a password change (otherwise an attacker with a valid req.userId but arbitrary cookie could trigger changes).

[mateacademy-ai-mentor]

In refresh you call userService.getByEmail(userData?.email ?? '') even when userData may be null. Move the check if (!userData) earlier and avoid the DB lookup when the refresh token is invalid to reduce unnecessary work.

[mateacademy-ai-mentor]

authenticate calls tokenService.create(normalizedUser.id, refreshToken) which upserts by userId (see token service). This intentionally enforces one refresh token per user (single active session). If you need multi-device sessions, change token storage to allow multiple tokens per user (remove unique constraint/upsert).

[mateacademy-ai-mentor]

create uses upsert with where: { userId } which enforces a single refresh token row per user (new logins overwrite previous refresh token). If you want to allow multiple concurrent sessions/devices, remove the uniqueness on userId in the DB and use create instead of upsert. If single-session is intentional, keep as-is but note the limitation.

[mateacademy-ai-mentor]

getByToken uses findUnique which expects refreshToken to be unique in the DB (this matches the migration/indexes). This is fine but relies on the DB uniqueness of refreshToken — keep the index in sync with this assumption.

[mateacademy-ai-mentor]

removeByToken calls db.token.delete. Prisma's delete will throw if no record matches the where clause. To avoid an exception when the token is already missing (race conditions), consider using deleteMany({ where: { refreshToken } }) or wrap delete in try/catch and ignore NotFound errors.

[mateacademy-ai-mentor]

Minor: the functions don't validate inputs (e.g. empty newToken or refreshToken). Validation is often done at higher layers, but adding simple checks here would harden the service against invalid calls.

[mateacademy-ai-mentor]

register creates a user without any timestamp fields here. Ensure your DB schema/migrations and Prisma model provide defaults for createdAt/updatedAt (or that Prisma sets them) so inserts do not fail.

[mateacademy-ai-mentor]

getProfile requires a refreshToken value and throws unauthorized if it's missing. The route mounting uses authMiddleware to validate an access token and set req.userId — keeping a cookie-check here duplicates authentication. Either rely on authMiddleware (remove cookie check) or fully validate the refresh token here (verify token and token record). See how the controller/route/middleware are wired for profile endpoints.

[mateacademy-ai-mentor]

getByPasswordResetToken uses findUnique({ where: { passwordResetToken } }). This is valid only if passwordResetToken is unique in the DB; the repo has migrations that add a unique index for the reset token, so verify those migrations are applied in your environment (migration adds passwordResetToken unique index) .

[mateacademy-ai-mentor]

setPendingEmail and updateEmail write pendingEmail and pendingEmailToken fields. Make sure those columns exist and have the intended constraints. The repo contains migrations that add pendingEmail and the pending-email token column/index — ensure these migrations are present and applied in your DB environment .

[mateacademy-ai-mentor]

getByPendingEmail uses findUnique({ where: { pendingEmail } }). That is appropriate only if pendingEmail is unique; the migration that adds pendingEmail also adds a unique index, so this call is consistent with the migration — just ensure the migration is applied before relying on this query in production.

[mateacademy-ai-mentor]

The constructor destructures its parameter but has no default; calling new ApiError() (no args) will throw because you cannot destructure undefined. Add a default empty object to the parameter, e.g. constructor({ message, status, errors = {} } = {}), to make the constructor safe to call directly.

[mateacademy-ai-mentor]

Consider setting an explicit error name for clarity in logs/stacks, e.g. after super(message); add this.name = 'ApiError';. This helps identify the error type in traces and when debugging.

[mateacademy-ai-mentor]

Check that process.env.JWT_SECRET and process.env.JWT_REFRESH_SECRET are defined at startup and throw a clear error if missing. Right now a missing secret would cause jwt.sign/verify to behave unexpectedly at runtime; failing fast makes debugging easier.

[mateacademy-ai-mentor]

sign currently signs the whole user object. Prefer signing a minimal payload such as { id, email } to avoid token bloat and accidental leakage if user objects gain extra fields later.

[mateacademy-ai-mentor]

Make expirations and other JWT options configurable via environment variables (instead of hardcoded '10m' and '7d') so you can tune them without code changes.

[mateacademy-ai-mentor]

Consider specifying the signing algorithm and adding standard claims (iss, aud, sub) when calling jwt.sign/jwt.verify to make tokens more explicit and harder to misuse.

[mateacademy-ai-mentor]

The verify and verifyRefresh functions return null on error which matches how the codebase checks them. Optionally add logging inside the catch blocks to assist debugging when token verification fails.

[mateacademy-ai-mentor]

This import depends on the @prisma/adapter-pg package. Ensure the package is intentionally used and declared in package.json; otherwise use the standard new PrismaClient() without an adapter. If you rely on a custom adapter, document why it's needed.

[mateacademy-ai-mentor]

connectionString may be undefined if process.env.DATABASE_URL is not set. Consider validating the env var and throwing a clear error (e.g. if (!process.env.DATABASE_URL) throw new Error('DATABASE_URL is required')) so problems fail fast with a helpful message.

[mateacademy-ai-mentor]

PrismaClient will lazily connect on first query. For predictable startup and proper resource cleanup consider calling await db.$connect() during app initialization and handle shutdown with await db.$disconnect() on process exit to avoid leaked connections.

[mateacademy-ai-mentor]

Review Summary

I am requesting changes because there are still critical blockers that affect core requirements and security. First, several sensitive profile endpoints (updateName, updatePassword, updateEmailNotification in src/services/auth.service.js) only check that a refreshToken string is present and never actually verify the token or link it to the current user; this allows an attacker with a valid userId context but arbitrary cookie value to modify another user’s data, which clearly breaks the authentication requirement. Second, the global API behavior requirement for a JSON 404 is not met: src/index.js is missing a catch‑all 404 middleware after all routers and before the error handler, so undefined routes won’t return the standardized 404 response mandated by the task. Third, the password-reset confirmation and password-change flows in the controllers (src/controllers/auth.controller.js and src/controllers/user.controller.js) do not check that password/newPassword and confirmation match before calling services, which violates the spec and can lead to inconsistent or insecure password updates.

To move forward, you should: 1) update the auth service methods that change user data to fully validate the refresh token (e.g., via jwtService.verifyRefresh and tokenService.getByToken) and ensure it belongs to the same userId before performing any changes; 2) add a global app.use((req, res) => res.status(404).json({ status: 404, message: 'Not Found' })) before errorMiddleware in src/index.js; and 3) add explicit equality checks for password/confirmation in the reset and change‑password controllers, returning a 400 error when they differ. Once these are in place, your implementation will be much closer to both the security expectations and the project’s functional requirements; the remaining items (lint fixes, more robust env validation, logger improvements, etc.) are important but non-blocking and can be addressed afterwards. Overall the structure and coverage of routes, services, and migrations is solid, so after resolving these few critical points you should be in good 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.

Footnotes

1. Rate AI review example ↩

2. Re-request AI review example ↩

3. Add AI review comments example ↩

[mateacademy-ai-mentor]

updatedAt is declared NOT NULL but has no default value. If the application does not explicitly set updatedAt on INSERT you will get errors. Consider adding a default such as DEFAULT CURRENT_TIMESTAMP or a trigger to set/refresh updatedAt on updates, or allow NULL if your app will set it.

[mateacademy-ai-mentor]

activationToken exists which can be used for activation, but there is no explicit activation flag/timestamp (for example isActive BOOLEAN DEFAULT FALSE or activatedAt TIMESTAMPTZ). The requirements state the user should be activated only after email confirmation — adding an isActive or activatedAt column makes the activation state explicit and simpler to query.

[mateacademy-ai-mentor]

tokens_auth contains only refreshToken and no column to distinguish token purposes. The app needs password-reset tokens and also an email-change confirmation token (or another mechanism). Either add a type column (e.g. 'refresh' | 'resetPassword' | 'emailChange') and a token column, or create a separate table for one-time action tokens. Without this, implementing password reset and email-change confirmation per the task will be awkward or error-prone.

[mateacademy-ai-mentor]

There is a UNIQUE index on tokens_auth.userId, which enforces at most one token row per user. That prevents multiple refresh tokens (multi-device sessions). If you want to support multiple active refresh tokens, remove the unique constraint on userId and use a non-unique index instead. If one-token-per-user is intentional, keep it but be aware of the limitation.

[mateacademy-ai-mentor]

tokens_auth.updatedAt is also NOT NULL with no default (same issue as the users table). Ensure updatedAt has an appropriate default or that the application always supplies it on INSERT, otherwise inserts will fail.

[mateacademy-ai-mentor]

Dropping the index users_auth_resetToken_key will fail if that index/column wasn't created by earlier migrations. Use DROP INDEX IF EXISTS or verify earlier migrations create resetToken before trying to drop it.

[mateacademy-ai-mentor]

This ALTER tries to DROP COLUMN "resetToken". If that column does not exist in the current schema this will fail. Either use DROP COLUMN IF EXISTS or ensure previous migration includes that column. Also note the in-file warning about data loss — migrate or back up values if needed.

[mateacademy-ai-mentor]

You add pendingEmailToken but do not add a pendingEmail column to store the new email address being confirmed. The task requires the user to "confirm the new email" and notifying the old email; without storing the pending email you cannot complete that flow. Add a pendingEmail VARCHAR(255) (and consider a pendingEmailRequestedAt) to support confirmation and notifications.

[mateacademy-ai-mentor]

Creating a UNIQUE index on passwordResetToken can fail if duplicate non-null values already exist (the file warning notes this). Ensure you clean duplicates before applying or consider a non-unique index or a separate tokens table to avoid migration failures.

[mateacademy-ai-mentor]

Similarly, a UNIQUE index on pendingEmailToken restricts tokens to be unique and will fail on duplicates. Also consider adding token metadata (createdAt/expiry) or using a dedicated tokens table so you can manage token expiry, revocation and multiple concurrent tokens per user.

[mateacademy-ai-mentor]

updateName currently only checks presence of refreshToken but does not validate it. An attacker could pass any non-empty value and the update will proceed as long as req.userId is valid. Verify the refresh token (e.g. jwtService.verifyRefresh) and ensure the token record from tokenService.getByToken(refreshToken) belongs to id before allowing the name change.

[mateacademy-ai-mentor]

updateEmailNotification also only checks for refreshToken presence. You must validate the refresh token and ensure it maps to userId (use jwtService.verifyRefresh and tokenService.getByToken) before proceeding with sensitive email-change actions.

[mateacademy-ai-mentor]

updatePassword similarly gates on refreshToken existence but never verifies it. Verify the refresh token and that it belongs to userId before allowing a password change (otherwise an attacker with a valid req.userId but arbitrary cookie could trigger changes).

[mateacademy-ai-mentor]

In refresh you call userService.getByEmail(userData?.email ?? '') even when userData may be null. Move the check if (!userData) earlier and avoid the DB lookup when the refresh token is invalid to reduce unnecessary work.

[mateacademy-ai-mentor]

authenticate calls tokenService.create(normalizedUser.id, refreshToken) which upserts by userId (see token service). This intentionally enforces one refresh token per user (single active session). If you need multi-device sessions, change token storage to allow multiple tokens per user (remove unique constraint/upsert).

[mateacademy-ai-mentor]

Check that process.env.JWT_SECRET and process.env.JWT_REFRESH_SECRET are defined at startup and throw a clear error if missing. Right now a missing secret would cause jwt.sign/verify to behave unexpectedly at runtime; failing fast makes debugging easier.

[mateacademy-ai-mentor]

sign currently signs the whole user object. Prefer signing a minimal payload such as { id, email } to avoid token bloat and accidental leakage if user objects gain extra fields later.

[mateacademy-ai-mentor]

Make expirations and other JWT options configurable via environment variables (instead of hardcoded '10m' and '7d') so you can tune them without code changes.

[mateacademy-ai-mentor]

Consider specifying the signing algorithm and adding standard claims (iss, aud, sub) when calling jwt.sign/jwt.verify to make tokens more explicit and harder to misuse.

[mateacademy-ai-mentor]

The verify and verifyRefresh functions return null on error which matches how the codebase checks them. Optionally add logging inside the catch blocks to assist debugging when token verification fails.

[mateacademy-ai-mentor]

This import depends on the @prisma/adapter-pg package. Ensure the package is intentionally used and declared in package.json; otherwise use the standard new PrismaClient() without an adapter. If you rely on a custom adapter, document why it's needed.

[mateacademy-ai-mentor]

connectionString may be undefined if process.env.DATABASE_URL is not set. Consider validating the env var and throwing a clear error (e.g. if (!process.env.DATABASE_URL) throw new Error('DATABASE_URL is required')) so problems fail fast with a helpful message.

[mateacademy-ai-mentor]

PrismaClient will lazily connect on first query. For predictable startup and proper resource cleanup consider calling await db.$connect() during app initialization and handle shutdown with await db.$disconnect() on process exit to avoid leaked connections.

[mateacademy-ai-mentor]

The import path is incorrect and will cause a runtime/module resolution error. Replace this with the official Prisma import: import { defineConfig } from "prisma";. Also ensure the prisma package (Prisma CLI/runtime) is installed in the project so this import resolves.

[mateacademy-ai-mentor]

Consider validating process.env['DATABASE_URL'] here and throwing a clear error if it's missing (e.g. if (!process.env.DATABASE_URL) throw new Error('DATABASE_URL is not set');). This prevents obscure errors from Prisma later and makes failures easier to debug.

[mateacademy-ai-mentor]

Registration handler doesn't communicate password rules or show them to the client. The task requires that users be informed of password rules and that they are enforced. Either (a) include a description of the password rules in the registration response, or (b) validate the password here and return a clear 4xx with the rule violations. If you rely on authService.register to enforce this, make sure it returns descriptive errors and document that here.

[mateacademy-ai-mentor]

Activation handler currently returns the activated user JSON. The specification requires redirecting to the Profile page after activation. Decide whether the server should perform the redirect (e.g. res.redirect('/profile')) or explicitly tell the frontend to redirect; implement the chosen behavior. Also ensure this route is only accessible to non-authenticated users (middleware or an explicit check).

[mateacademy-ai-mentor]

Password reset confirmation does not validate that password and confirmation are equal before calling the service. The spec mandates these fields must match. Add a check and return a 400 (or suitable error) when they differ to avoid unnecessary service calls and to satisfy the requirement.

[mateacademy-ai-mentor]

Email-change confirmation endpoint exists (good), but the task requires that the new email is confirmed before updating and that the OLD email is notified about the change. Ensure authService.confirmEmailChange performs the final update only after confirmation and sends a notification to the previous email. If that logic is in the service, add an assertion/comment here or move/duplicate checks as needed.

[mateacademy-ai-mentor]

Logout and some other endpoints have access-control requirements (e.g. only authenticated users can logout; some flows only for non-authenticated). This controller does not enforce those checks itself — make sure route-level middleware or explicit checks are in place. If you prefer controller-level enforcement, add guards here (e.g. check req.user).

[mateacademy-ai-mentor]

The top comment warns that adding a unique constraint could fail if there are existing duplicate pendingEmail values. Since this migration adds the pendingEmail column (it didn't exist before), there can't be pre-existing values for it — the warning is misleading. Consider removing or clarifying this comment so it doesn't confuse future maintainers.

[mateacademy-ai-mentor]

This ALTER TABLE adds pendingEmail as VARCHAR(255) (nullable). That's appropriate for storing a pending/unconfirmed email. Make sure your application normalizes emails (e.g., lowercases) before inserting into this column, or add DB-level normalization to avoid case-related duplicates.

[mateacademy-ai-mentor]

The UNIQUE INDEX on pendingEmail is case-sensitive in PostgreSQL. To avoid duplicates that differ only by case, either:

• normalize emails to lowercase in the application, or
• create a case-insensitive index (e.g. on LOWER("pendingEmail")) or use the citext extension. Also, for very large tables think about index creation strategy (e.g. CREATE INDEX CONCURRENTLY) — but be careful: some migration runners wrap statements in transactions and CONCURRENTLY cannot run inside a transaction.

[mateacademy-ai-mentor]

Good that the migration includes a warning about duplicates. Add an explicit pre-migration check (and instructions or SQL) so the migration fails fast with a clear message — e.g. run SELECT resetToken, count(*) FROM users_auth WHERE resetToken IS NOT NULL GROUP BY resetToken HAVING count(*) > 1; and clean any duplicates before applying this migration.

[mateacademy-ai-mentor]

Be explicit about the column nullability and intent. For reset tokens you likely want a nullable column; consider ADD COLUMN "resetToken" VARCHAR(255) NULL; so the intent is clear to readers and future maintainers.

[mateacademy-ai-mentor]

Creating a UNIQUE index may lock the table while it builds. For large tables consider CREATE UNIQUE INDEX CONCURRENTLY ... to avoid long locks (note: CONCURRENTLY cannot run inside a transaction). Also consider making the index partial to be explicit: CREATE UNIQUE INDEX "users_auth_resetToken_key" ON "users_auth" ("resetToken") WHERE "resetToken" IS NOT NULL; Ensure your migration runner (Prisma) supports the chosen approach.

[mateacademy-ai-mentor]

getProfile assumes req.userId is set. The task requires the profile page to be accessible only to authenticated users — add an explicit guard (return 401 if !req.userId) or ensure route-level authentication middleware is documented and always applied.

[mateacademy-ai-mentor]

getProfile passes refreshToken from cookies to the service but doesn't handle a missing cookie. Either validate and return a clear error or ensure the service gracefully handles absent refreshToken and document that assumption.

[mateacademy-ai-mentor]

updateName correctly calls the service to change the name (good). However, validate the name input (required, non-empty, max length) before calling authService.updateName to avoid invalid updates. Also ensure this endpoint is restricted to authenticated users (check req.userId).

[mateacademy-ai-mentor]

updatePassword accepts password, newPassword, and confirmation but does not check that newPassword and confirmation are equal. The spec mandates these must match — add a check and return a 400 if they differ before calling the service.

[mateacademy-ai-mentor]

Also for updatePassword: validate that password (old password) is provided and that newPassword meets the required password rules. The spec requires enforcing password rules when changing passwords.

[mateacademy-ai-mentor]

updatePassword clears the refreshToken cookie after changing password (good). Ensure the service verifies the old password and returns appropriate errors; if it does not, add that verification here or in the service.

[mateacademy-ai-mentor]

updateEmailNotification starts the email-change flow by sending a notification to the new email (good). The specification requires the new email to be confirmed before updating and that the OLD email be notified about the change — ensure authService.updateEmailNotification only sends a confirmation email (does not swap emails immediately) and that authService.confirmEmailChange performs the final update and notifies the old email. If that logic is in the service, add a short comment here to make the contract explicit.

[mateacademy-ai-mentor]

updateEmailNotification does not validate newEmail format or presence of password. Add input validation for newEmail (valid email format) and ensure password is provided for authentication as required by the spec.

[mateacademy-ai-mentor]

Relying only on the presence of the refreshToken cookie is brittle: a cookie may be stale or removed server-side. Consider verifying the token (e.g., check it exists in DB via tokenService.getByToken or validate/verify the token) before treating the request as authenticated. This prevents false positives for logged-in state.

[mateacademy-ai-mentor]

You log req.userId here but this middleware does not populate req.userId (that is done by authMiddleware after token verification). Either decode/verify the token here and set req.userId before logging, or remove userId from the log to avoid undefined values in logs. See how authMiddleware sets req.userId after verification for reference.

[mateacademy-ai-mentor]

Throwing ApiError.forbidden() is acceptable, but include a descriptive message (e.g. ApiError.forbidden('Already logged in')) so clients and logs get clearer information. Also ensure the route-level usage of this middleware matches the spec (these routes should be used only for non-authenticated users — the router shows it's applied to registration/login/reset endpoints).

[mateacademy-ai-mentor]

This file lacks the required global catch‑all 404 handler. The task requires returning a standardized JSON 404 for undefined API routes. Add a middleware after all routers and before the error handler that sends a 404 JSON response (for example: app.use((req, res) => res.status(404).json({ status: 404, message: 'Not Found' }))). Place it before errorMiddleware so not-found routes are handled correctly.

[mateacademy-ai-mentor]

Ensure that endpoints which must only be accessible to non-authenticated users (registration, activation, login, password reset flows) are actually protected by middleware — either inside authRouter or by adding route-level guards here. Right now authRouter is mounted without explicit guards in this file; confirm the router implements the required access-control checks.

[mateacademy-ai-mentor]

Calling jwtService.verify directly may throw or may return a Promise. Wrap verification in a try/catch and await it if the service is async. If verification fails, throw ApiError.unauthorized() so the error is handled consistently instead of crashing the process.

[mateacademy-ai-mentor]

After loading the user you only set req.userId. Attach the full user object (for example req.user = user) so downstream controllers can access user fields without another DB lookup. This also makes it easy to check user.isActive if needed.

[mateacademy-ai-mentor]

When the user is not found you throw ApiError.notFound(). For tokens that reference deleted users it's safer/clearer to return ApiError.unauthorized() to avoid exposing resource existence differences. Decide and align this behavior with the rest of your API.

[mateacademy-ai-mentor]

Consider checking user.isActive here (or ensure another layer does it). Some protected routes should deny access for non-activated users — if you want that globally enforced, add the check and return ApiError.forbidden() or ApiError.unauthorized() as appropriate.

[mateacademy-ai-mentor]

SMTP config uses process.env.SMTP_PORT directly (string). Parse it as a number and consider setting secure (true for port 465) to avoid nodemailer treating the port incorrectly. Example: port: Number(process.env.SMTP_PORT), secure: Number(process.env.SMTP_PORT) === 465.

[mateacademy-ai-mentor]

Transporter is created without validating required environment variables. If SMTP_* env vars are missing the transporter creation or send will fail with unclear errors — consider checking for required env values and throwing a clear error or logging a warning at startup.

[mateacademy-ai-mentor]

transporter.sendMail call omits a from address. Many SMTP providers require from. Add a from: process.env.SMTP_FROM || process.env.SMTP_USER (or explicit default) to ensure messages have a sender.

[mateacademy-ai-mentor]

When building links with tokens (activation/reset/new-email), encode the token with encodeURIComponent(token) to avoid issues if the token contains characters that break URLs. First occurrence here; same applies to the other link builders.

[mateacademy-ai-mentor]

Same token-encoding issue for reset-password link — use encodeURIComponent(token) when interpolating into the URL to prevent malformed links.

[mateacademy-ai-mentor]

Same token-encoding issue for new-email activation link — encode the token when constructing the URL to avoid URL issues.

[mateacademy-ai-mentor]

Consider verifying process.env.CLIENT_HOST exists (or provide a sensible default) and failing fast with a descriptive error if missing. This avoids sending emails with broken links (templates rely on CLIENT_HOST).

[mateacademy-ai-mentor]

Do not call next() after sending a response — calling next() here can cause "headers already sent" or duplicate-response issues. The middleware should end the request cycle after res.send()/res.status().send() and only call next if you intend to pass error handling onward (not applicable after a response).

[mateacademy-ai-mentor]

Consider logging unexpected errors before returning the 500 response so you capture stack and context. For example, use your logger to log error.message and error.stack. This helps debugging when non-ApiError exceptions occur.

[mateacademy-ai-mentor]

Optional: avoid exposing full error details in production. If you want to include a stack trace in responses during development, gate it behind process.env.NODE_ENV !== 'production' so sensitive info is not leaked in production responses.

[mateacademy-ai-mentor]

Good: registration route is protected with checkNotLoggedIn. Make sure checkNotLoggedIn actually verifies the token (not just cookie presence) so already-authenticated users are reliably detected — the middleware currently only checks for refreshToken cookie which can be brittle. See check-login.middleware.js.

[mateacademy-ai-mentor]

Activation route uses checkNotLoggedIn (good), but the spec requires redirecting to the Profile page after activation. The controller currently returns JSON — ensure you implement a redirect (res.redirect('/profile')) or a clear response instructing the frontend to redirect in authController.activate. See auth.controller.js.

[mateacademy-ai-mentor]

Login route is protected with checkNotLoggedIn (good). The specification requires redirect to profile after login; confirm whether that redirect is performed by the controller (server-side) or by the client after receiving the auth response. If server should redirect, implement it in authController.login.

[mateacademy-ai-mentor]

Logout route correctly uses authMiddleware so only authenticated users can call it. The spec asks to redirect to the login page after logout — ensure your authController.logout either issues a redirect (res.redirect('/login')) or the client handles navigation after receiving the logout response.

[mateacademy-ai-mentor]

The /activate-new-email/:pendingEmailToken route does not use checkNotLoggedIn. That may be intentional (activation links can be clicked when logged out), but confirm this is the desired behavior. If you want new-email activation to be available only to non-authenticated users, add checkNotLoggedIn. Otherwise document why it's left open.

[mateacademy-ai-mentor]

These routes rely on the authMiddleware being applied when this router is mounted. Confirm this is done (I see app.use('/profile', authMiddleware, userRouter) in src/index.js) so these endpoints are accessible only to authenticated users.

[mateacademy-ai-mentor]

GET profile and the update routes lack request validation middleware. Add validation for inputs (e.g. ensure name is present for PATCH /, ensure password fields exist and meet rules for PATCH /password). You can reuse validators from src/validations/auth.validation.js or add dedicated validators. This prevents invalid data reaching the controllers.

[mateacademy-ai-mentor]

/change-email forwards to the controller but the task requires that the new email must be confirmed before updating and the old email must be notified. Ensure authService.updateEmailNotification and the follow-up confirmation flow implement: (1) set pendingEmail + token, (2) send activation link to the new email, and (3) after confirmation, send notification to the old email. The controller/service code is in src/controllers/user.controller.js / src/services/email.service.js — verify those implement these steps.

[mateacademy-ai-mentor]

Minor: consider using PATCH /change-email instead of POST /change-email because this is a partial update to the user resource — not required, but improves semantic clarity.

[mateacademy-ai-mentor]

When registering a new user you call db.user.create directly. If the email already exists this will throw a DB error — it's better to check for an existing user with this email first and throw an ApiError.badRequest('Email already in use') (or convert DB uniqueness errors to a friendly ApiError) so clients receive a clear validation response.

[mateacademy-ai-mentor]

activate uses db.user.update which will throw if activationToken doesn't match any row. Prefer first finding the user by token and throwing ApiError.notFound('Invalid activation token') if missing, or catch the DB error and translate it to a user-friendly ApiError. This avoids leaking DB errors to clients and satisfies the requirement that activation happens only after proper confirmation.

[mateacademy-ai-mentor]

getProfile requires a refreshToken and throws ApiError.unauthorized() when it's absent. Note: the route protecting profile already uses authMiddleware (which verifies the Authorization bearer token). Consider whether requiring the refresh token here is necessary (and document the rationale) or rely solely on the authentication middleware. If keeping this check, ensure tests and client behavior expect the cookie requirement.

[mateacademy-ai-mentor]

updateEmail clears pendingEmail and pendingEmailToken and sets email to pendingEmail via db.user.update. As with other direct update() calls, handle the case where the user ID doesn't exist and translate DB errors into ApiError responses. Also ensure uniqueness checks for pendingEmail are enforced/handled to give clear errors if the new email is already in use.

[mateacademy-ai-mentor]

badRequest accepts a custom message and errors (good). Keep this pattern for the other helpers so callers can pass descriptive messages/errors consistently.

[mateacademy-ai-mentor]

unauthorized should accept an optional message parameter (e.g. message = 'Unauthorized') so callers can provide more context when needed. Otherwise custom messages passed by callers will be ignored.

[mateacademy-ai-mentor]

forbidden should accept an optional message parameter (and optionally errors) so the thrown error can carry a specific explanation (e.g. ApiError.forbidden('Already logged in')).

[mateacademy-ai-mentor]

notFound is currently defined without parameters, but elsewhere the code calls ApiError.notFound('User not found') and expects that message to be used. Update this method signature to accept an optional message (and optionally errors).

[mateacademy-ai-mentor]

internal should also accept an optional message so unexpected internal errors can carry custom context when created programmatically (e.g. ApiError.internal('DB unavailable')).

[mateacademy-ai-mentor]

Using JSON.stringify({ level: 'info', msg, ...meta }) can throw if meta contains circular references or non-serializable values (e.g. Error objects). Consider using a safe serializer or wrap serialization in a try/catch to avoid runtime errors when logging complex objects.

[mateacademy-ai-mentor]

Add a timestamp to each log entry (e.g. timestamp: new Date().toISOString()) to make logs easier to correlate and debug across systems and requests.

[mateacademy-ai-mentor]

Using db.token.upsert assumes there is a unique constraint on userId in the tokens table. Verify your Prisma schema/migrations include that unique index; otherwise upsert will fail. If the schema guarantees uniqueness, this is fine — otherwise add or document the constraint.

[mateacademy-ai-mentor]

getByToken returns the DB record directly. Consider normalizing or documenting the returned shape, and validate refreshToken input (e.g. throw badRequest when missing) to avoid passing undefined to the DB query.

[mateacademy-ai-mentor]

removeByToken calls db.token.delete which will throw if no record matches the refreshToken. To avoid a runtime exception use deleteMany({ where: { refreshToken } }) (returns a count) or check existence with findUnique before deleting, or wrap in try/catch and handle P2025-style not-found errors from Prisma.

[mateacademy-ai-mentor]

invalidateSessions uses deleteMany which is fine. Consider returning a more explicit result (e.g. number of deleted rows) or normalizing the return value so callers can reliably know whether anything was removed.

[mateacademy-ai-mentor]

The EMAIL_PATTERN is reasonable for basic validation. If you need to support internationalized addresses or more complex rules consider updating this, but for the task this is fine.

[mateacademy-ai-mentor]

The password rule here enforces only minimum length (6). The task requires informing users about password rules and enforcing them — consider exporting a human-readable PASSWORD_RULES constant or a getPasswordRules() so the frontend can display rules to users, and document which rules are enforced server-side.

[mateacademy-ai-mentor]

There is no helper here to validate confirmation equality. The task requires reset/password-change pages with password and confirmation that must be equal — consider adding validateConfirmation(password, confirmation) or a combined validator so the same logic can be reused by controllers/services.