refactor: extract elevation routines to lib/shared/permissions.js (#1351)

The elevation mechanism currently embedded in
`lib/child-writer/writer-proxy.js` is extracted as a separate re-usable
function in `lib/shared/permissions.js`.

This change hugely simplifies the writer proxy, while allowing us to
iterate faster on the elevation core functionality.

Signed-off-by: Juan Cruz Viotti <jviotti@openmailbox.org>

lib/child-writer/writer-proxy.js

@@ -1,5 +1,5 @@
 /*
- * Copyright 2016 resin.io
+ * Copyright 2017 resin.io
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -18,15 +18,12 @@
 
 const Bluebird = require('bluebird');
 const childProcess = require('child_process');
-const commandJoin = require('command-join');
 const ipc = require('node-ipc');
 const _ = require('lodash');
 const os = require('os');
 const path = require('path');
-const sudoPrompt = Bluebird.promisifyAll(require('sudo-prompt'));
 const utils = require('./utils');
 const EXIT_CODES = require('../shared/exit-codes');
-const errors = require('../shared/errors');
 const robot = require('../shared/robot');
 const permissions = require('../shared/permissions');
 const packageJSON = require('../../package.json');
@@ -72,61 +69,8 @@ return permissions.isElevated().then((elevated) => {
   if (!elevated) {
     console.log('Attempting to elevate');
 
-    if (os.platform() === 'win32') {
-      const elevator = Bluebird.promisifyAll(require('elevator'));
-
-      const commandArguments = [
-        'set',
-        'ELECTRON_RUN_AS_NODE=1',
-        '&&',
-        'set',
-        `IPC_SERVER_ID=${process.env.IPC_SERVER_ID}`,
-        '&&',
-        'set',
-        `IPC_CLIENT_ID=${process.env.IPC_CLIENT_ID}`,
-        '&&',
-
-        // This is a trick to make the binary afterwards catch
-        // the environment variables set just previously.
-        'call'
-
-      ].concat(process.argv);
-
-      // For debugging purposes
-      console.log(`Running: ${commandArguments.join(' ')}`);
-
-      return elevator.executeAsync(commandArguments, {
-        hidden: true,
-        terminating: true,
-        doNotPushdCurrentDirectory: true,
-        waitForTermination: true
-      }).catch({
-        code: 'ELEVATE_CANCELLED'
-      }, () => {
-        process.exit(EXIT_CODES.CANCELLED);
-      });
-    }
-
     const commandArguments = _.attempt(() => {
-      const commandPrefix = [
+      if (os.platform() === 'linux' && process.env.APPIMAGE && process.env.APPDIR) {
-
-        // Some elevation tools, like `pkexec` or `kdesudo`, don't
-        // provide a way to preserve the environment, therefore we
-        // have to make sure the environment variables we're interested
-        // in are manually inherited.
-        'env',
-        'ELECTRON_RUN_AS_NODE=1',
-        `IPC_SERVER_ID=${process.env.IPC_SERVER_ID}`,
-        `IPC_CLIENT_ID=${process.env.IPC_CLIENT_ID}`,
-
-        // This environment variable prevents the AppImages
-        // desktop integration script from presenting the
-        // "installation" dialog.
-        'SKIP=1'
-
-      ];
-
-      if (process.env.APPIMAGE && process.env.APPDIR) {
 
         // Translate the current arguments to point to the AppImage
         // Relative paths are resolved from `/tmp/.mount_XXXXXX/usr`
@@ -135,44 +79,32 @@ return permissions.isElevated().then((elevated) => {
           .invokeMap('replace', path.join(process.env.APPDIR, 'usr/'), '')
           .value();
 
-        return commandPrefix
+        return _.concat([ process.env.APPIMAGE ], translatedArguments);
-          .concat([ process.env.APPIMAGE ])
-          .concat(translatedArguments);
       }
 
-      return commandPrefix.concat(process.argv);
+      return process.argv;
     });
 
-    const command = commandJoin(commandArguments);
-
     // For debugging purposes
-    console.log(`Running: ${command}`);
+    console.log(`Running: ${commandArguments.join(' ')}`);
+
+    return permissions.elevateCommand(commandArguments, {
+      applicationName: packageJSON.displayName,
+      environment: {
+        ELECTRON_RUN_AS_NODE: 1,
+        IPC_SERVER_ID: process.env.IPC_SERVER_ID,
+        IPC_CLIENT_ID: process.env.IPC_CLIENT_ID,
+
+        // This environment variable prevents the AppImages
+        // desktop integration script from presenting the
+        // "installation" dialog.
+        SKIP: 1
 
-    return sudoPrompt.execAsync(command, {
-      name: packageJSON.displayName
-    }).then((stdout, stderr) => {
-      if (!_.isEmpty(stderr)) {
-        throw errors.createError({
-          title: stderr
-        });
       }
-
+    }).then((results) => {
-    // We're hardcoding internal error messages declared by `sudo-prompt`.
+      if (results.cancelled) {
-    // There doesn't seem to be a better way to handle these errors, so
+        process.exit(EXIT_CODES.CANCELLED);
-    // for now, we should make sure we double check if the error messages
+      }
-    // have changed every time we upgrade `sudo-prompt`.
-
-    }).catch({
-      message: 'User did not grant permission.'
-    }, () => {
-      process.exit(EXIT_CODES.CANCELLED);
-    }).catch({
-      message: 'No polkit authentication agent found.'
-    }, () => {
-      throw errors.createUserError({
-        title: 'No polkit authentication agent found',
-        description: 'Please install a polkit authentication agent for your desktop environment of choice to continue'
-      });
     });
   }
 

lib/shared/permissions.js

@@ -1,5 +1,5 @@
 /*
- * Copyright 2016 resin.io
+ * Copyright 2017 resin.io
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -19,7 +19,10 @@
 const os = require('os');
 const Bluebird = require('bluebird');
 const childProcess = Bluebird.promisifyAll(require('child_process'));
+const sudoPrompt = Bluebird.promisifyAll(require('sudo-prompt'));
+const commandJoin = require('command-join');
 const _ = require('lodash');
+const errors = require('./errors');
 
 /**
  * @summary The user id of the UNIX "superuser"
@@ -52,7 +55,7 @@ const UNIX_SUPERUSER_USER_ID = 0;
  * });
  */
 exports.isElevated = () => {
-  if (process.platform === 'win32') {
+  if (os.platform() === 'win32') {
 
     // `fltmc` is available on WinPE, XP, Vista, 7, 8, and 10
     // Works even when the "Server" service is disabled
@@ -67,3 +70,132 @@ exports.isElevated = () => {
 
   return Bluebird.resolve(process.geteuid() === UNIX_SUPERUSER_USER_ID);
 };
+
+/**
+ * @summary Get environment command prefix
+ * @function
+ * @private
+ *
+ * @param {Object} environment - environment map
+ * @returns {String[]} command arguments
+ *
+ * @example
+ * const commandPrefix = permissions.getEnvironmentCommandPrefix({
+ *   FOO: 'bar',
+ *   BAR: 'baz'
+ * });
+ *
+ * childProcess.execSync(_.join(_.concat(commandPrefix, [ 'mycommand' ]), ' '));
+ */
+exports.getEnvironmentCommandPrefix = (environment) => {
+  const isWindows = os.platform() === 'win32';
+
+  if (_.isEmpty(environment)) {
+    return [];
+  }
+
+  const argv = _.flatMap(environment, (value, key) => {
+    if (_.isNil(value)) {
+      return [];
+    }
+
+    if (isWindows) {
+      return [ 'set', `${key}=${value}`, '&&' ];
+    }
+
+    return [ `${key}=${value}` ];
+  });
+
+  if (isWindows) {
+
+    // This is a trick to make the binary afterwards catch
+    // the environment variables set just previously.
+    return _.concat(argv, [ 'call' ]);
+
+  }
+
+  return _.concat([ 'env' ], argv);
+};
+
+/**
+ * @summary Elevate a command
+ * @function
+ * @public
+ *
+ * @param {String[]} command - command arguments
+ * @param {Object} options - options
+ * @param {String} options.applicationName - application name
+ * @param {Object} options.environment - environment variables
+ * @fulfil {Object} - elevation results
+ * @returns {Promise}
+ *
+ * @example
+ * permissions.elevateCommand([ 'foo', 'bar' ], {
+ *   applicationName: 'My App',
+ *   environment: {
+ *     FOO: 'bar'
+ *   }
+ * }).then((results) => {
+ *   if (results.cancelled) {
+ *     console.log('Elevation has been cancelled');
+ *   }
+ * });
+ */
+exports.elevateCommand = (command, options) => {
+  const prefixedCommand = _.concat(exports.getEnvironmentCommandPrefix(options.environment), command);
+
+  if (os.platform() === 'win32') {
+    const elevator = Bluebird.promisifyAll(require('elevator'));
+
+    return elevator.executeAsync(prefixedCommand, {
+      hidden: true,
+      terminating: true,
+      doNotPushdCurrentDirectory: true,
+      waitForTermination: true
+    }).then(() => {
+      return {
+        cancelled: false
+      };
+    }).catch({
+      code: 'ELEVATE_CANCELLED'
+    }, () => {
+      return {
+        cancelled: true
+      };
+    });
+  }
+
+  return sudoPrompt.execAsync(commandJoin(prefixedCommand), {
+    name: options.applicationName
+  }).then((stdout, stderr) => {
+    if (!_.isEmpty(stderr)) {
+      throw errors.createError({
+        title: stderr
+      });
+    }
+
+    return {
+      cancelled: false
+    };
+
+  // We're hardcoding internal error messages declared by `sudo-prompt`.
+  // There doesn't seem to be a better way to handle these errors, so
+  // for now, we should make sure we double check if the error messages
+  // have changed every time we upgrade `sudo-prompt`.
+
+  }).catch({
+    message: 'User did not grant permission.'
+  }, () => {
+    return {
+      cancelled: true
+    };
+  }).catch({
+    message: 'No polkit authentication agent found.'
+  }, () => {
+    throw errors.createUserError({
+      title: 'No polkit authentication agent found',
+      description: 'Please install a polkit authentication agent for your desktop environment of choice to continue'
+    });
+  });
+
+};

tests/shared/permissions.spec.js

@@ -0,0 +1,246 @@
+/*
+ * Copyright 2017 resin.io
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+'use strict';
+
+const m = require('mochainon');
+const os = require('os');
+const permissions = require('../../lib/shared/permissions');
+
+describe('Shared: permissions', function() {
+
+  describe('.getEnvironmentCommandPrefix()', function() {
+
+    describe('given windows', function() {
+
+      beforeEach(function() {
+        this.osPlatformStub = m.sinon.stub(os, 'platform');
+        this.osPlatformStub.returns('win32');
+      });
+
+      afterEach(function() {
+        this.osPlatformStub.restore();
+      });
+
+      it('should return an empty array if no environment', function() {
+        m.chai.expect(permissions.getEnvironmentCommandPrefix()).to.deep.equal([]);
+      });
+
+      it('should return an empty array environment is an empty object', function() {
+        m.chai.expect(permissions.getEnvironmentCommandPrefix({})).to.deep.equal([]);
+      });
+
+      it('should create an environment command prefix out of one variable', function() {
+        m.chai.expect(permissions.getEnvironmentCommandPrefix({
+          FOO: 'bar'
+        })).to.deep.equal([
+          'set',
+          'FOO=bar',
+          '&&',
+          'call'
+        ]);
+      });
+
+      it('should create an environment command prefix out of many variables', function() {
+        m.chai.expect(permissions.getEnvironmentCommandPrefix({
+          FOO: 'bar',
+          BAR: 'baz',
+          BAZ: 'qux'
+        })).to.deep.equal([
+          'set',
+          'FOO=bar',
+          '&&',
+          'set',
+          'BAR=baz',
+          '&&',
+          'set',
+          'BAZ=qux',
+          '&&',
+          'call'
+        ]);
+      });
+
+      it('should ignore undefined and null variable values', function() {
+        m.chai.expect(permissions.getEnvironmentCommandPrefix({
+          FOO: null,
+          BAR: undefined,
+          BAZ: 'qux'
+        })).to.deep.equal([
+          'set',
+          'BAZ=qux',
+          '&&',
+          'call'
+        ]);
+      });
+
+      it('should stringify number values', function() {
+        m.chai.expect(permissions.getEnvironmentCommandPrefix({
+          FOO: 1,
+          BAR: 0,
+          BAZ: -1
+        })).to.deep.equal([
+          'set',
+          'FOO=1',
+          '&&',
+          'set',
+          'BAR=0',
+          '&&',
+          'set',
+          'BAZ=-1',
+          '&&',
+          'call'
+        ]);
+      });
+
+    });
+
+    describe('given linux', function() {
+
+      beforeEach(function() {
+        this.osPlatformStub = m.sinon.stub(os, 'platform');
+        this.osPlatformStub.returns('linux');
+      });
+
+      afterEach(function() {
+        this.osPlatformStub.restore();
+      });
+
+      it('should return an empty array if no environment', function() {
+        m.chai.expect(permissions.getEnvironmentCommandPrefix()).to.deep.equal([]);
+      });
+
+      it('should return an empty array environment is an empty object', function() {
+        m.chai.expect(permissions.getEnvironmentCommandPrefix({})).to.deep.equal([]);
+      });
+
+      it('should create an environment command prefix out of one variable', function() {
+        m.chai.expect(permissions.getEnvironmentCommandPrefix({
+          FOO: 'bar'
+        })).to.deep.equal([
+          'env',
+          'FOO=bar'
+        ]);
+      });
+
+      it('should create an environment command prefix out of many variables', function() {
+        m.chai.expect(permissions.getEnvironmentCommandPrefix({
+          FOO: 'bar',
+          BAR: 'baz',
+          BAZ: 'qux'
+        })).to.deep.equal([
+          'env',
+          'FOO=bar',
+          'BAR=baz',
+          'BAZ=qux'
+        ]);
+      });
+
+      it('should ignore undefined and null variable values', function() {
+        m.chai.expect(permissions.getEnvironmentCommandPrefix({
+          FOO: null,
+          BAR: undefined,
+          BAZ: 'qux'
+        })).to.deep.equal([
+          'env',
+          'BAZ=qux'
+        ]);
+      });
+
+      it('should stringify number values', function() {
+        m.chai.expect(permissions.getEnvironmentCommandPrefix({
+          FOO: 1,
+          BAR: 0,
+          BAZ: -1
+        })).to.deep.equal([
+          'env',
+          'FOO=1',
+          'BAR=0',
+          'BAZ=-1'
+        ]);
+      });
+
+    });
+
+    describe('given darwin', function() {
+
+      beforeEach(function() {
+        this.osPlatformStub = m.sinon.stub(os, 'platform');
+        this.osPlatformStub.returns('darwin');
+      });
+
+      afterEach(function() {
+        this.osPlatformStub.restore();
+      });
+
+      it('should return an empty array if no environment', function() {
+        m.chai.expect(permissions.getEnvironmentCommandPrefix()).to.deep.equal([]);
+      });
+
+      it('should return an empty array environment is an empty object', function() {
+        m.chai.expect(permissions.getEnvironmentCommandPrefix({})).to.deep.equal([]);
+      });
+
+      it('should create an environment command prefix out of one variable', function() {
+        m.chai.expect(permissions.getEnvironmentCommandPrefix({
+          FOO: 'bar'
+        })).to.deep.equal([
+          'env',
+          'FOO=bar'
+        ]);
+      });
+
+      it('should create an environment command prefix out of many variables', function() {
+        m.chai.expect(permissions.getEnvironmentCommandPrefix({
+          FOO: 'bar',
+          BAR: 'baz',
+          BAZ: 'qux'
+        })).to.deep.equal([
+          'env',
+          'FOO=bar',
+          'BAR=baz',
+          'BAZ=qux'
+        ]);
+      });
+
+      it('should ignore undefined and null variable values', function() {
+        m.chai.expect(permissions.getEnvironmentCommandPrefix({
+          FOO: null,
+          BAR: undefined,
+          BAZ: 'qux'
+        })).to.deep.equal([
+          'env',
+          'BAZ=qux'
+        ]);
+      });
+
+      it('should stringify number values', function() {
+        m.chai.expect(permissions.getEnvironmentCommandPrefix({
+          FOO: 1,
+          BAR: 0,
+          BAZ: -1
+        })).to.deep.equal([
+          'env',
+          'FOO=1',
+          'BAR=0',
+          'BAZ=-1'
+        ]);
+      });
+
+    });
+
+  });
+
+});