Skip to content

Commit

Permalink
Replace api_key querystring or body parameter by more secure header
Browse files Browse the repository at this point in the history
  • Loading branch information
Rui Marinho committed Jan 28, 2017
1 parent 355b6e7 commit 26befd7
Show file tree
Hide file tree
Showing 18 changed files with 40 additions and 75 deletions.
14 changes: 4 additions & 10 deletions src/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,16 +55,16 @@ export default class Client {
// Request default options.
this.defaults = {
headers: {
'User-Agent': `${manifest.name}/${manifest.version} (${manifest.homepage})`
'User-Agent': `${manifest.name}/${manifest.version} (${manifest.homepage})`,
'X-Authy-API-Key': this.key
},
json: true,
strictSSL: true,
timeout: this.timeout
};

this.rpc = Promise.promisifyAll(request.defaults(_.defaultsDeep({
baseUrl: `${this.host}/protected/json/`,
qs: { api_key: this.key }
baseUrl: `${this.host}/protected/json/`
}, this.defaults)), { multiArgs: true });

this.onetouch = Promise.promisifyAll(request.defaults(_.defaultsDeep({
Expand Down Expand Up @@ -95,7 +95,6 @@ export default class Client {

return this.onetouch.postAsync({
body: {
api_key: this.key,
details: details.visible,
hidden_details: details.hidden,
logos,
Expand Down Expand Up @@ -196,12 +195,7 @@ export default class Client {
id: [is.required(), is.string()]
});

return this.onetouch.getAsync({
qs: {
api_key: this.key
},
uri: esc`approval_requests/${id}`
})
return this.onetouch.getAsync({ uri: esc`approval_requests/${id}` })
.bind(this)
.then(parseResponse)
.tap(response => {
Expand Down
14 changes: 6 additions & 8 deletions src/logging/request-obfuscator.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,24 +3,22 @@
* Module dependencies.
*/

import { isString } from 'lodash';
import { has } from 'lodash';

/**
* Instances.
*/

const replacement = /(api_key=)([^&])*/;
const key = 'X-Authy-API-Key';

/**
* Export `RequestObfuscator`.
*/

export function obfuscate(request) {
// Obfuscate the API key on `uri`.
request.uri = request.uri.replace(replacement, '$1*****');

// Obfuscate the API key on `body`.
if (isString(request.body)) {
request.body = request.body.replace(replacement, '$1*****');
if (!has(request, `headers.${key}`)) {
return request;
}

request.headers[key] = '*****';
}
2 changes: 0 additions & 2 deletions test/client_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -696,7 +696,6 @@ describe('Client', () => {
mocks.verifyPhone.succeed({
request: {
query: {
api_key: '{key}',
country_code: '1',
phone_number: '7754615609',
verification_code: '1234'
Expand Down Expand Up @@ -1086,7 +1085,6 @@ describe('Client', () => {
mocks.createApprovalRequest.succeed({
request: {
body: {
api_key: client.key,
details: {
'Account Number': '981266321',
location: 'California, USA',
Expand Down
24 changes: 9 additions & 15 deletions test/logging/request-obfuscator_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,29 +12,23 @@ import { obfuscate } from '../../src/logging/request-obfuscator';

describe('RequestObfuscator', () => {
describe('obfuscate()', () => {
it('should strip credentials from query string', () => {
const request = {
id: '354f8341-eb27-4c91-a8f7-a30e303a0976',
method: 'GET',
uri: 'foo=bar&api_key=foobar'
};

obfuscate(request);
it('should ignore a request that does not include headers', () => {
// A request object with type `response` won't have headers.
const request = {};

request.should.eql(defaults({ uri: 'foo=bar&api_key=*****' }, request));
obfuscate(request).should.equal(request);
});

it('should strip credentials from request `body`', () => {
it('should strip credentials from header', () => {
const request = {
body: 'foo=bar&api_key=foobar',
id: '354f8341-eb27-4c91-a8f7-a30e303a0976',
method: 'GET',
uri: 'foo=bar&api_key=foobar'
headers: {
'X-Authy-API-Key': 'foobar'
}
};

obfuscate(request);

request.should.eql(defaults({ body: 'foo=bar&api_key=*****' }, request));
request.should.eql(defaults({ headers: { 'X-Authy-API-Key': '*****' } }, request));
});
});
});
1 change: 0 additions & 1 deletion test/mocks/create-approval-request-mock.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import uuid from '../utils/uuid';
function mock({ request = {}, response = {} }) {
return nock(/\.authy\.com/)
.filteringPath(path => path.replace(/\d+/, '{authyId}'))
.filteringRequestBody(body => body.replace(/key=.*?(&|$)/, 'key={key}$1'))
.post('/onetouch/json/users/{authyId}/approval_requests', request.body)
.reply(response.code, response.body);
}
Expand Down
4 changes: 2 additions & 2 deletions test/mocks/delete-user-mock.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ import nock from 'nock';

function mock({ request = {}, response = {} }) {
return nock(/\.authy\.com/)
.filteringPath(path => path.replace(/\=[^&].+/, '={key}').replace(/\/[0-9].*\//, '/{authyId}/'))
.post('/protected/json/users/{authyId}/delete?api_key={key}', request.body)
.filteringPath(path => path.replace(/\/[0-9].*\//, '/{authyId}/'))
.post('/protected/json/users/{authyId}/delete', request.body)
.reply(response.code, response.body);
}

Expand Down
4 changes: 1 addition & 3 deletions test/mocks/get-application-details-mock.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
* Module dependencies.
*/

import { defaults } from 'lodash';
import nock from 'nock';

/**
Expand All @@ -12,9 +11,8 @@ import nock from 'nock';

function mock({ request = {}, response = {} }) {
return nock(/\.authy\.com/)
.filteringPath(path => path.replace(/key=.*?(&|$)/, 'key={key}$1'))
.get('/protected/json/app/details')
.query(request.query ? defaults({ api_key: '{key}' }, request.query) : true)
.query(request.query ? request.query : true)
.reply(response.code, response.body);
}

Expand Down
5 changes: 2 additions & 3 deletions test/mocks/get-application-statistics-mock.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/

import nock from 'nock';
import { defaults, random, times } from 'lodash';
import { random, times } from 'lodash';

/**
* List of month names.
Expand All @@ -18,9 +18,8 @@ const months = ['January', 'February', 'March', 'April', 'May', 'June', 'July',

function mock({ request = {}, response = {} }) {
return nock(/\.authy\.com/)
.filteringPath(path => path.replace(/key=.*?(&|$)/, 'key={key}$1'))
.get('/protected/json/app/stats')
.query(request.query ? defaults({ api_key: '{key}' }, request.query) : true)
.query(request.query ? request.query : true)
.reply(response.code, response.body);
}

Expand Down
5 changes: 2 additions & 3 deletions test/mocks/get-approval-request-mock.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
* Module dependencies.
*/

import { defaults } from 'lodash';
import nock from 'nock';
import uuid from '../utils/uuid';

Expand All @@ -13,9 +12,9 @@ import uuid from '../utils/uuid';

function mock({ request = {}, response = {} }) {
return nock(/\.authy\.com/)
.filteringPath(path => path.replace(/key=.*?(&|$)/, 'key={key}$1').replace(/[\w]{8}(-[\w]{4}){3}-[\w]{12}/, '{id}'))
.filteringPath(path => path.replace(/[\w]{8}(-[\w]{4}){3}-[\w]{12}/, '{id}'))
.get('/onetouch/json/approval_requests/{id}')
.query(request.query ? defaults({ api_key: '{key}' }, request.query) : true)
.query(request.query ? request.query : true)
.reply(response.code, response.body);
}

Expand Down
4 changes: 1 addition & 3 deletions test/mocks/get-phone-information-mock.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
* Module dependencies.
*/

import { defaults } from 'lodash';
import nock from 'nock';

/**
Expand All @@ -12,9 +11,8 @@ import nock from 'nock';

function mock({ request = {}, response = {} }) {
return nock(/\.authy\.com/)
.filteringPath(path => path.replace(/key=.*?(&|$)/, 'key={key}$1'))
.get('/protected/json/phones/info')
.query(request.query ? defaults({ api_key: '{key}' }, request.query) : true)
.query(request.query ? request.query : true)
.reply(response.code, response.body);
}

Expand Down
5 changes: 2 additions & 3 deletions test/mocks/get-user-status-mock.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
* Module dependencies.
*/

import { defaults } from 'lodash';
import nock from 'nock';

/**
Expand All @@ -12,9 +11,9 @@ import nock from 'nock';

function mock({ request = {}, response = {} }) {
return nock(/\.authy\.com/)
.filteringPath(path => path.replace(/\=[^&].+/, '={key}').replace(/\/[0-9].*\//, '/{authyId}/'))
.filteringPath(path => path.replace(/\/[0-9].*\//, '/{authyId}/'))
.get('/protected/json/users/{authyId}/status', request.body)
.query(request.query ? defaults({ api_key: '{key}' }, request.query) : true)
.query(request.query ? request.query : true)
.reply(response.code, response.body);
}

Expand Down
5 changes: 2 additions & 3 deletions test/mocks/register-activity-mock.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
* Module dependencies.
*/

import { defaults } from 'lodash';
import nock from 'nock';

/**
Expand All @@ -12,9 +11,9 @@ import nock from 'nock';

function mock({ request = {}, response = {} }) {
return nock(/\.authy\.com/)
.filteringPath(path => path.replace(/\=[^&].+/, '={key}').replace(/\/[0-9].*\//, '/{authyId}/'))
.filteringPath(path => path.replace(/\/[0-9].*\//, '/{authyId}/'))
.post('/protected/json/users/{authyId}/register_activity', request.body)
.query(request.query ? defaults({ api_key: '{key}' }, request.query) : true)
.query(request.query ? request.query : true)
.reply(response.code, response.body);
}

Expand Down
4 changes: 1 addition & 3 deletions test/mocks/register-user-mock.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
* Module dependencies.
*/

import { defaults } from 'lodash';
import nock from 'nock';

/**
Expand All @@ -12,9 +11,8 @@ import nock from 'nock';

function mock({ request = {}, response = {} }) {
return nock(/\.authy\.com/)
.filteringPath(path => path.replace(/\=[^&].+/, '={key}'))
.post('/protected/json/users/new', request.body)
.query(request.query ? defaults({ api_key: '{key}' }, request.query) : true)
.query(request.query ? request.query : true)
.reply(response.code, response.body);
}

Expand Down
5 changes: 2 additions & 3 deletions test/mocks/request-call-mock.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
* Module dependencies.
*/

import { defaults } from 'lodash';
import nock from 'nock';

/**
Expand All @@ -12,9 +11,9 @@ import nock from 'nock';

function mock({ request = {}, response = {} }) {
return nock(/\.authy\.com/)
.filteringPath(path => path.replace(/key=.*?(&|$)/, 'key={key}$1').replace(/\/[0-9].*\?/, '/{authyId}?'))
.filteringPath(path => path.replace(/\/[0-9].*/, '/{authyId}'))
.get('/protected/json/call/{authyId}')
.query(request.query ? defaults({ api_key: '{key}' }, request.query) : true)
.query(request.query ? request.query : true)
.reply(response.code, response.body);
}

Expand Down
5 changes: 2 additions & 3 deletions test/mocks/request-sms-mock.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
* Module dependencies.
*/

import { defaults } from 'lodash';
import nock from 'nock';

/**
Expand All @@ -12,9 +11,9 @@ import nock from 'nock';

function mock({ request = {}, response = {} }) {
return nock(/\.authy\.com/)
.filteringPath(path => path.replace(/key=.*?(&|$)/, 'key={key}$1').replace(/\/[0-9].*\?/, '/{authyId}?'))
.filteringPath(path => path.replace(/\/[0-9].*/, '/{authyId}'))
.get('/protected/json/sms/{authyId}')
.query(request.query ? defaults({ api_key: '{key}' }, request.query) : true)
.query(request.query ? request.query : true)
.reply(response.code, response.body);
}

Expand Down
4 changes: 1 addition & 3 deletions test/mocks/start-phone-verification-mock.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
* Module dependencies.
*/

import { defaults } from 'lodash';
import nock from 'nock';

/**
Expand All @@ -12,9 +11,8 @@ import nock from 'nock';

function mock({ request = {}, response = {} }) {
return nock(/\.authy\.com/)
.filteringPath(path => path.replace(/\=[^&].+/, '={key}'))
.post('/protected/json/phones/verification/start', request.body)
.query(request.query ? defaults({ api_key: '{key}' }, request.query) : true)
.query(request.query ? request.query : true)
.reply(response.code, response.body);
}

Expand Down
5 changes: 1 addition & 4 deletions test/mocks/verify-phone-mock.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
* Module dependencies.
*/

import { defaults } from 'lodash';
import nock from 'nock';

/**
Expand All @@ -13,16 +12,14 @@ import nock from 'nock';
function mock({ request = {}, response = {} }) {
return nock(/\.authy\.com/)
.filteringPath(path => {
path = path.replace(/key=.*?(&|$)/, 'key={key}$1');

if (!(request.query && request.query.verification_code)) {
return path.replace(/verification_code=.*?(&|$)/, 'verification_code={token}$1');
}

return path;
})
.get('/protected/json/phones/verification/check')
.query(request.query ? defaults({ api_key: '{key}' }, request.query) : true)
.query(request.query ? request.query : true)
.reply(response.code, response.body);
}

Expand Down
5 changes: 2 additions & 3 deletions test/mocks/verify-token-mock.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
* Module dependencies.
*/

import { defaults } from 'lodash';
import nock from 'nock';

/**
Expand All @@ -12,9 +11,9 @@ import nock from 'nock';

function mock({ request = {}, response = {} }) {
return nock(/\.authy\.com/)
.filteringPath(path => path.replace(/key=.*?(&|$)/, 'key={key}$1').replace(/verify\/.*?\//, 'verify/{token}/').replace(/\/[0-9].*\?/, '/{authyId}?'))
.filteringPath(path => path.replace(/verify\/.*?\//, 'verify/{token}/').replace(/\/[0-9].*/, '/{authyId}'))
.get('/protected/json/verify/{token}/{authyId}')
.query(request.query ? defaults({ api_key: '{key}' }, request.query) : true)
.query(request.query ? request.query : true)
.reply(response.code, response.body);
}

Expand Down

0 comments on commit 26befd7

Please sign in to comment.