Bug 1121144

seems to me the NIC errors are not related, tried on a different user and still there's no UserVmManager.
Omer?

Anyway, the template was Blank and there was no NICs, so why ReorderVmNics?

(better the other way around)
Lior, any insight regarding the ReorderVmNics err?

Yes, the command is triggered without checking the number of NICs, and does nothing if there are no NICs (but doesn't fail - the failure here is due to the permissions). It's easy to change but I don't think it requires changing.

Mihcal, what client did you use to create the vm?
the permissions are created upon client request, and afair, the webadmin always sends false, user portal sends true, and the rest-api has some logic when to request create-permissions

that's an interesting behavior, ok.
though it doesn't make sense for ReadOnlyAdmin role. We should check whether one can actually add the permission afterwards

Oved, thoughts? I guess this is part of the ReadOnlyAdmin feature…we should reconsider the check or leave as is?

You didn't answer what client did you use?

Webadmin; browser…not sure, either FF 24.7.0 ESR or Safari 6
is it relevant?

(In reply to Michal Skrivanek from comment #8)
> Webadmin; browser…not sure, either FF 24.7.0 ESR or Safari 6
> is it relevant?

It is relevant, as when adding VMs through the webadmin (or admin API), you are not supposed to grant any permissions on them. Only when doing that through the User Portal (or user API).

The only issue in this bug is that we reorder the VM nics, and in order to do that you must have the CONFIGURE_VM_NETWORK permission. See VdcActionType
ReorderVmNics(17, ActionGroup.CONFIGURE_VM_NETWORK, false, QuotaDependency.NONE)

Moving the bug back to network to consider whether to change this requirement or not.

Created Bug 1134554 for the re-ordering issue, leaving this in case there's a design issue with ReadOnlyAdmin.

(In reply to Lior Vernia from comment #10)
> Created Bug 1134554 for the re-ordering issue, leaving this in case there's
> a design issue with ReadOnlyAdmin.

I don't understand what design issue are you talking about.
Closing this one as not a bug.

(In reply to Oved Ourfali from comment #11)
> (In reply to Lior Vernia from comment #10)
> > Created Bug 1134554 for the re-ordering issue, leaving this in case there's
> > a design issue with ReadOnlyAdmin.
> 
> I don't understand what design issue are you talking about.
> Closing this one as not a bug.

the design issue is that the current check for isAdmin() doesn't make sense. In case of ReadOnlyAdmin the user is not able to add any permission, even for himself. comment #5

It is a bug. If you are ReadOnlyAdmin, and create a VM, the VM apaprently requires admin privileges for its management (i.e. to add network interface).

However, you don't have these admin privileges, thus you cannot manage a VM you created, which is a serious flaw.

See the traceback from my script which tries to add network interface to VM that it has just created.

Traceback (most recent call last):
  File "./make_vm_template.py", line 42, in <module>
    rhev_vm.create(memory = 3*rhevm.GB)
  File "/home/pspacek/rhev/rhevm.py", line 208, in create
    self._get_me().nics.add(nic)
  File "/usr/lib/python2.6/site-packages/ovirtsdk/infrastructure/brokers.py", line 18002, in add
    headers={"Expect":expect, "Correlation-Id":correlation_id}
  File "/usr/lib/python2.6/site-packages/ovirtsdk/infrastructure/proxy.py", line 88, in add
    return self.request('POST', url, body, headers)
  File "/usr/lib/python2.6/site-packages/ovirtsdk/infrastructure/proxy.py", line 118, in request
    persistent_auth=self._persistent_auth)
  File "/usr/lib/python2.6/site-packages/ovirtsdk/infrastructure/proxy.py", line 140, in __doRequest
    persistent_auth=persistent_auth
  File "/usr/lib/python2.6/site-packages/ovirtsdk/web/connection.py", line 134, in doRequest
    raise RequestError, response
ovirtsdk.infrastructure.errors.RequestError: 
status: 400

The same behaviour can be reproduced using webUI.
reason: Bad Request
detail: User is not authorized to perform this action.

(In reply to Michal Skrivanek from comment #12)
> (In reply to Oved Ourfali from comment #11)
> > (In reply to Lior Vernia from comment #10)
> > > Created Bug 1134554 for the re-ordering issue, leaving this in case there's
> > > a design issue with ReadOnlyAdmin.
> > 
> > I don't understand what design issue are you talking about.
> > Closing this one as not a bug.
> 
> the design issue is that the current check for isAdmin() doesn't make sense.
> In case of ReadOnlyAdmin the user is not able to add any permission, even
> for himself. comment #5

What isAdmin check?
Please elaborate more here.
ReadOnlyAdmin is an admin role that doesn't allow doing operations.
So he shouldn't be able to add permissions as well.
If he can add a VM then something is wrong, and you need to address that as part of the permissions required for adding a VM.

(In reply to Tomas Babej from comment #13)
> It is a bug. If you are ReadOnlyAdmin, and create a VM, the VM apaprently
> requires admin privileges for its management (i.e. to add network interface).
> 

If you only have ReadOnlyAdmin then you shouldn't be able to add VMs at all.
If you can then it is a bug in the AddVm flow.

The same user has a user level permission allowing to create VM, PowerUserRole.

(In reply to Michal Skrivanek from comment #16)
> The same user has a user level permission allowing to create VM,
> PowerUserRole.

So he is lacking permissions to reorder the nics, right?

In addition, there was a decision not to grant permissions on VMs automatically when creating a VM through the admin portal.
We can easily change it, but that's the behavior that was decided.
So if you need further permissions then you should add them.
The ReadOnlyAdmin was created to support a ReadOnlyAdmin use-case.
If you add other roles you need to make sure they are complete, and satisfies this users needs.

Do you have another suggestion here?

(In reply to Oved Ourfali from comment #17)
> (In reply to Michal Skrivanek from comment #16)
> > The same user has a user level permission allowing to create VM,
> > PowerUserRole.
> 
> So he is lacking permissions to reorder the nics, right?

right. that's supposedly fixed in 3.5

 
> In addition, there was a decision not to grant permissions on VMs
> automatically when creating a VM through the admin portal.

that's fine, understood (though probably not documented)
when done via User Portal the permissions are correct
However REST API is the same as webadmin, so the VM is not usable.

> We can easily change it, but that's the behavior that was decided.
> So if you need further permissions then you should add them.
> The ReadOnlyAdmin was created to support a ReadOnlyAdmin use-case.
> If you add other roles you need to make sure they are complete, and
> satisfies this users needs.
> 
> Do you have another suggestion here?

One solution would be to fix REST API to not require admin role for any connection to API (bug 1160331). I thought ReadOnlyAdmin might be a suitable workaround but it seems it's not

Still, the fact that the permissions required to manage the VM differ when using different interfaces (userportal vs. adminportal) seems as a serious bug to me.

VMs created by the same user should require the same pemissions regardless of the way they were created.

No comment here explained the real issue and use-case... so it is not possible to solve this without these details.

1. It originally started with an issue to reorder nics, which was resolved.
2. Then moved to some lack of design of ReadOnlyAdmin.
3. The purpose of ReadOnlyAdmin is to be able to see everything, and do nothing... and that's what it does.
4. If you want the user to be able to do more than that, then you need to give him proper permissions for it.
5. The fact that the behavior is different for admins and users is by design, as the use-case is different. Users usually create VMs for themselves, whereas admins create them for others.

Again, I suggest to close this one.
Also, based on that, reducing urgency.

Scott?

Even if you're ReadOnlyAdmin, it's legitimate use case to create a VM for yourself using the webadmin interface.

Currently, you can create it, but you cannot do anything with it (delete, add disks / interfaces).

The attributes of the created VMs for the user should not differ depending on the what interface (UserPortal or WebAdmin) he used. This is simply not a sane behaviour and not something that any user will expect.

If that's the behaviour according to the design, than the design is flawed.

(In reply to Tomas Babej from comment #21)
> Even if you're ReadOnlyAdmin, it's legitimate use case to create a VM for
> yourself using the webadmin interface.
> 
> Currently, you can create it, but you cannot do anything with it (delete,
> add disks / interfaces).
> 
> The attributes of the created VMs for the user should not differ depending
> on the what interface (UserPortal or WebAdmin) he used. This is simply not a
> sane behaviour and not something that any user will expect.
> 
> If that's the behaviour according to the design, than the design is flawed.

ReadOnlyAdmin is a role, and it is up to the administrator to use it.
We don't protect the user from setting wrong permissions.
We could have, but we don't.
In some cases it might be a good thing, and in others it might not, but we decided not to protect/limit/monitor the usage of permissions.

my 2 cents:
first, the idea of different permissions to create and login the vm came from this kind of scenario: Admin creates vms for users, he is using the webadmin (or api), so we dont want him to be the owner of ther vms, he needs to give permissions to the right user on the vm, this is why owner is not set when using webadmin/api.

the second scenario, is the power user, that creates vms to himself, he should be using the user portal, so he is automatically granted with vm owner.

this bug describes a scenario where power user, with some other admin permission (not on the vms, like the readOnlyAdmin) is creating vms, and using the webadmin (which wasn't the intention), this is why he dont have the permission, and can't use the vms.

i suggest to expose the flag "grant owner permission" in the webadmin and api (default to false) and allow the admin to set it (assuming there is no security issue with that..)

Responding to Comment 22.

The problem here is not monitoring setting the permissions. The problem here is that user with ReadOnlyAdmin role can create VMs for himself which are then not managable.

ReadOnlyAdmin has no way to set the permissions himself.

The way I see it, you should either:
a) Prohibit creation of VMs in the webadmin for users with ReadOnlyAdmin role, since then they cannot manage them.
b) Make the VMs created in the webadmin managable as though they were created using the UserPortal (this can be optional as Comment 23 suggests).

(In reply to Tomas Babej from comment #25)
> Responding to Comment 22.
> 
> The problem here is not monitoring setting the permissions. The problem here
> is that user with ReadOnlyAdmin role can create VMs for himself which are
> then not managable.
> 
> ReadOnlyAdmin has no way to set the permissions himself.
> 
> The way I see it, you should either:
> a) Prohibit creation of VMs in the webadmin for users with ReadOnlyAdmin
> role, since then they cannot manage them.

We won't check what permissions was used to create a VM. Again, we don't protect administrators in that way. They need to set proper permissions.

> b) Make the VMs created in the webadmin managable as though they were
> created using the UserPortal (this can be optional as Comment 23 suggests).

That's possible. PM decision.

(In reply to Oved Ourfali from comment #26)
> (In reply to Tomas Babej from comment #25)
> > Responding to Comment 22.
> > 
> > The problem here is not monitoring setting the permissions. The problem here
> > is that user with ReadOnlyAdmin role can create VMs for himself which are
> > then not managable.
> > 
> > ReadOnlyAdmin has no way to set the permissions himself.
> > 
> > The way I see it, you should either:
> > a) Prohibit creation of VMs in the webadmin for users with ReadOnlyAdmin
> > role, since then they cannot manage them.
> 
> We won't check what permissions was used to create a VM. Again, we don't
> protect administrators in that way. They need to set proper permissions.
> 

My whole objection here is that by adding a ReadOnlyAdmin role to a user, you effectively (from the user's point of view) remove the ability to manage VMs he created for himself.

Which will take users aback. What was working before is not anymore, and the only thing what changed is that I gained additional role and used different interface.

Can this be at least shown as warning in the UI?

> > b) Make the VMs created in the webadmin managable as though they were
> > created using the UserPortal (this can be optional as Comment 23 suggests).
> 
> That's possible. PM decision.

(In reply to Tomas Babej from comment #27)
> (In reply to Oved Ourfali from comment #26)
> > (In reply to Tomas Babej from comment #25)
> > > Responding to Comment 22.
> > > 
> > > The problem here is not monitoring setting the permissions. The problem here
> > > is that user with ReadOnlyAdmin role can create VMs for himself which are
> > > then not managable.
> > > 
> > > ReadOnlyAdmin has no way to set the permissions himself.
> > > 
> > > The way I see it, you should either:
> > > a) Prohibit creation of VMs in the webadmin for users with ReadOnlyAdmin
> > > role, since then they cannot manage them.
> > 
> > We won't check what permissions was used to create a VM. Again, we don't
> > protect administrators in that way. They need to set proper permissions.
> > 
> 
> My whole objection here is that by adding a ReadOnlyAdmin role to a user,
> you effectively (from the user's point of view) remove the ability to manage
> VMs he created for himself.

This is incorrect.
ReadOnlyAdmin is a role for viewing only.
In case a user is a power user than he should create the VMs from user portal and than he can manage them.
It is confusing in this specific case ... but this case is far from being a common case. 

> 
> Which will take users aback. What was working before is not anymore, and the
> only thing what changed is that I gained additional role and used different
> interface.
> 
> Can this be at least shown as warning in the UI?
> 
> > > b) Make the VMs created in the webadmin managable as though they were
> > > created using the UserPortal (this can be optional as Comment 23 suggests).
> > 
> > That's possible. PM decision.


This is a minor issue,
One can argue whether ReadOnlyAdmin should override all other permissions and not allow doing anything through the webadmin ... but as said above this use case is very specific.

(In reply to Barak from comment #28)
> (In reply to Tomas Babej from comment #27)
> > My whole objection here is that by adding a ReadOnlyAdmin role to a user,
> > you effectively (from the user's point of view) remove the ability to manage
> > VMs he created for himself.
> 
> This is incorrect.
> ReadOnlyAdmin is a role for viewing only.
> In case a user is a power user than he should create the VMs from user
> portal and than he can manage them.

Could you give us an advice which permission we should add to make it work, please?

> It is confusing in this specific case ... but this case is far from being a
> common case. 

(I have re-ordered paragraph so related ideas are on the same spot.)
> One can argue whether ReadOnlyAdmin should override all other permissions
> and not allow doing anything through the webadmin ... but as said above this
> use case is very specific.

From identity management point of view it would make sense to use different user account for web admin (this would be equivalent to admin/username principal in Kerberos world) and do not apply different access controls depending on the API/interface you use. How does current rules apply to RHEV API?

(Parallel: Does you file system make a difference if you create a file using command line 'touch' or a dialog in your web browser?)

> > Which will take users aback. What was working before is not anymore, and the
> > only thing what changed is that I gained additional role and used different
> > interface.
> > 
> > Can this be at least shown as warning in the UI?
> > 
> > > > b) Make the VMs created in the webadmin managable as though they were
> > > > created using the UserPortal (this can be optional as Comment 23 suggests).
> > > 
> > > That's possible. PM decision.
> 
> 
> This is a minor issue,

This is an usability issue in general because this does not follow principle of least surprise. Sure, it makes perfect sense for developers but usual usability guidelines (e.g. http://www.nngroup.com/articles/ten-usability-heuristics/) put 'help and documentation' as 'last resort' option - and the ordering here really does matter :-)

(In reply to Petr Spacek from comment #29)
> (In reply to Barak from comment #28)
> > (In reply to Tomas Babej from comment #27)
> > > My whole objection here is that by adding a ReadOnlyAdmin role to a user,
> > > you effectively (from the user's point of view) remove the ability to manage
> > > VMs he created for himself.
> > 
> > This is incorrect.
> > ReadOnlyAdmin is a role for viewing only.
> > In case a user is a power user than he should create the VMs from user
> > portal and than he can manage them.
> 
> Could you give us an advice which permission we should add to make it work,
> please?

Oved or anybody else, could you tell us which permission we should add to make it work as suggested in comment 26, please? Thank you in advance!

(In reply to Petr Spacek from comment #30)
> (In reply to Petr Spacek from comment #29)
> > (In reply to Barak from comment #28)
> > > (In reply to Tomas Babej from comment #27)
> > > > My whole objection here is that by adding a ReadOnlyAdmin role to a user,
> > > > you effectively (from the user's point of view) remove the ability to manage
> > > > VMs he created for himself.
> > > 
> > > This is incorrect.
> > > ReadOnlyAdmin is a role for viewing only.
> > > In case a user is a power user than he should create the VMs from user
> > > portal and than he can manage them.
> > 
> > Could you give us an advice which permission we should add to make it work,
> > please?
> 
> Oved or anybody else, could you tell us which permission we should add to
> make it work as suggested in comment 26, please? Thank you in advance!

There are many admin roles that will allow you to create VMs, and also some user roles that allow that.
Please read the documentation with regards to permissions, it has the details needed to understand what permissions are needed in order to do different tasks, among those also creating VMs.

Some useful links:
https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Virtualization/3.5-Beta/html/Administration_Guide/sect-Virtual_Machines_and_Permissions.html
https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Virtualization/3.5-Beta/html/Administration_Guide/Data_center_storage_entities.html
https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Virtualization/3.5-Beta/html/Administration_Guide/Virtual_Machine_User_Roles_Explained.html

and more...

Thank you for links to documentation!

Unfortunately I'm not able to find any privilege which would solve the problem, i.e. add the ability to manage VMs I created via API/web admin (which seem to be the same from privilege perspective) or at least an ability to add UserVmManager privilege to myself.

Did I overlook something?

Thank you for advice!

(In reply to Petr Spacek from comment #32)
> Thank you for links to documentation!
> 
> Unfortunately I'm not able to find any privilege which would solve the
> problem, i.e. add the ability to manage VMs I created via API/web admin
> (which seem to be the same from privilege perspective) or at least an
> ability to add UserVmManager privilege to myself.
> 
> Did I overlook something?
> 
> Thank you for advice!

You can have UserVmManager (user role) on the DC/cluster, for example, to have access to all VMs in that DC/cluster.
You can make yourself a ClusterAdmin (admin role), which will also allow that, iirc. I referred you to the documentation because there are many roles you can use... You can also define a new custom role if you want, to allow you to set permissions, and put it on the system/DC/cluster level.

I'm afraid that putting *Manager role on DC or cluster level is not an option. The cluster is shared among multiple teams and it would unnecesairly escalate privileges for Jenkins instance if we put *Manager role on the whole DC.

Naturally, Jenkins should manage its own VMs and should not have privileges to touch anything else. (The same applies to advanced users who want to use admin portal because user portal is ... not that advanced, to say it politely.)

Referring to *Manager is wrong here. UserVmManager is allowing you to manage VMs. Even if being granted on the DC, it won't allow you to manage other entities/services.

This bug is here for a while, with no clear action item.
Closing it.

Scott - if you think otherwise please reopen.

(In reply to Oved Ourfali from comment #35)
> Referring to *Manager is wrong here. UserVmManager is allowing you to manage
> VMs. Even if being granted on the DC, it won't allow you to manage other
> entities/services.
Maybe I misunderstood something ... but I'm asking about privilege which would allow me to manage VMs *I* created (and do not allow me to touch VMs created by other people/teams) or alternativelly a way how to assign UserVmManager persmission *for myself* for VMs created via API.

When creating VMs through the user-level API, the user who created the VM is also assigned as the owner.
If you're an administrator, and you create a VM, you need to add permissions to a user explicitly.

Created attachment 977831 [details]
Unable to progress, please help us

This is a good solution. In theory. Reality is that user-level API does not work.

Our goals is to allow each user of rhevm instance manage his own VMs via API.

See attached graphical representation of our progress in reaching the goal.

This bug seems as the path of least resistance when wanting to achieve this goal in RHEVM 3.4.

Please, guide us how to solve this problem.

If it doesn't work then a bug should be filed.

Sure, bugs 1160331 and 1155967 were filled a while ago.

Itamar, would it be possible to scope these bugs and if possible to prioritize them to some near release, please? Thank you!

To clarify: Comment #39 (and the diagram from Tomas attached to this bug) is trying to explain that all attemps to meaningfully use API without write-admin privileges on DC/cluster level (which is of course undesirable) failed because of (bugs 1160331 and 1155967) or this bug.

(In reply to Petr Spacek from comment #42)

while waiting for fixing the user level api, would it work for you if we create a new role which is:
- admin role because you have to use admin level api
- being able to manipulate permissions so you're able to explicitly add the UserVmManager

Technically it should work, sure, except the fact that it defeats purpose of access control system :-)

The account we are talking about is going to be baked into multiple scripts and used by multiple persons (for user-written scripts used as replacement for web UI & automated systems (Jenkins & internal reporting scripts).

I'm reopening this because it was reassigned to different team for consideration.

there's no easy solution other than wait for proper filtering support in user level api

hence moving to 3.6 but still keeping open for a backport if we decide to create a new role

I think that an option to add UserVmManager permission for user logged-in admin API (during VM creation) could do the trick and should be relatively easy to do. Am I missing something?

admin doesn't get it - per design
user does get it - ok in the User Portal, but not in API due to:
    bug 1160331 - "Filter: true" is the differentiation between user/admin API at the moment, it is not an easy fix. So you have to use filtering for now
    bug 1155967 - search doesn't work with filtering so most of the queries are unusable anyway

so yes, this would be a good enough workaround. I don't think we even need any option, just at the point where we decide to add or not add the UserVmManager we should check if there is a permissions to add it later, combined with the filter true/false check.

if filter:true || do_not_have_perms_to_add_later
  grant UserVmManager
    
users would always get it (same as today, if user api would work)
admins will get it when filter:false
even readonlyadmin via API would get it

Oved?

(In reply to Michal Skrivanek from comment #49)
> admin doesn't get it - per design
> user does get it - ok in the User Portal, but not in API due to:
>     bug 1160331 - "Filter: true" is the differentiation between user/admin
> API at the moment, it is not an easy fix. So you have to use filtering for
> now
>     bug 1155967 - search doesn't work with filtering so most of the queries
> are unusable anyway
> 
> so yes, this would be a good enough workaround. I don't think we even need
> any option, just at the point where we decide to add or not add the
> UserVmManager we should check if there is a permissions to add it later,
> combined with the filter true/false check.
> 
> if filter:true || do_not_have_perms_to_add_later
>   grant UserVmManager
>     
> users would always get it (same as today, if user api would work)
> admins will get it when filter:false
> even readonlyadmin via API would get it
> 
> Oved?

As far as I know there is a flag saying whether to add permissions or not,so you can pass it along the chain. By design the decision was not to add permissions for administrators. Making it configurable at ui/api level also wasn't accepted. And changing it to set permissions also wasn't accepted. 

Michal - you need to decide with PMs whether to change the behaviour by applying one of the alternatives above or not.

Sure, I was just checking if it does sound reasonable to you.
I'd go with the differentiation in rest api (since we already have the logic based on filtered true/false), just extend it to check the actual permission the client has
this should be ok for the timebeing till we have a true user level api implemented as per the linked bugs

(In reply to Michal Skrivanek from comment #49)

> if filter:true || do_not_have_perms_to_add_later
>   grant UserVmManager
>     
> users would always get it (same as today, if user api would work)
> admins will get it when filter:false
> even readonlyadmin via API would get it

sure, let's make a mistake in a concise summary;-)
it should read:

if filter:true || do_not_have_perms_to_add_later
  grant UserVmManager

users would always get it (same as today, if user api would work)
admins will get it when filter:true (also in userportal)
admins will NOT get it when filter:false (also in webadmin)
even readonlyadmin via API would get it

(In reply to Michal Skrivanek from comment #51)
> Sure, I was just checking if it does sound reasonable to you.
> I'd go with the differentiation in rest api (since we already have the logic
> based on filtered true/false), just extend it to check the actual permission
> the client has
> this should be ok for the timebeing till we have a true user level api
> implemented as per the linked bugs

I wouldn't check permissions on the rest api level. This is wrong, and will probably also be nacked by maintainers. 

I'd expose the flag of whether to assign permissions or not in the API. So, changing the API call to set this field to true will assign permissions. Not setting it at all will leave previous behavior, as we need to be backward compatible. You can expose this as checkbook in the UI as well. That's what I would do. In my opinion, implicit logic in any layer is unacceptable.

The RESTAPI doesn't have any explicit flag to indicate if or what permissions should be added when creating a VM. It just takes the value of the "Filter" header and puts it in the "makeCreatorExplicitOwner" parameter of the VM creation operation. I think this is wrong, as "filter query results according to user permissions" (that is the theoretical meaning of the "Filter" header) and "make creator explicitly owner" are different meanings. Doing this makes the meaning of "Filter" even more confusing than it already is. Unfortunately this can't be changed before 4.0, as it would break backwards compatibility. 

The business rule suggested in comment 52 goes in the same direction, and increases confusion. Try to reformulate without using "Filter", for example:

  if user_is_not_admin || do_not_have_perms_to_add_later
    grant UserVmManager
  
This business rule (any business rule) should be implemented in the backend, not in the RESTAPI.

If for whatever the reason we don't want to implement this or a similar business rule in the backend then the correct way to solve this issue in the RESTAPI would be to allow the user to POST simultaneously the VM and its permissions:

  POST /vms
  <vm>
    <name>myvm</name>
    ...
    <permissions>
      <permission>
        <role id="..."/>
        <user id="...."/>
      </permission>
      ...
    </permission>
    ...
  </vm>

To implement this the backend would need to provide a command that receives as parameters the VM and the permissions, it should then check if the caller is allowed or not to create that VM with those permissions.

Or we can just expose the flag that sets creator as owner through the API.

That is RPC style, not REST.

I know. I'm suggesting to add rest support to that as well.

I reject that suggestion based on the fact that there is a reasonable "restful" alternative, as described in comment 54.

(In reply to Juan Hernández from comment #54)
> The RESTAPI doesn't have any explicit flag to indicate if or what
> permissions should be added when creating a VM. It just takes the value of
> the "Filter" header and puts it in the "makeCreatorExplicitOwner" parameter
> of the VM creation operation. I think this is wrong, as "filter query
> results according to user permissions" (that is the theoretical meaning of
> the "Filter" header) and "make creator explicitly owner" are different
> meanings. Doing this makes the meaning of "Filter" even more confusing than
> it already is. Unfortunately this can't be changed before 4.0, as it would
> break backwards compatibility. 

well, today the business logic _is_ in REST, I don't think we will be able to change this before 4.0 hence I don't see other *easy* way other than adding the piece of additional logic as I suggested in comment #52

 
> The business rule suggested in comment 52 goes in the same direction, and
> increases confusion. Try to reformulate without using "Filter", for example:
> 
>   if user_is_not_admin || do_not_have_perms_to_add_later
>     grant UserVmManager

this is not exactly the same thing. We don't actually care which user is it, admin uses both filter: true and false and by that differentiate between the mode of operation.

Juan, I understand how you think it should look like in 4.0, however what would be your suggestion short-term?
Seems to me other solutions than extending the business logic in REST layer require quite some effort

My suggestion for short term is to implement it correctly, as described in comment 54. If this isn't feasible and the bug is really urgent then you can implement as you consider appropriate, assuming that it will be re-implemented correctly in 4.0.

3.5.1 is already full with bugs (over 80), and since none of these bugs were added as urgent for 3.5.1 release in the tracker bug, moving to 3.5.2

(In reply to Juan Hernández from comment #61)
> My suggestion for short term is to implement it correctly, as described in
> comment 54. If this isn't feasible and the bug is really urgent then you can
> implement as you consider appropriate, assuming that it will be
> re-implemented correctly in 4.0.

ok, we don't have much time left, let's go with Oved's suggestion to expose a flag and I'll open a follow-up bug for proper 4.0 implementation based on comment #54

Please note that this bug most probably affects VM import too.

Environment:
I'm importing VM As a member of idm-dev-lab group which has PowerUserRole on whole DC + ReadOnlyAdmin permission too.

Actual result:
I'm able to import a VM using Admin Portal but the VM does not have any permissions associated with it and so it is unusable (and I do not have any extra permissions so I'm unable to add any).

Expected result:
The VM implicitly gets a permission which allows the user who imported the VM to modify VM parameters and permissions too.

PowerUserRole cannot do import by default, so you might have another permission allowing you to do the import?
basically to do import you have to have IMPORT_EXPORT_VM on the storage domain.
the system, by default, ship with ClusterAdmin and DataCenterAdmin that have this action, and both has the required MANIPULATE_PERMISSIONS to set permissions later on the imported vm.

the only issue here that whoever gave you the permission, gave it on the export domain, this allows you to import a vm to cluster X, but not on cluster X, so you cannot manipulate the permissions...

on the other hand, if you were given ClusterAdmin on this cluster, you could manipulate permissions of any vm in this cluster, which might be too much.

i guess the assumption was that import is an admin task, and this admin will have all permissions needed.
we might need to allow (by default?) setting vm_owner on the vm to the user who performs the action

Omer, I forgot to mention that we have ClusterAdmin and UserVmManager roles on the export domain, too.

What can we do to fix this? Is there a permission we need somewhere to make imported VM useful, i.e. to enable us to edit them and run them as necessary (after the import finishes)?

Thank you for answers!

as i said above in comment 67
if you have also cluster admin on the target cluster (the cluster you import the vm to) then you will have manipulate permission on the vm on you can add permissions to it. please be aware that you will have this permission on ANY VM in this cluster, so give it to a trusted user.

the other solution is to ask for permission from the admin that can do it for you..

As you said in comment #67, ClusterAdmin on cluster level would be too much. We have the ClusterAdmin permission only on export domain level so it has limited scope: It allows us to import VMs and delete imported VMs from export domain after that and that is all.

I very much agree with your idea
> we might need to allow (by default?) setting vm_owner on the vm to the user who performs the action

That would be elegant and solve the problem! Thank you!

I would be really glad if you could contact us while fixing this so we can make sure that the change will work for our specific use case.

Thank you very much!

I've posted a patch implementing the logic explained in comment 52: https://gerrit.ovirt.org/#/c/42392/

This will affect current behaviour as follows:

1. Users of User Portal have isMakeCreatorExplicitOwner = true, so they'll get ownership of all of their VMs. No changes here.
2. Users of Admin Portal that have admin rights will not get the ownership. No changes here.
3. Users of Admin Portal that don't have admin rights will get the ownership.
4. Users of REST API with Filter: true will get the ownership. No changes here.
5. Users of REST API with Filter: false that have admin rights will not get the ownership. No changes here.
6. Users of REST API with Filter: false that don't have admin rights will get the ownership.

I think, there is no sense to expose an option for this. The behaviour is mostly the same except the situation when a non-admin is creating a VM through admin UI or REST API. In this situation he must always get the ownership or he will loose access to his VM. The general rule: users without admin rights must be given permission to manage the VM if they have permission to create it.

On the other side, users with admin privileges may set the ownership afterwards or they may use User Portal or REST API with Filter: true to create VMs, so this option is not critical for them.

Taking into account that this option is anyway a temporary solution that will be removed in 4.0, we can easily go without it.

Any objections?

From user's point of view it sounds good to me.

please document it in the bug so a proper explanation gets to admin guide. It will now be even more confusing than before. We can live with that indeed, but worth a line in docs...

ovirt-3.6.0-3 release

ok in 3.6.0-3

1) Admin User with CREATE_VM perm and without MANIPULATE_PERMS will obtain UserVmManager role on his newly creted vm for webadmin/userportal/restAPI filter=true/restAPI filter=false.

2) Admin User with CREATE_VM perm and with MANIPULATE_PERMS will not obtain UserVmManager role on his newly creted vm for webadmin/restAPI filter=false, but it do for userportal/restAPI filter=true