[Freeipa-devel] [HELP] Regular users should not be able to add OTP tokens with custom name

[Petr Spacek]

On 14.10.2014 20:33, Nathaniel McCallum wrote:
> On Tue, 2014-10-14 at 10:38 +0200, Jan Cholasta wrote:
>> Dne 14.10.2014 v 10:23 Petr Viktorin napsal(a):
>>> On 10/14/2014 08:51 AM, Jan Cholasta wrote:
>>>> Dne 14.10.2014 v 08:37 Martin Kosek napsal(a):
>>>>> On 10/13/2014 07:23 PM, Nathaniel McCallum wrote:
>>>>>> On Mon, 2014-10-13 at 12:39 +0200, Martin Kosek wrote:
>>>>>>> Also, few comments to your current patch set (though the patches
>>>>>>> themselves
>>>>>>> will probably not land in 4.1):
>>>>>>>
>>>>>>> Patch 0001:
>>>>>>> - while it may work (after DS fixes), it adds PostRead for all our
>>>>>>> commands,
>>>>>>> not just otptoken-add. I would rather have some attribute like
>>>>>>> "read_dn_from_postread" on LDAPObject which defaults to False to
>>>>>>> make sure
>>>>>>> there is no regression or performance hit with other LDAP calls.
>>>
>>> As Honza says later in the mail, this should be an argument for
>>> add_entry (or alternatively an additional add_entry_with_postread
>>> method). Storing settings in data objects leads to trouble – when you
>>> get such an object from somewhere, you never know what an operation on
>>> it will do.
>>
>> I would prefer add_entry argument rather than a new method, as that's
>> how find_entries does controls.
>>
>>>
>>>>>> In the new code, we actually get a performance gain as the manual post
>>>>>> read is eliminated if the post read control is successful. Only one
>>>>>> issue can arise, which is when the post read control evaluates ACIs
>>>>>> in a
>>>>>> different context than a normal manual read. This problem is well known
>>>>>> and is trivial to fix (s/USERDN/SELFDN/).
>>>>>
>>>>> That's my point - with such a big change, we can hit many unforeseen
>>>>> issues and
>>>>> we are close to deadline. I would rather like to use the PostRead
>>>>> control only
>>>>> in otptoken_add command. CCing Petr and Honza to chime in.
>>>>
>>>> I agree it should be opt-in for now. Add a boolean argument to add_entry
>>>> as a switch.
>>>
>>> +1
>>>
>>> Also, I think the add_entry should not return anything; rather the
>>> PostRead result should be merged into the added entry object. Honza,
>>> what do you think?
>>
>> It should, good point.
>>
>>>
>>> In the future it might be useful for add_entry to return a list of
>>> controls, the `ctrls` in this patch.
>
> If we're going to implement temporary workarounds like this in order to
> merge this patch, I'd prefer to just wait until 4.2. Without activating
> the post read control for all add operations, there is really no benefit
> to this patch and added risk.

It would be really nice to has this opt-in feature in 4.1 - it would make 
DNSSEC support easier for me.

I guess that answer is 'no' so I'm going to implement a workaround.

> I propose that for 4.1, we use the attached patch to remove the field
> from the UI. Once we have proper ACI/UUID-plugin integration in 389, we
> can revisit these patches.

-- 
Petr^2 Spacek