Skip to content

Commit

Permalink
Merge remote-tracking branch 'antranig/KETTLE-73'
Browse files Browse the repository at this point in the history
* antranig/KETTLE-73:
  KETTLE-73: Doc fixes after review
  KETTLE-73: Further improvements to censoring following review
  KETTLE-73: Further improvements following review and further dep update
  KETTLE-73: Removed debugging definition after review
  KETTLE-73: Improvements to censoring following review
  KETTLE-73: Linting and doc fixes
  KETTLE-73: Allow censoring of sensitive information which may be present in URL of DataSource
  • Loading branch information
cindyli committed Dec 5, 2018
2 parents 0bd0603 + 1fbf663 commit 281d9aa
Show file tree
Hide file tree
Showing 6 changed files with 121 additions and 22 deletions.
5 changes: 5 additions & 0 deletions History.md
@@ -1,5 +1,10 @@
# Version History

## 1.9.0 / 2018-10-17

* KETTLE-73: Allow censoring of sensitive information which may be present in URL of DataSource
* General dependency updates, particularly to recent Infusion with support for `fluid.loggingEvent`

## 1.8.1 / 2018-10-04

* KETTLE-71: Vagrant build courtesy of waharnum
Expand Down
13 changes: 12 additions & 1 deletion docs/DataSources.md
Expand Up @@ -144,6 +144,17 @@ We document these configuration options in the next section:
nonexistent file, or an HTTP resource giving a 404) will result in a <code>resolve</code> with an empty
payload rather than a <code>reject</code> response.</td>
</tr>
<tr>
<td><code>censorRequestOptionsLog</code></td>
<td><code>Object</code> (map of <code>String</code> to <code>Boolean</code>) (default:
<code>{auth: true, "headers.Authorization": true}</code>)
</td>
<td>A map of paths into the <a href="https://nodejs.org/api/http.html#http_http_request_options_callback">
request options</a> which should be censored from appearing in logs. Any path which maps to <code>true</code>
will not appear either in the logging output derived from the request options parsed from the url
or the url itself.
</td>
</tr>
<tr>
<td><code>components.encoding.type</code></td>
<td><code>String</code> (grade name)</td>
Expand All @@ -153,7 +164,7 @@ We document these configuration options in the next section:
<code>kettle.dataSource.encoding.JSON</code>. Other builtin encodings are
<code>kettle.dataSource.encoding.formenc</code> operating HTML
<a href="http://www.w3.org/TR/html401/interact/forms.html#didx-applicationx-www-form-urlencoded">form
encoding</code> and <code>kettle.dataSource.encoding.none</code> which applies no encoding.
encoding</a> and <code>kettle.dataSource.encoding.none</code> which applies no encoding.
More details in <a href="#using-content-encodings-with-a-datasource">Using Content Encodings with a
DataSource</a>.</td>
</tr>
Expand Down
31 changes: 29 additions & 2 deletions lib/dataSource-url.js
Expand Up @@ -33,6 +33,10 @@ fluid.defaults("kettle.dataSource.URL", {
args: ["{that}", "{arguments}.0", "{arguments}.1"] // options, directModel
}
},
censorRequestOptionsLog: {
auth: true,
"headers.Authorization": true
},
components: {
cookieJar: "{cookieJar}"
},
Expand Down Expand Up @@ -128,7 +132,7 @@ kettle.dataSource.URL.isErrorStatus = function (statusCode) {
* @param {Object} userOptions - An options block that encodes:
* @param {String} userOptions.operation - "set"/"get"
* @param {Boolean} userOptions.notFoundIsEmpty - <code>true</code> if a missing file on read should count as a successful empty payload rather than a rejection
* writeMethod {String}: "PUT"/ "POST" (option - if not provided will be defaulted by the concrete dataSource implementation)
* writeMethod {String}: "PUT"/ "POST" (optional - if not provided will be defaulted by the concrete dataSource implementation)
* @param {Object} directModel - a model holding the coordinates of the data to be read or written
* @param {Object} [model] - [optional] the payload to be written by this write operation
* @return {Promise} a promise for the successful or failed datasource operation
Expand Down Expand Up @@ -202,6 +206,27 @@ kettle.dataSource.URL.errorCallback = function (promise, whileMsg) {
};
};

/** Prepare a URL for logging by censoring sensitive parts of the URL (satisfy KETTLE-73, GPII-3309)
* @param requestOptions {Object} A hash of request options holding a set of parsed URL fields as returned from
* https://nodejs.org/api/url.html#url_url_parse_urlstring_parsequerystring_slashesdenotehost , as well as some others
* including the overall `url`.
* @param toCensor {Object} A hash of member paths within `requestOptions` to `true`, holding those which should be censored
* @return A requestOptions object with the sensitive members removed where they appear at top level as well as where they might occur encoded
* within the `url` member
*/

kettle.dataSource.URL.censorRequestOptions = function (requestOptions, toCensor) {
var togo = fluid.copy(requestOptions);
fluid.each(toCensor, function (troo, key) {
var original = fluid.get(togo, key);
if (original) {
fluid.set(togo, key, "(SENSITIVE)");
}
});
togo.url = urlModule.format(togo);
return togo;
};

/** Central strategy point for all HTTP-backed DataSource operations (both read and write).
* Accumulates options to be sent to the underlying node.js `http.request` primitives, collects and interprets the
* results back into promise resolutions.
Expand Down Expand Up @@ -230,7 +255,9 @@ kettle.dataSource.URL.handle.http = function (that, baseOptions, data) {
}
var requestOptions = fluid.extend(true, defaultOptions, baseOptions);
var whileMsg = " while executing HTTP " + requestOptions.method + " on url " + requestOptions.url;
fluid.log("DataSource Issuing " + (requestOptions.protocol.toUpperCase()).slice(0, -1) + " request with options ", requestOptions);
var loggingOptions = kettle.dataSource.URL.censorRequestOptions(requestOptions, that.options.censorRequestOptionsLog);
fluid.log("DataSource Issuing " + (requestOptions.protocol.toUpperCase()).slice(0, -1) + " request with options ",
loggingOptions);
promise.accumulateRejectionReason = function (originError) {
return kettle.upgradeError(originError, whileMsg);
};
Expand Down
16 changes: 8 additions & 8 deletions package.json
@@ -1,7 +1,7 @@
{
"name": "kettle",
"description": "Declarative IoC-based framework for HTTP and WebSockets servers on node.js based on express and ws",
"version": "1.8.1",
"version": "1.9.0",
"author": {
"name": "The Fluid Project"
},
Expand All @@ -17,27 +17,27 @@
},
"homepage": "http://wiki.fluidproject.org/display/fluid/Kettle",
"dependencies": {
"express": "4.16.3",
"express": "4.16.4",
"body-parser": "1.18.3",
"cookie-parser": "1.4.3",
"express-session": "1.15.6",
"serve-static": "1.13.2",
"ws": "6.0.0",
"infusion": "3.0.0-dev.20180222T160835Z.6e1311a",
"ws": "6.1.2",
"infusion": "3.0.0-dev.20181017T162149Z.787f7d5e5",
"jsonlint": "1.6.3",
"fluid-resolve": "1.3.0",
"path-to-regexp": "1.7.0",
"multer": "1.4.0",
"form-data": "2.3.2",
"multer": "1.4.1",
"form-data": "2.3.3",
"json5": "2.1.0"
},
"devDependencies": {
"eslint-config-fluid": "1.3.0",
"gpii-grunt-lint-all": "1.0.5",
"gpii-express": "1.0.14",
"gpii-express": "1.0.15",
"gpii-pouchdb": "1.0.12",
"grunt": "1.0.3",
"nyc": "13.0.1",
"nyc": "13.1.0",
"grunt-shell": "2.1.0",
"node-jqunit": "1.1.8",
"rimraf": "2.6.2"
Expand Down
73 changes: 64 additions & 9 deletions tests/DataSourceSimpleTests.js
Expand Up @@ -24,11 +24,16 @@ require("./shared/DataSourceTestUtils.js");

kettle.tests.dataSource.ensureDirectoryEmpty("%kettle/tests/data/writeable");

/** KETTLE-34 options resolution and merging test **/

fluid.defaults("kettle.tests.KETTLE34dataSource", {
gradeNames: "kettle.dataSource.URL",
url: "https://user:password@thing.available:997/path",
headers: {
"x-custom-header": "x-custom-value"
},
censorRequestOptionsLog: {
auth: false
}
});

Expand Down Expand Up @@ -75,33 +80,82 @@ kettle.tests.dataSource.resolutionTests = [ {
}
];

kettle.tests.dataSource.resolutionTest = function (fixture) {
jqUnit.test("KETTLE-34 request option resolution test", function () {
var httpRequest = http.request,
capturedOptions;
kettle.tests.capturedHttpOptions = {};

kettle.tests.withMockHttp = function (toapply) {
return function () {
var httpRequest = http.request;
kettle.tests.capturedHttpOptions = {};
http.request = function (requestOptions) {
capturedOptions = requestOptions;
kettle.tests.capturedHttpOptions = requestOptions;
return {
on: fluid.identity,
end: fluid.identity
};
};
try {
var dataSource = fluid.invokeGlobalFunction(fixture.gradeName, [fixture.creatorArgs]);
dataSource.get(null, fixture.directOptions);
jqUnit.assertLeftHand("Resolved expected requestOptions", fixture.expectedOptions, capturedOptions);
toapply();
} finally {
http.request = httpRequest;
}
});
};
};

kettle.tests.dataSource.resolutionTest = function (fixture) {
jqUnit.test("KETTLE-34 request option resolution test", kettle.tests.withMockHttp(function () {
var dataSource = fluid.invokeGlobalFunction(fixture.gradeName, [fixture.creatorArgs]);
dataSource.get(null, fixture.directOptions);
jqUnit.assertLeftHand("Resolved expected requestOptions", fixture.expectedOptions, kettle.tests.capturedHttpOptions);
}));
};

fluid.each(kettle.tests.dataSource.resolutionTests, function (fixture) {
kettle.tests.dataSource.resolutionTest(fixture);
});

/** KETTLE-73 censoring sensitive options test **/

fluid.defaults("kettle.tests.KETTLE73dataSource", {
gradeNames: "kettle.dataSource.URL"
});

kettle.tests.censoringTests = [{
url: "https://secret-user:secret-password@thing.available:997/path",
headers: {
Authorization: "secret-authorization",
"X-unrelated-header": "998"
},
shouldNotApper: "secret",
shouldAppear: ["997", "998"]
}, {
url: "https://secret-user:secret-%25-password@thing.available:997/path",
headers: {
Authorization: "secret-%25-authorization",
"X-unrelated-header": "998"
},
shouldNotAppear: "secret",
shouldAppear: ["997", "998"]
}];

jqUnit.test("KETTLE-73 censoring test", kettle.tests.withMockHttp(function () {
var memoryLog;
var memoryLogger = function (args) {
memoryLog.push(JSON.stringify(args.slice(1)));
};
fluid.loggingEvent.addListener(memoryLogger, "memoryLogger", "before:log");
kettle.tests.censoringTests.forEach(function (fixture) {
memoryLog = [];
var that = kettle.tests.KETTLE73dataSource({url: fixture.url});
that.get(null, {headers: fixture.headers});
jqUnit.assertTrue("Secret information should not have been logged", memoryLog[0].indexOf(fixture.shouldNotAppear) === -1);
fixture.shouldAppear.forEach(function (shouldAppear) {
jqUnit.assertTrue("URL port and header should have been logged", memoryLog[0].indexOf(shouldAppear) !== -1);
});
});
fluid.loggingEvent.removeListener("memoryLogger");
}));

/** KETTLE-38 writeable grade resolution test **/

fluid.defaults("kettle.tests.KETTLE38base", {
gradeNames: "kettle.dataSource.URL",
Expand All @@ -117,6 +171,7 @@ jqUnit.test("KETTLE-38 derived writable dataSource test", function () {
jqUnit.assertValue("Resolved writable grade via base grade", dataSource.set);
});

/** Form encoding test **/

kettle.tests.formencData = {
"text1": "text default",
Expand Down
5 changes: 3 additions & 2 deletions tests/StaticTests.js
Expand Up @@ -57,9 +57,10 @@ fluid.defaults("kettle.tests.middleware.verifyingUnmarked", {

var infusionPackage = fluid.require("%infusion/package.json");

//------------- Test defs for GET, POST, PUT ---------------
/** Test defs for static hosting **/

kettle.tests["static"].testDefs = [{
name: "HTTPMethods GET test",
name: "Static hosting tests",
expect: 9,
config: {
configName: "kettle.tests.static.config",
Expand Down

0 comments on commit 281d9aa

Please sign in to comment.