Bug 869334

Summary: if "filter: ..." header is set, use query instead of search to list items
Product: Red Hat Enterprise Virtualization Manager Reporter: David Jaša <djasa>
Component: ovirt-engine-restapiAssignee: Ravi Nori <rnori>
Status: CLOSED CURRENTRELEASE QA Contact: Ondra Machacek <omachace>
Severity: medium Docs Contact:
Priority: unspecified    
Version: 3.1.0CC: dyasny, ecohen, iheim, italkohe, mpastern, omachace, oourfali, Rhev-m-bugs, sgrinber, ykaul, yzaslavs
Target Milestone: ---Keywords: Reopened, ZStream
Target Release: 3.2.0   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard: infra
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 08:30:54 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: Infra RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 882946, 915537    
Attachments:
Description Flags
r-v debug output none

Description David Jaša 2012-10-23 15:34:44 UTC
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 15:37:05 UTC
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 16:11:49 UTC
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 08:30:54 UTC
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 12:36:09 UTC
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 13:19:00 UTC
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 14:21:33 UTC
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 08:03:22 UTC
*** Bug 868921 has been marked as a duplicate of this bug. ***

Comment 12 Ravi Nori 2012-11-01 21:16:56 UTC
link : http://gerrit.ovirt.org/#/c/8987/

hash : 687fcf3b31ca5627bc31187097321197a9f2f42c

Comment 13 Michael Pasternak 2012-11-04 07:31:38 UTC
*** Bug 872147 has been marked as a duplicate of this bug. ***

Comment 14 Michael Pasternak 2012-11-11 07:43:12 UTC
*** Bug 874705 has been marked as a duplicate of this bug. ***

Comment 15 Ravi Nori 2012-11-13 16:55:18 UTC
link : http://gerrit.ovirt.org/#/c/8987/

Hash : I54b79dc86e4f6d6b28c5e720200b093d246f0806

Comment 16 David Jaša 2012-11-28 16:33:50 UTC
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 19:07:39 UTC
(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 21:02:48 UTC
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 09:12:16 UTC
(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 12:44:30 UTC
(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 16:20:06 UTC
(In reply to comment #21)
> I don't recall we have any permission model for tags, Oved?

Comment 23 Oved Ourfali 2013-03-11 07:07:23 UTC
(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 18:40:55 UTC
(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 08:57:10 UTC
3.2 has been released

Comment 26 Itamar Heim 2013-06-11 08:57:11 UTC
3.2 has been released

Comment 27 Itamar Heim 2013-06-11 08:57:11 UTC
3.2 has been released

Comment 28 Itamar Heim 2013-06-11 08:59:32 UTC
3.2 has been released

Comment 29 Itamar Heim 2013-06-11 09:28:49 UTC
3.2 has been released