[Freeipa-devel] [PATCH] 531-541 OTP UI

[Petr Vobornik]

On 10.2.2014 17:49, Nathaniel McCallum wrote:
> On Mon, 2014-02-10 at 17:16 +0100, Petr Vobornik wrote:
>> On 10.2.2014 17:09, Nathaniel McCallum wrote:
>>> On Mon, 2014-02-10 at 14:12 +0100, Petr Vobornik wrote:
>>>> On 13.1.2014 17:09, Petr Vobornik wrote:
>>>>> Hi,
>>>>>
>>>>> these patches implements the OTP Web UI.
>>>>>
>>>>> Last 5 patches is the OTP UI.
>>>>>
>>>>> First 6 patches is a little refactoring/bug fixes needed for them.
>>>>> General password dialog is introduced to avoid another implementation.
>>>>>
>>>>> Self-service UI is implemented to be very simple. Atm user can choose
>>>>> only token name. Admin interface allows to enter all values.
>>>>>
>>>>> It's based on the RCUE work -> we need to push RCUE first. Thanks
>>>>> Nathaniel for review of the last font package. It will speed things up.
>>>>>
>>>>> Know bugs:
>>>>> - there is clash in id's of checkboxes preventing editation of
>>>>> subsequently displayed ones with the same name. Will be fixed in
>>>>> separate patch.
>>>>> - bugs caused by bugs in API (adding/removal of own tokens in
>>>>> self-service, inability to enter key on token creation -
>>>>> https://fedorahosted.org/freeipa/ticket/4099)
>>>>> - datetime format (widget+validator) will be implemented in separate patch
>>>>> - no support of not reviewed CLI patches (HOTP..)
>>>>>
>>>>> Cgit:
>>>>> http://fedorapeople.org/cgit/pvoborni/public_git/freeipa.git/log/?h=otp
>>>>>
>>>>> https://fedorahosted.org/freeipa/ticket/3369
>>>>>
>>>>
>>>> patch 540-1 has been updated
>>>> - QR code is centered
>>>> - QR code correction level was lowered from H to M
>>>>
>>>> All other current patches from sub-threads are attached as well (it was
>>>> getting hard to keep track of them).
>>>
>>> When specifying a token using the admin user, the token URI generated is
>>> invalid.
>>>
>> Yes, but that's API issue: https://fedorahosted.org/freeipa/ticket/4169
>
> I agree there is an API issue part of this. However, I think the UI
> would be cleaner without the "default" radio button. Putting on my admin
> hat, when I look at that page I immediately think "What is the
> default!?" I think it would be better to leave out the "default" option
> and just have the actual default value selected by default.
>
> Nathaniel
>

I agree with the "What's the default?" part. However The Web UI should 
be consistent with CLI.  In this case the default is not clearly defined 
and both options are defined as optional, i.e. I would change the UI to 
your proposal if the ipalib definition was:

         IntEnum('ipatokenotpdigits',
             cli_name='digits',
             label=_('Display length'),
             values=(6, 8),
             default=6,
             flags=('no_update'),
         ),

Btw, why are these two options optional? In one otptoken.py comment you 
proposed to introduce global configuration in near future. With it, 
every global configuration change might break existing tokens. So in any 
case the value should be written to LDAP (which is not happening atm).
-- 
Petr Vobornik