Added some missing parameter unit tests; added docstring about Command._repr_iter() and Param.safe_value()

3 files changed, 82 insertions, 6 deletions

diff --git a/ipalib/frontend.py b/ipalib/frontend.py
index 565060b5..3665cc0e 100644
--- a/ipalib/frontend.py
+++ b/ipalib/frontend.py
@@ -98,7 +98,7 @@ class Command(plugable.Plugin):
         """
         params = self.args_options_2_params(*args, **options)
         self.info(
-            '%s(%s)', self.name, ', '.join(self.__repr_iter(params))
+            '%s(%s)', self.name, ', '.join(self._repr_iter(**params))
         )
         params = self.normalize(**params)
         params = self.convert(**params)
@@ -109,10 +109,28 @@ class Command(plugable.Plugin):
         self.debug('result from %s(): %r', self.name, result)
         return result
 
-    def __repr_iter(self, params):
+    def _repr_iter(self, **params):
+        """
+        Iterate through ``repr()`` of *safe* values of args and options.
+
+        This method uses `parameters.Param.safe_value()` to mask passwords when
+        logging.  Logging the exact call is extremely useful, but we obviously
+        don't want to log the cleartext password.
+
+        For example:
+
+        >>> class my_cmd(Command):
+        ...     takes_args = ('login',)
+        ...     takes_options=(Password('passwd'),)
+        ...
+        >>> c = my_cmd()
+        >>> c.finalize()
+        >>> list(c._repr_iter(login=u'Okay.', passwd=u'Private!'))
+        ["u'Okay.'", "passwd=u'********'"]
+        """
         for arg in self.args():
             value = params.get(arg.name, None)
-            yield '%r' % (arg.safe_value(value),)
+            yield repr(arg.safe_value(value))
         for option in self.options():
             if option.name not in params:
                 continue
diff --git a/ipalib/parameters.py b/ipalib/parameters.py
index 6ce0ad80..fe32c92f 100644
--- a/ipalib/parameters.py
+++ b/ipalib/parameters.py
@@ -359,6 +359,26 @@ class Param(ReadOnly):
         return value
 
     def safe_value(self, value):
+        """
+        Return a value safe for logging.
+
+        This is used so that passwords don't get logged.  If this is a
+        `Password` instance and ``value`` is not ``None``, a constant
+        ``u'********'`` is returned.  For example:
+
+        >>> p = Password('my_password')
+        >>> p.safe_value(u'This is my password')
+        u'********'
+        >>> p.safe_value(None) is None
+        True
+
+        If this is not a `Password` instance, ``value`` is returned unchanged.
+        For example:
+
+        >>> s = Str('my_str')
+        >>> s.safe_value(u'Some arbitrary value')
+        u'Some arbitrary value'
+        """
         if self.password and value is not None:
             return u'********'
         return value
diff --git a/tests/test_ipalib/test_parameters.py b/tests/test_ipalib/test_parameters.py
index b3ab996d..70430714 100644
--- a/tests/test_ipalib/test_parameters.py
+++ b/tests/test_ipalib/test_parameters.py
@@ -144,6 +144,7 @@ class test_Param(ClassChecker):
         assert o.param_spec is name
         assert o.name is name
         assert o.nice == "Param('my_param')"
+        assert o.password is False
         assert o.__islocked__() is True
 
         # Test default rules:
@@ -243,6 +244,20 @@ class test_Param(ClassChecker):
         o = self.cls('name', multivalue=True)
         assert repr(o) == "Param('name', multivalue=True)"
 
+    def test_safe_value(self):
+        """
+        Test the `ipalib.parameters.Param.safe_value` method.
+        """
+        values = (unicode_str, binary_bytes, utf8_bytes)
+        o = self.cls('my_param')
+        for value in values:
+            assert o.safe_value(value) is value
+        assert o.safe_value(None) is None
+        p = parameters.Password('my_passwd')
+        for value in values:
+            assert_equal(p.safe_value(value), u'********')
+        assert p.safe_value(None) is None
+
     def test_clone(self):
         """
         Test the `ipalib.parameters.Param.clone` method.
@@ -326,7 +341,7 @@ class test_Param(ClassChecker):
         """
         Test the `ipalib.parameters.Param.convert` method.
         """
-        okay = ('Hello', u'Hello', 0, 4.2, True, False)
+        okay = ('Hello', u'Hello', 0, 4.2, True, False, unicode_str)
         class Subclass(self.cls):
             def _convert_scalar(self, value, index=None):
                 return value
@@ -610,6 +625,7 @@ class test_Data(ClassChecker):
         """
         o = self.cls('my_data')
         assert o.type is NoneType
+        assert o.password is False
         assert o.rules == tuple()
         assert o.class_rules == tuple()
         assert o.all_rules == tuple()
@@ -663,6 +679,7 @@ class test_Bytes(ClassChecker):
         """
         o = self.cls('my_bytes')
         assert o.type is str
+        assert o.password is False
         assert o.rules == tuple()
         assert o.class_rules == tuple()
         assert o.all_rules == tuple()
@@ -800,6 +817,7 @@ class test_Str(ClassChecker):
         """
         o = self.cls('my_str')
         assert o.type is unicode
+        assert o.password is False
         assert o.minlength is None
         assert o.maxlength is None
         assert o.length is None
@@ -811,9 +829,10 @@ class test_Str(ClassChecker):
         """
         o = self.cls('my_str')
         mthd = o._convert_scalar
-        for value in (u'Hello', 42, 1.2):
+        for value in (u'Hello', 42, 1.2, unicode_str):
             assert mthd(value) == unicode(value)
-        for value in [True, 'Hello', (u'Hello',), [42.3], dict(one=1)]:
+        bad = [True, 'Hello', (u'Hello',), [42.3], dict(one=1), utf8_bytes]
+        for value in bad:
             e = raises(errors2.ConversionError, mthd, value)
             assert e.name == 'my_str'
             assert e.index is None
@@ -903,6 +922,25 @@ class test_Str(ClassChecker):
             dummy.reset()
 
 
+class test_Password(ClassChecker):
+    """
+    Test the `ipalib.parameters.Password` class.
+    """
+    _cls = parameters.Password
+
+    def test_init(self):
+        """
+        Test the `ipalib.parameters.Password.__init__` method.
+        """
+        o = self.cls('my_password')
+        assert o.type is unicode
+        assert o.minlength is None
+        assert o.maxlength is None
+        assert o.length is None
+        assert o.pattern is None
+        assert o.password is True
+
+
 class test_StrEnum(ClassChecker):
     """
     Test the `ipalib.parameters.StrEnum` class.