Skip to content
This repository was archived by the owner on Mar 11, 2022. It is now read-only.

Commit aaa895b

Browse files
committed
Disable cookie authentication if credentials are missing.
1 parent 87b9677 commit aaa895b

File tree

8 files changed

+132
-4
lines changed

8 files changed

+132
-4
lines changed

CHANGES.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
- [FIXED] Ensure IAM API key can be correctly changed.
77
- [FIXED] Callback with an error when a user cannot be authenticated using IAM.
88
- [FIXED] Ensure authorization tokens are not unnecessarily requested.
9+
- [IMPROVED] Do not apply cookie authentication by default in the case that no
10+
credentials are provided.
911
- [IMPROVED] Preemptively renew authorization tokens that are due to expire.
1012

1113
# 4.2.1 (2019-08-29)

cloudant.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,7 @@ function Cloudant(options, callback) {
294294
nano.generate_api_key = generate_api_key; // eslint-disable-line camelcase
295295

296296
if (callback) {
297-
nano.cc._addPlugins('cookieauth');
297+
nano.cc._addPlugins({ cookieauth: { errorOnNoCreds: false } });
298298
nano.ping(function(error, pong) {
299299
if (error) {
300300
callback(error);

lib/client.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ class CloudantClient {
5151
plugins = [ { iamauth: { iamApiKey: self._cfg.creds.iamApiKey } } ];
5252
} else if (typeof this._cfg.plugins === 'undefined') {
5353
// => No plugins specified - Add 'cookieauth' plugin.
54-
plugins = [ 'cookieauth' ];
54+
plugins = [ { cookieauth: { errorOnNoCreds: false } } ];
5555
}
5656

5757
// Add user specified plugins.

lib/clientutils.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,9 @@ var runHooks = function(hookName, r, data, end) {
131131
async.eachSeries(r.plugins, function(plugin, done) {
132132
if (typeof plugin[hookName] !== 'function') {
133133
done(); // no hooks for plugin
134+
} else if (plugin.disabled) {
135+
debug(`Skipping hook ${hookName} for disabled plugin '${plugin.id}'.`);
136+
done();
134137
} else {
135138
debug(`Running hook ${hookName} for plugin '${plugin.id}'.`);
136139
var oldState = Object.assign({}, r.state);

plugins/base.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,23 @@ class BasePlugin {
2929
constructor(client, cfg) {
3030
this._client = client;
3131
this._cfg = cfg;
32+
this._disabled = false;
3233
this._lockFile = tmp.tmpNameSync({ postfix: '.lock' });
3334
}
3435

3536
get id() {
3637
return this.constructor.id;
3738
}
3839

40+
get disabled() {
41+
return this._disabled;
42+
}
43+
44+
// Disable a plugin to prevent all hooks from being executed.
45+
set disabled(disabled) {
46+
this._disabled = disabled;
47+
}
48+
3949
// NOOP Base Hooks
4050

4151
onRequest(state, request, callback) {

plugins/cookieauth.js

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,22 @@ const CookieTokenManager = require('../lib/tokens/CookieTokenManager');
2525
*/
2626
class CookiePlugin extends BasePlugin {
2727
constructor(client, cfg) {
28-
cfg = Object.assign({ autoRenew: true }, cfg);
28+
cfg = Object.assign({
29+
autoRenew: true,
30+
errorOnNoCreds: true
31+
}, cfg);
2932

3033
super(client, cfg);
3134

3235
let sessionUrl = new u.URL(cfg.serverUrl);
3336
sessionUrl.pathname = '/_session';
3437
if (!sessionUrl.username || !sessionUrl.password) {
35-
throw new Error('Credentials are required for cookie authentication.');
38+
if (cfg.errorOnNoCreds) {
39+
throw new Error('Credentials are required for cookie authentication.');
40+
}
41+
debug('Missing credentials for cookie authentication. Permanently disabling plugin.');
42+
this.disabled = true;
43+
return;
3644
}
3745

3846
this._jar = request.jar();

test/client.js

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,77 @@ describe('CloudantClient', function() {
316316
});
317317
});
318318

319+
describe('disables a plugin', function() {
320+
it('skips all plugin hooks on good response', function(done) {
321+
var mocks = nock(SERVER)
322+
.get(DBNAME)
323+
.reply(200, {doc_count: 1});
324+
325+
var cloudantClient = new Client({ plugins: [] });
326+
cloudantClient._addPlugins(testPlugin.NoopPlugin);
327+
assert.equal(cloudantClient._plugins.length, 1);
328+
329+
var options = {
330+
url: SERVER + DBNAME,
331+
auth: { username: ME, password: PASSWORD },
332+
method: 'GET'
333+
};
334+
335+
// disable the plugin
336+
cloudantClient.getPlugin('noop').disabled = true;
337+
338+
cloudantClient.request(options, function(err, resp, data) {
339+
assert.equal(err, null);
340+
assert.equal(resp.statusCode, 200);
341+
assert.ok(data.indexOf('"doc_count":1') > -1);
342+
343+
// assert hooks are _not_ executed
344+
assert.equal(cloudantClient._plugins[0].onRequestCallCount, 0);
345+
assert.equal(cloudantClient._plugins[0].onErrorCallCount, 0);
346+
assert.equal(cloudantClient._plugins[0].onResponseCallCount, 0);
347+
348+
mocks.done();
349+
done();
350+
});
351+
});
352+
353+
it('skips all plugin hooks on error response', function(done) {
354+
if (process.env.NOCK_OFF) {
355+
this.skip();
356+
}
357+
358+
var mocks = nock(SERVER)
359+
.get(DBNAME)
360+
.replyWithError({code: 'ECONNRESET', message: 'socket hang up'});
361+
362+
var cloudantClient = new Client({ plugins: [] });
363+
cloudantClient._addPlugins(testPlugin.NoopPlugin);
364+
assert.equal(cloudantClient._plugins.length, 1);
365+
366+
var options = {
367+
url: SERVER + DBNAME,
368+
auth: { username: ME, password: PASSWORD },
369+
method: 'GET'
370+
};
371+
372+
// disable the plugin
373+
cloudantClient.getPlugin('noop').disabled = true;
374+
375+
cloudantClient.request(options, function(err, resp, data) {
376+
assert.equal(err.code, 'ECONNRESET');
377+
assert.equal(err.message, 'socket hang up');
378+
379+
// assert hooks are _not_ executed
380+
assert.equal(cloudantClient._plugins[0].onRequestCallCount, 0);
381+
assert.equal(cloudantClient._plugins[0].onErrorCallCount, 0);
382+
assert.equal(cloudantClient._plugins[0].onResponseCallCount, 0);
383+
384+
mocks.done();
385+
done();
386+
});
387+
});
388+
});
389+
319390
describe('#db using callbacks', function() {
320391
describe('with no plugins', function() {
321392
it('performs request and returns response', function(done) {

test/plugins/cookieauth.js

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,40 @@ describe('#db CookieAuth Plugin', function() {
151151
});
152152
});
153153

154+
it('disables cookie authentication for missing credentials when using default cookieauth plugin', function(done) {
155+
var mocks = nock(SERVER)
156+
.get(DBNAME)
157+
.reply(401, { error: 'unauthorized' });
158+
159+
// 'cookieauth' plugin is added by default with `errorOnNoCreds: false`
160+
var cloudantClient = new Client({ creds: { outUrl: SERVER } });
161+
var req = { url: SERVER + DBNAME, method: 'GET' };
162+
163+
cloudantClient.request(req, function(err, resp, data) {
164+
assert.equal(err, null);
165+
assert.equal(resp.request.uri.auth, null);
166+
assert.equal(resp.statusCode, 401);
167+
assert.ok(data.indexOf('unauthorized') > -1);
168+
169+
// assert 'cookieauth' plugin has been disabled
170+
assert.ok(cloudantClient.getPlugin('cookieauth').disabled);
171+
172+
mocks.done();
173+
done();
174+
});
175+
});
176+
177+
it('throws error for missing credentials when cookieauth plugin is specified', function() {
178+
assert.throws(
179+
() => {
180+
/* eslint-disable no-new */
181+
new Client({ creds: { outUrl: SERVER }, plugins: COOKIEAUTH_PLUGIN });
182+
},
183+
/Credentials are required for cookie authentication/,
184+
'did not throw with expected message'
185+
);
186+
});
187+
154188
it('performs request and returns 500 response', function(done) {
155189
if (process.env.NOCK_OFF) {
156190
this.skip();

0 commit comments

Comments
 (0)