-
Notifications
You must be signed in to change notification settings - Fork 359
add task solution #288
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
add task solution #288
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,172 @@ | ||
| import { ApiError } from '../exceptions/api.error.js'; | ||
| import { jwtService } from '../services/jwt.service.js'; | ||
| import { tokenService } from '../services/token.service.js'; | ||
| import { userService } from '../services/user.service.js'; | ||
| import bcrypt from 'bcrypt'; | ||
|
|
||
| export function validateEmail(email) { | ||
| const emailPattern = /^[\w.+-]+@([\w-]+\.){1,3}[\w-]{2,}$/; | ||
|
|
||
| if (!email) { | ||
| return 'Email is required'; | ||
| } | ||
|
|
||
| if (!emailPattern.test(email)) { | ||
| return 'Email is not valid'; | ||
| } | ||
| } | ||
|
|
||
| export function validatePassword(password) { | ||
| if (!password) { | ||
| return 'Password is required'; | ||
| } | ||
|
|
||
| if (password.length < 6) { | ||
| return 'At least 6 characters'; | ||
| } | ||
| } | ||
|
|
||
| const register = async (req, res, next) => { | ||
| const { name, email, password } = req.body; | ||
|
|
||
| const errors = { | ||
| email: validateEmail(email), | ||
| password: validatePassword(password), | ||
| }; | ||
|
|
||
| if (Object.values(errors).some((error) => error)) { | ||
| throw ApiError.badRequest('Validation error', errors); | ||
| } | ||
|
|
||
| const hashedPass = await bcrypt.hash(password, 10); | ||
|
|
||
| await userService.register(name, email, hashedPass); | ||
|
|
||
| res.send({ | ||
| message: 'OK', | ||
| }); | ||
| }; | ||
|
Comment on lines
+45
to
+48
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This email change activation endpoint should require authentication. Users should be logged in to confirm email changes. Add |
||
|
|
||
| const activate = async (req, res, next) => { | ||
| const { activationToken } = req.params; | ||
| const user = await userService.findByActivationToken(activationToken); | ||
|
|
||
| if (!user) { | ||
|
Comment on lines
+50
to
+54
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The email change activation endpoint uses authMiddleware but should be accessible without authentication since the user confirms via the token in the email link. Consider using guestMiddleware instead, similar to the account activation endpoint. |
||
| throw ApiError.notFound({ email: 'this email doesn`t exist' }); | ||
| } | ||
|
|
||
| user.activationToken = null; | ||
| await user.save(); | ||
|
|
||
| res.send({ | ||
| message: 'OK', | ||
| }); | ||
| }; | ||
|
|
||
| const login = async (req, res, next) => { | ||
| const { email, password } = req.body; | ||
|
|
||
| const user = await userService.findByEmail(email); | ||
|
|
||
| if (!user) { | ||
| throw ApiError.unauthorized({ email: 'this email doesn`t exist' }); | ||
| } | ||
|
|
||
| const isPasswordValid = await bcrypt.compare(password, user.password); | ||
|
|
||
| if (!isPasswordValid) { | ||
| throw ApiError.unauthorized({ password: 'wrong password' }); | ||
| } | ||
|
|
||
| if (user.activationToken) { | ||
| throw ApiError.unauthorized({ email: 'please activate the email' }); | ||
| } | ||
|
|
||
| await generateTokens(res, user); | ||
| }; | ||
|
|
||
| const refresh = async (req, res, next) => { | ||
| const { refreshToken } = req.cookies; | ||
|
|
||
| const userData = jwtService.verifyRefresh(refreshToken); | ||
| const token = await tokenService.getByToken(refreshToken); | ||
|
|
||
| if (!userData || !token) { | ||
| throw ApiError.unauthorized(); | ||
| } | ||
|
|
||
| const user = await userService.findByEmail(userData.email); | ||
|
|
||
| await generateTokens(res, user); | ||
| }; | ||
|
|
||
| const generateTokens = async (res, user) => { | ||
| const normalizedUser = userService.normalize(user); | ||
| const accessToken = jwtService.sign(normalizedUser); | ||
| const refreshToken = jwtService.signRefresh(normalizedUser); | ||
|
|
||
| await tokenService.save(normalizedUser.id, refreshToken); | ||
|
|
||
| res.cookie('refreshToken', refreshToken, { | ||
| maxAge: 30 * 24 * 60 * 60 * 1000, | ||
| httpOnly: true, | ||
| }); | ||
|
|
||
| res.send({ | ||
| user: normalizedUser, | ||
| accessToken, | ||
| }); | ||
| }; | ||
|
|
||
| const sendResetMessage = async (req, res, next) => { | ||
| const { email } = req.body; | ||
|
|
||
| await userService.sendResetMessage(email); | ||
|
|
||
| res.send({ | ||
| message: 'OK', | ||
| }); | ||
| }; | ||
|
|
||
| const resetPassword = async (req, res, next) => { | ||
| const { resetPasswordToken } = req.params; | ||
| const { newPassword, confirmPassword } = req.body; | ||
|
|
||
| const errors = { | ||
|
Comment on lines
+134
to
+135
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The error key is 'password' but the validated parameter is 'newPassword'. For consistency with other validation error responses (like changePasswordAuthenticated which correctly uses 'newPassword'), this should be |
||
| password: validatePassword(newPassword), | ||
| }; | ||
|
|
||
| const user = await userService.findByResetPasswordToken(resetPasswordToken); | ||
|
|
||
| if (newPassword !== confirmPassword) { | ||
| throw ApiError.badRequest('password and confirmation fields must be equal'); | ||
| } | ||
|
|
||
| if (!user) { | ||
| throw ApiError.notFound({ email: 'this email doesn`t exist' }); | ||
| } | ||
|
|
||
| if (Object.values(errors).some((error) => error)) { | ||
| throw ApiError.badRequest('Validation error', errors); | ||
| } | ||
|
|
||
| const hashedPass = await bcrypt.hash(newPassword, 10); | ||
|
|
||
| await userService.changePassword(user.email, hashedPass); | ||
|
|
||
| user.resetPasswordToken = null; | ||
| await user.save(); | ||
|
|
||
| res.send({ | ||
| message: 'OK', | ||
| }); | ||
| }; | ||
|
|
||
| export const authController = { | ||
| register, | ||
| activate, | ||
| login, | ||
| refresh, | ||
| sendResetMessage, | ||
| resetPassword, | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,144 @@ | ||
| import { ApiError } from '../exceptions/api.error.js'; | ||
| import { tokenService } from '../services/token.service.js'; | ||
| import { userService } from '../services/user.service.js'; | ||
| import { validateEmail, validatePassword } from './auth.controller.js'; | ||
| import bcrypt from 'bcrypt'; | ||
|
|
||
| const getOneActivated = async (req, res, next) => { | ||
| const id = req.user.id; | ||
|
|
||
| const user = await userService.findActivatedById(id); | ||
|
|
||
| res.send(user); | ||
| }; | ||
|
|
||
| const changePasswordAuthenticated = async (req, res, next) => { | ||
| const { oldPassword, newPassword, confirmPassword } = req.body; | ||
| const userData = req.user; | ||
|
|
||
| if (!userData) { | ||
| throw ApiError.unauthorized(); | ||
| } | ||
|
|
||
| const user = await userService.findByEmail(userData.email); | ||
|
|
||
| if (newPassword !== confirmPassword) { | ||
| throw ApiError.badRequest('password and confirmation fields must be equal'); | ||
| } | ||
|
Comment on lines
+19
to
+27
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The password validation only checks for minimum 6 characters. The task mentions 'Inform the users about the rules for a password and check them' - consider defining and documenting the complete password requirements (uppercase, lowercase, numbers, special characters, etc.) if these are part of the application specification. |
||
|
|
||
| const isPasswordValid = await bcrypt.compare(oldPassword, user.password); | ||
|
|
||
| if (!isPasswordValid) { | ||
| throw ApiError.unauthorized({ password: 'wrong password' }); | ||
| } | ||
|
|
||
| const errors = { | ||
| newPassword: validatePassword(newPassword), | ||
| }; | ||
|
|
||
| if (Object.values(errors).some((error) => error)) { | ||
| throw ApiError.badRequest('Validation error', errors); | ||
| } | ||
|
|
||
| const hashedPass = await bcrypt.hash(newPassword, 10); | ||
|
|
||
| await userService.changePassword(userData.email, hashedPass); | ||
|
|
||
| res.send({ | ||
|
Comment on lines
+28
to
+47
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Registration endpoint is missing guestMiddleware. All other non-authenticated routes (login, activation, password reset) have this protection, but registration should also be restricted to non-authenticated users only. |
||
| message: 'OK', | ||
| }); | ||
| }; | ||
|
|
||
| const changeName = async (req, res, next) => { | ||
| const { name } = req.body; | ||
| const userData = req.user; | ||
|
|
||
| if (!userData) { | ||
| throw ApiError.unauthorized(); | ||
| } | ||
|
|
||
| await userService.changeName(userData.email, name); | ||
|
|
||
| res.send({ | ||
| message: 'OK', | ||
| }); | ||
| }; | ||
|
|
||
| const changeEmail = async (req, res, next) => { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
| const { oldEmail, newEmail, password } = req.body; | ||
| const userData = req.user; | ||
|
|
||
| if (!userData) { | ||
| throw ApiError.unauthorized(); | ||
| } | ||
|
|
||
| const user = await userService.findByEmail(userData.email); | ||
|
|
||
| const isPasswordValid = await bcrypt.compare(password, user.password); | ||
|
|
||
| if (!isPasswordValid) { | ||
| throw ApiError.unauthorized({ password: 'wrong password' }); | ||
| } | ||
|
|
||
| const errors = { | ||
| email: validateEmail(newEmail), | ||
| }; | ||
|
|
||
| if (Object.values(errors).some((error) => error)) { | ||
| throw ApiError.badRequest('Validation error', errors); | ||
| } | ||
|
|
||
| if (oldEmail === newEmail) { | ||
| throw ApiError.badRequest('new email must be different'); | ||
| } | ||
|
|
||
| await userService.changeEmail(userData.email, newEmail); | ||
|
|
||
| res.send({ | ||
| message: 'OK', | ||
| }); | ||
| }; | ||
|
Comment on lines
+87
to
+100
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The /refresh endpoint has guestMiddleware, but this could block valid refresh attempts if users include a Bearer token in the Authorization header. The refresh endpoint typically needs to handle both authenticated refreshes and guest token generation. |
||
|
|
||
| const activateChangedEmail = async (req, res, next) => { | ||
| const { emailChangeToken } = req.params; | ||
| const user = await userService.findByEmailChangeToken(emailChangeToken); | ||
|
|
||
| if (!user) { | ||
| throw ApiError.notFound({ email: 'this email doesn`t exist' }); | ||
| } | ||
|
|
||
| const existingUser = await userService.findByEmail(user.pendingEmail); | ||
|
|
||
| if (existingUser) { | ||
| throw ApiError.badRequest('email already exists'); | ||
| } | ||
|
|
||
| user.email = user.pendingEmail; | ||
| user.pendingEmail = null; | ||
| user.emailChangeToken = null; | ||
|
|
||
| await user.save(); | ||
|
|
||
| res.send({ | ||
| message: 'OK', | ||
| }); | ||
| }; | ||
|
|
||
| const logout = async (req, res, next) => { | ||
| res.clearCookie('refreshToken'); | ||
|
|
||
| await tokenService.remove(req.user.id); | ||
|
|
||
| res.send({ | ||
| message: 'OK', | ||
| }); | ||
| }; | ||
|
|
||
| export const userController = { | ||
| getOneActivated, | ||
| changeName, | ||
| changeEmail, | ||
| changePasswordAuthenticated, | ||
| activateChangedEmail, | ||
| logout, | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| export class ApiError extends Error { | ||
| constructor({ message, status, errors = {} }) { | ||
| super(message); | ||
|
|
||
| this.status = status; | ||
| this.errors = errors; | ||
| } | ||
|
|
||
| static badRequest(message, errors = {}) { | ||
| return new ApiError({ | ||
| message, | ||
| errors, | ||
|
Comment on lines
+11
to
+12
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The registration route is missing guestMiddleware. According to the requirements, registration should be 'only non authenticated'. Other non-authenticated routes (login, refresh, password reset) correctly use guestMiddleware at lines 18, 23, 29, 35 - this route should follow the same pattern. |
||
| status: 400, | ||
| }); | ||
| } | ||
|
|
||
| static unauthorized(errors = {}) { | ||
| return new ApiError({ | ||
| message: 'unauthorized user', | ||
| errors, | ||
| status: 401, | ||
| }); | ||
| } | ||
|
|
||
| static notFound(errors) { | ||
| return new ApiError({ | ||
| message: 'not found', | ||
| errors, | ||
| status: 404, | ||
| }); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1,31 @@ | ||
| /* eslint-disable no-console */ | ||
| 'use strict'; | ||
|
|
||
| import express from 'express'; | ||
| import 'dotenv/config'; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing |
||
| import { authRouter } from './routers/auth.route.js'; | ||
| import { userRouter } from './routers/user.route.js'; | ||
| import { errorMiddleware } from './middlewares/errorMiddleware.js'; | ||
| import cookieParser from 'cookie-parser'; | ||
|
|
||
| const PORT = process.env.PORT || 3005; | ||
|
|
||
| const app = express(); | ||
|
|
||
| app.use(express.json()); | ||
| app.use(cookieParser()); | ||
|
|
||
| app.use(authRouter); | ||
| app.use('/user', userRouter); | ||
|
|
||
| app.use((req, res) => { | ||
| res.status(404).send(` | ||
| <h1>Page not found</h1> | ||
| `); | ||
| }); | ||
|
|
||
| app.use(errorMiddleware); | ||
|
|
||
| app.listen(PORT, () => { | ||
| console.log('server is running'); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The registration route is missing
guestMiddleware. According to requirements, registration should only be accessible to non-authenticated users. Add guestMiddleware to this route.