Merge pull request #747 from ParsePlatform/nlutsenko.schema.middleware

Add promise-based master-key enforce middleware and move SchemasRouter, FeaturesRouter to use it.

Author: nlutsenko

spec/features.spec.js

@@ -1,4 +1,7 @@
-var features = require('../src/features')
+'use strict';
+
+var features = require('../src/features');
+const request = require("request");
 
 describe('features', () => {
   it('set and get features', (done) => {
@@ -23,4 +26,19 @@ describe('features', () => {
     expect(_features.test).toBeUndefined();
     done();
   });
+
+  it('requires the master key to get all schemas', done => {
+    request.get({
+      url: 'http://localhost:8378/1/features',
+      json: true,
+      headers: {
+        'X-Parse-Application-Id': 'test',
+        'X-Parse-REST-API-Key': 'rest'
+      }
+    }, (error, response, body) => {
+      expect(response.statusCode).toEqual(403);
+      expect(body.error).toEqual('unauthorized: master key is required');
+      done();
+    });
+  });
 });

spec/schemas.spec.js

@@ -98,8 +98,8 @@ describe('schemas', () => {
       json: true,
       headers: restKeyHeaders,
     }, (error, response, body) => {
-      expect(response.statusCode).toEqual(401);
-      expect(body.error).toEqual('master key not specified');
+      expect(response.statusCode).toEqual(403);
+      expect(body.error).toEqual('unauthorized: master key is required');
       done();
     });
   });
@@ -110,8 +110,8 @@ describe('schemas', () => {
       json: true,
       headers: restKeyHeaders,
     }, (error, response, body) => {
-      expect(response.statusCode).toEqual(401);
-      expect(body.error).toEqual('master key not specified');
+      expect(response.statusCode).toEqual(403);
+      expect(body.error).toEqual('unauthorized: master key is required');
       done();
     });
   });
@@ -206,8 +206,8 @@ describe('schemas', () => {
         className: 'MyClass',
       },
     }, (error, response, body) => {
-      expect(response.statusCode).toEqual(401);
-      expect(body.error).toEqual('master key not specified');
+      expect(response.statusCode).toEqual(403);
+      expect(body.error).toEqual('unauthorized: master key is required');
       done();
     });
   });

src/Routers/FeaturesRouter.js

@@ -1,32 +1,13 @@
 import PromiseRouter from '../PromiseRouter';
-import {getFeatures} from '../features';
-
-let masterKeyRequiredResponse = () => {
-  return Promise.resolve({
-    status: 401,
-    response: {error: 'master key not specified'},
-  })
-}
+import * as middleware from "../middlewares";
+import { getFeatures } from '../features';
 
 export class FeaturesRouter extends PromiseRouter {
-  
   mountRoutes() {
-    this.route('GET','/features', (req) => {
-      return this.handleGET(req);
-    });
-  }
-
-  handleGET(req) {
-    if (!req.auth.isMaster) {
-      return masterKeyRequiredResponse();
-    }
-    
-    return Promise.resolve({
-      response: {
+    this.route('GET','/features', middleware.promiseEnforceMasterKeyAccess, () => {
+      return { response: {
         results: [getFeatures()]
-      }
+      } };
     });
   }
 }
-
-export default FeaturesRouter;

src/Routers/SchemasRouter.js

@@ -5,14 +5,7 @@ var express = require('express'),
   Schema = require('../Schema');
 
 import PromiseRouter from '../PromiseRouter';
-
-// TODO: refactor in a SchemaController at one point...
-function masterKeyRequiredResponse() {
-  return Promise.resolve({
-    status: 401,
-    response: {error: 'master key not specified'},
-  })
-}
+import * as middleware from "../middlewares";
 
 function classNameMismatchResponse(bodyClass, pathClass) {
   return Promise.resolve({
@@ -45,9 +38,6 @@ function mongoSchemaToSchemaAPIResponse(schema) {
 }
 
 function getAllSchemas(req) {
-  if (!req.auth.isMaster) {
-    return masterKeyRequiredResponse();
-  }
   return req.config.database.collection('_SCHEMA')
     .then(coll => coll.find({}).toArray())
     .then(schemas => ({response: {
@@ -56,9 +46,6 @@ function getAllSchemas(req) {
 }
 
 function getOneSchema(req) {
-  if (!req.auth.isMaster) {
-    return masterKeyRequiredResponse();
-  }
   return req.config.database.collection('_SCHEMA')
     .then(coll => coll.findOne({'_id': req.params.className}))
     .then(schema => ({response: mongoSchemaToSchemaAPIResponse(schema)}))
@@ -72,9 +59,6 @@ function getOneSchema(req) {
 }
 
 function createSchema(req) {
-  if (!req.auth.isMaster) {
-    return masterKeyRequiredResponse();
-  }
   if (req.params.className && req.body.className) {
     if (req.params.className != req.body.className) {
       return classNameMismatchResponse(req.body.className, req.params.className);
@@ -100,10 +84,6 @@ function createSchema(req) {
 }
 
 function modifySchema(req) {
-  if (!req.auth.isMaster) {
-    return masterKeyRequiredResponse();
-  }
-
   if (req.body.className && req.body.className != req.params.className) {
     return classNameMismatchResponse(req.body.className, req.params.className);
   }
@@ -168,10 +148,6 @@ var removeJoinTables = (database, mongoSchema) => {
 };
 
 function deleteSchema(req) {
-  if (!req.auth.isMaster) {
-    return masterKeyRequiredResponse();
-  }
-
   if (!Schema.classNameIsValid(req.params.className)) {
     throw new Parse.Error(Parse.Error.INVALID_CLASS_NAME, Schema.invalidClassNameMessage(req.params.className));
   }
@@ -214,11 +190,11 @@ function deleteSchema(req) {
 
 export class SchemasRouter extends PromiseRouter {
   mountRoutes() {
-    this.route('GET', '/schemas', getAllSchemas);
-    this.route('GET', '/schemas/:className', getOneSchema);
-    this.route('POST', '/schemas', createSchema);
-    this.route('POST', '/schemas/:className', createSchema);
-    this.route('PUT', '/schemas/:className', modifySchema);
-    this.route('DELETE', '/schemas/:className', deleteSchema);
+    this.route('GET', '/schemas', middleware.promiseEnforceMasterKeyAccess, getAllSchemas);
+    this.route('GET', '/schemas/:className', middleware.promiseEnforceMasterKeyAccess, getOneSchema);
+    this.route('POST', '/schemas', middleware.promiseEnforceMasterKeyAccess, createSchema);
+    this.route('POST', '/schemas/:className', middleware.promiseEnforceMasterKeyAccess, createSchema);
+    this.route('PUT', '/schemas/:className', middleware.promiseEnforceMasterKeyAccess, modifySchema);
+    this.route('DELETE', '/schemas/:className', middleware.promiseEnforceMasterKeyAccess, deleteSchema);
   }
 }

src/middlewares.js

@@ -194,6 +194,16 @@ function enforceMasterKeyAccess(req, res, next) {
   next();
 }
 
+function promiseEnforceMasterKeyAccess(request) {
+  if (!request.auth.isMaster) {
+    let error = new Error();
+    error.status = 403;
+    error.message = "unauthorized: master key is required";
+    throw error;
+  }
+  return Promise.resolve();
+}
+
 function invalidRequest(req, res) {
   res.status(403);
   res.end('{"error":"unauthorized"}');
@@ -204,5 +214,6 @@ module.exports = {
   allowMethodOverride: allowMethodOverride,
   handleParseErrors: handleParseErrors,
   handleParseHeaders: handleParseHeaders,
-  enforceMasterKeyAccess: enforceMasterKeyAccess
+  enforceMasterKeyAccess: enforceMasterKeyAccess,
+  promiseEnforceMasterKeyAccess
 };