[Freeipa-devel] [PATCH 0024] Add OTP support to ipalib CLI

[Jan Cholasta]

On 16.12.2013 18:01, Nathaniel McCallum wrote:
> On Mon, 2013-12-16 at 17:31 +0100, Jan Cholasta wrote:
>> Please rename _normalize_owner to _convert_owner and vice versa, to
>> match the convention used in other plugins (sorry for noticing this
>> earlier).
>
> Fixed.
>
>> This bit in otptoken_add should be replaced by a call to
>> _normalize_owner (after the rename):
>>
>> +        # Resolve the user's dn
>> +        owner = entry_attrs.get('ipatokenowner', None)
>> +        if owner is not None:
>> +            owner = self.api.Object.user.get_dn(owner)
>> +            entry_attrs['ipatokenowner'] = owner
>
> Fixed.

ipalib/plugins/otptoken.py:230: [E0602(undefined-variable), 
otptoken_add.pre_callback] Undefined variable 'owner')
ipalib/plugins/otptoken.py:232: [E0602(undefined-variable), 
otptoken_add.pre_callback] Undefined variable 'owner')

(just put a "owner = entry_attrs.get('ipatokenowner')" line somewhere in 
there)

>
>> You do the conversion from uid to DN in otptoken_find twice:
>>
>> +        _convert_owner(self.api.Object.user, kwargs)
>> +        return super(otptoken_find, self).pre_callback(ldap, filters,
>> *args, **kwargs)
>> +
>> +    def args_options_2_entry(self, *args, **options):
>> +        o = 'ipatokenowner'
>> +        if o in options:
>> +            options[o] = self.api.Object.user.get_dn(options[o])
>>
>> I would suggest to do it only in args_options_2_entry like this (again,
>> after the rename):
>>
>>       def args_options_2_entry(self, *args, **options):
>>           entry = super(otptoken_find, self).args_options_2_entry(*args,
>> **options)
>>           _convert_owner(self.api.Object.user, entry)
>>           return entry
>
> Fixed.

Thanks. Also sorry for misleading you to use _convert_owner here, I see 
you did the right thing and used _normalize_owner.

>
>> The patch needs a rebase (for the sake of ipapermlocation default value).
>
> Fixed.
>
> Nathaniel
>

-- 
Jan Cholasta