fix(object): deriving object silently drops unauthorized objects

speckleworks · Jan 17, 2020 · abdcc7d · abdcc7d
1 parent e93d6ce
commit abdcc7d
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 17 deletions.
diff --git a/app/api/objects/ObjectDerive.js b/app/api/objects/ObjectDerive.js
@@ -3,6 +3,7 @@ const { merge } = require( 'lodash' )
 
 const SpeckleObject = require( '../../../models/SpeckleObject' )
 const BulkObjectSave = require( '../middleware/BulkObjectSave' )
+const PermissionCheck = require( '../middleware/PermissionCheck' )
 
 // Derives an object from an existing object
 module.exports = ( req, res ) => {
@@ -14,21 +15,25 @@ module.exports = ( req, res ) => {
   let objects = req.body
 
   SpeckleObject.find( { _id: { $in: objects.map( obj => obj._id ) } } ).lean()
+    .then( objects => Promise.all( objects.map( o => PermissionCheck( req.user, 'read', o ) ).map( prom => prom.catch( e => e ) ) ) )
     .then( existingObjects => {
       let toSave = [ ]
+
       for ( let original of existingObjects ) {
-        let found = objects.find( o => o._id === original._id.toString() )
-        let mod = {}
+        if ( original._id ) {
+          let found = objects.find( o => o._id === original._id.toString() )
+          let mod = {}
 
-        merge( mod, original, found )
+          merge( mod, original, found )
 
-        // delete hash to prepare for rehashing in bulk save
-        delete mod.hash
-        delete mod._id
-        delete mod.createdAt
-        toSave.push( mod )
+          // delete hash to prepare for rehashing in bulk save
+          delete mod.hash
+          delete mod._id
+          delete mod.createdAt
+          toSave.push( mod )
+        }
       }
-      return BulkObjectSave( toSave, req.user )
+      return BulkObjectSave( toSave, req.user );
     } )
     .then( newObjects => {
       res.send( { success: true, message: 'Saved objects to database.', resources: newObjects.map( o => { return { type: 'Placeholder', _id: o._id } } ) } )

diff --git a/test/api/object.test.js b/test/api/object.test.js
@@ -458,16 +458,14 @@ describe( 'objects', () => {
           .set( 'Authorization', unauthorizedUser.apiToken )
           .send( derivePayload )
           .end( ( err, res ) => {
-            res.body.resources.should.have.lengthOf( '3' )
-
-            // res.body.resources[0].type.should.be.equal( 'String' )
-            // res.body.resources[0].value.should.be.equal( 'You do not have permissions to view this object' )
-            // res.body.resources[1].type.should.be.equal( 'Placeholder' )
-            // res.body.resources[2].type.should.be.equal( 'String' )
-            // res.body.resources[2].value.should.be.equal( 'You do not have permissions to view this object' )
+            res.body.resources.should.have.lengthOf( '2' )
 
             SpeckleObject.find( { owner: unauthorizedUser._id } ).then(
-              objects => objects.length.should.equal( 1 )
+              objects => {
+                objects.length.should.equal( 2 );
+                objects[0].name = object1.name;
+                objects[1].name = object2.name;
+              }
             ).catch( err => done( err ) )
 
             done()