Bug 1494328 - Fix launchWebAuthFlow to use default redirect_uri. r=aswan, a=RyanVM
authorShane Caraveo <scaraveo@mozilla.com>
Mon, 08 Oct 2018 15:07:01 +0000
changeset 450228 333276fac37c
parent 450227 791c8ecf252d
child 450229 cb73374a0e4e
push id207
push userryanvm@gmail.com
push dateWed, 10 Oct 2018 19:31:49 +0000
treeherdermozilla-esr60@f6bb138ad0ab [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaswan, RyanVM
bugs1494328
milestone60.2.3
Bug 1494328 - Fix launchWebAuthFlow to use default redirect_uri. r=aswan, a=RyanVM Some oauth services require the redirect uri be configured on their service, and the reject the redirect_uri param if we send it. Chrome works fine in this scenario, but we have been requiring the redirect_uri be provided. This addresses that requirement by using our own default redirect url, which would be the url used to configure the oauth service. Differential Revision: https://phabricator.services.mozilla.com/D8004
toolkit/components/extensions/ext-c-identity.js
toolkit/components/extensions/ext-identity.js
toolkit/components/extensions/test/mochitest/redirect_auto.sjs
toolkit/components/extensions/test/mochitest/test_ext_identity.html
--- a/toolkit/components/extensions/ext-c-identity.js
+++ b/toolkit/components/extensions/ext-c-identity.js
@@ -26,12 +26,28 @@ this.identity = class extends ExtensionA
     return {
       identity: {
         getRedirectURL: function(path = "") {
           let hash = computeHash(extension.id);
           let url = new URL(`https://${hash}.${redirectDomain}/`);
           url.pathname = path;
           return url.href;
         },
+        launchWebAuthFlow: function(details) {
+          // Validate the url and retreive redirect_uri if it was provided.
+          let url, redirectURI;
+          try {
+            url = new URL(details.url);
+          } catch (e) {
+            return Promise.reject({message: "details.url is invalid"});
+          }
+          try {
+            redirectURI = new URL(url.searchParams.get("redirect_uri") || this.getRedirectURL());
+          } catch (e) {
+            return Promise.reject({message: "redirect_uri is invalid"});
+          }
+
+          return context.childManager.callParentAsyncFunction("identity.launchWebAuthFlowInParent", [details, redirectURI.href]);
+        },
       },
     };
   }
 };
--- a/toolkit/components/extensions/ext-identity.js
+++ b/toolkit/components/extensions/ext-identity.js
@@ -1,16 +1,16 @@
 /* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* vim: set sts=2 sw=2 et tw=80: */
 "use strict";
 
 ChromeUtils.defineModuleGetter(this, "Services",
                                "resource://gre/modules/Services.jsm");
 
-Cu.importGlobalProperties(["URL", "XMLHttpRequest"]);
+Cu.importGlobalProperties(["XMLHttpRequest"]);
 
 var {
   promiseDocumentLoaded,
 } = ExtensionUtils;
 
 const checkRedirected = (url, redirectURI) => {
   return new Promise((resolve, reject) => {
     let xhr = new XMLHttpRequest();
@@ -89,33 +89,17 @@ const openOAuthWindow = (details, redire
     });
   });
 };
 
 this.identity = class extends ExtensionAPI {
   getAPI(context) {
     return {
       identity: {
-        launchWebAuthFlow: function(details) {
-          // In OAuth2 the url should have a redirect_uri param, parse the url and grab it
-          let url, redirectURI;
-          try {
-            url = new URL(details.url);
-          } catch (e) {
-            return Promise.reject({message: "details.url is invalid"});
-          }
-          try {
-            redirectURI = new URL(url.searchParams.get("redirect_uri"));
-            if (!redirectURI) {
-              return Promise.reject({message: "redirect_uri is missing"});
-            }
-          } catch (e) {
-            return Promise.reject({message: "redirect_uri is invalid"});
-          }
-
+        launchWebAuthFlowInParent: function(details, redirectURI) {
           // If the request is automatically redirected the user has already
           // authorized and we do not want to show the window.
           return checkRedirected(details.url, redirectURI).catch((requestError) => {
             // requestError is zero or xhr.status
             if (requestError !== 0) {
               Cu.reportError(`browser.identity auth check failed with ${requestError}`);
               return Promise.reject({message: "Invalid request"});
             }
--- a/toolkit/components/extensions/test/mochitest/redirect_auto.sjs
+++ b/toolkit/components/extensions/test/mochitest/redirect_auto.sjs
@@ -5,13 +5,13 @@ Components.utils.importGlobalProperties(
 
 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");
-    let url = new URL(params.get("redirect_uri"));
+    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,22 +133,31 @@ add_task(async function test_otherRedire
     },
   });
 
   await extension.startup();
   await extension.awaitMessage("done");
   await extension.unload();
 });
 
-function background_launchWebAuthFlow(interactive, path, redirect = true) {
-  let expected_redirect = "https://35b64b676900f491c00e7f618d43f7040e88422e.example.com/identity_cb";
+function background_launchWebAuthFlow(interactive, path, 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("/identity_cb");
+  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}?redirect_uri=${encodeURIComponent(redirect_uri)}`;
+  let url = `${base_uri}${path}`;
+  if (useRedirectUri) {
+    url = `${url}?redirect_uri=${encodeURIComponent(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)}`;
+  }
   if (!redirect) {
     url = `${url}&no_redirect=1`;
   }
 
   // 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");
@@ -191,16 +200,38 @@ add_task(async function test_autoRedirec
     background: `(${background_launchWebAuthFlow})(false, "redirect_auto.sjs")`,
   });
 
   await extension.startup();
   await extension.awaitMessage("done");
   await extension.unload();
 });
 
+add_task(async function test_autoRedirect_noRedirectURI() {
+  let extension = ExtensionTestUtils.loadExtension({
+    manifest: {
+      "applications": {
+        "gecko": {
+          "id": "identity@mozilla.org",
+        },
+      },
+      "permissions": [
+        "webRequest",
+        "identity",
+        "https://*.example.com/*",
+      ],
+    },
+    background: `(${background_launchWebAuthFlow})(false, "redirect_auto.sjs", true, 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
 add_task(async function test_noRedirect() {
   let extension = ExtensionTestUtils.loadExtension({
     manifest: {
       "applications": {
         "gecko": {
           "id": "identity@mozilla.org",
         },