Send 403 when a user does not have permission to add writers

salesforce · Jan 5, 2017 · ce8f00d · ce8f00d
1 parent 9c9214c
commit ce8f00d
Show file tree

Hide file tree

Showing 13 changed files with 205 additions and 88 deletions.
diff --git a/api/v1/controllers/aspects.js b/api/v1/controllers/aspects.js
@@ -18,6 +18,8 @@ const doDeleteAllAssoc =
                     require('../helpers/verbs/doDeleteAllBToMAssoc');
 const doDeleteOneAssoc =
                     require('../helpers/verbs/doDeleteOneBToMAssoc');
+const doPostAssoc =
+                    require('../helpers/verbs/doPostBToMAssoc');
 const doFind = require('../helpers/verbs/doFind');
 const doGet = require('../helpers/verbs/doGet');
 const doPatch = require('../helpers/verbs/doPatch');
@@ -127,23 +129,11 @@ module.exports = {
     const params = req.swagger.params;
     const toPost = params.queryBody.value;
     const options = {};
-    let usersInsts;
     options.where = u.whereClauseForNameInArr(toPost);
     userProps.model.findAll(options)
     .then((usrs) => {
-      usersInsts = usrs;
-      return u.findByKey(helper, req.swagger.params);
-    })
-    .then((asp) => asp.addWriters(usersInsts))
-    .then((o) => {
-      /*
-       * The resolved object is either an array of arrays (when
-       * writers are added) or just an empty array when no writers are added.
-       * The popping is done to get the array from the array of arrays
-       */
-      let retval = o.length ? o.pop() : o;
-      retval = u.responsify(retval, helper, req.method);
-      res.status(httpStatus.CREATED).json(retval);
+      doPostAssoc(req, res, next, helper,
+        helper.belongsToManyAssoc.users, usrs);
     })
     .catch((err) => u.handleError(next, err, helper.modelName));
   }, // postAspectWriters

diff --git a/api/v1/controllers/lenses.js b/api/v1/controllers/lenses.js
@@ -18,6 +18,8 @@ const doDeleteAllAssoc =
                     require('../helpers/verbs/doDeleteAllBToMAssoc');
 const doDeleteOneAssoc =
                     require('../helpers/verbs/doDeleteOneBToMAssoc');
+const doPostAssoc =
+                    require('../helpers/verbs/doPostBToMAssoc');
 const doFind = require('../helpers/verbs/doFind');
 const u = require('../helpers/verbs/utils');
 const httpStatus = require('../constants').httpStatus;
@@ -250,23 +252,11 @@ module.exports = {
     const params = req.swagger.params;
     const toPost = params.queryBody.value;
     const options = {};
-    let usersInsts;
     options.where = u.whereClauseForNameInArr(toPost);
     userProps.model.findAll(options)
     .then((usrs) => {
-      usersInsts = usrs;
-      return u.findByKey(helper, req.swagger.params);
-    })
-    .then((lns) => lns.addWriters(usersInsts))
-    .then((o) => {
-      /*
-       * The resolved object is either an array of arrays (when
-       * writers are added) or just an empty array when no writers are added.
-       * The popping is done to get the array from the array of arrays
-       */
-      let retval = o.length ? o.pop() : o;
-      retval = u.responsify(retval, helper, req.method);
-      res.status(httpStatus.CREATED).json(retval);
+      doPostAssoc(req, res, next, helper,
+        helper.belongsToManyAssoc.users, usrs);
     })
     .catch((err) => u.handleError(next, err, helper.modelName));
   }, // postLensWriters

diff --git a/api/v1/controllers/perspectives.js b/api/v1/controllers/perspectives.js
@@ -17,6 +17,8 @@ const doDeleteAllAssoc =
                     require('../helpers/verbs/doDeleteAllBToMAssoc');
 const doDeleteOneAssoc =
                     require('../helpers/verbs/doDeleteOneBToMAssoc');
+const doPostAssoc =
+                    require('../helpers/verbs/doPostBToMAssoc');
 const httpStatus = require('../constants').httpStatus;
 const u = require('../helpers/verbs/utils');
 const doDelete = require('../helpers/verbs/doDelete');
@@ -155,23 +157,11 @@ module.exports = {
     const params = req.swagger.params;
     const toPost = params.queryBody.value;
     const options = {};
-    let usersInsts;
     options.where = u.whereClauseForNameInArr(toPost);
     userProps.model.findAll(options)
     .then((usrs) => {
-      usersInsts = usrs;
-      return u.findByKey(helper, req.swagger.params);
-    })
-    .then((pers) => pers.addWriters(usersInsts))
-    .then((o) => {
-      /*
-       * The resolved object is either an array of arrays (when
-       * writers are added) or just an empty array when no writers are added.
-       * The popping is done to get the array from the array of arrays
-       */
-      let retval = o.length ? o.pop() : o;
-      retval = u.responsify(retval, helper, req.method);
-      res.status(httpStatus.CREATED).json(retval);
+      doPostAssoc(req, res, next, helper,
+        helper.belongsToManyAssoc.users, usrs);
     })
     .catch((err) => u.handleError(next, err, helper.modelName));
   }, // postPerspectiveWriters

diff --git a/api/v1/controllers/subjects.js b/api/v1/controllers/subjects.js
@@ -17,6 +17,8 @@ const doDeleteAllAssoc =
                     require('../helpers/verbs/doDeleteAllBToMAssoc');
 const doDeleteOneAssoc =
                     require('../helpers/verbs/doDeleteOneBToMAssoc');
+const doPostAssoc =
+                    require('../helpers/verbs/doPostBToMAssoc');
 const doDelete = require('../helpers/verbs/doDelete');
 const doFind = require('../helpers/verbs/doFind');
 const doGet = require('../helpers/verbs/doGet');
@@ -264,23 +266,11 @@ module.exports = {
     const params = req.swagger.params;
     const toPost = params.queryBody.value;
     const options = {};
-    let usersInsts;
     options.where = u.whereClauseForNameInArr(toPost);
     userProps.model.findAll(options)
     .then((usrs) => {
-      usersInsts = usrs;
-      return u.findByKey(helper, req.swagger.params);
-    })
-    .then((subj) => subj.addWriters(usersInsts))
-    .then((o) => {
-      /*
-       * The resolved object is either an array of arrays (when
-       * writers are added) or just an empty array when no writers are added.
-       * The popping is done to get the array from the array of arrays
-       */
-      let retval = o.length ? o.pop() : o;
-      retval = u.responsify(retval, helper, req.method);
-      res.status(httpStatus.CREATED).json(retval);
+      doPostAssoc(req, res, next, helper,
+        helper.belongsToManyAssoc.users, usrs);
     })
     .catch((err) => u.handleError(next, err, helper.modelName));
   }, // postSubjectWriters

diff --git a/api/v1/helpers/verbs/doDeleteAllBToMAssoc.js b/api/v1/helpers/verbs/doDeleteAllBToMAssoc.js
@@ -16,8 +16,7 @@ const httpStatus = require('../../constants').httpStatus;
 
 /**
  * Deletes all the associations of the resource and sends back no content
- * with a status of code 204. When a "value" is passed only the name or the id
- * of the association matching the resouce will be deleted.
+ * with a status of code 204.
  *
  * @param {IncomingMessage} req - The request object`
  * @param {ServerResponse} res - The response object

diff --git a/api/v1/helpers/verbs/doPostBToMAssoc.js b/api/v1/helpers/verbs/doPostBToMAssoc.js
@@ -0,0 +1,66 @@
+/**
+ * Copyright (c) 2016, salesforce.com, inc.
+ * All rights reserved.
+ * Licensed under the BSD 3-Clause license.
+ * For full license text, see LICENSE.txt file in the repo root or
+ * https://opensource.org/licenses/BSD-3-Clause
+ */
+
+/**
+ * api/v1/helpers/verbs/doPostBToMAssoc.js
+ */
+'use strict';
+
+const u = require('./utils');
+const httpStatus = require('../../constants').httpStatus;
+
+/**
+ * Creates an one to many association between a instance of the model name
+ * identified by "props.Name" and the instances of the model in assocArray. The
+ * association is defined through "assocName"
+ *
+ * @param {IncomingMessage} req - The request object`
+ * @param {ServerResponse} res - The response object
+ * @param {Function} next - The next middleware function in the stack
+ * @param {Module} props - The module containing the properties of the resource
+ * whose association has to be deleted
+ * @param {String} assocName - The name of the association associated with the
+ * model
+ * @param {Array} assocArray - The instances of a model for which the
+ * association has to be created.
+ *
+ */
+function doPostBToMAssoc(req, res, next, props, assocName, assocArray) { // eslint-disable-line
+  const params = req.swagger.params;
+  let modelInst;
+  u.findByKey(props, params)
+  .then((o) => {
+    modelInst = o;
+    return u.isWritable(req, o);
+  })
+  .then((ok) => {
+    if (ok) {
+      const addAssocfuncName = `add${u.capitalizeFirstLetter(assocName)}`;
+      return modelInst[addAssocfuncName](assocArray);
+    }
+
+    return u.forbidden(next);
+  })
+  .then((o) => {
+    if (o) {
+
+      /**
+       *
+       * The resolved object is either an array of arrays (when
+       * writers are added) or just an empty array when no writers are added.
+       * The popping is done to get the array from the array of arrays
+       */
+      let retval = o.length ? o.pop() : o;
+      retval = u.responsify(retval, props, req.method);
+      res.status(httpStatus.CREATED).json(retval);
+    }
+  })
+  .catch((err) => u.handleError(next, err, props.modelName));
+}
+
+module.exports = doPostBToMAssoc;
diff --git a/tests/api/v1/aspects/delete.js b/tests/api/v1/aspects/delete.js
@@ -45,7 +45,7 @@ describe(`api: DELETE ${path}`, () => {
   function notFound() {
     Aspect.findById(aspectId)
     .then((aspect) => {
-      expect(aspect).to.not.be.defined();
+      expect(aspect).to.equal(null);
     });
   }
 

diff --git a/tests/api/v1/aspects/postWriters.js b/tests/api/v1/aspects/postWriters.js
@@ -27,6 +27,7 @@ describe('api: aspects: post writers', () => {
   let firstUser;
   let secondUser;
   let thirdUser;
+  let otherValidToken;
   const userNameArray = [];
   const aspectToCreate = {
     name: `${tu.namePrefix}ASPECTNAME`,
@@ -52,7 +53,7 @@ describe('api: aspects: post writers', () => {
        * tu.createToken creates an user and an admin user is already created,
        * so one use of these.
        */
-      User.findOne())
+      User.findOne({ where: { name: tu.userName } }))
     .then((usr) => {
       firstUser = usr;
       userNameArray.push(firstUser.name);
@@ -65,7 +66,10 @@ describe('api: aspects: post writers', () => {
     })
     .then((tUsr) => {
       thirdUser = tUsr;
-      userNameArray.push(thirdUser.name);
+      return tu.createTokenFromUserName(tUsr.name);
+    })
+    .then((tkn) => {
+      otherValidToken = tkn;
     })
     .then(() => done())
     .catch(done);
@@ -80,20 +84,16 @@ describe('api: aspects: post writers', () => {
     .send(userNameArray)
     .expect(constants.httpStatus.CREATED)
     .expect((res) => {
-      expect(res.body).to.have.length(3);
+      expect(res.body).to.have.length(2);
 
       const userOne = res.body[0];
       const userTwo = res.body[1];
-      const userThree = res.body[2];
 
       expect(userOne.aspectId).to.not.equal(undefined);
       expect(userOne.userId).to.not.equal(undefined);
 
       expect(userTwo.aspectId).to.not.equal(undefined);
       expect(userTwo.userId).to.not.equal(undefined);
-
-      expect(userThree.aspectId).to.not.equal(undefined);
-      expect(userThree.userId).to.not.equal(undefined);
     })
     .end((err /* , res */) => {
       if (err) {
@@ -104,6 +104,21 @@ describe('api: aspects: post writers', () => {
     });
   });
 
+  it('return 403 for adding writers using an user that is not '+
+    'already a writer of that resource', (done) => {
+    api.post(postWritersPath.replace('{key}', aspect.id))
+    .set('Authorization', otherValidToken)
+    .send(userNameArray)
+    .expect(constants.httpStatus.FORBIDDEN)
+    .end((err /* , res */) => {
+      if (err) {
+        done(err);
+      }
+
+      done();
+    });
+  });
+
   it('a request body that is not an array should not be accepted', (done) => {
     const firstUserName = firstUser.name;
     api.post(postWritersPath.replace('{key}', aspect.id))

diff --git a/tests/api/v1/lenses/postWriters.js b/tests/api/v1/lenses/postWriters.js
@@ -23,6 +23,7 @@ const postWritersPath = '/v1/lenses/{key}/writers';
 describe('api: lenses: post writers', () => {
   let lens;
   let token;
+  let otherValidToken;
   const userNameArray = [];
 
   before((done) => {
@@ -44,9 +45,15 @@ describe('api: lenses: post writers', () => {
        * tu.createToken creates an user and an admin user is already created,
        * so one use of these.
        */
-      User.findOne())
+      User.findOne({ where: { name: tu.userName } }))
     .then((usr) => {
       userNameArray.push(usr.name);
+
+      /*
+       * user is added to make sure users are not given write
+       * permission twice
+       */
+      return lens.addWriter(usr);
     })
     .then(() => tu.createSecondUser())
     .then((secUsr) => {
@@ -55,12 +62,11 @@ describe('api: lenses: post writers', () => {
     })
     .then((tUsr) => {
       userNameArray.push(tUsr.name);
-
-      /*
-       * third user is added to make sure users are not given write
-       * permission twice
-       */
-      return lens.addWriter(tUsr);
+      return tu.createUser('myUNiqueUser');
+    })
+    .then((fusr) => tu.createTokenFromUserName(fusr.name))
+    .then((tkn) => {
+      otherValidToken = tkn;
     })
     .then(() => done())
     .catch(done);
@@ -96,6 +102,21 @@ describe('api: lenses: post writers', () => {
     });
   });
 
+  it('return 403 for adding writers using an user that is not ' +
+    'already a writer of that resource', (done) => {
+    api.post(postWritersPath.replace('{key}', lens.id))
+    .set('Authorization', otherValidToken)
+    .send(userNameArray)
+    .expect(constants.httpStatus.FORBIDDEN)
+    .end((err /* , res */) => {
+      if (err) {
+        done(err);
+      }
+
+      done();
+    });
+  });
+
   it('a request body that is not an array should not be accepted', (done) => {
     const userName = 'userName';
     api.post(postWritersPath.replace('{key}', lens.id))

diff --git a/tests/api/v1/perspectives/deleteWriters.js b/tests/api/v1/perspectives/deleteWriters.js
@@ -71,7 +71,6 @@ describe('api: perspectives: delete writer(s)', () => {
     .catch((err) => done(err));
   });
 
-
   afterEach(u.forceDelete);
   afterEach(tu.forceDeleteUser);