[Freeipa-devel] [PATCH 0064] Create ipa-otp-decrement 389DS plugin

[thierry bordaz]

Hello Nathaniel,

    sanitize_input translates MOD/REPLACE into MOD/DEL+MOD/ADD. It looks
    good but difficult to think to all possible cases.
    I think to the following corner case:
    The initial entry has ipatokenHOTPcounter=5
    ldapmodify..
    changetype: modify
    add: ipatokenHOTPcounter
    ipatokenHOTPcounter: 6
    -
    replace: ipatokenHOTPcounter
    ipatokenHOTPcounter: 7

    It translates
    add: 6
    del: 5
    add: 7

    This operation will fail because ipatokenHOTPcounter is
    single-valued although IMHO it should succeed.
    This is a so special operation that is may not really be a concern.

    It is important that attribute are single valued. The replication
    changelog will replicated MOD/DEL + MOD/ADD for a MOD/REPL.
    That means that if the attributes are updated on several masters,
    the number of values can likely increase. Where for single value it
    should only keep the most recent value.

    thanks
    theirry

On 09/19/2014 10:15 PM, Nathaniel McCallum wrote:
> This new version fixes a small style issue pointed out to me by richm
> (thanks!).
>
> On Fri, 2014-09-19 at 13:39 -0400, Nathaniel McCallum wrote:
>> The attached version of the patch should solve all of these issues. It
>> should also be more performant and use less memory.
>>
>> Nathaniel
>>
>>
>> On Wed, 2014-09-17 at 15:33 +0200, thierry bordaz wrote:
>>> On 09/15/2014 09:05 PM, Nathaniel McCallum wrote:
>>>
>>>> This plugin ensures that all counter/watermark operations are atomic
>>>> and never decrement. Also, deletion is not permitted.
>>>>
>>>> https://fedorahosted.org/freeipa/ticket/4494
>>>>
>>>>
>>>> _______________________________________________
>>>> Freeipa-devel mailing list
>>>> Freeipa-devel at redhat.com
>>>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>>> Hello Nathaniel,
>>>
>>>          More thoughts.. I think being "betxnpreoperation" you are safe
>>>          with parallel client ops and the check is atomic. I have few
>>>          comments:
>>>                * Shouldn't be implemented
>>>                  SLAPI_PLUGIN_CLOSE_FN/SLAPI_PLUGIN_START_FN callback,
>>>                  even if they are empty.
>>>                * In load_counters, in case we have a target entry that
>>>                  has not 'objectclass'  'ipatokenHOTP|ipatokenTOTP' and
>>>                  the mod operation:
>>>                  dn: <entry>
>>>                  changetype: modify
>>>                  replace: ipatokenHOTPcounter
>>>                  ipatokenHOTPcounter: 200
>>>                  -
>>>                  add: objectclass
>>>                  objectclass: ipatokenHOTP
>>>                  
>>>                  
>>>                  I wonder if the operation will not fail although IMHO
>>>                  it should succeeds.
>>>                  Shouldn't let the schema checking reject the operation
>>>                  if the attribute is not granted by the entry
>>>                  objectclass
>>>                * in load_counters, I am under the impression it may
>>>                  return  ipatokenHOTPcounter or ipatokenTOTPwatermark
>>>                  (ipatokenHOTPcounter is missing).
>>>                  But then how the caller knows that the returned value
>>>                  is a counter or a watermark ?
>>>                * in ldapmod_is_attrs you may prefer PL_strcasecmp to
>>>                  strcasecmp
>>>          
>>>          About replicated updates, if updates of counters/watermark are
>>>          done on several servers. Then a replicated operation may want
>>>          to set counters/watermark with a smaller value that the
>>>          existing one. In that case, it will return unwilling to
>>>          perform. That could break replication.
>>>          If the update comes from replication and the value is going
>>>          backward, we could make the operation a nop operation (setting
>>>          the attribute to its current value).
>>>          
>>>          
>>>          thanks
>>>          theirry
>>>          
>>>          
>> _______________________________________________
>> Freeipa-devel mailing list
>> Freeipa-devel at redhat.com
>> https://www.redhat.com/mailman/listinfo/freeipa-devel

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