On Mon, 2013-10-07 at 13:47 +0200, Petr Viktorin wrote:
> On 10/04/2013 07:34 PM, Nathaniel McCallum wrote:
> > This patch is preparatory for the OTP CLI patch.
>
> Thanks for the patch; it needs some work.
>
> >>From 2678ff4e2f22e7e81bf40b30ffcd0efe0ecf08c2 Mon Sep 17 00:00:00 2001
> > From: Nathaniel McCallum<npmccal...@redhat.com>
> > Date: Mon, 30 Sep 2013 13:06:37 -0400
> > Subject: [PATCH] Don't special case the Password class in Param.__init__()
> >
> > ---
> > ipalib/parameters.py | 20 ++++++++++----------
> > 1 file changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/ipalib/parameters.py b/ipalib/parameters.py
> > index
> > fbcb87537ba662763a00e12178d424a8718baa8a..925f442968ab93b2b6df4e386d03558300bf5990
> > 100644
> > --- a/ipalib/parameters.py
> > +++ b/ipalib/parameters.py
> > @@ -398,11 +398,11 @@ class Param(ReadOnly):
> > # We keep these values to use in __repr__():
> > self.param_spec = name
> > self.__kw = dict(kw)
> > -
> > - if isinstance(self, Password):
> > - self.password = True
> > - else:
> > +
> > + try:
> > self.password = False
> > + except AttributeError:
> > + pass
>
>
> Setting the attribute here will always pass, and always re-set
> `password` to False, even for Password instances.
>
> A class-level attribute (both in Param and Password) would work better here:
>
> class Param(ReadOnly):
> ...
> password = False
> ...
>
> class Password(Str):
> ...
> password = True
> ...
>
>
> You can run a part of the test suite to verify changes in ipalib
> (test_ipalib happens to not need an installed server):
> ./make-test ipatests/test_ipalib
> or just for parameters:
> ./make-test ipatests/test_ipalib/test_parameters.py
>
>
> BTW, Git complains trailing whitespace. I found the following Git
> setting useful to spot this before `git am`:
> $ git config color.diff.whitespace 'red reverse'
Fixed.
>From 4746f1b94ce8534abac2b7cd370125d361fe47d1 Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum <npmccal...@redhat.com>
Date: Mon, 30 Sep 2013 13:06:37 -0400
Subject: [PATCH] Don't special case the Password class in Param.__init__()
---
ipalib/parameters.py | 17 ++++++-----------
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/ipalib/parameters.py b/ipalib/parameters.py
index 70bb9a0e88642cbf645311db4c04fde860574ad7..05dde93e0ce7c05ce557b2d33d698b99f7414be0 100644
--- a/ipalib/parameters.py
+++ b/ipalib/parameters.py
@@ -372,6 +372,8 @@ class Param(ReadOnly):
# _convert_scalar operates only on scalar values
scalar_error = _('Only one value is allowed')
+ password = False
+
kwargs = (
('cli_name', str, None),
('cli_short_name', str, None),
@@ -413,11 +415,6 @@ class Param(ReadOnly):
self.param_spec = name
self.__kw = dict(kw)
- if isinstance(self, Password):
- self.password = True
- else:
- self.password = False
-
# Merge in kw from parse_param_spec():
(name, kw_from_spec) = parse_param_spec(name)
if not 'required' in kw:
@@ -647,9 +644,8 @@ class Param(ReadOnly):
"""
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:
+ This is used so that sensitive values like passwords don't get logged.
+ For example:
>>> p = Password('my_password')
>>> p.safe_value(u'This is my password')
@@ -657,9 +653,6 @@ class Param(ReadOnly):
>>> 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'
@@ -1484,6 +1477,8 @@ class Password(Str):
A parameter for passwords (stored in the ``unicode`` type).
"""
+ password = True
+
kwargs = Str.kwargs + (
('confirm', bool, True),
)
--
1.8.3.1
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
