Hi Juan,

Why not fix this to rhevm 3.5 ?

I'm targeting 4.0 because I think that the solution will be complicated, so we won't have time to fix it in the 3.5 or 3.6 time frame. However, if the rest of the stakeholders believe that it should be fixed earlier I'm open to discuss and eventually re-target.

This bug breaks user-built scripts, i.e. I often use api.templates/vms/something.get() with the python-SDK. If the search parameter is ignored, the get method obtains mutliple results, and raises an exception.

It can be worked around by direct object definition, or by listing all objects and filtering done in Python, however, both approaches require non-trivial changes to already proven and tested scripts.

Juan,

would it be possible to allocate some reasonable amount of time for initial bug investigation? I would really like to avoiding shifting of this bug to future releases unless absolutelly necessary.

(In reply to Petr Spacek from comment #4)
> Juan,
> 
> would it be possible to allocate some reasonable amount of time for initial
> bug investigation? I would really like to avoiding shifting of this bug to
> future releases unless absolutelly necessary.

As I wrote in comment 2 I'm open to re-target this bug. I count Tareq's, Tomas' and your comments as positive stakeholder votes. Barak, I would like to know your view on this.

Eli please take (talk to Juan) a look and share your thoughts.
The target version for this bug should depend on the complexity and instabilities it might cause (introducing filters to the search backend). as long as there are workarounds for the deeper problem (using filer in searches ... might lead to a severe situation), we can allow postponing it to a later stage).
BTW it never worked.

the reason for this request actually originates from bug 1160331. If that one works this bug would have less priority...

Clarification:
We did not notice this bug in our testing environment because we were using 'admin' user for everything.

Now we tried to move from testing to "supported"/"production" RHEV instance where admin is not for everyone anymore and found out that it does not work.

Adding the following information I had got from Juan in order to make the problem more clear :


If you are user U1 and you try to use the RESTAPI to get
the list of VMs you will by default get a permissions error. This
happens because the RESTAPI uses SearchType.VM to perform this request.

To be able to run the request without permission errors you have to use
the "Filter" header, like this:

#!/bin/sh -ex

url="https://ovirt.example.com/ovirt-engine/api";
user="admin@internal"
password="******"

curl \
--verbose \
--insecure \
--user "${user}:${password}" \
--header "Accept: application/xml" \
--header "Filter: true" \
"${url}/vms"

What will happen when you use this "Filter" header is that the RESTAPI
use VdcQueryType.GetAllVms instead of SearchType.VM. Then you will get
all the VMs that you have permissions for.

The problem is that when you use VdcQueryType.GetAllVms the "search"
parameter is completely ignored. For example, if you do this:

curl \
--verbose \
--insecure \
--user "${user}:${password}" \
--header "Accept: application/xml" \
--header "Filter: true" \
"${url}/vms?search=name%3Dmyvm"

The "search" parameter is ignored, and instead of getting the "myvm"
virtual machine you get all the VMs that you have permissions for. This
is very dangerous, because you request one VM and you may get a
different one as the result. Exactly that happens with the CLI, and
results in removing the wrong VM, for example.

All this is by design of the user API feature, and it is wrong design,
in my opinion.

The fix for this, in my opinion, is to modify the implementation of
SearchType.VM so that it does filtering, like VdcQueryType.GetAllVms
does, so that you get only the VMs that you have permissions for.

Same happens for other objects, not only VMs.

I agree with Juan, we should extend the Search Engine to support filtering according to the user permissions.

However, this is a major change, IMO , in order to see if we can do that fro 3.6 we should do a POC that takes one of the entities like VM and add filtering support for it in the Search Engine. After that we can estimate the work and decide if to do that for 3.6 or 4.0

well, considering user level api doesn't support search, i agree its misleading and we should consider failing such a request with an error as search isn't supported.
supporting search is not necessarily trivial.
Petr - is there anything you are doing which the user level api prevents you from doing without the search capabilities?

(In reply to Michal Skrivanek from comment #8)
> the reason for this request actually originates from bug 1160331. If that
> one works this bug would have less priority...

michal - how would this solve the fact admin queries should be still blocked?
(and why would a non admin should succeed logging in as admin[1])

[1] yes, i know usability of this is very low today.

(In reply to Michal Skrivanek from comment #8)
> the reason for this request actually originates from bug 1160331. If that
> one works this bug would have less priority...

Note also that fixing bug 1160331 only won't bring any benefit, as even if a normal (non admin) user is allowed to connect to the API without the "Filter: true" header she/he won't be able to get any useful result later, as most queries will just return a permissions error. The fix for that bug has to be combined with the ability to use "search" and "Filter: true" together, otherwise it is useless.

Responding to Comment 12.

Pretty much nothing more complex can be scripted using the user level API with this bug.

We're using python-sdk, and anytime I call api.<something>.get(key=value), I trigger this bug and I am getting AmbiguousError (only one value was expected, but API returned multiple, since search was ignored).

This prevents you from doing simple things like:

api.template.get('Blank')
api.vms.get(name='myvm')

and such. See my Comment 3 for unacceptable workaround.

Tomas, note that the currently proposed fix for bug 1155678 may also be helpful for you (if it is eventually merged) as it will make the Python SDK "get" method work as you expect, without the workaround.

*** Bug 1132501 has been marked as a duplicate of this bug. ***

(In reply to Itamar Heim from comment #13)
> (In reply to Michal Skrivanek from comment #8)
> > the reason for this request actually originates from bug 1160331. If that
> > one works this bug would have less priority...
> 
> michal - how would this solve the fact admin queries should be still blocked?
> (and why would a non admin should succeed logging in as admin[1])

right. I suppose the confusion is about a non-obvious fact that 
we differentiate between admin and non-admin access only by the "Filter" header
The use of this is misleading and should not be needed, right, since we know if the connecting user is an admin or not.

Because the admin may not want to use his admin rights. Think user portal.

(In reply to Itamar Heim from comment #19)
> Because the admin may not want to use his admin rights. Think user portal.

I'm sorry but I have to comment on this. This approach really seems as security through obscurity to me. What prevents e.g. rogue Javascript in user's web browser to issue API requests with Filter = False despite the fact that user is logged into User portal?

I would argue that admin principal should simply be separate account with other username/password so access control does not need to depend on user-supplied parameters like 'Filter'.

filter=false is blocked for non admins.
this is not security by obscurity.
we distingiush between user permissions and admin permissions. we let the (same) user specify the role in which they want to act (if they have both).
this only affects queries/searches.

(In reply to Itamar Heim from comment #21)
> this is not security by obscurity.
My point is that filter=true gives admins false sense of security while using User Portal.

> we distingiush between user permissions and admin permissions. we let the
> (same) user specify the role in which they want to act (if they have both).
> this only affects queries/searches.
That would be understandable as ugly but workable hack for user interface implementation but it is not only that: It actually changes API behavior outside of query/search:
See https://bugzilla.redhat.com/show_bug.cgi?id=1121144: User doesn't get the UserVmManager permission on a VM when VM is created with using via API with filter=false.

*** This bug has been marked as a duplicate of bug 1163105 ***