[master] Merge branch 'trac2713'

configure.ac

@@ -1204,6 +1204,7 @@ AC_CONFIG_FILES([Makefile
                  src/bin/stats/tests/Makefile
                  src/bin/stats/tests/testdata/Makefile
                  src/bin/usermgr/Makefile
+                 src/bin/usermgr/tests/Makefile
                  src/bin/tests/Makefile
                  src/lib/Makefile
                  src/lib/asiolink/Makefile

src/bin/usermgr/Makefile.am

+SUBDIRS = tests
+
 sbin_SCRIPTS = b10-cmdctl-usermgr
+noinst_SCRIPTS = run_b10-cmdctl-usermgr.sh
 
 b10_cmdctl_usermgrdir = $(pkgdatadir)
 
-CLEANFILES=	b10-cmdctl-usermgr
+CLEANFILES=	b10-cmdctl-usermgr b10-cmdctl-usermgr.pyc
 
 man_MANS = b10-cmdctl-usermgr.8
 DISTCLEANFILES = $(man_MANS)
@@ -25,3 +28,7 @@ endif
 b10-cmdctl-usermgr: b10-cmdctl-usermgr.py
 	$(SED) "s|@@PYTHONPATH@@|@pyexecdir@|" b10-cmdctl-usermgr.py >$@
 	chmod a+x $@
+
+CLEANDIRS = __pycache__
+clean-local:
+	rm -rf $(CLEANDIRS)

src/bin/usermgr/b10-cmdctl-usermgr.py.in

@@ -11,115 +11,234 @@
 # IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
 # INTERNET SYSTEMS CONSORTIUM BE LIABLE FOR ANY SPECIAL, DIRECT,
 # INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING
-# FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
-# NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+# FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN COMMAND OF CONTRACT,
+# NEGLIGENCE OR OTHER TORTIOUS COMMAND, ARISING OUT OF OR IN CONNECTION
 # WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 
 '''
-This file implements user management program. The user name and 
-its password is appended to csv file.
+This tool implements user management for b10-cmdctl. It is used to
+add and remove users from the accounts file.
 '''
+from bind10_config import SYSCONFPATH
+from collections import OrderedDict
 import random
 from hashlib import sha1
 import csv
 import getpass
-import getopt
+from optparse import OptionParser, OptionValueError
+import os
 import sys; sys.path.append ('@@PYTHONPATH@@')
 import isc.util.process
 
 isc.util.process.rename()
 
-VERSION_NUMBER = 'bind10'
-DEFAULT_FILE = 'cmdctl-accounts.csv'
-
-def gen_password_hash(password):
-    salt = "".join(chr(random.randint(33, 127)) for x in range(64))
-    saltedpwd = sha1((password + salt).encode()).hexdigest()
-    return salt, saltedpwd
-
-def username_exist(name, filename):
-    # The file may doesn't exist.
-    exist = False
-    csvfile = None
-    try:
-        csvfile = open(filename)        
-        reader = csv.reader(csvfile)
-        for row in reader:
-            if name == row[0]:
-                exist = True               
-                break
-    except Exception:
-        pass
-            
-    if csvfile:    
-        csvfile.close()
-    return exist
-
-def save_userinfo(username, pw, salt, filename):
-    csvfile = open(filename, 'a')
-    writer = csv.writer(csvfile)
-    writer.writerow([username, pw, salt])
-    csvfile.close()
-    print("\n create new account successfully! \n")
-
-def usage():
-    print('''Usage: usermgr [options]
-           -h, --help \t Show this help message and exit
-           -f, --file \t Specify the file to append user name and password
-           -v, --version\t Get version number
-           ''')           
+VERSION_STRING = "b10-cmdctl-usermgr @PACKAGE_VERSION@"
+DEFAULT_FILE = SYSCONFPATH + "/cmdctl-accounts.csv"
 
-def main():
-    filename = DEFAULT_FILE
-    try: 
-        opts, args = getopt.getopt(sys.argv[1:], 'f:hv', 
-                                   ['file=', 'help', 'version']) 
-    except getopt.GetoptError as err: 
-        print(err) 
-        usage() 
-        sys.exit(2)
-    for op, param in opts:        
-        if op in ('-h', '--help'): 
-            usage()
-            sys.exit()
-        elif op in ('-v', '--version'): 
-            print(VERSION_NUMBER)
-            sys.exit()                     
-        elif op in ('-f', "--file"):
-            filename = param
-        else: 
-            assert False, 'unknown option' 
-            usage()
-    
-    try:
-        while True :
-            name = input("Desired Login Name:")
-            if name == '':
-                print("error, user name can't be empty")
+# Actions that can be performed (used for argument parsing,
+# code paths, and output)
+COMMAND_ADD = "add"
+COMMAND_DELETE = "delete"
+
+# Non-zero return codes, used in tests
+BAD_ARGUMENTS = 1
+FILE_ERROR = 2
+USER_EXISTS = 3
+USER_DOES_NOT_EXIST = 4
+
+class UserManager:
+    def __init__(self, options, args):
+        self.options = options
+        self.args = args
+
+    def __print(self, msg):
+        if not self.options.quiet:
+            print(msg)
+
+    def __gen_password_hash(self, password):
+        salt = "".join(chr(random.randint(ord('!'), ord('~')))\
+                    for x in range(64))
+        saltedpwd = sha1((password + salt).encode()).hexdigest()
+        return salt, saltedpwd
+
+    def __read_user_info(self):
+        """
+        Read the existing user info
+        Raises an IOError if the file can't be read
+        """
+        # Currently, this is done quite naively (there is no
+        # check that the file is modified between read and write)
+        # But taking multiple simultaneous users of this tool on the
+        # same file seems unnecessary at this point.
+        self.user_info = OrderedDict()
+        if os.path.exists(self.options.output_file):
+            # Just let any file read error bubble up; it will
+            # be caught in the run() method
+            with open(self.options.output_file, newline='') as csvfile:
+                reader = csv.reader(csvfile)
+                for row in reader:
+                    self.user_info[row[0]] = row
+
+    def __save_user_info(self):
+        """
+        Write out the (modified) user info
+        Raises an IOError if the file can't be written
+        """
+        # Just let any file write error bubble up; it will
+        # be caught in the run() method
+        with open(self.options.output_file, 'w',
+                  newline='') as csvfile:
+            writer = csv.writer(csvfile)
+            for row in self.user_info.values():
+                writer.writerow(row)
+
+    def __add_user(self, name, password):
+        """
+        Add the given username/password combination to the stored user_info.
+        First checks if the username exists, and returns False if so.
+        If not, it is added, and this method returns True.
+        """
+        if name in self.user_info:
+            return False
+        salt, pw = self.__gen_password_hash(password)
+        self.user_info[name] = [name, pw, salt]
+        return True
+
+    def __delete_user(self, name):
+        """
+        Removes the row with the given name from the stored user_info
+        First checks if the username exists, and returns False if not.
+        Otherwise, it is removed, and this mehtod returns True
+        """
+        if name not in self.user_info:
+            return False
+        del self.user_info[name]
+        return True
+
+    # overridable input() call, used in testing
+    def _input(self, prompt):
+        return input(prompt)
+
+    # in essence this is private, but made 'protected' for ease
+    # of testing
+    def _prompt_for_username(self, command):
+        # Note, direct prints here are intentional
+        while True:
+            name = self._input("Username to " + command + ": ")
+            if name == "":
+                print("Error username can't be empty")
                 continue
 
-            if username_exist(name, filename):
-                 print("user name already exists!")
+            if command == COMMAND_ADD and name in self.user_info:
+                 print("user already exists")
+                 continue
+            elif command == COMMAND_DELETE and name not in self.user_info:
+                 print("user does not exist")
                  continue
 
-            while True:
-                pwd1 = getpass.getpass("Choose a password:")
-                pwd2 = getpass.getpass("Re-enter password:")
-                if pwd1 != pwd2:
-                    print("password is not same, please input again")
+            return name
+
+    # in essence this is private, but made 'protected' for ease
+    # of testing
+    def _prompt_for_password(self):
+        # Note, direct prints here are intentional
+        while True:
+            pwd1 = getpass.getpass("Choose a password: ")
+            if pwd1 == "":
+                print("Error: password cannot be empty")
+                continue
+            pwd2 = getpass.getpass("Re-enter password: ")
+            if pwd1 != pwd2:
+                print("passwords do not match, try again")
+                continue
+            return pwd1
+
+    def __verify_options_and_args(self):
+        """
+        Basic sanity checks on command line arguments.
+        Returns False if there is a problem, True if everything seems OK.
+        """
+        if len(self.args) < 1:
+            self.__print("Error: no command specified")
+            return False
+        if len(self.args) > 3:
+            self.__print("Error: extraneous arguments")
+            return False
+        if self.args[0] not in [ COMMAND_ADD, COMMAND_DELETE ]:
+            self.__print("Error: command must be either add or delete")
+            return False
+        if self.args[0] == COMMAND_DELETE and len(self.args) > 2:
+            self.__print("Error: delete only needs username, not a password")
+            return False
+        return True
+
+    def run(self):
+        if not self.__verify_options_and_args():
+            return BAD_ARGUMENTS
+
+        try:
+            self.__print("Using accounts file: " + self.options.output_file)
+            self.__read_user_info()
+
+            command = self.args[0]
+
+            if len(self.args) > 1:
+                username = self.args[1]
+            else:
+                username = self._prompt_for_username(command)
+
+            if command == COMMAND_ADD:
+                if len(self.args) > 2:
+                    password = self.args[2]
                 else:
-                    break;
-            
-            salt, pw = gen_password_hash(pwd1)
-            save_userinfo(name, pw, salt, filename)
-            inputdata = input('continue to create new account by input \'y\' or \'Y\':')
-            if inputdata not in ['y', 'Y']:
-                break                
-                
-    except KeyboardInterrupt:
-        pass                
+                    password = self._prompt_for_password()
+                if not self.__add_user(username, password):
+                    print("Error: username exists")
+                    return USER_EXISTS
+            elif command == COMMAND_DELETE:
+                if not self.__delete_user(username):
+                    print("Error: username does not exist")
+                    return USER_DOES_NOT_EXIST
+
+            self.__save_user_info()
+            return 0
+        except IOError as ioe:
+            self.__print("Error accessing " + ioe.filename +\
+                         ": " + str(ioe.strerror))
+            return FILE_ERROR
+        except csv.Error as csve:
+            self.__print("Error parsing csv file: " + str(csve))
+            return FILE_ERROR
+
+def set_options(parser):
+    parser.add_option("-f", "--file",
+                      dest="output_file", default=DEFAULT_FILE,
+                      help="Accounts file to modify"
+                     )
+    parser.add_option("-q", "--quiet",
+                      dest="quiet", action="store_true", default=False,
+                      help="Quiet mode, don't print any output"
+                     )
+
+def main():
+    usage = "usage: %prog [options] <command> [username] [password]\n\n"\
+            "Arguments:\n"\
+            "  command\t\teither 'add' or 'delete'\n"\
+            "  username\t\tthe username to add or delete\n"\
+            "  password\t\tthe password to set for the added user\n"\
+            "\n"\
+            "If username or password are not specified, %prog will\n"\
+            "prompt for them. It is recommended practice to let the\n"\
+            "tool prompt for the password, as command-line\n"\
+            "arguments can be visible through history or process\n"\
+            "viewers."
+    parser = OptionParser(usage=usage, version=VERSION_STRING)
+    set_options(parser)
+    (options, args) = parser.parse_args()
 
+    usermgr = UserManager(options, args)
+    return usermgr.run()
 
 if __name__ == '__main__':
-    main()
+    sys.exit(main())
 

src/bin/usermgr/b10-cmdctl-usermgr.xml

@@ -12,8 +12,8 @@
  - REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
  - AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
  - INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
- - LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
- - OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+ - LOSS OF USE, DATA OR PROFITS, WHETHER IN AN COMMAND OF CONTRACT, NEGLIGENCE
+ - OR OTHER TORTIOUS COMMAND, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
  - PERFORMANCE OF THIS SOFTWARE.
 -->
 
@@ -47,10 +47,12 @@
 
       <arg><option>-f <replaceable>filename</replaceable></option></arg>
       <arg><option>-h</option></arg>
-      <arg><option>-v</option></arg>
-      <arg><option>--file <replaceable>filename</replaceable></option></arg>
+      <arg><option>--file=<replaceable>filename</replaceable></option></arg>
       <arg><option>--help</option></arg>
       <arg><option>--version</option></arg>
+      <arg choice="plain"><replaceable>command</replaceable></arg>
+      <arg><option>username</option></arg>
+      <arg><option>password</option></arg>
 
     </cmdsynopsis>
   </refsynopsisdiv>
@@ -58,24 +60,22 @@
   <refsect1>
     <title>DESCRIPTION</title>
     <para>The <command>b10-cmdctl-usermgr</command> tool may be used
-      to add accounts with passwords for the
+      to add and remove accounts with passwords for the
       <citerefentry><refentrytitle>b10-cmdctl</refentrytitle><manvolnum>8</manvolnum></citerefentry>
       daemon.
     </para>
 
     <para>
       By default, the accounts are saved in the
-      <filename>cmdctl-accounts.csv</filename> file in the current directory,
-      unless the <option>--filename</option> switch is used.
-      The entry is appended to the file.
-<!-- TODO: default should have full path? -->
+      <filename>cmdctl-accounts.csv</filename> file in the system config
+      directory, unless the <option>--filename</option> switch is used.
+      The entry is appended to or removed from the file.
     </para>
 
     <para>
-      The tool can't remove or replace existing entries.
+      The tool can't replace existing entries, but this can easily be
+      accomplished by removing the entry and adding a new one.
     </para>
-<!-- TODO: the tool can't remove or replace existing entries -->
-
   </refsect1>
 
   <refsect1>
@@ -83,6 +83,18 @@
 
     <para>The arguments are as follows:</para>
 
+    <para>
+      command is either 'add' or 'delete', respectively to add or delete users.
+    </para>
+
+    <para>
+      If a username and password are given (or just a username in case of
+      deletion), these are used. Otherwise, the tool shall prompt for a
+      username and/or password. It is recommended practice to let the
+      tool prompt for the password, as command-line arguments can be
+      visible through history or process viewers.
+    </para>
+
     <variablelist>
 
       <varlistentry>
@@ -97,14 +109,13 @@
         <term><option>-f <replaceable>filename</replaceable></option></term>
         <term><option>--file <replaceable>filename</replaceable></option></term>
         <listitem><para>
-          Define the filename to append the account to. The default
-          is <filename>cmdctl-accounts.csv</filename> in the current directory.
-<!-- TODO: default should have full path? -->
+          Specify the accounts file to update. The default is
+          <filename>cmdctl-accounts.csv</filename> in the system config
+          directory.
         </para></listitem>
       </varlistentry>
 
       <varlistentry>
-        <term><option>-v</option></term>
         <term><option>--version</option></term>
         <listitem><para>
           Report the version and exit.

src/bin/usermgr/run_b10-cmdctl-usermgr.sh.in

@@ -18,6 +18,9 @@
 PYTHON_EXEC=${PYTHON_EXEC:-@PYTHON@}
 export PYTHON_EXEC
 
+PYTHONPATH=@abs_top_builddir@/src/lib/python
+export PYTHONPATH
+
 MYPATH_PATH=@abs_top_builddir@/src/bin/usermgr
 
 BIND10_MSGQ_SOCKET_FILE=@abs_top_builddir@/msgq_socket

src/bin/usermgr/tests/Makefile.am

+PYCOVERAGE_RUN=@PYCOVERAGE_RUN@
+PYTESTS = b10-cmdctl-usermgr_test.py
+EXTRA_DIST = $(PYTESTS)
+
+CLEANFILES = *.csv
+
+# test using command-line arguments, so use check-local target instead of TESTS
+check-local:
+if ENABLE_PYTHON_COVERAGE
+	touch $(abs_top_srcdir)/.coverage
+	rm -f .coverage
+	${LN_S} $(abs_top_srcdir)/.coverage .coverage
+endif
+	for pytest in $(PYTESTS) ; do \
+	echo Running test: $$pytest ; \
+	$(LIBRARY_PATH_PLACEHOLDER) \
+	PYTHONPATH=$(COMMON_PYTHON_PATH):$(abs_top_builddir)/src/bin/cmdctl \
+	CMDCTL_BUILD_PATH=$(abs_top_builddir)/src/bin/cmdctl \
+	CMDCTL_SRC_PATH=$(abs_top_srcdir)/src/bin/cmdctl \
+	B10_LOCKFILE_DIR_FROM_BUILD=$(abs_top_builddir) \
+	$(PYCOVERAGE_RUN) $(abs_srcdir)/$$pytest || exit ; \
+	done

src/bin/usermgr/tests/b10-cmdctl-usermgr_test.py

+# Copyright (C) 2013  Internet Systems Consortium.
+#
+# Permission to use, copy, modify, and distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND INTERNET SYSTEMS CONSORTIUM
+# DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
+# INTERNET SYSTEMS CONSORTIUM BE LIABLE FOR ANY SPECIAL, DIRECT,
+# INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING
+# FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN COMMAND OF CONTRACT,
+# NEGLIGENCE OR OTHER TORTIOUS COMMAND, ARISING OUT OF OR IN CONNECTION
+# WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+
+import csv
+from hashlib import sha1
+import getpass
+import imp
+import os
+import subprocess
+import stat
+import sys
+import unittest
+from bind10_config import SYSCONFPATH
+
+class PrintCatcher:
+    def __init__(self):
+        self.stdout_lines = []
+
+    def __enter__(self):
+        self.__orig_stdout_write = sys.stdout.write
+        def new_write(line):
+            self.stdout_lines.append(line)
+
+        sys.stdout.write = new_write
+        return self
+
+    def __exit__(self, type, value, traceback):
+        sys.stdout.write = self.__orig_stdout_write
+
+class OverrideGetpass:
+    def __init__(self, new_getpass):
+        self.__new_getpass = new_getpass
+        self.__orig_getpass = getpass.getpass
+
+    def __enter__(self):
+        getpass.getpass = self.__new_getpass
+        return self
+
+    def __exit__(self, type, value, traceback):
+        getpass.getpass = self.__orig_getpass
+
+# input() is a built-in function and not easily overridable
+# so this one uses usermgr for that
+class OverrideInput:
+    def __init__(self, usermgr, new_getpass):
+        self.__usermgr = usermgr
+        self.__new_input = new_getpass
+        self.__orig_input = usermgr._input
+
+    def __enter__(self):
+        self.__usermgr._input = self.__new_input
+        return self
+
+    def __exit__(self, type, value, traceback):
+        self.__usermgr._input = self.__orig_input
+
+def run(command):
+    """
+    Small helper function that returns a tuple of (rcode, stdout, stderr)
+    after running the given command (an array of command and arguments, as
+    passed on to subprocess).
+    Parameters:
+    command: an array of command and argument strings, which will be
+             passed to subprocess.Popen()
+    """
+    subp = subprocess.Popen(command, stdout=subprocess.PIPE,
+                            stderr=subprocess.PIPE)
+    (stdout, stderr) = subp.communicate()
+    return (subp.returncode, stdout, stderr)
+
+class TestUserMgr(unittest.TestCase):
+    TOOL = '../b10-cmdctl-usermgr'
+    OUTPUT_FILE = 'test_users.csv'
+
+    def setUp(self):
+        self.delete_output_file()
+        # For access to the actual module, we load it directly
+        self.usermgr_module = imp.load_source('usermgr',
+                                              '../b10-cmdctl-usermgr.py')
+        # And instantiate 1 instance (with fake options/args)
+        self.usermgr = self.usermgr_module.UserManager(object(), object())
+
+    def tearDown(self):
+        self.delete_output_file()
+
+    def delete_output_file(self):
+        if os.path.exists(self.OUTPUT_FILE):
+            os.remove(self.OUTPUT_FILE)
+
+    def check_output_file(self, expected_content):
+        self.assertTrue(os.path.exists(self.OUTPUT_FILE))
+
+        csv_entries = []
+        with open(self.OUTPUT_FILE, newline='') as csvfile:
+            reader = csv.reader(csvfile)
+            csv_entries = [row for row in reader]
+
+        self.assertEqual(len(expected_content), len(csv_entries))
+        csv_entries.reverse()
+        for expected_entry in expected_content:
+            expected_name = expected_entry[0]
+            expected_pass = expected_entry[1]
+
+            csv_entry = csv_entries.pop()
+            entry_name = csv_entry[0]
+            entry_salt = csv_entry[2]
+            entry_hash = csv_entry[1]
+
+            self.assertEqual(expected_name, entry_name)
+            expected_hash =\
+                sha1((expected_pass + entry_salt).encode()).hexdigest()
+            self.assertEqual(expected_hash, entry_hash)
+
+    def run_check(self, expected_returncode, expected_stdout, expected_stderr,
+                  command):
+        """
+        Runs the given command, and checks return code, and outputs (if provided).
+        Arguments:
+        expected_returncode, return code of the command
+        expected_stdout, (multiline) string that is checked against stdout.
+                         May be None, in which case the check is skipped.
+        expected_stderr, (multiline) string that is checked against stderr.
+                         May be None, in which case the check is skipped.
+        """
+        (returncode, stdout, stderr) = run(command)
+        if expected_stderr is not None:
+            self.assertEqual(expected_stderr, stderr.decode())
+        if expected_stdout is not None:
+            self.assertEqual(expected_stdout, stdout.decode())
+        self.assertEqual(expected_returncode, returncode, " ".join(command))
+
+    def test_help(self):
+        self.run_check(0,
+'''Usage: b10-cmdctl-usermgr [options] <command> [username] [password]
+
+Arguments: