This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours
Bug 869334 - if "filter: ..." header is set, use query instead of search to list items
if "filter: ..." header is set, use query instead of search to list items
Status: CLOSED CURRENTRELEASE
Product: Red Hat Enterprise Virtualization Manager
Classification: Red Hat
Component: ovirt-engine-restapi (Show other bugs)
3.1.0
Unspecified Unspecified
unspecified Severity medium
: ---
: 3.2.0
Assigned To: Ravi Nori
Ondra Machacek
infra
: Reopened, ZStream
: 868921 (view as bug list)
Depends On:
Blocks: 882946 915537
  Show dependency treegraph
 
Reported: 2012-10-23 11:34 EDT by David Jaša
Modified: 2016-02-10 14:43 EST (History)
11 users (show)

See Also:
Fixed In Version: sf4
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
: 882946 (view as bug list)
Environment:
Last Closed: 2012-10-24 04:30:54 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: Infra
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)
r-v debug output (19.59 KB, text/plain)
2012-11-28 11:33 EST, David Jaša
no flags Details

  None (edit)
Description David Jaša 2012-10-23 11:34:44 EDT
Description of problem:
if "filter: ..." header is set, do not return links that the user has no permission to access.

Version-Release number of selected component (if applicable):
si19.1 / 3.1.0-18

How reproducible:
always

Steps to Reproduce:
1. curl -u regular_user ... -H "filter: true" https://rhevm.example.com/api
2. curl -u regular_user ... -H "filter: true" https://rhevm.example.com/api/users
3.
  
Actual results:
1. ...<link rel="users" .../>
2. permission denied

Expected results:
1. this link is omitted from results

Additional info:
Comment 1 David Jaša 2012-10-23 11:37:05 EDT
one clarification: omit results inaccessible to user with that mode of filtering on - don't show "administrative" links to user with "filter: false" or unset and don't show user-only links (if any) to user with "filter: true"
Comment 2 Michael Pasternak 2012-10-23 12:11:49 EDT
this is not feasible, permission based filtering done on a backend side, api is a
pipe to backend only, implementing user aware context in api is incorrect from
business logic PoV and will override backend rules,

api considerate only representation logic (which is works perfectly from 
REST PoV)
Comment 3 Michael Pasternak 2012-10-24 04:30:54 EDT
closing based the fact that we also return all actions regardless of 
resource state what is justified by the stateless nature of api and Comment 2.
Comment 4 Michael Pasternak 2012-10-24 08:36:09 EDT
apparently we have another use case where user should not see collections
regardless of his permissions, such as /disks, /networks etc.,
(i.e this is admin only collections)

this is a only case where api collection resources should be filtered out
in user api.
Comment 6 Oved Ourfali 2012-10-24 09:19:00 EDT
Posted patch:
http://gerrit.ovirt.org/8781

It removes the unsupported resources from the API resource for filtered requests:

<link rel="events" href="/api/events"/>
<link rel="events/search" href="/api/events;from={event_id}?search={query}"/>
<link rel="hosts" href="/api/hosts"/>
<link rel="hosts/search" href="/api/hosts?search={query}"/>
<link rel="networks" href="/api/networks"/>
{query}"/>
<link rel="tags" href="/api/tags"/>
<link rel="users" href="/api/users"/>
<link rel="users/search" href="/api/users?search={query}"/>
<link rel="groups" href="/api/groups"/>
<link rel="groups/search" href="/api/groups?search={query}"/>
<link rel="vmpools" href="/api/vmpools"/>
<link rel="vmpools/search" href="/api/vmpools?search={query}"/>
<link rel="disks" href="/api/disks"/>
<link rel="disks/search" href="/api/disks?search={query}"/>
Comment 7 Michael Pasternak 2012-10-24 10:21:33 EDT
after thinking on this a bit more, mentioned solution won't scale from
the client PoV,

for instance we will have to generate two sdks, one for admins and
another one for non-admins ...,

moving this bz to feature for future discussion.
Comment 11 Michael Pasternak 2012-10-25 04:03:22 EDT
*** Bug 868921 has been marked as a duplicate of this bug. ***
Comment 12 Ravi Nori 2012-11-01 17:16:56 EDT
link : http://gerrit.ovirt.org/#/c/8987/

hash : 687fcf3b31ca5627bc31187097321197a9f2f42c
Comment 13 Michael Pasternak 2012-11-04 02:31:38 EST
*** Bug 872147 has been marked as a duplicate of this bug. ***
Comment 14 Michael Pasternak 2012-11-11 02:43:12 EST
*** Bug 874705 has been marked as a duplicate of this bug. ***
Comment 15 Ravi Nori 2012-11-13 11:55:18 EST
link : http://gerrit.ovirt.org/#/c/8987/

Hash : I54b79dc86e4f6d6b28c5e720200b093d246f0806
Comment 16 David Jaša 2012-11-28 11:33:50 EST
Created attachment 653601 [details]
r-v debug output

I managed to hit yet another error shortly after the r-v actually connected - not sure if it is the same or something else...
Comment 18 Ravi Nori 2013-01-08 14:07:39 EST
(In reply to comment #16)
> Created attachment 653601 [details]
> r-v debug output
> 
> I managed to hit yet another error shortly after the r-v actually connected
> - not sure if it is the same or something else...

Does look like the exception is related to this bug
Comment 19 Ondra Machacek 2013-02-04 16:02:48 EST
Should user have access also to /tags url? 
Because it is still returning "query execution failed due to insufficient permissions."
Comment 20 Michael Pasternak 2013-02-05 04:12:16 EST
(In reply to comment #19)
> Should user have access also to /tags url? 
> Because it is still returning "query execution failed due to insufficient
> permissions."

Simon?
Comment 21 Simon Grinberg 2013-02-05 07:44:30 EST
(In reply to comment #20)
> (In reply to comment #19)
> > Should user have access also to /tags url? 
> > Because it is still returning "query execution failed due to insufficient
> > permissions."
> 
> Simon?

I don't recall we have any permission model for tags, Miki?
Comment 22 Simon Grinberg 2013-03-10 12:20:06 EDT
(In reply to comment #21)
> I don't recall we have any permission model for tags, Oved?
Comment 23 Oved Ourfali 2013-03-11 03:07:23 EDT
(In reply to comment #22)
> (In reply to comment #21)
> > I don't recall we have any permission model for tags, Oved?

Well, I saw two parts of permissions for tags:
1. In webadmin, the administrator can assign a tag for users/groups, and when asking for tags the user will only get tags that are part of his user or groups he is a member of.
2. All the queries to get tags are not user queries, so the user is not allowed to run them at all, that's why it fails for user queries.

So the question here is whether or not we want users to see tags.
Simon?
Comment 24 Simon Grinberg 2013-03-21 14:40:55 EDT
(In reply to comment #23)
> (In reply to comment #22)
> > (In reply to comment #21)
> > > I don't recall we have any permission model for tags, Oved?
> 
> Well, I saw two parts of permissions for tags:
> 1. In webadmin, the administrator can assign a tag for users/groups, and
> when asking for tags the user will only get tags that are part of his user
> or groups he is a member of.
> 2. All the queries to get tags are not user queries, so the user is not
> allowed to run them at all, that's why it fails for user queries.
> 
> So the question here is whether or not we want users to see tags.
> Simon?

OK, thanks, now I understand the question.
No, users should not be allowed to see tags since they can't use them in the first place. To see a tag a user has to have Admin permissions and not just user permissions. The behavior should be similar to the UI in this case. Generally, the same concept as the UI should be applied. If a user has access to the webadmin then he is also allowed to see the same object in the API, if he has no access to the webadmin but only to the user portal, then he should only be allowed to see objects that are exposed in the userPortal (which tags are not available there)
Comment 25 Itamar Heim 2013-06-11 04:57:10 EDT
3.2 has been released
Comment 26 Itamar Heim 2013-06-11 04:57:11 EDT
3.2 has been released
Comment 27 Itamar Heim 2013-06-11 04:57:11 EDT
3.2 has been released
Comment 28 Itamar Heim 2013-06-11 04:59:32 EDT
3.2 has been released
Comment 29 Itamar Heim 2013-06-11 05:28:49 EDT
3.2 has been released

Note You need to log in before you can comment on or make changes to this bug.