diff --git a/resources/static/dialog/js/modules/dialog.js b/resources/static/dialog/js/modules/dialog.js index f7102480a..e59ee3257 100644 --- a/resources/static/dialog/js/modules/dialog.js +++ b/resources/static/dialog/js/modules/dialog.js @@ -109,6 +109,16 @@ BrowserID.Modules.Dialog = (function() { throw "must be an absolute path: (" + path + ")"; } + function fixupReturnTo(origin_url, path) { + // "/" is a valid returnTo, but it is not a valid path for any other + // parameter. If the path is "/", allow it, otherwise pass the path down + // the normal checks. + var returnTo = path === "/" ? + origin_url + path : + fixupAbsolutePath(origin_url, path); + return returnTo; + } + function validateRPAPI(rpAPI) { var VALID_RP_API_VALUES = [ "watch_without_onready", @@ -225,7 +235,7 @@ BrowserID.Modules.Dialog = (function() { // returnTo is used for post verification redirection. Redirect back // to the path specified by the RP. if (paramsFromRP.returnTo) { - var returnTo = fixupAbsolutePath(origin_url, paramsFromRP.returnTo); + var returnTo = fixupReturnTo(origin_url, paramsFromRP.returnTo); user.setReturnTo(returnTo); } diff --git a/resources/static/test/cases/dialog/js/modules/dialog.js b/resources/static/test/cases/dialog/js/modules/dialog.js index 207d9cca5..ca94b8128 100644 --- a/resources/static/test/cases/dialog/js/modules/dialog.js +++ b/resources/static/test/cases/dialog/js/modules/dialog.js @@ -59,7 +59,7 @@ // dialog as an individual unit. var options = $.extend({ window: winMock, - startExternalDependencies: false, + startExternalDependencies: false }, config); controller = BrowserID.Modules.Dialog.create(); @@ -72,13 +72,13 @@ }); } - function testExpectGetFailure(options, expectedErrorMessage) { + function testExpectGetFailure(options, expectedErrorMessage, domain) { _.extend(options, { ready: function() { testMessageNotExpected("kpi_data"); testMessageNotExpected("start"); - var retval = controller.get(HTTPS_TEST_DOMAIN, options); + var retval = controller.get(domain || HTTPS_TEST_DOMAIN, options); if (expectedErrorMessage) { equal(retval, expectedErrorMessage, "expected error: " + expectedErrorMessage); @@ -87,6 +87,9 @@ ok(retval, "error message returned"); } + // If a parameter is not properly escaped, scriptRun will be true. + equal(typeof window.scriptRun, "undefined", "script was not run"); + testErrorVisible(); start(); } @@ -94,6 +97,35 @@ createController(options); } + function testRelativeURLNotAllowed(options, path) { + testExpectGetFailure(options, "relative urls not allowed: (" + path + ")"); + } + + function testMustBeAbsolutePath(options, path) { + testExpectGetFailure(options, "must be an absolute path: (" + path + ")"); + } + + function testExpectGetSuccess(options, expected, domain, done) { + createController({ + ready: function() { + var startInfo; + mediator.subscribe("start", function(msg, info) { + startInfo = info; + }); + + var retval = controller.get(domain || HTTPS_TEST_DOMAIN, options); + testHelpers.testObjectValuesEqual(startInfo, expected); + + equal(typeof retval, "undefined", "no error expected"); + testErrorNotVisible(); + + done && done(); + + start(); + } + }); + } + module("dialog/js/modules/dialog", { setup: function() { @@ -237,174 +269,82 @@ asyncTest("get with relative termsOfService & valid privacyPolicy - 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, { - termsOfService: "relative.html", - privacyPolicy: "/privacy.html" - }); - equal(retval, "relative urls not allowed: (relative.html)", "expected error"); - testErrorVisible(); - start(); - } - }); + testRelativeURLNotAllowed({ + termsOfService: "relative.html", + privacyPolicy: "/privacy.html" + }, "relative.html"); }); asyncTest("get with script containing termsOfService - 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, { - termsOfService: "relative.html", - privacyPolicy: "/privacy.html" - }); - - // If termsOfService 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)", "expected error"); - testErrorVisible(); - start(); - } - }); + var URL = "relative.html"; + testRelativeURLNotAllowed({ + termsOfService: URL, + privacyPolicy: "/privacy.html" + }, URL); }); asyncTest("get with valid termsOfService & relative privacyPolicy - 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, { - termsOfService: "/tos.html", - privacyPolicy: "relative.html" - }); - equal(retval, "relative urls not allowed: (relative.html)", "expected error"); - testErrorVisible(); - start(); - } - }); + var URL = "relative.html"; + testRelativeURLNotAllowed({ + termsOfService: "/tos.html", + privacyPolicy: URL + }, URL); }); - asyncTest("get with script containing privacyPolicy - 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, { - termsOfService: "/tos.html", - privacyPolicy: "relative.html" - }); - - // If privacyPolicy 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)", "expected error"); - testErrorVisible(); - start(); - } - }); + asyncTest("get with valid termsOfService & privacyPolicy='/' - print error screen", function() { + var URL = "/"; + testRelativeURLNotAllowed({ + termsOfService: "/tos.html", + privacyPolicy: URL + }, URL); }); - asyncTest("get with privacyPolicy - 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, { - termsOfService: "/tos.html", - privacyPolicy: "relative.html" - }); + asyncTest("get with valid termsOfService='/' and valid privacyPolicy - print error screen", function() { + var URL = "/" + testRelativeURLNotAllowed({ + termsOfService: URL, + privacyPolicy: "/privacy.html" + }, URL); + }); - // If privacyPolicy 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)", "expected error"); - testErrorVisible(); - start(); - } - }); + asyncTest("get with script containing privacyPolicy - print error screen", function() { + var URL = "relative.html"; + testRelativeURLNotAllowed({ + termsOfService: "/tos.html", + privacyPolicy: URL + }, URL); }); asyncTest("get with javascript protocol for privacyPolicy - 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, { - termsOfService: "/tos.html", - privacyPolicy: "javascript:alert(1)" - }); - - equal(retval, "relative urls not allowed: (javascript:alert(1))", "expected error"); - testErrorVisible(); - start(); - } - }); + var URL = "javascript:alert(1)"; + testRelativeURLNotAllowed({ + termsOfService: "/tos.html", + privacyPolicy: URL + }, URL); }); asyncTest("get with invalid httpg protocol for privacyPolicy - 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, { - termsOfService: "/tos.html", - privacyPolicy: "httpg://testdomain.com/privacy.html" - }); - - equal(retval, "relative urls not allowed: (httpg://testdomain.com/privacy.html)", "expected error"); - testErrorVisible(); - start(); - } - }); + var URL = "httpg://testdomain.com/privacy.html"; + testRelativeURLNotAllowed({ + termsOfService: "/tos.html", + privacyPolicy: URL + }, URL); }); - function testValidTermsOfServicePrivacyPolicy(options, expected) { - createController({ - ready: function() { - var startInfo; - mediator.subscribe("start", function(msg, info) { - startInfo = info; - }); - - var retval = controller.get(HTTP_TEST_DOMAIN, options); - testHelpers.testObjectValuesEqual(startInfo, expected); - - equal(typeof retval, "undefined", "no error expected"); - testErrorNotVisible(); - start(); - } - }); - } - asyncTest("get with valid absolute termsOfService & privacyPolicy - go to start", function() { - testValidTermsOfServicePrivacyPolicy({ + testExpectGetSuccess({ termsOfService: "/tos.html", privacyPolicy: "/privacy.html" }, { - termsOfService: HTTP_TEST_DOMAIN + "/tos.html", - privacyPolicy: HTTP_TEST_DOMAIN + "/privacy.html" + termsOfService: HTTPS_TEST_DOMAIN + "/tos.html", + privacyPolicy: HTTPS_TEST_DOMAIN + "/privacy.html" }); }); asyncTest("get with valid fully qualified http termsOfService & privacyPolicy - go to start", function() { - testValidTermsOfServicePrivacyPolicy({ + testExpectGetSuccess({ termsOfService: HTTP_TEST_DOMAIN + "/tos.html", privacyPolicy: HTTP_TEST_DOMAIN + "/privacy.html" }, @@ -416,7 +356,7 @@ asyncTest("get with valid fully qualified https termsOfService & privacyPolicy - go to start", function() { - testValidTermsOfServicePrivacyPolicy({ + testExpectGetSuccess({ termsOfService: HTTPS_TEST_DOMAIN + "/tos.html", privacyPolicy: HTTPS_TEST_DOMAIN + "/privacy.html" }, @@ -427,166 +367,58 @@ }); asyncTest("get with valid termsOfService, tosURL & privacyPolicy, privacyURL - use termsOfService and privacyPolicy", function() { - testValidTermsOfServicePrivacyPolicy({ + testExpectGetSuccess({ termsOfService: "/tos.html", tosURL: "/tos_deprecated.html", privacyPolicy: "/privacy.html", privacyURL: "/privacy_deprecated.html" }, { - termsOfService: HTTP_TEST_DOMAIN + "/tos.html", - privacyPolicy: HTTP_TEST_DOMAIN + "/privacy.html" + termsOfService: HTTPS_TEST_DOMAIN + "/tos.html", + privacyPolicy: HTTPS_TEST_DOMAIN + "/privacy.html" }); }); asyncTest("get with relative siteLogo - not allowed", 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, { - siteLogo: "logo.png", - }); - - equal(retval, "must be an absolute path: (logo.png)", "expected error"); - testErrorVisible(); - start(); - } - }); + var URL = "logo.png"; + testMustBeAbsolutePath({ siteLogo: URL }, URL); }); asyncTest("get with javascript: siteLogo - not allowed", 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, { - siteLogo: "javascript:alert('xss')", - }); - - equal(retval, "must be an absolute path: (javascript:alert('xss'))", "expected error"); - testErrorVisible(); - start(); - } - }); + var URL = "javascript:alert('xss')"; + testMustBeAbsolutePath({ siteLogo: URL }, URL); }); asyncTest("get with data-uri: siteLogo - not allowed", 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, { - siteLogo: "data:image/png,FAKEDATA", - }); - - equal(retval, "must be an absolute path: (data:image/png,FAKEDATA)", "expected error"); - testErrorVisible(); - start(); - } - }); + var URL = "data:image/png,FAKEDATA"; + testMustBeAbsolutePath({ siteLogo: URL }, URL); }); asyncTest("get with http: siteLogo - not allowed", 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, { - siteLogo: HTTP_TEST_DOMAIN + "://logo.png", - }); - - equal(retval, "must be an absolute path: (" + HTTP_TEST_DOMAIN + "://logo.png)", "expected error"); - testErrorVisible(); - start(); - } - }); + var URL = HTTP_TEST_DOMAIN + "://logo.png"; + testMustBeAbsolutePath({ siteLogo: URL }, URL); }); asyncTest("get with https: siteLogo - not allowed", 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, { - siteLogo: HTTPS_TEST_DOMAIN + "://logo.png", - }); - - equal(retval, "must be an absolute path: (" + HTTPS_TEST_DOMAIN + "://logo.png)", "expected error"); - testErrorVisible(); - start(); - } - }); + var URL = HTTPS_TEST_DOMAIN + "://logo.png"; + testMustBeAbsolutePath({ siteLogo: URL }, URL); }); asyncTest("get with absolute path and http RP - not allowed", function() { - createController({ - ready: function() { - mediator.subscribe("start", function(msg, info) { - ok(false, "start should not have been called"); - }); - - var siteLogo = '/i/card.png'; - var retval = controller.get(HTTP_TEST_DOMAIN, { - siteLogo: siteLogo - }); - - equal(retval, "only https sites can specify a siteLogo", "expected error"); - testErrorVisible(); - start(); - } - }); + var siteLogo = '/i/card.png'; + testExpectGetFailure({ siteLogo: siteLogo }, "only https sites can specify a siteLogo", HTTP_TEST_DOMAIN); }); asyncTest("get with absolute path that is too long - not allowed", function() { - createController({ - ready: function() { - mediator.subscribe("start", function(msg, info) { - ok(false, "start should not have been called"); - }); - - // create a logo path that is one character too long - var siteLogo = '/' + testHelpers.generateString(bid.PATH_MAX_LENGTH); - var retval = controller.get(HTTPS_TEST_DOMAIN, { - siteLogo: siteLogo - }); - - equal(retval, "path portion of a url must be < " + bid.PATH_MAX_LENGTH + " characters"); - testErrorVisible(); - start(); - } - }); + var siteLogo = '/' + testHelpers.generateString(bid.PATH_MAX_LENGTH); + testExpectGetFailure({ siteLogo: siteLogo }, "path portion of a url must be < " + bid.PATH_MAX_LENGTH + " characters"); }); asyncTest("get with absolute path causing too long of a URL - not allowed", function() { - createController({ - ready: function() { - mediator.subscribe("start", function(msg, info) { - ok(false, "start should not have been called"); - }); - - var shortHTTPSDomain = "https://test.com"; - // create a URL that is one character too long - var siteLogo = '/' + testHelpers.generateString(bid.URL_MAX_LENGTH - shortHTTPSDomain.length); - var retval = controller.get(shortHTTPSDomain, { - siteLogo: siteLogo - }); - - equal(retval, "urls must be < " + bid.URL_MAX_LENGTH + " characters"); - testErrorVisible(); - start(); - } - }); + var shortHTTPSDomain = "https://test.com"; + // create a URL that is one character too long + var siteLogo = '/' + testHelpers.generateString(bid.URL_MAX_LENGTH - shortHTTPSDomain.length); + testExpectGetFailure({ siteLogo: siteLogo }, "urls must be < " + bid.URL_MAX_LENGTH + " characters"); }); asyncTest("get with absolute path and https RP - allowed URL but is properly escaped", function() { @@ -613,73 +445,35 @@ }); asyncTest("get with a scheme-relative siteLogo URL - not allowed", function() { - createController({ - ready: function() { - mediator.subscribe("start", function(msg, info) { - ok(false, "start should not have been called"); - }); - - var retval = controller.get(HTTPS_TEST_DOMAIN, { - siteLogo: "//example.com/image.png" - }); - - equal(retval, "must be an absolute path: (//example.com/image.png)", "expected error"); - testErrorVisible(); - start(); - } - }); + var URL = "//example.com/image.png"; + testMustBeAbsolutePath({ siteLogo: URL }, URL); }); - asyncTest("get with returnTo with https - not allowed", function() { - createController({ - ready: function() { - var URL = HTTP_TEST_DOMAIN + "/path"; - - mediator.subscribe("start", function(msg, info) { - ok(false, "unexpected start"); - }); - - var retval = controller.get(HTTP_TEST_DOMAIN, { - returnTo: URL - }); + asyncTest("get with siteLogo='/' URL - not allowed", function() { + testMustBeAbsolutePath({ siteLogo: "/" }, "/"); + }); - equal(retval, "must be an absolute path: (" + URL + ")", "expected error"); - testErrorVisible(); - start(); - } - }); + asyncTest("get with fully qualified returnTo - not allowed", function() { + var URL = HTTPS_TEST_DOMAIN + "/path"; + testMustBeAbsolutePath({ returnTo: URL }, URL); }); asyncTest("get with a scheme-relative returnTo URL - not allowed", function() { - createController({ - ready: function() { - mediator.subscribe("start", function(msg, info) { - ok(false, "unexpected start"); - }); - - var retval = controller.get(HTTP_TEST_DOMAIN, { - returnTo: '//example.com/return' - }); - - equal(retval, "must be an absolute path: (//example.com/return)", "expected error"); - testErrorVisible(); - start(); - } - }); + var URL = '//example.com/return'; + testMustBeAbsolutePath({ returnTo: URL }, URL); }); asyncTest("get with absolute path returnTo - allowed", function() { - createController({ - ready: function() { - mediator.subscribe("start", function(msg, info) { - equal(user.getReturnTo(), HTTPS_TEST_DOMAIN + "/path", "returnTo correctly set"); - start(); - }); + testExpectGetSuccess({ returnTo: "/path"}, {}, undefined, function() { + equal(user.getReturnTo(), + HTTPS_TEST_DOMAIN + "/path", "returnTo correctly set"); + }); + }); - var retval = controller.get(HTTPS_TEST_DOMAIN, { - returnTo: "/path" - }); - } + asyncTest("get with returnTo='/' - allowed", function() { + testExpectGetSuccess({ returnTo: "/"}, {}, undefined, function() { + equal(user.getReturnTo(), + HTTPS_TEST_DOMAIN + "/", "returnTo correctly set"); }); });