feat(validation): improve validation logic

- add more descriptive error messages - return all errors found in request instead of single error - refactor controllers to have consistent return [Delivers #159616864]
olusoladavid · Aug 8, 2018 · d0e99f0 · d0e99f0
1 parent 92ee81b
commit d0e99f0
Show file tree

Hide file tree

Showing 6 changed files with 122 additions and 58 deletions.
diff --git a/server/controllers/entryController.js b/server/controllers/entryController.js
@@ -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 } });
       },
     );
   }
@@ -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]);
       },
     );
   }
@@ -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]);
       },
     );
   }
@@ -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
@@ -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' },
           });
         }
@@ -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]);
       },
     );
   }
@@ -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);
       },
     );
   }

diff --git a/server/controllers/userController.js b/server/controllers/userController.js
@@ -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
@@ -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 });
         });
       });
     });
@@ -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 });
       });
     });
   }
@@ -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,
+        });
+      });
   }
 }
 

diff --git a/server/utils/validate.js b/server/utils/validate.js
@@ -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 })
@@ -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 = [

diff --git a/server/utils/verifyToken.js b/server/utils/verifyToken.js
@@ -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);

diff --git a/test/sampleData.js b/test/sampleData.js
@@ -8,6 +8,10 @@ const sampleData = {
     password: null,
   },
   incorrectUser: {
+    email: 'john.dave3@gmail.com',
+    password: 'nopassword',
+  },
+  incorrectUser2: {
     email: 'john.dave@gmail.com',
     password: 'nopassword',
   },

diff --git a/test/test.js b/test/test.js
@@ -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);
     }
@@ -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();
       });
   });
@@ -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', () => {
@@ -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');
@@ -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', () => {