On 07/29/2014 03:28 PM, Jan Cholasta wrote:
Dne 29.7.2014 v 14:11 David Kupka napsal(a):
On 07/29/2014 01:21 PM, Jan Cholasta wrote:
Dne 24.7.2014 v 10:00 David Kupka napsal(a):
On 07/23/2014 05:07 PM, Jan Cholasta wrote:
Hi,
On 23.7.2014 15:46, David Kupka wrote:
https://fedorahosted.org/freeipa/ticket/4244
1) Use "isinstance(X, Y)" instead of "type(X) is Y".
Thanks for advice, will try to remember.
2) When is "type(not_before) is str" or "type(not_after) is str" true?
The values coming from command options or LDAP should always be
datetime, never str.
Actually, it is never true. I don't know why I thought that there is
such option.
3) There are some misindentations:
+ raise ValidationError(name='not_after',
+ error='is before not_before!')
+ raise ValidationError(name='not_after',
+ error='is before not_before!')
+ raise ValidationError(name='not_before',
+ error='is after not_after!')
4) We don't do exclamation marks in errors messages.
You re right, it's probably better not to shout at customer :)
5) Generally, when you want to validate command options, you should
look
into "options", not "entry_attrs".
6) This is not right:
+ result = self.api.Command.otptoken_find(ipatokenuniqueid=
+ entry_attrs.get('ipatokenuniqueid', None))['result']
This is:
+ result =
self.api.Command.otptoken_show(keys[-1])['result']
Both works, but Martin explained me why is otptoken_show better and how
it actually works.
Honza
Few more nitpicks:
1) You can make _check_interval a little bit shorter by removing a
redundant if statement:
+def _check_interval(not_before, not_after):
+ if not_before and not_after:
+ return not_before <= not_after
+ return True
Nice :)
2) Please keep the 2 line space between the last global function and
first class in the module.
It's good to know that there is one less rule to discover.
3) Error messages are for users' eyes, please don't use internal
identifiers in them.
Done.
4) You don't need to check if the result of otptoken_show is empty, it
never is.
Removed. Although, needless check can't hurt.
One more thing, sorry I didn't notice it earlier: You must check if
'ipatokennotbefore' and 'ipatokennotafter' keys are in opttoken-show
result before using them, otherwise you might end up with:
ipa: ERROR: non-public: KeyError: 'ipatokennotbefore'
Traceback (most recent call last):
File "/usr/lib/python2.7/site-packages/ipaserver/rpcserver.py",
line 348, in wsgi_execute
result = self.Command[name](*args, **options)
File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line
439, in __call__
ret = self.run(*args, **options)
File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line
754, in run
return self.execute(*args, **options)
File
"/usr/lib/python2.7/site-packages/ipalib/plugins/baseldap.py", line
1384, in execute
*keys, **options)
File
"/usr/lib/python2.7/site-packages/ipalib/plugins/otptoken.py", line 356,
in pre_callback
notbefore = result['ipatokennotbefore'][0]
KeyError: 'ipatokennotbefore'
Sorry for sending fifth wersion of the patch. I must test my patches
more carefully.
--
David Kupka
From 0458522dff5be7cb8b7718d379d5cfb3e32c7b33 Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Tue, 29 Jul 2014 15:45:21 +0200
Subject: [PATCH] Verify otptoken timespan is valid
When creating or modifying otptoken check that token validity start is not after
validity end.
https://fedorahosted.org/freeipa/ticket/4244
---
ipalib/plugins/otptoken.py | 31 ++++++++++++++++++++++++++++++-
1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/ipalib/plugins/otptoken.py b/ipalib/plugins/otptoken.py
index 2880ee660d5dcdb18c504f50d7b72f5b8fb43d48..dfd010e7f8663b424d9c536907fcc93229181fa3 100644
--- a/ipalib/plugins/otptoken.py
+++ b/ipalib/plugins/otptoken.py
@@ -21,7 +21,7 @@ from ipalib.plugins.baseldap import DN, LDAPObject, LDAPAddMember, LDAPRemoveMem
from ipalib.plugins.baseldap import LDAPCreate, LDAPDelete, LDAPUpdate, LDAPSearch, LDAPRetrieve
from ipalib import api, Int, Str, Bool, DateTime, Flag, Bytes, IntEnum, StrEnum, Password, _, ngettext
from ipalib.plugable import Registry
-from ipalib.errors import PasswordMismatch, ConversionError, LastMemberError, NotFound
+from ipalib.errors import PasswordMismatch, ConversionError, LastMemberError, NotFound, ValidationError
from ipalib.request import context
from ipalib.frontend import Local
@@ -103,6 +103,11 @@ def _normalize_owner(userobj, entry_attrs):
if owner is not None:
entry_attrs['ipatokenowner'] = userobj.get_dn(owner)
+def _check_interval(not_before, not_after):
+ if not_before and not_after:
+ return not_before <= not_after
+ return True
+
@register()
class otptoken(LDAPObject):
@@ -254,6 +259,11 @@ class otptoken_add(LDAPCreate):
entry_attrs['ipatokenuniqueid'] = str(uuid.uuid4())
dn = DN("ipatokenuniqueid=%s" % entry_attrs['ipatokenuniqueid'], dn)
+ if not _check_interval(options.get('ipatokennotbefore', None),
+ options.get('ipatokennotafter', None)):
+ raise ValidationError(name='not_after',
+ error='is before the validity start')
+
# Set the object class and defaults for specific token types
entry_attrs['objectclass'] = otptoken.object_class + ['ipatoken' + options['type']]
for ttype, tattrs in TOKEN_TYPES.items():
@@ -336,6 +346,25 @@ class otptoken_mod(LDAPUpdate):
msg_summary = _('Modified OTP token "%(value)s"')
def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
+ notafter_set = True
+ notbefore = options.get('ipatokennotbefore', None)
+ notafter = options.get('ipatokennotafter', None)
+ # notbefore xor notafter, exactly one of them is not None
+ if bool(notbefore) ^ bool(notafter):
+ result = self.api.Command.otptoken_show(keys[-1])['result']
+ if notbefore is None:
+ notbefore = result.get('ipatokennotbefore', [None])[0]
+ if notafter is None:
+ notafter_set = False
+ notafter = result.get('ipatokennotafter', [None])[0]
+
+ if not _check_interval(notbefore, notafter):
+ if notafter_set:
+ raise ValidationError(name='not_after',
+ error='is before the validity start')
+ else:
+ raise ValidationError(name='not_before',
+ error='is after the validity end')
_normalize_owner(self.api.Object.user, entry_attrs)
return dn
--
1.9.3
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
