Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions src/config/database.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/* eslint-disable no-console */
require('dotenv').config();

const { Sequelize } = require('sequelize');

const sequelize = new Sequelize(
process.env.DATABASE,
process.env.USERNAME_DATABASE,
process.env.PASSWORD_DATABASE,
{
host: process.env.HOST_DATABASE,
dialect: 'postgres',
},
);

module.exports = { sequelize };
106 changes: 106 additions & 0 deletions src/controllers/auth.controller.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
/* eslint-disable no-console */
const User = require('../models/user.model');
const bcrypt = require('bcrypt');
const {
validateEmail,
validatePassword,
send,
} = require('../services/auth.services');
const { v4: uuidv4 } = require('uuid');
const jwt = require('jsonwebtoken');

const registration = async (req, res) => {
const { name, email, password } = req.body;
const errorEmail = validateEmail(email);
const errorPassword = validatePassword(password);

if (errorEmail) {
return res.status(400).send(errorEmail);
}

if (errorPassword) {
return res.status(400).send(errorPassword);
}

const exist = await User.findOne({ where: { email } });

if (exist !== null) {
return res.status(401).send('User already exist');
}

const cashedPassword = await bcrypt.hash(password, 10);

const uuid = uuidv4();

await User.create({
name: name,
email: email,
password: cashedPassword,
activationToken: uuid,
});

await send(
email,
'Activate email',
`go to link http://localhost:3000/activation/${uuid}`,
);
res.send(201);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

While res.send(201) works, it's more idiomatic in Express to be explicit with your status codes. Consider using res.sendStatus(201) or res.status(201).send('User created'). This makes the code's intent clearer.

};

const activation = async (req, res) => {
const { activationToken } = req.params;

const user = await User.findOne({ where: { activationToken } });

if (!user) {
res.status(404).send('invalind activation token');
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

When the activation token is invalid, you send a 404 response but don't stop the function's execution. The code will then proceed to line 59 and try to access properties of user, which is null, causing a TypeError and crashing the server. You should add a return statement after sending the response.

}

user.isActivated = true;
user.activationToken = null;
await user.save();
res.redirect('http://localhost:3000/profile');
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The task requires redirecting to the profile page after activation, which you've done. However, the profile page is protected and requires the user to be authenticated. Currently, after activation, the user isn't logged in, so they'll be blocked from accessing the profile.

To fix this, you should log the user in by creating a JWT and setting it as a cookie, just like you do in the login function, before redirecting them to their profile.

};

const login = async (req, res) => {
const { email, password } = req.body;

const existUser = await User.findOne({ where: { email } });

if (!existUser) {
res.send('User not found');
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If the user does not exist, you send a response but do not stop the function's execution. The code will then attempt to access existUser.password on line 74, which will cause a TypeError because existUser is null. You need to add a return statement here.

}

const isValid = await bcrypt.compare(password, existUser.password);

if (!isValid) {
res.send('Invalid credentials');
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Similarly, if the password is not valid, you should return after sending the response. While it might not crash immediately in this case, continuing execution after sending a response is incorrect and can lead to unexpected behavior, such as trying to set headers after they have already been sent.

}

if (!existUser.isActivated) {
await send(
email,
'Activate email',
`go to link http://localhost:3000/activation/${existUser.activationToken}`,
);

return res.status(403).send('Activate email first');
}

const token = jwt.sign(
{ id: existUser.id, email: existUser.email },
process.env.JWT_SECRET,
{ expiresIn: '30 days' },
);

res.cookie('token', token, { httpOnly: true });
res.redirect('http://localhost:3000/profile');
};

module.exports = {
authController: {
registration,
activation,
login,
},
};
147 changes: 147 additions & 0 deletions src/controllers/user.controller.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
/* eslint-disable no-console */
const User = require('../models/user.model');
const bcrypt = require('bcrypt');
const { v4: uuidv4 } = require('uuid');

const {
validateEmail,
validatePassword,
send,
} = require('../services/auth.services');

const profile = async (req, res) => {
const userId = req.user.id;

const user = await User.findByPk(userId, {
attributes: ['id', 'name', 'email'],
});

res.send(user);
};

const changeName = async (req, res) => {
const userId = req.user.id;
const { name } = req.body;

const user = await User.findByPk(userId);

user.name = name;
await user.save();
res.send('Username changed');
};

const changePassword = async (req, res) => {
const userId = req.user.id;
const { password, newPassword, confirm } = req.body;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

According to the task requirements, this function should expect fields named newPassword and confirmation for changing the password. The current code is using confirm.


if (!confirm) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The task requires a confirmation field for the new password. The logic should verify that newPassword and confirm are identical. Currently, you are only checking if confirm is truthy, which doesn't meet the requirement.

res.send('You need confirm change');
}

const user = await User.findByPk(userId);

const isValid = await bcrypt.compare(password, user.password);

if (!isValid) {
res.status(401).send('Invalid credentials');
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

After sending a response for an invalid old password, you must add a return statement. Without it, the function will continue to execute and will change the password even if the old one was incorrect.

}

user.password = await bcrypt.hash(newPassword, 10);
await user.save();
res.send('password changed');
};

const resetPassword = async (req, res) => {
const { email } = req.body;

const user = await User.findOne({ where: { email } });

if (!user) {
return res.status(401).send('Invalid credentials');
}

const token = uuidv4();

user.resetToken = token;

await send(
user.email,
'password reset',
`go to link http://localhost:3000/reset/${token}`,
);

await user.save();
res.send('email sent');
};

const resetPasswordConfirm = async (req, res) => {
const { resetToken } = req.params;
const { password, repeatPassword } = req.body;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The task requirements specify that the password reset confirmation should expect password and confirmation fields. This implementation is using repeatPassword instead.


const errorPassword = validatePassword(password);

if (errorPassword) {
return res.status(400).send(errorPassword);
}

if (password !== repeatPassword) {
res.send('passwords not equil');
}

const user = await User.findOne({ where: { resetToken } });
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You should check if a user was found with the provided resetToken. If user is null (because the token is invalid), the subsequent code will throw an error when trying to access user.password. Add a check for user and return an error response if it's not found.


user.password = await bcrypt.hash(password, 10);
user.resetToken = null;
user.save();
res.send('password changed');
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The task requires showing a success page with a link to login. Simply sending the text 'password changed' is not sufficient. Consider redirecting to a success page or returning a more structured response that the client-side can use to render the correct view.

};

const changeEmail = async (req, res) => {
const userId = req.user.id;
const { password, email } = req.body;

const user = await User.findByPk(userId);

const isValid = await bcrypt.compare(password, user.password);

if (!isValid) {
res.status(401).send('Invalid credentials');
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You're missing return statements after sending error responses on lines 38, 46, 88, and 108. Without them, the function continues execution even after an error has been sent, leading to unexpected behavior. For example, here, an invalid password would still allow the email change process to continue.

}

const errorEmail = validateEmail(email);

if (errorEmail) {
return res.status(400).send(errorEmail);
}

const uuid = uuidv4();

await send(
user.email,
'Activate email',
`go to link http://localhost:3000/activation/${uuid}`,
);
Comment on lines +131 to +141
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The implementation for changing an email doesn't fully meet the requirements:

  1. Confirm new email: The task requires confirming the new email, which usually means the user submits the new email twice and the backend verifies they match. This check is missing.
  2. Notify old email: The task requires you to notify the old email address about the change. Instead, you are sending a new activation link to the old address. The activation link should be sent to the new email address for verification.


user.email = email;
user.isActivated = false;
user.activationToken = uuid;
await user.save();
res.redirect('http://localhost:3000/login');
};

const logout = async (req, res) => {
res.clearCookie('token');
res.redirect('/login');
};

module.exports = {
userController: {
profile,
changeName,
changePassword,
changeEmail,
logout,
resetPassword,
resetPasswordConfirm,
},
};
31 changes: 31 additions & 0 deletions src/index.js
Original file line number Diff line number Diff line change
@@ -1 +1,32 @@
/* eslint-disable max-len */
/* eslint-disable no-console */
'use strict';

const express = require('express');
const cookieParser = require('cookie-parser');
const { sequelize } = require('../src/config/database'); // import connect to database
const { router: authRouter } = require('../src/routes/auth.routes');
const { router: userRouter } = require('../src/routes/user.routes'); // import routes

async function start() {
try {
await sequelize.authenticate(); // connect to database
console.log('database connected');

await sequelize.sync({ force: true }); // create tables
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Using { force: true } is useful for development as it resets the database on every server start. However, be very careful with this in a production environment, as it will drop all your tables and delete all data. For production apps, it's safer to use database migration tools to manage schema changes.

console.log('tables created');

const app = express(); // create express app

app.use(cookieParser());
app.use(express.json()); // allow json in body
app.use(authRouter);
app.use(userRouter);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The task requires you to return a 404 error for any pages that are not defined by your routers. You should add a catch-all middleware here, after all other routes, to handle requests to non-existent pages.


app.listen(3000, () => console.log('server running on 3000 port')); // start server
} catch (err) {
console.error('unable connect to database', err);
}
}

start();
32 changes: 32 additions & 0 deletions src/middlewares/auth.middleware.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
const jwt = require('jsonwebtoken');

require('dotenv').config();

function authMiddleware(req, res, next) {
const token = req.cookies.token;

if (!token) {
return res.status(401).send('unautorised');
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

There's a small typo here. It should be "unauthorized".

}

try {
const userData = jwt.verify(token, process.env.JWT_SECRET);

req.user = userData;
next();
} catch {
return res.status(401).send('Invalid token');
}
}

function guestMiddleware(req, res, next) {
const token = req.cookies.token;

if (token) {
return res.redirect('/profile');
}

next();
}

module.exports = { authMiddleware, guestMiddleware };
33 changes: 33 additions & 0 deletions src/models/user.model.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
const { DataTypes } = require('sequelize');
const { sequelize } = require('../config/database');

const Users = sequelize.define('Users', {
name: {
type: DataTypes.STRING,
allowNull: false,
},
email: {
type: DataTypes.STRING,
allowNull: false,
unique: true,
},
password: {
type: DataTypes.STRING,
allowNull: false,
},
isActivated: {
type: DataTypes.BOOLEAN,
allowNull: true,
defaultValue: false,
},
activationToken: {
type: DataTypes.STRING,
allowNull: true,
},
resetToken: {
type: DataTypes.STRING,
allowNull: true,
},
});

module.exports = Users;
21 changes: 21 additions & 0 deletions src/routes/auth.routes.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/* eslint-disable no-unused-vars */
const express = require('express');
const router = express.Router();
const User = require('../models/user.model');
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The User model is imported but never used in this file. It's good practice to remove any unused imports to keep the code clean.


const { authController } = require('../controllers/auth.controller');
const {
authMiddleware,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The authMiddleware is imported here but it's not used on any of the routes in this file. This import can be safely removed.

guestMiddleware,
} = require('../middlewares/auth.middleware');

router.post('/registration', guestMiddleware, authController.registration);

router.get(
'/activation/:activationToken',
guestMiddleware,
authController.activation,
);
router.post('/login', guestMiddleware, authController.login);

module.exports = { router };
Loading
Loading