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

[Jan Cholasta]

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


-- 
Jan Cholasta