BYO creds related issue (#66)

* Code refactoring to abstract TVM behaviour from Files impl
Fixed issue with BYO creds
Fixed issue with presign URL feature not working with BYO creds

* Made changes as per review comments

* added tests

* support hostName in BYO credentials
Added tests

* Throw error in case of sas byo creds
Updated doc
fixed tests for above change

* minor fix for byo sas creds

* closed review comments

* removed unnecessary tests

* added jsdoc comment

doc/api.md

@@ -539,6 +539,7 @@ Azure account credentials. Must have the permission to create containers.
 | storageAccount | <code>string</code> | name of azure storage account |
 | storageAccessKey | <code>string</code> | access key for azure storage account |
 | containerName | <code>string</code> | name of container to store files. Another `${containerName}-public` will also be used for public files. |
+| [hostName] | <code>string</code> | custom domain for returned URLs |
 
 <a name="RemotePathString"></a>
 

lib/FilesError.js

@@ -45,6 +45,7 @@ E('ERROR_FILE_NOT_EXISTS', 'file `%s` does not exist')
 E('ERROR_BAD_FILE_TYPE', '%s')
 E('ERROR_OUT_OF_RANGE', '%s')
 E('ERROR_MISSING_OPTION', '%s')
+E('ERROR_UNSUPPORTED_OPERATION', '%s')
 
 module.exports = {
   codes,

lib/impl/AzureBlobFiles.js

@@ -70,9 +70,12 @@ class AzureBlobFiles extends Files {
    * @memberof AzureBlobFiles
    * @private
    */
-  constructor (credentials, tvm) {
+  constructor (credentials, tvm = null) {
     super()
+    /** @private */
     this.tvm = tvm
+    /** @private */
+    this.hasOwnCredentials = (tvm === null)
     const cloned = utils.withHiddenFields(credentials, ['storageAccessKey', 'sasURLPrivate', 'sasURLPublic'])
     logger.debug(`init AzureBlobFiles with config ${JSON.stringify(cloned, null, 2)}`)
 
@@ -95,6 +98,8 @@ class AzureBlobFiles extends Files {
     /** @private */
     this._azure = {}
     this._azure.aborter = azure.Aborter.none
+    /** @private */
+    this.credentials = utils.clone(credentials)
     if (credentials.sasURLPrivate) {
       const azureCreds = new azure.AnonymousCredential()
       const pipeline = azure.StorageURL.newPipeline(azureCreds)
@@ -335,9 +340,17 @@ class AzureBlobFiles extends Files {
    * @private
    */
   _getUrl (filePath) {
+    let hostName = DEFAULT_CDN_STORAGE_HOST
     const azureURL = this._propsForPath(filePath).blockBlobURL.url.split('?')[0]
+    if (this.hasOwnCredentials) {
+      if (this.credentials.hostName) {
+        hostName = this.credentials.hostName
+      } else {
+        return azureURL
+      }
+    }
     const index = azureURL.indexOf(AZURE_STORAGE_DOMAIN)
-    return 'https://' + DEFAULT_CDN_STORAGE_HOST + azureURL.substring(index + AZURE_STORAGE_DOMAIN.length)
+    return 'https://' + hostName + azureURL.substring(index + AZURE_STORAGE_DOMAIN.length)
   }
 
   /**
@@ -355,10 +368,46 @@ class AzureBlobFiles extends Files {
     }
 
     const presignOptions = Object.assign({ blobName: filePath }, options)
-    const cred = await utils.wrapTVMRequest(this.tvm.getAzureBlobPresignCredentials(presignOptions))
+
+    let cred
+    if (this.hasOwnCredentials) {
+      // generate signature based on options and using own credentials
+      cred = await this._getAzureBlobPresignCredentials(presignOptions)
+    } else {
+      cred = await utils.wrapTVMRequest(this.tvm.getAzureBlobPresignCredentials(presignOptions))
+    }
+
     return this._getUrl(filePath) + '?' + cred.signature
   }
 
+  /**
+   * @memberof AzureBlobFiles
+   * @private
+   */
+  async _getAzureBlobPresignCredentials (params) {
+    if (this._azure.sasCreds) {
+      const msg = 'generatePresignURL is not supported with Azure Container SAS credentials, please initialize the SDK with Azure storage account credentials instead'
+      logAndThrow(new codes.ERROR_UNSUPPORTED_OPERATION({ messageValues: [msg], sdkDetails: params }))
+    }
+
+    const sharedKeyCredential = new azure.SharedKeyCredential(this.credentials.storageAccount, this.credentials.storageAccessKey)
+    const containerName = this.credentials.containerName
+    // generate SAS token
+    const expiryTime = new Date(Date.now() + (1000 * params.expiryInSeconds))
+
+    const permissions = azure.BlobSASPermissions.parse(params.permissions)
+    const commonSasParams = {
+      permissions: permissions.toString(),
+      expiryTime: expiryTime,
+      blobName: params.blobName
+    }
+
+    const sasQueryParamsPrivate = azure.generateBlobSASQueryParameters({ ...commonSasParams, containerName: containerName }, sharedKeyCredential)
+    return {
+      signature: sasQueryParamsPrivate.toString()
+    }
+  }
+
   /**
    * @memberof AzureBlobFiles
    * @override

lib/init.js

@@ -57,24 +57,20 @@ async function init (config = {}) {
   // 1. set provider
   const provider = 'azure' // only azure is supported for now
 
-  // 2. instantiate tvm
-  let tvm
-  /* istanbul ignore else */
-  if (provider === 'azure') {
-    logger.debug('init with openwhisk credentials.')
-    // remember config.ow can be empty if env vars are set
-    const tvmArgs = { ow: config.ow, ...config.tvm }
-    tvm = await TvmClient.init(tvmArgs)
-  }
-
-  // 3. return state store based on provider
+  // 2. return state store based on provider
   switch (provider) {
     case 'azure':
       if (config.azure) {
         logger.debug('init with azure blob credentials.')
-        return AzureBlobFiles.init(config.azure, tvm)
+        return AzureBlobFiles.init(config.azure, null)
+      } else {
+        logger.debug('init with openwhisk credentials.')
+        // remember config.ow can be empty if env vars are set
+        const tvmArgs = { ow: config.ow, ...config.tvm }
+        const tvm = await TvmClient.init(tvmArgs)
+        return AzureBlobFiles.init(await utils.wrapTVMRequest(tvm.getAzureBlobCredentials()), tvm)
       }
-      return AzureBlobFiles.init(await utils.wrapTVMRequest(tvm.getAzureBlobCredentials()), tvm)
+
     // default:
     //   throw new FilesError(`provider '${provider}' is not supported.`, FilesError.codes.BadArgument)
   }

lib/types.jsdoc.js

@@ -43,6 +43,7 @@ governing permissions and limitations under the License.
  * @property {string} storageAccessKey access key for azure storage account
  * @property {string} containerName name of container to store files.
  * Another `${containerName}-public` will also be used for public files.
+ * @property {string} [hostName] custom domain for returned URLs
  */
 /**
  * @typedef RemotePathString

lib/utils.js

@@ -30,7 +30,13 @@ function withHiddenFields (toCopy, fields) {
   return copyConfig
 }
 
+// eslint-disable-next-line jsdoc/require-jsdoc
+function clone (toCopy) {
+  return cloneDeep(toCopy)
+}
+
 module.exports = {
   withHiddenFields,
-  wrapTVMRequest
+  wrapTVMRequest,
+  clone
 }

test/impl/AzureBlobFiles.test.js

@@ -21,6 +21,11 @@ const fakeSASCredentials = {
   sasURLPrivate: 'https://fake.com/private?secret=abcd',
   sasURLPublic: 'https://fake.com/public?secret=abcd'
 }
+const fakeUserCredentials = {
+  containerName: 'fake',
+  storageAccessKey: 'fakeKey',
+  storageAccount: 'fakeAccount'
+}
 const fakeAborter = 'fakeAborter'
 
 const DEFAULT_CDN_STORAGE_HOST = 'https://firefly.azureedge.net'
@@ -52,11 +57,6 @@ const testWithProviderError = async (boundFunc, providerMock, errorDetails, file
 describe('init', () => {
   const fakeAzureAborter = 'fakeAborter'
   const mockContainerCreate = jest.fn()
-  const fakeUserCredentials = {
-    containerName: 'fake',
-    storageAccessKey: 'fakeKey',
-    storageAccount: 'fakeAccount'
-  }
 
   beforeEach(async () => {
     mockContainerCreate.mockReset()
@@ -495,18 +495,35 @@ describe('_copyRemoteToRemoteFile', () => {
 })
 
 describe('_getUrl', () => {
+  const fakeAzureAborter = 'fakeAborter'
+  const mockContainerCreate = jest.fn()
+
   const mockBlockBlob = jest.fn()
   const setMockBlobUrl = url => {
     azure.BlockBlobURL.fromContainerURL = mockBlockBlob.mockReturnValue({ url })
   }
+
+  const tvm = jest.fn()
+
   /** @type {AzureBlobFiles} */
   let files
 
   beforeEach(async () => {
     mockBlockBlob.mockReset()
+
+    tvm.getAzureBlobPresignCredentials = jest.fn()
+    tvm.getAzureBlobPresignCredentials.mockResolvedValue({
+      signature: 'fakesign'
+    })
+
     azure.ContainerURL = jest.fn()
-    files = await AzureBlobFiles.init(fakeSASCredentials)
+    files = await AzureBlobFiles.init(fakeSASCredentials, tvm)
     files._azure.aborter = fakeAborter
+
+    mockContainerCreate.mockReset()
+    azure.ContainerURL = { fromServiceURL: jest.fn() }
+    azure.Aborter = { none: fakeAzureAborter }
+    azure.ContainerURL.fromServiceURL.mockReturnValue({ create: mockContainerCreate })
   })
 
   test('url with no query args', async () => {
@@ -524,28 +541,54 @@ describe('_getUrl', () => {
     const url = files._getUrl('fakesub/afile')
     expect(url).toEqual(expectedUrl)
   })
+
+  test('test _getUrl custom host', async () => {
+    files = new AzureBlobFiles({ ...fakeUserCredentials, hostName: 'fakeHost' }, null)
+    const cleanUrl = 'https://fake.blob.core.windows.net/fake/fakesub/afile'
+    setMockBlobUrl(cleanUrl)
+    const expectedUrl = 'https://fakeHost/fake/fakesub/afile'
+    const url = files._getUrl('fakesub/afile')
+    expect(url).toEqual(expectedUrl)
+  })
 })
 
 describe('_getPresignUrl', () => {
+  const fakeAzureAborter = 'fakeAborter'
+  const mockContainerCreate = jest.fn()
+
   const mockBlockBlob = jest.fn()
   const setMockBlobUrl = url => {
     azure.BlockBlobURL.fromContainerURL = mockBlockBlob.mockReturnValue({ url })
   }
   const tvm = jest.fn()
   /** @type {AzureBlobFiles} */
   let files
+  azure.generateBlobSASQueryParameters = jest.fn()
+  azure.BlobSASPermissions.parse = jest.fn()
+
   beforeEach(async () => {
     tvm.mockReset()
+    azure.generateBlobSASQueryParameters.mockReset()
+    azure.BlobSASPermissions.parse.mockReset()
+
+    mockContainerCreate.mockReset()
+    mockBlockBlob.mockReset()
+    azure.ContainerURL = jest.fn()
+    azure.ContainerURL.fromServiceURL = jest.fn()
+    azure.ContainerURL.fromServiceURL.mockReturnValue({ create: mockContainerCreate })
+    azure.Aborter = { none: fakeAzureAborter }
+
+    // defaults that work
+    azure.generateBlobSASQueryParameters.mockReturnValue({ toString: () => 'fakeSAS' })
+    azure.BlobSASPermissions.parse.mockReturnValue({ toString: () => 'fakePermissionStr' })
+
     tvm.getAzureBlobPresignCredentials = jest.fn()
     tvm.getAzureBlobPresignCredentials.mockResolvedValue({
       signature: 'fakesign'
     })
 
-    mockBlockBlob.mockReset()
-    azure.ContainerURL = jest.fn()
-    files = await AzureBlobFiles.init(fakeSASCredentials)
+    files = await AzureBlobFiles.init(fakeSASCredentials, tvm)
     files._azure.aborter = fakeAborter
-    files.tvm = tvm
   })
 
   test('_getPresignUrl with no options', async () => {
@@ -571,6 +614,29 @@ describe('_getPresignUrl', () => {
     expect(url).toEqual(expectedUrl)
     expect(tvm.getAzureBlobPresignCredentials).toHaveBeenCalledWith({ blobName: 'fakesub/afile', expiryInSeconds: 60, permissions: 'fake' })
   })
+
+  test('_getPresignUrl with correct options default permission own credentials', async () => {
+    files = await AzureBlobFiles.init(fakeUserCredentials)
+    const cleanUrl = 'https://fake.blob.core.windows.net/fake/fakesub/afile'
+    setMockBlobUrl(cleanUrl)
+    const expectedUrl = 'https://fake.blob.core.windows.net/fake/fakesub/afile?fakeSAS'
+    const url = await files._getPresignUrl('fakesub/afile', { expiryInSeconds: 60 })
+    expect(url).toEqual(expectedUrl)
+  })
+
+  test('_getPresignUrl with correct options explicit permission own credentials', async () => {
+    files = await AzureBlobFiles.init(fakeUserCredentials)
+    const cleanUrl = 'https://fake.blob.core.windows.net/fake/fakesub/afile'
+    setMockBlobUrl(cleanUrl)
+    const expectedUrl = 'https://fake.blob.core.windows.net/fake/fakesub/afile?fakeSAS'
+    const url = await files._getPresignUrl('fakesub/afile', { expiryInSeconds: 60, permissions: 'fake' })
+    expect(url).toEqual(expectedUrl)
+  })
+
+  test('_getPresignUrl with correct options explicit permission own sas credentials', async () => {
+    files = await AzureBlobFiles.init(fakeSASCredentials)
+    await expect(files._getAzureBlobPresignCredentials('fakesub/afile', { expiryInSeconds: 60 })).rejects.toThrow('[FilesLib:ERROR_UNSUPPORTED_OPERATION] generatePresignURL is not supported with Azure Container SAS credentials, please initialize the SDK with Azure storage account credentials instead')
+  })
 })
 
 describe('_statusFromProviderError', () => {

test/init.test.js

@@ -63,10 +63,8 @@ describe('init', () => {
     test('with azure config', async () => {
       await filesLib.init({ azure: fakeAzureBlobConfig })
       expect(AzureBlobFiles.init).toHaveBeenCalledTimes(1)
-      expect(AzureBlobFiles.init).toHaveBeenCalledWith(fakeAzureBlobConfig, {
-        getAzureBlobCredentials: azureBlobTvmMock
-      })
-      expect(TvmClient.init).toHaveBeenCalledTimes(1)
+      expect(AzureBlobFiles.init).toHaveBeenCalledWith(fakeAzureBlobConfig, null)
+      expect(TvmClient.init).toHaveBeenCalledTimes(0)
       expect(global.mockLogDebug).toHaveBeenCalledWith(expect.stringContaining('azure'))
       checkInitDebugLogNoSecrets(fakeAzureBlobConfig.storageAccessKey)
       checkInitDebugLogNoSecrets(fakeAzureBlobConfig.sasURLPrivate)

types.d.ts

@@ -311,11 +311,13 @@ export type AzureCredentialsSAS = {
  * @property storageAccessKey - access key for azure storage account
  * @property containerName - name of container to store files.
  * Another `${containerName}-public` will also be used for public files.
+ * @property [hostName] - custom domain for returned URLs
  */
 export type AzureCredentialsAccount = {
     storageAccount: string;
     storageAccessKey: string;
     containerName: string;
+    hostName?: string;
 };
 
 /**