680: BlobService.getBlobUrl puts permissions in sas url even if parameter sharedAccessPolicy.Permission is left empty

lib/services/blob/blobservice.js

@@ -596,7 +596,7 @@ BlobService.prototype.getContainerAcl = function (container, optionsOrCallback,
     if (!responseObject.error) {
       responseObject.containerResult = new ContainerResult(container);
       responseObject.containerResult.getPropertiesFromHeaders(responseObject.response.headers);
-      responseObject.containerResult.signedIdentifiers = ContainerAclResult.parse(responseObject.response.body.SignedIdentifiers.SignedIdentifier);
+      responseObject.containerResult.signedIdentifiers = ContainerAclResult.parse(responseObject.response.body);
     }
 
     var finalCallback = function (returnObject) {
@@ -642,11 +642,17 @@ BlobService.prototype.setContainerAcl = function (container, publicAccessLevel,
 
   var policies = null;
   if (options && options.signedIdentifiers) {
+    if (!_.isArray(options.signedIdentifiers)) {
+      throw new Error('Signed identifiers need to be an array');
+    }
+
     policies = ContainerAclResult.serialize(options.signedIdentifiers);
   }
 
   webResource.addOptionalHeader(HeaderConstants.CONTENT_LENGTH, !azureutil.objectIsNull(policies) ? Buffer.byteLength(policies) : 0);
-  webResource.addOptionalHeader(HeaderConstants.BLOB_PUBLIC_ACCESS_HEADER, publicAccessLevel);
+  if (publicAccessLevel) {
+    webResource.addOptionalHeader(HeaderConstants.BLOB_PUBLIC_ACCESS_HEADER, publicAccessLevel);
+  }
 
   var processResponseCallback = function (responseObject, next) {
     responseObject.containerResult = null;
@@ -2311,24 +2317,22 @@ BlobService.prototype.generateSharedAccessSignature = function (container, blob,
     resourceType = BlobConstants.ResourceTypes.BLOB;
   }
 
-  if (azureutil.objectIsNull(sharedAccessPolicy.AccessPolicy.Permissions)) {
+  if (sharedAccessPolicy.AccessPolicy) {
-    sharedAccessPolicy.AccessPolicy.Permissions = BlobConstants.SharedAccessPermissions.READ;
+    if (!azureutil.objectIsNull(sharedAccessPolicy.AccessPolicy.Start)) {
-  }
+      if (!_.isDate(sharedAccessPolicy.AccessPolicy.Start)) {
+        sharedAccessPolicy.AccessPolicy.Start = new Date(sharedAccessPolicy.AccessPolicy.Start);
+      }
 
-  if (!azureutil.objectIsNull(sharedAccessPolicy.AccessPolicy.Start)) {
+      sharedAccessPolicy.AccessPolicy.Start = azureutil.truncatedISO8061Date(sharedAccessPolicy.AccessPolicy.Start);
-    if (!_.isDate(sharedAccessPolicy.AccessPolicy.Start)) {
-      sharedAccessPolicy.AccessPolicy.Start = new Date(sharedAccessPolicy.AccessPolicy.Start);
     }
 
-    sharedAccessPolicy.AccessPolicy.Start = azureutil.truncatedISO8061Date(sharedAccessPolicy.AccessPolicy.Start);
+    if (!azureutil.objectIsNull(sharedAccessPolicy.AccessPolicy.Expiry)) {
-  }
+      if (!_.isDate(sharedAccessPolicy.AccessPolicy.Expiry)) {
+        sharedAccessPolicy.AccessPolicy.Expiry = new Date(sharedAccessPolicy.AccessPolicy.Expiry);
+      }
 
-  if (!azureutil.objectIsNull(sharedAccessPolicy.AccessPolicy.Expiry)) {
+      sharedAccessPolicy.AccessPolicy.Expiry = azureutil.truncatedISO8061Date(sharedAccessPolicy.AccessPolicy.Expiry);
-    if (!_.isDate(sharedAccessPolicy.AccessPolicy.Expiry)) {
-      sharedAccessPolicy.AccessPolicy.Expiry = new Date(sharedAccessPolicy.AccessPolicy.Expiry);
     }
-
-    sharedAccessPolicy.AccessPolicy.Expiry = azureutil.truncatedISO8061Date(sharedAccessPolicy.AccessPolicy.Expiry);
   }
 
   var resourceName = createResourceName(container, blob);

lib/services/blob/internal/sharedaccesssignature.js

@@ -52,13 +52,21 @@ function SharedAccessSignature(storageAccount, storageAccessKey, permissionSet)
 * @return {object} The shared access signature query string.
 */
 SharedAccessSignature.prototype.generateSignedQueryString = function (path, queryString, resourceType, sharedAccessPolicy) {
-  if (sharedAccessPolicy.AccessPolicy.Start) {
+  if (sharedAccessPolicy.AccessPolicy) {
-    queryString[QueryStringConstants.SIGNED_START] = sharedAccessPolicy.AccessPolicy.Start;
+    if (sharedAccessPolicy.AccessPolicy.Start) {
+      queryString[QueryStringConstants.SIGNED_START] = sharedAccessPolicy.AccessPolicy.Start;
+    }
+
+    if (sharedAccessPolicy.AccessPolicy.Expiry) {
+      queryString[QueryStringConstants.SIGNED_EXPIRY] = sharedAccessPolicy.AccessPolicy.Expiry;
+    }
+
+    if (sharedAccessPolicy.AccessPolicy.Permissions) {
+      queryString[QueryStringConstants.SIGNED_PERMISSIONS] = sharedAccessPolicy.AccessPolicy.Permissions;
+    }
   }
 
-  queryString[QueryStringConstants.SIGNED_EXPIRY] = sharedAccessPolicy.AccessPolicy.Expiry;
   queryString[QueryStringConstants.SIGNED_RESOURCE] = resourceType;
-  queryString[QueryStringConstants.SIGNED_PERMISSIONS] = sharedAccessPolicy.AccessPolicy.Permissions;
 
   if (sharedAccessPolicy.Id) {
     queryString[QueryStringConstants.SIGNED_IDENTIFIER] = sharedAccessPolicy.Id;
@@ -138,11 +146,11 @@ SharedAccessSignature.prototype._generateSignature = function (path, resourceTyp
   var canonicalizedResource = '/' + this.storageAccount + path;
 
   var stringToSign =
-      getvalueToAppend(sharedAccessPolicy.AccessPolicy.Permissions) +
+    getvalueToAppend(sharedAccessPolicy.AccessPolicy ? sharedAccessPolicy.AccessPolicy.Permissions : '') +
-      getvalueToAppend(sharedAccessPolicy.AccessPolicy.Start) +
+    getvalueToAppend(sharedAccessPolicy.AccessPolicy ? sharedAccessPolicy.AccessPolicy.Start : '') +
-      getvalueToAppend(sharedAccessPolicy.AccessPolicy.Expiry) +
+    getvalueToAppend(sharedAccessPolicy.AccessPolicy ? sharedAccessPolicy.AccessPolicy.Expiry : '') +
-      getvalueToAppend(canonicalizedResource) +
+    getvalueToAppend(canonicalizedResource) +
-      getvalueToAppend(sharedAccessPolicy.Id, true);
+    getvalueToAppend(sharedAccessPolicy.Id, true);
 
   return this.signer.sign(stringToSign);
 };

lib/services/blob/models/containeraclresult.js

@@ -17,6 +17,7 @@
 var _ = require('underscore');
 var xmlbuilder = require('xmlbuilder');
 
+var azureutil = require('../../../util/util');
 var Constants = require('../../../util/constants');
 var ISO8061Date = require('../../../util/iso8061date');
 
@@ -92,7 +93,12 @@ ContainerAclResult.serialize = function (signedIdentifiersJs) {
 ContainerAclResult.parse = function (signedIdentifiersXml) {
   var signedIdentifiers = [];
 
+  signedIdentifiersXml = azureutil.tryGetValueChain(signedIdentifiersXml, [ 'SignedIdentifiers', 'SignedIdentifier' ]);
   if (signedIdentifiersXml) {
+    if (!_.isArray(signedIdentifiersXml)) {
+      signedIdentifiersXml = [ signedIdentifiersXml ];
+    }
+
     signedIdentifiersXml.forEach(function (signedIdentifier) {
       var si = {};
       si.Id = signedIdentifier.Id;

lib/util/util.js

@@ -264,6 +264,30 @@ exports.tryGetValueInsensitive = function (key, haystack, defaultValue) {
   return defaultValue;
 };
 
+/**
+* Returns the value in a chained object.
+*
+* @param {object} object   The object with the values.
+* @param {array}  keys     The keys.
+* @param {mix}    default  The value to return if $key is not found in $array.
+*
+* @static
+*
+* @return mix
+*/
+exports.tryGetValueChain = function (object, keys, defaultValue) {
+  if (keys.length === 0) {
+    return object;
+  }
+
+  var currentKey = keys.shift();
+  if (object && object[currentKey]) {
+    return exports.tryGetValueChain(object[currentKey], keys, defaultValue);
+  }
+
+  return defaultValue;
+};
+
 /**
 * Rounds a date off to seconds.
 *

test/services/blob/blobservice-tests.js

@@ -19,6 +19,9 @@ var fs = require('fs');
 var path = require('path');
 var util = require('util');
 var sinon = require('sinon');
+var url = require('url');
+
+var request = require('request');
 
 // Test includes
 var testutil = require('../../util/util');
@@ -1123,31 +1126,6 @@ describe('BlobService', function () {
     });
   });
 
-  it('GenerateSharedAccessSignature', function (done) {
-    var containerName = 'images';
-    var blobName = 'pic1.png';
-
-    var devStorageBlobService = azure.createBlobService(ServiceClient.DEVSTORE_STORAGE_ACCOUNT, ServiceClient.DEVSTORE_STORAGE_ACCESS_KEY);
-
-    var sharedAccessPolicy = {
-      AccessPolicy: {
-        Permissions: BlobConstants.SharedAccessPermissions.READ,
-        Start: new Date('October 11, 2011 11:03:40 am GMT'),
-        Expiry: new Date('October 12, 2011 11:53:40 am GMT')
-      }
-    };
-
-    var sharedAccessSignature = devStorageBlobService.generateSharedAccessSignature(containerName, blobName, sharedAccessPolicy);
-
-    assert.equal(sharedAccessSignature.queryString[QueryStringConstants.SIGNED_START], '2011-10-11T11:03:40Z');
-    assert.equal(sharedAccessSignature.queryString[QueryStringConstants.SIGNED_EXPIRY], '2011-10-12T11:53:40Z');
-    assert.equal(sharedAccessSignature.queryString[QueryStringConstants.SIGNED_RESOURCE], BlobConstants.ResourceTypes.BLOB);
-    assert.equal(sharedAccessSignature.queryString[QueryStringConstants.SIGNED_PERMISSIONS], BlobConstants.SharedAccessPermissions.READ);
-    assert.equal(sharedAccessSignature.queryString[QueryStringConstants.SIGNATURE], '7NIEip+VOrQ5ZV80pORPK1MOsJc62wwCNcbMvE+lQ0s=');
-
-    done();
-  });
-
   it('CreateBlobWithBars', function (done) {
     var containerName = testutil.generateId(containerNamesPrefix, containerNames, suiteUtil.isMocked);
     var blobName = 'blobs/' + testutil.generateId(blobNamesPrefix, blobNames, suiteUtil.isMocked);
@@ -1206,60 +1184,134 @@ describe('BlobService', function () {
     });
   });
 
-  it('GetBlobUrl', function (done) {
+  describe('shared access signature', function () {
-    var containerName = testutil.generateId(containerNamesPrefix, containerNames, suiteUtil.isMocked);
+    describe('getBlobUrl', function () {
-    var blobName = testutil.generateId(blobNamesPrefix, blobNames, suiteUtil.isMocked);
+      it('should work', function (done) {
+        var containerName = testutil.generateId(containerNamesPrefix, containerNames, suiteUtil.isMocked);
+        var blobName = testutil.generateId(blobNamesPrefix, blobNames, suiteUtil.isMocked);
 
-    var blobServiceassert = azure.createBlobService('storageAccount', 'storageAccessKey', 'host.com:80');
+        var blobServiceassert = azure.createBlobService('storageAccount', 'storageAccessKey', 'host.com:80');
 
-    var blobUrl = blobServiceassert.getBlobUrl(containerName);
+        var blobUrl = blobServiceassert.getBlobUrl(containerName);
-    assert.equal(blobUrl, 'https://host.com:80/' + containerName);
+        assert.equal(blobUrl, 'https://host.com:80/' + containerName);
 
-    blobUrl = blobServiceassert.getBlobUrl(containerName, blobName);
+        blobUrl = blobServiceassert.getBlobUrl(containerName, blobName);
-    assert.equal(blobUrl, 'https://host.com:80/' + containerName + '/' + blobName);
+        assert.equal(blobUrl, 'https://host.com:80/' + containerName + '/' + blobName);
 
-    done();
+        done();
-  });
+      });
 
-  it('GetBlobSharedUrl', function (done) {
+      it('should work with shared access policy', function (done) {
-    var containerName = 'container';
+        var containerName = 'container';
-    var blobName = 'blob';
+        var blobName = 'blob';
 
-    var blobServiceassert = azure.createBlobService('storageAccount', 'storageAccessKey', 'host.com:80');
+        var blobServiceassert = azure.createBlobService('storageAccount', 'storageAccessKey', 'host.com:80');
 
-    var sharedAccessPolicy = {
+        var sharedAccessPolicy = {
-      AccessPolicy: {
+          AccessPolicy: {
-        Expiry: new Date('October 12, 2011 11:53:40 am GMT')
+            Expiry: new Date('October 12, 2011 11:53:40 am GMT')
-      }
+          }
-    };
+        };
 
-    var blobUrl = blobServiceassert.getBlobUrl(containerName, blobName, sharedAccessPolicy);
+        var blobUrl = blobServiceassert.getBlobUrl(containerName, blobName, sharedAccessPolicy);
-    assert.equal(blobUrl, 'https://host.com:80/' + containerName + '/' + blobName + '?se=2011-10-12T11%3A53%3A40Z&sr=b&sp=r&sig=eVkH%2BFxxShel2hcN50ZUmgPAHk%2FmqRVeaBfyry%2BVacw%3D');
+        assert.equal(blobUrl, 'https://host.com:80/' + containerName + '/' + blobName + '?se=2011-10-12T11%3A53%3A40Z&sr=b&sig=P5rp4qr5wWJdT3%2Fpys210lFcBzamGwjEYXaN2sf%2FHss%3D');
 
-    done();
+        done();
-  });
+      });
 
-  it('GetBlobSharedUrlWithDuration', function (done) {
+      it('should work with container acl permissions', function (done) {
-    var containerName = 'container';
+        var containerName = testutil.generateId(containerNamesPrefix, containerNames, suiteUtil.isMocked);
-    var blobName = 'blob';
+        var blobName = testutil.generateId(blobNamesPrefix, blobNames, suiteUtil.isMocked);
+        var fileNameSource = testutil.generateId('prefix') + '.bmp'; // fake bmp file with text...
+        var blobText = 'Hello World!';
 
-    var blobServiceassert = azure.createBlobService('storageAccount', 'storageAccessKey', 'host.com:80');
+        blobService.createContainer(containerName, function (error) {
+          assert.equal(error, null);
 
-    // Mock Date just to ensure a fixed signature
+          var startTime = new Date('April 15, 2013 11:53:40 am GMT');
-    this.clock = sinon.useFakeTimers(0, 'Date');
 
-    var sharedAccessPolicy = {
+          var readWriteSharedAccessPolicy = {
-      AccessPolicy: {
+            Id: 'readwrite',
-        Expiry: azure.date.minutesFromNow(10)
+            AccessPolicy: {
-      }
+              Start: startTime,
-    };
+              Permissions: 'rwdl'
+            }
+          };
 
-    this.clock.restore();
+          blobService.setContainerAcl(containerName, null, { signedIdentifiers: [ readWriteSharedAccessPolicy ] }, function (err) {
+            assert.equal(err, null);
 
-    var blobUrl = blobServiceassert.getBlobUrl(containerName, blobName, sharedAccessPolicy);
+            blobService.createBlockBlobFromText(containerName, blobName, blobText, function (err) {
-    assert.equal(blobUrl, 'https://host.com:80/' + containerName + '/' + blobName + '?se=1970-01-01T00%3A10%3A00Z&sr=b&sp=r&sig=LofuDUzdHPpiteauMetANWzDpzd0Vw%2BVMOHyXYCipAM%3D');
+              assert.equal(err, null);
 
-    done();
+              var blobUrl = blobService.getBlobUrl(containerName, blobName, {
+                Id: 'readwrite',
+                AccessPolicy: {
+                  Expiry: new Date('April 15, 2099 11:53:40 am GMT')
+                }
+              });
+
+              function responseCallback(err, rsp) {
+                assert.equal(rsp.statusCode, 200);
+                assert.equal(err, null);
+
+                fs.unlink(fileNameSource, done);
+              }
+
+              request.get(blobUrl, responseCallback).pipe(fs.createWriteStream(fileNameSource));
+            });
+          });
+        });
+      });
+
+      it('should work with duration', function (done) {
+        var containerName = 'container';
+        var blobName = 'blob';
+
+        var blobServiceassert = azure.createBlobService('storageAccount', 'storageAccessKey', 'host.com:80');
+
+        // Mock Date just to ensure a fixed signature
+        this.clock = sinon.useFakeTimers(0, 'Date');
+
+        var sharedAccessPolicy = {
+          AccessPolicy: {
+            Expiry: azure.date.minutesFromNow(10)
+          }
+        };
+
+        this.clock.restore();
+
+        var blobUrl = blobServiceassert.getBlobUrl(containerName, blobName, sharedAccessPolicy);
+        assert.equal(blobUrl, 'https://host.com:80/' + containerName + '/' + blobName + '?se=1970-01-01T00%3A10%3A00Z&sr=b&sig=78rp23g%2FozomP%2FwwJHZ8BpyLIj0t%2B97oZgzFl1w3OAU%3D');
+
+        done();
+      });
+    });
+
+    it('GenerateSharedAccessSignature', function (done) {
+      var containerName = 'images';
+      var blobName = 'pic1.png';
+
+      var devStorageBlobService = azure.createBlobService(ServiceClient.DEVSTORE_STORAGE_ACCOUNT, ServiceClient.DEVSTORE_STORAGE_ACCESS_KEY);
+
+      var sharedAccessPolicy = {
+        AccessPolicy: {
+          Permissions: BlobConstants.SharedAccessPermissions.READ,
+          Start: new Date('October 11, 2011 11:03:40 am GMT'),
+          Expiry: new Date('October 12, 2011 11:53:40 am GMT')
+        }
+      };
+
+      var sharedAccessSignature = devStorageBlobService.generateSharedAccessSignature(containerName, blobName, sharedAccessPolicy);
+
+      assert.equal(sharedAccessSignature.queryString[QueryStringConstants.SIGNED_START], '2011-10-11T11:03:40Z');
+      assert.equal(sharedAccessSignature.queryString[QueryStringConstants.SIGNED_EXPIRY], '2011-10-12T11:53:40Z');
+      assert.equal(sharedAccessSignature.queryString[QueryStringConstants.SIGNED_RESOURCE], BlobConstants.ResourceTypes.BLOB);
+      assert.equal(sharedAccessSignature.queryString[QueryStringConstants.SIGNED_PERMISSIONS], BlobConstants.SharedAccessPermissions.READ);
+      assert.equal(sharedAccessSignature.queryString[QueryStringConstants.SIGNATURE], '7NIEip+VOrQ5ZV80pORPK1MOsJc62wwCNcbMvE+lQ0s=');
+
+      done();
+    });
   });
 
   it('responseEmits', function (done) {
@@ -1381,9 +1433,9 @@ describe('BlobService', function () {
 });
 
 function repeat(s, n) {
-  var ret = "";
+  var ret = '';
   for (var i = 0; i < n; i++) {
     ret += s;
   }
   return ret;
-};
+}

test/services/blob/filters-tests.js

@@ -20,13 +20,7 @@ var testutil = require('../../util/util');
 var blobtestutil = require('../../framework/blob-test-utils');
 
 // Lib includes
-var azureutil = testutil.libRequire('util/util');
 var azure = testutil.libRequire('azure');
-var BlobService = testutil.libRequire('services/blob/blobservice');
-var Constants = testutil.libRequire('util/constants');
-
-var BlobConstants = Constants.BlobConstants;
-
 var testPrefix = 'filter-tests';
 
 var blobService;

test/util/util-tests.js

@@ -185,4 +185,13 @@ suite('util-tests', function() {
 
     done();
   });
+
+  test('Get property from object', function (done) {
+    // int positives
+    assert.equal(util.tryGetValueChain({a: { b: { c: 'd' }}}, [ 'a', 'b', 'c' ]), 'd');
+    assert.equal(util.tryGetValueChain({a: { b: { c: 'd' }}}, [ 'a', 'b', 'k' ]), null);
+    assert.equal(util.tryGetValueChain(null, [ 'a', 'b', 'k' ]), null);
+
+    done();
+  });
 });