Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate current window origin and redirecturi origin to prevent mismatches #615

Merged
merged 3 commits into from
Jan 16, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion src/helper/object.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

var assert = require('./assert');
var objectAssign = require('./object-assign');
var windowHelper = require('./window');

function pick(object, keys) {
return keys.reduce(function(prev, key) {
Expand Down Expand Up @@ -117,12 +118,23 @@ function toCamelCase(object, exceptions) {
}, {});
}

function getOriginFromUrl(url) {
if (!url) {
return undefined;
}
var doc = windowHelper.getDocument();
var anchor = doc.createElement('a');
anchor.href = url;
return anchor.origin;
}

module.exports = {
toSnakeCase: toSnakeCase,
toCamelCase: toCamelCase,
blacklist: blacklist,
merge: merge,
pick: pick,
getKeysNotIn: getKeysNotIn,
extend: extend
extend: extend,
getOriginFromUrl: getOriginFromUrl
};
13 changes: 12 additions & 1 deletion src/helper/window.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,19 @@ function getWindow() {
return global.window;
}

function getOrigin() {
var location = global.window.location;
var origin = location.origin;
if (!origin) {
origin =
location.protocol + '//' + location.hostname + (location.port ? ':' + location.port : '');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the new line?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prettier auto formatting.

}
return origin;
}

module.exports = {
redirect: redirect,
getDocument: getDocument,
getWindow: getWindow
getWindow: getWindow,
getOrigin: getOrigin
};
3 changes: 2 additions & 1 deletion src/web-auth/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,7 @@ WebAuth.prototype.checkSession = function(options, cb) {
.merge(this.baseOptions, [
'clientID',
'responseType',
'redirectUri',
'scope',
'audience',
'_csrf',
Expand All @@ -387,7 +388,7 @@ WebAuth.prototype.checkSession = function(options, cb) {
.with(options);

if (params.responseType === 'code') {
return cb(new Error("responseType can't be `code`"));
return cb({ error: 'error', error_description: "responseType can't be `code`" });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why has this changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it was wrong before. you couldn't catch the error because it was not using the standard error formatting {error, error_description}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok good was wondering that

}

if (!options.nonce) {
Expand Down
21 changes: 19 additions & 2 deletions src/web-auth/web-message-handler.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
var IframeHandler = require('../helper/iframe-handler');
var objectHelper = require('../helper/object');
var windowHelper = require('../helper/window');

function runWebMessageFlow(authorizeUrl, options, callback) {
var handler = new IframeHandler({
Expand All @@ -11,8 +12,10 @@ function runWebMessageFlow(authorizeUrl, options, callback) {
timeout: options.timeout,
eventValidator: {
isValid: function(eventData) {
return eventData.event.data.type === 'authorization_response'
&& options.state === eventData.event.data.response.state;
return (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also prettier

eventData.event.data.type === 'authorization_response' &&
options.state === eventData.event.data.response.state
);
}
},
timeoutCallback: function() {
Expand All @@ -33,6 +36,20 @@ WebMessageHandler.prototype.run = function(options, cb) {
var _this = this;
options.responseMode = 'web_message';
options.prompt = 'none';

var currentOrigin = windowHelper.getOrigin();
var redirectUriOrigin = objectHelper.getOriginFromUrl(options.redirectUri);
if (currentOrigin !== redirectUriOrigin) {
return cb({
error: 'origin_mismatch',
error_description: "The redirectUri's origin (" +
redirectUriOrigin +
") should match the window's origin (" +
currentOrigin +
').'
});
}

runWebMessageFlow(this.webAuth.client.buildAuthorizeUrl(options), options, function(
err,
eventData
Expand Down
25 changes: 25 additions & 0 deletions test/helper/object.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ var stub = require('sinon').stub;

var objectAssign = require('../../src/helper/object-assign');
var objectHelper = require('../../src/helper/object');
var windowHelper = require('../../src/helper/window');

describe('helpers', function() {
describe('getKeysNotIn', function() {
Expand Down Expand Up @@ -515,4 +516,28 @@ describe('helpers', function() {
});
});
});
describe('getOriginFromUrl', function() {
it('should return undefined if there is no url', function() {
expect(objectHelper.getOriginFromUrl()).to.be(undefined);
expect(objectHelper.getOriginFromUrl('')).to.be(undefined);
expect(objectHelper.getOriginFromUrl(null)).to.be(undefined);
});
it('should use an anchor to parse the url and return the origin', function() {
var anchor = {
origin: 'https://test.com'
};
stub(windowHelper, 'getDocument', function() {
return {
createElement: function createElement(e) {
expect(e).to.be('a');
return anchor;
}
};
});
var url = 'https://test.com/example';
expect(objectHelper.getOriginFromUrl(url)).to.be('https://test.com');
expect(anchor.href).to.be(url);
windowHelper.getDocument.restore();
});
});
});
10 changes: 10 additions & 0 deletions test/helper/window.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,14 @@ describe('helpers window', function() {
var _window = windowHelper.getWindow();
expect(_window).to.eql({ document: { body: {} }, location: '' });
});
describe('getOrigin', function() {
it('should use window.location.origin when available', function() {
global.window = { location: { origin: 'origin' } };
expect(windowHelper.getOrigin()).to.be('origin');
});
it('should build current origin when location.origin is not available', function() {
global.window = { location: { protocol: 'http:', hostname: 'hostname', port: 30 } };
expect(windowHelper.getOrigin()).to.be('http://hostname:30');
});
});
});
26 changes: 25 additions & 1 deletion test/web-auth/web-auth.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1623,6 +1623,12 @@ describe('auth0.WebAuth', function() {
stub(TransactionManager.prototype, 'process', function(params) {
return Object.assign({}, params, { from: 'transaction-manager' });
});
stub(windowHelper, 'getOrigin', function() {
return 'https://test-origin.com';
});
stub(objectHelper, 'getOriginFromUrl', function() {
return 'https://test-origin.com';
});
});
afterEach(function() {
TransactionManager.prototype.process.restore();
Expand All @@ -1632,10 +1638,27 @@ describe('auth0.WebAuth', function() {
if (WebAuth.prototype.validateAuthenticationResponse.restore) {
WebAuth.prototype.validateAuthenticationResponse.restore();
}
windowHelper.getOrigin.restore();
objectHelper.getOriginFromUrl.restore();
});
it('throws an error if responseType is code', function() {
this.auth0.checkSession({ responseType: 'code' }, function(err) {
expect(err.message).to.be.eql("responseType can't be `code`");
expect(err).to.be.eql({
error: 'error',
error_description: "responseType can't be `code`"
});
});
});
it('throws an error if there is an origin mismatch between current window and redirectUrl', function() {
objectHelper.getOriginFromUrl.restore();
stub(objectHelper, 'getOriginFromUrl', function() {
return 'some-other-origin';
});
this.auth0.checkSession({}, function(err) {
expect(err).to.be.eql({
error: 'origin_mismatch',
error_description: "The redirectUri's origin (some-other-origin) should match the window's origin (https://test-origin.com)."
});
});
});
it('inits IframeHandler with correct params', function(done) {
Expand Down Expand Up @@ -1714,6 +1737,7 @@ describe('auth0.WebAuth', function() {
expect(options).to.be.eql({
clientID: '...',
responseType: 'token',
redirectUri: 'http://page.com/callback',
from: 'transaction-manager',
responseMode: 'web_message',
prompt: 'none'
Expand Down