diff --git a/resources/static/dialog/js/modules/dialog.js b/resources/static/dialog/js/modules/dialog.js index f7102480aafea9ca45b1b0e29d02473f262bc870..e59ee32575e1e23bc8a83cee4ba82e6fd6a4250e 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 207d9cca50b0bba930b60a1c8f23d524f45b9717..ee9fb9eaac50a8d9bb2beec97c20007f5031f6d9 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(); @@ -94,6 +94,32 @@ 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) { + 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(); + } + }); + } + module("dialog/js/modules/dialog", { setup: function() { @@ -293,6 +319,20 @@ }); }); + asyncTest("get with valid termsOfService & privacyPolicy='/' - print error screen", function() { + testRelativeURLNotAllowed({ + termsOfService: "/tos.html", + privacyPolicy: "/" + }, "/"); + }); + + asyncTest("get with valid termsOfService='/' and valid privacyPolicy - print error screen", function() { + testRelativeURLNotAllowed({ + termsOfService: "/", + privacyPolicy: "/privacy.html" + }, "/"); + }); + asyncTest("get with script containing privacyPolicy - print error screen", function() { createController({ ready: function() { @@ -374,26 +414,8 @@ }); - 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" }, @@ -404,7 +426,7 @@ }); 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 +438,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,7 +449,7 @@ }); 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", @@ -630,7 +652,11 @@ }); }); - asyncTest("get with returnTo with https - not allowed", function() { + asyncTest("get with siteLogo='/' URL - not allowed", function() { + testMustBeAbsolutePath({ siteLogo: "/" }, "/"); + }); + + asyncTest("get with fully qualified URL for returnTo - not allowed", function() { createController({ ready: function() { var URL = HTTP_TEST_DOMAIN + "/path"; @@ -683,6 +709,10 @@ }); }); + asyncTest("get with returnTo='/' - allowed", function() { + testExpectGetSuccess({ returnTo: "/"}, {}); + }); + asyncTest("get with valid rp_api - allowed", function() { createController({ ready: function() {