more checks and tests cleanup

salesforce · Jan 31, 2017 · 5a2f2d8 · 5a2f2d8
1 parent b88bfe0
commit 5a2f2d8
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 23 deletions.
diff --git a/api/v1/controllers/userTokens.js b/api/v1/controllers/userTokens.js
@@ -82,7 +82,9 @@ module.exports = {
 
       // Default token cannot be deleted
       if (token.name === token.User.name) {
-        u.forbidden(next);
+        throw new apiErrors.ForbiddenError({
+          explanation: 'Forbidden.',
+        });
       }
 
       tokenToDel = token;
@@ -96,6 +98,7 @@ module.exports = {
         return tokenToDel.destroy();
       }
 
+      // Get user details from req
       return authUtils.getUser(req)
       .then((currentUser) => {
         // OK to delete if user is NOT admin but is deleting own token
@@ -104,14 +107,24 @@ module.exports = {
         }
 
         // else forbidden
-        return u.forbidden(next);
+        throw new apiErrors.ForbiddenError({
+          explanation: 'Forbidden.',
+        });
       })
       .catch((err) => {
         throw err;
       });
     })
-    .then((o) => res.status(httpStatus.OK)
-      .json(u.responsify(o, helper, req.method)))
+    .then((o) => {
+      if (o) {
+        // object deleted successfully
+        res.status(httpStatus.OK)
+        .json(u.responsify(o, helper, req.method));
+      } else if (o instanceof Error) {
+        // forbidden err
+        throw o;
+      }
+    })
     .catch((err) => u.handleError(next, err, helper.modelName));
   },
 

diff --git a/tests/api/v1/userTokens/delete.js b/tests/api/v1/userTokens/delete.js
@@ -18,9 +18,7 @@ const tu = require('../../../testUtils');
 const u = require('./utils');
 const path = '/v1/users';
 const expect = require('chai').expect;
-const Profile = tu.db.Profile;
-const User = tu.db.User;
-const Token = tu.db.Token;
+
 const jwtUtil = require('../../../../utils/jwtUtil');
 const adminUser = require('../../../../config').db.adminUser;
 const regPath = '/v1/register';
@@ -176,9 +174,9 @@ describe(`api: DELETE ${path}/U/tokens/T`, () => {
     });
   });
 
-  it('not admin user, user found but token name not found', (done) => {
+  it('non-admin user, user found but token name not found', (done) => {
     api.delete(`${path}/${uname}/tokens/foo`)
-    .set('Authorization', '???')
+    .set('Authorization', unameToken)
     .expect(constants.httpStatus.NOT_FOUND)
     .end((err, res) => {
       if (err) {

diff --git a/tests/api/v1/userTokens/get.js b/tests/api/v1/userTokens/get.js
@@ -30,6 +30,7 @@ describe(`api: GET ${path}/U/tokens`, () => {
   const tname2 = `${tu.namePrefix}Tom`;
   const tnameOther = `${tu.namePrefix}Dumbledore`;
   let userId;
+  let unameToken;
 
   before((done) => {
     // create user __test@refocus.com
@@ -45,6 +46,7 @@ describe(`api: GET ${path}/U/tokens`, () => {
       }
 
       userId = res.body.id;
+      unameToken = res.body.token;
 
       // create token ___Voldemort
       api.post(tokenPath)
@@ -97,7 +99,7 @@ describe(`api: GET ${path}/U/tokens`, () => {
 
   it('user by name and token found', (done) => {
     api.get(`${path}/${uname}/tokens/${tname1}`)
-    .set('Authorization', '???')
+    .set('Authorization', unameToken)
     .expect(constants.httpStatus.OK)
     .end((err, res) => {
       if (err) {
@@ -112,7 +114,7 @@ describe(`api: GET ${path}/U/tokens`, () => {
 
   it('user by Id and token found', (done) => {
     api.get(`${path}/${userId}/tokens/${tname1}`)
-    .set('Authorization', '???')
+    .set('Authorization', unameToken)
     .expect(constants.httpStatus.OK)
     .end((err, res) => {
       if (err) {
@@ -127,7 +129,7 @@ describe(`api: GET ${path}/U/tokens`, () => {
 
   it('user not found, one token', (done) => {
     api.get(`${path}/who@what.com/tokens/foo`)
-    .set('Authorization', '???')
+    .set('Authorization', unameToken)
     .expect(constants.httpStatus.NOT_FOUND)
     .end((err, res) => {
       if (err) {
@@ -141,7 +143,7 @@ describe(`api: GET ${path}/U/tokens`, () => {
 
   it('user not found, all tokens', (done) => {
     api.get(`${path}/who@what.com/tokens`)
-    .set('Authorization', '???')
+    .set('Authorization', unameToken)
     .expect(constants.httpStatus.OK)
     .end((err, res) => {
       if (err) {
@@ -155,7 +157,7 @@ describe(`api: GET ${path}/U/tokens`, () => {
 
   it('user found but token name not found', (done) => {
     api.get(`${path}/${uname}/tokens/foo`)
-    .set('Authorization', '???')
+    .set('Authorization', unameToken)
     .expect(constants.httpStatus.NOT_FOUND)
     .end((err, res) => {
       if (err) {
@@ -169,7 +171,7 @@ describe(`api: GET ${path}/U/tokens`, () => {
 
   it('user found, token found, but token of different user', (done) => {
     api.get(`${path}/${uname}/tokens/${tnameOther}`)
-    .set('Authorization', '???')
+    .set('Authorization', unameToken)
     .expect(constants.httpStatus.NOT_FOUND)
     .end((err, res) => {
       if (err) {
@@ -183,7 +185,7 @@ describe(`api: GET ${path}/U/tokens`, () => {
 
   it('user, get all tokens', (done) => {
     api.get(`${path}/${uname}/tokens`)
-    .set('Authorization', '???')
+    .set('Authorization', unameToken)
     .expect(constants.httpStatus.OK)
     .end((err, res) => {
       if (err) {

diff --git a/tests/api/v1/users/delete.js b/tests/api/v1/users/delete.js
@@ -18,18 +18,11 @@ const tu = require('../../../testUtils');
 const u = require('./utils');
 const path = '/v1/users';
 const expect = require('chai').expect;
-// const Profile = tu.db.Profile;
-// const User = tu.db.User;
 const Token = tu.db.Token;
-const jwtUtil = require('../../../../utils/jwtUtil');
-const adminUser = require('../../../../config').db.adminUser;
 const registerPath = '/v1/register';
 const tokenPath = '/v1/tokens';
 
 describe(`api: DELETE ${path}/:id`, () => {
-  const predefinedAdminUserToken = jwtUtil.createToken(
-    adminUser.name, adminUser.name
-  );
   const uname = `${tu.namePrefix}test@refocus.com`;
   const tname = `${tu.namePrefix}Voldemort`;
   let userId;