[Freeipa-devel] [PATCH] 0210 frontend: fix output validation for multiple type choices

[Jan Cholasta]

On 18.7.2016 10:12, Alexander Bokovoy wrote:
> On Mon, 18 Jul 2016, Jan Cholasta wrote:
>> Hi,
>>
>> On 16.7.2016 12:46, Alexander Bokovoy wrote:
>>> Hi,
>>>
>>> I had some time and was blocked by these bugs to do my tickets so I
>>> actually fixed these three problems that are assigned to Martin
>>> Babinsky. Hopefully, Martin wouldn't be offended by that. :)
>>>
>>> ------
>>>
>>> Output entry elements may have multiple types allowed. We need to check
>>> all of them to properly validate the output. Right now, thin client
>>> receives type specifications for elements as tuples of types, so
>>> what is seen as 'None' on the server side becomes (type(None),) tuple
>>> on the thin client side.
>>>
>>> Change validation to account this by processing each separate type
>>> of the element and account for both None and type(None). Raise type
>>> error only if all of the type checks failed.
>>>
>>> https://fedorahosted.org/freeipa/ticket/6061
>>
>> NACK, this only hides the real issue, which is that trustconfig-show
>> (and automember-set-default in #6037) claims to return the primary key
>> of the object in the 'value' output field, but the object does not
>> have a primary key, so the client rightfully expects None.
> Why did it work before introducing thin client?

Because I took the liberty of not putting any extra effort just to keep 
old hacks working on thin client. In this case, output.PrimaryKey was 
used for the 'value' output field before, which always allowed unicode 
in addition to the primary key type. On thin client, output.Output with 
the primary key type is used instead, which is less forgiving and 
uncovers bogus command definitions. (There aren't any besides the ones 
already mentioned, I remember fixing them but the commit got lost 
somehow. Oh well.)

>
> Aside from that, comparing "(type(None),) is None" will never give you
> True on the thin client side. At the server side we have "None is None"
> and that works. So the question is also why there is a change like that
> between client and server sides?

I don't see how this has anything to do with anything. In output 
validation, "type = None" means that value of any type is allowed, "type 
= (type(None),)" means that only None is allowed. Nothing changed with 
regard to that in thin client.

>
>> A proper fix would be to set "has_output = output.simple_value" for
>> these commands (all of automember_default_group_{set,remove,show},
>> trustconfig_{mod,show}).
>>
>> Honza
>>
>> --
>> Jan Cholasta
>


-- 
Jan Cholasta