[Freeipa-devel] [PATCH 0074] Make token window sizes configurable

[Petr Vobornik]

On 7.11.2014 08:58, Martin Kosek wrote:
> On 11/04/2014 05:17 PM, Nathaniel McCallum wrote:
>> On Wed, 2014-10-29 at 09:34 -0400, Nathaniel McCallum wrote:
>>> On Wed, 2014-10-29 at 12:21 +0100, Petr Viktorin wrote:
>>>> On 10/29/2014 10:37 AM, Martin Kosek wrote:
>>>>> On 10/28/2014 09:59 PM, Nathaniel McCallum wrote:
>>>>>> On Thu, 2014-10-23 at 18:07 -0400, Nathaniel McCallum wrote:
>>>>>>> This patch gives the administrator variables to control the size of
>>>>>>> the authentication and synchronization windows for OTP tokens.
>>>>>>>
>>>>>>> https://fedorahosted.org/freeipa/ticket/4511
>>>>>>>
>>>>>>> NOTE: There is one known issue with this patch which I don't know
>>>>>>> how to
>>>>>>> solve. This patch changes the schema in
>>>>>>> install/share/60ipaconfig.ldif.
>>>>>>> On an upgrade, all of the new attributeTypes appear correctly.
>>>>>>> However,
>>>>>>> the modifications to the pre-existing objectClass do not show up
>>>>>>> on the
>>>>>>> server. What am I doing wrong?
>>>>>>>
>>>>>>> After modifying ipaGuiConfig manually, everything in this patch
>>>>>>> works
>>>>>>> just fine.
>>>>>>
>>>>>> This new version takes into account the new (proper) OIDs and
>>>>>> attribute
>>>>>> names.
>>>>>
>>>>> Thanks Nathaniel!
>>>>>
>>>>>> The above known issue still remains.
>>>>>
>>>>> Petr3, any idea what could have gone wrong? ObjectClass MAY list
>>>>> extension
>>>>> should work just fine, AFAIK.
>>>>
>>>> You added a blank line to the LDIF file. This is an entry separator, so
>>>> the objectClasses after the blank line don't belong to cn=schema, so
>>>> they aren't considered in the update.
>>>> Without the blank line it works fine.
>>>
>>> Thanks for the catch!
>>>
>>> Here is a version without the blank line.
>>
>> I forgot to remove the old steps defines. This patch performs this
>> cleanup.
>
> I am now wondering, is the global config object really the nest place to
> add these OTP specific settings?
>
> I would prefer not to overload the object and instead:
> - create new ipaOTPConfig objectclass
> - add it to cn=otp,$SUFFIX
> - create otpconfig-mod and otpconfig-show commands to follow an example
> of dnsconfig-* and trustconfig-* commands
>
> IMO, this would allow more flexibility for the OTP settings and would
> also scale better for the future updates.

+1

I will comment the patch as if ^^ would not exist because it will still 
be needed in the new plugin.

Because of ^^ I did not test, just read.

1. Got:
install/ui/src/freeipa/serverconfig.js(135): lint warning: extra comma 
is not recommended in array initializers

Please run:
   jsl -nofilelisting -nosummary -nologo -conf jsl.conf
in install/ui directory

The goal is no have no warnings and errors.

2. new attrs should be added to 'System: Read Global Configuration' 
managed permission

-- 
Petr Vobornik