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

chore: CLI Profile Update - Using config file instead of the system keychain #264

Merged
merged 5 commits into from
Jun 28, 2021
Merged

chore: CLI Profile Update - Using config file instead of the system keychain #264

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8de51d3
chore: CLI Profile Update - Using config file instead of the system k…
ravali-rimmalapudi Jun 25, 2021
dac8bcb
Added the test cases.
ravali-rimmalapudi Jun 25, 2021
d918763
Addressed the review comments
ravali-rimmalapudi Jun 28, 2021
e621ea6
Added the missing alignment.
ravali-rimmalapudi Jun 28, 2021
8f7b136
Addressed the review comments
ravali-rimmalapudi Jun 28, 2021
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
13 changes: 12 additions & 1 deletion src/commands/profiles/create.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,17 @@ class ProfilesCreate extends BaseCommand {
}
}

async removeKeytarKeysByProfileId(profileId) {
childish-sambino marked this conversation as resolved.
Show resolved Hide resolved
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 key from keytar.');
childish-sambino marked this conversation as resolved.
Show resolved Hide resolved
}
}
}

async saveCredentials() {
const apiKeyFriendlyName = this.getApiKeyFriendlyName();
let apiKey = null;
Expand All @@ -230,7 +241,7 @@ class ProfilesCreate extends BaseCommand {
this.logger.debug(error);
throw new TwilioCliError('Could not create an API Key.');
}

await this.removeKeytarKeysByProfileId(this.profileId);
Sindhura3 marked this conversation as resolved.
Show resolved Hide resolved
this.userConfig.addProfile(this.profileId, this.accountSid, this.region, apiKey.sid, apiKey.secret);
const configSavedMessage = await this.configFile.save(this.userConfig);

Expand Down
43 changes: 42 additions & 1 deletion test/commands/profiles/create.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,15 @@ const helpMessages = require('../../../src/services/messaging/help-messages');
describe('commands', () => {
describe('profiles', () => {
describe('create', () => {
const createTest = (commandArgs = [], profileId = 'default') =>
const createTest = (commandArgs = [], { profileId = 'default', addProjects = 0, removeCred = true } = {}) =>
test
.twilioFakeProfile(ConfigData)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are using these methods in testcasestwilioFakeProfile, twilioCliEnv and twilioCreateCommand, but I am not able to find the definitions. Are these taken care by Mocha?

Copy link
Contributor

Choose a reason for hiding this comment

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

https://code.hq.twilio.com/twilio/cli-test/blob/master/src/index.js

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for pointing the repo, It has twilioCliEnv and twilioCreateCommand methods but not twilioFakeProfile(It used to be twilioFakeProject - as part Switch naming from 'project(s)' to 'profile(s)' we changed it to twilioFakeProfile). Even though the method is not there in cli-test our test are passing(I am not sure why).

.do((ctx) => {
ctx.userConfig = new ConfigData();
for (let i = 1; i <= addProjects; i++) {
ctx.userConfig.addProject(`profile${i}`, constants.FAKE_ACCOUNT_SID);
}
})
.twilioCliEnv(Config)
.twilioCreateCommand(ProfilesCreate, commandArgs)
.stdout()
Expand All @@ -35,6 +41,11 @@ describe('commands', () => {
overwrite: true,
});
ctx.testCmd.inquirer.prompt = fakePrompt;
})
.do((ctx) => {
ctx.testCmd.secureStorage.removeCredentials = () => {
return removeCred;
};
});

const mockSuccess = (api) => {
Expand Down Expand Up @@ -65,6 +76,36 @@ describe('commands', () => {
);
});

createTest([], { profileId: 'profile1', addProjects: 1 })
childish-sambino marked this conversation as resolved.
Show resolved Hide resolved
.nock('https://api.twilio.com', mockSuccess)
.do((ctx) => ctx.testCmd.run())
.it('runs profiles:create with existing profile in Projects', (ctx) => {
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(
`See: https://www.twilio.com/console/runtime/api-keys/${constants.FAKE_API_KEY}`,
);
});

createTest([], { profileId: 'profile1', addProjects: 1, removeCred: false })
.nock('https://api.twilio.com', mockSuccess)
.do((ctx) => ctx.testCmd.run())
.it('runs profiles:create with existing profile in Projects with Keytar remove failed', (ctx) => {
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 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(
`See: https://www.twilio.com/console/runtime/api-keys/${constants.FAKE_API_KEY}`,
);
});

createTest()
.do((ctx) => {
sinon.stub(os, 'hostname').returns('some_super_long_fake_hostname');
Expand Down