Bug 1019714 - Forbid overloading apps not being pushed via devtools. r=jryans, r=fabrice

dom/apps/AppsUtils.jsm

@@ -108,6 +108,7 @@ function _setAppProperties(aObj, aApp) {
   aObj.widgetPages = aApp.widgetPages || [];
   aObj.kind = aApp.kind;
   aObj.enabled = aApp.enabled !== undefined ? aApp.enabled : true;
+  aObj.sideloaded = aApp.sideloaded;
 }
 
 this.AppsUtils = {

toolkit/devtools/apps/tests/data/mochitest.ini

@@ -4,3 +4,5 @@ support-files =
   app-updated.zip
   app.zip
   app-certified.zip
+  app-overload.zip
+  app-system.zip

toolkit/devtools/apps/tests/debugger-protocol-helper.js

@@ -145,6 +145,18 @@ addMessageListener("addFrame", function (aMessage) {
   sendAsyncMessage("frameAdded");
 });
 
+addMessageListener("tweak-app-object", function (aMessage) {
+  let appId = aMessage.appId;
+  Cu.import('resource://gre/modules/Webapps.jsm');
+  let reg = DOMApplicationRegistry;
+  if ("removable" in aMessage) {
+    reg.webapps[appId].removable = aMessage.removable;
+  }
+  if ("sideloaded" in aMessage) {
+    reg.webapps[appId].sideloaded = aMessage.sideloaded;
+  }
+});
+
 addMessageListener("cleanup", function () {
   webappActorRequest({type: "unwatchApps"}, function () {
     gClient.close();

toolkit/devtools/apps/tests/test_webapps_actor.html

@@ -65,6 +65,7 @@ const PACKAGED_APP_MANIFEST = PACKAGED_APP_ORIGIN + "/manifest.webapp";
 const CERTIFIED_APP_ID = "test-certified-id";
 const CERTIFIED_APP_ORIGIN = "app://" + CERTIFIED_APP_ID;
 const CERTIFIED_APP_MANIFEST = CERTIFIED_APP_ORIGIN + "/manifest.webapp";
+const SYSTEM_APP_ID = "test-system-id";
 
 var steps = [
   function() {
@@ -96,26 +97,31 @@ var steps = [
     installTestApp = function (url, appId, callback) {
       let installResponse, appObject;
       let installedEvent = false;
-      mm.addMessageListener("installed", function onInstalled(aResponse) {
-        mm.removeMessageListener("installed", onInstalled);
+      function onInstalled(aResponse) {
         ok(true, "install request replied");
         installResponse = aResponse;
         checkEnd();
-      });
-      mm.addMessageListener("installed-event", function onInstalledEvent(aResponse) {
-        mm.removeMessageListener("installed-event", onInstalledEvent);
+      }
+      mm.addMessageListener("installed", onInstalled);
+      function onInstalledEvent(aResponse) {
         ok(true, "received appInstall actor event");
         installedEvent = true;
         checkEnd();
-      });
+      }
+      mm.addMessageListener("installed-event", onInstalledEvent);
       navigator.mozApps.mgmt.oninstall = function(evt) {
         appObject = evt.application;
         ok(true, "mozApps.mgmt install event fired");
         checkEnd();
       };
       function checkEnd() {
-        if (appObject && installResponse && installedEvent)
+        if ( (appObject && installResponse && installedEvent) ||
+             (installResponse && installResponse.error) ) {
+          mm.removeMessageListener("installed", onInstalled);
+          mm.removeMessageListener("installed-event", onInstalledEvent);
+          navigator.mozApps.mgmt.oninstall = null;
           callback(installResponse, appObject);
+        }
       }
       mm.sendAsyncMessage("install", {url: url, appId: appId});
     };
@@ -178,9 +184,40 @@ var steps = [
       }
     );
   },
+  function() {
+    info("== TEST == Try overloading non-removable app");
+    let url = SimpleTest.getTestFileURL("data/app-overload.zip");
+    installTestApp(url, CERTIFIED_APP_ID,
+      function (aResponse, aApp) {
+        // First time we install the app, it works just fine
+        ok(true, "Installed");
+        is(aResponse.appId, "overload.gaiamobile.org", "Got overloaded app id");
+        if ("error" in aResponse) {
+          ok(false, "Error: " + aResponse.error);
+        }
+        if ("message" in aResponse) {
+          ok(false, "Error message: " + aResponse.message);
+        }
+        ok(!("error" in aResponse), "app installed without any error");
+        is(aApp.manifest.name, "System app", "app name is correct");
+
+        // Then use some magic to make it non-removable
+        mm.sendAsyncMessage("tweak-app-object", {appId: CERTIFIED_APP_ID, removable: false});
+
+        // Then when trying to install it again, it will be rejected
+        installTestApp(url, CERTIFIED_APP_ID,
+          function (aResponse, aApp) {
+            is(aResponse.error, "installationFailed", "Overloading non-removable app without the pref is rejected");
+            is(aResponse.message, "The application " + CERTIFIED_APP_ID + " can't be overridden.");
+            next();
+          });
+
+      }
+    );
+  },
   function() {
     info("== TEST == Get all apps");
-    getAll(false);
+    getAll(true);
   },
   function() {
     info("== TEST == Get packaged app");
@@ -194,7 +231,7 @@ var steps = [
     getApp({
       id: CERTIFIED_APP_ID,
       manifestURL: CERTIFIED_APP_MANIFEST
-    }, false);
+    }, true);
   },
   function() {
     info("== SETUP == Enable certified app access");
@@ -220,6 +257,25 @@ var steps = [
       manifestURL: CERTIFIED_APP_MANIFEST
     }, true);
   },
+  function() {
+    info("== TEST == Overload of non-removable apps is allowed with CERTIFIED ENABLE");
+    let url = SimpleTest.getTestFileURL("data/app-overload.zip");
+    installTestApp(url, SYSTEM_APP_ID,
+      function (aResponse, aApp) {
+        ok(true, "Installed");
+        is(aResponse.appId, "overload.gaiamobile.org", "Got overloaded app id");
+        if ("error" in aResponse) {
+          ok(false, "Error: " + aResponse.error);
+        }
+        if ("message" in aResponse) {
+          ok(false, "Error message: " + aResponse.message);
+        }
+        ok(!("error" in aResponse), "app installed without any error");
+        is(aApp.manifest.name, "System app", "app name is correct");
+        next();
+      }
+    );
+  },
   function() {
     info("== SETUP == Disable certified app access");
     SpecialPowers.popPrefEnv(next);
@@ -245,7 +301,19 @@ var steps = [
   },
   function() {
     info("== TEST == Uninstall certified app");
-    uninstall(CERTIFIED_APP_MANIFEST);
+    // Make the app removable again, but make it appear as a regular app (not being pushed via devtools)
+    mm.sendAsyncMessage("tweak-app-object", {appId: CERTIFIED_APP_ID, removable: true, sideloaded: false});
+
+    mm.addMessageListener("appActorResponse", function onResponse(response) {
+      mm.removeMessageListener("appActorResponse", onResponse);
+      is(response.error, "forbidden", "Uninstalling apps that have not being sideloaded is forbidden");
+      next();
+    });
+
+    mm.sendAsyncMessage("appActorRequest", {
+      type: "uninstall",
+      manifestURL: CERTIFIED_APP_MANIFEST
+    });
   },
   function() {
     ok(true, "all done!\n");

toolkit/devtools/apps/tests/unit/test_webappsActor.js

@@ -270,6 +270,22 @@ add_test(function testCheckHostedApp() {
   });
 });
 
+add_test(function testInstallOverrideSystem() {
+  let appId = "actor-test"; // Match app.zip id
+
+  // Make the test app non-removable, like system apps
+  DOMApplicationRegistry.webapps[appId].removable = false;
+
+  let packageFile = do_get_file("data/app.zip");
+  gActorFront.installPackaged(packageFile.path, appId)
+    .then(function ({ appId }) {
+      do_throw("Override of a non-removable app has been accepted.");
+    }, function (e) {
+      do_check_eq(e.message, "The application " + appId + " can't be overridden.");
+      run_next_test();
+    });
+});
+
 function run_test() {
   setup();
 

toolkit/devtools/server/actors/webapps.js

@@ -244,6 +244,11 @@ WebappsActor.prototype = {
     let reg = DOMApplicationRegistry;
     let self = this;
 
+    if (aId in reg.webapps && !reg.webapps[aId].sideloaded &&
+        !this._isUnrestrictedAccessAllowed()) {
+      throw new Error("Replacing non-sideloaded apps is not permitted.");
+    }
+
     // Clean up the deprecated manifest cache if needed.
     if (aId in reg._manifestCache) {
       delete reg._manifestCache[aId];
@@ -256,6 +261,7 @@ WebappsActor.prototype = {
     aApp.basePath = reg.getWebAppsBasePath();
     aApp.localId = (aId in reg.webapps) ? reg.webapps[aId].localId
                                         : reg._nextLocalId();
+    aApp.sideloaded = true;
 
     reg.webapps[aId] = aApp;
     reg.updatePermissionsForApp(aId);
@@ -383,24 +389,21 @@ WebappsActor.prototype = {
         return AppsUtils.loadJSONAsync(manFile);
       }
     }
-    function checkSideloading(aManifest) {
-      return self._getAppType(aManifest.type);
-    }
-    function writeManifest(aAppType) {
+    function writeManifest(resolution) {
       // Move manifest.webapp to the destination directory.
       // The destination directory for this app.
       let installDir = DOMApplicationRegistry._getAppDir(aId);
       if (aManifest) {
         let manFile = OS.Path.join(installDir.path, "manifest.webapp");
         return DOMApplicationRegistry._writeFile(manFile, JSON.stringify(aManifest)).then(() => {
-          return aAppType;
+          return resolution;
         });
       } else {
         let manFile = aDir.clone();
         manFile.append("manifest.webapp");
         manFile.moveTo(installDir, "manifest.webapp");
       }
-      return null;
+      return promise.resolve(resolution);
     }
     function readMetadata(aAppType) {
       if (aMetadata) {
@@ -413,7 +416,7 @@ WebappsActor.prototype = {
           throw("Error parsing metadata.json.");
         }
         if (!aMetadata.origin) {
-          throw("Missing 'origin' property in metadata.json");
+          throw("Missing 'origin' property in metadata.json.");
         }
         return { metadata: aMetadata, appType: aAppType };
       });
@@ -421,9 +424,8 @@ WebappsActor.prototype = {
     let runnable = {
       run: function run() {
         try {
+          let metadata, appType;
           readManifest().
-            then(writeManifest).
-            then(checkSideloading).
             then(readMetadata).
             then(function ({ metadata, appType }) {
               let origin = metadata.origin;
@@ -438,6 +440,8 @@ WebappsActor.prototype = {
                 receipts: aReceipts,
               };
 
+              return writeManifest(app);
+            }).then(function (app) {
               self._registerApp(deferred, app, aId, aDir);
             }, function (error) {
               self._sendError(deferred, error, aId);
@@ -483,6 +487,7 @@ WebappsActor.prototype = {
             manifest = JSON.parse(jsonString);
           } catch(e) {
             self._sendError(deferred, "Error Parsing manifest.webapp: " + e, aId);
+            return;
           }
 
           let appType = self._getAppType(manifest.type);
@@ -505,6 +510,14 @@ WebappsActor.prototype = {
             id = uri.prePath.substring(6);
           }
 
+          // Prevent overriding preinstalled apps
+          if (id in DOMApplicationRegistry.webapps &&
+              DOMApplicationRegistry.webapps[id].removable === false &&
+              !self._isUnrestrictedAccessAllowed()) {
+            self._sendError(deferred, "The application " + id + " can't be overridden.");
+            return;
+          }
+
           // Only after security checks are made and after final app id is computed
           // we can move application.zip to the destination directory, and
           // extract manifest.webapp there.
@@ -586,9 +599,9 @@ WebappsActor.prototype = {
 
     // Check that we are not overriding a preinstalled application.
     if (appId in reg.webapps && reg.webapps[appId].removable === false) {
-      return { error: "badParameterType",
-               message: "The application " + appId + " can't be overriden."
-             }
+      return { error: "installationFailed",
+               message: "The application " + appId + " can't be overridden."
+             };
     }
 
     let appDir = FileUtils.getDir("TmpD", ["b2g", appId], false, false);
@@ -680,38 +693,36 @@ WebappsActor.prototype = {
       return { error: "appNotFound" };
     }
 
-    return this._isAppAllowedForURL(app.manifestURL).then(allowed => {
-      if (!allowed) {
-        return { error: "forbidden" };
-      }
-      return reg.getManifestFor(manifestURL).then(function (manifest) {
-        app.manifest = manifest;
-        return { app: app };
-      });
+    if (!this._isAppAllowedForURL(app.manifestURL)) {
+      return { error: "forbidden" };
+    }
+
+    return reg.getManifestFor(manifestURL).then(function (manifest) {
+      app.manifest = manifest;
+      return { app: app };
     });
   },
 
-  _areCertifiedAppsAllowed: function wa__areCertifiedAppsAllowed() {
+  _isUnrestrictedAccessAllowed: function() {
     let pref = "devtools.debugger.forbid-certified-apps";
     return !Services.prefs.getBoolPref(pref);
   },
 
-  _isAppAllowedForManifest: function wa__isAppAllowedForManifest(aManifest) {
-    if (this._areCertifiedAppsAllowed()) {
+  _isAppAllowed: function(aApp) {
+    if (this._isUnrestrictedAccessAllowed()) {
       return true;
     }
-    let type = this._getAppType(aManifest.type);
-    return type !== Ci.nsIPrincipal.APP_STATUS_CERTIFIED;
+    return aApp.sideloaded;
   },
 
   _filterAllowedApps: function wa__filterAllowedApps(aApps) {
-    return aApps.filter(app => this._isAppAllowedForManifest(app.manifest));
+    return aApps.filter(app => this._isAppAllowed(app));
   },
 
   _isAppAllowedForURL: function wa__isAppAllowedForURL(aManifestURL) {
-    return this._findManifestByURL(aManifestURL).then(manifest => {
-      return this._isAppAllowedForManifest(manifest);
-    });
+    let reg = DOMApplicationRegistry;
+    let app = reg.getAppByManifestURL(aManifestURL);
+    return this._isAppAllowed(app);
   },
 
   uninstall: function wa_actorUninstall(aRequest) {
@@ -719,8 +730,12 @@ WebappsActor.prototype = {
 
     let manifestURL = aRequest.manifestURL;
     if (!manifestURL) {
-      return Promise.resolve({ error: "missingParameter",
-                         message: "missing parameter manifestURL" });
+      return { error: "missingParameter",
+               message: "missing parameter manifestURL" };
+    }
+
+    if (!this._isAppAllowedForURL(manifestURL)) {
+      return { error: "forbidden" };
     }
 
     return DOMApplicationRegistry.uninstall(manifestURL);
@@ -882,17 +897,12 @@ WebappsActor.prototype = {
       if (apps.indexOf(manifestURL) != -1) {
         continue;
       }
-
-      appPromises.push(this._isAppAllowedForURL(manifestURL).then(allowed => {
-        if (allowed) {
-          apps.push(manifestURL);
-        }
-      }));
+      if (this._isAppAllowedForURL(manifestURL)) {
+        apps.push(manifestURL);
+      }
     }
 
-    return promise.all(appPromises).then(() => {
-      return { apps: apps };
-    });
+    return { apps: apps };
   },
 
   getAppActor: function ({ manifestURL }) {
@@ -927,32 +937,30 @@ WebappsActor.prototype = {
       return notFoundError;
     }
 
-    return this._isAppAllowedForURL(manifestURL).then(allowed => {
-      if (!allowed) {
-        return notFoundError;
-      }
+    if (!this._isAppAllowedForURL(manifestURL)) {
+      return notFoundError;
+    }
 
-      // Only create a new actor, if we haven't already
-      // instanciated one for this connection.
-      let map = this._appActorsMap;
-      let mm = appFrame.QueryInterface(Ci.nsIFrameLoaderOwner)
-                       .frameLoader
-                       .messageManager;
-      let actor = map.get(mm);
-      if (!actor) {
-        let onConnect = actor => {
-          map.set(mm, actor);
-          return { actor: actor };
-        };
-        let onDisconnect = mm => {
-          map.delete(mm);
-        };
-        return DebuggerServer.connectToChild(this.conn, appFrame, onDisconnect)
-                             .then(onConnect);
-      }
+    // Only create a new actor, if we haven't already
+    // instanciated one for this connection.
+    let map = this._appActorsMap;
+    let mm = appFrame.QueryInterface(Ci.nsIFrameLoaderOwner)
+                     .frameLoader
+                     .messageManager;
+    let actor = map.get(mm);
+    if (!actor) {
+      let onConnect = actor => {
+        map.set(mm, actor);
+        return { actor: actor };
+      };
+      let onDisconnect = mm => {
+        map.delete(mm);
+      };
+      return DebuggerServer.connectToChild(this.conn, appFrame, onDisconnect)
+                           .then(onConnect);
+    }
 
-      return { actor: actor };
-    });
+    return { actor: actor };
   },
 
   watchApps: function () {
@@ -988,14 +996,12 @@ WebappsActor.prototype = {
       return;
     }
 
-    this._isAppAllowedForURL(manifestURL).then(allowed => {
-      if (allowed) {
-        this.conn.send({ from: this.actorID,
-                         type: "appOpen",
-                         manifestURL: manifestURL
-                       });
-      }
-    });
+    if (this._isAppAllowedForURL(manifestURL)) {
+      this.conn.send({ from: this.actorID,
+                       type: "appOpen",
+                       manifestURL: manifestURL
+                     });
+    }
   },
 
   onFrameDestroyed: function (frame, isLastAppFrame) {
@@ -1010,14 +1016,12 @@ WebappsActor.prototype = {
       return;
     }
 
-    this._isAppAllowedForURL(manifestURL).then(allowed => {
-      if (allowed) {
-        this.conn.send({ from: this.actorID,
-                         type: "appClose",
-                         manifestURL: manifestURL
-                       });
-      }
-    });
+    if (this._isAppAllowedForURL(manifestURL)) {
+      this.conn.send({ from: this.actorID,
+                       type: "appClose",
+                       manifestURL: manifestURL
+                     });
+    }
   },
 
   observe: function (subject, topic, data) {