Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: cleanup keytar and port command #436

Merged
merged 2 commits into from
Aug 5, 2022
Merged

fix: cleanup keytar and port command #436

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a87c006
fix: cleanup keytar and port command
kridai Aug 1, 2022
a46ac66
fix: cleanup keytar and port command
kridai Aug 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 0 additions & 12 deletions src/commands/profiles/create.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,6 @@ class ProfilesCreate extends BaseCommand {
this.logger.debug(error);
throw new TwilioCliError('Could not create an API Key.');
}
await this.removeKeytarKeysByProfileId(this.profileId);
this.userConfig.addProfile(this.profileId, this.accountSid, this.region, apiKey.sid, apiKey.secret);
const configSavedMessage = await this.configFile.save(this.userConfig);

Expand All @@ -242,17 +241,6 @@ class ProfilesCreate extends BaseCommand {
);
this.logger.info(configSavedMessage);
}

async removeKeytarKeysByProfileId(profileId) {
if (this.userConfig.projects.find((p) => p.id === profileId)) {
const removed = await this.secureStorage.removeCredentials(profileId);
if (removed === true) {
this.logger.info('Deleted key from keytar.');
} else {
this.logger.warn(`Could not delete ${profileId} key from keytar.`);
}
}
}
}

ProfilesCreate.aliases = ['profiles:add', 'login'];
Expand Down
85 changes: 0 additions & 85 deletions src/commands/profiles/port.js
View file
Open in desktop

This file was deleted.

21 changes: 3 additions & 18 deletions src/commands/profiles/remove.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,7 @@ class ProfilesRemove extends TwilioClientCommand {
const verdict = await this.confirmRemoveProfile();
if (verdict === true) {
const keyVerdict = await this.confirmRemoveKey();
let credentials;
if (!this.userConfig.profiles[deleteProfile.id]) {
credentials = await this.deleteLocalKey(deleteProfile);
}
await this.deleteRemoteKey(deleteProfile, keyVerdict, credentials);
await this.deleteRemoteKey(deleteProfile, keyVerdict);
this.logger.info(`Deleted ${deleteProfile.id} profile.`);
this.userConfig.removeProfile(deleteProfile);
} else {
Expand Down Expand Up @@ -43,21 +39,10 @@ class ProfilesRemove extends TwilioClientCommand {
}
}

async deleteLocalKey(profileDelete) {
const credentials = this.secureStorage.getCredentials(profileDelete.id);
const removed = await this.secureStorage.removeCredentials(profileDelete.id);
if (removed === true) {
this.logger.info('Deleted local key.');
} else {
this.logger.warn('Could not delete local key.');
}
return credentials;
}

async deleteRemoteKey(profileDelete, keyVerdict, credentials) {
async deleteRemoteKey(profileDelete, keyVerdict) {
if (keyVerdict === true) {
try {
const apiKey = profileDelete.apiKey || credentials.apiKey;
const { apiKey } = profileDelete;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be profileDelete.apikey ? Didn't find any reference above for this change

Copy link
Contributor Author

@kridai kridai Aug 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its object destructuring being used here. It was recommended as part of sonar scan. its same as setting it up as profileDelete.apikey

await this.twilioClient.api.keys(apiKey).remove();
this.logger.info('The API Key has been deleted from The Twilio console.');
} catch (error) {
Expand Down
11 changes: 5 additions & 6 deletions src/services/post-install-utility.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@ const chalk = require('chalk');
const { Config } = require('@twilio/cli-core').services.config;
const { configureEnv } = require('@twilio/cli-core');

const PORT_WARNING = `Profiles exist with API keys in system keychain. Please run ${chalk.bold(
'twilio profiles:port',
)} to port all keys to the config file`;

const AUTOCOMLETE_WARNING = `If you’re using autocomplete, you’ll need to run '${chalk.bold(
'twilio autocomplete',
)}' after the install and then open a new terminal window. The CLI needs to re-build its cache.`;

const UPDATE_PROFILE_WARNING = `Profiles exist with API keys in system keychain. Please reconfigure profile using ${chalk.bold(
'twilio profiles:create',
)} to add profiles to the config file.`;

class PostInstallDisplayManager {
constructor(configDir, userConfig) {
configureEnv();
Expand Down Expand Up @@ -51,14 +51,13 @@ class PostInstallDisplayManager {
}

if (this.hasProjects()) {
console.warn(chalk.yellowBright(` » ${PORT_WARNING}`));
console.warn(chalk.yellowBright(` » ${UPDATE_PROFILE_WARNING}`));
}
console.warn(chalk.yellowBright(` » ${AUTOCOMLETE_WARNING}`));
}
}

module.exports = {
PostInstallDisplayManager,
PORT_WARNING,
AUTOCOMLETE_WARNING,
};
8 changes: 7 additions & 1 deletion test/commands/phone-numbers/update.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,13 @@ describe('commands', () => {
return test
.do((ctx) => {
ctx.userConfig = new ConfigData();
ctx.userConfig.addProfile('default', constants.FAKE_ACCOUNT_SID);
ctx.userConfig.addProfile(
'default',
constants.FAKE_ACCOUNT_SID,
'',
constants.FAKE_API_KEY,
constants.FAKE_API_SECRET,
);
ctx.userConfig.setActiveProfile('default');

if (promptAcked) {
Expand Down
9 changes: 1 addition & 8 deletions test/commands/profiles/create.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const helpMessages = require('../../../src/services/messaging/help-messages');
describe('commands', () => {
describe('profiles', () => {
describe('create', () => {
const createTest = (commandArgs = [], { profileId = 'default', addProjects = [], removeCred = true } = {}) =>
const createTest = (commandArgs = [], { profileId = 'default', addProjects = [] } = {}) =>
test
.twilioFakeProfile(ConfigData)
.do((ctx) => {
Expand Down Expand Up @@ -39,11 +39,6 @@ describe('commands', () => {
overwrite: true,
});
ctx.testCmd.inquirer.prompt = fakePrompt;
})
.do((ctx) => {
ctx.testCmd.secureStorage.removeCredentials = () => {
return removeCred;
};
});

const mockSuccess = (api) => {
Expand Down Expand Up @@ -81,7 +76,6 @@ describe('commands', () => {
expect(ctx.stdout).to.equal('');
expect(ctx.stderr).to.contain(helpMessages.AUTH_TOKEN_NOT_SAVED);
expect(ctx.stderr).to.contain('Saved profile1.');
expect(ctx.stderr).to.contain('Deleted key from keytar.');
expect(ctx.stderr).to.contain('configuration saved');
expect(ctx.stderr).to.contain(`Created API Key ${constants.FAKE_API_KEY} and stored the secret in Config.`);
expect(ctx.stderr).to.contain(
Expand All @@ -96,7 +90,6 @@ describe('commands', () => {
expect(ctx.stdout).to.equal('');
expect(ctx.stderr).to.contain(helpMessages.AUTH_TOKEN_NOT_SAVED);
expect(ctx.stderr).to.contain('Saved profile1.');
expect(ctx.stderr).to.contain('Could not delete profile1 key from keytar.');
expect(ctx.stderr).to.contain('configuration saved');
expect(ctx.stderr).to.contain(`Created API Key ${constants.FAKE_API_KEY} and stored the secret in Config.`);
expect(ctx.stderr).to.contain(
Expand Down
20 changes: 1 addition & 19 deletions test/commands/profiles/list.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,25 +143,6 @@ describe('commands', () => {
expect(ctx.stderr).to.equal('');
});

test
.do((ctx) => {
ctx.userConfig = new ConfigData();
ctx.userConfig.addProject('profile1', constants.FAKE_ACCOUNT_SID);
ctx.userConfig.addProject('profile2', constants.FAKE_ACCOUNT_SID);
})
.twilioCliEnv(Config)
.stdout()
.stderr()
.twilioCommand(ProfilesList, [])
.it('runs profiles:list with multiple profiles from projects', (ctx) => {
expect(ctx.stdout).to.contain('profile1');
expect(ctx.stdout).to.contain('profile2');
expect(ctx.stdout).to.contain(constants.FAKE_ACCOUNT_SID);
expect(ctx.stdout).to.not.contain('Region');
expect(ctx.stdout.match(/true/g)).to.have.length(1);
expect(ctx.stderr).to.equal('');
});

test
.do((ctx) => {
ctx.userConfig = new ConfigData();
Expand Down Expand Up @@ -255,6 +236,7 @@ describe('commands', () => {
constants.FAKE_API_SECRET,
);
ctx.userConfig.addProject('profile2', constants.FAKE_ACCOUNT_SID);
ctx.userConfig.activeProfile = 'profile1';
})
.twilioCliEnv(Config)
.stdout()
Expand Down
128 changes: 0 additions & 128 deletions test/commands/profiles/port.test.js
View file
Open in desktop

This file was deleted.

Loading