Skip to content
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

#159616864 Improve validation logic #32

Merged
merged 1 commit into from
Aug 8, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
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
60 changes: 31 additions & 29 deletions server/controllers/entryController.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ class entryController {
[req.authorizedUser.email, limit, page * limit],
(err, result) => {
if (err) {
console.log(err);
return res.status(500).json({ error: { message: 'An error occurred on the server' } });
res.status(500).json({ error: { message: 'An error occurred on the server' } });
return;
}
return res.status(200).json({ entries: result.rows, meta: { limit, page } });
res.status(200).json({ entries: result.rows, meta: { limit, page } });
},
);
}
Expand All @@ -45,24 +45,22 @@ class entryController {
// validate entry fields - 400
const errorsFound = validationResult(req);
if (!errorsFound.isEmpty()) {
return res.status(400).json({ error: { message: errorsFound.array()[0].msg } });
res.status(400).json({ error: { message: errorsFound.array() } });
return;
}

const entry = {
title: req.body.title,
content: req.body.content,
isFavorite: req.body.is_favorite,
};
const { title, content, is_favorite: isFavorite } = req.body;

// add entry to database
query(
queries.insertOneEntry,
[req.authorizedUser.email, entry.title, entry.content, entry.isFavorite],
[req.authorizedUser.email, title, content, isFavorite],
(err, result) => {
if (err) {
console.log(err);
return res.status(500).json({ error: { message: 'An error occurred on the server' } });
res.status(500).json({ error: { message: 'An error occurred on the server' } });
return;
}
return res.status(201).json(result.rows[0]);
res.status(201).json(result.rows[0]);
},
);
}
Expand All @@ -80,13 +78,14 @@ class entryController {
queries.getOneEntry, [req.authorizedUser.email, req.params.id],
(err, result) => {
if (err) {
console.log(err);
return res.status(500).json({ error: { message: 'An error occurred on the server' } });
res.status(500).json({ error: { message: 'An error occurred on the server' } });
return;
}
if (!result.rowCount) {
return res.status(404).json({ error: { message: 'Entry not found' } });
res.status(404).json({ error: { message: 'Entry not found' } });
return;
}
return res.status(200).json(result.rows[0]);
res.status(200).json(result.rows[0]);
},
);
}
Expand All @@ -104,7 +103,8 @@ class entryController {
// validate entry fields - 400
const errorsFound = validationResult(req);
if (!errorsFound.isEmpty()) {
return res.status(400).json({ error: { message: errorsFound.array()[0].msg } });
res.status(400).json({ error: { message: errorsFound.array() } });
return;
}

// deconstruct request body
Expand All @@ -119,16 +119,17 @@ class entryController {
queries.getEntryCreationDate, [req.authorizedUser.email, req.params.id],
(err, result) => {
if (err) {
console.log(err);
return res.status(500).json({ error: { message: 'An error occurred on the server' } });
res.status(500).json({ error: { message: 'An error occurred on the server' } });
return;
}
if (!result.rowCount) {
return res.status(404).json({ error: { message: 'Entry not found' } });
res.status(404).json({ error: { message: 'Entry not found' } });
return;
}
const createdOn = new Date(result.rows[0].created_on).getTime();
const hoursSinceCreated = (Date.now() - createdOn) / (1000 * 60 * 60);
if (hoursSinceCreated > 24) {
return res.status(403).json({
res.status(403).json({
error: { message: 'Cannot update entry after 24 hours' },
});
}
Expand All @@ -140,10 +141,10 @@ class entryController {
[title, content, isFavorite, req.authorizedUser.email, req.params.id],
(err, result) => {
if (err) {
console.log(err);
return res.status(500).json({ error: { message: 'An error occurred on the server' } });
res.status(500).json({ error: { message: 'An error occurred on the server' } });
return;
}
return res.status(200).json(result.rows[0]);
res.status(200).json(result.rows[0]);
},
);
}
Expand All @@ -161,13 +162,14 @@ class entryController {
queries.deleteOneEntry, [req.authorizedUser.email, req.params.id],
(err, result) => {
if (err) {
console.log(err);
return res.status(500).json({ error: { message: 'An error occurred on the server' } });
res.status(500).json({ error: { message: 'An error occurred on the server' } });
return;
}
if (!result.rowCount) {
return res.status(404).json({ error: { message: 'Entry not found' } });
res.status(404).json({ error: { message: 'Entry not found' } });
return;
}
return res.status(204).json();
res.sendStatus(204);
},
);
}
Expand Down
48 changes: 30 additions & 18 deletions server/controllers/userController.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,28 +22,33 @@ class userController {
// validate email and password - 400
const errorsFound = validationResult(req);
if (!errorsFound.isEmpty()) {
return res.status(400).json({ error: { message: errorsFound.array()[0].msg } });
res.status(400).json({ error: { message: errorsFound.array() } });
return;
}

// check if email already exists - 409
query(queries.getOneUser, [email], (qErr, user) => {
if (qErr) {
return res.status(500).json({ error: { message: 'An error occurred on the server.' } });
res.status(500).json({ error: { message: 'An error occurred on the server.' } });
return;
}
if (user.rows.length) {
return res.status(409).json({ error: { message: 'User already exists. Please login.' } });
res.status(409).json({ error: { message: 'User already exists. Please login.' } });
return;
}

// hash the password
bcrypt.hash(password, 5, (bErr, hash) => {
if (bErr) {
return res.status(500).json({ error: { message: 'An error occurred on the server.' } });
res.status(500).json({ error: { message: 'An error occurred on the server.' } });
return;
}

// insert new user into table, returning data
query(queries.insertOneUser, [email, hash], (qErr2, newUser) => {
if (qErr2) {
return res.status(500).json({ error: { message: 'An error occurred on the server.' } });
res.status(500).json({ error: { message: 'An error occurred on the server.' } });
return;
}

// create token using new data and sign with password hash+lastLogin+lastLogout
Expand All @@ -52,8 +57,8 @@ class userController {
const data = { email: userInfo.email, createdOn: userInfo.created_on };
const token = jwt.sign(data, jwtSecret, { expiresIn: '2h' });

// return signed token - 201
return res.status(201).json({ token });
// signed token - 201
res.status(201).json({ token });
});
});
});
Expand All @@ -74,27 +79,33 @@ class userController {
// validate email and password - 400
const errorsFound = validationResult(req);
if (!errorsFound.isEmpty()) {
return res.status(400).json({ error: { message: errorsFound.array()[0].msg } });
res.status(400).json({ error: { message: errorsFound.array() } });
return;
}
// fetch user
query(queries.getOneUser, [email], (qErr, userData) => {
if (qErr) {
return res.status(404).json({ error: { message: 'An error occurred on the server.' } });
res.status(404).json({ error: { message: 'An error occurred on the server.' } });
return;
}
// if error in fetch, user does not exist - 404
if (!userData.rows.length) return res.status(404).json({ error: { message: 'User does not exist' } });
if (!userData.rows.length) {
res.status(422).json({ error: { message: 'Email or Password is incorrect' } });
return;
}
// check password
bcrypt.compare(password, userData.rows[0].password, (bErr, valid) => {
if (!valid) {
return res.status(422).json({ error: { message: 'Email or Password is incorrect' } });
res.status(422).json({ error: { message: 'Email or Password is incorrect' } });
return;
}
// create token
const jwtSecret = process.env.SECRET_KEY;
const token = jwt.sign({
email: userData.rows[0].email, createdOn: userData.rows[0].created_on,
}, jwtSecret);
// return signed token - 200
return res.status(200).json({ token });
res.status(200).json({ token });
});
});
}
Expand All @@ -111,13 +122,14 @@ class userController {
query(queries.getEntriesCount, [req.authorizedUser.email],
(err, result) => {
if (err) {
console.log(err);
return res.status(500).json({ error: { message: 'An error occurred on the server' } });
res.status(500).json({ error: { message: 'An error occurred on the server' } });
return;
}
console.log(result.rows);
return res.status(200).json({ email: req.authorizedUser.email, entriesCount: result.rows[0].count });
},
);
res.status(200).json({
email: req.authorizedUser.email,
entriesCount: result.rows[0].count,
});
});
}
}

Expand Down
30 changes: 24 additions & 6 deletions server/utils/validate.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,14 @@ class validate {
validate.signupInputs = [
check('email')
.isEmail()
.withMessage('Your email is invalid'),
.withMessage('Your email is invalid')
.not()
.isEmpty()
.withMessage('Your email should not be empty'),
check('password')
.not()
.isEmpty()
.withMessage('Your password should not be empty')
.isString()
.withMessage('Your password is invalid')
.isLength({ min: 5 })
Expand All @@ -27,22 +33,34 @@ validate.signupInputs = [
validate.loginInputs = [
check('email')
.isEmail()
.withMessage('Your email is invalid'),
.withMessage('Your email is invalid')
.not()
.isEmpty()
.withMessage('Your email should not be empty'),
check('password')
.isString()
.withMessage('Your password is invalid'),
.withMessage('Your password is invalid')
.not()
.isEmpty()
.withMessage('Your password should not be empty'),
];

validate.newEntry = [
check('title')
.isString()
.withMessage('Title should be a string'),
.withMessage('Title should be a string')
.not()
.isEmpty()
.withMessage('Title should not be empty'),
check('content')
.isString()
.withMessage('Content should be a string'),
.withMessage('Content should be a string')
.not()
.isEmpty()
.withMessage('Content should not be empty'),
check('is_favorite')
.isBoolean()
.withMessage('isFavorite property of a story should be boolean'),
.withMessage('Story should either be favorited or not'),
];

validate.modifyEntry = [
Expand Down
3 changes: 2 additions & 1 deletion server/utils/verifyToken.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ dotenv.config();
const verifyToken = (req, res, next) => {
try {
if (!req.headers.authorization) {
return res
res
.status(401)
.json({ error: { message: 'Authorization failed. Please provide a token' } });
return;
}
const token = req.headers.authorization.split(' ')[1];
const authorizedUser = jwt.verify(token, process.env.SECRET_KEY);
Expand Down
4 changes: 4 additions & 0 deletions test/sampleData.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ const sampleData = {
password: null,
},
incorrectUser: {
email: 'john.dave3@gmail.com',
password: 'nopassword',
},
incorrectUser2: {
email: 'john.dave@gmail.com',
password: 'nopassword',
},
Expand Down
35 changes: 31 additions & 4 deletions test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@ const makeAuthHeader = authToken => `Bearer ${authToken}`;

before(() => {
// remove all entries
query('TRUNCATE TABLE entries CASCADE', (err, res) => {
query('TRUNCATE TABLE entries CASCADE', (err) => {
if (err) {
console.log(err);
}
});
// remove all users
query('TRUNCATE TABLE users CASCADE', (err, res) => {
query('TRUNCATE TABLE users CASCADE', (err) => {
if (err) {
console.log(err);
}
Expand Down Expand Up @@ -93,7 +93,7 @@ describe('/POST /auth/login', () => {
expect(res).to.have.status(200);
expect(res.body).to.be.an('object');
expect(res.body).to.have.property('token');
token = res.body.token;
({ token } = res.body);
done();
});
});
Expand Down Expand Up @@ -123,6 +123,19 @@ describe('/POST /auth/login', () => {
done();
});
});

it('should return 422 unprocessable request when a user tries to login with the valid but wrong credentials', (done) => {
chai
.request(app)
.post('/api/v1/auth/login')
.send(sampleData.incorrectUser2)
.end((err, res) => {
expect(res).to.have.status(422);
expect(res.body).to.be.an('object');
expect(res.body).to.have.property('error');
done();
});
});
});

describe('/GET entries', () => {
Expand Down Expand Up @@ -157,7 +170,7 @@ describe('/GET entries', () => {
chai
.request(app)
.get('/api/v1/entries')
.set('Authorization', makeAuthHeader(''))
.set('Authorization', null)
.end((err, res) => {
expect(res).to.have.status(401);
expect(res.body).to.be.an('object');
Expand Down Expand Up @@ -213,6 +226,20 @@ describe('/POST entries', () => {
done();
});
});

it('should return 401 unauthorized error when passed valid data with no token', (done) => {
chai
.request(app)
.post('/api/v1/entries')
.set('Authorization', null)
.send(sampleData.validEntry)
.end((err, res) => {
expect(res).to.have.status(401);
expect(res.body).to.be.an('object');
expect(res.body).to.be.have.property('error');
done();
});
});
});

describe('/GET entries', () => {
Expand Down