diff --git a/resources/static/dialog/controllers/actions.js b/resources/static/dialog/controllers/actions.js index 322749710534ba216c7ef8fd509c647e84260256..abef40d6f2678c4cf7f014e8c82eeda7be06e84c 100644 --- a/resources/static/dialog/controllers/actions.js +++ b/resources/static/dialog/controllers/actions.js @@ -56,20 +56,6 @@ BrowserID.Modules.Actions = (function() { if(data.ready) _.defer(data.ready); }, - /** - * Show an error message - * @method doError - * @param {string} [template] - template to use, if not given, use "error" - * @param {object} [info] - info to send to template - */ - doError: function(template, info) { - if(!info) { - info = template; - template = "error"; - } - this.renderError(template, info); - }, - doCancel: function() { if(onsuccess) onsuccess(null); }, diff --git a/resources/static/dialog/controllers/dialog.js b/resources/static/dialog/controllers/dialog.js index 8fbc31ce67dee8db48bbac55611a4a4a08ab03b0..25236e5db1f815516fbe76db3d6d2b5d61e781fc 100644 --- a/resources/static/dialog/controllers/dialog.js +++ b/resources/static/dialog/controllers/dialog.js @@ -13,6 +13,7 @@ BrowserID.Modules.Dialog = (function() { errors = bid.Errors, dom = bid.DOM, win = window, + startExternalDependencies = true, channel, sc; @@ -83,7 +84,7 @@ BrowserID.Modules.Dialog = (function() { function fixupURL(origin, url) { var u; - if (/^http/.test(url)) u = URLParse(url); + if (/^http(s)?:\/\//.test(url)) u = URLParse(url); else if (/^\//.test(url)) u = URLParse(origin + url); else throw "relative urls not allowed: (" + url + ")"; // encodeURI limits our return value to [a-z0-9:/?%], excluding <script> @@ -98,8 +99,23 @@ BrowserID.Modules.Dialog = (function() { win = options.window || window; + // startExternalDependencies is used in unit testing and can only be set + // by the creator/starter of this module. If startExternalDependencies + // is set to false, the channel, state machine, and actions controller + // are not started. These dependencies can interfere with the ability to + // unit test this module because they can throw exceptions and show error + // messages. + startExternalDependencies = true; + if (typeof options.startExternalDependencies === "boolean") { + startExternalDependencies = options.startExternalDependencies; + } + sc.start.call(self, options); - startChannel.call(self); + + if (startExternalDependencies) { + startChannel.call(self); + } + options.ready && _.defer(options.ready); }, @@ -118,8 +134,11 @@ BrowserID.Modules.Dialog = (function() { setOrigin(origin_url); - var actions = startActions.call(self, success, error); - startStateMachine.call(self, actions); + + if (startExternalDependencies) { + var actions = startActions.call(self, success, error); + startStateMachine.call(self, actions); + } // Security Note: paramsFromRP is the output of a JSON.parse on an // RP-controlled string. Most of these fields are expected to be simple @@ -132,9 +151,9 @@ BrowserID.Modules.Dialog = (function() { // verify params try { - if (typeof(paramsFromRP.requiredEmail) !== "undefined") { + if (paramsFromRP.requiredEmail) { if (!bid.verifyEmail(paramsFromRP.requiredEmail)) - throw "invalid requiredEmail: ("+paramsFromRP.requiredEmail+")"; + throw "invalid requiredEmail: (" + paramsFromRP.requiredEmail + ")"; params.requiredEmail = paramsFromRP.requiredEmail; } if (paramsFromRP.tosURL && paramsFromRP.privacyURL) { @@ -144,12 +163,14 @@ BrowserID.Modules.Dialog = (function() { } catch(e) { // note: renderError accepts HTML and cheerfully injects it into a // frame with a powerful origin. So convert 'e' first. - return self.renderError("error", { + self.renderError("error", { action: { title: "error in " + _.escape(origin_url), message: "improper usage of API: " + _.escape(e) } }); + + return e; } // after this point, "params" can be relied upon to contain safe data diff --git a/resources/static/test/cases/controllers/actions.js b/resources/static/test/cases/controllers/actions.js index 11c58cd960ce705d77743c6f011ba3b466e0e1a1..8d90d69f88dfd2bcd2b72865732ff267e1593e3a 100644 --- a/resources/static/test/cases/controllers/actions.js +++ b/resources/static/test/cases/controllers/actions.js @@ -47,30 +47,6 @@ } }); - asyncTest("doError with no template - display default error screen", function() { - createController({ - ready: function() { - equal(testHelpers.errorVisible(), false, "Error is not yet visible"); - controller.doError({}); - ok(testHelpers.errorVisible(), "Error is visible"); - equal($("#defaultError").length, 1, "default error screen is shown"); - start(); - } - }); - }); - - asyncTest("doError with with template - display error screen", function() { - createController({ - ready: function() { - equal(testHelpers.errorVisible(), false, "Error is not yet visible"); - controller.doError("invalid_required_email", {email: "email"}); - equal($("#invalidRequiredEmail").length, 1, "default error screen is shown"); - ok(testHelpers.errorVisible(), "Error is visible"); - start(); - } - }); - }); - asyncTest("doProvisionPrimaryUser - start the provision_primary_user service", function() { testActionStartsModule("doProvisionPrimaryUser", {email: TEST_EMAIL}, "provision_primary_user"); diff --git a/resources/static/test/cases/controllers/dialog.js b/resources/static/test/cases/controllers/dialog.js index ee944e7d4e4b471795188f44523add65d394d051..58fd614062b66e2b16f39665cab9486cd6d235f1 100644 --- a/resources/static/test/cases/controllers/dialog.js +++ b/resources/static/test/cases/controllers/dialog.js @@ -11,15 +11,18 @@ network = bid.Network, mediator = bid.Mediator, testHelpers = bid.TestHelpers, + testErrorVisible = testHelpers.testErrorVisible, + testErrorNotVisible = testHelpers.testErrorNotVisible, + screens = bid.Screens, xhr = bid.Mocks.xhr, + HTTP_TEST_DOMAIN = "http://testdomain.org", + HTTPS_TEST_DOMAIN = "https://testdomain.org", + TESTEMAIL = "testuser@testuser.com", controller, el, winMock, navMock; - function reset() { - } - function WinMock() { this.location.hash = "#1234"; } @@ -49,60 +52,59 @@ }; function createController(config) { - var config = $.extend({ - window: winMock + // startExternalDependencies defaults to true, for most of our tests we + // want to turn this off to prevent the state machine, channel, and actions + // controller from starting up and throwing errors. This allows us to test + // dialog as an individual unit. + var options = $.extend({ + window: winMock, + startExternalDependencies: false, }, config); controller = BrowserID.Modules.Dialog.create(); - controller.start(config); + controller.start(options); } module("controllers/dialog", { setup: function() { winMock = new WinMock(); - reset(); testHelpers.setup(); }, teardown: function() { controller.destroy(); - reset(); testHelpers.teardown(); } }); - function checkNetworkError() { - ok($("#error .contents").text().length, "contents have been written"); - ok($("#error #action").text().length, "action contents have been written"); - ok($("#error #network").text().length, "network contents have been written"); - } - asyncTest("initialization with channel error", function() { // Set the hash so that the channel cannot be found. winMock.location.hash = "#1235"; createController({ + startExternalDependencies: true, ready: function() { - ok($("#error .contents").text().length, "contents have been written"); + testErrorVisible(); start(); } }); }); asyncTest("initialization with add-on navigator.id.channel", function() { - var ok_p = false; + var registerControllerCalled = false; // expect registerController to be called. winMock.navigator.id = { channel : { registerController: function(controller) { - ok_p = controller.getVerifiedEmail && controller.get; + registerControllerCalled = controller.getVerifiedEmail && controller.get; } } }; createController({ + startExternalDependencies: true, ready: function() { - ok(ok_p, "registerController was not called with proper controller"); + ok(registerControllerCalled, "registerController was not called with proper controller"); start(); } }); @@ -113,7 +115,7 @@ createController({ ready: function() { - ok($("#error .contents").text().length == 0, "no error should be reported"); + testErrorNotVisible(); start(); } }); @@ -125,7 +127,7 @@ createController({ ready: function() { - ok($("#error .contents").text().length == 0, "no error should be reported"); + testErrorNotVisible(); start(); } }); @@ -138,7 +140,7 @@ ready: function() { mediator.subscribe("start", function(msg, info) { equal(info.type, "primary", "correct type"); - equal(info.email, "testuser@testuser.com", "email_chosen with correct email"); + equal(info.email, TESTEMAIL, "email_chosen with correct email"); equal(info.add, false, "add is not specified with CREATE_EMAIL option"); start(); }); @@ -161,7 +163,7 @@ ready: function() { mediator.subscribe("start", function(msg, info) { equal(info.type, "primary", "correct type"); - equal(info.email, "testuser@testuser.com", "email_chosen with correct email"); + equal(info.email, TESTEMAIL, "email_chosen with correct email"); equal(info.add, true, "add is specified with ADD_EMAIL option"); start(); }); @@ -179,7 +181,6 @@ asyncTest("onWindowUnload", function() { createController({ - requiredEmail: "registered@testuser.com", ready: function() { var error; @@ -196,6 +197,263 @@ }); }); + asyncTest("get with invalid requiredEmail - print error screen", function() { + createController({ + ready: function() { + mediator.subscribe("start", function(msg, info) { + ok(false, "start should not have been called"); + }); + + var retval = controller.get(HTTP_TEST_DOMAIN, { + requiredEmail: "bademail" + }); + equal(retval, "invalid requiredEmail: (bademail)", "expected error"); + testErrorVisible(); + start(); + } + }); + }); + + asyncTest("get with script containing requiredEmail - print error screen", function() { + createController({ + ready: function() { + mediator.subscribe("start", function(msg, info) { + ok(false, "start should not have been called"); + }); + + var retval = controller.get(HTTP_TEST_DOMAIN, { + requiredEmail: "<script>window.scriptRun=true;</script>testuser@testuser.com" + }); + + // If requiredEmail is not properly escaped, scriptRun will be true. + equal(typeof window.scriptRun, "undefined", "script was not run"); + equal(retval, "invalid requiredEmail: (<script>window.scriptRun=true;</script>testuser@testuser.com)", "expected error"); + testErrorVisible(); + start(); + } + }); + }); + + asyncTest("get with valid requiredEmail - go to start", function() { + createController({ + ready: function() { + var startCalled = false; + mediator.subscribe("start", function(msg, info) { + startCalled = true; + }); + + var retval = controller.get(HTTP_TEST_DOMAIN, { + requiredEmail: TESTEMAIL + }); + + equal(typeof retval, "undefined", "no error expected"); + equal(startCalled, true, "start has been called"); + start(); + } + }); + }); + + asyncTest("get with relative tosURL & valid privacyURL - print error screen", function() { + createController({ + ready: function() { + mediator.subscribe("start", function(msg, info) { + ok(false, "start should not have been called"); + }); + + var retval = controller.get(HTTP_TEST_DOMAIN, { + tosURL: "relative.html", + privacyURL: "/privacy.html" + }); + equal(retval, "relative urls not allowed: (relative.html)", "expected error"); + testErrorVisible(); + start(); + } + }); + }); + + asyncTest("get with script containing tosURL - print error screen", function() { + createController({ + ready: function() { + mediator.subscribe("start", function(msg, info) { + ok(false, "start should not have been called"); + }); + + var retval = controller.get(HTTP_TEST_DOMAIN, { + tosURL: "relative.html<script>window.scriptRun=true;</script>", + privacyURL: "/privacy.html" + }); + + // If tosURL is not properly escaped, scriptRun will be true. + equal(typeof window.scriptRun, "undefined", "script was not run"); + equal(retval, "relative urls not allowed: (relative.html<script>window.scriptRun=true;</script>)", "expected error"); + testErrorVisible(); + start(); + } + }); + }); + + asyncTest("get with valid tosURL & relative privacyURL - print error screen", function() { + createController({ + ready: function() { + mediator.subscribe("start", function(msg, info) { + ok(false, "start should not have been called"); + }); + + var retval = controller.get(HTTP_TEST_DOMAIN, { + tosURL: "/tos.html", + privacyURL: "relative.html" + }); + equal(retval, "relative urls not allowed: (relative.html)", "expected error"); + testErrorVisible(); + start(); + } + }); + }); + + asyncTest("get with script containing privacyURL - print error screen", function() { + createController({ + ready: function() { + mediator.subscribe("start", function(msg, info) { + ok(false, "start should not have been called"); + }); + + var retval = controller.get(HTTP_TEST_DOMAIN, { + tosURL: "/tos.html", + privacyURL: "relative.html<script>window.scriptRun=true;</script>" + }); + + // If privacyURL is not properly escaped, scriptRun will be true. + equal(typeof window.scriptRun, "undefined", "script was not run"); + equal(retval, "relative urls not allowed: (relative.html<script>window.scriptRun=true;</script>)", "expected error"); + testErrorVisible(); + start(); + } + }); + }); + + asyncTest("get with privacyURL - print error screen", function() { + createController({ + ready: function() { + mediator.subscribe("start", function(msg, info) { + ok(false, "start should not have been called"); + }); + + var retval = controller.get(HTTP_TEST_DOMAIN, { + tosURL: "/tos.html", + privacyURL: "relative.html<script>window.scriptRun=true;</script>" + }); + + // If privacyURL is not properly escaped, scriptRun will be true. + equal(typeof window.scriptRun, "undefined", "script was not run"); + equal(retval, "relative urls not allowed: (relative.html<script>window.scriptRun=true;</script>)", "expected error"); + testErrorVisible(); + start(); + } + }); + }); + + asyncTest("get with javascript protocol for privacyURL - print error screen", function() { + createController({ + ready: function() { + mediator.subscribe("start", function(msg, info) { + ok(false, "start should not have been called"); + }); + + var retval = controller.get(HTTP_TEST_DOMAIN, { + tosURL: "/tos.html", + privacyURL: "javascript:alert(1)" + }); + + equal(retval, "relative urls not allowed: (javascript:alert(1))", "expected error"); + testErrorVisible(); + start(); + } + }); + }); + + asyncTest("get with invalid httpg protocol for privacyURL - print error screen", function() { + createController({ + ready: function() { + mediator.subscribe("start", function(msg, info) { + ok(false, "start should not have been called"); + }); + + var retval = controller.get(HTTP_TEST_DOMAIN, { + tosURL: "/tos.html", + privacyURL: "httpg://testdomain.com/privacy.html" + }); + + equal(retval, "relative urls not allowed: (httpg://testdomain.com/privacy.html)", "expected error"); + testErrorVisible(); + start(); + } + }); + }); + + + asyncTest("get with valid absolute tosURL & privacyURL - go to start", function() { + createController({ + ready: function() { + var startCalled = false; + mediator.subscribe("start", function(msg, info) { + startCalled = true; + }); + + var retval = controller.get(HTTP_TEST_DOMAIN, { + tosURL: "/tos.html", + privacyURL: "/privacy.html" + }); + + equal(typeof retval, "undefined", "no error expected"); + equal(startCalled, true, "start has been called"); + testErrorNotVisible(); + start(); + } + }); + }); + + asyncTest("get with valid fully qualified http tosURL & privacyURL - go to start", function() { + createController({ + ready: function() { + var startCalled = false; + mediator.subscribe("start", function(msg, info) { + startCalled = true; + }); + + var retval = controller.get(HTTP_TEST_DOMAIN, { + tosURL: HTTP_TEST_DOMAIN + "/tos.html", + privacyURL: HTTP_TEST_DOMAIN + "/privacy.html" + }); + + equal(typeof retval, "undefined", "no error expected"); + equal(startCalled, true, "start has been called"); + testErrorNotVisible(); + start(); + } + }); + }); + + + asyncTest("get with valid fully qualified https tosURL & privacyURL - go to start", function() { + createController({ + ready: function() { + var startCalled = false; + mediator.subscribe("start", function(msg, info) { + startCalled = true; + }); + + var retval = controller.get(HTTP_TEST_DOMAIN, { + tosURL: HTTPS_TEST_DOMAIN + "/tos.html", + privacyURL: HTTPS_TEST_DOMAIN + "/privacy.html" + }); + + equal(typeof retval, "undefined", "no error expected"); + equal(startCalled, true, "start has been called"); + testErrorNotVisible(); + start(); + } + }); + }); }()); diff --git a/resources/static/test/cases/resources/state.js b/resources/static/test/cases/resources/state.js index a8d859f1953e82c540e3723183dbc00dcd168b77..16b634350c30c0e48c836aaa93640b9c742afb63 100644 --- a/resources/static/test/cases/resources/state.js +++ b/resources/static/test/cases/resources/state.js @@ -376,28 +376,6 @@ equal(actions.called.doCheckAuth, true, "checking auth on start"); }); - test("start with invalid requiredEmail - print error screen", function() { - mediator.publish("start", { - requiredEmail: "bademail" - }); - - equal(actions.called.doError, true, "error screen is shown"); - }); - - test("start with empty requiredEmail - prints error screen", function() { - mediator.publish("start", { - requiredEmail: "" - }); - - equal(actions.called.doError, true, "error screen is shown"); - }); - - test("start with valid requiredEmail - go to doCheckAuth", function() { - mediator.publish("start", { requiredEmail: TEST_EMAIL }); - - equal(actions.called.doCheckAuth, true, "checking auth on start"); - }); - asyncTest("start to complete successful primary email verification - goto 'primary_user'", function() { mediator.subscribe("primary_user", function(msg, info) { equal(info.email, TEST_EMAIL, "correct email given"); diff --git a/resources/static/test/testHelpers/helpers.js b/resources/static/test/testHelpers/helpers.js index 287ba8393c039d0519f9c892fcc79495f62d051f..833f31e6e08744532b5aceaffc83413f1620d880 100644 --- a/resources/static/test/testHelpers/helpers.js +++ b/resources/static/test/testHelpers/helpers.js @@ -127,6 +127,10 @@ BrowserID.TestHelpers = (function() { equal(TestHelpers.errorVisible(), true, "error screen is visible"); }, + testErrorNotVisible: function() { + equal(TestHelpers.errorVisible(), false, "error screen is not visible"); + }, + waitVisible: function() { return screens.wait.visible; }, diff --git a/resources/views/test.ejs b/resources/views/test.ejs index 30c4faf532c2d3c034eb70b9fe8a96402c0a3e40..bb56772ace6e7a4a98aab70aa0521dc61da6e2a7 100644 --- a/resources/views/test.ejs +++ b/resources/views/test.ejs @@ -76,6 +76,7 @@ <script src="/lib/hub.js"></script> <script src="/lib/module.js"></script> <script src="/lib/jschannel.js"></script> + <script src="/lib/urlparse.js"></script> <script src="mocks/mocks.js"></script> <script src="mocks/xhr.js"></script> <script src="mocks/templates.js"></script>