[Freeipa-devel] [PATCH] 0005 Verify otptoken timespan is valid

[David Kupka]

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
>

-- 
David Kupka
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-dkupka-0005-2-Verify-otptoken-timespan-is-valid.patch
Type: text/x-patch
Size: 3580 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140724/7afed606/attachment.bin>