Merge pull request #753 from ParsePlatform/nlutsenko.middleware.routers

Use shared middleware to enforce masterkey security on logs/global config/hooks APIs.

Author: nlutsenko

spec/LogsRouter.spec.js

@@ -1,3 +1,6 @@
+'use strict';
+
+const request = require('request');
 var LogsRouter = require('../src/Routers/LogsRouter').LogsRouter;
 var LoggerController = require('../src/Controllers/LoggerController').LoggerController;
 var FileLoggerAdapter = require('../src/Adapters/Logger/FileLoggerAdapter').FileLoggerAdapter;
@@ -45,23 +48,18 @@ describe('LogsRouter', () => {
     done();
   });
 
-  it('can check invalid master key of request', (done) => {
-    // Make mock request
-    var request = {
-      auth: {
-        isMaster: false
-      },
-      query: {},
-      config: {
-        loggerController: loggerController
+  it('can check invalid master key of request', done => {
+    request.get({
+      url: 'http://localhost:8378/1/logs',
+      json: true,
+      headers: {
+        'X-Parse-Application-Id': 'test',
+        'X-Parse-REST-API-Key': 'rest'
       }
-    };
-
-   var router = new LogsRouter();
-
-    expect(() => {
-      router.handleGET(request);
-    }).toThrow();
-    done();
+    }, (error, response, body) => {
+      expect(response.statusCode).toEqual(403);
+      expect(body.error).toEqual('unauthorized: master key is required');
+      done();
+    });
   });
 });

spec/ParseGlobalConfig.spec.js

@@ -53,8 +53,8 @@ describe('a GlobalConfig', () => {
         'X-Parse-REST-API-Key': 'rest'
       },
     }, (error, response, body) => {
-      expect(response.statusCode).toEqual(401);
-      expect(body.error).toEqual('unauthorized');
+      expect(response.statusCode).toEqual(403);
+      expect(body.error).toEqual('unauthorized: master key is required');
       done();
     });
   });  

src/Routers/GlobalConfigRouter.js

@@ -3,6 +3,7 @@
 var Parse = require('parse/node').Parse;
 
 import PromiseRouter from '../PromiseRouter';
+import * as middleware from "../middlewares";
 
 export class GlobalConfigRouter extends PromiseRouter {
   getGlobalConfig(req) {
@@ -18,13 +19,6 @@ export class GlobalConfigRouter extends PromiseRouter {
       }));
   }
   updateGlobalConfig(req) {
-    if (!req.auth.isMaster) {
-      return Promise.resolve({
-        status: 401,
-        response: {error: 'unauthorized'},
-      });
-    }
-
     return req.config.database.rawCollection('_GlobalConfig')
       .then(coll => coll.findOneAndUpdate({ _id: 1 }, { $set: req.body }))
       .then(response => {
@@ -41,7 +35,7 @@ export class GlobalConfigRouter extends PromiseRouter {
 
   mountRoutes() {
     this.route('GET', '/config', req => { return this.getGlobalConfig(req) });
-    this.route('PUT', '/config', req => { return this.updateGlobalConfig(req) });
+    this.route('PUT', '/config', middleware.promiseEnforceMasterKeyAccess, req => { return this.updateGlobalConfig(req) });
   }
 }
 

src/Routers/HooksRouter.js

@@ -1,15 +1,9 @@
 import { Parse } from 'parse/node';
 import PromiseRouter from '../PromiseRouter';
 import { HooksController } from '../Controllers/HooksController';
- 
-function enforceMasterKeyAccess(req) {
-  if (!req.auth.isMaster) {
-    throw new Parse.Error(403, "unauthorized: master key is required");
-  }
-} 
+import * as middleware from "../middlewares";
 
 export class HooksRouter extends PromiseRouter {
-  
   createHook(aHook, config) {
     return config.hooksController.createHook(aHook).then( (hook) => ({response: hook}));
   };
@@ -93,14 +87,14 @@ export class HooksRouter extends PromiseRouter {
   }
 
   mountRoutes() {
-    this.route('GET',  '/hooks/functions', enforceMasterKeyAccess, this.handleGetFunctions.bind(this));
-    this.route('GET',  '/hooks/triggers', enforceMasterKeyAccess, this.handleGetTriggers.bind(this));
-    this.route('GET',  '/hooks/functions/:functionName', enforceMasterKeyAccess, this.handleGetFunctions.bind(this));
-    this.route('GET',  '/hooks/triggers/:className/:triggerName', enforceMasterKeyAccess, this.handleGetTriggers.bind(this));
-    this.route('POST', '/hooks/functions', enforceMasterKeyAccess, this.handlePost.bind(this));
-    this.route('POST', '/hooks/triggers', enforceMasterKeyAccess, this.handlePost.bind(this));
-    this.route('PUT',  '/hooks/functions/:functionName', enforceMasterKeyAccess, this.handlePut.bind(this));
-    this.route('PUT',  '/hooks/triggers/:className/:triggerName', enforceMasterKeyAccess, this.handlePut.bind(this));
+    this.route('GET',  '/hooks/functions', middleware.promiseEnforceMasterKeyAccess, this.handleGetFunctions.bind(this));
+    this.route('GET',  '/hooks/triggers', middleware.promiseEnforceMasterKeyAccess, this.handleGetTriggers.bind(this));
+    this.route('GET',  '/hooks/functions/:functionName', middleware.promiseEnforceMasterKeyAccess, this.handleGetFunctions.bind(this));
+    this.route('GET',  '/hooks/triggers/:className/:triggerName', middleware.promiseEnforceMasterKeyAccess, this.handleGetTriggers.bind(this));
+    this.route('POST', '/hooks/functions', middleware.promiseEnforceMasterKeyAccess, this.handlePost.bind(this));
+    this.route('POST', '/hooks/triggers', middleware.promiseEnforceMasterKeyAccess, this.handlePost.bind(this));
+    this.route('PUT',  '/hooks/functions/:functionName', middleware.promiseEnforceMasterKeyAccess, this.handlePut.bind(this));
+    this.route('PUT',  '/hooks/triggers/:className/:triggerName', middleware.promiseEnforceMasterKeyAccess, this.handlePut.bind(this));
   }
 }
 

src/Routers/LogsRouter.js

@@ -1,23 +1,11 @@
 import { Parse } from 'parse/node';
 import PromiseRouter from '../PromiseRouter';
-
-// only allow request with master key
-let enforceSecurity = (auth) => {
-  if (!auth || !auth.isMaster) {
-    throw new Parse.Error(
-      Parse.Error.OPERATION_FORBIDDEN,
-      'Clients aren\'t allowed to perform the ' +
-      'get' + ' operation on logs.'
-    );
-  }
-}
+import * as middleware from "../middlewares";
 
 export class LogsRouter extends PromiseRouter {
 
   mountRoutes() {
-    this.route('GET','/logs', (req) => {
-      return this.handleGET(req);
-    });
+    this.route('GET','/logs', middleware.promiseEnforceMasterKeyAccess, req => { return this.handleGET(req); });
   }
 
   // Returns a promise for a {response} object.
@@ -29,31 +17,26 @@ export class LogsRouter extends PromiseRouter {
   // size (optional) Number of rows returned by search. Defaults to 10
   handleGET(req) {
     if (!req.config || !req.config.loggerController) {
-      throw new Parse.Error(Parse.Error.PUSH_MISCONFIGURED,
-        'Logger adapter is not availabe');
+      throw new Parse.Error(Parse.Error.PUSH_MISCONFIGURED, 'Logger adapter is not available.');
     }
 
-    let promise = new Parse.Promise();
     let from = req.query.from;
     let until = req.query.until;
     let size = req.query.size;
     let order = req.query.order
     let level = req.query.level;
-    enforceSecurity(req.auth);
 
     const options = {
       from,
       until,
       size,
       order,
-      level,
-    }
+      level
+    };
 
-    return req.config.loggerController.getLogs(options).then((result) => {
-      return Promise.resolve({
-        response: result
-      });
-    })
+    return req.config.loggerController
+      .getLogs(options)
+      .then(result => ({ response: result }));
   }
 }