From 05301b1e6a6fc685be288ade2b318c5e334e78ad Mon Sep 17 00:00:00 2001
From: Abhishek Kumar <abhishek.kumar@shapeblue.com>
Date: Tue, 2 Feb 2021 14:19:25 +0530
Subject: [PATCH] server: prevent update vm read-only details (#4629)

---
 .../java/com/cloud/vm/UserVmManagerImpl.java  | 15 ++++++---
 ui/scripts/instances.js                       | 32 ++++++++++++++++---
 2 files changed, 38 insertions(+), 9 deletions(-)

diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
index adf00ba1935..684434b95bb 100644
--- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
+++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
@@ -2444,12 +2444,16 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
         final List<String> userBlacklistedSettings = Stream.of(QueryService.UserVMBlacklistedDetails.value().split(","))
                 .map(item -> (item).trim())
                 .collect(Collectors.toList());
+        final List<String> userReadOnlySettings = Stream.of(QueryService.UserVMReadOnlyUIDetails.value().split(","))
+                .map(item -> (item).trim())
+                .collect(Collectors.toList());
         if (cleanupDetails){
             if (caller != null && caller.getType() == Account.ACCOUNT_TYPE_ADMIN) {
                 userVmDetailsDao.removeDetails(id);
             } else {
                 for (final UserVmDetailVO detail : userVmDetailsDao.listDetails(id)) {
-                    if (detail != null && !userBlacklistedSettings.contains(detail.getName())) {
+                    if (detail != null && !userBlacklistedSettings.contains(detail.getName())
+                            && !userReadOnlySettings.contains(detail.getName())) {
                         userVmDetailsDao.removeDetail(id, detail.getName());
                     }
                 }
@@ -2461,15 +2465,18 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
                 }
 
                 if (caller != null && caller.getType() != Account.ACCOUNT_TYPE_ADMIN) {
-                    // Ensure blacklisted detail is not passed by non-root-admin user
+                    // Ensure blacklisted or read-only detail is not passed by non-root-admin user
                     for (final String detailName : details.keySet()) {
                         if (userBlacklistedSettings.contains(detailName)) {
                             throw new InvalidParameterValueException("You're not allowed to add or edit the restricted setting: " + detailName);
                         }
+                        if (userReadOnlySettings.contains(detailName)) {
+                            throw new InvalidParameterValueException("You're not allowed to add or edit the read-only setting: " + detailName);
+                        }
                     }
-                    // Add any hidden/blacklisted detail
+                    // Add any hidden/blacklisted or read-only detail
                     for (final UserVmDetailVO detail : userVmDetailsDao.listDetails(id)) {
-                        if (userBlacklistedSettings.contains(detail.getName())) {
+                        if (userBlacklistedSettings.contains(detail.getName()) || userReadOnlySettings.contains(detail.getName())) {
                             details.put(detail.getName(), detail.getValue());
                         }
                     }
diff --git a/ui/scripts/instances.js b/ui/scripts/instances.js
index ee3a0ff7b49..6a3ad4f5425 100644
--- a/ui/scripts/instances.js
+++ b/ui/scripts/instances.js
@@ -3944,9 +3944,15 @@
                                         // It could happen that a stale web page has been opened up when VM was stopped but
                                         // vm was turned on through another route - UI or API. so we should check again.
                                         var existingDetails = virtualMachine.details;
+                                        var readOnlyUIDetails = [];
+                                        if (virtualMachine.readonlyuidetails && virtualMachine.readonlyuidetails.length > 0) {
+                                            $.each(virtualMachine.readonlyuidetails.split(","), function(){
+                                                readOnlyUIDetails.push($.trim(this));
+                                            });
+                                        }
                                         var newDetails = {};
                                         for (d in existingDetails) {
-                                            if (d != data.name) {
+                                            if (d != data.name && $.inArray(d, readOnlyUIDetails) < 0) {
                                                 newDetails['details[0].' + d] = existingDetails[d];
                                             }
                                         }
@@ -3992,9 +3998,15 @@
                                         // vm was turned on through another route - UI or API. so we should check again.
                                         var detailToDelete = args.data.jsonObj.name;
                                         var existingDetails = virtualMachine.details;
+                                        var readOnlyUIDetails = [];
+                                        if (virtualMachine.readonlyuidetails && virtualMachine.readonlyuidetails.length > 0) {
+                                            $.each(virtualMachine.readonlyuidetails.split(","), function(){
+                                                readOnlyUIDetails.push($.trim(this));
+                                            });
+                                        }
                                         var newDetails = {};
                                         for (detail in existingDetails) {
-                                            if (detail != detailToDelete) {
+                                            if (detail != detailToDelete && $.inArray(detail, readOnlyUIDetails) < 0) {
                                                 newDetails['details[0].' + detail] = existingDetails[detail];
                                             }
                                         }
@@ -4027,12 +4039,20 @@
 									var value = args.data.value;
 									
 									var details;
+                                    var readOnlyUIDetails = [];
 									$.ajax({
 										url: createURL('listVirtualMachines&id=' + args.context.instances[0].id),
 										async:false,
 										success: function(json) {
-											var dets = json.listvirtualmachinesresponse.virtualmachine[0].details;
-											details = dets;
+                                            var virtualMachine = json.listvirtualmachinesresponse.virtualmachine[0]
+										    if (virtualMachine) {
+                                                details = virtualMachine.details;
+                                                if (virtualMachine.readonlyuidetails && virtualMachine.readonlyuidetails.length > 0) {
+                                                    $.each(virtualMachine.readonlyuidetails.split(","), function(){
+                                                        readOnlyUIDetails.push($.trim(this));
+                                                    });
+                                                }
+                                            }
 										},
 
 										error: function(json) {
@@ -4042,7 +4062,9 @@
 									
 									var detailsFormat = '';
 									for (key in details) {
-										detailsFormat += "details[0]." + key + "=" + details[key] + "&";
+									    if ($.inArray(key, readOnlyUIDetails) < 0) {
+									        detailsFormat += "details[0]." + key + "=" + details[key] + "&";
+									    }
 									}
 									// Add new detail to the existing ones
 									detailsFormat += "details[0]." + name + "=" + value;