From 7fed333b16d537ea95bdb88b60e6e0b3d67eb358 Mon Sep 17 00:00:00 2001 From: spaliwal Date: Tue, 29 Sep 2020 20:24:21 +0530 Subject: [PATCH 1/9] 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 --- lib/AuthManager.js | 88 ++++++++++++++++++++++++++ lib/TVMWrapper.js | 64 +++++++++++++++++++ lib/impl/AzureBlobFiles.js | 90 ++++++++------------------- lib/impl/AzureTVMWrapper.js | 119 ++++++++++++++++++++++++++++++++++++ lib/init.js | 21 +------ 5 files changed, 299 insertions(+), 83 deletions(-) create mode 100644 lib/AuthManager.js create mode 100644 lib/TVMWrapper.js create mode 100644 lib/impl/AzureTVMWrapper.js diff --git a/lib/AuthManager.js b/lib/AuthManager.js new file mode 100644 index 0000000..ab0e6fe --- /dev/null +++ b/lib/AuthManager.js @@ -0,0 +1,88 @@ +/* +Copyright 2020 Adobe. All rights reserved. +This file is licensed to you under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. You may obtain a copy +of the License at http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software distributed under +the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS +OF ANY KIND, either express or implied. See the License for the specific language +governing permissions and limitations under the License. +*/ + +const TvmClient = require('@adobe/aio-lib-core-tvm') +const { AzureTVMWrapper } = require('./impl/AzureTVMWrapper') +const utils = require('./utils') +const logger = require('@adobe/aio-lib-core-logging')('@adobe/aio-lib-files', { provider: 'debug' }) + +/** + * Provides creds either from remote or local TVM + * @class AuthManager + * @classdesc proxy to local or remote TVM + */ +class AuthManager { + + /** + * [INTERNAL] Creates an instance of AuthManager. Use static init instead. + * + * @param {TVMClient} remoteTVM tvm client instance + * @param {TVMWrapper} localTVM TVMWrapper instance + * @memberof AuthManager + * @private + */ + constructor(remoteTVM, localTVM) { + this.remoteTVM = remoteTVM + this.localTVM = localTVM + } + + /** + * Creates and return an instance of AuthManager + * @static + * @param {object} credentials abstract credentials + * @returns {Promise} a new Files instance + * @memberof Files + * @abstract + * @private + */ + static async init (config, provider) { + /* 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 } + const remoteTVM = await TvmClient.init(tvmArgs) + + let localTVM + if (config.azure) { + logger.debug('init with azure blob credentials.') + localTVM = await AzureTVMWrapper.init(config.azure) + } + + const authMgr = new AuthManager(remoteTVM, localTVM) + return authMgr + } + } + + async getAzureBlobCredentials() { + let creds + if(this.localTVM !== undefined) { + creds = await this.localTVM.getAzureBlobCredentials() + creds.sasCreds = false + } else { + creds = await utils.wrapTVMRequest(this.remoteTVM.getAzureBlobCredentials()) + creds.sasCreds = true + } + return creds + } + + async getAzureBlobPresignCredentials(presignOptions) { + if(this.localTVM !== undefined) { + return await this.localTVM.getAzureBlobPresignCredentials(presignOptions) + } else { + return await utils.wrapTVMRequest(this.remoteTVM.getAzureBlobPresignCredentials(presignOptions)) + } + } + +} + +module.exports = { AuthManager } diff --git a/lib/TVMWrapper.js b/lib/TVMWrapper.js new file mode 100644 index 0000000..c841275 --- /dev/null +++ b/lib/TVMWrapper.js @@ -0,0 +1,64 @@ +/* +Copyright 2020 Adobe. All rights reserved. +This file is licensed to you under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. You may obtain a copy +of the License at http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software distributed under +the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS +OF ANY KIND, either express or implied. See the License for the specific language +governing permissions and limitations under the License. +*/ +const joi = require('@hapi/joi') +const { Files, FilePermissions } = require('./Files') +const { codes, logAndThrow } = require('./FilesError') +const logger = require('@adobe/aio-lib-core-logging')('@adobe/aio-lib-files', { provider: 'debug' }) + +/** + * Local TVM proxy to emulate functionality of TVM + * @abstract + * @class TVMWrapper + * @classdesc Emulates TVM + */ +class TVMWrapper { + + /** + * Initializes and returns a new TVMWrapper instance + * + * @param {boolean} _isTest set this to true to allow construction + * @memberof TVMWrapper + * @abstract + * @private + */ + constructor (_isTest) { + if (new.target === TVMWrapper && !_isTest) throwNotImplemented('TVMWrapper') + } + + /** + * @static + * @param {object} credentials abstract credentials + * @returns {Promise} a new TVMWrapper instance + * @memberof TVMWrapper + * @abstract + * @private + */ + static async init (credentials) { + throwNotImplemented('init') + } + + async getAzureBlobCredentials() { + throwNotImplemented('getAzureBlobCredentials') + } + + async getAzureBlobPresignCredentials(presignOptions) { + throwNotImplemented('getAzureBlobPresignCredentials') + } + +} + +// eslint-disable-next-line jsdoc/require-jsdoc +function throwNotImplemented (funcName) { + logAndThrow(new codes.ERROR_NOT_IMPLEMENTED({ messageValues: [funcName] })) +} + +module.exports = { TVMWrapper } diff --git a/lib/impl/AzureBlobFiles.js b/lib/impl/AzureBlobFiles.js index f4f2d24..2d1923a 100644 --- a/lib/impl/AzureBlobFiles.js +++ b/lib/impl/AzureBlobFiles.js @@ -29,25 +29,6 @@ const STREAM_MAX_BUFFERS = 20 const AZURE_STORAGE_DOMAIN = 'blob.core.windows.net' const DEFAULT_CDN_STORAGE_HOST = 'firefly.azureedge.net' -/** - * Creates a container if it does not exist - * - * @param {azure.ContainerURL} containerURL azure ContainerUrl - * @param {azure.Aborter} aborter azure Aborter - * @param {boolean} [isPublic=false] set to true to create a public container - * @private - */ -async function createContainerIfNotExists (containerURL, aborter, isPublic = false) { - try { - logger.debug(`creating ${isPublic ? 'public' : 'private'} azure blob container`) - await containerURL.create(aborter, isPublic ? { access: 'blob' } : {}) - } catch (e) { - // bug in the past where randomly switch from Code to code.. weird - if (!(typeof e.body === 'object' && (e.body.Code === 'ContainerAlreadyExists' || e.body.code === 'ContainerAlreadyExists'))) throw e - logger.debug(`${isPublic ? 'public' : 'private'} azure blob container already exists`) - } -} - // todo move somewhere else // eslint-disable-next-line jsdoc/require-jsdoc function lookupMimeType (filePath) { @@ -70,44 +51,34 @@ class AzureBlobFiles extends Files { * @memberof AzureBlobFiles * @private */ - constructor (credentials, tvm) { + constructor (credentials, authMgr) { super() - this.tvm = tvm - const cloned = utils.withHiddenFields(credentials, ['storageAccessKey', 'sasURLPrivate', 'sasURLPublic']) - logger.debug(`init AzureBlobFiles with config ${JSON.stringify(cloned, null, 2)}`) - - const res = joi.object().label('credentials').keys({ - // either - sasURLPrivate: joi.string().uri(), - sasURLPublic: joi.string().uri(), - // or - storageAccessKey: joi.string(), - storageAccount: joi.string(), - containerName: joi.string() - }).required().unknown().and('storageAccount', 'storageAccessKey', 'containerName').and('sasURLPrivate', 'sasURLPublic').xor('sasURLPrivate', 'storageAccount') - .validate(credentials) - if (res.error) { - logAndThrow(new codes.ERROR_BAD_ARGUMENT({ messageValues: [res.error.message], sdkDetails: cloned })) - } - - // todo parse containerName for invalid chars - - /** @private */ + this.authMgr = authMgr this._azure = {} this._azure.aborter = azure.Aborter.none - if (credentials.sasURLPrivate) { + if(credentials.sasCreds) { + const cloned = utils.withHiddenFields(credentials, ['storageAccessKey', 'sasURLPrivate', 'sasURLPublic']) + logger.debug(`init AzureBlobFiles with config ${JSON.stringify(cloned, null, 2)}`) + + const res = joi.object().label('credentials').keys({ + sasURLPrivate: joi.string().uri(), + sasURLPublic: joi.string().uri(), + + }).required().unknown().and('sasURLPrivate', 'sasURLPublic') + .validate(credentials) + if (res.error) { + logAndThrow(new codes.ERROR_BAD_ARGUMENT({ messageValues: [res.error.message], sdkDetails: credentials })) + } + + /** @private */ const azureCreds = new azure.AnonymousCredential() const pipeline = azure.StorageURL.newPipeline(azureCreds) this._azure.containerURLPrivate = new azure.ContainerURL(credentials.sasURLPrivate, pipeline) this._azure.containerURLPublic = new azure.ContainerURL(credentials.sasURLPublic, pipeline) - this._azure.sasCreds = true + } else { - const azureCreds = new azure.SharedKeyCredential(credentials.storageAccount, credentials.storageAccessKey) - const pipeline = azure.StorageURL.newPipeline(azureCreds) - const serviceURL = new azure.ServiceURL(`https://${credentials.storageAccount}.blob.core.windows.net/`, pipeline) - this._azure.containerURLPrivate = azure.ContainerURL.fromServiceURL(serviceURL, credentials.containerName + '') - this._azure.containerURLPublic = azure.ContainerURL.fromServiceURL(serviceURL, credentials.containerName + '-public') - this._azure.sasCreds = false + this._azure.containerURLPrivate = credentials.sasURLPrivate + this._azure.containerURLPublic = credentials.sasURLPublic } } @@ -121,21 +92,10 @@ class AzureBlobFiles extends Files { * @returns {Promise} new instance * @memberof AzureBlobFiles */ - static async init (credentials, tvm) { - const azureFiles = new AzureBlobFiles(credentials, tvm) - - // todo don't do those requests for perf reasons? - // container sas creds are not allowed to create containers and so those - // credentials must point to already existing containers - if (!azureFiles._azure.sasCreds) { - logger.debug('using azure storage account credentials') - // for the non sasCreds case we can make sure those containers exists - const errorDetails = { containerName: credentials.containerName, storageAccount: credentials.storageAccount } - await azureFiles._wrapProviderRequest(createContainerIfNotExists(azureFiles._azure.containerURLPrivate, azureFiles._azure.aborter), errorDetails) - await azureFiles._wrapProviderRequest(createContainerIfNotExists(azureFiles._azure.containerURLPublic, azureFiles._azure.aborter, true), errorDetails) - return azureFiles - } - logger.debug('using azure SAS credentials') + static async init (authMgr) { + //get credentails from AuthManager + const credentials = await authMgr.getAzureBlobCredentials() + const azureFiles = new AzureBlobFiles(credentials, authMgr) return azureFiles } @@ -355,7 +315,7 @@ class AzureBlobFiles extends Files { } const presignOptions = Object.assign({ blobName: filePath }, options) - const cred = await utils.wrapTVMRequest(this.tvm.getAzureBlobPresignCredentials(presignOptions)) + const cred = await utils.wrapTVMRequest(this.authMgr.getAzureBlobPresignCredentials(presignOptions)) return this._getUrl(filePath) + '?' + cred.signature } diff --git a/lib/impl/AzureTVMWrapper.js b/lib/impl/AzureTVMWrapper.js new file mode 100644 index 0000000..1f2d938 --- /dev/null +++ b/lib/impl/AzureTVMWrapper.js @@ -0,0 +1,119 @@ +/* +Copyright 2020 Adobe. All rights reserved. +This file is licensed to you under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. You may obtain a copy +of the License at http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software distributed under +the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS +OF ANY KIND, either express or implied. See the License for the specific language +governing permissions and limitations under the License. +*/ + +const azure = require('@azure/storage-blob') +const joi = require('@hapi/joi') +const { TVMWrapper } = require('../TVMWrapper') +const { codes, logAndThrow } = require('./FilesError') +const logger = require('@adobe/aio-lib-core-logging')('@adobe/aio-lib-files', { provider: 'debug' }) + +/** + * Creates a container if it does not exist + * + * @param {azure.ContainerURL} containerURL azure ContainerUrl + * @param {azure.Aborter} aborter azure Aborter + * @param {boolean} [isPublic=false] set to true to create a public container + * @private + */ +async function createContainerIfNotExists (containerURL, aborter, isPublic = false) { + try { + logger.debug(`creating ${isPublic ? 'public' : 'private'} azure blob container`) + await containerURL.create(aborter, isPublic ? { access: 'blob' } : {}) + } catch (e) { + // bug in the past where randomly switch from Code to code.. weird + if (!(typeof e.body === 'object' && (e.body.Code === 'ContainerAlreadyExists' || e.body.code === 'ContainerAlreadyExists'))) throw e + logger.debug(`${isPublic ? 'public' : 'private'} azure blob container already exists`) + } +} + +/** + * Local TVM proxy to emulate Azure based functionality of TVM + * @class AzureTVMWrapper + * @classdesc Local TVM Implementation on top of Azure Blob + * @augments LocalTVM + * @hideconstructor + * @private + */ +class AzureTVMWrapper extends TVMWrapper { + + /** + * [INTERNAL] Creates an instance of AzureTVMWrapper. Use static init instead. + * + * @param {AzureCredentials} credentials {@link AzureCredentials} + * @memberof AzureTVMWrapper + * @private + */ + constructor (credentials) { + super() + this.credentials = credentials + const res = joi.object().label('credentials').keys({ + storageAccessKey: joi.string(), + storageAccount: joi.string(), + containerName: joi.string() + }).required().unknown().and('storageAccount', 'storageAccessKey', 'containerName') + .validate(credentials) + if (res.error) { + logAndThrow(new codes.ERROR_BAD_ARGUMENT({ messageValues: [res.error.message], sdkDetails: cloned })) + } + } + + /** + * Creates and return an instance of AzureTVMWrapper + * + * @static + * @param {AzureCredentials} credentials {@link AzureCredentials} + * @returns {Promise} new instance + * @memberof AzureTVMWrapper + */ + static async init (credentials) { + return new AzureTVMWrapper(credentials) + } + + async getAzureBlobCredentials() { + const azureCreds = new azure.SharedKeyCredential(this.credentials.storageAccount, this.credentials.storageAccessKey) + const pipeline = azure.StorageURL.newPipeline(azureCreds) + const serviceURL = new azure.ServiceURL(`https://${this.credentials.storageAccount}.blob.core.windows.net/`, pipeline) + const privateSASURL = azure.ContainerURL.fromServiceURL(serviceURL, this.credentials.containerName + '') + const publicSASURL = azure.ContainerURL.fromServiceURL(serviceURL, this.credentials.containerName + '-public') + + const errorDetails = { containerName: this.credentials.containerName, storageAccount: this.credentials.storageAccount } + await createContainerIfNotExists(privateSASURL, azure.Aborter.none) + await createContainerIfNotExists(publicSASURL, azure.Aborter.none, true) + + return { + sasURLPrivate: privateSASURL, + sasURLPublic: publicSASURL + } + } + + async getAzureBlobPresignCredentials(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 perm = (params.permissions === undefined) ? 'r' : params.permissions + const permissions = azure.BlobSASPermissions.parse(perm) + const commonSasParams = { + permissions: permissions.toString(), + expiryTime: expiryTime, + blobName: params.blobName + } + + const sasQueryParamsPrivate = azure.generateBlobSASQueryParameters({ ...commonSasParams, containerName: containerName }, sharedKeyCredential) + return { + signature: sasQueryParamsPrivate.toString() + } + } + +} + +module.exports = { AzureTVMWrapper } diff --git a/lib/init.js b/lib/init.js index 6079e56..3b1375a 100644 --- a/lib/init.js +++ b/lib/init.js @@ -10,12 +10,12 @@ OF ANY KIND, either express or implied. See the License for the specific languag governing permissions and limitations under the License. */ -const TvmClient = require('@adobe/aio-lib-core-tvm') const logger = require('@adobe/aio-lib-core-logging')('@adobe/aio-lib-files', { provider: 'debug' }) const utils = require('./utils') const { AzureBlobFiles } = require('./impl/AzureBlobFiles') const { Files, FilePermissions } = require('./Files') +const { AuthManager } = require('./AuthManager') require('./types.jsdoc') // for VS Code autocomplete /* global OpenWhiskCredentials, AzureCredentialsAccount, AzureCredentialsSAS */ // for linter @@ -56,27 +56,12 @@ 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) - } + const authMgr = await AuthManager.init(config, provider) // 3. 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(await utils.wrapTVMRequest(tvm.getAzureBlobCredentials()), tvm) - // default: - // throw new FilesError(`provider '${provider}' is not supported.`, FilesError.codes.BadArgument) + return AzureBlobFiles.init(authMgr) } } From b2131a4c77e35bead07fe5d23e79583558a41467 Mon Sep 17 00:00:00 2001 From: spaliwal Date: Mon, 5 Oct 2020 11:44:14 +0530 Subject: [PATCH 2/9] Made changes as per review comments --- lib/AuthManager.js | 88 ------------------------ lib/TVMWrapper.js | 64 ------------------ lib/impl/AzureBlobFiles.js | 131 ++++++++++++++++++++++++++++-------- lib/impl/AzureTVMWrapper.js | 119 -------------------------------- lib/init.js | 19 ++++-- test/init.test.js | 6 +- 6 files changed, 121 insertions(+), 306 deletions(-) delete mode 100644 lib/AuthManager.js delete mode 100644 lib/TVMWrapper.js delete mode 100644 lib/impl/AzureTVMWrapper.js diff --git a/lib/AuthManager.js b/lib/AuthManager.js deleted file mode 100644 index ab0e6fe..0000000 --- a/lib/AuthManager.js +++ /dev/null @@ -1,88 +0,0 @@ -/* -Copyright 2020 Adobe. All rights reserved. -This file is licensed to you under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. You may obtain a copy -of the License at http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software distributed under -the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS -OF ANY KIND, either express or implied. See the License for the specific language -governing permissions and limitations under the License. -*/ - -const TvmClient = require('@adobe/aio-lib-core-tvm') -const { AzureTVMWrapper } = require('./impl/AzureTVMWrapper') -const utils = require('./utils') -const logger = require('@adobe/aio-lib-core-logging')('@adobe/aio-lib-files', { provider: 'debug' }) - -/** - * Provides creds either from remote or local TVM - * @class AuthManager - * @classdesc proxy to local or remote TVM - */ -class AuthManager { - - /** - * [INTERNAL] Creates an instance of AuthManager. Use static init instead. - * - * @param {TVMClient} remoteTVM tvm client instance - * @param {TVMWrapper} localTVM TVMWrapper instance - * @memberof AuthManager - * @private - */ - constructor(remoteTVM, localTVM) { - this.remoteTVM = remoteTVM - this.localTVM = localTVM - } - - /** - * Creates and return an instance of AuthManager - * @static - * @param {object} credentials abstract credentials - * @returns {Promise} a new Files instance - * @memberof Files - * @abstract - * @private - */ - static async init (config, provider) { - /* 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 } - const remoteTVM = await TvmClient.init(tvmArgs) - - let localTVM - if (config.azure) { - logger.debug('init with azure blob credentials.') - localTVM = await AzureTVMWrapper.init(config.azure) - } - - const authMgr = new AuthManager(remoteTVM, localTVM) - return authMgr - } - } - - async getAzureBlobCredentials() { - let creds - if(this.localTVM !== undefined) { - creds = await this.localTVM.getAzureBlobCredentials() - creds.sasCreds = false - } else { - creds = await utils.wrapTVMRequest(this.remoteTVM.getAzureBlobCredentials()) - creds.sasCreds = true - } - return creds - } - - async getAzureBlobPresignCredentials(presignOptions) { - if(this.localTVM !== undefined) { - return await this.localTVM.getAzureBlobPresignCredentials(presignOptions) - } else { - return await utils.wrapTVMRequest(this.remoteTVM.getAzureBlobPresignCredentials(presignOptions)) - } - } - -} - -module.exports = { AuthManager } diff --git a/lib/TVMWrapper.js b/lib/TVMWrapper.js deleted file mode 100644 index c841275..0000000 --- a/lib/TVMWrapper.js +++ /dev/null @@ -1,64 +0,0 @@ -/* -Copyright 2020 Adobe. All rights reserved. -This file is licensed to you under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. You may obtain a copy -of the License at http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software distributed under -the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS -OF ANY KIND, either express or implied. See the License for the specific language -governing permissions and limitations under the License. -*/ -const joi = require('@hapi/joi') -const { Files, FilePermissions } = require('./Files') -const { codes, logAndThrow } = require('./FilesError') -const logger = require('@adobe/aio-lib-core-logging')('@adobe/aio-lib-files', { provider: 'debug' }) - -/** - * Local TVM proxy to emulate functionality of TVM - * @abstract - * @class TVMWrapper - * @classdesc Emulates TVM - */ -class TVMWrapper { - - /** - * Initializes and returns a new TVMWrapper instance - * - * @param {boolean} _isTest set this to true to allow construction - * @memberof TVMWrapper - * @abstract - * @private - */ - constructor (_isTest) { - if (new.target === TVMWrapper && !_isTest) throwNotImplemented('TVMWrapper') - } - - /** - * @static - * @param {object} credentials abstract credentials - * @returns {Promise} a new TVMWrapper instance - * @memberof TVMWrapper - * @abstract - * @private - */ - static async init (credentials) { - throwNotImplemented('init') - } - - async getAzureBlobCredentials() { - throwNotImplemented('getAzureBlobCredentials') - } - - async getAzureBlobPresignCredentials(presignOptions) { - throwNotImplemented('getAzureBlobPresignCredentials') - } - -} - -// eslint-disable-next-line jsdoc/require-jsdoc -function throwNotImplemented (funcName) { - logAndThrow(new codes.ERROR_NOT_IMPLEMENTED({ messageValues: [funcName] })) -} - -module.exports = { TVMWrapper } diff --git a/lib/impl/AzureBlobFiles.js b/lib/impl/AzureBlobFiles.js index 2d1923a..c836511 100644 --- a/lib/impl/AzureBlobFiles.js +++ b/lib/impl/AzureBlobFiles.js @@ -29,6 +29,25 @@ const STREAM_MAX_BUFFERS = 20 const AZURE_STORAGE_DOMAIN = 'blob.core.windows.net' const DEFAULT_CDN_STORAGE_HOST = 'firefly.azureedge.net' +/** + * Creates a container if it does not exist + * + * @param {azure.ContainerURL} containerURL azure ContainerUrl + * @param {azure.Aborter} aborter azure Aborter + * @param {boolean} [isPublic=false] set to true to create a public container + * @private + */ +async function createContainerIfNotExists (containerURL, aborter, isPublic = false) { + try { + logger.debug(`creating ${isPublic ? 'public' : 'private'} azure blob container`) + await containerURL.create(aborter, isPublic ? { access: 'blob' } : {}) + } catch (e) { + // bug in the past where randomly switch from Code to code.. weird + if (!(typeof e.body === 'object' && (e.body.Code === 'ContainerAlreadyExists' || e.body.code === 'ContainerAlreadyExists'))) throw e + logger.debug(`${isPublic ? 'public' : 'private'} azure blob container already exists`) + } +} + // todo move somewhere else // eslint-disable-next-line jsdoc/require-jsdoc function lookupMimeType (filePath) { @@ -51,34 +70,46 @@ class AzureBlobFiles extends Files { * @memberof AzureBlobFiles * @private */ - constructor (credentials, authMgr) { + constructor (credentials, tvm) { super() - this.authMgr = authMgr + this.tvm = tvm + this.hasOwnCredentials = (tvm === null) ? true : false + const cloned = utils.withHiddenFields(credentials, ['storageAccessKey', 'sasURLPrivate', 'sasURLPublic']) + logger.debug(`init AzureBlobFiles with config ${JSON.stringify(cloned, null, 2)}`) + + const res = joi.object().label('credentials').keys({ + // either + sasURLPrivate: joi.string().uri(), + sasURLPublic: joi.string().uri(), + // or + storageAccessKey: joi.string(), + storageAccount: joi.string(), + containerName: joi.string() + }).required().unknown().and('storageAccount', 'storageAccessKey', 'containerName').and('sasURLPrivate', 'sasURLPublic').xor('sasURLPrivate', 'storageAccount') + .validate(credentials) + if (res.error) { + logAndThrow(new codes.ERROR_BAD_ARGUMENT({ messageValues: [res.error.message], sdkDetails: cloned })) + } + + // todo parse containerName for invalid chars + + /** @private */ this._azure = {} this._azure.aborter = azure.Aborter.none - if(credentials.sasCreds) { - const cloned = utils.withHiddenFields(credentials, ['storageAccessKey', 'sasURLPrivate', 'sasURLPublic']) - logger.debug(`init AzureBlobFiles with config ${JSON.stringify(cloned, null, 2)}`) - - const res = joi.object().label('credentials').keys({ - sasURLPrivate: joi.string().uri(), - sasURLPublic: joi.string().uri(), - - }).required().unknown().and('sasURLPrivate', 'sasURLPublic') - .validate(credentials) - if (res.error) { - logAndThrow(new codes.ERROR_BAD_ARGUMENT({ messageValues: [res.error.message], sdkDetails: credentials })) - } - - /** @private */ + if (credentials.sasURLPrivate) { const azureCreds = new azure.AnonymousCredential() const pipeline = azure.StorageURL.newPipeline(azureCreds) this._azure.containerURLPrivate = new azure.ContainerURL(credentials.sasURLPrivate, pipeline) this._azure.containerURLPublic = new azure.ContainerURL(credentials.sasURLPublic, pipeline) - + this._azure.sasCreds = true } else { - this._azure.containerURLPrivate = credentials.sasURLPrivate - this._azure.containerURLPublic = credentials.sasURLPublic + const azureCreds = new azure.SharedKeyCredential(credentials.storageAccount, credentials.storageAccessKey) + const pipeline = azure.StorageURL.newPipeline(azureCreds) + const serviceURL = new azure.ServiceURL(`https://${credentials.storageAccount}.blob.core.windows.net/`, pipeline) + this._azure.containerURLPrivate = azure.ContainerURL.fromServiceURL(serviceURL, credentials.containerName + '') + this._azure.containerURLPublic = azure.ContainerURL.fromServiceURL(serviceURL, credentials.containerName + '-public') + this._azure.sasCreds = false + this.credentials = cloned } } @@ -92,10 +123,21 @@ class AzureBlobFiles extends Files { * @returns {Promise} new instance * @memberof AzureBlobFiles */ - static async init (authMgr) { - //get credentails from AuthManager - const credentials = await authMgr.getAzureBlobCredentials() - const azureFiles = new AzureBlobFiles(credentials, authMgr) + static async init (credentials, tvm) { + const azureFiles = new AzureBlobFiles(credentials, tvm) + + // todo don't do those requests for perf reasons? + // container sas creds are not allowed to create containers and so those + // credentials must point to already existing containers + if (!azureFiles._azure.sasCreds) { + logger.debug('using azure storage account credentials') + // for the non sasCreds case we can make sure those containers exists + const errorDetails = { containerName: credentials.containerName, storageAccount: credentials.storageAccount } + await azureFiles._wrapProviderRequest(createContainerIfNotExists(azureFiles._azure.containerURLPrivate, azureFiles._azure.aborter), errorDetails) + await azureFiles._wrapProviderRequest(createContainerIfNotExists(azureFiles._azure.containerURLPublic, azureFiles._azure.aborter, true), errorDetails) + return azureFiles + } + logger.debug('using azure SAS credentials') return azureFiles } @@ -296,8 +338,12 @@ class AzureBlobFiles extends Files { */ _getUrl (filePath) { const azureURL = this._propsForPath(filePath).blockBlobURL.url.split('?')[0] - const index = azureURL.indexOf(AZURE_STORAGE_DOMAIN) - return 'https://' + DEFAULT_CDN_STORAGE_HOST + azureURL.substring(index + AZURE_STORAGE_DOMAIN.length) + if(this.hasOwnCredentials) { + return azureURL + } else { + const index = azureURL.indexOf(AZURE_STORAGE_DOMAIN) + return 'https://' + DEFAULT_CDN_STORAGE_HOST + azureURL.substring(index + AZURE_STORAGE_DOMAIN.length) + } } /** @@ -315,10 +361,41 @@ class AzureBlobFiles extends Files { } const presignOptions = Object.assign({ blobName: filePath }, options) - const cred = await utils.wrapTVMRequest(this.authMgr.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) { + 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 perm = (params.permissions === undefined) ? 'r' : params.permissions + const permissions = azure.BlobSASPermissions.parse(perm) + 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 diff --git a/lib/impl/AzureTVMWrapper.js b/lib/impl/AzureTVMWrapper.js deleted file mode 100644 index 1f2d938..0000000 --- a/lib/impl/AzureTVMWrapper.js +++ /dev/null @@ -1,119 +0,0 @@ -/* -Copyright 2020 Adobe. All rights reserved. -This file is licensed to you under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. You may obtain a copy -of the License at http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software distributed under -the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS -OF ANY KIND, either express or implied. See the License for the specific language -governing permissions and limitations under the License. -*/ - -const azure = require('@azure/storage-blob') -const joi = require('@hapi/joi') -const { TVMWrapper } = require('../TVMWrapper') -const { codes, logAndThrow } = require('./FilesError') -const logger = require('@adobe/aio-lib-core-logging')('@adobe/aio-lib-files', { provider: 'debug' }) - -/** - * Creates a container if it does not exist - * - * @param {azure.ContainerURL} containerURL azure ContainerUrl - * @param {azure.Aborter} aborter azure Aborter - * @param {boolean} [isPublic=false] set to true to create a public container - * @private - */ -async function createContainerIfNotExists (containerURL, aborter, isPublic = false) { - try { - logger.debug(`creating ${isPublic ? 'public' : 'private'} azure blob container`) - await containerURL.create(aborter, isPublic ? { access: 'blob' } : {}) - } catch (e) { - // bug in the past where randomly switch from Code to code.. weird - if (!(typeof e.body === 'object' && (e.body.Code === 'ContainerAlreadyExists' || e.body.code === 'ContainerAlreadyExists'))) throw e - logger.debug(`${isPublic ? 'public' : 'private'} azure blob container already exists`) - } -} - -/** - * Local TVM proxy to emulate Azure based functionality of TVM - * @class AzureTVMWrapper - * @classdesc Local TVM Implementation on top of Azure Blob - * @augments LocalTVM - * @hideconstructor - * @private - */ -class AzureTVMWrapper extends TVMWrapper { - - /** - * [INTERNAL] Creates an instance of AzureTVMWrapper. Use static init instead. - * - * @param {AzureCredentials} credentials {@link AzureCredentials} - * @memberof AzureTVMWrapper - * @private - */ - constructor (credentials) { - super() - this.credentials = credentials - const res = joi.object().label('credentials').keys({ - storageAccessKey: joi.string(), - storageAccount: joi.string(), - containerName: joi.string() - }).required().unknown().and('storageAccount', 'storageAccessKey', 'containerName') - .validate(credentials) - if (res.error) { - logAndThrow(new codes.ERROR_BAD_ARGUMENT({ messageValues: [res.error.message], sdkDetails: cloned })) - } - } - - /** - * Creates and return an instance of AzureTVMWrapper - * - * @static - * @param {AzureCredentials} credentials {@link AzureCredentials} - * @returns {Promise} new instance - * @memberof AzureTVMWrapper - */ - static async init (credentials) { - return new AzureTVMWrapper(credentials) - } - - async getAzureBlobCredentials() { - const azureCreds = new azure.SharedKeyCredential(this.credentials.storageAccount, this.credentials.storageAccessKey) - const pipeline = azure.StorageURL.newPipeline(azureCreds) - const serviceURL = new azure.ServiceURL(`https://${this.credentials.storageAccount}.blob.core.windows.net/`, pipeline) - const privateSASURL = azure.ContainerURL.fromServiceURL(serviceURL, this.credentials.containerName + '') - const publicSASURL = azure.ContainerURL.fromServiceURL(serviceURL, this.credentials.containerName + '-public') - - const errorDetails = { containerName: this.credentials.containerName, storageAccount: this.credentials.storageAccount } - await createContainerIfNotExists(privateSASURL, azure.Aborter.none) - await createContainerIfNotExists(publicSASURL, azure.Aborter.none, true) - - return { - sasURLPrivate: privateSASURL, - sasURLPublic: publicSASURL - } - } - - async getAzureBlobPresignCredentials(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 perm = (params.permissions === undefined) ? 'r' : params.permissions - const permissions = azure.BlobSASPermissions.parse(perm) - const commonSasParams = { - permissions: permissions.toString(), - expiryTime: expiryTime, - blobName: params.blobName - } - - const sasQueryParamsPrivate = azure.generateBlobSASQueryParameters({ ...commonSasParams, containerName: containerName }, sharedKeyCredential) - return { - signature: sasQueryParamsPrivate.toString() - } - } - -} - -module.exports = { AzureTVMWrapper } diff --git a/lib/init.js b/lib/init.js index 3b1375a..76ec362 100644 --- a/lib/init.js +++ b/lib/init.js @@ -10,12 +10,12 @@ OF ANY KIND, either express or implied. See the License for the specific languag governing permissions and limitations under the License. */ +const TvmClient = require('@adobe/aio-lib-core-tvm') const logger = require('@adobe/aio-lib-core-logging')('@adobe/aio-lib-files', { provider: 'debug' }) const utils = require('./utils') const { AzureBlobFiles } = require('./impl/AzureBlobFiles') const { Files, FilePermissions } = require('./Files') -const { AuthManager } = require('./AuthManager') require('./types.jsdoc') // for VS Code autocomplete /* global OpenWhiskCredentials, AzureCredentialsAccount, AzureCredentialsSAS */ // for linter @@ -56,12 +56,23 @@ async function init (config = {}) { // 1. set provider const provider = 'azure' // only azure is supported for now - const authMgr = await AuthManager.init(config, provider) - // 3. return state store based on provider + // 2. return state store based on provider switch (provider) { case 'azure': - return AzureBlobFiles.init(authMgr) + if (config.azure) { + logger.debug('init with azure blob credentials.') + 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) + } + + // default: + // throw new FilesError(`provider '${provider}' is not supported.`, FilesError.codes.BadArgument) } } diff --git a/test/init.test.js b/test/init.test.js index ff3f0dd..4d38e7c 100644 --- a/test/init.test.js +++ b/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) From 48128e412933a2e7657dc4f3bef64c04f18dc7d5 Mon Sep 17 00:00:00 2001 From: spaliwal Date: Mon, 5 Oct 2020 12:57:30 +0530 Subject: [PATCH 3/9] added tests --- lib/impl/AzureBlobFiles.js | 13 +++++---- lib/utils.js | 8 +++++- test/impl/AzureBlobFiles.test.js | 48 ++++++++++++++++++++++++++++++++ 3 files changed, 62 insertions(+), 7 deletions(-) diff --git a/lib/impl/AzureBlobFiles.js b/lib/impl/AzureBlobFiles.js index c836511..54187de 100644 --- a/lib/impl/AzureBlobFiles.js +++ b/lib/impl/AzureBlobFiles.js @@ -73,7 +73,7 @@ class AzureBlobFiles extends Files { constructor (credentials, tvm) { super() this.tvm = tvm - this.hasOwnCredentials = (tvm === null) ? true : false + this.hasOwnCredentials = false const cloned = utils.withHiddenFields(credentials, ['storageAccessKey', 'sasURLPrivate', 'sasURLPublic']) logger.debug(`init AzureBlobFiles with config ${JSON.stringify(cloned, null, 2)}`) @@ -109,7 +109,8 @@ class AzureBlobFiles extends Files { this._azure.containerURLPrivate = azure.ContainerURL.fromServiceURL(serviceURL, credentials.containerName + '') this._azure.containerURLPublic = azure.ContainerURL.fromServiceURL(serviceURL, credentials.containerName + '-public') this._azure.sasCreds = false - this.credentials = cloned + this.credentials = utils.clone(credentials) + this.hasOwnCredentials = true } } @@ -338,7 +339,7 @@ class AzureBlobFiles extends Files { */ _getUrl (filePath) { const azureURL = this._propsForPath(filePath).blockBlobURL.url.split('?')[0] - if(this.hasOwnCredentials) { + if (this.hasOwnCredentials) { return azureURL } else { const index = azureURL.indexOf(AZURE_STORAGE_DOMAIN) @@ -363,8 +364,8 @@ class AzureBlobFiles extends Files { const presignOptions = Object.assign({ blobName: filePath }, options) let cred - if(this.hasOwnCredentials) { - //generate signature based on options and using own credentials + 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)) @@ -377,7 +378,7 @@ class AzureBlobFiles extends Files { * @memberof AzureBlobFiles * @private */ - async _getAzureBlobPresignCredentials(params) { + async _getAzureBlobPresignCredentials (params) { const sharedKeyCredential = new azure.SharedKeyCredential(this.credentials.storageAccount, this.credentials.storageAccessKey) const containerName = this.credentials.containerName // generate SAS token diff --git a/lib/utils.js b/lib/utils.js index 8dfdbfb..baac8e7 100644 --- a/lib/utils.js +++ b/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 } diff --git a/test/impl/AzureBlobFiles.test.js b/test/impl/AzureBlobFiles.test.js index 3e08424..dc6dd65 100644 --- a/test/impl/AzureBlobFiles.test.js +++ b/test/impl/AzureBlobFiles.test.js @@ -130,6 +130,12 @@ describe('init', () => { checkInitDebugLogNoSecrets(fakeUserCredentials.storageAccessKey) }) + test('test constructor', async () => { + const files = new AzureBlobFiles(fakeUserCredentials) + expect(files).toBeInstanceOf(AzureBlobFiles) + expect(files.hasOwnCredentials).toBe(true) + }) + // eslint-disable-next-line jest/expect-expect test('when there is a provider error on blob container creation', async () => testWithProviderError(AzureBlobFiles.init.bind(null, fakeUserCredentials), mockContainerCreate, { containerName: fakeUserCredentials.containerName, storageAccount: fakeUserCredentials.storageAccount }) @@ -534,8 +540,23 @@ describe('_getPresignUrl', () => { const tvm = jest.fn() /** @type {AzureBlobFiles} */ let files + azure.generateBlobSASQueryParameters = jest.fn() + azure.BlobSASPermissions.parse = jest.fn() + const fakeCreds = { + storageAccount: 'fakeacc', + storageAccessKey: 'fakeKey', + containerName: 'fakeContainer' + } + beforeEach(async () => { tvm.mockReset() + azure.generateBlobSASQueryParameters.mockReset() + azure.BlobSASPermissions.parse.mockReset() + + // defaults that work + azure.generateBlobSASQueryParameters.mockReturnValue({ toString: () => 'fakeSAS' }) + azure.BlobSASPermissions.parse.mockReturnValue({ toString: () => 'fakePermissionStr' }) + tvm.getAzureBlobPresignCredentials = jest.fn() tvm.getAzureBlobPresignCredentials.mockResolvedValue({ signature: 'fakesign' @@ -571,6 +592,33 @@ 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.hasOwnCredentials = true + files.credentials = fakeCreds + 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.hasOwnCredentials = true + files.credentials = fakeCreds + 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('_getAzureBlobPresignCredentials with correct options', async () => { + files.hasOwnCredentials = true + files.credentials = fakeCreds + const ret = await files._getAzureBlobPresignCredentials('fakesub/afile', { expiryInSeconds: 60 }) + expect(ret).toEqual({ signature: 'fakeSAS' }) + }) }) describe('_statusFromProviderError', () => { From b406e00ca3be4044687c2926e21dbf74aa7115cb Mon Sep 17 00:00:00 2001 From: spaliwal Date: Mon, 5 Oct 2020 13:43:24 +0530 Subject: [PATCH 4/9] support hostName in BYO credentials Added tests --- lib/impl/AzureBlobFiles.js | 12 +++++--- test/impl/AzureBlobFiles.test.js | 52 ++++++++++++++++++++------------ 2 files changed, 41 insertions(+), 23 deletions(-) diff --git a/lib/impl/AzureBlobFiles.js b/lib/impl/AzureBlobFiles.js index 54187de..a27f920 100644 --- a/lib/impl/AzureBlobFiles.js +++ b/lib/impl/AzureBlobFiles.js @@ -338,13 +338,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) { - return azureURL - } else { - const index = azureURL.indexOf(AZURE_STORAGE_DOMAIN) - return 'https://' + DEFAULT_CDN_STORAGE_HOST + azureURL.substring(index + AZURE_STORAGE_DOMAIN.length) + if (this.credentials.hostName) { + hostName = this.credentials.hostName + } else { + return azureURL + } } + const index = azureURL.indexOf(AZURE_STORAGE_DOMAIN) + return 'https://' + hostName + azureURL.substring(index + AZURE_STORAGE_DOMAIN.length) } /** diff --git a/test/impl/AzureBlobFiles.test.js b/test/impl/AzureBlobFiles.test.js index dc6dd65..318cb6c 100644 --- a/test/impl/AzureBlobFiles.test.js +++ b/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() @@ -130,12 +130,6 @@ describe('init', () => { checkInitDebugLogNoSecrets(fakeUserCredentials.storageAccessKey) }) - test('test constructor', async () => { - const files = new AzureBlobFiles(fakeUserCredentials) - expect(files).toBeInstanceOf(AzureBlobFiles) - expect(files.hasOwnCredentials).toBe(true) - }) - // eslint-disable-next-line jest/expect-expect test('when there is a provider error on blob container creation', async () => testWithProviderError(AzureBlobFiles.init.bind(null, fakeUserCredentials), mockContainerCreate, { containerName: fakeUserCredentials.containerName, storageAccount: fakeUserCredentials.storageAccount }) @@ -501,6 +495,9 @@ describe('_copyRemoteToRemoteFile', () => { }) describe('_getUrl', () => { + const fakeAzureAborter = 'fakeAborter' + const mockContainerCreate = jest.fn() + const mockBlockBlob = jest.fn() const setMockBlobUrl = url => { azure.BlockBlobURL.fromContainerURL = mockBlockBlob.mockReturnValue({ url }) @@ -513,6 +510,11 @@ describe('_getUrl', () => { azure.ContainerURL = jest.fn() files = await AzureBlobFiles.init(fakeSASCredentials) 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 () => { @@ -530,9 +532,21 @@ describe('_getUrl', () => { const url = files._getUrl('fakesub/afile') expect(url).toEqual(expectedUrl) }) + + test('test _getUrl custom host', async () => { + files = new AzureBlobFiles({ ...fakeUserCredentials, hostName: 'fakeHost' }) + 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 }) @@ -542,17 +556,17 @@ describe('_getPresignUrl', () => { let files azure.generateBlobSASQueryParameters = jest.fn() azure.BlobSASPermissions.parse = jest.fn() - const fakeCreds = { - storageAccount: 'fakeacc', - storageAccessKey: 'fakeKey', - containerName: 'fakeContainer' - } beforeEach(async () => { tvm.mockReset() azure.generateBlobSASQueryParameters.mockReset() azure.BlobSASPermissions.parse.mockReset() + mockContainerCreate.mockReset() + azure.ContainerURL = { fromServiceURL: jest.fn() } + azure.Aborter = { none: fakeAzureAborter } + azure.ContainerURL.fromServiceURL.mockReturnValue({ create: mockContainerCreate }) + // defaults that work azure.generateBlobSASQueryParameters.mockReturnValue({ toString: () => 'fakeSAS' }) azure.BlobSASPermissions.parse.mockReturnValue({ toString: () => 'fakePermissionStr' }) @@ -595,7 +609,7 @@ describe('_getPresignUrl', () => { test('_getPresignUrl with correct options default permission own credentials', async () => { files.hasOwnCredentials = true - files.credentials = fakeCreds + files.credentials = 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' @@ -605,7 +619,7 @@ describe('_getPresignUrl', () => { test('_getPresignUrl with correct options explicit permission own credentials', async () => { files.hasOwnCredentials = true - files.credentials = fakeCreds + files.credentials = 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' @@ -615,7 +629,7 @@ describe('_getPresignUrl', () => { test('_getAzureBlobPresignCredentials with correct options', async () => { files.hasOwnCredentials = true - files.credentials = fakeCreds + files.credentials = fakeUserCredentials const ret = await files._getAzureBlobPresignCredentials('fakesub/afile', { expiryInSeconds: 60 }) expect(ret).toEqual({ signature: 'fakeSAS' }) }) From 3b237111d3aaf0a6b4daf293754f9e364a097ab4 Mon Sep 17 00:00:00 2001 From: spaliwal Date: Mon, 5 Oct 2020 14:47:00 +0530 Subject: [PATCH 5/9] Throw error in case of sas byo creds Updated doc fixed tests for above change --- doc/api.md | 1 + lib/FilesError.js | 1 + lib/impl/AzureBlobFiles.js | 5 +++-- lib/types.jsdoc.js | 1 + test/impl/AzureBlobFiles.test.js | 12 ++++++++++-- types.d.ts | 2 ++ 6 files changed, 18 insertions(+), 4 deletions(-) diff --git a/doc/api.md b/doc/api.md index d4251ed..b3836b2 100644 --- a/doc/api.md +++ b/doc/api.md @@ -539,6 +539,7 @@ Azure account credentials. Must have the permission to create containers. | storageAccount | string | name of azure storage account | | storageAccessKey | string | access key for azure storage account | | containerName | string | name of container to store files. Another `${containerName}-public` will also be used for public files. | +| [hostName] | string | custom domain for returned URLs | diff --git a/lib/FilesError.js b/lib/FilesError.js index b5781e3..1d57a31 100644 --- a/lib/FilesError.js +++ b/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', 'Operation %s not supported for SAS credentails') module.exports = { codes, diff --git a/lib/impl/AzureBlobFiles.js b/lib/impl/AzureBlobFiles.js index a27f920..1057f1a 100644 --- a/lib/impl/AzureBlobFiles.js +++ b/lib/impl/AzureBlobFiles.js @@ -73,7 +73,7 @@ class AzureBlobFiles extends Files { constructor (credentials, tvm) { super() this.tvm = tvm - this.hasOwnCredentials = false + this.hasOwnCredentials = (tvm === null) const cloned = utils.withHiddenFields(credentials, ['storageAccessKey', 'sasURLPrivate', 'sasURLPublic']) logger.debug(`init AzureBlobFiles with config ${JSON.stringify(cloned, null, 2)}`) @@ -110,7 +110,6 @@ class AzureBlobFiles extends Files { this._azure.containerURLPublic = azure.ContainerURL.fromServiceURL(serviceURL, credentials.containerName + '-public') this._azure.sasCreds = false this.credentials = utils.clone(credentials) - this.hasOwnCredentials = true } } @@ -383,6 +382,8 @@ class AzureBlobFiles extends Files { * @private */ async _getAzureBlobPresignCredentials (params) { + if (this._azure.sasCreds) { logAndThrow(new codes.ERROR_UNSUPPORTED_OPERATION({ messageValues: ['generatePresignURL'], sdkDetails: params })) } + const sharedKeyCredential = new azure.SharedKeyCredential(this.credentials.storageAccount, this.credentials.storageAccessKey) const containerName = this.credentials.containerName // generate SAS token diff --git a/lib/types.jsdoc.js b/lib/types.jsdoc.js index 9c0d677..cae66ef 100644 --- a/lib/types.jsdoc.js +++ b/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 diff --git a/test/impl/AzureBlobFiles.test.js b/test/impl/AzureBlobFiles.test.js index 318cb6c..71a07bd 100644 --- a/test/impl/AzureBlobFiles.test.js +++ b/test/impl/AzureBlobFiles.test.js @@ -534,7 +534,7 @@ describe('_getUrl', () => { }) test('test _getUrl custom host', async () => { - files = new AzureBlobFiles({ ...fakeUserCredentials, hostName: 'fakeHost' }) + 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' @@ -610,6 +610,7 @@ describe('_getPresignUrl', () => { test('_getPresignUrl with correct options default permission own credentials', async () => { files.hasOwnCredentials = true files.credentials = fakeUserCredentials + files._azure.sasCreds = false 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' @@ -620,6 +621,7 @@ describe('_getPresignUrl', () => { test('_getPresignUrl with correct options explicit permission own credentials', async () => { files.hasOwnCredentials = true files.credentials = fakeUserCredentials + files._azure.sasCreds = false 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' @@ -627,9 +629,15 @@ describe('_getPresignUrl', () => { expect(url).toEqual(expectedUrl) }) - test('_getAzureBlobPresignCredentials with correct options', async () => { + test('_getAzureBlobPresignCredentials with sas creds', async () => { + files.hasOwnCredentials = true + await expect(files._getAzureBlobPresignCredentials('fakesub/afile', { expiryInSeconds: 60 })).rejects.toThrow('[FilesLib:ERROR_UNSUPPORTED_OPERATION] Operation generatePresignURL not supported for SAS credentails') + }) + + test('_getAzureBlobPresignCredentials with account creds', async () => { files.hasOwnCredentials = true files.credentials = fakeUserCredentials + files._azure.sasCreds = false const ret = await files._getAzureBlobPresignCredentials('fakesub/afile', { expiryInSeconds: 60 }) expect(ret).toEqual({ signature: 'fakeSAS' }) }) diff --git a/types.d.ts b/types.d.ts index 926d544..42a9c79 100644 --- a/types.d.ts +++ b/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; }; /** From 1b7053d252186026c85d89e92e14d7bad4a27064 Mon Sep 17 00:00:00 2001 From: spaliwal Date: Mon, 5 Oct 2020 15:12:07 +0530 Subject: [PATCH 6/9] minor fix for byo sas creds --- lib/impl/AzureBlobFiles.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/impl/AzureBlobFiles.js b/lib/impl/AzureBlobFiles.js index 1057f1a..6144caf 100644 --- a/lib/impl/AzureBlobFiles.js +++ b/lib/impl/AzureBlobFiles.js @@ -96,6 +96,7 @@ class AzureBlobFiles extends Files { /** @private */ this._azure = {} this._azure.aborter = azure.Aborter.none + this.credentials = utils.clone(credentials) if (credentials.sasURLPrivate) { const azureCreds = new azure.AnonymousCredential() const pipeline = azure.StorageURL.newPipeline(azureCreds) @@ -109,7 +110,6 @@ class AzureBlobFiles extends Files { this._azure.containerURLPrivate = azure.ContainerURL.fromServiceURL(serviceURL, credentials.containerName + '') this._azure.containerURLPublic = azure.ContainerURL.fromServiceURL(serviceURL, credentials.containerName + '-public') this._azure.sasCreds = false - this.credentials = utils.clone(credentials) } } From 17de9098852dfe537bdac57ed9dc11520d048646 Mon Sep 17 00:00:00 2001 From: spaliwal Date: Mon, 5 Oct 2020 16:50:34 +0530 Subject: [PATCH 7/9] closed review comments --- lib/FilesError.js | 2 +- lib/impl/AzureBlobFiles.js | 9 +++++++-- test/impl/AzureBlobFiles.test.js | 32 ++++++++++++++++++-------------- 3 files changed, 26 insertions(+), 17 deletions(-) diff --git a/lib/FilesError.js b/lib/FilesError.js index 1d57a31..6f87470 100644 --- a/lib/FilesError.js +++ b/lib/FilesError.js @@ -45,7 +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', 'Operation %s not supported for SAS credentails') +E('ERROR_UNSUPPORTED_OPERATION', '%s') module.exports = { codes, diff --git a/lib/impl/AzureBlobFiles.js b/lib/impl/AzureBlobFiles.js index 6144caf..6c06d9e 100644 --- a/lib/impl/AzureBlobFiles.js +++ b/lib/impl/AzureBlobFiles.js @@ -70,9 +70,11 @@ 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)}`) @@ -382,7 +384,10 @@ class AzureBlobFiles extends Files { * @private */ async _getAzureBlobPresignCredentials (params) { - if (this._azure.sasCreds) { logAndThrow(new codes.ERROR_UNSUPPORTED_OPERATION({ messageValues: ['generatePresignURL'], sdkDetails: 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 diff --git a/test/impl/AzureBlobFiles.test.js b/test/impl/AzureBlobFiles.test.js index 71a07bd..c75170c 100644 --- a/test/impl/AzureBlobFiles.test.js +++ b/test/impl/AzureBlobFiles.test.js @@ -502,13 +502,22 @@ describe('_getUrl', () => { 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() @@ -563,9 +572,11 @@ describe('_getPresignUrl', () => { azure.BlobSASPermissions.parse.mockReset() mockContainerCreate.mockReset() - azure.ContainerURL = { fromServiceURL: jest.fn() } - azure.Aborter = { none: fakeAzureAborter } + 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' }) @@ -576,11 +587,8 @@ describe('_getPresignUrl', () => { 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 () => { @@ -608,9 +616,7 @@ describe('_getPresignUrl', () => { }) test('_getPresignUrl with correct options default permission own credentials', async () => { - files.hasOwnCredentials = true - files.credentials = fakeUserCredentials - files._azure.sasCreds = false + 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' @@ -619,9 +625,7 @@ describe('_getPresignUrl', () => { }) test('_getPresignUrl with correct options explicit permission own credentials', async () => { - files.hasOwnCredentials = true - files.credentials = fakeUserCredentials - files._azure.sasCreds = false + 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' @@ -631,7 +635,7 @@ describe('_getPresignUrl', () => { test('_getAzureBlobPresignCredentials with sas creds', async () => { files.hasOwnCredentials = true - await expect(files._getAzureBlobPresignCredentials('fakesub/afile', { expiryInSeconds: 60 })).rejects.toThrow('[FilesLib:ERROR_UNSUPPORTED_OPERATION] Operation generatePresignURL not supported for SAS credentails') + 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') }) test('_getAzureBlobPresignCredentials with account creds', async () => { From ff2e6b8599257006d5ae87691a28bad9c3f3e7c9 Mon Sep 17 00:00:00 2001 From: spaliwal Date: Mon, 5 Oct 2020 17:04:48 +0530 Subject: [PATCH 8/9] removed unnecessary tests --- lib/impl/AzureBlobFiles.js | 4 ++-- test/impl/AzureBlobFiles.test.js | 12 ++---------- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/lib/impl/AzureBlobFiles.js b/lib/impl/AzureBlobFiles.js index 6c06d9e..465da25 100644 --- a/lib/impl/AzureBlobFiles.js +++ b/lib/impl/AzureBlobFiles.js @@ -393,8 +393,8 @@ class AzureBlobFiles extends Files { const containerName = this.credentials.containerName // generate SAS token const expiryTime = new Date(Date.now() + (1000 * params.expiryInSeconds)) - const perm = (params.permissions === undefined) ? 'r' : params.permissions - const permissions = azure.BlobSASPermissions.parse(perm) + + const permissions = azure.BlobSASPermissions.parse(params.permissions) const commonSasParams = { permissions: permissions.toString(), expiryTime: expiryTime, diff --git a/test/impl/AzureBlobFiles.test.js b/test/impl/AzureBlobFiles.test.js index c75170c..1896bd0 100644 --- a/test/impl/AzureBlobFiles.test.js +++ b/test/impl/AzureBlobFiles.test.js @@ -633,18 +633,10 @@ describe('_getPresignUrl', () => { expect(url).toEqual(expectedUrl) }) - test('_getAzureBlobPresignCredentials with sas creds', async () => { - files.hasOwnCredentials = true + 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') }) - - test('_getAzureBlobPresignCredentials with account creds', async () => { - files.hasOwnCredentials = true - files.credentials = fakeUserCredentials - files._azure.sasCreds = false - const ret = await files._getAzureBlobPresignCredentials('fakesub/afile', { expiryInSeconds: 60 }) - expect(ret).toEqual({ signature: 'fakeSAS' }) - }) }) describe('_statusFromProviderError', () => { From 12ebe86e82e1f1810a47f9ebe80052d6d4e4bd63 Mon Sep 17 00:00:00 2001 From: spaliwal Date: Mon, 5 Oct 2020 18:15:04 +0530 Subject: [PATCH 9/9] added jsdoc comment --- lib/impl/AzureBlobFiles.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/impl/AzureBlobFiles.js b/lib/impl/AzureBlobFiles.js index 465da25..b8d9267 100644 --- a/lib/impl/AzureBlobFiles.js +++ b/lib/impl/AzureBlobFiles.js @@ -98,6 +98,7 @@ 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()