Bug 1528562 support POST with 303 redirect in identity.launchWebAuthFlow r=rpl,Ehsan a=pascalc
authorShane Caraveo <scaraveo@mozilla.com>
Thu, 11 Apr 2019 21:26:14 +0000
changeset 526195 5d798451164267c5f4a4b69684361bc028d3d07a
parent 526194 cd1e0b429eff6abc69f7aa0b547e06b8afaa17a4
child 526196 095f7046554c6134f2ecff417634301f6413e1da
push id2032
push userffxbld-merge
push dateMon, 13 May 2019 09:36:57 +0000
treeherdermozilla-release@455c1065dcbe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrpl, Ehsan, pascalc
bugs1528562
milestone67.0
Bug 1528562 support POST with 303 redirect in identity.launchWebAuthFlow r=rpl,Ehsan a=pascalc 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