diff --git a/ipalib/frontend.py b/ipalib/frontend.py index 565060b5d..3665cc0e7 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 6ce0ad80a..fe32c92f6 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 b3ab996df..70430714e 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.