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

[thierry bordaz]

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.
>
>
>
>
> _______________________________________________
> Freeipa-devel mailing list
> Freeipa-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel
Hello Nathaniel,

    Few comments on the review:

      * in authcfg
          o in string_to_types, I would prefer you set the last element
            of 'map' to { NULL, 0 }.
          o in entry_to_window, you may declare the 'defaults' array as
            'static const'
          o Would use define for  "ipaUserAuthType","ipaHOTPAuthWindow",
            "ipaTOTPAuthWindow", "ipaHOTPSyncWindow","ipaTOTPSyncWindow"
            that are present multiple times
          o suffix_to_config: cfg is set (and returned) calling
            entry_to_config(entry). Now the entry_to_config returns a
            structure on the stack so it is not valid to access outside
            of the entry_to_config
          o   authcfg_fini free the configs. config->cfg should have
            been allocated and must be freed (be care that configs->cfg
            may contains DEFAULTS)
          o authcfg_get_auth_types:322 should it return 'gbl' or
            AUTHCFG_AUTH_TYPE_PASSWORD
          o authcfg_get_auth_window/authcfg_get_sync_window returns a
            window structure that is on the stack. It is not valid
            outside of those functions

    thanks
    thierry

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20141107/514d84ba/attachment.htm>