fecthing DefaultGeneralTimeZone and DefaultWindowsTimeZone config values by non admin user is not permitted #1488

sgratch · 2021-07-19T11:45:33Z

As part of fetching server config values after every login phase, the web-ui always tries to fetch the following config parameters for both admin and non admin users:
DefaultGeneralTimeZone
DefaultWindowsTimeZone

ovirt-web-ui/src/sagas/server-configs.js

Lines 32 to 33 in 7a92043

    
           defaultGeneralTimezone: call(fetchGeneralEngineOption, 'DefaultGeneralTimeZone', DefaultEngineOptions.DefaultGeneralTimeZone), 
        
           defaultWindowsTimezone: call(fetchGeneralEngineOption, 'DefaultWindowsTimeZone', DefaultEngineOptions.DefaultWindowsTimeZone),

When the current user has no administrator role granted, this action always fails and backed return 404:

The error appears on backend server log as well:

2021-07-16 07:35:23,756+02 ERROR [org.jboss.resteasy.resteasy_jaxrs.i18n] (default task-2) RESTEASY002010: Failed to execute: javax.ws.rs.WebApplicationException: HTTP 404 Not Found
	at org.ovirt.engine.api.restapi-jaxrs//org.ovirt.engine.api.restapi.resource.BackendSystemOptionResource.get(BackendSystemOptionResource.java:46)
	at org.ovirt.engine.api.restapi-definition//org.ovirt.engine.api.resource.SystemOptionResource.doGet(SystemOptionResource.java:30)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.jboss.resteasy.resteasy-jaxrs@3.15.1.Final//org.jboss.resteasy.core.MethodInjectorImpl.invoke(MethodInjectorImpl.java:138)
	at org.jboss.resteasy.resteasy-jaxrs@3.15.1.Final//org.jboss.resteasy.core.ResourceMethodInvoker.internalInvokeOnTarget(ResourceMethodInvoker.java:546)
	at org.jboss.resteasy.resteasy-jaxrs@3.15.1.Final//org.jboss.resteasy.core.ResourceMethodInvoker.invokeOnTargetAfterFilter(ResourceMethodInvoker.java:435)
	at org.jboss.resteasy.resteasy-jaxrs@3.15.1.Final//org.jboss.resteasy.core.ResourceMethodInvoker.lambda$invokeOnTarget$0(ResourceMethodInvoker.java:396)
	at org.jboss.resteasy.resteasy-jaxrs@3.15.1.Final//org.jboss.resteasy.core.interception.PreMatchContainerRequestContext.filter(PreMatchContainerRequestContext.java:358)
	at org.jboss.resteasy.resteasy-jaxrs@3.15.1.Final//org.jboss.resteasy.core.ResourceMethodInvoker.invokeOnTarget(ResourceMethodInvoker.java:398)
	at org.jboss.resteasy.resteasy-jaxrs@3.15.1.Final//org.jboss.resteasy.core.ResourceMethodInvoker.invoke(ResourceMethodInvoker.java:365)
	at org.jboss.resteasy.resteasy-jaxrs@3.15.1.Final//org.jboss.resteasy.core.ResourceLocatorInvoker.invokeOnTargetObject(ResourceLocatorInvoker.java:150)
	at org.jboss.resteasy.resteasy-jaxrs@3.15.1.Final//org.jboss.resteasy.core.ResourceLocatorInvoker.invoke(ResourceLocatorInvoker.java:110)
	at org.jboss.resteasy.resteasy-jaxrs@3.15.1.Final//org.jboss.resteasy.core.ResourceLocatorInvoker.invokeOnTargetObject(ResourceLocatorInvoker.java:141)
	at org.jboss.resteasy.resteasy-jaxrs@3.15.1.Final//org.jboss.resteasy.core.ResourceLocatorInvoker.invoke(ResourceLocatorInvoker.java:104)
	at org.jboss.resteasy.resteasy-jaxrs@3.15.1.Final//org.jboss.resteasy.core.SynchronousDispatcher.invoke(SynchronousDispatcher.java:440)
	at org.jboss.resteasy.resteasy-jaxrs@3.15.1.Final//org.jboss.resteasy.core.SynchronousDispatcher.lambda$invoke$4(SynchronousDispatcher.java:229)
	at org.jboss.resteasy.resteasy-jaxrs@3.15.1.Final//org.jboss.resteasy.core.SynchronousDispatcher.lambda$preprocess$0(SynchronousDispatcher.java:135)
	at org.jboss.resteasy.resteasy-jaxrs@3.15.1.Final//org.jboss.resteasy.core.interception.PreMatchContainerRequestContext.filter(PreMatchContainerRequestContext.java:358)
	at org.jboss.resteasy.resteasy-jaxrs@3.15.1.Final//org.jboss.resteasy.core.SynchronousDispatcher.preprocess(SynchronousDispatcher.java:138)
	at org.jboss.resteasy.resteasy-jaxrs@3.15.1.Final//org.jboss.resteasy.core.SynchronousDispatcher.invoke(SynchronousDispatcher.java:215)
	at org.jboss.resteasy.resteasy-jaxrs@3.15.1.Final//org.jboss.resteasy.plugins.server.servlet.ServletContainerDispatcher.service(ServletContainerDispatcher.java:245)
	at org.jboss.resteasy.resteasy-jaxrs@3.15.1.Final//org.jboss.resteasy.plugins.server.servlet.HttpServletDispatcher.service(HttpServletDispatcher.java:61)
	at org.jboss.resteasy.resteasy-jaxrs@3.15.1.Final//org.jboss.resteasy.plugins.server.servlet.HttpServletDispatcher.service(HttpServletDispatcher.java:56)
	at javax.servlet.api@2.0.0.Final//javax.servlet.http.HttpServlet.service(HttpServlet.java:590)
	at io.undertow.servlet@2.2.5.Final//io.undertow.servlet.handlers.ServletHandler.handleRequest(ServletHandler.java:74)
	at io.undertow.servlet@2.2.5.Final//io.undertow.servlet.handlers.FilterHandler.handleRequest(FilterHandler.java:81)
	at io.undertow.servlet@2.2.5.Final//io.undertow.servlet.handlers.security.ServletSecurityRoleHandler.handleRequest(ServletSecurityRoleHandler.java:62)

The reason is that both config parameters are currently set to be exposed to admin users only:
https://github.com/oVirt/ovirt-engine/blob/36cc7b294ad0c1545bbbbefc0b172302d0767647/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java#L481-L485

Need to consider if to change permission on backend or to always use default hard-coded values on web-ui in case of non admin.

The text was updated successfully, but these errors were encountered:

sgratch · 2021-07-19T14:58:55Z

It was discussed with mwperina and michalskrivanek and we agreed on changing the permission access for those two DefaultTimeZone vals to User instead of Admin on backend:
https://github.com/oVirt/ovirt-engine/blob/36cc7b294ad0c1545bbbbefc0b172302d0767647/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java#L481-L485

ahadas · 2021-07-20T12:58:39Z

It would be better to solve the core issue here though and that is the fact users cannot set the timezone + they can change the operating system when provisioning from a template (and in case of Windows template I guess they usually do since the OS type is always defaults to 'Other OS')
How about changing the create-vm wizard so when the source is set to a template, users won't be able to override its operating system - we'll hide the operating system field and the add-vm request won't include the operating system and timezone fields?
Then the backend would take the timezone of the template or set the default timezone per the operating system type in case of the Blank template

sgratch · 2021-08-01T14:12:07Z

@ahadas
As a basic overview for the discussion, the current web-ui's TZ (=timezone)/OS settings are supported as part of the following UI use cases:

Editing an existing VM and changing its OS will influence the VM's TZ to one of the defaults (VM's TZ not editable/visible to the user).
Editing an existing VM and changing the OS might change the Sysprep/Clound-init TZ settings (TZ is editable/visible for the user in case of Sysprep).
Creating a new VM based on a CD:
3.1. The user can set the OS and default TZ will be chosen accordingly (chosen TZ is non-editable but visible within the review step).
3.2. The user can also view/edit the Sysprep TZ in case of setting the Windows OS (chosen Sysprep TZ is visible within the review step).
Creating a new VM based on a template:
4.1. the provisioned template's OS is set as default and is editable.
4.2. The provisioned template's TZ and template's Sysprep TZ are both set as default.
4.3 If OS is manually changed then VM's TZ might be changed to the default TZ accordingly (VM's TZ is non-editable but visible within the review step).
4.4 Sysprep TZ is visible/editable (chosen Sysprep TZ is also visible within the review step).

Regarding your suggested solution:

It would be better to solve the core issue here though and that is the fact users cannot set the timezone + they can change the operating system when provisioning from a template (and in case of Windows template I guess they usually do since the OS type is always defaults to 'Other OS')

As mentioned above, users can change OS not only when provisioned from a template (for ISO and when editing a VM) and they can set the TZ for sysprep with all existed use cases.

How about changing the create-vm wizard so when the source is set to a template, users won't be able to override its operating system - we'll hide the operating system field and the add-vm request won't include the operating system

Displaying and editing OS for an existing/new VMs were both supported for old VM portal >= 3.1 and therefore it's not recommended to remove this functionality.
old VM portal - Edit a VM:

User portal 3.1 - Create a VM:

old VM portal - Create a VM:

and timezone fields?

The same goes for cloud-init/sysprep functionality: #498

Then the backend would take the timezone of the template or set the default timezone per the operating system type in case of the Blank template

Setting all VM's TZ correct values on backend for all mentioned use cases above seems possible but will require a pretty big fix on all mentioned use cases for web-ui and based on OS set on web-ui as well. In addition, the functionality of setting Sysprep TZ will be dismissed (supported since oVirt 4.3.7 #1004).

ahadas · 2021-08-01T14:32:48Z

@ahadas
As a basic overview for the discussion, the current web-ui's TZ (=timezone)/OS settings are supported as part of the following UI use cases:
1. Editing an existing VM and changing its OS will influence the VM's TZ to one of the defaults (VM's TZ not editable/visible to the user).

Changing the OS from Windows to non-Windows, or vice versa, should be handled at the backend side - but that's really not that important (why would someone change the VM's operating system type? it can happen but it's really not that likely)

2. Editing an existing VM and changing the OS might change the Sysprep/Clound-init TZ settings (TZ is editable/visible for the user in case of Sysprep).

OK, so if the TZ is available for sysprep in that case, there's no issue here, right?

3. Creating a new VM based on a CD:
   **3.1**. The user can set the OS and default TZ will be chosen accordingly (chosen TZ is non-editable but visible within the review step).
   **3.2**. The user can also view/edit the Sysprep TZ in case of setting the Windows OS (chosen Sysprep TZ is visible within the review step).

In this case, it makes sense for the user to define the OS so I don't think we need to change it

4. Creating a new VM based on a template:
   **4.1**. the provisioned template's OS is set as default and is editable.
   **4.2**. The provisioned template's TZ and template's Sysprep TZ are both set as default.
   **4.3** If OS is manually changed then VM's TZ might be changed to the default TZ accordingly (VM's TZ is non-editable but visible within the review step).
   **4.4** Sysprep TZ is visible/editable (chosen Sysprep TZ is also visible within the review step).

This is the problematic part -
if I have a template of a Windows VM that is set with EST and I want to provision a VM out of this template from the VM portal, I must know someone that I should set the operating system to Windows and the timezone to EST, right? otherwise either the OS or the TZ would not be aligned to that of the template

Regarding your suggested solution:

It would be better to solve the core issue here though and that is the fact users cannot set the timezone + they can change the operating system when provisioning from a template (and in case of Windows template I guess they usually do since the OS type is always defaults to 'Other OS')

As mentioned above, users can change OS not only when provisioned from a template (for ISO and when editing a VM) and they can set the TZ for sysprep with all existed use cases.

Yes, but let's concentrate on the most common case - changing the OS of existing VMs is not that likely, I'm OK with you setting an arbitrary TZ in that case

How about changing the create-vm wizard so when the source is set to a template, users won't be able to override its operating system - we'll hide the operating system field and the add-vm request won't include the operating system

Displaying and editing OS for an existing/new VMs were both supported for old VM portal >= 3.1 and therefore it's not recommended to remove this functionality.

The VM portal is not that commonly used so I wouldn't avoid making a change since it was that way for a long time
I think we should talk about why changing the template's settings - not only that it makes the provisioning of the VM more difficult but it can lead to such awkward conflicts when the VM portal sets default values for things that are not exposed to the user

and timezone fields?

The same goes for cloud-init/sysprep functionality: #498

Then the backend would take the timezone of the template or set the default timezone per the operating system type in case of the Blank template

Setting all VM's TZ correct values on backend for all mentioned use cases above seems possible but will require a pretty big fix on all mentioned use cases for web-ui and based on OS set on web-ui as well. In addition, the functionality of setting Sysprep TZ will be dismissed (supported since oVirt 4.3.7 #1004).

So let's concentrate only on the provisioning from templates - that would likely solve most of the cases

ahadas · 2021-08-01T14:49:22Z

Displaying and editing OS for an existing/new VMs were both supported for old VM portal >= 3.1 and therefore it's not recommended to remove this functionality.

The VM portal is not that commonly used so I wouldn't avoid making a change since it was that way for a long time
I think we should talk about why changing the template's settings - not only that it makes the provisioning of the VM more difficult but it can lead to such awkward conflicts when the VM portal sets default values for things that are not exposed to the user

Also take into account that the VM portal works quite different than how the user portal used to work -
the VM portal uses rest-api and provides partial information to the backend whereas the user portal was similar to the webadmin and probably also used to set the defaults from the template

sgratch · 2021-08-02T11:40:10Z

@ahadas

Yes, but let's concentrate on the most common case - changing the OS of existing VMs is not that likely, I'm OK with you setting an arbitrary TZ in that case

That was a bug - it failed if didn't fit the OS. We need to set the TZ with same defaults as on backend. Since it's configured via config val then we can't just arbitrary chose it,

This is the problematic part -
if I have a template of a Windows VM that is set with EST and I want to provision a VM out of this template from the VM portal, I must know someone that I should set the operating system to Windows and the timezone to EST, right? otherwise either the OS or the TZ would not be aligned to that of the template

As I mentioned, this is auto set for the created VM as long as the user is not overridden it.
I.e. in that case the create VM wizard's OS field will display "Windows" , Timezone will be set to "EST" and Sysprep TZ will be set according to Template's settings as well. All is taken from the template as a default.
Now the user will be able to view it and change OS and/or Sysprep TZ if needed.
If OS is changed to non Windows then default TZ will be changed accordingly and vice versa.
No other gaps or problems were found. I agree that in most cases the user won't change the Template's OS unless he has a very good reason to do so but it is still supported and working as expected so there is no good reason to remove it.

I'm still not convinced why removing the OS for a specific use case is needed instead of a simple fix of changing the permission access for those two DefaultTimeZone vals.
We are not going to display a different create VM dialog for ISO, Blank template and non Blank templates provision sources which will complicate the whole dialog. And since we are displaying the OS for an existing VM then there is no good reason to hide it for Create VM.
In addition, removing the OS field for provisioned VMs is also an important flow for enabling/disabling Sysprep/Clound-init configuration.

As an alternative we can consider changing the API on backend such that the web-ui will supply all relevant data including chosen OS, which might override the provisioned template one, and backend will decide which TZ to set in instead of doing that on web-ui side. Since the user can't choose the VM's TZ for both new/existed VM then it's possible to do. I don't think it worth the effort though.

sgratch · 2022-01-24T13:37:50Z

@ahadas let's decide on this issue. AFAICS There are 3 options for solving:

Changing the permission on backend for both config values to non-admin
Always use default hard-coded values on web-ui in case of non admin users. use the same default values as the config ones and hope that it won't change sometimes soon since otherwise there will be a bug - a bad solution.
changing the Rest APIs such that web-ui will supply all relevant data including chosen OS for create/edit VM, which might override the provisioned template one, and backend will decide which TZ to set in instead of doing that on web-ui side.

ahadas · 2022-01-24T19:46:56Z

I don't see how the third option is feasible - semantically we can either specify a value for a field or omit the field and then it defaults to the corresponding value from the template. And anyway, I saw this as an opportunity to sort out virtual machine provisioning by the web-ui but if we can't achieve that (and as we approach the GA of oVirt 4.5 it becomes more likely we won't) then I would suggest not to put much efforts into this - do the simplest thing that seems reasonable, probably the first option, and hopefully users won't face similar issues with other fields in the future..

rszwajko · 2022-01-26T17:07:14Z

Implemented option no 1 - see https://gerrit.ovirt.org/c/ovirt-engine/+/118358

Change-Id: Ib5f64b5d8bec10cca4e679590fe4cecf500a05a7 Reference-Url: oVirt/ovirt-web-ui#1488 Signed-off-by: Radoslaw Szwajkowski <rszwajko@redhat.com>

sgratch · 2022-02-14T17:23:11Z

Implemented on Gerrit, closing this issue as DONE.

sgratch changed the title ~~fecthing DefaultGeneralTimeZone and DefaultWindowsTimeZone config values by non admin user causes 404 erros since not permitted~~ fecthing DefaultGeneralTimeZone and DefaultWindowsTimeZone config values by non admin user is not permitted Jul 19, 2021

sgratch assigned rszwajko Jan 24, 2022

gerrit-ovirt-org pushed a commit to oVirt/ovirt-engine that referenced this issue Feb 1, 2022

restapi: expose default TZ to all users

f609cd6

Change-Id: Ib5f64b5d8bec10cca4e679590fe4cecf500a05a7 Reference-Url: oVirt/ovirt-web-ui#1488 Signed-off-by: Radoslaw Szwajkowski <rszwajko@redhat.com>

sgratch closed this as completed Feb 14, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fecthing DefaultGeneralTimeZone and DefaultWindowsTimeZone config values by non admin user is not permitted #1488

fecthing DefaultGeneralTimeZone and DefaultWindowsTimeZone config values by non admin user is not permitted #1488

sgratch commented Jul 19, 2021

sgratch commented Jul 19, 2021

ahadas commented Jul 20, 2021 •

edited

sgratch commented Aug 1, 2021 •

edited

ahadas commented Aug 1, 2021 •

edited

ahadas commented Aug 1, 2021

sgratch commented Aug 2, 2021

sgratch commented Jan 24, 2022

ahadas commented Jan 24, 2022 •

edited

rszwajko commented Jan 26, 2022

sgratch commented Feb 14, 2022

fecthing DefaultGeneralTimeZone and DefaultWindowsTimeZone config values by non admin user is not permitted #1488

fecthing DefaultGeneralTimeZone and DefaultWindowsTimeZone config values by non admin user is not permitted #1488

Comments

sgratch commented Jul 19, 2021

sgratch commented Jul 19, 2021

ahadas commented Jul 20, 2021 • edited

sgratch commented Aug 1, 2021 • edited

ahadas commented Aug 1, 2021 • edited

ahadas commented Aug 1, 2021

sgratch commented Aug 2, 2021

sgratch commented Jan 24, 2022

ahadas commented Jan 24, 2022 • edited

rszwajko commented Jan 26, 2022

sgratch commented Feb 14, 2022

ahadas commented Jul 20, 2021 •

edited

sgratch commented Aug 1, 2021 •

edited

ahadas commented Aug 1, 2021 •

edited

ahadas commented Jan 24, 2022 •

edited