[Freeipa-devel] [PATCH 0071] Display token type when viewing token

[Martin Kosek]

On 10/17/2014 06:03 PM, Nathaniel McCallum wrote:
> On Fri, 2014-10-17 at 10:59 +0200, Martin Kosek wrote:
>> On 10/16/2014 11:37 PM, Nathaniel McCallum wrote:
>>> On Thu, 2014-10-16 at 17:35 +0200, Martin Kosek wrote:
>>>> On 10/15/2014 06:32 PM, Nathaniel McCallum wrote:
>>>>> When viewing a token from the CLI or UI, the type of the token
>>>>> should be displayed.
>>>>>
>>>>> https://fedorahosted.org/freeipa/ticket/4563
>>>>
>>>> Adding objectclass to default_attributes is unprecedented and something we
>>>> should not do before release. It would also put objectclass attribute in
>>>> default otptoken-* operations.
>>>>
>>>> What I would do in this case is to
>>>> - keep default_attributes as is
>>>> - add 'objectclass' to attrs_list in pre_callback
>>>> - do whatever you already do with it in post_callback
>>>> - remove the objectclass if it was not called for explicitly, e.g.:
>>>>
>>>>             if not options.get('all', False) or options.get('pkey_only', False):
>>>>                 entry_attrs.pop('objectclass', None)
>>>>
>>>> This approach is used for example in idrange.py so I would stick with it (I am
>>>> not saying it is pretty, I am just saying our API should give consistent output).
>>>
>>> Fixed and tested.
>>>
>>
>> Works fine in CLI, though I found couple more issues:
>>
>> 1) In type you return "HOTP" or "TOTP" while on the input you accept lowercased
>> versions - is that OK?
>>
>> # ipa otptoken-add --type=TOTP --owner=fbar barbar
>> ipa: ERROR: invalid 'type': must be one of 'totp', 'hotp'
>>
>> It just did not seem consistent to me.
> 
> It was an oversight. In an ideal world, we'd have a case-insensitive
> StrEnum type. For now, this patch accepts either fully lowercase or
> fully uppercase. Display is always uppercase.
> 
>> 2) You are suddenly adding camelcased attribute, compared to other places:
>>
>> +        attrs_list.append("objectClass")
> 
> Argh. Fixed.
> 
>> 3) I do not see the OTP token type in Web UI OTP token view - is it just me or
>> is it really missing?
> 
> I think it is just you. The token type shows on the token details page,
> not in the overview list of all tokens.

Yeah, I found out it was just a bad Web UI build. All the issues are fixed. You
just forgot to regenerate API.txt to get extended StrEnum, so I did it for you.

ACK.

Pushed to:
master: 560606a9910b5f289cedbf341ea5a2cbd011aee2
ipa-4-1: 23878c36bbe7943b662e4c9f9dd438c083730c8f

Martin