From 62c3c5f460951eb99779fc8c32ee19e48d07f2c7 Mon Sep 17 00:00:00 2001 From: childish-sambino Date: Mon, 29 Jun 2020 09:32:16 -0500 Subject: [PATCH] feat: improve 'access denied' error messaging (#95) When users attempt to access resources that require auth greater than what Standard API Keys grant, they are met with an access denied error. Since CLI profiles use such keys, messaging has been added instructing how to use non-standard auth when working with such resources. --- src/base-commands/twilio-client-command.js | 16 ++- src/services/messaging/help-messages.js | 17 ++- .../twilio-client-command.test.js | 124 +++++++++--------- 3 files changed, 86 insertions(+), 71 deletions(-) diff --git a/src/base-commands/twilio-client-command.js b/src/base-commands/twilio-client-command.js index eb9adcb7..faab0497 100644 --- a/src/base-commands/twilio-client-command.js +++ b/src/base-commands/twilio-client-command.js @@ -5,11 +5,13 @@ const { TwilioApiClient, TwilioApiFlags } = require('../services/twilio-api'); const { TwilioCliError } = require('../services/error'); const { translateValues } = require('../services/javascript-utilities'); const { camelCase, kebabCase } = require('../services/naming-conventions'); -const { HELP_ENVIRONMENT_VARIABLES } = require('../services/messaging/help-messages'); +const { ACCESS_DENIED, HELP_ENVIRONMENT_VARIABLES } = require('../services/messaging/help-messages'); // CLI flags are kebab-cased, whereas API flags are PascalCased. const CliFlags = translateValues(TwilioApiFlags, kebabCase); +const ACCESS_DENIED_CODE = 20003; + class TwilioClientCommand extends BaseCommand { constructor(argv, config) { super(argv, config); @@ -51,6 +53,18 @@ class TwilioClientCommand extends BaseCommand { this.httpClient = new CliRequestClient(this.id, this.logger); } + async catch(error) { + // Append to the error message when catching API access denied errors with + // profile-auth (i.e., standard API key auth). + if (error instanceof TwilioCliError && error.exitCode === ACCESS_DENIED_CODE) { + if (!this.currentProfile.id.startsWith('${TWILIO')) { // Auth *not* using env vars. + error.message += '\n\n' + ACCESS_DENIED; + } + } + + return super.catch(error); + } + parseProperties() { if (!this.constructor.PropertyFlags) { return null; diff --git a/src/services/messaging/help-messages.js b/src/services/messaging/help-messages.js index 1fc736b3..d25a2e78 100644 --- a/src/services/messaging/help-messages.js +++ b/src/services/messaging/help-messages.js @@ -1,19 +1,24 @@ const { CLI_NAME } = require('../config'); const ENV_VAR_CMD = process.platform === 'win32' ? 'set' : 'export'; - -exports.HELP_ENVIRONMENT_VARIABLES = `Alternatively, ${CLI_NAME} can use credentials stored in environment variables: - -# OPTION 1 (recommended) +const ENV_VARS_USAGE = `# OPTION 1 (recommended) ${ENV_VAR_CMD} TWILIO_ACCOUNT_SID=your Account SID from twil.io/console ${ENV_VAR_CMD} TWILIO_API_KEY=an API Key created at twil.io/get-api-key ${ENV_VAR_CMD} TWILIO_API_SECRET=the secret for the API Key # OPTION 2 ${ENV_VAR_CMD} TWILIO_ACCOUNT_SID=your Account SID from twil.io/console -${ENV_VAR_CMD} TWILIO_AUTH_TOKEN=your Auth Token from twil.io/console +${ENV_VAR_CMD} TWILIO_AUTH_TOKEN=your Auth Token from twil.io/console`; + +exports.HELP_ENVIRONMENT_VARIABLES = `Alternatively, ${CLI_NAME} can use credentials stored in environment variables: + +${ENV_VARS_USAGE} + +Once these environment variables are set, a ${CLI_NAME} profile is not required and you may skip the "login" step.`; + +exports.ACCESS_DENIED = `${CLI_NAME} profiles use Standard API Keys which are not permitted to manage Accounts (e.g., create Subaccounts) and other API Keys. If you require this functionality a Master API Key or Auth Token must be stored in environment variables: -Once these environment variables are set, a ${CLI_NAME} profile is not required to move forward with installation.`; +${ENV_VARS_USAGE}`; exports.NETWORK_ERROR = `${CLI_NAME} encountered a network connectivity error. \ Please check your network connection and try your command again. \ diff --git a/test/base-commands/twilio-client-command.test.js b/test/base-commands/twilio-client-command.test.js index e68cd1f8..fe86c419 100644 --- a/test/base-commands/twilio-client-command.test.js +++ b/test/base-commands/twilio-client-command.test.js @@ -1,15 +1,14 @@ const { expect, test, constants } = require('@twilio/cli-test'); const TwilioClientCommand = require('../../src/base-commands/twilio-client-command'); const { Config, ConfigData } = require('../../src/services/config'); - -const ORIGINAL_ENV = process.env; +const { TwilioCliError } = require('../../src/services/error'); describe('base-commands', () => { describe('twilio-client-command', () => { class TestClientCommand extends TwilioClientCommand { } - class ThrowingClientCommand extends TwilioClientCommand { + class ThrowingUnknownClientCommand extends TwilioClientCommand { async run() { await super.run(); @@ -17,25 +16,51 @@ describe('base-commands', () => { } } + class Throwing20003ClientCommand extends TwilioClientCommand { + async run() { + await super.run(); + + throw new TwilioCliError('Access Denied!', 20003); + } + } + class AccountSidClientCommand extends TwilioClientCommand { } TestClientCommand.flags = TwilioClientCommand.flags; - ThrowingClientCommand.flags = TwilioClientCommand.flags; + ThrowingUnknownClientCommand.flags = TwilioClientCommand.flags; + Throwing20003ClientCommand.flags = TwilioClientCommand.flags; AccountSidClientCommand.flags = Object.assign({}, TwilioClientCommand.flags, TwilioClientCommand.accountSidFlag); const setUpTest = ( args = [], - { setUpUserConfig = undefined, mockSecureStorage = true, commandClass: CommandClass = TestClientCommand } = {} + { + setUpUserConfig = undefined, + mockSecureStorage = true, + commandClass: CommandClass = TestClientCommand, + envRegion, envEdge, configRegion = 'configRegion', configEdge + } = {} ) => { return test .do(ctx => { ctx.userConfig = new ConfigData(); + ctx.userConfig.edge = configEdge; + + if (envRegion) { + process.env.TWILIO_REGION = envRegion; + process.env.TWILIO_ACCOUNT_SID = constants.FAKE_ACCOUNT_SID; + process.env.TWILIO_AUTH_TOKEN = constants.FAKE_API_SECRET; + } + + if (envEdge) { + process.env.TWILIO_EDGE = envEdge; + } + if (setUpUserConfig) { setUpUserConfig(ctx.userConfig); } else { ctx.userConfig.addProfile('MyFirstProfile', constants.FAKE_ACCOUNT_SID); - ctx.userConfig.addProfile('twilio-cli-unit-testing', constants.FAKE_ACCOUNT_SID, 'stage'); + ctx.userConfig.addProfile('region-edge-testing', constants.FAKE_ACCOUNT_SID, configRegion); } }) .twilioCliEnv(Config) @@ -100,27 +125,41 @@ describe('base-commands', () => { expect(ctx.stderr).to.contain('TWILIO_ACCOUNT_SID'); }); - setUpTest(['-p', 'twilio-cli-unit-testing']).it('should create a client for a non-default profile', ctx => { + setUpTest(['-p', 'region-edge-testing']).it('should create a client for a non-default profile', ctx => { expect(ctx.testCmd.twilioClient.accountSid).to.equal(constants.FAKE_ACCOUNT_SID); expect(ctx.testCmd.twilioClient.username).to.equal(constants.FAKE_API_KEY); - expect(ctx.testCmd.twilioClient.password).to.equal(constants.FAKE_API_SECRET + 'twilio-cli-unit-testing'); - expect(ctx.testCmd.twilioClient.region).to.equal('stage'); + expect(ctx.testCmd.twilioClient.password).to.equal(constants.FAKE_API_SECRET + 'region-edge-testing'); + expect(ctx.testCmd.twilioClient.region).to.equal('configRegion'); }); - setUpTest(['-p', 'twilio-cli-unit-testing'], { mockSecureStorage: false }) + setUpTest(['-p', 'region-edge-testing'], { mockSecureStorage: false }) .exit(1) .it('should handle a secure storage error', ctx => { - expect(ctx.stderr).to.contain('Could not get credentials for profile "twilio-cli-unit-testing"'); + expect(ctx.stderr).to.contain('Could not get credentials for profile "region-edge-testing"'); expect(ctx.stderr).to.contain('To reconfigure the profile, run:'); - expect(ctx.stderr).to.contain('twilio profiles:create --profile "twilio-cli-unit-testing"'); + expect(ctx.stderr).to.contain('twilio profiles:create --profile "region-edge-testing"'); }); - setUpTest([], { commandClass: ThrowingClientCommand }) + setUpTest([], { commandClass: ThrowingUnknownClientCommand }) .exit(1) .it('should catch unhandled errors', ctx => { expect(ctx.stderr).to.contain('unexpected error'); }); + setUpTest([], { commandClass: Throwing20003ClientCommand }) + .exit(20003) + .it('should catch access denied errors and enhance the message', ctx => { + expect(ctx.stderr).to.contain('Access Denied'); + expect(ctx.stderr).to.contain('Standard API Keys'); + }); + + setUpTest([], { commandClass: Throwing20003ClientCommand, envRegion: 'region' }) + .exit(20003) + .it('should catch access denied errors but not enhance the message when using env var auth', ctx => { + expect(ctx.stderr).to.contain('Access Denied'); + expect(ctx.stderr).to.not.contain('Standard API Keys'); + }); + describe('parseProperties', () => { setUpTest().it('should ignore empty PropertyFlags', ctx => { const updatedProperties = ctx.testCmd.parseProperties(); @@ -236,69 +275,26 @@ describe('base-commands', () => { }); describe('regional and edge support', () => { - const envTest = ( - args = [], - { envRegion, envEdge, configRegion = 'configRegion', configEdge } = {} - ) => { - return test - .do(ctx => { - ctx.userConfig = new ConfigData(); - ctx.userConfig.edge = configEdge; - - if (envRegion) { - process.env.TWILIO_REGION = envRegion; - process.env.TWILIO_ACCOUNT_SID = constants.FAKE_ACCOUNT_SID; - process.env.TWILIO_AUTH_TOKEN = constants.FAKE_API_SECRET; - } - if (envEdge) { - process.env.TWILIO_EDGE = envEdge; - } - - ctx.userConfig.addProfile('default-profile', constants.FAKE_ACCOUNT_SID); - ctx.userConfig.addProfile('region-edge-testing', constants.FAKE_ACCOUNT_SID, configRegion); - }) - .twilioCliEnv(Config) - .do(async ctx => { - ctx.testCmd = new TwilioClientCommand(args, ctx.fakeConfig); - ctx.testCmd.secureStorage = - { - async getCredentials(profileId) { - return { - apiKey: constants.FAKE_API_KEY, - apiSecret: constants.FAKE_API_SECRET + profileId - }; - } - }; - - // This is essentially what oclif does behind the scenes. - try { - await ctx.testCmd.run(); - } catch (error) { - await ctx.testCmd.catch(error); - } - process.env = ORIGINAL_ENV; - }); - }; - - envTest([], { configEdge: 'edge' }).it('should use the config edge when defined', ctx => { + setUpTest([], { configEdge: 'edge' }).it('should use the config edge when defined', ctx => { expect(ctx.testCmd.twilioApiClient.edge).to.equal('edge'); expect(ctx.testCmd.twilioApiClient.region).to.be.undefined; }); - envTest(['-p', 'region-edge-testing']).it('should use the config region when defined', ctx => { + setUpTest(['-p', 'region-edge-testing']).it('should use the config region when defined', ctx => { expect(ctx.testCmd.twilioApiClient.region).to.equal('configRegion'); expect(ctx.testCmd.twilioApiClient.edge).to.be.undefined; }); - envTest([], { envRegion: 'region' }).it('should use the env region over a config region', ctx => { + setUpTest([], { envRegion: 'region' }).it('should use the env region over a config region', ctx => { expect(ctx.testCmd.twilioApiClient.region).to.equal('region'); expect(ctx.testCmd.twilioApiClient.edge).to.be.undefined; }); - envTest([], { configEdge: 'configEdge', envEdge: 'edge', envRegion: 'region' }).it('should use the env edge over a config edge', ctx => { - expect(ctx.testCmd.twilioApiClient.edge).to.equal('edge'); - expect(ctx.testCmd.twilioApiClient.region).to.equal('region'); - }); + setUpTest([], { configEdge: 'configEdge', envEdge: 'edge', envRegion: 'region' }) + .it('should use the env edge over a config edge', ctx => { + expect(ctx.testCmd.twilioApiClient.edge).to.equal('edge'); + expect(ctx.testCmd.twilioApiClient.region).to.equal('region'); + }); }); }); });