From ef752ac395324230dee146df9b501108fd19ce95 Mon Sep 17 00:00:00 2001 From: Tobias Sorn Date: Wed, 20 May 2020 14:37:39 +0200 Subject: [PATCH 01/11] [FEATURE] csp: enable writing of reports file Reports file is written with the new config option "cspReportPath" set. json file written contains all csp-reports in an array. --- lib/middleware/csp.js | 67 +++++++++++++++++- test/lib/server/middleware/csp.js | 112 +++++++++++++++++++++++++++++- 2 files changed, 177 insertions(+), 2 deletions(-) diff --git a/lib/middleware/csp.js b/lib/middleware/csp.js index 541a3094..52d60b99 100644 --- a/lib/middleware/csp.js +++ b/lib/middleware/csp.js @@ -1,5 +1,10 @@ +const logger = require("@ui5/logger"); +const log = logger.getLogger("server:middleware:csp"); const parseurl = require("parseurl"); const querystring = require("querystring"); +const {promisify} = require("util"); +const fs = require("graceful-fs"); +const writeFile = promisify(fs.writeFile); const HEADER_CONTENT_SECURITY_POLICY = "Content-Security-Policy"; const HEADER_CONTENT_SECURITY_POLICY_REPORT_ONLY = "Content-Security-Policy-Report-Only"; @@ -16,7 +21,43 @@ function addHeader(res, header, value) { } } -function createMiddleware(sCspUrlParameterName, oConfig) { + +/** + * Writes the csp report entries to the given file path + * + * @param {string} filePath + * @param {object[]} cspReportEntries + * @returns {Promise} which resolves when the file was written + */ +const writeCspReportsFiles = async (filePath, cspReportEntries) => { + const objectToWrite = { + "csp-reports": cspReportEntries + }; + return writeFile(filePath, JSON.stringify(objectToWrite)); +}; + +/** + * @typedef {object} CspConfig + * @property {boolean} allowDynamicPolicySelection + * @property {boolean} allowDynamicPolicyDefinition + * @property {string} defaultPolicy + * @property {boolean} defaultPolicyIsReportOnly + * @property {string} defaultPolicy2 + * @property {boolean} defaultPolicy2IsReportOnly + * @property {object} definedPolicies + */ + +/** + * @module @ui5/server/middleware/csp + * Middleware which enables CSP (content security policy) support + * @see https://www.w3.org/TR/CSP/ + * + * @param {string} sCspUrlParameterName + * @param {CspConfig} oConfig + * @param {string} cspReportPath path where the reports should be written to + * @returns {Function} Returns a server middleware closure. + */ +function createMiddleware(sCspUrlParameterName, oConfig, cspReportPath) { const { allowDynamicPolicySelection = false, allowDynamicPolicyDefinition = false, @@ -27,6 +68,19 @@ function createMiddleware(sCspUrlParameterName, oConfig) { definedPolicies = {} } = oConfig; + + /** + * List of CSP Report entries + */ + const cspReportEntries = []; + + // initially write the file + if (cspReportPath) { + writeCspReportsFiles(cspReportPath, cspReportEntries).then(() => { + log.verbose(`Wrote csp reports initially to ${cspReportPath}`); + }); + } + return function csp(req, res, next) { const oParsedURL = parseurl(req); @@ -35,6 +89,17 @@ function createMiddleware(sCspUrlParameterName, oConfig) { oParsedURL.pathname.endsWith("/dummy.csplog") ) { // In report-only mode there must be a report-uri defined // For now just ignore the violation. It will be logged in the browser anyway. + if (cspReportPath && req.body && req.body["csp-report"]) { + // extract the csp-report + // add the resource to the cspReportEntries list + cspReportEntries.push(req.body["csp-report"]); + // write the csp report file + writeCspReportsFiles(cspReportPath, cspReportEntries).then(() => { + log.verbose(`Wrote csp reports, length: ${cspReportEntries.length}`); + }); + } + + return; } next(); diff --git a/test/lib/server/middleware/csp.js b/test/lib/server/middleware/csp.js index 9c1786ef..f8ff31b3 100644 --- a/test/lib/server/middleware/csp.js +++ b/test/lib/server/middleware/csp.js @@ -1,5 +1,49 @@ const test = require("ava"); -const cspMiddleware = require("../../../../lib/middleware/csp"); +let cspMiddleware = require("../../../../lib/middleware/csp"); +const mock = require("mock-require"); + +const sinon = require("sinon"); +const fs = require("graceful-fs"); + + +test.before((t) => { + // csp-reports file is written async and afterwards a verbose log is done. + // wait until the file was written initially and until the first entry was made + // using t.context.logVerboseStubCalled promise being called + let wasResolved = () => {}; + t.context.logVerboseStubCalled = new Promise((resolve) => { + wasResolved = resolve; + }); + + + t.context.logVerboseStub = sinon.stub(); + + let callCount = 0; + mock("@ui5/logger", { + getLogger: () => { + return { + verbose: (message) => { + t.context.logVerboseStub(message); + // 2 calls: + // * initially creating the file + // * adding a csp-violation + if (++callCount === 2) { + wasResolved(); + } + } + }; + } + }); + mock.reRequire("@ui5/logger"); + t.context.writeFileStub = sinon.stub(fs, "writeFile").callsArg(2); + + // Re-require to ensure that mocked modules are used + cspMiddleware = mock.reRequire("../../../../lib/middleware/csp"); +}); + +test.after.always(() => { + sinon.restore(); +}); test("Default Settings", (t) => { t.plan(3 + 7); // fourth request should end in middleware and not call next! @@ -38,6 +82,72 @@ test("Default Settings", (t) => { ); }); +test("Default Settings CSP violation", async (t) => { + t.plan(3 + 15); // fourth request should end in middleware and not call next! + const middleware = cspMiddleware("sap-ui-xx-csp-policy", {}, "my-report.json"); + const res = { + getHeader: function() { + return undefined; + }, + setHeader: function(header, value) { + t.fail(`should not be called with header ${header} and value ${value}`); + } + }; + const next = function() { + t.pass("Next was called."); + }; + const noNext = function() { + t.fail("Next should not be called"); + }; + + const cspReport = { + "document-uri": "https://otherserver:8080/index.html", + "referrer": "", + "violated-directive": "script-src-elem", + "effective-directive": "script-src-elem", + "original-policy": "default-src 'self' myserver:443; report-uri /report-csp-violation", + "disposition": "report", + "blocked-uri": "inline", + "line-number": 17, + "source-file": "https://otherserver:8080/index.html", + "status-code": 0, + "script-sample": "" + }; + + middleware({method: "GET", url: "/test.html", headers: {}}, res, next); + middleware({ + method: "GET", + url: "/test.html?sap-ui-xx-csp-policy=sap-target-level-2", + headers: {} + }, res, next); + middleware({method: "POST", url: "somePath", headers: {}}, res, next); + middleware({ + method: "POST", + url: "/dummy.csplog", + headers: {"content-type": "application/csp-report"}, + body: { + "csp-report": cspReport + } + }, res, noNext); + + // check that unsupported methods result in a call to next() + ["CONNECT", "DELETE", "HEAD", "OPTIONS", "PATCH", "PUT", "TRACE"].forEach( + (method) => middleware({method, url: "/dummy.csplog", headers: {}}, res, next) + ); + + await t.context.logVerboseStubCalled; + + + t.is(t.context.logVerboseStub.callCount, 2, "should be called 2 times"); + t.deepEqual(t.context.logVerboseStub.getCall(0).args, ["Wrote csp reports initially to my-report.json"], "first log verbose"); + t.deepEqual(t.context.logVerboseStub.getCall(1).args, ["Wrote csp reports, length: 1"], "second log verbose"); + t.is(t.context.writeFileStub.callCount, 2, "should be called 2 times"); + t.is(t.context.writeFileStub.getCall(0).args[0], "my-report.json", "filename"); + t.is(t.context.writeFileStub.getCall(0).args[1], "{\"csp-reports\":[]}", "initial content"); + t.is(t.context.writeFileStub.getCall(1).args[0], "my-report.json", "filename"); + t.is(t.context.writeFileStub.getCall(1).args[1], "{\"csp-reports\":[" + JSON.stringify(cspReport) + "]}", "content with reports"); +}); + test("Custom Settings", (t) => { const middleware = cspMiddleware("csp", { definedPolicies: { From 44694f69d5b9d1c144b191563025f658d7be8fce Mon Sep 17 00:00:00 2001 From: Tobias Sorn Date: Wed, 20 May 2020 17:23:07 +0200 Subject: [PATCH 02/11] [FEATURE] csp: reduce test to relevant parts adjust documentation --- lib/middleware/csp.js | 12 ++++-------- test/lib/server/middleware/csp.js | 30 ++---------------------------- 2 files changed, 6 insertions(+), 36 deletions(-) diff --git a/lib/middleware/csp.js b/lib/middleware/csp.js index 52d60b99..7dd1a31b 100644 --- a/lib/middleware/csp.js +++ b/lib/middleware/csp.js @@ -25,8 +25,8 @@ function addHeader(res, header, value) { /** * Writes the csp report entries to the given file path * - * @param {string} filePath - * @param {object[]} cspReportEntries + * @param {string} filePath target file, e.g. csp-reports.json + * @param {object[]} cspReportEntries array of csp-report json objects * @returns {Promise} which resolves when the file was written */ const writeCspReportsFiles = async (filePath, cspReportEntries) => { @@ -87,19 +87,15 @@ function createMiddleware(sCspUrlParameterName, oConfig, cspReportPath) { if (req.method === "POST" ) { if (req.headers["content-type"] === "application/csp-report" && oParsedURL.pathname.endsWith("/dummy.csplog") ) { - // In report-only mode there must be a report-uri defined - // For now just ignore the violation. It will be logged in the browser anyway. + // Write the violation into a csp-reports json file if cspReportPath parameter is set if (cspReportPath && req.body && req.body["csp-report"]) { - // extract the csp-report - // add the resource to the cspReportEntries list + // extract the csp-report and add it to the cspReportEntries list cspReportEntries.push(req.body["csp-report"]); // write the csp report file writeCspReportsFiles(cspReportPath, cspReportEntries).then(() => { log.verbose(`Wrote csp reports, length: ${cspReportEntries.length}`); }); } - - return; } next(); diff --git a/test/lib/server/middleware/csp.js b/test/lib/server/middleware/csp.js index f8ff31b3..fe5e8300 100644 --- a/test/lib/server/middleware/csp.js +++ b/test/lib/server/middleware/csp.js @@ -83,22 +83,8 @@ test("Default Settings", (t) => { }); test("Default Settings CSP violation", async (t) => { - t.plan(3 + 15); // fourth request should end in middleware and not call next! + t.plan(8); const middleware = cspMiddleware("sap-ui-xx-csp-policy", {}, "my-report.json"); - const res = { - getHeader: function() { - return undefined; - }, - setHeader: function(header, value) { - t.fail(`should not be called with header ${header} and value ${value}`); - } - }; - const next = function() { - t.pass("Next was called."); - }; - const noNext = function() { - t.fail("Next should not be called"); - }; const cspReport = { "document-uri": "https://otherserver:8080/index.html", @@ -114,13 +100,6 @@ test("Default Settings CSP violation", async (t) => { "script-sample": "" }; - middleware({method: "GET", url: "/test.html", headers: {}}, res, next); - middleware({ - method: "GET", - url: "/test.html?sap-ui-xx-csp-policy=sap-target-level-2", - headers: {} - }, res, next); - middleware({method: "POST", url: "somePath", headers: {}}, res, next); middleware({ method: "POST", url: "/dummy.csplog", @@ -128,12 +107,7 @@ test("Default Settings CSP violation", async (t) => { body: { "csp-report": cspReport } - }, res, noNext); - - // check that unsupported methods result in a call to next() - ["CONNECT", "DELETE", "HEAD", "OPTIONS", "PATCH", "PUT", "TRACE"].forEach( - (method) => middleware({method, url: "/dummy.csplog", headers: {}}, res, next) - ); + }, {}, undefined); await t.context.logVerboseStubCalled; From 264432cdff54b9ef95b4d4c636585e625a2f3cc9 Mon Sep 17 00:00:00 2001 From: Tobias Sorn Date: Wed, 20 May 2020 17:33:32 +0200 Subject: [PATCH 03/11] [FEATURE] csp: Add cspReportPath parameter such that it can be set from the outside when serving --- lib/middleware/MiddlewareManager.js | 5 +++++ lib/server.js | 4 +++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/middleware/MiddlewareManager.js b/lib/middleware/MiddlewareManager.js index 537d70fd..5586c1d5 100644 --- a/lib/middleware/MiddlewareManager.js +++ b/lib/middleware/MiddlewareManager.js @@ -118,6 +118,11 @@ class MiddlewareManager { defaultPolicy2IsReportOnly: true, }); } + if (this.options.cspReportPath) { + Object.assign(oCspConfig, { + cspReportPath: this.options.cspReportPath + }); + } return () => { return cspModule("sap-ui-xx-csp-policy", oCspConfig); }; diff --git a/lib/server.js b/lib/server.js index 8cf1b0f4..1d2cd61a 100644 --- a/lib/server.js +++ b/lib/server.js @@ -111,6 +111,7 @@ module.exports = { * aim for (AKA 'target policies'), are send for any requested * *.html file * @param {boolean} [options.simpleIndex=false] Use a simplified view for the server directory listing + * @param {string} [options.cspReportPath] Enable csp report logging to the given path * @returns {Promise} Promise resolving once the server is listening. * It resolves with an object containing the port, * h2-flag and a close function, @@ -118,7 +119,7 @@ module.exports = { */ async serve(tree, { port: requestedPort, changePortIfInUse = false, h2 = false, key, cert, - acceptRemoteConnections = false, sendSAPTargetCSP = false, simpleIndex = false + acceptRemoteConnections = false, sendSAPTargetCSP = false, simpleIndex = false, cspReportPath }) { const projectResourceCollections = resourceFactory.createCollectionsForTree(tree); @@ -140,6 +141,7 @@ module.exports = { resources, options: { sendSAPTargetCSP, + cspReportPath, simpleIndex } }); From 039e1579be199f4655ce06a10aa0c0c058f93449 Mon Sep 17 00:00:00 2001 From: Tobias Sorn Date: Thu, 28 May 2020 15:36:06 +0200 Subject: [PATCH 04/11] [FEATURE] csp: only write file on exit Use exit hook and server close function to write csp reports file. This avoids writing to a file with every request. The csp reports are stored in an array and only written once the server closes or gets killed (SIGINT) --- lib/middleware/MiddlewareManager.js | 13 ++++++-- lib/middleware/MiddlewareUtil.js | 22 +++++++++++++ lib/middleware/csp.js | 29 +++++++++-------- lib/server.js | 50 ++++++++++++++++++++++++++++- test/lib/server/middleware/csp.js | 26 +++++++-------- 5 files changed, 109 insertions(+), 31 deletions(-) diff --git a/lib/middleware/MiddlewareManager.js b/lib/middleware/MiddlewareManager.js index 5586c1d5..10c9b160 100644 --- a/lib/middleware/MiddlewareManager.js +++ b/lib/middleware/MiddlewareManager.js @@ -123,8 +123,8 @@ class MiddlewareManager { cspReportPath: this.options.cspReportPath }); } - return () => { - return cspModule("sap-ui-xx-csp-policy", oCspConfig); + return ({middlewareUtil}) => { + return cspModule("sap-ui-xx-csp-policy", oCspConfig, middlewareUtil); }; } }); @@ -209,6 +209,15 @@ class MiddlewareManager { }); } } + + /** + * The middlewareUtil used by the manager + * + * @returns {module.MiddlewareUtil} + */ + getMiddlewareUtil() { + return this.middlewareUtil; + } } module.exports = MiddlewareManager; diff --git a/lib/middleware/MiddlewareUtil.js b/lib/middleware/MiddlewareUtil.js index 95cf81bd..605f820f 100644 --- a/lib/middleware/MiddlewareUtil.js +++ b/lib/middleware/MiddlewareUtil.js @@ -11,6 +11,10 @@ * @memberof module:@ui5/server.middleware */ class MiddlewareUtil { + constructor() { + this.onExitFunctions = []; + } + /** * Get an interface to an instance of this class that only provides those functions * that are supported by the given custom middleware extension specification version. @@ -91,6 +95,24 @@ class MiddlewareUtil { contentType: type + (charset ? "; charset=" + charset : "") }; } + + /** + * @param {Function} onExitFunction function which gets executed on exit + */ + registerExitFunction(onExitFunction) { + this.onExitFunctions.push(onExitFunction); + } + + /** + * Executes all onExit functions in parallel + * + * @returns {Promise} once all exitFunctions finished + */ + async executeExitFunctions() { + await Promise.all(this.onExitFunctions.map((callback) => { + return callback(); + })); + } } module.exports = MiddlewareUtil; diff --git a/lib/middleware/csp.js b/lib/middleware/csp.js index 7dd1a31b..1ac11b50 100644 --- a/lib/middleware/csp.js +++ b/lib/middleware/csp.js @@ -45,19 +45,19 @@ const writeCspReportsFiles = async (filePath, cspReportEntries) => { * @property {string} defaultPolicy2 * @property {boolean} defaultPolicy2IsReportOnly * @property {object} definedPolicies + * @property {string} cspReportPath path where the reports should be written to */ /** * @module @ui5/server/middleware/csp * Middleware which enables CSP (content security policy) support * @see https://www.w3.org/TR/CSP/ - * * @param {string} sCspUrlParameterName * @param {CspConfig} oConfig - * @param {string} cspReportPath path where the reports should be written to + * @param {module.MiddlewareUtil} middlewareUtil used to register an exit hook * @returns {Function} Returns a server middleware closure. */ -function createMiddleware(sCspUrlParameterName, oConfig, cspReportPath) { +function createMiddleware(sCspUrlParameterName, oConfig, middlewareUtil) { const { allowDynamicPolicySelection = false, allowDynamicPolicyDefinition = false, @@ -65,7 +65,8 @@ function createMiddleware(sCspUrlParameterName, oConfig, cspReportPath) { defaultPolicyIsReportOnly = false, defaultPolicy2 = null, defaultPolicy2IsReportOnly = false, - definedPolicies = {} + definedPolicies = {}, + cspReportPath = null } = oConfig; @@ -76,8 +77,10 @@ function createMiddleware(sCspUrlParameterName, oConfig, cspReportPath) { // initially write the file if (cspReportPath) { - writeCspReportsFiles(cspReportPath, cspReportEntries).then(() => { - log.verbose(`Wrote csp reports initially to ${cspReportPath}`); + middlewareUtil.registerExitFunction(async () => { + await writeCspReportsFiles(cspReportPath, cspReportEntries).then(() => { + log.verbose(`Wrote csp reports initially to ${cspReportPath}`); + }); }); } @@ -88,13 +91,13 @@ function createMiddleware(sCspUrlParameterName, oConfig, cspReportPath) { if (req.headers["content-type"] === "application/csp-report" && oParsedURL.pathname.endsWith("/dummy.csplog") ) { // Write the violation into a csp-reports json file if cspReportPath parameter is set - if (cspReportPath && req.body && req.body["csp-report"]) { - // extract the csp-report and add it to the cspReportEntries list - cspReportEntries.push(req.body["csp-report"]); - // write the csp report file - writeCspReportsFiles(cspReportPath, cspReportEntries).then(() => { - log.verbose(`Wrote csp reports, length: ${cspReportEntries.length}`); - }); + if (cspReportPath && req.body) { + const reqBodyObject = typeof req.body === "string" ? JSON.parse(req.body) : req.body; + const cspReportObject = reqBodyObject["csp-report"]; + if (cspReportObject) { + // extract the csp-report and add it to the cspReportEntries list + cspReportEntries.push(cspReportObject); + } } return; } diff --git a/lib/server.js b/lib/server.js index 1d2cd61a..b8a742d6 100644 --- a/lib/server.js +++ b/lib/server.js @@ -88,6 +88,50 @@ function _addSsl({app, key, cert}) { return require("spdy").createServer({cert, key}, app); } +async function executeExitTasks(middlewareManager) { + await middlewareManager.getMiddlewareUtil().executeExitFunctions(); +} + +function registerExitSigHooks(middlewareManager) { + function createListener(exitCode) { + return function() { + // Asynchronously cleanup resources, then exit + executeExitTasks(middlewareManager).then(() => { + process.exit(exitCode); + }); + }; + } + + const processSignals = { + "SIGHUP": createListener(128 + 1), + "SIGINT": createListener(128 + 2), + "SIGTERM": createListener(128 + 15), + "SIGBREAK": createListener(128 + 21) + }; + + for (const signal of Object.keys(processSignals)) { + process.on(signal, processSignals[signal]); + } + + // == TO BE DISCUSSED: Also cleanup for unhandled rejections and exceptions? + // Add additional events like signals since they are registered on the process + // event emitter in a similar fashion + // processSignals["unhandledRejection"] = createListener(1); + // process.once("unhandledRejection", processSignals["unhandledRejection"]); + // processSignals["uncaughtException"] = function(err, origin) { + // const fs = require("fs"); + // fs.writeSync( + // process.stderr.fd, + // `Caught exception: ${err}\n` + + // `Exception origin: ${origin}` + // ); + // createListener(1)(); + // }; + // process.once("uncaughtException", processSignals["uncaughtException"]); + + return processSignals; +} + /** * @public * @namespace @@ -155,11 +199,15 @@ module.exports = { const {port, server} = await _listen(app, requestedPort, changePortIfInUse, acceptRemoteConnections); + registerExitSigHooks(middlewareManager); + return { h2, port, close: function(callback) { - server.close(callback); + executeExitTasks(middlewareManager).then(() => { + server.close(callback); + }); } }; } diff --git a/test/lib/server/middleware/csp.js b/test/lib/server/middleware/csp.js index fe5e8300..d165f845 100644 --- a/test/lib/server/middleware/csp.js +++ b/test/lib/server/middleware/csp.js @@ -1,5 +1,6 @@ const test = require("ava"); let cspMiddleware = require("../../../../lib/middleware/csp"); +const MiddlewareUtil = require("../../../../lib/middleware/MiddlewareUtil"); const mock = require("mock-require"); const sinon = require("sinon"); @@ -18,18 +19,12 @@ test.before((t) => { t.context.logVerboseStub = sinon.stub(); - let callCount = 0; mock("@ui5/logger", { getLogger: () => { return { verbose: (message) => { t.context.logVerboseStub(message); - // 2 calls: - // * initially creating the file - // * adding a csp-violation - if (++callCount === 2) { - wasResolved(); - } + wasResolved(); } }; } @@ -83,8 +78,11 @@ test("Default Settings", (t) => { }); test("Default Settings CSP violation", async (t) => { - t.plan(8); - const middleware = cspMiddleware("sap-ui-xx-csp-policy", {}, "my-report.json"); + t.plan(5); + const oMiddlewareUtil = new MiddlewareUtil(); + const middleware = cspMiddleware("sap-ui-xx-csp-policy", { + cspReportPath: "my-report.json" + }, oMiddlewareUtil); const cspReport = { "document-uri": "https://otherserver:8080/index.html", @@ -109,17 +107,15 @@ test("Default Settings CSP violation", async (t) => { } }, {}, undefined); + await oMiddlewareUtil.executeExitFunctions(); await t.context.logVerboseStubCalled; - t.is(t.context.logVerboseStub.callCount, 2, "should be called 2 times"); + t.is(t.context.logVerboseStub.callCount, 1, "should be called 1 times"); t.deepEqual(t.context.logVerboseStub.getCall(0).args, ["Wrote csp reports initially to my-report.json"], "first log verbose"); - t.deepEqual(t.context.logVerboseStub.getCall(1).args, ["Wrote csp reports, length: 1"], "second log verbose"); - t.is(t.context.writeFileStub.callCount, 2, "should be called 2 times"); + t.is(t.context.writeFileStub.callCount, 1, "should be called 2 times"); t.is(t.context.writeFileStub.getCall(0).args[0], "my-report.json", "filename"); - t.is(t.context.writeFileStub.getCall(0).args[1], "{\"csp-reports\":[]}", "initial content"); - t.is(t.context.writeFileStub.getCall(1).args[0], "my-report.json", "filename"); - t.is(t.context.writeFileStub.getCall(1).args[1], "{\"csp-reports\":[" + JSON.stringify(cspReport) + "]}", "content with reports"); + t.is(t.context.writeFileStub.getCall(0).args[1], "{\"csp-reports\":[" + JSON.stringify(cspReport) + "]}", "content with reports"); }); test("Custom Settings", (t) => { From 7a9f9ab1d04e33584213f6cdff5e479e39c768f1 Mon Sep 17 00:00:00 2001 From: Tobias Sorn Date: Fri, 29 May 2020 10:28:08 +0200 Subject: [PATCH 05/11] [FEATURE] csp: changed report to report serving report is served for URL "/.ui5/csp/csp-reports.json" when parameter serveCSPReports is active. Rather than saving the file on exit, the content now can be accessed on demand. Additionally the served content is valid json. --- lib/middleware/MiddlewareManager.js | 20 ++-- lib/middleware/MiddlewareUtil.js | 22 ----- lib/middleware/csp.js | 59 +++++------- lib/server.js | 56 +---------- test/lib/server/middleware/csp.js | 138 +++++++++++++++++----------- 5 files changed, 117 insertions(+), 178 deletions(-) diff --git a/lib/middleware/MiddlewareManager.js b/lib/middleware/MiddlewareManager.js index 10c9b160..259a240c 100644 --- a/lib/middleware/MiddlewareManager.js +++ b/lib/middleware/MiddlewareManager.js @@ -8,7 +8,8 @@ const MiddlewareUtil = require("./MiddlewareUtil"); */ class MiddlewareManager { constructor({tree, resources, options = { - sendSAPTargetCSP: false + sendSAPTargetCSP: false, + serveCSPReports: false }}) { if (!tree || !resources || !resources.all || !resources.rootProject || !resources.dependencies) { throw new Error("[MiddlewareManager]: One or more mandatory parameters not provided"); @@ -118,13 +119,13 @@ class MiddlewareManager { defaultPolicy2IsReportOnly: true, }); } - if (this.options.cspReportPath) { + if (this.options.serveCSPReports) { Object.assign(oCspConfig, { - cspReportPath: this.options.cspReportPath + serveCSPReports: true, }); } - return ({middlewareUtil}) => { - return cspModule("sap-ui-xx-csp-policy", oCspConfig, middlewareUtil); + return () => { + return cspModule("sap-ui-xx-csp-policy", oCspConfig); }; } }); @@ -209,15 +210,6 @@ class MiddlewareManager { }); } } - - /** - * The middlewareUtil used by the manager - * - * @returns {module.MiddlewareUtil} - */ - getMiddlewareUtil() { - return this.middlewareUtil; - } } module.exports = MiddlewareManager; diff --git a/lib/middleware/MiddlewareUtil.js b/lib/middleware/MiddlewareUtil.js index 605f820f..95cf81bd 100644 --- a/lib/middleware/MiddlewareUtil.js +++ b/lib/middleware/MiddlewareUtil.js @@ -11,10 +11,6 @@ * @memberof module:@ui5/server.middleware */ class MiddlewareUtil { - constructor() { - this.onExitFunctions = []; - } - /** * Get an interface to an instance of this class that only provides those functions * that are supported by the given custom middleware extension specification version. @@ -95,24 +91,6 @@ class MiddlewareUtil { contentType: type + (charset ? "; charset=" + charset : "") }; } - - /** - * @param {Function} onExitFunction function which gets executed on exit - */ - registerExitFunction(onExitFunction) { - this.onExitFunctions.push(onExitFunction); - } - - /** - * Executes all onExit functions in parallel - * - * @returns {Promise} once all exitFunctions finished - */ - async executeExitFunctions() { - await Promise.all(this.onExitFunctions.map((callback) => { - return callback(); - })); - } } module.exports = MiddlewareUtil; diff --git a/lib/middleware/csp.js b/lib/middleware/csp.js index 1ac11b50..d5fc9614 100644 --- a/lib/middleware/csp.js +++ b/lib/middleware/csp.js @@ -1,10 +1,5 @@ -const logger = require("@ui5/logger"); -const log = logger.getLogger("server:middleware:csp"); const parseurl = require("parseurl"); const querystring = require("querystring"); -const {promisify} = require("util"); -const fs = require("graceful-fs"); -const writeFile = promisify(fs.writeFile); const HEADER_CONTENT_SECURITY_POLICY = "Content-Security-Policy"; const HEADER_CONTENT_SECURITY_POLICY_REPORT_ONLY = "Content-Security-Policy-Report-Only"; @@ -22,20 +17,6 @@ function addHeader(res, header, value) { } -/** - * Writes the csp report entries to the given file path - * - * @param {string} filePath target file, e.g. csp-reports.json - * @param {object[]} cspReportEntries array of csp-report json objects - * @returns {Promise} which resolves when the file was written - */ -const writeCspReportsFiles = async (filePath, cspReportEntries) => { - const objectToWrite = { - "csp-reports": cspReportEntries - }; - return writeFile(filePath, JSON.stringify(objectToWrite)); -}; - /** * @typedef {object} CspConfig * @property {boolean} allowDynamicPolicySelection @@ -45,7 +26,7 @@ const writeCspReportsFiles = async (filePath, cspReportEntries) => { * @property {string} defaultPolicy2 * @property {boolean} defaultPolicy2IsReportOnly * @property {object} definedPolicies - * @property {string} cspReportPath path where the reports should be written to + * @property {boolean} serveCSPReports whether to serve the csp resources */ /** @@ -66,7 +47,7 @@ function createMiddleware(sCspUrlParameterName, oConfig, middlewareUtil) { defaultPolicy2 = null, defaultPolicy2IsReportOnly = false, definedPolicies = {}, - cspReportPath = null + serveCSPReports = false } = oConfig; @@ -75,23 +56,14 @@ function createMiddleware(sCspUrlParameterName, oConfig, middlewareUtil) { */ const cspReportEntries = []; - // initially write the file - if (cspReportPath) { - middlewareUtil.registerExitFunction(async () => { - await writeCspReportsFiles(cspReportPath, cspReportEntries).then(() => { - log.verbose(`Wrote csp reports initially to ${cspReportPath}`); - }); - }); - } - return function csp(req, res, next) { const oParsedURL = parseurl(req); if (req.method === "POST" ) { if (req.headers["content-type"] === "application/csp-report" && - oParsedURL.pathname.endsWith("/dummy.csplog") ) { - // Write the violation into a csp-reports json file if cspReportPath parameter is set - if (cspReportPath && req.body) { + oParsedURL.pathname.endsWith("/.ui5/csp/report.csplog") ) { + // Write the violation into a csp-reports json object which can be retrieved via request to '/.ui5/csp/csp-reports.json' + if (serveCSPReports && req.body) { const reqBodyObject = typeof req.body === "string" ? JSON.parse(req.body) : req.body; const cspReportObject = reqBodyObject["csp-report"]; if (cspReportObject) { @@ -103,6 +75,19 @@ function createMiddleware(sCspUrlParameterName, oConfig, middlewareUtil) { } next(); return; + } else if (serveCSPReports && req.method === "GET") { + // serve csp reports + if (req.headers["content-type"] === "application/json" && + oParsedURL.pathname.endsWith("/.ui5/csp/csp-reports.json")) { + const body = JSON.stringify({ + "csp-reports": cspReportEntries + }); + res.writeHead(200, { + "Content-Type": "application/json" + }); + res.end(body); + return; + } } // add CSP headers only to get requests for *.html pages @@ -145,16 +130,16 @@ function createMiddleware(sCspUrlParameterName, oConfig, middlewareUtil) { // collect header values based on configuration if (policy) { if (reportOnly) { - // Add dummy report-uri. This is mandatory for the report-only mode. - addHeader(res, HEADER_CONTENT_SECURITY_POLICY_REPORT_ONLY, policy + " report-uri dummy.csplog;"); + // Add report-uri. This is mandatory for the report-only mode. + addHeader(res, HEADER_CONTENT_SECURITY_POLICY_REPORT_ONLY, policy + " report-uri /.ui5/csp/report.csplog;"); } else { addHeader(res, HEADER_CONTENT_SECURITY_POLICY, policy); } } if (policy2) { if (reportOnly2) { - // Add dummy report-uri. This is mandatory for the report-only mode. - addHeader(res, HEADER_CONTENT_SECURITY_POLICY_REPORT_ONLY, policy2 + " report-uri dummy.csplog;"); + // Add report-uri. This is mandatory for the report-only mode. + addHeader(res, HEADER_CONTENT_SECURITY_POLICY_REPORT_ONLY, policy2 + " report-uri /.ui5/csp/report.csplog;"); } else { addHeader(res, HEADER_CONTENT_SECURITY_POLICY, policy2); } diff --git a/lib/server.js b/lib/server.js index b8a742d6..56ee9a6d 100644 --- a/lib/server.js +++ b/lib/server.js @@ -88,50 +88,6 @@ function _addSsl({app, key, cert}) { return require("spdy").createServer({cert, key}, app); } -async function executeExitTasks(middlewareManager) { - await middlewareManager.getMiddlewareUtil().executeExitFunctions(); -} - -function registerExitSigHooks(middlewareManager) { - function createListener(exitCode) { - return function() { - // Asynchronously cleanup resources, then exit - executeExitTasks(middlewareManager).then(() => { - process.exit(exitCode); - }); - }; - } - - const processSignals = { - "SIGHUP": createListener(128 + 1), - "SIGINT": createListener(128 + 2), - "SIGTERM": createListener(128 + 15), - "SIGBREAK": createListener(128 + 21) - }; - - for (const signal of Object.keys(processSignals)) { - process.on(signal, processSignals[signal]); - } - - // == TO BE DISCUSSED: Also cleanup for unhandled rejections and exceptions? - // Add additional events like signals since they are registered on the process - // event emitter in a similar fashion - // processSignals["unhandledRejection"] = createListener(1); - // process.once("unhandledRejection", processSignals["unhandledRejection"]); - // processSignals["uncaughtException"] = function(err, origin) { - // const fs = require("fs"); - // fs.writeSync( - // process.stderr.fd, - // `Caught exception: ${err}\n` + - // `Exception origin: ${origin}` - // ); - // createListener(1)(); - // }; - // process.once("uncaughtException", processSignals["uncaughtException"]); - - return processSignals; -} - /** * @public * @namespace @@ -155,7 +111,7 @@ module.exports = { * aim for (AKA 'target policies'), are send for any requested * *.html file * @param {boolean} [options.simpleIndex=false] Use a simplified view for the server directory listing - * @param {string} [options.cspReportPath] Enable csp report logging to the given path + * @param {boolean} [options.serveCSPReports=false] Enable csp reports serving for request url '/.ui5/csp/csp-reports.json' * @returns {Promise} Promise resolving once the server is listening. * It resolves with an object containing the port, * h2-flag and a close function, @@ -163,7 +119,7 @@ module.exports = { */ async serve(tree, { port: requestedPort, changePortIfInUse = false, h2 = false, key, cert, - acceptRemoteConnections = false, sendSAPTargetCSP = false, simpleIndex = false, cspReportPath + acceptRemoteConnections = false, sendSAPTargetCSP = false, simpleIndex = false, serveCSPReports = false }) { const projectResourceCollections = resourceFactory.createCollectionsForTree(tree); @@ -185,7 +141,7 @@ module.exports = { resources, options: { sendSAPTargetCSP, - cspReportPath, + serveCSPReports, simpleIndex } }); @@ -199,15 +155,11 @@ module.exports = { const {port, server} = await _listen(app, requestedPort, changePortIfInUse, acceptRemoteConnections); - registerExitSigHooks(middlewareManager); - return { h2, port, close: function(callback) { - executeExitTasks(middlewareManager).then(() => { - server.close(callback); - }); + server.close(callback); } }; } diff --git a/test/lib/server/middleware/csp.js b/test/lib/server/middleware/csp.js index d165f845..ce398378 100644 --- a/test/lib/server/middleware/csp.js +++ b/test/lib/server/middleware/csp.js @@ -1,44 +1,6 @@ const test = require("ava"); -let cspMiddleware = require("../../../../lib/middleware/csp"); -const MiddlewareUtil = require("../../../../lib/middleware/MiddlewareUtil"); -const mock = require("mock-require"); +const cspMiddleware = require("../../../../lib/middleware/csp"); -const sinon = require("sinon"); -const fs = require("graceful-fs"); - - -test.before((t) => { - // csp-reports file is written async and afterwards a verbose log is done. - // wait until the file was written initially and until the first entry was made - // using t.context.logVerboseStubCalled promise being called - let wasResolved = () => {}; - t.context.logVerboseStubCalled = new Promise((resolve) => { - wasResolved = resolve; - }); - - - t.context.logVerboseStub = sinon.stub(); - - mock("@ui5/logger", { - getLogger: () => { - return { - verbose: (message) => { - t.context.logVerboseStub(message); - wasResolved(); - } - }; - } - }); - mock.reRequire("@ui5/logger"); - t.context.writeFileStub = sinon.stub(fs, "writeFile").callsArg(2); - - // Re-require to ensure that mocked modules are used - cspMiddleware = mock.reRequire("../../../../lib/middleware/csp"); -}); - -test.after.always(() => { - sinon.restore(); -}); test("Default Settings", (t) => { t.plan(3 + 7); // fourth request should end in middleware and not call next! @@ -67,22 +29,21 @@ test("Default Settings", (t) => { middleware({method: "POST", url: "somePath", headers: {}}, res, next); middleware({ method: "POST", - url: "/dummy.csplog", + url: "/.ui5/csp/report.csplog", headers: {"content-type": "application/csp-report"} }, res, noNext); // check that unsupported methods result in a call to next() ["CONNECT", "DELETE", "HEAD", "OPTIONS", "PATCH", "PUT", "TRACE"].forEach( - (method) => middleware({method, url: "/dummy.csplog", headers: {}}, res, next) + (method) => middleware({method, url: "/.ui5/csp/report.csplog", headers: {}}, res, next) ); }); test("Default Settings CSP violation", async (t) => { - t.plan(5); - const oMiddlewareUtil = new MiddlewareUtil(); + t.plan(1); const middleware = cspMiddleware("sap-ui-xx-csp-policy", { - cspReportPath: "my-report.json" - }, oMiddlewareUtil); + serveCSPReports: true + }); const cspReport = { "document-uri": "https://otherserver:8080/index.html", @@ -98,24 +59,95 @@ test("Default Settings CSP violation", async (t) => { "script-sample": "" }; + const res = { + writeHead: function(status, contentType) { + }, + end: function(content) { + t.is(content, JSON.stringify({"csp-reports": [cspReport]}), "content matches"); + }, + }; + middleware({ method: "POST", - url: "/dummy.csplog", + url: "/.ui5/csp/report.csplog", headers: {"content-type": "application/csp-report"}, body: { "csp-report": cspReport } }, {}, undefined); - await oMiddlewareUtil.executeExitFunctions(); - await t.context.logVerboseStubCalled; + middleware({ + method: "GET", + url: "/.ui5/csp/csp-reports.json", + headers: {"content-type": "application/json"} + }, res, undefined); +}); +test("Default Settings two CSP violations", async (t) => { + t.plan(1); + const middleware = cspMiddleware("sap-ui-xx-csp-policy", { + serveCSPReports: true + }); - t.is(t.context.logVerboseStub.callCount, 1, "should be called 1 times"); - t.deepEqual(t.context.logVerboseStub.getCall(0).args, ["Wrote csp reports initially to my-report.json"], "first log verbose"); - t.is(t.context.writeFileStub.callCount, 1, "should be called 2 times"); - t.is(t.context.writeFileStub.getCall(0).args[0], "my-report.json", "filename"); - t.is(t.context.writeFileStub.getCall(0).args[1], "{\"csp-reports\":[" + JSON.stringify(cspReport) + "]}", "content with reports"); + const cspReport1 = { + "document-uri": "https://otherserver:8080/index.html", + "referrer": "", + "violated-directive": "script-src-elem", + "effective-directive": "script-src-elem", + "original-policy": "default-src 'self' myserver:443; report-uri /report-csp-violation", + "disposition": "report", + "blocked-uri": "inline", + "line-number": 17, + "source-file": "https://otherserver:8080/index.html", + "status-code": 0, + "script-sample": "" + }; + + const cspReport2 = { + "document-uri": "https://otherserver:8080/imprint.html", + "referrer": "", + "violated-directive": "script-src-elem", + "effective-directive": "script-src-elem", + "original-policy": "default-src 'self' myserver:443; report-uri /report-csp-violation", + "disposition": "report", + "blocked-uri": "inline", + "line-number": 15, + "source-file": "https://otherserver:8080/imprint.html", + "status-code": 0, + "script-sample": "" + }; + + const res = { + writeHead: function(status, contentType) { + }, + end: function(content) { + t.is(content, JSON.stringify({"csp-reports": [cspReport1, cspReport2]}), "content matches"); + }, + }; + + middleware({ + method: "POST", + url: "/.ui5/csp/report.csplog", + headers: {"content-type": "application/csp-report"}, + body: { + "csp-report": cspReport1 + } + }, {}, undefined); + + middleware({ + method: "POST", + url: "/.ui5/csp/report.csplog", + headers: {"content-type": "application/csp-report"}, + body: { + "csp-report": cspReport2 + } + }, {}, undefined); + + middleware({ + method: "GET", + url: "/.ui5/csp/csp-reports.json", + headers: {"content-type": "application/json"} + }, res, undefined); }); test("Custom Settings", (t) => { From 481dcd427a3995e850485aed869f1c792ecee21f Mon Sep 17 00:00:00 2001 From: Tobias Sorn Date: Fri, 29 May 2020 10:32:27 +0200 Subject: [PATCH 06/11] [FEATURE] csp: added test removed middleUtil --- lib/middleware/csp.js | 6 +++--- test/lib/server/middleware/csp.js | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/lib/middleware/csp.js b/lib/middleware/csp.js index d5fc9614..22e03ac8 100644 --- a/lib/middleware/csp.js +++ b/lib/middleware/csp.js @@ -35,10 +35,9 @@ function addHeader(res, header, value) { * @see https://www.w3.org/TR/CSP/ * @param {string} sCspUrlParameterName * @param {CspConfig} oConfig - * @param {module.MiddlewareUtil} middlewareUtil used to register an exit hook * @returns {Function} Returns a server middleware closure. */ -function createMiddleware(sCspUrlParameterName, oConfig, middlewareUtil) { +function createMiddleware(sCspUrlParameterName, oConfig) { const { allowDynamicPolicySelection = false, allowDynamicPolicyDefinition = false, @@ -62,7 +61,8 @@ function createMiddleware(sCspUrlParameterName, oConfig, middlewareUtil) { if (req.method === "POST" ) { if (req.headers["content-type"] === "application/csp-report" && oParsedURL.pathname.endsWith("/.ui5/csp/report.csplog") ) { - // Write the violation into a csp-reports json object which can be retrieved via request to '/.ui5/csp/csp-reports.json' + // Write the violation into an array + // They can be retrieved via a request to '/.ui5/csp/csp-reports.json' if (serveCSPReports && req.body) { const reqBodyObject = typeof req.body === "string" ? JSON.parse(req.body) : req.body; const cspReportObject = reqBodyObject["csp-report"]; diff --git a/test/lib/server/middleware/csp.js b/test/lib/server/middleware/csp.js index ce398378..2c5d6991 100644 --- a/test/lib/server/middleware/csp.js +++ b/test/lib/server/middleware/csp.js @@ -150,6 +150,27 @@ test("Default Settings two CSP violations", async (t) => { }, res, undefined); }); +test("Default Settings no CSP violations", async (t) => { + t.plan(1); + const middleware = cspMiddleware("sap-ui-xx-csp-policy", { + serveCSPReports: true + }); + + const res = { + writeHead: function(status, contentType) { + }, + end: function(content) { + t.is(content, JSON.stringify({"csp-reports": []}), "content matches"); + }, + }; + + middleware({ + method: "GET", + url: "/.ui5/csp/csp-reports.json", + headers: {"content-type": "application/json"} + }, res, undefined); +}); + test("Custom Settings", (t) => { const middleware = cspMiddleware("csp", { definedPolicies: { From 88301fedbec2a2c730a3f5dd5627266e34fffefd Mon Sep 17 00:00:00 2001 From: Tobias Sorn Date: Tue, 2 Jun 2020 13:39:21 +0200 Subject: [PATCH 07/11] [FEATURE] csp: improve code --- lib/middleware/csp.js | 37 ++++++------ test/lib/server/middleware/csp.js | 93 ++++++++++++++++++++++++++++++- 2 files changed, 109 insertions(+), 21 deletions(-) diff --git a/lib/middleware/csp.js b/lib/middleware/csp.js index 22e03ac8..4502af5d 100644 --- a/lib/middleware/csp.js +++ b/lib/middleware/csp.js @@ -60,34 +60,35 @@ function createMiddleware(sCspUrlParameterName, oConfig) { if (req.method === "POST" ) { if (req.headers["content-type"] === "application/csp-report" && - oParsedURL.pathname.endsWith("/.ui5/csp/report.csplog") ) { + oParsedURL.pathname === "/.ui5/csp/report.csplog") { // Write the violation into an array // They can be retrieved via a request to '/.ui5/csp/csp-reports.json' if (serveCSPReports && req.body) { - const reqBodyObject = typeof req.body === "string" ? JSON.parse(req.body) : req.body; - const cspReportObject = reqBodyObject["csp-report"]; - if (cspReportObject) { - // extract the csp-report and add it to the cspReportEntries list - cspReportEntries.push(cspReportObject); + try { + const reqBodyObject = typeof req.body === "string" ? JSON.parse(req.body) : req.body; + const cspReportObject = reqBodyObject["csp-report"]; + if (cspReportObject) { + // extract the csp-report and add it to the cspReportEntries list + cspReportEntries.push(cspReportObject); + } + } catch (e) { + next(new Error(`Invalid csp-report supplied for ${req.url}`)); } } return; } next(); return; - } else if (serveCSPReports && req.method === "GET") { + } else if (serveCSPReports && req.method === "GET" && oParsedURL.pathname === "/.ui5/csp/csp-reports.json") { // serve csp reports - if (req.headers["content-type"] === "application/json" && - oParsedURL.pathname.endsWith("/.ui5/csp/csp-reports.json")) { - const body = JSON.stringify({ - "csp-reports": cspReportEntries - }); - res.writeHead(200, { - "Content-Type": "application/json" - }); - res.end(body); - return; - } + const body = JSON.stringify({ + "csp-reports": cspReportEntries + }, null, "\t"); + res.writeHead(200, { + "Content-Type": "application/json" + }); + res.end(body); + return; } // add CSP headers only to get requests for *.html pages diff --git a/test/lib/server/middleware/csp.js b/test/lib/server/middleware/csp.js index 2c5d6991..69c50080 100644 --- a/test/lib/server/middleware/csp.js +++ b/test/lib/server/middleware/csp.js @@ -63,7 +63,7 @@ test("Default Settings CSP violation", async (t) => { writeHead: function(status, contentType) { }, end: function(content) { - t.is(content, JSON.stringify({"csp-reports": [cspReport]}), "content matches"); + t.is(content, JSON.stringify({"csp-reports": [cspReport]}, null, "\t"), "content matches"); }, }; @@ -83,6 +83,93 @@ test("Default Settings CSP violation", async (t) => { }, res, undefined); }); +test("Default Settings CSP violation without body parser", async (t) => { + t.plan(1); + const middleware = cspMiddleware("sap-ui-xx-csp-policy", { + serveCSPReports: true + }); + + const cspReport = { + "document-uri": "https://otherserver:8080/index.html", + "referrer": "", + "violated-directive": "script-src-elem", + "effective-directive": "script-src-elem", + "original-policy": "default-src 'self' myserver:443; report-uri /report-csp-violation", + "disposition": "report", + "blocked-uri": "inline", + "line-number": 17, + "source-file": "https://otherserver:8080/index.html", + "status-code": 0, + "script-sample": "" + }; + + const res = { + writeHead: function(status, contentType) { + }, + end: function(content) { + t.is(content, JSON.stringify({"csp-reports": [cspReport]}, null, "\t"), "content matches"); + }, + }; + + middleware({ + method: "POST", + url: "/.ui5/csp/report.csplog", + headers: {"content-type": "application/csp-report"}, + body: JSON.stringify({ + "csp-report": cspReport + }) + }, {}, undefined); + + middleware({ + method: "GET", + url: "/.ui5/csp/csp-reports.json", + headers: {"content-type": "application/json"} + }, res, undefined); +}); + +test("Default Settings CSP violation without body parser, failure", async (t) => { + t.plan(1); + const middleware = cspMiddleware("sap-ui-xx-csp-policy", { + serveCSPReports: true + }); + + const cspReport = { + "document-uri": "https://otherserver:8080/index.html", + "referrer": "", + "violated-directive": "script-src-elem", + "effective-directive": "script-src-elem", + "original-policy": "default-src 'self' myserver:443; report-uri /report-csp-violation", + "disposition": "report", + "blocked-uri": "inline", + "line-number": 17, + "source-file": "https://otherserver:8080/index.html", + "status-code": 0, + "script-sample": "" + }; + + const res = { + writeHead: function(status, contentType) { + }, + end: function(content) { + t.is(content, JSON.stringify({"csp-reports": [cspReport]}, null, "\t"), "content matches"); + }, + }; + + middleware({ + method: "POST", + url: "/.ui5/csp/report.csplog", + headers: {"content-type": "application/csp-report"}, + body: "test" + }, {}, undefined); + + middleware({ + method: "GET", + url: "/.ui5/csp/csp-reports.json", + headers: {"content-type": "application/json"} + }, res, undefined); +}); + + test("Default Settings two CSP violations", async (t) => { t.plan(1); const middleware = cspMiddleware("sap-ui-xx-csp-policy", { @@ -121,7 +208,7 @@ test("Default Settings two CSP violations", async (t) => { writeHead: function(status, contentType) { }, end: function(content) { - t.is(content, JSON.stringify({"csp-reports": [cspReport1, cspReport2]}), "content matches"); + t.is(content, JSON.stringify({"csp-reports": [cspReport1, cspReport2]}, null, "\t"), "content matches"); }, }; @@ -160,7 +247,7 @@ test("Default Settings no CSP violations", async (t) => { writeHead: function(status, contentType) { }, end: function(content) { - t.is(content, JSON.stringify({"csp-reports": []}), "content matches"); + t.is(content, JSON.stringify({"csp-reports": []}, null, "\t"), "content matches"); }, }; From 00e86de522af54438c1a1c42b31666efe1af06be Mon Sep 17 00:00:00 2001 From: Tobias Sorn Date: Tue, 2 Jun 2020 16:37:48 +0200 Subject: [PATCH 08/11] [FEATURE] csp: add error handling Add body parser to parse request body in order to get csp-report. Add res.end() such that csp report requests are not stale (pending). --- lib/middleware/MiddlewareManager.js | 7 ++ lib/middleware/csp.js | 36 +++--- lib/middleware/middlewareRepository.js | 1 + package.json | 1 + test/lib/server/main.js | 55 +++++++++ .../server/middleware/MiddlewareManager.js | 3 +- test/lib/server/middleware/csp.js | 115 +++++------------- 7 files changed, 117 insertions(+), 101 deletions(-) diff --git a/lib/middleware/MiddlewareManager.js b/lib/middleware/MiddlewareManager.js index 259a240c..4d8610e0 100644 --- a/lib/middleware/MiddlewareManager.js +++ b/lib/middleware/MiddlewareManager.js @@ -85,6 +85,13 @@ class MiddlewareManager { } async addStandardMiddleware() { + await this.addMiddleware("bodyParser", { + wrapperCallback: ({middleware}) => { + return () => { + return middleware.json({type: "application/csp-report"}); + }; + } + }); await this.addMiddleware("csp", { wrapperCallback: ({middleware: cspModule}) => { const oCspConfig = { diff --git a/lib/middleware/csp.js b/lib/middleware/csp.js index 4502af5d..6e273cf5 100644 --- a/lib/middleware/csp.js +++ b/lib/middleware/csp.js @@ -1,6 +1,8 @@ const parseurl = require("parseurl"); const querystring = require("querystring"); +const log = require("@ui5/logger").getLogger("server:middleware:csp"); + const HEADER_CONTENT_SECURITY_POLICY = "Content-Security-Policy"; const HEADER_CONTENT_SECURITY_POLICY_REPORT_ONLY = "Content-Security-Policy-Report-Only"; const rPolicy = /^([-_a-zA-Z0-9]+)(:report-only|:ro)?$/i; @@ -58,26 +60,24 @@ function createMiddleware(sCspUrlParameterName, oConfig) { return function csp(req, res, next) { const oParsedURL = parseurl(req); - if (req.method === "POST" ) { - if (req.headers["content-type"] === "application/csp-report" && - oParsedURL.pathname === "/.ui5/csp/report.csplog") { - // Write the violation into an array - // They can be retrieved via a request to '/.ui5/csp/csp-reports.json' - if (serveCSPReports && req.body) { - try { - const reqBodyObject = typeof req.body === "string" ? JSON.parse(req.body) : req.body; - const cspReportObject = reqBodyObject["csp-report"]; - if (cspReportObject) { - // extract the csp-report and add it to the cspReportEntries list - cspReportEntries.push(cspReportObject); - } - } catch (e) { - next(new Error(`Invalid csp-report supplied for ${req.url}`)); - } + if (req.method === "POST" && req.headers["content-type"] === "application/csp-report" && + oParsedURL.pathname === "/.ui5/csp/report.csplog") { + // Write the violation into an array + // They can be retrieved via a request to '/.ui5/csp/csp-reports.json' + if (serveCSPReports) { + if (typeof req.body !== "object") { + const error = new Error(`No body content available: ${req.url}`); + log.error(error); + next(error); + return; + } + const cspReportObject = req.body["csp-report"]; + if (cspReportObject) { + // extract the csp-report and add it to the cspReportEntries list + cspReportEntries.push(cspReportObject); } - return; } - next(); + res.end(); return; } else if (serveCSPReports && req.method === "GET" && oParsedURL.pathname === "/.ui5/csp/csp-reports.json") { // serve csp reports diff --git a/lib/middleware/middlewareRepository.js b/lib/middleware/middlewareRepository.js index d85955e4..f2c5396f 100644 --- a/lib/middleware/middlewareRepository.js +++ b/lib/middleware/middlewareRepository.js @@ -1,4 +1,5 @@ const middlewareInfos = { + bodyParser: {path: "body-parser"}, compression: {path: "compression"}, cors: {path: "cors"}, csp: {path: "./csp"}, diff --git a/package.json b/package.json index ebb52507..b053b712 100644 --- a/package.json +++ b/package.json @@ -100,6 +100,7 @@ "@ui5/builder": "^2.0.3", "@ui5/fs": "^2.0.1", "@ui5/logger": "^2.0.0", + "body-parser": "^1.19.0", "compression": "^1.7.4", "connect-openui5": "^0.9.0", "cors": "^2.8.5", diff --git a/test/lib/server/main.js b/test/lib/server/main.js index 6a49bec4..1900cd43 100644 --- a/test/lib/server/main.js +++ b/test/lib/server/main.js @@ -564,6 +564,61 @@ test("CSP (sap policies)", (t) => { }); }); +test("CSP serveCSPReports", (t) => { + const port = 3450; + const request = supertest(`http://localhost:${port}`); + let localServeResult; + return normalizer.generateProjectTree({ + cwd: "./test/fixtures/application.a" + }).then((tree) => { + return server.serve(tree, { + port, + serveCSPReports: true, + simpleIndex: false + }); + }).then((serveResult) => { + localServeResult = serveResult; + const cspReport = { + "csp-report": { + "document-uri": "https://otherserver:8080/index.html", + "referrer": "", + "violated-directive": "script-src-elem", + "effective-directive": "script-src-elem", + "original-policy": "default-src 'self' myserver:443; report-uri /report-csp-violation", + "disposition": "report", + "blocked-uri": "inline", + "line-number": 17, + "source-file": "https://otherserver:8080/index.html", + "status-code": 0, + "script-sample": "" + } + }; + return request.post("/.ui5/csp/report.csplog") + .set("Content-Type", "application/csp-report") + // to allow setting the content type the argument for sending must be a string + .send(JSON.stringify(cspReport)) + .expect(200); + }).then(() => { + return request.get("/.ui5/csp/csp-reports.json") + .then((res) => { + t.true(typeof res.body === "object", "the body is an object"); + t.true(Array.isArray(res.body["csp-reports"]), "csp-reports is an array"); + t.is(res.body["csp-reports"].length, 1, "one csp report in result"); + }); + }).then(() => { + return new Promise((resolve, reject) => { + localServeResult.close((error) => { + if (error) { + reject(error); + } else { + t.pass("Server closing"); + resolve(); + } + }); + }); + }); +}); + test("Get index of resources", (t) => { return Promise.all([ request.get("").then((res) => { diff --git a/test/lib/server/middleware/MiddlewareManager.js b/test/lib/server/middleware/MiddlewareManager.js index 7bab4fac..e026de82 100644 --- a/test/lib/server/middleware/MiddlewareManager.js +++ b/test/lib/server/middleware/MiddlewareManager.js @@ -282,12 +282,13 @@ test("addStandardMiddleware: Adds standard middleware in correct order", async ( const addMiddlewareStub = sinon.stub(middlewareManager, "addMiddleware").resolves(); await middlewareManager.addStandardMiddleware(); - t.deepEqual(addMiddlewareStub.callCount, 11, "Expected count of middleware got added"); + t.deepEqual(addMiddlewareStub.callCount, 12, "Expected count of middleware got added"); const addedMiddlewareNames = []; for (let i = 0; i < addMiddlewareStub.callCount; i++) { addedMiddlewareNames.push(addMiddlewareStub.getCall(i).args[0]); } t.deepEqual(addedMiddlewareNames, [ + "bodyParser", "csp", "compression", "cors", diff --git a/test/lib/server/middleware/csp.js b/test/lib/server/middleware/csp.js index 69c50080..d8a216d7 100644 --- a/test/lib/server/middleware/csp.js +++ b/test/lib/server/middleware/csp.js @@ -3,12 +3,15 @@ const cspMiddleware = require("../../../../lib/middleware/csp"); test("Default Settings", (t) => { - t.plan(3 + 7); // fourth request should end in middleware and not call next! + t.plan(3 + 8); // fourth request should end in middleware and not call next! const middleware = cspMiddleware("sap-ui-xx-csp-policy", {}); const res = { getHeader: function() { return undefined; }, + end: function() { + t.true(true, "end is called"); + }, setHeader: function(header, value) { t.fail(`should not be called with header ${header} and value ${value}`); } @@ -74,85 +77,28 @@ test("Default Settings CSP violation", async (t) => { body: { "csp-report": cspReport } - }, {}, undefined); - - middleware({ - method: "GET", - url: "/.ui5/csp/csp-reports.json", - headers: {"content-type": "application/json"} - }, res, undefined); + }, { + end: () => { + middleware({ + method: "GET", + url: "/.ui5/csp/csp-reports.json", + headers: {"content-type": "application/json"} + }, res, undefined); + } + }, undefined); }); -test("Default Settings CSP violation without body parser", async (t) => { - t.plan(1); - const middleware = cspMiddleware("sap-ui-xx-csp-policy", { - serveCSPReports: true - }); - - const cspReport = { - "document-uri": "https://otherserver:8080/index.html", - "referrer": "", - "violated-directive": "script-src-elem", - "effective-directive": "script-src-elem", - "original-policy": "default-src 'self' myserver:443; report-uri /report-csp-violation", - "disposition": "report", - "blocked-uri": "inline", - "line-number": 17, - "source-file": "https://otherserver:8080/index.html", - "status-code": 0, - "script-sample": "" - }; - - const res = { - writeHead: function(status, contentType) { - }, - end: function(content) { - t.is(content, JSON.stringify({"csp-reports": [cspReport]}, null, "\t"), "content matches"); - }, - }; - - middleware({ - method: "POST", - url: "/.ui5/csp/report.csplog", - headers: {"content-type": "application/csp-report"}, - body: JSON.stringify({ - "csp-report": cspReport - }) - }, {}, undefined); - - middleware({ - method: "GET", - url: "/.ui5/csp/csp-reports.json", - headers: {"content-type": "application/json"} - }, res, undefined); -}); -test("Default Settings CSP violation without body parser, failure", async (t) => { - t.plan(1); +test("Default Settings CSP violation without body parser, invalid body content", async (t) => { + t.plan(2); const middleware = cspMiddleware("sap-ui-xx-csp-policy", { serveCSPReports: true }); - const cspReport = { - "document-uri": "https://otherserver:8080/index.html", - "referrer": "", - "violated-directive": "script-src-elem", - "effective-directive": "script-src-elem", - "original-policy": "default-src 'self' myserver:443; report-uri /report-csp-violation", - "disposition": "report", - "blocked-uri": "inline", - "line-number": 17, - "source-file": "https://otherserver:8080/index.html", - "status-code": 0, - "script-sample": "" - }; - const res = { - writeHead: function(status, contentType) { - }, - end: function(content) { - t.is(content, JSON.stringify({"csp-reports": [cspReport]}, null, "\t"), "content matches"); - }, + const nextFunction = function(error) { + t.true(error instanceof Error); + t.is(error.message, "No body content available: /.ui5/csp/report.csplog", "error message matches"); }; middleware({ @@ -160,18 +106,15 @@ test("Default Settings CSP violation without body parser, failure", async (t) => url: "/.ui5/csp/report.csplog", headers: {"content-type": "application/csp-report"}, body: "test" - }, {}, undefined); - - middleware({ - method: "GET", - url: "/.ui5/csp/csp-reports.json", - headers: {"content-type": "application/json"} - }, res, undefined); + }, { + end: function() { + t.fail("res.end should not be called"); + }}, nextFunction); }); test("Default Settings two CSP violations", async (t) => { - t.plan(1); + t.plan(3); const middleware = cspMiddleware("sap-ui-xx-csp-policy", { serveCSPReports: true }); @@ -219,7 +162,11 @@ test("Default Settings two CSP violations", async (t) => { body: { "csp-report": cspReport1 } - }, {}, undefined); + }, { + end: function() { + t.true(true, "end is called"); + } + }, undefined); middleware({ method: "POST", @@ -228,7 +175,11 @@ test("Default Settings two CSP violations", async (t) => { body: { "csp-report": cspReport2 } - }, {}, undefined); + }, { + end: function() { + t.true(true, "end is called"); + } + }, undefined); middleware({ method: "GET", From 7e5274c42d67ddd97a620c00a44c7ef90e28c6d6 Mon Sep 17 00:00:00 2001 From: Tobias Sorn Date: Wed, 3 Jun 2020 13:11:09 +0200 Subject: [PATCH 09/11] [FEATURE] csp: use router Instead of having the body-parser part of the MiddlewareManager it is now part of the csp middleware itself using router module. It allows also to split up the request branches. --- lib/middleware/MiddlewareManager.js | 7 -- lib/middleware/csp.js | 55 ++++++--- lib/middleware/middlewareRepository.js | 1 - package-lock.json | 26 +++++ package.json | 1 + .../server/middleware/MiddlewareManager.js | 3 +- test/lib/server/middleware/csp.js | 105 +++++++++++++----- 7 files changed, 146 insertions(+), 52 deletions(-) diff --git a/lib/middleware/MiddlewareManager.js b/lib/middleware/MiddlewareManager.js index 4d8610e0..259a240c 100644 --- a/lib/middleware/MiddlewareManager.js +++ b/lib/middleware/MiddlewareManager.js @@ -85,13 +85,6 @@ class MiddlewareManager { } async addStandardMiddleware() { - await this.addMiddleware("bodyParser", { - wrapperCallback: ({middleware}) => { - return () => { - return middleware.json({type: "application/csp-report"}); - }; - } - }); await this.addMiddleware("csp", { wrapperCallback: ({middleware: cspModule}) => { const oCspConfig = { diff --git a/lib/middleware/csp.js b/lib/middleware/csp.js index 6e273cf5..e93918bb 100644 --- a/lib/middleware/csp.js +++ b/lib/middleware/csp.js @@ -1,4 +1,6 @@ const parseurl = require("parseurl"); +const Router = require("router"); +const bodyParser = require("body-parser"); const querystring = require("querystring"); const log = require("@ui5/logger").getLogger("server:middleware:csp"); @@ -56,15 +58,15 @@ function createMiddleware(sCspUrlParameterName, oConfig) { * List of CSP Report entries */ const cspReportEntries = []; - - return function csp(req, res, next) { - const oParsedURL = parseurl(req); - - if (req.method === "POST" && req.headers["content-type"] === "application/csp-report" && - oParsedURL.pathname === "/.ui5/csp/report.csplog") { - // Write the violation into an array - // They can be retrieved via a request to '/.ui5/csp/csp-reports.json' - if (serveCSPReports) { + const router = new Router(); + // .csplog + // body parser is required to parse csp-report in body (json) + if (serveCSPReports) { + router.post("/.ui5/csp/report.csplog", bodyParser.json({type: "application/csp-report"})); + router.post("/.ui5/csp/report.csplog", function(req, res, next) { + if (req.headers["content-type"] === "application/csp-report") { + // Write the violation into an array + // They can be retrieved via a request to '/.ui5/csp/csp-reports.json' if (typeof req.body !== "object") { const error = new Error(`No body content available: ${req.url}`); log.error(error); @@ -76,10 +78,24 @@ function createMiddleware(sCspUrlParameterName, oConfig) { // extract the csp-report and add it to the cspReportEntries list cspReportEntries.push(cspReportObject); } + res.end(); + } else { + next(); } - res.end(); - return; - } else if (serveCSPReports && req.method === "GET" && oParsedURL.pathname === "/.ui5/csp/csp-reports.json") { + }); + } else { + router.post("/.ui5/csp/report.csplog", function(req, res, next) { + if (req.headers["content-type"] === "application/csp-report") { + res.end(); + } else { + next(); + } + }); + } + + // csp-reports.json + if (serveCSPReports) { + router.get("/.ui5/csp/csp-reports.json", function(req, res, next) { // serve csp reports const body = JSON.stringify({ "csp-reports": cspReportEntries @@ -88,11 +104,16 @@ function createMiddleware(sCspUrlParameterName, oConfig) { "Content-Type": "application/json" }); res.end(body); - return; - } + }); + } + + // html get requests + // add csp headers + router.get("*", function(req, res, next) { + const oParsedURL = parseurl(req); // add CSP headers only to get requests for *.html pages - if (req.method !== "GET" || !oParsedURL.pathname.endsWith(".html")) { + if (!oParsedURL.pathname.endsWith(".html")) { next(); return; } @@ -147,7 +168,9 @@ function createMiddleware(sCspUrlParameterName, oConfig) { } next(); - }; + }); + + return router; } module.exports = createMiddleware; diff --git a/lib/middleware/middlewareRepository.js b/lib/middleware/middlewareRepository.js index f2c5396f..d85955e4 100644 --- a/lib/middleware/middlewareRepository.js +++ b/lib/middleware/middlewareRepository.js @@ -1,5 +1,4 @@ const middlewareInfos = { - bodyParser: {path: "body-parser"}, compression: {path: "compression"}, cors: {path: "cors"}, csp: {path: "./csp"}, diff --git a/package-lock.json b/package-lock.json index b3f6ea2e..b461e818 100644 --- a/package-lock.json +++ b/package-lock.json @@ -6388,6 +6388,32 @@ "glob": "^7.1.3" } }, + "router": { + "version": "1.3.5", + "resolved": "https://registry.npmjs.org/router/-/router-1.3.5.tgz", + "integrity": "sha512-kozCJZUhuSJ5VcLhSb3F8fsmGXy+8HaDbKCAerR1G6tq3mnMZFMuSohbFvGv1c5oMFipijDjRZuuN/Sq5nMf3g==", + "requires": { + "array-flatten": "3.0.0", + "debug": "2.6.9", + "methods": "~1.1.2", + "parseurl": "~1.3.3", + "path-to-regexp": "0.1.7", + "setprototypeof": "1.2.0", + "utils-merge": "1.0.1" + }, + "dependencies": { + "array-flatten": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/array-flatten/-/array-flatten-3.0.0.tgz", + "integrity": "sha512-zPMVc3ZYlGLNk4mpK1NzP2wg0ml9t7fUgDsayR5Y5rSzxQilzR9FGu/EH2jQOcKSAeAfWeylyW8juy3OkWRvNA==" + }, + "setprototypeof": { + "version": "1.2.0", + "resolved": "https://registry.npmjs.org/setprototypeof/-/setprototypeof-1.2.0.tgz", + "integrity": "sha512-E5LDX7Wrp85Kil5bhZv46j8jOeboKq5JMmYM3gVGdGH8xFpPWXUMsNrlODCrkoxMEeNi/XZIwuRvY4XNwYMJpw==" + } + } + }, "run-async": { "version": "2.4.1", "resolved": "https://registry.npmjs.org/run-async/-/run-async-2.4.1.tgz", diff --git a/package.json b/package.json index b053b712..dabb8844 100644 --- a/package.json +++ b/package.json @@ -115,6 +115,7 @@ "parseurl": "^1.3.3", "portscanner": "^2.1.1", "replacestream": "^4.0.3", + "router": "^1.3.5", "spdy": "^4.0.2", "treeify": "^1.0.1", "yesno": "^0.3.1" diff --git a/test/lib/server/middleware/MiddlewareManager.js b/test/lib/server/middleware/MiddlewareManager.js index e026de82..7bab4fac 100644 --- a/test/lib/server/middleware/MiddlewareManager.js +++ b/test/lib/server/middleware/MiddlewareManager.js @@ -282,13 +282,12 @@ test("addStandardMiddleware: Adds standard middleware in correct order", async ( const addMiddlewareStub = sinon.stub(middlewareManager, "addMiddleware").resolves(); await middlewareManager.addStandardMiddleware(); - t.deepEqual(addMiddlewareStub.callCount, 12, "Expected count of middleware got added"); + t.deepEqual(addMiddlewareStub.callCount, 11, "Expected count of middleware got added"); const addedMiddlewareNames = []; for (let i = 0; i < addMiddlewareStub.callCount; i++) { addedMiddlewareNames.push(addMiddlewareStub.getCall(i).args[0]); } t.deepEqual(addedMiddlewareNames, [ - "bodyParser", "csp", "compression", "cors", diff --git a/test/lib/server/middleware/csp.js b/test/lib/server/middleware/csp.js index d8a216d7..6775d20f 100644 --- a/test/lib/server/middleware/csp.js +++ b/test/lib/server/middleware/csp.js @@ -2,8 +2,8 @@ const test = require("ava"); const cspMiddleware = require("../../../../lib/middleware/csp"); -test("Default Settings", (t) => { - t.plan(3 + 8); // fourth request should end in middleware and not call next! +test("OPTIONS request", (t) => { + t.plan(2); const middleware = cspMiddleware("sap-ui-xx-csp-policy", {}); const res = { getHeader: function() { @@ -13,33 +13,72 @@ test("Default Settings", (t) => { t.true(true, "end is called"); }, setHeader: function(header, value) { - t.fail(`should not be called with header ${header} and value ${value}`); + if (header.startsWith("Content-Security-Policy")) { + t.fail(`should not be called with header ${header} and value ${value}`); + } + if (header === "Allow") { + t.is(value, "GET, HEAD, POST", "GET, HEAD, POST are set"); + } } }; const next = function() { - t.pass("Next was called."); + t.fail("should not be called."); }; + + middleware({method: "OPTIONS", url: "/.ui5/csp/report.csplog", headers: {}}, res, next); +}); + +test("Default Settings", async (t) => { + const middleware = cspMiddleware("sap-ui-xx-csp-policy", {}); + const res = { + getHeader: function() { + return undefined; + }, + end: function() { + t.fail(`end should not be called`); + }, + setHeader: function(header, value) { + t.fail(`should not be called with header ${header} and value ${value}`); + } + }; + const noNext = function() { t.fail("Next should not be called"); }; - - middleware({method: "GET", url: "/test.html", headers: {}}, res, next); - middleware({ - method: "GET", - url: "/test.html?sap-ui-xx-csp-policy=sap-target-level-2", - headers: {} - }, res, next); - middleware({method: "POST", url: "somePath", headers: {}}, res, next); - middleware({ - method: "POST", - url: "/.ui5/csp/report.csplog", - headers: {"content-type": "application/csp-report"} - }, res, noNext); + await new Promise((resolve) => { + middleware({method: "GET", url: "/test.html", headers: {}}, res, resolve); + }); + await new Promise((resolve) => { + middleware({ + method: "GET", + url: "/test.html?sap-ui-xx-csp-policy=sap-target-level-2", + headers: {} + }, res, resolve); + }); + await new Promise((resolve) => { + middleware({method: "POST", url: "somePath", headers: {}}, res, resolve); + }); + await new Promise((resolve) => { + middleware({ + method: "POST", + url: "/.ui5/csp/report.csplog", + headers: {"content-type": "application/csp-report"} + }, { + end: resolve + }, noNext); + }); // check that unsupported methods result in a call to next() - ["CONNECT", "DELETE", "HEAD", "OPTIONS", "PATCH", "PUT", "TRACE"].forEach( - (method) => middleware({method, url: "/.ui5/csp/report.csplog", headers: {}}, res, next) + const otherMethods = ["CONNECT", "DELETE", "HEAD", "PATCH", "PUT", "TRACE"].map( + (method) => { + return new Promise((resolve) => { + middleware({method, url: "/.ui5/csp/report.csplog", headers: {}}, res, resolve); + }); + } ); + + await Promise.all(otherMethods); + t.true(true, "no failure"); }); test("Default Settings CSP violation", async (t) => { @@ -70,6 +109,10 @@ test("Default Settings CSP violation", async (t) => { }, }; + const noNext = function() { + t.fail("Next should not be called"); + }; + middleware({ method: "POST", url: "/.ui5/csp/report.csplog", @@ -83,9 +126,9 @@ test("Default Settings CSP violation", async (t) => { method: "GET", url: "/.ui5/csp/csp-reports.json", headers: {"content-type": "application/json"} - }, res, undefined); + }, res, noNext); } - }, undefined); + }, noNext); }); @@ -155,6 +198,10 @@ test("Default Settings two CSP violations", async (t) => { }, }; + const noNext = function() { + t.fail("Next should not be called"); + }; + middleware({ method: "POST", url: "/.ui5/csp/report.csplog", @@ -166,7 +213,7 @@ test("Default Settings two CSP violations", async (t) => { end: function() { t.true(true, "end is called"); } - }, undefined); + }, noNext); middleware({ method: "POST", @@ -179,13 +226,13 @@ test("Default Settings two CSP violations", async (t) => { end: function() { t.true(true, "end is called"); } - }, undefined); + }, noNext); middleware({ method: "GET", url: "/.ui5/csp/csp-reports.json", headers: {"content-type": "application/json"} - }, res, undefined); + }, res, noNext); }); test("Default Settings no CSP violations", async (t) => { @@ -202,11 +249,15 @@ test("Default Settings no CSP violations", async (t) => { }, }; + const noNext = function() { + t.fail("Next should not be called"); + }; + middleware({ method: "GET", url: "/.ui5/csp/csp-reports.json", headers: {"content-type": "application/json"} - }, res, undefined); + }, res, noNext); }); test("Custom Settings", (t) => { @@ -282,7 +333,8 @@ test("No Dynamic Policy Definition", (t) => { middleware({method: "GET", url: "/test.html?csp=default-src%20ftp:;", headers: {}}, res, next); }); -test("Header Manipulation", (t) => { +test("Header Manipulation, add headers to existing header", (t) => { + t.plan(3); const middleware = cspMiddleware("csp", { definedPolicies: { policy1: "default-src 'self';", @@ -301,6 +353,7 @@ test("Header Manipulation", (t) => { setHeader: function(header, value) { if ( header.toLowerCase() === "content-security-policy" ) { cspHeader = value; + t.true(true, "header is manipulated"); } else { t.fail(`should not be called with header ${header} and value ${value}`); } From ebd252d93ba7b52c9bc886fe6c2e6abe7034f938 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Mon, 8 Jun 2020 11:42:30 +0200 Subject: [PATCH 10/11] Change HTML middleware back to "use" / fix tests (async) --- lib/middleware/csp.js | 56 +++--- test/lib/server/middleware/csp.js | 321 +++++++++++++++++------------- 2 files changed, 209 insertions(+), 168 deletions(-) diff --git a/lib/middleware/csp.js b/lib/middleware/csp.js index e93918bb..44d4bb34 100644 --- a/lib/middleware/csp.js +++ b/lib/middleware/csp.js @@ -61,41 +61,35 @@ function createMiddleware(sCspUrlParameterName, oConfig) { const router = new Router(); // .csplog // body parser is required to parse csp-report in body (json) - if (serveCSPReports) { - router.post("/.ui5/csp/report.csplog", bodyParser.json({type: "application/csp-report"})); - router.post("/.ui5/csp/report.csplog", function(req, res, next) { - if (req.headers["content-type"] === "application/csp-report") { - // Write the violation into an array - // They can be retrieved via a request to '/.ui5/csp/csp-reports.json' - if (typeof req.body !== "object") { - const error = new Error(`No body content available: ${req.url}`); - log.error(error); - next(error); - return; - } - const cspReportObject = req.body["csp-report"]; - if (cspReportObject) { - // extract the csp-report and add it to the cspReportEntries list - cspReportEntries.push(cspReportObject); - } + router.post("/.ui5/csp/report.csplog", bodyParser.json({type: "application/csp-report"})); + router.post("/.ui5/csp/report.csplog", function(req, res, next) { + if (req.headers["content-type"] === "application/csp-report") { + if (!serveCSPReports) { res.end(); - } else { - next(); + return; } - }); - } else { - router.post("/.ui5/csp/report.csplog", function(req, res, next) { - if (req.headers["content-type"] === "application/csp-report") { - res.end(); - } else { - next(); + // Write the violation into an array + // They can be retrieved via a request to '/.ui5/csp/csp-reports.json' + if (typeof req.body !== "object") { + const error = new Error(`No body content available: ${req.url}`); + log.error(error); + next(error); + return; } - }); - } + const cspReportObject = req.body["csp-report"]; + if (cspReportObject) { + // extract the csp-report and add it to the cspReportEntries list + cspReportEntries.push(cspReportObject); + } + res.end(); + } else { + next(); + } + }); // csp-reports.json if (serveCSPReports) { - router.get("/.ui5/csp/csp-reports.json", function(req, res, next) { + router.get("/.ui5/csp/csp-reports.json", (req, res, next) => { // serve csp reports const body = JSON.stringify({ "csp-reports": cspReportEntries @@ -109,11 +103,11 @@ function createMiddleware(sCspUrlParameterName, oConfig) { // html get requests // add csp headers - router.get("*", function(req, res, next) { + router.use((req, res, next) => { const oParsedURL = parseurl(req); // add CSP headers only to get requests for *.html pages - if (!oParsedURL.pathname.endsWith(".html")) { + if (req.method !== "GET" || !oParsedURL.pathname.endsWith(".html")) { next(); return; } diff --git a/test/lib/server/middleware/csp.js b/test/lib/server/middleware/csp.js index 6775d20f..23c97179 100644 --- a/test/lib/server/middleware/csp.js +++ b/test/lib/server/middleware/csp.js @@ -2,30 +2,34 @@ const test = require("ava"); const cspMiddleware = require("../../../../lib/middleware/csp"); -test("OPTIONS request", (t) => { +test("OPTIONS request", async (t) => { t.plan(2); const middleware = cspMiddleware("sap-ui-xx-csp-policy", {}); - const res = { - getHeader: function() { - return undefined; - }, - end: function() { - t.true(true, "end is called"); - }, - setHeader: function(header, value) { - if (header.startsWith("Content-Security-Policy")) { - t.fail(`should not be called with header ${header} and value ${value}`); - } - if (header === "Allow") { - t.is(value, "GET, HEAD, POST", "GET, HEAD, POST are set"); - } - } - }; - const next = function() { - t.fail("should not be called."); - }; - middleware({method: "OPTIONS", url: "/.ui5/csp/report.csplog", headers: {}}, res, next); + await new Promise((resolve) => { + const res = { + getHeader: function() { + return undefined; + }, + end: function() { + t.true(true, "end is called"); + resolve(); + }, + setHeader: function(header, value) { + if (header.startsWith("Content-Security-Policy")) { + t.fail(`should not be called with header ${header} and value ${value}`); + } + if (header === "Allow") { + t.is(value, "POST", "POST should be allowed"); + } + } + }; + const next = function() { + t.fail("should not be called."); + resolve(); + }; + middleware({method: "OPTIONS", url: "/.ui5/csp/report.csplog", headers: {}}, res, next); + }); }); test("Default Settings", async (t) => { @@ -101,34 +105,42 @@ test("Default Settings CSP violation", async (t) => { "script-sample": "" }; - const res = { - writeHead: function(status, contentType) { - }, - end: function(content) { - t.is(content, JSON.stringify({"csp-reports": [cspReport]}, null, "\t"), "content matches"); - }, - }; - - const noNext = function() { - t.fail("Next should not be called"); - }; + await new Promise((resolve) => { + const noNext = function() { + t.fail("Next should not be called"); + resolve(); + }; + middleware({ + method: "POST", + url: "/.ui5/csp/report.csplog", + headers: {"content-type": "application/csp-report"}, + body: { + "csp-report": cspReport + } + }, { + end: resolve + }, noNext); + }); - middleware({ - method: "POST", - url: "/.ui5/csp/report.csplog", - headers: {"content-type": "application/csp-report"}, - body: { - "csp-report": cspReport - } - }, { - end: () => { - middleware({ - method: "GET", - url: "/.ui5/csp/csp-reports.json", - headers: {"content-type": "application/json"} - }, res, noNext); - } - }, noNext); + await new Promise((resolve) => { + const noNext = function() { + t.fail("Next should not be called"); + resolve(); + }; + const res = { + writeHead: function(status, contentType) { + }, + end: function(content) { + t.is(content, JSON.stringify({"csp-reports": [cspReport]}, null, "\t"), "content matches"); + resolve(); + }, + }; + middleware({ + method: "GET", + url: "/.ui5/csp/csp-reports.json", + headers: {"content-type": "application/json"} + }, res, noNext); + }); }); @@ -138,21 +150,23 @@ test("Default Settings CSP violation without body parser, invalid body content", serveCSPReports: true }); - - const nextFunction = function(error) { - t.true(error instanceof Error); - t.is(error.message, "No body content available: /.ui5/csp/report.csplog", "error message matches"); - }; - - middleware({ - method: "POST", - url: "/.ui5/csp/report.csplog", - headers: {"content-type": "application/csp-report"}, - body: "test" - }, { - end: function() { - t.fail("res.end should not be called"); - }}, nextFunction); + await new Promise((resolve) => { + const nextFunction = function(error) { + t.true(error instanceof Error); + t.is(error.message, "No body content available: /.ui5/csp/report.csplog", "error message matches"); + resolve(); + }; + middleware({ + method: "POST", + url: "/.ui5/csp/report.csplog", + headers: {"content-type": "application/csp-report"}, + body: "test" + }, { + end: function() { + t.fail("res.end should not be called"); + resolve(); + }}, nextFunction); + }); }); @@ -190,49 +204,62 @@ test("Default Settings two CSP violations", async (t) => { "script-sample": "" }; - const res = { - writeHead: function(status, contentType) { - }, - end: function(content) { - t.is(content, JSON.stringify({"csp-reports": [cspReport1, cspReport2]}, null, "\t"), "content matches"); - }, - }; - - const noNext = function() { - t.fail("Next should not be called"); - }; + await new Promise((resolve) => { + middleware({ + method: "POST", + url: "/.ui5/csp/report.csplog", + headers: {"content-type": "application/csp-report"}, + body: { + "csp-report": cspReport1 + } + }, { + end: function() { + t.true(true, "end is called"); + resolve(); + } + }, () => { + t.fail("Next should not be called"); + resolve(); + }); + }); - middleware({ - method: "POST", - url: "/.ui5/csp/report.csplog", - headers: {"content-type": "application/csp-report"}, - body: { - "csp-report": cspReport1 - } - }, { - end: function() { - t.true(true, "end is called"); - } - }, noNext); - - middleware({ - method: "POST", - url: "/.ui5/csp/report.csplog", - headers: {"content-type": "application/csp-report"}, - body: { - "csp-report": cspReport2 - } - }, { - end: function() { - t.true(true, "end is called"); - } - }, noNext); + await new Promise((resolve) => { + middleware({ + method: "POST", + url: "/.ui5/csp/report.csplog", + headers: {"content-type": "application/csp-report"}, + body: { + "csp-report": cspReport2 + } + }, { + end: function() { + t.true(true, "end is called"); + resolve(); + } + }, () => { + t.fail("Next should not be called"); + resolve(); + }); + }); - middleware({ - method: "GET", - url: "/.ui5/csp/csp-reports.json", - headers: {"content-type": "application/json"} - }, res, noNext); + await new Promise((resolve) => { + const res = { + writeHead: function(status, contentType) { + }, + end: function(content) { + t.is(content, JSON.stringify({"csp-reports": [cspReport1, cspReport2]}, null, "\t"), "content matches"); + resolve(); + }, + }; + middleware({ + method: "GET", + url: "/.ui5/csp/csp-reports.json", + headers: {"content-type": "application/json"} + }, res, () => { + t.fail("Next should not be called"); + resolve(); + }); + }); }); test("Default Settings no CSP violations", async (t) => { @@ -241,26 +268,29 @@ test("Default Settings no CSP violations", async (t) => { serveCSPReports: true }); - const res = { - writeHead: function(status, contentType) { - }, - end: function(content) { - t.is(content, JSON.stringify({"csp-reports": []}, null, "\t"), "content matches"); - }, - }; - - const noNext = function() { - t.fail("Next should not be called"); - }; - - middleware({ - method: "GET", - url: "/.ui5/csp/csp-reports.json", - headers: {"content-type": "application/json"} - }, res, noNext); + await new Promise((resolve) => { + const res = { + writeHead: function(status, contentType) { + }, + end: function(content) { + t.is(content, JSON.stringify({"csp-reports": []}, null, "\t"), "content matches"); + resolve(); + }, + }; + + const noNext = function() { + t.fail("Next should not be called"); + resolve(); + }; + middleware({ + method: "GET", + url: "/.ui5/csp/csp-reports.json", + headers: {"content-type": "application/json"} + }, res, noNext); + }); }); -test("Custom Settings", (t) => { +test("Custom Settings", async (t) => { const middleware = cspMiddleware("csp", { definedPolicies: { policy1: "default-src 'self';", @@ -287,21 +317,33 @@ test("Custom Settings", (t) => { } } }; - const next = function() { - t.pass("Next was called."); - }; expected = ["default-src 'self';", "default-src http:;"]; - middleware({method: "GET", url: "/test.html", headers: {}}, res, next); + await new Promise((resolve) => { + middleware({method: "GET", url: "/test.html", headers: {}}, res, () => { + t.pass("Next was called."); + resolve(); + }); + }); expected = ["default-src https:;", "default-src http:;"]; - middleware({method: "GET", url: "/test.html?csp=policy3", headers: {}}, res, next); + await new Promise((resolve) => { + middleware({method: "GET", url: "/test.html?csp=policy3", headers: {}}, res, () => { + t.pass("Next was called."); + resolve(); + }); + }); expected = ["default-src ftp:;", "default-src http:;"]; - middleware({method: "GET", url: "/test.html?csp=default-src%20ftp:;", headers: {}}, res, next); + await new Promise((resolve) => { + middleware({method: "GET", url: "/test.html?csp=default-src%20ftp:;", headers: {}}, res, () => { + t.pass("Next was called."); + resolve(); + }); + }); }); -test("No Dynamic Policy Definition", (t) => { +test("No Dynamic Policy Definition", async (t) => { const middleware = cspMiddleware("csp", { definedPolicies: { policy1: "default-src 'self';", @@ -325,15 +367,17 @@ test("No Dynamic Policy Definition", (t) => { } } }; - const next = function() { - t.pass("Next was called."); - }; const expected = ["default-src 'self';", "default-src http:;"]; - middleware({method: "GET", url: "/test.html?csp=default-src%20ftp:;", headers: {}}, res, next); + await new Promise((resolve) => { + middleware({method: "GET", url: "/test.html?csp=default-src%20ftp:;", headers: {}}, res, () => { + t.pass("Next was called."); + resolve(); + }); + }); }); -test("Header Manipulation, add headers to existing header", (t) => { +test("Header Manipulation, add headers to existing header", async (t) => { t.plan(3); const middleware = cspMiddleware("csp", { definedPolicies: { @@ -359,8 +403,11 @@ test("Header Manipulation, add headers to existing header", (t) => { } } }; - const next = function() {}; - middleware({method: "GET", url: "/test.html", headers: {}}, res, next); - t.deepEqual(cspHeader, ["default-src: spdy:", "default-src 'self';", "default-src http:;"]); + await new Promise((resolve) => { + middleware({method: "GET", url: "/test.html", headers: {}}, res, () => { + t.deepEqual(cspHeader, ["default-src: spdy:", "default-src 'self';", "default-src http:;"]); + resolve(); + }); + }); }); From 0c7a241e3af023f37ed82be7fd11194be4e2a374 Mon Sep 17 00:00:00 2001 From: Tobias Sorn Date: Tue, 9 Jun 2020 07:51:28 +0200 Subject: [PATCH 11/11] [FEATURE] csp: lazy require body-parser --- lib/middleware/csp.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/middleware/csp.js b/lib/middleware/csp.js index 44d4bb34..0ab10817 100644 --- a/lib/middleware/csp.js +++ b/lib/middleware/csp.js @@ -1,6 +1,5 @@ const parseurl = require("parseurl"); const Router = require("router"); -const bodyParser = require("body-parser"); const querystring = require("querystring"); const log = require("@ui5/logger").getLogger("server:middleware:csp"); @@ -61,7 +60,10 @@ function createMiddleware(sCspUrlParameterName, oConfig) { const router = new Router(); // .csplog // body parser is required to parse csp-report in body (json) - router.post("/.ui5/csp/report.csplog", bodyParser.json({type: "application/csp-report"})); + if (serveCSPReports) { + const bodyParser = require("body-parser"); + router.post("/.ui5/csp/report.csplog", bodyParser.json({type: "application/csp-report"})); + } router.post("/.ui5/csp/report.csplog", function(req, res, next) { if (req.headers["content-type"] === "application/csp-report") { if (!serveCSPReports) {