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

[Petr Viktorin]

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.

>>> 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?

In the future it might be useful for add_entry to return a list of 
controls, the `ctrls` in this patch.

-- 
Petr³