Bug 1401350 fix proxy auth for system requests, r=kmag

MozReview-Commit-ID: CAh89djQobI

--HG--
extra : rebase_source : c85ff5148d4ac1df4e21622d2d615040ba9e09d7

toolkit/components/extensions/ext-webRequest.js

@@ -17,9 +17,17 @@ function WebRequestEventManager(context, eventName) {
   let name = `webRequest.${eventName}`;
   let register = (fire, filter, info) => {
     let listener = data => {
+      // data.isProxy is only set from the WebRequest AuthRequestor handler,
+      // in which case we know that this is onAuthRequired.  If this is proxy
+      // authorization, we allow without any additional matching/filtering.
+      let isProxyAuth = data.isProxy && context.extension.hasPermission("proxy");
+
       // Prevent listening in on requests originating from system principal to
-      // prevent tinkering with OCSP, app and addon updates, etc.
+      // prevent tinkering with OCSP, app and addon updates, etc.  However,
-      if (data.isSystemPrincipal) {
+      // proxy addons need to be able to provide auth for any request so we
+      // allow those through.  The exception is for proxy extensions handling
+      // proxy authentication.
+      if (data.isSystemPrincipal && !isProxyAuth) {
         return;
       }
 
@@ -31,7 +39,7 @@ function WebRequestEventManager(context, eventName) {
       // and the origin that is loading the resource.
       const origin = data.documentUrl;
       const own = origin && origin.startsWith(context.extension.getURL());
-      if (origin && !own && !hosts.matches(data.documentURI)) {
+      if (origin && !own && !isProxyAuth && !hosts.matches(data.documentURI)) {
         return;
       }
 

toolkit/components/extensions/test/xpcshell/test_ext_proxy_auth.js

@@ -1,5 +1,7 @@
 "use strict";
 
+const XMLHttpRequest = Components.Constructor("@mozilla.org/xmlextras/xmlhttprequest;1", "nsIXMLHttpRequest");
+
 const proxy = createHttpServer();
 
 // accept proxy connections for mozilla.org
@@ -8,14 +10,35 @@ proxy.identity.add("http", "mozilla.org", 80);
 proxy.registerPathHandler("/", (request, response) => {
   if (request.hasHeader("Proxy-Authorization")) {
     response.setStatusLine(request.httpVersion, 200, "OK");
-    response.write("ok");
+    response.setHeader("Content-Type", "text/plain", false);
+    response.write("ok, got proxy auth");
   } else {
     response.setStatusLine(request.httpVersion, 407, "Proxy authentication required");
+    response.setHeader("Content-Type", "text/plain", false);
     response.setHeader("Proxy-Authenticate", 'Basic realm="foobar"', false);
-    response.write("ok");
+    response.write("auth required");
   }
 });
 
+function getExtension(background) {
+  return ExtensionTestUtils.loadExtension({
+    manifest: {
+      permissions: [
+        "proxy",
+        "webRequest",
+        "webRequestBlocking",
+        "<all_urls>",
+      ],
+    },
+    background: `(${background})(${proxy.identity.primaryPort})`,
+    files: {
+      "proxy.js": `
+        function FindProxyForURL(url, host) {
+          return "PROXY localhost:${proxy.identity.primaryPort}; DIRECT";
+        }`,
+    },
+  });
+}
 add_task(async function test_webRequest_auth_proxy() {
   async function background(port) {
     browser.webRequest.onBeforeRequest.addListener(details => {
@@ -48,23 +71,7 @@ add_task(async function test_webRequest_auth_proxy() {
     browser.test.sendMessage("pac-ready");
   }
 
-  let handlingExt = ExtensionTestUtils.loadExtension({
+  let handlingExt = getExtension(background);
-    manifest: {
-      permissions: [
-        "proxy",
-        "webRequest",
-        "webRequestBlocking",
-        "<all_urls>",
-      ],
-    },
-    background: `(${background})(${proxy.identity.primaryPort})`,
-    files: {
-      "proxy.js": `
-        function FindProxyForURL(url, host) {
-          return "PROXY localhost:${proxy.identity.primaryPort}; DIRECT";
-        }`,
-    },
-  });
 
   await handlingExt.startup();
   await handlingExt.awaitMessage("pac-ready");
@@ -75,3 +82,47 @@ add_task(async function test_webRequest_auth_proxy() {
   await contentPage.close();
   await handlingExt.unload();
 });
+
+add_task(async function test_webRequest_auth_proxy_system() {
+  async function background(port) {
+    browser.webRequest.onBeforeRequest.addListener(details => {
+      browser.test.fail("onBeforeRequest");
+    }, {urls: ["<all_urls>"]});
+    browser.webRequest.onAuthRequired.addListener(details => {
+      browser.test.sendMessage("onAuthRequired");
+      // cancel is silently ignored, if it were not (e.g someone messes up in
+      // WebRequest.jsm and allows cancel) this test would fail.
+      return {
+        cancel: true,
+        authCredentials: {username: "puser", password: "ppass"},
+      };
+    }, {urls: ["<all_urls>"]}, ["blocking"]);
+
+    await browser.proxy.register("proxy.js");
+    browser.test.sendMessage("pac-ready");
+  }
+
+  let handlingExt = getExtension(background);
+
+  await handlingExt.startup();
+  await handlingExt.awaitMessage("pac-ready");
+
+  function fetch(url) {
+    return new Promise((resolve, reject) => {
+      let xhr = new XMLHttpRequest();
+      xhr.mozBackgroundRequest = true;
+      xhr.open("GET", url);
+      xhr.onload = () => { resolve(xhr.responseText); };
+      xhr.onerror = () => { reject(xhr.status); };
+      // use a different contextId to avoid auth cache.
+      xhr.setOriginAttributes({userContextId: 1});
+      xhr.send();
+    });
+  }
+
+  await Promise.all([
+    handlingExt.awaitMessage("onAuthRequired"),
+    fetch("http://mozilla.org"),
+  ]);
+  await handlingExt.unload();
+});

toolkit/modules/addons/WebRequest.jsm

@@ -833,7 +833,9 @@ HttpObserverManager = {
         try {
           let result = callback(data);
 
-          if (channel.canModify && result && typeof result === "object" && opts.blocking) {
+          // isProxy is set during onAuth if the auth request is for a proxy.
+          // We allow handling proxy auth regardless of canModify.
+          if ((channel.canModify || data.isProxy) && typeof result === "object" && opts.blocking) {
             handlerResults.push({opts, result});
           }
         } catch (e) {
@@ -865,6 +867,16 @@ HttpObserverManager = {
           }
         }
 
+        if (kind === "authRequired" && result.authCredentials && channel.authPromptCallback) {
+          channel.authPromptCallback(result.authCredentials);
+        }
+
+        // We allow proxy auth to cancel or handle authCredentials regardless of
+        // canModify, but ensure we do nothing else.
+        if (!channel.canModify) {
+          continue;
+        }
+
         if (result.cancel) {
           channel.suspended = false;
           channel.cancel(Cr.NS_ERROR_ABORT);
@@ -890,11 +902,8 @@ HttpObserverManager = {
         if (opts.responseHeaders && result.responseHeaders && responseHeaders) {
           responseHeaders.applyChanges(result.responseHeaders);
         }
-
-        if (kind === "authRequired" && result.authCredentials && channel.authPromptCallback) {
-          channel.authPromptCallback(result.authCredentials);
-        }
       }
+
       // If a listener did not cancel the request or provide credentials, we
       // forward the auth request to the base handler.
       if (kind === "authRequired" && channel.authPromptForward) {