Skip to content

Commit

Permalink
feat: improve 'access denied' error messaging (twilio#95)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
childish-sambino authored Jun 29, 2020
1 parent 91a1898 commit 62c3c5f
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 71 deletions.
16 changes: 15 additions & 1 deletion src/base-commands/twilio-client-command.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down
17 changes: 11 additions & 6 deletions src/services/messaging/help-messages.js
View file
Original file line number Diff line number Diff line change
@@ -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. \
Expand Down
124 changes: 60 additions & 64 deletions test/base-commands/twilio-client-command.test.js
View file
Original file line number Diff line number Diff line change
@@ -1,41 +1,66 @@
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();

throw new Error('We were so wrong!');
}
}

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)
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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');
});
});
});
});

0 comments on commit 62c3c5f

Please sign in to comment.