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

[thierry bordaz]

Hello Nathaniel,

    I have a separated remark about updating the mods.

    modifications of the entry occurs in two phases:

      * call BE_PREOP plugins then apply the mods on the entry
      * call BE_TXN_PREOP plugin then apply *only* extra mods on the entry

    The plugin (BE_TXN_PREOP) translates a MOD/replace from MOD/DEL
    (current value) + MOD/ADD(new value).
    The translation is done like this:
    MOD[0]: MOD/REPL ipatokenHOTPcounter new_value
    MOD[1]: MOD/REPL modifiersname
    MOD[2]: MOD/REPL modifytimestamp
    MOD[3]: MOD/REPL entryusn

    -->
    MOD[0]: MOD/DEL ipatokenHOTPcounter current_value
    MOD[1]: MOD/ADD ipatokenHOTPcounter new_value
    MOD[2]: MOD/REPL modifiersname
    MOD[3]: MOD/REPL modifytimestamp
    MOD[4]: MOD/REPL entryusn

    That means, we have one added mod (MOD[4]=entryusn) after
    BE_TXN_PREOP that will be (re)applied on the second phase.
    So the MOD/DEL+MOD/ADD will not be processed. MOD/REPL will be
    processed before MOD/DEL+MOD/ADD are generated.
    If MOD/DEL+MOD/ADD is needed, then we may need to replace
    REPL->DEL/ADD in BE_PREOP callback.

    The control done in plugin BE_TXN_PREOP is working fine and
    guaranties the counter does not decrease.

    thanks
    thierry

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/20140922/d7d789d5/attachment.htm>