Bug 1528562 support POST with 303 redirect in identity.launchWebAuthFlow r=rpl,Ehsan
authorShane Caraveo <scaraveo@mozilla.com>
Thu, 11 Apr 2019 21:26:14 +0000
changeset 469092 ca9b52a04b085152ad38b468d3d87e4f56df86f3
parent 469091 1a92c531a9af2815c1a8bb79716f0dd9d8b883a8
child 469093 af57f8ff4600757ecef6e5491f8e5d6e51df67ed
push id35856
push usercsabou@mozilla.com
push dateFri, 12 Apr 2019 03:19:48 +0000
treeherdermozilla-central@940684cd1065 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrpl, Ehsan
bugs1528562
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
toolkit/components/extensions/test/mochitest/oauth.html
toolkit/components/extensions/test/mochitest/redirect_auto.sjs
toolkit/components/extensions/test/mochitest/test_ext_identity.html
toolkit/components/statusfilter/nsBrowserStatusFilter.cpp
--- a/toolkit/components/extensions/test/mochitest/oauth.html
+++ b/toolkit/components/extensions/test/mochitest/oauth.html
@@ -1,15 +1,26 @@
 <!DOCTYPE html>
 <html>
 <head>
   <script>
   "use strict";
 
-  var url = new URL(location);
-  var end = new URL(url.searchParams.get("redirect_uri"));
-  end.searchParams.set("access_token", "here ya go");
-  location.href = end.href;
+  onload = () => {
+    let url = new URL(location);
+    if (url.searchParams.get("post")) {
+      let server_redirect = `${url.searchParams.get("server_uri")}?redirect_uri=${encodeURIComponent(url.searchParams.get("redirect_uri"))}`;
+      let form = document.forms.testform;
+      form.setAttribute("action", server_redirect);
+      form.submit();
+    } else {
+      let end = new URL(url.searchParams.get("redirect_uri"));
+      end.searchParams.set("access_token", "here ya go");
+      location.href = end.href;
+    }
+  };
   </script>
 </head>
 <body>
+  <form name="testform" action="" method="POST">
+  </form>
 </body>
 </html>
--- a/toolkit/components/extensions/test/mochitest/redirect_auto.sjs
+++ b/toolkit/components/extensions/test/mochitest/redirect_auto.sjs
@@ -4,14 +4,18 @@
 Components.utils.importGlobalProperties(["URLSearchParams", "URL"]);
 
 function handleRequest(request, response) {
   let params = new URLSearchParams(request.queryString);
   if (params.has("no_redirect")) {
     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);
   }
 }
--- a/toolkit/components/extensions/test/mochitest/test_ext_identity.html
+++ b/toolkit/components/extensions/test/mochitest/test_ext_identity.html
@@ -133,41 +133,49 @@ add_task(async function test_otherRedire
     },
   });
 
   await extension.startup();
   await extension.awaitMessage("done");
   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/";
   let redirect_uri = browser.identity.getRedirectURL(useRedirectUri ? uri_path : undefined);
   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 => {
-    if (details.url.startsWith(expected_redirect)) {
-      browser.test.fail("onBeforeRequest called for redirect url");
-    }
-  }, {urls: ["https://35b64b676900f491c00e7f618d43f7040e88422e.example.com/*"]});
+  // Ensure we do not start the actual request for the redirect url.  In the case
+  // of a 303 POST redirect we are getting a request started.
+  if (params.post !== 303) {
+    browser.webRequest.onBeforeRequest.addListener(details => {
+      if (details.url.startsWith(expected_redirect)) {
+        browser.test.fail("onBeforeRequest called for redirect url");
+      }
+    }, {urls: ["https://35b64b676900f491c00e7f618d43f7040e88422e.example.com/*"]});
+  }
 
   browser.identity.launchWebAuthFlow({interactive, url}).then((redirectURL) => {
     browser.test.assertTrue(redirectURL.startsWith(redirect_uri), `correct redirect url ${redirectURL}`);
     if (redirect) {
       let url = new URL(redirectURL);
       browser.test.assertEq("here ya go", url.searchParams.get("access_token"), "Handled auto redirection");
     }
     browser.test.sendMessage("done");
@@ -192,17 +200,17 @@ add_task(async function test_autoRedirec
         },
       },
       "permissions": [
         "webRequest",
         "identity",
         "https://*.example.com/*",
       ],
     },
-    background: `(${background_launchWebAuthFlow})(false, "redirect_auto.sjs")`,
+    background: `(${background_launchWebAuthFlow})()`,
   });
 
   await extension.startup();
   await extension.awaitMessage("done");
   await extension.unload();
 });
 
 add_task(async function test_autoRedirect_noRedirectURI() {
@@ -214,17 +222,17 @@ add_task(async function test_autoRedirec
         },
       },
       "permissions": [
         "webRequest",
         "identity",
         "https://*.example.com/*",
       ],
     },
-    background: `(${background_launchWebAuthFlow})(false, "redirect_auto.sjs", true, false)`,
+    background: `(${background_launchWebAuthFlow})({useRedirectUri: false})`,
   });
 
   await extension.startup();
   await extension.awaitMessage("done");
   await extension.unload();
 });
 
 // Tests the situation where the oauth provider has not granted access and interactive=false
@@ -237,17 +245,17 @@ add_task(async function test_noRedirect(
         },
       },
       "permissions": [
         "webRequest",
         "identity",
         "https://*.example.com/*",
       ],
     },
-    background: `(${background_launchWebAuthFlow})(false, "redirect_auto.sjs", false)`,
+    background: `(${background_launchWebAuthFlow})({redirect: false})`,
   });
 
   await extension.startup();
   await extension.awaitMessage("done");
   await extension.unload();
 });
 
 // Tests the situation where the oauth provider must show a window where
@@ -263,19 +271,44 @@ add_task(async function test_interaction
         },
       },
       "permissions": [
         "webRequest",
         "identity",
         "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();
+});
+
 </script>
 
 </body>
 </html>
--- a/toolkit/components/statusfilter/nsBrowserStatusFilter.cpp
+++ b/toolkit/components/statusfilter/nsBrowserStatusFilter.cpp
@@ -156,18 +156,19 @@ nsBrowserStatusFilter::OnStateChange(nsI
         ProcessTimeout();
       }
     }
   } else {
     // No need to forward this state change.
     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);
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP