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.
--
David Kupka
From 64aff33276a2e929806fac296830b6bdf5b6f621 Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Tue, 29 Jul 2014 13:58:33 +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..aea9891c7e2c3662aa22eb32cc2fadbc78687faa 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['ipatokennotbefore'][0]
+ if notafter is None:
+ notafter_set = False
+ notafter = result['ipatokennotafter'][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
