Non-admin User receives an error message when entering disk offering section of create instance wizard

[PaulAngus]

ISSUE TYPE

• Bug Report

COMPONENT NAME

UI - Add instance wizard

CLOUDSTACK VERSION

4.11.1

CONFIGURATION

4.11.1 build

OS / ENVIRONMENT

SUMMARY

An error message pops up for all (non-admin) users saying
"Filter all can be specified by admin only"
when using the add vm instance wizard at the 'Disk Offering' stage.

searched for "Filter can be specified by admin only" and found that QueryManagerImpl.java has that message and that it had been modified by this recent commit:
https://github.com/apache/cloudstack/commit/8b09620d777805bdbb59255ef722e0a6ddbb85b8#commitcomment-29087677

STEPS TO REPRODUCE

Create a non-admin user.  Log in as that user.  try to create a new VM instance using the wizard.

EXPECTED RESULTS

more through wizard without error messages, when entering Disk Offering selection page.

ACTUAL RESULTS

![image](https://user-images.githubusercontent.com/4810220/40418282-c9cdbb0e-5e79-11e8-9ba5-c33253de10d4.png)


Using developer tools we can see the following response:

{"listtemplatesresponse":{"uuidList":[],"errorcode":431,"cserrorcode":4350,"errortext":"Filter all can be specified by admin only"}}

[rafaelweingartner]

@PaulAngus I just built the 4.11 (commit: eb23d91), and indeed this error happens there. However, it does not seem to be a result of PR #2639. As you can see, with PR #2639, I did a small fix in com.cloud.api.query.QueryManagerImpl.searchForVolumesInternal(ListVolumesCmd). However, the internal method use by the listTemplates API method is com.cloud.api.query.QueryManagerImpl.searchForTemplatesInternal(ListTemplatesCmd), which has not been touched by PR #2639.

If we look a little closer, we can see the validation at line 3044 that is throwing the message you see. Therefore, the user is a normal one (as described in your test case) and the templateFilter == TemplateFilter.all. Then, if we take a look at instanceWizard.js, we can see at line 392 that, for all users, it is being used templatefilter: 'all'. This is the problem, according to listTemplates API method documentation, the all filter should only be used by admins; hence the validation and exception that is being thrown. I have no idea how we have not seen this before.

Now, how to fix it? I understand that for admins we can keep using the all flag. However, for normal users we should use something else. Reading the listTemplats API method documentation, it feels that we should use executable as the templatefilter. What do you think?

[DaanHoogland]

@rafaelweingartner I do not quite follow. Is either QueryManagerImpl or instanceWizard changed lately? One of them must. I'll do some searching.
I don't see what you are proposing as fix but it seems a change in instanceWizard. Or you creating a PR?

[rafaelweingartner]

I am not looking for the commit that broke the wizard; I am looking for a solution. For instance, I just tested with 02ece53 commit, which is the commit right before mine and it has the same problem. Therefore, it was a coincidence that @PaulAngus noticed the problem right after the merge of 8b09620.

My idea is to discuss a solution to the problem (addressing it as a bug) and then I would create a PR to fix it. That is why I explained the root cause of the problem, which is happening due to an inconsistent use of parameters in instanceWizard.js when we list templates during the disk offering listing. According to listTemplates API, the all option should not be used by normal users. Therefore, we should change that parameter for normal users.

[DaanHoogland]

copy @rafaelweingartner I am investigating further. Not sure why the templateFilter problem didn't come up before.

[rafaelweingartner]

Maybe we all test using admin users?

I understand that these files have not been changed lately, but it seems that something slipped through our reviewing and testing process.

[DaanHoogland]

right, it has to do with multidisk ova, which is an experimental version so I am inclined to say, "let's add an isAdmin() check"

[rafaelweingartner]

That is what I thought. We can check if the user is an admin. Then we do the following:

• isAdmin() == true, we set templatefilter: 'all'
• otherwise, templatefilter: 'executable'

What do you think?

[DaanHoogland]

dit dat, have a look at #2669 (untested)