From 6b00d1270acaf33f32ee68c4254ce06951ddcb8c Mon Sep 17 00:00:00 2001 From: Mike Jancar Date: Tue, 20 Jul 2021 11:42:55 -0400 Subject: [PATCH] fix(@angular/cli): handle NPM_CONFIG environment variables during ng update and ng add Some organizations are moving away from storing tokens/secrets in an NPM config file in favor of environment variables that only exist for the span of a terminal session. This commit will make sure those variables are read even when there is no NPM config file present. --- .../angular/cli/utilities/package-metadata.ts | 126 +++++++++++------- .../e2e/tests/commands/add/npm-env-vars.ts | 33 +++++ .../e2e/tests/commands/add/registry-option.ts | 11 +- .../e2e/tests/commands/add/secure-registry.ts | 11 +- .../tests/update/update-secure-registry.ts | 11 +- tests/legacy-cli/e2e/utils/registry.ts | 20 ++- 6 files changed, 150 insertions(+), 62 deletions(-) create mode 100644 tests/legacy-cli/e2e/tests/commands/add/npm-env-vars.ts diff --git a/packages/angular/cli/utilities/package-metadata.ts b/packages/angular/cli/utilities/package-metadata.ts index 403428111d79..c96ad8cdc5be 100644 --- a/packages/angular/cli/utilities/package-metadata.ts +++ b/packages/angular/cli/utilities/package-metadata.ts @@ -131,7 +131,7 @@ function readOptions( logger.info(`Locating potential ${baseFilename} files:`); } - const options: PackageManagerOptions = {}; + let options: PackageManagerOptions = {}; for (const location of [...defaultConfigLocations, ...projectConfigLocations]) { if (existsSync(location)) { if (showPotentials) { @@ -142,58 +142,84 @@ function readOptions( // Normalize RC options that are needed by 'npm-registry-fetch'. // See: https://github.com/npm/npm-registry-fetch/blob/ebddbe78a5f67118c1f7af2e02c8a22bcaf9e850/index.js#L99-L126 const rcConfig: PackageManagerOptions = yarn ? lockfile.parse(data) : ini.parse(data); - for (const [key, value] of Object.entries(rcConfig)) { - let substitutedValue = value; - // Substitute any environment variable references. - if (typeof value === 'string') { - substitutedValue = value.replace(/\$\{([^\}]+)\}/, (_, name) => process.env[name] || ''); - } + options = normalizeOptions(rcConfig, location); + } + } + + for (const [key, value] of Object.entries(process.env)) { + if (!value || !key.toLowerCase().startsWith('npm_config_')) { + continue; + } + + const normalizedName = key + .substr(11) + .replace(/(?!^)_/g, '-') // don't replace _ at the start of the key + .toLowerCase(); + options[normalizedName] = value; + } - switch (key) { - // Unless auth options are scope with the registry url it appears that npm-registry-fetch ignores them, - // even though they are documented. - // https://github.com/npm/npm-registry-fetch/blob/8954f61d8d703e5eb7f3d93c9b40488f8b1b62ac/README.md - // https://github.com/npm/npm-registry-fetch/blob/8954f61d8d703e5eb7f3d93c9b40488f8b1b62ac/auth.js#L45-L91 - case '_authToken': - case 'token': - case 'username': - case 'password': - case '_auth': - case 'auth': - options['forceAuth'] ??= {}; - options['forceAuth'][key] = substitutedValue; - break; - case 'noproxy': - case 'no-proxy': - options['noProxy'] = substitutedValue; - break; - case 'maxsockets': - options['maxSockets'] = substitutedValue; - break; - case 'https-proxy': - case 'proxy': - options['proxy'] = substitutedValue; - break; - case 'strict-ssl': - options['strictSSL'] = substitutedValue; - break; - case 'local-address': - options['localAddress'] = substitutedValue; - break; - case 'cafile': - if (typeof substitutedValue === 'string') { - const cafile = path.resolve(path.dirname(location), substitutedValue); - try { - options['ca'] = readFileSync(cafile, 'utf8').replace(/\r?\n/g, '\n'); - } catch {} - } - break; - default: - options[key] = substitutedValue; - break; + options = normalizeOptions(options); + + return options; +} + +function normalizeOptions( + rawOptions: PackageManagerOptions, + location = process.cwd(), +): PackageManagerOptions { + const options: PackageManagerOptions = {}; + + for (const [key, value] of Object.entries(rawOptions)) { + let substitutedValue = value; + + // Substitute any environment variable references. + if (typeof value === 'string') { + substitutedValue = value.replace(/\$\{([^\}]+)\}/, (_, name) => process.env[name] || ''); + } + + switch (key) { + // Unless auth options are scope with the registry url it appears that npm-registry-fetch ignores them, + // even though they are documented. + // https://github.com/npm/npm-registry-fetch/blob/8954f61d8d703e5eb7f3d93c9b40488f8b1b62ac/README.md + // https://github.com/npm/npm-registry-fetch/blob/8954f61d8d703e5eb7f3d93c9b40488f8b1b62ac/auth.js#L45-L91 + case '_authToken': + case 'token': + case 'username': + case 'password': + case '_auth': + case 'auth': + options['forceAuth'] ??= {}; + options['forceAuth'][key] = substitutedValue; + break; + case 'noproxy': + case 'no-proxy': + options['noProxy'] = substitutedValue; + break; + case 'maxsockets': + options['maxSockets'] = substitutedValue; + break; + case 'https-proxy': + case 'proxy': + options['proxy'] = substitutedValue; + break; + case 'strict-ssl': + options['strictSSL'] = substitutedValue; + break; + case 'local-address': + options['localAddress'] = substitutedValue; + break; + case 'cafile': + if (typeof substitutedValue === 'string') { + const cafile = path.resolve(path.dirname(location), substitutedValue); + try { + options['ca'] = readFileSync(cafile, 'utf8').replace(/\r?\n/g, '\n'); + } catch {} } - } + break; + default: + options[key] = substitutedValue; + break; } } diff --git a/tests/legacy-cli/e2e/tests/commands/add/npm-env-vars.ts b/tests/legacy-cli/e2e/tests/commands/add/npm-env-vars.ts new file mode 100644 index 000000000000..f58c1dab247e --- /dev/null +++ b/tests/legacy-cli/e2e/tests/commands/add/npm-env-vars.ts @@ -0,0 +1,33 @@ +import { expectFileNotToExist, expectFileToExist } from '../../../utils/fs'; +import { getActivePackageManager } from '../../../utils/packages'; +import { git, ng } from '../../../utils/process'; +import { + createNpmConfigForAuthentication, + setNpmEnvVarsForAuthentication, +} from '../../../utils/registry'; + +export default async function () { + const packageManager = getActivePackageManager(); + + if (packageManager === 'npm') { + const originalEnvironment = { ...process.env }; + try { + const command = ['add', '@angular/pwa', '--skip-confirmation']; + + // Environment variables only + await expectFileNotToExist('src/manifest.webmanifest'); + setNpmEnvVarsForAuthentication(); + await ng(...command); + await expectFileToExist('src/manifest.webmanifest'); + await git('clean', '-dxf'); + + // Mix of config file and env vars works + await expectFileNotToExist('src/manifest.webmanifest'); + await createNpmConfigForAuthentication(false, true); + await ng(...command); + await expectFileToExist('src/manifest.webmanifest'); + } finally { + process.env = originalEnvironment; + } + } +} diff --git a/tests/legacy-cli/e2e/tests/commands/add/registry-option.ts b/tests/legacy-cli/e2e/tests/commands/add/registry-option.ts index e8e1127a588b..197d3bc6dfe0 100644 --- a/tests/legacy-cli/e2e/tests/commands/add/registry-option.ts +++ b/tests/legacy-cli/e2e/tests/commands/add/registry-option.ts @@ -11,8 +11,11 @@ export default async function () { '.npmrc': 'registry=http://127.0.0.1:9999', }); // The environment variable has priority over the .npmrc - const originalRegistryVariable = process.env['NPM_CONFIG_REGISTRY']; - delete process.env['NPM_CONFIG_REGISTRY']; + let originalRegistryVariable; + if (process.env['NPM_CONFIG_REGISTRY']) { + originalRegistryVariable = process.env['NPM_CONFIG_REGISTRY']; + delete process.env['NPM_CONFIG_REGISTRY']; + } try { await expectToFail(() => ng('add', '@angular/pwa', '--skip-confirmation')); @@ -20,6 +23,8 @@ export default async function () { await ng('add', `--registry=${testRegistry}`, '@angular/pwa', '--skip-confirmation'); await expectFileToExist('src/manifest.webmanifest'); } finally { - process.env['NPM_CONFIG_REGISTRY'] = originalRegistryVariable; + if (originalRegistryVariable) { + process.env['NPM_CONFIG_REGISTRY'] = originalRegistryVariable; + } } } diff --git a/tests/legacy-cli/e2e/tests/commands/add/secure-registry.ts b/tests/legacy-cli/e2e/tests/commands/add/secure-registry.ts index fb09e4771525..da514b8750bf 100644 --- a/tests/legacy-cli/e2e/tests/commands/add/secure-registry.ts +++ b/tests/legacy-cli/e2e/tests/commands/add/secure-registry.ts @@ -5,8 +5,11 @@ import { expectToFail } from '../../../utils/utils'; export default async function () { // The environment variable has priority over the .npmrc - const originalRegistryVariable = process.env['NPM_CONFIG_REGISTRY']; - delete process.env['NPM_CONFIG_REGISTRY']; + let originalRegistryVariable; + if (process.env['NPM_CONFIG_REGISTRY']) { + originalRegistryVariable = process.env['NPM_CONFIG_REGISTRY']; + delete process.env['NPM_CONFIG_REGISTRY']; + } try { const command = ['add', '@angular/pwa', '--skip-confirmation']; @@ -32,6 +35,8 @@ export default async function () { await createNpmConfigForAuthentication(true, true); await expectToFail(() => ng(...command)); } finally { - process.env['NPM_CONFIG_REGISTRY'] = originalRegistryVariable; + if (originalRegistryVariable) { + process.env['NPM_CONFIG_REGISTRY'] = originalRegistryVariable; + } } } diff --git a/tests/legacy-cli/e2e/tests/update/update-secure-registry.ts b/tests/legacy-cli/e2e/tests/update/update-secure-registry.ts index 1d903bd1e794..2c00be697f26 100644 --- a/tests/legacy-cli/e2e/tests/update/update-secure-registry.ts +++ b/tests/legacy-cli/e2e/tests/update/update-secure-registry.ts @@ -4,8 +4,11 @@ import { expectToFail } from '../../utils/utils'; export default async function () { // The environment variable has priority over the .npmrc - const originalRegistryVariable = process.env['NPM_CONFIG_REGISTRY']; - delete process.env['NPM_CONFIG_REGISTRY']; + let originalRegistryVariable; + if (process.env['NPM_CONFIG_REGISTRY']) { + originalRegistryVariable = process.env['NPM_CONFIG_REGISTRY']; + delete process.env['NPM_CONFIG_REGISTRY']; + } const worksMessage = 'We analyzed your package.json'; @@ -30,6 +33,8 @@ export default async function () { await createNpmConfigForAuthentication(true, true); await expectToFail(() => ng('update')); } finally { - process.env['NPM_CONFIG_REGISTRY'] = originalRegistryVariable; + if (originalRegistryVariable) { + process.env['NPM_CONFIG_REGISTRY'] = originalRegistryVariable; + } } } diff --git a/tests/legacy-cli/e2e/utils/registry.ts b/tests/legacy-cli/e2e/utils/registry.ts index 48a56993bb12..a22c75858132 100644 --- a/tests/legacy-cli/e2e/utils/registry.ts +++ b/tests/legacy-cli/e2e/utils/registry.ts @@ -19,6 +19,10 @@ export function createNpmRegistry(withAuthentication = false): ChildProcess { }); } +// Token was generated using `echo -n 'testing:s3cret' | openssl base64`. +const VALID_TOKEN = `dGVzdGluZzpzM2NyZXQ=`; +const SECURE_REGISTRY = `//localhost:4876/`; + export function createNpmConfigForAuthentication( /** * When true, the authentication token will be scoped to the registry URL. @@ -37,9 +41,8 @@ export function createNpmConfigForAuthentication( /** When true, an incorrect token is used. Use this to validate authentication failures. */ invalidToken = false, ): Promise { - // Token was generated using `echo -n 'testing:s3cret' | openssl base64`. - const token = invalidToken ? `invalid=` : `dGVzdGluZzpzM2NyZXQ=`; - const registry = `//localhost:4876/`; + const token = invalidToken ? `invalid=` : VALID_TOKEN; + const registry = SECURE_REGISTRY; return writeFile( '.npmrc', @@ -54,3 +57,14 @@ export function createNpmConfigForAuthentication( `, ); } + +export function setNpmEnvVarsForAuthentication( + /** When true, an incorrect token is used. Use this to validate authentication failures. */ + invalidToken = false, +): void { + const token = invalidToken ? `invalid=` : VALID_TOKEN; + const registry = SECURE_REGISTRY; + + process.env['NPM_CONFIG_REGISTRY'] = `http:${registry}`; + process.env['NPM_CONFIG__AUTH'] = token; +}