Full-Stack Authentication App#246
Full-Stack Authentication App#246Andrew17-2006 wants to merge 22 commits intomate-academy:masterfrom
Conversation
add latest deps
…e_update Feature/gitignore_update
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
This is a very strong submission with a well-implemented architecture. However, I am requesting changes because several core features are incomplete or contain critical bugs that block functionality.
Here are the main issues to address:
- Incomplete Registration: The backend does not save the
namefield during registration. Please updateauth.controller.jsanduser.service.jsto handle and persist the user's name. - Missing Profile Validation:
- The change password feature is missing server-side validation. The
user.controller.jsmust check that the new password and its confirmation match. - The change email feature is missing the required confirmation field entirely. This needs to be added to the frontend (
ProfilePage.jsx), the API call (userApi.js), and the backend controller (user.controller.js).
- The change password feature is missing server-side validation. The
- Broken Database Setup: The root
setup.jsfile is missing theTokenmodel import. Without this, thetokenstable won't be created, which breaks the refresh token functionality. - UI Bug: The loader component at
frontend/src/components/ui/Loader.jsxhas a typo (<sv>instead of<svg>), which prevents it from rendering.
Once these key issues are resolved, this will be an excellent project. Keep up the great work!
✨ 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
| } | ||
|
|
||
| const register = async (req, res) => { | ||
| const { email, password } = req.body; |
There was a problem hiding this comment.
The task description requires registration with name, email, and password. The name field is missing from the request body destructuring. You'll also need to pass it to the userService.register function and update that service to save the user's name.
| }; | ||
|
|
||
| const changePassword = async (req, res) => { | ||
| const { password, newPassword } = req.body; |
There was a problem hiding this comment.
The task requires the change password functionality to include fields for new password and confirmation. This function handles the old and new passwords but is missing the logic to check that the new password and its confirmation match. It's best practice to validate this on the server, not just the client.
| }; | ||
|
|
||
| const changeEmail = async (req, res) => { | ||
| const { password, newEmail } = req.body; |
There was a problem hiding this comment.
According to the requirements, changing an email should involve confirming the new email address. This implementation is missing that confirmation step. You should expect a confirmation field for the new email (e.g., newEmail and confirmNewEmail) and validate that they are the same before proceeding.
backend/src/services/user.service.js
Outdated
| return User.findOne({ where: { email } }); | ||
| } | ||
|
|
||
| async function register({ email, password }) { |
There was a problem hiding this comment.
The registration process requires a name field, but this function only destructures email and password from its arguments. You need to include name here to handle it.
backend/src/services/user.service.js
Outdated
| }); | ||
| } | ||
|
|
||
| await User.create({ email, password, activationToken }); |
There was a problem hiding this comment.
When creating a new user, the name is not being saved to the database. The User.create call should also include the name field to fulfill the registration requirement.
| changeEmail: (password, newEmail) => | ||
| client.patch('/users/email', { password, newEmail }), |
There was a problem hiding this comment.
According to the task description, changing an email requires the user to confirm the new email address. This function is missing a parameter for this confirmation. You should add it to the function signature and include it in the request payload.
| <sv | ||
| className="animate-spin h-6 w-6 text-blue-600" | ||
| viewBox="0 0 24 24" | ||
| fill="none" | ||
| > | ||
| <circle | ||
| className="opacity-25" | ||
| cx="12" | ||
| cy="12" | ||
| r="10" | ||
| stroke="currentColor" | ||
| strokeWidth="4" | ||
| /> | ||
| <path | ||
| className="opacity-75" | ||
| fill="currentColor" | ||
| d="M4 12a8 8 0 018-8v8z" | ||
| /> | ||
| </sv> |
There was a problem hiding this comment.
There appears to be a typo in the SVG element. The tag should be svg, not sv. This will prevent the loader from rendering correctly.
| } | ||
|
|
||
| try { | ||
| await userApi.changePassword(password, newPassword); |
There was a problem hiding this comment.
While you correctly validate that newPassword and confirmation match on the client, the userApi.changePassword call is only sending the old and new passwords. According to the task requirements, the confirmation should also be part of the request for server-side validation. You should update the API call to include it.
| <Section title="Змінити email"> | ||
| <div style={{ display: 'grid', gridTemplateColumns: '1fr 1fr', gap: 12 }}> | ||
| <GlassInput placeholder="Поточний пароль" type="password" value={emailPassword} onChange={(e) => setEmailPassword(e.target.value)} /> | ||
| <GlassInput placeholder="Новий email" type="email" value={newEmail} onChange={(e) => setNewEmail(e.target.value)} /> |
There was a problem hiding this comment.
The task requires users to "confirm the new email" when making a change. This implementation is missing the input field for this confirmation. You'll need to add a state for the confirmed email, an input field in this section, and update the handleChangeEmail function to validate that the two new email fields match before making the API call.
| @@ -0,0 +1,5 @@ | |||
| import 'dotenv/config'; | |||
| import { client } from './src/utils/db.js'; | |||
| import { User } from './src/models/user.js'; | |||
There was a problem hiding this comment.
The script is missing the import for the Token model. Sequelize's client.sync() can only create tables for the models it's aware of. You need to import the Token model here so that the tokens table is created in the database. Without it, the refresh token functionality will fail.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great work on the progress so far! You've successfully addressed several key issues from the previous review. This requires a few changes, as some core functionality is not yet complete or is broken.
Here are the critical issues to fix:
- Missing Server-Side Validation: In
backend/src/controllers/user.controller.js, the password change function is missing the required check to ensure the new password and its confirmation match. The same file is also missing the confirmation check for changing an email. - Incomplete UI for Profile Changes: The UI in
frontend/src/pages/ProfilePage.jsxand the API calls infrontend/src/api/userApi.jsneed to be updated to include the confirmation fields for both password and email changes. - Broken Google Sign-In: The application crashes during Google sign-in because the
googleIdfield is missing from the User model inbackend/src/models/user.js. - Database Setup Script Error: The
backend/setup.jsscript fails because it uses an undefinedsequelizevariable instead of the exportedclient.
Once these are resolved, the application should meet all the requirements. You're very close!
✨ 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
| # MacOS | ||
| .DS_Store | ||
| .env | ||
| backend/.env |
There was a problem hiding this comment.
Good job ignoring the environment file for the backend. To ensure frontend secrets are also kept safe, you should add an entry for the frontend's environment file as well, for example frontend/.env.
|
|
||
| const resetPassword = async (req, res) => { | ||
| const { token } = req.params; | ||
| const { password } = req.body; |
There was a problem hiding this comment.
When resetting a password, it's important to validate the new password to ensure it meets the required criteria (e.g., minimum length). You can reuse the validatePassword function here to check the new password before hashing and saving it.
backend/setup.js
Outdated
| import { sequelize } from './src/utils/db.js'; | ||
| import './src/models/user.js'; | ||
| import './src/models/token.js'; | ||
|
|
||
| await sequelize.sync({ alter: true }); |
There was a problem hiding this comment.
The Sequelize instance is exported as client from ./src/utils/db.js, but you are trying to import and use sequelize. This will cause an error. Please update the import and the variable name used for sync to match the exported client.
| }; | ||
|
|
||
| const changePassword = async (req, res) => { | ||
| const { password, newPassword } = req.body; |
There was a problem hiding this comment.
The task requires a confirmation field for the new password to ensure the user typed it correctly. You're missing this check. Please update the function to receive the confirmation from the request body and throw an error if it doesn't match newPassword.
| }; | ||
|
|
||
| const changeEmail = async (req, res) => { | ||
| const { password, newEmail } = req.body; |
There was a problem hiding this comment.
According to the task description, the user must confirm their new email address. This implementation is missing a field for email confirmation. Please add a check to ensure the newEmail and its confirmation from the request body are identical before proceeding with the update.
| } | ||
|
|
||
| try { | ||
| await userApi.changePassword(password, newPassword); |
There was a problem hiding this comment.
You're correctly collecting and validating the new password's confirmation on the client side. However, the confirmation value is not being sent to the backend in the API call. Please update this call to include the confirmation so that the server can perform its own validation as required.
| <div style={{ display: 'grid', gridTemplateColumns: '1fr 1fr', gap: 12 }}> | ||
| <GlassInput placeholder="Поточний пароль" type="password" value={emailPassword} onChange={(e) => setEmailPassword(e.target.value)} /> | ||
| <GlassInput placeholder="Новий email" type="email" value={newEmail} onChange={(e) => setNewEmail(e.target.value)} /> | ||
| </div> |
There was a problem hiding this comment.
The task requires users to "confirm the new email". This form is missing an input field for the new email confirmation. You need to add a new state variable to hold the confirmation, add a corresponding <GlassInput /> for it here, and update the handleChangeEmail function to validate that the emails match before making the API call.
| const PasswordStrength = ({ password }) => { | ||
| const getStrength = () => { | ||
| if (!password) return 0; | ||
| let score = 0; | ||
| if (password.length >= 8) score++; | ||
| if (/[A-Z]/.test(password)) score++; | ||
| if (/[0-9]/.test(password)) score++; | ||
| if (/[^A-Za-z0-9]/.test(password)) score++; | ||
| return score; | ||
| }; | ||
| const strength = getStrength(); | ||
| const labels = ['', 'Слабкий', 'Середній', 'Добрий', 'Сильний']; | ||
| const colors = ['', '#ff4444', '#ffaa00', '#88cc00', '#44cc44']; | ||
| if (!password) return null; | ||
| return ( | ||
| <div style={{ marginTop: 6 }}> | ||
| <div style={{ display: 'flex', gap: 4 }}> | ||
| {[1,2,3,4].map(i => ( | ||
| <div key={i} style={{ | ||
| flex: 1, height: 3, borderRadius: 2, | ||
| background: i <= strength ? colors[strength] : 'rgba(255,255,255,0.1)', | ||
| transition: 'background 0.3s', | ||
| }} /> | ||
| ))} | ||
| </div> | ||
| <p style={{ color: colors[strength], fontSize: 11, marginTop: 4 }}>{labels[strength]}</p> | ||
| </div> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
This PasswordStrength component is defined locally within the RegisterPage. However, a similar shared component already exists at frontend/src/components/forms/PasswordStrength.jsx. Using the shared component would improve code reuse and consistency.
Additionally, the password rules in this local version (e.g., length >= 8) seem to differ from the shared component and the backend validation, which could confuse the user.
|
|
||
| <div style={{ textAlign: 'center', color: 'rgba(255,255,255,0.4)', fontSize: 12 }}>або</div> | ||
|
|
||
| <a href="http://localhost:3005/auth/google" style={{ display: 'flex', alignItems: 'center', justifyContent: 'center', gap: 8, padding: '12px', background: 'rgba(255,255,255,0.08)', border: '1px solid rgba(255,255,255,0.2)', borderRadius: 12, color: '#fff', fontSize: 14, textDecoration: 'none' }}> |
There was a problem hiding this comment.
The URL for Google authentication is hardcoded. To make the application more maintainable, it would be better to construct this URL dynamically using the baseURL from your Axios client instance. This ensures that if the backend URL changes, you only need to update it in one place.
For example:
import { client } from '../api/client';
const googleAuthUrl = `${client.defaults.baseURL}/auth/google`;| @@ -0,0 +1,5 @@ | |||
| import 'dotenv/config'; | |||
| import { client } from './src/utils/db.js'; | |||
| import { User } from './src/models/user.js'; | |||
There was a problem hiding this comment.
To ensure the application functions correctly, you must import the Token model here. Without this import, Sequelize will not be aware of the model, and the client.sync() operation will not create the necessary tokens table in the database.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving this submission as an educational override to help you progress, but there are a few key requirements that still need to be addressed. Excellent work on fixing the Google sign-in and the database setup errors! The application's foundation is now much stronger.
The main remaining task is to implement server-side validation for confirmation fields.
- In
backend/src/controllers/user.controller.jsandauth.controller.js, you need to add logic to ensure that thepasswordandnewEmailfields match theirconfirmationcounterparts from the request body. - To support this, the frontend needs to send this data. For instance,
frontend/src/pages/ProfilePage.jsxis missing the email confirmation input, and the API calls infrontend/src/api/userApi.jsandfrontend/src/api/authApi.jsmust be updated to include the confirmation values in their payloads. - Lastly, please correct the import paths in both the root
setup.jsandbackend/setup.jsfiles, as they are currently incorrect and will prevent the scripts from running.
You're doing great work. Keep focusing on these details, and you'll have it perfect!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| import { client } from './backend/src/utils/db.js'; | ||
| import './backend/src/models/user.js'; | ||
| import './backend/src/models/token.js'; |
There was a problem hiding this comment.
These import paths are incorrect. This script is located in the backend directory, so the paths to the modules should be relative to it (e.g., './src/utils/db.js'). The current paths will resolve to backend/backend/... which will cause the script to fail.
|
|
||
| const resetPassword = async (req, res) => { | ||
| const { token } = req.params; | ||
| const { password } = req.body; |
There was a problem hiding this comment.
According to the task requirements, the password reset functionality must validate that the password and confirmation fields are equal. You need to get the confirmation from the request body and add a check to ensure it matches the password before updating it.
| }; | ||
|
|
||
| const changePassword = async (req, res) => { | ||
| const { password, newPassword } = req.body; |
There was a problem hiding this comment.
The task requires that changing a password includes a confirmation field for the new password. You need to get the confirmation from the request body and add a check to ensure it matches the newPassword before hashing and saving it.
| }; | ||
|
|
||
| const changeEmail = async (req, res) => { | ||
| const { password, newEmail } = req.body; |
There was a problem hiding this comment.
The requirements state that to change an email, the user must "confirm the new email". This implies that you should expect a confirmation field for the newEmail in the request body and validate that they match before proceeding with the change.
| resetPassword: (token, password) => | ||
| client.post(`/auth/reset-password/${token}`, { password }), |
There was a problem hiding this comment.
The task requires the password reset functionality to validate that the new password and its confirmation match. While this check might exist on the client side, it's crucial for the server to perform this validation as well. The API call should send both the password and its confirmation to the backend.
| changeEmail: (password, newEmail) => | ||
| client.patch('/users/email', { password, newEmail }), |
There was a problem hiding this comment.
The requirement to "confirm the new email" implies that a confirmation field should be sent to the backend. This function needs to be updated to accept a confirmation for the newEmail and include it in the request payload.
| } | ||
|
|
||
| try { | ||
| await userApi.changePassword(password, newPassword); |
There was a problem hiding this comment.
While you are correctly checking if the newPassword and confirmation match on the client side, the call to userApi.changePassword is missing the confirmation argument. This value needs to be passed to the API layer and sent to the backend to fulfill the server-side validation requirement.
| <Section title="Змінити email"> | ||
| <div style={{ display: 'grid', gridTemplateColumns: '1fr 1fr', gap: 12 }}> | ||
| <GlassInput placeholder="Поточний пароль" type="password" value={emailPassword} onChange={(e) => setEmailPassword(e.target.value)} /> | ||
| <GlassInput placeholder="Новий email" type="email" value={newEmail} onChange={(e) => setNewEmail(e.target.value)} /> |
There was a problem hiding this comment.
According to the task requirements, you need to add an input field here to confirm the new email. Currently, the form only allows the user to enter the new email once, but it needs to be confirmed before sending the request.
| e.preventDefault(); | ||
| if (password !== confirmation) return setError('Паролі не співпадають'); | ||
| try { | ||
| await authApi.resetPassword(token, password); |
There was a problem hiding this comment.
The task requires server-side validation for the password and confirmation. Although you have a client-side check on line 14, the API call should also send the confirmation value to the backend. You'll need to update this call to pass the confirmation state along with the password.
| import { client } from './src/utils/db.js'; | ||
| import { User } from './src/models/user.js'; |
There was a problem hiding this comment.
The paths to client and User are incorrect. Because this script is in the root directory, the paths should point to the files inside the backend directory, like './backend/src/utils/db.js'.
Built a complete auth system with email activation, JWT refresh tokens, password reset, and Google OAuth. Users can manage their profile — update name, password, and email. Secured routes on both frontend and backend.