From 306ffa02183ab1a35f2a004016c0f1159e7dfb23 Mon Sep 17 00:00:00 2001 From: Devdeep Singh Date: Fri, 7 Feb 2014 20:02:31 +0530 Subject: [PATCH] CLOUDSTACK-6053: While adding a primary or secondary of type smb the password wasn't encoded. This cause createStoragePool or addImageStore command to fail if special characters were present. Updated the code to pass user, password and domain as part of details while adding primary or secondary. Also made changes on server side to handle it. --- .../storage/datastore/db/ImageStoreVO.java | 3 -- .../storage/datastore/db/StoragePoolVO.java | 3 -- .../image/datastore/ImageStoreHelper.java | 26 +++++++++++++++++ .../datastore/PrimaryDataStoreHelper.java | 28 +++++++++++++++++++ .../api/query/dao/ImageStoreJoinDaoImpl.java | 4 +-- ui/scripts/sharedFunctions.js | 2 +- ui/scripts/system.js | 20 +++++++++---- ui/scripts/zoneWizard.js | 15 ++++++++-- utils/src/com/cloud/utils/UriUtils.java | 20 ++++++------- 9 files changed, 91 insertions(+), 30 deletions(-) diff --git a/engine/schema/src/org/apache/cloudstack/storage/datastore/db/ImageStoreVO.java b/engine/schema/src/org/apache/cloudstack/storage/datastore/db/ImageStoreVO.java index 36cc57c5d92..2d1a3eddd69 100644 --- a/engine/schema/src/org/apache/cloudstack/storage/datastore/db/ImageStoreVO.java +++ b/engine/schema/src/org/apache/cloudstack/storage/datastore/db/ImageStoreVO.java @@ -159,9 +159,6 @@ public class ImageStoreVO implements ImageStore { public void setUrl(String url) { this.url = url; - if ("cifs".equalsIgnoreCase(this.protocol)) { - this.url = UriUtils.getUpdateUri(url, true); - } } public Date getCreated() { diff --git a/engine/schema/src/org/apache/cloudstack/storage/datastore/db/StoragePoolVO.java b/engine/schema/src/org/apache/cloudstack/storage/datastore/db/StoragePoolVO.java index e1e21e1178d..1508ce0b28c 100644 --- a/engine/schema/src/org/apache/cloudstack/storage/datastore/db/StoragePoolVO.java +++ b/engine/schema/src/org/apache/cloudstack/storage/datastore/db/StoragePoolVO.java @@ -298,9 +298,6 @@ public class StoragePoolVO implements StoragePool { public void setPath(String path) { this.path = path; - if (this.poolType == StoragePoolType.SMB) { - this.path = UriUtils.getUpdateUri(this.path, true); - } } public void setUserInfo(String userInfo) { diff --git a/engine/storage/src/org/apache/cloudstack/storage/image/datastore/ImageStoreHelper.java b/engine/storage/src/org/apache/cloudstack/storage/image/datastore/ImageStoreHelper.java index a4c423c861b..5e29f7136aa 100644 --- a/engine/storage/src/org/apache/cloudstack/storage/image/datastore/ImageStoreHelper.java +++ b/engine/storage/src/org/apache/cloudstack/storage/image/datastore/ImageStoreHelper.java @@ -18,6 +18,8 @@ */ package org.apache.cloudstack.storage.image.datastore; +import java.io.UnsupportedEncodingException; +import java.net.URLEncoder; import java.util.Iterator; import java.util.Map; import java.util.UUID; @@ -34,6 +36,7 @@ import org.apache.cloudstack.storage.datastore.db.ImageStoreDetailsDao; import org.apache.cloudstack.storage.datastore.db.ImageStoreVO; import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao; +import com.cloud.exception.InvalidParameterValueException; import com.cloud.storage.DataStoreRole; import com.cloud.storage.ScopeType; import com.cloud.utils.crypt.DBEncryptionUtil; @@ -95,7 +98,30 @@ public class ImageStoreHelper { if (store.getName() == null) { store.setName(store.getUuid()); } + store.setRole((DataStoreRole)params.get("role")); + + if ("cifs".equalsIgnoreCase((String) params.get("protocol")) && details != null) { + String user = details.get("user"); + String password = details.get("password"); + String domain = details.get("domain"); + String updatedPath = (String) params.get("url"); + + if (user == null || password == null) { + String errMsg = "Missing cifs user and password details. Add them as details parameter."; + throw new InvalidParameterValueException(errMsg); + } else { + try { + password = DBEncryptionUtil.encrypt(URLEncoder.encode(password, "UTF-8")); + details.put("password", password); + updatedPath += "?user=" + user + "&password=" + password + "&domain=" + domain; + } catch (UnsupportedEncodingException e) { + throw new CloudRuntimeException("Error while generating the cifs url. " + e.getMessage()); + } + store.setUrl(updatedPath); + } + } + store = imageStoreDao.persist(store); // persist details diff --git a/engine/storage/src/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelper.java b/engine/storage/src/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelper.java index 99128421197..6b129755009 100644 --- a/engine/storage/src/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelper.java +++ b/engine/storage/src/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelper.java @@ -18,6 +18,8 @@ */ package org.apache.cloudstack.storage.volume.datastore; +import java.io.UnsupportedEncodingException; +import java.net.URLEncoder; import java.util.List; import java.util.Map; @@ -38,12 +40,15 @@ import com.cloud.capacity.Capacity; import com.cloud.capacity.CapacityVO; import com.cloud.capacity.dao.CapacityDao; import com.cloud.hypervisor.Hypervisor.HypervisorType; +import com.cloud.exception.InvalidParameterValueException; import com.cloud.storage.DataStoreRole; import com.cloud.storage.ScopeType; import com.cloud.storage.StorageManager; import com.cloud.storage.StoragePoolHostVO; import com.cloud.storage.StoragePoolStatus; +import com.cloud.storage.Storage.StoragePoolType; import com.cloud.storage.dao.StoragePoolHostDao; +import com.cloud.utils.crypt.DBEncryptionUtil; import com.cloud.utils.db.TransactionLegacy; import com.cloud.utils.exception.CloudRuntimeException; @@ -87,6 +92,29 @@ public class PrimaryDataStoreHelper { dataStoreVO.setHypervisor(params.getHypervisorType()); Map details = params.getDetails(); + if (params.getType() == StoragePoolType.SMB && details != null) { + String user = details.get("user"); + String password = details.get("password"); + String domain = details.get("domain"); + String updatedPath = params.getPath(); + + if (user == null || password == null) { + String errMsg = "Missing cifs user and password details. Add them as details parameter."; + s_logger.warn(errMsg); + throw new InvalidParameterValueException(errMsg); + } else { + try { + password = DBEncryptionUtil.encrypt(URLEncoder.encode(password, "UTF-8")); + details.put("password", password); + updatedPath += "?user=" + user + "&password=" + password + "&domain=" + domain; + } catch (UnsupportedEncodingException e) { + throw new CloudRuntimeException("Error while generating the cifs url. " + e.getMessage()); + } + } + + dataStoreVO.setPath(updatedPath); + } + String tags = params.getTags(); if (tags != null) { String[] tokens = tags.split(","); diff --git a/server/src/com/cloud/api/query/dao/ImageStoreJoinDaoImpl.java b/server/src/com/cloud/api/query/dao/ImageStoreJoinDaoImpl.java index bcf8d4cd440..f1f873c43b4 100644 --- a/server/src/com/cloud/api/query/dao/ImageStoreJoinDaoImpl.java +++ b/server/src/com/cloud/api/query/dao/ImageStoreJoinDaoImpl.java @@ -81,7 +81,7 @@ public class ImageStoreJoinDaoImpl extends GenericDaoBase 0 ){ + if ( detailName != null && detailName.length() > 0 && !detailName.equals(ApiConstants.PASSWORD)) { String detailValue = ids.getDetailValue(); if (detailName.equals(ApiConstants.KEY) || detailName.equals(ApiConstants.S3_SECRET_KEY)) { detailValue = DBEncryptionUtil.decrypt(detailValue); @@ -96,7 +96,7 @@ public class ImageStoreJoinDaoImpl extends GenericDaoBase 0 ){ + if ( detailName != null && detailName.length() > 0 && !detailName.equals(ApiConstants.PASSWORD)) { String detailValue = ids.getDetailValue(); if (detailName.equals(ApiConstants.KEY) || detailName.equals(ApiConstants.S3_SECRET_KEY)) { detailValue = DBEncryptionUtil.decrypt(detailValue); diff --git a/ui/scripts/sharedFunctions.js b/ui/scripts/sharedFunctions.js index b9dc2f37767..2a159675f31 100644 --- a/ui/scripts/sharedFunctions.js +++ b/ui/scripts/sharedFunctions.js @@ -1253,7 +1253,7 @@ var processPropertiesInImagestoreObject = function(jsonObj) { url += 'cifs://'; } - url += (server + path + '?user=' + smbUsername + '&password=' + smbPassword + '&domain=' + smbDomain); + url += (server + path); return url; } diff --git a/ui/scripts/system.js b/ui/scripts/system.js index 7832bef173b..242fa804afa 100644 --- a/ui/scripts/system.js +++ b/ui/scripts/system.js @@ -15661,9 +15661,12 @@ } else if (args.data.protocol == "SMB") { var path = args.data.path; if (path.substring(0, 1) != "/") - path = "/" + path; - url = smbURL(server, path, args.data.smbUsername, args.data.smbPassword, args.data.smbDomain); - } else if (args.data.protocol == "PreSetup") { + path = "/" + path; + url = smbURL(server, path); + array1.push("&details[0].user=" + args.data.smbUsername); + array1.push("&details[1].password=" + todb(args.data.smbPassword)); + array1.push("&details[2].domain=" + args.data.smbDomain); + } else if (args.data.protocol == "PreSetup") { var path = args.data.path; if (path.substring(0, 1) != "/") path = "/" + path; @@ -17065,12 +17068,17 @@ var zoneid = args.data.zoneid; var nfs_server = args.data.nfsServer; var path = args.data.path; - var url = smbURL(nfs_server, path, args.data.smbUsername, args.data.smbPassword, args.data.smbDomain); - + var url = smbURL(nfs_server, path); $.extend(data, { provider: args.data.provider, zoneid: zoneid, - url: url + url: url, + 'details[0].key': 'user', + 'details[0].value': args.data.smbUsername, + 'details[1].key': 'password', + 'details[1].value': args.data.smbPassword, + 'details[2].key': 'domain', + 'details[2].value': args.data.smbDomain }); $.ajax({ diff --git a/ui/scripts/zoneWizard.js b/ui/scripts/zoneWizard.js index 13630c14efd..fd5705be7e3 100755 --- a/ui/scripts/zoneWizard.js +++ b/ui/scripts/zoneWizard.js @@ -4420,7 +4420,10 @@ var path = args.data.primaryStorage.path; if (path.substring(0, 1) != "/") path = "/" + path; - url = smbURL(server, path, args.data.primaryStorage.smbUsername, args.data.primaryStorage.smbPassword, args.data.primaryStorage.smbDomain); + url = smbURL(server, path); + array1.push("&details[0].user=" + args.data.primaryStorage.smbUsername); + array1.push("&details[1].password=" + todb(args.data.primaryStorage.smbPassword)); + array1.push("&details[2].domain=" + args.data.primaryStorage.smbDomain); } else if (args.data.primaryStorage.protocol == "PreSetup") { var path = args.data.primaryStorage.path; if (path.substring(0, 1) != "/") @@ -4529,12 +4532,18 @@ } else if (args.data.secondaryStorage.provider == 'SMB') { var nfs_server = args.data.secondaryStorage.nfsServer; var path = args.data.secondaryStorage.path; - var url = smbURL(nfs_server, path, args.data.secondaryStorage.smbUsername, args.data.secondaryStorage.smbPassword, args.data.secondaryStorage.smbDomain); + var url = smbURL(nfs_server, path); $.extend(data, { provider: args.data.secondaryStorage.provider, zoneid: args.data.returnedZone.id, - url: url + url: url, + 'details[0].key': 'user', + 'details[0].value': args.data.secondaryStorage.smbUsername, + 'details[1].key': 'password', + 'details[1].value': args.data.secondaryStorage.smbPassword, + 'details[2].key': 'domain', + 'details[2].value': args.data.secondaryStorage.smbDomain }); $.ajax({ diff --git a/utils/src/com/cloud/utils/UriUtils.java b/utils/src/com/cloud/utils/UriUtils.java index 42456b52147..fab8ffc7002 100644 --- a/utils/src/com/cloud/utils/UriUtils.java +++ b/utils/src/com/cloud/utils/UriUtils.java @@ -108,13 +108,7 @@ public class UriUtils { public static String getCifsUriParametersProblems(URI uri) { if (!UriUtils.hostAndPathPresent(uri)) { - String errMsg = "cifs URI missing host and/or path. " + " Make sure it's of the format " + "cifs://hostname/path?user=&password="; - s_logger.warn(errMsg); - return errMsg; - } - if (!UriUtils.cifsCredentialsPresent(uri)) { - String errMsg = - "cifs URI missing user and password details. " + "Add them as query parameters, e.g. " + "cifs://example.com/some_share?user=foo&password=bar"; + String errMsg = "cifs URI missing host and/or path. Make sure it's of the format cifs://hostname/path"; s_logger.warn(errMsg); return errMsg; } @@ -185,11 +179,13 @@ public class UriUtils { private static List getUserDetails(String query) { List details = new ArrayList(); - StringTokenizer allParams = new StringTokenizer(query, "&"); - while (allParams.hasMoreTokens()) { - String param = allParams.nextToken(); - details.add(new BasicNameValuePair(param.substring(0, param.indexOf("=")), - param.substring(param.indexOf("=") + 1))); + if (query != null && !query.isEmpty()) { + StringTokenizer allParams = new StringTokenizer(query, "&"); + while (allParams.hasMoreTokens()) { + String param = allParams.nextToken(); + details.add(new BasicNameValuePair(param.substring(0, param.indexOf("=")), + param.substring(param.indexOf("=") + 1))); + } } return details;