From e5dba8b292e8b706a07af0b4f5a335a76084836b Mon Sep 17 00:00:00 2001 From: Shane Caraveo Date: Thu, 11 Apr 2019 11:25:02 +0000 Subject: [PATCH] Bug 1528562 support POST with 303 redirect in identity.launchWebAuthFlow r=rpl,Ehsan nsBrowserStatusFilter is updated to not filter out STATE_IS_REDIRECTED_DOCUMENT. The test here is adding a way to have a "login form" do a post to a server script, which then does a 303 redirect. This mimics what some services, including LinkedIn do during this stage. Differential Revision: https://phabricator.services.mozilla.com/D26969 --HG-- extra : moz-landing-system : lando --- .../extensions/test/mochitest/oauth.html | 19 ++++++-- .../test/mochitest/redirect_auto.sjs | 6 ++- .../test/mochitest/test_ext_identity.html | 46 +++++++++++++++---- .../statusfilter/nsBrowserStatusFilter.cpp | 5 +- 4 files changed, 61 insertions(+), 15 deletions(-) diff --git a/toolkit/components/extensions/test/mochitest/oauth.html b/toolkit/components/extensions/test/mochitest/oauth.html index 428ffb8ebca5..8b9b1d65ec42 100644 --- a/toolkit/components/extensions/test/mochitest/oauth.html +++ b/toolkit/components/extensions/test/mochitest/oauth.html @@ -4,12 +4,23 @@ +
+
diff --git a/toolkit/components/extensions/test/mochitest/redirect_auto.sjs b/toolkit/components/extensions/test/mochitest/redirect_auto.sjs index fce223ba85f0..27d249f022a9 100644 --- a/toolkit/components/extensions/test/mochitest/redirect_auto.sjs +++ b/toolkit/components/extensions/test/mochitest/redirect_auto.sjs @@ -9,7 +9,11 @@ function handleRequest(request, response) { response.setStatusLine(request.httpVersion, 200, "OK"); response.write("ok"); } else { - response.setStatusLine(request.httpVersion, 302, "Moved Temporarily"); + if (request.method == "POST") { + response.setStatusLine(request.httpVersion, 303, "Redirected"); + } else { + response.setStatusLine(request.httpVersion, 302, "Moved Temporarily"); + } let url = new URL(params.get("redirect_uri") || params.get("default_redirect")); url.searchParams.set("access_token", "here ya go"); response.setHeader("Location", url.href); diff --git a/toolkit/components/extensions/test/mochitest/test_ext_identity.html b/toolkit/components/extensions/test/mochitest/test_ext_identity.html index 42011451645a..d758223f6780 100644 --- a/toolkit/components/extensions/test/mochitest/test_ext_identity.html +++ b/toolkit/components/extensions/test/mochitest/test_ext_identity.html @@ -138,7 +138,7 @@ add_task(async function test_otherRedirectURL() { await extension.unload(); }); -function background_launchWebAuthFlow(interactive, path, redirect = true, useRedirectUri = true) { +function background_launchWebAuthFlow({interactive = false, path = "redirect_auto.sjs", params = {}, redirect = true, useRedirectUri = true} = {}) { let uri_path = useRedirectUri ? "identity_cb" : ""; let expected_redirect = `https://35b64b676900f491c00e7f618d43f7040e88422e.example.com/${uri_path}`; let base_uri = "https://example.com/tests/toolkit/components/extensions/test/mochitest/"; @@ -146,16 +146,21 @@ function background_launchWebAuthFlow(interactive, path, redirect = true, useRed browser.test.assertEq(expected_redirect, redirect_uri, "expected redirect uri matches hash"); let url = `${base_uri}${path}`; if (useRedirectUri) { - url = `${url}?redirect_uri=${encodeURIComponent(redirect_uri)}`; + params.redirect_uri = redirect_uri; } else { // We kind of fake it with the redirect url that would normally be configured // in the oauth service. This does still test that the identity service falls back // to the extensions redirect url. - url = `${url}?default_redirect=${encodeURIComponent(expected_redirect)}`; + params.default_redirect = expected_redirect; } if (!redirect) { - url = `${url}&no_redirect=1`; + params.no_redirect = 1; } + let query = []; + for (let [param, value] of Object.entries(params)) { + query.push(`${param}=${encodeURIComponent(value)}`); + } + url = `${url}?${query.join("&")}`; // Ensure we do not start the actual request for the redirect url. browser.webRequest.onBeforeRequest.addListener(details => { @@ -197,7 +202,7 @@ add_task(async function test_autoRedirect() { "https://*.example.com/*", ], }, - background: `(${background_launchWebAuthFlow})(false, "redirect_auto.sjs")`, + background: `(${background_launchWebAuthFlow})()`, }); await extension.startup(); @@ -219,7 +224,7 @@ add_task(async function test_autoRedirect_noRedirectURI() { "https://*.example.com/*", ], }, - background: `(${background_launchWebAuthFlow})(false, "redirect_auto.sjs", true, false)`, + background: `(${background_launchWebAuthFlow})({useRedirectUri: false})`, }); await extension.startup(); @@ -242,7 +247,7 @@ add_task(async function test_noRedirect() { "https://*.example.com/*", ], }, - background: `(${background_launchWebAuthFlow})(false, "redirect_auto.sjs", false)`, + background: `(${background_launchWebAuthFlow})({redirect: false})`, }); await extension.startup(); @@ -268,13 +273,38 @@ add_task(async function test_interaction() { "https://*.example.com/*", ], }, - background: `(${background_launchWebAuthFlow})(true, "oauth.html")`, + background: `(${background_launchWebAuthFlow})({interactive: true, path: "oauth.html"})`, }); await extension.startup(); await extension.awaitMessage("done"); await extension.unload(); }); + + +// Tests the situation where the oauth provider redirects with a 303. +add_task(async function test_auto303Redirect() { + let extension = ExtensionTestUtils.loadExtension({ + manifest: { + "applications": { + "gecko": { + "id": "identity@mozilla.org", + }, + }, + "permissions": [ + "webRequest", + "identity", + "https://*.example.com/*", + ], + }, + background: `(${background_launchWebAuthFlow})({interactive: true, path: "oauth.html", params: {post: 303, server_uri: "redirect_auto.sjs"}})`, + }); + + await extension.startup(); + await extension.awaitMessage("done"); + await extension.unload(); +}); + diff --git a/toolkit/components/statusfilter/nsBrowserStatusFilter.cpp b/toolkit/components/statusfilter/nsBrowserStatusFilter.cpp index c71337fd4721..456dd90aea63 100644 --- a/toolkit/components/statusfilter/nsBrowserStatusFilter.cpp +++ b/toolkit/components/statusfilter/nsBrowserStatusFilter.cpp @@ -161,8 +161,9 @@ nsBrowserStatusFilter::OnStateChange(nsIWebProgress *aWebProgress, return NS_OK; } - // Only notify listener for STATE_IS_NETWORK. - if (aStateFlags & STATE_IS_NETWORK) { + // Only notify listener for STATE_IS_NETWORK or STATE_IS_REDIRECTED_DOCUMENT + if (aStateFlags & STATE_IS_NETWORK || + aStateFlags & STATE_IS_REDIRECTED_DOCUMENT) { return mListener->OnStateChange(aWebProgress, aRequest, aStateFlags, aStatus); }