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.

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

Secrets encryption from CLI #141

Merged
merged 6 commits into from
Jun 21, 2024

Conversation

nvyasadobe
Copy link
Contributor

Description

This PR is to encrypt secrets data from CLI. Whenever a create OR update command will be executed with --secrets followed by a secrets yaml file, we need to encrypt the secrets data before sending it over.

Related Issue

https://jira.corp.adobe.com/browse/CEXT-3188
(point 3)

Motivation and Context

This change is required for secrets encryption.

How Has This Been Tested?

aio plugins link . in this PR branch, make sure the endpoint returns the public key
aio api-mesh create mesh.json --secrets mysecrets.yaml
aio api-mesh update mesh.json --secrets mysecrets.yaml

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have signed the Adobe Open Source CLA.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@nvyasadobe nvyasadobe requested a review from revanth0212 June 19, 2024 13:12
Copy link
Contributor

@revanth0212 revanth0212 left a comment

Choose a reason for hiding this comment

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

Just a few changes

Comment on lines 116 to 117
const publicKey = await getPublicEncryptionKey(imsOrgCode);
const secretsData = await interpolateSecrets(secretsFilePath, this);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const publicKey = await getPublicEncryptionKey(imsOrgCode);
const secretsData = await interpolateSecrets(secretsFilePath, this);
const secretsData = await interpolateSecrets(secretsFilePath, this);
const publicKey = await getPublicEncryptionKey(imsOrgCode);

I woluld switch them because if interpolateSecrets failed due to any error, there wont be any use for the publicKey. Lets avoid unnecessary fetches where possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, addressed in latest commit

Comment on lines 122 to 123
const publicKey = await getPublicEncryptionKey(imsOrgCode);
const secretsData = await interpolateSecrets(secretsFilePath, this);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const publicKey = await getPublicEncryptionKey(imsOrgCode);
const secretsData = await interpolateSecrets(secretsFilePath, this);
const secretsData = await interpolateSecrets(secretsFilePath, this);
const publicKey = await getPublicEncryptionKey(imsOrgCode);

Same as before

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, addressed in latest commit

src/utils.js Outdated
* @param publicKey Public key for (AES + RSA) encryption
* @param secrets Secrets Data that needs encryption
*/
async function encryptSecret(publicKey, secrets) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
async function encryptSecret(publicKey, secrets) {
async function encryptSecrets(publicKey, secrets) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, addressed in latest commit

src/utils.js Outdated
async function encryptSecret(publicKey, secrets) {
if (!publicKey || typeof publicKey !== 'string' || !publicKey.trim()) {
throw new Error(
chalk.red('Something went wrong in secerts encryption. Invalid publicKey provided.'),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
chalk.red('Something went wrong in secerts encryption. Invalid publicKey provided.'),
chalk.red('Unable to encrypt secerts. Invalid Public Key.'),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, addressed in latest commit

src/utils.js Outdated
Comment on lines 532 to 533
logger.error('Error generating AES key, IV OR encryption package:', error.message);
throw new Error(chalk.red(`Failed to generate encryption parameters. Please try again.`));
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
logger.error('Error generating AES key, IV OR encryption package:', error.message);
throw new Error(chalk.red(`Failed to generate encryption parameters. Please try again.`));
logger.error('Unable to encrypt secrets. Please try again. :', error.message);
throw new Error(`Unable to encrypt secerts. ${error.message}`);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, addressed in latest commit

Comment on lines 1007 to 1009
let errorMessage = `Failed to load encryption keys. Please contact support.`;
logger.error(`${errorMessage}. Received ${response.status} response instead of 200`);
throw new Error(chalk.red(errorMessage));
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
let errorMessage = `Failed to load encryption keys. Please contact support.`;
logger.error(`${errorMessage}. Received ${response.status} response instead of 200`);
throw new Error(chalk.red(errorMessage));
let errorMessage = `Failed to load encryption keys. Please contact support.`;
logger.error(`${errorMessage}. Received ${response.status}, expected 200`);
throw new Error(chalk.red(errorMessage));

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, addressed in latest commit

throw new Error(chalk.red(errorMessage));
}
} catch (error) {
let errorMessage = `Something went wrong in secerts encryption. Please try after some time.`;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
let errorMessage = `Something went wrong in secerts encryption. Please try after some time.`;
let errorMessage = `Something went wrong while encrypting secrets. Please try again.`;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, addressed in latest commit

@revanth0212 revanth0212 merged commit 4367252 into epic/secretsmanage Jun 21, 2024
3 checks passed
@revanth0212 revanth0212 deleted the secrets-encryption-from-cli branch June 21, 2024 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants