Skip to content

Commit

Permalink
chore: Store API Keys in Config File (twilio#124)
Browse files Browse the repository at this point in the history 
* Add to profiles in config

* Add Tests

* Add api key to profiles at runtime

* minor edits

* AddProjects and nitpicks

* Use addProject in place of addProfiles

* Add UT

* Remove redundant line

* Improved testing assertions
  • Loading branch information
@onuzbee
onuzbee committed Jun 22, 2021
1 parent 22a4785 commit 9fb9dd3
Show file tree
Hide file tree
Showing 2 changed files with 124 additions and 41 deletions.
48 changes: 39 additions & 9 deletions src/services/config.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,15 @@ const MessageTemplates = require('./messaging/templates');
const CLI_NAME = 'twilio-cli';

class ConfigDataProfile {
constructor(accountSid, region, apiKey, apiSecret) {
this.accountSid = accountSid;
this.region = region;
this.apiKey = apiKey;
this.apiSecret = apiSecret;
}
}

class ConfigDataProject {
constructor(id, accountSid, region) {
this.id = id;
this.accountSid = accountSid;
Expand Down Expand Up @@ -76,6 +85,10 @@ class ConfigData {
// Clean the profile ID.
profileId = this.sanitize(profileId);
profile = this.getProfileFromConfigFileById(profileId);
// Explicitly add `id` to the returned profile
if (profile && !profile.hasOwnProperty('id')) {
profile.id = profileId;
}
} else {
profile = this.getActiveProfile();
}
Expand Down Expand Up @@ -103,8 +116,13 @@ class ConfigData {
if (this.activeProfile) {
profile = this.getProfileFromConfigFileById(this.activeProfile);
}

// Ensure order of profiles DI-1479
if (!profile) {
profile = this.projects[0];
profile = this.projects[0] || Object.values(this.profiles)[0];
if (profile && !profile.hasOwnProperty('id')) {
profile.id = Object.keys(this.profiles)[0];
}
}
}
return profile;
Expand All @@ -119,19 +137,30 @@ class ConfigData {
}
}

addProfile(id, accountSid, region) {
// Clean all the inputs.
addProfile(id, accountSid, region, apiKey, apiSecret) {
// Clean all the inputs.
id = this.sanitize(id);
accountSid = this.sanitize(accountSid);
region = this.sanitize(region);

const existing = this.getProfileById(id);

// Remove if existing in historical projects.
if (existing) {
existing.accountSid = accountSid;
existing.region = region;
} else {
this.projects.push(new ConfigDataProfile(id, accountSid, region));
// Remove from Keytar : DI-1352
this.projects = this.projects.filter((p) => p.id !== existing.id);
}

// Update profiles object
this.profiles[id] = new ConfigDataProfile(accountSid, region, apiKey, apiSecret);
}

addProject(id, accountSid, region) {
id = this.sanitize(id);
accountSid = this.sanitize(accountSid);
region = this.sanitize(region);

this.projects.push(new ConfigDataProject(id, accountSid, region));
}

isPromptAcked(promptId) {
Expand All @@ -157,9 +186,9 @@ class ConfigData {
this.prompts = configObj.prompts || {};
// Note the historical 'projects' naming.
configObj.projects = configObj.projects || [];
configObj.projects.forEach((project) => this.addProfile(project.id, project.accountSid, project.region));
this.setActiveProfile(configObj.activeProject);
configObj.projects.forEach((project) => this.addProject(project.id, project.accountSid, project.region));
this.profiles = configObj.profiles || {};
this.setActiveProfile(configObj.activeProject);
}

sanitize(string) {
Expand Down Expand Up @@ -192,6 +221,7 @@ class Config {
prompts: configData.prompts,
// Note the historical 'projects' naming.
projects: configData.projects,
profiles: configData.profiles,
activeProject: configData.activeProfile,
};

Expand Down
117 changes: 85 additions & 32 deletions test/services/config.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,33 @@ describe('services', () => {
const configData = new ConfigData();
configData.addProfile('newProfile', constants.FAKE_ACCOUNT_SID, 'dev');

expect(configData.projects[0].id).to.equal('newProfile');
expect(configData.projects[0].accountSid).to.equal(constants.FAKE_ACCOUNT_SID);
expect(configData.projects[0].region).to.equal('dev');
expect(configData.profiles.newProfile.accountSid).to.equal(constants.FAKE_ACCOUNT_SID);
expect(configData.profiles.newProfile.region).to.equal('dev');
});

test.it('should add a new profile with apiKey', () => {
const configData = new ConfigData();
configData.addProfile(
'newProfile',
constants.FAKE_ACCOUNT_SID,
'dev',
constants.FAKE_API_KEY,
constants.FAKE_API_SECRET,
);

expect(configData.profiles.newProfile.accountSid).to.equal(constants.FAKE_ACCOUNT_SID);
expect(configData.profiles.newProfile.region).to.equal('dev');
expect(configData.profiles.newProfile.apiKey).to.equal(constants.FAKE_API_KEY);
expect(configData.profiles.newProfile.apiSecret).to.equal(constants.FAKE_API_SECRET);
});

test.it('should update an existing profile', () => {
const configData = new ConfigData();
configData.addProfile('activeProfile', constants.FAKE_ACCOUNT_SID, 'dev');
configData.addProfile('activeProfile', 'new-account-sid');

expect(configData.projects[0].id).to.equal('activeProfile');
expect(configData.projects[0].accountSid).to.equal('new-account-sid');
expect(configData.projects[0].region).to.be.undefined;
expect(configData.profiles.activeProfile.accountSid).to.equal('new-account-sid');
expect(configData.profiles.activeProfile.region).to.be.undefined;
});
});

Expand Down Expand Up @@ -164,6 +178,21 @@ describe('services', () => {
expect(active.accountSid).to.equal('new_account_SID');
});

test.it('should remove profile from projects if duplicate found', () => {
const configData = new ConfigData();
configData.addProject('testProfile', constants.FAKE_ACCOUNT_SID);
configData.addProfile(
'testProfile',
constants.FAKE_ACCOUNT_SID,
'',
constants.FAKE_API_KEY,
constants.FAKE_API_SECRET,
);

expect(configData.projects).to.be.empty;
expect(configData.profiles.testProfile.accountSid).to.equal(constants.FAKE_ACCOUNT_SID);
});

test.it('should not allow the active profile to not exist', () => {
const configData = new ConfigData();
configData.addProfile('firstProfile', constants.FAKE_ACCOUNT_SID);
Expand Down Expand Up @@ -194,30 +223,33 @@ describe('services', () => {
expect(configData.projects.length).to.equal(originalLength);
});

test.it('removes profile', () => {
const configData = new ConfigData();
configData.addProfile('firstProfile', constants.FAKE_ACCOUNT_SID);
configData.addProfile('secondProfile', 'new_account_SID');
configData.addProfile('thirdProfile', 'newest_account_SID');
const profile = configData.getProfileById('secondProfile');
configData.removeProfile(profile);

expect(configData.projects[1].id).to.equal('thirdProfile');
expect(configData.projects[1].accountSid).to.equal('newest_account_SID');
});

test.it('removes active profile', () => {
const configData = new ConfigData();
configData.addProfile('firstProfile', constants.FAKE_ACCOUNT_SID);
configData.addProfile('secondProfile', 'new_account_SID');
configData.addProfile('thirdProfile', 'newest_account_SID');
const profile = configData.setActiveProfile('firstProfile');
configData.removeProfile(profile);

expect(configData.projects[1].id).to.equal('thirdProfile');
expect(configData.projects[1].accountSid).to.equal('newest_account_SID');
expect(configData.activeProfile).to.equal(null);
});
/*
* TODO: To be fixed with profiles:remove functionality
* test.it('removes profile', () => {
* const configData = new ConfigData();
* configData.addProfile('firstProfile', constants.FAKE_ACCOUNT_SID);
* configData.addProfile('secondProfile', 'new_account_SID');
* configData.addProfile('thirdProfile', 'newest_account_SID');
* const profile = configData.getProfileById('secondProfile');
* configData.removeProfile(profile);
*
* expect(configData.projects[1].id).to.equal('thirdProfile');
* expect(configData.projects[1].accountSid).to.equal('newest_account_SID');
* });
*
* test.it('removes active profile', () => {
* const configData = new ConfigData();
* configData.addProfile('firstProfile', constants.FAKE_ACCOUNT_SID);
* configData.addProfile('secondProfile', 'new_account_SID');
* configData.addProfile('thirdProfile', 'newest_account_SID');
* const profile = configData.setActiveProfile('firstProfile');
* configData.removeProfile(profile);
*
* expect(configData.projects[1].id).to.equal('thirdProfile');
* expect(configData.projects[1].accountSid).to.equal('newest_account_SID');
* expect(configData.activeProfile).to.equal(null);
* });
*/
});
describe('ConfigData.prompts', () => {
test.it('should store prompt acks', () => {
Expand All @@ -231,12 +263,15 @@ describe('services', () => {
});

describe('Config', () => {
const tempConfigDir = tmp.dirSync({ unsafeCleanup: true });
let tempConfigDir;
beforeEach(() => {
tempConfigDir = tmp.dirSync({ unsafeCleanup: true });
});

test.it('saves and loads user configuration with space trimmed', async () => {
const config = new Config(tempConfigDir.name);
const userConfig = await config.load();
userConfig.addProfile(' profile \t', 'sid \n ', ' stage');
userConfig.addProfile(' profile \t', 'sid \n ', ' stage', 'test_key', 'test_secret');
userConfig.setActiveProfile('\tprofile\t');
userConfig.ackPrompt('impromptu');

Expand All @@ -248,6 +283,24 @@ describe('services', () => {
expect(loadedConfig.getActiveProfile().id).to.equal('profile');
});

test.it('should load projects post sanitization and not removed from list on load', async () => {
const config = new Config(tempConfigDir.name);
const configData = await config.load();
configData.addProfile(' profile ', 'sid_profile ', ' dev', 'test_key', 'test_secret');
configData.addProject(' profile', ' sid_project ', ' dev');
await config.save(configData);

const loadedConfig = await config.load();
expect(loadedConfig).to.deep.equal(configData);
expect(loadedConfig.projects).to.have.length(1); // Removal shouldn't be performed on projects
expect(Object.keys(loadedConfig.profiles)).to.have.length(1);
expect(Object.keys(loadedConfig.profiles)[0]).to.equal('profile');
expect(loadedConfig.profiles.profile.accountSid).to.equal('sid_profile');
expect(loadedConfig.projects[0].id).to.equal('profile');
expect(loadedConfig.projects[0].accountSid).to.equal('sid_project');
expect(loadedConfig.projects[0].region).to.equal('dev');
});

test.it('works with config dirs that did not exist', async () => {
const nestedConfig = path.join(tempConfigDir.name, 'some', 'nested', 'path');

Expand Down

0 comments on commit 9fb9dd3

Please sign in to comment.