From 8ce0bd84fb7723e0e55b442800f472470332cb04 Mon Sep 17 00:00:00 2001 From: Nikita Lutsenko Date: Tue, 1 Mar 2016 15:16:51 -0800 Subject: [PATCH 1/3] Add promise-based master-key only middleware. --- src/middlewares.js | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/middlewares.js b/src/middlewares.js index 8489cda0..56ebdc1d 100644 --- a/src/middlewares.js +++ b/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 }; From e58c935f22d6413e88fa39303d745442b26f5315 Mon Sep 17 00:00:00 2001 From: Nikita Lutsenko Date: Tue, 1 Mar 2016 15:17:10 -0800 Subject: [PATCH 2/3] Use middleware instead of custom checks inside SchemasRouter. --- spec/schemas.spec.js | 12 ++++++------ src/Routers/SchemasRouter.js | 38 +++++++----------------------------- 2 files changed, 13 insertions(+), 37 deletions(-) diff --git a/spec/schemas.spec.js b/spec/schemas.spec.js index 145b2134..24589a38 100644 --- a/spec/schemas.spec.js +++ b/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(); }); }); diff --git a/src/Routers/SchemasRouter.js b/src/Routers/SchemasRouter.js index 352b1caf..1ed606b1 100644 --- a/src/Routers/SchemasRouter.js +++ b/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); } } From fb4a2524b117ef993d50f372276a230494f91b66 Mon Sep 17 00:00:00 2001 From: Nikita Lutsenko Date: Tue, 1 Mar 2016 16:03:37 -0800 Subject: [PATCH 3/3] Cleanup and use masterkey middleware in FeaturesRouter. --- spec/features.spec.js | 20 +++++++++++++++++++- src/Routers/FeaturesRouter.js | 29 +++++------------------------ 2 files changed, 24 insertions(+), 25 deletions(-) diff --git a/spec/features.spec.js b/spec/features.spec.js index b0a27459..3ddd7a60 100644 --- a/spec/features.spec.js +++ b/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(); + }); + }); }); diff --git a/src/Routers/FeaturesRouter.js b/src/Routers/FeaturesRouter.js index 65ca1b71..05ccad5b 100644 --- a/src/Routers/FeaturesRouter.js +++ b/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;