Skip to content

Commit

Permalink
[FIX] Makes CSP middleware work in an environment without express ser…
Browse files Browse the repository at this point in the history
…ver (#184)

* [FIX] Makes CSP middleware work in an environment without express server

With change 4f05967, the CSP middleware
made use of 'express' APIs on the request object. When used in an
environment without 'express' (e.g. the OpenUI5 legacy grunt tooling),
the CSP middleware failed for all requests.

It now uses the native NodeJS APIs again.

Fixes https://github.com/SAP/ui5-server/issues/183 .

* adapt tests
  • Loading branch information
codeworrior authored May 13, 2019
1 parent 7113942 commit c3089ad
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 20 deletions.
14 changes: 10 additions & 4 deletions lib/middleware/csp.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
const url = require("url");
const querystring = require("querystring");

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;
Expand Down Expand Up @@ -25,9 +28,11 @@ function createMiddleware(sCspUrlParameterName, oConfig) {
} = oConfig;

return function csp(req, res, next) {
const oParsedURL = url.parse(req.url);

if (req.method === "POST" ) {
if (req.headers["content-type"] === "application/csp-report" &&
req.path.endsWith("/dummy.csplog") ) {
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.
return;
Expand All @@ -37,7 +42,7 @@ function createMiddleware(sCspUrlParameterName, oConfig) {
}

// add CSP headers only to get requests for *.html pages
if (req.method !== "GET" || !req.path.endsWith(".html")) {
if (req.method !== "GET" || !oParsedURL.pathname.endsWith(".html")) {
next();
return;
}
Expand All @@ -48,7 +53,8 @@ function createMiddleware(sCspUrlParameterName, oConfig) {
const policy2 = defaultPolicy2 && definedPolicies[defaultPolicy2];
const reportOnly2 = defaultPolicy2IsReportOnly;

const sCspUrlParameterValue = req.query[sCspUrlParameterName];
const oQuery = querystring.parse(oParsedURL.query);
const sCspUrlParameterValue = oQuery[sCspUrlParameterName];
if (sCspUrlParameterValue) {
const mPolicyMatch = rPolicy.exec(sCspUrlParameterValue);

Expand Down
28 changes: 12 additions & 16 deletions test/lib/server/middleware/csp.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,26 +19,22 @@ test("Default Settings", (t) => {
t.fail("Next should not be called");
};

middleware({method: "GET", path: "/test.html", headers: {}, query: {}}, res, next);
middleware({method: "GET", url: "/test.html", headers: {}}, res, next);
middleware({
method: "GET",
path: "/test.html",
headers: {},
query: {
"sap-ui-xx-csp-policy": "sap-target-level-2"
}
url: "/test.html?sap-ui-xx-csp-policy=sap-target-level-2",
headers: {}
}, res, next);
middleware({method: "POST", path: "somePath", headers: {}, query: {}}, res, next);
middleware({method: "POST", url: "somePath", headers: {}}, res, next);
middleware({
method: "POST",
path: "/dummy.csplog",
headers: {"content-type": "application/csp-report"},
query: {}
url: "/dummy.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, path: "/dummy.csplog", headers: {}, query: {}}, res, next)
(method) => middleware({method, url: "/dummy.csplog", headers: {}}, res, next)
);
});

Expand Down Expand Up @@ -74,13 +70,13 @@ test("Custom Settings", (t) => {
};

expected = ["default-src 'self';", "default-src http:;"];
middleware({method: "GET", path: "/test.html", headers: {}, query: {}}, res, next);
middleware({method: "GET", url: "/test.html", headers: {}}, res, next);

expected = ["default-src https:;", "default-src http:;"];
middleware({method: "GET", path: "/test.html", headers: {}, query: {"csp": "policy3"}}, res, next);
middleware({method: "GET", url: "/test.html?csp=policy3", headers: {}}, res, next);

expected = ["default-src ftp:;", "default-src http:;"];
middleware({method: "GET", path: "/test.html", headers: {}, query: {"csp": "default-src ftp:;"}}, res, next);
middleware({method: "GET", url: "/test.html?csp=default-src%20ftp:;", headers: {}}, res, next);
});

test("No Dynamic Policy Definition", (t) => {
Expand Down Expand Up @@ -112,7 +108,7 @@ test("No Dynamic Policy Definition", (t) => {
};

const expected = ["default-src 'self';", "default-src http:;"];
middleware({method: "GET", path: "/test.html", headers: {}, query: {"csp": "default-src ftp:;"}}, res, next);
middleware({method: "GET", url: "/test.html?csp=default-src%20ftp:;", headers: {}}, res, next);
});

test("Header Manipulation", (t) => {
Expand Down Expand Up @@ -141,6 +137,6 @@ test("Header Manipulation", (t) => {
};
const next = function() {};

middleware({method: "GET", path: "/test.html", headers: {}, query: {}}, res, next);
middleware({method: "GET", url: "/test.html", headers: {}}, res, next);
t.deepEqual(cspHeader, ["default-src: spdy:", "default-src 'self';", "default-src http:;"]);
});

0 comments on commit c3089ad

Please sign in to comment.