Add options in [Accounts] for user/group commands (#443)

Add the follow new options in the [Accounts] section of the
configuration file:
useradd_cmd
userdel_cmd
usermod_cmd
groupadd_cmd

This allows the user to specifies which commands and arguments to use.
It also allows easier customization for other platforms as BSD
systems.

5 files changed, 84 insertions, 39 deletions

diff --git a/google_compute_engine/accounts/accounts_daemon.py b/google_compute_engine/accounts/accounts_daemon.py
index ab63e9b..aec123d 100755
--- a/google_compute_engine/accounts/accounts_daemon.py
+++ b/google_compute_engine/accounts/accounts_daemon.py
@@ -37,12 +37,18 @@ class AccountsDaemon(object):
   invalid_users = set()
   user_ssh_keys = {}
 
-  def __init__(self, groups=None, remove=False, debug=False):
+  def __init__(
+      self, groups=None, remove=False, useradd_cmd=None, userdel_cmd=None,
+      usermod_cmd=None, groupadd_cmd=None, debug=False):
     """Constructor.
 
     Args:
       groups: string, a comma separated list of groups.
       remove: bool, True if deprovisioning a user should be destructive.
+      useradd_cmd: string, command to create a new user.
+      userdel_cmd: string, command to delete a user.
+      usermod_cmd: string, command to modify user's groups.
+      groupadd_cmd: string, command to add a new group.
       debug: bool, True if debug output should write to the console.
     """
     facility = logging.handlers.SysLogHandler.LOG_DAEMON
@@ -50,7 +56,9 @@ class AccountsDaemon(object):
         name='google-accounts', debug=debug, facility=facility)
     self.watcher = metadata_watcher.MetadataWatcher(logger=self.logger)
     self.utils = accounts_utils.AccountsUtils(
-        logger=self.logger, groups=groups, remove=remove)
+        logger=self.logger, groups=groups, remove=remove,
+        useradd_cmd=useradd_cmd, userdel_cmd=userdel_cmd,
+        usermod_cmd=usermod_cmd, groupadd_cmd=groupadd_cmd)
     try:
       with file_utils.LockFile(LOCKFILE):
         self.logger.info('Starting Google Accounts daemon.')
@@ -226,6 +234,10 @@ def main():
     AccountsDaemon(
         groups=instance_config.GetOptionString('Accounts', 'groups'),
         remove=instance_config.GetOptionBool('Accounts', 'deprovision_remove'),
+        useradd_cmd=instance_config.GetOptionBool('Accounts', 'useradd_cmd'),
+        userdel_cmd=instance_config.GetOptionBool('Accounts', 'userdel_cmd'),
+        usermod_cmd=instance_config.GetOptionBool('Accounts', 'usermod_cmd'),
+        groupadd_cmd=instance_config.GetOptionBool('Accounts', 'groupadd_cmd'),
         debug=bool(options.debug))
 
 
diff --git a/google_compute_engine/accounts/accounts_utils.py b/google_compute_engine/accounts/accounts_utils.py
index 7590649..9608c86 100644
--- a/google_compute_engine/accounts/accounts_utils.py
+++ b/google_compute_engine/accounts/accounts_utils.py
@@ -27,6 +27,10 @@ from google_compute_engine import constants
 from google_compute_engine import file_utils
 
 USER_REGEX = re.compile(r'\A[A-Za-z0-9._][A-Za-z0-9._-]*\Z')
+DEFAULT_USERADD_CMD = 'useradd -m -s /bin/bash -p * {user}'
+DEFAULT_USERDEL_CMD = 'userdel -r {user}'
+DEFAULT_USERMOD_CMD = 'groupadd {group}'
+DEFAULT_GROUPADD_CMD = 'groupadd {group}'
 
 
 class AccountsUtils(object):
@@ -34,14 +38,24 @@ class AccountsUtils(object):
 
   google_comment = '# Added by Google'
 
-  def __init__(self, logger, groups=None, remove=False):
+  def __init__(
+      self, logger, groups=None, remove=False, useradd_cmd=None,
+      userdel_cmd=None, usermod_cmd=None, groupadd_cmd=None):
     """Constructor.
 
     Args:
       logger: logger object, used to write to SysLog and serial port.
       groups: string, a comma separated list of groups.
       remove: bool, True if deprovisioning a user should be destructive.
+      useradd_cmd: string, command to create a new user.
+      userdel_cmd: string, command to delete a user.
+      usermod_cmd: string, command to modify user's groups.
+      groupadd_cmd: string, command to add a new group.
     """
+    self.useradd_cmd = useradd_cmd or DEFAULT_USERADD_CMD
+    self.userdel_cmd = userdel_cmd or DEFAULT_USERDEL_CMD
+    self.usermod_cmd = usermod_cmd or DEFAULT_USERMOD_CMD
+    self.groupadd_cmd = groupadd_cmd or DEFAULT_GROUPADD_CMD
     self.logger = logger
     self.google_sudoers_group = 'google-sudoers'
     self.google_sudoers_file = (
@@ -73,7 +87,9 @@ class AccountsUtils(object):
     """Create a Linux group for Google added sudo user accounts."""
     if not self._GetGroup(self.google_sudoers_group):
       try:
-        subprocess.check_call(['groupadd', self.google_sudoers_group])
+        subprocess.check_call(
+            self.groupadd_cmd.format(group=self.google_sudoers_group),
+            shell=True)
       except subprocess.CalledProcessError as e:
         self.logger.warning('Could not create the sudoers group. %s.', str(e))
 
@@ -117,20 +133,9 @@ class AccountsUtils(object):
     """
     self.logger.info('Creating a new user account for %s.', user)
 
-    # The encrypted password is set to '*' for SSH on Linux systems
-    # without PAM.
-    #
-    # SSH uses '!' as its locked account token:
-    # https://github.com/openssh/openssh-portable/blob/master/configure.ac
-    #
-    # When the token is specified, SSH denies login:
-    # https://github.com/openssh/openssh-portable/blob/master/auth.c
-    #
-    # To solve the issue, make the password '*' which is also recognized
-    # as locked but does not prevent SSH login.
-    command = ['useradd', '-m', '-s', '/bin/bash', '-p', '*', user]
+    command = self.useradd_cmd.format(user=user)
     try:
-      subprocess.check_call(command)
+      subprocess.check_call(command, shell=True)
     except subprocess.CalledProcessError as e:
       self.logger.warning('Could not create user %s. %s.', user, str(e))
       return False
@@ -150,9 +155,9 @@ class AccountsUtils(object):
     """
     groups = ','.join(groups)
     self.logger.debug('Updating user %s with groups %s.', user, groups)
-    command = ['usermod', '-G', groups, user]
+    command = self.usermod_cmd.format(user=user, groups=groups)
     try:
-      subprocess.check_call(command)
+      subprocess.check_call(command, shell=True)
     except subprocess.CalledProcessError as e:
       self.logger.warning('Could not update user %s. %s.', user, str(e))
       return False
@@ -319,9 +324,9 @@ class AccountsUtils(object):
     """
     self.logger.info('Removing user %s.', user)
     if self.remove:
-      command = ['userdel', '-r', user]
+      command = self.userdel_cmd.format(user=user)
       try:
-        subprocess.check_call(command)
+        subprocess.check_call(command, shell=True)
       except subprocess.CalledProcessError as e:
         self.logger.warning('Could not remove user %s. %s.', user, str(e))
       else:
diff --git a/google_compute_engine/accounts/tests/accounts_daemon_test.py b/google_compute_engine/accounts/tests/accounts_daemon_test.py
index 805c3f7..b6e9a28 100644
--- a/google_compute_engine/accounts/tests/accounts_daemon_test.py
+++ b/google_compute_engine/accounts/tests/accounts_daemon_test.py
@@ -54,7 +54,9 @@ class AccountsDaemonTest(unittest.TestCase):
           mock.call.logger.Logger(name=mock.ANY, debug=True, facility=mock.ANY),
           mock.call.watcher.MetadataWatcher(logger=mock_logger_instance),
           mock.call.utils.AccountsUtils(
-              logger=mock_logger_instance, groups='foo,bar', remove=True),
+              logger=mock_logger_instance, groups='foo,bar', remove=True,
+              useradd_cmd=mock.ANY, userdel_cmd=mock.ANY, usermod_cmd=mock.ANY,
+              groupadd_cmd=mock.ANY),
           mock.call.lock.LockFile(accounts_daemon.LOCKFILE),
           mock.call.lock.LockFile().__enter__(),
           mock.call.logger.Logger().info(mock.ANY),
@@ -85,7 +87,9 @@ class AccountsDaemonTest(unittest.TestCase):
               name=mock.ANY, debug=False, facility=mock.ANY),
           mock.call.watcher.MetadataWatcher(logger=mock_logger_instance),
           mock.call.utils.AccountsUtils(
-              logger=mock_logger_instance, groups=None, remove=False),
+              logger=mock_logger_instance, groups=None, remove=False,
+              useradd_cmd=mock.ANY, userdel_cmd=mock.ANY, usermod_cmd=mock.ANY,
+              groupadd_cmd=mock.ANY),
           mock.call.lock.LockFile(accounts_daemon.LOCKFILE),
           mock.call.logger.Logger().warning('Test Error'),
       ]
diff --git a/google_compute_engine/accounts/tests/accounts_utils_test.py b/google_compute_engine/accounts/tests/accounts_utils_test.py
index 7ced004..b8be8bb 100644
--- a/google_compute_engine/accounts/tests/accounts_utils_test.py
+++ b/google_compute_engine/accounts/tests/accounts_utils_test.py
@@ -31,6 +31,10 @@ class AccountsUtilsTest(unittest.TestCase):
     self.sudoers_file = '/sudoers/file'
     self.users_dir = '/users'
     self.users_file = '/users/file'
+    self.useradd_cmd = 'useradd -m -s /bin/bash -p * {user}'
+    self.userdel_cmd = 'userdel -r {user}'
+    self.usermod_cmd = 'usermod -G {groups} {user}'
+    self.groupadd_cmd = 'groupadd {group}'
 
     self.mock_utils = mock.create_autospec(accounts_utils.AccountsUtils)
     self.mock_utils.google_comment = accounts_utils.AccountsUtils.google_comment
@@ -39,6 +43,10 @@ class AccountsUtilsTest(unittest.TestCase):
     self.mock_utils.google_users_dir = self.users_dir
     self.mock_utils.google_users_file = self.users_file
     self.mock_utils.logger = self.mock_logger
+    self.mock_utils.useradd_cmd = self.useradd_cmd
+    self.mock_utils.userdel_cmd = self.userdel_cmd
+    self.mock_utils.usermod_cmd = self.usermod_cmd
+    self.mock_utils.groupadd_cmd = self.groupadd_cmd
 
   @mock.patch('google_compute_engine.accounts.accounts_utils.AccountsUtils._GetGroup')
   @mock.patch('google_compute_engine.accounts.accounts_utils.AccountsUtils._CreateSudoersGroup')
@@ -82,7 +90,7 @@ class AccountsUtilsTest(unittest.TestCase):
     mocks.attach_mock(self.mock_logger, 'logger')
     self.mock_utils._GetGroup.return_value = False
     mock_exists.return_value = False
-    command = ['groupadd', self.sudoers_group]
+    command = self.groupadd_cmd.format(group=self.sudoers_group)
 
     with mock.patch('%s.open' % builtin, mock_open, create=False):
       accounts_utils.AccountsUtils._CreateSudoersGroup(self.mock_utils)
@@ -90,7 +98,7 @@ class AccountsUtilsTest(unittest.TestCase):
 
     expected_calls = [
         mock.call.group(self.sudoers_group),
-        mock.call.call(command),
+        mock.call.call(command, shell=True),
         mock.call.exists(self.sudoers_file),
         mock.call.permissions(self.sudoers_file, mode=0o440, uid=0, gid=0),
     ]
@@ -136,12 +144,12 @@ class AccountsUtilsTest(unittest.TestCase):
     self.mock_utils._GetGroup.return_value = False
     mock_exists.return_value = True
     mock_call.side_effect = subprocess.CalledProcessError(1, 'Test')
-    command = ['groupadd', self.sudoers_group]
+    command = self.groupadd_cmd.format(group=self.sudoers_group)
 
     accounts_utils.AccountsUtils._CreateSudoersGroup(self.mock_utils)
     expected_calls = [
         mock.call.group(self.sudoers_group),
-        mock.call.call(command),
+        mock.call.call(command, shell=True),
         mock.call.logger.warning(mock.ANY, mock.ANY),
         mock.call.exists(self.sudoers_file),
         mock.call.permissions(self.sudoers_file, mode=0o440, uid=0, gid=0),
@@ -186,23 +194,23 @@ class AccountsUtilsTest(unittest.TestCase):
   @mock.patch('google_compute_engine.accounts.accounts_utils.subprocess.check_call')
   def testAddUser(self, mock_call):
     user = 'user'
-    command = ['useradd', '-m', '-s', '/bin/bash', '-p', '*', user]
+    command = self.useradd_cmd.format(user=user)
 
     self.assertTrue(
         accounts_utils.AccountsUtils._AddUser(self.mock_utils, user))
-    mock_call.assert_called_once_with(command)
+    mock_call.assert_called_once_with(command, shell=True)
     expected_calls = [mock.call.info(mock.ANY, user)] * 2
     self.assertEqual(self.mock_logger.mock_calls, expected_calls)
 
   @mock.patch('google_compute_engine.accounts.accounts_utils.subprocess.check_call')
   def testAddUserError(self, mock_call):
     user = 'user'
-    command = ['useradd', '-m', '-s', '/bin/bash', '-p', '*', user]
+    command = self.useradd_cmd.format(user=user)
     mock_call.side_effect = subprocess.CalledProcessError(1, 'Test')
 
     self.assertFalse(
         accounts_utils.AccountsUtils._AddUser(self.mock_utils, user))
-    mock_call.assert_called_once_with(command)
+    mock_call.assert_called_once_with(command, shell=True)
     expected_calls = [
         mock.call.info(mock.ANY, user),
         mock.call.warning(mock.ANY, user, mock.ANY),
@@ -214,12 +222,12 @@ class AccountsUtilsTest(unittest.TestCase):
     user = 'user'
     groups = ['a', 'b', 'c']
     groups_string = ','.join(groups)
-    command = ['usermod', '-G', groups_string, user]
+    command = self.usermod_cmd.format(user=user, groups=groups_string)
 
     self.assertTrue(
         accounts_utils.AccountsUtils._UpdateUserGroups(
             self.mock_utils, user, groups))
-    mock_call.assert_called_once_with(command)
+    mock_call.assert_called_once_with(command, shell=True)
     expected_calls = [
         mock.call.debug(mock.ANY, user, groups_string),
         mock.call.debug(mock.ANY, user),
@@ -231,13 +239,13 @@ class AccountsUtilsTest(unittest.TestCase):
     user = 'user'
     groups = ['a', 'b', 'c']
     groups_string = ','.join(groups)
-    command = ['usermod', '-G', groups_string, user]
+    command = self.usermod_cmd.format(user=user, groups=groups_string)
     mock_call.side_effect = subprocess.CalledProcessError(1, 'Test')
 
     self.assertFalse(
         accounts_utils.AccountsUtils._UpdateUserGroups(
             self.mock_utils, user, groups))
-    mock_call.assert_called_once_with(command)
+    mock_call.assert_called_once_with(command, shell=True)
     expected_calls = [
         mock.call.debug(mock.ANY, user, groups_string),
         mock.call.warning(mock.ANY, user, mock.ANY),
@@ -575,11 +583,11 @@ class AccountsUtilsTest(unittest.TestCase):
   @mock.patch('google_compute_engine.accounts.accounts_utils.subprocess.check_call')
   def testRemoveUserForce(self, mock_call):
     user = 'user'
-    command = ['userdel', '-r', user]
+    command = self.userdel_cmd.format(user=user)
     self.mock_utils.remove = True
 
     accounts_utils.AccountsUtils.RemoveUser(self.mock_utils, user)
-    mock_call.assert_called_once_with(command)
+    mock_call.assert_called_once_with(command, shell=True)
     expected_calls = [mock.call.info(mock.ANY, user)] * 2
     self.assertEqual(self.mock_logger.mock_calls, expected_calls)
     self.mock_utils._RemoveAuthorizedKeys.assert_called_once_with(user)
@@ -587,12 +595,12 @@ class AccountsUtilsTest(unittest.TestCase):
   @mock.patch('google_compute_engine.accounts.accounts_utils.subprocess.check_call')
   def testRemoveUserError(self, mock_call):
     user = 'user'
-    command = ['userdel', '-r', user]
+    command = self.userdel_cmd.format(user=user)
     mock_call.side_effect = subprocess.CalledProcessError(1, 'Test')
     self.mock_utils.remove = True
 
     accounts_utils.AccountsUtils.RemoveUser(self.mock_utils, user)
-    mock_call.assert_called_once_with(command)
+    mock_call.assert_called_once_with(command, shell=True)
     expected_calls = [
         mock.call.info(mock.ANY, user),
         mock.call.warning(mock.ANY, user, mock.ANY),
diff --git a/google_compute_engine/instance_setup/instance_config.py b/google_compute_engine/instance_setup/instance_config.py
index 99c8e00..7f1ec0c 100644
--- a/google_compute_engine/instance_setup/instance_config.py
+++ b/google_compute_engine/instance_setup/instance_config.py
@@ -46,6 +46,22 @@ class InstanceConfig(config_manager.ConfigManager):
       'Accounts': {
           'deprovision_remove': 'false',
           'groups': 'adm,dip,docker,lxd,plugdev,video',
+
+          # The encrypted password is set to '*' for SSH on Linux systems
+          # without PAM.
+          #
+          # SSH uses '!' as its locked account token:
+          # https://github.com/openssh/openssh-portable/blob/master/configure.ac
+          #
+          # When the token is specified, SSH denies login:
+          # https://github.com/openssh/openssh-portable/blob/master/auth.c
+          #
+          # To solve the issue, make the password '*' which is also recognized
+          # as locked but does not prevent SSH login.
+          'useradd_cmd': 'useradd -m -s /bin/bash -p * {user}',
+          'userdel_cmd': 'userdel -r {user}',
+          'usermod_cmd': 'groupadd {group}',
+          'groupadd_cmd': 'usermod -G {groups} {user}',
       },
       'Daemons': {
           'accounts_daemon': 'true',